fix: reconcile maestro replay with latest main

thymikee · thymikee · commit ead2022eda95 · 2026-07-15T07:45:11.000+02:00
diff --git a/src/daemon/handlers/__tests__/session-replay-resume.test.ts b/src/daemon/handlers/__tests__/session-replay-resume.test.ts
@@ -2,7 +2,6 @@ import assert from 'node:assert/strict';
 import { test } from 'vitest';
 import {
   buildReplayDivergenceResume,
-  evaluateReplayResumePreflight,
   stampPendingRecordAndHealWatermark,
 } from '../session-replay-resume.ts';
 import type { SessionAction, SessionState } from '../../types.ts';
@@ -12,29 +11,6 @@ function action(overrides: Partial<SessionAction> = {}): SessionAction {
   return { ts: 0, command: 'click', positionals: ['label="Save"'], flags: {}, ...overrides };
 }
 
-test('generic .ad replay allows resuming from the first action', () => {
-  const actions: SessionAction[] = [action({ command: 'back', positionals: [] }), action()];
-  assert.deepEqual(evaluateReplayResumePreflight({ from: 1, actions }), { allowed: true });
-});
-
-test('generic .ad replay allows resuming after earlier actions', () => {
-  const actions: SessionAction[] = [
-    action({ command: 'open', positionals: ['Demo'] }),
-    action({ command: 'click', positionals: ['label="Continue"'] }),
-    action({ command: 'click', positionals: ['label="Save"'] }),
-  ];
-  assert.deepEqual(evaluateReplayResumePreflight({ from: 3, actions }), { allowed: true });
-});
-
-test('repeated generic .ad action lines occupy distinct plan indices', () => {
-  const actions: SessionAction[] = [
-    action({ command: 'click', positionals: ['label="Item"'] }),
-    action({ command: 'click', positionals: ['label="Item"'] }),
-    action({ command: 'click', positionals: ['label="Item"'] }),
-  ];
-  assert.deepEqual(evaluateReplayResumePreflight({ from: 3, actions }), { allowed: true });
-});
-
 test('buildReplayDivergenceResume reports a resumable generic .ad failure', () => {
   const actions: SessionAction[] = [
     action({ command: 'open', positionals: ['Demo'] }),
@@ -91,13 +67,11 @@ test('buildReplayDivergenceResume with repairHint record-and-heal on the LAST pl
 });
 // --- 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). ---
+// ONLY when a `--from failedIndex + 1` request would actually be accepted.
+// Generic `.ad` plans contain no runtime variable producers or control
+// wrappers, so only the empty-tail session requirement can block it. ---
 
-test('buildReplayDivergenceResume: caution mid-plan (skip-safe diverged step) carries alternateFrom = failedIndex + 1', () => {
+test('buildReplayDivergenceResume: caution mid-plan carries alternateFrom = failedIndex + 1', () => {
   const actions: SessionAction[] = [
     action({ command: 'open' }),
     action({ command: 'click' }),
@@ -116,7 +90,7 @@ test('buildReplayDivergenceResume: caution mid-plan (skip-safe diverged step) ca
   assert.equal(resume.alternateFrom, 3);
 });
 
-test('buildReplayDivergenceResume: manual last-step (skip-safe diverged step) carries alternateFrom = actions.length + 1', () => {
+test('buildReplayDivergenceResume: manual last-step carries alternateFrom = actions.length + 1', () => {
   const actions: SessionAction[] = [action({ command: 'open' }), action({ command: 'click' })];
   const resume = buildReplayDivergenceResume({
     failedIndex: 2,
@@ -131,44 +105,6 @@ test('buildReplayDivergenceResume: manual last-step (skip-safe diverged step) ca
   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: record-and-heal and state-repair never carry alternateFrom (no separate recorded-action alternate)', () => {
   const actions: SessionAction[] = [
     action({ command: 'open' }),
@@ -381,34 +317,6 @@ test('stampPendingRecordAndHealWatermark does NOT stamp for a MID-PLAN caution/m
   }
 });
 
-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({
diff --git a/src/daemon/handlers/interaction-runtime.ts b/src/daemon/handlers/interaction-runtime.ts
@@ -77,7 +77,7 @@ function createInteractionBackend(
       (await dispatchGestureViewport(
         session.device,
         params.contextFromFlags(req.flags, session.appBundleId, session.trace?.outPath),
-      ),
+      )),
     tap: async (_context, point): Promise<BackendActionResult> => {
       // ADR 0014 side-effect seam: the point is resolved; expire the ref frame
       // synchronously before dispatching so a later step cannot reuse it.
diff --git a/src/daemon/handlers/session-replay-divergence.ts b/src/daemon/handlers/session-replay-divergence.ts
@@ -289,7 +289,7 @@ export async function captureDivergenceObservation(params: {
  * produce a narrowed divergence `screen`. Only the interactive-only policy is
  * carried, since it governs whether static-text suggestion targets survive.
  */
-function divergenceCaptureFlags(action: SessionAction): CommandFlags {
+function divergenceCaptureFlags(action: ReplayReportAction): CommandFlags {
   return { snapshotInteractiveOnly: divergenceCaptureInteractiveOnly(action) };
 }
 
diff --git a/src/daemon/handlers/session-replay-resume.ts b/src/daemon/handlers/session-replay-resume.ts
@@ -1,17 +1,6 @@
 import type { SessionAction, SessionState } from '../types.ts';
 import type { ReplayDivergenceResume, ReplayRepairHint } from '../../replay/divergence.ts';
 
-/** Generic `.ad` actions have no runtime variable producers or control wrappers. */
-export type ReplayResumePreflightResult = { allowed: true } | { allowed: false; reason: string };
-
-export function evaluateReplayResumePreflight(params: {
-  from: number;
-  actions: SessionAction[];
-}): ReplayResumePreflightResult {
-  void params;
-  return { allowed: true };
-}
-
 /**
  * Builds the `resume` object attached to every divergence report. `from` is
  * the ordinal the agent should actually pass to `--from`, not merely the
@@ -28,12 +17,10 @@ export function evaluateReplayResumePreflight(params: {
  * sites resolve it from the plan they are actively executing), so the shifted
  * `from` is at most `actions.length + 1` — never further out of range. That
  * boundary case (`record-and-heal` diverged on the plan's LAST step) is a
- * legal EMPTY-TAIL resume, not an error: `evaluateReplayResumePreflight`
- * already proves it safe (it only checks whether the SKIPPED range 1..from-1
- * is safe to skip; there is no `from`-th step to reject), and the runtime
- * loop (`runReplayScriptFile`) executes zero steps and reaches the normal
+ * legal EMPTY-TAIL resume, not an error: the runtime loop
+ * (`runReplayScriptFile`) executes zero steps and reaches the normal
  * end-of-plan completion path, correctly flipping a repair transaction
- * COMPLETE. Rejecting it here would send the agent to `close` instead —
+ * COMPLETE. Rejecting it would send the agent to `close` instead —
  * which discards the just-recorded corrective action, since commit is gated
  * on that same COMPLETE flag.
  *
@@ -53,8 +40,6 @@ export function buildReplayDivergenceResume(params: {
 }): ReplayDivergenceResume {
   const { failedIndex, actions, planDigest, repairHint, sessionExists } = params;
   const from = repairHint === 'record-and-heal' ? failedIndex + 1 : failedIndex;
-  const preflight = evaluateReplayResumePreflight({ from, actions });
-  if (!preflight.allowed) return { allowed: false, from, planDigest, reason: preflight.reason };
   const alternateFrom = computeReplayResumeAlternateFrom({
     failedIndex,
     actions,
@@ -71,18 +56,13 @@ export function buildReplayDivergenceResume(params: {
 
 /**
  * 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.
+ * ordinal, the record-and-heal-shaped alternate (`failedIndex + 1`). Generic
+ * `.ad` plans contain neither runtime variable producers nor control wrappers,
+ * so every in-range ordinal is resumable.
  *
  * 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.
+ *    watermark and is session-independent.
  *  - 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`),
@@ -95,10 +75,8 @@ export function buildReplayDivergenceResume(params: {
  *    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
+ * `state-repair` (no recorded-action alternate). 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: {
@@ -110,7 +88,6 @@ function computeReplayResumeAlternateFrom(params: {
   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;
@@ -138,10 +115,8 @@ function computeReplayResumeAlternateFrom(params: {
  * 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
+ * Only THAT boundary ordinal is newly authorized here. 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`
@@ -150,9 +125,8 @@ function computeReplayResumeAlternateFrom(params: {
  *
  * 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.
+ * later one: an ineligible hint or a mid-plan `caution`/`manual` divergence
+ * clears the field.
  */
 export function stampPendingRecordAndHealWatermark(params: {
   session: SessionState;
@@ -187,6 +161,5 @@ function computeRecordAndHealWatermark(params: {
   // 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;
+  return { expectedFrom, actionsCountAtDivergence };
 }
diff --git a/src/daemon/handlers/session-replay-runtime-plan.ts b/src/daemon/handlers/session-replay-runtime-plan.ts
@@ -1,8 +1,7 @@
 import type { CommandFlags } from '../../core/dispatch.ts';
 import type { ReplayPlanDigestMetadata } from '../../replay/plan-digest.ts';
 import type { ReplayScriptMetadata } from '../../replay/script.ts';
-import type { DaemonResponse, SessionAction } from '../types.ts';
-import { evaluateReplayResumePreflight } from './session-replay-resume.ts';
+import type { DaemonResponse } from '../types.ts';
 import { errorResponse } from './response.ts';
 
 export function buildReplayMetadataFlags(
@@ -64,7 +63,6 @@ export function resolveReplayEntryIndex(
   flags: CommandFlags | undefined,
   actionCount: number,
   planDigest: string,
-  actions: SessionAction[],
   pendingRecordAndHeal: PendingRecordAndHeal | undefined,
   sessionActionsLength: number,
 ): ReplayEntryIndexResult {
@@ -81,7 +79,6 @@ export function resolveReplayEntryIndex(
     digest,
     planDigest,
     actionCount,
-    actions,
     pendingRecordAndHeal,
     sessionActionsLength,
   });
@@ -100,24 +97,15 @@ function validateReplayResumeRequest(params: {
   digest: string;
   planDigest: string;
   actionCount: number;
-  actions: SessionAction[];
   pendingRecordAndHeal: PendingRecordAndHeal | undefined;
   sessionActionsLength: number;
 }): string | undefined {
-  const {
-    from,
-    digest,
-    planDigest,
-    actionCount,
-    actions,
-    pendingRecordAndHeal,
-    sessionActionsLength,
-  } = params;
+  const { from, digest, planDigest, actionCount, pendingRecordAndHeal, sessionActionsLength } =
+    params;
   const checks: ReplayResumeCheck[] = [
     () => describeOutOfRangeResumeFrom({ from, actionCount, pendingRecordAndHeal }),
     () => describeUnperformedRecordAndHeal({ from, pendingRecordAndHeal, sessionActionsLength }),
     () => describeStaleResumeDigest(digest, planDigest),
-    () => describeUnsafeResumePreflight(from, actions),
   ];
   for (const check of checks) {
     const message = check();
@@ -194,8 +182,3 @@ function describeStaleResumeDigest(digest: string, planDigest: string): string |
     'replay to get a new digest.'
   );
 }
-
-function describeUnsafeResumePreflight(from: number, actions: SessionAction[]): string | undefined {
-  const preflight = evaluateReplayResumePreflight({ from, actions });
-  return preflight.allowed ? undefined : `replay --from ${from} cannot resume: ${preflight.reason}`;
-}
diff --git a/src/daemon/handlers/session-replay-runtime.ts b/src/daemon/handlers/session-replay-runtime.ts
@@ -206,7 +206,6 @@ export async function runReplayScriptFile(params: {
       req.flags,
       actions.length,
       planDigest,
-      actions,
       preEntrySession?.pendingRecordAndHeal,
       preEntrySession?.actions.length ?? 0,
     );
diff --git a/src/daemon/handlers/session-replay-target-verification.ts b/src/daemon/handlers/session-replay-target-verification.ts
@@ -174,10 +174,9 @@ function buildTargetBindingDivergenceResponse(
     // executed — resuming AT `step` re-runs exactly the action that did not
     // send (unless `repairHint` is `record-and-heal`, in which case the agent
     // performs it manually and `buildReplayDivergenceResume` targets `step +
-    // 1` instead). `buildReplayDivergenceResume` runs the same skip-safety
-    // preflight as an action-failure divergence (allowed unless a skipped
-    // step produces outputEnv or the range crosses runtime control flow).
-    // This is the only resume site for target-binding divergences.
+    // 1` instead). Generic `.ad` actions have no runtime variable producers
+    // or control wrappers, so the reported ordinal is resumable. This is the
+    // only resume site for target-binding divergences.
     resume,
     repairHint,
     targetBinding,
diff --git a/src/daemon/types.ts b/src/daemon/types.ts
@@ -409,9 +409,9 @@ export type SessionState = {
    * 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
+   * stamped by `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
@@ -423,9 +423,8 @@ export type SessionState = {
    * 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),
+   * `undefined` for any hint outside the eligible set or a mid-plan
+   * `caution`/`manual` divergence),
    * and cleared once a `--from` request observes the action count having
    * grown, so it never fires against an unrelated later request.
    */
diff --git a/src/replay/divergence.ts b/src/replay/divergence.ts

Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,7 @@ export async function captureDivergenceObservation(params: {`
`289`	`289`	* produce a narrowed divergence `screen`. Only the interactive-only policy is
`290`	`290`	`* carried, since it governs whether static-text suggestion targets survive.`
`291`	`291`	`*/`
`292`		`-function divergenceCaptureFlags(action: SessionAction): CommandFlags {`
	`292`	`+function divergenceCaptureFlags(action: ReplayReportAction): CommandFlags {`
`293`	`293`	`return { snapshotInteractiveOnly: divergenceCaptureInteractiveOnly(action) };`
`294`	`294`	`}`
`295`	`295`