diff --git a/docs/roadmap/audit-findings.md b/docs/roadmap/audit-findings.md index 029ca32..7f05616 100644 --- a/docs/roadmap/audit-findings.md +++ b/docs/roadmap/audit-findings.md @@ -1,10 +1,47 @@ # Codebase Audit Findings -Status: in progress +Status: complete This document is the working ledger and final report for the audit defined by the [audit plan](audit-plan.md) and [audit sequence](audit-sequence.md). The -audit is investigative: candidate findings below are not remediation changes. +audit is investigative: findings below are not remediation changes. +The classifications and backlog describe proposed future work; no production +change is included in this report. + +## Executive Assessment + +Narratio has a clear stage-oriented architecture, deterministic normal +execution, strong package ownership for most parsing and protocol behavior, and +substantial focused coverage of ordinary lifecycle, adapter, artifact, and +workflow paths. The session manifest is consistently treated as progress +authority, the canonical stage order is explicit, run-local outputs are usually +validated before canonical materialization, publish writes its pointer last, +restore installs the manifest last, and the default test suite is fast, +offline, and credential-free. The audit found no production dependency +reversal, generic workflow-engine drift, unsupported live-service dependency in +tests, or broad need for new abstraction. + +Those strengths do not close several consequential boundary defects. The most +urgent findings are symlink-following mutation/deletion and publish reads +(`COR-003`, `COR-005`), restore accepting state not bound to the selected +remote commit (`COR-008`), unsafe identity components crossing local and remote +namespaces (`COR-002`), and secret/private-data exposure through permissive +modes, filesystem-secret reads, and subprocess diagnostics (`RSK-004`, +`RSK-010`, `RSK-012`). Remote replacement publish can also invalidate the +previous readable commit before its final pointer write (`COR-004`). These are +high-consequence integrity, confidentiality, destructive-operation, and +recovery boundaries even though several require an uncommon failure, hostile +filesystem state, or multi-user deployment. + +The broader result is not a recommendation for wholesale redesign. The +preferred remediation is a dependency-ordered set of narrow owners: establish +confined filesystem and durable-write capabilities; make manifest identity and +terminal persistence singular; repair publish/restore commit authority; then +correct configuration, adapter, stage, and analyze contracts. Structural, +test, efficiency, and comment work should follow or accompany those roots only +where it makes the corrected invariant easier to preserve. The ordered backlog +below consolidates 77 confirmed IDs into coherent workstreams so remediation +does not repeat discovery or fix the same mechanism in several callers. ## Audit Identity And Baseline @@ -110,7 +147,7 @@ These are prioritization signals, not findings: | 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 | 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 | complete | All 749 tests were inventoried by behavior owner and consequential risk; every prior `TST` candidate was classified, five suite-wide candidates were added, and coverage, doubles, helpers, determinism, offline behavior, runtime, fuzzing, and automation were assessed. The prescribed shuffled run exposed `TST-011`; the prescribed race run reproduced only `TST-001`. | -| 13: synthesis | not_started | No final ranking or accepted-risk decisions yet. | +| 13: synthesis | complete | Revalidated the pinned implementation boundary, reconciled all registers and matrices, retained 77 confirmed IDs with four merged/rejected dispositions, recorded positive conclusions, dimensions and accepted risks, and produced an 11-workstream dependency-ordered remediation backlog. Report links and document integrity were validated; no production change occurred. | ## Area Coverage And Ownership @@ -313,18 +350,18 @@ terminalization/reconciliation harder to test. ## Cross-Boundary Scenario Assignments -| Scenario | Primary audit stage | Supporting packages and focused tests | -| --- | --- | --- | -| 1. Success becomes non-resumable, rerun fails, later reuse decision | 2 | `internal/app`, `internal/manifest`, `stage.ResumeValidator`; `runner_test.go`, `extract_lifecycle_test.go`, manifest transition tests | -| 2. Forced/changed upstream outcome with succeeded, self-skipped, disabled downstream | 2 | `internal/app`, `internal/stage`, `internal/manifest`; run-control, runner, extraction-lifecycle tests; disabled-stage detail revisited in 8 | -| 3. Extraction bundle followed by configuration/transitive-input change | 9 | Extract/resume, Notarius adapter, artifacts/fileops tests; downstream resolution revisited in 10 | -| 4. Published/restored/prepared previous state consumed locally by analyze | 5 | `internal/app`, `internal/previouscache`, `internal/audio`, `internal/artifacts`; restore/prepare tests; analyze consumption revisited in 10 | -| 5. Publish failure at every upload boundary, then status/restore/retry | 4 | Publish stage, storage fake/adapter, app status/restore; publish and operator-helper tests; restore interpretation revisited in 5 | -| 6. Restore identical/conflict/unsafe/cache/pre-manifest-install cases | 5 | Restore discovery/plan/execute/report, artifacts, previouscache, audio; restore test suite | -| 7. Cleanup after skipped/failed/locked/partial/committed publish | 4 | Publish metadata, post-publish cleanup, cleanup targets, pathsafe; publish/cleanup tests | -| 8. Cancellation through workers, HTTP, subprocess, storage, manifests | 7 | Adapter and subprocess tests; transcribe/stage tests in 8; runner reporting in 2 | -| 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 | Primary audit stage | Final conclusion | Principal findings or accepted boundary | +| --- | --- | --- | --- | +| 1. Success becomes non-resumable, rerun fails, later reuse decision | 2 | Session reuse is conservative and correct; handled validation errors leave inaccurate invocation audit state. | `RSK-001`, `TST-002`; abrupt-death residue is accepted below after handled errors are repaired. | +| 2. Forced/changed upstream outcome with succeeded, self-skipped, disabled downstream | 2 | Canonical invalidation is sound; durable success, self-skip, and idempotent skip are distinct, but documentation blurs them. | `COM-002`; no production lifecycle defect. | +| 3. Extraction bundle followed by configuration/transitive-input change | 9 | Configured values are fingerprinted and external same-path changes are operator-forced; the direct trimmed input is incorrectly omitted. | `COR-020`; accepted external-force boundary below. | +| 4. Published/restored/prepared previous state consumed locally by analyze | 5 | Analyze is correctly local-only, but commit binding, readiness, source mapping, and effective selection are incomplete. | `COR-008`, `COR-010`, `COR-011`, `COR-022`. | +| 5. Publish failure at every upload boundary, then status/restore/retry | 4 | Pointer-last order alone is insufficient because the fixed manifest replaces the prior pair and readers do not share one strict authority rule. | `COR-004`, `COR-008`, `TST-004`. | +| 6. Restore identical/conflict/unsafe/cache/pre-manifest-install cases | 5 | Ordinary deterministic planning is strong; committed scope, directory force, coherent transition, audio identity, and foreign paths remain unsafe. | `COR-008`, `COR-009`, `RSK-006` through `RSK-008`, `TST-005`. | +| 7. Cleanup after skipped/failed/locked/partial/committed publish | 4 | Gates correctly require an explicit commit, but automatic cleanup is not durable/retryable and path mutation is not symlink-confined. | `COR-003`, `COR-006`, `COR-007`. | +| 8. Cancellation through workers, HTTP, subprocess, storage, manifests | 7 | Direct operations generally receive cancellation and release resources; transcribe can report partial success and subprocess descendants can survive. | `COR-018`, `RSK-011`, `RSK-014`, `EFF-002`. | +| 9. Disabled/unselected/reused/generated/extraction/previous source then publish filtering | 10 | Source families and publish filtering are deterministic, but effective selection does not consistently drive validation/prerequisites and some optional/passthrough contracts fail. | `COR-021` through `COR-024`, `RSK-015`, `ARC-007`. | +| 10. Concurrent same-session invocation and lock cleanup failures | 3 | Live local exclusion spans the full mutation lifetime; stale/release failures and remote lock snapshots remain operational risks. | `RSK-003`, `RSK-005`, `TST-003`. | Scenario 3 is resolved. The fingerprint deterministically observes the resolved executable and top-level config *paths*, pipeline ID, normalized timeout, @@ -1542,6 +1579,153 @@ 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. +## Audit Completion And Final Disposition + +The implementation, tests, and canonical current-behavior documentation remain +identical to the pinned audited revision. `git diff` from +`74e2d21de5fb2ada0be5ef3fe9333e0d48ac7fb3` through the Stage 12 entry revision +contains only this ledger and its two audit specifications; there is no Go, +module, example, policy, CLI, configuration, operations, integration, or +internal-document change. Stage 13 re-ran graph ownership searches across the +runner, canonical paths, file operations, publish/current state, restore, +configuration, adapters, ordinary stages, extraction, analyze, and their +focused tests. Every detailed finding below still resolves to its recorded +implementation owner and consumer boundary. No item rests on a metric alone, +and no confirmed item was downgraded or rejected during revalidation. + +### Completion criteria + +| Criterion | Final evidence and conclusion | +| --- | --- | +| Every inspection area reviewed | All 15 area-ledger rows are `reviewed`; Stages 2-12 record contracts, production owners, focused tests, commands, findings, and explicit no-finding conclusions. | +| Lifecycle and scenarios concluded | Every lifecycle outcome is source-backed for both manifests. The ten-scenario matrix above now records a final conclusion and root finding or accepted boundary for each row. | +| Duplication classified | `DUP-001` through `DUP-003` and `DUP-005` through `DUP-008` are narrow shared mechanisms/policies; `DUP-004` is rejected because its accepted language differs. Broad stage, workflow, resolver, validator, adapter, fake, and manifest abstractions are explicitly rejected. | +| Simplification and efficiency bounded | `SIM-001`, `SIM-003`, and `SIM-004` name smaller owners without hiding policy; `SIM-002` merges into `DUP-007`. `EFF-001` and `EFF-002` name representative byte/latency/memory measurements; no other performance claim survived workload review. | +| Comments preserve rationale | `COM-001` owns ledger ordering rationale; `COM-002` owns durable no-output terminology; `COM-003`, `COM-005`, and `COM-006` repair verified stale/incomplete claims. `COM-004` merges into `COM-002`. | +| Test suite assessed by risk | The final matrix covers integrity, destructive actions, compatibility, security, concurrency, idempotency, recovery, cancellation, partial success, redundancy, doubles, helpers, coverage, fuzzing, determinism, offline behavior, runtime, and automation. | +| Findings deduplicated and ranked | 77 IDs remain confirmed. Four stable IDs are non-independent dispositions: `ARC-002` and `COM-004` merge into `COM-002`, `SIM-002` merges into `DUP-007`, and `DUP-004` is rejected. The backlog below is authoritative for dependency/risk order. | +| Questions, accepted risks, and limitations explicit | The final decision table after the registers assigns an owner and safe interim boundary to every unresolved contract family, followed by intentionally accepted risks and evidence limitations. | + +### Positive conclusions + +- The canonical stage registry and planner are deterministic, and force, + changed outcomes, self-skip, ordinary success, failure, and retry invalidate + or preserve downstream state conservatively. +- Session state is consistently the cross-invocation authority; individual + atomic saves preserve the previous file on ordinary pre-rename failure, and + terminal session-first ordering preserves safe reuse even when the audit + ledger becomes inaccurate. +- Directory promotion strongly validates and identity-checks its source, + rejects replacement of an existing immutable destination, syncs its durable + tree, and fails explicitly on unsupported platforms. +- Publish uses deterministic upload ordering and writes the current pointer + last. Manual cleanup is explicit, scoped, dry-runnable, cache-preserving by + default, and correctly independent of automatic postcommit policy. +- Restore planning is deterministic, confines lexical remote mappings, keeps + dry-run free of durable local writes, validates downloads before install, and + installs the session manifest last. Analyze consumes previous-session state + locally and never performs an implicit remote read. +- Configuration rejects unknown fields, centralizes defaults, validates + maintained examples, keeps ordinary secrets indirect, and conditionally + composes external adapters rather than contacting live services at startup. +- Adapter protocol suites protect command arguments, schemas, retries, + cancellation at direct boundaries, error adaptation, and deterministic + outputs. Stages generally validate run-local results before canonical + materialization and record artifacts separately from diagnostics. +- Production dependencies are active and confined to their boundary. The audit + found no generic workflow-engine need, broad dependency-direction inversion, + additional material hot path, or reason to chase uniform coverage. +- The normal suite passes in about 3.5 seconds without live services, paid APIs, + ambient credentials, or fixed external ports. Most tests use real temporary + files, loopback HTTP, or the current test binary and assert observable state + rather than private choreography. + +### Final prioritization dimensions + +The 42 detailed `COR`, `RSK`, `EFF`, and confirmed `ARC-004` entries already +record impact, likelihood, confidence, scope, owner, tests, and dependencies +separately; Stage 13 revalidation did not change those ratings. The following +compact register supplies the same dimensions for the confirmed structural, +test, and clarity entries. “Inherited” impact means the item is required to +make its named correctness/security root durable rather than representing a +second production defect. + +| IDs | Impact | Likelihood | Confidence | Scope | +| --- | --- | --- | --- | --- | +| `ARC-001` | Low current; medium future contract misuse | Low until a consumer appears | High | Small removal | +| `ARC-003` | Medium remote/local state ambiguity | Low-to-moderate as consumers grow | High | Small-to-medium model/protocol clarification | +| `ARC-005` | Medium adapter/test contract drift | Moderate during adapter change | High | Small request/constructor change | +| `ARC-006` | High run-local output-authority risk | Low currently; moderate with a divergent adapter | High | Medium shared contract and caller repair | +| `ARC-007` | High effective-selection correctness | Moderate for explicit selection | High | Medium app/config/artifact planning change | +| `DUP-001`, `DUP-003`, `DUP-005` | High durability/security drift inherited from `RSK-002`/`COR-003` | Moderate during shared repair | High | Medium shared fileops capability and caller migration | +| `DUP-002` | Medium run-identity drift | Low-to-moderate | High | Tiny path-owner correction | +| `DUP-006` | Medium transcript-source drift | Moderate during source changes | High | Small-to-medium artifact resolver migration | +| `DUP-007` | High extraction evidence drift | Moderate during compatibility/security changes | High | Medium typed evidence owner | +| `DUP-008` | High analyze/publish/operator catalog drift | Moderate as source families change | High | Medium shared bootstrap with explicit caller policy | +| `SIM-001` | Medium audit/persistence correctness support | Moderate on new error paths | High | Small-to-medium runner extraction | +| `SIM-003` | Medium maintainability and ordering support | Moderate during analyze changes | High | Medium typed plan/context refactor | +| `SIM-004` | Low dead-code cost | Certain but harmless | High | Tiny deletion | +| `COM-001` | Medium risk of weakening ledger order | Moderate during runner repair | High | Tiny comment after `SIM-001` | +| `COM-002`, `COM-005` | Medium operator/developer lifecycle misunderstanding | Present in current docs | High | Small documentation correction | +| `COM-003`, `COM-006` | Low stale-contract friction | Present in current comments | High | Tiny comment deletion/update | +| `TST-001`, `TST-011` | Medium loss of diagnostic determinism | Certain under race/repetition runs | High, reproduced | Small fake/environment cleanup | +| `TST-002` through `TST-010`, `TST-013` | Inherited critical/high confidence for named integrity, recovery, security, and concurrency roots | Same plausible scenarios as linked findings | High after cross-layer inventory | Medium distributed regression additions, each at its named owner | +| `TST-012` | High release-validation exposure | Ongoing on every unvalidated change/tag | High | Small-to-medium automation addition | +| `TST-014`, `TST-015` | Low-to-medium recurring maintenance friction | High during legitimate config/stage changes | High | Medium test consolidation | + +### Dependency-ordered remediation backlog + +This order is authoritative over the category-number order used by the detailed +registers. Regression tests named by a workstream should land with that repair; +standalone test-suite and documentation cleanup follows the owning behavior. + +1. Establish safe identity, filesystem, secret, and durable-write foundations: + `COR-002`, `COR-003`, `COR-005`, `RSK-002`, `RSK-004`, `RSK-010`, + `RSK-012`, and `RSK-013`, together with `DUP-001`, `DUP-003`, `DUP-005`, + `TST-003`, the relevant `TST-007` cases, and `TST-013`. This capability must + exist before caller-specific restore, cleanup, adapter, or promotion fixes. +2. Make manifest identity and terminal persistence singular: `COR-001`, + `RSK-001`, `SIM-001`, `COM-001`, `TST-002`, and `TST-006`. Preserve session + authority and visible running-versus-terminal save order; do not introduce a + generic lifecycle framework or promise cross-file atomicity. +3. Repair remote publication and postcommit cleanup: `COR-004`, `COR-006`, + `COR-007`, `RSK-005`, `ARC-003`, `DUP-002`, and `TST-004`. Use one immutable + commit model and stateful store tests; retain pointer-last semantics and make + cleanup evidence durable/retryable. +4. Bind restore and previous state to that commit model: `COR-008` through + `COR-011`, `RSK-006` through `RSK-009`, `TST-005`, and then `EFF-001`. + Reuse the safe filesystem capability and current-state truth table rather + than creating restore-only variants. +5. Make configuration and composition truthful before execution: `COR-012` + through `COR-016`, `COR-024`, and `ARC-004`, with `TST-011` and `TST-014`. + Resolve the storage, notification, previous-session, and Scriptorium contract + decisions explicitly; reject unsupported values rather than guessing. +6. Complete transport liveness and bounded resource behavior: `RSK-011`, + `RSK-014`, `EFF-002`, `TST-001`, and the remaining `TST-007` cases. Reuse + the step-1 output/redaction owner and keep platform process mechanics in the + shared launcher. +7. Correct ordinary-stage and extraction transitions: `COR-017` through + `COR-020`, `ARC-006`, `DUP-006`, `DUP-007`, `TST-008`, and `TST-009`. + Preserve explicit stage policy while sharing only artifact resolution and + typed extraction evidence. +8. Unify effective analyze selection and dependency behavior: `COR-021` + through `COR-023`, `RSK-015`, `ARC-007`, `DUP-008`, `SIM-003`, and + `TST-010`. Coordinate `COR-024` from step 5 rather than inventing wire + semantics in this refactor. +9. Perform remaining architectural and structural cleanup: `ARC-001`, + `ARC-005`, `SIM-004`, and any now-obsolete wrapper code. These are + independently small but should not distract from data-safety work. +10. Enforce and streamline the suite: `TST-012`, then `TST-014` and `TST-015`. + Keep focused owners and representative assembled workflows; add the race + job only after `TST-001` makes its signal trustworthy. +11. Apply documentation/comment repairs after their contracts settle: + `COM-002`, `COM-003`, `COM-005`, and `COM-006`. Update canonical current- + behavior documents in the same changes that implement the decisions. + +`EFF-001` and `EFF-002` deliberately follow their correctness owners and require +the measurements stated in their detailed entries. No other performance work +should be added to this backlog without a representative workload. + ## Confirmed Findings ### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity @@ -2399,8 +2583,9 @@ credible workload large enough to justify a performance finding. boundary cases. Filesystem crash durability itself remains a Stage 3 concern. - Dependencies: `SIM-001` may provide one failure-finalization path and `TST-002` records the missing persistence seam. Stage 5 should check how - status/restore presents abandoned runs; Stage 13 should decide whether true - process interruption is accepted risk after handled errors are fixed. + status/restore presents abandoned runs. The final accepted-risk section + permits genuinely abrupt interruption residue only after handled errors are + terminalized and session progress remains authoritative. ### `RSK-002`: single-file atomic replacement is not crash-durable @@ -2502,8 +2687,9 @@ credible workload large enough to justify a performance finding. promoted-bundle paths; document platform/ACL limitations instead of assuming POSIX bits are universal. - Dependencies: Stage 6 confirmed there is no mode/config override or documented - deployment privacy guarantee. Stage 7 owns adapter diagnostics, and Stage 13 - should rank the risk against deployment assumptions. + deployment privacy guarantee. Stage 7 owns adapter diagnostics; the final + backlog places secure defaults in the first safety workstream while retaining + deployment ACL/umask details as an explicit evidence limitation. ### `RSK-005`: remote publish locks are race-prone snapshots @@ -3002,14 +3188,12 @@ credible workload large enough to justify a performance finding. the later `ARC-002`/maintainability synthesis; this finding owns transport selection and operator truthfulness, not stage-state mechanics. -## Candidate Register +## Classified Structural, Test, And Clarity Register -The `TST` entries now carry their Stage 12 classifications. Structural entries -retain their original locations for traceability, but their Stage 11 -classifications are also 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. +The entries retain their original locations for traceability, but their Stage +11 or Stage 12 classifications are final: confirmed recommendations are ordered +in the remediation backlog above, 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 @@ -3921,7 +4105,7 @@ reopening. | --- | --- | --- | | 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 | 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. | +| Coverage percentages | rejected as standalone findings | Stage 12 used them only to revisit consequential branches and high-coverage duplication; the final test assessment sets no 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. | @@ -3984,69 +4168,82 @@ reopening. | 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 `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 +## Open Decisions, Accepted Risks, And Limitations -- Should startup/status reconcile abandoned `running` invocation records, mark - them interrupted, or retain them as an explicitly accepted audit limitation? -- Which manifest model should retain run-scoped identity after `COR-001` is - repaired, given that publish archives and automatic cleanup both consume it? -- Which cross-platform root-relative filesystem primitive should own the - symlink-safe mutation capability required by `COR-003` without moving stage - policy into low-level `fileops`? -- Should sentinel locking be replaced by an OS-released lock, or can a - conservative ownership/recovery protocol satisfy `RSK-003` across supported - platforms? -- Which existing identifier spellings constrain the strict segment validation - required by `COR-002`? -- 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? -- 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. -- Should remote lock activation be snapshot-based or immediate, and what - conditional-write capability can enforce the chosen rule across hosts? -- 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? -- Should restore's local commit point use a staging tree/snapshot swap, or can a - journal plus locked revalidation make incremental replacement and recovery - sufficiently explicit under `RSK-006`? -- What durable identity should govern audio cache and existing-file reuse: - storage version, ETag with provider-specific rules, or a published checksum? -- Should restored manifests be rewritten to canonical local paths, or should all - consumers ignore persisted absolute paths in favor of workspace-relative - identities? -- How should publish metadata retain a stable source-to-destination mapping for - previous artifacts without reintroducing mutable configuration authority? -- Should `--previous-session-id` remain a strict expectation or become an - explicit override, and which compatibility cases already depend on omission? -- Is any storage backend other than explicit S3 or disabled/local behavior part - of the supported configuration language? -- What ownership and maximum-size guarantees do deployment packages establish - for the filesystem secrets directory, and can all supported platforms enforce - no-follow regular-file reads? -- Should remote session loading retain bytes until canonical preparation or - expose an owned temporary handle whose cleanup spans every command consumer? -- Is notification delivery a product requirement, and if so which provider - protocol should become the canonical integration contract? Until that is - answered, `ARC-004` requires non-placeholder settings to fail explicitly. -- Which supported-platform process-tree primitive can give `RSK-011` equivalent - 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`? -- Can subprocess environments be reduced to explicit allowlists, or must - `RSK-012` retain broad inheritance and stream-redact every known sensitive - value? -- Should Notarius expose or Narratio require an explicit dependency-generation - digest that covers profiles, prompts, modules, references, executable build, - and other transitive inputs, or is documented operator-forced invalidation - the intended long-term contract? -- Which automation service and cadence should carry the validation required by - `TST-012`, particularly the roughly 54-second race run, and should tagged - release builds consume an already validated revision or repeat validation? +The audit establishes the unsafe current behavior and smallest safe interim +boundary; it does not make product or platform choices that require deployment +knowledge. These decisions are inputs to the remediation sequence, not reasons +to leave the current defects implicit. -No accepted risks or final audit conclusions are recorded yet. +| Decision family | Owner | Required safe interim boundary | Related findings | +| --- | --- | --- | --- | +| Run-scoped manifest identity and abandoned invocation presentation | App/manifest maintainers | Recompute or validate one identity unit; terminalize every handled error. Status may label genuinely abandoned records without changing session progress authority. | `COR-001`, `RSK-001`, `SIM-001` | +| Cross-platform confined mutation, durable replacement, and local locking | Fileops/artifacts owners with supported-platform maintainers | No path-based destructive operation may follow an untrusted ancestor; unsupported safe primitives must fail closed. Lock release failure must be observable. | `COR-003`, `RSK-002`, `RSK-003` | +| Identifier compatibility | Config/artifacts owners with operators | Inventory deployed spellings before enforcing one opaque-segment grammar; meanwhile no constructor may return an escaped namespace. | `COR-002` | +| Runtime permissions and filesystem-secret directory guarantees | Packaging/operations owner with app/fileops | Use private defaults and no-follow, bounded regular-file acquisition unless a documented service ownership/ACL contract proves a stronger equivalent. | `RSK-004`, `RSK-010` | +| Scriptorium `artifact`/`path` semantics | Product owner and Scriptorium integration owner | Reject non-empty unsupported fields or remove them from the public contract until exact wire semantics are specified. | `COR-024` | +| Remote commit representation, lock activation, and committed restore scope | Publish/restore/artifacts owners with storage capability owner | Preserve the old readable commit until one final atomic selection; restore and status must validate the same selected identity. Do not emulate conditional writes with an unsafe load/replace race. | `COR-004`, `COR-008`, `RSK-005`, `ARC-003` | +| Restore local commit/recovery, content identity, canonical paths, and previous-source mapping | Restore/artifacts/audio/previouscache owners | Keep the manifest last, revalidate under the lock, never report unresolved conflicts as success, and prefer workspace-relative typed identity over foreign absolute paths or size-only reuse. | `COR-009` through `COR-011`, `RSK-006` through `RSK-008` | +| Previous-session expectation and storage backend language | CLI/config/product owner | Preserve the documented flag as a strict expectation unless a compatibility review deliberately changes it; accept only explicitly implemented backend values. | `COR-014`, `COR-015` | +| Remote session configuration lifetime | App configuration owner | Return an owned temporary handle/lifetime whose cleanup occurs after all command consumers finish; never persist ephemeral provenance as a durable path. | `RSK-009` | +| Notification product requirement | Product and integration owner | Reject or clearly reserve non-placeholder settings until a provider contract and production sender exist. A no-op must be an explicit operator choice. | `ARC-004` | +| Process trees, diagnostic redaction, and output size limits | Shared subprocess/adapter owners with platform and operations input | Kill and wait the owned tree, never persist known secret values, and use bounded no-follow regular-file acquisition. Choose limits per external contract rather than one global constant. | `RSK-011` through `RSK-013` | +| External Notarius dependency identity | Notarius integration and operations owners | Continue to require/document `--force` for same-path external/transitive changes unless a version/digest contract is introduced; direct Narratio input bytes must still join the fingerprint. | `COR-020` | +| Validation automation and race cadence | Repository maintainers | Every change and release revision must receive normal test/vet/build validation. Add race automation after `TST-001`; choose per-change versus scheduled cadence from measured capacity, not by omitting the check entirely. | `TST-001`, `TST-012` | + +### Intentionally accepted risks + +- The two manifest files do not form one atomic transaction. This is accepted + because the session manifest is the sole progress authority and a run + manifest is an invocation audit record. Remediation must test every durable + disagreement boundary and terminalize handled errors; it need not introduce + a distributed transaction. +- An uncatchable process or host death may leave an old invocation record + `running`. After handled errors are terminalized, that residual historical + inaccuracy is accepted provided startup/status can distinguish or document it + and the session manifest continues to drive conservative retry. It is not + acceptable for controlled errors to leave the same ambiguity. +- Restore remains an incremental operation without rollback. The cost and + portability of a full workspace transaction are not justified by current + evidence. This acceptance does not waive `RSK-006`: planning must be + revalidated under the lock, partial state must be diagnosable/retryable, and + the old manifest must not silently authorize replaced bytes. +- Same-path changes to external Notarius executables, profiles, prompts, + modules, references, environment, and provider behavior remain an + operator-forced invalidation boundary. Narratio cannot infer all transitive + external state without a new integration digest contract. Its direct trimmed + transcript is not part of this acceptance and remains `COR-020`. +- A configured artifact that is deliberately non-executable may be reused from + its canonical non-empty file without prior analyze-manifest provenance. + Freshness is operator-owned for that workflow. Explicit selection and + prerequisite planning must still use one effective-set authority under + `ARC-007`/`COR-022`. +- The normal test suite remains serial where process-global fixtures exist; no + blanket `t.Parallel` conversion is justified by its roughly 3.5-second + runtime. Repeated shuffled execution may be scheduled rather than run on + every change, provided `TST-011` is fixed and ordinary CI remains mandatory. + +### Evidence limitations + +- Dynamic validation ran on Linux/amd64 with Go 1.26.5. macOS and Windows were + cross-build/reasoning targets only; platform-specific no-follow, directory + sync, process-tree, and lock choices require focused native validation. +- No live S3 service, paid API, production subprocess, credentialed operation, + deliberate power loss, or destructive external workflow was exercised. The + audit used source reasoning, existing deterministic fakes, temporary + filesystems, loopback HTTP, and helper subprocesses as required by policy. +- Crash durability, upload accepted-with-error behavior, remote conditional + writes, and deployment ACL/umask protection remain environment-dependent. + Their absence was confirmed in the current contracts, but exact provider and + filesystem failure rates were not estimated. +- The moderate code graph excludes documentation, examples, and the command + entry point. Those artifacts were reviewed directly with repository tools; + graph metrics were never used alone to confirm a finding. +- This report is pinned to the implementation revision in the audit identity. + Later commits through Stage 13 alter audit documentation only. Any production, + test, canonical-contract, example, or dependency change invalidates the + affected evidence and must trigger focused re-audit before remediation uses + the conclusions. ## Completed-Stage Evidence @@ -4577,3 +4774,43 @@ No accepted risks or final audit conclusions are recorded yet. remediation ordering, accepted-risk decisions, and final audit closeout. It should not reopen the risk-based test ownership and marginal-value decisions recorded here without new evidence. + +### Stage 13 + +- Entry revision: `14ef59a` (`Document test suite policy audit conclusions`), + with a clean worktree. The implementation, tests, examples, dependencies, and + canonical current-behavior documents are byte-identical to the pinned audit + revision; commits since it add only the audit plan, sequence, and ledger. +- Scope and policy reviewed: development guide, all architecture, + documentation, and testing policy, exact Stage 13 sequence, audit completion + criteria, every lifecycle/scenario/risk matrix, all 81 stable finding IDs and + their detailed evidence, the classification log, unresolved questions, and + completed-stage records. +- Revalidation evidence: current graph architecture and targeted ownership + searches covered the runner/manifest, path/fileops, publish/current-state, + restore/previous/audio, configuration/composition, adapters, ordinary stages, + extraction, analyze/artifact, and focused-test boundaries. A repository diff + from the pinned revision confirmed no affected source, test, contract, + example, or module change. The prior exact snippets, callers, tests, and + canonical contracts therefore remain current; no metric-only finding was + admitted and no confirmed root required downgrade. +- Reconciliation: 77 IDs remain confirmed. `ARC-002` and `COM-004` merge into + `COM-002`; `SIM-002` merges into `DUP-007`; `DUP-004` remains rejected; + `COM-005` remains independently confirmed only for its source/catalog + omission while its no-output wording is owned by `COM-002`. Rejected signal + themes remain in the classification log to prevent rediscovery. +- Final report additions: executive assessment; completed scenario and audit- + criteria matrices; positive conclusions; separate impact, likelihood, + confidence, and scope ratings for structural/test/clarity items; an + 11-workstream dependency/risk-ordered remediation backlog; owned open + decisions; intentionally accepted residual risks; and environmental/evidence + limitations. +- Validation: all relative links in `audit-findings.md`, `audit-plan.md`, and + `audit-sequence.md` resolve; `git diff --check` passes. The full Stage 0 + implementation baseline was not rerun because Stage 13 proved that no + implementation, test, dependency, example, or canonical contract changed; + Stage 12's final normal suite and vet results remain the latest validation. +- Exit conclusion: every audit-plan completion criterion and Stage 13 + deliverable is satisfied. No production, test, example, dependency, or + current-behavior documentation change is included. The report is sufficient + to prepare remediation work without repeating discovery.