Files
narratio/docs/roadmap/audit.md

36 KiB
Raw Blame History

Maintainability Audit

Status

Complete. This audit evaluates the hypotheses in potential-audit.md in their original order. That source was committed unchanged as 5e4d201 and treated as immutable. The code under review is the v1.6.0 tree at 545aa68; the source-document commit adds no production or test code.

This document records findings and recommended changes. It is not an implementation plan and does not authorize behavior changes by itself.

Method

The review combined repository metrics, package and call-graph inspection, focused source reading, change-history review, policy and internal-documentation comparison, and test-suite inspection. The named functions and files in the source document were read directly. Baseline go test ./... and go vet ./... passed with the maintained Go 1.25 configuration. The complete test suite and go vet also passed under Go 1.27.0, and the release asset builder successfully cross-compiled all six supported OS/architecture targets with Go 1.27.0.

Verdicts use these terms:

  • Confirmed: the hypothesis identifies work worth scheduling.
  • Focused opportunity: improvement is justified, but only at a narrow seam.
  • Already satisfied: the desired end state is present.
  • Retain: the current application-owned implementation is justified.
  • Not substantiated: the evidence does not justify the proposed change.

Executive Summary

Narratio does not have a broad maintainability problem. Its package direction, explicit contracts, documentation, tests, and small dependency surface are all healthy. Most size is a consequence of real orchestration, recovery, and integration requirements. The exception is filesystem code, where mechanisms originally chosen for adversarial races and power-loss durability have been treated as durable product requirements without sufficient operational need.

The audit did identify one correctness remediation and several proportional maintainability improvements:

  1. Simplify internal/fileops around Narratio's actual trusted-workspace model. Retain root-scoped destructive and restore operations, atomic visibility for corruption-sensitive file replacement, bounded I/O, group-owned workspace modes, and stricter handling of API-key material. Drop power-loss durability, hostile-race defenses, universal symlink and regular-file restrictions, and atomic no-replace directory promotion as application contracts.
  2. Restore planning must still classify local targets safely within the session root. classifyRestoreAction currently uses os.Stat and path-based hashing, so a symlink to a regular file can be classified as skip_same; correct this within the simpler root-scoped restore design rather than adding more identity-checking machinery to the current abstraction.
  3. Decompose executePlan around its existing lifecycle boundaries, and share the pre-execution stage-decision model used by planning and execution. Do not introduce a generic workflow engine or change transition ordering during the extraction.
  4. Split the long per-artifact analyze transaction, configuration validation domains, publish source/transfer/commit concerns, and modern-versus-legacy previous-cache resolution at their existing internal seams. Keep the current packages and public contracts.
  5. Upgrade the maintained baseline to Go 1.27, use os.Process.WithHandle for Windows job assignment, and replace the transcription worker pool with errgroup bounded concurrency. Use slices, maps, and go-cmp only where they make simple mechanics or test failures clearer.
  6. Replace mutable package-global function seams in internal/app with a small invocation-scoped dependency set as affected command areas are refactored. This will reduce test reset boilerplate and permit safe parallel tests without creating a general dependency-injection framework.

The audit does not recommend package-wide reorganization, a CLI framework, an external flock library, generic configuration frameworks, generic subprocess machinery, or new identifier/retry/storage abstractions. Atomic replacement is a generic operation under the reduced contract, but a small local helper is preferable to a dependency that does not support Narratio's full platform set.

Findings In Source Order

H01 — Repository footprint

Verdict: Already satisfied; size alone is not a problem.

Recommended change: None based on aggregate line count.

Reasoning: The measured tree contains 36,107 production Go lines and 45,132 test Go lines across 26 packages. The implementation owns substantially more than command wrapping, and the observed size maps to documented responsibilities.

H02 — Trusted-environment threat model

Verdict: Confirmed as a refactoring constraint.

Recommended change: Preserve root-scoped destructive and restore operations and the checks needed to avoid accidental path escape. Remove tests and identity rechecks whose only purpose is to defeat a concurrent adversary, and stop applying universal no-follow and regular-file rules to ordinary reads, writes, and copies. Update the owning architecture, operations, and internal documents when the policy changes.

