fix(sync): skip Unset local prefs so remote drives the write#72
Merged
Conversation
`diffMacOSPrefs` was treating locally-captured Unset prefs as if their
placeholder value (catalog default) were the actual machine state. When
a remote pref had the same value, the diff falsely matched and no
`defaults write` was issued — so the key remained absent on disk and
macOS kept showing the default behaviour.
Concretely: `fullstackjam`'s published config includes
`com.apple.controlcenter Bluetooth=18` (Always Show in Menu Bar). On a
fresh consumer machine, `CaptureMacOSPrefs` records `Bluetooth` with
Unset=true and the catalog's "18" placeholder. The diff matched 18==18
and skipped the write, so Bluetooth never appeared in the menu bar.
Filter Unset prefs out of the local lookup map so they count as
"missing" and surface in MacOSChanged. This matches the semantics
already used in `internal/diff/compare.go:diffMacOS` ("Unset prefs on
either side mean 'no opinion'").
Refactor the comparison core out into `computeMacOSPrefDiff` so the
fix is unit-testable without going through real `defaults read`.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
diffMacOSPrefswas including locally-capturedUnsetprefs in the lookup map. TheirValueis the catalog default placeholder, not whatdefaults readactually returned — so when a remote pref had the same value (the common case after feat(macos): capture "unset" state instead of silently dropping prefs #67/fix(macos): publish Unset prefs too — they carry the catalog default #68), the diff falsely matched and nodefaults writewas issued. Result: the key stayed absent on disk and macOS kept showing the default behaviour (e.g. menu bar Sound/Bluetooth/WiFi/Battery stuck on "Show When Active" after installing a config that explicitly sets them to "Always Show").Unsetprefs from the local lookup map so they count as "missing" and surface inMacOSChanged. Matches the existing semantics ininternal/diff/compare.go:diffMacOS("Unset prefs on either side mean 'no opinion'").computeMacOSPrefDiffso the regression is unit-testable without going through realdefaults read.Repro that motivated this
Installing
fullstackjam/jam-s-packageson a machine whereBluetooth/WiFi/Batterywere never explicitly set:com.apple.controlcenter Bluetooth=18(Always Show in Menu Bar), same for WiFi/BatteryBluetoothrecorded asUnsetwith placeholder value18(catalog default)18 == 18→ no diff → no write →defaults read com.apple.controlcenter Bluetoothstill says "does not exist" after installBluetoothsurfaces as needing a write; install runs thedefaults writeTest plan
internal/sync/diff_extra_test.goexercising the three meaningful cases (Unset local → missing, explicit match → no diff, explicit differ → diff)go test ./internal/...(L1 incl. archtest) — greengo vet ./...— clean