diff --git a/README.md b/README.md index 91a7e67..bcde817 100644 --- a/README.md +++ b/README.md @@ -410,7 +410,7 @@ The daemon routes `POST /tool/:name` requests through the registry, applies Zod | `get_contract_address` | Looks up the deployed address of a contract by name from the session's deployment registry. | | `list_contracts` | Lists all contracts deployed in the current session with addresses and deployment timestamps. | | **Batching** | | -| `run_steps` | Executes a batch of tool invocations sequentially. Supports `stopOnError` to halt on first failure, a top-level `observe` level for the single observation collected after the batch (default `diff`; without one, the last executed step's `observe` is used), and `batchTimeoutMs` to set an overall deadline (remaining steps are skipped on timeout). Accepts tool aliases like `navigate_home` / `navigate-home`. Returns per-step results with timing. | +| `run_steps` | Executes a batch of tool invocations sequentially. Supports `stopOnError` to halt on first failure, a top-level `observe` level for the single observation collected after the batch (default `diff`; without one, the last executed step's `observe` is used), and `batchTimeoutMs` (1 000–30 000 ms) to set an overall deadline (checked between steps; remaining steps are skipped). The whole `run_steps` call has the default 30 s request limit, so to get partial results set `batchTimeoutMs` below it (for example 20 000) and keep batches short. Accepts tool aliases like `navigate_home` / `navigate-home`. Returns per-step results with timing. | | **Advanced** | | | `mock_network` | Adds, clears, lists, and inspects targeted Playwright network mocks on the active browser context. Unmatched same-origin requests are continued unchanged. | | `cdp` | Sends a raw Chrome DevTools Protocol command, dispatched through the active platform driver. **Browser:** targets the page's Chrome CDP session (full Runtime/DOM/Network/Page surface); destructive methods (`Browser.close`, `Target.closeTarget`, etc.) are blocked. **Mobile (iOS/Android):** targets the app's React Native Hermes JS runtime via Metro's inspector proxy (delegates to `@metamask/device-mcp`, needs a DEBUG build with Metro running); only the JS-engine subset exists (`Runtime`, `Debugger`, `Log`, `HeapProfiler` — no DOM/Page/Network) and `Runtime.terminateExecution` / `Inspector.detached` are blocked. Optional `metroPort` / `appId` are mobile-only (ignored on browser). Categorized as `cdp`: no observation is collected and refs are not refreshed, so run `describe_screen` after a call that changes state. See [Browser vs Mobile CDP](#browser-vs-mobile-hermes-cdp). | @@ -549,11 +549,15 @@ Every tool input (and the `/launch` and `/cleanup` bodies) accepts an optional ` When `observe` is absent, the default comes from the tool category: `diff` for **mutating** tools (click, type, navigate, launch, cleanup, build, etc.) and `run_steps`; `none` for **read-only**, **discovery** and `cdp` tools. An explicit `diff` or `full` on a read-only, discovery or `cdp` tool collects and returns an observation. +`observe: 'none'` does not refresh the refs. After a call with `none` that changes the screen, the refs from the last observation or `describe_screen` still point at the previous screen. Refs backed by a test ID or an element id fail with `MM_TARGET_NOT_FOUND` when the element is gone. Refs built from role and name, a DOM path, or a mobile label can match a different element on the new screen. After `none`, target by test ID, or call `describe_screen` (or use `observe: 'full'`) before using refs again. + +On the browser, only mutating, batch and `cdp` calls wait for animations to settle (up to 1 s; every running finite animation counts, also off screen, so a long finite animation adds up to 1 s) before the observation. A read-only or discovery call with `observe: 'diff'` or `'full'` (and `describe_screen`) reads the page at once, so right after a `none` mutation it can catch an animation in progress. To get a settled observation, let the mutation itself observe (`diff` or `full`) instead of chaining it with `none`. + For `run_steps`, the top-level `observe` sets the level of the one observation collected after the batch. Without it, the `observe` in the last executed step's `args` is used; per-step `observe` on other steps is ignored. `includeObservations` was removed. 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. +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. `click` and `type` check the signal again after they wait for the target, so a call that timed out during that wait does not click or type. An action that already started is not cancelled. On the browser, the action includes Playwright's own checks inside `locator.click` / `locator.fill` (stable, enabled, not covered), which can wait up to the rest of the call's `timeoutMs`. On mobile, `@metamask/device-mcp` takes no signal: its element wait, tap and `typeText` run until they end, and the driver checks the signal between them. ## CLI Reference diff --git a/SKILL.md b/SKILL.md index 81d9f39..f6be665 100644 --- a/SKILL.md +++ b/SKILL.md @@ -51,6 +51,10 @@ mm navigate-home --observe full # after navigation: get the whole screen mm get-state --observe full # a read that also returns an observation ``` +**After `--observe none`, refs are not refreshed.** If the call changed the screen, the refs you have still describe the previous screen. A ref backed by a testId fails loudly (`MM_TARGET_NOT_FOUND`), but a ref built from role and name, a DOM path or a mobile label can click a **different element**. After `--observe none`, target by `--testid`, or run `mm describe-screen` before you use refs again. + +**Only mutating and batch calls wait for animations** (browser, up to 1 s) before they observe. `mm describe-screen` and a read with `--observe full` read the page at once, so `mm click … --observe none && mm get-state --observe full` can catch a screen mid-animation. To get a settled screen after an action, let the action observe: `mm click … --observe full`. + Observations contain `testIds` and `a11y` only — no app `state`. Call `mm get-state` for state. On mobile, each observation and `describe-screen` comes from one device snapshot. The Android system status bar and navigation bar and the Expo DevLauncher "Tools" button (iOS `gearshape.fill`) are not part of the app and are removed from `testIds` and `a11y`. Other system windows, such as the biometric prompt (`com.android.systemui:id/biometric_*`), are kept. @@ -91,7 +95,7 @@ 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. 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. +**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. A `click` or `type` that timed out while waiting for its target does not click or type. An action that already started may still have happened (browser: Playwright's checks inside the click; mobile: a tap in progress is not stopped), so run `describe-screen` before retrying. **Quick reference:** @@ -468,7 +472,7 @@ Executes multiple tool invocations in sequence from a JSON array. Each step spec mm run-steps '{"steps":[{"tool":"click","args":{"a11yRef":"e3"}},{"tool":"wait_for","args":{"a11yRef":"e5"}}]}' ``` -Supports `stopOnError` (halt on first failure) and returns per-step results with timing. A top-level `observe` (`none` / `diff` / `full`, default `diff`) sets the observation collected after the batch; see `run_steps` and `observe` above. Use `batchTimeoutMs` to set an overall deadline — if exceeded, remaining steps are marked as skipped and partial results are returned immediately. The summary includes a `skipped` count alongside `succeeded` and `failed`. +Supports `stopOnError` (halt on first failure) and returns per-step results with timing. A top-level `observe` (`none` / `diff` / `full`, default `diff`) sets the observation collected after the batch; see `run_steps` and `observe` above. Use `batchTimeoutMs` (1000–30000 ms) to set an overall deadline — it is checked between steps; if exceeded, remaining steps are marked as skipped and partial results are returned. The whole `run_steps` call is stopped after 30 s (the request limit), so use a lower `batchTimeoutMs` (for example 20000) to get partial results, and split longer flows into several batches. The summary includes a `skipped` count alongside `succeeded` and `failed`. Tool aliases are supported in steps: `navigate_home` / `navigate-home`, `navigate_settings` / `navigate-settings`, and `navigate_notification` / `navigate-notification` resolve to `navigate` with the appropriate `screen` argument. You can also use `ref` as shorthand for `a11yRef` in step args and within targets. diff --git a/src/cli/mm.ts b/src/cli/mm.ts index 8b60472..f19d5f5 100644 --- a/src/cli/mm.ts +++ b/src/cli/mm.ts @@ -1866,7 +1866,9 @@ Global Options: Overrides MM_PROJECT and git-based discovery. --observe Observation returned after the call: none | diff | full. Default: diff for mutating commands and run-steps, - none for reads, discovery and cdp. + none for reads, discovery and cdp. none does not + refresh refs: run describe-screen before using refs + again. --pretty Indent JSON output. Default: one line per result when stdout is not a terminal (agents, pipes), indented on a terminal. diff --git a/src/platform/mobile-platform-driver.test.ts b/src/platform/mobile-platform-driver.test.ts index 7219e44..84ef23b 100644 --- a/src/platform/mobile-platform-driver.test.ts +++ b/src/platform/mobile-platform-driver.test.ts @@ -692,6 +692,101 @@ describe('MobilePlatformDriver', () => { }); }); + describe('abort during the target wait', () => { + it('does not tap when the signal aborts while waiting', async () => { + const controller = new AbortController(); + const reason = new Error('Tool execution timed out after 30000ms'); + const backend = createMockBackend({ + waitForElement: vi.fn().mockImplementation(async () => { + controller.abort(reason); + return makeElement(); + }), + }); + const driver = new MobilePlatformDriver(backend); + + await expect( + driver.click( + 'testId', + 'confirm', + new Map(), + 5000, + undefined, + controller.signal, + ), + ).rejects.toBe(reason); + expect(backend.tapElement).not.toHaveBeenCalled(); + }); + + it('does not tap or type when the signal aborts while waiting', async () => { + const controller = new AbortController(); + const backend = createMockBackend({ + waitForElement: vi.fn().mockImplementation(async () => { + controller.abort(new Error('timed out')); + return makeElement(); + }), + }); + const driver = new MobilePlatformDriver(backend); + + await expect( + driver.type( + 'testId', + 'input', + 'text', + new Map(), + 5000, + undefined, + controller.signal, + ), + ).rejects.toThrowError('timed out'); + expect(backend.tapElement).not.toHaveBeenCalled(); + expect(backend.typeText).not.toHaveBeenCalled(); + }); + + it('taps and types when the signal is not aborted', async () => { + const backend = createMockBackend(); + const driver = new MobilePlatformDriver(backend); + const { signal } = new AbortController(); + + await driver.click('testId', 'btn', new Map(), 5000, undefined, signal); + await driver.type( + 'testId', + 'input', + 'text', + new Map(), + 5000, + undefined, + signal, + ); + + expect(backend.tapElement).toHaveBeenCalledTimes(2); + expect(backend.typeText).toHaveBeenCalledWith('text'); + }); + + it('does not type when the signal aborts during the focus tap', async () => { + const controller = new AbortController(); + const backend = createMockBackend({ + tapElement: vi.fn().mockImplementation(async () => { + controller.abort(new Error('timed out')); + return { success: true, x: 1, y: 1, targetDescription: 'input' }; + }), + }); + const driver = new MobilePlatformDriver(backend); + + await expect( + driver.type( + 'testId', + 'input', + 'text', + new Map(), + 5000, + undefined, + controller.signal, + ), + ).rejects.toThrowError('timed out'); + expect(backend.typeText).not.toHaveBeenCalled(); + }); + }); + describe('type', () => { it('waits, taps to focus, then types', async () => { const backend = createMockBackend(); diff --git a/src/platform/mobile-platform-driver.ts b/src/platform/mobile-platform-driver.ts index 0dfc37a..e1180cf 100644 --- a/src/platform/mobile-platform-driver.ts +++ b/src/platform/mobile-platform-driver.ts @@ -118,6 +118,9 @@ export class MobilePlatformDriver implements IPlatformDriver { * @param refMap - Map of a11y refs to stable identifiers. * @param timeoutMs - Maximum wait time in milliseconds. * @param within - Must be undefined — scoped search is not supported on mobile. + * @param signal - When aborted during the target wait, throws instead of + * tapping. device-mcp takes no signal: a wait or tap that is already + * running inside the backend is not cancelled. * @returns Click result with success status and target info. */ async click( @@ -126,11 +129,14 @@ export class MobilePlatformDriver implements IPlatformDriver { refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise { rejectWithinScope(within); const query = resolveTargetToQuery(targetType, targetValue, refMap); const deadline = Date.now() + timeoutMs; await this.#backend.waitForElement(query, timeoutMs); + // The request may have timed out while we waited for the target. + signal?.throwIfAborted(); const remaining = deadline - Date.now(); await withTimeout(this.#backend.tapElement(query), remaining, 'tapElement'); return { clicked: true, target: `${targetType}:${targetValue}` }; @@ -143,6 +149,9 @@ export class MobilePlatformDriver implements IPlatformDriver { * @param refMap - Map of a11y refs to stable identifiers. * @param timeoutMs - Maximum wait time in milliseconds. * @param within - Must be undefined — scoped search is not supported on mobile. + * @param signal - When aborted during the target wait or after the focus + * tap, throws instead of tapping or typing. device-mcp takes no signal: a + * wait, tap or typeText that is already running is not cancelled. * @returns Type result with success status and text length. */ async type( @@ -152,13 +161,16 @@ export class MobilePlatformDriver implements IPlatformDriver { refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise { rejectWithinScope(within); const query = resolveTargetToQuery(targetType, targetValue, refMap); const deadline = Date.now() + timeoutMs; await this.#backend.waitForElement(query, timeoutMs); + signal?.throwIfAborted(); let remaining = deadline - Date.now(); await withTimeout(this.#backend.tapElement(query), remaining, 'tapElement'); + signal?.throwIfAborted(); remaining = deadline - Date.now(); await withTimeout(this.#backend.typeText(text), remaining, 'typeText'); return { diff --git a/src/platform/playwright-driver.test.ts b/src/platform/playwright-driver.test.ts index 477105d..2677f19 100644 --- a/src/platform/playwright-driver.test.ts +++ b/src/platform/playwright-driver.test.ts @@ -154,6 +154,76 @@ describe('PlaywrightPlatformDriver', () => { }); }); + describe('abort during the target wait', () => { + it('does not click when the signal aborts while waiting', async () => { + const locator = createMockLocator(); + const controller = new AbortController(); + const reason = new Error('Tool execution timed out after 30000ms'); + vi.spyOn(discoveryModule, 'waitForTarget').mockImplementation( + async () => { + controller.abort(reason); + return locator as any; + }, + ); + + const driver = createDriver(); + await expect( + driver.click( + 'testId', + 'confirm', + new Map(), + 5000, + undefined, + controller.signal, + ), + ).rejects.toBe(reason); + expect(locator.click).not.toHaveBeenCalled(); + }); + + it('does not type when the signal aborts while waiting', async () => { + const locator = createMockLocator(); + const controller = new AbortController(); + vi.spyOn(discoveryModule, 'waitForTarget').mockImplementation( + async () => { + controller.abort(new Error('timed out')); + return locator as any; + }, + ); + + const driver = createDriver(); + await expect( + driver.type( + 'testId', + 'input', + 'text', + new Map(), + 5000, + undefined, + controller.signal, + ), + ).rejects.toThrowError('timed out'); + expect(locator.fill).not.toHaveBeenCalled(); + }); + + it('clicks when the signal is not aborted', async () => { + const locator = createMockLocator(); + vi.spyOn(discoveryModule, 'waitForTarget').mockResolvedValue( + locator as any, + ); + + const driver = createDriver(); + await driver.click( + 'testId', + 'btn', + new Map(), + 5000, + undefined, + new AbortController().signal, + ); + expect(locator.click).toHaveBeenCalled(); + }); + }); + describe('type', () => { it('delegates to waitForTarget and locator.fill', async () => { const locator = createMockLocator(); diff --git a/src/platform/playwright-driver.ts b/src/platform/playwright-driver.ts index 5f37ff7..268cf35 100644 --- a/src/platform/playwright-driver.ts +++ b/src/platform/playwright-driver.ts @@ -66,6 +66,8 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { * @param refMap - Map of a11y refs to selectors. * @param timeoutMs - Maximum time in milliseconds for the interaction. * @param within - Optional parent scope for chained locator resolution. + * @param signal - When aborted during the target wait, throws instead of + * clicking. * @returns The click result with success status and target info. */ async click( @@ -74,6 +76,7 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise { const page = this.#getPage(); const deadline = Date.now() + timeoutMs; @@ -94,6 +97,8 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { ); } + // The request may have timed out while we waited for the target. + signal?.throwIfAborted(); try { await locator.click({ timeout: remaining }); } catch (error) { @@ -122,6 +127,8 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { * @param refMap - Map of a11y refs to selectors. * @param timeoutMs - Maximum time in milliseconds. * @param within - Optional parent scope. + * @param signal - When aborted during the target wait, throws instead of + * typing. * @returns The type result with success status and text length. */ async type( @@ -131,6 +138,7 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise { const page = this.#getPage(); const deadline = Date.now() + timeoutMs; @@ -151,6 +159,7 @@ export class PlaywrightPlatformDriver implements IPlatformDriver { ); } + signal?.throwIfAborted(); await locator.fill(text, { timeout: remaining }); return { diff --git a/src/platform/types.ts b/src/platform/types.ts index caf9e28..0a4d837 100644 --- a/src/platform/types.ts +++ b/src/platform/types.ts @@ -64,14 +64,25 @@ export type ScreenCapture = { }; export type IPlatformDriver = { + /** + * Waits for the target, then clicks it. When `signal` is aborted while the + * driver waits for the target, the driver throws the abort reason and does + * not click. A click that already started is not cancelled. + */ click( targetType: TargetType, targetValue: string, refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise; + /** + * Waits for the target, then types into it. When `signal` is aborted while + * the driver waits for the target, the driver throws the abort reason and + * does not type. Typing that already started is not cancelled. + */ type( targetType: TargetType, targetValue: string, @@ -79,6 +90,7 @@ export type IPlatformDriver = { refMap: Map, timeoutMs: number, within?: WithinScope, + signal?: AbortSignal, ): Promise; waitForElement( diff --git a/src/server/create-server.ts b/src/server/create-server.ts index d9dd8cd..84ef8e8 100644 --- a/src/server/create-server.ts +++ b/src/server/create-server.ts @@ -471,7 +471,7 @@ export function createServer(config: ServerConfig): ServerInstance { if (category !== 'readonly' && category !== 'discovery') { await page.waitForLoadState('domcontentloaded').catch(() => undefined); signal.throwIfAborted(); - // Waits only for running, finite, in-viewport animations (infinite + // Waits only for running, finite animations of elements (infinite // spinners are ignored), checked once per frame, capped at 1 s. await waitForAnimationsToSettle( page, diff --git a/src/tools/driver-delegation.test.ts b/src/tools/driver-delegation.test.ts index df2c349..f499701 100644 --- a/src/tools/driver-delegation.test.ts +++ b/src/tools/driver-delegation.test.ts @@ -213,9 +213,22 @@ describe('tool delegation to context.driver', () => { expect.any(Map), expect.any(Number), undefined, + undefined, ); }); + it('clickTool and typeTool pass the request signal to the driver', async () => { + const driver = createMockDriver(); + const { signal } = new AbortController(); + const context = { ...createContextWithDriver(driver), signal }; + + await clickTool({ testId: 'btn' }, context); + await typeTool({ testId: 'input', text: 'hello' }, context); + + expect(vi.mocked(driver.click).mock.calls[0]?.[5]).toBe(signal); + expect(vi.mocked(driver.type).mock.calls[0]?.[6]).toBe(signal); + }); + it('typeTool delegates to driver.type', async () => { const driver = createMockDriver(); const context = createContextWithDriver(driver); @@ -233,6 +246,7 @@ describe('tool delegation to context.driver', () => { expect.any(Map), expect.any(Number), undefined, + undefined, ); }); diff --git a/src/tools/interaction.test.ts b/src/tools/interaction.test.ts index 9d9c1c1..bc6e268 100644 --- a/src/tools/interaction.test.ts +++ b/src/tools/interaction.test.ts @@ -1280,5 +1280,44 @@ describe('interaction', () => { expect(waitSpy).not.toHaveBeenCalled(); }, ); + + it.each([ + [ + 'click', + async (context: ToolContext) => + clickTool({ testId: 'confirm' }, context), + 'click', + ], + [ + 'type', + async (context: ToolContext) => + typeTool({ testId: 'input', text: 'x' }, context), + 'fill', + ], + ] as const)( + '%s returns MM_REQUEST_ABORTED and does not act when the request times out during the target wait', + async (_name, run, action) => { + const controller = new AbortController(); + const locator = createMockLocator(); + vi.spyOn(discoveryModule, 'waitForTarget').mockImplementation( + async () => { + controller.abort(new Error('Tool execution timed out')); + return locator as any; + }, + ); + const context = { + ...createMockContext(), + signal: controller.signal, + } as ToolContext; + + const result = await run(context); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error.code).toBe(ErrorCodes.MM_REQUEST_ABORTED); + } + expect(locator[action]).not.toHaveBeenCalled(); + }, + ); }); }); diff --git a/src/tools/interaction.ts b/src/tools/interaction.ts index 7dcc3b4..1f60c75 100644 --- a/src/tools/interaction.ts +++ b/src/tools/interaction.ts @@ -149,9 +149,14 @@ export async function clickTool( context.refMap, timeoutMs, resolveWithinScope(input.within), + context.signal, ); return createToolSuccess(result); } catch (error) { + const aborted = abortedResponse(context); + if (aborted) { + return aborted; + } const errorInfo = classifyClickError(error); if (errorInfo.code === ErrorCodes.MM_WAIT_TIMEOUT) { return createToolError( @@ -190,9 +195,14 @@ export async function typeTool( context.refMap, timeoutMs, resolveWithinScope(input.within), + context.signal, ); return createToolSuccess(result); } catch (error) { + const aborted = abortedResponse(context); + if (aborted) { + return aborted; + } const errorInfo = classifyTypeError(error); if (errorInfo.code === ErrorCodes.MM_WAIT_TIMEOUT) { return createToolError( diff --git a/src/tools/utils/constants.ts b/src/tools/utils/constants.ts index 2d6b087..0313c10 100644 --- a/src/tools/utils/constants.ts +++ b/src/tools/utils/constants.ts @@ -18,8 +18,11 @@ export const MAX_QUEUE_TIMEOUT_MS = 600_000; /** * Cap on the wait for animations to settle after a mutating browser call. - * Only running, finite animations of elements in the viewport are waited - * for; infinite ones (spinners, the fox, skeleton shimmer) are ignored. + * Only running, finite animations of elements are waited for (also off + * screen or at zero size, e.g. a drawer sliding in); infinite ones (spinners, + * the fox, skeleton shimmer) are ignored. A long finite animation anywhere on + * the page (a slow off-screen carousel, a Web Animations API animation on a + * hidden element) can add up to this cap to a mutating call. */ export const ANIMATION_SETTLE_TIMEOUT_MS = 1_000; diff --git a/src/tools/utils/discovery.ts b/src/tools/utils/discovery.ts index 2ac5b33..e205cc8 100644 --- a/src/tools/utils/discovery.ts +++ b/src/tools/utils/discovery.ts @@ -274,7 +274,7 @@ async function collectTestIdsViaLocators( } /** - * Waits until the page's running, finite, in-viewport animations end (see + * Waits until the page's running, finite element animations end (see * `animationsSettled`), checking once per animation frame, for at most * `timeoutMs`. Never throws: a timeout or an unreadable page ends the wait. * diff --git a/src/tools/utils/in-page.test.ts b/src/tools/utils/in-page.test.ts index 985b2f2..d92e34e 100644 --- a/src/tools/utils/in-page.test.ts +++ b/src/tools/utils/in-page.test.ts @@ -514,14 +514,14 @@ function targetElement(rect: { } /** - * Runs animationsSettled against fake animations in a 800x600 viewport. + * Runs animationsSettled against fake animations. Target rects are relative + * to an 800x600 viewport, but animationsSettled does not read them. * * @param animations - The page's animations. * @returns The predicate result. */ function settled(animations: FakeAnimation[]): boolean { const fn = compileInPage(animationsSettled, { - window: { innerWidth: 800, innerHeight: 600 }, document: { getAnimations: () => animations }, }); return fn(); @@ -565,30 +565,20 @@ describe('animationsSettled', () => { ).toBe(true); }); - it('ignores targets outside the viewport', () => { - expect( - settled([ - animation({ target: targetElement({ x: -10, y: 0 }) }), - animation({ target: targetElement({ x: 0, y: -10 }) }), - animation({ target: targetElement({ x: 800, y: 0 }) }), - animation({ target: targetElement({ x: 0, y: 600 }) }), - ]), - ).toBe(true); - }); - - it('ignores targets with an empty box', () => { - expect( - settled([ - animation({ target: targetElement({ x: 5, y: 5, width: 0 }) }), - animation({ target: targetElement({ x: 5, y: 5, height: 0 }) }), - ]), - ).toBe(true); + it.each([ + ['right of the viewport (drawer sliding in)', { x: 800, y: 0 }], + ['below the viewport (bottom sheet sliding up)', { x: 0, y: 600 }], + ['left of the viewport', { x: -10, y: 0 }], + ['above the viewport', { x: 0, y: -10 }], + ])('counts a target that starts %s', (_label, rect) => { + expect(settled([animation({ target: targetElement(rect) })])).toBe(false); }); - it('counts targets that only partly intersect the viewport', () => { - expect( - settled([animation({ target: targetElement({ x: -5, y: 595 }) })]), - ).toBe(false); + it.each([ + ['zero height (accordion opening)', { x: 5, y: 5, height: 0 }], + ['zero width', { x: 5, y: 5, width: 0 }], + ])('counts a target with %s', (_label, rect) => { + expect(settled([animation({ target: targetElement(rect) })])).toBe(false); }); it('is not settled when one animation still counts', () => { diff --git a/src/tools/utils/in-page.ts b/src/tools/utils/in-page.ts index 8631e73..1137391 100644 --- a/src/tools/utils/in-page.ts +++ b/src/tools/utils/in-page.ts @@ -25,23 +25,23 @@ export type InPageTestIdItem = { /** * Reports whether the page's animations have settled. * - * Only animations that can end soon and that the user can see are counted: - * an animation keeps the page unsettled when it is running, it is finite - * (its computed `iterations` and `endTime` are not `Infinity`), and its - * effect targets an element whose bounding box has a non-zero size and - * intersects the viewport. Animations without an effect or without an - * element target are ignored. Infinite animations (spinners, the fox, - * skeleton shimmer) never settle, so they are ignored too. A finite - * animation that is still in a long `delay` phase reports `running`, so it - * counts as running until it ends or the wait reaches its cap. + * Only animations that will end are counted: an animation keeps the page + * unsettled when it is running, it is finite (its computed `iterations` and + * `endTime` are not `Infinity`), and its effect targets an element. + * Animations without an effect or without an element target are ignored. + * Infinite animations (spinners, the fox, skeleton shimmer) never settle, so + * they are ignored too. The target's position and size are not checked: an + * element that slides in from outside the viewport (drawer, bottom sheet) or + * grows from zero size (accordion) is still moving even while it is not + * visible yet. A finite animation that is still in a long `delay` phase + * reports `running`, so it counts as running until it ends or the wait + * reaches its cap. * * Runs in the page: must stay self-contained. * - * @returns True when no running, finite, in-viewport animation remains. + * @returns True when no running, finite animation of an element remains. */ export function animationsSettled(): boolean { - const viewportWidth = window.innerWidth; - const viewportHeight = window.innerHeight; for (const animation of document.getAnimations()) { if (animation.playState !== 'running') { continue; @@ -55,17 +55,8 @@ export function animationsSettled(): boolean { continue; } const target = (effect as Partial).target ?? null; - if (!target || typeof target.getBoundingClientRect !== 'function') { - continue; - } - const rect = target.getBoundingClientRect(); - const hasSize = rect.width > 0 && rect.height > 0; - const inViewport = - rect.right > 0 && - rect.bottom > 0 && - rect.left < viewportWidth && - rect.top < viewportHeight; - if (hasSize && inViewport) { + // An element target (it has a layout box API). Other targets are ignored. + if (target && typeof target.getBoundingClientRect === 'function') { return false; } } diff --git a/src/validation/schemas.test.ts b/src/validation/schemas.test.ts index 2071230..c340a73 100644 --- a/src/validation/schemas.test.ts +++ b/src/validation/schemas.test.ts @@ -930,3 +930,23 @@ describe('runStepsInputSchema includeObservations', () => { expect(parsed.error?.issues[0]?.message).toContain('use observe'); }); }); + +describe('runStepsInputSchema batchTimeoutMs', () => { + it.each([1000, 30_000])('accepts %i', (batchTimeoutMs) => { + expect( + toolSchemas.run_steps.safeParse({ + steps: [{ tool: 'get_state' }], + batchTimeoutMs, + }).success, + ).toBe(true); + }); + + it('rejects values above the 30 s request ceiling', () => { + expect( + toolSchemas.run_steps.safeParse({ + steps: [{ tool: 'get_state' }], + batchTimeoutMs: 30_001, + }).success, + ).toBe(false); + }); +}); diff --git a/src/validation/schemas.ts b/src/validation/schemas.ts index 1d037c5..719e1b9 100644 --- a/src/validation/schemas.ts +++ b/src/validation/schemas.ts @@ -591,10 +591,13 @@ export const runStepsInputSchema = z.object({ .number() .int() .min(1000) - .max(300_000) + .max(30_000) .describe( 'Overall timeout for the batch in milliseconds. ' + - 'When exceeded, remaining steps are marked as skipped and partial results are returned.', + 'When exceeded, remaining steps are marked as skipped and partial results are returned. ' + + 'The deadline is checked between steps. Capped at 30 000, the default request limit of ' + + 'the queue and the CLI; to get partial results, leave room for the step in progress ' + + 'and the observation (e.g. 20 000).', ) .optional(), // Removed in favour of `observe`. Rejected (not ignored) so old callers