Reasoning: Narratio operates on trusted D&D campaign workspaces under a session lock; their contents are intended to be shared and are not sensitive. The application still needs safe destructive scope and special protection for API keys, but it does not need to prove inode identity against hostile races or reject ordinary workspace symlinks by default. os.Root remains useful at the few boundaries where containment is itself the requirement.

H03 — Production-code concentration

Verdict: Not substantiated as a package-boundary problem.

Recommended change: Keep internal/stage, internal/app, and internal/config intact; use file- and function-level decomposition described below.

Reasoning: Their measured 8,340, 7,869, and 5,542 lines correspond to three coherent owners: pipeline stages, application lifecycle/commands, and strict configuration. Splitting by size would add cross-package contracts without improving dependency direction.

H04 — Production files over 1,000 lines

Verdict: Confirmed selectively.

Recommended change: Split runner.go, publish.go, and validate.go along the seams identified below. Refactor analyze.go functions without forcing a purely mechanical file-count target.

Reasoning: Several large files contain natural modules, but other large files remain cohesive. File size is useful here as a navigation and change-blast signal, not as a universal limit.

H05 — Application, stage, and artifact interaction

Verdict: Already satisfied at the architectural boundary.

Recommended change: Retain the current dependency direction. Reduce executePlan collaborator handling with a private execution context rather than moving stage or artifact policy across packages.

Reasoning: Production imports remain acyclic: the application composes stages and adapters; stages consume Narratio-owned configuration, manifest, artifact, and adapter contracts; artifact services do not choose stage policy. The high interaction count reflects the application boundary's intended role.

H06 — Confined-filesystem and directory-promotion complexity

Verdict: Confirmed more broadly than the preliminary hypothesis.

Recommended change: Replace the general confined-filesystem and directory- promotion subsystem with ordinary standard-library operations plus a few focused helpers for the retained policies in H21. Delete both directory-copy algorithms, their race hooks and identity rechecks, and the platform-specific no-replace and directory-sync machinery. Let Notarius write a validated staging bundle in the session workspace and rename that directory directly to its unique durable destination.

Reasoning: The duplicate implementation is real, but removing only the test-only half would preserve a much larger abstraction whose guarantees are not operational requirements. The session lock, trusted workspace, same- workspace staging location, and unique run destination remove the practical need for adversarial no-replace promotion. A direct rename also avoids the second full-tree copy that the current implementation performs.

H07 — Test code larger than production code

Verdict: Focused opportunity, not evidence of overtesting.

Recommended change: Preserve behavior and retained-invariant coverage. Delete tests for abandoned power-loss, hostile-race, universal no-follow, and directory-promotion guarantees along with the implementation they constrain. Add focused tests for the smaller atomic-file helper, bounded I/O, modes, root-scoped cleanup and restore, validated Notarius rename, and API-key handling. Also consolidate repeated fixture setup within individual packages and replace app-level mutable global function seams with invocation-scoped dependencies as related code is touched.

Reasoning: The suite makes extensive legitimate use of real temporary filesystems, fake object stores, and subprocess helpers. The clearest hygiene signals are a few very large test files, 11 mutable production function seams that are reset repeatedly by tests, and a filesystem suite that treats unneeded implementation guarantees as policy; the overall test-to-production ratio is not itself a defect.

H08 — Repository-wide tests and vet

Verdict: Confirmed counter-signal.

Recommended change: Keep both checks as required validation for every refactoring stage.

Reasoning: go test ./... and go vet ./... pass on the maintained baseline and on Go 1.27.0. This argues for incremental, behavior-preserving changes rather than a broad rewrite.

H09 — Acyclic package dependencies

Verdict: Confirmed counter-signal.

Recommended change: Preserve the existing direction and reject proposed extractions that create app/stage/artifact cycles.

Reasoning: go list resolves all 26 packages without a cycle, and focused import inspection matches the ownership policy.

H10 — Localized rather than systemic complexity

Verdict: Confirmed counter-signal.

Recommended change: Refactor only the named hotspots and their immediate helpers.

