feat!: collect observations only for mutating and batch tools

SKILL.md

@@ -25,12 +25,13 @@ mm cleanup --shutdown      # 5. Clean up when done
 
 Tool responses include different data based on the tool's category:
 
-| Category      | Examples                                                                                                                                                                                                                                        | Observations in response?                      |
-| ------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------- |
-| **Mutating**  | click, type, navigate, launch, cleanup, build, clipboard, cdp, 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`                    |
-| **Batch**     | run_steps                                                                                                                                                                                                                                       | Controlled by `includeObservations` param      |
+| 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      |
 
 **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.
 
@@ -44,7 +45,7 @@ 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`, `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 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).
 
 ### Using inline observations (mutating tools)
 

src/server/create-server.test.ts

@@ -167,12 +167,16 @@ describe('shouldCollectObservations', () => {
     expect(shouldCollectObservations('mutating')).toBe(true);
   });
 
-  it('returns true for readonly', () => {
-    expect(shouldCollectObservations('readonly')).toBe(true);
+  it('returns false for readonly', () => {
+    expect(shouldCollectObservations('readonly')).toBe(false);
+  });
+
+  it('returns false for discovery', () => {
+    expect(shouldCollectObservations('discovery')).toBe(false);
   });
 
-  it('returns true for discovery', () => {
-    expect(shouldCollectObservations('discovery')).toBe(true);
+  it('returns false for cdp', () => {
+    expect(shouldCollectObservations('cdp')).toBe(false);
   });
 
   it('returns true for batch with default policy', () => {
@@ -227,6 +231,10 @@ describe('shouldIncludeObservationsInResponse', () => {
     );
   });
 
+  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,
@@ -881,7 +889,7 @@ describe('createServer with active session', () => {
     }
   });
 
-  it('observation Playwright helpers called for read-only tools', async () => {
+  it('observation Playwright helpers not called for read-only tools', async () => {
     const { collectTestIds, collectTrimmedA11ySnapshot } =
       await import('../tools/utils/discovery.js');
     const collectTestIdsSpy = vi.mocked(collectTestIds);
@@ -896,8 +904,8 @@ describe('createServer with active session', () => {
       body: JSON.stringify({}),
     });
 
-    expect(collectTestIdsSpy).toHaveBeenCalled();
-    expect(collectA11ySpy).toHaveBeenCalled();
+    expect(collectTestIdsSpy).not.toHaveBeenCalled();
+    expect(collectA11ySpy).not.toHaveBeenCalled();
   });
 
   it('observation Playwright helpers called for mutating tools', async () => {
@@ -1004,7 +1012,7 @@ describe('createServer with active session', () => {
       expect(body.observations?.state.currentScreen).toBe('unknown');
     });
 
-    it('does not recheck for readonly tool category', async () => {
+    it('collects no observation for readonly tool category', async () => {
       mockSM.getExtensionState.mockReset();
       mockSM.getExtensionState.mockResolvedValue({
         isLoaded: true,
@@ -1020,12 +1028,14 @@ describe('createServer with active session', () => {
           body: JSON.stringify({}),
         },
       );
+      const body = (await res.json()) as { observations?: unknown };
 
       expect(res.status).toBe(200);
-      expect(mockSM.getExtensionState).toHaveBeenCalledTimes(1);
+      expect(body.observations).toBeUndefined();
+      expect(mockSM.getExtensionState).not.toHaveBeenCalled();
     });
 
-    it('does not recheck for discovery tool category', async () => {
+    it('collects no observation for discovery tool category', async () => {
       mockSM.getExtensionState.mockReset();
       mockSM.getExtensionState.mockResolvedValue({
         isLoaded: true,
@@ -1041,9 +1051,11 @@ describe('createServer with active session', () => {
           body: JSON.stringify({}),
         },
       );
+      const body = (await res.json()) as { observations?: unknown };
 
       expect(res.status).toBe(200);
-      expect(mockSM.getExtensionState).toHaveBeenCalledTimes(1);
+      expect(body.observations).toBeUndefined();
+      expect(mockSM.getExtensionState).not.toHaveBeenCalled();
     });
 
     it('does not recheck for batch tool category', async () => {
@@ -1141,6 +1153,48 @@ describe('createServer with active session', () => {
       expect(body.meta?.timing.observationMs).toBeGreaterThanOrEqual(50);
     });
 
+    it.each([
+      ['readonly', '/tool/get_state'],
+      ['discovery', '/tool/list_testids'],
+      ['cdp', '/tool/cdp', JSON.stringify({ method: 'Runtime.evaluate' })],
+    ])(
+      'reports observationMs 0 for %s tools with an active session',
+      async (category, urlPath, requestBody?: string) => {
+        if (category === 'cdp') {
+          mockSM.getPlatformDriver.mockReturnValue({
+            getPlatform: () => 'browser',
+            cdp: vi.fn().mockResolvedValue({ ok: true, result: { value: 2 } }),
+          });
+        }
+
+        const { status, body } = await post(urlPath, requestBody);
+
+        expect(status).toBe(200);
+        expect(body.ok).toBe(true);
+        expect(
+          (body as TimedBody & { observations?: unknown }).observations,
+        ).toBeUndefined();
+        expectValidTiming(body);
+        expect(body.meta?.timing.observationMs).toBe(0);
+      },
+    );
+
+    it('reports observationMs above 0 for mutating tools', async () => {
+      mockSM.getExtensionState.mockImplementationOnce(
+        async () =>
+          new Promise((resolve) =>
+            setTimeout(() => resolve({ isLoaded: true, currentUrl: '' }), 20),
+          ),
+      );
+
+      const { body } = await post('/cleanup');
+
+      expect(
+        (body as TimedBody & { observations?: unknown }).observations,
+      ).toBeDefined();
+      expect(body.meta?.timing.observationMs).toBeGreaterThan(0);
+    });
+
     it('reports observationMs 0 when no session is active', async () => {
       mockSM.hasActiveSession.mockReturnValue(false);
 
@@ -1547,6 +1601,105 @@ describe('observation compaction in HTTP responses', () => {
     expect(body.observations?.a11y.nodes.length).toBeGreaterThan(1);
   });
 
+  it('accessibility_snapshot resets the diff baseline', async () => {
+    const { collectTrimmedA11ySnapshot } =
+      await import('../tools/utils/discovery.js');
+    vi.mocked(collectTrimmedA11ySnapshot)
+      .mockResolvedValueOnce({
+        nodes: initialButtons as never,
+        refMap: new Map(),
+      })
+      .mockResolvedValueOnce({
+        nodes: initialButtons as never,
+        refMap: new Map(),
+      })
+      .mockResolvedValueOnce({
+        nodes: changedButtons as never,
+        refMap: new Map(),
+      });
+
+    await httpRequest(`http://127.0.0.1:${state.port}/tool/click`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ a11yRef: 'e1' }),
+    });
+
+    await httpRequest(
+      `http://127.0.0.1:${state.port}/tool/accessibility_snapshot`,
+      {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({}),
+      },
+    );
+
+    const res = await httpRequest(`http://127.0.0.1:${state.port}/tool/click`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ a11yRef: 'e1' }),
+    });
+    const body = (await res.json()) as {
+      ok: boolean;
+      observations?: {
+        a11y: {
+          nodes: unknown[];
+          diff?: unknown;
+        };
+      };
+    };
+
+    expect(res.status).toBe(200);
+    expect(body.observations).toBeDefined();
+    expect(body.observations?.a11y.diff).toBeUndefined();
+    expect(body.observations?.a11y.nodes.length).toBeGreaterThan(1);
+  });
+
+  it('read tools do not move the diff baseline', 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(),
+      });
+
+    await httpRequest(`http://127.0.0.1:${state.port}/tool/click`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ a11yRef: 'e1' }),
+    });
+
+    await httpRequest(`http://127.0.0.1:${state.port}/tool/get_state`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({}),
+    });
+
+    const res = await httpRequest(`http://127.0.0.1:${state.port}/tool/click`, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ a11yRef: 'e1' }),
+    });
+    const body = (await res.json()) as {
+      ok: boolean;
+      observations?: {
+        a11y: {
+          diff?: { added: string[]; removed: string[]; unchanged: number };
+        };
+      };
+    };
+
+    expect(res.status).toBe(200);
+    expect(body.observations?.a11y.diff).toStrictEqual({
+      added: ['e3'],
+      removed: ['e2'],
+      unchanged: 1,
+    });
+  });
   it('falls back to the full observation when the diff is not smaller', async () => {
     const { collectTrimmedA11ySnapshot } =
       await import('../tools/utils/discovery.js');

src/server/create-server.ts

@@ -22,6 +22,7 @@ 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 {
@@ -74,11 +75,12 @@ export function buildResponseBody(
 }
 
 /**
- * Whether to run Playwright observation collection for this tool invocation.
+ * Whether to collect an observation (state, test IDs, a11y tree) after a tool
+ * runs. Only tools that change the UI pay for it:
  *
- * Observations are collected for every tool, whether or not they appear in
- * the HTTP response (they also feed the next response's diff). The only
- * exception is batch with `'none'` policy, which skips collection entirely.
+ * - `mutating`: yes.
+ * - `batch` (`run_steps`): yes, unless `includeObservations` is `'none'`.
+ * - `readonly`, `discovery`, `cdp`: no.
  *
  * @param category - The tool category to check.
  * @param validatedInput - The validated input payload (checked for batch policy).
@@ -88,17 +90,19 @@ export function shouldCollectObservations(
   category: ToolCategory,
   validatedInput?: Record<string, unknown>,
 ): boolean {
+  if (category === 'mutating') {
+    return true;
+  }
   if (category === 'batch') {
-    const policy =
-      (validatedInput as { includeObservations?: string })
-        ?.includeObservations ?? 'all';
-    return policy !== 'none';
+    return batchObservationPolicy(validatedInput) !== 'none';
   }
-  return true;
+  return false;
 }
 
 /**
- * Whether to include observations in the HTTP response.
+ * 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.
  *
  * @param category - The tool category.
  * @param toolResult - The result returned by the tool.
@@ -110,27 +114,35 @@ export function shouldIncludeObservationsInResponse(
   toolResult: ToolResponse,
   validatedInput?: Record<string, unknown>,
 ): boolean {
-  if (category === 'mutating') {
-    return true;
+  if (!shouldCollectObservations(category, validatedInput)) {
+    return false;
   }
-  if (category === 'batch') {
-    const policy =
-      (validatedInput as { includeObservations?: string })
-        ?.includeObservations ?? 'all';
-    if (policy === 'none') {
-      return false;
+  if (
+    category === 'batch' &&
+    batchObservationPolicy(validatedInput) === 'failures'
+  ) {
+    if (!toolResult.ok) {
+      return true;
     }
-    if (policy === 'failures') {
-      if (!toolResult.ok) {
-        return true;
-      }
-      const result = toolResult.result as Record<string, unknown>;
-      const summary = result?.summary as Record<string, unknown> | undefined;
-      return summary !== undefined && !summary.ok;
-    }
-    return true; // 'all'
+    const result = toolResult.result as Record<string, unknown>;
+    const summary = result?.summary as Record<string, unknown> | undefined;
+    return summary !== undefined && !summary.ok;
   }
-  return false; // readonly, discovery
+  return true;
+}
+
+/**
+ * Reads the `run_steps` observation policy.
+ *
+ * @param validatedInput - The validated `run_steps` input.
+ * @returns The policy, `'all'` by default.
+ */
+function batchObservationPolicy(
+  validatedInput: Record<string, unknown> | undefined,
+): NonNullable<RunStepsInput['includeObservations']> {
+  const policy =
+    validatedInput?.includeObservations as RunStepsInput['includeObservations'];
+  return policy ?? 'all';
 }
 
 /**
@@ -455,6 +467,7 @@ export function createServer(config: ServerConfig): ServerInstance {
 
       if (
         toolName === 'describe_screen' ||
+        toolName === 'accessibility_snapshot' ||
         toolName === 'launch' ||
         toolName === 'cleanup'
       ) {

src/tools/registry.test.ts

@@ -85,6 +85,10 @@ describe('TOOL_CATEGORIES and getToolCategory', () => {
     expect(getToolCategory('describe_screen')).toBe('discovery');
   });
 
+  it('getToolCategory returns cdp for cdp', () => {
+    expect(getToolCategory('cdp')).toBe('cdp');
+  });
+
   it('getToolCategory returns batch for run_steps', () => {
     expect(getToolCategory('run_steps')).toBe('batch');
   });