Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions internal/cli/snapshot_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 8 additions & 5 deletions internal/cli/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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) {
Expand Down
9 changes: 5 additions & 4 deletions internal/ui/snapshot_editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,18 @@ 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)
}
prefItems = append(prefItems, editorItem{
name: fmt.Sprintf("%s.%s", p.Domain, p.Key),
description: desc,
selected: !p.Unset,
selected: true,
itemType: editorItemMacOSPref,
})
}
Expand Down
12 changes: 7 additions & 5 deletions internal/ui/snapshot_editor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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) {
Expand Down
Loading