Reasoning: Most functions are modest. High scores cluster in explicit orchestration, semantic validation, compatibility, and filesystem invariants. There is no evidence for repository-wide abstraction or style replacement.

H11 — Small direct dependency set

Verdict: Confirmed counter-signal.

Recommended change: Add only golang.org/x/sync/errgroup and selectively github.com/google/go-cmp/cmp if the planned changes use them; keep other candidates out.

Reasoning: Direct dependencies are limited to the AWS SDK boundary, x/sys, and YAML. The small surface is consistent with the architecture policy and raises the bar for dependencies that merely replace a few lines.

H12 — Structured documentation ownership

Verdict: Confirmed counter-signal.

Recommended change: Update the existing owner document when a refactor changes mechanics; do not add another overlapping architecture narrative.

Reasoning: Policy, operator, integration, and internal implementation documents have explicit ownership and reading paths. The relevant invariants were readily discoverable.

H13 — Automated coverage by volume

Verdict: Confirmed, with a qualification.

Recommended change: Judge future test changes by behavior and risk rather than preserving a line ratio. Add focused restore-symlink coverage and transition parity coverage for the recommended changes.

Reasoning: There are 164 test files and 45,132 test lines. A focused go test -cover ./... run reported roughly 74–90% statement coverage for most stateful production packages (with internal/fileops lower at 59.6%), but the policy correctly treats behavioral risk—not a repository-wide percentage—as the objective. Existing tests are strongest around lifecycle, recovery, filesystem, and adapter boundaries.

H14 — Cohesion of app, stage, and config

Verdict: Already satisfied at package level; focused internal work remains.

Recommended change: Keep package ownership stable. Make application lifecycle phases, analyze transactions, and validation domains explicit through private types and files.

Reasoning: Moving commands, stages, or validation into new packages would mostly expose private data and increase interfaces. The internal seams are real, but they do not require new architectural layers.

H15 — Natural modules hidden in large files

Verdict: Confirmed.

Recommended change: Use these existing seams: runner initialization, composition, stage transition, and finalization; publish source collection, transfer preparation, immutable upload, and current-pointer commit; adapter and artifact validation domains; modern and legacy previous-cache selection.

Reasoning: The functions and types already cluster around these concerns. Extraction can therefore be primarily mechanical and contract-preserving.

H16 — Duplication in stages and external adapters

Verdict: Not substantiated as a broad problem.

Recommended change: Keep tool-specific validation, request construction, and error context local. Consolidate only exact mechanics with a stable shared contract; do not add base-stage or base-adapter types.

Reasoning: Adapters already share internal/adapters/subprocess, and stage helpers already own run layout, materialization, and transcript validation. Remaining similarities are mostly small wrappers or deliberately different tool contracts. The meaningful duplicate found by this audit is in fileops, not the stage/adapter boundary.

H17 — Durable versus implementation-coupled tests

Verdict: Mostly healthy, with focused opportunities.

Recommended change: Retain behavior-level manifest, subprocess, recovery, and filesystem tests for the reduced contract. Delete tests tied to abandoned fileops mechanisms and guarantees; use a small dependency struct instead of mutable app globals; use structural diffs for large values where useful.

Reasoning: Most tests assert outputs, persisted state, CLI behavior, or external command contracts. The main incidental coupling comes from private hook algorithms, tests that elevate defensive implementation choices into contracts, and package-global function substitution, not from the suite as a whole.

H18 — Invariant discoverability across packages

Verdict: Confirmed for filesystem policy; otherwise already satisfied.

Recommended change: Revise the architecture, operations, and internal filesystem documents so they distinguish required outcomes from the current implementation's power-loss, no-follow, identity-race, and no-replace mechanisms. Fix restore target inspection as described in H46. Keep transaction- order comments adjacent to runner and publish code.

Reasoning: The invariants are discoverable, but the filesystem documents currently make several defensive mechanisms appear to be permanent product contracts. Restore planning's use of os.Stat is also inconsistent with the root-scoped restore rule that remains justified.

H19 — Upgrade Go 1.25 to Go 1.27

Verdict: Confirmed.

