From bb2d606dbbd4b3d8aa43f1e145bf7a10d7eed2d9 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 10 Aug 2026 12:28:36 +0000 Subject: [PATCH] Audit runner and manifest lifecycle behavior --- docs/roadmap/audit-findings.md | 321 +++++++++++++++++++++++++++++---- 1 file changed, 289 insertions(+), 32 deletions(-) diff --git a/docs/roadmap/audit-findings.md b/docs/roadmap/audit-findings.md index cb8e08e..ddfba7f 100644 --- a/docs/roadmap/audit-findings.md +++ b/docs/roadmap/audit-findings.md @@ -99,7 +99,7 @@ These are prioritization signals, not findings: | --- | --- | --- | | 0: baseline | complete | Revision/environment pinned; graph refreshed; inventories and every prescribed baseline command recorded. `TST-001` owns the non-blocking race limitation. | | 1: contract and boundary map | complete | Canonical contracts and focused internal docs read; ownership, stage-contract, lifecycle, scenario, area, and preliminary risk-to-test matrices recorded below. | -| 2: runner and manifest | not_started | Assigned lifecycle and dual-ledger questions below. | +| 2: runner and manifest | complete | Full/single-stage entry paths, every lifecycle outcome, both manifest models/transitions, save disagreement states, canonical invalidation boundaries, and runner lock lifetime reviewed. Focused app/manifest test and race commands passed. Confirmed `COR-001` and `RSK-001`; assigned `DUP-001`, `SIM-001`, `COM-001`, `TST-002`, and lock-release details to later stages. | | 3: paths and filesystem | not_started | Assigned path, lock, artifact, and mutation questions below. | | 4: publish and cleanup | not_started | Assigned remote commit and cleanup scenarios below. | | 5: restore and previous state | not_started | Assigned restore and previous-cache scenarios below. | @@ -120,14 +120,14 @@ has been mapped but not behaviorally audited. | Inspection area | Canonical implementation owner | Primary audit stage | Status | | --- | --- | --- | --- | | Process and application boundary | `cmd/narratio`, `internal/app` | 6 (runner lifecycle portions in 2; publish/restore portions in 4-5) | assigned | -| Stage registry and runner | `internal/stage`, `internal/app` | 2 | assigned | +| Stage registry and runner | `internal/stage`, `internal/app` | 2 | reviewed | | Configuration | `internal/config` | 6 | assigned | | Prepare and audio | `internal/stage`, `internal/audio`, `internal/previouscache` | 8 | assigned | | Transcript stages | `internal/stage` plus tool adapters | 8 | assigned | | 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 | assigned | -| Manifest state | `internal/manifest`, transition policy in `internal/app` | 2 | assigned | +| Manifest state | `internal/manifest`, transition policy in `internal/app` | 2 | reviewed | | Artifacts, paths, and policy | `internal/artifacts`, `internal/artifactpolicy`, `internal/pathsafe` | 3 (resolution consumption revisited in 10) | assigned | | Restore | `internal/app`, `internal/artifacts`, `internal/previouscache`, `internal/audio` | 5 | assigned | | File operations | `internal/fileops`, `internal/pathsafe`, local artifact store | 3 (promotion vertical slice in 9) | assigned | @@ -197,8 +197,8 @@ partial display type or become an enforceable declaration is deferred as ## Lifecycle Matrix -This is the intended contract map to be walked through both durable ledgers in -Stage 2. Cells marked `unknown` are not treated as implementation conclusions. +This began as the intended contract map and is now source-backed for both +durable ledgers by the Stage 2 review. | Outcome | Session manifest intent | Invocation manifest intent | Downstream and next-invocation intent | | --- | --- | --- | --- | @@ -206,14 +206,110 @@ Stage 2. Cells marked `unknown` are not treated as implementation conclusions. | Already-succeeded skip | Existing succeeded session record and payload remain unchanged, subject to resume validation | Action/status record a skip and stable reason for this invocation | Reusable result remains authoritative; pipeline continues. | | Explicit self-skip | Session stage becomes skipped, clears older result payload, and may record bounded current skip details | Action was `run`, outcome is skipped with reason | Reconsidered later; a changed effective upstream outcome stales succeeded downstream work; identical extraction disabled skip is stable. | | Failure | Current stage becomes failed with error; current output/log/config/metadata payload is cleared | Action `run`, failed outcome and overall failed run | Current execution stops; affected succeeded downstream work is intended to stale; later invocation reruns non-succeeded stages. | -| Interruption | Model admits `interrupted`, but no production transition reference has yet been found; process death can leave persisted `running` state | A process death can leave the run non-terminal; exact recovery semantics are unknown | CLI promises continuation of interrupted/partial sessions because non-succeeded stages run; explicit status normalization is `RSK-001` for Stage 2. | +| Interruption | Model admits `interrupted`, but production never writes it; process death leaves the last durable status `running` and the running transition has already cleared the target's prior result details | The run remains non-terminal at its last durable per-stage state; no load or startup reconciliation changes it | Non-succeeded session stages execute on the next included plan, so continuation is conservative; the old invocation record remains inaccurate under confirmed `RSK-001`. | | Forced replacement | Target execution starts fresh; succeeded downstream records are pre-marked stale; current target payload clears on running | Force flag and `run` action recorded | Replacement result determines later execution; locks and safety policy remain authoritative. | | Non-resumable success | Prior success becomes stale while retaining details long enough for diagnosis/validation, then running clears them | Current invocation records execution after validation rejects skip | Obsolete result reruns; unsafe inability to decide stops without silently replacing current success. | -| Successful rerun | Target becomes succeeded with only new outputs/diagnostics/config/metadata | Current invocation records its own new success; earlier run manifests remain immutable | Changed effective outcome stales succeeded downstream work; identical effective outcome should avoid unnecessary invalidation. | +| Successful rerun | Target becomes succeeded with only new outputs/diagnostics/config/metadata | Current invocation records its own new success; earlier run manifests remain unchanged | A rerun after a non-succeeded state stales succeeded downstream work; force already stales it before execution. The runner does not compare output contents; identical repeated self-skip is the narrow no-invalidation case. | -Stage 2 must separately verify save failures before and after each session/run -transition, first- and last-stage behavior, and which fields are retained in -historical invocation records. +Stage 2 verified the matrix. The runner treats the session manifest as the only +cross-invocation decision source and each run manifest as a record of one +invocation. The resulting field behavior is: + +- entering `running` clears the session stage's former outputs, logs, generated + configuration, metadata, completion, and error; success installs only the + current result and clears the stage error, while failure and self-skip clear + result data before bounded current skip diagnostics are reapplied; +- staling deliberately retains prior result data and timestamps for diagnosis, + changes status/error/updated time, and prevents the result from being reused; +- an already-succeeded skip does not mutate the session record; the run record + separately stores action `skip`, status `skipped`, and reason + `already_succeeded` without copying the reusable outputs; +- an executed self-skip stores action `run` and status `skipped` in the run + record, so it remains distinguishable from an idempotent skip; +- a stage failure marks the session stage and run stage failed, records the + error in both ledgers, makes the run overall failed, and stops execution; +- force is stored at run level and pre-stales all succeeded canonical + downstream stages; non-resumable validation first stales and saves the target + and succeeded downstream stages, then executes; and a successful execution + following any non-succeeded prior state stales remaining succeeded downstream + work; and +- the session-level `last_error` is retained as historical information after a + later success. No production reader treats it as current status; stage and + run status are the operative fields. + +The invalidation helper derives position from the complete canonical registry, +not the selected plan. Consequently a single-stage replacement has the same +downstream effect as that stage in a full run. `prepare` can invalidate every +later succeeded stage and `notify` has no downstream target. Only succeeded +records need explicit staling: failed, skipped, stale, pending, running, and +interrupted records already execute on the next included plan. + +### Runner Entry, Lock, And Persistence Conclusions + +`Run` validates the assembled configuration and selection, builds the full +canonical plan, and delegates to `executeStages`. `RunStage`, `Analyze`, and +`Publish` select one canonical stage; the latter two force it. Single-stage +execution still uses the same lifecycle, session lock, invalidation, dual +manifests, cleanup check, and final run transition as a full run. + +The session lock is acquired after layout creation and before the session +manifest is loaded, then held through stage execution, all manifest saves, +post-publish cleanup, and the final run save. A competing same-session runner +therefore cannot enter manifest decision-making while the first holds the lock. +The deferred release error is discarded. Whether close/unlink failures can +leave a blocking or misleading lock requires the filesystem implementation +review assigned to Stage 3; the assembled runner suite has no concurrent-runner +or release-failure case. + +Both manifest stores use temp-file write, file sync, close, and same-directory +rename, so an error before rename leaves the prior individual file in place. +There is no atomic transaction or reconciliation protocol across the two +manifest files. The runner saves the run record first when +announcing execution, then saves the session record; for terminal outcomes it +saves the session authority first, then the run audit. The possible durable +states and their later interpretation are: + +| Failure boundary | Durable state | Later invocation behavior | +| --- | --- | --- | +| New session or identity save fails before initial run save | No run record; session is absent or remains at its prior contents. An identity-save attempt mutates the in-memory identity and `updated_at`, but none of it becomes durable. | The command stops before a stage. A later invocation loads/creates from the last durable session state. | +| Initial run save fails after session identity save | Session points at the new run ID, but that run's audit file may not exist. | Session stage states still govern reuse; a later invocation creates a different run ID. | +| Resume validation reports unsafe/indeterminate | Session success is preserved, but the already-created run remains overall `running`. | The prior success remains authoritative and validation is attempted again; the abandoned run is never reconciled. | +| Saving non-resumable staleness fails | Run remains initially `running`; session remains at the prior success. No stage executes. | Validation is attempted again without silently replacing the prior success. | +| Saving an ordinary skip to the run file fails | Session remains succeeded; the run file remains at its preceding state. | The stage is safely reconsidered as another skip. | +| Run `running` save fails | Session is unchanged and the stage does not execute. | Session authority makes the next decision conservatively. | +| Session `running` save fails after the run save | Run stage is `running`; session remains at its prior state and the stage does not execute. Forced downstream staleness is not durable. | A non-succeeded target retries; a prior success skips unless force/resume validation again requires replacement. | +| Terminal session save fails after stage work | Both durable records remain `running`, although the stage may already have external or canonical effects. | The session stage reruns because `running` is not reusable. Stage-owned idempotency remains essential. | +| Terminal run save fails after terminal session save | Session has the authoritative success, skip, failure, and downstream state; run stays `running`. | Execution resumes safely from the session, but the historical run remains inaccurate. | +| Post-publish cleanup fails | Session stage outcomes remain terminal; a successful run save marks the invocation failed, while a failed run save leaves its prior overall `running` state. | A later invocation decides stages from the session and re-enters the cleanup gate; Stage 4 owns whether that retry is safe for every cleanup state. | +| Final overall run save fails | Session and per-stage run records are terminal, but overall run status remains `running`. | A later invocation skips or reruns from session state and does not repair the old run. | + +Save errors are returned with both the stage error and persistence error when +both exist. The session store is injectable, but run persistence is a concrete +`LocalStore` outside the `manifest.Store` interface. This leaves the run-side +failure rows above unexercised by focused runner tests and makes centralized +terminalization/reconciliation harder to test. + +### Lifecycle Scenario Conclusions + +- Scenario 1 is functionally safe for reuse: a non-resumable success and its + succeeded downstream records are persisted stale before execution; a failed + rerun leaves the target failed and downstream stale; an ordinary retry runs + both. An inability to validate preserves the prior success rather than + replacing it. `RSK-001` records the inaccurate invocation audit left by that + controlled error. +- Scenario 2 is conservative and source-backed: force pre-stales succeeded + downstream work; failure and a changed effective outcome stale it; a changed + self-skip stales it; and an identical repeated self-skip does not. Here + identical means prior status skipped, zero outputs, and the same reason; + diagnostic/metadata differences are not compared. Disabled stages represented + as success remain reusable success, while an explicitly skipped downstream + stage is naturally reconsidered because only success is ever skipped. The + runner-level distinction is coherent; `ARC-002` remains assigned to Stages 4 + and 8 for the stage-specific contract and wording. +- The lock portion of Scenario 10 is resolved at the application boundary: + acquisition occurs before manifest access and the lock spans the entire + mutation lifetime. Stage 3 must decide the ignored-release and underlying + lock-file questions. ## Cross-Boundary Scenario Assignments @@ -255,11 +351,97 @@ This matrix identifies intended owners only. It makes no sufficiency judgment. | Secrets never persist or appear in diagnostics | Config/app composition and each adapter/logging boundary | Owning config/adapter tests plus selected assembled redaction checks | | Default suite remains deterministic, offline, and credential-free | Every package; automation | Stage 12 repository-wide execution and test-policy audit | +Stage 2 test observations for this matrix: + +| Risk | Existing focused protection | Gap or disposition | +| --- | --- | --- | +| Normal lifecycle and payload clearing | Manifest helper tests plus runner/run-stage/extraction-lifecycle tests cover first success, existing-success skip, force, failure, self-skip, repeated self-skip, unsafe and obsolete resume validation, retry, and canonical downstream invalidation. | Strong behavior coverage for successful persistence; no finding. | +| Session/run invocation identity | Per-invocation runner test asserts distinct run IDs, manifest paths, and the latest session `run_id`. | It does not assert refreshed local/spool/remote derived fields or reject loaded identity conflicts; required by `COR-001`. | +| Partial persistence and handled pre-stage errors | Session manifest is injectable and the unsafe-resume test proves old success is preserved. | Run persistence is concrete; no disagreement-boundary tests and no terminal run assertion on resume error; candidate `TST-002`. | +| Interruption and restart | Non-succeeded action logic and retry tests indirectly prove `running` is rerunnable. | No kill/reload normalization, reconciliation, or abandoned-run status test; confirmed `RSK-001`. | +| Same-session concurrency | Artifact store has focused lock tests. | No assembled concurrent runner or release-failure test; Stage 3 owns the mechanism and sufficiency decision. | + +## Confirmed Findings + +### `COR-001`: session identity initialization preserves stale invocation paths and accepts conflicting identity + +- Category: confirmed correctness defect. +- Locations/invariant: `internal/app/runner.go` in `ensureManifestIdentity` and + `syncRunManifestIdentityFromSession`; consumers include prepare's work/spool + resolution, publish's run prefix, and post-publish cleanup. The session and + invocation manifests must describe one internally consistent campaign, + session, and run. +- Evidence: every invocation replaces `Manifest.RunID`, but + `LocalWorkDir`, `LocalSpoolDir`, and `S3RunPrefix` are computed only when + empty. A second invocation therefore records run B while retaining paths and + the remote prefix derived from run A, and copies those stale values into run + B's manifest. The same helper fills an empty campaign but neither rejects nor + reconciles a loaded campaign/session that conflicts with the configured + manifest path. `TestExecuteStagesCreatesRunManifestPerInvocation` proves IDs + and run-manifest paths differ but does not assert the dependent identities. +- Realistic scenario: a forced second prepare writes through run A's work/spool + identity; a forced second publish can target run A's remote prefix while its + manifests claim run B. A misplaced or incorrectly restored manifest can also + make the runner hold session A's lock while stages derive paths from the + manifest's session B identity. +- Impact/likelihood/confidence: high integrity impact; stale derived identity + occurs on every ordinary second invocation after the fields are initialized, + while a conflicting loaded identity is less common; high confidence from the + assignment guards and direct consumers. +- Estimated remediation scope and owner: small-to-medium application/manifest + change. Define whether run-scoped locations belong in the session manifest, + recompute them as one identity unit whenever the run changes, and reject + configured/persisted campaign or session conflicts before stage execution. +- Test changes: extend the existing per-invocation test to load both manifests + and assert every run-derived field against run B; add a loaded-identity + mismatch test that proves no stage or cross-session path is touched. Stages 3 + and 4 should add the path and remote-prefix boundary assertions after their + focused review. +- Dependencies: Stage 3 owns exact path confinement consequences, Stage 4 owns + publish/cleanup impact, Stage 5 owns restored-manifest provenance, and Stage + 6 owns configuration identity validation. They should reference this root + finding rather than create duplicates. + +### `RSK-001`: invocation audit records can remain indefinitely `running` + +- Category: confirmed correctness/operational risk. +- Locations/invariant: `internal/app/runner.go` in `executeStages`, + `internal/manifest/store.go` normalization, and the unused production + `StatusInterrupted` model value. Every completed or handled invocation should + have an intelligible terminal audit outcome, while process interruption must + remain safely resumable. +- Evidence: the initial run manifest is saved with overall status `running`. + A resume-validator error returns directly without marking it failed; terminal + session-save and run-save failures leave the last run state running; and a + process death after either running save has the same effect. Neither load + normalizer converts running records to interrupted, and later invocations + consult only the session manifest and never reconcile older run manifests. + `StatusInterrupted` has no production writer. The existing resume-validation + error test checks preservation of the session success but not the run record. +- Realistic scenario: extraction resume validation encounters an unsafe or + unreadable receipt. The command returns a controlled error, the reusable + session result is correctly preserved, and the run audit file remains + `running` forever. A kill or persistence failure can leave analogous dual- + ledger disagreement. +- Impact/likelihood/confidence: medium operator/audit impact and low risk of + unsafe reuse because non-succeeded session stages rerun; moderate likelihood + over the life of a long-running pipeline; high confidence. +- Estimated remediation scope and owner: medium application/manifest change. + Terminalize handled post-creation errors when persistence is available and + define an explicit startup/status reconciliation policy for abandoned + running records. Preserve the current conservative session-authority rule. +- Test changes: extend the resume-validation error integration test to assert a + terminal failed run; add interruption/restart and injectable session/run-save + 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. + ## Candidate Register -No candidate is confirmed by Stage 1. Later owning stages must inspect the -implementation, focused tests, canonical contract, realistic scenario, and -callers before promoting or rejecting it. +The remaining candidates require inspection by their named owners. Stage 2 +promoted `COR-001` and `RSK-001` into the confirmed register above. ### `ARC-001`: `IODecl` is not a complete or consistently classified stage contract @@ -294,21 +476,8 @@ callers before promoting or rejecting it. pipeline continuation or cleanup semantics. - Confirmation owners: Stage 2 for runner truth table, Stage 4 for publish and cleanup, Stage 8 for ordinary disabled stages. Treat wording and behavior as - unresolved until those flows are traced. - -### `RSK-001`: interruption has a documented state but no mapped production transition - -- Category: correctness/operational risk candidate. -- Evidence: `manifest.StatusInterrupted` is admitted and external docs promise - continuation of interrupted sessions, but graph-augmented code search found - the constant only in its declaration and an artifact rejection test. No - production transition to it was found during mapping. -- Realistic risk: a killed process may leave session/run records as `running`, - producing confusing status or dual-ledger interpretation even though the - planner reruns all non-succeeded stages. -- Confirmation owner: Stage 2 must trace load normalization, process failure - boundaries, status reporting, and next-invocation behavior before deciding - whether this is a defect, compatibility state, or unused model value. + unresolved until those flows are traced. Stage 2 confirmed that the two + outcomes are deliberate and internally distinguishable at runner level. ### `TST-001`: full race baseline fails in the concurrent transcribe test @@ -323,6 +492,62 @@ callers before promoting or rejecting it. 12 should classify suite impact and the smallest durable fix. Do not change the fake during this investigative stage. +### `TST-002`: runner tests cannot exercise invocation-manifest save failures + +- Category: test-suite sufficiency candidate. +- Evidence: `Env.ManifestStore` injects only session `Create`, `Load`, and + `Save`; `executeStages` constructs a concrete `manifest.LocalStore` for run + creation and every `SaveRun`. Focused tests cover normal and stage-failure + transitions but no save disagreement row. The existing resume-validation + error test also omits the surviving run status. +- Realistic risk: future ordering or error-path changes can advertise a handled + invocation as running, lose the audit half of a terminal transition, or + weaken conservative retry behavior without an assembled test failing. +- Confirmation owner: Stage 12 should decide the smallest persistence seam and + representative boundary cases; avoid exhaustive choreography tests for every + mechanically identical save call. + +### `DUP-001`: session save duplicates the shared atomic JSON writer + +- Category: duplicated mechanism candidate. +- Evidence: `LocalStore.Save` contains its own temp/create/write/sync/close/ + context-check/rename sequence, while `SaveRun` delegates the same mechanism + to `writeJSONAtomically`. Error prefixes differ, but the durability mechanism + is otherwise repeated. +- Realistic risk: a future durability, cleanup, permission, or platform fix may + reach only one manifest type, creating different guarantees for the two + ledgers. +- Confirmation owners: Stage 3 should compare filesystem guarantees and Stage + 11 should decide whether sharing the helper preserves useful error context. + +### `SIM-001`: runner terminalization and persistence ordering lack a narrow owner + +- Category: simplification candidate. +- Evidence: `executeStages` is 274 lines with cyclomatic complexity 54 and + cognitive complexity 96. Much of the length is justified visible state- + machine ordering, but session-first terminal save, run-first running save, + result mapping, identity sync, and compound error handling are repeated + inline. The resume-validation return bypasses run terminalization. +- Realistic risk: adding another pre-stage or terminal error path can repeat the + abandoned-run behavior in `RSK-001`; extracting too broadly could instead + hide the critical order. +- Confirmation owner: Stage 11 should consider a small terminal failure helper + or typed transition operation only after Stages 3-10 settle ordering needs. + +### `COM-001`: dual-ledger save order and partial-failure policy lack rationale + +- Category: comment/clarity candidate. +- Evidence: the runner visibly saves run-before-session for `running` and + session-before-run for terminal outcomes, but no local comment explains which + file is authoritative, why the order differs, or how partial state is meant + to be interpreted. +- Realistic risk: a maintainer may make the calls symmetrical or reorder them, + inadvertently allowing a stage to execute without a durable running session + transition or preferring an audit record over resume authority. +- Confirmation owner: Stage 11, after any `SIM-001` restructuring. Prefer a + named operation that makes the invariant obvious; add a concise rationale + only where code structure cannot. + ## Candidate Classification Log | Candidate signal | Classification | Reason | @@ -330,13 +555,19 @@ callers before promoting or rejecting it. | Graph rollups `stage -> app`, `adapters -> app`, `config -> app` | rejected as a production reversal at Stage 1 | `go list` production imports contain no lower-level import of `internal/app`; graph connections include tests and ambiguous package grouping. Reopen only with a concrete production edge. | | Similar wrapper/manifest/adapter functions | deferred metric signals, not findings | Similarity alone does not establish duplicated policy; owning behavior stages must first establish contracts. | | Coverage percentages | deferred diagnostic signals, not findings | Stage 12 must reason from risk and test ownership, not a numeric target. | +| 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 | assigned to Stage 3 | The application discards the error, but whether close/unlink failure preserves an OS lock or merely a harmless lock file depends on the local-store implementation and platform semantics. | +| Two durable meanings of “skip” | runner portion confirmed, stage-contract conclusion deferred | Run action/status distinguishes idempotent skip from executed self-skip. Stages 4 and 8 must determine whether successful disabled outcomes and their wording are intentional. | ## Unresolved Questions And Follow-Up -- Does loading a persisted `running` stage or run normalize it to - `interrupted`, or is `interrupted` only a compatibility value? -- What exact session/run disagreement states are possible when either save - fails at each transition boundary? +- Should startup/status reconcile abandoned `running` invocation records, mark + them interrupted, or retain them as an explicitly accepted audit limitation? +- Do Stage 3 path mechanics and Stage 4 publish/cleanup behavior confirm every + concrete consequence of the stale run-derived fields in `COR-001`? +- Can any `ReleaseSessionLock` error retain exclusion or make a subsequent + acquisition unsafe, and how should the runner surface such a deferred error? - Are disabled render/analyze/publish outcomes intentionally successful so pipeline continuation and optional outputs work, and do all operator views describe that distinction accurately? @@ -383,3 +614,29 @@ No accepted risks or final audit conclusions are recorded yet. registry, internal overview, CLI, and operations contract. - Follow-up: all unresolved behavior has a named owner in Stages 2-12; every area and invariant has an implementation owner and intended test owner. + +### Stage 2 + +- Contracts and code reviewed: planner and full/single-stage entry points; + `executeStages`, run-control and identity helpers; session/run manifest + models, creation, loading, validation, normalization, atomic persistence, and + every transition method; runner lock lifetime; focused internal manifest + documentation and Stage 1 matrices. +- Graph/source evidence: call traces into full and selected execution; all + identity-field consumers; manifest transition/save callers; status and + `last_error` usages; runner complexity and atomic-save similarity; complete + focused test-function inventory. +- Validation: `go test -count=1 ./internal/app ./internal/manifest` passed + (`internal/app` 0.708 s, `internal/manifest` 0.010 s; 1.60 s command wall + time). `go test -race -count=1 ./internal/app ./internal/manifest` passed + (`internal/app` 45.842 s, `internal/manifest` 1.026 s). +- Conclusions: every lifecycle cell and dual-save boundary is recorded above; + scenarios 1 and 2 are resolved at runner level; lock acquisition/lifetime is + resolved and release mechanics assigned to Stage 3. Confirmed `COR-001` and + `RSK-001`; added `DUP-001`, `SIM-001`, `COM-001`, and `TST-002` for named + later owners. +- Explicit no-finding conclusions: canonical invalidation works at both first + and last stage and is independent of selected-plan width; stale transitions + intentionally retain diagnosis data while running/failure/skip clear it; + session authority makes all enumerated disagreement states retry or reuse + conservatively; the runner's two skip forms are durably distinguishable.