diff --git a/README.md b/README.md index 4b89d34..5697626 100644 --- a/README.md +++ b/README.md @@ -369,7 +369,7 @@ type ToolContext = { }; ``` -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 (extension state, test IDs, a11y snapshot) after mutating tools and `run_steps`. +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:** @@ -395,7 +395,7 @@ The daemon routes `POST /tool/:name` requests through the registry, applies Zod | `accessibility_snapshot` | Captures a trimmed accessibility tree of the current page with deterministic refs (`e1`, `e2`, ...). Supports scoping to a root CSS selector. | | `list_testids` | Collects all visible `data-testid` attributes from the current page with text previews and visibility status. | | **State** | | -| `get_state` | Retrieves the current extension state (URL, screen, network, balance, account) and tracked tab information. | +| `get_state` | Retrieves the current extension state (URL, screen, network, balance, account) and tracked tab information. On mobile the state is partial (`source: 'stub'`): fields that were not read from the app are omitted, not guessed. | | `get_context` | Returns the current environment context (`e2e` or `prod`), session status, available capabilities, and whether context switching is allowed. | | `set_context` | Switches the session environment between `e2e` and `prod` modes. Blocked while a session is active. | | **Screenshots** | | @@ -406,7 +406,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, `includeObservations` (`'all'`, `'none'`, `'failures'`) to control observations, 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` 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. | | **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). | @@ -519,13 +519,27 @@ All responses follow a consistent shape: ```typescript // Success -{ ok: true, result: T, observations?: { state, testIds, a11y } } +{ ok: true, result: T, observations?: { testIds, a11y } } // Error { ok: false, error: { code: string, message: string } } ``` -The `observations` field is included for **mutating** tools (click, type, navigate, launch, cleanup, build, etc.) and for `run_steps` when its `includeObservations` parameter is `'all'` (default) or `'failures'`. **Read-only**, **discovery** and `cdp` calls skip observation collection entirely (`meta.timing.observationMs` is 0). +### Observations + +Every tool input (and the `/launch` and `/cleanup` bodies) accepts an optional `observe` level: + +| Level | Behaviour | +| ------ | ---------------------------------------------------------------------------------------------------------------- | +| `none` | No observation is collected or returned (`meta.timing.observationMs` is 0). | +| `diff` | The observation is diffed against the previous one: `a11y.nodes` holds only changed nodes, `a11y.diff` the refs. | +| `full` | The whole option-collapsed observation, with no diff. It becomes the new diff baseline. | + +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. + +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. ## CLI Reference @@ -533,9 +547,10 @@ The `mm` CLI provides a unified interface for agents and developers. All command ### Global Options -| Option | Description | -| ------------------ | ----------------------------------------------------------------------------------------------------------- | -| `--project ` | Target a specific project directory (absolute or relative). Overrides `MM_PROJECT` and git-based discovery. | +| Option | Description | +| ------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `--project ` | Target a specific project directory (absolute or relative). Overrides `MM_PROJECT` and git-based discovery. | +| `--observe ` | Observation to return after the call: `none`, `diff` or `full`. Sent as `observe` in every tool, `/launch` and `/cleanup` request body. Default: `diff` for mutating commands and `run-steps`, `none` otherwise. | | Environment Variable | Description | | -------------------- | -------------------------------------------------------------------------------------------------------- | @@ -597,11 +612,11 @@ mm describe-screen ### State & Context -| Command | Description | -| ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `mm get-state` | Returns the current extension state: loaded status, current URL, screen name, network, chain ID, account address, and balance. Also lists all tracked browser tabs. | -| `mm get-context` | Returns the current environment context (`e2e` or `prod`), session status, available capabilities, and whether context switching is allowed. | -| `mm set-context ` | Switches the session environment between `e2e` and `prod` modes. Blocked while a session is active — run `mm cleanup` first. | +| Command | Description | +| ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `mm get-state` | Returns the current extension state: loaded status, current URL, screen name, network, chain ID, account address, and balance. Also lists all tracked browser tabs. Observations do not carry state, so use this command for it. On mobile the state is partial (`source: "stub"`): fields that were not read from the app are omitted. | +| `mm get-context` | Returns the current environment context (`e2e` or `prod`), session status, available capabilities, and whether context switching is allowed. | +| `mm set-context ` | Switches the session environment between `e2e` and `prod` modes. Blocked while a session is active — run `mm cleanup` first. | ### Batching @@ -625,7 +640,7 @@ mm cdp Network.enable mm cdp DOM.getDocument '{"depth":2}' --timeout 60000 ``` -The `params-json` argument must be a valid JSON object. The `--timeout` flag sets a per-command timeout (default: 30 000 ms, max: 30 000 ms). The tool is categorized as `cdp`: it returns no observation and does not refresh refs, so run `mm describe-screen` after a call that changes state. +The `params-json` argument must be a valid JSON object. The `--timeout` flag sets a per-command timeout (default: 30 000 ms, max: 30 000 ms). The tool is categorized as `cdp`: by default it returns no observation and does not refresh refs, so pass `--observe full` (or run `mm describe-screen`) after a call that changes state. #### Browser vs Mobile (Hermes) CDP diff --git a/SKILL.md b/SKILL.md index 2f7f222..94923e9 100644 --- a/SKILL.md +++ b/SKILL.md @@ -4,6 +4,8 @@ You control a browser extension through the `mm` CLI. Every command talks to a l If you are running outside the target project directory, use `--project ` or set the `MM_PROJECT` environment variable to point at the project root. All commands accept `--project` before the command name (e.g., `mm --project ../metamask-extension launch`). +All commands also accept the global `--observe none|diff|full` flag to choose the observation returned after the call (see Observation Behavior). The CLI consumes `--observe ` anywhere in the command line, so to type the literal text `--observe` use `run-steps` or the HTTP API. + ## Core Loop ``` @@ -17,25 +19,41 @@ 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` after interacting** — OR use inline `observations` from mutating tool responses. Mutating tools (click, type, navigate, etc.) return an `observations` object with fresh `state`, `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. +- **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). ## Observation Behavior -Tool responses include different data based on the tool's category: +Every tool accepts an `observe` level (CLI: global `--observe ` flag; HTTP / `run_steps`: an `observe` field in the input): + +| Level | Response | +| ------ | ------------------------------------------------------------------------------------------ | +| `none` | No `observations`, no collection cost (`meta.timing.observationMs` is 0). Fastest. | +| `diff` | Only what changed since the previous observation (see Diff-Based Observations below). | +| `full` | The whole compacted screen (`a11y` + `testIds`), no diff. Use after navigation or a modal. | + +The default depends on the tool's category: + +| Category | Examples | Default `observe` | +| ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------- | +| **Mutating** | click, type, navigate, launch, cleanup, build, clipboard, mock_network, scroll_to_element, device_swipe, long_press, tap_coordinates, dismiss_keyboard, dismiss_alert, open_app, close_app, press_button, device_context, device_clipboard | `diff` | +| **Read-only** | get_state, get_text, get_context, set_context, get_alert_text, screen_recording, device_logs, generate_locators, hermes_targets | `none` | +| **Discovery** | describe_screen, list_testids, accessibility_snapshot, screenshot (data is already in `result`) | `none` | +| **CDP** | cdp | `none` | +| **Batch** | run_steps | `diff` | -| Category | Examples | Observations in response? | -| ------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------- | -| **Mutating** | click, type, navigate, launch, cleanup, build, clipboard, mock_network, scroll_to_element, device_swipe, long_press, tap_coordinates, dismiss_keyboard, dismiss_alert, open_app, close_app, press_button, device_context, device_clipboard | Yes — `state` + `a11y` (compacted) + `testIds` | -| **Read-only** | get_state, get_text, get_context, set_context, get_alert_text, screen_recording, device_logs, generate_locators, hermes_targets | No — faster response | -| **Discovery** | describe_screen, list_testids, accessibility_snapshot, screenshot | Data is already in `result` | -| **CDP** | cdp | No — raw result only | -| **Batch** | run_steps | Controlled by `includeObservations` param | +```bash +mm click e3 --observe none # chaining clicks: skip the observation +mm navigate-home --observe full # after navigation: get the whole screen +mm get-state --observe full # a read that also returns an observation +``` -**Observation Compaction:** Mutating tool observations are **compacted** before returning: option runs of 3 or more under a combobox or listbox are replaced with a single summary node (e.g., `"55 options (refs e2–e56)"`). The `describe-screen` tool always returns the **full, unfiltered** a11y tree — use it when you need the complete option list. +Observations contain `testIds` and `a11y` only — no app `state`. Call `mm get-state` for state. -**Diff-Based Observations:** After the first mutating tool call sets a baseline, subsequent mutations return **diff-based** observations. The `observations.a11y.diff` field (when present) shows what changed: +**Observation Compaction:** Observations are **compacted** before returning: option runs of 3 or more under a combobox or listbox are replaced with a single summary node (e.g., `"55 options (refs e2–e56)"`). The `describe-screen` tool always returns the **full, unfiltered** a11y tree — use it when you need the complete option list. + +**Diff-Based Observations:** After the first observation sets a baseline, later calls at level `diff` return **diff-based** observations. The `observations.a11y.diff` field (when present) shows what changed: ```json { @@ -45,18 +63,17 @@ Tool responses include different data based on the tool's category: } ``` -The `observations.a11y.nodes` field contains **only the changed and new nodes** (not all nodes). The baseline resets after `describe-screen`, `accessibility-snapshot`, `launch`, or `cleanup` — the next mutation returns a full compact observation (no `diff` field). When the diff would be larger than the full observation, the full option-filtered observation is returned instead (no `diff` field). +The `observations.a11y.nodes` field contains **only the changed and new nodes** (not all nodes). The baseline resets after `describe-screen`, `accessibility-snapshot`, `launch`, or `cleanup` — the next observation is a full compact one (no `diff` field). A `full` observation also becomes the new baseline; `none` leaves the baseline unchanged. When the diff would be larger than the full observation, the full option-filtered observation is returned instead (no `diff` field). ### Using inline observations (mutating tools) -After a mutating action, the response includes fresh screen state: +After a mutating action, the response includes fresh screen data: ```json { "ok": true, "result": { ... }, "observations": { - "state": { "screen": "send", "url": "...", "balance": "1.5 ETH" }, "testIds": ["send-amount-input", "send-button"], "a11y": { "nodes": [ @@ -72,8 +89,8 @@ You can use the `ref` values from `observations.a11y.nodes` for the next interac **Quick reference:** -- Use `observations.state` for quick checks (screen name, loading status, balance, etc.) - Use `observations.a11y.nodes` with the compact refs for the next interaction +- Call `get-state` for app state (screen, account, network, balance) - Call `describe-screen` only when you need the full tree ```bash @@ -88,15 +105,9 @@ Call `describe-screen` explicitly when you need: - Full context after unexpected navigation - The complete, unfiltered a11y tree (e.g., all options in a dropdown) -### `run_steps` and `includeObservations` - -The `run_steps` tool collects observations once after all steps complete. Control inclusion with the `includeObservations` parameter: +### `run_steps` and `observe` -| Value | Behavior | -| ----------------- | --------------------------------------------- | -| `'all'` (default) | Always include final state observations | -| `'none'` | Never include observations (fastest response) | -| `'failures'` | Include observations only if any step failed | +The `run_steps` tool collects one observation after all steps complete. A top-level `observe` sets its level (default `diff`). Without one, the `observe` in the last executed step's `args` is used; `observe` on other steps is ignored. (`includeObservations` was removed; to react to failures, check `summary.ok`.) ```json { @@ -104,7 +115,7 @@ The `run_steps` tool collects observations once after all steps complete. Contro { "tool": "click", "args": { "a11yRef": "e3" } }, { "tool": "type", "args": { "a11yRef": "e5", "text": "0.01" } } ], - "includeObservations": "failures" + "observe": "none" } ``` @@ -129,7 +140,7 @@ mm launch [--context e2e|prod] [--state default|onboarding|custom] [--extension- | `--extension-path ` | Override the extension build directory | | `--force` | Replace an existing active session | -Returns: `sessionId`, `extensionId`, `state` (current extension state). +Returns: `sessionId`, `extensionId`, `state` (current extension state; on mobile it is partial: `source: "stub"` and only the fields that were read, e.g. `isLoaded`, `extensionId`). #### `mm cleanup` @@ -380,7 +391,7 @@ Returns extension state and tracked tabs without the full a11y tree. mm get-state ``` -Returns: `state` (extension state) and `tabs` (active + tracked tabs with roles and URLs). +Returns: `state` (extension state) and `tabs` (active + tracked tabs with roles and URLs). This is the only way to read app state — observations do not include it. On mobile the state is partial (`source: "stub"`): usually only `isLoaded` and `extensionId`; fields that were not read from the app (`isUnlocked`, `currentScreen`, account, network, balance) are omitted, not guessed. #### `mm get-context` @@ -447,7 +458,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. The `includeObservations` param controls whether final-state observations appear in the response: `'all'` (default), `'none'`, or `'failures'` (only on partial failure). 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` 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`. 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. @@ -547,7 +558,7 @@ mm cdp Runtime.evaluate '{"expression":"document.querySelector(\'#personalSign\' | Blocked methods | `Browser.close`, `Target.closeTarget`, `Target.disposeBrowserContext`, `Browser.crashGpuProcess` | `Runtime.terminateExecution`, `Inspector.detached` | `Browser.close`, `Target.closeTarget`, `Target.disposeBrowserContext`, `Browser.crashGpuProcess` | | Result shape | Standard CDP response | `Runtime.evaluate` nests the value at `result.result.value` | `Runtime.evaluate` nests the value at `result.result.value` | -Blocked methods return `MM_CDP_BLOCKED` on either platform; other failures return `MM_CDP_FAILED` (on mobile the underlying `HERMES_*` code is preserved in the message). The tool is categorized as **mutating** — run `describe-screen` afterward to re-sync if the call changed runtime/page state. +Blocked methods return `MM_CDP_BLOCKED` on either platform; other failures return `MM_CDP_FAILED` (on mobile the underlying `HERMES_*` code is preserved in the message). The tool returns no observation by default — pass `--observe full` (or run `describe-screen`) to re-sync if the call changed runtime/page state. #### `mm hermes-targets [--all] [--metro-port

] [--app-id ]` diff --git a/src/capabilities/types.ts b/src/capabilities/types.ts index 13bc0d7..a0c46c8 100644 --- a/src/capabilities/types.ts +++ b/src/capabilities/types.ts @@ -45,16 +45,29 @@ export type ScreenshotResult = { export type ScreenName = string; +/** + * App state returned by `get_state` and `launch`. Fields the platform cannot + * read are omitted rather than guessed (for example, the mobile driver only + * knows whether the app is running). + */ export type ExtensionState = { isLoaded: boolean; - currentUrl: string; + /** Omitted on platforms with no URL concept (mobile). */ + currentUrl?: string; extensionId: string; - isUnlocked: boolean; - currentScreen: ScreenName; - accountAddress: string | null; - networkName: string | null; - chainId: number | null; - balance: string | null; + isUnlocked?: boolean; + currentScreen?: ScreenName; + accountAddress?: string | null; + networkName?: string | null; + chainId?: number | null; + balance?: string | null; + /** + * `'stub'` when the state is partial: fields that were not read from the + * app are omitted (for example, the mobile driver only knows `isLoaded` + * and `extensionId`; a consumer may add fields it did read). Absent when + * the state was fully read. + */ + source?: 'stub'; activeTab?: { role: string; url: string; diff --git a/src/cli/mm.test.ts b/src/cli/mm.test.ts index 2252cc0..ca2a0c2 100644 --- a/src/cli/mm.test.ts +++ b/src/cli/mm.test.ts @@ -11,7 +11,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { MockInstance } from 'vitest'; import { + applyObserveLevel, + extractObserveFlag, extractProjectFlag, + setObserveLevel, resolveTargetFromArgs, resolveWithinFromArgs, getPositionalTarget, @@ -171,6 +174,78 @@ describe('extractProjectFlag', () => { }); }); +describe('extractObserveFlag', () => { + it('returns args unchanged when no --observe flag', () => { + expect(extractObserveFlag(['click', 'e1'])).toStrictEqual({ + args: ['click', 'e1'], + observe: undefined, + }); + }); + + it.each(['none', 'diff', 'full'])( + 'extracts --observe %s from anywhere in argv', + (level) => { + expect( + extractObserveFlag(['click', '--observe', level, 'e1']), + ).toStrictEqual({ args: ['click', 'e1'], observe: level }); + expect( + extractObserveFlag(['--observe', level, 'get-state']), + ).toStrictEqual({ args: ['get-state'], observe: level }); + }, + ); + + it.each([ + [['--observe']], + [['--observe', 'all']], + [['--observe', '--force']], + ])('exits on a missing or invalid value: %j', (argv) => { + expect(() => extractObserveFlag(argv)).toThrowError('process.exit'); + expect(stderrSpy).toHaveBeenCalledWith( + 'Error: --observe requires one of: none, diff, full\n', + ); + }); +}); + +describe('applyObserveLevel', () => { + afterEach(() => { + setObserveLevel(undefined); + }); + + it('returns the body unchanged when no level is set', () => { + const body = { a11yRef: 'e1' }; + expect(applyObserveLevel('POST', '/tool/click', body)).toBe(body); + }); + + it.each(['/tool/click', '/tool/run_steps', '/launch', '/cleanup'])( + 'adds observe to POST %s bodies', + (requestPath) => { + setObserveLevel('none'); + expect(applyObserveLevel('POST', requestPath, { x: 1 })).toStrictEqual({ + observe: 'none', + x: 1, + }); + }, + ); + + it('keeps an observe already in the body', () => { + setObserveLevel('none'); + expect( + applyObserveLevel('POST', '/tool/run_steps', { observe: 'full' }), + ).toStrictEqual({ observe: 'full' }); + }); + + it('leaves GET, other paths and non-object bodies alone', () => { + setObserveLevel('full'); + expect(applyObserveLevel('GET', '/status', null)).toBeNull(); + expect(applyObserveLevel('POST', '/status', { x: 1 })).toStrictEqual({ + x: 1, + }); + expect(applyObserveLevel('POST', '/tool/click', null)).toBeNull(); + const list = [1]; + expect(applyObserveLevel('POST', '/tool/click', list)).toBe(list); + }); +}); + describe('resolveTargetFromArgs', () => { it('returns selector for --selector flag', () => { expect(resolveTargetFromArgs(['--selector', '.my-button'])).toStrictEqual({ @@ -785,6 +860,7 @@ describe('printHelp', () => { expect(output).toContain('mm — MetaMask CLI'); expect(output).toContain('Usage:'); expect(output).toContain('mm launch'); + expect(output).toContain('--observe '); }); }); @@ -1134,6 +1210,34 @@ describe('sendRequest', () => { expect(stdoutSpy).toHaveBeenCalled(); }); + it('adds the global --observe level to POST tool bodies', async () => { + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ ok: true, result: { clicked: true } }), + } as Response); + setObserveLevel('none'); + + try { + await sendRequest(3000, 'POST', '/tool/click', { a11yRef: 'e1' }); + await sendRequest(3000, 'GET', '/status', null); + } finally { + setObserveLevel(undefined); + } + + expect(globalThis.fetch).toHaveBeenNthCalledWith( + 1, + 'http://127.0.0.1:3000/tool/click', + expect.objectContaining({ + body: '{"observe":"none","a11yRef":"e1"}', + }), + ); + expect(globalThis.fetch).toHaveBeenNthCalledWith( + 2, + 'http://127.0.0.1:3000/status', + expect.not.objectContaining({ body: expect.anything() }), + ); + }); + it('sends POST request with JSON body', async () => { vi.spyOn(globalThis, 'fetch').mockResolvedValue({ ok: true, @@ -2954,6 +3058,40 @@ describe('main', () => { process.argv = origArgv; }); + it('sends the global --observe level with the request', async () => { + const { readDaemonState, isDaemonAlive, isDaemonVersionMatch } = + await import('../server/daemon-state.js'); + vi.mocked(readDaemonState).mockResolvedValueOnce({ + port: 3000, + pid: 123, + nonce: 'abc', + startedAt: '2024-01-01', + version: '1.0.0', + subPorts: {}, + }); + vi.mocked(isDaemonAlive).mockResolvedValueOnce(true); + vi.mocked(isDaemonVersionMatch).mockReturnValueOnce(true); + vi.spyOn(globalThis, 'fetch').mockResolvedValue({ + ok: true, + json: async () => ({ ok: true, result: { state: {} } }), + } as Response); + + const origArgv = process.argv; + process.argv = ['node', 'mm', 'get-state', '--observe', 'full']; + + try { + await main(); + } finally { + process.argv = origArgv; + setObserveLevel(undefined); + } + + expect(globalThis.fetch).toHaveBeenCalledWith( + 'http://127.0.0.1:3000/tool/get_state', + expect.objectContaining({ body: '{"observe":"full"}' }), + ); + }); + it('sends the stdin recovery phrase with a self-onboarding launch', async () => { const { readDaemonState, isDaemonAlive, isDaemonVersionMatch } = await import('../server/daemon-state.js'); diff --git a/src/cli/mm.ts b/src/cli/mm.ts index 96ffc0e..7ccea37 100644 --- a/src/cli/mm.ts +++ b/src/cli/mm.ts @@ -18,6 +18,8 @@ import { import { BEST_EFFORT_CLEANUP_TIMEOUT_MS } from '../tools/utils/constants.js'; import type { DaemonState } from '../types/http.js'; import { computeTimeoutBudget } from '../utils/timeout-budget.js'; +import { observeLevelSchema } from '../validation/schemas.js'; +import type { ObserveLevel } from '../validation/schemas.js'; const COMMAND_TIMEOUTS_MS: Record = { launch: 120_000, @@ -82,6 +84,78 @@ export function extractProjectFlag(argv: string[]): { return { args: remaining, projectPath: value }; } +const OBSERVE_LEVELS: readonly ObserveLevel[] = observeLevelSchema.options; + +/** + * Extracts and consumes the global `--observe ` flag from + * argv, returning the remaining args and the level (if any). + * + * @param argv - Raw CLI arguments (after the node/script entries). + * @returns The remaining arguments and the optional observe level. + */ +export function extractObserveFlag(argv: string[]): { + args: string[]; + observe: ObserveLevel | undefined; +} { + const idx = argv.indexOf('--observe'); + if (idx < 0) { + return { args: argv, observe: undefined }; + } + const value = argv[idx + 1]; + if (!OBSERVE_LEVELS.includes(value as ObserveLevel)) { + process.stderr.write( + `Error: --observe requires one of: ${OBSERVE_LEVELS.join(', ')}\n`, + ); + process.exit(1); + } + const remaining = [...argv.slice(0, idx), ...argv.slice(idx + 2)]; + return { args: remaining, observe: value as ObserveLevel }; +} + +let globalObserveLevel: ObserveLevel | undefined; + +/** + * Sets the `--observe` level that {@link sendRequest} adds to every tool, + * `/launch` and `/cleanup` request body. + * + * @param level - The level, or undefined to send none (daemon default). + */ +export function setObserveLevel(level: ObserveLevel | undefined): void { + globalObserveLevel = level; +} + +/** + * Adds the global `--observe` level to a request body. Only POST requests to + * `/tool/*`, `/launch` and `/cleanup` with a plain-object body get it, and an + * `observe` already in the body (e.g. in `run-steps` JSON) is kept. + * + * @param method - The HTTP method. + * @param requestPath - The URL path. + * @param body - The request body. + * @returns The body, with `observe` added when it applies. + */ +export function applyObserveLevel( + method: string, + requestPath: string, + body: unknown, +): unknown { + if ( + globalObserveLevel === undefined || + method !== 'POST' || + !( + requestPath.startsWith('/tool/') || + requestPath === '/launch' || + requestPath === '/cleanup' + ) || + typeof body !== 'object' || + body === null || + Array.isArray(body) + ) { + return body; + } + return { observe: globalObserveLevel, ...(body as Record) }; +} + /** * Resolves the target project root directory using the following precedence: * 1. `--project ` CLI flag @@ -143,9 +217,11 @@ export async function resolveWorktreeRoot( * CLI entry point that parses arguments and routes to the appropriate handler. */ export async function main(): Promise { - const { args: remainingArgs, projectPath } = extractProjectFlag( + const { args: projectlessArgs, projectPath } = extractProjectFlag( process.argv.slice(2), ); + const { args: remainingArgs, observe } = extractObserveFlag(projectlessArgs); + setObserveLevel(observe); if ( remainingArgs.length === 0 || @@ -1070,7 +1146,13 @@ export async function sendRequest( } try { - const data = await daemonFetch(port, method, requestPath, body, timeout); + const data = await daemonFetch( + port, + method, + requestPath, + applyObserveLevel(method, requestPath, body), + timeout, + ); const { meta, ...rest } = data; const result = rest.result ?? rest; @@ -1704,11 +1786,14 @@ export function parseLaunchArgs(args: string[]): Record { export function printHelp(): void { process.stdout.write(`mm — MetaMask CLI -Usage: mm [--project ] [options] +Usage: mm [--project ] [--observe ] [options] Global Options: --project Target a specific project directory (absolute or relative). 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. Environment Variables: MM_PROJECT Default project directory when --project is not provided. diff --git a/src/platform/mobile-platform-driver.test.ts b/src/platform/mobile-platform-driver.test.ts index d87e91c..9e8cf07 100644 --- a/src/platform/mobile-platform-driver.test.ts +++ b/src/platform/mobile-platform-driver.test.ts @@ -857,21 +857,15 @@ describe('MobilePlatformDriver', () => { }); describe('getAppState', () => { - it('returns ExtensionState with mobile defaults for running app', async () => { + it('returns only what it knows for a running app, marked as a stub', async () => { const driver = new MobilePlatformDriver(createMockBackend()); const state = await driver.getAppState(); expect(state).toStrictEqual({ isLoaded: true, - currentUrl: '', extensionId: 'io.metamask', - isUnlocked: true, - currentScreen: 'unknown', - accountAddress: null, - networkName: null, - chainId: null, - balance: null, + source: 'stub', }); }); @@ -887,7 +881,7 @@ describe('MobilePlatformDriver', () => { const state = await driver.getAppState(); expect(state.isLoaded).toBe(true); - expect(state.isUnlocked).toBe(true); + expect(state).not.toHaveProperty('isUnlocked'); }); it('returns isLoaded false for non-running app', async () => { @@ -902,7 +896,7 @@ describe('MobilePlatformDriver', () => { const state = await driver.getAppState(); expect(state.isLoaded).toBe(false); - expect(state.isUnlocked).toBe(false); + expect(state).not.toHaveProperty('isUnlocked'); }); it('uses custom bundleId', async () => { diff --git a/src/platform/mobile-platform-driver.ts b/src/platform/mobile-platform-driver.ts index bc97f8f..7aea944 100644 --- a/src/platform/mobile-platform-driver.ts +++ b/src/platform/mobile-platform-driver.ts @@ -239,7 +239,11 @@ export class MobilePlatformDriver implements IPlatformDriver { } /** - * @returns Extension state with mobile-appropriate defaults. + * Reads only what the device backend knows: whether the app is running. + * Wallet fields (unlock, screen, account, network, balance) are omitted + * rather than guessed, and `source: 'stub'` marks the result as partial. + * + * @returns The partial app state. */ async getAppState(): Promise { const appState = await this.#backend.getAppState(this.#bundleId); @@ -248,14 +252,8 @@ export class MobilePlatformDriver implements IPlatformDriver { appState.state === APPIUM_STATE_RUNNING_IN_FOREGROUND; return { isLoaded: isRunning, - currentUrl: '', extensionId: this.#bundleId, - isUnlocked: isRunning, - currentScreen: 'unknown', - accountAddress: null, - networkName: null, - chainId: null, - balance: null, + source: 'stub', }; } diff --git a/src/server/create-server.test.ts b/src/server/create-server.test.ts index 087d665..dbc648a 100644 --- a/src/server/create-server.test.ts +++ b/src/server/create-server.test.ts @@ -9,8 +9,7 @@ import type { ServerInstance } from './create-server.js'; import { createServer, buildResponseBody, - shouldCollectObservations, - shouldIncludeObservationsInResponse, + resolveObserveLevel, } from './create-server.js'; import { readDaemonState } from './daemon-state.js'; import pkg from '../../package.json'; @@ -154,7 +153,7 @@ describe('buildResponseBody', () => { it('merges observations into result', () => { const result = { ok: true }; - const obs = { state: {}, testIds: [], a11y: { nodes: [] } }; + const obs = { testIds: [], a11y: { nodes: [] } }; expect(buildResponseBody(result, obs as any)).toStrictEqual({ ok: true, observations: obs, @@ -162,127 +161,117 @@ describe('buildResponseBody', () => { }); }); -describe('shouldCollectObservations', () => { - it('returns true for mutating', () => { - expect(shouldCollectObservations('mutating')).toBe(true); - }); - - it('returns false for readonly', () => { - expect(shouldCollectObservations('readonly')).toBe(false); - }); - - it('returns false for discovery', () => { - expect(shouldCollectObservations('discovery')).toBe(false); - }); - - it('returns false for cdp', () => { - expect(shouldCollectObservations('cdp')).toBe(false); - }); - - it('returns true for batch with default policy', () => { - expect(shouldCollectObservations('batch')).toBe(true); - }); - - it("returns true for batch with 'all' policy", () => { - expect( - shouldCollectObservations('batch', { includeObservations: 'all' }), - ).toBe(true); - }); - - it("returns false for batch with 'none' policy", () => { - expect( - shouldCollectObservations('batch', { includeObservations: 'none' }), - ).toBe(false); - }); - - it("returns true for batch with 'failures' policy", () => { - expect( - shouldCollectObservations('batch', { includeObservations: 'failures' }), - ).toBe(true); - }); -}); - -describe('shouldIncludeObservationsInResponse', () => { - const okResult: ToolResponse = { ok: true, result: {} }; - const failResult: ToolResponse = { - ok: false, - error: { code: 'ERR', message: 'fail' }, - }; - const summaryFailResult: ToolResponse = { +describe('resolveObserveLevel', () => { + const batchResult = (steps: { skipped?: boolean }[]): ToolResponse => ({ ok: true, - result: { summary: { ok: false } }, - }; - - it('returns true for mutating', () => { - expect(shouldIncludeObservationsInResponse('mutating', okResult)).toBe( - true, - ); - }); - - it('returns false for readonly', () => { - expect(shouldIncludeObservationsInResponse('readonly', okResult)).toBe( - false, - ); - }); - - it('returns false for discovery', () => { - expect(shouldIncludeObservationsInResponse('discovery', okResult)).toBe( - false, + result: { + steps: steps.map((step) => ({ + tool: 'click', + ok: true, + meta: { durationMs: 0, timestamp: '', skipped: step.skipped }, + })), + summary: { ok: true }, + }, + }); + + it.each([ + ['mutating', 'diff'], + ['readonly', 'none'], + ['discovery', 'none'], + ['cdp', 'none'], + ['batch', 'diff'], + ] as const)('defaults %s to %s', (category, expected) => { + expect(resolveObserveLevel(category)).toBe(expected); + expect(resolveObserveLevel(category, {})).toBe(expected); + }); + + it.each([ + ['mutating', 'none'], + ['mutating', 'diff'], + ['mutating', 'full'], + ['readonly', 'none'], + ['readonly', 'diff'], + ['readonly', 'full'], + ['discovery', 'none'], + ['discovery', 'diff'], + ['discovery', 'full'], + ['cdp', 'none'], + ['cdp', 'diff'], + ['cdp', 'full'], + ['batch', 'none'], + ['batch', 'diff'], + ['batch', 'full'], + ] as const)('honours an explicit level on %s: %s', (category, level) => { + expect(resolveObserveLevel(category, { observe: level })).toBe(level); + }); + + it('ignores an invalid observe value', () => { + expect(resolveObserveLevel('mutating', { observe: 'bogus' })).toBe('diff'); + expect(resolveObserveLevel('readonly', { observe: 'bogus' })).toBe('none'); + }); + + it('uses the last executed step observe for run_steps', () => { + const input = { + steps: [ + { tool: 'click', args: { observe: 'none' } }, + { tool: 'click', args: { observe: 'full' } }, + ], + }; + expect(resolveObserveLevel('batch', input, batchResult([{}, {}]))).toBe( + 'full', ); }); - it('returns false for cdp', () => { - expect(shouldIncludeObservationsInResponse('cdp', okResult)).toBe(false); - }); - - it("returns true for batch with 'all' (default)", () => { - expect(shouldIncludeObservationsInResponse('batch', okResult, {})).toBe( - true, + it('ignores observe on steps other than the last executed one', () => { + const input = { + steps: [{ tool: 'click', args: { observe: 'none' } }, { tool: 'click' }], + }; + expect(resolveObserveLevel('batch', input, batchResult([{}, {}]))).toBe( + 'diff', ); }); - it("returns false for batch with 'none'", () => { - expect( - shouldIncludeObservationsInResponse('batch', okResult, { - includeObservations: 'none', - }), - ).toBe(false); - }); - - it("returns true for batch with 'failures' when tool failed", () => { - expect( - shouldIncludeObservationsInResponse('batch', failResult, { - includeObservations: 'failures', - }), - ).toBe(true); + it('uses the last executed step when run_steps stopped early', () => { + const input = { + steps: [ + { tool: 'click', args: { observe: 'full' } }, + { tool: 'click', args: { observe: 'none' } }, + ], + }; + expect(resolveObserveLevel('batch', input, batchResult([{}]))).toBe('full'); }); - it("returns true for batch with 'failures' when summary.ok is false", () => { + it('does not count steps skipped by batchTimeoutMs', () => { + const input = { + steps: [ + { tool: 'click', args: { observe: 'none' } }, + { tool: 'click', args: { observe: 'full' } }, + ], + }; expect( - shouldIncludeObservationsInResponse('batch', summaryFailResult, { - includeObservations: 'failures', - }), - ).toBe(true); + resolveObserveLevel('batch', input, batchResult([{}, { skipped: true }])), + ).toBe('none'); }); - it("returns false for batch with 'failures' when tool succeeded", () => { - const batchOk: ToolResponse = { - ok: true, - result: { summary: { ok: true } }, + it('prefers the top-level run_steps observe over the last step', () => { + const input = { + observe: 'none', + steps: [{ tool: 'click', args: { observe: 'full' } }], }; - expect( - shouldIncludeObservationsInResponse('batch', batchOk, { - includeObservations: 'failures', - }), - ).toBe(false); + expect(resolveObserveLevel('batch', input, batchResult([{}]))).toBe('none'); }); - it("returns false for batch with 'failures' when summary is missing", () => { - expect( - shouldIncludeObservationsInResponse('batch', okResult, { - includeObservations: 'failures', - }), - ).toBe(false); + it('defaults run_steps to diff when the batch failed or has no steps', () => { + const input = { steps: [{ tool: 'click', args: { observe: 'full' } }] }; + const failed: ToolResponse = { + ok: false, + error: { code: 'ERR', message: 'fail' }, + }; + expect(resolveObserveLevel('batch', input, failed)).toBe('diff'); + expect(resolveObserveLevel('batch', input, { ok: true, result: {} })).toBe( + 'diff', + ); + expect(resolveObserveLevel('batch', input, batchResult([]))).toBe('diff'); }); }); @@ -823,7 +812,7 @@ describe('createServer with active session', () => { expect(body.observations).toBeUndefined(); }); - it('mutating tool response includes observations with state, testIds, a11y', async () => { + it('mutating tool response includes observations with testIds and a11y but no state', async () => { const res = await httpRequest(`http://127.0.0.1:${state.port}/cleanup`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -831,12 +820,12 @@ describe('createServer with active session', () => { }); const body = (await res.json()) as { ok: boolean; - observations?: { state: unknown; testIds: unknown[]; a11y: unknown }; + observations?: Record; }; expect(res.status).toBe(200); expect(body.observations).toBeDefined(); - expect(body.observations?.state).toBeDefined(); + expect(body.observations).not.toHaveProperty('state'); expect(body.observations?.testIds).toBeDefined(); expect(body.observations?.a11y).toBeDefined(); }); @@ -927,99 +916,39 @@ describe('createServer with active session', () => { expect(collectA11ySpy).toHaveBeenCalled(); }); - describe('post-mutation state recheck', () => { - it('resolves immediately when getExtensionState returns a known screen', async () => { - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState.mockResolvedValue({ - isLoaded: true, - currentScreen: 'home', - currentUrl: 'chrome-extension://test/home.html', - }); - + describe('observation without app state', () => { + it('does not call getExtensionState when observing a mutating tool', async () => { const res = await httpRequest(`http://127.0.0.1:${state.port}/cleanup`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({}), }); - const body = (await res.json()) as { - ok: boolean; - observations?: { state: { currentScreen?: string } }; - }; - - expect(res.status).toBe(200); - expect(mockSM.getExtensionState).toHaveBeenCalledTimes(1); - expect(body.observations?.state.currentScreen).toBe('home'); - }); - - it("retries when first call returns 'unknown', resolves on second call", async () => { - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState - .mockResolvedValueOnce({ - isLoaded: true, - currentScreen: 'unknown', - currentUrl: 'chrome-extension://test/unknown.html', - }) - .mockResolvedValueOnce({ - isLoaded: true, - currentScreen: 'home', - currentUrl: 'chrome-extension://test/home.html', - }); - - const res = await httpRequest(`http://127.0.0.1:${state.port}/cleanup`, { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({}), - }); - const body = (await res.json()) as { - ok: boolean; - observations?: { state: { currentScreen?: string } }; - }; + const body = (await res.json()) as { observations?: unknown }; expect(res.status).toBe(200); - expect(mockSM.getExtensionState).toHaveBeenCalledTimes(2); - expect(body.observations?.state.currentScreen).toBe('home'); + expect(body.observations).toBeDefined(); + expect(mockSM.getExtensionState).not.toHaveBeenCalled(); }); - it("retries up to deadline and returns 'unknown' if all calls return 'unknown'", async () => { - vi.useFakeTimers(); - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState.mockResolvedValue({ - isLoaded: true, - currentScreen: 'unknown', - currentUrl: 'chrome-extension://test/unknown.html', - }); - - const responsePromise = httpRequest( - `http://127.0.0.1:${state.port}/cleanup`, + it('does not call getExtensionState when observing run_steps', async () => { + const res = await httpRequest( + `http://127.0.0.1:${state.port}/tool/run_steps`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({}), + body: JSON.stringify({ + steps: [{ tool: 'get_context', args: {} }], + }), }, ); - - await vi.advanceTimersByTimeAsync(500); - vi.useRealTimers(); - - const res = await responsePromise; - const body = (await res.json()) as { - ok: boolean; - observations?: { state: { currentScreen?: string } }; - }; + const body = (await res.json()) as { observations?: unknown }; expect(res.status).toBe(200); - expect(mockSM.getExtensionState).toHaveBeenCalledTimes(6); - expect(body.observations?.state.currentScreen).toBe('unknown'); + expect(body.observations).toBeDefined(); + expect(mockSM.getExtensionState).not.toHaveBeenCalled(); }); it('collects no observation for readonly tool category', async () => { - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState.mockResolvedValue({ - isLoaded: true, - currentScreen: 'unknown', - currentUrl: 'chrome-extension://test/unknown.html', - }); - const res = await httpRequest( `http://127.0.0.1:${state.port}/tool/get_context`, { @@ -1032,17 +961,9 @@ describe('createServer with active session', () => { expect(res.status).toBe(200); expect(body.observations).toBeUndefined(); - expect(mockSM.getExtensionState).not.toHaveBeenCalled(); }); it('collects no observation for discovery tool category', async () => { - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState.mockResolvedValue({ - isLoaded: true, - currentScreen: 'unknown', - currentUrl: 'chrome-extension://test/unknown.html', - }); - const res = await httpRequest( `http://127.0.0.1:${state.port}/tool/list_testids`, { @@ -1055,30 +976,134 @@ describe('createServer with active session', () => { expect(res.status).toBe(200); expect(body.observations).toBeUndefined(); - expect(mockSM.getExtensionState).not.toHaveBeenCalled(); }); + }); - it('does not recheck for batch tool category', async () => { - mockSM.getExtensionState.mockReset(); - mockSM.getExtensionState.mockResolvedValue({ - isLoaded: true, - currentScreen: 'unknown', - currentUrl: 'chrome-extension://test/unknown.html', - }); + describe('observe level', () => { + type ObservedBody = { + ok: boolean; + result?: Record; + observations?: Record; + meta?: { timing: { observationMs: number } }; + }; + const post = async ( + urlPath: string, + requestBody: unknown, + ): Promise<{ status: number; body: ObservedBody }> => { const res = await httpRequest( - `http://127.0.0.1:${state.port}/tool/run_steps`, + `http://127.0.0.1:${state.port}${urlPath}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ - steps: [{ tool: 'get_context', args: {} }], - }), + body: JSON.stringify(requestBody), }, ); + return { status: res.status, body: (await res.json()) as ObservedBody }; + }; - expect(res.status).toBe(200); - expect(mockSM.getExtensionState).toHaveBeenCalledTimes(1); + it('click with observe none returns no observations and observationMs 0', async () => { + const { collectTestIds } = await import('../tools/utils/discovery.js'); + vi.mocked(collectTestIds).mockClear(); + + const { status, body } = await post('/tool/click', { + a11yRef: 'e1', + observe: 'none', + }); + + expect(status).toBe(200); + expect(body.observations).toBeUndefined(); + expect(body.meta?.timing.observationMs).toBe(0); + expect(collectTestIds).not.toHaveBeenCalled(); + }); + + it('get_state with observe full includes an observation without state', async () => { + mockSM.getPlatformDriver.mockReturnValue({ + getPlatform: () => 'browser', + getAppState: vi.fn().mockResolvedValue({ + isLoaded: true, + extensionId: 'test-ext', + }), + }); + + const { status, body } = await post('/tool/get_state', { + observe: 'full', + }); + + expect(status).toBe(200); + expect(body.ok).toBe(true); + expect(body.result).toHaveProperty('state'); + expect(body.observations).toBeDefined(); + expect(body.observations).not.toHaveProperty('state'); + expect(body.meta?.timing.observationMs).toBeGreaterThanOrEqual(0); + }); + + it('get_state with observe diff includes an observation', async () => { + const { body } = await post('/tool/get_state', { observe: 'diff' }); + + expect(body.observations).toBeDefined(); + }); + + it('cdp with observe full waits for animations and returns an observation', async () => { + const page = { + waitForLoadState: vi.fn(async () => undefined), + waitForFunction: vi.fn(async () => undefined), + }; + mockSM.getPage.mockReturnValue(page as never); + mockSM.getPlatformDriver.mockReturnValue({ + getPlatform: () => 'browser', + cdp: vi.fn().mockResolvedValue({ ok: true, result: { value: 2 } }), + }); + + const { body } = await post('/tool/cdp', { + method: 'Runtime.evaluate', + observe: 'full', + }); + + expect(body.ok).toBe(true); + expect(body.observations).toBeDefined(); + expect(page.waitForFunction).toHaveBeenCalled(); + }); + + it('get_state with observe full does not wait for animations', async () => { + const page = { + waitForLoadState: vi.fn(async () => undefined), + waitForFunction: vi.fn(async () => undefined), + url: () => '', + }; + mockSM.getPage.mockReturnValue(page as never); + + const { body } = await post('/tool/get_state', { observe: 'full' }); + + expect(body.observations).toBeDefined(); + expect(page.waitForFunction).not.toHaveBeenCalled(); + }); + + it('does not pass observe to the tool handler', async () => { + await post('/launch', { force: true, observe: 'none' }); + + expect(mockSM.launch).toHaveBeenCalledWith( + expect.not.objectContaining({ observe: expect.anything() }), + ); + }); + + it('honours observe in the /cleanup body', async () => { + const { status, body } = await post('/cleanup', { observe: 'none' }); + + expect(status).toBe(200); + expect(body.ok).toBe(true); + expect(body.observations).toBeUndefined(); + expect(body.meta?.timing.observationMs).toBe(0); + }); + + it('rejects an invalid observe level', async () => { + const { status, body } = await post('/tool/click', { + a11yRef: 'e1', + observe: 'everything', + }); + + expect(status).toBe(400); + expect(body.ok).toBe(false); }); }); @@ -1139,11 +1164,9 @@ describe('createServer with active session', () => { async () => new Promise((resolve) => setTimeout(() => resolve(true), 60)), ); - mockSM.getExtensionState.mockImplementationOnce( - async () => - new Promise((resolve) => - setTimeout(() => resolve({ isLoaded: true, currentUrl: '' }), 60), - ), + const { collectTestIds } = await import('../tools/utils/discovery.js'); + vi.mocked(collectTestIds).mockImplementationOnce( + async () => new Promise((resolve) => setTimeout(() => resolve([]), 60)), ); const { body } = await post('/cleanup'); @@ -1180,11 +1203,9 @@ describe('createServer with active session', () => { ); it('reports observationMs above 0 for mutating tools', async () => { - mockSM.getExtensionState.mockImplementationOnce( - async () => - new Promise((resolve) => - setTimeout(() => resolve({ isLoaded: true, currentUrl: '' }), 20), - ), + const { collectTestIds } = await import('../tools/utils/discovery.js'); + vi.mocked(collectTestIds).mockImplementationOnce( + async () => new Promise((resolve) => setTimeout(() => resolve([]), 20)), ); const { body } = await post('/cleanup'); @@ -1740,7 +1761,166 @@ describe('observation compaction in HTTP responses', () => { expect(body.observations?.a11y.nodes).toHaveLength(10); }); - it('batch with includeObservations=all returns compact observations', async () => { + describe('observe levels on a second mutation', () => { + type CompactBody = { + ok: boolean; + observations?: { + a11y: { + nodes: unknown[]; + diff?: { added: string[]; removed: string[]; unchanged: number }; + }; + }; + }; + + const post = async ( + urlPath: string, + requestBody: unknown, + ): Promise => { + const res = await httpRequest( + `http://127.0.0.1:${state.port}${urlPath}`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(requestBody), + }, + ); + return (await res.json()) as CompactBody; + }; + + beforeEach(async () => { + const { collectTrimmedA11ySnapshot } = + await import('../tools/utils/discovery.js'); + vi.mocked(collectTrimmedA11ySnapshot) + .mockResolvedValueOnce({ + nodes: initialButtons as never, + refMap: new Map(), + }) + .mockResolvedValueOnce({ + nodes: changedButtons as never, + refMap: new Map(), + }); + }); + + afterEach(async () => { + // Drop queued snapshots a test did not consume. + const { collectTrimmedA11ySnapshot } = + await import('../tools/utils/discovery.js'); + vi.mocked(collectTrimmedA11ySnapshot).mockReset(); + }); + + it('diff returns only the changes', async () => { + await post('/tool/click', { a11yRef: 'e1' }); + const body = await post('/tool/click', { + a11yRef: 'e1', + observe: 'diff', + }); + + expect(body.observations?.a11y.diff).toStrictEqual({ + added: ['e3'], + removed: ['e2'], + unchanged: 1, + }); + expect(body.observations?.a11y.nodes).toHaveLength(1); + }); + + it('full returns the whole screen with no diff', async () => { + await post('/tool/click', { a11yRef: 'e1' }); + const body = await post('/tool/click', { + a11yRef: 'e1', + observe: 'full', + }); + + expect(body.observations?.a11y.diff).toBeUndefined(); + expect(body.observations?.a11y.nodes).toStrictEqual(changedButtons); + }); + + it('full still moves the diff baseline', async () => { + const { collectTrimmedA11ySnapshot } = + await import('../tools/utils/discovery.js'); + vi.mocked(collectTrimmedA11ySnapshot).mockResolvedValueOnce({ + nodes: initialButtons as never, + refMap: new Map(), + }); + + await post('/tool/click', { a11yRef: 'e1' }); + await post('/tool/click', { a11yRef: 'e1', observe: 'full' }); + const body = await post('/tool/click', { a11yRef: 'e1' }); + + expect(body.observations?.a11y.diff).toStrictEqual({ + added: ['e2'], + removed: ['e3'], + unchanged: 1, + }); + }); + + it('none does not move the diff baseline', async () => { + const { collectTrimmedA11ySnapshot } = + await import('../tools/utils/discovery.js'); + vi.mocked(collectTrimmedA11ySnapshot).mockClear(); + + await post('/tool/click', { a11yRef: 'e1' }); + await post('/tool/click', { a11yRef: 'e1', observe: 'none' }); + const body = await post('/tool/click', { a11yRef: 'e1' }); + + // The `none` call collected nothing, so the second snapshot (changed + // buttons) is diffed against the first one. + expect(vi.mocked(collectTrimmedA11ySnapshot)).toHaveBeenCalledTimes(2); + expect(body.observations?.a11y.diff).toStrictEqual({ + added: ['e3'], + removed: ['e2'], + unchanged: 1, + }); + }); + + it('run_steps with top-level observe full returns no diff', async () => { + await post('/tool/click', { a11yRef: 'e1' }); + const body = await post('/tool/run_steps', { + steps: [{ tool: 'get_context' }], + observe: 'full', + }); + + expect(body.observations?.a11y.diff).toBeUndefined(); + expect(body.observations?.a11y.nodes).toStrictEqual(changedButtons); + }); + + it('run_steps uses the last step observe when there is no top-level one', async () => { + await post('/tool/click', { a11yRef: 'e1' }); + const body = await post('/tool/run_steps', { + steps: [ + { tool: 'get_context', args: { observe: 'none' } }, + { tool: 'get_context', args: { observe: 'full' } }, + ], + }); + + expect(body.observations?.a11y.diff).toBeUndefined(); + expect(body.observations?.a11y.nodes).toStrictEqual(changedButtons); + }); + + it('run_steps with last step observe none omits observations', async () => { + const body = await post('/tool/run_steps', { + steps: [{ tool: 'get_context', args: { observe: 'none' } }], + }); + + expect(body.ok).toBe(true); + expect(body.observations).toBeUndefined(); + }); + + it('run_steps top-level observe wins over the last step', async () => { + await post('/tool/click', { a11yRef: 'e1' }); + const body = await post('/tool/run_steps', { + steps: [{ tool: 'get_context', args: { observe: 'full' } }], + observe: 'diff', + }); + + expect(body.observations?.a11y.diff).toStrictEqual({ + added: ['e3'], + removed: ['e2'], + unchanged: 1, + }); + }); + }); + + it('batch without observe returns compact observations', async () => { const res = await httpRequest( `http://127.0.0.1:${state.port}/tool/run_steps`, { @@ -1748,7 +1928,6 @@ describe('observation compaction in HTTP responses', () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ steps: [{ tool: 'get_state' }], - includeObservations: 'all', }), }, ); @@ -1762,7 +1941,7 @@ describe('observation compaction in HTTP responses', () => { expect(body.observations?.a11y.nodes).toHaveLength(3); }); - it('batch with includeObservations=none omits observations', async () => { + it('batch with observe none omits observations', async () => { const res = await httpRequest( `http://127.0.0.1:${state.port}/tool/run_steps`, { @@ -1770,7 +1949,7 @@ describe('observation compaction in HTTP responses', () => { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ steps: [{ tool: 'get_state' }], - includeObservations: 'none', + observe: 'none', }), }, ); @@ -1820,14 +1999,8 @@ describe('createServer observation driver freshness after launch', () => { getPlatform: vi.fn().mockReturnValue('ios'), getAppState: vi.fn().mockResolvedValue({ isLoaded: true, - currentUrl: '', extensionId: 'io.metamask', - isUnlocked: true, - currentScreen: 'unknown', - accountAddress: null, - networkName: null, - chainId: null, - balance: null, + source: 'stub', }), getTestIds: vi.fn().mockResolvedValue([]), getAccessibilityTree: vi @@ -1870,18 +2043,19 @@ describe('createServer observation driver freshness after launch', () => { }); const body = (await res.json()) as { ok: boolean; - observations?: { state: unknown }; + observations?: Record; }; expect(res.status).toBe(200); expect(body.ok).toBe(true); - // The mobile driver's getAppState should have been called for - // observation collection, NOT the Playwright path (which would - // fail since there's no browser page). - expect(mobileDriver.getAppState).toHaveBeenCalled(); + // The mobile driver should have been used for observation collection, + // NOT the Playwright path (which would fail since there's no browser + // page). Observations no longer read app state. + expect(mobileDriver.getAppState).not.toHaveBeenCalled(); expect(mobileDriver.getTestIds).toHaveBeenCalled(); expect(mobileDriver.getAccessibilityTree).toHaveBeenCalled(); expect(body.observations).toBeDefined(); + expect(body.observations).not.toHaveProperty('state'); }); it('blocks browser-only tools on a mobile platform', async () => { diff --git a/src/server/create-server.ts b/src/server/create-server.ts index 4430025..2e792a9 100644 --- a/src/server/create-server.ts +++ b/src/server/create-server.ts @@ -22,9 +22,10 @@ import { checkPlatformGate, } from '../tools/registry.js'; import type { ToolCategory } from '../tools/registry.js'; -import type { RunStepsInput } from '../tools/types'; import { createDefaultObservation } from '../tools/types/observation.js'; import type { StepRecordObservation } from '../tools/types/observation.js'; +import type { RunStepsInput } from '../tools/types/tool-inputs.js'; +import type { RunStepsResult } from '../tools/types/tool-outputs.js'; import { ANIMATION_SETTLE_TIMEOUT_MS, GRACEFUL_SHUTDOWN_TIMEOUT_MS, @@ -44,8 +45,12 @@ import type { } from '../types/http.js'; import { extractErrorMessage } from '../utils/errors.js'; import { computeTimeoutBudget } from '../utils/timeout-budget.js'; -import type { ToolName } from '../validation/schemas.js'; -import { toolSchemas } from '../validation/schemas.js'; +import type { ObserveLevel, ToolName } from '../validation/schemas.js'; +import { + observeLevelSchema, + omitObserve, + toolSchemas, +} from '../validation/schemas.js'; export type ServerInstance = { start(): Promise; @@ -75,74 +80,77 @@ export function buildResponseBody( } /** - * Whether to collect an observation (state, test IDs, a11y tree) after a tool - * runs. Only tools that change the UI pay for it: - * - * - `mutating`: yes. - * - `batch` (`run_steps`): yes, unless `includeObservations` is `'none'`. - * - `readonly`, `discovery`, `cdp`: no. + * Reads a valid `observe` level from an input payload. * - * @param category - The tool category to check. - * @param validatedInput - The validated input payload (checked for batch policy). - * @returns True if observations should be collected. + * @param input - The input payload. + * @returns The level, or undefined when absent or invalid. */ -export function shouldCollectObservations( - category: ToolCategory, - validatedInput?: Record, -): boolean { - if (category === 'mutating') { - return true; - } - if (category === 'batch') { - return batchObservationPolicy(validatedInput) !== 'none'; +function readObserveLevel(input: unknown): ObserveLevel | undefined { + if (typeof input !== 'object' || input === null) { + return undefined; } - return false; + const parsed = observeLevelSchema.safeParse( + (input as { observe?: unknown }).observe, + ); + return parsed.success ? parsed.data : undefined; } /** - * Whether to include the collected observation in the HTTP response. Never - * true when {@link shouldCollectObservations} is false. For `run_steps` with - * `includeObservations: 'failures'`, only failed batches include it. + * Finds the `observe` level of the last step that `run_steps` executed. + * Steps skipped by `batchTimeoutMs` do not count. * - * @param category - The tool category. - * @param toolResult - The result returned by the tool. - * @param validatedInput - The validated input payload (used for batch policy). - * @returns True if observations should be included in the response. + * @param validatedInput - The validated `run_steps` input. + * @param toolResult - The `run_steps` result. + * @returns The last executed step's level, or undefined when it has none. */ -export function shouldIncludeObservationsInResponse( - category: ToolCategory, - toolResult: ToolResponse, - validatedInput?: Record, -): boolean { - if (!shouldCollectObservations(category, validatedInput)) { - return false; +function lastExecutedStepObserveLevel( + validatedInput: Record | undefined, + toolResult: ToolResponse | undefined, +): ObserveLevel | undefined { + if (!toolResult?.ok) { + return undefined; } - if ( - category === 'batch' && - batchObservationPolicy(validatedInput) === 'failures' - ) { - if (!toolResult.ok) { - return true; - } - const result = toolResult.result as Record; - const summary = result?.summary as Record | undefined; - return summary !== undefined && !summary.ok; + const steps = (validatedInput as Partial | undefined)?.steps; + const results = (toolResult.result as Partial | undefined) + ?.steps; + if (!Array.isArray(steps) || !Array.isArray(results)) { + return undefined; } - return true; + const executed = results.filter((step) => step.meta?.skipped !== true); + return readObserveLevel(steps[executed.length - 1]?.args); } /** - * Reads the `run_steps` observation policy. + * Resolves the observation level for a tool call. An explicit `observe` in + * the input wins. Otherwise: * - * @param validatedInput - The validated `run_steps` input. - * @returns The policy, `'all'` by default. + * - `mutating`: `'diff'`. + * - `batch` (`run_steps`): the `observe` of the last executed step, else + * `'diff'`. Other per-step `observe` values are ignored. + * - `readonly`, `discovery`, `cdp`: `'none'`. + * + * @param category - The tool category. + * @param validatedInput - The validated input payload. + * @param toolResult - The tool result (used by `run_steps` to find the last + * executed step). + * @returns The observation level. */ -function batchObservationPolicy( - validatedInput: Record | undefined, -): NonNullable { - const policy = - validatedInput?.includeObservations as RunStepsInput['includeObservations']; - return policy ?? 'all'; +export function resolveObserveLevel( + category: ToolCategory, + validatedInput?: Record, + toolResult?: ToolResponse, +): ObserveLevel { + const explicit = readObserveLevel(validatedInput); + if (explicit) { + return explicit; + } + if (category === 'mutating') { + return 'diff'; + } + if (category === 'batch') { + return lastExecutedStepObserveLevel(validatedInput, toolResult) ?? 'diff'; + } + return 'none'; } /** @@ -253,8 +261,11 @@ export function createServer(config: ServerConfig): ServerInstance { res: express.Response, ): Promise { const timer = new RequestTimer(requestStartOf(res)); - const tool = toolRegistry.get(toolName); - if (!tool) { + // Only names registered in the tool registry may be dispatched. + const tool = toolRegistry.has(toolName) + ? toolRegistry.get(toolName) + : undefined; + if (typeof tool !== 'function') { res.status(404).json( withMeta( { @@ -338,21 +349,16 @@ export function createServer(config: ServerConfig): ServerInstance { try { timer.markEnqueued(); - const { toolResult, observations } = await queue.enqueue( + const { toolResult, observeLevel, observations } = await queue.enqueue( async () => { timer.markTaskStart(); const context = buildToolContext(currentWorkflowContext); - const result = await tool(validatedInput, context); + const result = await tool(omitObserve(validatedInput), context); timer.markToolEnd(); + const level = resolveObserveLevel(category, inputRecord, result); let obs: StepRecordObservation | undefined; - if ( - shouldCollectObservations( - category, - validatedInput as Record, - ) && - config.sessionManager.hasActiveSession() - ) { + if (level !== 'none' && config.sessionManager.hasActiveSession()) { timer.markObservationStart(); try { obs = await Promise.race([ @@ -370,19 +376,21 @@ export function createServer(config: ServerConfig): ServerInstance { currentDriver && currentDriver.getPlatform() !== 'browser' ) { - const state = await currentDriver.getAppState(); const testIds = await currentDriver.getTestIds( OBSERVATION_TESTID_LIMIT, ); const { nodes, refMap: newRefMap } = await currentDriver.getAccessibilityTree(); config.sessionManager.setRefMap(newRefMap); - return createDefaultObservation(state, testIds, nodes); + return createDefaultObservation(testIds, nodes); } const page = config.sessionManager.getPage(); - if (category === 'mutating') { + // 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); @@ -403,28 +411,6 @@ export function createServer(config: ServerConfig): ServerInstance { ) .catch(() => undefined); } - let state = await config.sessionManager.getExtensionState(); - - if ( - category === 'mutating' && - state.currentScreen === 'unknown' - ) { - const RECHECK_DEADLINE_MS = 500; - const RECHECK_INTERVAL_MS = 100; - const deadline = Date.now() + RECHECK_DEADLINE_MS; - - while (Date.now() < deadline) { - await new Promise((resolve) => - setTimeout(resolve, RECHECK_INTERVAL_MS), - ); - const rechecked = - await config.sessionManager.getExtensionState(); - if (rechecked.currentScreen !== 'unknown') { - state = rechecked; - break; - } - } - } const testIds = await collectTestIds( page, OBSERVATION_TESTID_LIMIT, @@ -432,7 +418,7 @@ export function createServer(config: ServerConfig): ServerInstance { const { nodes, refMap: newRefMap } = await collectTrimmedA11ySnapshot(page); config.sessionManager.setRefMap(newRefMap); - return createDefaultObservation(state, testIds, nodes); + return createDefaultObservation(testIds, nodes); })(), new Promise((resolve) => setTimeout(() => resolve(undefined), OBSERVATION_TIMEOUT_MS), @@ -444,20 +430,23 @@ export function createServer(config: ServerConfig): ServerInstance { timer.markObservationEnd(); } - return { toolResult: result, observations: obs }; + return { + toolResult: result, + observeLevel: level, + observations: obs, + }; }, queueTimeoutMs ? { timeoutMs: queueTimeoutMs } : undefined, ); - const includeInResponse = shouldIncludeObservationsInResponse( - category, - toolResult, - validatedInput as Record, - ); - const responseObservations = - includeInResponse && observations - ? compactObservation(observations, lastObservation) - : undefined; + // 'full' skips the diff but is still option-collapsed, like the first + // observation of a session. + const responseObservations = observations + ? compactObservation( + observations, + observeLevel === 'full' ? null : lastObservation, + ) + : undefined; res.json( withMeta( buildResponseBody(toolResult, responseObservations), @@ -495,8 +484,10 @@ export function createServer(config: ServerConfig): ServerInstance { await executeTool('launch', req.body, res); }); - app.post('/cleanup', async (_req, res) => { - await executeTool('cleanup', {}, res); + app.post('/cleanup', async (req, res) => { + // Only `observe` is read from the body; other fields are ignored. + const observe = (req.body as { observe?: unknown } | undefined)?.observe; + await executeTool('cleanup', observe === undefined ? {} : { observe }, res); }); app.post( diff --git a/src/server/observation-compaction.test.ts b/src/server/observation-compaction.test.ts index f54b5f3..c88e857 100644 --- a/src/server/observation-compaction.test.ts +++ b/src/server/observation-compaction.test.ts @@ -39,7 +39,6 @@ function createObservation( overrides: Partial = {}, ): StepRecordObservation { return { - state: overrides.state ?? ({} as StepRecordObservation['state']), testIds: overrides.testIds ?? [], a11y: { nodes, @@ -189,10 +188,8 @@ describe('collapseOptionSubtrees', () => { describe('compactObservation', () => { it('preserves non-a11y fields by reference while returning a new object', () => { - const state = { connected: true }; const testIds = [{ testId: 'submit', tag: 'button', visible: true }]; const observation = { - state, testIds, a11y: { nodes: [createNode('e1', 'combobox'), ...createOptionRun(4, 2)], @@ -202,7 +199,7 @@ describe('compactObservation', () => { const result = compactObservation(observation); expect(result).not.toBe(observation); - expect(result.state).toBe(state); + expect(result).not.toHaveProperty('state'); expect(result.testIds).toBe(testIds); expect(result.a11y).not.toBe(observation.a11y); expect(result.a11y.nodes).toStrictEqual([ @@ -218,7 +215,6 @@ describe('compactObservation', () => { it('is idempotent when called repeatedly on the same result', () => { const observation = { - state: {}, testIds: [], a11y: { nodes: [createNode('e1', 'listbox'), ...createOptionRun(6, 2)], @@ -269,7 +265,6 @@ describe('compactObservation', () => { it('falls back to the original observation when compaction throws', () => { const observation = { - state: {}, testIds: [], a11y: { nodes: [createNode('e1', 'combobox')] }, } as unknown as StepRecordObservation; @@ -287,7 +282,6 @@ describe('compactObservation', () => { it('handles empty node arrays gracefully', () => { const observation = { - state: {}, testIds: [], a11y: { nodes: [] }, } as unknown as StepRecordObservation; @@ -297,59 +291,6 @@ describe('compactObservation', () => { expect(result).not.toBe(observation); expect(result.a11y.nodes).toStrictEqual([]); }); - - describe('activeTab passthrough', () => { - it('preserves state.activeTab when present', () => { - const state = { - isLoaded: true, - currentUrl: 'chrome-extension://extension/home.html', - extensionId: 'extension-id', - isUnlocked: true, - currentScreen: 'home', - accountAddress: '0x123', - networkName: 'Ethereum Mainnet', - chainId: 1, - balance: '1 ETH', - activeTab: { - role: 'dapp', - url: 'https://app.uniswap.org/', - title: 'Uniswap', - }, - } satisfies StepRecordObservation['state']; - const observation = createObservation( - [createNode('e1', 'listbox'), ...createOptionRun(6, 2)], - { state }, - ); - - const result = compactObservation(observation); - - expect(result.state).toBe(state); - expect(result.state.activeTab).toStrictEqual(state.activeTab); - }); - - it('works when state.activeTab is undefined (backward compat)', () => { - const state = { - isLoaded: true, - currentUrl: 'chrome-extension://extension/home.html', - extensionId: 'extension-id', - isUnlocked: false, - currentScreen: 'unlock', - accountAddress: null, - networkName: null, - chainId: null, - balance: null, - } satisfies StepRecordObservation['state']; - const observation = createObservation( - [createNode('e1', 'listbox'), ...createOptionRun(6, 2)], - { state }, - ); - - const result = compactObservation(observation); - - expect(result.state).toBe(state); - expect(result.state.activeTab).toBeUndefined(); - }); - }); }); describe('nodeChanged', () => { @@ -521,20 +462,16 @@ describe('diffObservation', () => { }); }); - it('preserves the current state and testIds', () => { - const state = { - mode: 'current', - } as unknown as StepRecordObservation['state']; + it('preserves the current testIds and adds no state', () => { const testIds = [{ testId: 'submit', tag: 'button', visible: true }]; const previous = createObservation([]); const current = createObservation([createNode('e1', 'button')], { - state, testIds, }); const result = diffObservation(current, previous); - expect(result.state).toBe(state); + expect(result).not.toHaveProperty('state'); expect(result.testIds).toBe(testIds); }); }); diff --git a/src/server/observation-compaction.ts b/src/server/observation-compaction.ts index 76aa658..7121811 100644 --- a/src/server/observation-compaction.ts +++ b/src/server/observation-compaction.ts @@ -246,7 +246,6 @@ export function diffObservation( } return { - state: current.state, testIds: current.testIds, a11y: { nodes: changedOrNewNodes, diff --git a/src/tools/batch.test.ts b/src/tools/batch.test.ts index 39d3bbc..742d4cd 100644 --- a/src/tools/batch.test.ts +++ b/src/tools/batch.test.ts @@ -479,7 +479,7 @@ describe('runStepsTool', () => { } }); - it('excludes observations when includeObservations is "none"', async () => { + it('accepts observe in step args and does not pass it to the handler', async () => { const clickHandler = vi.fn().mockResolvedValue({ ok: true, result: { clicked: true }, @@ -490,16 +490,41 @@ describe('runStepsTool', () => { const result = await runStepsTool( { - steps: [{ tool: 'click', args: { testId: 'btn' } }], - includeObservations: 'none', + steps: [{ tool: 'click', args: { testId: 'btn', observe: 'full' } }], }, context, ); expect(result.ok).toBe(true); if (result.ok) { + expect(result.result.steps[0].ok).toBe(true); expect(result.result.steps[0]).not.toHaveProperty('observation'); } + expect(clickHandler).toHaveBeenCalledWith( + expect.not.objectContaining({ observe: expect.anything() }), + context, + ); + expect(clickHandler.mock.calls[0][0]).toMatchObject({ testId: 'btn' }); + }); + + it('rejects an invalid observe level in step args', async () => { + const clickHandler = vi.fn(); + const context = createMockContext({ + toolRegistry: new Map([['click', clickHandler]]), + }); + + const result = await runStepsTool( + { + steps: [{ tool: 'click', args: { testId: 'btn', observe: 'all' } }], + }, + context, + ); + + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.result.steps[0].error?.code).toBe('VALIDATION_ERROR'); + } + expect(clickHandler).not.toHaveBeenCalled(); }); it('marks remaining steps as skipped when batchTimeoutMs is exceeded', async () => { diff --git a/src/tools/batch.ts b/src/tools/batch.ts index 45c7e36..c4a44bc 100644 --- a/src/tools/batch.ts +++ b/src/tools/batch.ts @@ -5,7 +5,7 @@ import { 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'; -import { toolSchemas } from '../validation/schemas.js'; +import { omitObserve, toolSchemas } from '../validation/schemas.js'; /** Tools whose args include a target selection (a11yRef/testId/selector). */ const TARGET_TOOLS = new Set(['click', 'type', 'wait_for']); @@ -231,7 +231,7 @@ export async function runStepsTool( } try { - const response = await handler(validatedArgs, context); + const response = await handler(omitObserve(validatedArgs), context); stepResults.push({ tool, diff --git a/src/tools/registry.test.ts b/src/tools/registry.test.ts index 8055ec4..1a6317b 100644 --- a/src/tools/registry.test.ts +++ b/src/tools/registry.test.ts @@ -7,6 +7,7 @@ import { isBrowserOnlyTool, isMobileOnlyTool, } from './registry.js'; +import { toolSchemas } from '../validation/schemas.js'; describe('toolRegistry', () => { it('has expected tool entries', () => { @@ -57,6 +58,12 @@ describe('toolRegistry', () => { }); describe('TOOL_CATEGORIES and getToolCategory', () => { + it('every key in toolRegistry has an input schema', () => { + for (const key of toolRegistry.keys()) { + expect(Object.hasOwn(toolSchemas, key)).toBe(true); + } + }); + it('every key in toolRegistry exists in TOOL_CATEGORIES', () => { for (const key of toolRegistry.keys()) { expect(TOOL_CATEGORIES).toHaveProperty(key); diff --git a/src/tools/registry.ts b/src/tools/registry.ts index 9656d98..16f6d03 100644 --- a/src/tools/registry.ts +++ b/src/tools/registry.ts @@ -105,15 +105,15 @@ export const toolRegistry = new Map>([ ]); /** - * Tool categories decide whether the daemon collects an observation after the - * tool runs (see `shouldCollectObservations` in `server/create-server.ts`): + * Tool categories set the default `observe` level, which decides whether the + * daemon collects an observation after the tool runs (see + * `resolveObserveLevel` in `server/create-server.ts`). An explicit `observe` + * in the input always wins. * - * - `mutating`: changes the UI; an observation is collected and returned. - * - `readonly` / `discovery`: reads only; no observation. - * - `cdp`: raw CDP calls; mostly reads (`Runtime.evaluate`), so no observation - * (until a per-call `observe` level lets the caller ask for one). - * - `batch`: `run_steps`; one observation after the batch unless - * `includeObservations` is `'none'`. + * - `mutating`: changes the UI; default `'diff'`. + * - `readonly` / `discovery`: reads only; default `'none'`. + * - `cdp`: raw CDP calls; mostly reads (`Runtime.evaluate`), default `'none'`. + * - `batch`: `run_steps`; one observation after the batch, default `'diff'`. */ export type ToolCategory = | 'mutating' diff --git a/src/tools/types/observation.test.ts b/src/tools/types/observation.test.ts index 4e8cfc1..81451a7 100644 --- a/src/tools/types/observation.test.ts +++ b/src/tools/types/observation.test.ts @@ -2,30 +2,16 @@ import { describe, expect, it } from 'vitest'; import type { A11yNodeTrimmed, TestIdItem } from './discovery.js'; import { createDefaultObservation } from './observation.js'; -import type { ExtensionState } from '../../capabilities/types.js'; - -const state = { - isLoaded: true, - currentUrl: 'chrome-extension://ext/home.html', - extensionId: 'ext', - isUnlocked: true, - currentScreen: 'home', - accountAddress: null, - networkName: null, - chainId: null, - balance: null, -} as ExtensionState; describe('createDefaultObservation', () => { it('defaults testIds and a11y nodes to empty arrays', () => { - expect(createDefaultObservation(state)).toStrictEqual({ - state, + expect(createDefaultObservation()).toStrictEqual({ testIds: [], a11y: { nodes: [] }, }); }); - it('includes the provided testIds and a11y nodes', () => { + it('includes the provided testIds and a11y nodes, and no state', () => { const testIds = [ { testId: 'send-button', tag: 'button', visible: true }, ] as TestIdItem[]; @@ -33,10 +19,12 @@ describe('createDefaultObservation', () => { { ref: 'e1', role: 'button', name: 'Send', path: [] }, ] as unknown as A11yNodeTrimmed[]; - expect(createDefaultObservation(state, testIds, nodes)).toStrictEqual({ - state, + const observation = createDefaultObservation(testIds, nodes); + + expect(observation).toStrictEqual({ testIds, a11y: { nodes }, }); + expect(observation).not.toHaveProperty('state'); }); }); diff --git a/src/tools/types/observation.ts b/src/tools/types/observation.ts index 8e2682e..3e489b3 100644 --- a/src/tools/types/observation.ts +++ b/src/tools/types/observation.ts @@ -1,12 +1,11 @@ import type { TestIdItem, A11yNodeTrimmed } from './discovery.js'; -import type { ExtensionState } from '../../capabilities/types.js'; /** - * Screen snapshot collected after a tool runs: state, visible test IDs and - * the trimmed accessibility tree. + * Screen snapshot collected after a tool runs: visible test IDs and the + * trimmed accessibility tree. It carries no app state; call `get_state` for + * that. */ export type StepRecordObservation = { - state: ExtensionState; testIds: TestIdItem[]; a11y: { nodes: A11yNodeTrimmed[]; @@ -22,18 +21,15 @@ export type StepRecordObservation = { /** * Builds an observation from its parts. * - * @param state - Extension or app state snapshot. * @param testIds - Visible test IDs. * @param a11yNodes - Trimmed accessibility nodes. * @returns The observation. */ export function createDefaultObservation( - state: ExtensionState, testIds: TestIdItem[] = [], a11yNodes: A11yNodeTrimmed[] = [], ): StepRecordObservation { return { - state, testIds, a11y: { nodes: a11yNodes }, }; diff --git a/src/tools/types/tool-inputs.ts b/src/tools/types/tool-inputs.ts index 58aa1ee..ef05426 100644 --- a/src/tools/types/tool-inputs.ts +++ b/src/tools/types/tool-inputs.ts @@ -127,7 +127,6 @@ export type RunStepsInput = { args?: Record; }[]; stopOnError?: boolean; - includeObservations?: 'none' | 'failures' | 'all'; batchTimeoutMs?: number; }; diff --git a/src/tools/utils/constants.ts b/src/tools/utils/constants.ts index 8b0f535..a9a5204 100644 --- a/src/tools/utils/constants.ts +++ b/src/tools/utils/constants.ts @@ -19,7 +19,7 @@ export const ANIMATION_SETTLE_TIMEOUT_MS = 3_000; /** Maximum interval between idle-shutdown checks */ export const MAX_IDLE_CHECK_INTERVAL_MS = 60_000; -/** Hard timeout for post-tool observation collection (getExtensionState, testIds, a11y) */ +/** Hard timeout for post-tool observation collection (testIds, a11y) */ export const OBSERVATION_TIMEOUT_MS = 8_000; /** Timeout for graceful HTTP server close during daemon shutdown */ diff --git a/src/validation/schemas.test.ts b/src/validation/schemas.test.ts index f831f02..2071230 100644 --- a/src/validation/schemas.test.ts +++ b/src/validation/schemas.test.ts @@ -28,6 +28,11 @@ import { screenRecordingInputSchema, deviceLogsInputSchema, generateLocatorsInputSchema, + observeLevelSchema, + omitObserve, + runStepsInputSchema, + toolSchemas, + withObserve, } from './schemas.js'; describe('switchToTabInputSchema', () => { @@ -817,3 +822,111 @@ describe('device tool schemas', () => { }); }); }); + +describe('observe field', () => { + it('accepts none, diff and full', () => { + for (const level of ['none', 'diff', 'full']) { + expect(observeLevelSchema.safeParse(level).success).toBe(true); + } + expect(observeLevelSchema.safeParse('all').success).toBe(false); + }); + + it('is accepted by every tool schema', () => { + const minimalInputs: Record> = { + launch: {}, + navigate: { screen: 'home' }, + switch_to_tab: { role: 'dapp' }, + close_tab: { role: 'dapp' }, + click: { testId: 'btn' }, + type: { testId: 'input', text: 'hi' }, + wait_for: { testId: 'btn' }, + get_text: { testId: 'btn' }, + seed_contract: { contractName: 'hst' }, + seed_contracts: { contracts: ['hst'] }, + get_contract_address: { contractName: 'hst' }, + run_steps: { steps: [{ tool: 'click' }] }, + set_context: { context: 'e2e' }, + clipboard: { action: 'read' }, + cdp: { method: 'Runtime.evaluate' }, + mock_network: { action: 'list' }, + scroll_to_element: { testId: 'row' }, + device_swipe: { direction: 'up' }, + long_press: { testId: 'row' }, + tap_coordinates: { x: 1, y: 1 }, + dismiss_alert: { accept: true }, + open_app: { bundleId: 'io.metamask' }, + close_app: { bundleId: 'io.metamask' }, + press_button: { button: 'home' }, + device_context: { action: 'list' }, + device_clipboard: { action: 'read' }, + screen_recording: { action: 'stop' }, + }; + + for (const [name, schema] of Object.entries(toolSchemas)) { + const input = { ...(minimalInputs[name] ?? {}), observe: 'full' }; + const parsed = schema.safeParse(input); + expect({ name, success: parsed.success }).toStrictEqual({ + name, + success: true, + }); + expect((parsed.data as { observe?: string }).observe).toBe('full'); + + const invalid = schema.safeParse({ ...input, observe: 'bogus' }); + expect({ name, success: invalid.success }).toStrictEqual({ + name, + success: false, + }); + } + }); + + it('keeps defaults and the inner refine checks', () => { + const click = toolSchemas.click.safeParse({ + testId: 'btn', + observe: 'none', + }); + expect(click.data).toStrictEqual({ + testId: 'btn', + timeoutMs: 15000, + observe: 'none', + }); + + expect( + toolSchemas.click.safeParse({ testId: 'a', selector: 'b' }).success, + ).toBe(false); + expect( + toolSchemas.device_context.safeParse({ + action: 'switch', + observe: 'none', + }).success, + ).toBe(false); + }); + + it('withObserve adds an optional observe field', () => { + const schema = withObserve(runStepsInputSchema); + expect(schema.safeParse({ steps: [{ tool: 'click' }] }).success).toBe(true); + }); + + it('omitObserve removes observe and leaves other values alone', () => { + expect(omitObserve({ testId: 'btn', observe: 'full' })).toStrictEqual({ + testId: 'btn', + }); + const withoutObserve = { testId: 'btn' }; + expect(omitObserve(withoutObserve)).toBe(withoutObserve); + expect(omitObserve(null)).toBeNull(); + expect(omitObserve('text')).toBe('text'); + const list = [{ observe: 'full' }]; + expect(omitObserve(list)).toBe(list); + }); +}); + +describe('runStepsInputSchema includeObservations', () => { + it('rejects the removed includeObservations field with a hint', () => { + const parsed = toolSchemas.run_steps.safeParse({ + steps: [{ tool: 'get_state' }], + includeObservations: 'none', + }); + + expect(parsed.success).toBe(false); + expect(parsed.error?.issues[0]?.message).toContain('use observe'); + }); +}); diff --git a/src/validation/schemas.ts b/src/validation/schemas.ts index 3238fe2..1d037c5 100644 --- a/src/validation/schemas.ts +++ b/src/validation/schemas.ts @@ -2,6 +2,56 @@ import { z } from 'zod'; import { SMART_CONTRACT_NAMES, HARDFORKS } from '../tools/types/seeding.js'; +export const observeLevelSchema = z + .enum(['none', 'diff', 'full']) + .describe( + 'Observation to return after the call: ' + + 'none = skip it (fastest), ' + + 'diff = only what changed since the previous observation, ' + + 'full = the whole screen. ' + + 'Default: diff for mutating tools and run_steps, none for the others.', + ); + +export type ObserveLevel = z.infer; + +const observeFieldSchema = z.object({ + observe: observeLevelSchema.optional(), +}); + +/** + * Adds the optional `observe` field to a tool input schema. Uses an + * intersection so it works for every schema shape (objects, refined objects, + * unions and discriminated unions). + * + * @param schema - The tool input schema. + * @returns The schema with an optional `observe` field. + */ +export function withObserve( + schema: Schema, +): z.ZodIntersection { + return schema.and(observeFieldSchema); +} + +/** + * Removes the `observe` field from validated tool input. `observe` is read by + * the daemon, so tool handlers get their input without it. + * + * @param input - Validated tool input. + * @returns The input without `observe` (non-objects are returned unchanged). + */ +export function omitObserve(input: Input): Input { + if ( + typeof input !== 'object' || + input === null || + Array.isArray(input) || + !('observe' in input) + ) { + return input; + } + const { observe: _observe, ...rest } = input as Record; + return rest as Input; +} + export const a11yRefPattern = z .string() .regex(/^e[0-9]+$/u) @@ -537,13 +587,6 @@ export const runStepsInputSchema = z.object({ .boolean() .default(false) .describe('Stop execution on first error (default: false - continue)'), - includeObservations: z - .enum(['none', 'failures', 'all']) - .default('all') - .describe( - 'When to include observations in results: ' + - 'none = never (fastest), failures = only for failed steps, all = always', - ), batchTimeoutMs: z .number() .int() @@ -554,6 +597,11 @@ export const runStepsInputSchema = z.object({ 'When exceeded, remaining steps are marked as skipped and partial results are returned.', ) .optional(), + // Removed in favour of `observe`. Rejected (not ignored) so old callers + // learn about it instead of silently getting an observation. + includeObservations: z + .never({ error: 'includeObservations was removed; use observe' }) + .optional(), }); export const setContextInputSchema = z.object({ @@ -860,49 +908,49 @@ export type GenerateLocatorsInputZ = z.infer< >; export const toolSchemas = { - build: buildInputSchema, - launch: launchInputSchema, - cleanup: cleanupInputSchema, - get_state: getStateInputSchema, - navigate: navigateInputSchema, - wait_for_notification: waitForNotificationInputSchema, - switch_to_tab: switchToTabInputSchema, - close_tab: closeTabInputSchema, - list_testids: listTestIdsInputSchema, - accessibility_snapshot: accessibilitySnapshotInputSchema, - describe_screen: describeScreenInputSchema, - screenshot: screenshotInputSchema, - click: clickInputSchema, - type: typeInputSchema, - wait_for: waitForInputSchema, - get_text: getTextInputSchema, - seed_contract: seedContractInputSchema, - seed_contracts: seedContractsInputSchema, - get_contract_address: getContractAddressInputSchema, - list_contracts: listDeployedContractsInputSchema, - run_steps: runStepsInputSchema, - set_context: setContextInputSchema, - get_context: getContextInputSchema, - clipboard: clipboardInputSchema, - cdp: cdpInputSchema, - hermes_targets: hermesTargetsInputSchema, - get_window_size: getWindowSizeInputSchema, - mock_network: mockNetworkInputSchema, - scroll_to_element: scrollToElementInputSchema, - device_swipe: deviceSwipeInputSchema, - long_press: longPressInputSchema, - tap_coordinates: tapCoordinatesInputSchema, - dismiss_keyboard: dismissKeyboardInputSchema, - dismiss_alert: dismissAlertInputSchema, - get_alert_text: getAlertTextInputSchema, - open_app: openAppInputSchema, - close_app: closeAppInputSchema, - press_button: pressButtonInputSchema, - device_context: deviceContextInputSchema, - device_clipboard: deviceClipboardInputSchema, - screen_recording: screenRecordingInputSchema, - device_logs: deviceLogsInputSchema, - generate_locators: generateLocatorsInputSchema, + build: withObserve(buildInputSchema), + launch: withObserve(launchInputSchema), + cleanup: withObserve(cleanupInputSchema), + get_state: withObserve(getStateInputSchema), + navigate: withObserve(navigateInputSchema), + wait_for_notification: withObserve(waitForNotificationInputSchema), + switch_to_tab: withObserve(switchToTabInputSchema), + close_tab: withObserve(closeTabInputSchema), + list_testids: withObserve(listTestIdsInputSchema), + accessibility_snapshot: withObserve(accessibilitySnapshotInputSchema), + describe_screen: withObserve(describeScreenInputSchema), + screenshot: withObserve(screenshotInputSchema), + click: withObserve(clickInputSchema), + type: withObserve(typeInputSchema), + wait_for: withObserve(waitForInputSchema), + get_text: withObserve(getTextInputSchema), + seed_contract: withObserve(seedContractInputSchema), + seed_contracts: withObserve(seedContractsInputSchema), + get_contract_address: withObserve(getContractAddressInputSchema), + list_contracts: withObserve(listDeployedContractsInputSchema), + run_steps: withObserve(runStepsInputSchema), + set_context: withObserve(setContextInputSchema), + get_context: withObserve(getContextInputSchema), + clipboard: withObserve(clipboardInputSchema), + cdp: withObserve(cdpInputSchema), + hermes_targets: withObserve(hermesTargetsInputSchema), + get_window_size: withObserve(getWindowSizeInputSchema), + mock_network: withObserve(mockNetworkInputSchema), + scroll_to_element: withObserve(scrollToElementInputSchema), + device_swipe: withObserve(deviceSwipeInputSchema), + long_press: withObserve(longPressInputSchema), + tap_coordinates: withObserve(tapCoordinatesInputSchema), + dismiss_keyboard: withObserve(dismissKeyboardInputSchema), + dismiss_alert: withObserve(dismissAlertInputSchema), + get_alert_text: withObserve(getAlertTextInputSchema), + open_app: withObserve(openAppInputSchema), + close_app: withObserve(closeAppInputSchema), + press_button: withObserve(pressButtonInputSchema), + device_context: withObserve(deviceContextInputSchema), + device_clipboard: withObserve(deviceClipboardInputSchema), + screen_recording: withObserve(screenRecordingInputSchema), + device_logs: withObserve(deviceLogsInputSchema), + generate_locators: withObserve(generateLocatorsInputSchema), } as const; export type ToolName = keyof typeof toolSchemas;