diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index e82c3de3b..e2139fbe8 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -2,8 +2,22 @@ ## Status -Accepted (2026-07-10). Migration steps 1-6 and 8 have shipped; step 7 (benchmark extension) remains -deferred. See [Migration progress](#migration-progress) for the per-step landing record. +Accepted (2026-07-10); partially implemented (last updated 2026-07-13). See [Migration progress](#migration-progress) for the per-step landing record. + +**Implemented and merged to `main`:** + +- Decisions 1-5 and their migration steps 1-7 — resolution disclosure (#1193), structured divergence + transport (#1197), `.ad` target annotations and target-binding verification (#1196, #1209), `replay + --from`/`--plan-digest` resume and `--update` retirement (#1211), and the selector-miss → + `REPLAY_DIVERGENCE` repair-loop fix (#1223). +- Decision 6, the base agent-supervised re-record repair — `replay --save-script` arming, the + post-watermark healed slice, `repairHint`, and the writer's bare-`@ref` fail-loud guard (#1228). + +**Accepted but NOT yet implemented** (this amendment; tracked by #1235 — repair-transaction lifecycle): +the R7 repair-transaction keep-alive and its distinct `resume.repairSessionHeld` signal, the ARMED → +COMPLETE → COMMITTED commit state machine, terminal-source-`close` skipping, the `REPAIR_SESSION_EXPIRED` +tombstone, and race-safe atomic publication. Until #1235 lands, `--save-script` on a diverging replay +does not carry the lifetime, abort-on-incomplete, or atomic-commit guarantees R7 specifies. ## Context (historical baseline) @@ -455,7 +469,8 @@ executable plan and must be in range. It is never a YAML line number, fractional repeat iteration label. Static includes, platform conditions, and fixed-count repeats expand before indexing, so repeated source lines are distinguished by their plan index. -Every divergence includes `resume: { allowed, from, reason?, planDigest }`. `planDigest` is SHA-256 over +Every divergence includes `resume: { allowed, from, reason?, planDigest, repairSessionHeld? }`. +`planDigest` is SHA-256 over the canonical fully expanded plan, including each action's command, normalized inputs, control shape, platform-conditioned expansion, and source provenance. Concretely "normalized inputs" bind each action's positionals/flags, its execution-affecting `runtime` hints, and its `target-v1` identity annotation @@ -471,6 +486,18 @@ so it can change the digest. A resume requires both `--from N` and retarget ordinal N. `allowed: false` explains why no resume is safe; its digest is still diagnostic, not an authorization to bypass preflight. +`allowed` conveys **plan-resumability only** — whether ordinal `from` can be safely resumed against a +matching plan — and is emitted for EVERY divergence; it says nothing about whether a session is still +alive to resume against. Session lifetime is a **distinct** signal: `repairSessionHeld: true` is set on +any divergence whose session carries an **active repair transaction** — one opened by a `--save-script` +replay and **persisted on the session** until commit/abort (decision 6, R7). It therefore keys off the +persisted transaction state, not the current request's flags, so a `--from` continuation that does NOT +repeat `--save-script` still reports `repairSessionHeld: true`. Only that flag promises the diverging +daemon/session is being kept live and addressable for the in-flight repair. Decision 6's R7 keys its +lifetime guarantee off `repairSessionHeld`, never off `allowed` — a plain (non-armed) replay may report +`allowed: true` for plan-resumability yet tear its session down normally, so a consumer must not read +`allowed: true` as "the session is still there." + Resume does not reconstruct execution state. For `N > 1`, preflight must reject with `INVALID_ARGS` when any skipped action can produce `outputEnv` values, or when the skipped range or resume target is inside runtime control flow (conditional, retry, or dynamic repeat). The only variables available after a resume @@ -530,8 +557,23 @@ Implementation is not accepted on benchmark evidence alone. Required automated c `target-v1` evidence when recording is armed from step 1 (R1); a `--from`-continuation test proving the already-recorded prefix is never duplicated by a second full replay on the same session (R2); a boundary-watermark test proving a reused session's pre-invocation actions are excluded from the healed - script (R6); and a writer fail-loud test proving a bare `@ref` that cannot materialize to a selector - errors with a non-zero exit rather than being silently dropped (R4). + script (R6); a writer fail-loud test proving a bare `@ref` that cannot materialize to a selector + errors with a non-zero exit rather than being silently dropped (R4); a repair-transaction lifetime test + proving a divergence with `resume.repairSessionHeld: true` keeps the session live and addressable for a + following `--from` while a non-armed divergence with only `resume.allowed: true` does not, that + continuation resumes off the session's **persisted** transaction with a `--from` that carries no + `--save-script`, that a reaped INCOMPLETE transaction surfaces `REPAIR_SESSION_EXPIRED` from a tombstone + keyed by session key (not `SESSION_NOT_FOUND`) and that a fresh `replay --save-script` on that key clears + it, and that a keep-alive-incapable implementation fails fast before step 1 (R7); a commit-state-machine + test proving **any teardown** (explicit `close`, idle-reap, and shutdown) commits atomically at + `COMPLETE`, aborts and publishes nothing (no prefix) before `COMPLETE` — with an idle-reap/shutdown of a + `COMPLETE` transaction committing (not tombstoning) — and is idempotent with no re-publish once + `COMMITTED`; a terminal-close test proving an armed replay reaching the terminal source `close` + **skips** it without deleting the session (C4); an atomic-publication test proving the temp file is + created in the target's own directory and the no-clobber is race-safe (create-exclusive/rename-if-absent); + and a no-clobber test proving an existing *complete* (sentinel-marked) healed artifact is protected + while an incomplete/partial one is + overwritable. Extend the settle benchmark (`~/.agent-device-bench/rnnav-matrix.py` pattern, external harness) with a replay arm only after these contracts pass: measure clean replay and one induced divergence repaired @@ -589,7 +631,7 @@ rather than routing blind. The two sub-flows use different recording discipline: steps, and must not pollute the healed script — then `replay --from N --plan-digest ` re-runs the *unchanged* step N, which now matches. -**Required protocol rules (normative).** These six rules are the difference between "the mechanism +**Required protocol rules (normative).** These seven rules are the difference between "the mechanism works" and "healed scripts are always valid": - **R1 — recording is armed from the first replay, not on divergence.** `replay .ad @@ -654,6 +696,42 @@ works" and "healed scripts are always valid": replay's own execution path — so a reused session's earlier, unrelated actions never leak into the healed script. This makes the healed output independent of prior session history without requiring a fresh session for the repair itself. +- **R7 — the repair transaction spans the whole live session; the session stays addressable until the + transaction ends (commit or abort).** `--save-script` opens a multi-invocation repair **transaction** + whose state **persists on the session** — it is not re-derived from each request's flags. A repair-armed + replay that returns `REPLAY_DIVERGENCE` with **`resume.repairSessionHeld: true`** (decision 4 — the + distinct session-lifetime signal, NOT `resume.allowed`, which conveys plan-resumability only and is + emitted for every divergence) **MUST NOT** tear down the owning daemon or session. **Continuation is by + persisted transaction state, not the per-request flag:** `replay --from --plan-digest ` resumes + the repair on that same armed session **without repeating `--save-script`** — the opener is the only + place that flag appears — and the implementation MUST key keep-alive and continuation off the session's + persisted transaction, never off whether the current request carried `--save-script`. The boundary + watermark (R6) and the accumulating `session.actions` slice only mean anything if that one session + survives across invocations. This strengthens R2: same-session continuation is not merely preferred, it + is a lifetime guarantee. + + **Commit is at teardown, gated on `COMPLETE`.** Any teardown of the session — an explicit `close`, an + idle-reap, or a daemon shutdown — **commits** the healed `.ad` atomically **iff** the transaction is + `COMPLETE`, and otherwise **aborts** with no publish (never a prefix), per the commit state machine + below. An idle-reap or shutdown of an INCOMPLETE transaction additionally leaves a **tombstone** (R7's + ownership guarantee, under "Repair-session tombstone" below), so a later command on that session key + surfaces a specific `REPAIR_SESSION_EXPIRED` recovery error with re-run guidance, never a bare + `SESSION_NOT_FOUND`; an explicit `close` of an incomplete transaction simply discards. A + **persistent-daemon precondition is explicitly rejected**: if some implementation constraint would + prevent keep-alive, the armed replay must **fail-fast before step 1** with actionable guidance, never + proceed and then fail a later `--from` with `SESSION_NOT_FOUND`. + +**Terminal lifecycle steps during a repair-armed `--from` resume.** Verification is scoped, per decision +3, to *annotated resolved targets* — so a non-target step (a source `close`, or any step carrying no +`target-v1` `targetEvidence`) is already exempt from target-binding divergence: it may still surface an +`action-failure` if its dispatch genuinely fails, but never a `selector-miss`/`identity-mismatch`/ +`identity-unverifiable` merely for lacking evidence. This is a clarification of decision 3's existing +scope, not a change to it. The **terminal source `close`** is defined precisely as *the last action of +the source plan being a `close`*. During a repair-armed replay or resume that terminal source `close` is +**SKIPPED — not dispatched**: dispatching it deletes the session mid-transaction (the very session R7 +keeps alive), so it must never run under an armed repair. Reaching it instead marks the transaction +`COMPLETE` (see commit state machine); the next teardown of the session — normally the agent's own +`close`/`close --save-script` — then commits (a teardown at `COMPLETE` always commits; R7). **Acceptance test (mandatory).** A healed sibling `.ad` produced by the repair loop must replay end-to-end in a **fresh session** with every selector step annotated and no bare `@ref`. @@ -674,14 +752,65 @@ by construction. The healed `.ad` is written only when the repair ends (below) a **and** records the repair-run boundary watermark `session.actions.length` (R6). Absent this flag, replay behaves exactly as today: no recording, no heal. The heal is opt-in, preserving decision 1's "no silent rewrite." -- The healed `.ad` is written when the agent ends the repair, reusing `close --save-script ` - (already serializes `session.actions` via `SessionScriptWriter.write`, - `src/daemon/session-script-writer.ts:30-52`) but over the post-watermark slice only (R6). When no +- The healed `.ad` is published when a teardown commits the transaction at state `COMPLETE` — normally the + agent's own `close`/`close --save-script `, but any teardown of a completed transaction commits + (see the commit state machine below) — reusing the existing `session.actions` serializer + (`SessionScriptWriter.write`, `src/daemon/session-script-writer.ts:30-52`) over the post-watermark slice + only (R6). When no `` path is given, the default is the **`.healed.ad` sibling**: the original script's path with its `.ad` extension replaced by `.healed.ad` (e.g. `flows/login.ad` → `flows/login.healed.ad`), written beside the original. The original is never overwritten in place without an explicit `` path, so a human reviews the diff and promotes it. +**Commit state machine — `ARMED` → `COMPLETE` → `COMMITTED`.** R6 defines the *slice*; this defines *when +the slice may be published*, as a machine-enforceable state, not a heuristic: + +- **`ARMED`** — set the instant `replay --save-script` arms the transaction (before step 1). The healed + slice is accumulating but MUST NOT be published. +- **`COMPLETE`** — reached when the source plan has executed/healed through its **last executable step** + with **no outstanding divergence** (the terminal source `close` is excluded from "executable" here — + see the terminal-close contract above; reaching it is what flips `ARMED` → `COMPLETE`). Only in this + state is a publish permitted. +- **`COMMITTED`** — the healed `.ad` has been atomically published (below). Terminal. + +**Teardown is the commit trigger, gated on `COMPLETE`.** Any teardown of the repair-armed session — an +explicit `close`/`close --save-script`, an idle-reap, or a daemon shutdown — commits the healed `.ad` +**iff** the state is `COMPLETE`. A teardown while **not** `COMPLETE` is an **ABORT**: discard the slice, +publish **nothing** — never a prefix. Reaching `COMPLETE` does not itself write a file; it only makes the +*next* teardown a commit rather than an abort, so the artifact is published exactly once — when the +transaction has both completed and ended. Commit is thus gated on `COMPLETE` but happens at *any* teardown +(safer for the artifact than requiring one specific finalize command, and the model #1235 implements). +Teardown after `COMMITTED` is **teardown-only, idempotent — no re-publish** (so it can neither +double-commit nor trip the no-clobber guard). + +**Atomic, race-safe publication.** The commit serializes the healed slice (ending with a serialized +`close` line so the artifact is self-contained) to a temp file created **in the same directory as the +final target** — atomic `rename` requires the same filesystem, and an explicit `--save-script=` +target may live on a different mount than any process-wide temp dir — then atomically renames it into +place. A reader therefore never observes a half-written healed script, and an aborted repair leaves no +partial behind. The no-clobber guard is enforced **race-safely** by the atomic primitive itself +(create-exclusive / rename-if-absent), not by a check-then-write. + +**No-clobber applies only to a COMPLETE healed artifact.** The guard protects a *complete, review-worthy* +artifact only. An incomplete or partial file is always overwritable by a fresh repair that reaches +`COMMITTED`. Completeness is established by a sentinel written **only on commit** — a trailing +`# agent-device:heal-complete` marker — or an equivalent structural check; the atomic temp→publish flow is +what makes that sentinel trustworthy (a partial never reaches the published path carrying the sentinel). +Auto-versioned output names (e.g. `.healed.2.ad`) are explicitly **out of scope** here — a separate +naming change, not part of this decision. + +**Repair-session tombstone (R7 ownership).** When a bounded-expiry escape hatch idle-reaps (or a daemon +shutdown tears down) a repair-armed transaction that has **not** reached `COMPLETE`, the teardown aborts +(no publish, per the state machine) and must leave a **tombstone** rather than deleting the session record +outright. (A teardown of a `COMPLETE` transaction instead *commits* and needs no tombstone — the +transaction ended successfully.) The tombstone is keyed by the **session key** (the same +owner-scoped session identifier a `--from` continuation addresses), records the **owner** and an +**expiry**, and lives for a bounded window after reap. While it exists, any command targeting that key — +a `--from` resume, an interactive heal step, or a `close` — resolves to `REPAIR_SESSION_EXPIRED` (with +re-run guidance to restart the repair from the original script), never a bare `SESSION_NOT_FOUND`. A new +`replay --save-script` on the same key **clears the tombstone and starts a fresh transaction** (`ARMED`). +The tombstone itself expires after its bounded window, after which the key is fully free. + ## Consequences - `--from` makes app state the caller's responsibility, and only accepts a resume when the planner can @@ -770,16 +899,27 @@ each states its dependencies explicitly. report before the write path is removed), with a no-write regression test. 7. **Benchmark extension** (decision 5) — follows the mandatory contracts; measures the economic claim (clean replay plus one induced divergence repaired through the allowed `--from` loop). -8. **Agent-supervised re-record repair** (decision 6) — depends on 2 and 5 (the repair loop is built - entirely on the existing divergence report and `--from`/`--plan-digest` resume machinery) and on 3/4 - (corrective actions record fresh `target-v1` evidence, so the healed script is self-consistent). - Prerequisite: the selector-miss → `REPLAY_DIVERGENCE` defensive fix (PR #1223) — a thrown per-action - selector-miss must route through the same divergence-wrapping path as a returned failure, or the - repair loop never sees a divergence report to act on. Net-new implementation is narrow: the daemon-side - `repairHint` computation over all four `kind`s (R3), `--save-script` arming plus the repair-run - boundary watermark on `replay` (R1/R6), the writer's post-watermark slice and bare-`@ref` fail-loud - guard (R4/R6) — the healed-script emission otherwise reuses `close --save-script`'s existing - `session.actions` serializer. +8. **Agent-supervised re-record repair, base** (decision 6, R1-R6) — **MERGED (#1228)**; depends on 2 + and 5 (the repair loop is built entirely on the existing divergence report and `--from`/`--plan-digest` + resume machinery) and on 3/4 (corrective actions record fresh `target-v1` evidence, so the healed + script is self-consistent). Prerequisite: the selector-miss → `REPLAY_DIVERGENCE` defensive fix + (PR #1223) — a thrown per-action selector-miss must route through the same divergence-wrapping path as + a returned failure, or the repair loop never sees a divergence report to act on. Delivered: the + daemon-side `repairHint` computation over all four `kind`s (R3), `--save-script` arming plus the + repair-run boundary watermark on `replay` (R1/R6), the writer's post-watermark slice and bare-`@ref` + fail-loud guard (R4/R6) — reusing `close --save-script`'s existing `session.actions` serializer. +9. **Repair-transaction lifecycle** (decision 6, R7 + commit state machine) — **NOT YET IMPLEMENTED, + tracked by #1235**; depends on 8. Adds: the distinct `resume.repairSessionHeld` divergence signal and + R7 keep-alive keyed off the session's **persisted** repair-transaction state (so `--from` continuations + need no repeat of `--save-script`), with `fail-fast-before-step-1` when keep-alive is impossible; the + `ARMED → COMPLETE → COMMITTED` commit state machine where **any teardown** (explicit `close`, + idle-reap, or daemon shutdown) commits atomically when `COMPLETE`, aborts (no prefix) when not, and is + idempotent once `COMMITTED`; terminal-source-`close` **skipping** during an armed replay/resume, with a + regression proving the session is NOT deleted when the terminal `close` is reached; the + `REPAIR_SESSION_EXPIRED` tombstone for an incomplete-transaction reap/shutdown (keyed by session key, + owner + bounded expiry, cleared by a fresh `replay --save-script`); and race-safe atomic publication + (temp file in the target's own directory, create-exclusive/rename-if-absent no-clobber against the + `# agent-device:heal-complete` sentinel). ## Migration progress @@ -795,10 +935,12 @@ not restate or amend the decisions. | 5. `replay --from` + `--plan-digest` resume | 4 (resume) | Shipped | #1211 | | 6. `--update` retirement | 1 | Shipped | #1211 | | 7. Benchmark extension | 5 | Deferred | — | -| 8. Agent-supervised re-record repair | 6 | Shipped | #1228 | +| 8. Agent-supervised re-record repair, base | 6 (R1-R6) | Shipped | #1228 | +| 9. Repair-transaction lifecycle | 6 (R7) | Not yet | #1235 | Step 4 (#1209) added the `selector-miss`/`identity-mismatch`/`identity-unverifiable` divergence kinds and a post-resolution target guard that cross-checks the dispatched winner against the verified member. Issue #1221 ("Complete ADR 0012 replay target-binding verification") was closed as already implemented: its verification scope is covered by step 4. Step 7 remains deferred. Step 8 implements the repair -design accepted in #1226 and shipped in #1228. +design accepted in #1226 and shipped in #1228; step 9 (R7 + commit machine) remains unimplemented, +tracked by #1235.