mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
fix(replay): extend resume.from record-and-heal shape to caution/manual (#1267)
* fix(replay): extend resume.from record-and-heal shape to caution/manual (#1262) caution/manual divergences kept resume.from unshifted (correct — per resolution item 1, N stays unconditionally legal) but never offered a concrete N+1 continuation for their record-and-heal-shaped alternate repair, and a last-step caution/manual divergence repaired by a recorded action was a dead end: pendingRecordAndHeal was only ever stamped for record-and-heal, so the N+1 empty-tail resume was unauthorized (out of range) and close on the not-yet-COMPLETE transaction discarded the just-recorded corrective action — the same trap #1260 closed only for record-and-heal. - buildRepairHintGuidance (src/replay/divergence.ts) now renders BOTH concrete commands for caution/manual when resume.allowed: --from N for a --no-record state fix, --from N+1 for a recorded corrective action. - stampPendingRecordAndHealWatermark (session-replay-resume.ts) now also stamps for caution/manual, but only when the diverged step is the plan's LAST one and N+1 is independently preflight-safe — a mid-plan --from N+1 was already unconditionally legal and un-gated for these hints (unlike record-and-heal, they never mandate a corrective action), so that pre-existing pattern stays un-gated. * fix(replay): add resume.alternateFrom so caution/manual dual-path never advertises a --from the daemon refuses (#1262) The dual-path text guidance offered `--from N + 1` whenever resuming AT `N` was allowed, but `--from N + 1` needs its OWN preflight — which additionally requires the diverged step `N` to be skip-safe. When `N` is a runScript (outputEnv producer) or inside runtime control flow, preflight(N) passes while preflight(N+1) fails, so the text advertised a command the daemon then refused (the text-vs-structured disagreement #1260 blocker 2 banned). - Add optional `resume.alternateFrom` to the decision-4 wire shape (`ReplayDivergenceResume`). The daemon populates it (`N + 1`) for caution/manual ONLY when `evaluateReplayResumePreflight({ from: N + 1 })` passes — the same acceptance condition on both the mid-plan (un-gated, in range) and last-step (watermark-stamp) paths. Its checked range is a strict superset of `from`'s, so alternateFrom present implies allowed. - The text renderer gates the `N + 1` command on `alternateFrom`'s PRESENCE and renders its value verbatim, never re-deriving resumability — so text and the structured wire can never disagree on the advertised next command. - This also closes a parity gap: a JSON/MCP caller now gets both ordinals (previously the dual-path was text-only, structured callers saw only `from`). - ADR-0012 decision-4 documents alternateFrom as additive/optional; projection and trigger tests cover both positions (runScript/control-flow → no alternateFrom, no `--from N + 1`; skip-safe → alternateFrom present). * fix(replay): withhold empty-tail alternateFrom when no session can stamp the watermark (#1262) The empty-tail alternate (`alternateFrom > actions.length`) is accepted by the range check ONLY when it matches a stamped `pendingRecordAndHeal` watermark, and that watermark can only be stamped on a live session. For a last-step caution/manual failure with no active session — a one-step `open` failure, or a session closed mid-replay — the watermark can never be stamped, so the advertised `--from length+1` is then rejected as out of range, re-introducing the text/structured mismatch this arc fixed. - Thread `sessionExists` into `buildReplayDivergenceResume`; gate the one-past-the-end `alternateFrom` on it (mid-plan alternate stays in-range and session-independent). Both divergence sites pass `session !== undefined`. - At the last step this makes `computeReplayResumeAlternateFrom`'s emit condition exactly `computeRecordAndHealWatermark`'s stamp condition, so alternateFrom present ⟺ the watermark gets stamped in the same request. - ADR-0012 decision 4: reword the mechanical resume.from parity statement for the dual-path hints (from=N + optional alternateFrom=N+1); update the empty-tail paragraph to cover caution/manual last-step stamping and the no-session withholding. - Tests: unit (last-step no-session → no alternateFrom; mid-plan no-session → still present) + integration (single-step failure, no session → no alternateFrom on the wire, no --from length+1 in text).
This commit is contained in:
committed by
GitHub
parent
236016ed8a
commit
0436793c25
@@ -475,29 +475,53 @@ 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, repairSessionHeld? }`.
|
||||
Every divergence includes `resume: { allowed, from, reason?, planDigest, alternateFrom?, repairSessionHeld? }`.
|
||||
`from` is not merely the failed step's ordinal — it is the ordinal the caller should actually pass to
|
||||
`--from`, computed from the same `repairHint` carried alongside it (decision 6, R2/R3): for `record-and-heal`,
|
||||
`from` is the failed step's index **+ 1** (the agent performs that step manually before resuming, so
|
||||
resuming AT it would re-diverge on the exact step just completed); for every other hint (`state-repair`,
|
||||
`caution`, `manual`), `from` equals the failed step's index unchanged. This keeps the structured `resume`
|
||||
block and the `repairHint` text guidance below in agreement — a JSON/MCP-first caller that blindly resumes
|
||||
at `resume.from` gets the same continuation a text caller reads out of the rendered guidance, never a stale
|
||||
`from` that loops the caller back onto the step it just repaired.
|
||||
`caution`, `manual`), `from` equals the failed step's index unchanged. For the SINGLE-path hints
|
||||
(`record-and-heal`, `state-repair`) `from` is the whole continuation, and a JSON/MCP-first caller that
|
||||
blindly resumes at `resume.from` reads the identical command the text guidance renders — never a stale `from`
|
||||
that loops the caller back onto the step it just repaired. The DUAL-path hints (`caution`, `manual`) are the
|
||||
exception: `from` (`N`) carries only their app-state-fix continuation, and a second ordinal — the optional
|
||||
`alternateFrom` (`N + 1`, below) — carries the record-and-heal-shaped alternate, so the structured caller
|
||||
reads BOTH fields to match the text guidance rather than `resume.from` alone.
|
||||
|
||||
If the shifted `from` equals `actions.length + 1` (the diverged step was the plan's LAST step), that is a
|
||||
`alternateFrom` is an **additive optional** ordinal (#1262) that makes the `caution`/`manual` dual-path
|
||||
structured-caller-legible, not text-only. Those two hints have TWO legitimate repairs the daemon cannot
|
||||
disambiguate at divergence time: an app-state fix (`--no-record`, then re-run the unchanged step at `from` =
|
||||
`N`), and a record-and-heal-shaped correction (perform the diverged step's intent as a recorded action, then
|
||||
resume PAST it at `N + 1`). `from` carries the first; `alternateFrom` carries the second (`N + 1`), present
|
||||
**only when a `--from N + 1` request for this divergence would actually be accepted** — the daemon computes
|
||||
it as `evaluateReplayResumePreflight({ from: N + 1, actions }).allowed`, which additionally requires the
|
||||
diverged step `N` itself to be skip-safe (so it is absent when `N` is a `runScript` outputEnv producer or
|
||||
sits inside runtime control flow, where `--from N + 1` would be refused). Because that preflight's checked
|
||||
range is a strict superset of `from`'s, `alternateFrom` present implies `allowed: true` — it never
|
||||
contradicts the primary. Absent for `record-and-heal` (its `from` already IS the `N + 1` continuation) and
|
||||
`state-repair` (no recorded-action alternate). The `repairHint` text guidance renders the `N + 1` command
|
||||
**iff `alternateFrom` is present**, never re-deriving resumability client-side, so the text surface and the
|
||||
structured wire advertise the identical next command — closing both the text-vs-structured disagreement a
|
||||
client-side re-derivation would reintroduce and the parity gap where a structured caller saw only `from`.
|
||||
|
||||
If the resume ordinal equals `actions.length + 1` (the diverged step was the plan's LAST step), that is a
|
||||
legal EMPTY-TAIL resume, not an error: there is nothing left to replay, so the resumed run executes zero
|
||||
device actions and falls straight through to the normal end-of-plan completion path, correctly flipping an
|
||||
armed repair transaction COMPLETE (decision 6, R7's `close` commit gate). Rejecting this ordinal outright
|
||||
would force the agent to `close` an INCOMPLETE transaction instead, which aborts and discards the corrective
|
||||
action it just recorded. This one-past-the-end ordinal is authorized ONLY for the EXACT session and target
|
||||
that produced it — the daemon stamps a per-session watermark (`expectedFrom`, the recorded action count at
|
||||
divergence time) whenever a `record-and-heal` divergence reports `allowed: true`, and a later `--from`
|
||||
request is accepted at `actions.length + 1` only when it matches that watermark AND the session's action
|
||||
count has grown since (proof the corrective press was actually recorded) — never a blanket "one past the
|
||||
end is fine" for any session or repair kind, which would let an unrelated or blind resume silently skip the
|
||||
plan's unresolved final step and commit an incomplete repair. The same watermark match, independent of
|
||||
whether `from` lands one past the end or still inside the plan, also gates every OTHER `record-and-heal`
|
||||
divergence time) when a `record-and-heal` divergence reports `allowed: true` (its own `from` is already
|
||||
`N + 1`), and — per #1262 — also for a `caution`/`manual` LAST-step divergence whose `alternateFrom` (`N + 1`)
|
||||
is preflight-safe (their `from` stays `N`, so the watermark tracks the alternate's `N + 1` empty-tail
|
||||
ordinal). Because the watermark can only be stamped on a live session, an empty-tail `alternateFrom` is
|
||||
withheld entirely when no session exists (a one-step `open` failure, or a session closed mid-replay) —
|
||||
otherwise the daemon would advertise a `--from N + 1` it must then reject. A later `--from` request is
|
||||
accepted at `actions.length + 1` only when it matches that watermark AND the session's action count has
|
||||
grown since (proof the corrective press was actually recorded) — never a blanket "one past the end is fine"
|
||||
for any session or repair kind, which would let an unrelated or blind resume silently skip the plan's
|
||||
unresolved final step and commit an incomplete repair. The same watermark match, independent of whether the
|
||||
resume ordinal lands one past the end or still inside the plan, also gates every OTHER `record-and-heal`
|
||||
continuation: resuming at the reported `from` with the action count unchanged is rejected as proof the
|
||||
corrective press never happened, rather than silently resuming past the unrepaired step.
|
||||
`planDigest` is SHA-256 over
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
/**
|
||||
* ADR 0012 decision 6, R2/R3: two behaviors introduced alongside the
|
||||
* `resume.from` / `repairHint` agreement fix (`session-replay-resume.ts`).
|
||||
* ADR 0012 decision 6, R2/R3, extended per #1262: behaviors introduced
|
||||
* alongside the `resume.from` / `repairHint` agreement fix
|
||||
* (`session-replay-resume.ts`).
|
||||
*
|
||||
* 1. A `record-and-heal` divergence on the plan's LAST step reports
|
||||
* `resume.from = actions.length + 1` — a legal EMPTY-TAIL resume, not an
|
||||
@@ -13,6 +14,13 @@
|
||||
* exactly on that reported target with NO new action recorded since the
|
||||
* divergence — proof the agent never performed the corrective press —
|
||||
* instead of silently resuming past the unrepaired step.
|
||||
* 3. #1262: `caution`/`manual` are genuinely dual-path. Their OWN
|
||||
* `resume.from` stays AT the failed step unconditionally (never shifted,
|
||||
* never made illegal — items 1/2 above still apply verbatim once resumed
|
||||
* there), but they ALSO get the SAME `actions.length + 1` empty-tail
|
||||
* authorization (gated behind the SAME recorded-corrective-action proof)
|
||||
* for their record-and-heal-shaped alternate repair, so a last-step
|
||||
* `caution`/`manual` divergence is no longer a dead end.
|
||||
*/
|
||||
import { test, expect, vi, beforeEach } from 'vitest';
|
||||
|
||||
@@ -170,16 +178,23 @@ test('a record-and-heal divergence on the LAST step resumes with an empty tail a
|
||||
expect(healedScript).not.toMatch(/^click /m);
|
||||
});
|
||||
|
||||
test('a --from one past the plan end is rejected as out of range when no record-and-heal watermark authorizes it', async () => {
|
||||
const root = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'agent-device-replay-empty-tail-unauthorized-'),
|
||||
);
|
||||
// --- #1262: `caution`/`manual` are genuinely dual-path — their OWN
|
||||
// `resume.from` stays AT the failed step (N) unconditionally (never shifted,
|
||||
// never made illegal), but they ALSO have a record-and-heal-shaped alternate
|
||||
// repair (the agent performs the diverged step's intent as a recorded action
|
||||
// instead), reachable at `--from N + 1`. `stampPendingRecordAndHealWatermark`
|
||||
// now stamps the SAME empty-tail watermark for these hints, gated behind the
|
||||
// SAME recorded-corrective-action proof, so a last-step `caution`/`manual`
|
||||
// divergence is no longer a dead end (#1260 fixed this only for
|
||||
// `record-and-heal`). ---
|
||||
|
||||
test('a manual divergence (unannotated action-failure) on the LAST step resumes with an empty tail and commits — but only after the corrective press is actually recorded', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-empty-tail-manual-'));
|
||||
const sessionStore = new SessionStore(path.join(root, 'sessions'));
|
||||
const sessionName = 'default';
|
||||
sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' }));
|
||||
// No target-v1 annotation: an unannotated action-failure always routes to
|
||||
// the `manual` fail-safe (no recorded targetEvidence), so NO
|
||||
// `pendingRecordAndHeal` watermark is ever stamped for this divergence.
|
||||
// the `manual` fail-safe (no recorded targetEvidence).
|
||||
const filePath = writeReplayFile(root, ['open "Demo" --relaunch', 'click label="Save"']);
|
||||
|
||||
const invoke = makeRecordingReplayInvoke({
|
||||
@@ -188,6 +203,262 @@ test('a --from one past the plan end is rejected as out of range when no record-
|
||||
failSteps: new Set(['click label="Save"']),
|
||||
});
|
||||
|
||||
// --- Leg 1: open records; "click label=Save" fails at dispatch with no
|
||||
// recorded target evidence, routing to `manual`. It is the plan's LAST
|
||||
// step, so the record-and-heal-shaped alternate target is 3 = actions
|
||||
// (2) + 1 — but `resume.from` itself stays at 2, unshifted. ---
|
||||
const leg1 = await runReplayScriptFile({
|
||||
req: baseReq({ positionals: [filePath], flags: { saveScript: true } }),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(leg1.ok).toBe(false);
|
||||
if (leg1.ok) return;
|
||||
const divergence = leg1.error.details?.divergence as {
|
||||
kind: string;
|
||||
repairHint: string;
|
||||
resume: { allowed: boolean; from: number; planDigest: string; alternateFrom?: number };
|
||||
};
|
||||
expect(divergence.kind).toBe('action-failure');
|
||||
expect(divergence.repairHint).toBe('manual');
|
||||
expect(divergence.resume.allowed).toBe(true);
|
||||
expect(divergence.resume.from).toBe(2);
|
||||
// #1262: the wire carries the record-and-heal-shaped alternate (N + 1 = 3)
|
||||
// — the diverged step is skip-safe, so `--from 3` would be accepted.
|
||||
expect(divergence.resume.alternateFrom).toBe(3);
|
||||
|
||||
const session = sessionStore.get(sessionName)!;
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open']);
|
||||
expect(session.saveScriptComplete).toBeFalsy();
|
||||
// The watermark IS now stamped for `manual` (#1262) — targeting N + 1 (3),
|
||||
// a DIFFERENT ordinal than the unshifted `resume.from` (2) above.
|
||||
expect(session.pendingRecordAndHeal).toEqual({ expectedFrom: 3, actionsCountAtDivergence: 1 });
|
||||
|
||||
// --- A blind resume at the empty-tail target BEFORE performing the
|
||||
// corrective press is rejected: no new action was recorded since the
|
||||
// divergence, so nothing proves the diverged step's intent was actually
|
||||
// performed. ---
|
||||
const blindResume = await runReplayScriptFile({
|
||||
req: baseReq({
|
||||
positionals: [filePath],
|
||||
flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest },
|
||||
}),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(blindResume.ok).toBe(false);
|
||||
if (!blindResume.ok) {
|
||||
expect(blindResume.error.code).toBe('INVALID_ARGS');
|
||||
expect(blindResume.error.message).toMatch(/no corrective action/);
|
||||
}
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open']);
|
||||
|
||||
// --- Agent performs the step's intent as a recorded action (blessed
|
||||
// @ref), recorded live. ---
|
||||
sessionStore.recordAction(session, {
|
||||
command: 'press',
|
||||
positionals: ['@e6'],
|
||||
flags: {},
|
||||
result: { selectorChain: ['label="Save"'] },
|
||||
targetEvidence: freshEvidence('save-v2', 'Save'),
|
||||
});
|
||||
|
||||
// --- Leg 2: the SAME `--from 3` now succeeds — the corrective press grew
|
||||
// session.actions, consuming the watermark. Zero steps execute, and the
|
||||
// runtime's normal end-of-plan path flips the transaction COMPLETE. ---
|
||||
const leg2 = await runReplayScriptFile({
|
||||
req: baseReq({
|
||||
positionals: [filePath],
|
||||
flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest },
|
||||
}),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(leg2.ok).toBe(true);
|
||||
if (leg2.ok) {
|
||||
expect((leg2.data as { replayed: number }).replayed).toBe(0);
|
||||
}
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']);
|
||||
expect(session.saveScriptComplete).toBe(true);
|
||||
|
||||
// --- Commit: the transaction is COMPLETE, so the healed script actually
|
||||
// publishes — the corrective press survives, "click" (never recorded,
|
||||
// since a `manual` divergence never dispatched it) does not. Proves the
|
||||
// empty-tail resume did not lead to a discarded repair (the #1260
|
||||
// discard-at-close trap, now also closed for `manual`). ---
|
||||
const writeResult = sessionStore.writeSessionLog(session);
|
||||
expect(writeResult.written).toBe(true);
|
||||
const healedPath = path.join(root, 'flow.healed.ad');
|
||||
expect(fs.existsSync(healedPath)).toBe(true);
|
||||
const healedScript = fs.readFileSync(healedPath, 'utf8');
|
||||
expect(healedScript).toContain('open');
|
||||
expect(healedScript).toContain('save-v2');
|
||||
expect(healedScript).not.toMatch(/^click /m);
|
||||
});
|
||||
|
||||
test('a caution (identity-mismatch) divergence on the LAST step resumes with an empty tail and commits — but only after the corrective press is actually recorded', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-empty-tail-caution-'));
|
||||
const sessionStore = new SessionStore(path.join(root, 'sessions'));
|
||||
const sessionName = 'default';
|
||||
sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' }));
|
||||
const SAVE_ANNOTATION =
|
||||
'# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[],"sibling":0,"viewportOrder":0,"verification":"verified"}';
|
||||
const filePath = writeReplayFile(root, [
|
||||
'open "Demo" --relaunch',
|
||||
SAVE_ANNOTATION,
|
||||
'click label="Save"',
|
||||
]);
|
||||
|
||||
// The pre-action verification capture finds a node with the recorded
|
||||
// LABEL but a renamed id — a genuine identity-mismatch (matchCount 1),
|
||||
// never reaching dispatch for "click".
|
||||
mockDispatchCommand.mockResolvedValue({
|
||||
nodes: [
|
||||
{
|
||||
index: 0,
|
||||
depth: 0,
|
||||
type: 'Button',
|
||||
identifier: 'save-v2',
|
||||
label: 'Save',
|
||||
rect: { x: 10, y: 10, width: 40, height: 20 },
|
||||
},
|
||||
],
|
||||
truncated: false,
|
||||
backend: 'xctest',
|
||||
});
|
||||
|
||||
const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName });
|
||||
|
||||
// --- Leg 1: open records; "click label=Save" diverges pre-action as
|
||||
// identity-mismatch/`caution`. It is the plan's LAST step, so the
|
||||
// record-and-heal-shaped alternate target is 3 = actions (2) + 1 — but
|
||||
// `resume.from` itself stays at 2, unshifted (#1262 item 1). ---
|
||||
const leg1 = await runReplayScriptFile({
|
||||
req: baseReq({ positionals: [filePath], flags: { saveScript: true } }),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(leg1.ok).toBe(false);
|
||||
if (leg1.ok) return;
|
||||
const divergence = leg1.error.details?.divergence as {
|
||||
kind: string;
|
||||
repairHint: string;
|
||||
resume: { allowed: boolean; from: number; planDigest: string; alternateFrom?: number };
|
||||
};
|
||||
expect(divergence.kind).toBe('identity-mismatch');
|
||||
expect(divergence.repairHint).toBe('caution');
|
||||
expect(divergence.resume.allowed).toBe(true);
|
||||
expect(divergence.resume.from).toBe(2);
|
||||
// #1262: the wire carries the record-and-heal-shaped alternate (N + 1 = 3).
|
||||
expect(divergence.resume.alternateFrom).toBe(3);
|
||||
|
||||
const session = sessionStore.get(sessionName)!;
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open']);
|
||||
expect(session.saveScriptComplete).toBeFalsy();
|
||||
expect(session.pendingRecordAndHeal).toEqual({ expectedFrom: 3, actionsCountAtDivergence: 1 });
|
||||
|
||||
// --- A blind resume at the empty-tail target BEFORE performing the
|
||||
// corrective press is rejected. ---
|
||||
const blindResume = await runReplayScriptFile({
|
||||
req: baseReq({
|
||||
positionals: [filePath],
|
||||
flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest },
|
||||
}),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(blindResume.ok).toBe(false);
|
||||
if (!blindResume.ok) {
|
||||
expect(blindResume.error.code).toBe('INVALID_ARGS');
|
||||
expect(blindResume.error.message).toMatch(/no corrective action/);
|
||||
}
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open']);
|
||||
|
||||
// --- Agent presses the actual (renamed) control via a blessed @ref,
|
||||
// recorded live — the record-and-heal-shaped repair for path (a) from
|
||||
// #1262 ("selector binds the wrong node on the right screen"). ---
|
||||
sessionStore.recordAction(session, {
|
||||
command: 'press',
|
||||
positionals: ['@e6'],
|
||||
flags: {},
|
||||
result: { selectorChain: ['id="save-v2"'] },
|
||||
targetEvidence: freshEvidence('save-v2', 'Save'),
|
||||
});
|
||||
|
||||
// --- Leg 2: the SAME `--from 3` now succeeds — zero steps execute, and
|
||||
// the runtime's normal end-of-plan path flips the transaction COMPLETE. ---
|
||||
const leg2 = await runReplayScriptFile({
|
||||
req: baseReq({
|
||||
positionals: [filePath],
|
||||
flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest },
|
||||
}),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(leg2.ok).toBe(true);
|
||||
if (leg2.ok) {
|
||||
expect((leg2.data as { replayed: number }).replayed).toBe(0);
|
||||
}
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open', 'press']);
|
||||
expect(session.saveScriptComplete).toBe(true);
|
||||
|
||||
// --- Commit: COMPLETE, so the healed script publishes the corrective
|
||||
// press; the pre-action "click" (never dispatched) does not appear. ---
|
||||
const writeResult = sessionStore.writeSessionLog(session);
|
||||
expect(writeResult.written).toBe(true);
|
||||
const healedPath = path.join(root, 'flow.healed.ad');
|
||||
expect(fs.existsSync(healedPath)).toBe(true);
|
||||
const healedScript = fs.readFileSync(healedPath, 'utf8');
|
||||
expect(healedScript).toContain('open');
|
||||
expect(healedScript).toContain('save-v2');
|
||||
expect(healedScript).not.toMatch(/^click /m);
|
||||
});
|
||||
|
||||
test('--from N stays legal for a caution divergence even after the N + 1 empty-tail watermark is stamped', async () => {
|
||||
const root = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'agent-device-replay-empty-tail-caution-from-n-'),
|
||||
);
|
||||
const sessionStore = new SessionStore(path.join(root, 'sessions'));
|
||||
const sessionName = 'default';
|
||||
sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' }));
|
||||
const SAVE_ANNOTATION =
|
||||
'# agent-device:target-v1 {"id":"save","role":"button","label":"Save","ancestry":[],"sibling":0,"viewportOrder":0,"verification":"verified"}';
|
||||
const filePath = writeReplayFile(root, [
|
||||
'open "Demo" --relaunch',
|
||||
SAVE_ANNOTATION,
|
||||
'click label="Save"',
|
||||
]);
|
||||
|
||||
// Leg 1's capture: a genuine identity-mismatch (renamed id, same label).
|
||||
mockDispatchCommand.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
index: 0,
|
||||
depth: 0,
|
||||
type: 'Button',
|
||||
identifier: 'save-v2',
|
||||
label: 'Save',
|
||||
rect: { x: 10, y: 10, width: 40, height: 20 },
|
||||
},
|
||||
],
|
||||
truncated: false,
|
||||
backend: 'xctest',
|
||||
});
|
||||
|
||||
const invoke = makeRecordingReplayInvoke({ sessionStore, sessionName });
|
||||
|
||||
const leg1 = await runReplayScriptFile({
|
||||
req: baseReq({ positionals: [filePath], flags: { saveScript: true } }),
|
||||
sessionName,
|
||||
@@ -201,36 +472,54 @@ test('a --from one past the plan end is rejected as out of range when no record-
|
||||
repairHint: string;
|
||||
resume: { allowed: boolean; from: number; planDigest: string };
|
||||
};
|
||||
expect(divergence.repairHint).toBe('manual');
|
||||
// `from` stays AT the failed step (2) — never shifted, since only
|
||||
// `record-and-heal` shifts `from`. The plan has 2 actions, so `--from 3`
|
||||
// (one past the end) is what an EMPTY-TAIL resume would use — but this
|
||||
// session never authorized it.
|
||||
expect(divergence.repairHint).toBe('caution');
|
||||
expect(divergence.resume.from).toBe(2);
|
||||
const session = sessionStore.get(sessionName)!;
|
||||
expect(session.pendingRecordAndHeal).toBeUndefined();
|
||||
// The N + 1 watermark is stamped (path (a) is available)...
|
||||
expect(session.pendingRecordAndHeal).toEqual({ expectedFrom: 3, actionsCountAtDivergence: 1 });
|
||||
|
||||
// --- A caller crafts `--from 3` directly — exactly the one-past-the-end
|
||||
// ordinal a record-and-heal empty-tail resume would use, but with no
|
||||
// matching watermark on this session. Must be rejected as out of range,
|
||||
// never silently executed with zero steps (which would let `close`
|
||||
// commit while the actual final "click" step remains unresolved). ---
|
||||
const exploitAttempt = await runReplayScriptFile({
|
||||
// --- ...but path (b) — a `--no-record` app-state fix, then resuming AT
|
||||
// the unshifted `resume.from` (N = 2) — must stay unconditionally legal:
|
||||
// the watermark for the N + 1 alternate must never make N itself illegal.
|
||||
// The recorded-corrective-action guard only ever matches `expectedFrom`
|
||||
// (3), never N, so this request sails through the entry-index checks
|
||||
// straight to re-running "click" — this time against a capture where the
|
||||
// recorded id is present again (the state fix), so it verifies and
|
||||
// dispatches for real. ---
|
||||
mockDispatchCommand.mockResolvedValueOnce({
|
||||
nodes: [
|
||||
{
|
||||
index: 0,
|
||||
depth: 0,
|
||||
type: 'Button',
|
||||
identifier: 'save',
|
||||
label: 'Save',
|
||||
rect: { x: 10, y: 10, width: 40, height: 20 },
|
||||
},
|
||||
],
|
||||
truncated: false,
|
||||
backend: 'xctest',
|
||||
});
|
||||
const resumeAtN = await runReplayScriptFile({
|
||||
req: baseReq({
|
||||
positionals: [filePath],
|
||||
flags: { saveScript: true, replayFrom: 3, replayPlanDigest: divergence.resume.planDigest },
|
||||
flags: {
|
||||
saveScript: true,
|
||||
replayFrom: divergence.resume.from,
|
||||
replayPlanDigest: divergence.resume.planDigest,
|
||||
},
|
||||
}),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke,
|
||||
});
|
||||
expect(exploitAttempt.ok).toBe(false);
|
||||
if (!exploitAttempt.ok) {
|
||||
expect(exploitAttempt.error.code).toBe('INVALID_ARGS');
|
||||
expect(exploitAttempt.error.message).toMatch(/out of range/);
|
||||
expect(resumeAtN.ok).toBe(true);
|
||||
if (resumeAtN.ok) {
|
||||
expect((resumeAtN.data as { replayed: number }).replayed).toBe(1);
|
||||
}
|
||||
expect(session.saveScriptComplete).toBeFalsy();
|
||||
expect(session.actions.map((a) => a.command)).toEqual(['open', 'click']);
|
||||
expect(session.saveScriptComplete).toBe(true);
|
||||
});
|
||||
|
||||
test('an unauthorized --from one past the plan end is rejected on an ARMED session whose last-step divergence hint is state-repair, not record-and-heal', async () => {
|
||||
|
||||
@@ -3,8 +3,10 @@ import { test } from 'vitest';
|
||||
import {
|
||||
buildReplayDivergenceResume,
|
||||
evaluateReplayResumePreflight,
|
||||
stampPendingRecordAndHealWatermark,
|
||||
} from '../session-replay-resume.ts';
|
||||
import type { SessionAction } from '../../types.ts';
|
||||
import type { SessionAction, SessionState } from '../../types.ts';
|
||||
import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts';
|
||||
|
||||
function action(overrides: Partial<SessionAction> = {}): SessionAction {
|
||||
return { ts: 0, command: 'click', positionals: ['label="Save"'], flags: {}, ...overrides };
|
||||
@@ -136,11 +138,15 @@ test('buildReplayDivergenceResume reports allowed:true with from/planDigest for
|
||||
action({ command: 'open', positionals: ['Demo'] }),
|
||||
action({ command: 'click', positionals: ['label="Save"'] }),
|
||||
];
|
||||
// `state-repair` carries no `alternateFrom` (no recorded-action alternate),
|
||||
// so this stays a clean base-shape assertion — the caution/manual
|
||||
// `alternateFrom` cases are covered by the dedicated tests below.
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
repairHint: 'state-repair',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.deepEqual(resume, { allowed: true, from: 2, planDigest: 'abc123' });
|
||||
});
|
||||
@@ -159,6 +165,7 @@ test('buildReplayDivergenceResume reports allowed:false with from/planDigest/rea
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.allowed, false);
|
||||
assert.equal(resume.from, 2);
|
||||
@@ -182,6 +189,7 @@ test('buildReplayDivergenceResume with repairHint record-and-heal resumes AFTER
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'record-and-heal',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.deepEqual(resume, { allowed: true, from: 3, planDigest: 'abc123' });
|
||||
});
|
||||
@@ -199,6 +207,7 @@ test('buildReplayDivergenceResume with repairHint record-and-heal on the LAST pl
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'record-and-heal',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.deepEqual(resume, { allowed: true, from: 3, planDigest: 'abc123' });
|
||||
});
|
||||
@@ -220,9 +229,407 @@ test('buildReplayDivergenceResume with repairHint record-and-heal still rejects
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'record-and-heal',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.allowed, false);
|
||||
assert.equal(resume.from, 3);
|
||||
if (resume.allowed) return;
|
||||
assert.match(resume.reason, /control flow/);
|
||||
});
|
||||
|
||||
// --- buildReplayDivergenceResume: `resume.alternateFrom` (#1262). The
|
||||
// `caution`/`manual` dual-path's SECOND ordinal (`failedIndex + 1`), present
|
||||
// ONLY when a `--from failedIndex + 1` request would actually be accepted —
|
||||
// i.e. `evaluateReplayResumePreflight({ from: failedIndex + 1 })` passes,
|
||||
// which additionally requires the DIVERGED step itself to be skip-safe. This
|
||||
// closes the parity bug where the text renderer offered `--from N + 1` based
|
||||
// on `N`'s preflight (which does not check step `N`'s own skip-safety). ---
|
||||
|
||||
test('buildReplayDivergenceResume: caution mid-plan (skip-safe diverged step) carries alternateFrom = failedIndex + 1', () => {
|
||||
const actions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'caution',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.allowed, true);
|
||||
assert.equal(resume.from, 2); // unshifted
|
||||
if (!resume.allowed) return;
|
||||
assert.equal(resume.alternateFrom, 3);
|
||||
});
|
||||
|
||||
test('buildReplayDivergenceResume: manual last-step (skip-safe diverged step) carries alternateFrom = actions.length + 1', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.allowed, true);
|
||||
assert.equal(resume.from, 2);
|
||||
if (!resume.allowed) return;
|
||||
assert.equal(resume.alternateFrom, 3); // empty-tail ordinal
|
||||
});
|
||||
|
||||
test('buildReplayDivergenceResume: caution whose diverged step is a runScript carries NO alternateFrom (mid-plan AND last-step)', () => {
|
||||
// The `N + 1` alternate would skip the runScript (outputEnv producer) at
|
||||
// step N, which `evaluateReplayResumePreflight({ from: N + 1 })` refuses —
|
||||
// so alternateFrom is absent even though resuming AT N stays allowed.
|
||||
const midPlan: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: '__maestroRunScript', positionals: ['./setup.js'] }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const midPlanResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: midPlan,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'caution',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(midPlanResume.allowed, true); // resuming AT 2 is fine
|
||||
assert.equal(midPlanResume.from, 2);
|
||||
if (!midPlanResume.allowed) return;
|
||||
assert.equal(midPlanResume.alternateFrom, undefined);
|
||||
|
||||
const lastStep: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: '__maestroRunScript', positionals: ['./setup.js'] }),
|
||||
];
|
||||
const lastStepResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: lastStep,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'caution',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(lastStepResume.allowed, true);
|
||||
assert.equal(lastStepResume.from, 2);
|
||||
if (!lastStepResume.allowed) return;
|
||||
assert.equal(lastStepResume.alternateFrom, undefined);
|
||||
});
|
||||
|
||||
test('buildReplayDivergenceResume: manual whose diverged step is inside runtime control flow carries NO alternateFrom (mid-plan AND last-step)', () => {
|
||||
// A control-flow diverged step is not even resumable AT (you cannot resume
|
||||
// INTO a retry/runFlowWhen wrapper), so `resume.allowed` is false and the
|
||||
// `allowed: false` shape has no `alternateFrom` field at all — the key
|
||||
// invariant is that no `N + 1` alternate is ever advertised for it.
|
||||
const controlStep = () =>
|
||||
action({
|
||||
command: 'back',
|
||||
positionals: [],
|
||||
replayControl: { kind: 'retry', maxRetries: 1, actions: [action({ command: 'back' })] },
|
||||
});
|
||||
const midPlan: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
controlStep(),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const midPlanResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: midPlan,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal('alternateFrom' in midPlanResume, false);
|
||||
|
||||
const lastStep: SessionAction[] = [action({ command: 'open' }), controlStep()];
|
||||
const lastStepResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: lastStep,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal('alternateFrom' in lastStepResume, false);
|
||||
});
|
||||
|
||||
test('buildReplayDivergenceResume: record-and-heal and state-repair never carry alternateFrom (no separate recorded-action alternate)', () => {
|
||||
const actions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
for (const repairHint of ['record-and-heal', 'state-repair'] as const) {
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint,
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.allowed, true);
|
||||
if (!resume.allowed) return;
|
||||
assert.equal(resume.alternateFrom, undefined, `expected no alternateFrom for ${repairHint}`);
|
||||
}
|
||||
});
|
||||
|
||||
// --- #1262 (re-review): the EMPTY-TAIL alternate (`failedIndex + 1 >
|
||||
// actions.length`) is authorizable only via the `pendingRecordAndHeal`
|
||||
// watermark, which can only be stamped on a LIVE session. With NO session — a
|
||||
// one-step `open` failure, or a session closed mid-replay — advertising
|
||||
// `--from actions.length + 1` would be rejected as out of range, so it must
|
||||
// not be emitted. A MID-PLAN alternate (in range) needs no watermark and stays
|
||||
// session-independent. ---
|
||||
|
||||
test('buildReplayDivergenceResume: LAST-step caution/manual with NO session carries NO alternateFrom (empty-tail needs a watermark, which needs a session)', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
for (const repairHint of ['caution', 'manual'] as const) {
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2, // last step → alternate would be the one-past-end ordinal 3
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint,
|
||||
sessionExists: false,
|
||||
});
|
||||
assert.equal(resume.allowed, true); // resuming AT the failed step (2) is still fine
|
||||
if (!resume.allowed) return;
|
||||
assert.equal(
|
||||
resume.alternateFrom,
|
||||
undefined,
|
||||
`expected no empty-tail alternateFrom without a session for ${repairHint}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('buildReplayDivergenceResume: MID-PLAN caution/manual with NO session STILL carries alternateFrom (in-range, no watermark needed)', () => {
|
||||
// 3-step plan; failedIndex 2 → alternate 3 is IN RANGE (<= actions.length),
|
||||
// so it needs no watermark and is emitted regardless of session existence.
|
||||
const actions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
for (const repairHint of ['caution', 'manual'] as const) {
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint,
|
||||
sessionExists: false,
|
||||
});
|
||||
assert.equal(resume.allowed, true);
|
||||
if (!resume.allowed) return;
|
||||
assert.equal(resume.alternateFrom, 3, `expected mid-plan alternateFrom for ${repairHint}`);
|
||||
}
|
||||
});
|
||||
|
||||
// --- stampPendingRecordAndHealWatermark (#1262): the watermark is now ALSO
|
||||
// stamped for `caution`/`manual`, but ONLY for the LAST-step empty-tail
|
||||
// alternate (`failedIndex === actions.length`, targeting `failedIndex + 1`).
|
||||
// A MID-PLAN `--from N + 1` was already unconditionally legal (in range) and
|
||||
// un-gated for these hints before #1262 — an agent may legitimately decide
|
||||
// to skip a `caution`/`manual` diverged step without repairing it, unlike
|
||||
// `record-and-heal`'s mandatory-repair contract — and that pattern must not
|
||||
// regress. ---
|
||||
|
||||
function sessionWithActions(count: number): SessionState {
|
||||
const session = makeIosSession('default');
|
||||
session.actions = Array.from({ length: count }, () => action());
|
||||
return session;
|
||||
}
|
||||
|
||||
test('stampPendingRecordAndHealWatermark stamps record-and-heal at its own (already-shifted) resume.from, mid-plan or last-step', () => {
|
||||
// Mid-plan: failedIndex (2) is NOT the plan's last step (3 actions total).
|
||||
const midPlanActions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const midPlanResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: midPlanActions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'record-and-heal',
|
||||
sessionExists: true,
|
||||
});
|
||||
const midPlanSession = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session: midPlanSession,
|
||||
resume: midPlanResume,
|
||||
repairHint: 'record-and-heal',
|
||||
failedIndex: 2,
|
||||
actions: midPlanActions,
|
||||
});
|
||||
assert.deepEqual(midPlanSession.pendingRecordAndHeal, {
|
||||
expectedFrom: 3,
|
||||
actionsCountAtDivergence: 1,
|
||||
});
|
||||
|
||||
// Last-step: failedIndex (2) IS the plan's last step (2 actions total).
|
||||
const lastStepActions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const lastStepResume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions: lastStepActions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'record-and-heal',
|
||||
sessionExists: true,
|
||||
});
|
||||
const lastStepSession = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session: lastStepSession,
|
||||
resume: lastStepResume,
|
||||
repairHint: 'record-and-heal',
|
||||
failedIndex: 2,
|
||||
actions: lastStepActions,
|
||||
});
|
||||
assert.deepEqual(lastStepSession.pendingRecordAndHeal, {
|
||||
expectedFrom: 3,
|
||||
actionsCountAtDivergence: 1,
|
||||
});
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark stamps caution at the LAST-step empty-tail (failedIndex + 1), NOT at its own unshifted resume.from', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'caution',
|
||||
sessionExists: true,
|
||||
});
|
||||
assert.equal(resume.from, 2); // unshifted — item 1 of #1262's resolution
|
||||
const session = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint: 'caution',
|
||||
failedIndex: 2, // the plan's LAST step (2 actions total)
|
||||
actions,
|
||||
});
|
||||
// The watermark targets failedIndex + 1 (3) — a DIFFERENT ordinal than
|
||||
// resume.from (2) — never blocking `--from 2` itself.
|
||||
assert.deepEqual(session.pendingRecordAndHeal, {
|
||||
expectedFrom: 3,
|
||||
actionsCountAtDivergence: 1,
|
||||
});
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark stamps manual the same way as caution at the last step', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'manual',
|
||||
sessionExists: true,
|
||||
});
|
||||
const session = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint: 'manual',
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
});
|
||||
assert.deepEqual(session.pendingRecordAndHeal, {
|
||||
expectedFrom: 3,
|
||||
actionsCountAtDivergence: 1,
|
||||
});
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark does NOT stamp for a MID-PLAN caution/manual divergence — that N + 1 was already unconditionally legal and stays un-gated', () => {
|
||||
// 3-step plan; failedIndex (2) is NOT the last step (3).
|
||||
const actions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: 'click' }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
for (const repairHint of ['caution', 'manual'] as const) {
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint,
|
||||
sessionExists: true,
|
||||
});
|
||||
const session = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({ session, resume, repairHint, failedIndex: 2, actions });
|
||||
assert.equal(
|
||||
session.pendingRecordAndHeal,
|
||||
undefined,
|
||||
`expected no watermark for ${repairHint}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark does not stamp for a LAST-step caution/manual divergence when the N + 1 target is not itself preflight-safe', () => {
|
||||
// failedIndex (3) IS the last step, but skipping it (to reach N + 1 = 4)
|
||||
// requires also skipping step 2 (an outputEnv-producing maestro runScript),
|
||||
// which cannot be proven safe — even though `resume.from` (unshifted, at
|
||||
// N = 3 — resuming AT it, not skipping it) is independently fine.
|
||||
const actions: SessionAction[] = [
|
||||
action({ command: 'open' }),
|
||||
action({ command: '__maestroRunScript', positionals: ['./setup.js'] }),
|
||||
action({ command: 'click' }),
|
||||
];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 3,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'caution',
|
||||
sessionExists: true,
|
||||
});
|
||||
const session = sessionWithActions(2);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint: 'caution',
|
||||
failedIndex: 3,
|
||||
actions,
|
||||
});
|
||||
assert.equal(session.pendingRecordAndHeal, undefined);
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark never stamps for state-repair (not in the extended eligible set)', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'state-repair',
|
||||
sessionExists: true,
|
||||
});
|
||||
const session = sessionWithActions(1);
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint: 'state-repair',
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
});
|
||||
assert.equal(session.pendingRecordAndHeal, undefined);
|
||||
});
|
||||
|
||||
test('stampPendingRecordAndHealWatermark clears a stale watermark from an earlier divergence when the new one is ineligible', () => {
|
||||
const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
|
||||
const session = sessionWithActions(1);
|
||||
session.pendingRecordAndHeal = { expectedFrom: 3, actionsCountAtDivergence: 0 };
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
planDigest: 'abc123',
|
||||
repairHint: 'state-repair',
|
||||
sessionExists: true,
|
||||
});
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint: 'state-repair',
|
||||
failedIndex: 2,
|
||||
actions,
|
||||
});
|
||||
assert.equal(session.pendingRecordAndHeal, undefined);
|
||||
});
|
||||
|
||||
@@ -12,6 +12,7 @@ import { SessionStore } from '../../session-store.ts';
|
||||
import type { DaemonResponse } from '../../types.ts';
|
||||
import { dispatchCommand } from '../../../core/dispatch.ts';
|
||||
import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts';
|
||||
import { formatReplayDivergenceReport } from '../../../replay/divergence.ts';
|
||||
import {
|
||||
baseReplayRequest as baseReq,
|
||||
writeReplayFile,
|
||||
@@ -82,13 +83,72 @@ test('a failing replay step returns REPLAY_DIVERGENCE with cause preserved and c
|
||||
|
||||
expect(divergence.suggestions).toEqual([]);
|
||||
expect(divergence.suggestionCount).toBe(0);
|
||||
const resume = divergence.resume as { allowed: boolean; from: number; planDigest: string };
|
||||
const resume = divergence.resume as {
|
||||
allowed: boolean;
|
||||
from: number;
|
||||
planDigest: string;
|
||||
alternateFrom?: number;
|
||||
};
|
||||
// Step 1 ("open") is a plain action with no control flow and no outputEnv
|
||||
// production, so resuming at the failed step (2) is safe.
|
||||
expect(resume).toEqual({ allowed: true, from: 2, planDigest: expect.any(String) });
|
||||
// production, so resuming at the failed step (2) is safe. This unannotated
|
||||
// action-failure routes to `manual`, and the diverged step (2) is the plan's
|
||||
// LAST and is itself skip-safe, so the #1262 record-and-heal-shaped alternate
|
||||
// ordinal (`alternateFrom = 3`) rides the wire alongside the unshifted `from`.
|
||||
expect(resume).toEqual({
|
||||
allowed: true,
|
||||
from: 2,
|
||||
planDigest: expect.any(String),
|
||||
alternateFrom: 3,
|
||||
});
|
||||
expect(resume.planDigest).toMatch(/^[0-9a-f]{64}$/);
|
||||
});
|
||||
|
||||
test('#1262: a LAST-step failure with NO active session carries NO empty-tail alternateFrom and never advertises --from length+1 in text', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-no-session-tail-'));
|
||||
const sessionStore = new SessionStore(path.join(root, 'sessions'));
|
||||
const sessionName = 'default';
|
||||
// NO session is pre-seeded — a single-step plan whose only (LAST) step fails
|
||||
// before any session exists (mirrors a one-step `open` failure, or a session
|
||||
// closed mid-replay). The empty-tail alternate `--from 2` would need a
|
||||
// `pendingRecordAndHeal` watermark, which can only be stamped on a live
|
||||
// session — with none, the daemon would reject `--from 2` as out of range,
|
||||
// so it must not be advertised.
|
||||
const filePath = writeReplayFile(root, ['click "Save"']);
|
||||
mockDispatchCommand.mockRejectedValue(new Error('no device runner available'));
|
||||
|
||||
const response = await runReplayScriptFile({
|
||||
req: baseReq({ positionals: [filePath] }),
|
||||
sessionName,
|
||||
logPath: path.join(root, 'daemon.log'),
|
||||
sessionStore,
|
||||
invoke: async () => ({
|
||||
ok: false,
|
||||
error: { code: 'COMMAND_FAILED', message: 'not hittable' },
|
||||
}),
|
||||
});
|
||||
|
||||
expect(response.ok).toBe(false);
|
||||
if (response.ok) return;
|
||||
expect(response.error.code).toBe('REPLAY_DIVERGENCE');
|
||||
const divergence = response.error.details?.divergence as {
|
||||
repairHint: string;
|
||||
resume: { allowed: boolean; from: number; alternateFrom?: number };
|
||||
};
|
||||
// No session → capture unavailable → `manual`; resuming AT the failed step
|
||||
// (1) is still fine, but the one-past-end alternate is withheld.
|
||||
expect(divergence.repairHint).toBe('manual');
|
||||
expect(divergence.resume.allowed).toBe(true);
|
||||
expect(divergence.resume.from).toBe(1);
|
||||
expect(divergence.resume.alternateFrom).toBeUndefined();
|
||||
|
||||
// Text side: the state-fix `--from 1` may appear, but the empty-tail
|
||||
// `--from 2` (which the daemon would reject) must NOT.
|
||||
const report = formatReplayDivergenceReport(response.error.details);
|
||||
expect(report).not.toBeNull();
|
||||
expect(report!).toMatch(/Repair hint: manual/);
|
||||
expect(report!).not.toMatch(/--from 2\b/);
|
||||
});
|
||||
|
||||
test('a normalized nested failure preserves typed recovery signals on REPLAY_DIVERGENCE', async () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-recovery-signals-'));
|
||||
const sessionStore = new SessionStore(path.join(root, 'sessions'));
|
||||
|
||||
@@ -124,9 +124,18 @@ export async function buildReplayFailureDivergence(params: {
|
||||
actions: planActions,
|
||||
planDigest,
|
||||
repairHint,
|
||||
// A live session is required to stamp the empty-tail watermark below, so it
|
||||
// gates whether the one-past-the-end `alternateFrom` may be advertised.
|
||||
sessionExists: session !== undefined,
|
||||
});
|
||||
if (session) {
|
||||
stampPendingRecordAndHealWatermark({ session, resume, repairHint });
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint,
|
||||
failedIndex: index + 1,
|
||||
actions: planActions,
|
||||
});
|
||||
sessionStore.set(sessionName, session);
|
||||
}
|
||||
|
||||
|
||||
@@ -84,44 +84,157 @@ function producesOutputEnv(action: SessionAction): boolean {
|
||||
* COMPLETE. Rejecting it here would send the agent to `close` instead —
|
||||
* which discards the just-recorded corrective action, since commit is gated
|
||||
* on that same COMPLETE flag.
|
||||
*
|
||||
* `alternateFrom` (#1262) is the `caution`/`manual` dual-path's SECOND
|
||||
* ordinal (`failedIndex + 1`, the record-and-heal-shaped alternate) — see
|
||||
* `computeReplayResumeAlternateFrom`.
|
||||
*/
|
||||
export function buildReplayDivergenceResume(params: {
|
||||
failedIndex: number; // 1-based
|
||||
actions: SessionAction[];
|
||||
planDigest: string;
|
||||
repairHint: ReplayRepairHint;
|
||||
// Whether a live session exists at divergence time — the `pendingRecordAndHeal`
|
||||
// watermark can only be stamped on a session, so it gates the empty-tail
|
||||
// (one-past-the-end) `alternateFrom`. See `computeReplayResumeAlternateFrom`.
|
||||
sessionExists: boolean;
|
||||
}): ReplayDivergenceResume {
|
||||
const { failedIndex, actions, planDigest, repairHint } = params;
|
||||
const { failedIndex, actions, planDigest, repairHint, sessionExists } = params;
|
||||
const from = repairHint === 'record-and-heal' ? failedIndex + 1 : failedIndex;
|
||||
const preflight = evaluateReplayResumePreflight({ from, actions });
|
||||
return preflight.allowed
|
||||
? { allowed: true, from, planDigest }
|
||||
: { allowed: false, from, planDigest, reason: preflight.reason };
|
||||
if (!preflight.allowed) return { allowed: false, from, planDigest, reason: preflight.reason };
|
||||
const alternateFrom = computeReplayResumeAlternateFrom({
|
||||
failedIndex,
|
||||
actions,
|
||||
repairHint,
|
||||
sessionExists,
|
||||
});
|
||||
return {
|
||||
allowed: true,
|
||||
from,
|
||||
planDigest,
|
||||
...(alternateFrom !== undefined ? { alternateFrom } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* ADR 0012 decision 6, R2/R3: a `record-and-heal` divergence's `resume.from`
|
||||
* assumes the agent performs the diverged step manually before continuing —
|
||||
* nothing else enforces that. Stamps a watermark on the LIVE session so a
|
||||
* later `--from` request that lands exactly on the expected target with NO
|
||||
* new action recorded since (proof the corrective press never happened) can
|
||||
* be rejected by `rejectUnperformedRecordAndHeal` instead of silently
|
||||
* resuming past the unrepaired step and, if the tail then completes,
|
||||
* committing a healed script with a hole at the diverged step.
|
||||
* ADR 0012 decision 4 / #1262: the `caution`/`manual` dual-path's SECOND
|
||||
* ordinal, the record-and-heal-shaped alternate (`failedIndex + 1`). Returned
|
||||
* ONLY when a `--from failedIndex + 1` request for this divergence would
|
||||
* actually be accepted by the daemon — i.e. exactly its own resume preflight
|
||||
* passes (`evaluateReplayResumePreflight`, which unlike `from`'s own preflight
|
||||
* ALSO requires the DIVERGED step to be skip-safe, since reaching
|
||||
* `failedIndex + 1` skips the diverged step). Its checked range is a strict
|
||||
* superset of `from`'s, so `alternateFrom` present implies `resume.allowed` —
|
||||
* never a contradiction.
|
||||
*
|
||||
* Called at every divergence site (not only `record-and-heal` ones) so a
|
||||
* stale watermark from an earlier divergence never survives an unrelated
|
||||
* later one: a non-`record-and-heal` hint, or a `record-and-heal` hint whose
|
||||
* `resume` was not `allowed`, clears the field.
|
||||
* The alternate has two acceptance regimes by position:
|
||||
* - MID-PLAN (`failedIndex + 1 <= actions.length`): in range, needs no
|
||||
* watermark — session-independent, gated on the preflight alone.
|
||||
* - LAST STEP / EMPTY-TAIL (`failedIndex + 1 > actions.length`, one past the
|
||||
* plan's end): the range check accepts this ordinal ONLY when it matches a
|
||||
* stamped `pendingRecordAndHeal` watermark (`describeOutOfRangeResumeFrom`),
|
||||
* and that watermark can only be stamped on a LIVE session (both divergence
|
||||
* sites gate `stampPendingRecordAndHealWatermark` on `if (session)`). With
|
||||
* no session — a one-step `open` failure, or a session closed mid-replay —
|
||||
* the watermark can never be stamped, so `--from actions.length + 1` would
|
||||
* be rejected as out of range; advertising it would re-introduce the exact
|
||||
* text/structured mismatch #1262 fixed. So the empty-tail alternate
|
||||
* additionally requires `sessionExists`.
|
||||
*
|
||||
* Absent for `record-and-heal` (its `from` already IS `failedIndex + 1`) and
|
||||
* `state-repair` (no recorded-action alternate); absent when the diverged step
|
||||
* is a `runScript` (outputEnv producer) or inside runtime control flow, so the
|
||||
* alternate `--from failedIndex + 1` would be refused. The text renderer gates
|
||||
* the `N + 1` command on this field's PRESENCE rather than re-deriving
|
||||
* resumability, keeping text and the structured wire in agreement.
|
||||
*/
|
||||
function computeReplayResumeAlternateFrom(params: {
|
||||
failedIndex: number;
|
||||
actions: SessionAction[];
|
||||
repairHint: ReplayRepairHint;
|
||||
sessionExists: boolean;
|
||||
}): number | undefined {
|
||||
const { failedIndex, actions, repairHint, sessionExists } = params;
|
||||
if (repairHint !== 'caution' && repairHint !== 'manual') return undefined;
|
||||
const alternateFrom = failedIndex + 1;
|
||||
if (!evaluateReplayResumePreflight({ from: alternateFrom, actions }).allowed) return undefined;
|
||||
// Empty-tail: authorizable only via a watermark, which needs a live session.
|
||||
if (alternateFrom > actions.length && !sessionExists) return undefined;
|
||||
return alternateFrom;
|
||||
}
|
||||
|
||||
/**
|
||||
* ADR 0012 decision 6, R2/R3, extended per #1262: a `record-and-heal`
|
||||
* divergence's `resume.from` assumes the agent performs the diverged step
|
||||
* manually before continuing — nothing else enforces that, mid-plan or at
|
||||
* the plan's LAST step, so the watermark is stamped unconditionally
|
||||
* (position-independent) whenever `resume.allowed`.
|
||||
*
|
||||
* `caution` (identity-mismatch) and `manual` are different in kind:
|
||||
* `resume.from` stays at the failed step's own index `N` unconditionally (a
|
||||
* `--no-record` app-state fix re-runs the unchanged step, and #1262 requires
|
||||
* `N` never be made illegal for these hints), and — UNLIKE `record-and-heal`
|
||||
* — a mid-plan `--from N + 1` was ALREADY unconditionally legal (in range,
|
||||
* `<= actionCount`) and un-gated before #1262: these hints never mandate a
|
||||
* corrective action the way `record-and-heal` does, so an agent may
|
||||
* legitimately decide to skip the diverged step's execution entirely
|
||||
* (dropping it from a healed script) and continue. That pre-existing,
|
||||
* un-gated pattern must not regress.
|
||||
*
|
||||
* The ONE gap #1262 closes is the boundary case: when the diverged step IS
|
||||
* the plan's LAST step, `N + 1` is one past the plan's end — previously
|
||||
* ALWAYS out of range for `caution`/`manual` (dead end: `close` on the
|
||||
* not-yet-COMPLETE transaction discards a just-recorded corrective action).
|
||||
* Only THAT boundary ordinal is newly authorized here, and only when it is
|
||||
* independently preflight-safe (`evaluateReplayResumePreflight` on `N + 1`,
|
||||
* NOT the unrelated preflight verdict `resume.allowed` already carries for
|
||||
* `N` — a different ordinal). Authorizing it stamps the SAME watermark
|
||||
* `record-and-heal` uses, which — as an unavoidable side effect of sharing
|
||||
* the mechanism — also puts `describeUnperformedRecordAndHeal`'s
|
||||
* recorded-corrective-action guard in front of that specific `N + 1`
|
||||
* request: the empty-tail exception is new, so proof it was earned is
|
||||
* required, same as record-and-heal's requirement.
|
||||
*
|
||||
* Called at every divergence site (not only the eligible hints/positions) so
|
||||
* a stale watermark from an earlier divergence never survives an unrelated
|
||||
* later one: an ineligible hint, a mid-plan `caution`/`manual` divergence, or
|
||||
* a last-step `caution`/`manual` divergence whose `N + 1` target is not
|
||||
* itself preflight-safe, all clear the field.
|
||||
*/
|
||||
export function stampPendingRecordAndHealWatermark(params: {
|
||||
session: SessionState;
|
||||
resume: ReplayDivergenceResume;
|
||||
repairHint: ReplayRepairHint;
|
||||
failedIndex: number; // 1-based, the diverged step's own plan ordinal (N)
|
||||
actions: SessionAction[];
|
||||
}): void {
|
||||
const { session, resume, repairHint } = params;
|
||||
session.pendingRecordAndHeal =
|
||||
repairHint === 'record-and-heal' && resume.allowed
|
||||
? { expectedFrom: resume.from, actionsCountAtDivergence: session.actions.length }
|
||||
: undefined;
|
||||
const { session, resume, repairHint, failedIndex, actions } = params;
|
||||
session.pendingRecordAndHeal = computeRecordAndHealWatermark({
|
||||
resume,
|
||||
repairHint,
|
||||
failedIndex,
|
||||
actions,
|
||||
actionsCountAtDivergence: session.actions.length,
|
||||
});
|
||||
}
|
||||
|
||||
function computeRecordAndHealWatermark(params: {
|
||||
resume: ReplayDivergenceResume;
|
||||
repairHint: ReplayRepairHint;
|
||||
failedIndex: number;
|
||||
actions: SessionAction[];
|
||||
actionsCountAtDivergence: number;
|
||||
}): { expectedFrom: number; actionsCountAtDivergence: number } | undefined {
|
||||
const { resume, repairHint, failedIndex, actions, actionsCountAtDivergence } = params;
|
||||
if (repairHint === 'record-and-heal') {
|
||||
return resume.allowed ? { expectedFrom: resume.from, actionsCountAtDivergence } : undefined;
|
||||
}
|
||||
if (repairHint !== 'caution' && repairHint !== 'manual') return undefined;
|
||||
// #1262: only the LAST-step empty-tail alternate is newly authorized —
|
||||
// see the function doc comment for why mid-plan `N + 1` stays un-gated.
|
||||
if (failedIndex !== actions.length) return undefined;
|
||||
const expectedFrom = failedIndex + 1;
|
||||
const authorized = evaluateReplayResumePreflight({ from: expectedFrom, actions }).allowed;
|
||||
return authorized ? { expectedFrom, actionsCountAtDivergence } : undefined;
|
||||
}
|
||||
|
||||
@@ -32,7 +32,12 @@ export function readEffectiveReplayPlanDigestMetadata(
|
||||
|
||||
type ReplayEntryIndexResult = { ok: true; value: number } | { ok: false; response: DaemonResponse };
|
||||
|
||||
/** The session-side state that gates an EMPTY-TAIL resume (`--from actionCount + 1`). */
|
||||
/**
|
||||
* The session-side state that gates an EMPTY-TAIL resume (`--from actionCount
|
||||
* + 1`). Stamped for `record-and-heal`, and per #1262 also for
|
||||
* `caution`/`manual`'s record-and-heal-shaped alternate repair (their own
|
||||
* unshifted `resume.from` is unaffected by this watermark).
|
||||
*/
|
||||
export type PendingRecordAndHeal = { expectedFrom: number; actionsCountAtDivergence: number };
|
||||
|
||||
/**
|
||||
@@ -41,14 +46,19 @@ export type PendingRecordAndHeal = { expectedFrom: number; actionsCountAtDiverge
|
||||
*
|
||||
* `pendingRecordAndHeal`/`sessionActionsLength` gate the ONE ordinal beyond
|
||||
* the plan's end (`actionCount + 1`): ADR 0012 decision 6, R2's `record-and-heal`
|
||||
* repair resumes past the plan's LAST step once the agent performs the
|
||||
* diverged step manually, and that resume must execute zero device actions
|
||||
* before reaching the normal completion path. That allowance is scoped to
|
||||
* the EXACT session + target that actually produced it (`stampPendingRecordAndHealWatermark`,
|
||||
* `session-replay-resume.ts`), and only once a new action proves the
|
||||
* corrective press happened — never a blanket "one past the end is fine" for
|
||||
* any session, which would let an unrelated or blind `--from actionCount + 1`
|
||||
* silently skip the plan's tail and commit an unfinished repair.
|
||||
* repair — and, per #1262, `caution`/`manual`'s record-and-heal-SHAPED
|
||||
* alternate repair — resumes past the plan's LAST step once the agent
|
||||
* performs the diverged step's intent as a recorded action, and that resume
|
||||
* must execute zero device actions before reaching the normal completion
|
||||
* path. That allowance is scoped to the EXACT session + target that actually
|
||||
* produced it (`stampPendingRecordAndHealWatermark`, `session-replay-resume.ts`),
|
||||
* and only once a new action proves the corrective press happened — never a
|
||||
* blanket "one past the end is fine" for any session, which would let an
|
||||
* unrelated or blind `--from actionCount + 1` silently skip the plan's tail
|
||||
* and commit an unfinished repair. `caution`/`manual`'s OWN `resume.from`
|
||||
* (the failed step's own index, unshifted) stays legal unconditionally
|
||||
* regardless of this watermark — it is always `<= actionCount`, never the
|
||||
* one-past-the-end ordinal this gate concerns.
|
||||
*/
|
||||
export function resolveReplayEntryIndex(
|
||||
flags: CommandFlags | undefined,
|
||||
@@ -118,11 +128,12 @@ function validateReplayResumeRequest(params: {
|
||||
|
||||
/**
|
||||
* `actionCount + 1` (one past the plan's end) is a legal EMPTY-TAIL resume
|
||||
* ONLY when it matches THIS session's own `record-and-heal` divergence
|
||||
* watermark (`stampPendingRecordAndHealWatermark`, `session-replay-resume.ts`)
|
||||
* — never a blanket "one past the end is fine" for any session or repair
|
||||
* kind. Absent a matching watermark, `actionCount + 1` is exactly as
|
||||
* out-of-range as any other ordinal beyond the plan.
|
||||
* ONLY when it matches THIS session's own record-and-heal-shaped divergence
|
||||
* watermark (`stampPendingRecordAndHealWatermark`, `session-replay-resume.ts`
|
||||
* — stamped for `record-and-heal`, and per #1262 also for `caution`/`manual`'s
|
||||
* recorded-action alternate) — never a blanket "one past the end is fine" for
|
||||
* any session or repair kind. Absent a matching watermark, `actionCount + 1`
|
||||
* is exactly as out-of-range as any other ordinal beyond the plan.
|
||||
*/
|
||||
function describeOutOfRangeResumeFrom(params: {
|
||||
from: number;
|
||||
@@ -142,12 +153,18 @@ function describeOutOfRangeResumeFrom(params: {
|
||||
}
|
||||
|
||||
/**
|
||||
* A `from` matching a pending `record-and-heal` watermark — in-range
|
||||
* (mid-plan) or the empty-tail boundary the range check above authorizes —
|
||||
* requires proof the agent actually performed the diverged step: the
|
||||
* session's recorded action count must have grown since the divergence.
|
||||
* Without that proof, this would silently resume past an unrepaired step
|
||||
* instead of rejecting.
|
||||
* A `from` matching a pending record-and-heal-shaped watermark — in-range
|
||||
* (mid-plan, `record-and-heal` only) or the empty-tail boundary the range
|
||||
* check above authorizes (`record-and-heal`, or per #1262 also
|
||||
* `caution`/`manual`'s alternate repair, which is ONLY ever stamped at that
|
||||
* boundary — see `stampPendingRecordAndHealWatermark`,
|
||||
* `session-replay-resume.ts`) — requires proof the agent actually performed
|
||||
* the diverged step: the session's recorded action count must have grown
|
||||
* since the divergence. Without that proof, this would silently resume past
|
||||
* an unrepaired step instead of rejecting. `caution`/`manual`'s own
|
||||
* `resume.from` stays at the failed step unchanged and is never subject to
|
||||
* this check (it never matches `expectedFrom`, which only ever targets
|
||||
* `failedIndex + 1`), so the message below is intentionally hint-neutral.
|
||||
*/
|
||||
function describeUnperformedRecordAndHeal(params: {
|
||||
from: number;
|
||||
@@ -162,9 +179,9 @@ function describeUnperformedRecordAndHeal(params: {
|
||||
return undefined;
|
||||
}
|
||||
return (
|
||||
`replay --from ${from} continues a record-and-heal repair, but no corrective action has been ` +
|
||||
'recorded on this session since that divergence; press the correct control via a blessed @ref ' +
|
||||
"from the divergence's screen.refs (recorded, no --no-record) before resuming with " +
|
||||
`replay --from ${from} continues a record-and-heal-shaped repair, but no corrective action has ` +
|
||||
'been recorded on this session since that divergence; press the correct control via a blessed ' +
|
||||
"@ref from the divergence's screen.refs (recorded, no --no-record) before resuming with " +
|
||||
`--from ${from}.`
|
||||
);
|
||||
}
|
||||
|
||||
@@ -135,15 +135,24 @@ function buildTargetBindingDivergenceResponse(
|
||||
targetEvidence: recorded,
|
||||
capture: built.repairCapture,
|
||||
});
|
||||
// Fetched before the resume so its existence can gate the empty-tail
|
||||
// `alternateFrom` (the watermark stamped below needs a live session).
|
||||
const session = sessionStore.get(sessionName);
|
||||
const resume = buildReplayDivergenceResume({
|
||||
failedIndex: step,
|
||||
actions: planActions,
|
||||
planDigest,
|
||||
repairHint,
|
||||
sessionExists: session !== undefined,
|
||||
});
|
||||
const session = sessionStore.get(sessionName);
|
||||
if (session) {
|
||||
stampPendingRecordAndHealWatermark({ session, resume, repairHint });
|
||||
stampPendingRecordAndHealWatermark({
|
||||
session,
|
||||
resume,
|
||||
repairHint,
|
||||
failedIndex: step,
|
||||
actions: planActions,
|
||||
});
|
||||
sessionStore.set(sessionName, session);
|
||||
}
|
||||
|
||||
|
||||
+28
-9
@@ -347,15 +347,34 @@ export type SessionState = {
|
||||
*/
|
||||
repairSourcePath?: string;
|
||||
/**
|
||||
* ADR 0012 decision 6, R2/R3: set whenever a `record-and-heal` divergence's
|
||||
* `resume` reports `allowed: true` — its `from` (the failed step's index +
|
||||
* 1) assumes the agent performs the diverged step manually before
|
||||
* continuing, and nothing else enforces that. A later `--from` request that
|
||||
* matches `expectedFrom` while `session.actions.length` is still exactly
|
||||
* `actionsCountAtDivergence` (no new action recorded since) is rejected —
|
||||
* proof the corrective press never happened, so the resume would silently
|
||||
* skip the unrepaired step instead of healing it. Overwritten by the next
|
||||
* divergence (cleared to `undefined` for any non-`record-and-heal` hint),
|
||||
* ADR 0012 decision 6, R2/R3, extended per #1262: set whenever a
|
||||
* `record-and-heal` divergence's `resume` reports `allowed: true` — its
|
||||
* `from` (the failed step's index + 1) assumes the agent performs the
|
||||
* diverged step manually before continuing, and nothing else enforces
|
||||
* that. Position-independent for `record-and-heal` (mid-plan or the
|
||||
* plan's last step).
|
||||
*
|
||||
* Also set for `caution`/`manual`, whose OWN `resume.from` stays at the
|
||||
* failed step's index unchanged (never made illegal) — but ONLY when the
|
||||
* diverged step is the plan's LAST one: those hints have a legitimate
|
||||
* record-and-heal-SHAPED alternate repair targeting `failedIndex + 1`,
|
||||
* stamped when that target is independently preflight-safe
|
||||
* (`stampPendingRecordAndHealWatermark`, `session-replay-resume.ts`). A
|
||||
* MID-PLAN `caution`/`manual` `failedIndex + 1` was already unconditionally
|
||||
* legal (in range) and un-gated before #1262 — these hints never mandate a
|
||||
* corrective action the way `record-and-heal` does, so an agent may
|
||||
* legitimately skip the diverged step without repairing it — and stays
|
||||
* un-gated: this field is never set for a mid-plan `caution`/`manual`
|
||||
* divergence.
|
||||
*
|
||||
* A later `--from` request that matches `expectedFrom` while
|
||||
* `session.actions.length` is still exactly `actionsCountAtDivergence` (no
|
||||
* new action recorded since) is rejected — proof the corrective press
|
||||
* never happened, so the resume would silently skip the unrepaired step
|
||||
* instead of healing it. Overwritten by the next divergence (cleared to
|
||||
* `undefined` for any hint outside the eligible set, a mid-plan
|
||||
* `caution`/`manual` divergence, or a last-step `caution`/`manual`
|
||||
* divergence whose `failedIndex + 1` target is not itself preflight-safe),
|
||||
* and cleared once a `--from` request observes the action count having
|
||||
* grown, so it never fires against an unrelated later request.
|
||||
*/
|
||||
|
||||
@@ -1089,6 +1089,53 @@ test('MCP tool error is a ref-issuing result: isError, structuredContent, and pi
|
||||
assert.deepEqual(runCalls[1]?.input, { target: { kind: 'ref', ref: '@e5~s12' } });
|
||||
});
|
||||
|
||||
// --- #1262: a `caution` divergence's dual-path must reach a structured caller,
|
||||
// not only text. `resume.alternateFrom` rides the MCP structuredContent
|
||||
// projection (parity with a text caller, who reads the second `--from`
|
||||
// command), and the MCP text carries BOTH concrete commands. ---
|
||||
test("MCP projections carry a caution divergence's alternateFrom (structuredContent) and both --from commands (text)", async () => {
|
||||
const executor = createCommandToolExecutor({
|
||||
createClient: () => ({}) as AgentDeviceClient,
|
||||
runCommand: async (_client, name) => {
|
||||
if (name === 'replay') {
|
||||
throw new AppError('REPLAY_DIVERGENCE', 'Replay diverged at step 2 (click label="Save")', {
|
||||
divergence: {
|
||||
version: 1,
|
||||
kind: 'identity-mismatch',
|
||||
step: { index: 2, source: { path: '/tmp/flow.ad', line: 3 } },
|
||||
action: 'click label="Save"',
|
||||
cause: { code: 'IDENTITY_MISMATCH', message: 'resolved a different element' },
|
||||
screen: {
|
||||
state: 'available',
|
||||
refsGeneration: 4,
|
||||
refs: [{ ref: 'e1', role: 'button' }],
|
||||
},
|
||||
suggestions: [],
|
||||
suggestionCount: 0,
|
||||
resume: { allowed: true, from: 2, planDigest: 'deadbeef', alternateFrom: 3 },
|
||||
repairHint: 'caution',
|
||||
},
|
||||
});
|
||||
}
|
||||
return {};
|
||||
},
|
||||
});
|
||||
|
||||
const result = await executor.execute('replay', { positionals: ['/tmp/flow.ad'] });
|
||||
assert.equal(result.isError, true);
|
||||
const structured = result.structuredContent as {
|
||||
details?: { divergence?: { resume?: { from?: number; alternateFrom?: number } } };
|
||||
};
|
||||
// Parity: the structured caller gets BOTH ordinals, not just `from`.
|
||||
assert.equal(structured.details?.divergence?.resume?.from, 2);
|
||||
assert.equal(structured.details?.divergence?.resume?.alternateFrom, 3);
|
||||
// The MCP text carries both concrete commands, the second from alternateFrom.
|
||||
const text = result.content[0]?.text ?? '';
|
||||
assert.match(text, /Repair hint: caution/);
|
||||
assert.match(text, /if you fixed app state with --no-record actions: replay --from 2/);
|
||||
assert.match(text, /if you performed the step's intent as a recorded action: replay --from 3/);
|
||||
});
|
||||
|
||||
test('MCP tool error without a divergence never touches existing pins (merge-only)', async () => {
|
||||
const runCalls: Array<{ name: string; input: unknown }> = [];
|
||||
const executor = createCommandToolExecutor({
|
||||
|
||||
@@ -551,3 +551,131 @@ test('formatReplayDivergenceReport falls back to a generic non-resumable sentenc
|
||||
assert.doesNotMatch(report!, /replay --from/);
|
||||
assert.match(report!, /cannot currently be resumed automatically \(resume not yet supported\)/);
|
||||
});
|
||||
|
||||
// --- #1262: `caution`/`manual` are genuinely dual-path — the daemon cannot
|
||||
// know at divergence time whether the agent will fix app state (--no-record,
|
||||
// resuming at the unshifted `resume.from`, N) or perform the step's intent as
|
||||
// a recorded action (record-and-heal-shaped, resuming at N + 1). The `N + 1`
|
||||
// command is rendered IFF the wire carries `resume.alternateFrom` (the
|
||||
// daemon's own verdict that `--from N + 1` would be accepted) — the renderer
|
||||
// NEVER re-derives resumability, so text and structured wire never disagree. ---
|
||||
|
||||
test('formatReplayDivergenceReport embeds BOTH concrete resume commands for a caution divergence whose wire carries alternateFrom', async () => {
|
||||
const { formatReplayDivergenceReport } = await import('../divergence.ts');
|
||||
const report = formatReplayDivergenceReport({
|
||||
divergence: {
|
||||
version: 1,
|
||||
kind: 'identity-mismatch',
|
||||
step: { index: 2, source: { path: '/tmp/flow.ad', line: 3 } },
|
||||
action: 'click label="Save"',
|
||||
cause: { code: 'IDENTITY_MISMATCH', message: 'resolved a different element' },
|
||||
screen: { state: 'available', refsGeneration: 1, refs: [{ ref: 'e1', role: 'button' }] },
|
||||
suggestions: [],
|
||||
suggestionCount: 0,
|
||||
// caution's resume.from is UNSHIFTED (N); alternateFrom carries N + 1,
|
||||
// present only because the daemon proved `--from 3` would be accepted.
|
||||
resume: { allowed: true, from: 2, planDigest: 'deadbeef', alternateFrom: 3 },
|
||||
repairHint: 'caution',
|
||||
},
|
||||
});
|
||||
assert.ok(report);
|
||||
assert.match(report!, /Repair hint: caution — something already matches the recorded selector/);
|
||||
// The state-fix command uses N (resume.from itself, unshifted)...
|
||||
assert.match(
|
||||
report!,
|
||||
/if you fixed app state with --no-record actions: replay --from 2 --plan-digest deadbeef/,
|
||||
);
|
||||
// ...and the recorded-action command uses alternateFrom (N + 1) verbatim —
|
||||
// never re-derived from `from` on the client side.
|
||||
assert.match(
|
||||
report!,
|
||||
/if you performed the step's intent as a recorded action: replay --from 3 --plan-digest deadbeef\./,
|
||||
);
|
||||
});
|
||||
|
||||
test('formatReplayDivergenceReport embeds BOTH concrete resume commands for a manual divergence whose wire carries alternateFrom', async () => {
|
||||
const { formatReplayDivergenceReport } = await import('../divergence.ts');
|
||||
const report = formatReplayDivergenceReport({
|
||||
divergence: {
|
||||
version: 1,
|
||||
kind: 'action-failure',
|
||||
step: { index: 5, source: { path: '/tmp/flow.ad', line: 9 } },
|
||||
action: 'click label="Confirm"',
|
||||
cause: { code: 'COMMAND_FAILED', message: 'not hittable' },
|
||||
screen: { state: 'available', refsGeneration: 1, refs: [{ ref: 'e1', role: 'button' }] },
|
||||
suggestions: [],
|
||||
suggestionCount: 0,
|
||||
resume: { allowed: true, from: 5, planDigest: 'cafef00d', alternateFrom: 6 },
|
||||
repairHint: 'manual',
|
||||
},
|
||||
});
|
||||
assert.ok(report);
|
||||
assert.match(report!, /Repair hint: manual — no safe automated repair could be proven/);
|
||||
assert.match(
|
||||
report!,
|
||||
/if you fixed app state with --no-record actions: replay --from 5 --plan-digest cafef00d/,
|
||||
);
|
||||
assert.match(
|
||||
report!,
|
||||
/if you performed the step's intent as a recorded action: replay --from 6 --plan-digest cafef00d\./,
|
||||
);
|
||||
});
|
||||
|
||||
test('formatReplayDivergenceReport renders ONLY the state-fix command for a caution divergence WITHOUT alternateFrom (the diverged step is not skip-safe)', async () => {
|
||||
const { formatReplayDivergenceReport } = await import('../divergence.ts');
|
||||
// resume.allowed is true (resuming AT N is fine), but the diverged step N
|
||||
// is a runScript/control-flow action, so `--from N + 1` would be refused —
|
||||
// the daemon omits alternateFrom, and the text must NOT offer `--from N + 1`.
|
||||
const report = formatReplayDivergenceReport({
|
||||
divergence: {
|
||||
version: 1,
|
||||
kind: 'identity-mismatch',
|
||||
step: { index: 2, source: { path: '/tmp/flow.ad', line: 3 } },
|
||||
action: 'click label="Save"',
|
||||
cause: { code: 'IDENTITY_MISMATCH', message: 'resolved a different element' },
|
||||
screen: { state: 'available', refsGeneration: 1, refs: [{ ref: 'e1', role: 'button' }] },
|
||||
suggestions: [],
|
||||
suggestionCount: 0,
|
||||
resume: { allowed: true, from: 2, planDigest: 'deadbeef' },
|
||||
repairHint: 'caution',
|
||||
},
|
||||
});
|
||||
assert.ok(report);
|
||||
assert.match(report!, /Repair hint: caution — something already matches the recorded selector/);
|
||||
// The state-fix command IS rendered (resuming AT N stays allowed)...
|
||||
assert.match(
|
||||
report!,
|
||||
/if you fixed app state with --no-record actions: replay --from 2 --plan-digest deadbeef\./,
|
||||
);
|
||||
// ...but the recorded-action alternate is NOT offered — no alternateFrom on
|
||||
// the wire means the renderer must never advertise `--from 3`.
|
||||
assert.doesNotMatch(report!, /if you performed the step's intent as a recorded action/);
|
||||
assert.doesNotMatch(report!, /--from 3/);
|
||||
});
|
||||
|
||||
test('formatReplayDivergenceReport renders neither caution command when resume is NOT allowed', async () => {
|
||||
const { formatReplayDivergenceReport } = await import('../divergence.ts');
|
||||
const report = formatReplayDivergenceReport({
|
||||
divergence: {
|
||||
version: 1,
|
||||
kind: 'identity-mismatch',
|
||||
step: { index: 2, source: { path: '/tmp/flow.ad', line: 3 } },
|
||||
action: 'click label="Save"',
|
||||
cause: { code: 'IDENTITY_MISMATCH', message: 'resolved a different element' },
|
||||
screen: { state: 'available', refsGeneration: 1, refs: [{ ref: 'e1', role: 'button' }] },
|
||||
suggestions: [],
|
||||
suggestionCount: 0,
|
||||
resume: {
|
||||
allowed: false,
|
||||
from: 2,
|
||||
planDigest: 'deadbeef',
|
||||
reason: 'step 1 is inside runtime control flow (retry); skipping it cannot be proven safe.',
|
||||
},
|
||||
repairHint: 'caution',
|
||||
},
|
||||
});
|
||||
assert.ok(report);
|
||||
assert.match(report!, /Repair hint: caution — something already matches the recorded selector/);
|
||||
assert.doesNotMatch(report!, /replay --from/);
|
||||
assert.match(report!, /cannot currently be resumed automatically/);
|
||||
});
|
||||
|
||||
+120
-32
@@ -108,23 +108,31 @@ export type ReplayDivergenceSuggestion = {
|
||||
};
|
||||
|
||||
/**
|
||||
* ADR 0012 decision 4 / migration step 5, refined by decision 6, R2. `from`
|
||||
* is the 1-based plan ordinal the caller should actually pass to `--from` —
|
||||
* NOT always the failed step's own index. It depends on the divergence's
|
||||
* `repairHint` (`ReplayRepairHint`, below): for `record-and-heal`, the agent
|
||||
* performs the diverged step manually before this report is acted on, so
|
||||
* `from` is the failed step's index **+ 1** (resuming AT the failed step
|
||||
* would re-diverge on the exact step just repaired); for every other hint
|
||||
* (`state-repair`, `caution`, `manual`, and a plain `action-failure`), `from`
|
||||
* equals the failed step's index unchanged. When the diverged step was the
|
||||
* ADR 0012 decision 4 / migration step 5, refined by decision 6, R2, and
|
||||
* #1262. `from` is the 1-based plan ordinal the caller should actually pass
|
||||
* to `--from` — NOT always the failed step's own index. It depends on the
|
||||
* divergence's `repairHint` (`ReplayRepairHint`, below): for
|
||||
* `record-and-heal`, the agent performs the diverged step manually before
|
||||
* this report is acted on, so `from` is the failed step's index **+ 1**
|
||||
* (resuming AT the failed step would re-diverge on the exact step just
|
||||
* repaired); for every other hint (`state-repair`, `caution`, `manual`, and a
|
||||
* plain `action-failure`), `from` equals the failed step's index unchanged —
|
||||
* and, per #1262, this is UNCONDITIONAL for `caution`/`manual`: `from`
|
||||
* (`N`) is never made illegal for these hints, since a `--no-record`
|
||||
* app-state fix re-runs the unchanged step. When the diverged step was the
|
||||
* plan's LAST step, a `record-and-heal` `from` can equal `actions.length + 1`
|
||||
* — a legal EMPTY-TAIL resume (there is nothing left to replay; the runtime
|
||||
* executes zero steps and reaches the normal end-of-plan completion path),
|
||||
* not an out-of-range error. That one-past-the-end ordinal is authorized
|
||||
* ONLY for the exact session + target that produced it (the daemon tracks a
|
||||
* per-session watermark, `session.pendingRecordAndHeal`) and only once a new
|
||||
* action proves the corrective press actually happened — never a general
|
||||
* "one past the end is fine" for any session.
|
||||
* not an out-of-range error. `caution`/`manual` are genuinely dual-path: they
|
||||
* ALSO have a record-and-heal-shaped alternate repair (the agent performs the
|
||||
* diverged step's intent as a recorded action instead), resumed at a DIFFERENT
|
||||
* ordinal than their own reported `from` — carried as `alternateFrom` (below),
|
||||
* never as `resume.from` itself. At the last step that alternate rides the
|
||||
* SAME empty-tail authorization `record-and-heal` uses. Every one-past-the-end
|
||||
* ordinal is authorized ONLY for the exact session + target that produced it
|
||||
* (the daemon tracks a per-session watermark, `session.pendingRecordAndHeal`)
|
||||
* and only once a new action proves the corrective press actually happened —
|
||||
* never a general "one past the end is fine" for any session.
|
||||
*
|
||||
* `planDigest` is the SHA-256 digest of the canonical fully expanded plan
|
||||
* (`computeReplayPlanDigest`) that produced this report — always present.
|
||||
@@ -137,6 +145,23 @@ export type ReplayDivergenceSuggestion = {
|
||||
* `allowed` is `false` the text guidance never renders a `--from` command,
|
||||
* surfacing `reason` instead.
|
||||
*
|
||||
* `alternateFrom` (#1262) is the `caution`/`manual` dual-path's SECOND
|
||||
* ordinal: their `from` (`N`) is the app-state-fix continuation, and
|
||||
* `alternateFrom` (`N + 1`) is the record-and-heal-shaped alternate — the
|
||||
* agent performs the diverged step's intent as a recorded action, then
|
||||
* resumes PAST it. It is present ONLY when a `--from N + 1` request for THIS
|
||||
* divergence would actually be accepted by the daemon: the daemon computes it
|
||||
* as `evaluateReplayResumePreflight({ from: N + 1, actions }).allowed` (which
|
||||
* additionally requires the DIVERGED step `N` itself to be skip-safe — a
|
||||
* strict superset of `from`'s own preflight, so its presence never
|
||||
* contradicts `allowed`). Absent for every other hint (`record-and-heal`'s
|
||||
* `from` already IS the `N + 1` continuation; `state-repair` has no
|
||||
* recorded-action alternate), and absent whenever the alternate is not
|
||||
* resumable (step `N` is a `runScript`/control-flow action). The text
|
||||
* guidance (`buildRepairHintGuidance`, below) renders the `N + 1` command iff
|
||||
* this field is present — it never re-derives resumability, so text and the
|
||||
* structured wire never disagree on the advertised next command.
|
||||
*
|
||||
* `repairSessionHeld` is decision 6, R7's repair-transaction liveness signal
|
||||
* (C1): set `true` by the daemon on ANY divergence from a repair-armed
|
||||
* (`--save-script`) replay — independent of `allowed`, which only reports
|
||||
@@ -146,7 +171,13 @@ export type ReplayDivergenceSuggestion = {
|
||||
* plain, non-repair divergence, which gets no keep-alive.
|
||||
*/
|
||||
export type ReplayDivergenceResume =
|
||||
| { allowed: true; from: number; planDigest: string; repairSessionHeld?: true }
|
||||
| {
|
||||
allowed: true;
|
||||
from: number;
|
||||
planDigest: string;
|
||||
alternateFrom?: number;
|
||||
repairSessionHeld?: true;
|
||||
}
|
||||
| { allowed: false; from: number; planDigest: string; reason: string; repairSessionHeld?: true };
|
||||
|
||||
export type ReplayDivergenceOverflow = {
|
||||
@@ -370,18 +401,27 @@ export function formatReplayDivergenceReport(
|
||||
}
|
||||
|
||||
/**
|
||||
* ADR 0012 decision 6: the repair-routing hint rendered on every text
|
||||
* surface (CLI, MCP text, `test` failures) — the same field that rides
|
||||
* `structuredContent`/JSON, so a text-only caller still learns which repair
|
||||
* sub-flow applies. `record-and-heal`/`state-repair` guidance embeds the
|
||||
* CONCRETE `resume.from`/`planDigest` values (computed by
|
||||
* ADR 0012 decision 6, extended per #1262: the repair-routing hint rendered
|
||||
* on every text surface (CLI, MCP text, `test` failures) — the same field
|
||||
* that rides `structuredContent`/JSON, so a text-only caller still learns
|
||||
* which repair sub-flow applies. `record-and-heal`/`state-repair` guidance
|
||||
* embeds the CONCRETE `resume.from`/`planDigest` values (computed by
|
||||
* `buildReplayDivergenceResume`, decision 6 R2) when `resume.allowed` is
|
||||
* true, so a text-only or JSON/MCP-first caller reads the identical next
|
||||
* command instead of deriving it. When `resume.allowed` is false (a skipped
|
||||
* step crosses runtime control flow or would produce unavailable
|
||||
* `outputEnv`), a resume command is never rendered — a text caller must not
|
||||
* be told to run a `--from` a structured caller would be refused — and the
|
||||
* reported `reason` is surfaced instead.
|
||||
* command instead of deriving it. `caution`/`manual` are genuinely dual-path
|
||||
* (the daemon cannot know at divergence time which repair applies), so their
|
||||
* guidance embeds `--from resume.from` (`N`, unshifted — a `--no-record`
|
||||
* app-state fix) whenever `resume.allowed` is true, AND `--from
|
||||
* resume.alternateFrom` (`N + 1`, a record-and-heal-shaped recorded corrective
|
||||
* action) IFF the wire carries `alternateFrom` — the daemon's own verdict that
|
||||
* a `--from N + 1` request would be accepted (`computeReplayResumeAlternateFrom`,
|
||||
* `session-replay-resume.ts`). The renderer gates the second command on that
|
||||
* field's PRESENCE and never re-derives resumability, so text and the
|
||||
* structured wire never disagree on the advertised next command. When
|
||||
* `resume.allowed` is false (a skipped step crosses runtime control flow or
|
||||
* would produce unavailable `outputEnv`), a resume command is never rendered
|
||||
* for ANY hint — a text caller must not be told to run a `--from` a structured
|
||||
* caller would be refused — and the reported `reason` is surfaced instead.
|
||||
*/
|
||||
function divergenceRepairHintLine(repairHint: unknown, resume: unknown): string[] {
|
||||
if (typeof repairHint !== 'string') return [];
|
||||
@@ -390,7 +430,7 @@ function divergenceRepairHintLine(repairHint: unknown, resume: unknown): string[
|
||||
}
|
||||
|
||||
type ResumeGuidance =
|
||||
| { allowed: true; command: string }
|
||||
| { allowed: true; from: number; planDigest: string; alternateFrom: number | undefined }
|
||||
| { allowed: false; reason: string | undefined };
|
||||
|
||||
/** Reads the parts of `resume` the repair-hint guidance needs; `undefined` when the shape is unreadable. */
|
||||
@@ -403,11 +443,23 @@ function readResumeGuidance(resume: unknown): ResumeGuidance | undefined {
|
||||
reason: typeof record.reason === 'string' ? record.reason : undefined,
|
||||
};
|
||||
}
|
||||
const { from, planDigest } = record;
|
||||
const { from, planDigest, alternateFrom } = record;
|
||||
if (typeof from !== 'number' || typeof planDigest !== 'string' || planDigest.length === 0) {
|
||||
return undefined;
|
||||
}
|
||||
return { allowed: true, command: `replay --from ${from} --plan-digest ${planDigest}` };
|
||||
return {
|
||||
allowed: true,
|
||||
from,
|
||||
planDigest,
|
||||
// Rendered VERBATIM from the wire; the daemon already proved a `--from
|
||||
// alternateFrom` request would be accepted (#1262). The renderer must NOT
|
||||
// re-derive it — that is precisely the bug the wire field fixed.
|
||||
alternateFrom: typeof alternateFrom === 'number' ? alternateFrom : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
function formatResumeCommand(from: number, planDigest: string): string {
|
||||
return `replay --from ${from} --plan-digest ${planDigest}`;
|
||||
}
|
||||
|
||||
function buildRepairHintGuidance(repairHint: string, resume: unknown): string | undefined {
|
||||
@@ -415,21 +467,57 @@ function buildRepairHintGuidance(repairHint: string, resume: unknown): string |
|
||||
switch (repairHint) {
|
||||
case 'record-and-heal':
|
||||
return guidance?.allowed
|
||||
? `press the correct control via a blessed @ref from screen.refs (recorded), then ${guidance.command}.`
|
||||
? `press the correct control via a blessed @ref from screen.refs (recorded), then ${formatResumeCommand(guidance.from, guidance.planDigest)}.`
|
||||
: `press the correct control via a blessed @ref from screen.refs (recorded). ${resumeUnavailableSentence(guidance)}`;
|
||||
case 'state-repair':
|
||||
return guidance?.allowed
|
||||
? `fix app state with --no-record actions, then ${guidance.command} to re-run it.`
|
||||
? `fix app state with --no-record actions, then ${formatResumeCommand(guidance.from, guidance.planDigest)} to re-run it.`
|
||||
: `fix app state with --no-record actions. ${resumeUnavailableSentence(guidance)}`;
|
||||
case 'caution':
|
||||
return 'something already matches the recorded selector; a blind re-press may repeat the mistake.';
|
||||
return buildDualPathRepairHintGuidance({
|
||||
lead: 'something already matches the recorded selector; a blind re-press may repeat the mistake.',
|
||||
guidance,
|
||||
});
|
||||
case 'manual':
|
||||
return 'no safe automated repair could be proven; inspect the screen and repair by hand.';
|
||||
return buildDualPathRepairHintGuidance({
|
||||
lead: 'no safe automated repair could be proven; inspect the screen and repair by hand.',
|
||||
guidance,
|
||||
});
|
||||
default:
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* `caution`/`manual` guidance (#1262): the daemon cannot know at divergence
|
||||
* time which of two repairs applies. Always offers `--from N` (an app-state
|
||||
* fix via `--no-record` actions, then re-run the unchanged step; `resume.from`
|
||||
* itself, never shifted for these hints — authorized by `resume.allowed`).
|
||||
* ADDITIONALLY offers `--from alternateFrom` (a record-and-heal-shaped
|
||||
* recorded corrective action, resuming PAST the diverged step) IFF the wire
|
||||
* carries `resume.alternateFrom` — the daemon has already proven such a
|
||||
* request would be accepted (`computeReplayResumeAlternateFrom`,
|
||||
* `session-replay-resume.ts`). The renderer gates on the field's PRESENCE and
|
||||
* never re-derives resumability, so it can never advertise a `--from` the
|
||||
* daemon would then refuse (the parity bug #1262's review flagged: `N + 1` is
|
||||
* unsafe exactly when the diverged step is a `runScript`/control-flow action,
|
||||
* which `resume.allowed` for `N` does not detect). When the alternate is
|
||||
* absent, only the state-fix command is shown. Neither command is rendered
|
||||
* when `resume.allowed` is false — `N` itself is not resumable, so its
|
||||
* alternate is moot.
|
||||
*/
|
||||
function buildDualPathRepairHintGuidance(params: {
|
||||
lead: string;
|
||||
guidance: ResumeGuidance | undefined;
|
||||
}): string {
|
||||
const { lead, guidance } = params;
|
||||
if (!guidance?.allowed) return `${lead} ${resumeUnavailableSentence(guidance)}`;
|
||||
const stateFixClause = `if you fixed app state with --no-record actions: ${formatResumeCommand(guidance.from, guidance.planDigest)}`;
|
||||
if (guidance.alternateFrom === undefined) return `${lead} ${stateFixClause}.`;
|
||||
const recordedActionClause = `if you performed the step's intent as a recorded action: ${formatResumeCommand(guidance.alternateFrom, guidance.planDigest)}`;
|
||||
return `${lead} ${stateFixClause}; ${recordedActionClause}.`;
|
||||
}
|
||||
|
||||
/** Never renders a `--from` command — only reached when `resume.allowed` is false (or unreadable). */
|
||||
function resumeUnavailableSentence(guidance: ResumeGuidance | undefined): string {
|
||||
if (guidance && !guidance.allowed && guidance.reason) {
|
||||
|
||||
Reference in New Issue
Block a user