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
4 changes: 2 additions & 2 deletions internal/archtest/baseline/no-direct-exec.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions internal/cli/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
16 changes: 12 additions & 4 deletions internal/cli/snapshot_import.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions internal/cli/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
15 changes: 15 additions & 0 deletions internal/diff/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,20 +119,32 @@ 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
}

md := &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 {
Expand All @@ -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{
Expand Down
27 changes: 27 additions & 0 deletions internal/diff/compare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
12 changes: 12 additions & 0 deletions internal/snapshot/capture.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
5 changes: 5 additions & 0 deletions internal/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
50 changes: 50 additions & 0 deletions internal/snapshot/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
11 changes: 9 additions & 2 deletions internal/ui/snapshot_editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}
Expand Down
18 changes: 18 additions & 0 deletions internal/ui/snapshot_editor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading