diff --git a/README.md b/README.md index 5697626..9de004b 100644 --- a/README.md +++ b/README.md @@ -366,9 +366,13 @@ type ToolContext = { get refMap(): Map; // lazy — returns empty map if no session workflowContext: WorkflowContext; toolRegistry: Map>; + driver?: IPlatformDriver; + signal?: AbortSignal; // aborted when the request times out }; ``` +Tools check `signal?.aborted` between steps and stop. In the daemon, `context.sessionManager.setRefMap` goes through a guard (`RefStore`) that drops a write from a request that timed out or was replaced by a newer one, so a cancelled call cannot overwrite the refs of later calls. + The daemon routes `POST /tool/:name` requests through the registry, applies Zod validation on inputs, executes the tool through the request queue, and captures observations (test IDs, a11y snapshot) according to the call's `observe` level (see [Observations](#observations)). **Registered tools:** @@ -443,6 +447,10 @@ mm type e3 "hello" → types into the element mapped to e3 This accessibility-first approach provides reliable element targeting that survives minor UI changes. +Refs are numbered in tree order each time the daemon reads the screen (`describe_screen`, `accessibility_snapshot`, or an observation). The same screen gives the same refs, so an unchanged screen diffs as unchanged. When elements appear or disappear above an element, its ref can change: use the refs from the latest response. When a call returns both a tree and an observation (for example `describe_screen` with `observe`), the observation's refs are the ones that resolve. `list_testids` does not change refs. + +On mobile, after a mutating or batch call, the daemon may take the snapshot again after 300 ms: while it shows a busy element (a label ending in `, busy`, an Android `ProgressBar`, or an iOS `ActivityIndicator` / `ProgressIndicator` / `UIActivityIndicatorView`), and at most once when it still equals the previous screen. In total at most 3 extra snapshots and about 2 s. The test IDs come from the same snapshot as the tree. `describe_screen` on mobile also takes one snapshot for both. + ### Session Metadata Consumers can persist launch metadata with `writeSessionMetadata(artifactsDir, metadata)`. It writes `//session.json` (for example `test-artifacts//session.json` with `artifactsDir: 'test-artifacts'`) and returns the file path. @@ -541,6 +549,8 @@ For `run_steps`, the top-level `observe` sets the level of the one observation c Observations contain `testIds` and `a11y` only. They do not include app state; call `get_state` for that. +A call that times out is cancelled: its signal is aborted, it stops at its next step, it collects no observation, and its refs are dropped. + ## CLI Reference The `mm` CLI provides a unified interface for agents and developers. All commands communicate with the daemon over HTTP — the daemon is auto-started on `mm launch` if not already running. @@ -795,6 +805,7 @@ Tool errors are classified into specific error codes for structured handling: | `MM_PORT_IN_USE` | Port already in use | | **System** | | | `MM_UNKNOWN_TOOL` | Unknown tool name | +| `MM_REQUEST_ABORTED` | Internal: a cancelled (timed-out) call stopped; the caller gets the timeout error | | `MM_INTERNAL_ERROR` | Internal server error | | `MM_BATCH_TIMEOUT` | `batchTimeoutMs` deadline exceeded in run_steps | | `MM_CDP_BLOCKED` | CDP method is blocked (destructive to session); on mobile the Hermes blocklist maps here too | diff --git a/SKILL.md b/SKILL.md index 94923e9..75bae0f 100644 --- a/SKILL.md +++ b/SKILL.md @@ -18,7 +18,7 @@ mm cleanup --shutdown # 5. Clean up when done **Critical rules:** -- **Always `describe-screen` before interacting.** Refs like `e1`, `e2` are ephemeral — they change after every action. +- **Always `describe-screen` before interacting.** Refs like `e1`, `e2` are ephemeral — use the refs from the latest response (`describe-screen` or the `observations` of your last action). - **Always `describe-screen` after interacting** — OR use inline `observations` from mutating tool responses. Mutating tools (click, type, navigate, etc.) return an `observations` object with fresh `testIds` and `a11y` refs. You can use these refs directly for the next interaction without calling `describe-screen`. Call `describe-screen` when you need the full tree or screenshots, and `get-state` when you need app state (observations carry none). - **One target per command.** Specify exactly ONE of: a11y ref (`e5`), testId, or CSS selector. - **Errors are structured.** Check the `error.code` field to decide recovery strategy (see Error Codes below). @@ -85,7 +85,9 @@ After a mutating action, the response includes fresh screen data: } ``` -You can use the `ref` values from `observations.a11y.nodes` for the next interaction — no `describe-screen` needed. Note that refs in compacted observations may be summary nodes (e.g., `"55 options (refs e2–e56)"`) when there are 3+ options under a combobox or listbox. +You can use the `ref` values from `observations.a11y.nodes` for the next interaction — no `describe-screen` needed. Refs are numbered in screen order: on an unchanged screen they stay the same, but when elements appear or disappear above an element its ref can change. With `--observe diff`, those elements are listed in the diff with their new refs, so always take refs from the latest response. Note that refs in compacted observations may be summary nodes (e.g., `"55 options (refs e2–e56)"`) when there are 3+ options under a combobox or listbox. + +**Timeouts cancel the call:** when a call times out (`Tool execution timed out`), it is cancelled. It stops at its next step and cannot overwrite the refs of later calls. The action itself may still have happened, so run `describe-screen` before retrying. **Quick reference:** diff --git a/src/index.ts b/src/index.ts index 3699cbf..50eb544 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ export type { TypeActionResult, GetTextActionResult, PlatformScreenshotOptions, + ScreenCapture, } from './platform'; export { PlaywrightPlatformDriver, MobilePlatformDriver } from './platform'; diff --git a/src/platform/index.ts b/src/platform/index.ts index 6ffb045..dd96aed 100644 --- a/src/platform/index.ts +++ b/src/platform/index.ts @@ -7,6 +7,7 @@ export type { PlatformScreenshotOptions, WithinScope, IPlatformDriver, + ScreenCapture, } from './types.js'; export { PlaywrightPlatformDriver } from './playwright-driver.js'; diff --git a/src/platform/mobile-platform-driver.test.ts b/src/platform/mobile-platform-driver.test.ts index 9e8cf07..0e6af62 100644 --- a/src/platform/mobile-platform-driver.test.ts +++ b/src/platform/mobile-platform-driver.test.ts @@ -71,6 +71,52 @@ describe('MobilePlatformDriver', () => { vi.restoreAllMocks(); }); + describe('captureScreen', () => { + it('returns the tree and the test IDs of one snapshot', async () => { + const backend = createMockBackend({ + getWindowSize: vi.fn().mockResolvedValue({ width: 402, height: 874 }), + snapshot: vi + .fn() + .mockResolvedValueOnce({ + platform: 'ios', + hierarchy: [ + makeElement({ identifier: 'login', label: 'Unlock' }), + makeElement({ + identifier: 'below', + type: 'StaticText', + frame: { x: 0, y: 2000, width: 10, height: 10 }, + }), + ], + raw: '[]', + timestamp: Date.now(), + }) + .mockResolvedValueOnce({ + platform: 'ios', + hierarchy: [makeElement({ identifier: 'wallet' })], + raw: '[]', + timestamp: Date.now(), + }), + }); + const driver = new MobilePlatformDriver(backend); + + const screen = await driver.captureScreen(); + const testIds = await screen.getTestIds(); + + expect(backend.snapshot).toHaveBeenCalledTimes(1); + expect(screen.nodes.map((node) => node.testId)).toStrictEqual([ + 'login', + 'below', + ]); + expect(screen.refMap.get('e1')).toBe('identifier:login'); + // Same items, order and visibility as getTestIds on that snapshot. + expect(testIds).toStrictEqual([ + { testId: 'login', tag: 'Button', text: 'Unlock', visible: true }, + { testId: 'below', tag: 'StaticText', text: undefined, visible: false }, + ]); + expect(await screen.getTestIds(1)).toHaveLength(1); + }); + }); + describe('getPlatform', () => { it('returns ios for ios backend', () => { const driver = new MobilePlatformDriver(createMockBackend()); diff --git a/src/platform/mobile-platform-driver.ts b/src/platform/mobile-platform-driver.ts index 7aea944..4eee7aa 100644 --- a/src/platform/mobile-platform-driver.ts +++ b/src/platform/mobile-platform-driver.ts @@ -22,6 +22,7 @@ import type { GetTextActionResult, PlatformScreenshotOptions, PlatformType, + ScreenCapture, WithinScope, } from './types.js'; import type { @@ -208,6 +209,33 @@ export class MobilePlatformDriver implements IPlatformDriver { */ async getTestIds(limit?: number): Promise { const snapshot = await this.#backend.snapshot(); + return this.#testIdsOf(snapshot.hierarchy, limit); + } + + /** + * Takes one device snapshot and returns its accessibility tree and test + * IDs, so both describe the same screen. + * + * @returns The nodes, refMap and a test ID reader for the same snapshot. + */ + async captureScreen(): Promise { + const snapshot = await this.#backend.snapshot(); + return { + ...normalizeSnapshot(snapshot.hierarchy), + getTestIds: async (limit?: number) => + this.#testIdsOf(snapshot.hierarchy, limit), + }; + } + + /** + * @param hierarchy - The UIElement tree of a snapshot. + * @param limit - Maximum number of test IDs to return. + * @returns The test IDs, with visibility against the device viewport. + */ + async #testIdsOf( + hierarchy: UIElement[], + limit?: number, + ): Promise { const items: TestIdItem[] = []; const max = limit ?? OBSERVATION_TESTID_LIMIT; let viewport: { width: number; height: number } | undefined; @@ -217,7 +245,7 @@ export class MobilePlatformDriver implements IPlatformDriver { // Without a viewport we cannot judge visibility; report everything. viewport = undefined; } - collectTestIds(snapshot.hierarchy, items, max, viewport); + collectTestIds(hierarchy, items, max, viewport); return items; } diff --git a/src/platform/types.ts b/src/platform/types.ts index 529e6ed..caf9e28 100644 --- a/src/platform/types.ts +++ b/src/platform/types.ts @@ -48,6 +48,21 @@ export type WithinScope = { value: string; }; +/** + * One snapshot of the screen: the accessibility tree, and the test IDs of the + * same snapshot. + */ +export type ScreenCapture = { + nodes: A11yNodeTrimmed[]; + refMap: Map; + /** + * Lists the test IDs of this snapshot (no new snapshot). + * + * @param limit - Maximum number of test IDs to return. + */ + getTestIds(limit?: number): Promise; +}; + export type IPlatformDriver = { click( targetType: TargetType, @@ -88,6 +103,13 @@ export type IPlatformDriver = { getTestIds(limit?: number): Promise; + /** + * Takes one snapshot and returns both the accessibility tree and its test + * IDs, so they describe the same screen. Optional: without it, callers use + * `getAccessibilityTree` and `getTestIds` (two snapshots). + */ + captureScreen?(): Promise; + screenshot(options: PlatformScreenshotOptions): Promise; getAppState(): Promise; diff --git a/src/server/create-server.test.ts b/src/server/create-server.test.ts index dbc648a..18e721a 100644 --- a/src/server/create-server.test.ts +++ b/src/server/create-server.test.ts @@ -10,11 +10,20 @@ import { createServer, buildResponseBody, resolveObserveLevel, + screenAfterResponse, } from './create-server.js'; import { readDaemonState } from './daemon-state.js'; import pkg from '../../package.json'; import type { PortMap, WorkflowContext } from '../capabilities/context.js'; -import type { DaemonState, ServerConfig, ToolResponse } from '../types/http.js'; +import { toolRegistry } from '../tools/registry.js'; +import type { StepRecordObservation } from '../tools/types/observation.js'; +import type { + DaemonState, + ServerConfig, + ToolContext, + ToolFunction, + ToolResponse, +} from '../types/http.js'; const tmpDir = path.join(os.tmpdir(), `mm-create-server-test-${Date.now()}`); @@ -73,6 +82,7 @@ function createMockSessionManager() { takeScreenshot: vi.fn(async () => ({ path: '', base64: '' })), getRefMap: vi.fn(() => new Map()), setRefMap: vi.fn(), + clearRefMap: vi.fn(), setWorkflowContext: vi.fn(), getEnvironmentMode: vi.fn(() => 'e2e'), setContext: vi.fn(), @@ -2150,3 +2160,872 @@ describe('createServer observation driver freshness after launch', () => { expect(body.error.code).toBe('MM_TOOL_NOT_SUPPORTED_ON_PLATFORM'); }); }); + +describe('createServer abort, ref generations and observations', () => { + type Node = { ref: string; role: string; name: string; path: string[] }; + type ObsBody = { + ok: boolean; + error?: { code: string; message: string }; + observations?: { + a11y: { + nodes: Node[]; + diff?: { added: string[]; removed: string[]; unchanged: number }; + }; + }; + }; + type Tree = { nodes: Node[]; refMap: Map }; + + const sleep = async (ms: number): Promise => + new Promise((resolve) => setTimeout(resolve, ms)); + + const buildTree = (names: string[]): Tree => ({ + nodes: names.map((name, index) => ({ + ref: `e${index + 1}`, + role: 'button', + name, + path: [], + })), + refMap: new Map( + names.map((name, index) => [`e${index + 1}`, `label:${name}`] as const), + ), + }); + + let server: ServerInstance | undefined; + let state: DaemonState; + let mockSM: ReturnType; + let storedRefMap: Map; + let driver: { + getPlatform: ReturnType; + getAppState: ReturnType; + getTestIds: ReturnType; + getAccessibilityTree: ReturnType; + click: ReturnType; + }; + const addedTools: string[] = []; + + const startServer = async ( + options: { requestTimeoutMs?: number; mobile?: boolean } = {}, + ): Promise => { + if (options.mobile !== false) { + mockSM.getPlatformDriver.mockReturnValue(driver); + } + server = createServer( + buildConfig({ + sessionManager: mockSM as unknown as ServerConfig['sessionManager'], + requestTimeoutMs: options.requestTimeoutMs, + }), + ); + state = await server.start(); + }; + + const post = async ( + urlPath: string, + requestBody: unknown = {}, + ): Promise<{ status: number; body: ObsBody }> => { + const res = await httpRequest(`http://127.0.0.1:${state.port}${urlPath}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(requestBody), + }); + return { status: res.status, body: (await res.json()) as ObsBody }; + }; + + const addTool = (name: string, fn: ToolFunction): void => { + toolRegistry.set(name, fn); + addedTools.push(name); + }; + + // A mutating tool without a `timeoutMs` default, so the server's + // requestTimeoutMs is the queue timeout (click defaults to 15 s). + const addTapTool = (): void => { + addTool('test_tap', async () => ({ ok: true, result: {} })); + }; + + beforeEach(async () => { + await fs.mkdir(tmpDir, { recursive: true }); + exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => {}) as never); + + storedRefMap = new Map(); + mockSM = createMockSessionManager(); + mockSM.hasActiveSession.mockReturnValue(true); + mockSM.setRefMap.mockImplementation((map: Map) => { + storedRefMap = map; + }); + mockSM.getRefMap.mockImplementation(() => storedRefMap); + + driver = { + getPlatform: vi.fn().mockReturnValue('android'), + getAppState: vi.fn().mockResolvedValue({ isLoaded: true }), + getTestIds: vi.fn().mockResolvedValue([]), + getAccessibilityTree: vi.fn().mockResolvedValue(buildTree(['Unlock'])), + click: vi.fn().mockResolvedValue({ clicked: true, target: 'x' }), + }; + }); + + afterEach(async () => { + const running = server; + server = undefined; + await running?.stop(); + for (const name of addedTools.splice(0)) { + toolRegistry.delete(name); + } + exitSpy.mockRestore(); + const discovery = await import('../tools/utils/discovery.js'); + vi.mocked(discovery.collectTrimmedA11ySnapshot).mockReset(); + vi.mocked(discovery.collectTrimmedA11ySnapshot).mockResolvedValue({ + nodes: [], + refMap: new Map(), + }); + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + }); + + describe('P1-08: timed-out requests', () => { + it('aborts the signal of a timed-out task and ignores its late setRefMap (AC1)', async () => { + await startServer({ requestTimeoutMs: 50 }); + let abortedWhenLate: boolean | undefined; + let finished!: () => void; + const done = new Promise((resolve) => { + finished = resolve; + }); + addTool('test_slow_tool', async (_input, context) => { + await sleep(120); + abortedWhenLate = context.signal?.aborted; + context.sessionManager.setRefMap(new Map([['e1', 'late']])); + finished(); + return { ok: true, result: {} }; + }); + + const { status, body } = await post('/tool/test_slow_tool'); + await done; + + expect(status).toBe(500); + expect(body.error?.message).toContain('timed out after 50ms'); + expect(abortedWhenLate).toBe(true); + expect(mockSM.setRefMap).not.toHaveBeenCalled(); + }); + + it('keeps the refMap of the second request after the first times out (AC2)', async () => { + await startServer({ requestTimeoutMs: 50 }); + const firstTree = buildTree(['Old screen']); + const secondTree = buildTree(['New screen']); + // First click: its observation snapshot arrives after the timeout. + driver.getAccessibilityTree + .mockImplementationOnce(async () => { + await sleep(120); + return firstTree; + }) + .mockResolvedValueOnce(secondTree); + addTapTool(); + + const first = await post('/tool/test_tap'); + const second = await post('/tool/test_tap'); + + expect(first.status).toBe(500); + expect(second.status).toBe(200); + expect(second.body.observations?.a11y.nodes).toStrictEqual( + secondTree.nodes, + ); + expect(mockSM.setRefMap).toHaveBeenCalledTimes(1); + expect(storedRefMap).toStrictEqual(secondTree.refMap); + // The aborted observation stopped before its test ID step. + expect(driver.getTestIds).toHaveBeenCalledTimes(1); + }); + + it('stops the browser observation at the next checkpoint after an abort', async () => { + const discovery = await import('../tools/utils/discovery.js'); + vi.mocked(discovery.collectTestIds).mockClear(); + mockSM.getPage.mockReturnValue({ + waitForLoadState: vi.fn(async () => sleep(120)), + waitForFunction: vi.fn(async () => undefined), + } as never); + await startServer({ requestTimeoutMs: 50, mobile: false }); + addTapTool(); + + const { status } = await post('/tool/test_tap'); + await sleep(150); + + expect(status).toBe(500); + expect(vi.mocked(discovery.collectTestIds)).not.toHaveBeenCalled(); + expect(mockSM.setRefMap).not.toHaveBeenCalled(); + }); + + it('cancels a tool that checks the signal between steps', async () => { + await startServer({ requestTimeoutMs: 50 }); + const steps: string[] = []; + addTool('test_steps_tool', async (_input, context) => { + steps.push('one'); + await sleep(100); + if (context.signal?.aborted) { + return { ok: false, error: { code: 'X', message: 'aborted' } }; + } + steps.push('two'); + return { ok: true, result: {} }; + }); + + await post('/tool/test_steps_tool'); + await sleep(80); + + expect(steps).toStrictEqual(['one']); + }); + }); + + describe('P1-09: observations and refs', () => { + it('a non-delivered observation does not change lastObservation (AC1)', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['A', 'B'])) + .mockResolvedValueOnce(buildTree(['A', 'B', 'C'])); + await post('/tool/click', { testId: 'x' }); + await post('/tool/click', { testId: 'x', observe: 'none' }); + const { body } = await post('/tool/click', { testId: 'x' }); + + // observe none collected nothing; the diff is against the first + // (delivered) observation. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + expect(body.observations?.a11y.diff).toStrictEqual({ + added: ['e3'], + removed: [], + unchanged: 2, + }); + }); + + it('lets the observation replace the refs the tool set, without a settle', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Send', 'Swap'])) + .mockResolvedValueOnce(buildTree(['Banner', 'Send', 'Swap'])); + + const { body } = await post('/tool/describe_screen', { + observe: 'full', + }); + + expect(body.ok).toBe(true); + // The observation is the last tree in the response: every ref in it + // resolves to the element it shows. + const nodes = body.observations?.a11y.nodes ?? []; + expect(nodes.map((node) => [node.ref, node.name])).toStrictEqual([ + ['e1', 'Banner'], + ['e2', 'Send'], + ['e3', 'Swap'], + ]); + expect(storedRefMap).toStrictEqual( + buildTree(['Banner', 'Send', 'Swap']).refMap, + ); + // No settle: the tool set refs, so it had just read the screen. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + + it('lets the run_steps observation replace refs a step set', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Send'])) + .mockResolvedValueOnce(buildTree(['Done'])); + + await post('/tool/run_steps', { + steps: [{ tool: 'accessibility_snapshot' }], + }); + + expect(mockSM.setRefMap).toHaveBeenCalledTimes(2); + expect(storedRefMap).toStrictEqual(new Map([['e1', 'label:Done']])); + }); + + it('sends the new refs when an element is inserted above the others', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Send', 'Swap'])) + .mockResolvedValueOnce(buildTree(['Banner', 'Send', 'Swap'])); + + await post('/tool/click', { testId: 'x' }); + const { body } = await post('/tool/click', { testId: 'x' }); + + // Refs are positional: Banner takes e1, Send and Swap move to e2/e3. + // Every node changed, so the response carries the full tree (a diff + // would not be smaller) and the agent sees the new refs. + expect(body.observations?.a11y.diff).toBeUndefined(); + expect( + body.observations?.a11y.nodes.map((node) => [node.ref, node.name]), + ).toStrictEqual([ + ['e1', 'Banner'], + ['e2', 'Send'], + ['e3', 'Swap'], + ]); + expect(storedRefMap.get('e1')).toBe('label:Banner'); + }); + + it('diffs two mobile observations of the same tree as unchanged', async () => { + await startServer(); + // Many unnamed containers (Android: android.view.ViewGroup), plus a + // status-bar label that changes. + const screen = (signal: string): Tree => { + const nodes: Node[] = [ + { + ref: 'e1', + role: 'android.widget.FrameLayout', + name: signal, + path: [], + }, + ...Array.from( + { length: 30 }, + (_, index): Node => ({ + ref: `e${index + 2}`, + role: 'android.view.ViewGroup', + name: '', + path: ['android.widget.FrameLayout'], + }), + ), + { ref: 'e32', role: 'android.widget.Button', name: 'Send', path: [] }, + ]; + return { + nodes, + refMap: new Map([ + ['e1', `label:${signal}|type:android.widget.FrameLayout`], + ['e32', 'label:Send|type:android.widget.Button'], + ]), + }; + }; + driver.getAccessibilityTree + .mockResolvedValueOnce(screen('T-Mobile, no signal.')) + .mockResolvedValueOnce(screen('T-Mobile, no signal.')) + // Settle retry (unchanged): the status bar changed meanwhile. + .mockResolvedValue(screen('T-Mobile, signal full.')); + + await post('/tool/click', { testId: 'x' }); + const same = await post('/tool/click', { testId: 'x' }); + + // Only the status-bar node whose name changed is sent. + expect( + same.body.observations?.a11y.nodes.map((node) => node.name), + ).toStrictEqual(['T-Mobile, signal full.']); + + const again = await post('/tool/click', { testId: 'x' }); + + expect(again.body.observations?.a11y.diff).toStrictEqual({ + added: [], + removed: [], + unchanged: 32, + }); + expect(again.body.observations?.a11y.nodes).toStrictEqual([]); + }); + + it('routes context.sessionManager.setRefMap through the request handle', async () => { + await startServer(); + const target = mockSM as unknown as Record; + target.whoAmI = function (this: unknown): boolean { + return this === mockSM; + }; + Object.defineProperty(mockSM, 'isSelf', { + configurable: true, + get(this: unknown): boolean { + return this === mockSM; + }, + }); + let seen: { bound?: boolean; getter?: boolean; id?: string } = {}; + addTool('test_consumer_tool', async (_input, context) => { + const sessionManager = context.sessionManager as unknown as { + whoAmI(): boolean; + isSelf: boolean; + } & ToolContext['sessionManager']; + const { whoAmI } = sessionManager; + seen = { + bound: whoAmI(), + getter: sessionManager.isSelf, + id: sessionManager.getSessionId(), + }; + context.sessionManager.setRefMap(new Map([['e1', 'consumer']])); + return { ok: true, result: {} }; + }); + + await post('/tool/test_consumer_tool', { observe: 'none' }); + + expect(seen).toStrictEqual({ + bound: true, + getter: true, + id: 'test-session', + }); + expect(mockSM.setRefMap).toHaveBeenCalledTimes(1); + expect(storedRefMap).toStrictEqual(new Map([['e1', 'consumer']])); + }); + + it('drops a late clearRefMap from a timed-out request', async () => { + await startServer({ requestTimeoutMs: 50 }); + storedRefMap = new Map([['e1', 'kept']]); + mockSM.clearRefMap.mockImplementation(() => { + storedRefMap = new Map(); + }); + let finished!: () => void; + const done = new Promise((resolve) => { + finished = resolve; + }); + addTool('test_slow_clear_tool', async (_input, context) => { + await sleep(120); + context.sessionManager.clearRefMap(); + finished(); + return { ok: true, result: {} }; + }); + + const { status } = await post('/tool/test_slow_clear_tool'); + await done; + + expect(status).toBe(500); + expect(mockSM.clearRefMap).not.toHaveBeenCalled(); + expect(storedRefMap).toStrictEqual(new Map([['e1', 'kept']])); + }); + + it('binds methods and getters of a session manager class with #private fields', async () => { + class PrivateSessionManager { + readonly #id = 'private-session'; + + #refMap = new Map(); + + get privateId(): string { + return this.#id; + } + + readPrivateId(): string { + return this.#id; + } + + hasActiveSession(): boolean { + return true; + } + + getSessionId(): string { + return this.#id; + } + + getPlatformDriver(): typeof driver { + return driver; + } + + setRefMap(map: Map): void { + this.#refMap = map; + } + + getRefMap(): Map { + return this.#refMap; + } + } + const real = new PrivateSessionManager(); + // Fill in the other members the server needs from the mock, as own + // properties of the real instance (the #private members stay real). + for (const [key, value] of Object.entries(mockSM)) { + if (!(key in real)) { + (real as unknown as Record)[key] = value; + } + } + server = createServer( + buildConfig({ + sessionManager: real as unknown as ServerConfig['sessionManager'], + }), + ); + state = await server.start(); + let seen: { getter?: string; method?: string; detached?: string } = {}; + addTool('test_private_tool', async (_input, context) => { + const proxied = context.sessionManager as unknown as Pick< + PrivateSessionManager, + 'privateId' | 'readPrivateId' + >; + const { readPrivateId } = proxied; + seen = { + getter: proxied.privateId, + method: proxied.readPrivateId(), + detached: readPrivateId(), + }; + context.sessionManager.setRefMap(new Map([['e1', 'private']])); + return { ok: true, result: {} }; + }); + + const { status } = await post('/tool/test_private_tool', { + observe: 'none', + }); + + expect(status).toBe(200); + expect(seen).toStrictEqual({ + getter: 'private-session', + method: 'private-session', + detached: 'private-session', + }); + // The write went through the RefStore to the real #refMap. + expect(real.getRefMap()).toStrictEqual(new Map([['e1', 'private']])); + }); + + describe('launch and cleanup', () => { + let realLaunch: ToolFunction | undefined; + + beforeEach(() => { + realLaunch = toolRegistry.get('launch'); + }); + + afterEach(() => { + if (realLaunch) { + toolRegistry.set('launch', realLaunch); + } + }); + + it('does not compare against the screen from before a launch', async () => { + await startServer(); + toolRegistry.set('launch', async () => ({ ok: true, result: {} })); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + + await post('/tool/click', { testId: 'x' }); + await post('/launch', { observe: 'none' }); + await post('/tool/click', { testId: 'x' }); + + // No unchanged retry after the launch: there is no previous screen. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + }); + }); + + describe('P1-09: mobile settle after a mutating tool', () => { + it('re-snapshots once after 300 ms when the screen looks unchanged', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValueOnce(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + const startedAt = Date.now(); + const { body } = await post('/tool/click', { testId: 'unlock' }); + + expect(Date.now() - startedAt).toBeGreaterThanOrEqual(290); + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(3); + expect(body.observations?.a11y.nodes.map((node) => node.name)).toContain( + 'Wallet', + ); + }); + + it('retries an unchanged, non-busy screen only once', async () => { + await startServer(); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + + await post('/tool/click', { testId: 'x' }); + await post('/tool/click', { testId: 'x' }); + + // 1 (first click) + 1 + 1 retry (second click). + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(3); + }); + + it('takes at most three extra snapshots while the screen is busy', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValue(buildTree(['Password', 'Unlock, busy'])); + + await post('/tool/click', { testId: 'x' }); + await post('/tool/click', { testId: 'x' }); + + // 1 (first click) + 1 + 3 extra (second click). + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(5); + }); + + it('keeps retrying when an unchanged screen turns busy, within the limits', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValueOnce(buildTree(['Password', 'Unlock, busy'])) + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValue(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + const { body } = await post('/tool/click', { + testId: 'x', + observe: 'full', + }); + + // Second click: unchanged → retry (the one unchanged retry), busy → + // retry, unchanged again → no second unchanged retry: stop. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(4); + expect( + body.observations?.a11y.nodes.map((node) => node.name), + ).toStrictEqual(['Password', 'Unlock']); + }); + + it('re-snapshots while a node is busy, even when the screen changed (Android unlock)', async () => { + await startServer(); + const busy: Tree = { + nodes: [ + { + ref: 'e1', + role: 'android.widget.EditText', + name: 'Password', + path: [], + }, + { + ref: 'e2', + role: 'android.widget.Button', + name: 'Unlock, busy', + path: [], + }, + ], + refMap: new Map([['e2', 'identifier:log-in-button']]), + }; + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Password', 'Unlock'])) + .mockResolvedValueOnce(busy) + .mockResolvedValueOnce(busy) + .mockResolvedValueOnce(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + const { body } = await post('/tool/click', { testId: 'log-in-button' }); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(4); + expect( + body.observations?.a11y.nodes.map((node) => node.name), + ).toStrictEqual(['Wallet']); + }); + + it('re-snapshots a busy first observation without a previous screen', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Unlock, busy'])) + .mockResolvedValueOnce(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + + it('does not compare against a describe_screen that timed out', async () => { + await startServer({ requestTimeoutMs: 100 }); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Login'])) + .mockImplementationOnce(async () => { + await sleep(200); + return buildTree(['Wallet']); + }) + .mockResolvedValue(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + mockSM.setRefMap.mockClear(); + const described = await post('/tool/describe_screen'); + await sleep(150); + + expect(described.status).toBe(500); + // The late describe_screen write was dropped. + expect(mockSM.setRefMap).not.toHaveBeenCalled(); + + await post('/tool/click', { testId: 'x' }); + + // Compared against the delivered Login tree, so Wallet is a change: + // no unchanged retry (3 snapshots so far + 1). + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(3); + expect(storedRefMap).toStrictEqual(buildTree(['Wallet']).refMap); + }); + + it('stops settling at MOBILE_SETTLE_MAX_MS', async () => { + await startServer(); + driver.getAccessibilityTree.mockImplementation(async () => { + await sleep(700); + return buildTree(['Unlock, busy']); + }); + + await post('/tool/click', { testId: 'x' }); + + // 1st snapshot, then (300 ms + 700 ms) twice = 2000 ms; a third wait + // would end after the 2000 ms cap. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(3); + }); + + it('does not settle when the tool set refs', async () => { + await startServer(); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + addTool('test_ref_tool', async (_input, context) => { + context.sessionManager.setRefMap(buildTree(['Same']).refMap); + return { ok: true, result: {} }; + }); + + await post('/tool/click', { testId: 'x' }); + await post('/tool/test_ref_tool'); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + + it('settles against the tree describe_screen returned', async () => { + await startServer(); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + + await post('/tool/describe_screen'); + await post('/tool/click', { testId: 'x' }); + + // One describe snapshot; the click snapshot is unchanged against the + // described tree, so it retries once. + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(3); + }); + + it('does not settle after cleanup', async () => { + await startServer(); + driver.getAccessibilityTree.mockResolvedValue( + buildTree(['Unlock, busy']), + ); + mockSM.cleanup.mockResolvedValue(true); + + await post('/cleanup', { observe: 'full' }); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(1); + }); + + it('takes the test IDs from the same snapshot as the final tree', async () => { + await startServer(); + const screens = [ + { names: ['Password', 'Unlock'], testIds: ['login'] }, + { names: ['Password', 'Unlock, busy'], testIds: ['login-busy'] }, + { names: ['Wallet'], testIds: ['wallet'] }, + ].map(({ names, testIds }) => ({ + ...buildTree(names), + getTestIds: vi.fn(async () => + testIds.map((testId) => ({ testId, tag: 'View', visible: true })), + ), + })); + const captureScreen = vi.fn(); + for (const screen of screens) { + captureScreen.mockResolvedValueOnce(screen); + } + Object.assign(driver, { captureScreen }); + + await post('/tool/click', { testId: 'x' }); + const { body } = (await post('/tool/click', { + testId: 'x', + })) as unknown as { + body: { + observations: { + testIds: { testId: string }[]; + a11y: { nodes: Node[] }; + }; + }; + }; + + expect(captureScreen).toHaveBeenCalledTimes(3); + expect(driver.getAccessibilityTree).not.toHaveBeenCalled(); + expect(driver.getTestIds).not.toHaveBeenCalled(); + expect(screens[1]?.getTestIds).not.toHaveBeenCalled(); + expect( + body.observations.testIds.map((item) => item.testId), + ).toStrictEqual(['wallet']); + expect( + body.observations.a11y.nodes.map((node) => node.name), + ).toStrictEqual(['Wallet']); + }); + + it('does not wait when the screen changed', async () => { + await startServer(); + driver.getAccessibilityTree + .mockResolvedValueOnce(buildTree(['Login'])) + .mockResolvedValueOnce(buildTree(['Wallet'])); + + await post('/tool/click', { testId: 'x' }); + await post('/tool/click', { testId: 'x' }); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + + it('does not wait for a readonly tool that asked for an observation', async () => { + await startServer(); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + + await post('/tool/click', { testId: 'x' }); + await post('/tool/get_state', { observe: 'full' }); + + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + }); + + it('stops waiting and sets no refs when the request is aborted', async () => { + await startServer({ requestTimeoutMs: 150 }); + driver.getAccessibilityTree.mockResolvedValue(buildTree(['Same'])); + + addTapTool(); + + await post('/tool/test_tap'); + mockSM.setRefMap.mockClear(); + const { status } = await post('/tool/test_tap'); + await sleep(250); + + expect(status).toBe(500); + expect(driver.getAccessibilityTree).toHaveBeenCalledTimes(2); + expect(mockSM.setRefMap).not.toHaveBeenCalled(); + }); + }); +}); + +describe('screenAfterResponse', () => { + const tree = [{ ref: 'e1', role: 'button', name: 'Go', path: [] }]; + const other = [{ ref: 'e1', role: 'button', name: 'Back', path: [] }]; + const observation = { + testIds: [], + a11y: { nodes: other }, + } as unknown as StepRecordObservation; + + it.each(['launch', 'cleanup'])('returns null after %s', (toolName) => { + expect( + screenAfterResponse( + toolName, + { ok: true, result: {} }, + observation, + tree, + ), + ).toBeNull(); + }); + + it('prefers the delivered observation', () => { + expect( + screenAfterResponse( + 'describe_screen', + { ok: true, result: { a11y: { nodes: tree } } }, + observation, + null, + ), + ).toBe(other); + }); + + it('returns the describe_screen and accessibility_snapshot trees', () => { + expect( + screenAfterResponse( + 'describe_screen', + { ok: true, result: { a11y: { nodes: tree } } }, + undefined, + null, + ), + ).toBe(tree); + expect( + screenAfterResponse( + 'accessibility_snapshot', + { ok: true, result: { nodes: tree } }, + undefined, + null, + ), + ).toBe(tree); + }); + + it('ignores an accessibility_snapshot of a subtree', () => { + expect( + screenAfterResponse( + 'accessibility_snapshot', + { ok: true, result: { nodes: tree } }, + undefined, + other, + '#modal', + ), + ).toBe(other); + }); + + it('keeps the previous tree for an error or a call without a tree', () => { + expect( + screenAfterResponse( + 'describe_screen', + { ok: false, error: { code: 'X', message: 'x' } }, + undefined, + tree, + ), + ).toBe(tree); + expect( + screenAfterResponse( + 'get_state', + { ok: true, result: {} }, + undefined, + tree, + ), + ).toBe(tree); + }); +}); diff --git a/src/server/create-server.ts b/src/server/create-server.ts index 2e792a9..5ec69f1 100644 --- a/src/server/create-server.ts +++ b/src/server/create-server.ts @@ -3,9 +3,12 @@ import { execSync } from 'node:child_process'; import { randomUUID } from 'node:crypto'; import * as fs from 'node:fs/promises'; import * as http from 'node:http'; +import { setTimeout as delay } from 'node:timers/promises'; import { writeDaemonState, removeDaemonState } from './daemon-state.js'; import { compactObservation } from './observation-compaction.js'; +import { RefStore } from './ref-store.js'; +import type { RefHandle } from './ref-store.js'; import { RequestQueue } from './request-queue.js'; import { RequestTimer, @@ -13,15 +16,19 @@ import { setRequestStart, withMeta, } from './response-timing.js'; +import { isScreenBusy, screenSignature } from './screen-settle.js'; +import type { ISessionManager } from './session-manager.js'; import pkg from '../../package.json'; import type { PortMap, WorkflowContext } from '../capabilities/context.js'; import { PlaywrightPlatformDriver } from '../platform/playwright-driver.js'; +import type { IPlatformDriver, ScreenCapture } from '../platform/types.js'; import { toolRegistry, getToolCategory, checkPlatformGate, } from '../tools/registry.js'; import type { ToolCategory } from '../tools/registry.js'; +import type { A11yNodeTrimmed, TestIdItem } from '../tools/types/discovery.js'; import { createDefaultObservation } from '../tools/types/observation.js'; import type { StepRecordObservation } from '../tools/types/observation.js'; import type { RunStepsInput } from '../tools/types/tool-inputs.js'; @@ -30,6 +37,10 @@ import { ANIMATION_SETTLE_TIMEOUT_MS, GRACEFUL_SHUTDOWN_TIMEOUT_MS, MAX_IDLE_CHECK_INTERVAL_MS, + MOBILE_SETTLE_MAX_MS, + MOBILE_SETTLE_MAX_SNAPSHOTS, + MOBILE_SETTLE_MS, + MOBILE_SETTLE_UNCHANGED_RETRIES, OBSERVATION_TESTID_LIMIT, OBSERVATION_TIMEOUT_MS, } from '../tools/utils/constants.js'; @@ -57,6 +68,91 @@ export type ServerInstance = { stop(): Promise; }; +type ObservedTree = { + testIds: TestIdItem[]; + nodes: A11yNodeTrimmed[]; + refMap: Map; +}; + +/** + * Wraps the session manager so a tool's `sessionManager.setRefMap` and + * `clearRefMap` go through the request's RefStore handle (dropped when the + * request is stale or aborted). Every other member is read from the real session manager; + * methods are bound to it, so `this` (and private fields) work. + * + * @param sessionManager - The real session manager. + * @param refs - The request's RefStore handle. + * @returns The wrapped session manager. + */ +function routeSetRefMap( + sessionManager: ISessionManager, + refs: RefHandle, +): ISessionManager { + const setRefMap = (map: Map): void => { + refs.set(map); + }; + const clearRefMap = (): void => { + refs.set(new Map()); + }; + return new Proxy(sessionManager, { + get(target, property): unknown { + if (property === 'setRefMap') { + return setRefMap; + } + if (property === 'clearRefMap') { + return clearRefMap; + } + const value: unknown = Reflect.get(target, property, target); + return typeof value === 'function' ? value.bind(target) : value; + }, + }); +} + +/** + * Returns the a11y tree the agent holds after a response: the delivered + * observation's tree, else the tree a discovery tool returned. Launch and + * cleanup start or end a session, so nothing is held after them. Only called + * for delivered responses: a request that timed out never updates it. + * + * @param toolName - The tool name. + * @param toolResult - The tool response. + * @param observations - The delivered observation, if any. + * @param previous - The tree held before this response. + * @param rootSelector - The `rootSelector` of an `accessibility_snapshot`. + * @returns The tree held now, or null. + */ +export function screenAfterResponse( + toolName: string, + toolResult: ToolResponse, + observations: StepRecordObservation | undefined, + previous: A11yNodeTrimmed[] | null, + rootSelector?: unknown, +): A11yNodeTrimmed[] | null { + if (toolName === 'launch' || toolName === 'cleanup') { + return null; + } + if (observations) { + // The observation is the last tree in the response. + return observations.a11y.nodes; + } + if (toolResult.ok) { + const result = toolResult.result as + | { nodes?: unknown; a11y?: { nodes?: unknown } } + | undefined; + let nodes: unknown; + if (toolName === 'accessibility_snapshot') { + // A rootSelector snapshot is a subtree, not the screen. + nodes = rootSelector === undefined ? result?.nodes : undefined; + } else if (toolName === 'describe_screen') { + nodes = result?.a11y?.nodes; + } + if (Array.isArray(nodes)) { + return nodes as A11yNodeTrimmed[]; + } + } + return previous; +} + /** * Merges a tool result with observation data into the HTTP response body. * @@ -162,6 +258,7 @@ export function resolveObserveLevel( export function createServer(config: ServerConfig): ServerInstance { const app = express(); const queue = new RequestQueue(config.requestTimeoutMs); + const refStore = new RefStore(config.sessionManager); const nonce = randomUUID(); let httpServer: http.Server | null = null; @@ -175,6 +272,10 @@ export function createServer(config: ServerConfig): ServerInstance { let lastRequestTime = Date.now(); let idleCheckInterval: ReturnType | null = null; let lastObservation: StepRecordObservation | null = null; + // The last a11y tree the agent received (from an observation or from + // describe_screen / accessibility_snapshot). The mobile settle compares + // against it to tell whether the screen changed yet. + let lastScreen: A11yNodeTrimmed[] | null = null; // Registered before the body parser so `meta.timing.totalMs` includes body // parsing, and so malformed-JSON errors still have a start time. @@ -218,11 +319,21 @@ export function createServer(config: ServerConfig): ServerInstance { /** * Builds a lazy ToolContext where `page` and `refMap` are only accessed * when a tool actually reads them, avoiding throws for non-session tools. + * `refMap` reads the session manager on each access; writes from older or + * aborted requests never reach it (see RefStore), so it always holds the + * refs of this request or of the last request that completed. + * `sessionManager.setRefMap` writes through the request's RefStore handle. * * @param wfCtx - The current workflow context to embed in the tool context. + * @param signal - The request's abort signal. + * @param refs - The request's RefStore handle. * @returns A ToolContext with lazy page and refMap accessors. */ - function buildToolContext(wfCtx: WorkflowContext): ToolContext { + function buildToolContext( + wfCtx: WorkflowContext, + signal: AbortSignal, + refs: RefHandle, + ): ToolContext { const driver = config.sessionManager.hasActiveSession() ? (config.sessionManager.getPlatformDriver?.() ?? new PlaywrightPlatformDriver( @@ -232,7 +343,7 @@ export function createServer(config: ServerConfig): ServerInstance { : undefined; return { - sessionManager: config.sessionManager, + sessionManager: routeSetRefMap(config.sessionManager, refs), get page(): ReturnType { return config.sessionManager.getPage(); }, @@ -244,9 +355,227 @@ export function createServer(config: ServerConfig): ServerInstance { workflowContext: wfCtx, toolRegistry, driver, + signal, }; } + /** + * Takes one mobile snapshot: one device snapshot when the driver has + * `captureScreen`, else the tree and the test IDs separately. + * + * @param driver - The mobile platform driver. + * @returns The tree and a test ID reader. + */ + async function captureMobile( + driver: IPlatformDriver, + ): Promise { + if (driver.captureScreen) { + return driver.captureScreen(); + } + const tree = await driver.getAccessibilityTree(); + return { ...tree, getTestIds: async (limit) => driver.getTestIds(limit) }; + } + + /** + * Takes the mobile accessibility tree and test IDs. + * + * After a mutating or batch tool (`settle`), the screen may not have + * changed yet, or may show work in progress (for example a "busy" button + * while the wallet unlocks). Limits: + * + * - Busy (see `isScreenBusy`): wait `MOBILE_SETTLE_MS` and take the + * snapshot again, while the screen stays busy. + * - Unchanged (equals the last screen the agent received, ignoring refs) + * and not busy: retry at most `MOBILE_SETTLE_UNCHANGED_RETRIES` time(s). + * Many taps leave the screen as it is (for example tapping the tab that is + * already open), so more retries would only add device snapshots. On + * Android the status bar (clock, signal) usually differs between + * snapshots, so this rule rarely fires there. + * - In total: at most `MOBILE_SETTLE_MAX_SNAPSHOTS` extra snapshots, and no + * wait that would end later than `MOBILE_SETTLE_MAX_MS` after the first + * snapshot. A spinner that never stops costs at most these limits. + * + * The test IDs come from the final snapshot, so they describe the same + * screen as the tree. + * + * @param driver - The mobile platform driver. + * @param settle - Whether to settle (see above). + * @param signal - Aborted when the request or the observation times out. + * @returns The test IDs, nodes and refMap. + */ + async function snapshotMobile( + driver: IPlatformDriver, + settle: boolean, + signal: AbortSignal, + ): Promise { + let screen = await captureMobile(driver); + if (settle) { + const settleStartedAt = Date.now(); + const previous = lastScreen ? screenSignature(lastScreen) : null; + let unchangedRetries = 0; + const shouldRetry = (nodes: A11yNodeTrimmed[]): boolean => { + if (isScreenBusy(nodes)) { + return true; + } + if ( + previous === null || + unchangedRetries >= MOBILE_SETTLE_UNCHANGED_RETRIES || + screenSignature(nodes) !== previous + ) { + return false; + } + unchangedRetries += 1; + return true; + }; + for ( + let extra = 0; + extra < MOBILE_SETTLE_MAX_SNAPSHOTS && + Date.now() - settleStartedAt + MOBILE_SETTLE_MS <= + MOBILE_SETTLE_MAX_MS && + shouldRetry(screen.nodes); + extra++ + ) { + await delay(MOBILE_SETTLE_MS, undefined, { signal }); + screen = await captureMobile(driver); + } + } + signal.throwIfAborted(); + const testIds = await screen.getTestIds(OBSERVATION_TESTID_LIMIT); + return { testIds, nodes: screen.nodes, refMap: screen.refMap }; + } + + /** + * Takes the browser test IDs and accessibility tree. Tools that may change + * the screen (mutating, batch, and a cdp call that asked for an + * observation) wait for the page to settle first. + * + * @param category - The tool category. + * @param signal - Aborted when the request or the observation times out. + * @returns The test IDs, nodes and refMap. + */ + async function snapshotBrowser( + category: ToolCategory, + signal: AbortSignal, + ): Promise { + const page = config.sessionManager.getPage(); + if (category !== 'readonly' && category !== 'discovery') { + await page.waitForLoadState('domcontentloaded').catch(() => undefined); + signal.throwIfAborted(); + await page + .waitForFunction( + async () => + new Promise((resolve) => { + requestAnimationFrame(() => { + const allSettled = document + .getAnimations() + .every((a: Animation) => a.playState !== 'running'); + resolve(allSettled); + }); + }), + { timeout: ANIMATION_SETTLE_TIMEOUT_MS }, + ) + .catch(() => undefined); + signal.throwIfAborted(); + } + const testIds = await collectTestIds(page, OBSERVATION_TESTID_LIMIT); + signal.throwIfAborted(); + const tree = await collectTrimmedA11ySnapshot(page); + return { testIds, ...tree }; + } + + /** + * Collects the post-tool observation. Its refMap is applied through the + * request's RefStore handle, which drops the write when the request is + * stale or aborted. The observation is the last tree in the response, so + * its refs replace any refs the tool set: every ref the agent receives + * last resolves. + * + * @param context - The tool context of the request. + * @param toolName - The tool name. + * @param category - The tool category. + * @param refs - The request's RefStore handle. + * @param signal - Aborted when the request or the observation times out. + * @returns The observation. + */ + async function collectObservation( + context: ToolContext, + toolName: string, + category: ToolCategory, + refs: RefHandle, + signal: AbortSignal, + ): Promise { + // Re-read the driver from the session manager rather than using + // context.driver, which was captured before the tool ran. The launch + // tool creates the session (and sets the platform driver) during + // execution, so the context snapshot would still be undefined then. + const driver = + config.sessionManager.getPlatformDriver?.() ?? context.driver; + const mobile = driver !== undefined && driver.getPlatform() !== 'browser'; + // No settle when a (non-batch) tool set refs: it just read the screen. + // run_steps: a later step may have changed the screen after a step read + // it, so it still settles. + const settle = + (category === 'mutating' || category === 'batch') && + toolName !== 'launch' && + toolName !== 'cleanup' && + !(refs.toolSetRefs && category !== 'batch'); + const tree = mobile + ? await snapshotMobile(driver, settle, signal) + : await snapshotBrowser(category, signal); + signal.throwIfAborted(); + + refs.set(tree.refMap, 'observation'); + return createDefaultObservation(tree.testIds, tree.nodes); + } + + /** + * Runs collectObservation bounded by `OBSERVATION_TIMEOUT_MS`. On timeout + * or request abort the collection is aborted, so it stops at the next + * checkpoint and cannot set refs later. + * + * @param context - The tool context of the request. + * @param toolName - The tool name. + * @param category - The tool category. + * @param refs - The request's RefStore handle. + * @param requestSignal - The request's abort signal. + * @returns The observation, or undefined on failure or timeout. + */ + async function observeWithTimeout( + context: ToolContext, + toolName: string, + category: ToolCategory, + refs: RefHandle, + requestSignal: AbortSignal, + ): Promise { + const controller = new AbortController(); + const forwardAbort = (): void => controller.abort(requestSignal.reason); + requestSignal.addEventListener('abort', forwardAbort, { once: true }); + let timeoutId: ReturnType | undefined; + try { + return await Promise.race([ + collectObservation( + context, + toolName, + category, + refs, + controller.signal, + ), + new Promise((resolve) => { + timeoutId = setTimeout(() => { + controller.abort(new Error('Observation timed out')); + resolve(undefined); + }, OBSERVATION_TIMEOUT_MS); + }), + ]); + } catch { + // non-fatal: observation failure must not block the tool response + return undefined; + } finally { + clearTimeout(timeoutId); + requestSignal.removeEventListener('abort', forwardAbort); + } + } + /** * Shared tool executor — validates input, runs through the queue, * and collects observations. @@ -350,83 +679,32 @@ export function createServer(config: ServerConfig): ServerInstance { try { timer.markEnqueued(); const { toolResult, observeLevel, observations } = await queue.enqueue( - async () => { + async (signal) => { timer.markTaskStart(); - const context = buildToolContext(currentWorkflowContext); + const refs = refStore.begin(signal); + const context = buildToolContext( + currentWorkflowContext, + signal, + refs, + ); const result = await tool(omitObserve(validatedInput), context); timer.markToolEnd(); const level = resolveObserveLevel(category, inputRecord, result); let obs: StepRecordObservation | undefined; - if (level !== 'none' && config.sessionManager.hasActiveSession()) { + if ( + level !== 'none' && + !signal.aborted && + config.sessionManager.hasActiveSession() + ) { timer.markObservationStart(); - try { - obs = await Promise.race([ - (async (): Promise => { - // Re-read the driver from the session manager rather than - // using context.driver, which was captured before the tool - // ran. The launch tool creates the session (and sets the - // platform driver) during execution, so the context snapshot - // would still be undefined at that point. - const currentDriver = - config.sessionManager.getPlatformDriver?.() ?? - context.driver; - - if ( - currentDriver && - currentDriver.getPlatform() !== 'browser' - ) { - const testIds = await currentDriver.getTestIds( - OBSERVATION_TESTID_LIMIT, - ); - const { nodes, refMap: newRefMap } = - await currentDriver.getAccessibilityTree(); - config.sessionManager.setRefMap(newRefMap); - return createDefaultObservation(testIds, nodes); - } - - const page = config.sessionManager.getPage(); - - // Tools that may change the screen (mutating, batch, and a - // cdp call that asked for an observation) wait for the page - // to settle first. - if (category !== 'readonly' && category !== 'discovery') { - await page - .waitForLoadState('domcontentloaded') - .catch(() => undefined); - await page - .waitForFunction( - async () => - new Promise((resolve) => { - requestAnimationFrame(() => { - const allSettled = document - .getAnimations() - .every( - (a: Animation) => a.playState !== 'running', - ); - resolve(allSettled); - }); - }), - { timeout: ANIMATION_SETTLE_TIMEOUT_MS }, - ) - .catch(() => undefined); - } - const testIds = await collectTestIds( - page, - OBSERVATION_TESTID_LIMIT, - ); - const { nodes, refMap: newRefMap } = - await collectTrimmedA11ySnapshot(page); - config.sessionManager.setRefMap(newRefMap); - return createDefaultObservation(testIds, nodes); - })(), - new Promise((resolve) => - setTimeout(() => resolve(undefined), OBSERVATION_TIMEOUT_MS), - ), - ]); - } catch { - // non-fatal: observation failure must not block the tool response - } + obs = await observeWithTimeout( + context, + toolName, + category, + refs, + signal, + ); timer.markObservationEnd(); } @@ -464,6 +742,15 @@ export function createServer(config: ServerConfig): ServerInstance { } else if (observations) { lastObservation = observations; } + // Updated after the response, like lastObservation: only for responses + // the agent received. + lastScreen = screenAfterResponse( + toolName, + toolResult, + observations, + lastScreen, + inputRecord?.rootSelector, + ); } catch (error) { res.status(500).json( withMeta( diff --git a/src/server/ref-store.test.ts b/src/server/ref-store.test.ts new file mode 100644 index 0000000..bb6ad3e --- /dev/null +++ b/src/server/ref-store.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, vi } from 'vitest'; + +import { RefStore } from './ref-store.js'; + +function createSessionManager() { + return { setRefMap: vi.fn() }; +} + +describe('RefStore', () => { + it('increments the generation on each begin()', () => { + const store = new RefStore(createSessionManager()); + + const first = store.begin(); + const second = store.begin(); + + expect(first.generation).toBe(1); + expect(second.generation).toBe(2); + expect(store.generation).toBe(2); + }); + + it('forwards set() from the current handle to the session manager', () => { + const sessionManager = createSessionManager(); + const store = new RefStore(sessionManager); + const map = new Map([['e1', 'role=button[name="Go"]']]); + + const applied = store.begin().set(map); + + expect(applied).toBe(true); + expect(sessionManager.setRefMap).toHaveBeenCalledWith(map); + }); + + it('drops set() from a handle of an older generation', () => { + const sessionManager = createSessionManager(); + const store = new RefStore(sessionManager); + const old = store.begin(); + store.begin(); + + const applied = old.set(new Map([['e1', 'late']])); + + expect(applied).toBe(false); + expect(old.isCurrent()).toBe(false); + expect(sessionManager.setRefMap).not.toHaveBeenCalled(); + }); + + it('drops set() once the request signal is aborted', () => { + const sessionManager = createSessionManager(); + const store = new RefStore(sessionManager); + const controller = new AbortController(); + const handle = store.begin(controller.signal); + + expect(handle.isCurrent()).toBe(true); + controller.abort(); + + expect(handle.isCurrent()).toBe(false); + expect(handle.set(new Map([['e1', 'late']]))).toBe(false); + expect(sessionManager.setRefMap).not.toHaveBeenCalled(); + }); + + it('sets toolSetRefs only for applied tool writes', () => { + const store = new RefStore(createSessionManager()); + const handle = store.begin(); + + expect(handle.toolSetRefs).toBe(false); + handle.set(new Map(), 'observation'); + expect(handle.toolSetRefs).toBe(false); + handle.set(new Map()); + expect(handle.toolSetRefs).toBe(true); + + const stale = store.begin(); + store.begin(); + stale.set(new Map()); + expect(stale.toolSetRefs).toBe(false); + }); +}); diff --git a/src/server/ref-store.ts b/src/server/ref-store.ts new file mode 100644 index 0000000..e334df7 --- /dev/null +++ b/src/server/ref-store.ts @@ -0,0 +1,94 @@ +import type { ISessionManager } from './session-manager.js'; +import { debugWarn } from '../utils/logger.js'; + +/** + * A request's write access to the refMap. Get one from `RefStore.begin()`. + */ +export type RefHandle = { + /** The generation this handle was created with. */ + readonly generation: number; + /** + * True once refs were set through this handle during the request. The + * observation then leaves those refs in place (see `create-server.ts`). + */ + readonly toolSetRefs: boolean; + /** + * @returns True while no newer request has started and the request is not + * aborted. + */ + isCurrent(): boolean; + /** + * Sets the refMap. Dropped when the handle is not current. + * + * @param map - The new ref map. + * @param source - Who writes (`tool` or `observation`). Only `tool` writes + * set `toolSetRefs`. + * @returns True when the map reached the session manager. + */ + set(map: Map, source?: 'tool' | 'observation'): boolean; +}; + +/** + * Guards `sessionManager.setRefMap` with a request generation. + * + * Each request calls `begin()` when it starts. A write from an older request + * (for example one that timed out and kept running) or from an aborted + * request is dropped before it reaches the session manager, so it cannot + * replace the refs of the request that runs now. + */ +export class RefStore { + readonly #sessionManager: Pick; + + #generation = 0; + + /** + * @param sessionManager - The session manager that owns the refMap. + */ + constructor(sessionManager: Pick) { + this.#sessionManager = sessionManager; + } + + /** + * @returns The generation of the most recent request. + */ + get generation(): number { + return this.#generation; + } + + /** + * Starts a new generation. Handles from earlier generations stop writing. + * + * @param signal - The request's abort signal. + * @returns The handle for this request. + */ + begin(signal?: AbortSignal): RefHandle { + this.#generation += 1; + const generation = this.#generation; + let toolSetRefs = false; + + const isCurrent = (): boolean => + generation === this.#generation && signal?.aborted !== true; + + return { + generation, + get toolSetRefs(): boolean { + return toolSetRefs; + }, + isCurrent, + set: (map, source = 'tool'): boolean => { + if (!isCurrent()) { + debugWarn( + 'ref-store', + `dropped ${source} refMap from generation ${generation} (current ${this.#generation}, aborted ${String(signal?.aborted === true)})`, + ); + return false; + } + this.#sessionManager.setRefMap(map); + if (source === 'tool') { + toolSetRefs = true; + } + return true; + }, + }; + } +} diff --git a/src/server/request-queue.test.ts b/src/server/request-queue.test.ts index 2ff4dee..9ea4ad4 100644 --- a/src/server/request-queue.test.ts +++ b/src/server/request-queue.test.ts @@ -1,6 +1,10 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; import { RequestQueue } from './request-queue.js'; +import { + MAX_QUEUE_TIMEOUT_MS, + QUEUE_SETTLE_TIMEOUT_MS, +} from '../tools/utils/constants.js'; async function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); @@ -139,4 +143,171 @@ describe('RequestQueue', () => { expect(order).toStrictEqual([1, 2]); }); }); + + describe('abort signal', () => { + it('passes a signal that is not aborted on the normal path', async () => { + const queue = new RequestQueue(1_000); + let seen: AbortSignal | undefined; + + await queue.enqueue(async (signal) => { + seen = signal; + return 'ok'; + }); + + expect(seen).toBeInstanceOf(AbortSignal); + expect(seen?.aborted).toBe(false); + }); + + it('aborts the signal with the timeout error before rejecting', async () => { + const queue = new RequestQueue(20); + let seen: AbortSignal | undefined; + + const rejection = await queue + .enqueue(async (signal) => { + seen = signal; + return sleep(200); + }) + .catch((error: unknown) => error); + + expect(seen?.aborted).toBe(true); + expect(seen?.reason).toBe(rejection); + expect((rejection as Error).message).toBe( + 'Tool execution timed out after 20ms', + ); + }); + + it('lets a timed-out task stop at its next checkpoint', async () => { + const queue = new RequestQueue(20); + const steps: string[] = []; + + await queue + .enqueue(async (signal) => { + steps.push('step 1'); + await sleep(60); + if (signal.aborted) { + steps.push('stopped'); + return; + } + steps.push('step 2'); + }) + .catch(() => undefined); + + expect(steps).toStrictEqual(['step 1', 'stopped']); + }); + + it('gives each task its own signal', async () => { + const queue = new RequestQueue(20); + const signals: AbortSignal[] = []; + + await queue + .enqueue(async (signal) => { + signals.push(signal); + return sleep(60); + }) + .catch(() => undefined); + await queue.enqueue(async (signal) => { + signals.push(signal); + }); + + expect(signals[0].aborted).toBe(true); + expect(signals[1].aborted).toBe(false); + expect(signals[0]).not.toBe(signals[1]); + }); + + it('waits for the timed-out task to settle before the next task starts', async () => { + const queue = new RequestQueue(20); + const order: string[] = []; + + const first = queue + .enqueue(async () => { + await sleep(60); + order.push('first settled'); + }) + .catch(() => order.push('first rejected')); + const second = queue.enqueue(async () => { + order.push('second started'); + }); + + await Promise.all([first, second]); + + expect(order).toContain('first rejected'); + expect(order.indexOf('first settled')).toBeLessThan( + order.indexOf('second started'), + ); + }); + }); + + it.each([ + { + name: 'configured timeout', + configuredTimeout: 25, + requestedTimeout: undefined, + expectedTimeout: 25, + }, + { + name: 'positive override', + configuredTimeout: 100, + requestedTimeout: 25, + expectedTimeout: 25, + }, + ...[0, -1, NaN, -Infinity].map((requestedTimeout) => ({ + name: `invalid override ${requestedTimeout}`, + configuredTimeout: 25, + requestedTimeout, + expectedTimeout: 25, + })), + ...[MAX_QUEUE_TIMEOUT_MS, Number.MAX_SAFE_INTEGER, Infinity].map( + (requestedTimeout) => ({ + name: `bounded override ${requestedTimeout}`, + configuredTimeout: 25, + requestedTimeout, + expectedTimeout: MAX_QUEUE_TIMEOUT_MS, + }), + ), + ...[0, -1, NaN, -Infinity].map((configuredTimeout) => ({ + name: `invalid configured timeout ${configuredTimeout}`, + configuredTimeout, + requestedTimeout: undefined, + expectedTimeout: 30_000, + })), + ...[Number.MAX_SAFE_INTEGER, Infinity].map((configuredTimeout) => ({ + name: `bounded configured timeout ${configuredTimeout}`, + configuredTimeout, + requestedTimeout: undefined, + expectedTimeout: MAX_QUEUE_TIMEOUT_MS, + })), + ])( + 'bounds the timer for $name', + async ({ configuredTimeout, requestedTimeout, expectedTimeout }) => { + vi.useFakeTimers(); + try { + const queue = new RequestQueue(configuredTimeout); + let seen: AbortSignal | undefined; + const pending = queue + .enqueue( + async (signal) => { + seen = signal; + return new Promise(() => undefined); + }, + { timeoutMs: requestedTimeout }, + ) + .catch((error: unknown) => error); + await vi.advanceTimersByTimeAsync(expectedTimeout - 1); + expect(seen?.aborted).toBe(false); + + await vi.advanceTimersByTimeAsync(1); + expect(seen?.aborted).toBe(true); + + await vi.advanceTimersByTimeAsync(QUEUE_SETTLE_TIMEOUT_MS); + const error = await pending; + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe( + `Tool execution timed out after ${expectedTimeout}ms`, + ); + expect(seen?.reason).toBe(error); + } finally { + vi.useRealTimers(); + } + }, + ); }); diff --git a/src/server/request-queue.ts b/src/server/request-queue.ts index 5914174..ebceab5 100644 --- a/src/server/request-queue.ts +++ b/src/server/request-queue.ts @@ -1,6 +1,26 @@ -import { QUEUE_SETTLE_TIMEOUT_MS } from '../tools/utils/constants.js'; +import { + MAX_QUEUE_TIMEOUT_MS, + QUEUE_SETTLE_TIMEOUT_MS, +} from '../tools/utils/constants.js'; import { debugWarn } from '../utils'; +/** + * Resolves a queue timeout within the maximum timer duration. + * + * @param timeoutMs - The requested timeout. + * @param fallbackMs - A positive, bounded timeout for invalid requests. + * @returns The bounded timeout in milliseconds. + */ +function clampQueueTimeout(timeoutMs: number, fallbackMs: number): number { + if (!(timeoutMs > 0)) { + return fallbackMs; + } + if (timeoutMs <= MAX_QUEUE_TIMEOUT_MS) { + return timeoutMs; + } + return MAX_QUEUE_TIMEOUT_MS; +} + /** * Async mutex for serializing concurrent tool requests. * Ensures only one tool executes at a time. @@ -14,19 +34,25 @@ export class RequestQueue { * @param timeoutMs - Maximum milliseconds a queued task may run. */ constructor(timeoutMs = 30_000) { - this.#timeoutMs = timeoutMs; + this.#timeoutMs = clampQueueTimeout(timeoutMs, 30_000); } /** * Enqueues an async task for serial execution with a timeout. * - * @param fn - The async function to execute. + * Each task gets its own `AbortSignal`. When the task times out, the signal + * is aborted (with the timeout error as its reason) before the returned + * promise rejects, so the task can stop its remaining work. The queue still + * waits up to `QUEUE_SETTLE_TIMEOUT_MS` for the task to settle before the + * next task starts. + * + * @param fn - The async function to execute. It receives the task's signal. * @param options - Optional configuration for the enqueued task. * @param options.timeoutMs - Override timeout in milliseconds for this task. * @returns The resolved value of the provided function. */ async enqueue( - fn: () => Promise, + fn: (signal: AbortSignal) => Promise, options?: { timeoutMs?: number }, ): Promise { let release!: () => void; @@ -36,36 +62,40 @@ export class RequestQueue { const prev = this.#queue; this.#queue = next; await prev; - const effectiveTimeout = options?.timeoutMs ?? this.#timeoutMs; + const effectiveTimeout = clampQueueTimeout( + options?.timeoutMs ?? this.#timeoutMs, + this.#timeoutMs, + ); + const controller = new AbortController(); let timer: ReturnType | undefined; - const fnPromise = fn(); + const fnPromise = fn(controller.signal); try { return await Promise.race([ fnPromise, new Promise((_resolve, reject) => { - timer = setTimeout( - () => - reject( - new Error( - `Tool execution timed out after ${effectiveTimeout}ms`, - ), - ), - effectiveTimeout, - ); + timer = setTimeout(() => { + const error = new Error( + `Tool execution timed out after ${effectiveTimeout}ms`, + ); + controller.abort(error); + reject(error); + }, effectiveTimeout); }), ]); } finally { if (timer !== undefined) { clearTimeout(timer); } + let settleTimer: ReturnType | undefined; await Promise.race([ fnPromise.catch((error) => { debugWarn('request-queue.enqueue', error); }), - new Promise((resolve) => - setTimeout(resolve, QUEUE_SETTLE_TIMEOUT_MS), - ), + new Promise((resolve) => { + settleTimer = setTimeout(resolve, QUEUE_SETTLE_TIMEOUT_MS); + }), ]); + clearTimeout(settleTimer); release(); } } diff --git a/src/server/screen-settle.test.ts b/src/server/screen-settle.test.ts new file mode 100644 index 0000000..07f92d1 --- /dev/null +++ b/src/server/screen-settle.test.ts @@ -0,0 +1,83 @@ +import { describe, it, expect } from 'vitest'; + +import { isBusyNode, isScreenBusy, screenSignature } from './screen-settle.js'; +import type { A11yNodeTrimmed } from '../tools/types/discovery.js'; + +const node = ( + role: string, + name: string, + extra: Partial = {}, +): A11yNodeTrimmed => ({ ref: 'e1', role, name, path: [], ...extra }); + +describe('isBusyNode', () => { + it('matches the React Native busy label (Android unlock proof)', () => { + // From proof/p1-08/android/click-unlock.txt: the login button while the + // wallet unlocks. + expect( + isBusyNode( + node('android.widget.Button', 'Unlock, busy', { + disabled: true, + testId: 'log-in-button', + }), + ), + ).toBe(true); + }); + + it('matches the busy label on iOS element types', () => { + expect(isBusyNode(node('Button', 'Unlock, busy'))).toBe(true); + expect(isBusyNode(node('Button', 'Confirm,busy'))).toBe(true); + expect(isBusyNode(node('Button', 'Send, Busy'))).toBe(true); + }); + + it('matches Android progress bars', () => { + expect(isBusyNode(node('android.widget.ProgressBar', ''))).toBe(true); + expect( + isBusyNode(node('androidx.core.widget.ContentLoadingProgressBar', '')), + ).toBe(true); + }); + + it('matches iOS activity and progress indicators (Appium and idb types)', () => { + expect(isBusyNode(node('ActivityIndicator', ''))).toBe(true); + expect(isBusyNode(node('ProgressIndicator', ''))).toBe(true); + expect(isBusyNode(node('UIActivityIndicatorView', ''))).toBe(true); + }); + + it('does not match other nodes', () => { + expect(isBusyNode(node('ActivityIndicatorContainer', ''))).toBe(false); + expect(isBusyNode(node('StaticText', 'ProgressIndicator'))).toBe(false); + expect(isBusyNode(node('android.widget.Button', 'Unlock'))).toBe(false); + expect(isBusyNode(node('android.widget.TextView', 'busy'))).toBe(false); + expect(isBusyNode(node('android.widget.TextView', 'Busy day'))).toBe(false); + expect(isBusyNode(node('android.widget.TextView', 'Not busy now'))).toBe( + false, + ); + // The spinner in the proof has no busy role or label; the button does. + expect( + isBusyNode( + node('android.view.ViewGroup', '', { testId: 'spinner-animated-view' }), + ), + ).toBe(false); + expect(isBusyNode(node('_UIScrollViewScrollIndicator', ''))).toBe(false); + }); +}); + +describe('isScreenBusy', () => { + it('is true when any node is busy', () => { + expect( + isScreenBusy([node('Text', 'Welcome'), node('Button', 'Unlock, busy')]), + ).toBe(true); + expect(isScreenBusy([node('Text', 'Welcome')])).toBe(false); + expect(isScreenBusy([])).toBe(false); + }); +}); + +describe('screenSignature', () => { + it('ignores refs', () => { + expect(screenSignature([{ ...node('Button', 'Go'), ref: 'e1' }])).toBe( + screenSignature([{ ...node('Button', 'Go'), ref: 'e9' }]), + ); + expect(screenSignature([node('Button', 'Go')])).not.toBe( + screenSignature([node('Button', 'Stop')]), + ); + }); +}); diff --git a/src/server/screen-settle.ts b/src/server/screen-settle.ts new file mode 100644 index 0000000..d7c8182 --- /dev/null +++ b/src/server/screen-settle.ts @@ -0,0 +1,56 @@ +import type { A11yNodeTrimmed } from '../tools/types/discovery.js'; + +/** + * React Native adds ", busy" to the accessibility label of an element with + * `accessibilityState={{ busy: true }}`. Seen on Android as the login + * button `"Unlock, busy"` while the wallet unlocks. + */ +const BUSY_LABEL = /,\s*busy$/iu; + +/** + * Roles of progress indicators, matched as a suffix of the node role (the + * mobile driver uses the backend element type as the role, see + * `normalizeSnapshot`): + * + * - Android (adb/Appium `class`): `android.widget.ProgressBar` and + * subclasses whose class name ends in `ProgressBar`. + * - iOS via Appium XCUITest: `XCUIElementTypeActivityIndicator` and + * `XCUIElementTypeProgressIndicator`, reported without the + * `XCUIElementType` prefix (`ActivityIndicator`, `ProgressIndicator`). + * - iOS via idb `describe-all`: the element `type`, either the AX role + * without `AX` (`ProgressIndicator`) or the UIKit class name + * (`UIActivityIndicatorView`, used by React Native `ActivityIndicator`). + */ +const BUSY_ROLE = + /(?:ProgressBar|ActivityIndicator(?:View)?|ProgressIndicator)$/u; + +/** + * Builds a ref-independent signature of a screen, used to tell whether the + * screen changed. + * + * @param nodes - The accessibility nodes. + * @returns The signature. + */ +export function screenSignature(nodes: A11yNodeTrimmed[]): string { + return JSON.stringify(nodes.map(({ ref: _ref, ...rest }) => rest)); +} + +/** + * Checks whether a node shows that work is in progress: its label ends in + * ", busy" (React Native busy state), or it is a progress indicator (see + * `BUSY_ROLE`). + * + * @param node - The accessibility node. + * @returns True when the node is busy. + */ +export function isBusyNode(node: A11yNodeTrimmed): boolean { + return BUSY_LABEL.test(node.name) || BUSY_ROLE.test(node.role); +} + +/** + * @param nodes - The accessibility nodes of a screen. + * @returns True when any node is busy (see `isBusyNode`). + */ +export function isScreenBusy(nodes: A11yNodeTrimmed[]): boolean { + return nodes.some(isBusyNode); +} diff --git a/src/tools/batch.test.ts b/src/tools/batch.test.ts index 742d4cd..9706b66 100644 --- a/src/tools/batch.test.ts +++ b/src/tools/batch.test.ts @@ -854,4 +854,34 @@ describe('runStepsTool', () => { expect(hermesTargetsHandler).toHaveBeenCalled(); } }); + + it('stops before the next step when the request is aborted', async () => { + const controller = new AbortController(); + const clickHandler = vi.fn().mockImplementation(async () => { + controller.abort(); + return { ok: true, result: 'clicked' }; + }); + const context = { + ...createMockContext({ + toolRegistry: new Map([['click', clickHandler]]), + }), + signal: controller.signal, + } as ToolContext; + + const result = await runStepsTool( + { + steps: [ + { tool: 'click', args: { testId: 'a' } }, + { tool: 'click', args: { testId: 'b' } }, + ], + }, + context, + ); + + expect(clickHandler).toHaveBeenCalledTimes(1); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.code).toBe(ErrorCodes.MM_REQUEST_ABORTED); + } + }); }); diff --git a/src/tools/batch.ts b/src/tools/batch.ts index c4a44bc..6e575b7 100644 --- a/src/tools/batch.ts +++ b/src/tools/batch.ts @@ -1,7 +1,11 @@ import { checkPlatformGate } from './registry.js'; import type { RunStepsInput, RunStepsResult, StepResult } from './types'; import { ErrorCodes } from './types'; -import { createToolError, createToolSuccess } from './utils.js'; +import { + abortedResponse, + createToolError, + createToolSuccess, +} from './utils.js'; import type { ToolContext, ToolFunction, ToolResponse } from '../types/http.js'; import { extractErrorMessage } from '../utils'; import type { ToolName } from '../validation/schemas.js'; @@ -125,6 +129,10 @@ export async function runStepsTool( : undefined; for (const stepInput of stepInputs) { + const aborted = abortedResponse(context); + if (aborted) { + return aborted; + } if (batchDeadline && Date.now() > batchDeadline) { const remainingIndex = stepInputs.indexOf(stepInput); for (const remaining of stepInputs.slice(remainingIndex)) { diff --git a/src/tools/discovery-tools.test.ts b/src/tools/discovery-tools.test.ts index 3e5fe1b..e86c07c 100644 --- a/src/tools/discovery-tools.test.ts +++ b/src/tools/discovery-tools.test.ts @@ -105,21 +105,22 @@ describe('discovery-tools', () => { ); }); - it('updates refMap in session manager', async () => { + it('does not take an a11y snapshot or change the refMap (no refs in its result)', async () => { const context = createMockContext(); - const mockRefMap = new Map([['e1', 'role=button[name="Submit"]']]); + const setRefMap = vi.fn(); vi.spyOn(discoveryModule, 'collectTestIds').mockResolvedValue([]); - vi.spyOn(discoveryModule, 'collectTrimmedA11ySnapshot').mockResolvedValue( - { - nodes: [], - refMap: mockRefMap, - }, - ); + const snapshot = vi.spyOn(discoveryModule, 'collectTrimmedA11ySnapshot'); - await listTestIdsTool({}, context); + const result = await listTestIdsTool({}, { + ...context, + setRefMap, + } as ToolContext); - expect(context.sessionManager.setRefMap).toHaveBeenCalledWith(mockRefMap); + expect(result).toStrictEqual({ ok: true, result: { items: [] } }); + expect(snapshot).not.toHaveBeenCalled(); + expect(setRefMap).not.toHaveBeenCalled(); + expect(context.sessionManager.setRefMap).not.toHaveBeenCalled(); }); it('returns error when no active session', async () => { @@ -423,6 +424,75 @@ describe('discovery-tools', () => { expect(context.sessionManager.setRefMap).toHaveBeenCalledWith(mockRefMap); }); + it('uses one captureScreen snapshot for test IDs and tree when the driver has it', async () => { + const context = createMockContext(); + const screenNodes: A11yNodeTrimmed[] = [ + { ref: 'e1', role: 'Button', name: 'Send', path: [] }, + ]; + const screenRefMap = new Map([['e1', 'label:Send|type:Button']]); + const screenTestIds: TestIdItem[] = [ + { testId: 'wallet-screen', tag: 'View', visible: true }, + ]; + const getTestIds = vi.fn().mockResolvedValue(screenTestIds); + const captureScreen = vi.fn().mockResolvedValue({ + nodes: screenNodes, + refMap: screenRefMap, + getTestIds, + }); + const driver = { + getAppState: vi.fn().mockResolvedValue({ isLoaded: true }), + getTestIds: vi.fn(), + getAccessibilityTree: vi.fn(), + captureScreen, + }; + + const result = await describeScreenTool({}, { + ...context, + driver, + } as unknown as ToolContext); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.result.testIds.items).toStrictEqual(screenTestIds); + expect(result.result.a11y.nodes).toStrictEqual(screenNodes); + } + expect(captureScreen).toHaveBeenCalledTimes(1); + expect(getTestIds).toHaveBeenCalledWith(150); + expect(driver.getTestIds).not.toHaveBeenCalled(); + expect(driver.getAccessibilityTree).not.toHaveBeenCalled(); + expect(context.sessionManager.setRefMap).toHaveBeenCalledWith( + screenRefMap, + ); + }); + + it('stops after captureScreen when the request is aborted', async () => { + const controller = new AbortController(); + const context = createMockContext(); + const driver = { + getAppState: vi.fn().mockResolvedValue({ isLoaded: true }), + captureScreen: vi.fn().mockResolvedValue({ + nodes: [], + refMap: new Map(), + getTestIds: vi.fn(async () => { + controller.abort(); + return []; + }), + }), + }; + + const result = await describeScreenTool({}, { + ...context, + driver, + signal: controller.signal, + } as unknown as ToolContext); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.code).toBe(ErrorCodes.MM_REQUEST_ABORTED); + } + expect(context.sessionManager.setRefMap).not.toHaveBeenCalled(); + }); + it('returns error when no active session', async () => { const context = createMockContext({ hasActive: false }); @@ -449,4 +519,68 @@ describe('discovery-tools', () => { } }); }); + + describe('refs and abort', () => { + const nodes: A11yNodeTrimmed[] = [ + { ref: 'e1', role: 'button', name: 'Go', path: [] }, + ]; + const refMap = new Map([['e1', 'role=button[name="Go"]']]); + + beforeEach(() => { + vi.spyOn(discoveryModule, 'collectTestIds').mockResolvedValue([]); + vi.spyOn(discoveryModule, 'collectTrimmedA11ySnapshot').mockResolvedValue( + { nodes, refMap }, + ); + }); + + const nodesOf = (result: unknown): A11yNodeTrimmed[] | undefined => { + const value = (result as { result?: Record }).result; + return ( + (value?.nodes as A11yNodeTrimmed[] | undefined) ?? + (value?.a11y as { nodes: A11yNodeTrimmed[] } | undefined)?.nodes + ); + }; + + it.each([ + ['accessibility_snapshot', accessibilitySnapshotTool], + ['describe_screen', describeScreenTool], + ] as const)( + '%s sets refs through sessionManager.setRefMap and returns the tree nodes', + async (_name, tool) => { + const context = createMockContext(); + const result = await tool({}, context); + + expect(context.sessionManager.setRefMap).toHaveBeenCalledWith(refMap); + expect(nodesOf(result)).toStrictEqual(nodes); + }, + ); + + it.each([['describe_screen', describeScreenTool]] as const)( + '%s stops before the a11y snapshot when the request is aborted', + async (_name, tool) => { + const controller = new AbortController(); + const context = createMockContext(); + vi.spyOn(discoveryModule, 'collectTestIds').mockImplementation( + async () => { + controller.abort(); + return []; + }, + ); + + const result = await tool({}, { + ...context, + signal: controller.signal, + } as ToolContext); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.code).toBe(ErrorCodes.MM_REQUEST_ABORTED); + } + expect( + discoveryModule.collectTrimmedA11ySnapshot, + ).not.toHaveBeenCalled(); + expect(context.sessionManager.setRefMap).not.toHaveBeenCalled(); + }, + ); + }); }); diff --git a/src/tools/discovery-tools.ts b/src/tools/discovery-tools.ts index 1d690d7..676fa82 100644 --- a/src/tools/discovery-tools.ts +++ b/src/tools/discovery-tools.ts @@ -10,6 +10,7 @@ import type { import { ErrorCodes } from './types'; import { DEFAULT_TESTID_LIMIT } from './utils/constants.js'; import { + abortedResponse, createToolError, createToolSuccess, requireActiveSession, @@ -61,10 +62,9 @@ export async function listTestIdsTool( const limit = input.limit ?? DEFAULT_TESTID_LIMIT; + // The result has no refs, so the refMap is left as it is. try { const items = await driver.getTestIds(limit); - const { refMap } = await driver.getAccessibilityTree(); - context.sessionManager.setRefMap(refMap); return createToolSuccess({ items }); } catch (error) { const errorInfo = classifyDiscoveryError(error); @@ -129,9 +129,19 @@ export async function describeScreenTool( try { const state = await driver.getAppState(); - const testIds = await driver.getTestIds(DEFAULT_TESTID_LIMIT); - const { nodes, refMap } = await driver.getAccessibilityTree(); - + // Mobile: one device snapshot for both the test IDs and the tree, when + // the driver supports it. + const screen = driver.captureScreen + ? await driver.captureScreen() + : undefined; + const testIds = screen + ? await screen.getTestIds(DEFAULT_TESTID_LIMIT) + : await driver.getTestIds(DEFAULT_TESTID_LIMIT); + const aborted = abortedResponse(context); + if (aborted) { + return aborted; + } + const { nodes, refMap } = screen ?? (await driver.getAccessibilityTree()); context.sessionManager.setRefMap(refMap); const trackedPages = context.sessionManager.getTrackedPages(); diff --git a/src/tools/driver-delegation.test.ts b/src/tools/driver-delegation.test.ts index 31b15d8..df2c349 100644 --- a/src/tools/driver-delegation.test.ts +++ b/src/tools/driver-delegation.test.ts @@ -392,7 +392,8 @@ describe('tool delegation to context.driver', () => { expect(result.ok).toBe(true); expect(driver.getTestIds).toHaveBeenCalledOnce(); - expect(driver.getAccessibilityTree).toHaveBeenCalledOnce(); + // Its result has no refs, so it does not read the a11y tree. + expect(driver.getAccessibilityTree).not.toHaveBeenCalled(); }); it('accessibilitySnapshotTool delegates to driver.getAccessibilityTree', async () => { diff --git a/src/tools/interaction.test.ts b/src/tools/interaction.test.ts index 2cba370..9d9c1c1 100644 --- a/src/tools/interaction.test.ts +++ b/src/tools/interaction.test.ts @@ -1240,4 +1240,45 @@ describe('interaction', () => { } }); }); + + describe('abort', () => { + it.each([ + [ + 'click', + async (context: ToolContext) => clickTool({ testId: 'a' }, context), + ], + [ + 'type', + async (context: ToolContext) => + typeTool({ testId: 'a', text: 'x' }, context), + ], + [ + 'wait_for', + async (context: ToolContext) => waitForTool({ testId: 'a' }, context), + ], + [ + 'get_text', + async (context: ToolContext) => getTextTool({ testId: 'a' }, context), + ], + ])( + '%s returns MM_REQUEST_ABORTED without touching the page when aborted', + async (_name, run) => { + const controller = new AbortController(); + controller.abort(); + const context = { + ...createMockContext(), + signal: controller.signal, + } as ToolContext; + const waitSpy = vi.spyOn(discoveryModule, 'waitForTarget'); + + const result = await run(context); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.code).toBe(ErrorCodes.MM_REQUEST_ABORTED); + } + expect(waitSpy).not.toHaveBeenCalled(); + }, + ); + }); }); diff --git a/src/tools/interaction.ts b/src/tools/interaction.ts index cb2fbc9..7dcc3b4 100644 --- a/src/tools/interaction.ts +++ b/src/tools/interaction.ts @@ -24,6 +24,7 @@ import { isValidTargetSelection, } from './utils/type-guards.js'; import { + abortedResponse, createToolError, createToolSuccess, requireActiveSession, @@ -58,6 +59,11 @@ function validateInteraction( return { error: missingSession }; } + const aborted = abortedResponse(context); + if (aborted) { + return { error: aborted }; + } + if (!context.driver) { return { error: createToolError( diff --git a/src/tools/types/errors.ts b/src/tools/types/errors.ts index 5aac6a3..ed42a79 100644 --- a/src/tools/types/errors.ts +++ b/src/tools/types/errors.ts @@ -49,6 +49,8 @@ export const ErrorCodes = { MM_TOOL_NOT_SUPPORTED_ON_PLATFORM: 'MM_TOOL_NOT_SUPPORTED_ON_PLATFORM', // tool gated off the active platform (browser-only / mobile-only) + MM_REQUEST_ABORTED: 'MM_REQUEST_ABORTED', // the request timed out; the tool stopped at its next checkpoint + MM_UNKNOWN_TOOL: 'MM_UNKNOWN_TOOL', MM_INTERNAL_ERROR: 'MM_INTERNAL_ERROR', } as const; diff --git a/src/tools/utils.ts b/src/tools/utils.ts index 6eae6a2..37edfc0 100644 --- a/src/tools/utils.ts +++ b/src/tools/utils.ts @@ -51,3 +51,22 @@ export function requireActiveSession( return undefined; } + +/** + * Returns an error response when the request was aborted (it timed out). + * Tools call it between steps so a timed-out request stops early. + * + * @param context - The tool execution context. + * @returns An error response when aborted, or undefined. + */ +export function abortedResponse( + context: ToolContext, +): ToolResponse | undefined { + if (context.signal?.aborted) { + return createToolError( + ErrorCodes.MM_REQUEST_ABORTED, + 'The request timed out and was cancelled.', + ); + } + return undefined; +} diff --git a/src/tools/utils/constants.ts b/src/tools/utils/constants.ts index a9a5204..686e11e 100644 --- a/src/tools/utils/constants.ts +++ b/src/tools/utils/constants.ts @@ -13,6 +13,9 @@ export const DEFAULT_INTERACTION_TIMEOUT_MS = 15000; /** Maximum time the RequestQueue finally block waits for a hung task to settle */ export const QUEUE_SETTLE_TIMEOUT_MS = 5_000; +/** Upper bound for any queued task timeout (largest schema timeout + budget). */ +export const MAX_QUEUE_TIMEOUT_MS = 600_000; + /** Timeout for waiting for CSS/Web animations to settle after a mutation */ export const ANIMATION_SETTLE_TIMEOUT_MS = 3_000; @@ -22,6 +25,29 @@ export const MAX_IDLE_CHECK_INTERVAL_MS = 60_000; /** Hard timeout for post-tool observation collection (testIds, a11y) */ export const OBSERVATION_TIMEOUT_MS = 8_000; +/** + * Mobile observation settle: after a mutating or batch tool, when a snapshot + * still shows the last screen the agent received or a busy element, wait + * this long and take the snapshot again. + */ +export const MOBILE_SETTLE_MS = 300; + +/** + * Mobile observation settle: no wait starts that would end later than this + * after the first snapshot (waits and snapshots both count). + */ +export const MOBILE_SETTLE_MAX_MS = 2_000; + +/** Mobile observation settle: maximum number of extra snapshots. */ +export const MOBILE_SETTLE_MAX_SNAPSHOTS = 3; + +/** + * Mobile observation settle: extra snapshots allowed because the screen + * still equals the last screen the agent received (no busy element). Busy + * screens are retried up to `MOBILE_SETTLE_MAX_SNAPSHOTS`. + */ +export const MOBILE_SETTLE_UNCHANGED_RETRIES = 1; + /** Timeout for graceful HTTP server close during daemon shutdown */ export const GRACEFUL_SHUTDOWN_TIMEOUT_MS = 10_000; diff --git a/src/types/http.ts b/src/types/http.ts index ff73d91..5322fd4 100644 --- a/src/types/http.ts +++ b/src/types/http.ts @@ -29,6 +29,11 @@ export type ToolContext = { toolRegistry: Map>; /** Platform driver for cross-platform element interaction (browser, iOS, Android) */ driver?: IPlatformDriver; + /** + * Aborted when the request times out. Tools check it between steps and + * stop. Absent when the context was not built by the daemon. + */ + signal?: AbortSignal; }; /**