narratio/docs/roadmap/implementation.md

Workstream 10 Implementation Plan — Quality, Testability, And Documentation Alignment

Status

Planned implementation for Workstream 10, Quality, testability, and documentation alignment, in refactoring.md. Workstreams 1 through 9 are complete prerequisites. This is the final actionable workstream in the consolidated refactoring roadmap; it closes cross-cutting gaps that were not already completed alongside the behavior-owning workstreams.

Authoritative Scope And Fixed Decisions

Workstream 10 in refactoring.md is authoritative. Its supporting first-pass findings are H07, H08, H12, H13, H17, H18, H23, and H25 in audit.md. The corresponding contract decisions are C01 through C06 in audit-decisions.md; their full rationale remains in contract-audit.md. Read those cited sections rather than re-auditing unrelated hypotheses.

Most of Workstream 10's risk coverage and documentation alignment landed with Workstreams 1 through 9. Preserve that completed work. The remaining decisions are:

• Do not reopen completed product, compatibility, filesystem, manifest, analyze, configuration, publication, lock, or cleanup decisions. This workstream changes internal testability and test diagnostics without changing CLI, configuration, artifact, manifest, integration, publish, restore, or cleanup behavior.
• Remove the seven remaining mutable application function seams: readSecretsDirectoryEntries, loadPipelineConfigFn, downloadObjectToTempFn, newObjectStoreFromConfigFn, discoverRemoteCurrentStateFn, buildRestorePlanFn, and executeRestorePlanFn. Compiled regular-expression values, canonical stage relations, and fixed command-name data are not collaborator seams and are not part of this removal.
• Use one package-private, value-owned commandDependencies set for these collaborators. A normalization function supplies production defaults for nil fields. Each command invocation receives its own value; there is no mutable default singleton, context-carried service locator, dependency-injection framework, or new public interface. Keep runner invocationDependencies separate because it owns a different lifecycle.
• Preserve exported command entry points and their existing signatures. Production wrappers use normalized defaults. Private dependency-aware entry points let tests supply only the external or phase boundary they control. Thread the dependency value explicitly through application-owned command, configuration-loading, secret-loading, object-store, and restore helpers.
• Tests must never replace or restore package-level functions. Use dependency values, real temporary filesystems, storage.FakeBackend, and existing controlled subprocess helpers at their durable boundaries. Process environment mutation remains permitted where credential behavior itself is under test, but such tests must remain non-parallel and restore state through t.Setenv or the existing focused helper.
• Add github.com/google/go-cmp/cmp only for the audit's selected large structural comparisons: composed configuration values, analyze plans/state, restore plans/actions, and publish output plans/mappings. Preserve nil versus empty and ordering semantics unless a domain contract explicitly equates them. Use direct comparison or slices.Equal for simple comparable values; do not mechanically replace every reflect.DeepEqual call.
• Apply slices or maps in production only when code touched by this workstream performs a genuinely shallow clone, membership check, or deterministic key operation with identical nil, empty, and ordering behavior. Do not conduct a repository-wide style rewrite and do not replace domain normalization, deep copies, provenance merges, or topological order.
• Retain the focused behavior coverage already established for atomic file visibility, bounded reads and adapter diagnostics, group modes, validated downloads, Notarius rename, destructive confinement, restore symlink conflicts, secret protection, session-only progress, per-artifact analyze checkpoints, planning/execution parity, remote-lock carry-forward, and idempotent cleanup. Delete only dead helpers and reset choreography associated with the removed mutable seams; do not delete a current-contract test merely to reduce test lines.
• Update only canonical internal documentation whose implemented dependency or test boundary changes. Because public behavior is unchanged, do not churn CLI, configuration, operations, troubleshooting, integration, examples, or release documentation unless implementation uncovers a factual inaccuracy in its owning document.

Instructions For Every Stage

Before each stage:

1. Read docs/development.md, all documents under docs/policy/, this plan, Workstream 10 in refactoring.md, and only its cited audit sections. Read ../internal/overview.md and, for restore changes, ../internal/command-restore.md.
2. Inspect the affected production code and focused tests. Prefer the codebase knowledge graph for Go definitions and call paths; use text search for mutable assignments, test reset choreography, documentation, fixtures, and final removal checks.
3. Confirm worktree state and preserve unrelated changes. Do not implement release work or any deferred/no-action audit hypothesis.

