Audit runner and manifest lifecycle behavior
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user