diff --git a/docs/roadmap/audit-decisions.md b/docs/roadmap/audit-decisions.md new file mode 100644 index 0000000..940b330 --- /dev/null +++ b/docs/roadmap/audit-decisions.md @@ -0,0 +1,148 @@ +# 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 index 8a4a0be..2c2aac3 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -33,23 +33,25 @@ Verdicts use these terms: 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, durability, recovery, -and integration requirements. +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. Restore planning must inspect existing local targets without following - symlinks. `classifyRestoreAction` currently uses `os.Stat` and path-based - hashing, so a symlink to a regular file can be classified as `skip_same` - despite the documented restore and path-confinement contract. -2. `internal/fileops/directory.go` contains a second directory-promotion and - tree-copy implementation used only by tests. Delete that alternate - implementation and test the production `os.Root` path. Then remove - identity-race hooks and rechecks that exist solely for an adversarial - concurrent-filesystem threat model, while retaining static symlink checks, - root confinement, regular-file restrictions, atomic no-replace promotion, - durability, workspace modes, and cleanup guardrails. +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 @@ -68,8 +70,10 @@ maintainability improvements: creating a general dependency-injection framework. The audit does **not** recommend package-wide reorganization, a CLI framework, -an external flock library, generic configuration or atomic-file frameworks, -generic subprocess machinery, or new identifier/retry/storage abstractions. +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 @@ -87,16 +91,19 @@ than command wrapping, and the observed size maps to documented responsibilities **Verdict:** Confirmed as a refactoring constraint. -**Recommended change:** Preserve accidental path-escape and destructive-action -guardrails, but remove tests and identity rechecks whose only purpose is to -defeat a concurrent adversary replacing directory entries after inspection. -Update internal file-operation documentation when that distinction changes. +**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 still needs safe scope, static symlink rejection, -regular-file checks, atomic updates, and crash durability. It does not need to -prove that every inspected inode remained at the same name in the presence of -an attacker racing each operation. `os.Root` already prevents rooted operations -from escaping their root. +**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 @@ -138,36 +145,42 @@ The high interaction count reflects the application boundary's intended role. ### H06 — Confined-filesystem and directory-promotion complexity -**Verdict:** Confirmed, with a concrete duplication issue. +**Verdict:** Confirmed more broadly than the preliminary hypothesis. -**Recommended change:** Delete the test-only `promoteDirectory`, -`promoteDirectoryWithHooks`, `copyRegularTree`, `copyRegularDirectory`, and -`copyRegularFile` implementation family from production code. Exercise -`PromoteDirectory` and its actual handle-relative install path in tests. Remove -adversarial replacement hooks and redundant `SameFile` rechecks where `os.Root` -already supplies the required confinement, while retaining the application -semantics listed in H21. +**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 first half of `directory.go` is the real `os.Root`-based -implementation; the second half reimplements the same tree copy for injectable -tests. Tests of the alternate algorithm can pass while production changes, and -the duplication accounts for a substantial portion of the file's size and -complexity. +**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 invariant coverage. Remove tests -of the alternate directory implementation, consolidate repeated fixture setup -within individual packages, and replace app-level mutable global function seams -with invocation-scoped dependencies as related code is touched. +**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 duplicated filesystem test machinery; -the overall test-to-production ratio is not itself a defect. +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 @@ -281,28 +294,31 @@ not the stage/adapter boundary. **Verdict:** Mostly healthy, with focused opportunities. -**Recommended change:** Retain behavior-level filesystem, manifest, subprocess, -and recovery tests. Remove tests tied to the alternate fileops algorithm; use a -small dependency struct instead of mutable app globals; use structural diffs for -large values where useful. +**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 and package-global function substitution, not from the suite as -a whole. +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:** Already satisfied, except for one implementation mismatch. +**Verdict:** Confirmed for filesystem policy; otherwise already satisfied. -**Recommended change:** Fix restore target inspection as described in H46. -Otherwise retain the current policy-to-internal-document structure and keep -transaction-order comments adjacent to runner and publish code. +**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:** Manifest durability, restore recovery, publish commit ordering, -workspace modes, lock ownership, and confined file operations are all described -in owner documents and protected by focused tests. Restore planning's use of -`os.Stat` is inconsistent with those otherwise clear rules. +**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 @@ -310,8 +326,9 @@ in owner documents and protected by focused tests. Restore planning's use of **Recommended change:** In one dedicated change, update `go.mod`, all Woodpecker images including the pinned release image, release documentation, and generated -module state. Run the full test, race, vet, documentation, example, and six-target -release-build checks required by the release procedure. +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 @@ -320,28 +337,40 @@ separate from refactors. ### H20 — Replace custom confinement with `os.Root` -**Verdict:** Already substantially satisfied. +**Verdict:** Focused opportunity, not a package-wide filesystem model. -**Recommended change:** Standardize remaining fileops paths on the existing -`os.Root` primitives and delete duplicate compatibility/test paths. Do not -rewrite already-rooted code merely to rename helpers. +**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:** Current confined writes, reads, cleanup, download installation, -and production directory promotion already use `os.Root`. The remaining value -is simplification and consolidation, not an initial migration. +**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:** Retain. +**Verdict:** Not substantiated as stated; retain only focused product policies. -**Recommended change:** Keep atomic replace and no-replace promotion ordering, -file and directory syncing, unsupported-platform errors, workspace modes, -bounded reads, regular-file restrictions, proper-descendant cleanup checks, and -application-owned destructive-operation gates. +**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:** These are observable Narratio contracts that `os.Root` does not -provide. Generic filesystem packages would either weaken them or require a -similar application-specific layer around the dependency. +**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 @@ -432,14 +461,20 @@ still requiring the existing semantic checks. ### H29 — Generic atomic-file package -**Verdict:** Not substantiated. +**Verdict:** Focused opportunity, but no new dependency is presently justified. -**Recommended change:** Retain and simplify `internal/fileops` rather than -replacing it. +**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:** Narratio requires explicit modes, file and parent syncing, -pre-commit cancellation, confined parents, downloaded-temp installation, and -clear unsupported guarantees. Generic packages do not own that full contract. +**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 @@ -637,33 +672,42 @@ navigation without distributing transaction ownership. ### H45 — `internal/fileops/directory.go` -**Verdict:** Confirmed; see H06 and H20. +**Verdict:** Confirmed for removal; see H06 and H21. -**Recommended change:** Remove the alternate path-based implementation and its -test hooks, keep one `os.Root` implementation, and simplify adversarial -identity-race checks according to H02. Retain platform-specific handle-relative -no-replace rename and durability checks. +**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:** This is the clearest actual code duplication found. It also -causes tests to exercise a different install implementation from production. +**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:** Pass the session root into classification and inspect an -existing target with `os.Lstat` plus the shared confined regular-file opener. -Hash the already-opened handle rather than reopening a mutable absolute path. -Classify a symlink at the leaf or in an ancestor as -`RestoreConflictNonRegular`, even when its target bytes match. Add tests for -leaf and ancestor symlinks in force and non-force plans. Optionally extract the -legacy remote-download digest comparison after this behavior is fixed. +**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 restore and confined-filesystem contracts. The rest of the function is -an explicit and readable state matrix whose branching is justified. +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` @@ -679,29 +723,31 @@ is easier to audit in one owner. ### H48 — Preliminary priority order -**Verdict:** Confirmed with one correction. +**Verdict:** Confirmed with a broader filesystem correction. -**Recommended change:** Address restore symlink classification first because it -is a correctness issue. Then remove duplicate fileops code and simplify the -threat-model-only machinery. Next decompose execution and share its narrow -planning decision model. Follow with analyze/config, publish/previous-cache, and -finally modernization/test ergonomics. +**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:** Execution remains the largest change-blast hotspot, and -configuration/analyze remain the next tier. The deeper review elevated the -restore mismatch and fileops duplication because they are concrete rather than -metric-only findings. +**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. Correct restore target confinement and add regression tests. +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. Consolidate fileops on the production `os.Root` path and revise its tests and - internal documentation to the trusted-environment threat model. +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 @@ -712,6 +758,7 @@ not staged implementation instructions: 9. Improve app test dependency injection and apply selective `slices`, `maps`, and `go-cmp` cleanup only in code touched by the preceding work. -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. +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 new file mode 100644 index 0000000..8a2e187 --- /dev/null +++ b/docs/roadmap/contract-audit.md @@ -0,0 +1,525 @@ +# 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 new file mode 100644 index 0000000..3bf57b6 --- /dev/null +++ b/docs/roadmap/fileops.md @@ -0,0 +1,273 @@ +# File Operations Simplification + +## Status + +Proposed 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 new file mode 100644 index 0000000..da29337 --- /dev/null +++ b/docs/roadmap/implementation.md @@ -0,0 +1,525 @@ +# Workstream 1 Implementation Plan + +## Purpose And Status + +This document is the executable implementation plan for +[`refactoring.md`](refactoring.md), Workstream 1: **Go baseline, process +supervision, and transcription concurrency**. The refactoring roadmap owns the +approved scope, rationale, and target state; the +[`Audit Decision Register`](audit-decisions.md) records the controlling audit +dispositions. This plan translates that settled direction into six bounded +stages suitable for one `gpt-5.6-terra` coding prompt apiece. + +All stages are pending and must be implemented in numeric order. Workstreams 2 +and later are out of scope. In particular, this plan routes adapter-owned file +replacement through the existing `internal/fileops` owner but does not perform +the broader FileOps simplification reserved for Workstream 2. + +## Settled Implementation Decisions + +The following decisions are final for this implementation: + +- The maintained language baseline is Go 1.26. The module directive is + `go 1.26.0`; ordinary verification and scheduled CI use the official + `golang:1.26` image, while tag CI uses the current explicitly pinned Go + 1.26 patch image, `golang:1.26.7`. The release-workflow documentation check + must assert that same pinned tag. Do not retain a Go 1.25 execution path. +- Go 1.26 is adopted before code relies on `os.Process.WithHandle`. Existing + release semantics, supported build targets, and the rule that tag push—not + asynchronous CI completion—is the synchronous release boundary do not + change. +- `golang.org/x/sync/errgroup` is the only new dependency. Add + `golang.org/x/sync v0.22.0` as a direct dependency when transcription starts + using it, then run `go mod tidy`. Do not add an external subprocess, + worker-pool, filesystem, or error framework. +- `subprocess.Run`, `subprocess.RunRequest`, and `subprocess.RunResult` remain + the adapter-facing contract. Their retained behavior includes environment + allowlisting and overrides, sensitive-value redaction, separate or shared + bounded diagnostics, timeouts and cancellation, process-tree termination, + reaping, exit information, error wrapping, and tool-specific diagnostic + hints. +- Split the existing subprocess implementation inside the current + `internal/adapters/subprocess` package. Do not create a public API, a new + repository package, or an external module. A private execution mechanism + owns process-tree creation, start, wait, interruption, bounded termination, + reaping, disposal, timestamps, exit state, and typed termination outcomes. + The existing `Run` boundary remains the Narratio policy wrapper and owns + request validation, timeout construction, environment selection, command + arguments and working directory, diagnostic paths and modes, bounded and + redacted sinks, public result mapping, diagnostic wording, and hints. +- Represent the mechanism result with a private outcome type and private + reason enum that distinguish: not started/start failure, natural exit, + parent cancellation, deadline expiry, and external abort (currently a + diagnostic-size limit). Preserve the underlying start, wait, context, abort, + and cleanup errors as separate fields so the policy layer can retain + `errors.Is`/`errors.As` behavior and join secondary cleanup failures without + replacing the primary classification. +- The mechanism accepts an already policy-configured `*exec.Cmd` and a generic + `<-chan error` abort signal. It must not import `internal/fileops`, inspect + environment variables or secrets, open diagnostic paths, know diagnostic + byte limits or owners, marshal configuration, or construct Narratio-facing + error text. The policy wrapper must not directly start, wait for, kill, or + reap the command. +- On Windows, continue creating the process suspended, configure a + kill-on-close Job Object, assign the child before resuming its initial + thread, and fail closed on every setup error. Perform assignment inside + `cmd.Process.WithHandle`, converting the callback's `uintptr` to + `windows.Handle` only for `windows.AssignProcessToJobObject`. Capture the + callback's assignment error separately from the error returned by + `WithHandle`. Remove the PID-based `windows.OpenProcess` and its associated + process-handle close; do not change the thread-discovery/resumption or job + disposal behavior. +- Remove `WriteYAMLAtomic` and `WriteFileAtomic` from the subprocess package. + Each of `audita`, `scriptorium`, and `seriatim` owns a small private + `writeGeneratedYAML` helper that marshals its generated configuration with + `gopkg.in/yaml.v3` and installs the bytes through + `fileops.WriteFileAtomic` using `fileops.WorkspaceFileMode`. Fake-adapter + byte placeholders call `fileops.WriteFileAtomic` directly. Do not create a + new generic adapter-I/O package merely to preserve the old wrapper. +- Generated configuration must continue to omit API-key values. Preserve the + current schemas and semantic content; incidental YAML key order and exact + wrapper punctuation are not contracts. +- Replace the transcription channel/worker/first-error implementation with + `errgroup.WithContext` and `SetLimit`. Submit one closure per prepared audio + job, use the errgroup context for every WhisperX call, and return + speaker-qualified adapter, authoritative-output, and JSON-validation errors + directly from that closure. Errgroup cancellation replaces the manual + cancel-on-first-error path. +- The transcription mutex protects successful shared aggregation only. It + must not coordinate errors or adapter execution. Canonical materialization + still begins only after every job succeeds and still occurs in sorted speaker + order. A parent cancellation returns an error wrapping the parent context + error and reports completed versus planned work; it does not need to retain + the current incidental “dispatched” count. +- Tests protect observable outcomes and cross-platform compilation, not the + exact internal helper graph, exact full diagnostic strings, or the literal + syscall sequence. Native Windows behavior must not be claimed where only a + Windows cross-compile is available. +- The original audits and decision records are historical inputs. Do not + rewrite them during implementation. Mark Workstream 1 `Complete` in + `refactoring.md` only after all six stages and final validation succeed. + +## Instructions For Every Stage + +Before changing files in each stage: + +1. Read `docs/development.md`, every document under `docs/policy/`, this plan, + and Workstream 1 plus the dependency and target-state sections of + [`refactoring.md`](refactoring.md). +2. Consult the relevant entries H11, H19, H22, H24, H33, and L01 in + [`audit-decisions.md`](audit-decisions.md). Use + [`library-audit.md`](library-audit.md) only for its subprocess-incubation + rationale; its “extract later” possibility is not current scope. +3. Inspect the current implementation and focused tests named by the stage. + Prefer the codebase knowledge graph for Go discovery and use text search for + YAML, shell, Markdown, literals, and generated configuration. +4. Confirm the worktree state and preserve unrelated changes. Do not begin a + later workstream merely because adjacent code would also benefit from it. + +During every stage: + +- Keep package ownership and dependency direction consistent with + `docs/policy/architecture.md`. Prefer private types and focused files to a new + architectural layer. +- Keep API keys out of arguments where the adapter contract already uses the + environment, and out of generated configuration, logs, errors, fixtures, and + test output. Ordinary workspace files retain group-owned modes. +- Preserve supported Linux, macOS, and Windows behavior. Use build constraints + for platform-specific process code and do not add a permissive unsupported- + platform fallback. +- Keep tests deterministic, offline, and behavior-focused. Exercise configured + and observed concurrency, retain `errors.Is`/`errors.As` assertions where + meaningful, and avoid freezing complete error wording or private helper + choreography. +- Update canonical current-behavior documentation in the same stage that makes + a behavior current. Roadmap documents may describe future work; canonical + docs may not. +- Run `gofmt` on changed Go files, `go mod tidy` when module state changes, + focused tests while iterating, and `git diff --check` before completing the + stage. Mark a stage `Completed` only after all of its exit criteria pass. + +## Stage 1 — Establish The Go 1.26 Baseline + +**Status: Pending** + +### Goal + +Move every maintained build and release surface to Go 1.26 before introducing +Go 1.26-only process APIs. + +### Required Work + +1. Change the module directive in `go.mod` from `go 1.25.0` to + `go 1.26.0`. Run `GOWORK=off go mod tidy` with Go 1.26 and commit any + resulting `go.mod`/`go.sum` changes. Do not add `x/sync` in this stage; it + becomes a used direct dependency in Stage 5. +2. Update every Go step in `.woodpecker/verify.yml` and + `.woodpecker/shuffle.yml` to `golang:1.26`. Update both Go steps in + `.woodpecker/release.yml` to `golang:1.26.7`. Preserve workflow triggers, + dependency ordering, commands, release-plugin configuration, and the + asynchronous release responsibility boundary. +3. Update `internal/doccheck/doccheck_test.go` so the release workflow contract + expects `golang:1.26.7` for validation and asset construction. Keep this + check semantic: it should still validate the shared candidate checker, + asset-builder delegation, step dependencies, and release-plugin settings. +4. Update Go-module fixture text under `internal/releasecheck` from 1.25 to + 1.26 where it represents an otherwise valid synthetic module. Do not change + unrelated decimal values such as model/configuration examples. +5. Add a concise Go 1.26 prerequisite to `docs/development.md` and + `docs/release.md`. In the release document, distinguish the maintained + Go 1.26 baseline from the currently pinned tag-CI patch image without + changing the release procedure or promising that automation waits for CI. +6. Search maintained source, workflows, scripts, canonical docs, and semantic + test fixtures for stale Go 1.25 baseline claims. Historical audit prose may + remain historical; controlling roadmap text already records the approved + Go 1.26 decision. + +### Exit Criteria + +- `go env GOVERSION` reports a Go 1.26 toolchain and + `GOWORK=off go mod tidy -diff` succeeds. +- `go test ./...`, `go test -race ./...`, `go vet ./...`, and + `go build ./...` succeed. +- `go test ./internal/doccheck` and + `go test ./internal/config -run '^TestExamplesLoadAndValidate$'` succeed. +- The six CGO-disabled release targets build through + `scripts/build-release-assets.sh` into a new temporary directory and the + host version smoke check succeeds. +- All maintained Go execution surfaces resolve to Go 1.26, and + `git diff --check` succeeds. + +## Stage 2 — Separate Subprocess Lifecycle Mechanics From Policy + +**Status: Pending** + +### Goal + +Make the process-tree lifecycle an internally reusable, typed mechanism while +keeping Narratio-specific environment, diagnostics, redaction, paths, and +error presentation at the existing adapter-facing boundary. + +### Required Work + +1. Add `internal/adapters/subprocess/execution.go`. Move the complete command + lifecycle behind one private function that receives a context, an already + configured `*exec.Cmd`, and a generic `<-chan error` abort signal. The + function must: + - record UTC start/completion times and initialize an unknown exit code; + - reject an already-done context without starting a child; + - create the platform-owned process tree; + - apply the bounded stream `WaitDelay` before start; + - start exactly once through the tree owner; + - wait once in a buffered result channel; + - on cancellation, deadline, or abort, perform the existing bounded + graceful/forceful termination and reaping sequence; + - dispose the tree on every path after creation; and + - return a private typed outcome containing its reason, timing, exit code, + and separate start, wait, context, abort, and cleanup errors. +2. Keep platform-specific process ownership in `process_tree_*.go`. Reduce + `process_tree.go` to the owned-tree interface, shared termination intervals, + and narrowly reusable termination/reaping helpers needed by + `execution.go`. The lifecycle mechanism, rather than `Run`, must be the only + caller of `Start`, `cmd.Wait`, tree termination, and `Dispose`. +3. Change the diagnostic-limit notification channel in `diagnostics.go` to the + generic error channel accepted by the mechanism. Continue retaining the + first concrete `*captureLimitError` separately so policy formatting can name + the stream, diagnostic owner, and configured limit after the command has + been reaped. Preserve the exact byte caps, redaction, tail bounds, shared-log + locking, flush, and close behavior. +4. Refactor `Run` in `run.go` into the policy/composition layer. It must still: + - validate the executable and normalize a nil parent context; + - create the optional timeout context; + - build the allowlisted, overridden child environment; + - select sensitive values and open bounded redacting log sinks; + - construct and configure `exec.Cmd` arguments, working directory, + environment, stdout, and stderr; + - call the private execution mechanism exactly once; + - flush diagnostics, map the mechanism outcome to `RunResult`, and format + the existing useful start/exit/timeout/cancellation/capture/cleanup + categories; and + - preserve stderr-tail redaction, log paths, bad-descriptor hints, and error + chains. +5. Keep `RunRequest` and `RunResult` source-compatible for every adapter. Exact + punctuation is not required, but existing callers and behavior tests must + continue to receive useful executable/argument/path/exit context and + `errors.Is`/`errors.As` access to cancellation, deadline, `exec.ExitError`, + capture, and cleanup causes. +6. Reorganize `run_test.go` and `process_tree_supported_test.go` only where the + new boundary makes ownership clearer. Retain public `Run` coverage for + success, environment filtering/override, secrets, separate/shared logs, + size limits, timeouts, cancellation, nonzero exit, hinting, and descendant + cleanup. Add focused private-mechanism tests only for a typed distinction + not already protected through `Run`; do not duplicate the whole suite or + assert private function names. +7. Update `internal/adapters/subprocess/doc.go` and + `docs/internal/adapters.md` to describe the implemented internal + mechanism/policy split. Keep the package internal and do not describe it as + a released reusable library. + +### Exit Criteria + +- The lifecycle mechanism has no dependency on `fileops`, YAML, environment + selection, secret catalogs/redaction policy, diagnostic paths or modes, or + Narratio-facing error wording. +- `Run` does not directly start, wait, terminate, reap, or dispose a process, + and its public request/result surface is unchanged. +- `go test ./internal/adapters/subprocess` and + `go test -race ./internal/adapters/subprocess` pass on the native supported + host. +- Existing adapter subprocess tests for Audita, Scriptorium, Seriatim, and + Notarius pass without loosening credential or diagnostic assertions. +- `go test ./...`, `go vet ./...`, `go build ./...`, and + `git diff --check` succeed. + +## Stage 3 — Use The Stable Windows Process Handle + +**Status: Pending** + +### Goal + +Eliminate PID reopening during Windows Job Object assignment while preserving +the fail-closed suspended-child lifecycle. + +### Required Work + +1. In `process_tree_windows.go`, keep Job Object creation and + `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` configuration before child start. Keep + `windows.CREATE_SUSPENDED` on the command. +2. After `cmd.Start`, call `cmd.Process.WithHandle`. Inside its callback, call + `windows.AssignProcessToJobObject(job, windows.Handle(handle))` and store + that assignment error in a callback-local variable visible after + `WithHandle` returns. Treat a `WithHandle` error and an assignment error as + distinct setup failures with accurate context. +3. Delete the `windows.OpenProcess` call, the requested process access-mask + constants, and the matching `windows.CloseHandle` for the reopened process. + Never retain or close the handle supplied to the `WithHandle` callback. +4. Resume the initial thread only after successful assignment. Preserve + `resumeInitialThread`, the suspended process's PID-based thread enumeration, + and the existing thread-handle lifetime. +5. On handle acquisition, assignment, or resume failure, kill and wait for the + suspended child, close the not-yet-owned Job Object, and return an error that + preserves the primary setup cause plus cleanup causes. On success, retain + the Job Object in the tree and preserve its kill-on-close disposal behavior. +6. Do not add a test that scans source text for `OpenProcess` or mocks the + standard library. Rely on Go 1.26 Windows compilation for API/type safety + and the existing supported-host process-tree behavior tests for lifecycle + semantics. If a native Windows runner is unavailable, state that limitation + in the implementation-stage handoff rather than claiming runtime coverage. + +### Exit Criteria + +- `process_tree_windows.go` uses `os.Process.WithHandle` for Job Object + assignment and no longer reopens the child PID as a process handle. +- CGO-disabled Windows `amd64` and `arm64` builds succeed. +- A Windows test binary for `./internal/adapters/subprocess` cross-compiles with + `go test -c`; it is not executed on a non-Windows host. +- Native-host subprocess tests, race tests, repository tests, vet, build, and + `git diff --check` succeed. +- The stage handoff accurately distinguishes cross-compilation from native + Windows runtime evidence. + +## Stage 4 — Remove File Writing From The Subprocess Package + +**Status: Pending** + +### Goal + +Make `internal/adapters/subprocess` exclusively about command execution and +move generated configuration and fake-output persistence to the adapters that +own those files. + +### Required Work + +1. Add one unexported `writeGeneratedYAML(path string, payload any) error` + helper within each of the `audita`, `scriptorium`, and `seriatim` adapter + packages. A focused `generated_config.go` file is preferred in each package + so production and fake runners share the same tool-owned serialization. +2. In each helper, marshal with `yaml.Marshal`, wrap marshal failures with the + destination path, and install the bytes through + `fileops.WriteFileAtomic(path, data, fileops.WorkspaceFileMode)`. Preserve a + path-bearing write error and the existing group-owned mode. Do not put API + key values into the payload while moving code. +3. Replace every production and fake use of + `subprocess.WriteYAMLAtomic` with its package-local helper. Replace every + fake use of `subprocess.WriteFileAtomic` with a direct + `fileops.WriteFileAtomic` call, keeping the existing operation-specific + caller context for outputs, reports, stdout, and stderr placeholders. +4. Remove `WriteYAMLAtomic` and `WriteFileAtomic` from + `internal/adapters/subprocess/run.go`, along with now-unused YAML, + filesystem, mode, and fileops imports. Update the package comment so it + claims only process execution/supervision and Narratio subprocess policy. +5. Delete the two generic YAML-writer tests from subprocess coverage. Do not + recreate tests for a trivial private wrapper in all three packages. Instead, + retain the existing Audita, Scriptorium, and Seriatim production/fake + adapter tests that read generated configurations, check placeholder files, + and assert that credentials are absent. Add a focused adapter test only if + one migrated output is not already covered at its stable adapter boundary. +6. Search all Go source to prove no caller or reference to the removed + subprocess writers remains. Do not move or simplify `fileops` itself; that + package is the temporary focused replacement owner until Workstream 2. + +### Exit Criteria + +- The subprocess package imports neither `gopkg.in/yaml.v3` nor + `internal/fileops` and exports no file-writing helper. +- All generated YAML and fake adapter files retain their schemas/content, + workspace mode, overwrite behavior, and contextual failures. +- Audita, Scriptorium, and Seriatim production and fake adapter tests pass, + including credential-absence checks. +- Subprocess tests pass after removing only writer-specific coverage. +- `go test ./...`, `go test -race ./internal/adapters/...`, `go vet ./...`, + `go build ./...`, and `git diff --check` succeed. + +## Stage 5 — Replace The Transcription Worker Pool With Errgroup + +**Status: Pending** + +### Goal + +Use the maintained bounded-concurrency primitive for per-speaker WhisperX work +while preserving cancellation, all-or-nothing canonical materialization, and +deterministic results. + +### Required Work + +1. Add `golang.org/x/sync v0.22.0` as a direct dependency and run + `GOWORK=off go mod tidy`. Import `golang.org/x/sync/errgroup` in + `internal/stage/transcribe.go`. +2. Preserve all pre-concurrency behavior: environment validation, session and + run-layout resolution, prepared-audio discovery, positive/default + concurrency calculation, duplicate speaker rejection, and deterministic job + construction. +3. Replace the explicit `context.WithCancel`, worker count, jobs channel, + `sync.WaitGroup`, dispatch loop, and mutex-protected `firstErr` with: + - `group, groupCtx := errgroup.WithContext(ctx)`; + - `group.SetLimit(concurrency)`; and + - one `group.Go` closure per prepared job, capturing a per-iteration job + value and checking `groupCtx` before beginning expensive work. +4. Each closure calls `env.WhisperX.Transcribe` with `groupCtx`, validates the + authoritative output path, validates the run-local transcript JSON, and + returns immediately on failure. Every returned failure must identify the + speaker; adapter failures should also retain the audio basename, and output + failures should retain the relevant output path. Do not store a shared first + error or call a manual cancel function. +5. On success, build that speaker's metadata locally, then lock only long + enough to add it to `perFile` and increment `completed`. Do not keep a + shared map of output refs: each job's planned run-local path is already in + the immutable job list and can be used after `group.Wait` succeeds. +6. After submissions, call `group.Wait` exactly once. Read `completed` under + the same mutex before constructing any diagnostic. Apply this result order: + - if the parent `ctx.Err()` is non-nil, return a transcribe cancellation + error that reports completed and total jobs and wraps that parent error; + - otherwise, if `group.Wait` returned an error, return its + speaker-qualified failure; and + - otherwise, require `completed == len(jobs)` as an internal completeness + guard. + A closure admitted after errgroup cancellation may return the group context + error without invoking WhisperX; the first originating errgroup error remains + authoritative. +7. Preserve the post-success transaction boundary. Sort speaker IDs, then + materialize each planned run-local output to the canonical transcript path + serially in that order. Preserve ordered outputs, run/canonical path + metadata, per-file metadata, retry/language/timeout metadata, and the rule + that no canonical partial output is installed after a job failure or parent + cancellation. +8. Update `internal/stage/transcribe_test.go` to protect the refactored + outcomes: + - configured concurrency remains the observed maximum and never exceeds the + configured limit; + - a speaker failure is returned with speaker context and cancels/blockades + remaining work without deadlock; + - invalid JSON and authoritative-output mismatch remain speaker-qualified; + - parent cancellation wraps `context.Canceled`, reports completed/planned + context, and installs no partial canonical output; + - an already-canceled context makes no WhisperX request; and + - successful results and metadata remain deterministic by speaker. + Use synchronization channels rather than timing alone where a new failure- + cancellation test needs ordering. +9. Update `docs/internal/stage-transcribe.md` to describe bounded per-job + errgroup execution and all-or-nothing canonical materialization. Keep + `docs/integrations/whisperx.md` implementation-neutral unless a retained + integration contract actually changed. + +### Exit Criteria + +- `transcribe.go` contains no manual worker channel, worker goroutine loop, + shared first-error slot, or manual stage-cancel function. +- `go.mod` records `golang.org/x/sync v0.22.0` directly and + `GOWORK=off go mod tidy -diff` succeeds. +- `go test ./internal/stage -run '^TestTranscribeStage'` and + `go test -race ./internal/stage -run '^TestTranscribeStage'` pass repeatedly, + including with `-shuffle=on -count=10`. +- `go test ./...`, `go test -race ./...`, `go vet ./...`, `go build ./...`, + and `git diff --check` succeed. + +## Stage 6 — Integrated Validation And Workstream Closure + +**Status: Pending** + +### Goal + +Verify the combined baseline, process, adapter-file, and transcription changes +at every maintained repository boundary and close only this workstream. + +### Required Work + +1. Review the complete Workstream 1 diff against this plan and + `refactoring.md`. Remove obsolete imports, helpers, comments, tests, and + documentation claims introduced solely by the replaced mechanisms. Do not + opportunistically implement Workstream 2 filesystem simplification or + external subprocess-library extraction. +2. Confirm canonical documentation is current: + - `docs/development.md` and `docs/release.md` identify Go 1.26; + - `docs/internal/adapters.md` describes the internal mechanism/policy split + and retained process-tree semantics; + - `docs/internal/stage-transcribe.md` describes bounded per-job execution, + cancellation, deterministic ordering, and all-or-nothing canonical + materialization; and + - integration documents retain accurate external contracts without exposing + unnecessary private implementation detail. +3. Search maintained code and configuration for stale Go 1.25 build-image or + baseline assertions, PID-based Windows process reopening, removed + subprocess writer calls, and the old transcription worker machinery. + Exclude historical audit descriptions and unrelated numeric values from + mechanical rewrites. +4. Run the complete release-candidate-equivalent local validation with Go 1.26: + - `GOWORK=off go mod tidy -diff`; + - `gofmt -l` across tracked Go files, requiring no output; + - `git diff --check` and `git diff --cached --check`; + - `go test -count=1 ./...`; + - `go test -race -count=1 ./...`; + - `go vet ./...`; + - `go build ./...`; + - `go test -count=1 ./internal/doccheck`; + - `go test -count=1 ./internal/config -run '^TestExamplesLoadAndValidate$'`; + and + - `scripts/build-release-assets.sh` with a valid synthetic version and a + fresh absolute temporary output directory, proving all six official + CGO-disabled targets compile and the host smoke binary reports that + version. +5. Run the scheduled-CI stress command + `go test -race -shuffle=on -count=3 ./...`. If a failure is genuinely flaky, + diagnose and correct the retained concurrency contract; do not weaken or + skip the test to close the stage. +6. After every check succeeds, change only Workstream 1's status in + `docs/roadmap/refactoring.md` from `Pending` to `Complete`. Leave all later + workstreams pending and leave historical audit decisions intact. Update the + status of all six stages in this document to `Completed` as implementation + history. + +### Exit Criteria + +- All commands above succeed from a clean, Go 1.26 repository checkout. +- Linux/macOS native-host evidence and Windows/macOS cross-build evidence are + reported accurately; no unrun native platform test is claimed. +- No API key value appears in generated files, diagnostics, errors, fixtures, + or the diff. +- Workstream 1's consolidated target state is fully implemented, its status is + `Complete`, every later workstream remains `Pending`, and the worktree is + clean after the implementation-stage commit. + +## Open Questions + +None. The audits, refactoring roadmap, approved Go 1.26 revision, and decisions +above resolve the product, compatibility, dependency, ownership, testing, and +platform choices required to implement this workstream. diff --git a/docs/roadmap/library-audit.md b/docs/roadmap/library-audit.md new file mode 100644 index 0000000..1f93a04 --- /dev/null +++ b/docs/roadmap/library-audit.md @@ -0,0 +1,395 @@ +# 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/refactoring.md b/docs/roadmap/refactoring.md new file mode 100644 index 0000000..7be7e27 --- /dev/null +++ b/docs/roadmap/refactoring.md @@ -0,0 +1,579 @@ +# 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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending + +**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:** Pending (cross-cutting) + +**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.