From d87bc14f9c317b59e283d7d9e0afa71a89bbab73 Mon Sep 17 00:00:00 2001 From: cryptotavares Date: Thu, 29 Oct 2026 19:35:27 +0000 Subject: [PATCH] perf: one device snapshot per mobile tap; parallel describe_screen reads - `click` / `type` pass the element returned by `waitForElement` to `tapElement(query, { element })`, so the backend does not snapshot the screen again before tapping. device-mcp releases without the option ignore it (same behaviour as before). - `describe_screen` on mobile runs `getAppState` and `captureScreen` at the same time; the browser keeps the old order (both use the page). --- CHANGELOG.md | 4 + src/platform/mobile-platform-driver.test.ts | 195 +++++++++++++++----- src/platform/mobile-platform-driver.ts | 41 +++- src/tools/discovery-tools.test.ts | 34 ++++ src/tools/discovery-tools.ts | 11 +- 5 files changed, 229 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0059e2a..25bd83c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add optional platform-driver disposal and forward mobile disposal to lifecycle-capable device-mcp backends. +### Changed + +- Mobile `click` and `type` tap the element that the target wait found, so a tap takes one device snapshot instead of two (needs a device-mcp release with `tapElement(query, { element })`; older releases ignore the option). `describe_screen` reads the app state and the screen at the same time on mobile. + ### Fixed - Dispose session-owned drivers after consumer cleanup, including cleanup errors, forced relaunch, failed launch, daemon shutdown, and server startup rollback. Retain failed disposals in one server-owned lifecycle, so the next cleanup or daemon stop retries them. diff --git a/src/platform/mobile-platform-driver.test.ts b/src/platform/mobile-platform-driver.test.ts index 84ef23b..a0f372f 100644 --- a/src/platform/mobile-platform-driver.test.ts +++ b/src/platform/mobile-platform-driver.test.ts @@ -464,6 +464,32 @@ describe('MobilePlatformDriver', () => { }); describe('click', () => { + it('taps the element that waitForElement returned (one snapshot)', async () => { + const found = makeElement({ + identifier: 'submit-btn', + frame: { x: 10, y: 600, width: 120, height: 48 }, + }); + const backend = createMockBackend({ + waitForElement: vi.fn().mockResolvedValue(found), + }); + const driver = new MobilePlatformDriver(backend); + + await driver.click('testId', 'submit-btn', new Map(), 5000); + await driver.type('testId', 'submit-btn', 'x', new Map(), 5000); + + expect(backend.tapElement).toHaveBeenNthCalledWith( + 1, + { identifier: 'submit-btn' }, + { element: found }, + ); + expect(backend.tapElement).toHaveBeenNthCalledWith( + 2, + { identifier: 'submit-btn' }, + { element: found }, + ); + expect(backend.snapshot).not.toHaveBeenCalled(); + }); + it('waits for element then taps', async () => { const backend = createMockBackend(); const driver = new MobilePlatformDriver(backend); @@ -475,9 +501,14 @@ describe('MobilePlatformDriver', () => { { identifier: 'submit-btn' }, 5000, ); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'submit-btn', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'submit-btn', + }, + { + element: makeElement(), + }, + ); expect(result).toStrictEqual({ clicked: true, target: 'a11yRef:e1', @@ -490,9 +521,14 @@ describe('MobilePlatformDriver', () => { await driver.click('testId', 'confirm-button', new Map(), 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'confirm-button', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'confirm-button', + }, + { + element: makeElement(), + }, + ); }); it('throws for selector target type', async () => { @@ -526,9 +562,14 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'bare-id', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'bare-id', + }, + { + element: makeElement(), + }, + ); }); it('resolves a11yRef with unknown prefix as identifier', async () => { @@ -538,9 +579,14 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'custom:some-value', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'custom:some-value', + }, + { + element: makeElement(), + }, + ); }); it('throws when within scope is provided', async () => { @@ -561,10 +607,15 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - label: 'OK', - type: 'Button', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + label: 'OK', + type: 'Button', + }, + { + element: makeElement(), + }, + ); }); it('disambiguates same-label elements by type', async () => { @@ -576,16 +627,26 @@ describe('MobilePlatformDriver', () => { ]); await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - label: 'OK', - type: 'Button', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + label: 'OK', + type: 'Button', + }, + { + element: makeElement(), + }, + ); await driver.click('a11yRef', 'e2', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - label: 'OK', - type: 'StaticText', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + label: 'OK', + type: 'StaticText', + }, + { + element: makeElement(), + }, + ); }); it('resolves value|type stable id with type in query', async () => { @@ -595,10 +656,15 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - text: '0.5 ETH', - type: 'StaticText', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + text: '0.5 ETH', + type: 'StaticText', + }, + { + element: makeElement(), + }, + ); }); it('ignores unknown pipe segments in stable id', async () => { @@ -608,7 +674,12 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ label: 'OK' }); + expect(backend.tapElement).toHaveBeenCalledWith( + { label: 'OK' }, + { + element: makeElement(), + }, + ); }); it('falls back to identifier for unknown prefix in stable id', async () => { @@ -618,10 +689,15 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'custom:something', - type: 'Button', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'custom:something', + type: 'Button', + }, + { + element: makeElement(), + }, + ); }); it('handles stable id with no pipe segments', async () => { @@ -631,9 +707,14 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'submit-btn', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'submit-btn', + }, + { + element: makeElement(), + }, + ); }); it('treats bare string without colon as identifier', async () => { @@ -643,9 +724,14 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'rawvalue', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'rawvalue', + }, + { + element: makeElement(), + }, + ); }); it('skips non-type keyed pipe segments in stable id', async () => { @@ -655,10 +741,15 @@ describe('MobilePlatformDriver', () => { await driver.click('a11yRef', 'e1', refMap, 5000); - expect(backend.tapElement).toHaveBeenCalledWith({ - label: 'OK', - type: 'Button', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + label: 'OK', + type: 'Button', + }, + { + element: makeElement(), + }, + ); }); it('throws when timeout budget is exceeded after waitForElement', async () => { @@ -804,9 +895,14 @@ describe('MobilePlatformDriver', () => { { identifier: 'password-input' }, 5000, ); - expect(backend.tapElement).toHaveBeenCalledWith({ - identifier: 'password-input', - }); + expect(backend.tapElement).toHaveBeenCalledWith( + { + identifier: 'password-input', + }, + { + element: makeElement(), + }, + ); expect(backend.typeText).toHaveBeenCalledWith('secret123'); expect(result).toStrictEqual({ typed: true, @@ -1165,7 +1261,12 @@ describe('MobilePlatformDriver', () => { expect(nodes.map((node) => node.ref)).toStrictEqual(['e1']); expect([...refMap]).toStrictEqual([['e1', 'identifier:send']]); await driver.click('a11yRef', 'e1', refMap, 1000); - expect(backend.tapElement).toHaveBeenCalledWith({ identifier: 'send' }); + expect(backend.tapElement).toHaveBeenCalledWith( + { identifier: 'send' }, + { + element: makeElement(), + }, + ); await expect( driver.click('a11yRef', 'e2', refMap, 1000), ).rejects.toThrowError('Unknown a11yRef: e2'); diff --git a/src/platform/mobile-platform-driver.ts b/src/platform/mobile-platform-driver.ts index 7cf96a2..a3ed274 100644 --- a/src/platform/mobile-platform-driver.ts +++ b/src/platform/mobile-platform-driver.ts @@ -150,11 +150,15 @@ export class MobilePlatformDriver implements IPlatformDriver { rejectWithinScope(within); const query = resolveTargetToQuery(targetType, targetValue, refMap); const deadline = Date.now() + timeoutMs; - await this.#backend.waitForElement(query, timeoutMs); + const element = await this.#backend.waitForElement(query, timeoutMs); // The request may have timed out while we waited for the target. signal?.throwIfAborted(); const remaining = deadline - Date.now(); - await withTimeout(this.#backend.tapElement(query), remaining, 'tapElement'); + await withTimeout( + this.#tapResolved(query, element), + remaining, + 'tapElement', + ); return { clicked: true, target: `${targetType}:${targetValue}` }; } @@ -182,10 +186,14 @@ export class MobilePlatformDriver implements IPlatformDriver { rejectWithinScope(within); const query = resolveTargetToQuery(targetType, targetValue, refMap); const deadline = Date.now() + timeoutMs; - await this.#backend.waitForElement(query, timeoutMs); + const element = await this.#backend.waitForElement(query, timeoutMs); signal?.throwIfAborted(); let remaining = deadline - Date.now(); - await withTimeout(this.#backend.tapElement(query), remaining, 'tapElement'); + await withTimeout( + this.#tapResolved(query, element), + remaining, + 'tapElement', + ); signal?.throwIfAborted(); remaining = deadline - Date.now(); await withTimeout(this.#backend.typeText(text), remaining, 'typeText'); @@ -196,6 +204,31 @@ export class MobilePlatformDriver implements IPlatformDriver { }; } + /** + * Taps the element that `waitForElement` just found, without a second + * snapshot. device-mcp releases without the `element` option ignore it and + * snapshot again; the tap still hits the same query. + * + * @param query - The element query. + * @param element - The element returned by `waitForElement`. + * @returns The backend's tap result. + */ + async #tapResolved( + query: ElementQuery, + element: UIElement, + ): Promise { + // device-mcp 0.7.0 declares `tapElement(query)`; the `element` option is + // newer (see device-mcp CHANGELOG). Extra arguments are ignored by older + // releases. + const backend = this.#backend as DeviceBackend & { + tapElement( + query: ElementQuery, + options?: { element?: UIElement }, + ): Promise; + }; + return backend.tapElement(query, { element }); + } + /** * @param targetType - Target identifier type. * @param targetValue - Target value for element lookup. diff --git a/src/tools/discovery-tools.test.ts b/src/tools/discovery-tools.test.ts index e86c07c..148c915 100644 --- a/src/tools/discovery-tools.test.ts +++ b/src/tools/discovery-tools.test.ts @@ -465,6 +465,40 @@ describe('discovery-tools', () => { ); }); + it('reads the app state and captures the screen at the same time', async () => { + const context = createMockContext(); + const order: string[] = []; + let releaseState: () => void = () => undefined; + const driver = { + getAppState: vi.fn(async () => { + order.push('state:start'); + await new Promise((resolve) => { + releaseState = resolve; + }); + order.push('state:end'); + return { isLoaded: true }; + }), + captureScreen: vi.fn(async () => { + order.push('capture:start'); + releaseState(); + return { nodes: [], refMap: new Map(), getTestIds: async () => [] }; + }), + getTestIds: vi.fn(), + getAccessibilityTree: vi.fn(), + }; + + const result = await describeScreenTool({}, { + ...context, + driver, + } as unknown as ToolContext); + + expect(result.ok).toBe(true); + // captureScreen started before getAppState finished. + expect(order.indexOf('capture:start')).toBeLessThan( + order.indexOf('state:end'), + ); + }); + it('stops after captureScreen when the request is aborted', async () => { const controller = new AbortController(); const context = createMockContext(); diff --git a/src/tools/discovery-tools.ts b/src/tools/discovery-tools.ts index 676fa82..5da81dd 100644 --- a/src/tools/discovery-tools.ts +++ b/src/tools/discovery-tools.ts @@ -128,12 +128,13 @@ export async function describeScreenTool( const { driver } = driverResult; try { - const state = await driver.getAppState(); // Mobile: one device snapshot for both the test IDs and the tree, when - // the driver supports it. - const screen = driver.captureScreen - ? await driver.captureScreen() - : undefined; + // the driver supports it. The app-state read does not depend on it, so + // the two device round trips run at the same time. The browser keeps the + // order: its state read and its tree both use the page. + const [state, screen] = driver.captureScreen + ? await Promise.all([driver.getAppState(), driver.captureScreen()]) + : [await driver.getAppState(), undefined]; const testIds = screen ? await screen.getTestIds(DEFAULT_TESTID_LIMIT) : await driver.getTestIds(DEFAULT_TESTID_LIMIT);