From 37f59b30220149c2967524655390fcbc0d173d67 Mon Sep 17 00:00:00 2001 From: Aly Raffauf Date: Sun, 9 Aug 2026 19:12:52 -0400 Subject: [PATCH] fix deployment plan and ordering contracts --- internal/deployment/alias.go | 63 --------- internal/deployment/hook.go | 61 +++++++++ internal/deployment/plan.go | 113 ++++++++++++---- internal/deployment/plan_test.go | 67 ++++++---- internal/deployment/sort.go | 18 ++- internal/deployment/sort_test.go | 23 ++++ internal/reconcile/decisions_test.go | 14 +- internal/reconcile/snapshot.go | 25 ++-- internal/reconcile/snapshot_helpers_test.go | 123 ++++++++++++++++++ internal/reconcile/snapshot_test.go | 135 ++++---------------- internal/repository/compiler.go | 6 +- internal/repository/compiler_test.go | 16 ++- 12 files changed, 419 insertions(+), 245 deletions(-) create mode 100644 internal/deployment/hook.go create mode 100644 internal/reconcile/snapshot_helpers_test.go diff --git a/internal/deployment/alias.go b/internal/deployment/alias.go index abc732b..96e1b27 100644 --- a/internal/deployment/alias.go +++ b/internal/deployment/alias.go @@ -2,34 +2,6 @@ package deployment import "fmt" -// HookPhase names the position a hook runs in relative to file deployment. The -// apply orchestrator, not this type, owns final ordering between phases. -type HookPhase string - -const ( - HookBefore HookPhase = "before" - HookAfter HookPhase = "after" -) - -// ParseHookPhase converts a raw string into a HookPhase, rejecting unknown -// values. -func ParseHookPhase(value string) (HookPhase, error) { - phase := HookPhase(value) - if !phase.Valid() { - return "", fmt.Errorf("deployment: unknown hook phase %q", value) - } - return phase, nil -} - -// Valid reports whether phase is one of the supported constants. -func (p HookPhase) Valid() bool { - switch p { - case HookBefore, HookAfter: - return true - } - return false -} - // Alias describes one explicitly declared symlink pointing at a canonical // target. Aliases are the only symlinks Cattery deploys; they are never the // primary deployment strategy. @@ -60,38 +32,3 @@ func validateAlias(alias Alias) error { } return nil } - -// Hook describes one validated hook descriptor. Final execution order is owned -// by the apply orchestrator; this descriptor carries identity and absolute -// location only. -type Hook struct { - Scope Scope - Phase HookPhase - Name string - AbsolutePath string - RepositoryPath string -} - -// NewHook validates candidate field-by-field and returns it on success. -func NewHook(candidate Hook) (Hook, error) { - if err := validateHook(candidate); err != nil { - return Hook{}, err - } - return candidate, nil -} - -func validateHook(hook Hook) error { - if !hook.Phase.Valid() { - return fmt.Errorf("deployment: hook has invalid phase") - } - if hook.Name == "" { - return fmt.Errorf("deployment: hook has empty name") - } - if hook.AbsolutePath == "" { - return fmt.Errorf("deployment: hook %q missing absolute path", hook.Name) - } - if hook.RepositoryPath == "" { - return fmt.Errorf("deployment: hook %q missing repository path", hook.Name) - } - return nil -} diff --git a/internal/deployment/hook.go b/internal/deployment/hook.go new file mode 100644 index 0000000..8e6302e --- /dev/null +++ b/internal/deployment/hook.go @@ -0,0 +1,61 @@ +package deployment + +import "fmt" + +// HookPhase names the position a hook runs in relative to file deployment. +type HookPhase string + +const ( + HookBefore HookPhase = "before" + HookAfter HookPhase = "after" +) + +// ParseHookPhase converts a raw string into a HookPhase, rejecting unknown values. +func ParseHookPhase(value string) (HookPhase, error) { + phase := HookPhase(value) + if !phase.Valid() { + return "", fmt.Errorf("deployment: unknown hook phase %q", value) + } + return phase, nil +} + +func (p HookPhase) Valid() bool { + switch p { + case HookBefore, HookAfter: + return true + } + return false +} + +// Hook describes one validated hook descriptor. +type Hook struct { + Scope Scope + Phase HookPhase + Name string + AbsolutePath string + RepositoryPath string +} + +// NewHook validates candidate and returns it on success. +func NewHook(candidate Hook) (Hook, error) { + if err := validateHook(candidate); err != nil { + return Hook{}, err + } + return candidate, nil +} + +func validateHook(hook Hook) error { + if !hook.Phase.Valid() { + return fmt.Errorf("deployment: hook has invalid phase") + } + if hook.Name == "" { + return fmt.Errorf("deployment: hook has empty name") + } + if hook.AbsolutePath == "" { + return fmt.Errorf("deployment: hook %q missing absolute path", hook.Name) + } + if hook.RepositoryPath == "" { + return fmt.Errorf("deployment: hook %q missing repository path", hook.Name) + } + return nil +} diff --git a/internal/deployment/plan.go b/internal/deployment/plan.go index f421aaf..a1049cf 100644 --- a/internal/deployment/plan.go +++ b/internal/deployment/plan.go @@ -1,10 +1,20 @@ package deployment -// Plan is the immutable, validated deployment plan compiled from a repository -// for one platform. Slice fields are defensively copied on construction and -// on every read, so callers can never mutate a Plan through slices they held -// before construction or hold after an accessor returns. +import "fmt" + +// Plan is the immutable deployment plan compiled from a repository for one +// platform. type Plan struct { + repositoryRoot string + platform string + groups []string + files []ManagedFile + aliases []Alias + hooks []Hook +} + +// PlanInput contains the validated records used to construct a Plan. +type PlanInput struct { RepositoryRoot string Platform string Groups []string @@ -13,37 +23,90 @@ type Plan struct { Hooks []Hook } -// NewPlan constructs a Plan from candidate, defensively copying every input -// slice so the caller's source slices cannot mutate the plan later. -func NewPlan(candidate Plan) Plan { +// NewPlan validates and freezes input. The returned Plan does not share any +// slice storage with input or with a later accessor result. +func NewPlan(input PlanInput) (Plan, error) { + if err := validatePlanInput(input); err != nil { + return Plan{}, err + } return Plan{ - RepositoryRoot: candidate.RepositoryRoot, - Platform: candidate.Platform, - Groups: copyStrings(candidate.Groups), - Files: copyFiles(candidate.Files), - Aliases: copyAliases(candidate.Aliases), - Hooks: copyHooks(candidate.Hooks), + repositoryRoot: input.RepositoryRoot, + platform: input.Platform, + groups: copyStrings(input.Groups), + files: copyFiles(input.Files), + aliases: copyAliases(input.Aliases), + hooks: copyHooks(input.Hooks), + }, nil +} + +func validatePlanInput(input PlanInput) error { + if input.RepositoryRoot == "" { + return fmt.Errorf("deployment: plan has empty repository root") } + if input.Platform == "" { + return fmt.Errorf("deployment: plan has empty platform") + } + if err := validateFiles(input.Files); err != nil { + return err + } + if err := validateAliases(input.Aliases); err != nil { + return err + } + if err := validateHooks(input.Hooks); err != nil { + return err + } + return nil +} + +func validateFiles(files []ManagedFile) error { + for _, file := range files { + if err := validateFile(file); err != nil { + return err + } + } + return nil +} + +func validateAliases(aliases []Alias) error { + for _, alias := range aliases { + if err := validateAlias(alias); err != nil { + return err + } + } + return nil +} + +func validateHooks(hooks []Hook) error { + for _, hook := range hooks { + if err := validateHook(hook); err != nil { + return err + } + } + return nil +} + +func (p Plan) RepositoryRoot() string { + return p.repositoryRoot +} + +func (p Plan) Platform() string { + return p.platform } -// AllGroups returns a defensive copy of the plan's group list. -func (p Plan) AllGroups() []string { - return copyStrings(p.Groups) +func (p Plan) Groups() []string { + return copyStrings(p.groups) } -// AllFiles returns a defensive copy of the plan's file list. -func (p Plan) AllFiles() []ManagedFile { - return copyFiles(p.Files) +func (p Plan) Files() []ManagedFile { + return copyFiles(p.files) } -// AllAliases returns a defensive copy of the plan's alias list. -func (p Plan) AllAliases() []Alias { - return copyAliases(p.Aliases) +func (p Plan) Aliases() []Alias { + return copyAliases(p.aliases) } -// AllHooks returns a defensive copy of the plan's hook list. -func (p Plan) AllHooks() []Hook { - return copyHooks(p.Hooks) +func (p Plan) Hooks() []Hook { + return copyHooks(p.hooks) } func copyStrings(items []string) []string { diff --git a/internal/deployment/plan_test.go b/internal/deployment/plan_test.go index 6f50ca4..ec5a39d 100644 --- a/internal/deployment/plan_test.go +++ b/internal/deployment/plan_test.go @@ -11,6 +11,7 @@ func TestPlanContract(t *testing.T) { run func(*testing.T) }{ {"constructor copies caller slices", testPlanCopiesInputs}, + {"constructor validates input", testPlanValidatesInput}, {"accessor copy cannot mutate plan", testAccessorCopyIsolation}, {"zero plan yields nil accessors", testZeroPlanAccessors}, } @@ -19,8 +20,8 @@ func TestPlanContract(t *testing.T) { } } -func samplePlanCandidate() Plan { - return Plan{ +func samplePlanCandidate() PlanInput { + return PlanInput{ RepositoryRoot: "/repo", Platform: "linux", Groups: []string{"atuin", "zsh"}, @@ -52,62 +53,78 @@ func testPlanCopiesInputs(t *testing.T) { files := candidate.Files aliases := candidate.Aliases hooks := candidate.Hooks - plan := NewPlan(candidate) + plan := mustPlan(candidate) groups = append(groups, " mutated") files = append(files, ManagedFile{TargetRelativePath: "x"}) aliases = append(aliases, Alias{AliasRelativePath: "x"}) hooks = append(hooks, Hook{Name: "x"}) - if len(plan.AllGroups()) != 2 { - t.Fatalf("groups leaked: %v", plan.AllGroups()) + if len(plan.Groups()) != 2 { + t.Fatalf("groups leaked: %v", plan.Groups()) } - if len(plan.AllFiles()) != 1 { - t.Fatalf("files leaked: %v", plan.AllFiles()) + if len(plan.Files()) != 1 { + t.Fatalf("files leaked: %v", plan.Files()) } - if len(plan.AllAliases()) != 1 { - t.Fatalf("aliases leaked: %v", plan.AllAliases()) + if len(plan.Aliases()) != 1 { + t.Fatalf("aliases leaked: %v", plan.Aliases()) } - if len(plan.AllHooks()) != 1 { - t.Fatalf("hooks leaked: %v", plan.AllHooks()) + if len(plan.Hooks()) != 1 { + t.Fatalf("hooks leaked: %v", plan.Hooks()) + } +} + +func testPlanValidatesInput(t *testing.T) { + candidate := samplePlanCandidate() + candidate.Files[0].Kind = "invalid" + if _, err := NewPlan(candidate); err == nil { + t.Fatal("invalid managed file must be rejected") } } func testAccessorCopyIsolation(t *testing.T) { - plan := NewPlan(samplePlanCandidate()) - groups := plan.AllGroups() - files := plan.AllFiles() - aliases := plan.AllAliases() - hooks := plan.AllHooks() + plan := mustPlan(samplePlanCandidate()) + groups := plan.Groups() + files := plan.Files() + aliases := plan.Aliases() + hooks := plan.Hooks() groups[0] = "corrupted" files[0] = ManagedFile{TargetRelativePath: "corrupted"} aliases[0] = Alias{AliasRelativePath: "corrupted"} hooks[0] = Hook{Name: "corrupted"} - again := NewPlan(samplePlanCandidate()) - if plan.AllGroups()[0] == "corrupted" || again.AllGroups()[0] == "corrupted" { + again := mustPlan(samplePlanCandidate()) + if plan.Groups()[0] == "corrupted" || again.Groups()[0] == "corrupted" { t.Fatal("accessor copy leaked into plan") } - if plan.AllFiles()[0].TargetRelativePath == "corrupted" { + if plan.Files()[0].TargetRelativePath == "corrupted" { t.Fatal("file accessor copy leaked into plan") } - if plan.AllAliases()[0].AliasRelativePath == "corrupted" { + if plan.Aliases()[0].AliasRelativePath == "corrupted" { t.Fatal("alias accessor copy leaked into plan") } - if plan.AllHooks()[0].Name == "corrupted" { + if plan.Hooks()[0].Name == "corrupted" { t.Fatal("hook accessor copy leaked into plan") } } +func mustPlan(input PlanInput) Plan { + plan, err := NewPlan(input) + if err != nil { + panic(err) + } + return plan +} + func testZeroPlanAccessors(t *testing.T) { var plan Plan - if got := plan.AllGroups(); got != nil { + if got := plan.Groups(); got != nil { t.Fatalf("zero plan groups = %v, want nil", got) } - if got := plan.AllFiles(); got != nil { + if got := plan.Files(); got != nil { t.Fatalf("zero plan files = %v, want nil", got) } - if got := plan.AllAliases(); got != nil { + if got := plan.Aliases(); got != nil { t.Fatalf("zero plan aliases = %v, want nil", got) } - if got := plan.AllHooks(); got != nil { + if got := plan.Hooks(); got != nil { t.Fatalf("zero plan hooks = %v, want nil", got) } } diff --git a/internal/deployment/sort.go b/internal/deployment/sort.go index f16c21b..4c62d6d 100644 --- a/internal/deployment/sort.go +++ b/internal/deployment/sort.go @@ -12,9 +12,7 @@ func LessScope(a, b Scope) bool { return a.Group < b.Group } -// LessManagedFile reports whether file a precedes b. Comparison is bytewise on -// TargetRelativePath, then Layer, then Kind, establishing a total order so -// stable sorts are deterministic across permutations. +// LessManagedFile reports whether file a precedes b in a bytewise total order. func LessManagedFile(a, b ManagedFile) bool { if a.TargetRelativePath != b.TargetRelativePath { return a.TargetRelativePath < b.TargetRelativePath @@ -22,7 +20,19 @@ func LessManagedFile(a, b ManagedFile) bool { if a.Layer != b.Layer { return a.Layer < b.Layer } - return a.Kind < b.Kind + if a.Kind != b.Kind { + return a.Kind < b.Kind + } + if a.Scope.Group != b.Scope.Group { + return a.Scope.Group < b.Scope.Group + } + if a.SourceRepositoryPath != b.SourceRepositoryPath { + return a.SourceRepositoryPath < b.SourceRepositoryPath + } + if a.SourceAbsolutePath != b.SourceAbsolutePath { + return a.SourceAbsolutePath < b.SourceAbsolutePath + } + return a.SourceExecutableBits < b.SourceExecutableBits } // LessAlias reports whether alias a precedes b in bytewise AliasRelativePath diff --git a/internal/deployment/sort_test.go b/internal/deployment/sort_test.go index 2c08976..9cfc5a8 100644 --- a/internal/deployment/sort_test.go +++ b/internal/deployment/sort_test.go @@ -15,6 +15,7 @@ func TestDeploymentOrdering(t *testing.T) { {"root scope sorts first", testRootScopeFirst}, {"unicode bytewise order", testUnicodeScopeOrder}, {"managed file total order", testManagedFileOrder}, + {"managed file tie breakers", testManagedFileTieBreakers}, {"alias bytewise order", testAliasOrder}, {"both hook phases", testHookPhaseOrder}, {"groups dedup and sort", testGroupsDedup}, @@ -75,6 +76,28 @@ func testManagedFileOrder(t *testing.T) { } } +func testManagedFileTieBreakers(t *testing.T) { + files := []ManagedFile{ + {TargetRelativePath: "same", Layer: LayerBase, Kind: FileOrdinary, Scope: NewScope("z"), SourceRepositoryPath: "z", SourceAbsolutePath: "/z"}, + {TargetRelativePath: "same", Layer: LayerBase, Kind: FileOrdinary, Scope: NewScope("a"), SourceRepositoryPath: "a", SourceAbsolutePath: "/a"}, + {TargetRelativePath: "same", Layer: LayerBase, Kind: FileOrdinary, Scope: NewScope("a"), SourceRepositoryPath: "b", SourceAbsolutePath: "/b"}, + {TargetRelativePath: "same", Layer: LayerBase, Kind: FileOrdinary, Scope: NewScope("a"), SourceRepositoryPath: "b", SourceAbsolutePath: "/a", SourceExecutableBits: 0o111}, + } + SortFiles(files) + if files[0].Scope.Group != "a" || files[0].SourceRepositoryPath != "a" { + t.Fatalf("scope and repository path tie breakers failed: %+v", files) + } + if files[1].SourceAbsolutePath != "/a" || files[1].SourceExecutableBits != 0o111 { + t.Fatalf("executable bits tie breaker failed: %+v", files) + } + if files[2].Scope.Group != "a" || files[2].SourceRepositoryPath != "b" || files[2].SourceAbsolutePath != "/b" { + t.Fatalf("absolute path tie breaker failed: %+v", files) + } + if files[3].Scope.Group != "z" { + t.Fatalf("scope must be the first tie breaker: %+v", files) + } +} + func testAliasOrder(t *testing.T) { aliases := []Alias{ {AliasRelativePath: ".config/zzz/alias"}, diff --git a/internal/reconcile/decisions_test.go b/internal/reconcile/decisions_test.go index 320b28d..d0c2acf 100644 --- a/internal/reconcile/decisions_test.go +++ b/internal/reconcile/decisions_test.go @@ -144,12 +144,12 @@ var decisionOrderCases = []orderCase{ // specOf builds one decision spec over the fixed file path. func specOf(reason Reason, choices []DecisionChoice) DecisionSpec { - return DecisionSpec{targetPath: decisionPath, action: ActionNeedsDecision, reason: reason, choices: choices} + return mustDecisionSpec(DecisionSpecInput{TargetPath: decisionPath, Action: ActionNeedsDecision, Reason: reason, Choices: choices}) } // orderSpec builds one ordinary drift spec at path. func orderSpec(path string) DecisionSpec { - return DecisionSpec{targetPath: path, action: ActionNeedsDecision, reason: ReasonTargetDrift, choices: diffOverwriteSkipAbort} + return mustDecisionSpec(DecisionSpecInput{TargetPath: path, Action: ActionNeedsDecision, Reason: ReasonTargetDrift, Choices: diffOverwriteSkipAbort}) } // checkDecisionSpec produces one spec from its row and compares the result. @@ -165,7 +165,7 @@ func checkDecisionSpec(t *testing.T, row specCase) { if err != nil { t.Fatalf("spec %s: %v", row.name, err) } - want := DecisionSpec{targetPath: row.path, action: row.action, reason: row.reason, choices: row.want} + want := mustDecisionSpec(DecisionSpecInput{TargetPath: row.path, Action: row.action, Reason: row.reason, Choices: row.want}) if !reflect.DeepEqual(spec, want) { t.Fatalf("spec = %+v, want %+v", spec, want) } @@ -179,6 +179,14 @@ func specAt(row specCase) (DecisionSpec, error) { return DecisionSpecForFile(FileClassification{TargetPath: row.path, Action: row.action, Reason: row.reason, Convergence: row.convergence}, row.kind) } +func mustDecisionSpec(input DecisionSpecInput) DecisionSpec { + spec, err := NewDecisionSpec(input) + if err != nil { + panic(err) + } + return spec +} + // checkInvalidDecision validates one spec row against the eligibility rule. func checkInvalidDecision(t *testing.T, row invalidCase) { t.Helper() diff --git a/internal/reconcile/snapshot.go b/internal/reconcile/snapshot.go index 8957120..24675ba 100644 --- a/internal/reconcile/snapshot.go +++ b/internal/reconcile/snapshot.go @@ -6,6 +6,7 @@ import ( "github.com/alyraffauf/cattery/internal/deployment" "github.com/alyraffauf/cattery/internal/secrets" + "github.com/alyraffauf/cattery/internal/state" ) // PlanEntryKind names the representation one plan entry produces at a path. @@ -69,19 +70,19 @@ func Assemble(plan deployment.Plan, state StateSnapshot, client *secrets.Client) sort.SliceStable(records, func(first, second int) bool { return records[first].TargetPath < records[second].TargetPath }) - return EvaluationSnapshot{RepositoryRoot: plan.RepositoryRoot, HomePath: state.HomePath(), - Platform: plan.Platform, records: records}, nil + return EvaluationSnapshot{RepositoryRoot: plan.RepositoryRoot(), HomePath: state.HomePath(), + Platform: plan.Platform(), records: records}, nil } // requireAssemblyPlan rejects a plan that cannot describe the state pair. func requireAssemblyPlan(plan deployment.Plan, state StateSnapshot) error { - if plan.RepositoryRoot == "" || state.HomePath() == "" { + if plan.RepositoryRoot() == "" || state.HomePath() == "" { return fmt.Errorf("reconcile: snapshot assembly requires canonical repository and home paths") } - if plan.RepositoryRoot != state.RepositoryRoot() { - return fmt.Errorf("reconcile: plan repository %q does not match state repository %q", plan.RepositoryRoot, state.RepositoryRoot()) + if plan.RepositoryRoot() != state.RepositoryRoot() { + return fmt.Errorf("reconcile: plan repository %q does not match state repository %q", plan.RepositoryRoot(), state.RepositoryRoot()) } - if plan.Platform == "" { + if plan.Platform() == "" { return fmt.Errorf("reconcile: snapshot assembly requires a selected platform") } return nil @@ -90,15 +91,17 @@ func requireAssemblyPlan(plan deployment.Plan, state StateSnapshot) error { // entryIndexes indexes plan entries by destination path, rejecting duplicate // file, duplicate alias, and file/alias collisions at one path. func entryIndexes(plan deployment.Plan) (map[string]deployment.ManagedFile, map[string]deployment.Alias, error) { - files := make(map[string]deployment.ManagedFile, len(plan.Files)) - for _, file := range plan.Files { + planFiles := plan.Files() + files := make(map[string]deployment.ManagedFile, len(planFiles)) + for _, file := range planFiles { if _, occupied := files[file.TargetRelativePath]; occupied { return nil, nil, fmt.Errorf("reconcile: plan has duplicate file entry %q", file.TargetRelativePath) } files[file.TargetRelativePath] = file } - aliases := make(map[string]deployment.Alias, len(plan.Aliases)) - for _, alias := range plan.Aliases { + planAliases := plan.Aliases() + aliases := make(map[string]deployment.Alias, len(planAliases)) + for _, alias := range planAliases { if _, occupied := aliases[alias.AliasRelativePath]; occupied { return nil, nil, fmt.Errorf("reconcile: plan has duplicate alias entry %q", alias.AliasRelativePath) } @@ -205,6 +208,7 @@ func cloneFileRecord(record *FileState) *FileState { return nil } copyRecord := *record + copyRecord.retiredAt = state.CloneTimestamp(record.retiredAt) return ©Record } @@ -214,6 +218,7 @@ func cloneAliasRecord(record *AliasState) *AliasState { return nil } copyRecord := *record + copyRecord.retiredAt = state.CloneTimestamp(record.retiredAt) return ©Record } diff --git a/internal/reconcile/snapshot_helpers_test.go b/internal/reconcile/snapshot_helpers_test.go new file mode 100644 index 0000000..bb8072d --- /dev/null +++ b/internal/reconcile/snapshot_helpers_test.go @@ -0,0 +1,123 @@ +package reconcile + +import ( + "os" + "path/filepath" + "testing" + + "github.com/alyraffauf/cattery/internal/deployment" +) + +func mustAssemble(t *testing.T, plan deployment.Plan, state StateSnapshot) EvaluationSnapshot { + t.Helper() + snapshot, err := Assemble(plan, state, nil) + if err != nil { + t.Fatalf("assemble: %v", err) + } + return snapshot +} + +func fixtureDir(t *testing.T) (repo, home string) { + t.Helper() + home = t.TempDir() + if err := os.MkdirAll(filepath.Join(home, "repo"), 0o700); err != nil { + t.Fatalf("mkdir repo: %v", err) + } + return filepath.Join(home, "repo"), home +} + +func planFile(t *testing.T, repo, target string) deployment.ManagedFile { + t.Helper() + path := filepath.Join(repo, filepath.FromSlash(target)) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + writeSource(t, path, []byte("source "+target)) + return deployment.ManagedFile{Scope: deployment.Scope{Group: "apps"}, Layer: deployment.LayerBase, Kind: deployment.FileOrdinary, + SourceAbsolutePath: path, SourceRepositoryPath: target, TargetRelativePath: target} +} + +func samplePlan(repo string, files []deployment.ManagedFile, aliases []deployment.Alias) deployment.Plan { + plan, err := deployment.NewPlan(deployment.PlanInput{RepositoryRoot: repo, Platform: "linux", Files: files, Aliases: aliases}) + if err != nil { + panic(err) + } + return plan +} + +func mustPlan(input deployment.PlanInput) deployment.Plan { + plan, err := deployment.NewPlan(input) + if err != nil { + panic(err) + } + return plan +} + +func sampleState(t *testing.T, repo string, rows StateRows) StateSnapshot { + t.Helper() + rows.RepositoryRoot, rows.HomePath = repo, filepath.Dir(repo) + return convertRows(t, rows) +} + +func findRecord(t *testing.T, records []Evaluation, path string) Evaluation { + t.Helper() + for _, record := range records { + if record.TargetPath == path { + return record + } + } + t.Fatalf("no record for %q", path) + return Evaluation{} +} + +func fixtureLink(t *testing.T, path, link string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.Symlink(link, path); err != nil { + t.Fatalf("symlink %s: %v", path, err) + } +} + +func requireFileJoin(t *testing.T, record Evaluation, target string) { + t.Helper() + if record.Entry != PlanEntryFile || record.File.TargetRelativePath != target || record.FileState == nil || !record.FileState.Active() { + t.Fatalf("record %s must join its file descriptor and row", target) + } + if record.Target.Kind() != KindFile || record.Source.Snapshot().Token() != TokenOfContent([]byte("source "+target)) { + t.Fatalf("record %s must join target and source observations", target) + } +} + +func requireRetiredJoin(t *testing.T, record Evaluation) { + t.Helper() + if record.Entry != PlanEntryNone || record.Source.Snapshot().Path() != "" { + t.Fatalf("record %s must carry no producer or source", record.TargetPath) + } + if record.FileState != nil { + if record.FileState.Active() { + t.Fatalf("record %s must join its retired file row", record.TargetPath) + } + return + } + if record.AliasState == nil || record.AliasState.Active() { + t.Fatalf("record %s must join its retired alias row", record.TargetPath) + } +} + +func requireTransition(t *testing.T, record Evaluation, canonical string) { + t.Helper() + if record.FileState == nil || record.AliasState == nil || record.FileState.Active() == record.AliasState.Active() { + t.Fatalf("record %s must join exactly one active representation row", record.TargetPath) + } + if record.Entry == PlanEntryAlias { + if record.Alias.CanonicalTargetRelativePath != canonical || record.Target.Kind() != KindSymlink || record.Target.Payload() != canonical { + t.Fatalf("record %s must join its alias entry and symlink", record.TargetPath) + } + return + } + if record.File.TargetRelativePath != record.TargetPath || record.Target.Kind() != KindFile { + t.Fatalf("record %s must join its file entry and observation", record.TargetPath) + } +} diff --git a/internal/reconcile/snapshot_test.go b/internal/reconcile/snapshot_test.go index 07719ee..0381801 100644 --- a/internal/reconcile/snapshot_test.go +++ b/internal/reconcile/snapshot_test.go @@ -28,106 +28,17 @@ func TestSnapshotAssembly(t *testing.T) { } } -func mustAssemble(t *testing.T, plan deployment.Plan, state StateSnapshot) EvaluationSnapshot { - t.Helper() - snapshot, err := Assemble(plan, state, nil) - if err != nil { - t.Fatalf("assemble: %v", err) - } - return snapshot -} -func fixtureDir(t *testing.T) (repo, home string) { - t.Helper() - home = t.TempDir() - if err := os.MkdirAll(filepath.Join(home, "repo"), 0o700); err != nil { - t.Fatalf("mkdir repo: %v", err) - } - return filepath.Join(home, "repo"), home -} -func planFile(t *testing.T, repo, target string) deployment.ManagedFile { - t.Helper() - path := filepath.Join(repo, filepath.FromSlash(target)) - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) - } - writeSource(t, path, []byte("source "+target)) - return deployment.ManagedFile{Scope: deployment.Scope{Group: "apps"}, Layer: deployment.LayerBase, Kind: deployment.FileOrdinary, - SourceAbsolutePath: path, SourceRepositoryPath: target, TargetRelativePath: target} -} -func samplePlan(repo string, files []deployment.ManagedFile, aliases []deployment.Alias) deployment.Plan { - return deployment.NewPlan(deployment.Plan{RepositoryRoot: repo, Platform: "linux", Files: files, Aliases: aliases}) -} -func sampleState(t *testing.T, repo string, rows StateRows) StateSnapshot { - t.Helper() - rows.RepositoryRoot, rows.HomePath = repo, filepath.Dir(repo) - return convertRows(t, rows) -} -func findRecord(t *testing.T, records []Evaluation, path string) Evaluation { - t.Helper() - for _, record := range records { - if record.TargetPath == path { - return record - } - } - t.Fatalf("no record for %q", path) - return Evaluation{} -} -func fixtureLink(t *testing.T, path, link string) { - t.Helper() - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) - } - if err := os.Symlink(link, path); err != nil { - t.Fatalf("symlink %s: %v", path, err) - } -} -func requireFileJoin(t *testing.T, record Evaluation, target string) { - t.Helper() - if record.Entry != PlanEntryFile || record.File.TargetRelativePath != target || record.FileState == nil || !record.FileState.Active() { - t.Fatalf("record %s must join its file descriptor and row", target) - } - if record.Target.Kind() != KindFile || record.Source.Snapshot().Token() != TokenOfContent([]byte("source "+target)) { - t.Fatalf("record %s must join target and source observations", target) - } -} -func requireRetiredJoin(t *testing.T, record Evaluation) { - t.Helper() - if record.Entry != PlanEntryNone || record.Source.Snapshot().Path() != "" { - t.Fatalf("record %s must carry no producer or source", record.TargetPath) - } - if record.FileState != nil { - if record.FileState.Active() { - t.Fatalf("record %s must join its retired file row", record.TargetPath) - } - return - } - if record.AliasState == nil || record.AliasState.Active() { - t.Fatalf("record %s must join its retired alias row", record.TargetPath) - } -} -func requireTransition(t *testing.T, record Evaluation, canonical string) { - t.Helper() - if record.FileState == nil || record.AliasState == nil || record.FileState.Active() == record.AliasState.Active() { - t.Fatalf("record %s must join exactly one active representation row", record.TargetPath) - } - if record.Entry == PlanEntryAlias { - if record.Alias.CanonicalTargetRelativePath != canonical || record.Target.Kind() != KindSymlink || record.Target.Payload() != canonical { - t.Fatalf("record %s must join its alias entry and symlink", record.TargetPath) - } - return - } - if record.File.TargetRelativePath != record.TargetPath || record.Target.Kind() != KindFile { - t.Fatalf("record %s must join its file entry and observation", record.TargetPath) - } -} func testSnapshotDeterministicJoins(t *testing.T) { repo, home := fixtureDir(t) files := []deployment.ManagedFile{planFile(t, repo, "a.conf"), planFile(t, repo, "c")} aliases := []deployment.Alias{{Platform: "linux", AliasRelativePath: "bin/z", CanonicalTargetRelativePath: "files/z"}} mustTargetFile(t, filepath.Join(home, "a.conf"), []byte("current a")) fixtureLink(t, filepath.Join(home, "bin", "z"), "files/z") - state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{fileRow("a.conf", "apps", "a.conf"), fileRow("c", "apps", "c")}, Aliases: []state.AliasBaseline{aliasRow("bin/z", "files/z", "apps")}}) - records := mustAssemble(t, samplePlan(repo, []deployment.ManagedFile{files[1], files[0]}, aliases), StateSnapshot{repositoryRoot: state.RepositoryRoot(), homePath: state.HomePath(), files: []FileState{state.AllFiles()[1], state.AllFiles()[0]}, aliases: state.AllAliases()}).All() + state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{fileRow("a.conf", "apps", "a.conf"), fileRow("c", "apps", "c")}, + Aliases: []state.AliasBaseline{aliasRow("bin/z", "files/z", "apps")}}) + records := mustAssemble(t, samplePlan(repo, []deployment.ManagedFile{files[1], files[0]}, aliases), + StateSnapshot{repositoryRoot: state.RepositoryRoot(), homePath: state.HomePath(), + files: []FileState{state.AllFiles()[1], state.AllFiles()[0]}, aliases: state.AllAliases()}).All() if len(records) != 3 || records[0].TargetPath > records[1].TargetPath || records[1].TargetPath > records[2].TargetPath { t.Fatal("records must be bytewise sorted in path order") } @@ -136,13 +47,12 @@ func testSnapshotDeterministicJoins(t *testing.T) { func testSnapshotMissingProducers(t *testing.T) { repo, home := fixtureDir(t) mustTargetFile(t, filepath.Join(home, "gone"), []byte("stale target")) - when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) retired := fileRow("gone", "apps", "files/gone") retired.Status = state.StatusRetired - retired.RetiredAt = &when + retired.RetiredAt = ptrTimestamp() alias := aliasRow("bin/old", "files/old", "apps") alias.Status = state.StatusRetired - alias.RetiredAt = &when + alias.RetiredAt = ptrTimestamp() state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{retired}, Aliases: []state.AliasBaseline{alias}}) records := mustAssemble(t, samplePlan(repo, nil, nil), state).All() if len(records) != 2 { @@ -158,13 +68,12 @@ func testSnapshotMissingProducers(t *testing.T) { func testSnapshotRepresentationPairs(t *testing.T) { repo, home := fixtureDir(t) fileToAlias := fileRow("bin/tool", "apps", "files/tool") - when := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) fileToAliasPair := aliasRow("bin/tool", "files/tool", "apps") fileToAliasPair.Status = state.StatusRetired - fileToAliasPair.RetiredAt = &when + fileToAliasPair.RetiredAt = ptrTimestamp() aliasToFile := fileRow("conf/app", "apps", "files/app") aliasToFile.Status = state.StatusRetired - aliasToFile.RetiredAt = &when + aliasToFile.RetiredAt = ptrTimestamp() aliasToFilePair := aliasRow("conf/app", "files/app", "apps") plan := samplePlan(repo, []deployment.ManagedFile{planFile(t, repo, "conf/app")}, []deployment.Alias{{Platform: "linux", AliasRelativePath: "bin/tool", CanonicalTargetRelativePath: "files/tool"}}) @@ -173,7 +82,8 @@ func testSnapshotRepresentationPairs(t *testing.T) { } mustTargetFile(t, filepath.Join(home, "conf", "app"), []byte("current app")) fixtureLink(t, filepath.Join(home, "bin", "tool"), "files/tool") - state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{fileToAlias, aliasToFile}, Aliases: []state.AliasBaseline{fileToAliasPair, aliasToFilePair}}) + state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{fileToAlias, aliasToFile}, + Aliases: []state.AliasBaseline{fileToAliasPair, aliasToFilePair}}) records := mustAssemble(t, plan, state).All() requireTransition(t, findRecord(t, records, "bin/tool"), "files/tool") requireTransition(t, findRecord(t, records, "conf/app"), "conf/app") @@ -188,10 +98,13 @@ func testSnapshotDefensiveCopies(t *testing.T) { plan := samplePlan(repo, []deployment.ManagedFile{planFile(t, repo, "a.conf")}, nil) state := sampleState(t, repo, StateRows{Files: []state.FileBaseline{row}}) snapshot := mustAssemble(t, plan, state) - plan.Files[0].TargetRelativePath = "mutated" - state.files[0].baselineContent = deployment.Digest{} + files := plan.Files() + files[0].TargetRelativePath = "mutated" + stateFiles := state.AllFiles() + stateFiles[0].baselineContent = deployment.Digest{} record := findRecord(t, snapshot.All(), "a.conf") - *record.FileState.RetiredAt() = time.Time{} + retiredAt := record.FileState.RetiredAt() + *retiredAt = time.Time{} *row.RetiredAt = time.Time{} fresh := findRecord(t, snapshot.All(), "a.conf") if fresh.FileState.RetiredAt() == nil || fresh.FileState.RetiredAt().IsZero() || fresh.FileState.BaselineContent() == (deployment.Digest{}) { @@ -226,17 +139,23 @@ func testSnapshotRejected(t *testing.T) { state := sampleState(t, repo, StateRows{}) other := samplePlan(filepath.Join(filepath.Dir(repo), "other"), []deployment.ManagedFile{file}, nil) noHome := StateSnapshot{repositoryRoot: repo, files: []FileState{{targetPath: "a.conf"}}} + duplicate := mustPlan(deployment.PlanInput{RepositoryRoot: repo, Platform: "linux", Files: []deployment.ManagedFile{file, file}}) + collision := mustPlan(deployment.PlanInput{RepositoryRoot: repo, Platform: "linux", Files: []deployment.ManagedFile{file}, + Aliases: []deployment.Alias{{Platform: "linux", AliasRelativePath: "a.conf", CanonicalTargetRelativePath: "files/a.conf"}}}) + duplicateAlias := mustPlan(deployment.PlanInput{RepositoryRoot: repo, Platform: "linux", + Aliases: []deployment.Alias{{Platform: "linux", AliasRelativePath: "bin/x", CanonicalTargetRelativePath: "files/x"}, + {Platform: "linux", AliasRelativePath: "bin/x", CanonicalTargetRelativePath: "files/y"}}}) cases := []struct { name string plan deployment.Plan state StateSnapshot }{ {"other repository", other, state}, - {"no platform", deployment.NewPlan(deployment.Plan{RepositoryRoot: repo}), state}, + {"no platform", deployment.Plan{}, state}, {"unset home", samplePlan(repo, []deployment.ManagedFile{file}, nil), noHome}, - {"duplicate files", deployment.NewPlan(deployment.Plan{RepositoryRoot: repo, Platform: "linux", Files: []deployment.ManagedFile{file, file}}), state}, - {"file and alias collision", deployment.NewPlan(deployment.Plan{RepositoryRoot: repo, Platform: "linux", Files: []deployment.ManagedFile{file}, Aliases: []deployment.Alias{{Platform: "linux", AliasRelativePath: "a.conf", CanonicalTargetRelativePath: "files/a.conf"}}}), state}, - {"duplicate aliases", deployment.NewPlan(deployment.Plan{RepositoryRoot: repo, Platform: "linux", Aliases: []deployment.Alias{{Platform: "linux", AliasRelativePath: "bin/x", CanonicalTargetRelativePath: "files/x"}, {Platform: "linux", AliasRelativePath: "bin/x", CanonicalTargetRelativePath: "files/y"}}}), state}, + {"duplicate files", duplicate, state}, + {"file and alias collision", collision, state}, + {"duplicate aliases", duplicateAlias, state}, } for _, scenario := range cases { if _, err := Assemble(scenario.plan, scenario.state, nil); err == nil { diff --git a/internal/repository/compiler.go b/internal/repository/compiler.go index 4b13d5b..7938b0b 100644 --- a/internal/repository/compiler.go +++ b/internal/repository/compiler.go @@ -47,7 +47,7 @@ func Compile(input CompileInput) (deployment.Plan, error) { CollisionScope{HomeRoot: input.HomeRoot, Protected: input.Protected}); err != nil { return deployment.Plan{}, err } - return finalize(input, records), nil + return finalize(input, records) } // compileRepository runs phases 1-7: scan, overlay, routes, hooks, paths. @@ -87,7 +87,7 @@ func scanAndSelect(input CompileInput) (ScanResult, error) { } // finalize filters to the selection, sorts, and wraps the immutable plan. -func finalize(input CompileInput, records compiled) deployment.Plan { +func finalize(input CompileInput, records compiled) (deployment.Plan, error) { selected := input.Selected groups := records.groups if len(input.Selected) > 0 { @@ -103,7 +103,7 @@ func finalize(input CompileInput, records compiled) deployment.Plan { deployment.SortFiles(keptFiles) deployment.SortAliases(keptAliases) deployment.SortHooks(keptHooks) - return deployment.NewPlan(deployment.Plan{ + return deployment.NewPlan(deployment.PlanInput{ RepositoryRoot: input.RepositoryRoot, Platform: string(input.Platform), Groups: groups, diff --git a/internal/repository/compiler_test.go b/internal/repository/compiler_test.go index 99040c8..1e7d6a9 100644 --- a/internal/repository/compiler_test.go +++ b/internal/repository/compiler_test.go @@ -78,7 +78,7 @@ func testPlanGolden(t *testing.T) { // goldenWant builds the expected plan for one golden scenario. func goldenWant(root string, scenario goldenScenario) deployment.Plan { - return deployment.Plan{ + return mustPlan(deployment.PlanInput{ RepositoryRoot: root, Platform: string(scenario.platform), Groups: []string{"atuin", "ghostty"}, @@ -89,7 +89,7 @@ func goldenWant(root string, scenario goldenScenario) deployment.Plan { expectHook(root, hookWant{scope: deployment.NewScope(""), phase: deployment.HookBefore, name: "install.sh"}), expectHook(root, hookWant{scope: deployment.NewScope("atuin"), phase: deployment.HookBefore, name: "init.sh"}), }, - } + }) } func testPlanDeterminism(t *testing.T) { @@ -120,7 +120,7 @@ func testPlanSelection(t *testing.T) { if err != nil { t.Fatal(err) } - want := deployment.Plan{ + want := mustPlan(deployment.PlanInput{ RepositoryRoot: root, Platform: "linux", Groups: []string{"atuin"}, @@ -132,7 +132,7 @@ func testPlanSelection(t *testing.T) { expectHook(root, hookWant{scope: deployment.NewScope(""), phase: deployment.HookBefore, name: "install.sh"}), expectHook(root, hookWant{scope: deployment.NewScope("atuin"), phase: deployment.HookBefore, name: "init.sh"}), }, - } + }) assertPlan(t, plan, want) } @@ -190,6 +190,14 @@ func assertPlan(t *testing.T, got deployment.Plan, want deployment.Plan) { } } +func mustPlan(input deployment.PlanInput) deployment.Plan { + plan, err := deployment.NewPlan(input) + if err != nil { + panic(err) + } + return plan +} + // fileWant describes one expected managed file record. type fileWant struct { scope deployment.Scope -- 2.51.2