fix(cloud): cancel a screen read that its caller gave up on (#2509)

A hosted page-source read that outran its request was dropped by the client while
the driver kept walking the UI tree. Nothing at the wire said the answer was no
longer wanted, so the read stayed in flight and held the session it was blocking.
On a screen that never goes still -- a looping video, a live ticker, continuous
animation -- every later command then queued behind a capture nobody was waiting
for, which is what makes one stuck `snapshot -i` look like a frozen session.

Bind the read to the request that asked for it, on both Android and iOS, and say
what a source read that runs out of budget was waiting for. The timeout keeps its
`webdriver_request_timeout` reason and gains a hint naming a screen that never goes
still, so the rented minutes end with a cause rather than a silent hang.

Closes #2509

Author: thymikee

packages/provider-webdriver/src/webdriver-client.test.ts

@@ -106,6 +106,29 @@ test('installApp aborts an in-flight provider request when its binding is cancel
   }
 });
 
+// #2509: a page-source read that outlived its request was dropped by the client
+// while Appium kept walking the tree server-side. Providers serialize commands
+// per session, so every later request queued behind an orphan nobody was
+// waiting for. Cancellation must reach `/source` the way it reaches `/install_app`.
+test('source aborts an in-flight provider request when its binding is cancelled', async () => {
+  const controller = new AbortController();
+  let requestSignal: AbortSignal | undefined;
+  const client = await connectedClient(
+    async (_input, init) =>
+      await new Promise<Response>((_resolve, reject) => {
+        requestSignal = init?.signal ?? undefined;
+        init?.signal?.addEventListener('abort', () => reject(init.signal?.reason as Error));
+      }),
+  );
+
+  const pending = client.source({ signal: controller.signal });
+  await Promise.resolve();
+  controller.abort(new Error('request cancelled'));
+
+  await assert.rejects(pending, /request cancelled/);
+  assert.equal(requestSignal?.aborted, true);
+});
+
 // #1774: `POST /session` is the one non-idempotent request, and a retry after a
 // failed create is a second billed device session. A 5xx that the transport
 // would retry on any other route must reach the caller unretried here.

packages/provider-webdriver/src/webdriver-client.ts

@@ -244,8 +244,14 @@ export class WebDriverClient {
     await this.sessionRequest('POST', '/orientation', { orientation });
   }
 