Recommended change: In one dedicated change, update go.mod, all Woodpecker images including the pinned release image, release documentation, and generated module state. Use the Go 1.27 os.Root.RemoveAll operation where root-scoped recursive removal remains required. Run the full test, race, vet, documentation, example, and six-target release-build checks required by the release procedure.

Reasoning: Go 1.27.0 is now released. The full suite and vet pass under it, and all supported release targets cross-compile successfully. A dedicated upgrade keeps any dependency or tool behavior changes separate from refactors.

H20 — Replace custom confinement with os.Root

Verdict: Focused opportunity, not a package-wide filesystem model.

Recommended change: Use os.Root only where a root-relative boundary is an actual requirement: destructive cleanup, restore, and any other explicitly scoped operation. Prefer direct os and io operations for ordinary trusted- workspace reads, writes, copies, and renames. Do not preserve a universal confined-filesystem facade merely because its current implementation uses os.Root.

Reasoning: os.Root is an idiomatic way to express scope, particularly with RemoveAll available in Go 1.27. Applying it to every filesystem operation, however, preserves no-follow and handle-relative complexity that Narratio's trusted workspace model does not require.

H21 — Retain Narratio-owned durability and policy code

Verdict: Not substantiated as stated; retain only focused product policies.

Recommended change: Keep atomic visibility for corruption-sensitive file replacement, centralized bounded-I/O limits and clear adapter errors, group- owned workspace modes, validated downloads before installation, root-scoped destructive and restore operations, and stricter treatment of API-key material. Drop file and directory syncing, power-loss durability claims, platform- unsupported directory-sync errors, atomic no-replace directory promotion, universal regular-file/no-follow restrictions, and hostile-race identity checks.

Reasoning: Most Narratio data is intentionally shareable D&D material, and the application runs under a session lock in a trusted workspace without heavy write concurrency. Atomic replacement still prevents readers from observing a partially written manifest or configuration, and bounded I/O still prevents a runaway adapter from exhausting disk or memory. The stronger guarantees do not address a realistic product risk proportionate to their implementation and test cost. In this context, “durable” should mean managed persistent state, not a promise of survival across every acknowledged power-loss window.

H22 — Use os.Process.WithHandle on Windows

Verdict: Confirmed after the Go baseline upgrade.

Recommended change: Assign the suspended Windows child to the owned job inside cmd.Process.WithHandle, removing the PID-based OpenProcess and its separate close. Retain suspended creation, job configuration, initial-thread resumption, failure cleanup, and job disposal.

Reasoning: WithHandle supplies a lifetime-stable process handle and eliminates the PID reopen race. It does not replace the other job-object lifecycle responsibilities.

H23 — Use slices and maps

Verdict: Focused opportunity.

Recommended change: Replace plain shallow clones, membership checks, and sorted key extraction with standard helpers when nil/empty and ordering semantics remain identical. Do not replace topological ordering, domain normalization, deep copies, or provenance-preserving merges.

Reasoning: The codebase contains many simple map-key/sort and slice-clone patterns, but many similarly shaped helpers encode real domain semantics. This is suitable for opportunistic cleanup with tests, not a bulk rewrite.

H24 — Use errgroup for transcription

Verdict: Confirmed.

Recommended change: Use errgroup.WithContext with SetLimit to run one goroutine per prepared audio job. Return speaker-qualified errors from each job, lock only successful aggregation, and preserve deterministic output ordering and the useful parent-cancellation diagnostic.

Reasoning: The current worker pool manually owns a job channel, wait group, cancel function, first-error lock, and three repeated error paths. errgroup directly provides the intended bounded cancellation and first-error behavior, while result aggregation remains Narratio-specific.

H25 — Use go-cmp selectively

Verdict: Focused opportunity.

Recommended change: Adopt cmp.Diff in tests comparing large composed configuration, analyze-state projections, restore plans, and publish mappings. Use slices.Equal or direct comparisons for simple values; do not migrate all 107 reflect.DeepEqual uses mechanically.

Reasoning: Large structural mismatches currently provide poor diagnostics, but small comparisons do not justify a new idiom or options boilerplate.

H26 — Adopt gofrs/flock

Verdict: Not substantiated.

