Harden Chrome shutdown/write safety, fix flag sync bugs, add test coverage - #26
Merged
Conversation
…st coverage Local State is now written atomically (temp file + fsync + rename) with a one-time pristine backup before the first modification, and Chrome is stopped with a graceful terminate-then-wait-then-kill sequence instead of a bare kill, so patching can never race a Chrome process that's still alive and about to overwrite the file on its own exit. Fixes several real bugs in syncManagedFlags (duplicate/unmanaged flag names, a no-op path that could inject an empty browser map), macOS helper-process misidentification during shutdown/restart, and CLI exit codes that previously reported success even when every installation failed to patch. AvailableAIDownloadFlags is now empty: live verification against Chrome 151 confirmed both previously-listed chrome://flags entries are stale (one removed outright, the other renamed with different, unverified option-index semantics that would silently enable the wrong mode rather than disable anything) — the GenAILocalFoundationalModelSettings Enterprise policy remains fully verified reliable and is what's actually doing the blocking by default. Also: GUI gained dry-run/no-restart checkboxes (previously its only path had no preview option at all), CI now runs on macOS/Linux/Windows with vet/lint/race/coverage, and the three per-platform policy apply/remove implementations were collapsed into one shared skeleton. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Windows CI checked out .go files with CRLF (no .gitattributes to force LF), which golangci-lint's gofmt check then flagged as "not properly formatted" on every file — not a real formatting issue, just a Windows-checkout artifact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both are test-only issues, not production bugs:
- TestExpandUserPath used a hardcoded POSIX absolute path ("/etc/opt/chrome")
to test "no ~/ prefix" passthrough; filepath.Abs on Windows treats that as
relative to the current drive rather than absolute. Now builds a
platform-native absolute path via filepath.Abs(t.TempDir()) instead.
- TestMacAppBundlePath exercises macAppBundlePath, which is only ever
called from launch() when runtime.GOOS == "darwin" and parses
forward-slash macOS paths — filepath.Dir/Base on Windows use backslash
semantics instead, so the test isn't meaningful (or reachable in
production) on non-darwin. Skipped there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Local State.go-chrome-ai.bak) before the first modification.Runaborts before touching any files if a process can't be confirmed stopped, instead of racing a Chrome process that could overwrite the patch on its own exit.syncManagedFlags(duplicate/unmanaged flag names written verbatim, a no-op path that could inject an emptybrowsermap).Google Chrome Helper*was being matched and "restarted" as if it were the browser).AvailableAIDownloadFlagsis now empty: live-verified against Chrome 151 that both previously-listedchrome://flagsentries are stale (one removed outright, the other renamed with unverified option-index semantics that would have silently enabled the wrong mode). TheGenAILocalFoundationalModelSettingsEnterprise policy is independently verified reliable and remains the default mechanism.policy.go); addedpolicy_other.goso the package compiles on any OS.go vet,golangci-lint,-race, and coverage.Test plan
go build ./...,go vet ./...,golangci-lint run ./...(0 issues),go test ./... -raceall passmake build+make release-check(goreleaser) pass with a version-stamped binarychrome://policyinternal/chrome15%→76%,internal/app0%→89%🤖 Generated with Claude Code