From f154b88d4ee4d97ff3b31e553c1f40e5d4560460 Mon Sep 17 00:00:00 2001 From: Aly Raffauf Date: Mon, 10 Aug 2026 07:45:55 -0400 Subject: [PATCH] refactor: make state and filesystem phases explicit --- internal/application/apply/execute_files.go | 11 +++--- internal/reconcile/snapshot.go | 11 +++--- internal/repository/compiler.go | 2 ++ internal/repository/overlay.go | 22 +++++++----- internal/repository/scan.go | 2 ++ internal/state/repositories.go | 37 +++++++++++++++------ 6 files changed, 54 insertions(+), 31 deletions(-) diff --git a/internal/application/apply/execute_files.go b/internal/application/apply/execute_files.go index bf5637c..c9805cf 100644 --- a/internal/application/apply/execute_files.go +++ b/internal/application/apply/execute_files.go @@ -74,9 +74,6 @@ func (service *Service) executeFile(ctx context.Context, job fileJob) (ItemResul if err := service.commitBaseline(ctx, job, baselineCommit{contentHash: contentHash, durable: durable}); err != nil { return partialRecord(job), err } - if !durable { - return partialRecord(job), nil - } return completedRecord(job), nil } @@ -115,10 +112,10 @@ type writeSpec struct { candidate Candidate } -// sourceContent returns the exact source bytes of one file action: the -// retained ordinary bytes, or the decrypted plaintext for secrets. The -// content hash is the keyed secret fingerprint or the ordinary digest. -func (service *Service) sourceContent(ctx context.Context, candidate Candidate) ([]byte, deployment.Digest, func(), error) { +// sourceContent returns exact source bytes, their semantic hash, and a cleanup +// function that must be called after the bytes are no longer needed. The +// cleanup function is non-nil only when the bytes contain decrypted plaintext. +func (service *Service) sourceContent(ctx context.Context, candidate Candidate) (content []byte, contentHash deployment.Digest, clear func(), err error) { snapshot := candidate.record.Source.Snapshot() if candidate.record.File.Kind != deployment.FileSecret { return candidate.record.Source.Bytes(), snapshot.Semantic(), nil, nil diff --git a/internal/reconcile/snapshot.go b/internal/reconcile/snapshot.go index fbdafd6..25b24d9 100644 --- a/internal/reconcile/snapshot.go +++ b/internal/reconcile/snapshot.go @@ -184,11 +184,12 @@ func recordFor(path string, input joinInput) (Evaluation, error) { return record, nil } -// byFileState indexes file records by target path. +// byFileState indexes the caller-owned state slice by target path. The slice +// is freshly allocated by StateSnapshot.All, so these pointers remain local. func byFileState(rows []FileState) map[string]*FileState { index := make(map[string]*FileState, len(rows)) - for number := range rows { - index[rows[number].TargetPath()] = &rows[number] + for rowIndex := range rows { + index[rows[rowIndex].TargetPath()] = &rows[rowIndex] } return index } @@ -196,8 +197,8 @@ func byFileState(rows []FileState) map[string]*FileState { // byAliasState indexes alias records by alias path. func byAliasState(rows []AliasState) map[string]*AliasState { index := make(map[string]*AliasState, len(rows)) - for number := range rows { - index[rows[number].AliasPath()] = &rows[number] + for rowIndex := range rows { + index[rows[rowIndex].AliasPath()] = &rows[rowIndex] } return index } diff --git a/internal/repository/compiler.go b/internal/repository/compiler.go index 705ad26..18a6772 100644 --- a/internal/repository/compiler.go +++ b/internal/repository/compiler.go @@ -173,6 +173,8 @@ func activateScope(input CompileInput, scope deployment.Scope, files []deploymen return nil, err } for index := range activated { + // routes.Activate validates paths but intentionally leaves scope ownership + // to the repository compiler. activated[index].Scope = scope } return activated, nil diff --git a/internal/repository/overlay.go b/internal/repository/overlay.go index fe289eb..e586dc0 100644 --- a/internal/repository/overlay.go +++ b/internal/repository/overlay.go @@ -14,7 +14,7 @@ func ResolvePlatform(root string, baseScan ScanResult, platformLayer deployment. if !platformLayer.Valid() { return nil, fmt.Errorf("repository: unknown platform layer %q", platformLayer) } - platformResolver := resolver{root: root, base: baseScan, platform: platformLayer} + platformResolver := platformResolver{root: root, base: baseScan, platform: platformLayer} platformRootView, err := scanLayerTree(root, deployment.NewScope(""), platformLayer) if err != nil { return nil, err @@ -34,13 +34,14 @@ func ResolvePlatform(root string, baseScan ScanResult, platformLayer deployment. return records, nil } -type resolver struct { +type platformResolver struct { root string base ScanResult platform deployment.Layer } -func (resolver *resolver) resolveScope(scope deployment.Scope, platformRootView layerView) ([]deployment.ManagedFile, error) { +func (resolver *platformResolver) resolveScope(scope deployment.Scope, platformRootView layerView) ([]deployment.ManagedFile, error) { + // A platform file at the group name replaces the entire group subtree. if _, replaced := platformRootView.files[scope.Group]; replaced { return nil, nil } @@ -53,7 +54,7 @@ func (resolver *resolver) resolveScope(scope deployment.Scope, platformRootView type layerView struct { files map[string]Candidate - dirs map[string]bool + dirs map[string]struct{} } // covers reports whether any base-layer entry suppresses the platform layer @@ -62,7 +63,10 @@ type layerView struct { // beneath it. That is why covers walks prefixes of the target rather than only // exact file/dir matches. func (view layerView) covers(target string) bool { - if _, ok := view.files[target]; ok || view.dirs[target] { + if _, ok := view.files[target]; ok { + return true + } + if _, ok := view.dirs[target]; ok { return true } segments := strings.Split(target, "/") @@ -76,7 +80,7 @@ func (view layerView) covers(target string) bool { } func resolveScopeFiles(base ScanResult, scope deployment.Scope, platform layerView) ([]deployment.ManagedFile, error) { - merged := layerView{files: map[string]Candidate{}, dirs: map[string]bool{}} + merged := layerView{files: map[string]Candidate{}, dirs: map[string]struct{}{}} for _, candidate := range base.Files { if candidate.Scope != scope { continue @@ -156,7 +160,7 @@ func scanLayerTree(root string, scope deployment.Scope, platform deployment.Laye } walker := layerWalker{ absolute: filepath.Join(root, relative), relative: relative, scope: scope, layer: platform, - view: layerView{files: map[string]Candidate{}, dirs: map[string]bool{}}, + view: layerView{files: map[string]Candidate{}, dirs: map[string]struct{}{}}, } if err := walker.walk("", deployment.FileOrdinary); err != nil { return layerView{}, err @@ -215,7 +219,9 @@ func (walker *layerWalker) visit(path string, entry os.DirEntry, kind deployment return err } if entry.IsDir() { - walker.view.dirs[target] = target != "" + if target != "" { + walker.view.dirs[target] = struct{}{} + } return walker.walk(path, kind) } if !entry.Type().IsRegular() { diff --git a/internal/repository/scan.go b/internal/repository/scan.go index e7bc6fe..61d7e47 100644 --- a/internal/repository/scan.go +++ b/internal/repository/scan.go @@ -156,6 +156,8 @@ func (scanner *scopeScanner) scanHookPhase(hooksDir string, phase deployment.Hoo return err } if !info.IsDir() { + // Missing hook phases are optional; a non-directory phase is ignored like + // an absent phase because hook discovery only consumes regular children. return nil } entries, err := os.ReadDir(path) diff --git a/internal/state/repositories.go b/internal/state/repositories.go index 8ae758c..98ede4f 100644 --- a/internal/state/repositories.go +++ b/internal/state/repositories.go @@ -82,8 +82,12 @@ func (store *Store) SetDefaultRepository(root, home string) (Repository, error) if err != nil { return Repository{}, err } - batches := [][]any{{root, home, now, now}, {home}, {now, root, home}} - if err := execBatch(transaction, []string{repositoryUpsertSQL, clearDefaultsSQL, markDefaultSQL}, batches); err != nil { + steps := []sqlStep{ + {operation: "register repository", statement: repositoryUpsertSQL, arguments: []any{root, home, now, now}}, + {operation: "clear repository defaults", statement: clearDefaultsSQL, arguments: []any{home}}, + {operation: "mark repository default", statement: markDefaultSQL, arguments: []any{now, root, home}}, + } + if err := execBatch(transaction, steps); err != nil { return Repository{}, err } repository, err := scanAndCommit(transaction, root, home) @@ -194,27 +198,38 @@ func scanAndCommit(transaction *sql.Tx, root, home string) (Repository, error) { return repository, nil } -// execBatch executes each statement with its own argument batch in order, -// stopping at the first failure. Statement and batch indices correspond. -func execBatch(transaction *sql.Tx, statements []string, batches [][]any) error { - for index, statement := range statements { - if err := execIn(transaction, statement, batches[index]...); err != nil { +type sqlStep struct { + operation string + statement string + arguments []any +} + +// execBatch executes named statements in order, stopping at the first failure. +func execBatch(transaction *sql.Tx, steps []sqlStep) error { + for _, step := range steps { + if err := execNamed(transaction, step.operation, step.statement, step.arguments...); err != nil { return err } } return nil } -// execIn executes one statement in the transaction, rolling back and wrapping -// the error when the statement fails. -func execIn(transaction *sql.Tx, statement string, arguments ...any) error { +// execNamed executes one named statement in the transaction, rolling back and +// wrapping the error when the statement fails. +func execNamed(transaction *sql.Tx, operation, statement string, arguments ...any) error { if _, err := transaction.Exec(statement, arguments...); err != nil { _ = transaction.Rollback() - return fmt.Errorf("state: %s: %w", statement, err) + return fmt.Errorf("state: %s: %w", operation, err) } return nil } +// execIn executes one statement in the transaction, rolling back and wrapping +// the error when the statement fails. +func execIn(transaction *sql.Tx, statement string, arguments ...any) error { + return execNamed(transaction, statement, statement, arguments...) +} + // canonicalRepositoryPair resolves both paths to canonical absolute form so // only canonical pairs are ever stored or matched. func canonicalRepositoryPair(root, home string) (string, string, error) { -- 2.51.2