Recommended change: Retain the current Unix and Windows lock primitives and context polling. Reconsider only if lock modes or platforms expand materially.

Reasoning: The platform-specific operations are very small. Narratio also owns confined creation, group-writable mode, metadata, held file lifetime, and error adaptation; an external flock object would not remove those obligations.

H27 — Adopt Cobra

Verdict: Deferred; not currently substantiated.

Recommended change: Keep flag and the existing shared parsers. Re-evaluate only if another round of nested commands produces demonstrated help, completion, or flag-inheritance costs that outweigh compatibility risk.

Reasoning: The command surface has grown, but dispatch and common bounded-run and config flags are already centralized. A migration would touch extensively tested parsing, help, error, and output behavior without addressing a current correctness or maintenance blocker.

H28 — General configuration framework or schema validator

Verdict: Not substantiated.

Recommended change: Retain strict YAML decoding, presence-aware composition, source ownership, profile overlays, defaults, and semantic Go validation.

Reasoning: Most complexity is Narratio's field relationships and provenance, not generic schema shape. A framework would add a second representation while still requiring the existing semantic checks.

H29 — Generic atomic-file package

Verdict: Focused opportunity, but no new dependency is presently justified.

Recommended change: Treat same-directory temporary-file replacement as a generic operation and expose only a small local helper for it. Localize any unavoidable platform-specific replace mechanic behind that helper. Use direct standard-library operations for everything else instead of retaining a broad fileops facade.

Reasoning: Once file and directory syncing, hostile-race defenses, and universal confinement are removed from the contract, atomic file replacement is no longer Narratio-specific. A thin local implementation removes more complexity than a general framework while keeping the supported-platform and group-mode behavior explicit. Dependency research may revisit that choice if a maintained package demonstrably removes more code than it adds.

H30 — Provider-neutral object-storage abstraction

Verdict: Already satisfied internally; no external abstraction is needed.

Recommended change: Keep the narrow Narratio-owned storage.ObjectStore and ReaderUploader interfaces, with AWS SDK types confined to the S3 adapter.

Reasoning: The boundary already expresses the required list/read/upload and conditional-write semantics without inferring session policy. A generic storage framework would be broader, not simpler.

H31 — Retry or backoff package

Verdict: Not substantiated.

Recommended change: Keep the explicit fixed-delay WhisperX retry loop and the AWS SDK's provider-owned retry behavior.

Reasoning: Narratio has one small application retry policy with specific status, cancellation, attempt, and error semantics. A general policy library would add configuration and indirection without removing domain decisions.

H32 — UUID or ULID package

Verdict: Not substantiated.

Recommended change: Retain the timestamp plus cryptographic-random suffix run ID and its injected test seam.

Reasoning: The identifier has a documented, readable format and needs no UUID interoperability or ULID parsing/ordering contract. Its implementation is small and correctly uses crypto/rand.

H33 — Generic subprocess framework

Verdict: Not substantiated.

Recommended change: Retain internal/adapters/subprocess and tool-specific adapters.

Reasoning: Shared execution already owns redaction, bounded diagnostic capture, environment policy, timeouts, and cross-platform process-tree cleanup. Individual adapters correctly retain command construction and output contracts.

H34 — internal/app.executePlan

Verdict: Confirmed; highest-priority maintainability refactor after the restore correctness fix.

Recommended change: Introduce a private invocation runtime carrying the resolved identity, paths, stores, manifests, environment, and summary state. Extract five lifecycle operations: prepare and lock the invocation; create durable invocation state; compose required collaborators; execute one stage transition; and finalize/clean up. Keep lock acquisition/release and all manifest persistence ordering explicit in the top-level function.

Reasoning: The approximately 456-line function has changed in at least 53 commits and spans preflight, locking, manifest creation, dependency composition, resume, transitions, failure persistence, and cleanup. These are existing seams; making them explicit reduces change blast without redesigning the state machine.

H35 — Shared plan/execution transition model

Verdict: Confirmed narrowly.

