fix!: review follow-ups: off-screen animations, no late clicks, batchTimeoutMs cap

README.md

@@ -410,7 +410,7 @@ The daemon routes `POST /tool/:name` requests through the registry, applies Zod
 | `get_contract_address`        | Looks up the deployed address of a contract by name from the session's deployment registry.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | `list_contracts`              | Lists all contracts deployed in the current session with addresses and deployment timestamps.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     |
 | **Batching**                  |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   |
-| `run_steps`                   | Executes a batch of tool invocations sequentially. Supports `stopOnError` to halt on first failure, a top-level `observe` level for the single observation collected after the batch (default `diff`; without one, the last executed step's `observe` is used), and `batchTimeoutMs` to set an overall deadline (remaining steps are skipped on timeout). Accepts tool aliases like `navigate_home` / `navigate-home`. Returns per-step results with timing.                                                                                                                                                                                                                                                                                                                                                                                                                                                      |
+| `run_steps`                   | Executes a batch of tool invocations sequentially. Supports `stopOnError` to halt on first failure, a top-level `observe` level for the single observation collected after the batch (default `diff`; without one, the last executed step's `observe` is used), and `batchTimeoutMs` (1 000–30 000 ms) to set an overall deadline (checked between steps; remaining steps are skipped). The whole `run_steps` call has the default 30 s request limit, so to get partial results set `batchTimeoutMs` below it (for example 20 000) and keep batches short. Accepts tool aliases like `navigate_home` / `navigate-home`. Returns per-step results with timing.                                                                                                                                                                                                                                                    |
 | **Advanced**                  |                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                   |
 | `mock_network`                | Adds, clears, lists, and inspects targeted Playwright network mocks on the active browser context. Unmatched same-origin requests are continued unchanged.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        |
 | `cdp`                         | Sends a raw Chrome DevTools Protocol command, dispatched through the active platform driver. **Browser:** targets the page's Chrome CDP session (full Runtime/DOM/Network/Page surface); destructive methods (`Browser.close`, `Target.closeTarget`, etc.) are blocked. **Mobile (iOS/Android):** targets the app's React Native Hermes JS runtime via Metro's inspector proxy (delegates to `@metamask/device-mcp`, needs a DEBUG build with Metro running); only the JS-engine subset exists (`Runtime`, `Debugger`, `Log`, `HeapProfiler` — no DOM/Page/Network) and `Runtime.terminateExecution` / `Inspector.detached` are blocked. Optional `metroPort` / `appId` are mobile-only (ignored on browser). Categorized as `cdp`: no observation is collected and refs are not refreshed, so run `describe_screen` after a call that changes state. See [Browser vs Mobile CDP](#browser-vs-mobile-hermes-cdp). |
@@ -549,11 +549,15 @@ Every tool input (and the `/launch` and `/cleanup` bodies) accepts an optional `
 
 When `observe` is absent, the default comes from the tool category: `diff` for **mutating** tools (click, type, navigate, launch, cleanup, build, etc.) and `run_steps`; `none` for **read-only**, **discovery** and `cdp` tools. An explicit `diff` or `full` on a read-only, discovery or `cdp` tool collects and returns an observation.
 
+`observe: 'none'` does not refresh the refs. After a call with `none` that changes the screen, the refs from the last observation or `describe_screen` still point at the previous screen. Refs backed by a test ID or an element id fail with `MM_TARGET_NOT_FOUND` when the element is gone. Refs built from role and name, a DOM path, or a mobile label can match a different element on the new screen. After `none`, target by test ID, or call `describe_screen` (or use `observe: 'full'`) before using refs again.
+
+On the browser, only mutating, batch and `cdp` calls wait for animations to settle (up to 1 s; every running finite animation counts, also off screen, so a long finite animation adds up to 1 s) before the observation. A read-only or discovery call with `observe: 'diff'` or `'full'` (and `describe_screen`) reads the page at once, so right after a `none` mutation it can catch an animation in progress. To get a settled observation, let the mutation itself observe (`diff` or `full`) instead of chaining it with `none`.
+
 For `run_steps`, the top-level `observe` sets the level of the one observation collected after the batch. Without it, the `observe` in the last executed step's `args` is used; per-step `observe` on other steps is ignored. `includeObservations` was removed.
 
 Observations contain `testIds` and `a11y` only. They do not include app state; call `get_state` for that.
 
-A call that times out is cancelled: its signal is aborted, it stops at its next step, it collects no observation, and its refs are dropped.
+A call that times out is cancelled: its signal is aborted, it stops at its next step, it collects no observation, and its refs are dropped. `click` and `type` check the signal again after they wait for the target, so a call that timed out during that wait does not click or type. An action that already started is not cancelled. On the browser, the action includes Playwright's own checks inside `locator.click` / `locator.fill` (stable, enabled, not covered), which can wait up to the rest of the call's `timeoutMs`. On mobile, `@metamask/device-mcp` takes no signal: its element wait, tap and `typeText` run until they end, and the driver checks the signal between them.
 
 ## CLI Reference
 

SKILL.md

@@ -51,6 +51,10 @@ mm navigate-home --observe full  # after navigation: get the whole screen
 mm get-state --observe full    # a read that also returns an observation
 ```
 
+**After `--observe none`, refs are not refreshed.** If the call changed the screen, the refs you have still describe the previous screen. A ref backed by a testId fails loudly (`MM_TARGET_NOT_FOUND`), but a ref built from role and name, a DOM path or a mobile label can click a **different element**. After `--observe none`, target by `--testid`, or run `mm describe-screen` before you use refs again.
+
+**Only mutating and batch calls wait for animations** (browser, up to 1 s) before they observe. `mm describe-screen` and a read with `--observe full` read the page at once, so `mm click … --observe none && mm get-state --observe full` can catch a screen mid-animation. To get a settled screen after an action, let the action observe: `mm click … --observe full`.
+
 Observations contain `testIds` and `a11y` only — no app `state`. Call `mm get-state` for state.
 
 On mobile, each observation and `describe-screen` comes from one device snapshot. The Android system status bar and navigation bar and the Expo DevLauncher "Tools" button (iOS `gearshape.fill`) are not part of the app and are removed from `testIds` and `a11y`. Other system windows, such as the biometric prompt (`com.android.systemui:id/biometric_*`), are kept.
@@ -91,7 +95,7 @@ After a mutating action, the response includes fresh screen data:
 
 You can use the `ref` values from `observations.a11y.nodes` for the next interaction — no `describe-screen` needed. Refs are numbered in screen order: on an unchanged screen they stay the same, but when elements appear or disappear above an element its ref can change. With `--observe diff`, those elements are listed in the diff with their new refs, so always take refs from the latest response. Note that refs in compacted observations may be summary nodes (e.g., `"55 options (refs e2–e56)"`) when there are 3+ options under a combobox or listbox.
 
-**Timeouts cancel the call:** when a call times out (`Tool execution timed out`), it is cancelled. It stops at its next step and cannot overwrite the refs of later calls. The action itself may still have happened, so run `describe-screen` before retrying.
+**Timeouts cancel the call:** when a call times out (`Tool execution timed out`), it is cancelled. It stops at its next step and cannot overwrite the refs of later calls. A `click` or `type` that timed out while waiting for its target does not click or type. An action that already started may still have happened (browser: Playwright's checks inside the click; mobile: a tap in progress is not stopped), so run `describe-screen` before retrying.
 
 **Quick reference:**
 
@@ -468,7 +472,7 @@ Executes multiple tool invocations in sequence from a JSON array. Each step spec
 mm run-steps '{"steps":[{"tool":"click","args":{"a11yRef":"e3"}},{"tool":"wait_for","args":{"a11yRef":"e5"}}]}'
 ```
 
-Supports `stopOnError` (halt on first failure) and returns per-step results with timing. A top-level `observe` (`none` / `diff` / `full`, default `diff`) sets the observation collected after the batch; see `run_steps` and `observe` above. Use `batchTimeoutMs` to set an overall deadline — if exceeded, remaining steps are marked as skipped and partial results are returned immediately. The summary includes a `skipped` count alongside `succeeded` and `failed`.
+Supports `stopOnError` (halt on first failure) and returns per-step results with timing. A top-level `observe` (`none` / `diff` / `full`, default `diff`) sets the observation collected after the batch; see `run_steps` and `observe` above. Use `batchTimeoutMs` (1000–30000 ms) to set an overall deadline — it is checked between steps; if exceeded, remaining steps are marked as skipped and partial results are returned. The whole `run_steps` call is stopped after 30 s (the request limit), so use a lower `batchTimeoutMs` (for example 20000) to get partial results, and split longer flows into several batches. The summary includes a `skipped` count alongside `succeeded` and `failed`.
 
 Tool aliases are supported in steps: `navigate_home` / `navigate-home`, `navigate_settings` / `navigate-settings`, and `navigate_notification` / `navigate-notification` resolve to `navigate` with the appropriate `screen` argument. You can also use `ref` as shorthand for `a11yRef` in step args and within targets.
 

src/cli/mm.ts

@@ -1866,7 +1866,9 @@ Global Options:
                       Overrides MM_PROJECT and git-based discovery.
   --observe <level>   Observation returned after the call: none | diff | full.
                       Default: diff for mutating commands and run-steps,
-                      none for reads, discovery and cdp.
+                      none for reads, discovery and cdp. none does not
+                      refresh refs: run describe-screen before using refs
+                      again.
   --pretty            Indent JSON output. Default: one line per result when
                       stdout is not a terminal (agents, pipes), indented on a
                       terminal.

src/platform/mobile-platform-driver.test.ts

@@ -692,6 +692,101 @@ describe('MobilePlatformDriver', () => {
     });
   });
 
+  describe('abort during the target wait', () => {
+    it('does not tap when the signal aborts while waiting', async () => {
+      const controller = new AbortController();
+      const reason = new Error('Tool execution timed out after 30000ms');
+      const backend = createMockBackend({
+        waitForElement: vi.fn().mockImplementation(async () => {
+          controller.abort(reason);
+          return makeElement();
+        }),
+      });
+      const driver = new MobilePlatformDriver(backend);
+
+      await expect(
+        driver.click(
+          'testId',
+          'confirm',
+          new Map(),
+          5000,
+          undefined,
+          controller.signal,
+        ),
+      ).rejects.toBe(reason);
+      expect(backend.tapElement).not.toHaveBeenCalled();
+    });
+
+    it('does not tap or type when the signal aborts while waiting', async () => {
+      const controller = new AbortController();
+      const backend = createMockBackend({
+        waitForElement: vi.fn().mockImplementation(async () => {
+          controller.abort(new Error('timed out'));
+          return makeElement();
+        }),
+      });
+      const driver = new MobilePlatformDriver(backend);
+
+      await expect(
+        driver.type(
+          'testId',
+          'input',
+          'text',
+          new Map(),
+          5000,
+          undefined,
+          controller.signal,
+        ),
+      ).rejects.toThrowError('timed out');
+      expect(backend.tapElement).not.toHaveBeenCalled();
+      expect(backend.typeText).not.toHaveBeenCalled();
+    });
+
+    it('taps and types when the signal is not aborted', async () => {
+      const backend = createMockBackend();
+      const driver = new MobilePlatformDriver(backend);
+      const { signal } = new AbortController();
+
+      await driver.click('testId', 'btn', new Map(), 5000, undefined, signal);
+      await driver.type(
+        'testId',
+        'input',
+        'text',
+        new Map(),
+        5000,
+        undefined,
+        signal,
+      );
+
+      expect(backend.tapElement).toHaveBeenCalledTimes(2);
+      expect(backend.typeText).toHaveBeenCalledWith('text');
+    });
+
+    it('does not type when the signal aborts during the focus tap', async () => {
+      const controller = new AbortController();
+      const backend = createMockBackend({
+        tapElement: vi.fn().mockImplementation(async () => {
+          controller.abort(new Error('timed out'));
+          return { success: true, x: 1, y: 1, targetDescription: 'input' };
+        }),
+      });
+      const driver = new MobilePlatformDriver(backend);
+
+      await expect(
+        driver.type(
+          'testId',
+          'input',
+          'text',
+          new Map(),
+          5000,
+          undefined,
+          controller.signal,
+        ),
+      ).rejects.toThrowError('timed out');
+      expect(backend.typeText).not.toHaveBeenCalled();
+    });
+  });
+
   describe('type', () => {
     it('waits, taps to focus, then types', async () => {
       const backend = createMockBackend();