-  async source(): Promise<string> {
-    const value = await this.sessionRequest('GET', '/source');
+  /**
+   * The driver's whole page source. This is the provider's most expensive call and
+   * the only one whose duration the device's own UI decides — a screen that never
+   * goes idle has no reason to settle — so a caller that can be cancelled should
+   * hand its `signal` down rather than abandon a read still running server-side.
+   */
+  async source(overrides?: WebDriverRequestOverrides): Promise<string> {
+    const value = await this.sessionRequest('GET', '/source', undefined, overrides);
     if (typeof value !== 'string') {
       throw new AppError('COMMAND_FAILED', 'WebDriver source response was not a string', {
         valueType: typeof value,

packages/provider-webdriver/src/webdriver-interactor.test.ts

@@ -5,6 +5,7 @@ import { AppError } from '@agent-device/kernel/errors';
 import { createCloudWebDriverCapabilities } from './capabilities.ts';
 import type { WebDriverClient, W3CActionSequence } from './webdriver-client.ts';
 import { createWebDriverInteractor } from './webdriver-interactor.ts';
+import { isWebDriverRequestTimeout } from './webdriver-transport.ts';
 
 // #1658: `fill` used to send its keys in the request right after the tap. A
 // WebView input takes first responder asynchronously, so on a web login form
@@ -291,6 +292,82 @@ test('Android WebDriver interactor keeps legacy-derived source facts at its call
   assert.equal(source.mock.calls.length, 1);
 });
 
+// #2509: the interactor took a request-bound signal and named it away. A capture
+// that ran past its budget could therefore never be cancelled: the client gave up
+// while the provider kept walking the tree, and being per-session-serial it made
+// every later command queue behind an orphan nobody was waiting for.
+test('Android snapshot binds the provider source read to its request signal', async () => {
+  const controller = new AbortController();
+  const forwarded: Array<{ signal?: AbortSignal } | undefined> = [];
+  const interactor = createWebDriverInteractor({
+    client: {
+      source: async (overrides?: { signal?: AbortSignal }) => {
+        forwarded.push(overrides);
+        return ANDROID_ONBOARDING_SOURCE;
+      },
+    } as unknown as WebDriverClient,
+    backend: 'android',
+    capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }),
+  });
+
+  await interactor.snapshot({ signal: controller.signal });
+
+  assert.deepEqual(forwarded, [{ signal: controller.signal }]);
+});
+
+// The iOS acquisition adapter reads the same route, so it needs the same binding.
+test('iOS snapshot binds the provider source read to its request signal', async () => {
+  const controller = new AbortController();
+  const forwarded: Array<{ signal?: AbortSignal } | undefined> = [];
+  const interactor = createWebDriverInteractor({
+    client: {
+      source: async (overrides?: { signal?: AbortSignal }) => {
+        forwarded.push(overrides);
+        return '<AppiumAUT><XCUIElementTypeApplication x="0" y="0" width="390" height="844" /></AppiumAUT>';
+      },
+    } as unknown as WebDriverClient,
+    backend: 'xctest',
+    capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'ios' }),
+    targetId: 'ios-1',
+  });
+
+  await interactor.snapshot({ signal: controller.signal });
+
+  assert.deepEqual(forwarded, [{ signal: controller.signal }]);
+});
+
+// #2509 asked for an error that names the problem: a screen that never goes idle
+// (looping video, live marquee) keeps the provider's tree walk from settling, and
+// on rented hardware every second of it is billed. The reason code stays the
+// transport's; what the capture adds is what it means.
+test('a source capture that runs out of budget keeps the timeout reason and names the cause', async () => {
+  const interactor = createWebDriverInteractor({
+    client: { source: async () => throwWebDriverSourceTimeout() } as unknown as WebDriverClient,
+    backend: 'android',
+    capabilities: createCloudWebDriverCapabilities({ provider: 'test', platform: 'android' }),
+  });
+
+  await assert.rejects(interactor.snapshot(), (error: unknown) => {
+    assert.ok(error instanceof AppError);
+    assert.equal(error.details?.reason, 'webdriver_request_timeout');
+    assert.equal(isWebDriverRequestTimeout(error), true);
+    assert.match(String(error.details?.hint), /never goes idle/);
+    return true;
+  });
+});
+
+function throwWebDriverSourceTimeout(): never {
+  throw new AppError('COMMAND_FAILED', 'WebDriver GET /source timed out after 30000ms.', {
+    reason: 'webdriver_request_timeout',
+    method: 'GET',
+    path: '/source',
+    timeoutMs: 30_000,
+  });
+}
+
+const ANDROID_ONBOARDING_SOURCE =
+  '<hierarchy rotation="0"><android.widget.Button content-desc="Continue" bounds="[0,0][100,40]" displayed="true" enabled="true" /></hierarchy>';
+
 async function runFill(world: ReturnType<typeof createTextEntryWorld>) {
   vi.useFakeTimers();
   try {

packages/provider-webdriver/src/webdriver-interactor.ts

@@ -22,6 +22,7 @@ import {
   type CloudWebDriverProviderCapabilities,
 } from './capabilities.ts';
 import type { W3CPointerAction, WebDriverClient, WebDriverWindowRect } from './webdriver-client.ts';
+import { isWebDriverRequestTimeout } from './webdriver-transport.ts';
 import { touchPointer } from './webdriver-gestures.ts';
 import {
   scrollFrameFromAndroidWebDriverSource,
@@ -303,20 +304,39 @@ class WebDriverInteractor implements Interactor {
     await this.client.screenshot(outPath);
   }
 
-  async snapshot(_options?: SnapshotOptions) {
+  async snapshot(options?: SnapshotOptions) {
     this.requireSupport('snapshot');
+    return await this.captureSource(options?.signal);
+  }
+
+  /**
+   * One page-source read, bound to the request that asked for it. Providers answer
+   * commands one at a time, so a read the client merely gave up on is not gone: the
+   * driver keeps walking the tree and every later command queues behind it (#2509).
+   */
+  private async captureSource(signal?: AbortSignal) {
+    const source = await this.readSource(signal);
     if (this.backend === 'xctest') {
-      const { captureWebDriverIosSnapshot } = await import('./webdriver-ios-snapshot.ts');
-      return await captureWebDriverIosSnapshot(this.client, this.targetId);
+      const { acquireWebDriverIosSnapshot } = await import('./webdriver-ios-snapshot.ts');
+      return acquireWebDriverIosSnapshot(source, this.targetId);
     }
     const { parseWebDriverSourceFacts } = await import('./webdriver-source.ts');
     return {
       backend: 'android' as const,
       producer: 'appium-source' as const,
-      nodes: parseWebDriverSourceFacts(await this.client.source(), 'android').nodes,
+      nodes: parseWebDriverSourceFacts(source, 'android').nodes,
     };
   }
 
+  private async readSource(signal?: AbortSignal): Promise<string> {
+    try {
+      return await this.client.source(signal === undefined ? {} : { signal });
+    } catch (error) {
+      if (!isWebDriverRequestTimeout(error)) throw error;
+      throw webDriverSourceTimeoutError(error);
+    }
+  }
+
   async back(_mode?: BackMode): Promise<void> {
     this.requireSupport('back');
     await this.client.back();
@@ -573,3 +593,26 @@ function webDriverOperationForGesture(plan: GesturePlan): CloudWebDriverOperatio
       return 'rotateGesture';
   }
 }
+
+/**
+ * A source read that ran out of budget is the one WebDriver timeout a caller can
+ * do something about, and #2509 showed it reading as an unexplained hang: the
+ * driver answers this call by walking the live UI tree, so a screen that never
+ * goes idle — looping video, live ticker, continuous animation — gives the walk no
+ * reason to settle. The transport's reason code is kept as-is; what the capture
+ * adds is what the wait was waiting for.
+ */
+function webDriverSourceTimeoutError(error: AppError): AppError {
+  return new AppError(
+    'COMMAND_FAILED',
+    'The cloud driver did not finish reading the screen in its budget.',
+    {
+      ...error.details,
+      hint:
+        'A screen that never goes idle (looping video, live ticker, continuous animation) ' +
+        'gives the driver no moment to read the UI tree. Retry with a larger --timeout, ' +
+        'or drive the screen from refs an earlier snapshot already captured.',
+    },
+    error,
+  );
+}

packages/provider-webdriver/src/webdriver-ios-snapshot.test.ts

@@ -1,9 +1,6 @@
 import assert from 'node:assert/strict';
-import { test, vi } from 'vitest';
-import {
-  captureWebDriverIosSnapshot,
-  acquireWebDriverIosSnapshot,
-} from './webdriver-ios-snapshot.ts';
+import { test } from 'vitest';
+import { acquireWebDriverIosSnapshot } from './webdriver-ios-snapshot.ts';
 
 const SOURCE = `<AppiumAUT>
   <XCUIElementTypeApplication type="XCUIElementTypeApplication" name="Example" label="Example" enabled="true" visible="true" x="0" y="0" width="390" height="844">
@@ -13,9 +10,8 @@ const SOURCE = `<AppiumAUT>
   </XCUIElementTypeApplication>
 </AppiumAUT>`;
 
-test('Appium iOS adapter returns provider facts with explicit unavailable residue', async () => {
-  const source = vi.fn(async () => SOURCE);
-  const result = await captureWebDriverIosSnapshot({ source }, 'cloud-ios-1');
+test('Appium iOS adapter returns provider facts with explicit unavailable residue', () => {
+  const result = acquireWebDriverIosSnapshot(SOURCE, 'cloud-ios-1');
 
   assert.equal(result.stage, 'acquired');
   assert.equal(result.acquisition.producer, 'appium-source');
@@ -34,7 +30,6 @@ test('Appium iOS adapter returns provider facts with explicit unavailable residu
     { kind: 'unavailable-fact', fact: 'acquisition-depth' },
     { kind: 'unavailable-fact', fact: 'truncation' },
   ]);
-  assert.equal(source.mock.calls.length, 1);
 });
 
 test('Appium iOS adapter preserves provider-reported node facts for the host presenter', () => {

packages/provider-webdriver/src/webdriver-ios-snapshot.ts

@@ -3,18 +3,11 @@ import {
   resolveIosViewportEvidenceFromRoots,
 } from '@agent-device/capture-kit/ios-snapshot-acquisition';
 import type { SnapshotRuntimeAcquiredResult } from '@agent-device/contracts/interactor-types';
-import type { WebDriverClient } from './webdriver-client.ts';
 import { parseWebDriverSourceFacts } from './webdriver-source.ts';
 
 const APPIUM_PRODUCER = 'appium-source' as const;
 
-export async function captureWebDriverIosSnapshot(
-  client: Pick<WebDriverClient, 'source'>,
-  targetId?: string,
-): Promise<SnapshotRuntimeAcquiredResult> {
-  return acquireWebDriverIosSnapshot(await client.source(), targetId);
-}
-
+/** Turns one already-read Appium page source into acquired iOS facts. */
 export function acquireWebDriverIosSnapshot(
   source: string,
   targetId?: string,