During each stage, keep dependencies package-private and invocation-owned, preserve acyclic package direction, keep default tests deterministic and offline, and retain credential redaction and filesystem safety. Prefer behavior assertions over collaborator choreography. Run gofmt on changed Go files, go mod tidy when module state changes, and git diff --check. Commit only a completed stage with a short plain-English message, and update a canonical internal document in the same stage only when its owned behavior changes.

Stage 1 — Replace Configuration And Secret-Loading Globals

Status: Completed

Goal

Establish the command-scoped dependency value and remove global substitution from configuration and API-key loading.

Required Work

1. Add a package-private commandDependencies type in internal/app with typed function fields for pipeline loading, temporary object download, secrets-directory reading, object-store construction, remote-current discovery, restore-plan construction, and restore-plan execution. Add a value-returning normalizer that fills nil fields with the current production functions. Do not store a normalized instance in a package variable.
2. Add dependency-aware private variants of the configuration-loading path: loadCommandConfig, loadPipelineCampaignConfig, and loadPipelineConfig should receive or delegate to an explicit dependency value. Preserve local session precedence, remote fallback, source provenance, bounded temporary download behavior, and idempotent temporary-file cleanup.
3. Add a dependency-aware private secret loader used by focused tests and by later command composition. Production callers continue through a wrapper using normalized defaults. Preserve directory/file privacy checks, symlink rejection, bounded reads, existing-environment precedence, redaction, and the ancestor-replacement failure case.
4. Remove loadPipelineConfigFn, downloadObjectToTempFn, and readSecretsDirectoryEntries. Rewrite config_loader_test.go, loader-focused cases in remote_session_test.go, and secrets_env_test.go to pass copied dependency values instead of saving, replacing, and restoring globals. Leave command-routing remote-session cases for the command stage that owns their entry point. Retain tests at the narrowest existing behavior boundary; do not reproduce the loader's internal call sequence with mocks.
5. Keep production command behavior unchanged while this foundation is introduced. It is acceptable for the object-store and restore fields to use their production defaults until Stages 2 through 4 migrate their callers.

Verification And Exit Criteria

Run:

go test ./internal/app -run 'Config|RemoteSession|Secret'
go test -race ./internal/app -run 'Config|RemoteSession|Secret'
git diff --check

The three named mutable globals are absent, remote/local configuration behavior and secret protections remain covered, and tests supply collaborators through local dependency values.

Stage 2 — Carry Dependencies Through Pipeline Commands

Status: Completed

Goal

Carry one dependency value through the pipeline-oriented command paths, including remote session fallback, without changing their public API.

Required Work

1. Add a private dependency-aware execution dispatcher alongside Execute. Execute retains its signature and uses normalized production dependencies. Extend the existing private pipeline-executor path rather than creating a second command switch or an application framework.
2. Thread commandDependencies through run, regenerate-artifacts, run-stage, analyze, publish, plan, and status paths and the shared configuration loader they use. Preserve the independent pipelineExecutor seam and pass both values explicitly where required.
3. Make remote session fallback initialize its object store through the dependency value and preserve the rule that secrets load before the factory. Public command wrappers continue to use production defaults.
4. Rewrite the assembled pipeline-command tests and only those remote-session tests reached through pipeline commands to call the dependency-aware dispatcher with a fake store. Remove their global object-store replacement/reset choreography. Leave remote-session cases reached through session commands for Stage 3. It is acceptable for the factory global itself to remain temporarily for restore and operator callers until Stages 3 and 4 migrate them.
5. Preserve command syntax, exit classification, stdout/stderr behavior, configuration discovery, artifact selection, executor injection, and remote session provenance. Do not move adapter construction into configuration or storage packages.

Verification And Exit Criteria

Run:

go test ./internal/app -run 'Run|Stage|Analyze|Publish|Plan|Status|RemoteSession|Command'
go test -race ./internal/app -run 'Run|Stage|Analyze|Publish|Plan|Status|RemoteSession'
git diff --check

Pipeline commands use invocation-owned dependencies, production defaults still select the real storage constructor, and focused tests remain offline.

Stage 3 — Carry Dependencies Through Operator Commands

Status: Completed

Goal

Migrate session and maintenance commands to the same invocation-owned command dependency path before removing the final shared storage seam.

