diff --git a/internal/cli/snapshot_import.go b/internal/cli/snapshot_import.go index 4c7e3e1..2c658ce 100644 --- a/internal/cli/snapshot_import.go +++ b/internal/cli/snapshot_import.go @@ -204,23 +204,24 @@ func buildImportConfig(edited *snapshot.Snapshot, dryRun bool) *config.Config { // Invalid URLs are silently skipped — validation at push time will catch them. } - // 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{ + // Pass every pref through, including those marked Unset on capture. + // Rationale: catalog defaults exist precisely to encode "what a sane + // macOS looks like" — when a user's machine sits at the system default + // (which macOS leaves out of the plist), `defaults read` errors and + // capture marks the pref Unset with the catalog value as a stand-in. + // Filtering those out at publish made the web UI lose entries like + // "Show Sound in menu bar" for any user who hadn't explicitly written + // the key. Treat Unset purely as informational metadata (still used + // by diff to avoid false-positive Missing/Changed). + cfg.SnapshotMacOS = make([]config.RemoteMacOSPref, len(edited.MacOSPrefs)) + for i, p := range edited.MacOSPrefs { + cfg.SnapshotMacOS[i] = 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 1b341dc..a7a79ae 100644 --- a/internal/cli/snapshot_test.go +++ b/internal/cli/snapshot_test.go @@ -111,7 +111,7 @@ func TestBuildImportConfig_MacOSPrefs(t *testing.T) { assert.Equal(t, "Autohide dock", cfg.SnapshotMacOS[0].Desc) } -func TestBuildImportConfig_MacOSPrefs_DropsUnset(t *testing.T) { +func TestBuildImportConfig_MacOSPrefs_PreservesUnset(t *testing.T) { snap := &snapshot.Snapshot{ MacOSPrefs: []snapshot.MacOSPref{ {Domain: "com.apple.dock", Key: "autohide", Type: "bool", Value: "1", Desc: "set"}, @@ -122,11 +122,14 @@ func TestBuildImportConfig_MacOSPrefs_DropsUnset(t *testing.T) { 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) + // All three prefs propagate. Unset entries carry the catalog default + // (the value we want to enforce anyway), so dropping them here would + // silently lose categories whose keys aren't in the user's plist — + // the exact bug that hid 8/9 Menu Bar items from openboot.dev. + require.Len(t, cfg.SnapshotMacOS, 3) assert.Equal(t, "autohide", cfg.SnapshotMacOS[0].Key) - assert.Equal(t, "ShowPathbar", cfg.SnapshotMacOS[1].Key) + assert.Equal(t, "tilesize", cfg.SnapshotMacOS[1].Key) + assert.Equal(t, "ShowPathbar", cfg.SnapshotMacOS[2].Key) } func TestBuildImportConfig_InvalidDotfilesURL_Skipped(t *testing.T) { diff --git a/internal/ui/snapshot_editor.go b/internal/ui/snapshot_editor.go index 51eefd5..6646412 100644 --- a/internal/ui/snapshot_editor.go +++ b/internal/ui/snapshot_editor.go @@ -119,9 +119,10 @@ 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. + // Unset prefs default selected too — Value carries the catalog + // default, which is the value we'd want to enforce anyway. The + // "(unset)" tag in the description is informational, telling the + // user the source machine sits at macOS default for this key. desc := fmt.Sprintf("= %s (%s)", p.Value, p.Desc) if p.Unset { desc = fmt.Sprintf("(unset, default = %s) %s", p.Value, p.Desc) @@ -129,7 +130,7 @@ func NewSnapshotEditor(snap *snapshot.Snapshot) SnapshotEditorModel { prefItems = append(prefItems, editorItem{ name: fmt.Sprintf("%s.%s", p.Domain, p.Key), description: desc, - selected: !p.Unset, + selected: true, itemType: editorItemMacOSPref, }) } diff --git a/internal/ui/snapshot_editor_test.go b/internal/ui/snapshot_editor_test.go index 5077922..8dabe1f 100644 --- a/internal/ui/snapshot_editor_test.go +++ b/internal/ui/snapshot_editor_test.go @@ -100,7 +100,7 @@ func TestNewSnapshotEditorSkipsInvalidMacOSPrefs(t *testing.T) { assert.Equal(t, 3, len(prefTab.items)) } -func TestNewSnapshotEditor_UnsetPrefsDefaultUnselected(t *testing.T) { +func TestNewSnapshotEditor_UnsetPrefsDefaultSelectedWithBadge(t *testing.T) { snap := makeTestSnapshot() snap.MacOSPrefs = []snapshot.MacOSPref{ {Domain: "com.apple.dock", Key: "autohide", Value: "true", Desc: "set pref"}, @@ -111,11 +111,13 @@ func TestNewSnapshotEditor_UnsetPrefsDefaultUnselected(t *testing.T) { 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") + // Both default selected. The catalog value is what we want to enforce + // regardless of whether the source machine had the key in its plist — + // dropping unset prefs here is what hid Menu Bar items from publish. + assert.True(t, prefTab.items[0].selected, "set pref should default selected") + assert.True(t, prefTab.items[1].selected, "unset pref should default selected too") assert.Contains(t, prefTab.items[1].description, "unset", - "unset pref description should advertise the (unset) marker") + "unset pref keeps the informational (unset) badge in description") } func TestNewSnapshotEditorAllItemsSelected(t *testing.T) {