Recommended change: Share a pure pre-execution decision value that combines recorded status, force, semantic/resume validation outcome, stale reason, and required invalidation. Share small manifest-transition helpers where planning and execution apply identical state. Keep execution persistence and actual stage outcomes separate, and add parity tests for run/skip, non-resumable, forced invalidation, self-skip, and changed-success cases.

Reasoning: Plan and executePlan use several shared helpers but still independently sequence stale marking and invalidation. A full transition engine would overfit because planning models outcomes while execution observes them; the common pre-execution decision is the stable seam.

H36 — analyzeStage.Run

Verdict: Focused opportunity; the preliminary description is partly stale.

Recommended change: Add a private execution accumulator for session and invocation records, outputs, logs, generated configs, reused artifacts, and metadata. Let Run retain setup, ordered execution, and final result ownership.

Reasoning: Planning, reconciliation, fingerprinting, input identity, and resume logic are already separated into dedicated files. Run still coordinates many parallel result collections, so an accumulator would make projection and final assembly easier to verify without another package.

H37 — executeAnalyzeArtifact

Verdict: Confirmed.

Recommended change: Split request/input preparation, adapter invocation, validated output materialization, and analyze-record/evidence construction into private helpers under one transaction wrapper. Replace the current ten parameters with a narrow execution context plus the artifact plan.

Reasoning: The 239-line function is cohesive as a transaction but has 10 parameters and mixes resolution, provenance aggregation, command preparation, filesystem validation, canonical installation, checksum/contract construction, and result projection. Those phases already have explicit data boundaries.

H38 — Aggregate complexity of config/validate.go

Verdict: Confirmed as a navigation and ownership issue, not a validation design failure.

Recommended change: Split the file within package config into core pipeline/session validation, external-adapter validation, publish/storage validation, and Scriptorium artifact/dependency validation. Preserve validation order and exact field-specific errors.

Reasoning: The file is 1,111 lines and has changed in at least 48 commits. Its guards are mostly necessary, but unrelated configuration domains sharing one file increases conflict and review scope.

H39 — validateScriptorium

Verdict: Confirmed.

Recommended change: Extract artifact declaration/output validation, dependency/input validation, and variable validation into helpers. Sort artifact keys before validation so error order does not depend on Go map iteration.

Reasoning: The function validates several distinct artifact-level contracts and accumulates cross-artifact state. The dependency graph already has a separate validator, demonstrating the natural boundary.

H40 — validateNotarius

Verdict: Confirmed narrowly.

Recommended change: Extract reference-binding normalization and output-lane normalization/collision checking into separate helpers, leaving enablement and top-level scalar checks in validateNotarius.

Reasoning: The function now has clear reference and output phases with different collision domains. Separating them improves reviewability while preserving the normalization mutation contract.

H41 — validateAudita

Verdict: Not substantiated as a function-level refactor.

Recommended change: Move it with the other adapter validators during the file split, but keep the function intact.

Reasoning: Its 54 lines are a straightforward sequence of independent, field-specific guards. Extracting each switch or numeric check would add names without reducing conceptual load.

H42 — splitPipelineCompositionEnvelope

Verdict: Focused opportunity.

Recommended change: Separate envelope shape decoding from cross-field semantic validation and root-metadata removal. Reuse the existing profile declaration parser and preserve every source-aware diagnostic.

Reasoning: The function still combines mapping/list/scalar decoding, unknown-field checks, profile parsing, default-profile membership, cloning, and root mutation. These are separable phases, but provenance makes a generic YAML decoder inappropriate.

H43 — previouscache.Resolve

Verdict: Confirmed.

Recommended change: Extract committed-artifact selection and legacy candidate probing into separate functions returning one common selected-object value. Keep required/optional policy, local record construction, and final sorting in Resolve. Mark the legacy helper as compatibility-only and removable after migration.

Reasoning: Immutable commits and legacy existence probing are distinct algorithms currently interleaved in the requirement loop. Isolating the legacy surface makes both paths easier to test and eventually delete.

H44 — internal/stage/publish.go

Verdict: Confirmed as a file-level decomposition.

Recommended change: Split source discovery/opening, output rule resolution, transfer preparation and verification, and commit/pointer encoding into focused files. Keep publishStage.Run as the visible transaction and preserve the exact ordering: immutable objects, immutable commit, lock revalidation, then one final current-pointer write.