Required Work

1. Thread commandDependencies through the session dispatcher, session initialization and validation, artifact listing, lock listing/add/remove, and session-scoped or cache-aware clean helpers. Exported command functions remain production-default wrappers.
2. Update the dependency-aware top-level dispatcher so every operator command reached through Execute receives the invocation's value. Keep local-only paths independent of remote construction; do not initialize an object store merely because a dependency value exists.
3. Rewrite object_store_test.go, operator_helpers_test.go, affected session/clean tests, and remote-session cases reached through the session dispatcher to supply a local object-store factory. Keep complete CLI routing tests at the private dispatcher and narrow construction-order tests at newCommandObjectStore. Remove original-value capture and reset boilerplate in the migrated tests.
4. Preserve secret-before-factory ordering, local/remote session initialization, validation findings, artifact availability, per-source remote lock behavior, cleanup scope, output, and errors. Continue using storage.FakeBackend for remote behavior.
5. Leave only restore callers on the temporary global storage seam. Do not remove it until Stage 4 can update restore and its tests atomically.

Verification And Exit Criteria

Run:

go test ./internal/app -run 'ObjectStore|SessionInit|SessionValidate|Artifacts|Locks|Clean|Operator'
go test -race ./internal/app -run 'ObjectStore|SessionInit|SessionValidate|Artifacts|Locks|Clean'
git diff --check

All non-restore commands use invocation-owned dependencies, their tests mutate no factory global, and remote behavior remains deterministic and offline.

Stage 4 — Replace Restore Phase Globals With Explicit Dependencies

Status: Completed

Goal

Make restore sequencing testable per invocation without global phase hooks or a new restore framework.

Required Work

1. Keep Restore as the public production wrapper and add one private dependency-aware restore entry point. Normalize dependencies once at that boundary and use its object-store constructor, remote-current discovery, plan builder, and plan executor throughout the invocation.
2. Remove newObjectStoreFromConfigFn, discoverRemoteCurrentStateFn, buildRestorePlanFn, and executeRestorePlanFn. Call the concrete production functions through the normalized dependency value; do not add interfaces around pure phase functions or move restore orchestration out of internal/app.
3. Rewrite restore_test.go, restore_execution_test.go, and restore_recovery_test.go to pass local dependencies. Consolidate repeated dependency construction into one small test helper that returns a fresh value and never mutates package state.
4. Preserve the observable restore boundaries: dry-run performs no local installation; non-dry-run planning occurs while holding the session lock; conflicts do not execute; the incomplete marker brackets installation; reports reflect failure or success; manifest installation remains last; and lock release errors remain joined with an earlier failure.
5. Prefer real buildRestorePlan and executeRestorePlan with storage.FakeBackend where the test is about restore results or filesystem state. Stub a phase only when the phase call or its timing is the behavior being tested, such as proving the transition lock is already held.
6. Update docs/internal/command-restore.md only if its description of phase composition or test seams is no longer accurate. Do not change the operator restore contract.

Verification And Exit Criteria

Run:

go test ./internal/app -run 'Restore'
go test -race -shuffle=on -count=3 ./internal/app -run 'Restore'
git diff --check

No command has a mutable production function seam, restore transaction ordering and recovery outcomes remain covered, and restore tests can execute independently without save/restore choreography.

Stage 5 — Improve Selected Large Structural Test Diagnostics

Status: Pending

Goal

Make failures in the four audit-selected large-value test categories readable without imposing a repository-wide assertion style.

Required Work

1. Add github.com/google/go-cmp/cmp as a direct test dependency and update go.mod and go.sum with the Go toolchain. Do not add an assertion framework or helper wrapper around cmp.Diff.
2. Replace existing whole-value reflect.DeepEqual assertions with cmp.Diff where a field-level diff is materially useful in these files:
   • composed/effective configuration comparisons in internal/config/pipeline_composition_test.go and internal/config/pipeline_profiles_test.go;
   • complete analyze plans, artifact-state maps, or stage records in internal/stage/analyze_plan_test.go, internal/stage/analyze_reconciliation_test.go, and internal/app/analyze_checkpoint_recovery_test.go;
   • restore action collections or complete plans in internal/app/restore_plan_test.go; and
   • complete output outcomes, declarations, or mappings in internal/stage/publish_output_plan_test.go.
