From 6959fe2fb7bc02a56db21528971a740769bcc10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 13 Jul 2026 10:24:43 +0200 Subject: [PATCH 1/3] =?UTF-8?q?docs:=20ADR-0012=20Decision=206=20=E2=80=94?= =?UTF-8?q?=20repair-transaction=20lifecycle=20(R7=20+=20commit=20semantic?= =?UTF-8?q?s)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frames --save-script as a multi-invocation repair TRANSACTION committed only on completion: - R7 (new normative rule): a repair-armed replay returning resume.allowed:true must keep its daemon/session live until close; heal/--from target that same session (strengthens R2). Plain close/teardown/idle-reap while armed = abort/ discard. Bounded-expiry must surface REPAIR_SESSION_EXPIRED, not bare SESSION_NOT_FOUND. Persistent-daemon precondition rejected: fail-fast before step 1, never a later SESSION_NOT_FOUND. - Decision 4 resume: one sentence noting the session is kept addressable so resume.allowed:true is not misleading. - Commit semantics: healed .ad committed only on full-plan completion or explicit close --save-script; never on divergence-only exit, teardown, or idle-reap; atomic temp->publish. R6 defines the slice, this defines when it is complete. - Terminal lifecycle steps: non-target steps (incl. source close, unannotated steps) are already exempt from target-binding divergence per decision 3 (clarification, not a change); prefer SKIPPING the source terminal close while armed and finalize via close --save-script. - Clobber P2: no-clobber guards a COMPLETE (heal-complete sentinel) artifact only; partials are overwritable; auto-versioned names out of scope. Validation + migration step 8 extended accordingly. --- docs/adr/0012-interactive-replay.md | 66 ++++++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index e82c3de3b..8666964d5 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -469,7 +469,10 @@ so it can change the digest. A resume requires both `--from N` and `--plan-digest ` from the report. The daemon rebuilds the current plan and rejects `INVALID_ARGS` before any action when its digest differs, so edits or parse-time expansion cannot silently retarget ordinal N. `allowed: false` explains why no resume is safe; its digest -is still diagnostic, not an authorization to bypass preflight. +is still diagnostic, not an authorization to bypass preflight. When `allowed: true`, the diverging +daemon/session is kept live and addressable for the in-flight repair so the reported `from`/`planDigest` +resume actually has a session to target — decision 6's R7 makes this a normative repair-transaction +guarantee, so `resume.allowed: true` is never a promise the session has already been torn down under. 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 @@ -530,8 +533,14 @@ 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.allowed: true` keeps the session live and addressable for a following + `--from`, that a reaped repair session surfaces `REPAIR_SESSION_EXPIRED` (not `SESSION_NOT_FOUND`), and + that a keep-alive-incapable implementation fails fast before step 1 (R7); a commit-semantics test + proving the healed `.ad` is published only on completion/explicit finalize, atomically, and is absent + after a divergence-only exit or abort `close`; 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 +598,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 +663,32 @@ 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 is kept alive until `close`.** + `--save-script` opens a multi-invocation repair **transaction**: a repair-armed replay that returns + `REPLAY_DIVERGENCE` with `resume.allowed: true` **MUST NOT** tear down the owning daemon or session. + The subsequent heal actions and `replay --from` continuations (R2) target that **same live session** + until the agent finishes with `close`; 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. A plain + `close` (without `--save-script`), a daemon teardown, or an idle-reap while the repair is still armed is + an **abort/discard** — no healed `.ad` is emitted; committing is the agent's explicit act + (`close --save-script`, or the full plan completing cleanly), per the commit semantics below. An + implementation MAY offer a bounded-expiry escape hatch, but if the repair + session is reaped it must surface a specific `REPAIR_SESSION_EXPIRED` recovery error (with re-run + guidance), never a bare `SESSION_NOT_FOUND`. 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. Because the repair transaction is finalized by the agent's own +`close --save-script` (R7 + commit semantics), the **source plan's terminal `close` should be SKIPPED +while the repair is armed** — running it would abort the transaction; the agent finalizes explicitly +instead. **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`. @@ -682,6 +717,23 @@ by construction. The healed `.ad` is written only when the repair ends (below) a 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 semantics — the healed `.ad` is committed only on successful repair completion.** R6 defines the +*slice*; this defines *when the slice is complete and durable*. The healed artifact is committed only when +the full plan finishes cleanly under the resume loop **or** the agent explicitly finalizes with +`close --save-script`. It is **never** emitted on a divergence-only exit, on daemon teardown, or on an +idle-reap — a prefix-only partial is not "the healed script IS `session.actions`", it is an incomplete +transaction. The write is **atomic**: serialize to a transaction-scoped temp path, then atomically publish +to the target path, so a reader never observes a half-written healed script and an aborted repair leaves +no partial artifact behind. + +**No-clobber applies only to a COMPLETE healed artifact.** The guard that refuses to overwrite an existing +healed `.ad` protects a *complete, review-worthy* artifact only. An incomplete or partial file is always +overwritable by a fresh repair that completes. Completeness is established by a sentinel written **only on +successful commit** — e.g. 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 with the sentinel). Auto-versioned output names (e.g. `.healed.2.ad`) are explicitly **out +of scope** here — that is a separate naming change, not part of this decision. + ## Consequences - `--from` makes app state the caller's responsibility, and only accepts a resume when the planner can @@ -777,8 +829,10 @@ each states its dependencies explicitly. 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 + boundary watermark on `replay` (R1/R6), the repair-transaction keep-alive with fail-fast/`REPAIR_SESSION_EXPIRED` + handling (R7), the writer's post-watermark slice, bare-`@ref` fail-loud guard, and + commit-on-completion atomic temp→publish with the completeness sentinel and no-clobber guard (R4/R6 + + commit semantics) — the healed-script emission otherwise reuses `close --save-script`'s existing `session.actions` serializer. ## Migration progress From 13a501bef0eaa9d71e40844ce47d1add6b102049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 13 Jul 2026 11:12:33 +0200 Subject: [PATCH 2/3] =?UTF-8?q?docs:=20ADR-0012=20R7=20=E2=80=94=20fix=206?= =?UTF-8?q?=20architecture=20blockers=20(transaction=20contract)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - C1: R7 keep-alive keys off a DISTINCT resume.repairSessionHeld signal, not resume.allowed (which means plan-resumability and fires for every divergence). - C2: define the ARMED -> COMPLETE -> COMMITTED commit state machine. close before COMPLETE = abort (publish nothing); close at COMPLETE = atomic commit; close after COMMITTED = idempotent teardown, no re-publish. No auto-commit. - C4: precise terminal-source-close contract (last source action == close) — SKIPPED (not dispatched) under armed repair so the session is not deleted; regression required (added to migration step 9). - C5a: REPAIR_SESSION_EXPIRED backed by a bounded tombstone keyed by session key (owner + expiry), cleared by a fresh replay --save-script. - C5b: atomic publication temp file in the target's own directory; race-safe no-clobber via create-exclusive/rename-if-absent. - C6: Status block corrected — Decisions 1-6 base MERGED (#1228 et al.); R7 + commit machine UNIMPLEMENTED, tracked by #1235. Migration split into step 8 (merged) and step 9 (#1235). Validation extended for all of the above. --- docs/adr/0012-interactive-replay.md | 198 +++++++++++++++++++--------- 1 file changed, 133 insertions(+), 65 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 8666964d5..89ef42b58 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 @@ -469,10 +484,16 @@ so it can change the digest. A resume requires both `--from N` and `--plan-digest ` from the report. The daemon rebuilds the current plan and rejects `INVALID_ARGS` before any action when its digest differs, so edits or parse-time expansion cannot silently retarget ordinal N. `allowed: false` explains why no resume is safe; its digest -is still diagnostic, not an authorization to bypass preflight. When `allowed: true`, the diverging -daemon/session is kept live and addressable for the in-flight repair so the reported `from`/`planDigest` -resume actually has a session to target — decision 6's R7 makes this a normative repair-transaction -guarantee, so `resume.allowed: true` is never a promise the session has already been torn down under. +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 +**only** on a divergence from a repair-armed replay (`--save-script` was in effect), and 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 @@ -535,12 +556,18 @@ Implementation is not accepted on benchmark evidence alone. Required automated c boundary-watermark test proving a reused session's pre-invocation actions are excluded from the healed 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.allowed: true` keeps the session live and addressable for a following - `--from`, that a reaped repair session surfaces `REPAIR_SESSION_EXPIRED` (not `SESSION_NOT_FOUND`), and - that a keep-alive-incapable implementation fails fast before step 1 (R7); a commit-semantics test - proving the healed `.ad` is published only on completion/explicit finalize, atomically, and is absent - after a divergence-only exit or abort `close`; and a no-clobber test proving an existing *complete* - (sentinel-marked) healed artifact is protected while an incomplete/partial one is overwritable. + 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 a reaped + repair session 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 a + `close` before `COMPLETE` aborts and publishes nothing, a `close` at `COMPLETE` commits atomically, and + a second `close` after `COMMITTED` is idempotent teardown with no re-publish; 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 @@ -664,31 +691,34 @@ works" and "healed scripts are always valid": 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 is kept alive until `close`.** - `--save-script` opens a multi-invocation repair **transaction**: a repair-armed replay that returns - `REPLAY_DIVERGENCE` with `resume.allowed: true` **MUST NOT** tear down the owning daemon or session. - The subsequent heal actions and `replay --from` continuations (R2) target that **same live session** - until the agent finishes with `close`; 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. A plain - `close` (without `--save-script`), a daemon teardown, or an idle-reap while the repair is still armed is - an **abort/discard** — no healed `.ad` is emitted; committing is the agent's explicit act - (`close --save-script`, or the full plan completing cleanly), per the commit semantics below. An - implementation MAY offer a bounded-expiry escape hatch, but if the repair - session is reaped it must surface a specific `REPAIR_SESSION_EXPIRED` recovery error (with re-run - guidance), never a bare `SESSION_NOT_FOUND`. 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`. + `--save-script` opens a multi-invocation repair **transaction**. 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. The subsequent heal actions and + `replay --from` continuations (R2) target that **same live session** until the agent finalizes with + `close`; 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. Whether a `close` commits or aborts is governed by the + commit state machine below — a plain `close` before the transaction reaches `COMPLETE`, a daemon + teardown, or an idle-reap while armed emits **nothing**. An implementation MAY offer a bounded-expiry + escape hatch, but idle-reaping a repair-armed, un-finalized session must leave a **tombstone** (R7's + ownership guarantee, detailed 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`. 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. Because the repair transaction is finalized by the agent's own -`close --save-script` (R7 + commit semantics), the **source plan's terminal `close` should be SKIPPED -while the repair is armed** — running it would abort the transaction; the agent finalizes explicitly -instead. +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 agent then finalizes the transaction with its own explicit +`close`/`close --save-script`, which commits. **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`. @@ -709,30 +739,59 @@ 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 the agent's `close`/`close --save-script ` commits the + transaction (state `COMPLETE` — 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 semantics — the healed `.ad` is committed only on successful repair completion.** R6 defines the -*slice*; this defines *when the slice is complete and durable*. The healed artifact is committed only when -the full plan finishes cleanly under the resume loop **or** the agent explicitly finalizes with -`close --save-script`. It is **never** emitted on a divergence-only exit, on daemon teardown, or on an -idle-reap — a prefix-only partial is not "the healed script IS `session.actions`", it is an incomplete -transaction. The write is **atomic**: serialize to a transaction-scoped temp path, then atomically publish -to the target path, so a reader never observes a half-written healed script and an aborted repair leaves -no partial artifact behind. - -**No-clobber applies only to a COMPLETE healed artifact.** The guard that refuses to overwrite an existing -healed `.ad` protects a *complete, review-worthy* artifact only. An incomplete or partial file is always -overwritable by a fresh repair that completes. Completeness is established by a sentinel written **only on -successful commit** — e.g. 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 with the sentinel). Auto-versioned output names (e.g. `.healed.2.ad`) are explicitly **out -of scope** here — that is a separate naming change, not part of this decision. +**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. + +A `close` (or `close --save-script`) on the repair-armed session is the **commit trigger**, and commits +**only when the state is `COMPLETE`**. `close` while **not** `COMPLETE` is an **ABORT**: discard the +slice, publish **nothing** — never a prefix, never on teardown. There is **no separate auto-commit +trigger**: reaching `COMPLETE` does not itself write a file; it only makes a *subsequent* `close` a commit +rather than an abort. This "write only on intentional completion" gate (grounded in the second design +review) is the rule, not a preference. `close` when already `COMMITTED` is **teardown-only, idempotent — +no re-publish** (so a second finalize 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 a repair-armed +session that is still `ARMED`/`COMPLETE` (not `COMMITTED`), it must leave a **tombstone** rather than +deleting the session record outright. 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 @@ -822,18 +881,25 @@ 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 repair-transaction keep-alive with fail-fast/`REPAIR_SESSION_EXPIRED` - handling (R7), the writer's post-watermark slice, bare-`@ref` fail-loud guard, and - commit-on-completion atomic temp→publish with the completeness sentinel and no-clobber guard (R4/R6 + - commit semantics) — 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 it (with `fail-fast-before-step-1` when keep-alive is impossible); the + `ARMED → COMPLETE → COMMITTED` commit state machine (abort-when-not-`COMPLETE`, commit on any teardown + at `COMPLETE`, idempotent teardown-when-`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 (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 @@ -849,10 +915,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. From ac367d6b3577ca53e7d520b9608496f45a0d40e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 13 Jul 2026 12:19:42 +0200 Subject: [PATCH 3/3] =?UTF-8?q?docs:=20ADR-0012=20R7=20=E2=80=94=20teardow?= =?UTF-8?q?n-commits=20model=20+=20persisted-state=20continuation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves the two contract ambiguities blocking merge (aligns with #1235): - Completion model = TEARDOWN-COMMITS (not explicit-close-only). A repair-armed session stays addressable until the transaction ends; ANY teardown (explicit close, idle-reap, daemon shutdown) commits the healed .ad atomically iff the transaction is COMPLETE, else aborts with no publish (never a prefix). An incomplete reap/shutdown leaves the REPAIR_SESSION_EXPIRED tombstone; an explicit close of an incomplete tx just discards. Kept the ARMED->COMPLETE->COMMITTED machine and idempotent post-COMMITTED teardown; removed the "no auto-commit / commit only on explicit close" language. - Continuation by PERSISTED transaction state, not the per-request flag: replay --from --plan-digest resumes on the persisted repair-armed session WITHOUT repeating --save-script; --save-script appears only on the transaction opener. Implementation MUST key keep-alive/continuation off persisted state. Decision 4 repairSessionHeld updated to match. Edited: R7, Decision 4 resume signal, terminal-close, Emitting, commit state machine, tombstone, migration step 9, and validation. --- docs/adr/0012-interactive-replay.md | 134 ++++++++++++++++------------ 1 file changed, 77 insertions(+), 57 deletions(-) diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 89ef42b58..e2139fbe8 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -488,12 +488,15 @@ 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 -**only** on a divergence from a repair-armed replay (`--save-script` was in effect), and 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." +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 @@ -557,16 +560,19 @@ Implementation is not accepted on benchmark evidence alone. Required automated c 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 a reaped - repair session 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 a - `close` before `COMPLETE` aborts and publishes nothing, a `close` at `COMPLETE` commits atomically, and - a second `close` after `COMMITTED` is idempotent teardown with no re-publish; 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 + 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 @@ -690,23 +696,30 @@ 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 is kept alive until `close`.** - `--save-script` opens a multi-invocation repair **transaction**. 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. The subsequent heal actions and - `replay --from` continuations (R2) target that **same live session** until the agent finalizes with - `close`; 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. Whether a `close` commits or aborts is governed by the - commit state machine below — a plain `close` before the transaction reaches `COMPLETE`, a daemon - teardown, or an idle-reap while armed emits **nothing**. An implementation MAY offer a bounded-expiry - escape hatch, but idle-reaping a repair-armed, un-finalized session must leave a **tombstone** (R7's - ownership guarantee, detailed 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`. 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`. +- **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 @@ -717,8 +730,8 @@ scope, not a change to it. The **terminal source `close`** is defined precisely 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 agent then finalizes the transaction with its own explicit -`close`/`close --save-script`, which commits. +`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`. @@ -739,10 +752,11 @@ 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 published when the agent's `close`/`close --save-script ` commits the - transaction (state `COMPLETE` — 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 +- 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 `` @@ -759,13 +773,15 @@ the slice may be published*, as a machine-enforceable state, not a heuristic: state is a publish permitted. - **`COMMITTED`** — the healed `.ad` has been atomically published (below). Terminal. -A `close` (or `close --save-script`) on the repair-armed session is the **commit trigger**, and commits -**only when the state is `COMPLETE`**. `close` while **not** `COMPLETE` is an **ABORT**: discard the -slice, publish **nothing** — never a prefix, never on teardown. There is **no separate auto-commit -trigger**: reaching `COMPLETE` does not itself write a file; it only makes a *subsequent* `close` a commit -rather than an abort. This "write only on intentional completion" gate (grounded in the second design -review) is the rule, not a preference. `close` when already `COMMITTED` is **teardown-only, idempotent — -no re-publish** (so a second finalize can neither double-commit nor trip the no-clobber guard). +**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 @@ -783,9 +799,11 @@ what makes that sentinel trustworthy (a partial never reaches the published path 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 a repair-armed -session that is still `ARMED`/`COMPLETE` (not `COMMITTED`), it must leave a **tombstone** rather than -deleting the session record outright. The tombstone is keyed by the **session key** (the same +**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 @@ -892,14 +910,16 @@ each states its dependencies explicitly. 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 it (with `fail-fast-before-step-1` when keep-alive is impossible); the - `ARMED → COMPLETE → COMMITTED` commit state machine (abort-when-not-`COMPLETE`, commit on any teardown - at `COMPLETE`, idempotent teardown-when-`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 (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). + 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