From f387222fcea42ce1cccbdaad0a12b95d60062778 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 10 Aug 2026 14:53:19 +0000 Subject: [PATCH] Document maintainability audit conclusions --- docs/roadmap/audit-findings.md | 471 +++++++++++++++++++++++++-------- 1 file changed, 363 insertions(+), 108 deletions(-) diff --git a/docs/roadmap/audit-findings.md b/docs/roadmap/audit-findings.md index 94c7859..1e36d74 100644 --- a/docs/roadmap/audit-findings.md +++ b/docs/roadmap/audit-findings.md @@ -75,9 +75,9 @@ These are prioritization signals, not findings: | Signal | Evidence | Assigned review | | --- | --- | --- | -| High fan-in | `app.Error` (207), stage `Run` symbols (151), `app.Execute` (108), `stage.sessionPathsForEnv` (105), `manifest.New` (72), `manifest.MarkStageSucceeded` (56), `app.executeStages` (48), `artifacts.S3SessionPrefix` (41), and `artifacts.SessionWorkDirForCampaign` (37) | Owning behavior stages, then Stage 11 | -| High complexity | `app.executeStages` cyclomatic 54/cognitive 96; `previouscache.BuildPlan` 22/38; `analyzeStage.Run` 20/27; `audita.NewSubprocessRunner` 17/25; `app.SessionInit` 20/21 | Stages 2, 5, 7, 10, then 11 | -| Exact similarity | `app.Analyze`/`app.Publish`, `manifest.Load`/`LoadRun`, `manifest.Create`/`CreateRun`, adapter constructors, and Seriatim fake methods | Owning behavior stages, then Stage 11 | +| High fan-in | `app.Error` (207), stage `Run` symbols (151), `app.Execute` (108), `stage.sessionPathsForEnv` (105), `manifest.New` (72), `manifest.MarkStageSucceeded` (56), `app.executeStages` (48), `artifacts.S3SessionPrefix` (41), and `artifacts.SessionWorkDirForCampaign` (37) | Reviewed through Stage 11; stable ownership seams, interface dispatch, and graph name ambiguity explain the broad signal. `SIM-001` owns the only justified runner extraction. | +| High complexity | `app.executeStages` cyclomatic 54/cognitive 96; `previouscache.BuildPlan` 22/38; `analyzeStage.Run` 20/27; `audita.NewSubprocessRunner` 17/25; `app.SessionInit` 20/21 | Reviewed through Stage 11. `SIM-001`, `SIM-003`, and `ARC-005` own narrow reductions; the remaining branching preserves distinct policy and validation. | +| Exact similarity | `app.Analyze`/`app.Publish`, `manifest.Load`/`LoadRun`, `manifest.Create`/`CreateRun`, adapter constructors, and Seriatim fake methods | Reviewed through Stage 11. Thin command, typed-model, protocol-constructor, and fake similarities are coincidental or deliberately explicit; atomic file writing remains the shared mechanism in `DUP-001`/`DUP-005`. | | Test-heavy hotspot noise | Several test functions and fakes rank highly in transitive-depth and fan-in results | Stage 12; do not infer production risk from the metric | ### Automation And Fixture Inventory @@ -108,7 +108,7 @@ These are prioritization signals, not findings: | 8: ordinary stages | complete | Prepare, transcribe, merge, polish, normalize, trim, and render were traced from resolved inputs through adapters, validation, run-local/canonical outputs, diagnostics, and manifest recording. The focused normal command passed; the required race command reproduced only `TST-001`. Confirmed `COR-017` through `COR-019`; added `ARC-006`, `DUP-006`, `COM-004`, and `TST-008`, and refined `ARC-001`, `ARC-002`, `ARC-005`, `RSK-013`, and scenario 8. | | 9: extraction | complete | Configuration, transcript resolution, fingerprinting, Notarius execution, receipt/index/lane validation, immutable promotion, manifest advertisement, catalog hydration, resume, and explicit analyze/publish consumption were traced as one slice. The exact focused command passed. Confirmed `COR-020`; added `DUP-007`, `SIM-002`, and `TST-009`, and refined `ARC-001`, `RSK-013`, and scenario 3. | | 10: analyze and dependencies | complete | All five source-policy families, seven built-in catalog entries, configured-artifact execution/reuse, dependency validation/order, previous-cache locality, lifecycle, and publish selection were traced as one slice. The exact focused command passed. Confirmed `COR-021` through `COR-024` and `RSK-015`; added `ARC-007`, `DUP-008`, `SIM-003`, `COM-005`, and `TST-010`; and refined `ARC-001`, `ARC-002`, `RSK-013`, and scenario 9. | -| 11: maintainability | not_started | Seeded by graph complexity, similarity, and fan-in signals only. | +| 11: maintainability | complete | Production graph metrics, change coupling, dead-code/static patterns, all structural candidates, efficiency workloads, comments, dependencies, and platform assumptions reviewed. Confirmed the narrow `ARC`, `DUP`, and `SIM` corrections recorded below, merged lifecycle wording and extraction proof candidates, rejected generic abstractions and micro-optimizations, added `SIM-004` and `COM-006`, and passed the full normal test suite and vet. | | 12: test policy | not_started | Seeded by intended owners and baseline execution observations. | | 13: synthesis | not_started | No final ranking or accepted-risk decisions yet. | @@ -1379,6 +1379,55 @@ availability policies differ intentionally, but the shared registration mechanics and source-definition construction are one policy repeated in three places; `DUP-008` assigns a narrow common bootstrap decision to Stage 11. +## Maintainability And Structural Conclusions + +Stage 11 re-ran production-only similarity, complexity, fan, loop-depth, and +change-coupling queries, then traced the resulting owners and callers. The +graph reported no production function with a direct scan-in-loop or +allocation-in-loop flag. High transitive loop depths came from composition and +tests rather than a new credible hot path. Change coupling was dominated by an +implementation and its focused tests, with expected cohesive changes among the +runner, stage registry, configuration, and command wrappers. + +The resulting design rule is to share mechanics and typed evidence, not whole +workflows: + +- `fileops` should own atomic replacement and durable installation mechanics; + manifest serialization, remote acquisition, validation, conflicts, and + reporting remain with their current policy owners; +- `artifacts` should own canonical artifact resolution, catalog bootstrap, and + extraction-bundle evidence; stages retain required/optional, lifecycle, and + publication decisions; +- the runner should expose one narrow terminal-failure transition while keeping + running-versus-terminal ledger order visible; and +- analyze should use a typed execution context and indexed effective plan, but + its five source-policy branches should remain explicit. + +No generic stage, workflow, validator, resolver, or graph framework is +justified. `previouscache.BuildPlan` is long because it visibly separates +required and optional absence, remote-current validation, candidate selection, +and deterministic ordering; extracting those branches without a new owner +would only move complexity. The repeated command wrappers, typed manifest +load/create methods, adapter constructors, and protocol fakes likewise share +shape but not policy. + +The dependency inventory has six direct module dependencies. AWS configuration, +credentials, S3, and Smithy are used by the S3 adapter; YAML is used by strict +configuration and generated adapter files; and `x/sys` supplies native +no-replace directory installation. No dependency can be replaced by the +standard library without losing a current protocol or platform guarantee. +Linux, macOS, and Windows have explicit file-operation implementations; +unsupported systems fail the no-replace capability clearly. The remaining +portability risks are already owned by `RSK-002`, `RSK-003`, and `RSK-011`, not +new dependency findings. + +No benchmarks exist in the repository. That is not a general test defect: the +two confirmed efficiency findings below now state representative workloads and +specific byte-count, latency, allocation, and peak-memory measurements. No +other sorting, copying, map/slice allocation, serialization, adapter +construction, remote-call, filesystem-pass, or goroutine/channel pattern had a +credible workload large enough to justify a performance finding. + ## Confirmed Findings ### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity @@ -2740,6 +2789,20 @@ places; `DUP-008` assigns a narrow common bootstrap decision to Stage 11. be retained for execution, or expose trustworthy digest/version metadata. Document dry-run as having no durable/session mutation unless truly streaming comparison eliminates all temporary writes. +- Scale/current-versus-proposed cost: for `n` same-size non-audio objects with + aggregate body size `B`, planning performs `n` full downloads and reads `B` + remote plus `B` local bytes for checksums. A forced apply of differing bodies + then downloads up to another `B`; current-state discovery also fetches the + pointer and manifest before the listed manifest can be classified again. + Retaining generation-bound verified candidates would keep apply to one body + transfer per changed object rather than two, while dry-run necessarily keeps + its one classification transfer unless trustworthy remote digests are + available. Metadata/list and local checksum costs remain linear. +- Measurement: use a counting/versioned object-store fake with representative + 100 MiB and 1 GiB aggregate bodies to record calls, bytes, and elapsed time for + missing, equal, size-different, and same-size-different objects in dry-run and + apply modes. The repair should demonstrate the lower byte bound without + weakening generation revalidation; no CPU-only microbenchmark is useful. - Test changes: count bytes/downloads for missing, equal, size-different, and same-size-different objects in dry-run and apply modes; assert relational upper bounds rather than exact private call choreography after snapshot design is @@ -2770,6 +2833,19 @@ places; `DUP-008` assigns a narrow common bootstrap decision to Stage 11. multipart content from a per-attempt reopenable audio source, propagate copy errors and cancellation through the request body, and preserve replay across retries without sharing a consumed reader. +- Scale/current-versus-proposed cost: with worker concurrency `c` and average + audio size `s`, request construction retains approximately `O(c*s)` bytes + before network progress, plus multipart overhead, and repeats that allocation + on each retry. A pipe-backed multipart producer with bounded copy buffers + keeps application buffering at `O(c)` while total network I/O remains + necessarily `O(c*s)` per attempt. This matters for ordinary multi-hour audio, + where each speaker track can be hundreds of MiB; it is not a small-allocation + optimization. +- Measurement: add a benchmark or controlled transport test using several + 100 MiB sparse/generated inputs at configured concurrency, reporting + `-benchmem`, peak heap/RSS, time until the transport receives its first byte, + cancellation latency, and retry reopen behavior. The expected win is bounded + peak memory and earlier upload progress, not fewer transmitted bytes. - Test changes: use a blocking/counting reader or transport to prove the request begins before the complete source is buffered, cancellation interrupts body production, retries reopen cleanly, and response/output bounds remain intact. @@ -2814,13 +2890,21 @@ places; `DUP-008` assigns a narrow common bootstrap decision to Stage 11. ## Candidate Register -The remaining candidates require inspection by their named owners. Completed -behavior stages have promoted their confirmed defects and risks into the -register above. +The `TST` entries remain candidates for Stage 12. Structural entries retain +their original locations for traceability, but their Stage 11 classifications +are final for this investigative pass: confirmed recommendations remain for +Stage 13 prioritization, merged entries defer to the named stronger root, and +rejected entries require new evidence before reopening. ### `ARC-001`: `IODecl` is not a complete or consistently classified stage contract - Category: architectural boundary/ownership candidate. +- Stage 11 classification: confirmed. Remove `Declares` and `IODecl` from the + runtime `Stage` interface and implementations rather than expanding an unused + partial model. Focused stage documentation already owns the complete dynamic + contract. If a future planner needs machine-readable contracts, introduce a + purpose-built model for that consumer rather than treating today’s static + path hints as authoritative. - Evidence: `prepare.Declares` lists files it produces under `Inputs`; `analyze.Declares` omits dynamic input families and has no outputs; `publish.Declares` exposes only the manifest; and `notify.Declares` advertises @@ -2839,14 +2923,18 @@ register above. interface as authoritative and make incorrect dependency or readiness decisions. Current likelihood appears low because the method has no production caller. -- Confirmation owners: Stages 8-10 completed ordinary, extraction, and analyze - rows. Stage 11 owns interface purpose/simplification. Smallest plausible - outcome may be clearer - naming/documentation, a complete contract, or removal; do not choose yet. +- Review history: Stages 8-10 completed ordinary, extraction, and analyze rows; + the final removal decision follows from the absent production consumer and + the model’s inability to describe those established contracts. ### `ARC-002`: disabled-stage “skip” terminology spans two different durable outcomes - Category: architectural/lifecycle ownership candidate. +- Stage 11 classification: merged into the documentation root `COM-002`. + Production behavior is coherent: `StageDispositionSkipped` means an executed + self-skip, while zero-disposition no-output results are ordinary success. + A new lifecycle abstraction or state change would obscure rather than repair + that distinction; the remaining defect is inconsistent language. - Evidence: production use of `StageDispositionSkipped` was found only in extraction. Disabled render, absent/no-op analyze, and disabled publish return zero-disposition results with skip metadata, which the runner treats as @@ -2856,20 +2944,24 @@ register above. outcomes clear state, are reconsidered, and invalidate downstream work in the same way. Conversely, changing them to explicit self-skip could break valid pipeline continuation or cleanup semantics. -- Confirmation owners: Stage 2 confirmed the runner truth table. Stage 4 +- Review history: Stage 2 confirmed the runner truth table. Stage 4 confirmed publish's ordinary-success behavior is used deliberately by the cleanup gate, while `COM-002` owns its incorrect “self-skip” documentation. Stage 8 confirmed disabled trim is real successful copy processing and disabled render is deliberately successful with no output; `COM-004` owns render wording. Stage 10 confirmed absent/no-executable analyze is likewise successful with no output so the pipeline can continue and publish's - prerequisite can be satisfied; `COM-005` owns its wording. Stage 11 can now - settle shared lifecycle vocabulary without changing those behaviors by - accident. + prerequisite can be satisfied; broadened `COM-002` now owns the shared + lifecycle vocabulary without changing those behaviors. ### `ARC-003`: committed and local manifests give `current_pointer_written` different meanings - Category: architectural boundary/ownership candidate. +- Stage 11 classification: confirmed. The remote immutable snapshot should not + serialize a postcommit assertion before commit. Prefer deriving commitment + from the loaded pointer, or represent precommit publish metadata separately + from local postcommit execution metadata. Never repair this with a + post-pointer overwrite, which would break pointer-last atomicity. - Evidence: publish must serialize `current/manifest.json` before the commit marker, so `publishMetadataPreview` records `current_pointer_written=false`. After pointer success, the local session and @@ -2881,14 +2973,19 @@ register above. commit as incomplete, while another consumer interprets the local copy as an assertion about remote state. Updating the fixed manifest after pointer would instead violate pointer-last ordering. -- Confirmation owner: Stage 11 should decide whether this is best represented - by distinct precommit/local metadata types, omission from the remote snapshot, - or derivation from loaded pointer identity. Any change must preserve the - protocol correction selected for `COR-004`; do not add a post-pointer upload. +- Remediation boundary: use distinct precommit/local representation, omit the + field remotely, or derive it from loaded pointer identity. Any implementation + must preserve `COR-004`’s pointer-last protocol; do not add a post-pointer + upload. ### `ARC-005`: Audita request and constructor both advertise ownership of static settings - Category: architectural boundary/ownership candidate. +- Stage 11 classification: confirmed. Constructor state should own base URL, + model, transcript description, config/schema paths, retention, and + concurrency. `PolishRequest` should retain invocation paths and the genuinely + per-run module override only. This matches the production runner and removes + fake-only apparent overrides. - Evidence: `PolishRequest` and its integration document carry base URL, model, transcript description, config path, output schema, work-dir retention, and concurrency values. `SubprocessRunner.Run` ignores those request fields and @@ -2901,16 +2998,20 @@ register above. and observes it in the fake, while the production runner silently uses its older constructor value. Future changes may update one representation only and make provenance disagree with the apparent request contract. -- Confirmation owners: Stage 8 found no polish use case for per-invocation - overrides and recommends constructor authority for static settings. Stage 11 - should make that contract explicit and remove redundant request fields; Stage - 12 should align the fake with the chosen contract. +- Review history: Stage 8 found no polish use case for per-invocation overrides + and recommended constructor authority for static settings. Stage 12 should + align the fake with the chosen contract. Do not merge genuinely request-specific paths/modules into static runner construction. ### `ARC-006`: stage authority for adapter-returned output paths is inconsistent - Category: architectural boundary/ownership candidate. +- Stage 11 classification: confirmed. The stage-requested run-local destination + is authoritative. Adapters should either return no path or return exactly the + requested path, and stages should validate that identity before consuming the + file. A redirect contract is not justified by any current adapter and would + expand filesystem authority across the isolation boundary. - Evidence: transcribe requires the adapter result path to equal its requested run-local destination and then validates the requested path. Transformation stages all request run-local paths, but merge, polish, normalize, render, and @@ -2925,15 +3026,18 @@ register above. - Realistic risk: a future adapter version or fake returns a valid stale, canonical, or outside-work path. Depending on the stage, Narratio may consume it, ignore it, or reject it, making tests and isolation guarantees disagree. -- Confirmation owners: Stage 11 should choose requested-path authority (the - current transcribe/production-adapter pattern) or specify a validated redirect - contract, then make result types and shared checks consistent. Stage 12 should - add one contract test across affected fakes/adapters rather than duplicating - every stage case. +- Test boundary: Stage 12 should add one requested-path contract test across + affected fakes/adapters rather than duplicating every stage case. ### `ARC-007`: enabled and selected artifact authority is split across boundaries - Category: architectural boundary/ownership candidate. +- Stage 11 classification: confirmed. Preserve the CLI and catalog’s tested + rule that an explicit selection is a one-invocation execution override; when + absent, `enabled` defines the default set. Compute a typed effective artifact + set before validation, prerequisite collection, catalog composition, and + planning, and validate selected disabled definitions as fully executable. + Publish keeps its distinct role as a filter over configured output rules. - Evidence: catalog registration makes `enabled` authoritative only when no explicit selection exists; a non-empty selection replaces it, and a unit test requires a selected disabled artifact to become executable. CLI validation @@ -2951,12 +3055,11 @@ register above. not require, while an enabled-but-unselected artifact is reported with disabled-output provenance. Future validation/publish changes can widen the disagreement. -- Confirmation owner: Stage 11 should choose and name one rule. If selection is - a one-off override, compute a typed effective set before configuration- - dependent composition/planning and validate selected entries as executable. - If it is only a filter, reject selected disabled entries and intersect the - sets everywhere. Preserve publish's documented rule that built-in/extraction - sources are unaffected, and do not overload availability with executability. +- Remediation boundary: compute the one-off override’s typed effective set + before configuration-dependent composition/planning and validate selected + entries as executable. Preserve publish's documented rule that built-in and + extraction sources are unaffected, and do not overload availability with + executability. ### `TST-001`: full race baseline fails in the concurrent transcribe test @@ -2998,6 +3101,12 @@ register above. ### `DUP-001`: session save duplicates the shared atomic JSON writer - Category: duplicated mechanism candidate. +- Stage 11 classification: confirmed shared mechanism. Both manifest models + should serialize and normalize in their typed owner, then delegate atomic + replacement to the same `fileops` capability while preserving the session + save’s context checkpoint and caller-specific error prefix. Coordinate this + with `DUP-005` and the directory-durability correction in `RSK-002`; do not + create a manifest-generic persistence interface. - Evidence: `LocalStore.Save` contains its own temp/create/write/sync/close/ context-check/rename sequence, while `SaveRun` delegates the same mechanism to `writeJSONAtomically`. Error prefixes differ, but the durability mechanism @@ -3006,14 +3115,17 @@ register above. - Realistic risk: a future durability, cleanup, permission, or platform fix may reach only one manifest type, creating different guarantees for the two ledgers. -- Confirmation owners: Stage 3 should compare filesystem guarantees and Stage - 11 should decide whether sharing the helper preserves useful error context. - The filesystem comparison is complete; the simplification decision remains - assigned to Stage 11. +- Review history: Stage 3 established the shared filesystem guarantees; Stage + 11 confirmed that caller error context can remain in wrappers over one + mechanism. ### `DUP-002`: publish reconstructs the canonical run-manifest path - Category: duplicated path-ownership candidate. +- Stage 11 classification: confirmed duplicated ownership. Publish should use + the canonical artifacts path helper or receive the already-derived path. + Keep the remote `manifest.json` spelling explicit as a publication protocol + constant; only local canonical path construction moves to its existing owner. - Evidence: `internal/stage/publish.go` in `resolvePublishRunManifestSource` joins the literal `manifest.json` to an already-derived run root, while `internal/artifacts` owns @@ -3023,14 +3135,18 @@ register above. without updating publish discovery, causing a completed run to fail publish or upload the wrong record. Current values agree, so this is not a correctness defect at the audited revision. -- Confirmation owner: Stage 11 should decide whether publish should receive the - canonical run-manifest path from its caller/model or use an artifacts helper; - preserve the explicit remote relative name separately if it is a protocol - constant. +- Remediation boundary: publish may receive the canonical path or call the + artifacts helper; preserve the explicit remote relative name separately as a + protocol constant. ### `DUP-003`: sibling-temp download and install mechanics are repeated across restore and prepare - Category: duplicated mechanism candidate. +- Stage 11 classification: confirmed shared mechanism. Add one narrow + destination-confined sibling-temp acquisition/install capability, or compose + a shared sibling-temp creator with the durable install primitive. Object + selection, download validation, force/conflict decisions, cache records, + manifest-last ordering, and reports remain in restore, audio, and prepare. - Evidence: restore's `downloadObjectToSiblingTemp` plus `InstallDownloadedTempFile`, audio's `downloadObjectAtomic`, and prepare's previous-cache loop each create a sibling directory/temp file, close it, @@ -3043,14 +3159,19 @@ register above. download-size fix reaches restore but not prepare/audio, creating different guarantees for the same remote-to-canonical transition. Conversely, sharing the entire workflows would incorrectly merge caller policy. -- Confirmation owner: Stage 11 should consider one narrow sibling-temp download - and durable-install helper/capability while leaving validation, conflict, - cache, manifest-last, and reporting in their current owners. Coordinate with - `DUP-001`/`RSK-002` rather than adding another incomplete atomic writer. +- Remediation boundary: leave validation, conflicts, cache, manifest-last, and + reporting in their current owners. Coordinate with `DUP-001`/`RSK-002` rather + than adding another incomplete atomic writer. ### `DUP-004`: configuration repeats lexical relative-path policy - Category: duplication/ownership candidate. +- Stage 11 classification: rejected as a shared-mechanism extraction. Exact + comparison shows config deliberately rejects every `..` segment, including + `a/../b`, while `pathsafe.NormalizeRelativeDestination` accepts and cleans a + non-escaping occurrence. Config also preserves field-specific errors. A + parameterized shared validator would hide the stricter accepted language for + only three small callers; keep this policy explicit and test its language. - Evidence: `internal/config.validateRelativeSafePath` independently checks absolute paths, cleaned dot/traversal forms, and separators already represented by `internal/pathsafe` primitives, while adding config-specific field/error @@ -3059,14 +3180,18 @@ register above. - Realistic risk: new artifact fields can be accepted by one validator and rejected by another, multiplying the path-policy drift behind `COR-002` and future artifact configuration work. -- Confirmation owner: Stage 11 should compare exact accepted-language and error - contracts and decide whether config should delegate lexical mechanics to a - narrow shared predicate. Do not collapse identifier, filesystem-root, or - artifact relationship policy into one generic validator. +- Rejection boundary: do not collapse identifier, filesystem-root, artifact + relationship, or strict configuration path language into one generic + validator. ### `DUP-005`: adapter packages repeat atomic byte-write mechanics - Category: duplicated mechanism candidate. +- Stage 11 classification: confirmed shared mechanism and the preferred owner + for the `DUP-001` family. Consolidate byte replacement in `fileops`, including + permission, cleanup, destination safety, and directory-sync guarantees; + callers wrap errors with protocol context. YAML serialization, context/state + policy, and output validation remain outside the low-level primitive. - Evidence: `internal/adapters/subprocess.WriteFileAtomic` and WhisperX's unexported `writeFileAtomic` independently implement same-directory temp-file creation, write, file sync, close, chmod, rename, and failure cleanup. Fakes @@ -3076,15 +3201,20 @@ register above. - Realistic risk: the durability or symlink-safe destination repair reaches the canonical fileops path but leaves adapter outputs/configuration with weaker guarantees, or adapter copies drift in permissions and cleanup behavior. -- Confirmation owner: Stage 11 should compare exact context, error, permission, - and dependency constraints and select one narrow mechanics owner. Keep YAML - serialization and protocol output validation local; coordinate with +- Remediation boundary: preserve caller context/error wrapping and keep YAML + serialization and protocol output validation local. Coordinate with `DUP-001`, `DUP-003`, and `RSK-002` rather than creating another utilities package. ### `DUP-006`: singleton transcript stages repeat manifest-first resolution policy - Category: duplication/ownership candidate. +- Stage 11 classification: confirmed duplicated policy. Route the three + singleton source wrappers through typed `artifacts.ResolveSessionArtifact` + identities, retaining only stage-specific provenance/guidance adapters. + Preserve the existing content validation and keep plural raw-transcript + discovery separate. This also removes current drift over whether a missing + manifest candidate is returned or treated as unavailable. - Evidence: `discoverMergedTranscript`, `discoverProcessedTranscript`, and `discoverNormalizedTranscript` each scan one producer's manifest outputs, trim/resolve candidate local paths, deduplicate/sort, stat candidates, select @@ -3095,15 +3225,19 @@ register above. - Realistic risk: restored-path handling, content validation, ambiguity, or provenance changes are applied to the registry resolver and one discovery helper but not the others, producing stage-specific source selection drift. -- Confirmation owner: Stage 11 should compare the singleton helpers with - `artifacts.ResolveSessionArtifact` and either route them through one typed - resolver or retain explicit wrappers over a shared candidate mechanism. Keep - plural raw-directory discovery separate and do not create a generic stage - framework. +- Remediation boundary: retain explicit stage wrappers only for provenance and + guidance over `artifacts.ResolveSessionArtifact`. Keep plural raw-directory + discovery separate and do not create a generic stage framework. ### `DUP-007`: extraction resume and catalog hydration duplicate bundle-evidence policy - Category: duplication/ownership candidate. +- Stage 11 classification: confirmed duplicated policy. `artifacts` should own + one typed extraction-bundle evidence proof whose result distinguishes absent, + obsolete, unsafe, and valid evidence. Resume maps that proof to rerun/error + lifecycle decisions; catalog hydration keeps its all-or-none fail-closed + mapping. Confinement-before-read and exact identity/checksum ordering remain + visible named proof steps, not a generic validation framework. - Evidence: `extractStage.ValidateResume` and `ArtifactCatalog.HydrateExtractionArtifacts` independently reconstruct the producer bundle, receipt identity, exact configured source/index set, @@ -3118,9 +3252,8 @@ register above. enforced by resume but not catalog (or vice versa). Extraction can then be skipped as reusable while its consumers refuse it, or a downstream catalog can expose evidence resume would reject. -- Confirmation owner: Stage 11 should consider one narrow, structured bundle - evidence validator owned by `internal/artifacts` or a purpose-specific model - package. It must accept explicit current definitions and return typed evidence +- Remediation boundary: the `internal/artifacts` evidence validator must accept + explicit current definitions and return typed evidence reasons; resume should map missing/obsolete versus unsafe reasons, while the catalog retains all-or-none fail-closed hydration. Preserve canonical bundle identity, root confinement before reads, exact index/source count, @@ -3130,6 +3263,11 @@ register above. ### `DUP-008`: runtime catalog bootstrap policy is repeated across three consumers - Category: duplication/ownership candidate. +- Stage 11 classification: confirmed duplicated bootstrap policy. Extract one + deterministic definition-registration function for built-ins, configured + definitions, and extraction definitions/evidence. Analyze, publish, and + operator helpers then apply their own explicit executable selection, disk + availability, publication filtering, and rendering policies. - Evidence: `buildAnalyzeRuntimeArtifactCatalog`, `buildPublishRuntimeArtifactCatalog`, and `buildHelperArtifactCatalog` each create a catalog, register built-ins, translate Scriptorium configuration to @@ -3143,7 +3281,7 @@ register above. configured-definition field is registered in one consumer and omitted from another. Analyze can then accept a source that publish/status cannot render, or publication can expose availability analyze classifies differently. -- Confirmation owner: Stage 11 should extract only common deterministic +- Remediation boundary: extract only common deterministic registration/definition bootstrap, returning a catalog that callers enrich with explicit availability/executability policy. Coordinate extraction proof mechanics with `DUP-007` but do not collapse resume, analyze reuse, publish @@ -3322,6 +3460,12 @@ register above. ### `SIM-001`: runner terminalization and persistence ordering lack a narrow owner - Category: simplification candidate. +- Stage 11 classification: confirmed narrow simplification. Introduce a typed + terminal-failure operation that updates session authority first, then the run + audit ledger, and compounds persistence errors without hiding the stage + failure. Keep the running transition separate and visibly run-first, and + route resume-validation failure through terminalization. Do not extract a + generic lifecycle state machine. - Evidence: `executeStages` is 274 lines with cyclomatic complexity 54 and cognitive complexity 96. Much of the length is justified visible state- machine ordering, but session-first terminal save, run-first running save, @@ -3330,12 +3474,17 @@ register above. - Realistic risk: adding another pre-stage or terminal error path can repeat the abandoned-run behavior in `RSK-001`; extracting too broadly could instead hide the critical order. -- Confirmation owner: Stage 11 should consider a small terminal failure helper - or typed transition operation only after Stages 3-10 settle ordering needs. +- Remediation boundary: use only the typed terminal failure operation described + above; Stages 3-10 established that broader transition extraction would hide + required ordering. ### `SIM-002`: extraction resume validation conflates distinct evidence decisions - Category: simplification candidate. +- Stage 11 classification: merged into `DUP-007`. The shared typed evidence + proof is the smaller control flow: named ordered proof steps reduce resume + complexity while leaving lifecycle mapping in `ValidateResume`. A second + abstraction would duplicate that owner. - Evidence: `extractStage.ValidateResume` spans 154 lines with cyclomatic complexity 40 and cognitive complexity 53. It validates environment and record state, producer identity, parsed/resolved configuration and @@ -3346,8 +3495,8 @@ register above. - Realistic risk: adding `COR-020`'s input evidence or sharing catalog mechanics can place an I/O check before confinement, accidentally turn an unsafe state into an automatic replacement, or omit one exact-set/identity check. -- Confirmation owner: Stage 11 should extract only named evidence decisions or - a small typed proof result, preferably alongside `DUP-007`. Preserve visible +- Merge boundary: `DUP-007` should extract only named evidence decisions or a + small typed proof result. Preserve visible ordering: cheap manifest/producer and fingerprint rejection; canonical root derivation and component confinement before filesystem access; receipt and exact index/source identity; contract/provenance; then regular-file/checksum @@ -3358,6 +3507,12 @@ register above. ### `SIM-003`: analyze execution passes a wide context through repeated resolution branches - Category: simplification candidate. +- Stage 11 classification: confirmed narrow simplification. Build a typed + analyze execution context containing immutable session/run paths, diagnostics, + manifest, catalog, and an indexed effective plan. Have each explicit source + branch return a typed resolution result containing path, provenance, optional + absence, or unavailability. Remove unused parameters and the uncalled helper; + retain distinct policy/guidance and visible dependency/output commit order. - Evidence: `executeAnalyzeArtifact` spans 239 lines and accepts ten parameters for environment, manifest, session paths/identity, run layout, plan, transcript diagnostics, and mutable catalog state. `resolveScriptoriumInput` @@ -3371,16 +3526,39 @@ register above. makes test fixtures construct states that production never uses. A generic resolver abstraction would create the opposite risk by erasing intentionally different source policies. -- Confirmation owner: Stage 11 should consider a small typed analyze execution - context, an indexed effective plan, and a resolution result carrying path, +- Remediation boundary: use a small typed analyze execution context, an indexed + effective plan, and a resolution result carrying path, optional absence, provenance, and typed unavailability. Keep policy dispatch explicit and source-specific guidance close to its producer. Remove the dead helper and unused parameters if confirmed; do not create a generic stage or graph framework, and coordinate set authority with `ARC-007`. +### `SIM-004`: three private path-resolution helpers are dead + +- Category: confirmed dead-code simplification. +- Evidence: production-only zero-inbound graph search followed by exact text + search found `stage.resolveInputPathForRead`, + `stage.resolvePublishSessionRoot`, and `app.resolveSessionConfigPath` only at + their definitions. Their active replacements are source-family resolution, + the publish session-path model, and `resolveSessionConfigPathWithCandidates`. + No interface, reflection, registration, test, or command path refers to them. +- Stage 11 classification: confirmed. Delete the three helpers when structural + remediation begins. Do not preserve wrappers for hypothetical callers inside + `internal` packages; their distinct fallback behavior otherwise suggests + authority that production does not use. +- Realistic risk and scope: low direct risk and tiny deletion scope, but the + unused fallbacks distract reviews of already-sensitive path authority and + made the analyze execution surface appear broader than it is. Existing + focused stage/app tests are sufficient after deletion. + ### `COM-001`: dual-ledger save order and partial-failure policy lack rationale - Category: comment/clarity candidate. +- Stage 11 classification: confirmed. After `SIM-001`, preserve one concise + why-comment at the transition owner: session state is resume authority, the + run manifest is the invocation audit ledger, running must be durable before + execution, and terminal disagreement must remain visible. A comment that + merely restates save order is insufficient. - Evidence: the runner visibly saves run-before-session for `running` and session-before-run for terminal outcomes, but no local comment explains which file is authoritative, why the order differs, or how partial state is meant @@ -3388,13 +3566,18 @@ register above. - Realistic risk: a maintainer may make the calls symmetrical or reorder them, inadvertently allowing a stage to execute without a durable running session transition or preferring an audit record over resume authority. -- Confirmation owner: Stage 11, after any `SIM-001` restructuring. Prefer a - named operation that makes the invariant obvious; add a concise rationale - only where code structure cannot. +- Remediation boundary: prefer a named operation that makes the invariant + obvious; add a concise rationale only where code structure cannot. -### `COM-002`: publish documentation calls ordinary success a self-skip +### `COM-002`: successful no-output stages are documented as skips - Category: comment/clarity candidate. +- Stage 11 classification: confirmed and broadened to absorb `ARC-002` and + `COM-004`. Publish, disabled render, and absent/no-executable analyze complete + successfully with no outputs; they are not durable self-skips and are reused + on ordinary reruns until forced. Documentation should reserve “self-skip” for + explicit `StageDispositionSkipped`, explain reconsideration, and describe + each no-output success directly. - Evidence: `docs/internal/stage-publish.md` says disabled publish or run upload can “self-skip.” The implementation returns zero disposition with skip metadata, so both manifests record succeeded and later normal runs reuse that @@ -3406,27 +3589,31 @@ register above. self-skip and unintentionally alter downstream/cleanup behavior. The `publish` command currently forces execution, which mitigates the common explicit operator workflow but not the terminology. -- Confirmation owner: documentation owner with Stage 8 coordination for the - broader `ARC-002` vocabulary. State the durable outcome and reconsideration - behavior precisely; change code only if a later lifecycle decision establishes - different intended semantics. +- Remediation boundary: state the durable outcome and reconsideration behavior + precisely; no production lifecycle change is indicated. ### `COM-003`: Audita's adapter contract still says its real adapter is unimplemented - Category: stale comment candidate. +- Stage 11 classification: confirmed stale comment. Remove the TODO. If + `ARC-005` leaves a non-obvious constructor/request boundary, replace it with a + rationale comment stating that constructor configuration is static and the + request carries invocation data; do not restate argument construction. - Evidence: `internal/adapters/audita/runner.go` carries a TODO to implement a real subprocess/service adapter, while `SubprocessRunner` is production composed, documented, and covered by extensive invocation tests. - Realistic risk: a maintainer treats the adapter as placeholder-only, bypasses the existing implementation, or leaves genuinely missing notification work confused with already completed Audita work. -- Confirmation owner: Stage 11 should remove the stale TODO and, only if useful, - replace it with rationale about static constructor settings versus request - settings after `ARC-005` is resolved. No production behavior change is needed. +- Remediation boundary: remove the stale TODO and, only if useful, replace it + with rationale about static constructor settings versus invocation request + data. No production behavior change is needed. ### `COM-004`: render documentation calls a durable success a skip - Category: comment/clarity candidate. +- Stage 11 classification: merged into broadened `COM-002`; it is the same + lifecycle-language defect and needs no separate implementation or comment. - Evidence: `docs/internal/stage-render.md` says disabled render “skips with stage metadata.” The implementation returns a zero-disposition result, so the runner records succeeded with no outputs and an ordinary later invocation @@ -3436,14 +3623,18 @@ register above. - Realistic risk: an operator expects enablement to be automatically reconsidered, or a maintainer changes render to `StageDispositionSkipped` and unintentionally alters downstream lifecycle behavior. -- Confirmation owner: Stage 11 documentation pass. Describe successful - no-output execution and the force consequence directly, using the lifecycle - vocabulary shared with `COM-002`; no production behavior change is indicated - by Stage 8. +- Merge boundary: broadened `COM-002` describes successful no-output execution + and the force consequence directly; no production behavior change is + indicated by Stage 8. ### `COM-005`: analyze documentation omits one source family and durable no-op consequences - Category: documentation/clarity candidate. +- Stage 11 classification: confirmed in part and merged in part. Add extraction + to the source/catalog descriptions and align executable-set wording with + `ARC-007`. The successful no-output/force consequence is owned by broadened + `COM-002`. Do not document the inert `artifact`/`path` fields as working + passthrough until `COR-024` is repaired. - Evidence: `docs/internal/stage-analyze.md` lists built-in, prepared, configured, and previous-session sources but omits supported `narratio.extraction.` inputs. It says missing configuration/no @@ -3460,28 +3651,51 @@ register above. analyze into an explicit self-skip and break pipeline/publish behavior. The dedicated `analyze` command force-runs, which mitigates but does not correct the contract. -- Confirmation owner: Stage 11 documentation pass after `ARC-007` decides - enabled/selection language. Add extraction to catalog/source lists and state - successful no-output persistence, continuation, and force requirements using - the shared `ARC-002` lifecycle vocabulary. Do not describe the inert +- Remediation boundary: add extraction to catalog/source lists and use + `ARC-007`’s effective-selection language. Broadened `COM-002` owns successful + no-output persistence and force wording. Do not describe the inert `artifact`/`path` fields until `COR-024` chooses implement versus reject. +### `COM-006`: source-classification comment omits extraction sources + +- Category: confirmed stale comment. +- Evidence: `artifactpolicy.ClassifySource` says it classifies built-in, + configured, or previous-session configured IDs, but its production branch + also recognizes `narratio.extraction.` and returns + `SourceKindExtraction`. +- Stage 11 classification: confirmed. Update the doc comment to name extraction + sources, because the list documents a supported compatibility vocabulary. + Keep the why-comments in `run_control` and the notification placeholder: the + former accurately records the explicit force/freshness limitation, and the + latter accurately identifies the missing transport already owned by + `ARC-004`. +- Realistic risk and scope: low implementation risk and one-line scope, but the + stale exported comment can cause new consumers to duplicate classification or + reject a supported source family. + ## Candidate Classification Log | Candidate signal | Classification | Reason | | --- | --- | --- | | Graph rollups `stage -> app`, `adapters -> app`, `config -> app` | rejected as a production reversal at Stage 1 | `go list` production imports contain no lower-level import of `internal/app`; graph connections include tests and ambiguous package grouping. Reopen only with a concrete production edge. | -| Similar wrapper/manifest/adapter functions | deferred metric signals, not findings | Similarity alone does not establish duplicated policy; owning behavior stages must first establish contracts. | +| Similar wrapper/manifest/adapter functions | rejected as broad abstractions at Stage 11 | Thin command wrappers, typed manifest models, distinct protocol constructors, and deliberately simple fakes share syntax rather than policy. Atomic byte writing is the narrow shared mechanism retained in `DUP-001`/`DUP-005`. | | Coverage percentages | deferred diagnostic signals, not findings | Stage 12 must reason from risk and test ownership, not a numeric target. | +| Production fan-in/fan-out leaders | rejected as standalone Stage 11 findings | Stable owners such as configuration load/validate, canonical path helpers, manifest transitions, subprocess launch, and command dispatch should have many callers. Ambiguous method names and interface dispatch inflate graph rollups; caller tracing found no new ownership inversion. | +| `previouscache.BuildPlan` complexity 22/38 | rejected as a simplification finding at Stage 11 | Its visible branches preserve required/optional absence, remote-current validation, candidate existence, and deterministic ordering. A helper would have no narrower policy owner; remote transfer cost is instead measured under `EFF-001`. | +| Direct scan/allocation-in-loop graph signals | rejected as a production efficiency source at Stage 11 | Production filtering found no direct flagged occurrence. High transitive depth was composition/test propagation, and Stage 10 already rejected tiny artifact-set sorting as immaterial. | +| Three private zero-inbound path helpers | confirmed as `SIM-004` | Exact text search found only their definitions and no registration/reflection seam; current production paths use newer typed/configurable owners. | +| Relative-path validators have similar lexical checks | rejected as `DUP-004` | Config rejects any `..` segment while the shared normalizer accepts non-escaping cleanup. Preserving that stricter language explicitly is clearer than a mode-heavy helper. | +| Source classifier comment lists every family | confirmed as `COM-006` | The exported comment omits the implemented extraction branch and compatibility spelling. | +| Direct module dependencies can be replaced by the standard library | rejected at Stage 11 | Each direct dependency owns an active S3, YAML, or native no-replace platform contract; `go mod why -m` resolved all six. | | Session `last_error` survives a later stage success | rejected as a current-state defect at Stage 2 | No production reader was found; current status and per-stage error are authoritative, so the field can serve as historical context. Reopen only if an operator surface treats it as the active error. | | Minimal loaded-manifest status/timestamp validation | rejected as a standalone Stage 2 finding | Unknown/non-succeeded statuses fail conservatively into execution, nil maps/records are normalized, and no realistic unsafe caller was established. Configured-versus-persisted identity conflict is separately confirmed in `COR-001`. | | Ignored runner lock-release error | confirmed as `RSK-003` at Stage 3 | The lock is an exclusive-create sentinel, not an OS-released lock. An unlink failure leaves the conflicting file while the runner suppresses the error; process death does the same without a release attempt. | -| Two durable meanings of “skip” | behavior confirmed; retained as `ARC-002` vocabulary candidate | Run action/status distinguishes idempotent skip from executed self-skip. Publish, disabled render, and absent/no-executable analyze deliberately return successful no-output metadata (`COM-002`, `COM-004`, `COM-005`); disabled trim performs a successful copy; extraction is the true self-skip. Stage 11 can settle language without assuming one behavior. | +| Two durable meanings of “skip” | architecture candidate merged into `COM-002` at Stage 11 | Run action/status already distinguishes idempotent skip, executed self-skip, and ordinary successful no-output execution. The behavior is coherent; publish/render/analyze documentation must use those exact terms. | | Exported previous-artifact helper accepts traversal | consolidated into `COR-002` | Current production callers normalize first, but the helper's under-root contract is false in isolation. The identity/relative-segment boundary should be repaired once rather than as separate caller bugs. | | Built-in and previous resolvers do not re-hash manifest records | documented trust distinction; deferred to Stages 5 and 10 | Extraction explicitly requires checksum/contract/provenance validation and enforces it. Other source families explicitly use content validation and a previous-cache filesystem fallback; consumer/restore threat models must establish a stronger requirement before this becomes a finding. | | Promotion destination is path-based while source is handle-confined | consolidated into `COR-003` | Source hardening is strong, but destination ancestors share the same symlink/replacement root cause as ordinary writers and cleanup. | | Pointer is written last, so failed publish cannot advance current | rejected as sufficient atomic-commit proof at Stage 4; reader enforcement corrected at Stage 5 | The fixed current manifest is overwritten first (`COR-004`). Strict callers reject old-pointer/new-manifest disagreement, but restore/status omit run validation and accept it (`COR-008`). | -| Remote current manifest records `current_pointer_written=false` | safe for current readers; retained as `ARC-003` | The snapshot is necessarily precommit, current-state loaders use actual pointer identity, and cleanup uses postcommit local metadata. The shared field remains ambiguous for future consumers. | +| Remote current manifest records `current_pointer_written=false` | safe for current readers; confirmed as `ARC-003` at Stage 11 | The snapshot is necessarily precommit and current-state loaders use actual pointer identity, but the shared remote/local field remains ambiguous. Derive commitment from the pointer or split precommit/local metadata without a post-pointer upload. | | Post-publish cleanup is revisited by later invocations | corrected and confirmed as `COR-006` | The runner invokes the helper, but its gate requires publish in the current `executed` list. Once session publish is succeeded, ordinary retry skips publish and therefore skips cleanup. | | Manual clean should require publish commit metadata | rejected as a policy requirement at Stage 4 | Manual clean is explicit operator authorization with session/global scope, dry-run, cache opt-in, and confined targets. Publish execution/upload/pointer gates correctly apply only to automatic cleanup. | | Restore dry-run performs no local writes | rejected as a literal implementation guarantee; retained as documentation precision under `EFF-001` | Dry-run avoids durable workspace, spool, cache, report, layout, and lock writes, but equal-size/unknown-size classification downloads remote bodies to system temporary files for checksumming and removes them afterward. | @@ -3501,32 +3715,32 @@ register above. | Repeated filesystem secret scans are an efficiency defect | rejected as material at Stage 6 | Object-store construction can repeat a deterministic bounded directory scan, but the scan is small, preserves secret-before-adapter ordering, and no material latency or external cost was established. | | Notification settings are consumed because notify succeeds | confirmed as `ARC-004` at Stage 7 | Production success is supplied by a no-op sender regardless of backend/recipient; no transport contract or composition path consumes the accepted public fields. Non-placeholder values must be rejected/reserved until a real integration exists, or a transport must be specified and composed. | | Injectable runner `Env` always represents the resolved config | retained as `TST-006` | Production composition does, but a non-nil injected `Env.Config` is retained and can differ from the explicit config used by other runner setup. | -| Configuration relative-path validation is distinct from shared lexical safety | retained as `DUP-004` | Config adds field context, but its cleaned relative-path language overlaps `pathsafe`; Stage 11 must compare semantics before centralizing mechanics. | +| Configuration relative-path validation is distinct from shared lexical safety | consolidation rejected as `DUP-004` at Stage 11 | Config rejects every `..` segment while `pathsafe` accepts non-escaping cleanup, and it owns field-specific diagnostics. A mode-heavy shared validator would obscure that deliberate policy. | | `exec.CommandContext` bounds a whole external-tool process tree | confirmed as `RSK-011` | The audited toolchain kills only `cmd.Process`; Narratio establishes no process group/job or descendant cleanup. | | Sensitive override-tail redaction makes subprocess diagnostics secret-safe | confirmed as `RSK-012` | Raw logs are unfiltered and inherited sensitive values are absent from the tail redaction set, so the architecture invariant does not hold. | | Successful exit plus JSON/non-empty validation establishes a safe subprocess result | confirmed as `RSK-013` | Ordinary adapters use unbounded, link-following reads/stats; Notarius's bounded regular-file checks show the stronger boundary is both necessary and locally expressible. | | A non-nil S3 continuation token guarantees pagination progress | confirmed as `RSK-014` | The adapter never compares tokens, so a malformed repeated token produces unbounded requests and duplicate accumulation. | | Similar subprocess adapter argument builders should be consolidated | rejected at Stage 7 | The shared launcher already owns common resource/process mechanics; protocol flags, schemas, exit mapping, and validation differ materially and remain clearer in their adapters. Only atomic byte writing is retained as `DUP-005`. | -| Audita request fields are authoritative per invocation | retained as `ARC-005`; constructor authority recommended at Stage 8 | Polish always passes configured static values and has no per-invocation override behavior. The real runner already uses constructor state while the fake captures the redundant request; Stages 11-12 should simplify and align that contract. | +| Audita request fields are authoritative per invocation | rejected; constructor authority confirmed as `ARC-005` at Stage 11 | Polish has no per-invocation override behavior and the real runner already uses constructor state. Remove redundant static request fields; keep invocation paths and modules in the request. | | Adapter fake request slices all require synchronization now | rejected as a blanket Stage 7 finding; WhisperX confirmed under `TST-001` | Transcribe legitimately calls WhisperX concurrently and the required Stage 8 race command reproduces its fake's slice race. Other fakes still have sequential production callers; Stage 12 should assess future fidelity from actual consumers. | | Zero previous requirements means no previous state is managed | confirmed as `COR-017` | Prepare's current test treats the directory as untouched, but publish later uploads it independently of current manifest inputs. Removing the final requirement must remove or exclude stale managed bytes. | | Repeating an explicit audio path is harmless deterministic input | confirmed as `COR-019` | Prepare sorts and records the duplicate twice, while the manifest-first transcribe boundary rejects it. Configuration, producer, and consumer must share one duplicate policy. | | Context cancellation necessarily becomes a transcribe error | confirmed as `COR-018` | Workers and dispatch silently stop on the derived context, while completion checks only a recorded adapter/validation error. Zero or partial results can therefore be returned as success. | -| Adapter result paths have one run-local authority rule | retained as `ARC-006` | Current production adapters return requested paths, but transformation stages inconsistently consume, ignore, or reject returned paths. Stage 11 must make the seam explicit before fakes are aligned. | -| Transcript discovery helpers are intentionally all distinct | plural raw discovery rejected; singleton repetition retained as `DUP-006` | Raw inputs need directory enumeration and plural ordering. Merged/processed/normalized singleton helpers repeat manifest-first/canonical-fallback mechanics already represented by the artifact resolver. | +| Adapter result paths have one run-local authority rule | confirmed as `ARC-006` at Stage 11 | Stages own run-local destinations; adapters must return no path or the exact requested path. Current redirects have no production use case and would broaden filesystem authority. | +| Transcript discovery helpers are intentionally all distinct | plural raw discovery rejected; singleton policy confirmed as `DUP-006` | Raw inputs need directory enumeration and plural ordering. Merged/processed/normalized singletons should use typed manifest-first/canonical resolution from the artifact owner. | | Similar ordinary-stage `Run` methods need a common framework | rejected at Stage 8 | Adapter sequencing, schemas, optional reports, disabled behavior, diagnostics, and multi-output failure order differ materially. Existing run-local helpers are the correct narrow shared mechanism. | | A valid immutable extraction bundle proves it represents the current transcript | confirmed as `COR-020` | Bundle checksums prove only promoted output integrity. The fingerprint and resume validator never resolve or hash the current direct transcript, so an internally valid old bundle can be reused after those input bytes change. | | Same-path external Notarius changes are automatically observable | documented force limitation, not a separate Stage 9 defect | Fingerprinting paths cannot prove executable, config, profile, prompt, module, reference, environment, provider, or runtime contents. Configuration/transitive changes are explicitly assigned to `--force`; operations should extend that wording to same-path executable replacement. Direct Narratio transcript identity is separately confirmed in `COR-020`. | | Every durable promoted bundle is advertised or reusable | rejected at Stage 9 | Promotion establishes immutable bytes, not success. A later sync/checksum/result-persistence failure may leave a uniquely named orphan bundle, but session-manifest success is advertisement authority and neither resume nor catalog scans incidental directories. | -| Resume and catalog bundle checks are merely coincidental similarity | retained as `DUP-007` | They repeat one exact-set, identity, contract/provenance, path, type, and checksum policy. Their error mapping differs intentionally, so sharing must return structured evidence rather than collapse lifecycle and fail-closed consumer behavior. | -| Extraction resume complexity justifies a generic validation framework | rejected; narrow simplification retained as `SIM-002` | The security proof and ordered failure classifications must remain visible. Named evidence decisions can reduce cognitive load without abstracting stage lifecycle or filesystem mechanics behind a generic framework. | +| Resume and catalog bundle checks are merely coincidental similarity | rejected; duplicated evidence policy confirmed as `DUP-007` at Stage 11 | They repeat one exact-set, identity, contract/provenance, path, type, and checksum proof. A typed proof can be shared while lifecycle and fail-closed mappings remain separate. | +| Extraction resume complexity justifies a generic validation framework | rejected; `SIM-002` merged into `DUP-007` at Stage 11 | The shared evidence proof supplies named ordered decisions without hiding security ordering or moving lifecycle policy into a framework. | | Optional analyze inputs behave uniformly across source families | confirmed as `COR-021` | Final, final-trimmed, and both Markdown built-ins return hard producer-guidance errors before the caller can honor `required: false`; other missing optional families are omitted. | -| Explicit selection is only a filter over enabled artifacts | retained as `ARC-007`, with concrete mismatch `COR-022` | Catalog tests require selection to override enabled, while configuration, previous planning, and focused docs assume enabled remains authoritative. The product rule needs one owner; current prerequisite omission is already a defect either way. | +| Explicit selection is only a filter over enabled artifacts | rejected; explicit override confirmed as `ARC-007` at Stage 11 | CLI help and catalog tests establish selection as a one-invocation override. A typed effective set must make validation and prerequisite planning follow that authority; publish retains separate filter semantics. | | Successful topological order implies deterministic dependency validation | confirmed as `RSK-015` | Edges and ready nodes are sorted for success, but the first unavailable-dependency preflight returns from unsorted selected-set map iteration. | | Existing configured output needs a prior analyze success to be reusable | rejected as the current contract | Non-executable configured artifacts intentionally use canonical non-empty files without manifest provenance so disabled/operator-prepared dependencies can be reused. Freshness is operator-owned; misleading enabled-but-unselected provenance stays under `ARC-007`. | | Previous-session resolution can fetch remotely during analyze | rejected at Stage 10 | Analyze uses only manifest-backed and filesystem `previous/` paths. Remote discovery/download belongs to restore/prepare composition, and a boundary test proves the object store is not called. Selection can omit that earlier planning under `COR-022`. | | Input `artifact` and `path` are adapter passthrough fields | confirmed as `COR-024` | Strict config accepts and documents them, but no production read, request field, CLI argument, or generated invocation field exists; values are silently discarded. | -| Repeated analyze resolution branches justify a generic resolver framework | rejected; narrow simplification retained as `SIM-003` | Source families deliberately differ in authority, provenance, optional absence, and repair guidance. A typed context/result and indexed plan can reduce width without erasing those policies. | +| Repeated analyze resolution branches justify a generic resolver framework | rejected; narrow `SIM-003` confirmed at Stage 11 | Source families deliberately differ in authority, provenance, optional absence, and repair guidance. A typed context/result and indexed effective plan reduce width without erasing those policies. | ## Unresolved Questions And Follow-Up @@ -3545,18 +3759,11 @@ register above. - Do deployment packaging and service definitions already enforce an umask, ownership, or ACL that reduces `RSK-004`, and where should that contract be documented and tested? -- Should explicit artifact selection override `enabled` for one-off execution, - or filter only enabled artifacts? `ARC-007` must choose one rule before - `COR-022` and related validation/documentation are repaired. - What exact Scriptorium wire semantics, if any, were intended for input-level `artifact` and `path`? Until `COR-024` is answered, non-empty values must not continue to be accepted as meaningful passthrough configuration. -- Is `IODecl` intended only for display/tests, or should it own enforceable - dependency declarations? - Should remote lock activation be snapshot-based or immediate, and what conditional-write capability can enforce the chosen rule across hosts? -- Should committed remote manifests omit precommit-only metadata, or should - local/remote publish metadata have distinct types after `COR-004` is fixed? - Should restore consume only objects declared by the pointer-selected manifest, or should the committed model carry an immutable generation map that can bind every downloaded artifact, audio object, and previous-cache source? @@ -3586,9 +3793,6 @@ register above. termination and wait semantics on Linux, macOS, and Windows? - Which per-contract maximum sizes are operationally valid for Seriatim, Audita, Scriptorium, and bounds outputs under `RSK-013`? -- Should adapter result types omit output paths when the stage-requested - destination is authoritative, or is there a valid redirected-output contract - that must be confined and normalized under `ARC-006`? - Can subprocess environments be reduced to explicit allowlists, or must `RSK-012` retain broad inheritance and stream-redact every known sensitive value? @@ -4020,3 +4224,54 @@ No accepted risks or final audit conclusions are recorded yet. `DUP-008`, `SIM-003`, and `COM-005`, coordinated with earlier candidates. Stage 12 owns the risk-based additions/consolidation in `TST-010` and the analyze reach of `RSK-013` alongside existing safe-output test candidates. + +### Stage 11 + +- Entry revision: `9cb9008` (`Document analyze dependency audit findings`). + Commits since the pinned audit revision modify audit documentation only, so + implementation/test evidence remains pinned to the identity recorded above. +- Contracts and code reviewed: development guide, exact Stage 11 sequence, all + repository policy, every accumulated `ARC`, `DUP`, `SIM`, `EFF`, and `COM` + entry, production callers for each candidate, direct dependencies, platform + implementations, TODO/FIXME/build-tag patterns, and existing benchmark + inventory. +- Graph/source evidence: production similarity, complexity, fan-in/fan-out, + loop-depth, direct scan/allocation-in-loop, change-coupling, zero-inbound, and + call-path queries followed by exact snippets and text search. Notable signals + were runner complexity 54/96, `previouscache.BuildPlan` 22/38, + `HydrateExtractionArtifacts` 17/25, analyze execution’s wide context, + identical 41-43-line atomic writers, three 30-34-line singleton transcript + resolvers, and three private zero-inbound helpers. Metrics were classified + only after their complete policy/caller paths were compared. +- Validation: `go test -count=1 ./...` passed all 22 packages in 3.22 s wall + time, and `go vet ./...` passed in 0.46 s. No benchmarks exist, so no invented + performance result is reported; `EFF-001` and `EFF-002` specify representative + byte/latency and memory/allocation measurements. No live adapter, credential, + remote, destructive, or paid operation was exercised. +- Structural conclusions: confirmed removal of the unused partial `IODecl` + interface, explicit-selection override authority, requested adapter output + path authority, constructor-owned Audita static settings, and a correction to + ambiguous remote/local commit metadata. Confirmed narrow shared owners for atomic + replacement, sibling-temp installation, canonical paths/singleton artifacts, + extraction evidence, catalog bootstrap, runner terminal failure, and analyze + execution context. Rejected shared config path validation and broad workflow, + resolver, graph, stage, adapter, or fake abstractions. +- Candidate disposition: `ARC-001`, `ARC-003`, and `ARC-005` through `ARC-007`; + `DUP-001` through `DUP-003` and `DUP-005` through `DUP-008`; `SIM-001` and + `SIM-003`; and `COM-001` through `COM-003`, the source/catalog portion of + `COM-005`, and new `SIM-004`/`COM-006` are confirmed. `ARC-002`, `COM-004`, + and the no-output portion of `COM-005` merge into broadened `COM-002`; + `SIM-002` merges into `DUP-007`; `DUP-004` is rejected after exact semantic + comparison. `EFF-001`, `EFF-002`, and `ARC-004` remain confirmed with sharper + remediation/measurement boundaries. +- Explicit no-finding conclusions: no direct production scan/allocation-in-loop + signal, material small-set sorting/copying issue, excessive adapter + initialization, removable direct dependency, unsupported-platform silent + fallback, generic framework opportunity, or additional concurrency/channel + defect was established. The accurate stale-detection and notification TODOs + remain rationale/future-contract markers; only the obsolete Audita TODO and + incomplete exported source-family comment are clarity findings. +- Later-stage assignments: Stage 12 owns only the risk-based `TST` inventory and + test-policy audit. Stage 13 owns prioritization, dependency ordering, and + accepted-risk decisions; it should treat the merged/rejected structural + dispositions above as resolved rather than reopening them from metric shape.