diff --git a/internal/archtest/baseline/no-direct-exec.txt b/internal/archtest/baseline/no-direct-exec.txt index b718c94..8f27222 100644 --- a/internal/archtest/baseline/no-direct-exec.txt +++ b/internal/archtest/baseline/no-direct-exec.txt @@ -4,8 +4,8 @@ internal/auth/login.go:191 internal/brew/brew_install.go:338 internal/cli/snapshot.go:21 -internal/diff/compare.go:230 -internal/diff/compare.go:236 +internal/diff/compare.go:245 +internal/diff/compare.go:251 internal/dotfiles/dotfiles.go:23 internal/dotfiles/dotfiles.go:32 internal/dotfiles/dotfiles.go:66 diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 2a3dcd1..37a247a 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -418,9 +418,21 @@ func showSnapshotPreview(snap *snapshot.Snapshot) { fmt.Fprintf(os.Stderr, " %s %d\n", snapBoldStyle.Render("NPM Packages:"), len(snap.Packages.Npm)) printSnapshotList(snap.Packages.Npm, 10) - fmt.Fprintf(os.Stderr, " %s %d\n", snapBoldStyle.Render("macOS Preferences:"), len(snap.MacOSPrefs)) + setCount := 0 for _, pref := range snap.MacOSPrefs { - fmt.Fprintf(os.Stderr, " %s.%s = %s\n", pref.Domain, pref.Key, pref.Value) + if !pref.Unset { + setCount++ + } + } + fmt.Fprintf(os.Stderr, " %s %d (%d set, %d unset)\n", + snapBoldStyle.Render("macOS Preferences:"), len(snap.MacOSPrefs), + setCount, len(snap.MacOSPrefs)-setCount) + for _, pref := range snap.MacOSPrefs { + marker := "" + if pref.Unset { + marker = " (unset)" + } + fmt.Fprintf(os.Stderr, " %s.%s = %s%s\n", pref.Domain, pref.Key, pref.Value, marker) } fmt.Fprintf(os.Stderr, " %s %s <%s>\n", diff --git a/internal/cli/snapshot_import.go b/internal/cli/snapshot_import.go index fd5fac1..4c7e3e1 100644 --- a/internal/cli/snapshot_import.go +++ b/internal/cli/snapshot_import.go @@ -204,15 +204,23 @@ func buildImportConfig(edited *snapshot.Snapshot, dryRun bool) *config.Config { // Invalid URLs are silently skipped — validation at push time will catch them. } - cfg.SnapshotMacOS = make([]config.RemoteMacOSPref, len(edited.MacOSPrefs)) - for i, p := range edited.MacOSPrefs { - cfg.SnapshotMacOS[i] = config.RemoteMacOSPref{ + // Drop Unset prefs at the boundary — they're informational only and + // must not propagate into state (which feeds both restore via Plan → + // Configure and publish via the remote API). RemoteMacOSPref has no + // Unset field by design: the remote config models "what should be + // enforced", with no place for "user had no opinion here". + cfg.SnapshotMacOS = make([]config.RemoteMacOSPref, 0, len(edited.MacOSPrefs)) + for _, p := range edited.MacOSPrefs { + if p.Unset { + continue + } + cfg.SnapshotMacOS = append(cfg.SnapshotMacOS, config.RemoteMacOSPref{ Domain: p.Domain, Key: p.Key, Type: p.Type, Value: p.Value, Desc: p.Desc, - } + }) } cfg.SnapshotShellOhMyZsh = edited.Shell.OhMyZsh diff --git a/internal/cli/snapshot_test.go b/internal/cli/snapshot_test.go index 1fa7403..1b341dc 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -111,6 +111,24 @@ func TestBuildImportConfig_MacOSPrefs(t *testing.T) { assert.Equal(t, "Autohide dock", cfg.SnapshotMacOS[0].Desc) } +func TestBuildImportConfig_MacOSPrefs_DropsUnset(t *testing.T) { + snap := &snapshot.Snapshot{ + MacOSPrefs: []snapshot.MacOSPref{ + {Domain: "com.apple.dock", Key: "autohide", Type: "bool", Value: "1", Desc: "set"}, + {Domain: "com.apple.dock", Key: "tilesize", Type: "int", Value: "48", Desc: "unset", Unset: true}, + {Domain: "com.apple.finder", Key: "ShowPathbar", Type: "bool", Value: "true", Desc: "set"}, + }, + } + + cfg := buildImportConfig(snap, false) + + // Only the two non-Unset prefs propagate into state — the Unset entry + // must not become enforced config. + require.Len(t, cfg.SnapshotMacOS, 2) + assert.Equal(t, "autohide", cfg.SnapshotMacOS[0].Key) + assert.Equal(t, "ShowPathbar", cfg.SnapshotMacOS[1].Key) +} + func TestBuildImportConfig_InvalidDotfilesURL_Skipped(t *testing.T) { snap := &snapshot.Snapshot{ Dotfiles: snapshot.DotfilesSnapshot{ diff --git a/internal/diff/compare.go b/internal/diff/compare.go index 64862df..d2bc491 100644 --- a/internal/diff/compare.go +++ b/internal/diff/compare.go @@ -119,13 +119,22 @@ func diffMacOS(system, reference []snapshot.MacOSPref) *MacOSDiff { Key string } + // Unset prefs on either side mean "no opinion" — they contribute + // nothing to a diff. Skip them when building the lookup maps and + // when iterating for missing/changed/extra. sysMap := make(map[prefKey]string, len(system)) for _, p := range system { + if p.Unset { + continue + } sysMap[prefKey{p.Domain, p.Key}] = p.Value } refMap := make(map[prefKey]string, len(reference)) for _, p := range reference { + if p.Unset { + continue + } refMap[prefKey{p.Domain, p.Key}] = p.Value } @@ -133,6 +142,9 @@ func diffMacOS(system, reference []snapshot.MacOSPref) *MacOSDiff { // Find missing and changed for _, p := range reference { + if p.Unset { + continue + } pk := prefKey{p.Domain, p.Key} sysVal, exists := sysMap[pk] if !exists { @@ -148,6 +160,9 @@ func diffMacOS(system, reference []snapshot.MacOSPref) *MacOSDiff { // Find extra (in system but not in reference) for _, p := range system { + if p.Unset { + continue + } pk := prefKey{p.Domain, p.Key} if _, exists := refMap[pk]; !exists { md.Extra = append(md.Extra, MacOSPrefEntry{ diff --git a/internal/diff/compare_test.go b/internal/diff/compare_test.go index 0f7209d..fb79eca 100644 --- a/internal/diff/compare_test.go +++ b/internal/diff/compare_test.go @@ -100,6 +100,33 @@ func TestCompareSnapshots_MacOSDifferences(t *testing.T) { assert.Empty(t, result.MacOS.Extra) } +func TestCompareSnapshots_MacOS_IgnoresUnset(t *testing.T) { + isolateHome(t) + // Both sides carry an Unset entry that would generate a false-positive + // Changed (different "default" values) and a false-positive Extra/Missing + // (key only on one side). diffMacOS must filter Unset on both sides. + system := &snapshot.Snapshot{ + MacOSPrefs: []snapshot.MacOSPref{ + {Domain: "com.apple.dock", Key: "autohide", Value: "false"}, + {Domain: "com.apple.dock", Key: "tilesize", Value: "48", Unset: true}, + {Domain: "com.apple.finder", Key: "ShowPathbar", Value: "true", Unset: true}, + }, + } + reference := &snapshot.Snapshot{ + MacOSPrefs: []snapshot.MacOSPref{ + {Domain: "com.apple.dock", Key: "autohide", Value: "false"}, + {Domain: "com.apple.dock", Key: "tilesize", Value: "32", Unset: true}, // different "default" + }, + } + + result := CompareSnapshots(system, reference, Source{}) + + assert.NotNil(t, result.MacOS) + assert.Empty(t, result.MacOS.Changed, "Unset entries must not produce Changed") + assert.Empty(t, result.MacOS.Missing, "Unset entries must not produce Missing") + assert.Empty(t, result.MacOS.Extra, "Unset entries must not produce Extra") +} + func TestCompareSnapshots_DevToolDifferences(t *testing.T) { isolateHome(t) system := &snapshot.Snapshot{ diff --git a/internal/snapshot/capture.go b/internal/snapshot/capture.go index 51c5987..8590c7d 100644 --- a/internal/snapshot/capture.go +++ b/internal/snapshot/capture.go @@ -239,6 +239,18 @@ func CaptureMacOSPrefs() ([]MacOSPref, error) { for _, p := range macos.DefaultPreferences { output, err := system.RunCommandOutput("defaults", "read", p.Domain, p.Key) if err != nil { + // Key isn't set on this machine — record it with the catalog's + // default value and the Unset marker so consumers (UI, restore, + // diff, publish) can distinguish "user has the macOS default" + // from "user explicitly chose the catalog value". + prefs = append(prefs, MacOSPref{ + Domain: p.Domain, + Key: p.Key, + Type: p.Type, + Value: p.Value, + Desc: p.Desc, + Unset: true, + }) continue } diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index fa85a10..0237ede 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -142,6 +142,11 @@ type MacOSPref struct { Type string `json:"type"` Value string `json:"value"` Desc string `json:"desc"` + // Unset is true when `defaults read` had no value for this key on the + // source machine — Value then holds the catalog's default for display + // only. Unset prefs are informational: restore skips them, diff ignores + // them on either side, and publish filters them out. + Unset bool `json:"unset,omitempty"` } type GitSnapshot struct { diff --git a/internal/snapshot/snapshot_test.go b/internal/snapshot/snapshot_test.go index 4fb764d..715c45a 100644 --- a/internal/snapshot/snapshot_test.go +++ b/internal/snapshot/snapshot_test.go @@ -165,6 +165,56 @@ func TestPackageSnapshot_MarshalJSON_RoundTrip(t *testing.T) { } } +func TestMacOSPref_JSON_UnsetRoundTrip(t *testing.T) { + tests := []struct { + name string + pref MacOSPref + wantHasKey bool + wantValue bool + }{ + { + name: "set pref omits unset key", + pref: MacOSPref{Domain: "d", Key: "k", Type: "bool", Value: "true", Desc: "x"}, + wantHasKey: false, + }, + { + name: "unset pref serializes unset:true", + pref: MacOSPref{Domain: "d", Key: "k", Type: "bool", Value: "false", Desc: "x", Unset: true}, + wantHasKey: true, + wantValue: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + data, err := json.Marshal(tt.pref) + require.NoError(t, err) + + var raw map[string]any + require.NoError(t, json.Unmarshal(data, &raw)) + + val, ok := raw["unset"] + assert.Equal(t, tt.wantHasKey, ok, "unset key presence") + if ok { + assert.Equal(t, tt.wantValue, val) + } + + var back MacOSPref + require.NoError(t, json.Unmarshal(data, &back)) + assert.Equal(t, tt.pref, back, "round-trip preserves Unset") + }) + } +} + +func TestMacOSPref_JSON_LegacyDecodeNoUnsetField(t *testing.T) { + // A snapshot written by an older version of openboot will have no + // "unset" key at all. It must decode cleanly with Unset == false so + // existing on-disk snapshots keep working. + legacy := `{"domain":"d","key":"k","type":"bool","value":"true","desc":"x"}` + var pref MacOSPref + require.NoError(t, json.Unmarshal([]byte(legacy), &pref)) + assert.False(t, pref.Unset) +} + func TestCaptureHealth(t *testing.T) { t.Run("default is healthy", func(t *testing.T) { snap := &Snapshot{Version: 1} diff --git a/internal/ui/snapshot_editor.go b/internal/ui/snapshot_editor.go index 2a1313f..51eefd5 100644 --- a/internal/ui/snapshot_editor.go +++ b/internal/ui/snapshot_editor.go @@ -119,10 +119,17 @@ func NewSnapshotEditor(snap *snapshot.Snapshot) SnapshotEditorModel { if p.Domain == "" || p.Key == "" { continue } + // Unset prefs default to unselected — the user had no opinion on + // the source machine, so don't ship a default value into their + // state/remote-config unless they explicitly tick it. + desc := fmt.Sprintf("= %s (%s)", p.Value, p.Desc) + if p.Unset { + desc = fmt.Sprintf("(unset, default = %s) %s", p.Value, p.Desc) + } prefItems = append(prefItems, editorItem{ name: fmt.Sprintf("%s.%s", p.Domain, p.Key), - description: fmt.Sprintf("= %s (%s)", p.Value, p.Desc), - selected: true, + description: desc, + selected: !p.Unset, itemType: editorItemMacOSPref, }) } diff --git a/internal/ui/snapshot_editor_test.go b/internal/ui/snapshot_editor_test.go index 27bc808..5077922 100644 --- a/internal/ui/snapshot_editor_test.go +++ b/internal/ui/snapshot_editor_test.go @@ -100,6 +100,24 @@ func TestNewSnapshotEditorSkipsInvalidMacOSPrefs(t *testing.T) { assert.Equal(t, 3, len(prefTab.items)) } +func TestNewSnapshotEditor_UnsetPrefsDefaultUnselected(t *testing.T) { + snap := makeTestSnapshot() + snap.MacOSPrefs = []snapshot.MacOSPref{ + {Domain: "com.apple.dock", Key: "autohide", Value: "true", Desc: "set pref"}, + {Domain: "com.apple.dock", Key: "tilesize", Value: "48", Desc: "unset pref", Unset: true}, + } + m := NewSnapshotEditor(snap) + + prefTab := m.tabs[4] + require.Len(t, prefTab.items, 2) + + // The order matches insertion order: set pref first, unset second. + assert.True(t, prefTab.items[0].selected, "set pref should default to selected") + assert.False(t, prefTab.items[1].selected, "unset pref should default to unselected") + assert.Contains(t, prefTab.items[1].description, "unset", + "unset pref description should advertise the (unset) marker") +} + func TestNewSnapshotEditorAllItemsSelected(t *testing.T) { snap := makeTestSnapshot() m := NewSnapshotEditor(snap)