diff --git a/docs/roadmap/audit-findings.md b/docs/roadmap/audit-findings.md index 08d1862..78b9ed1 100644 --- a/docs/roadmap/audit-findings.md +++ b/docs/roadmap/audit-findings.md @@ -105,7 +105,7 @@ These are prioritization signals, not findings: | 5: restore and previous state | complete | Restore discovery/planning/execution/reporting, remote-current identity and scope, every local failure boundary, audio cache/spool identity, previous-cache planning/consumption, and status/validate policy reviewed. Focused app/previouscache/audio/artifacts/storage tests passed. Confirmed `COR-008` through `COR-011`, `RSK-006` through `RSK-008`, and `EFF-001`; added `DUP-003` and `TST-005`. | | 6: configuration and composition | complete | Discovery/precedence, strict loading, defaults/normalization, templates, cross-field validation, CLI selection, filesystem secrets, conditional adapter composition, and maintained examples reviewed. Exact focused tests and repository vet passed. Confirmed `COR-012` through `COR-015`, `RSK-009`, and `RSK-010`; added `ARC-004`, `DUP-004`, and `TST-006`. | | 7: adapters and shared support | complete | All HTTP, subprocess, notification, storage, audio, shared-model, and diagnostic boundaries were compared with their contracts and production callers. Focused normal and race commands passed. Confirmed `COR-016`, `RSK-011` through `RSK-014`, `EFF-002`, and `ARC-004`; added `ARC-005`, `DUP-005`, `COM-003`, and `TST-007`, and refined `TST-001`. | -| 8: ordinary stages | not_started | Assigned prepare/transcript behavior and disabled-outcome questions below. | +| 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 | not_started | Assigned extraction promotion, provenance, and resume scenario below. | | 10: analyze and dependencies | not_started | Assigned artifact dependency/source and selection scenario below. | | 11: maintainability | not_started | Seeded by graph complexity, similarity, and fan-in signals only. | @@ -122,8 +122,8 @@ has been mapped but not behaviorally audited. | Process and application boundary | `cmd/narratio`, `internal/app` | 6 (runner lifecycle portions in 2; publish/restore portions in 4-5) | reviewed | | Stage registry and runner | `internal/stage`, `internal/app` | 2 | reviewed | | Configuration | `internal/config` | 6 | reviewed | -| Prepare and audio | `internal/stage`, `internal/audio`, `internal/previouscache` | 8 | assigned | -| Transcript stages | `internal/stage` plus tool adapters | 8 | assigned | +| Prepare and audio | `internal/stage`, `internal/audio`, `internal/previouscache` | 8 | reviewed | +| Transcript stages | `internal/stage` plus tool adapters | 8 | reviewed | | Extraction | `internal/stage`, Notarius adapter, `internal/fileops` | 9 | assigned | | Analyze and artifact dependencies | `internal/stage`, `internal/artifacts`, `internal/artifactpolicy` | 10 | assigned | | Publish and cleanup | `internal/stage`, `internal/app` | 4 | reviewed | @@ -177,14 +177,14 @@ succeeded downstream work, while force pre-stales succeeded downstream work. | Order and stage | Inputs and outputs | Configuration and adapters | Skip/resume behavior | Materialization and manifest effects | | --- | --- | --- | --- | --- | -| 1 `prepare` | Config, stable inputs, one audio mode, optional previous requirements -> canonical `inputs/**`, `audio/**`, optional `previous/**`, `manifest.inputs` | All resolved config; storage for S3/current previous state; audio/artifact/previous-cache services | No stage-specific resume validator or explicit self-skip | Writes canonical session inputs and deterministic input records; unlike processing stages, `Declares` labels produced canonical files as inputs. | -| 2 `transcribe` | Prepared FLAC files -> raw per-speaker JSON | WhisperX URL/language/retry/timeout/concurrency; `whisperx.Client` | Ordinary succeeded-record skip; no validator/self-skip | Bounded concurrent run-local writes, validation, then canonical transcript materialization. | -| 3 `merge` | Raw transcripts, speakers, autocorrect -> base transcript, optional report | Seriatim merge fields; `seriatim.Runner` | Ordinary succeeded-record skip | Normalized scratch inputs and run-local results validate before canonical transcript/report materialization. | -| 4 `polish` | Base transcript, glossary -> polished transcript, optional report | Audita fields/credential reference; `audita.Runner` | Ordinary succeeded-record skip | Run-local output, report, logs, and generated config; validates before canonical materialization. | -| 5 `normalize` | Polished transcript -> final transcript, optional report | Normalize plus Seriatim fields; `seriatim.Runner` | Ordinary succeeded-record skip | Manifest-first input; run-local validation then configurable canonical output/report. | -| 6 `trim` | Final transcript -> final-trimmed transcript and, when enabled, bounds | Trim, bounds, Scriptorium, and Seriatim fields; both runners when enabled | Disabled trim copies input and still succeeds; no explicit self-skip or resume validator | Run-local bounds/trim result validates then materializes; debug render is diagnostic, not output. | +| 1 `prepare` | Config, stable inputs, one audio mode, optional previous requirements -> canonical `inputs/**`, `audio/**`, optional `previous/**`, `manifest.inputs` | All resolved config; storage for S3/current previous state; audio/artifact/previous-cache services | No stage-specific resume validator or explicit self-skip | Writes canonical session inputs and sorted input records; unlike processing stages, `Declares` labels produced canonical files as inputs. Repeated explicit audio paths create duplicate records (`COR-019`), and zero previous requirements leave stale managed state (`COR-017`). | +| 2 `transcribe` | Prepared FLAC files -> one raw JSON per unique filename-derived speaker | WhisperX language/retry/timeout/concurrency; `whisperx.Client` | Ordinary succeeded-record skip; no validator/self-skip | Bounded concurrent run-local writes, exact adapter path check, JSON validation, sorted results, then canonical materialization. Cancellation can nevertheless return a successful incomplete set (`COR-018`). | +| 3 `merge` | Manifest raw transcripts or directory fallback, speakers, autocorrect -> base transcript, optional report | Seriatim merge fields; `seriatim.Runner` | Ordinary succeeded-record skip | Deterministically normalized scratch inputs and run-local transcript/report validate before canonical materialization; logs/config are diagnostics. | +| 4 `polish` | Manifest base transcript or canonical fallback, glossary -> polished transcript, optional report | Audita fields/credential reference; `audita.Runner` | Ordinary succeeded-record skip | Run-local transcript/report validate before canonical materialization; logs/config are diagnostics. Static request/constructor ownership remains `ARC-005`. | +| 5 `normalize` | Manifest polished transcript or canonical fallback -> final transcript, optional report | Normalize plus Seriatim fields; `seriatim.Runner` | Ordinary succeeded-record skip | Run-local schema/report validation then configured canonical materialization; logs/config are diagnostics. | +| 6 `trim` | Manifest final transcript or configured canonical fallback -> final-trimmed transcript and, when enabled, bounds | Trim, bounds, Scriptorium, and Seriatim fields; both runners when enabled | Disabled trim copies and validates the normalized transcript, then succeeds; no explicit self-skip or resume validator | Enabled bounds/trim results validate before canonical materialization; render-debug and subprocess logs/config are diagnostics, not outputs. | | 7 `extract` | Final-trimmed source -> immutable index and configured lane outputs | Notarius executable/config/pipeline/timeout/output contracts; `notarius.Runner` | Disabled is explicit `notarius_disabled` self-skip; only current `ResumeValidator`; obsolete reruns, unsafe validation errors | Validates run-local receipt/bundle completely, promotes to unique immutable bundle, records checksums/contracts/provenance; identical repeated self-skip is stable. | -| 8 `render` | Final and final-trimmed JSON -> two Markdown transcripts | Render and Seriatim fields; `seriatim.Runner` | Disabled returns a zero-disposition result with skip metadata, therefore runner-level success rather than explicit self-skip; no validator | Run-local text validates non-empty before canonical materialization when enabled. | +| 8 `render` | Manifest final/final-trimmed JSON or canonical fallback -> two Markdown transcripts | Render and Seriatim fields; `seriatim.Runner` | Disabled returns a zero-disposition no-output result, therefore durable success rather than explicit self-skip; later enablement needs force; no validator | Enabled run-local text validates non-empty before either canonical result is recorded; logs/config are diagnostics. Focused wording is imprecise under `COM-004`. | | 9 `analyze` | Dynamic built-in, prepared, extraction, configured, and previous sources -> selected configured artifact outputs | Scriptorium artifact graph/selection; `scriptorium.Runner` | Missing config or no executable artifacts returns success with skip metadata; no validator | Topological run-local generation/reuse, validation, canonical outputs, deterministic metadata; static `Declares` omits dynamic outputs and several input families. | | 10 `publish` | Session/run state, selected output rules, locks, previous cache -> remote run/output/current objects | Publish/storage/selection fields; `storage.ObjectStore` | Disabled publish or run upload returns ordinary success with skip metadata, not a self-skip; force cannot bypass locks; no validator | Deterministic uploads; `current/manifest.json` before `current/run_id.txt`; post-commit local metadata gates cleanup. Static prerequisites omit extract because disabled extraction is valid and lane resolution enforces required extraction state when selected. | | 11 `notify` | No implemented persisted pipeline input/output | Optional `notify.Sender`; default no-op | Ordinary succeeded-record skip; no explicit self-skip or validator | Placeholder metadata and optional notification call; no returned output. `Declares` nevertheless advertises placeholder input/output paths. | @@ -326,15 +326,18 @@ terminalization/reconciliation harder to test. | 9. Disabled/unselected/reused/generated/extraction/previous source then publish filtering | 10 | Analyze, artifact catalog/resolver/policy, publish tests; config ownership in 6 and publish result in 4 | | 10. Concurrent same-session invocation and lock cleanup failures | 3 | Runner lock lifetime in 2; local artifact store, path/file cleanup and lock tests in 3 | -Scenario 8 is resolved at the adapter boundary. Parent cancellation reaches +Scenario 8 is resolved through the ordinary stages. Parent cancellation reaches HTTP attempts, retry waits, every AWS call, audio downloads, and each direct child process. HTTP/S3/local-file resources are released on error, and a started direct child is waited. Cancellation is not complete for multipart body construction (`EFF-002`) or subprocess descendants (`RSK-011`), and a malformed repeated S3 continuation token needs cancellation to escape its -otherwise non-progressing loop (`RSK-014`). Runner reporting remains the Stage -2 dual-ledger behavior; Stage 8 owns worker aggregation and partial transcript -results rather than reopening these transport conclusions. +otherwise non-progressing loop (`RSK-014`). Merge, polish, normalize, trim, and +render invoke subprocess adapters synchronously and propagate their errors; +they do not introduce another worker lifetime. Transcribe alone aggregates +workers, and `COR-018` confirms that parent cancellation before an adapter +records an error can be mistaken for successful zero or partial output. Runner +reporting remains the Stage 2 dual-ledger behavior. ## Preliminary Risk-To-Test Matrix @@ -381,6 +384,15 @@ Stage 7 test observations for this matrix: | Adapter fakes under production concurrency | No-op/fake tests cover cancellation/error and deterministic placeholder materialization. The focused adapter race command passes. | The WhisperX fake's unsynchronized request slice fails the full race suite when the transcribe stage uses it concurrently; `TST-001`. Other fakes currently have sequential production callers, so no blanket race finding. | | Shared models and logging | Artifact-model JSON/conversion tests and bounds success/error tables protect current serialized shapes; logger tests protect output/nil-writer construction. | Output acquisition bounds/type remain `RSK-013`; suite-wide fake/model/logging value and redundancy remain Stage 12 work. | +Stage 8 test observations for this matrix: + +| Risk | Existing focused protection | Gap or disposition | +| --- | --- | --- | +| Stable prepare inputs and source modes | Prepare tests cover explicit files, directory enumeration, S3 sorting/cache behavior, provenance, local/S3 conflict, idempotence, required/optional previous hydration, and replacement while requirements remain. | The no-requirement test asserts stale previous state survives, and no repeated-explicit-audio case crosses into transcribe; `COR-017`, `COR-019`, and `TST-008`. | +| Bounded deterministic transcription | Tests cover concurrency bounds, filename-derived identity, adapter error, invalid JSON, exact run-local output use, canonical materialization, and sorted result assertions. | No pre-canceled or mid-dispatch context case proves all jobs complete before success; `COR-018` and `TST-008`. The required race command also reproduces `TST-001` in the concurrent fake. | +| Manifest-first transformation and schema/report validation | Merge, polish, normalize, trim, and render suites cover manifest-first and fallback sources, missing/invalid inputs, adapter failures, configured reports/schemas, disabled paths, diagnostics, run-local paths, and canonical materialization. | Alternate valid adapter-returned paths, link/non-regular/oversized results, and multi-output materialization failure boundaries are not coherently tested; `ARC-006`, `RSK-013`, and `TST-008`. | +| Disabled ordinary-stage lifecycle | Trim tests prove disabled execution copies a valid canonical output; render tests prove disabled execution returns no outputs. Runner tests separately define durable success versus self-skip. | Render's focused document says only “skips,” without the durable-success and later-force consequence; `ARC-002` is resolved for these stages and `COM-004` owns the wording. Analyze remains Stage 10. | + ## Path, Artifact, Filesystem, And Lock Conclusions ### Canonical ownership and normalization @@ -900,8 +912,8 @@ owned by the transcribe stage, while every subprocess call is synchronous. The focused adapter race command consequently passes; the full baseline race still fails only when the unsynchronized WhisperX fake is exercised concurrently by the stage. `TST-001` therefore represents a fake/consumer contract defect, not -an HTTP-client race, and Stage 8 must inspect the worker behavior before Stage -12 chooses the test-double repair. +an HTTP-client race. Stage 8 confirmed the worker's bounded concurrent contract; +Stage 12 owns the test-double repair. ### HTTP, storage, retry, and malformed-response behavior @@ -991,6 +1003,106 @@ fake placeholder materialization is test support whose suite-wide value and fidelity remain assigned to Stage 12; it is not a production abstraction candidate from this pass. +## Prepare And Transcript-Processing Conclusions + +### Prepare inputs, previous state, and deterministic recording + +Prepare rechecks the resolved configuration and runtime collaborators, enforces +local-versus-S3 audio exclusivity, resolves stable inputs according to campaign/ +session precedence, and copies the selected campaign, session, resolved +pipeline, stable-input, and audio bytes into canonical session paths. S3 object +sets and local directory entries are sorted; colliding basenames from different +sources are rejected; checksums and remote/session provenance are recorded; and +the final manifest input slice is sorted by kind and path. Previous-cache +planning and hydration use the Stage 5 owner, validate required artifacts, and +add deterministic manifest/artifact input records. No secret value enters the +resolved pipeline copy. + +Two configuration transitions violate that otherwise deterministic handoff. +First, managed `previous/**` state is cleared only when the newly resolved +configuration has at least one previous requirement. Removing the last +requirement leaves old bytes outside `manifest.inputs`; publish later walks and +uploads the directory independently of those records. This is `COR-017`, and +the focused test currently encodes the stale-state behavior. Second, explicit +`audio_files` entries are sorted but not deduplicated. Repeating the same source +therefore lets prepare succeed with duplicate manifest records, while +transcribe's manifest-first validator rejects the duplicate path. Configuration +validation does not reject it; `COR-019` owns the inconsistent boundary. + +### Transcription concurrency, cancellation, and result identity + +Transcribe derives one speaker identity from each prepared FLAC basename, +rejects duplicate speakers/paths, clamps positive configured concurrency to the +job count, requests one distinct run-local JSON output per speaker, and requires +the adapter-returned path to equal that request. Successful files are validated +before any canonical copy; metadata and outputs are ordered by speaker, and any +recorded adapter/validation error cancels peers and prevents canonical +materialization. This satisfies unique identity, output-path authority, +bounded concurrency, and ordinary partial-error ordering. + +Cancellation itself is not included in the completion decision. A worker that +observes the derived context before its adapter call exits silently, dispatch +stops silently on that same context, and the coordinator checks only the first +recorded adapter/validation error. A pre-canceled context can therefore return +success with no outputs, and cancellation after some completions can return and +materialize a successful subset. `COR-018` records the correctness defect. +The required race test also confirms that the WhisperX fake is invoked under a +legitimate concurrent interface contract and races while appending requests; +this closes Stage 8's behavioral check for `TST-001` without suggesting a +production HTTP-client race. + +### Transformation, rendering, and output classification + +Merge, polish, normalize, trim, and render prefer recorded producer outputs and +use documented canonical fallbacks. They pass run-local destinations to their +synchronous adapters, validate JSON/transcript/report/bounds or non-empty text +as appropriate, and materialize only validated results into canonical session +paths. Merge sorts raw inputs and normalizes each into run scratch before its +merge; trim validates bounds ordering, membership, and selector behavior; and +render waits for both requested render calls before canonical materialization. +Subprocess logs, generated invocation configuration, and trim render-debug are +diagnostics, while requested transcript, report, bounds, and Markdown results +are stage outputs. Their ordinary output acquisition still inherits +`RSK-013`'s unbounded, link-following validators. + +Current production adapters return the requested destination, but authority is +inconsistent at the stage seam: transcribe rejects any alternate returned path, +while several transformation stages validate and materialize a result path +returned by the adapter and others treat the request path as authoritative. +No current production adapter intentionally redirects output, so this is the +contract candidate `ARC-006`, not a confirmed data defect. Polish likewise has +no need for per-invocation overrides of the static Audita values duplicated in +its request; `ARC-005` should make constructor state authoritative and retain +only truly invocation-specific request fields unless a later product contract +introduces overrides. + +Disabled trim is real processing: it validates and copies normalized JSON to +the configured trimmed output and is correctly durable success. Disabled +render deliberately returns successful no-output metadata so the pipeline can +continue without Markdown; like any previously succeeded stage, later +enablement requires force. Those ordinary-stage outcomes are coherent and +resolve their part of `ARC-002`; the focused render document's generic “skips” +wording is retained as `COM-004`. Analyze's remaining no-op outcome stays in +Stage 10. + +### Similarity classification + +- `resolveRunStageLayout`, `runLocalPathForCanonical`, and + `materializeRunLocalOutput` already form the narrow shared owner for run-local + isolation and canonical copying; keeping adapter calls explicit is + intentional. +- Raw-transcript discovery is meaningfully plural and directory-aware. The + three singleton manifest-first transcript resolvers, however, repeat nearly + identical candidate, local-path, existence, and fallback mechanics even + though the artifact registry already owns the same policy shape; `DUP-006` + assigns a narrow resolver decision to Stage 11. +- JSON, transcript, report, bounds, and text semantic checks should remain + contract-specific. Safe bounded regular-file acquisition is the shared + mechanism already required by `RSK-013`, not another generic validator. +- Stage metadata and adapter requests expose protocol-specific facts. A generic + stage/template framework would hide important ordering and failure + differences and is rejected from this audit pass. + ## Confirmed Findings ### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity @@ -1544,6 +1656,104 @@ candidate from this pass. - Dependencies: `COR-013` separately owns duration validation disagreement; neither finding requires changing retry behavior. +### `COR-017`: removing the final previous-artifact requirement leaves stale publishable state + +- Category: confirmed correctness and data-integrity defect. +- Locations/invariant: `internal/stage/prepare.go` in `prepareStage.Run` and + `clearManagedPreviousState`, plus `internal/stage/publish.go` previous-file + collection. Prepare must make managed `previous/**` state match the current + requirement set, and publish must not expose bytes excluded from that set. +- Evidence: prepare calls `clearManagedPreviousState` only inside + `len(previousRequirements) > 0`. With zero current requirements it replaces + `manifest.inputs` without previous records but leaves the directory intact. + `TestPrepareStageWithoutPreviousRequirementsDoesNotTouchPreviousState` + explicitly requires a stale file to survive. Publish independently walks all + files below the previous directory and uploads them under the current session + prefix; it does not filter that walk through current input records. +- Realistic scenario: one pipeline revision consumes a previous recap and + prepare hydrates it. The operator removes the final previous input and forces + prepare for the same session. Later publish uploads the old recap again even + though current configuration and manifest inputs no longer declare it. +- Impact/likelihood/confidence: high stale-data and possible confidentiality + impact; moderate configuration-evolution likelihood; high confidence from + the focused test and publish caller. +- Estimated remediation scope and owner: small-to-medium prepare/publish + invariant correction. Clear the managed previous tree on every prepare before + optionally hydrating the current requirement set, or make publication consume + one explicit current-set record. Preserve confined deletion requirements. +- Test changes: replace the stale-survival assertion with a transition case that + hydrates a requirement, removes the final requirement, reruns prepare, and + proves both the local tree and publish upload set omit it. Retain the optional + missing requirement clear case. +- Dependencies: `COR-011` owns source identity for requirements that remain; + `COR-003` governs safe recursive clearing. Stage 4's deterministic publish + behavior is not reopened. + +### `COR-018`: transcribe can report successful partial work after cancellation + +- Category: confirmed correctness and lifecycle defect. +- Locations/invariant: `internal/stage/transcribe.go` worker, dispatcher, and + completion decision; `internal/app/runner.go` success recording. A successful + transcription result must represent every discovered audio job, while parent + cancellation must produce an error rather than a reusable success. +- Evidence: a worker returns without recording an error when `stageCtx.Err()` + is already non-nil, and dispatch breaks silently on `stageCtx.Done()`. After + waiting, the coordinator checks only `firstErr`; it never checks the parent + context or verifies completed results equal planned jobs. A pre-canceled + context therefore produces a successful result with zero outputs. If some + workers complete before cancellation stops dispatch, their subset is sorted, + materialized, and returned as success. The runner has no independent context + check and records that result as succeeded. +- Realistic scenario: an operator cancels a multi-speaker session while one + fast request has completed but other work is queued. The command can report + success and make the partial raw transcript set reusable by merge on the next + invocation. +- Impact/likelihood/confidence: high transcript completeness/integrity impact; + moderate cancellation likelihood; high confidence from the explicit channel, + context, and result-count control flow. +- Estimated remediation scope and owner: small transcribe coordinator change. + Track planned/dispatched/completed jobs and return the parent cancellation + cause whenever the complete set was not produced, while retaining the first + concrete adapter/validation error when it caused cancellation. Materialize + canonical outputs only after complete success. +- Test changes: add pre-canceled and barrier-controlled mid-dispatch cases that + assert an error, no canonical partial materialization, and stable adapter-error + precedence. Keep the bounded concurrency and deterministic ordering cases. +- Dependencies: `EFF-002` and `RSK-011` govern transport/process cancellation + latency; this finding owns worker aggregation after cancellation. `TST-001` + separately owns the racing fake used under valid concurrency. + +### `COR-019`: repeated explicit audio input is accepted by prepare and rejected by transcribe + +- Category: confirmed correctness and operability defect. +- Locations/invariant: `internal/config.validateSession`, + `internal/stage.resolveLocalAudioFiles`, `materializeLocalAudioInputs`, and + `discoverPreparedAudio`. One accepted audio selection must have a consistent + identity from configuration through prepared manifest consumption. +- Evidence: configuration validation accepts any non-empty `audio_files` slice + and does not check duplicates. Local resolution sorts but does not deduplicate + it. Materialization rejects only when the same destination basename maps to a + *different* source, so the same path repeated is copied and registered twice; + prepare succeeds with duplicate audio input records. Transcribe then prefers + those manifest records and `validateAudioFiles` rejects the duplicate clean + path before any adapter call. +- Realistic scenario: a generated or hand-merged session file repeats one FLAC + entry. Validation and prepare both succeed, but the deterministic next stage + fails with `duplicate audio file path`, requiring a configuration correction + and rerun. +- Impact/likelihood/confidence: medium delayed-configuration-failure impact; + low-to-moderate authoring/tooling likelihood; high confidence from the + consecutive producer/consumer checks. +- Estimated remediation scope and owner: small config/prepare decision. Prefer + rejecting duplicate cleaned sources during configuration/resolution with + field context; alternatively deduplicate deterministically before copying and + recording, but do not allow duplicate manifest identities. +- Test changes: cover exact and clean-path-equivalent duplicates, distinct + sources with the same basename, and a valid multi-file set at the earliest + chosen owner; retain transcribe's defensive duplicate-manifest rejection. +- Dependencies: `COR-002` owns unsafe identity segments and `RSK-008` foreign + restored paths; neither changes duplicate semantics. + ### `RSK-001`: invocation audit records can remain indefinitely `running` - Category: confirmed correctness/operational risk. @@ -1967,7 +2177,12 @@ candidate from this pass. satisfy validation and be copied as the stage output. Notarius already uses `Lstat`, same-file checks, regular-file enforcement, and explicit size limits, while WhisperX bounds responses to 10 MiB, demonstrating a compatible local - boundary pattern. + boundary pattern. Stage 8 confirmed the same acquisition path is used again + by `validateTranscriptJSONFile`, `validateProcessedTranscriptOutput`, + `copyTranscript`, `requireNonEmptyFile`, and run-local materialization before + merge, polish, normalize, trim, and render record canonical outputs. The risk + therefore crosses the adapter/stage boundary rather than ending at adapter + first-pass validation. - Realistic scenario: a faulty external CLI writes a multi-gigabyte JSON result or leaves the requested output as a symlink to stale data. It exits zero; Narratio then exhausts memory or records unrelated bytes as a successful @@ -2072,9 +2287,10 @@ candidate from this pass. - 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. -- Dependencies: Stage 8 owns the worker concurrency limit and stable result - ordering; streaming should make each worker bounded rather than changing that - stage policy. +- Dependencies: Stage 8 confirmed the worker concurrency limit and stable + successful result ordering; streaming should make each worker bounded rather + than changing that stage policy. `COR-018` separately owns cancellation being + mistaken for complete success. ### `ARC-004`: notification configuration has no production transport consumer @@ -2106,8 +2322,9 @@ candidate from this pass. transport exists, or, after a transport is specified, add adapter contract tests for timeout/cancellation/error adaptation and an assembled composition test proving selection. Preserve the no-op path only when explicitly chosen. -- Dependencies: Stage 8 owns the notify stage's ordinary-success lifecycle and - `ARC-002` vocabulary, not transport selection. +- Dependencies: the ordinary-success lifecycle vocabulary must be settled with + the later `ARC-002`/maintainability synthesis; this finding owns transport + selection and operator truthfulness, not stage-state mechanics. ## Candidate Register @@ -2121,8 +2338,11 @@ register above. - 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 - placeholder paths although its result has no persisted output. No production - caller of `Declares` was found. + placeholder paths although its result has no persisted output. Stage 8 also + found unconditional optional report/bounds declarations, configured output + paths that can differ from the static declaration, and render outputs declared + even when disabled render succeeds with none. No production caller of + `Declares` was found. - Contract tension: architecture says every stage declares required inputs, produced output state, configuration, adapters, lifecycle, and failure behavior; the Go interface declares only partial static artifacts. @@ -2130,8 +2350,9 @@ 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 for dynamic contracts, then Stage 11 for - interface purpose/simplification. Smallest plausible outcome may be clearer +- Confirmation owners: Stage 8 completed the ordinary-stage rows; Stages 9-10 + own extraction/analyze, then Stage 11 owns interface purpose/simplification. + Smallest plausible outcome may be clearer naming/documentation, a complete contract, or removal; do not choose yet. ### `ARC-002`: disabled-stage “skip” terminology spans two different durable outcomes @@ -2149,7 +2370,9 @@ register above. - Confirmation owners: 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 must decide the remaining ordinary disabled-stage behavior before + 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 must decide analyze's remaining no-op behavior before this architectural candidate can close. ### `ARC-003`: committed and local manifests give `current_pointer_written` different meanings @@ -2186,12 +2409,36 @@ 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 should decide whether polish needs per-invocation - overrides; Stage 11 should then remove the redundant request fields or make - them authoritative. Stage 12 should align the fake with the chosen 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. 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. +- 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 + Scriptorium bounds/render-debug prefer non-empty returned paths, while the + Seriatim trim branch validates the requested destination. Current Audita, + Seriatim, and Scriptorium production adapters return the requested path, so + production agrees by convention rather than an explicit shared rule. +- Contract tension: stages own run-local isolation and canonical materialization, + while adapters own protocol execution. Allowing an adapter to redirect output + gives it filesystem-placement authority that the request appears to reserve + to the stage; ignoring a returned path makes that result field misleading. +- 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. + ### `TST-001`: full race baseline fails in the concurrent transcribe test - Category: test-suite execution candidate. @@ -2206,9 +2453,13 @@ register above. synchronization, while the transcribe stage is contractually allowed to call the client concurrently. The defect is therefore in fake fidelity at the adapter/consumer seam, not evidence of a production HTTP-client race. -- Confirmation owners: Stage 8 should verify worker cancellation/result - behavior; Stage 12 should classify suite impact and the smallest durable fix. - Do not change the fake during this investigative stage. +- Stage 8 resolution: transcribe intentionally invokes the interface + concurrently within a configured bound, and the required focused race command + reproduces the fake's request-slice race. Worker cancellation has a separate + correctness defect in `COR-018`; it does not make concurrent fake mutation + valid. +- Confirmation owner: Stage 12 should classify suite impact and the smallest + durable fake fix. Do not change the fake during this investigative stage. ### `TST-002`: runner tests cannot exercise invocation-manifest save failures @@ -2312,6 +2563,25 @@ register above. `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. +- Evidence: `discoverMergedTranscript`, `discoverProcessedTranscript`, and + `discoverNormalizedTranscript` each scan one producer's manifest outputs, + trim/resolve candidate local paths, deduplicate/sort, stat candidates, select + one, and fall back to a canonical transcript path. Their output kinds, + producer records, fallbacks, and ambiguity messages differ, but the policy + skeleton is nearly identical. The artifact registry/resolver already owns a + manifest-first/canonical-fallback abstraction used by render and later stages. +- 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. + ### `TST-003`: filesystem safety tests omit destination and lock-recovery boundaries - Category: test-suite sufficiency candidate. @@ -2411,6 +2681,27 @@ register above. automatic fake materialization by the stage risks it protects rather than mechanically rewriting every fake. +### `TST-008`: ordinary-stage tests omit transition, cancellation, and output-authority boundaries + +- Category: test-suite sufficiency candidate. +- Evidence: focused prepare and transcript-stage tests strongly cover ordinary + sources, manifest-first fallback, deterministic concurrency/order, adapter + errors, invalid schemas/reports/bounds, disabled behavior, diagnostics, + run-local paths, and canonical materialization. They do not cover removing + the final previous requirement, repeated explicit audio, pre-canceled or + mid-dispatch transcription, a valid alternate adapter-returned output path, + or failure while materializing the second of multiple validated outputs. One + prepare test actively requires stale previous state to survive. +- Realistic risk: `COR-017` through `COR-019`, `ARC-006`, and the stage-side + reach of `RSK-013` can remain or receive partial fixes while broad normal + stage tests pass. The existing race command is also unusable until `TST-001` + is repaired. +- Confirmation owner: Stage 12 should add behavior tests at the narrowest owner: + one prepare transition, duplicate-input validation, barrier-controlled + transcribe cancellation, and one cross-adapter output-authority contract. + Reuse shared safe-output acquisition tests from `TST-007`; do not duplicate + every schema error or stage fixture already covered. + ### `SIM-001`: runner terminalization and persistence ordering lack a narrow owner - Category: simplification candidate. @@ -2471,6 +2762,23 @@ register above. replace it with rationale about static constructor settings versus request settings after `ARC-005` is resolved. No production behavior change is needed. +### `COM-004`: render documentation calls a durable success a skip + +- Category: comment/clarity candidate. +- 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 + reuses that success. Enabling render after that outcome requires force under + the documented global lifecycle. This is intentionally different from + extraction's explicit self-skip and from trim's successful copy output. +- 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. + ## Candidate Classification Log | Candidate signal | Classification | Reason | @@ -2481,7 +2789,7 @@ register above. | 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” | runner and publish portions confirmed; ordinary stages deferred | Run action/status distinguishes idempotent skip from executed self-skip. Publish deliberately returns success metadata consumed by cleanup, but its focused document incorrectly calls this self-skip (`COM-002`). Stage 8 owns remaining disabled stages. | +| Two durable meanings of “skip” | runner/publish/ordinary stages confirmed; analyze deferred | Run action/status distinguishes idempotent skip from executed self-skip. Publish and disabled render deliberately return successful no-output metadata (`COM-002`, `COM-004`); disabled trim performs a successful copy; extraction is the true self-skip. Stage 10 owns analyze's remaining no-op outcome before `ARC-002` closes. | | 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. | @@ -2512,8 +2820,14 @@ register above. | 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` | The real runner uses constructor values for most duplicated fields while the stage passes equal values by convention and the fake captures the request; Stages 8, 11, and 12 own the contract decision. | -| Adapter fake request slices all require synchronization now | rejected as a blanket Stage 7 finding | Only WhisperX is currently used concurrently by production-shaped stage behavior and is refined under `TST-001`; other fakes are sequential today. Stage 12 should assess future fidelity from actual consumers. | +| 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. | +| 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. | +| 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. | ## Unresolved Questions And Follow-Up @@ -2532,9 +2846,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? -- Are disabled render/analyze outcomes intentionally successful so pipeline - continuation works, and do all operator views describe that distinction - accurately? Publish is now resolved as successful with a documentation gap. +- Is analyze's missing/no-executable-artifact outcome intentionally successful + so pipeline continuation works, and do all operator views describe that + distinction accurately? Publish and render are now resolved as successful + no-output outcomes with documentation gaps; disabled trim is successful copy + processing. - 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 @@ -2570,6 +2886,9 @@ 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? @@ -2848,3 +3167,54 @@ No accepted risks or final audit conclusions are recorded yet. ordinary stage output consumption, notify lifecycle, and Audita override intent. Stage 11 owns `ARC-005`, `DUP-005`, and `COM-003`; Stage 12 owns the risk-based additions/consolidation in `TST-001` and `TST-007`. + +### Stage 8 + +- Entry revision: `0a772e0` (`Document external adapter 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, Stage 8 sequence, all + repository policy, overview, focused prepare/transcribe/merge/polish/ + normalize/trim/render documents, WhisperX/Seriatim/Audita/Scriptorium + integration contracts, and the complete production vertical slices in + `internal/stage`, with supporting config, audio, previous-cache, publish, and + runner callers where they establish the scoped input/lifecycle boundary. +- Graph/source evidence: scoped stage architecture and symbol inventory, + manifest/config/caller traces, and exact source inspection of audio selection + and materialization, previous hydration/clearing, every stage `Run`, run-local + helpers, transcript discovery/validation, result path use, declarations, + disabled branches, adapter fakes, and all focused test names/cases. Similarity + was evaluated only after stage-specific differences were recorded. +- Validation: the exact focused normal command passed all seven packages (1.34 + s command wall time). `go test -race -count=1 ./internal/stage + ./internal/audio` failed in `internal/stage` at the already registered + WhisperX fake request-slice race from + `TestTranscribeStageTranscribesPreparedAudio`; `internal/audio` passed (1.85 + s command wall time). This is the expected `TST-001` limitation and no new + race signature appeared. No live adapter, credential, destructive, or paid + operation was exercised. +- Contract conclusions: prepare's ordinary source selection, stable copying, + provenance, sorting, S3/local exclusivity, and non-empty previous hydration + are coherent. Transcribe has unique filename-derived speakers, bounded + concurrency, exact returned-path identity, deterministic successful ordering, + and all-or-nothing handling of recorded adapter failures. Transformations are + manifest-first, run-local, schema/report aware, and preserve diagnostic/output + classification. Disabled trim is successful copy processing; disabled render + is successful no-output execution whose later enablement requires force. +- Findings: confirmed `COR-017`, `COR-018`, and `COR-019`; added `ARC-006`, + `DUP-006`, `COM-004`, and `TST-008`; refined `ARC-001`, `ARC-002`, `ARC-005`, + `RSK-013`, and `TST-001`. Scenario 8 is resolved through worker aggregation + and subprocess-backed ordinary stages. +- Explicit no-finding conclusions: sorted current inputs and transformation + results are deterministic; basename collisions from different sources fail; + transcribe rejects redirected results; recorded adapter/schema failure does + not canonically materialize a successful subset; subprocess-backed stages are + synchronous and propagate adapter errors; trim debug render, logs, and + generated configs remain diagnostics; a generic stage framework is not + justified. Current production transformation adapters return their requested + output paths, so inconsistent result-path authority remains architectural, + not a present production data defect. +- Later-stage assignments: Stage 10 owns analyze's remaining `ARC-002` outcome. + Stage 11 owns `ARC-001`, `ARC-005`, `ARC-006`, `DUP-006`, `COM-004`, and the + narrow shared-owner decisions. Stage 12 owns the risk-based additions in + `TST-001`, `TST-007`, and `TST-008`.