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..1ceed001b --- /dev/null +++ b/src/daemon/handlers/__tests__/session-replay-dispatch-selector-miss.test.ts @@ -0,0 +1,280 @@ +/** + * 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'); +}); + +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 217d4d7ea..ae0444b90 100644 --- a/src/daemon/handlers/session-replay-action-runtime.ts +++ b/src/daemon/handlers/session-replay-action-runtime.ts @@ -13,6 +13,7 @@ import { invokeReplayRetryBlock, type ReplayActionBlockInvoker, } from '../../replay/control-flow-runtime.ts'; +import { AppError, normalizeError } from '../../kernel/errors.ts'; type ReplayBaseRequest = Omit; @@ -60,16 +61,37 @@ 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) { + // 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(); appendReplayTraceEvent(tracePath, {