Reasoning: The 1,175-line file already consists of cohesive helper groups. The main risk is obscuring the commit boundary, so decomposition should improve navigation without distributing transaction ownership.

H45 — internal/fileops/directory.go

Verdict: Confirmed for removal; see H06 and H21.

Recommended change: Delete both directory-promotion implementations, their test hooks, copy helpers, identity checks, directory syncing, and platform- specific no-replace rename code. Have extract validate the Notarius staging bundle and use a direct same-workspace os.Rename to its absent, unique run destination. Treat a destination collision as an ordinary error and clean up only verified staging paths.

Reasoning: The duplicate implementation is the clearest literal duplication found, but the production implementation is also unnecessary under the revised contract. Notarius already writes beneath the invocation output root, and the destination is protected by the session lock and unique run ID. Direct rename removes an entire tree copy and the need to preserve a second pre-promotion copy; the receipt and diagnostic log remain the appropriate troubleshooting record.

H46 — classifyRestoreAction

Verdict: Confirmed for correctness, not merely complexity.

Recommended change: Fold classification into the simpler root-scoped restore implementation. Resolve and inspect targets relative to an os.Root, without layering inode-identity rechecks onto the operation. Classify a symlink at the leaf or in an ancestor as RestoreConflictNonRegular, even when its target bytes match. Hash a verified regular target without reopening an unconfined absolute path. Add leaf- and ancestor-symlink tests for force and non-force plans. Optionally extract the legacy remote-download digest comparison after this behavior is fixed.

Reasoning: os.Stat(localPath) follows a leaf symlink, and artifacts.SHA256File(localPath) follows it again. A matching target can therefore produce skip_same and leave an unmanaged symlink in place, contrary to the retained restore-scope contract. This is an accidental-scope issue, not a reason to retain hostile-race defenses throughout ordinary file operations. The rest of the function is an explicit and readable state matrix whose branching is justified.

H47 — internal/manifest/analyze_state.go

Verdict: Retain.

Recommended change: Keep validation together with the state model. Apply only opportunistic slices.Clone cleanup where semantics are unchanged.

Reasoning: The file is organized into small validators for record status, current output, dependencies, fingerprints, provenance, bounds, normalization, and deep cloning. Its complexity directly expresses durable state invariants and is easier to audit in one owner.

H48 — Preliminary priority order

Verdict: Confirmed with a broader filesystem correction.

Recommended change: Define the reduced filesystem policy and replace the current fileops subsystem, incorporating the restore classification correction into the root-scoped restore design. Next decompose execution and share its narrow planning decision model. Follow with analyze/config, publish/previous- cache, and finally modernization/test ergonomics.

Reasoning: The restore mismatch remains a correctness issue, but preserving the surrounding abstraction solely to fix it would reinforce the mistaken contract. Execution remains the largest non-filesystem change-blast hotspot, and configuration/analyze remain the next tier.

This is ordering guidance for a future decision-complete implementation plan, not staged implementation instructions:

  1. Define the reduced filesystem policy in its canonical documents.
  2. Upgrade the Go baseline to 1.27 in isolation and run the full release candidate checks.
  3. Replace the broad fileops facade with direct standard-library operations and focused helpers, simplify extract to validated directory rename, incorporate the root-scoped restore correction, and replace obsolete mechanism tests with tests of the retained outcomes.
  4. Adopt os.Process.WithHandle and simplify transcription with errgroup as separate, bounded modernization changes.
  5. Decompose executePlan, then introduce the shared narrow pre-execution decision model with parity tests.
  6. Refactor analyze orchestration and the per-artifact transaction.
  7. Split configuration validation and composition-envelope responsibilities.
  8. Split previous-cache compatibility resolution and publish file concerns.
  9. Improve app test dependency injection and apply selective slices, maps, and go-cmp cleanup only in code touched by the preceding work.

The filesystem decisions are specified in fileops.md. No open product or architecture decision is required before turning these findings into an implementation plan. The implementation plan should preserve the scope boundaries and rejected alternatives recorded here.