3. Keep direct field assertions that intentionally isolate one invariant. Use slices.Equal for simple ordered slices in the touched files where it is clearer; retain reflect.DeepEqual when its value is small and the mismatch is already obvious. Do not expand this stage to every test package.
4. Do not use cmpopts.EquateEmpty, cmp.AllowUnexported, blanket ignore options, or custom comparers merely to force a passing comparison. Compare the same observable values as before and preserve distinctions in nil, empty, order, timestamps, and provenance.
5. Run go mod tidy and confirm the dependency is used only by tests. Apply slices and maps to production touched in Stages 1–4 only where the fixed decisions above permit an identical shallow operation; do not manufacture a production change to demonstrate their use.

Verification And Exit Criteria

Run:

go mod tidy
go test ./internal/config ./internal/stage ./internal/app
go test -race ./internal/config ./internal/stage ./internal/app
git diff --check

The selected large comparisons emit useful structural diffs, simple assertions remain simple, module state is tidy, and no broad assertion-style migration has occurred.

Stage 6 — Verify Coverage, Align Internal Documentation, And Close The Roadmap

Status: Pending

Goal

Prove that the reduced test and documentation system protects the final refactored architecture, then close Workstream 10 and the consolidated roadmap.

Required Work

1. Search production and tests for all seven retired function-seam names and for assignments that replace application functions. Remove any remaining dead wrappers, original-value captures, reset helpers, or cleanup callbacks whose sole purpose was global substitution. Do not remove compiled regexes, canonical data tables, or legitimate local function values.
2. Review the existing focused owners—not the entire suite indiscriminately—to confirm retained coverage for:
   • atomic visibility and bounded reads in internal/fileops and storage adapter tests;
   • POSIX group modes in internal/fileops/modes_posix_test.go;
   • validated storage downloads and Notarius promotion in storage adapter and extract tests;
   • destructive confinement and restore symlink conflicts in fileops, cleanup, and restore tests;
   • credential privacy and redaction in secrets, logging, subprocess, and adapter tests;
   • session-only progress and per-artifact recovery in manifest, stage, and application checkpoint tests;
   • planning/execution parity in internal/app/run_control_test.go;
   • remote-lock carry-forward in publish planning/execution tests; and
   • idempotent final cleanup in stage and application cleanup tests. Add a test only if a listed consequential boundary is genuinely absent; consolidate or delete only demonstrably redundant mechanism coverage.
3. Review docs/policy/architecture.md, docs/internal/overview.md, focused internal component documents, and their test inventories for current accuracy. Remove stale implementation claims and duplicate volatile detail; retain links to the canonical CLI, configuration, operations, integration, and release owners rather than copying those contracts.
4. Confirm no current-behavior document treats a retired compatibility path, run manifest, cleanup obligation, central lock manifest, custom directory promotion, or mutable test hook as supported behavior. Historical roadmap and audit records remain unchanged.
5. Run the complete validation matrix below. Use a fresh temporary directory for the release-asset build and remove it afterward; do not create a release note, tag, or publication. Record native-platform limitations exactly as in docs/development.md rather than treating cross-compilation as runtime evidence.
6. Only after every check passes, mark Stages 1–6 in this plan and Workstream 10 in refactoring.md Complete. Confirm every numbered workstream is then complete. Do not delete the audits, roadmap, or implementation plan and do not prepare or publish a release.

Verification And Exit Criteria

At minimum run:

go mod tidy -diff
test -z "$(gofmt -l $(git ls-files '*.go'))"
go test -count=1 ./...
go test -race -count=1 ./...
go test -race -shuffle=on -count=3 ./...
go vet ./...
go build ./...
go test -count=1 ./internal/doccheck
go test -count=1 ./internal/config -run '^TestExamplesLoadAndValidate$'
git diff --check

Also run scripts/build-release-assets.sh v0.0.0-test <fresh-absolute-temp-dir> and delete only that verified temporary directory afterward. The workstream is complete when application collaborators are invocation-scoped, selected large test failures are diagnostic, retained high-risk behavior is credibly covered, canonical documentation describes the implemented reduced contracts, all ten workstreams are complete, and the worktree contains no release artifact.

Open Questions

None. The audits, completed prerequisite workstreams, and fixed decisions above make the remaining quality and documentation work decision-complete.