From 89695b21a17537e03b963818f318df2924847af4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 12 Jul 2026 12:05:54 +0200 Subject: [PATCH 1/3] fix: wrap replay selector-miss as REPLAY_DIVERGENCE (repair-loop regression) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dispatch failure thrown by a replay action (e.g. a selector-miss during press/click/fill/longpress) escaped runReplayScriptFile's `if (!response.ok)` divergence wrapping and fell through to the outer catch, returning a bare COMMAND_FAILED with the legacy diagnostics shape instead of the ADR 0012 REPLAY_DIVERGENCE report (no resume, no screen refs, no suggestions) — breaking the interactive repair loop for the common drift case. Fix at the invocation layer: invokeReplayAction (the single choke point every replay action dispatch funnels through — the top-level loop, retry blocks, and nested Maestro runFlow invocations) now normalizes a thrown AppError into a `{ok:false}` response before returning, matching DaemonInvokeFn's own never-reject contract. This is more localized than wrapping the per-action loop body in session-replay-runtime.ts and additionally closes the same gap for retry/control-flow and Maestro nested dispatch, which funnel through this exact function and previously had no throw protection either. Adds regression coverage for the exact gap: end-to-end replay tests asserting REPLAY_DIVERGENCE (not COMMAND_FAILED) with resume + screen refs for an annotated press, an unannotated press, and a fill selector-miss thrown at dispatch. --- ...sion-replay-dispatch-selector-miss.test.ts | 204 ++++++++++++++++++ .../handlers/session-replay-action-runtime.ts | 37 +++- 2 files changed, 231 insertions(+), 10 deletions(-) create mode 100644 src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts diff --git a/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts new file mode 100644 index 000000000..6ce8a6573 --- /dev/null +++ b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts @@ -0,0 +1,204 @@ +/** + * Regression coverage for the merged-main divergence (surfaced by the #1210 + * architecture-consolidation merge): a raw dispatch selector-miss during + * press/click/fill/longpress THROWS an AppError instead of resolving to + * `{ok:false}`. `invokeReplayAction` (session-replay-action-runtime.ts) + * previously let that throw escape the per-action `if (!response.ok)` + * handling in `runReplayScriptFile`'s loop, so it hit the outer catch and + * returned a bare `COMMAND_FAILED` with the legacy diagnostics shape instead + * of the ADR 0012 `REPLAY_DIVERGENCE` report — breaking the interactive + * repair loop (no `resume`, no `screen` refs, no `suggestions`) for the + * commonest drift case. + * + * CI previously missed this because every existing divergence-shape test + * (`session-replay-target-verification-runtime.test.ts`, + * `session-replay-runtime-failure-response.test.ts`) drives the failure + * through a RETURNED `{ok:false}` `invoke` response, which was never broken — + * only a THROWN dispatch failure regressed. These tests mock `invoke` to + * throw, matching the real production shape (`resolution.ts` throws + * `AppError('COMMAND_FAILED', ..., { hint: selectorFailureHint(...) })` on a + * zero-match selector) instead of returning it. + */ +import { test, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, dispatchCommand: vi.fn(async () => ({})), resolveTargetDevice: vi.fn() }; +}); + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { runReplayScriptFile } from '../session-replay-runtime.ts'; +import { SessionStore } from '../../session-store.ts'; +import { AppError } from '../../../kernel/errors.ts'; +import { dispatchCommand } from '../../../core/dispatch.ts'; +import { makeIosSession } from '../../../__tests__/test-utils/session-factories.ts'; +import { + baseReplayRequest as baseReq, + writeReplayFile, +} from './session-replay-runtime.fixtures.ts'; +import { + bottomTabsRealCaptureFixture, + recordArticleEvidence, +} from './session-replay-target-classification-fixtures.ts'; + +const mockDispatchCommand = vi.mocked(dispatchCommand); + +beforeEach(() => { + mockDispatchCommand.mockReset(); + mockDispatchCommand.mockResolvedValue({}); +}); + +function setupSession(root: string): { sessionStore: SessionStore; sessionName: string } { + const sessionStore = new SessionStore(path.join(root, 'sessions')); + const sessionName = 'default'; + sessionStore.set(sessionName, makeIosSession(sessionName, { appBundleId: 'com.example.app' })); + return { sessionStore, sessionName }; +} + +function throwSelectorMiss(selector: string): never { + // Mirrors the real dispatch throw site (resolution.ts): a raw + // AppError('COMMAND_FAILED', ..., { hint: selectorFailureHint(...) }) + // surfaced when a selector matches zero nodes. + throw new AppError('COMMAND_FAILED', `No element matched selector ${selector}`, { + hint: 'Run snapshot -i ... or use find ...', + }); +} + +function assertDivergenceShape(response: Awaited>): { + divergence: Record; + targetBinding: Record | undefined; +} { + expect(response.ok).toBe(false); + if (response.ok) throw new Error('expected failure response'); + expect(response.error.code).toBe('REPLAY_DIVERGENCE'); + const divergence = response.error.details?.divergence as Record; + expect(divergence).toBeDefined(); + expect(typeof divergence.kind).toBe('string'); + + const resume = divergence.resume as { allowed: boolean; from?: number; planDigest?: string }; + expect(resume.allowed).toBe(true); + expect(resume.from).toBe(1); + expect(typeof resume.planDigest).toBe('string'); + expect((resume.planDigest ?? '').length).toBeGreaterThan(0); + + const screen = divergence.screen as { state: string; refs?: unknown[] }; + expect(screen.state).toBe('available'); + expect(Array.isArray(screen.refs)).toBe(true); + expect((screen.refs ?? []).length).toBeGreaterThan(0); + + return { divergence, targetBinding: divergence.targetBinding as Record }; +} + +test('(a) an ANNOTATED press whose dispatch throws a selector-miss yields REPLAY_DIVERGENCE, not COMMAND_FAILED', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-annotated-')); + const { sessionStore, sessionName } = setupSession(root); + const evidence = recordArticleEvidence(); + const filePath = writeReplayFile(root, [ + `# agent-device:target-v1 ${JSON.stringify(evidence)}`, + 'click id="article"', + ]); + + // Pre-action verification (and the post-failure divergence capture) both + // see the recorded target still present and unique — verification passes + // (verified:true, guard minted) — but the REAL dispatch call independently + // throws a selector-miss (e.g. a downstream resolution path the daemon + // tree capture does not share). This is exactly the scenario the fix must + // still wrap: verification passing must not suppress a thrown dispatch + // failure. + mockDispatchCommand.mockResolvedValue({ + nodes: bottomTabsRealCaptureFixture(), + truncated: false, + backend: 'xctest', + }); + + const invoked: string[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req.command); + throwSelectorMiss('id="article"'); + }, + }); + + expect(invoked).toEqual(['click']); + const { divergence } = assertDivergenceShape(response); + // A thrown dispatch failure is a generic action-failure divergence — it is + // NOT re-derived as a target-binding classification (that only happens + // when verification's OWN pre-action check finds the mismatch). + expect(divergence.kind).toBe('action-failure'); + const cause = divergence.cause as { code: string; message: string }; + expect(cause.code).toBe('COMMAND_FAILED'); + expect(cause.message).toContain('No element matched selector'); +}); + +test('(b) an UNANNOTATED press whose dispatch throws a selector-miss yields REPLAY_DIVERGENCE, not COMMAND_FAILED', async () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-unannotated-'), + ); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, ['click label="Renamed Button"']); + + // No target-v1 annotation: `verifyReplayActionTarget` returns + // `{verified: true}` immediately and dispatch proceeds straight to + // `invoke`, which throws for the drifted label. + mockDispatchCommand.mockResolvedValue({ + nodes: bottomTabsRealCaptureFixture(), + truncated: false, + backend: 'xctest', + }); + + const invoked: string[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req.command); + throwSelectorMiss('label="Renamed Button"'); + }, + }); + + expect(invoked).toEqual(['click']); + const { divergence } = assertDivergenceShape(response); + expect(divergence.kind).toBe('action-failure'); + const cause = divergence.cause as { code: string; message: string }; + expect(cause.code).toBe('COMMAND_FAILED'); +}); + +test('(c) a fill selector-miss thrown at dispatch yields REPLAY_DIVERGENCE, not COMMAND_FAILED', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-fill-')); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, [`fill 'label="Email"' "someone@example.com"`]); + + mockDispatchCommand.mockResolvedValue({ + nodes: bottomTabsRealCaptureFixture(), + truncated: false, + backend: 'xctest', + }); + + const invoked: string[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req.command); + throwSelectorMiss('label="Email"'); + }, + }); + + expect(invoked).toEqual(['fill']); + const { divergence } = assertDivergenceShape(response); + expect(divergence.kind).toBe('action-failure'); + const cause = divergence.cause as { code: string; message: string }; + expect(cause.code).toBe('COMMAND_FAILED'); + // The fill text itself must never leak into the divergence. + expect(JSON.stringify(divergence)).not.toContain('someone@example.com'); +}); diff --git a/src/daemon/handlers/session-replay-action-runtime.ts b/src/daemon/handlers/session-replay-action-runtime.ts index 217d4d7ea..9dbd25549 100644 --- a/src/daemon/handlers/session-replay-action-runtime.ts +++ b/src/daemon/handlers/session-replay-action-runtime.ts @@ -13,6 +13,8 @@ import { invokeReplayRetryBlock, type ReplayActionBlockInvoker, } from '../../replay/control-flow-runtime.ts'; +import { asAppError } from '../../kernel/errors.ts'; +import { errorResponse } from './response.ts'; type ReplayBaseRequest = Omit; @@ -60,16 +62,31 @@ export async function invokeReplayAction(params: { positionals: resolved.positionals ?? [], }); - const response = await invokeResolvedReplayAction({ - req, - sessionName, - resolved, - scope, - line, - step, - invoke, - invokeReplayAction: invokeNestedReplayAction, - }); + // A raw dispatch failure (e.g. a selector-miss during press/click/fill/ + // longpress) can THROW an AppError instead of resolving to `{ok:false}` — + // every caller reachable from here (the top-level replay loop, retry + // blocks, and nested Maestro runFlow invocations) only ever branches on + // `response.ok`, so this is the single place every replay action dispatch + // funnels through, regardless of nesting depth. Normalizing here means the + // top-level loop's existing `if (!response.ok)` divergence wrapping (and + // retry/control-flow's own `response.ok` checks) apply uniformly instead of + // the throw escaping unwrapped to the outer catch. + let response: DaemonResponse; + try { + response = await invokeResolvedReplayAction({ + req, + sessionName, + resolved, + scope, + line, + step, + invoke, + invokeReplayAction: invokeNestedReplayAction, + }); + } catch (dispatchErr) { + const appErr = asAppError(dispatchErr); + response = errorResponse(appErr.code, appErr.message, appErr.details); + } const finishedAt = Date.now(); appendReplayTraceEvent(tracePath, { From 5ee04cdc07801d0b8aac11117fb169ad2484555c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 12 Jul 2026 12:36:52 +0200 Subject: [PATCH 2/3] style: wrap over-width line in replay selector-miss test (oxfmt) --- .../__tests__/session-replay-dispatch-selector-miss.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts index 6ce8a6573..a5aec1ff4 100644 --- a/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts @@ -92,7 +92,9 @@ function assertDivergenceShape(response: Awaited { - const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-annotated-')); + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-annotated-'), + ); const { sessionStore, sessionName } = setupSession(root); const evidence = recordArticleEvidence(); const filePath = writeReplayFile(root, [ From 8f66358cdd5ef53a0f6b15ada18ff7b6ce89cb58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 12 Jul 2026 13:56:29 +0200 Subject: [PATCH 3/3] fix: narrow replay dispatch catch to AppError, normalize via normalizeError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit invokeReplayAction's catch previously coerced ANY thrown dispatch failure (via asAppError) into a {ok:false} response so the replay loop's divergence wrapping applies — including plain TypeError/ReferenceError from programmer bugs, which asAppError silently reclassified as a repairable REPLAY_DIVERGENCE ("heal a selector"), masking a real crash. Narrow the catch to only wrap actual AppError dispatch failures; anything else re-throws to the outer internal-error path. Switch the wrap itself from errorResponse(appErr.code, appErr.message, appErr.details) to { ok: false, error: normalizeError(dispatchErr) } — the canonical daemon thrown->response pattern (request-generic-dispatch.ts, selector-runtime.ts) — so retriable/supportedOn and hint/diagnosticId/logPath hoisting survive on the thrown path instead of being dropped. --- ...sion-replay-dispatch-selector-miss.test.ts | 74 +++++++++++++++++++ .../handlers/session-replay-action-runtime.ts | 13 +++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts index a5aec1ff4..1ceed001b 100644 --- a/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts +++ b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts @@ -204,3 +204,77 @@ test('(c) a fill selector-miss thrown at dispatch yields REPLAY_DIVERGENCE, not // The fill text itself must never leak into the divergence. expect(JSON.stringify(divergence)).not.toContain('someone@example.com'); }); + +test('(d) a thrown NON-AppError at dispatch propagates as an internal error, not REPLAY_DIVERGENCE', async () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-nonapperror-'), + ); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, ['click label="Renamed Button"']); + + mockDispatchCommand.mockResolvedValue({ + nodes: bottomTabsRealCaptureFixture(), + truncated: false, + backend: 'xctest', + }); + + const invoked: string[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req.command); + // A programmer bug (unrelated to selector resolution), not an expected + // dispatch failure — must NOT be coerced into a repairable divergence. + throw new TypeError('boom'); + }, + }); + + expect(invoked).toEqual(['click']); + expect(response.ok).toBe(false); + if (response.ok) throw new Error('expected failure response'); + expect(response.error.code).not.toBe('REPLAY_DIVERGENCE'); + expect(response.error.code).toBe('UNKNOWN'); + expect(response.error.message).toContain('boom'); +}); + +test('(e) a thrown AppError with retriable/supportedOn preserves them at the top level of the divergence response', async () => { + const root = fs.mkdtempSync( + path.join(os.tmpdir(), 'agent-device-replay-dispatch-miss-retriable-'), + ); + const { sessionStore, sessionName } = setupSession(root); + const filePath = writeReplayFile(root, ['click label="Renamed Button"']); + + mockDispatchCommand.mockResolvedValue({ + nodes: bottomTabsRealCaptureFixture(), + truncated: false, + backend: 'xctest', + }); + + const invoked: string[] = []; + const response = await runReplayScriptFile({ + req: baseReq({ positionals: [filePath] }), + sessionName, + logPath: path.join(root, 'daemon.log'), + sessionStore, + invoke: async (req) => { + invoked.push(req.command); + throw new AppError('COMMAND_FAILED', 'device busy mid-gesture', { + retriable: true, + supportedOn: 'ios', + }); + }, + }); + + expect(invoked).toEqual(['click']); + const { divergence } = assertDivergenceShape(response); + expect(divergence.kind).toBe('action-failure'); + expect(response.ok).toBe(false); + if (response.ok) throw new Error('expected failure response'); + // Normalized via normalizeError: hoisted onto the top-level error, not + // buried in divergence.cause (which only carries code/message/hint). + expect(response.error.retriable).toBe(true); + expect(response.error.supportedOn).toBe('ios'); +}); diff --git a/src/daemon/handlers/session-replay-action-runtime.ts b/src/daemon/handlers/session-replay-action-runtime.ts index 9dbd25549..ae0444b90 100644 --- a/src/daemon/handlers/session-replay-action-runtime.ts +++ b/src/daemon/handlers/session-replay-action-runtime.ts @@ -13,8 +13,7 @@ import { invokeReplayRetryBlock, type ReplayActionBlockInvoker, } from '../../replay/control-flow-runtime.ts'; -import { asAppError } from '../../kernel/errors.ts'; -import { errorResponse } from './response.ts'; +import { AppError, normalizeError } from '../../kernel/errors.ts'; type ReplayBaseRequest = Omit; @@ -84,8 +83,14 @@ export async function invokeReplayAction(params: { invokeReplayAction: invokeNestedReplayAction, }); } catch (dispatchErr) { - const appErr = asAppError(dispatchErr); - response = errorResponse(appErr.code, appErr.message, appErr.details); + // Only an expected AppError dispatch failure (e.g. a selector-miss) gets + // normalized into a `{ok:false}` response so the loop's `if + // (!response.ok)` divergence wrapping applies. A plain TypeError/ + // ReferenceError or other programmer bug must propagate to the outer + // internal-error path rather than being coerced into a repairable + // REPLAY_DIVERGENCE that would mask a crash. + if (!(dispatchErr instanceof AppError)) throw dispatchErr; + response = { ok: false, error: normalizeError(dispatchErr) }; } const finishedAt = Date.now();