From 5fb454a714e6e0324b9fc1b773a691c24ea7ff2d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 5 Sep 2026 19:18:48 +0000 Subject: [PATCH] Remove completed roadmap and audit documents --- docs/roadmap/audit-decisions.md | 148 ------- docs/roadmap/audit.md | 764 -------------------------------- docs/roadmap/contract-audit.md | 525 ---------------------- docs/roadmap/fileops.md | 273 ------------ docs/roadmap/implementation.md | 431 ------------------ docs/roadmap/library-audit.md | 395 ----------------- docs/roadmap/potential-audit.md | 224 ---------- docs/roadmap/refactoring.md | 579 ------------------------ 8 files changed, 3339 deletions(-) delete mode 100644 docs/roadmap/audit-decisions.md delete mode 100644 docs/roadmap/audit.md delete mode 100644 docs/roadmap/contract-audit.md delete mode 100644 docs/roadmap/fileops.md delete mode 100644 docs/roadmap/implementation.md delete mode 100644 docs/roadmap/library-audit.md delete mode 100644 docs/roadmap/potential-audit.md delete mode 100644 docs/roadmap/refactoring.md diff --git a/docs/roadmap/audit-decisions.md b/docs/roadmap/audit-decisions.md deleted file mode 100644 index 940b330..0000000 --- a/docs/roadmap/audit-decisions.md +++ /dev/null @@ -1,148 +0,0 @@ -# Audit Decision Register - -## Status - -Complete. This document inventories the conclusions of Narratio's three -maintainability audit passes and records their final disposition. It is a -traceability register, not an implementation plan and not an additional audit. - -The source audits are: - -- the contract-preserving [Maintainability Audit](audit.md); -- the contract-relaxing [Contract Simplification Audit](contract-audit.md); and -- the [Reusable Library Extraction Audit](library-audit.md). - -The detailed [File Operations Simplification](fileops.md) roadmap supplies the -approved filesystem rationale and policy that refine the audit findings. The -consolidated [Refactoring Roadmap](refactoring.md) is the controlling inventory -of adopted work for the upcoming refactor. - -## Dispositions - -- **Immediate action:** approved work included in `refactoring.md`. -- **Constraint:** a rule governing immediate work, but not a separate code - change by itself. -- **Superseded:** a later approved decision replaces the earlier recommendation. -- **Conditional:** reconsider only when a stated future trigger occurs. -- **Deferred:** deliberately outside the upcoming refactor. -- **No action:** already satisfied, unsubstantiated, retained as-is, or merely - an observation. - -An immediate action can also carry a constraint. When findings overlap, the -contract audit controls product semantics, `fileops.md` resolves the detailed -filesystem policy, and the library audit controls extraction decisions. The -resulting immediate work is consolidated in `refactoring.md`. - -## Pass 1 — Contract-Preserving Maintainability Audit - -All identifiers in this table refer to sections of [`audit.md`](audit.md). - -| ID | Subject | Final disposition | Controlling decision and destination | -| --- | --- | --- | --- | -| H01 | Repository footprint | No action | Size alone was not a defect. | -| H02 | Trusted-environment threat model | Immediate action and constraint | Adopt the trusted, group-owned workspace model in `fileops.md`; retain secret handling and confined destructive scope. See `refactoring.md`, Filesystem. | -| H03 | Production-code concentration | No action | Do not split packages merely because they are large. | -| H04 | Production files over 1,000 lines | Immediate action | Decompose only the identified runner, analyze, configuration, publish, and previous-cache seams. See the corresponding `refactoring.md` workstreams. | -| H05 | Application, stage, and artifact interaction | Constraint | Preserve current package ownership and dependency direction. | -| H06 | Confined filesystem and directory promotion | Immediate action | Replace the broad subsystem under `fileops.md`; delete custom directory promotion. See Filesystem. | -| H07 | Test code larger than production | Immediate action | Remove tests only with retired contracts, consolidate focused fixtures, and replace mutable app globals as affected code is refactored. See Quality and testability. | -| H08 | Repository tests and vet | Constraint | Keep repository-wide tests and vet as required refactoring validation. | -| H09 | Acyclic package dependencies | Constraint | Preserve the existing acyclic dependency direction; no reorganization is required. | -| H10 | Localized complexity | Constraint | Limit work to audited hotspots and immediate collaborators. | -| H11 | Small dependency surface | Constraint | Add dependencies only where an adopted finding demonstrates net value. | -| H12 | Documentation ownership | Immediate action and constraint | Update canonical owners with each behavior change and avoid parallel narratives. See Quality and documentation. | -| H13 | Coverage by volume | Immediate action | Add risk-focused restore and transition coverage; do not preserve or target a test-line or coverage ratio. | -| H14 | Cohesion of app, stage, and config | Immediate action and constraint | Keep packages intact while introducing private lifecycle, transaction, and validation-domain seams. | -| H15 | Natural modules in large files | Immediate action | Use the audited internal seams for runner, analyze, configuration, publish, and current previous-cache logic. | -| H16 | Duplication in stages and adapters | No action | Do not introduce base-stage or base-adapter abstractions; keep tool contracts local. | -| H17 | Durable versus implementation-coupled tests | Immediate action | Preserve behavior tests, delete mechanism tests for retired contracts, use invocation-scoped dependencies, and improve large structural diffs selectively. | -| H18 | Invariant discoverability | Immediate action | Correct filesystem documentation and restore behavior; retain transaction-order comments near runner and publish ownership. | -| H19 | Go baseline upgrade | Immediate action, revised to Go 1.26 | The audit proposed Go 1.27, but the approved implementation baseline is Go 1.26: it provides `os.Process.WithHandle`, while `os.Root.RemoveAll` was already available in Go 1.25. Upgrade the module, CI/release images, module state, and release documentation in one bounded change. See Platform modernization. | -| H20 | `os.Root` confinement | Immediate action | Use `os.Root` only for genuinely root-scoped cleanup and restore; use ordinary filesystem operations elsewhere. See Filesystem. | -| H21 | Narratio-owned durability and policy | Superseded in part; immediate action | `fileops.md` retains atomic visibility, limits, modes, validated downloads, destructive scope, and secret protection while retiring stronger guarantees. | -| H22 | `os.Process.WithHandle` | Immediate action | Adopt after the Go baseline upgrade while preserving Windows Job Object lifecycle. See Platform modernization. | -| H23 | `slices` and `maps` | Immediate opportunistic action | Apply only in touched code where semantics are identical; no bulk rewrite. See Quality and testability. | -| H24 | `errgroup` for transcription | Immediate action | Replace the manual worker pool while preserving cancellation, qualified errors, and deterministic aggregation. See Platform modernization. | -| H25 | `go-cmp` in tests | Immediate opportunistic action | Use for selected large structural comparisons only. See Quality and testability. | -| H26 | Adopt `gofrs/flock` | No action | Retain the small existing platform lock primitives. | -| H27 | Adopt Cobra | Deferred | Reconsider only after demonstrated command/help/flag inheritance costs. | -| H28 | Generic configuration framework | No action | Retain typed strict decoding, composition, defaults, and semantic validation. | -| H29 | Generic atomic-file package | Immediate action with constraint | Keep a small local helper unless focused research finds a maintained dependency that removes more code while meeting platform and mode needs. See Filesystem. | -| H30 | Provider-neutral object storage | No action | The narrow internal storage ports already satisfy the requirement. | -| H31 | Retry or backoff package | No action | Retain the explicit WhisperX policy and provider-owned AWS retries. | -| H32 | UUID or ULID package | No action | Retain the existing readable cryptographically random run ID. | -| H33 | Generic subprocess framework | Superseded in part | The library audit retains the code internally now, requires local mechanism/policy separation, and makes external extraction conditional. See Platform modernization and L01. | -| H34 | `executePlan` | Immediate action | Decompose around invocation preparation, collaborator composition, one-stage transition, and finalization while keeping lock and persistence ordering explicit. See Runner lifecycle. | -| H35 | Shared planning/execution transition model | Immediate action | Introduce only the shared pure pre-execution decision and small identical transition helpers. See Runner lifecycle. | -| H36 | `analyzeStage.Run` | Superseded in part; immediate action | Contract Finding C03 removes invocation projections. Use a session-only execution accumulator or equivalent private state under the checkpointing model. See Analyze recovery. | -| H37 | `executeAnalyzeArtifact` | Immediate action | Split preparation, adapter invocation, validated materialization, and evidence construction under one transaction owner. See Analyze recovery. | -| H38 | `config/validate.go` | Superseded in part; immediate action | Split by semantic domain, but C06 removes exact validation order and full prose as compatibility requirements. See Configuration. | -| H39 | `validateScriptorium` | Immediate action | Separate declaration/output, dependency/input, and variable validation; retain deterministic semantics without promising diagnostic choreography. | -| H40 | `validateNotarius` | Immediate action | Separate reference binding from output-lane normalization and collision checks. | -| H41 | `validateAudita` | Immediate mechanical action | Move it with its adapter-validation domain but do not decompose the function. | -| H42 | Composition-envelope splitting | Immediate action | Separate shape decoding, cross-field semantics, and root metadata removal while preserving useful source-aware conflict diagnostics. | -| H43 | `previouscache.Resolve` | Superseded | C01 requires complete removal of legacy candidate probing rather than isolation. Simplify the remaining canonical path as part of the compatibility purge. | -| H44 | `stage/publish.go` | Immediate action | Decompose internal concerns while retaining one publish transaction; C05 controls the new lock and carry-forward semantics. See Remote publication. | -| H45 | `fileops/directory.go` | Immediate action | Delete directory promotion and replace it with validated same-workspace Notarius rename. See Filesystem. | -| H46 | Restore classification | Immediate correctness action | Classify leaf and ancestor symlinks as non-regular conflicts inside a root-scoped restore design. See Filesystem. | -| H47 | Manifest analyze-state validation | No action | Retain the cohesive state-model owner; only trivial clone cleanup is permitted when touched. | -| H48 | Preliminary priority order | Constraint | Use as dependency guidance for implementation planning; it creates no separate deliverable. | - -## Pass 2 — Contract Simplification Audit - -All identifiers in this table refer to findings of -[`contract-audit.md`](contract-audit.md). These approved decisions control any -conflicting recommendation from the first pass. - -| ID | Subject | Final disposition | Destination | -| --- | --- | --- | --- | -| C01 | Deprecated compatibility | Immediate action | Purge all named legacy configuration, party/player, remote-state, previous-cache, extraction, analyze, and manifest compatibility paths. See Canonical contracts. | -| C02 | Structured run manifests | Immediate action | Eliminate the run-manifest schema and lifecycle; keep the session manifest as the sole local progress authority. See Manifest authority. | -| C03 | Analyze recovery | Immediate action | Checkpoint each successful artifact in dependency order, retain valid successes across later failure, and resume failed, stale, or missing work. See Analyze recovery. | -| C04 | Cleanup hidden inside publish | Immediate action | Add cleanup as the final idempotent pipeline stage and remove post-publish cleanup obligations. See Cleanup stage. | -| C05 | Shared mutable remote locks | Immediate action | Replace the central/static lock model with per-source remote lock objects and carry prior locked declarations into new commits. See Remote publication. | -| C06 | Configuration diagnostic contracts | Immediate action and constraint | Preserve strict deterministic semantics and useful source-aware errors while retiring exact validation order, wording, punctuation, formatting, and incidental provenance. See Configuration. | - -The contract audit also records that the combined work remains eligible for a -minor release because the compatibility surfaces were already deprecated and -removable. Release execution is outside this refactoring roadmap. - -## Pass 3 — Reusable Library Extraction Audit - -Identifiers refer to [`library-audit.md`](library-audit.md). Candidate 5 is -expanded into its separately evaluated helper groups, and the audit's explicit -non-candidates are listed so the register is complete. - -| ID | Candidate | Final disposition | Controlling decision and destination | -| --- | --- | --- | --- | -| L01 | Cross-platform subprocess supervision | Conditional extraction; immediate internal cleanup | Keep internal until a real second consumer, overlapping multi-platform contract, alternative review, clean API, and maintainer exist. Separate process mechanism from Narratio policy locally and remove unrelated file writers. See Platform modernization. | -| L02 | Hardened FileOps | Immediate simplification; no extraction | Follow `fileops.md`; version control preserves reusable algorithms without making Narratio consume rejected guarantees. | -| L03 | YAML composition and provenance | No extraction | Keep Narratio's configuration DSL internal. Dependency evaluation for low-level node mechanics is distinct and not required now. | -| L04 | Immutable remote commit protocol | No extraction | Keep the domain protocol internal; evaluate established content-addressed specifications only if interoperable cross-application snapshots become a real requirement. | -| L05a | Path-safety helpers | No extraction; opportunistic local action | Keep small and internal; use standard-library path primitives only where semantics match exactly. | -| L05b | Bounded I/O and streaming redaction | No extraction | Retain adapter limits, secret policy, and errors in Narratio. | -| L05c | Atomic file replacement | No extraction; immediate local action | Use the small local or established implementation selected under H29 and `fileops.md`. | -| L05d | Object-store ports, file locks, IDs, and DAG helpers | No extraction | Keep the narrow ports and domain algorithms internal. | -| L06 | Release automation | No action | Keep repository-local; reconsider a standalone tool or template only after materially identical scripts drift across repositories. | -| L07 | Artifact source and destination policy | No extraction | Domain ownership is the source of its value. | -| L08 | Manifest and durable state validation | No extraction | Keep state schemas and validation with Narratio. | -| L09 | Stage ordering, selection, invalidation, resume, and force | No extraction | Keep pipeline lifecycle semantics with Narratio. | -| L10 | Analyze checkpoint and dependency recovery | No extraction | Implement C03 as Narratio domain behavior. | -| L11 | Cleanup and remote lock policy | No extraction | Implement C04 and C05 as Narratio operational policy. | -| L12 | External tool and storage adapters | No extraction | Keep integration-specific requests, validation, and errors local. | -| L13 | Configuration validation, party resolution, and artifact-family expansion | No extraction | Keep product configuration rules internal. | - -## Completeness and Routing Summary - -Every audited hypothesis or candidate is represented above. Only entries marked -**Immediate action** or **Immediate opportunistic action** produce scope in -[`refactoring.md`](refactoring.md). Constraints govern that work without -creating independent implementation tasks. Superseded, conditional, deferred, -and no-action entries remain here for traceability and are intentionally absent -from the actionable roadmap except where a boundary must be stated to prevent -scope creep. - -The consolidation adds no hypothesis or feature that was not identified by an -audit. A future implementation plan should derive its stages from -`refactoring.md`, use this register to verify completeness, and return to the -source audits only when it needs the supporting analysis behind a decision. diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md deleted file mode 100644 index 2c2aac3..0000000 --- a/docs/roadmap/audit.md +++ /dev/null @@ -1,764 +0,0 @@ -# Maintainability Audit - -## Status - -Complete. This audit evaluates the hypotheses in -[`potential-audit.md`](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](https://go.dev/doc/go1.27) 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. - -## Recommended Implementation Sequence - -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`](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. diff --git a/docs/roadmap/contract-audit.md b/docs/roadmap/contract-audit.md deleted file mode 100644 index 8a2e187..0000000 --- a/docs/roadmap/contract-audit.md +++ /dev/null @@ -1,525 +0,0 @@ -# Contract Simplification Audit - -## Status - -Complete and approved as a statement of intended direction. This document -records product and architecture decisions for future work; it does not describe -implemented behavior and is not an implementation plan. - -This audit applies the same contract-challenge premise used by the -[File Operations Simplification roadmap](fileops.md) to other maintainability -findings in [`audit.md`](audit.md): current behavior is not automatically a -durable product requirement merely because production code, tests, or -documentation currently preserve it. - -## Context - -Narratio is a trusted, single-session workflow orchestrator for intentionally -shareable D&D material. Its most important operational outcomes are correct -pipeline progress, economical reuse of expensive successful work, coherent -remote publication, understandable recovery, safe cleanup scope, and protection -of API keys. It does not need compliance-grade invocation history, indefinite -compatibility with already-deprecated formats, or distributed coordination -machinery disproportionate to its actual use. - -The maintainability audit found several areas where implementation mechanisms -had become apparent contracts. This follow-up review explicitly permits those -contracts to change when a simpler policy preserves the user outcome that -matters. - -## Governing Decisions - -- Remove deprecated legacy and compatibility behavior completely rather than - continuing to isolate and maintain it. -- Keep the session manifest as the sole durable local progress authority. -- Preserve successful analyze artifacts individually and checkpoint them as - they complete. -- Express local cleanup as an ordinary final pipeline stage rather than a - publish side effect or special manifest obligation. -- Represent each remote artifact lock as one independently managed remote - object and carry locked artifacts forward into later current snapshots. -- Preserve configuration semantics and useful diagnostics without treating - exact validation order, prose, or formatting as compatibility contracts. -- Release this work as a minor version. The removed compatibility surfaces were - already deprecated and explicitly subject to removal; their deletion does not - require a major-version release under the project decision recorded here. - -## Finding 1 — Deprecated Compatibility Has Become Cross-Cutting Product Code - -### Underlying issue - -Narratio retains multiple compatibility paths for state and configuration that -predate current canonical contracts. These paths include legacy remote current -state, candidate probing for previous-session artifacts, unversioned party and -separate players inputs, aggregate-only analyze state, older extraction -evidence, and manifests without current semantic evidence. - -Although several paths are labeled temporary or migration-only, their presence -still requires alternate parsing and selection algorithms, ambiguity handling, -branches in current workflows, compatibility-specific errors, fixtures, tests, -and documentation. Isolating these paths makes later removal easier, but does -not remove their ongoing cost. Continued support also makes it harder to reason -about which representation is authoritative. - -### Approved changes - -Purge deprecated compatibility behavior across production code, tests, -configuration, documentation, examples, and schemas. The purge includes: - -- the legacy remote `current/manifest.json` and `current/run_id.txt` reader; -- legacy previous-session artifact path candidates and existence probing; -- unversioned party documents and separate `players_file` behavior; -- legacy session-level party and players overrides that exist only for that - input model; -- aggregate-only legacy analyze availability and fallback to incidental files; -- compatibility for extraction records that lack the current contract and - provenance evidence; -- compatibility branches for manifests or artifact records that predate the - current supported schema; and -- associated legacy constants, types, helpers, errors, tests, fixtures, - documentation, and examples. - -Current state must have an explicit supported version or enough required -authority markers to be recognized unambiguously. Unsupported old local or -remote state must fail early with a clear migration/version error. Narratio must -not silently normalize, reinterpret, probe around, or partially accept it. - -Migration guidance may explain how to regenerate, republish, or otherwise -replace old state, but migration readers must not remain in the steady-state -runtime merely to preserve automatic compatibility. - -### Rationale - -Compatibility was already deprecated and bounded for removal. Retaining it -indefinitely converts a temporary cost into permanent architecture. One -canonical representation is easier to validate, test, document, restore, and -extend, and a clear rejection is safer than heuristic interpretation of old -state. - -### Final state - -Every supported configuration, party document, local manifest, remote current -snapshot, extraction record, analyze record, and previous-session artifact is -resolved through one canonical contract. Deprecated formats are absent from -production code and tests and receive a clear unsupported-version or migration -error rather than fallback behavior. - -## Finding 2 — Invocation Diagnostics Participate in Pipeline Correctness - -### Underlying issue - -Narratio currently maintains both an authoritative session manifest and a -structured run manifest for each invocation. The runner persists overlapping -stage actions, statuses, outputs, semantic evidence, failures, and terminal -state to both documents. This creates dual-save ordering, joined persistence -errors, separate session and invocation projections, terminal-failure recovery -machinery, and tests for keeping a diagnostic record synchronized with the -actual progress ledger. - -Historical per-invocation records are not operationally important. They are at -most useful diagnostics and do not justify being a second participant in -pipeline transactions. Their placement inside a run directory that cleanup may -delete further demonstrates that they are not durable progress authority. - -### Approved changes - -Eliminate the structured run manifest and its persistence lifecycle. - -Retain: - -- the run ID used for invocation identity, run-local paths, adapter provenance, - and immutable remote publication; -- the session manifest as the sole durable local record of current pipeline and - artifact state; -- ordinary bounded logs, generated configuration, receipts, and other - diagnostics where already useful; and -- the in-memory command result or summary reported to the caller. - -Remove: - -- run-manifest types, storage methods, path helpers, schema and validation; -- before-stage, after-stage, skip, failure, and terminal run-manifest saves; -- invocation-only analyze projections and output projections; -- publish handling that archives or classifies a run manifest; -- terminal persistence code whose only purpose is to coordinate session and - run records; and -- run-manifest documentation, compatibility handling, fixtures, and tests. - -No replacement historical record is required. A lightweight diagnostic summary -may be reconsidered later only if it can remain independent of pipeline -correctness and cleanup. - -### Rationale - -One durable authority eliminates synchronization and split-brain questions. -The session manifest already records the state needed for resume, status, -restore, artifact resolution, and publication. Logs and the immediate command -summary provide proportionate invocation diagnostics without making diagnostic -write failures fail otherwise valid pipeline work. - -### Final state - -The session manifest alone determines durable local pipeline progress. A run ID -still identifies an invocation and its files, but no second manifest mirrors -session transitions. Pipeline success or failure cannot be changed by an error -writing historical invocation metadata. - -## Finding 3 — Analyze Recovery Is Obscured by Dual Projection Machinery - -### Underlying issue - -Narratio correctly models configured analyze artifacts individually, but the -implementation combines that valuable behavior with separate session and -invocation collections, partial-result projection validation, rollback of -in-memory authority after persistence failure, and special runner handling for -a stage that returns both a result and an error. - -The desired user behavior is simpler than the machinery. Expensive successful -artifacts should remain reusable; a failed artifact and work that depends on it -should be retried. Invocation-specific audit projections are unnecessary once -run manifests are removed. - -### Approved behavior - -Assume an ordered artifact plan contains A, B, C, and D. If A and B succeed, C -fails, and D is not attempted: - -- A and B remain current, provided their output and semantic evidence still - validate; -- C is recorded as failed and has no current output authority; -- D remains missing if it has never succeeded, or becomes stale if an older - current D depends directly or transitively on C; -- the aggregate analyze stage is failed and execution stops; -- an ordinary later run or resume retains A and B, retries C, and proceeds to D - after C succeeds; and -- a forced analyze invocation reruns the effective selection regardless of - current records. - -Without `--artifacts`, force applies to the full configured analyze artifact -set. With `--artifacts`, force applies to the selected artifacts and the -dependency closure required by the established artifact-selection contract; -unrelated valid artifacts remain outside that invocation. - -### Approved changes - -Use the session manifest's analyze artifact collection as the only durable -artifact authority. Remove the invocation collection and all logic that exists -only to construct, validate, apply, or roll back an invocation-specific -projection. - -Execute the effective artifact set in deterministic dependency order. After -each artifact succeeds and its canonical output has been validated and -materialized: - -1. construct or update its current session artifact record; -2. stale any current transitive dependents when the new result invalidates - their dependency evidence; -3. atomically save the session manifest; and -4. only then proceed to the next artifact. - -If the checkpoint save fails, the new file alone does not establish authority; -the invocation fails and the artifact is eligible to run again. This safe -recomputation is preferable to a rollback transaction around incidental output -bytes. - -When an artifact fails, record its failed state, stale current transitive -dependents, mark the aggregate stage failed, and atomically save those changes -to the session manifest before returning the artifact error. A later run -reconciles each selected artifact from the persisted collection instead of -rerunning the entire stage automatically. - -Retain: - -- per-artifact fingerprints and validated output evidence; -- deterministic dependency ordering; -- selected-artifact and dependency-closure semantics; -- invalidation when configuration, inputs, dependencies, or output identity - changes; and -- the rule that incidental canonical files never create current authority. - -Simplify failure and persistence tests around these observable checkpoints -rather than the current projection and rollback choreography. - -### Rationale - -Checkpointing after each successful artifact matches the economic reality that -LLM work is expensive while one manifest write is cheap. It preserves A and B -after an ordinary error, cancellation, or process interruption. The manifest -remains the one authority, and a failed checkpoint safely causes recomputation -rather than ambiguous reuse. - -Dependency-aware reuse is worth retaining because character families, -artifact selection, and ongoing prompt development make whole-stage reruns -unnecessarily expensive. Invocation-only state is not. - -### Final state - -Analyze behaves as a resumable deterministic artifact queue backed by one -session collection. Successful valid artifacts survive later failures, the -first failed or invalid artifact is retried, downstream work resumes when its -prerequisites become current, and `--force` deliberately replaces normal reuse -within its effective selection. - -## Finding 4 — Cleanup Is a Hidden Publish Transaction - -### Underlying issue - -Remote publication and local deletion are independent operator outcomes, but -Narratio currently couples them. After publish commits remotely, the runner -creates a special durable cleanup obligation in the session manifest, deletes -eligible local targets one by one, persists completion evidence after each, -and reports the invocation as incomplete if cleanup fails. An operator can -therefore observe an apparent publish failure even though the remote commit is -already authoritative. - -The special obligation duplicates behavior already provided by pipeline stage -status and resume. - -### Approved changes - -Add `cleanup` as the final stage in the canonical pipeline, after every stage -that may need run-local material or diagnostics. Remove post-publish cleanup -from the runner and remove the special `post_publish_cleanup` manifest model. - -The cleanup stage must: - -- use existing cleanup configuration to determine whether spool audio, run - workspace data, both, or neither should be removed; -- require evidence of a successful current remote publish before deleting data - whose retention policy is conditioned on publication; -- bind cleanup targets to the applicable successfully published run rather than - infer arbitrary paths from current configuration; -- self-skip clearly when no cleanup is configured; -- use the root-scoped destructive safeguards defined by - [`fileops.md`](fileops.md); -- treat an already-absent eligible target as success so retry is idempotent; -- record ordinary stage success or failure in the session manifest; and -- produce no canonical artifact outputs. - -A cleanup failure must not alter the succeeded publish record or remote current -state. A later ordinary run skips still-valid upstream work and retries cleanup. -If an earlier stage is forced or otherwise changes, the fixed invalidation -relation must make the prior cleanup result stale where appropriate. - -Selecting a range that ends before cleanup must not delete local data. A full -default run includes cleanup because it is the canonical final stage. - -### Rationale - -Stage boundaries should match outcomes an operator can understand. Publication -has succeeded when the remote commit becomes current; local deletion has -succeeded when the selected local targets are absent. Modeling them separately -gives each its own status, errors, retry, selection, and documentation without a -custom transaction layered after the stage loop. - -The removal of run manifests also eliminates the present problem in which -cleanup deletes the directory containing an invocation record that the runner -otherwise attempts to finalize. - -### Final state - -`publish` owns only coherent remote publication. `cleanup` is the final, -idempotent local-maintenance stage. A failure is reported as `cleanup` failure, -publish remains visibly successful, and normal resume retries cleanup without -republishing unchanged artifacts. - -## Finding 5 — Remote Locks Use a Shared Mutable Control Plane - -### Underlying issue - -The current lock system stores remote locks in one mutable `locks.yml`, merges -them with static pipeline locks, defines precedence, validates the combined -document, uses generation-bound conditional replacement and bounded retries to -avoid concurrent lost updates, injects the effective set into runtime -configuration, and reloads it before current-pointer selection. - -This complexity follows from choosing one shared mutable document and two lock -authorities. The product requirement is narrower: an already canonical logical -artifact can be frozen so a later publish does not replace the canonical -version until an operator unlocks it. - -There is an additional semantic mismatch in treating a locked artifact as a -simple omission. Narratio publishes immutable run-scoped objects and advances a -current commit pointer; it does not overwrite a stable artifact object in -place. Omitting a locked artifact from a later commit can make it disappear -from current-state discovery rather than preserve the locked canonical version. - -### Approved lock model - -Use one remote lock object per stable logical artifact source. Its location is: - -```text -{session_prefix}/locks/{sha256(normalized_source_id)}.lock -``` - -The lowercase hexadecimal SHA-256 name provides a fixed, portable object-key -segment; the bounded object payload is versioned JSON containing the normalized -source ID, an optional reason, and its creation time. Readers verify that the -payload source hashes to the object name. The stable artifact source ID, not the -configured remote destination, is the lock identity. Configuration can change -a destination; it must not accidentally evade a lock on the logical artifact. - -Lock object presence is authoritative, subject to successful bounded decoding -and identity validation of that payload. No central index is authoritative. - -### Approved changes - -- Remove static `pipeline.publish.locks`, the mutable central lock-store schema, - static/remote merging and precedence, shared-document mutation retries, and - runtime injection of locks into resolved pipeline configuration. -- Extend the narrow object-store contract with deletion so an individual lock - object can be removed. Implement and test the operation in each storage - backend and fake. -- Keep the existing lock list/add/remove user workflow, adapted to per-source - objects: - - list enumerates and validates the lock prefix; - - add creates one object, fails clearly when it already exists, and permits - `--force` to replace its non-authoritative reason and creation metadata; and - - remove deletes that object, treating an already-absent object as an - idempotent success with a clear informational result. -- Check each relevant lock before spending work to publish its artifact and - check again immediately before advancing the current commit pointer. -- Do not introduce a distributed transaction protocol. Narratio accepts the - proportionate, documented boundary that a lock created after the final check - may race the pointer write; its trusted, low-concurrency operating model does - not justify cross-object transaction machinery. - -### Locked-artifact carry-forward - -When building a new remote commit: - -- an unlocked artifact uses the newly validated and uploaded artifact - declaration; -- a locked artifact already present in the prior current commit carries that - exact prior immutable declaration into the new commit; -- a locked artifact with no prior current declaration remains absent and is not - treated as a missing required output; and -- invalid or unverifiable prior current state fails publication rather than - silently replacing or dropping an artifact that is supposed to be locked. - -The initial lock read determines which prior declarations are carried into the -new immutable commit. If the final recheck finds that an artifact became locked -after that commit was constructed, publication must not advance the current -pointer; the unselected immutable attempt may remain, and the next run rebuilds -the commit with the prior declaration carried forward. If a lock was removed -after the initial read, the in-progress commit may conservatively carry the old -declaration once; a later publish can advance it. - -The remote commit contract and validation must permit a carried declaration to -reference an immutable object under an earlier run prefix within the same -campaign/session namespace. Its destination key, checksum, size, storage -generation, source identity, and originating run identity remain unchanged and -fully declared. Restore and current-state readers continue to trust only the -objects explicitly declared by the selected commit; they must not scan mutable -prefixes to find a carried artifact. - -Publish metadata and inspection commands should distinguish newly published, -carried-forward locked, locked-but-never-published, optional missing, and -unselected artifacts. - -### Rationale - -Independent lock objects make the existence check the concurrency model. Adding -or removing one lock cannot overwrite another lock's state, so a shared YAML -merge and retry loop are unnecessary. A stable source identity prevents a -configuration destination change from bypassing the operator's intent. - -Carrying the prior declaration forward matches the meaning of canonical lock: -other artifacts may advance while the locked artifact remains exactly the -version that was current when locked. It preserves coherent immutable snapshots -without freezing the entire session or copying the old bytes into every new run -prefix. - -### Final state - -Each artifact lock is one independently visible remote object managed by small -list/add/remove commands. Publish performs a direct lock lookup and a final -recheck. New commits preserve prior locked artifact declarations while allowing -unlocked artifacts to advance, with no static lock authority, shared mutable -lock manifest, precedence rules, or lost-update retry loop. - -## Finding 6 — Configuration Diagnostics Are Treated as Semantic Contracts - -### Underlying issue - -Configuration composition and validation correctly enforce meaningful product -rules, but maintenance guidance and tests have also tended to preserve exact -validation order, exact error prose, incidental formatting, and provenance -detail whether or not a user-facing operation consumes it. These expectations -increase refactoring cost and can discourage clearer validation structure. - -The same configuration area also carries deprecated party/player and other -legacy compatibility branches addressed by Finding 1. - -### Approved changes - -Retain these configuration contracts: - -- strict YAML decoding and rejection of unknown fields; -- additive imports with no duplicate field ownership; -- deterministic selection and application of one profile; -- meaningful explicit empty, false, zero, list, and map overlay behavior; -- centralized defaults and semantic validation; -- canonical party and artifact-family resolution; -- deterministic effective configuration and secret-free digest behavior; and -- diagnostics that identify the relevant field and, for composition conflicts, - the conflicting sources well enough for an operator to correct the problem. - -Do not treat these as durable contracts unless a specific machine-readable -interface explicitly says otherwise: - -- the order in which independent validation failures are discovered; -- complete prose wording or punctuation; -- incidental formatting of errors; -- private helper boundaries or validation call order; and -- per-node provenance retained only because the current implementation can - produce it. - -Remove legacy configuration fields and branches under Finding 1. Refactor -validators and composition code around semantic domains without preserving -obsolete diagnostics solely to keep exact-string tests unchanged. - -Tests should prefer typed errors, error categories, field paths, structured -data, or the smallest stable semantic fragment. Exact full-string assertions -are appropriate only for documented CLI syntax or an intentionally stable -machine-consumed contract. - -### Rationale - -Strictness and deterministic effective behavior prevent ambiguous execution; -exact diagnostic choreography does not. Useful errors can remain precise -without making their full prose a versioned API. This aligns configuration -tests with the repository testing policy: protect meaningful outcomes while -allowing legitimate internal reorganization. - -### Final state - -Configuration remains strict, deterministic, source-aware where useful, and -operator-friendly. Validators may be reorganized and diagnostics improved -without preserving incidental order or wording. Only the canonical party, -composition, profile, and artifact-family models remain. - -## Consolidated Target State - -After the approved work is complete: - -- Narratio supports one current configuration and state model with no legacy - runtime fallbacks; -- the session manifest is the only durable local progress ledger; -- analyze checkpoints each successful artifact, preserves valid completed work, - and resumes from failed, stale, or missing work in dependency order; -- `cleanup` is the final canonical stage and cannot make a committed publish - appear to have failed; -- remote locks are independent per-artifact objects, and locked canonical - artifacts are carried forward across new current snapshots; -- configuration tests protect semantic rules rather than diagnostic - choreography; -- ordinary logs and command summaries provide proportionate diagnostics without - participating in correctness; and -- the combined simplification is eligible for a minor release because the - removed compatibility behavior was already deprecated and removable. - -## Open Questions - -None. The policy and target-state decisions required for a later feature -roadmap and staged implementation plan are resolved by this audit. diff --git a/docs/roadmap/fileops.md b/docs/roadmap/fileops.md deleted file mode 100644 index 77b82b9..0000000 --- a/docs/roadmap/fileops.md +++ /dev/null @@ -1,273 +0,0 @@ -# File Operations Simplification - -## Status - -Accepted feature roadmap. This document defines the desired filesystem policy -and target state. It is not an implementation plan and does not describe -implemented behavior. It refines the filesystem conclusions in -[`audit.md`](audit.md). - -## Goal - -Make Narratio's filesystem code proportionate to its real operating model: -single-invocation work under a session lock in a trusted, group-owned workspace -containing intentionally shareable D&D material. Use the Go standard library -directly for ordinary filesystem work and retain custom code only where it -expresses a concrete Narratio requirement. - -The change should materially reduce production code, platform-specific code, -test machinery, full-tree copying, and the number of filesystem guarantees that -maintainers must preserve. - -## Underlying Issue - -`internal/fileops` has accumulated a general confined-filesystem abstraction, -two directory-promotion implementations, adversarial race hooks and inode -identity rechecks, handle-relative no-replace promotion, explicit file and -directory syncing, platform capability errors, universal symlink and regular- -file restrictions, and tests for each mechanism. - -Those mechanisms are internally coherent, but they solve a stronger problem -than Narratio has. Narratio does not operate on a hostile multi-writer tree, and -its ordinary workspace data is not private. The session lock prevents normal -concurrent mutation by Narratio, while run IDs make durable extract destinations -unique. Treating every current mechanism as a permanent policy has therefore -created a self-reinforcing contract: implementation detail drives tests and -documentation, which in turn appear to require the implementation detail. - -This roadmap resets that contract around required outcomes. It does not assume -that a replacement must preserve identical mechanics or failure behavior. - -## Policy Decisions - -### Operating and threat model - -- Campaign and session workspaces are trusted operational inputs, not hostile - filesystems. Narratio does not defend ordinary reads and writes against an - attacker racing directory entries during an invocation. -- Workspace transcripts, artifacts, manifests, logs, and configuration are - intended to be shareable. World readability is acceptable. Group-owned, - group-writable operation remains a requirement. -- API keys are the exception. Narratio must not write credentials into runtime - artifacts or diagnostics, and any filesystem path that can contain API-key - material retains appropriately strict handling. -- Narratio must still prevent accidental destructive action or restore outside - the selected application-owned root. Trusted input does not justify an - unbounded delete or an escaped restore target. - -### Guarantees to retain - -- Corruption-sensitive single-file updates use same-directory temporary files - and replacement so readers do not observe partially written content. This - applies to such state as manifests and generated configuration; it is an - atomic-visibility requirement, not a power-loss durability promise. -- Files downloaded from remote storage are completely received and validated - before they become the canonical local file. -- Adapter and configuration input remains bounded by centralized, discoverable - constants. Limits remain deliberately far above expected workloads and - produce unmistakable errors naming both the limit and the enforcing adapter. -- Workspace files and directories preserve the repository's group-owned, - group-writable mode policy, including after temporary-file replacement or - directory rename. -- Cleanup and restore operate relative to an explicitly opened root and reject - targets outside that scope. Recursive removal uses the standard-library root - primitive available at the maintained Go baseline. -- Restore treats leaf or ancestor symlinks as non-regular conflicts rather than - following them to classify or overwrite a target. This is a restore boundary, - not a universal rule for ordinary workspace access. -- Staging cleanup is limited to paths that Narratio created and verified as - descendants of the applicable invocation or session workspace. - -### Guarantees to retire - -- Narratio does not promise that a successful write or directory install has - been forced through every operating-system and storage cache before return. - File and directory `fsync` calls, directory-sync capability probes, and - unsupported-durability errors are not part of the target contract. -- Narratio does not defend against a concurrent adversary replacing a file or - directory after it has been inspected. Inode identity snapshots, `SameFile` - rechecks, and race-injection hooks are not part of the target contract. -- Ordinary workspace operations do not universally reject symlinks or require - every source to be a regular file. Components may validate a concrete external - contract where needed, but there is no package-wide no-follow policy. -- Directory installation does not require a custom atomic no-replace primitive - or an exact cross-platform race guarantee. The session lock, absent-destination - check, and unique run ID are the intended concurrency controls. -- “Durable” in Narratio documentation means managed persistent application - state. It does not mean guaranteed survival of every acknowledged update - across immediate power loss. - -## Required Changes - -### Narrow the filesystem API - -Replace the broad `internal/fileops` facade with direct `os`, `io`, and -`path/filepath` operations at ordinary call sites. Retain only small helpers -that enforce a shared product rule rather than wrapping the standard library. -Expected shared concerns are: - -- corruption-resistant single-file replacement with explicit final mode; -- bounded reads or copies with centralized limits and contextual errors; -- application workspace file and directory modes; and -- narrowly scoped root-relative cleanup and restore operations. - -The target is not a one-for-one rewrite of existing exported functions. Remove -unused or single-purpose wrappers when the calling package can express the -operation clearly with the standard library. Keep platform-specific code only -if it is necessary for the small atomic-file replacement helper to provide the -supported cross-platform behavior. - -Do not add a general filesystem framework. Same-directory atomic replacement is -a generic abstraction, but a small local helper is the preferred target unless -implementation research identifies a maintained cross-platform dependency that -removes more code than it adds and preserves the required mode behavior. - -### Simplify ordinary reads, writes, and copies - -Use ordinary standard-library path operations for trusted workspace data. -Preserve contextual errors and the existing centralized safety limits, but do -not route every access through an `os.Root`, reopen files to prove identity, or -reject symlinks merely because the current facade does so. - -Callers that consume a defined external format may still validate type, size, -checksum, structure, or placement because their own contract requires it. Those -checks should live with the consumer and must not reconstitute a universal -FileOps policy indirectly. - -### Replace file updates without power-loss machinery - -Use a minimal same-directory temporary-file flow for corruption-sensitive file -updates: create, write, close, apply the intended mode, and replace the canonical -path. Ensure temporary files are removed after pre-commit failures. Preserve -context cancellation checks where a caller can still abandon an update before -replacement. - -Remove file and parent-directory syncing, sync-related platform probes and -sentinel errors, and tests that exist only to assert power-loss guarantees. If a -small operating-system-specific replace shim is still required, keep it private -to this helper and test its observable replacement behavior rather than its -internal syscall sequence. - -### Simplify download installation - -Keep remote downloads in a temporary file until transfer, size, checksum, and -format validation succeeds. Install the validated file using the same minimal -replacement helper or a direct rename when the destination is known absent. -Remove standalone download-install APIs that have no production caller and -avoid duplicating replacement policy between download and ordinary writes. - -### Replace Notarius directory promotion - -Have Notarius write its output bundle to a unique staging directory within the -invocation workspace. Validate that the reported bundle is inside the requested -output root and satisfies the Notarius integration contract. Normalize required -workspace modes, confirm that the unique durable destination is absent, and use -`os.Rename` to move the validated directory directly to that destination. - -Delete the production and test-only directory tree-copy implementations, -promotion hooks, handle-relative no-replace code, directory syncing, and their -platform files. A destination collision must return a clear ordinary error; it -does not require adversarial race recovery. - -The successfully renamed bundle becomes the sole retained artifact tree. The -pre-promotion Notarius tree is not duplicated for diagnostics. Existing receipt -and log records remain the troubleshooting evidence, and failed staging trees -are cleaned up only after their scope is verified. - -### Keep destructive and restore operations root-scoped - -Retain `os.Root` at boundaries where containment is the requirement. Use the -maintained Go version's root-relative recursive removal for cleanup. Continue to -require proper-descendant relationships and application-owned eligibility -before destructive operations. - -Rework restore classification and installation around a session root. Inspect -targets without following leaf or ancestor symlinks, classify such targets as -non-regular conflicts in both force and non-force plans, and hash only a target -verified within that root. Do not add inode-stability or hostile-race rechecks. - -### Preserve group-owned workspace operation and protect secrets - -Centralize the intended workspace modes in one discoverable location and apply -them consistently to created directories, ordinary files, atomic replacements, -downloads, and renamed Notarius bundles. Do not tighten general workspace -permissions on the assumption that transcripts or artifacts are sensitive. - -Audit API-key ingress separately from general workspace operations. Secret -values must remain out of files unless an existing explicit credential contract -requires a file, must never enter logs or errors, and must not inherit permissive -workspace modes. The simplification must not weaken existing credential -redaction or environment handling. - -### Align documentation and tests with outcomes - -Revise the canonical architecture, operations, troubleshooting, and focused -internal documents to remove power-loss durability, universal confinement, -universal no-follow, and atomic directory-promotion claims. Keep volatile modes, -paths, limits, and operational behavior in their existing canonical owners -rather than duplicating them across documents. - -Delete tests for removed algorithms, syscall ordering, race hooks, directory- -sync capability behavior, and guarantees no longer offered. Retain or add -focused tests for: - -- successful and failed atomic file replacement on every supported platform; -- cleanup of temporary files before replacement; -- bounded input and clear adapter-specific limit errors; -- group-owned file and directory modes; -- validated download installation; -- validated Notarius bundle rename, collision, and cleanup behavior; -- root-scoped cleanup and refusal of escaped destructive targets; -- restore handling of leaf and ancestor symlinks; and -- API-key redaction and any credential-file permission contract. - -Tests should assert externally meaningful results and failure boundaries, not -recreate the removed implementation through new private hooks. - -## Compatibility and Migration - -This work changes internal guarantees rather than CLI, configuration, manifest, -or external artifact schemas. Existing valid workspaces should remain usable. -Users may observe fewer failures on filesystems that cannot sync directories, -faster extract installation, less temporary disk consumption, and ordinary -symlink behavior outside explicitly scoped restore and destructive operations. - -The removal of the duplicate pre-promotion Notarius output tree is an intentional -operational change and must be reflected in the operations and troubleshooting -documentation. No compatibility layer should preserve the old FileOps API solely -for internal callers; migrate callers and delete obsolete surfaces together. - -## Target End State - -At completion: - -- ordinary workspace file operations are clear standard-library code located - near their domain owners; -- shared filesystem code is small and limited to atomic file replacement, - bounded I/O and modes, plus explicitly root-scoped destructive or restore - helpers; -- directory promotion, tree-copy duplication, adversarial race defenses, - directory syncing, and unsupported-durability machinery no longer exist; -- extract validates and renames one Notarius staging bundle without copying the - complete tree or retaining a duplicate source tree; -- manifests and other corruption-sensitive files are never intentionally - exposed partially, without claiming power-loss durability; -- accidental out-of-root deletion and restore remain prevented; -- group-owned workspaces remain first-class, and API keys remain the only data - requiring secret-specific filesystem treatment; -- tests cover the reduced observable contract rather than removed mechanisms; - and -- canonical documentation accurately distinguishes persistent application state, - atomic visibility, trusted-workspace assumptions, and destructive-operation - confinement. - -## Out of Scope - -- Changing CLI commands, configuration schemas, artifact schemas, or manifest - formats. -- Adding multi-process writes to one session or removing the session lock. -- Defending trusted workspaces against malicious concurrent mutation. -- Adding encryption or restrictive permissions for shareable campaign data. -- Redesigning remote publish commit ordering or object-store concurrency. -- Adding user-configurable size limits in this feature; constants should remain - easy to promote to configuration later. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index b7fde5e..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,431 +0,0 @@ -# Workstream 10 Implementation Plan — Quality, Testability, And Documentation Alignment - -## Status - -Planned implementation for Workstream 10, **Quality, testability, and -documentation alignment**, in [`refactoring.md`](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`](refactoring.md) is authoritative. Its -supporting first-pass findings are H07, H08, H12, H13, H17, H18, H23, and H25 -in [`audit.md`](audit.md). The corresponding contract decisions are C01 through -C06 in [`audit-decisions.md`](audit-decisions.md); their full rationale remains -in [`contract-audit.md`](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`](refactoring.md), and only its cited audit - sections. Read [`../internal/overview.md`](../internal/overview.md) and, for - restore changes, [`../internal/command-restore.md`](../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: - -```sh -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: - -```sh -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: - -```sh -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: - -```sh -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:** Completed - -### 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: - -```sh -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:** Completed - -### 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: - -```sh -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 ` -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. diff --git a/docs/roadmap/library-audit.md b/docs/roadmap/library-audit.md deleted file mode 100644 index 1f93a04..0000000 --- a/docs/roadmap/library-audit.md +++ /dev/null @@ -1,395 +0,0 @@ -# Reusable Library Extraction Audit - -## Status - -Complete. This audit reviews the Narratio code at `af5fbc1c4f8d` and records -candidate-extraction decisions for future refactoring. It does not authorize an -extraction, create a new module, or change application behavior. - -This is the third maintainability pass. It complements the two earlier passes: - -1. [`audit.md`](audit.md) asks whether implementation can be simplified while - preserving current behavior and contracts. -2. [`contract-audit.md`](contract-audit.md) asks whether apparent contracts can - reasonably be changed to permit substantially simpler behavior. -3. This document asks whether code that Narratio no longer needs should instead - be preserved as a generic, independently reusable library. - -The question in this pass is deliberately separate from whether Narratio should -continue using an extracted library. Code may be valuable in a more demanding -application while remaining the wrong dependency for Narratio. - -## Method and Decision Standard - -The review inventoried Narratio's internal packages, concentrated on mechanisms -whose contracts can be stated without D&D or pipeline terminology, inspected -their call sites and tests, and compared the most plausible candidates with the -actual implementations in the neighboring Notarius and WeatherReporter -repositories. It also checked the Go standard library and representative -maintained or emerging third-party implementations before treating custom code -as a library opportunity. - -A candidate is suitable for extraction only when all of the following are true: - -- its contract can be explained without Narratio types or product policy; -- Narratio-specific configuration, paths, modes, diagnostics, and error prose - can remain in Narratio rather than leak into the generic API; -- at least one credible independent consumer needs materially the same - guarantees; -- extraction removes meaningful implementation and test ownership from - consumers instead of adding adapters around a small helper; -- the generic API is narrow, stable, and independently testable on every - promised platform; -- an existing maintained library does not already satisfy the need; and -- the extracted module has an identified maintenance, compatibility, release, - and vulnerability-response owner. - -The verdicts used below are: - -- **Extract now:** a proven shared contract and consumer justify a separate - maintained module. -- **Incubate:** the code contains a credible generic core, but extraction should - wait for a concrete trigger. -- **Keep internal:** the implementation is coherent but belongs to Narratio's - domain or is too small to justify an external lifecycle. -- **Simplify or delete:** independent reuse does not justify retaining the code - in Narratio; source history is enough to preserve ideas for later recovery. -- **Prefer an established abstraction:** a future generic consumer should start - from an existing standard or library rather than Narratio's implementation. - -## Executive Summary - -No package should be extracted before the approved Narratio refactoring begins. -The audit found one serious incubation candidate: the cross-platform subprocess -supervision core. It implements a substantial and reasonably generic process- -tree lifecycle that the Go standard library does not provide as one operation. -It is nevertheless mixed with Narratio-owned environment, logging, redaction, -filesystem, and diagnostic policy, and neither Notarius nor WeatherReporter is -currently a matching second consumer. Extraction now would therefore invent a -public API before its reuse requirements are known. - -The current FileOps subsystem may be useful source material for an application -that genuinely needs hostile-race resistance, confined traversal, no-replace -directory promotion, or power-loss durability. It is not a cohesive generic -library as written, and its strongest guarantees are specifically the ones -Narratio has decided to retire. The approved FileOps simplification should -proceed. Git history is a sufficient preservation mechanism unless a real -external consumer appears before deletion. - -Configuration composition, remote commit snapshots, artifact policy, session -state, and pipeline recovery contain useful patterns, but their contracts are -Narratio product contracts rather than generic library seams. Small path, -bounded-I/O, locking, identifier, and atomic-write helpers do not carry enough -independent value to justify separate versioning. Release automation is a -possible shared tool or template, not a Go library, and current sibling -repositories do not yet share one implementation that needs central ownership. - -| Candidate | Verdict | Refactoring consequence | -| --- | --- | --- | -| Cross-platform subprocess supervision | Incubate | Separate generic mechanism from Narratio policy locally; extract only after a second consumer and owner exist. | -| Hardened FileOps subsystem | Simplify or delete | Follow `fileops.md`; preserve history, not a Narratio dependency on an overbuilt library. | -| YAML composition and provenance | Keep internal | Retain as Narratio's configuration DSL; reconsider structural merge dependencies separately if they reduce code. | -| Immutable remote commit and pointer protocol | Keep internal | Preserve as Narratio's publication contract; do not market it as a generic object-store transaction library. | -| Path-safety and bounded-I/O helpers | Keep internal | Prefer small local helpers and newer standard-library primitives where semantics match. | -| Atomic file replacement | Keep internal for Narratio | Use a small cross-platform helper or an established dependency after focused implementation research; do not extract a new module. | -| Object-store interface and session locks | Keep internal | These are narrow ports and product coordination rules, not reusable implementations. | -| Release scripts and checks | Keep repository-local | Share a template or standalone tool only if multiple repositories develop materially identical behavior and drift. | -| Artifact, manifest, analyze, and pipeline policy | Keep internal | These are Narratio domain code even where individual algorithms are conventional. | - -## Candidate 1 — Cross-Platform Subprocess Supervision - -### Why it is a credible library seed - -`internal/adapters/subprocess` contains substantially more than a wrapper around -`os/exec`. Its process owner: - -- starts each command in an owned Unix process group or Windows Job Object; -- prevents the Windows child from running before job assignment; -- terminates the owned tree on cancellation or timeout; -- attempts graceful then forceful shutdown where the platform supports it; -- reaps the leader and bounds waits when descendants retain stream handles; -- distinguishes exit, timeout, cancellation, capture-limit, and cleanup - outcomes; and -- has focused Unix and Windows tests for the lifecycle. - -That core contract is understandable without Narratio terminology and is large -enough that a proven consumer could benefit from sharing it. The Go -[`os/exec`](https://pkg.go.dev/os/exec) package supplies command execution, -custom cancellation, and `WaitDelay`, but its default cancellation targets the -leader process rather than establishing cross-platform process-tree ownership. - -### What is not generic - -The package currently also owns policy that must not cross an extraction -boundary: - -- Narratio's inherited-environment allowlist and test-helper variables; -- API-key environment names and extraction of their secret values; -- fixed diagnostic size limits and application-specific owner wording; -- log paths, group-owned file modes, and FileOps calls; -- stderr-tail formatting and Python/file-descriptor diagnostic hints; and -- atomic YAML and byte-writing helpers unrelated to process ownership. - -A reusable core should accept an explicit environment and ordinary `io.Writer` -or capture interfaces. Narratio should remain responsible for choosing and -redacting the environment, opening bounded diagnostic sinks, selecting paths -and modes, and translating typed results into operator-facing errors. - -### Existing alternatives - -The standard library remains the foundation but does not provide the whole-tree -contract. Third-party packages now exist in this space, including -[`processkit-go`](https://github.com/ZelAnton/processkit-go), but the candidates -found during this audit are comparatively new or offer a materially broader -contract than Narratio needs. They should be reevaluated at extraction time; -the existence and maturity of this category are changing quickly, and copying -Narratio code into a new module is not automatically preferable. - -### Decision and extraction trigger - -**Verdict: Incubate; do not extract now.** - -During Narratio refactoring, make the internal mechanism/policy boundary clear -and remove unrelated file-writing helpers from the package. Do not contort the -internal API solely to resemble a hypothetical public module. - -Reopen extraction only when all of these triggers are present: - -1. a second production application needs whole-process-tree ownership on at - least two of Narratio's supported operating systems; -2. its timeout, graceful-shutdown, cleanup, and result semantics substantially - overlap Narratio's rather than merely needing `exec.CommandContext`; -3. maintained alternatives have been reevaluated and rejected for documented - technical or maintenance reasons; -4. an independent module API can exclude all Narratio filesystem, environment, - redaction, and diagnostic policy; and -5. a maintainer accepts cross-platform CI, semantic versioning, releases, and - security ownership for the module. - -If those conditions are met, extract the generic core from the simplified -internal boundary and let Narratio consume it through a thin policy adapter. - -## Candidate 2 — Hardened FileOps - -### Why the code may be useful elsewhere - -The present `internal/fileops` implementation includes confined root operations, -symlink-resistant traversal, identity rechecks, same-directory replacement, -checksummed copy and download installation, directory tree validation, atomic -no-replace promotion, mode enforcement, file and directory syncing, and -platform-specific implementations. An application operating on an untrusted, -concurrently modified tree could reasonably need some of those mechanisms. - -### Why it is not an extraction candidate as written - -The package is a bundle of distinct concerns rather than one small generic -contract. It combines: - -- atomic single-file visibility and durability; -- hostile-race-resistant traversal and copying; -- directory transaction and no-replace semantics; -- Narratio workspace ownership modes; -- bounded application I/O; and -- application-specific unsupported-platform and contextual errors. - -A credible external library would first need to choose which threat model and -durability level it promises, define a much smaller API, specify filesystem and -platform assumptions, and split policy such as modes and limits from mechanism. -That is a new library design project, not a package move. - -There is also no matching current consumer nearby. Notarius implements its -ordinary atomic byte write in a small local helper, and WeatherReporter likewise -uses a focused same-directory temporary-file replacement helper. Neither needs -Narratio's complete confined traversal and directory-promotion contract. - -Existing components cover portions of the space. Go provides -[`os.Root`](https://pkg.go.dev/os#Root) for root-scoped operations; libraries -such as [`renameio`](https://github.com/google/renameio) and -[`moby/sys/atomicwriter`](https://github.com/moby/sys/tree/main/atomicwriter) -address atomic file replacement with different platform, durability, and mode -tradeoffs. No single dependency should be selected merely to preserve the -current aggregate abstraction. - -### Decision - -**Verdict: Simplify or delete; do not extract before refactoring.** - -Implement the reduced contract in [`fileops.md`](fileops.md). Preserve the -current implementation through version control and, if useful, a short design -note identifying the historical commit. Do not make Narratio depend on an -externalized version of guarantees that Narratio has explicitly rejected. - -If another application later demonstrates a need, design a new module from its -requirements and selectively recover proven algorithms and tests from history. -That work should not block or enlarge the Narratio refactor. - -## Candidate 3 — YAML Composition and Provenance - -`internal/config/composition.go` implements a presence-aware YAML tree with -source identity, duplicate-key detection, additive disjoint-source composition, -profile overlays, deterministic canonical rendering, semantic leaf projection, -and source attribution. These mechanics look generic at first glance. - -The actual merge algebra is nevertheless Narratio's configuration language: -root-only composition metadata, disjoint imports, named pipeline profiles, -overlay ownership, atomic list treatment, null and kind-change policy, pipeline -field provenance, effective digests, and strict decoding into `PipelineConfig`. -The useful diagnostics also depend on those policy choices. Extracting only the -tree mechanics would leave much of the code and all product decisions in -Narratio, while extracting the whole package would publish Narratio's DSL as a -supposedly generic API. - -Notarius does not implement the same import/profile language, and -WeatherReporter's configuration merge is a short typed decode into its own -configuration. They are not current consumers. Structural YAML libraries such -as [`yaml/go-yaml-dom`](https://github.com/yaml/go-yaml-dom) now provide clone -and configurable merge operations, but they do not by themselves establish -Narratio's provenance and additive-ownership rules. - -**Verdict: Keep internal.** Keep product semantics explicit in Narratio. A -future implementation pass may evaluate a maintained YAML DOM/merge dependency -as a way to delete low-level node mechanics, but dependency adoption and public -library extraction are different decisions. Do not create a generic module -without a second consumer that independently chooses the same merge algebra. - -## Candidate 4 — Immutable Remote Commit Protocol - -The remote publication design uses immutable run-scoped objects, checksummed -and generation-bound artifact descriptors, an immutable commit manifest, and a -single mutable current pointer. The pattern is reusable in the abstract and -provides coherent publication on an object store without pretending the store -offers a multi-object transaction. - -Its concrete schema and validation are Narratio-specific: campaign and session -identity, run IDs, session and run manifests, configured artifact sources, -canonical S3 key layout, publication categories, previous-session resolution, -and current-state reconstruction. The storage interface alone is not the -protocol; extracting it would either export Narratio's schema or require a new -generic content model and transaction API. - -Established content-addressed specifications already model similar generic -ideas. The [OCI image specification](https://github.com/opencontainers/image-spec) -defines descriptors, digests, manifests, indexes, and mutable names over -content-addressed objects. Narratio should not migrate to OCI merely for reuse, -but any future cross-application protocol should evaluate established formats -before turning Narratio's private contract into a library. - -**Verdict: Keep internal.** Treat the implementation as a good domain protocol, -not a generic object-store transaction package. Reconsider only if multiple -applications need interoperable snapshots; at that point start with a shared -protocol specification and compatibility requirements, not a code extraction. - -## Candidate 5 — Small Infrastructure Helpers - -### Path safety - -`internal/pathsafe` is small, pure, and widely used, but it encodes Narratio's -slash-style relative destination rules and cross-platform object-key behavior. -It is below the threshold where a separate module's dependency and release -lifecycle would pay for itself. Newer Go path APIs such as -[`filepath.IsLocal`](https://pkg.go.dev/path/filepath#IsLocal) should be used -where their host-path semantics exactly match; object keys and canonical slash -paths may still warrant a few local functions. - -**Verdict: Keep internal and small.** - -### Bounded I/O and streaming redaction - -Bounded reads, tail capture, and streaming secret redaction are useful generic -techniques. Narratio's instances are tied to adapter-specific limits, owner -names, failure wording, log files, and its API-key policy. Separating them into -external modules would trade a few local helpers for additional APIs without a -known consumer. - -**Verdict: Keep internal.** If subprocess incubation later proceeds, bounded -capture may become part of that module only if consumers agree on typed limit -semantics; secret selection and error prose remain application policy. - -### Atomic file replacement - -Same-directory temporary-file replacement is generic, but Narratio's reduced -need is a small operation with explicit final mode and supported-platform -behavior. Notarius and WeatherReporter each maintain a short local variant, but -their differing modes, cancellation points, and parent-directory behavior show -that superficial duplication is not yet one shared contract. - -**Verdict: Do not create a new library.** During FileOps implementation, -reevaluate established atomic-writer packages against Narratio's exact Windows, -mode, and atomic-visibility requirements. Otherwise retain a small local helper. - -### Object-store ports, file locks, IDs, and DAG helpers - -The object-store interfaces are deliberately narrow ports over provider SDKs; -the implementations and error semantics remain application adapters. Session -locks enforce Narratio's one-session execution rule. Run/session identifiers -and artifact dependency traversal are bound to Narratio schemas and lifecycle. -Extracting any of these would either expose domain types or create generic -abstractions with less clarity than the current code. - -**Verdict: Keep internal.** Use established provider SDKs and standard-library -algorithms directly rather than creating thin generic wrapper modules. - -## Candidate 6 — Release Automation - -Narratio's release scripts combine source-tree validation, asset-matrix builds, -archive naming, checksum generation, Git state checks, tag creation, and push -behavior. Those ideas are reusable, but the concrete release contract includes -Narratio's binary name, supported target matrix, documentation, tag policy, -asset layout, and responsibility boundary. Notarius currently has a narrower -source-release check, while WeatherReporter documents its procedure without a -matching shared script suite. - -**Verdict: Keep repository-local.** This is not a runtime Go library candidate. -If two or more repositories later carry materially identical scripts and fixes -begin to drift, extract a versioned standalone release tool or a maintained -template with per-project declarative metadata. Until then, local scripts keep -release changes atomic with each repository and avoid a bootstrap dependency in -the release path. - -## Explicit Non-Candidates - -The following code is intentionally excluded from generic extraction despite -containing reusable programming techniques: - -- artifact source classification and published-destination policy; -- session and remote manifest schemas and validation; -- stage ordering, selection, invalidation, resume, and force semantics; -- analyze artifact checkpointing and dependency-aware recovery; -- cleanup and remote lock policy; -- Notarius, Seriatim, Scriptorium, Audita, WhisperX, and S3 adapters; and -- configuration validation domains and party/artifact-family expansion. - -Their value comes from making Narratio's product rules explicit. Moving them to -a generic module would hide ownership rather than create reuse. Conventional -algorithms inside domain code are not by themselves library candidates. - -## Refactoring Guidance - -This audit does not add an extraction prerequisite to the approved refactoring. -Use the following sequence: - -1. implement the FileOps and contract simplifications already recorded in the - roadmap documents; -2. keep refactored domain ownership within Narratio unless a stage explicitly - calls for a package seam; -3. while touching subprocess code, separate process ownership from Narratio's - diagnostics and filesystem policy, but retain an internal API; -4. record deleted FileOps history rather than preserving dead compatibility or - an external dependency for hypothetical users; and -5. revisit subprocess extraction only when the trigger conditions in this - audit are satisfied. - -An extraction, if later approved, should be planned and released as its own -project. It should not be smuggled into a Narratio refactoring stage, because -public API design, multi-platform CI, module ownership, migration, and release -policy are independent deliverables. - -## Final State of This Audit - -The audit preserves one future option without delaying simplification: -Narratio's subprocess supervision core is a credible library seed, while all -other reviewed areas should remain internal, use established abstractions, or -be simplified and deleted. No current code must be retained merely because it -might someday be reusable. Version control preserves implementation history, -and a demonstrated second consumer—not speculative generality—will trigger any -future extraction. diff --git a/docs/roadmap/potential-audit.md b/docs/roadmap/potential-audit.md deleted file mode 100644 index dc8070f..0000000 --- a/docs/roadmap/potential-audit.md +++ /dev/null @@ -1,224 +0,0 @@ -# Potential Maintainability Audit - -## Status - -Initial impressions only. This document does not define action items, commit to -an audit, or recommend specific refactoring. Its purpose is to preserve signals -that may warrant investigation in a subsequent, deeper pass. - -## Context - -Narratio contains approximately 36,000 lines of production Go and 45,000 lines -of test Go across 26 packages. That footprint appears broadly consistent with a -durable workflow orchestrator that owns configuration, pipeline sequencing, -artifact and manifest state, recovery, storage, subprocess integration, and -filesystem safety. It is larger than would be expected of a simple command -wrapper, but the current size alone is not evidence of a maintainability -problem. - -Narratio handles non-sensitive pipeline data and is expected to run in a -trusted environment. A future review therefore need not assume an adversary who -can manipulate workspace paths, symlinks, or directory entries concurrently -with the process. Protection against accidental path escape and destructive -operator mistakes remains useful, as do atomic replacement, crash durability, -and recoverable state transitions. The distinction between adversarial -filesystem hardening and crash-safe data integrity should guide any proposed -simplification. - -## Preliminary Signals - -The following observations may justify a focused maintainability audit: - -- Production code is concentrated in `internal/stage`, `internal/app`, and - `internal/config`. These packages contain approximately 8,300, 7,900, and - 5,500 lines respectively. -- Several production files exceed 1,000 lines, including stage publishing and - analysis, configuration validation, and application runner code. File size - does not establish poor cohesion, but it can conceal multiple responsibilities - or make change impact harder to assess. -- The application, stage, and artifact areas have substantial interaction. - Their effective boundaries and the blast radius of changes may merit closer - examination. -- The most notable function-level complexity observed so far is concentrated in - confined filesystem and directory-promotion code. This is security-sensitive - logic built around a stronger threat model than Narratio now requires. A - review could determine which mechanisms exist only to resist adversarial - filesystem races and which are necessary for path safety, atomic updates, and - crash-safe integrity. -- Test code is larger than production code. This is a positive verification - signal, but a deeper pass could determine whether the suite also contains - repetition, brittle implementation coupling, or fixture and helper sprawl. - -## Counter-Signals - -The initial review also found evidence against a conclusion of broad or urgent -maintainability trouble: - -- Repository-wide tests and `go vet` pass. -- Package dependencies are acyclic. -- Average function complexity is modest, with higher complexity appearing - localized rather than systemic. -- The direct dependency set is small. -- Documentation is structured around explicit ownership and reading paths. -- The repository has substantial automated test coverage by volume. - -## Questions for a Deeper Pass - -A later audit could investigate, without presuming that changes are required: - -- whether responsibilities within `internal/app`, `internal/stage`, and - `internal/config` remain cohesive; -- whether the largest files contain natural internal modules that are currently - obscured; -- whether stage and external-adapter implementations contain meaningful - duplication; -- whether tests are organized around durable behavior or coupled to incidental - implementation structure; and -- whether manifest, restore, publishing, and confined-filesystem invariants are - easy to locate, understand, and verify across package boundaries. - -The deeper pass should also evaluate these specific modernization candidates: - -- upgrading the maintained toolchain baseline from Go 1.25 to Go 1.27, subject - to ordinary compatibility and release validation; -- replacing custom root-confinement, no-follow traversal, safe cleanup, and - confined-read machinery with the standard library's `os.Root` operations - where their semantics match Narratio's reduced threat model; -- retaining Narratio-owned code where it still provides application-specific - crash durability, atomic replacement, no-replace directory promotion, mode - policy, bounded reads, or destructive-operation guardrails; and -- evaluating the `os.Process.WithHandle` API added in Go 1.26 as a way to avoid - reopening a Windows child process by PID before assigning it to an owned job - object, while recognizing that it may not replace suspended startup, job - object management, or initial-thread resumption; -- using the standard library's `slices` and `maps` packages to replace generic - cloning, membership, key extraction, sorting, and deduplication helpers where - those helpers do not encode domain-specific ordering or copy semantics; -- using `golang.org/x/sync/errgroup`, including its bounded-concurrency support, - to simplify transcription worker coordination, cancellation, and first-error - propagation while retaining Narratio-owned result aggregation and error - context; -- adopting `github.com/google/go-cmp/cmp` selectively in tests where structural - diffs would replace `reflect.DeepEqual` or hand-written comparisons with more - useful failure output; -- evaluating `github.com/gofrs/flock` after the `os.Root` design is understood, - to determine whether it can replace Unix and Windows lock primitives and - context-aware polling without losing lock metadata or desired file ownership - semantics; and -- considering a CLI framework such as Cobra only if command and flag evolution - proves costly enough to justify migration risk, dependency growth, and - possible changes to existing parsing, help, and error-output behavior. - -The review may also document why a replacement is not appropriate. Current -low-confidence candidates include general configuration frameworks or schema -validators, generic atomic-file packages, provider-neutral object-storage -abstractions, retry or backoff packages, UUID or ULID packages, and generic -subprocess frameworks. Narratio's composition provenance, semantic validation, -durability guarantees, conditional object writes, identifier needs, and -tool-specific adapter contracts may leave little generic machinery for these -libraries to replace. - -## Preliminary Complexity Findings - -Complexity metrics are heuristics rather than findings of defects. Guard -clauses, validation branches, and explicit state transitions can score highly -while remaining appropriate. The following production-only measurements are -initial signals for focused code reading. - -### Execution And Planning - -`internal/app.executePlan` is the clearest outlier observed so far: - -- approximately 456 lines; -- cyclomatic complexity of 79; -- cognitive complexity of 149; and -- more than 60 direct collaborators in the code graph. - -The function currently spans invocation preparation, collaborator construction, -locking, session and run manifest initialization, resume decisions, stage -transitions, invalidation, terminal-failure persistence, and post-publish -cleanup. A deeper pass should look for existing lifecycle boundaries that could -be made explicit without redesigning the state machine during extraction. - -`internal/app.Plan` is approximately 110 lines with cyclomatic complexity 27 -and cognitive complexity 51. More importantly, it independently models parts -of the run/skip, resume-validation, stale-marking, and invalidation behavior -also present in execution. The audit should examine whether a shared, pure stage -transition model could reduce drift between planning and execution. - -### Analyze Orchestration - -The analyze implementation has two notable orchestration functions: - -- `analyzeStage.Run` is approximately 252 lines with cyclomatic complexity 33 - and cognitive complexity 47; and -- `executeAnalyzeArtifact` is approximately 259 lines with cyclomatic - complexity 28 and cognitive complexity 39. - -`internal/stage/analyze.go` combines setup, catalog construction, -reconciliation, planning, per-artifact execution, state projection, dependency -invalidation, reuse aggregation, evidence registration, and result assembly. -Possible seams to investigate include execution-state initialization, -current/reused record projection, a per-artifact transaction, evidence -registration, and final result accumulation. - -### Configuration Validation And Composition - -`internal/config/validate.go` contains approximately 1,100 lines and has the -highest aggregate file complexity observed: cyclomatic complexity 264 and -cognitive complexity 394 across its functions. Notable functions include: - -- `validateScriptorium`: approximately 117 lines, cyclomatic complexity 32, - cognitive complexity 74; -- `validateNotarius`: approximately 129 lines, cyclomatic complexity 29, - cognitive complexity 42; and -- `validateAudita`: approximately 54 lines, cyclomatic complexity 21, - cognitive complexity 34. - -These scores are partly driven by explicit validation guards. A deeper review -should determine whether artifact-level, dependency-graph, path/output, -input-source, and variable validation can be separated while preserving precise -field-specific errors. - -Composition complexity is concentrated in `internal/config/composition.go` and -`internal/config/pipeline_composition.go`. In particular, -`splitPipelineCompositionEnvelope` is approximately 87 lines with cyclomatic -complexity 21 and cognitive complexity 63. It combines YAML shape checking, -source-aware diagnostics, profile parsing, default-profile validation, cloning, -and removal of composition metadata. Structural decoding and semantic -validation may be separable, but the audit should preserve source provenance -and field ownership. - -### Previous-Session Cache - -`internal/previouscache.Resolve` is approximately 180 lines with cyclomatic -complexity 29 and cognitive complexity 60, within a package of only about 483 -production lines. It combines configuration checks, current-state loading, -required and optional behavior, immutable-commit resolution, legacy candidate -probing, local-path construction, record creation, and sorting. Modern -immutable-commit resolution and legacy probing are distinct algorithms and may -form a useful extraction boundary. - -### Additional Areas To Inspect - -Lower-priority complexity concentrations include: - -- `internal/stage/publish.go`, approximately 1,175 lines with aggregate - cognitive complexity 239, where file collection, output resolution, - publication, and commit assembly may be separable; -- `internal/fileops/directory.go`, approximately 555 lines with aggregate - cognitive complexity 184, some of which may be removed by the `os.Root` - evaluation; -- `internal/app/restore_plan.go`, particularly the approximately 129-line - `classifyRestoreAction`; and -- `internal/manifest/analyze_state.go`, whose moderately complex validation may - be justified by its ownership of explicit state invariants. - -On the initial evidence, execution and shared plan/execution transition logic -should receive the first detailed review. Configuration and analyze -orchestration form the next tier, while their higher complexity may more often -reflect detailed domain rules. - -These questions should be revisited with change-history analysis, focused code -reading, and maintenance experience before forming recommendations or creating -work items. diff --git a/docs/roadmap/refactoring.md b/docs/roadmap/refactoring.md deleted file mode 100644 index ffcacfa..0000000 --- a/docs/roadmap/refactoring.md +++ /dev/null @@ -1,579 +0,0 @@ -# Narratio Refactoring Roadmap - -## Status - -Consolidated roadmap for the upcoming refactoring work. This document defines -the approved actionable scope and target state. It does not describe current -behavior and is not a staged implementation plan. - -The complete audit inventory and final dispositions are recorded in the -[Audit Decision Register](audit-decisions.md). Supporting analysis remains in -the three source audits. This roadmap includes only work that at least one audit -identified and that the final decision register classifies for immediate -action. - -## Goal - -Reduce Narratio's implementation and maintenance burden while preserving the -operational outcomes that matter: correct resumable progress, economical reuse -of expensive generated artifacts, coherent remote publication, understandable -cleanup and recovery, safe destructive scope, group-owned workspaces, and -protection of API keys. - -The refactor deliberately permits approved internal and deprecated-contract -changes. It is not limited to producing an implementation mechanically -equivalent to the current one. - -## Governing Boundaries - -- Keep the existing package-level ownership and acyclic dependency direction. - Prefer private types, functions, and focused files over new architectural - layers. -- Treat campaign and session workspaces as trusted, intentionally shareable, - group-owned data. API keys remain sensitive, and cleanup and restore remain - confined to explicit application-owned roots. -- Keep the session manifest as the sole durable local progress authority. -- Preserve strict, deterministic configuration semantics without preserving - incidental diagnostic order, complete wording, punctuation, or private - validation choreography. -- Keep integration-specific requests, validation, and errors with their - adapters. Shared infrastructure should expose only exact stable mechanics. -- Add dependencies selectively when they remove audited complexity and preserve - the required supported-platform behavior. -- Update tests and canonical documentation with the behavior they own rather - than preserving retired implementation mechanisms. - -## Actionable Workstreams - -Each workstream uses one of three status identifiers: `Pending`, `In Progress`, -or `Complete`. Workstreams are numbered in dependency-aware implementation -order. The first workstream not marked `Complete` is the next workstream to -address unless its status or an implementation plan explicitly records a -blocker. - -### Workstream 1 — Go baseline, process supervision, and transcription concurrency - -**Status:** Complete - -**Audit sources:** H11, H19, H22, H24, H33, and L01. - -#### Underlying issue - -The maintained Go baseline prevents use of newer root and process primitives, -the Windows subprocess owner reopens a PID instead of using a lifetime-stable -handle, transcription manually implements bounded cancellation and first-error -coordination, and the shared subprocess package mixes generic process ownership -with Narratio environment, diagnostic, redaction, and file-writing policy. - -#### Required changes - -- Upgrade `go.mod`, generated module state, every maintained CI and release Go - image, and release documentation to Go 1.26 in one bounded change. Go 1.26 - provides `os.Process.WithHandle`; the required `os.Root.RemoveAll` operation - was already available in Go 1.25. -- Validate the upgrade with the repository-wide tests, race checks, vet, - maintained documentation and examples, and all supported release builds. -- Assign the suspended Windows child to its Job Object inside - `os.Process.WithHandle`. Preserve suspended creation, job configuration, - initial-thread resumption, fail-closed cleanup, and job disposal. -- Replace the transcription worker pool with `errgroup.WithContext` and - `SetLimit`. Preserve speaker-qualified failures, parent-cancellation context, - aggregation of successful results under a narrow lock, and deterministic - output ordering. -- Clarify the internal subprocess boundary: the reusable mechanism owns process - tree start, wait, termination, reaping, and typed outcomes; Narratio owns - environment selection, secret-value selection and redaction, bounded log - sinks, paths, modes, diagnostic wording, and tool-specific hints. -- Remove atomic YAML and byte-writing helpers from the subprocess package and - route file replacement through the focused filesystem owner. - -#### Target state - -Narratio builds and tests on Go 1.26, uses the safer native Windows handle API, -and has concise bounded transcription concurrency. Process-tree ownership is -internally separable from Narratio policy without creating or depending on a -new external subprocess library during this refactor. - -### Workstream 2 — Filesystem simplification and restore correctness - -**Status:** Complete - -**Audit sources:** H02, H06, H07, H18, H20, H21, H29, H45, H46, C04, -L02, L05a, and L05c. Supporting filesystem rationale and policy are recorded -in [`fileops.md`](fileops.md); the requirements below are the consolidated -action scope. - -#### Underlying issue - -The current FileOps subsystem promises adversarial race resistance, universal -confinement and no-follow behavior, atomic no-replace directory promotion, and -power-loss durability that Narratio's trusted, session-locked workspace does -not require. The complexity also obscures one retained correctness boundary: -restore classification can follow a symlink and treat it as an already matching -regular file. - -#### Required changes - -- Replace ordinary workspace reads, writes, copies, and renames with direct - `os`, `io`, and `path/filepath` operations near their domain owners. -- Retain small shared helpers only for corruption-resistant same-directory file - replacement, centralized bounded I/O, workspace modes, and explicitly - root-scoped cleanup or restore. -- Keep atomic visibility for manifests and other corruption-sensitive single - files without file or directory syncing or a power-loss durability promise. -- Receive remote downloads into temporary files and validate size, checksum, - format, and any adapter contract before installation. Remove redundant - download-install surfaces. -- Have Notarius write one unique staging bundle inside the invocation workspace; - validate its location and integration contract, normalize workspace modes, - require an absent unique destination, and rename the directory directly. -- Delete directory tree-copy promotion, race hooks, identity rechecks, - no-replace platform code, directory syncing, capability errors, and the - tests that exist solely for those retired mechanisms. -- Use `os.Root` for destructive cleanup and restore. Reject escaped targets and - classify a leaf or ancestor symlink as a non-regular restore conflict in both - force and non-force planning. Hash only a regular target verified within the - root. -- Centralize and preserve group-owned directory and file modes. Keep API-key - values out of permissive workspace files, logs, and errors, and preserve all - explicit credential protections. -- Keep size limits centralized and discoverable, deliberately above ordinary - workloads, with errors that name the limit and enforcing adapter. - -#### Target state - -Ordinary filesystem work is clear standard-library code. Shared filesystem -code is small and policy-focused; extract performs one validated rename rather -than a second full-tree copy; restore and cleanup cannot escape their roots or -follow symlinked targets; group-owned operation remains first-class; and API -keys remain protected without treating ordinary D&D data as private. - -### Workstream 3 — Canonical contracts and compatibility purge - -**Status:** Complete - -**Audit sources:** H43 and C01. - -#### Underlying issue - -Deprecated configuration and state formats remain active runtime branches. -They require alternate parsing, probing, ambiguity handling, fixtures, errors, -and documentation, and prevent one canonical representation from being clearly -authoritative. - -#### Required changes - -- Remove the legacy remote `current/manifest.json` and `current/run_id.txt` - reader. -- Remove legacy previous-session artifact candidates and existence probing; - resolve only artifacts declared by the current committed state. -- Remove unversioned party documents, separate `players_file` handling, and - legacy session-level party or player overrides tied to that model. -- Remove aggregate-only analyze availability, fallback to incidental files, - extraction records without current contract and provenance evidence, and - manifest or artifact branches predating the supported schema. -- Delete associated fields, constants, types, helpers, schemas, tests, fixtures, - examples, and documentation rather than retaining an internal compatibility - facade. -- Require explicit supported versions or unambiguous current authority markers. - Reject old state early with a clear unsupported-version or migration error. -- Provide migration guidance through regeneration or republication without - keeping steady-state migration readers in production. - -#### Target state - -Every supported configuration, party document, local manifest, remote current -snapshot, extraction record, analyze record, and previous-session input has one -canonical resolution path. Deprecated representations are absent from runtime -code and fail clearly instead of being inferred or normalized. - -### Workstream 4 — One durable local manifest authority - -**Status:** Complete - -**Audit sources:** H14, H15, H17, C02, and C03. - -#### Underlying issue - -The session manifest and structured run manifest currently mirror stage and -artifact progress. Keeping both synchronized makes diagnostic persistence part -of pipeline correctness and creates dual-save ordering, projections, rollback, -terminal recovery, and cleanup conflicts. - -#### Required changes - -- Eliminate the structured run-manifest schema, store, paths, validation, - before/after/skip/failure/terminal saves, invocation-only projections, - publish classification, compatibility code, tests, fixtures, and - documentation. -- Retain the run ID for invocation identity, run-local paths, adapter provenance, - and immutable remote publication. -- Retain bounded logs, generated configuration, receipts, immediate command - summaries, and other useful diagnostics without making their persistence a - second progress transaction. -- Make the session manifest the sole local authority for stage status, artifact - currency, resume, status, restore, and publication. - -#### Target state - -One atomic session-manifest update determines durable local progress. Diagnostic -write failure cannot overturn otherwise valid pipeline work, and no invocation -manifest must remain synchronized with the progress ledger. - -### Workstream 5 — Runner lifecycle and pre-execution decisions - -**Status:** Complete - -**Audit sources:** H04, H07, H14, H15, H17, H34, and H35. - -#### Underlying issue - -`executePlan` combines preflight, locking, invocation setup, collaborator -construction, resume decisions, stage transitions, failure persistence, and -cleanup in one high-change function. Planning and execution also independently -sequence some stale marking and invalidation decisions, while application tests -replace package-global functions repeatedly. - -#### Required changes - -- Introduce a private invocation runtime or equivalent cohesive private state - carrying resolved identity, paths, stores, the session manifest, environment, - collaborators, and command summary state. -- Extract lifecycle operations for preparation and lock acquisition, - invocation-state setup, collaborator composition, one-stage execution, and - finalization. -- Keep session-lock acquisition and release, session-manifest persistence, and - the top-level transaction order visible in the lifecycle owner. -- Introduce a pure pre-execution decision value combining recorded status, - force, resume validation, stale reason, and required invalidation. -- Share only manifest-transition helpers that planning and execution apply - identically. Keep planned outcomes distinct from actual stage results and - persistence. -- Add parity tests for run versus skip, non-resumable records, forced - invalidation, self-skip, and changed successful results. -- Replace affected mutable package-global function seams with a small - invocation-scoped dependency set. Do not introduce a general dependency - injection framework. - -#### Target state - -The top-level runner reads as an explicit lifecycle with one visible lock and -persistence authority. Planning and execution agree through one narrow pure -decision model, actual stage outcomes remain execution-owned, and focused tests -do not require process-global substitution. - -### Workstream 6 — Per-artifact analyze checkpointing and recovery - -**Status:** Complete - -**Audit sources:** H04, H14, H15, H36, H37, C02, and C03. - -#### Underlying issue - -Analyze already has useful per-artifact identity and dependency semantics, but -successful results participate in session and invocation projections and are -not expressed as the simple resumable queue operators need. The per-artifact -transaction also mixes preparation, invocation, validation, installation, and -evidence construction behind a broad parameter list. - -#### Required changes - -- Use only the session manifest's analyze artifact collection as durable - authority. Remove invocation projections and their validation, application, - and rollback machinery. -- Execute the effective artifact set in deterministic dependency order. -- After each artifact succeeds and its canonical output is validated and - materialized, update its current record, stale any transitive dependents whose - evidence is invalidated, atomically save the session manifest, and only then - continue. -- If the checkpoint save fails, leave incidental output non-authoritative and - allow safe recomputation on a later run. -- On artifact failure, persist its failed state, stale current transitive - dependents, mark the aggregate analyze stage failed, and stop before later - artifacts. -- On an ordinary later run, retain valid successes, retry the first failed, - stale, or missing selected artifact, and continue after it succeeds. -- Preserve established force and artifact-selection semantics: unqualified - force reruns the configured analyze set; selected force applies to the - selected artifacts and required dependency closure without rerunning - unrelated valid artifacts. -- Retain fingerprints, output evidence, deterministic dependency ordering, - dependency-aware invalidation, and the rule that incidental files cannot - create current authority. -- Give `analyzeStage.Run` a private session-only execution accumulator or - equivalent focused state. Split each artifact transaction into input/request - preparation, adapter invocation, validated materialization, and - record/evidence construction under one wrapper and narrow context. - -#### Target state - -Analyze is a deterministic resumable artifact queue backed by one manifest. -Successful expensive work survives a later failure, invalid dependents cannot -remain current, and ordinary resume continues at the first work that actually -needs attention. - -### Workstream 7 — Configuration structure and semantic diagnostics - -**Status:** Complete - -**Audit sources:** H04, H14, H15, H17, H38, H39, H40, H41, H42, C01, and C06. - -#### Underlying issue - -Configuration behavior is appropriately strict, but unrelated validation -domains share one large file, several functions combine distinct phases, legacy -fields remain active, and tests treat exact diagnostic choreography as though -it were a versioned semantic contract. - -#### Required changes - -- Complete the legacy configuration purge defined by the canonical-contract - workstream. -- Split validation within package `config` into core pipeline/session, - external-adapter, publish/storage, and Scriptorium artifact/dependency - domains. -- Separate Scriptorium artifact declaration and output checks, - dependency/input checks, and variable checks. Traverse declarations - deterministically where ordering affects reproducibility, without promising - one particular independent-error discovery order. -- Separate Notarius reference-binding normalization from output-lane - normalization and collision checks; retain enablement and scalar checks in - the top-level validator. -- Move the cohesive Audita validator with the other adapter validators without - fragmenting its straightforward guards. -- Split composition-envelope shape decoding, cross-field semantic validation, - and removal of root-only metadata while retaining Narratio's product-specific - composition model. -- Preserve strict YAML decoding, unknown-field rejection, additive imports with - no duplicate ownership, deterministic profile selection, meaningful overlay - behavior for explicit values and collections, centralized defaults, canonical - party and artifact-family resolution, and secret-free effective digests. -- Preserve actionable field identification and enough source information to - correct composition conflicts. Stop preserving complete prose, punctuation, - formatting, validation call order, or unused per-node provenance. -- Rewrite brittle tests to assert typed categories, field paths, structured - details, or the smallest stable semantic fragment. Retain exact strings only - for intentionally stable CLI or machine-consumed contracts. - -#### Target state - -Configuration remains strict, deterministic, and operator-friendly, but its -implementation is organized by semantic owner and can improve diagnostics -without treating private ordering or wording as compatibility behavior. - -### Workstream 8 — Remote publication, per-artifact locks, and publish decomposition - -**Status:** Complete - -**Audit sources:** H04, H15, H18, H44, and C05. - -#### Underlying issue - -Publish contains several cohesive concerns in one large file, while remote -locks use a shared mutable YAML control plane with static precedence, merging, -conditional replacement, and retry machinery. Omitting a locked artifact from -a new immutable snapshot can also make the canonical artifact disappear rather -than keep it frozen. - -#### Required changes - -- Split publish source discovery and opening, output-rule resolution, transfer - preparation and verification, immutable upload, and commit/pointer encoding - into focused files while retaining `publishStage.Run` as the transaction - owner. -- Replace static pipeline locks and the central remote lock manifest with one - versioned JSON lock object per normalized stable source ID at the approved - hashed key. Validate that each payload source hashes to its object name. -- Remove static/remote merging, precedence, configuration injection, shared - document replacement retries, and the retired lock schema and tests. -- Extend the narrow object-store port and each implementation or fake with the - deletion operation needed to remove one lock. -- Adapt lock list/add/remove behavior to independent objects: list validates the - prefix, add fails if present unless force replaces metadata, and remove treats - absence as idempotent success. -- Check each relevant lock before spending work to publish its artifact and - recheck immediately before advancing the current pointer. -- Build a new commit so an unlocked artifact uses its newly uploaded - declaration, while a locked artifact carries the exact prior immutable - declaration when one exists. A locked artifact never previously published may - remain absent without becoming a required-output error. -- If the final recheck discovers a newly added lock, do not advance the pointer; - allow unselected immutable attempt objects to remain and rebuild on the next - run. A lock removed after the initial read may conservatively carry the prior - declaration once. -- Allow carried declarations to reference an earlier immutable run prefix in - the same campaign and session, retaining source identity, originating run, - destination, checksum, size, and generation. Readers must use only objects - declared by the selected commit and must not scan mutable prefixes. -- Distinguish newly published, carried-forward locked, - locked-but-never-published, optional missing, and unselected outcomes in - publish metadata and inspection. -- Preserve the visible transaction boundary: immutable objects, immutable - commit, final lock revalidation, then one mutable current-pointer write. - -#### Target state - -Publish remains one coherent immutable-snapshot transaction with navigable -internal files. Each lock is an independent remote object, changes to one lock -cannot overwrite another, and later snapshots retain the exact prior canonical -version of a locked artifact while unlocked artifacts advance. - -### Workstream 9 — Cleanup as the final pipeline stage - -**Status:** Complete - -**Audit sources:** H20, H21, C02, and C04. - -#### Underlying issue - -Local deletion currently occurs as a post-publish transaction with a special -manifest obligation. A cleanup error can therefore make a successfully -committed remote publication appear to have failed, and the extra state model -duplicates ordinary stage retry behavior. - -#### Required changes - -- Add `cleanup` as the final canonical stage, after every stage that may need - run-local data or diagnostics. -- Remove post-publish cleanup from the runner and delete the - `post_publish_cleanup` manifest model, persistence, compatibility code, - tests, and documentation. -- Use existing cleanup configuration to select spool audio, run workspace data, - both, or neither. -- Require evidence of a successful current remote publish before deleting data - whose retention depends on publication, and bind targets to that published - run rather than inferring arbitrary paths from current configuration. -- Self-skip clearly when nothing is configured, treat already-absent eligible - targets as success, use the filesystem workstream's root-scoped safeguards, - and produce no canonical artifact output. -- Record cleanup through ordinary session-manifest stage status. A cleanup - failure must leave publish success and remote current state unchanged; an - ordinary later run should retry cleanup while reusing valid upstream stages. -- Include cleanup in the fixed invalidation relation so a changed or forced - prerequisite makes an earlier cleanup result stale where appropriate. -- Ensure a selected range ending before cleanup performs no deletion, while a - default full run includes cleanup. - -#### Target state - -Remote publication and local deletion are distinct, understandable outcomes. -Publish succeeds when its pointer becomes current; cleanup succeeds when its -eligible local targets are absent. Cleanup failure is reported and retried as a -cleanup-stage failure without republishing unchanged data. - -### Workstream 10 — Quality, testability, and documentation alignment - -**Status:** Complete - -**Audit sources:** H07, H08, H12, H13, H17, H18, H23, H25, C01, C02, C03, -C04, C05, and C06. - -#### Underlying issue - -Most tests protect meaningful behavior, but some preserve mechanisms and -contracts now approved for removal, some application tests rely on mutable -global seams, and large structural failures can be difficult to diagnose. -Canonical documentation also describes several filesystem mechanisms as -durable policy. - -#### Required changes - -- Delete tests, fixtures, hooks, and helpers only when their underlying legacy - format, run-manifest model, cleanup obligation, central lock store, or FileOps - guarantee is removed. -- Add or retain focused behavior coverage for atomic visibility, bounded limits - and adapter errors, group modes, validated downloads, Notarius rename, - destructive scope, restore symlink conflicts, secret redaction, session-only - progress, per-artifact checkpoints, planning/execution parity, lock - carry-forward, and idempotent cleanup. -- Prefer real temporary filesystems, fake object stores, and controlled helper - processes at their current durable boundaries. Do not recreate removed - algorithms through private test hooks. -- Replace affected mutable application globals with the invocation-scoped - dependency set defined by the runner workstream. -- Use `cmp.Diff` for selected large composed configurations, analyze state, - restore plans, and publish mappings. Use direct or standard-library equality - for simple values. -- Apply `slices` and `maps` only to shallow clones, membership checks, and - deterministic key handling in code already being touched, with nil, empty, - and ordering semantics verified. -- Update architecture, operations, troubleshooting, integration, internal, - configuration, CLI, examples, and release documentation only where each is - the canonical owner of changed behavior. Remove retired claims rather than - maintaining compatibility narratives as current behavior. -- Keep repository-wide tests and vet mandatory throughout. Apply focused race, - documentation/example, and cross-platform release-build checks in proportion - to each workstream, with the complete release-candidate validation at the - integrated boundary. - -#### Target state - -Tests protect retained outcomes and failure boundaries instead of obsolete -mechanisms. Collaborators are injectable per invocation without a framework, -large diffs are readable where useful, and canonical documentation accurately -describes the simplified implemented contracts as each workstream lands. - -## Dependency Relationships - -These relationships constrain a future implementation plan without defining -its numbered stages: - -- The Go 1.26 baseline is required before relying on the process-handle API; - the root-removal API is already present in the current Go 1.25 baseline. -- The compatibility purge should remove legacy branches before maintainers - invest in decomposing the corresponding current-only previous-cache, - configuration, analyze, and publication paths. -- Removing the run manifest establishes the session-only authority needed by - runner decomposition, per-artifact analyze checkpointing, and the ordinary - cleanup stage. -- Runner lifecycle decomposition and its pre-execution decision model provide - the clean stage boundary used by analyze recovery and cleanup retry. -- The per-artifact lock representation and carry-forward rules must be reflected - together in publish construction, commit validation, current-state reading, - operator commands, and tests. -- Filesystem simplification and root-scoped destructive helpers must be - available to the cleanup stage, while restore correctness belongs in the same - filesystem change rather than the old facade. -- Test and documentation alignment accompanies the workstream that changes the - behavior; it is not deferred to a final bulk cleanup. - -## Compatibility and Release Intent - -The compatibility purge intentionally rejects deprecated formats rather than -silently migrating them. Migration guidance should direct users to regenerate, -republish, or otherwise replace old state. No runtime compatibility layer should -survive solely to make the internal refactor invisible. - -The approved audit decision is that this combined work remains eligible for a -minor release because the removed compatibility surfaces were already -deprecated and explicitly removable. Preparing or publishing that release is -not part of this roadmap. - -## Consolidated Target State - -After this roadmap is implemented: - -- Narratio has one canonical configuration and state model with no deprecated - runtime fallback paths; -- the session manifest alone owns durable local progress; -- analyze preserves each valid successful artifact and resumes failed, stale, - or missing work in dependency order; -- application orchestration has explicit private lifecycle phases and one - narrow planning/execution decision model; -- ordinary filesystem work uses the standard library, while small focused - helpers retain atomic visibility, bounds, modes, destructive scope, restore - safety, and secret protection; -- Notarius output is installed by one validated rename without a duplicate tree; -- publish owns coherent remote publication, per-artifact locks preserve prior - canonical declarations, and cleanup is a separate final stage; -- configuration remains strict and deterministic without freezing incidental - diagnostic choreography; -- Windows subprocess ownership and transcription concurrency use the audited - Go 1.26 primitives, and subprocess mechanism is internally distinct from - Narratio policy; and -- tests and documentation protect and describe the reduced observable contract. - -## Open Questions - -None. The audits and subsequent approved decisions resolve the product and -architecture choices required to prepare a staged implementation plan.