From 07a5bac0d1e7c7ff465ee7cb4f7ce65658485207 Mon Sep 17 00:00:00 2001 From: quanru Date: Mon, 15 Jun 2026 15:48:22 +0800 Subject: [PATCH 1/3] fix(android): add scrcpy adb features fallback --- .../android-playground/src/scrcpy-server.ts | 2 + .../tests/unit/scrcpy-server.test.ts | 68 ++++++++++++-- .../adb-server-client-features-fallback.ts | 93 +++++++++++++++++++ packages/android/src/index.ts | 1 + packages/android/src/scrcpy-device-adapter.ts | 2 + ...db-server-client-features-fallback.test.ts | 78 ++++++++++++++++ .../tests/unit-test/scrcpy-adapter.test.ts | 47 +++++++++- 7 files changed, 280 insertions(+), 11 deletions(-) create mode 100644 packages/android/src/adb-server-client-features-fallback.ts create mode 100644 packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts diff --git a/packages/android-playground/src/scrcpy-server.ts b/packages/android-playground/src/scrcpy-server.ts index d0f9248956..46fbaa6cd0 100644 --- a/packages/android-playground/src/scrcpy-server.ts +++ b/packages/android-playground/src/scrcpy-server.ts @@ -5,6 +5,7 @@ import type { Server as HttpServer } from 'node:http'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; +import { installAdbServerClientFeaturesFallback } from '@midscene/android'; import { SCRCPY_ADB_CONNECT_TIMEOUT_MS, SCRCPY_PREVIEW_METADATA_TIMEOUT_MS, @@ -240,6 +241,7 @@ export default class ScrcpyServer { port: 5037, }), ); + installAdbServerClientFeaturesFallback(this.adbClient); await debugPage('success to initialize adb client'); } else { debugPage('use existing adb client'); diff --git a/packages/android-playground/tests/unit/scrcpy-server.test.ts b/packages/android-playground/tests/unit/scrcpy-server.test.ts index fd60852ef8..b83df3b2af 100644 --- a/packages/android-playground/tests/unit/scrcpy-server.test.ts +++ b/packages/android-playground/tests/unit/scrcpy-server.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import ScrcpyServer, { resolveRequestedDeviceId, } from '../../src/scrcpy-server'; @@ -9,12 +9,43 @@ const { mockReadableFrom, mockCreateReadStream, mockOptionsCtor, -} = vi.hoisted(() => ({ - mockPushServer: vi.fn(), - mockStart: vi.fn(), - mockReadableFrom: vi.fn(), - mockCreateReadStream: vi.fn(), - mockOptionsCtor: vi.fn((options) => options), + mockExec, + mockInstallFeaturesFallback, + mockAdbClient, + mockAdbServerClient, + mockAdbServerNodeTcpConnector, +} = vi.hoisted(() => { + const mockAdbClient = {}; + + return { + mockPushServer: vi.fn(), + mockStart: vi.fn(), + mockReadableFrom: vi.fn(), + mockCreateReadStream: vi.fn(), + mockOptionsCtor: vi.fn((options) => options), + mockExec: vi.fn((_command, callback) => callback(null, '', '')), + mockInstallFeaturesFallback: vi.fn(), + mockAdbClient, + mockAdbServerClient: vi.fn().mockImplementation(() => mockAdbClient), + mockAdbServerNodeTcpConnector: vi.fn(), + }; +}); + +vi.mock('node:child_process', () => ({ + exec: mockExec, +})); + +vi.mock('@midscene/android', () => ({ + installAdbServerClientFeaturesFallback: mockInstallFeaturesFallback, +})); + +vi.mock('@yume-chan/adb', () => ({ + Adb: vi.fn().mockImplementation(() => ({})), + AdbServerClient: mockAdbServerClient, +})); + +vi.mock('@yume-chan/adb-server-node-tcp', () => ({ + AdbServerNodeTcpConnector: mockAdbServerNodeTcpConnector, })); vi.mock('@yume-chan/adb-scrcpy', () => ({ @@ -44,6 +75,10 @@ vi.mock('node:fs', async (importOriginal) => { }); describe('ScrcpyServer', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it('prefers the explicit device from the preview handshake', () => { expect( resolveRequestedDeviceId( @@ -91,6 +126,25 @@ describe('ScrcpyServer', () => { ]); }); + it('installs the multi-device features fallback when initializing the ADB client', async () => { + const server = new ScrcpyServer(); + + await expect((server as any).getAdbClient()).resolves.toBe(mockAdbClient); + + expect(mockExec).toHaveBeenCalledWith( + 'adb start-server', + expect.any(Function), + ); + expect(mockAdbServerNodeTcpConnector).toHaveBeenCalledWith({ + host: '127.0.0.1', + port: 5037, + }); + expect(mockAdbServerClient).toHaveBeenCalledTimes(1); + expect(mockInstallFeaturesFallback).toHaveBeenCalledWith(mockAdbClient); + + server.close(); + }); + it('can consume device list updates from an external discovery source', async () => { const unsubscribe = vi.fn(); const getDevices = vi.fn().mockResolvedValue([ diff --git a/packages/android/src/adb-server-client-features-fallback.ts b/packages/android/src/adb-server-client-features-fallback.ts new file mode 100644 index 0000000000..ef7d2298a9 --- /dev/null +++ b/packages/android/src/adb-server-client-features-fallback.ts @@ -0,0 +1,93 @@ +import { + type AdbFeature, + AdbServerClient, + type AdbServerClient as AdbServerClientType, +} from '@yume-chan/adb'; + +type DeviceFeatures = { + transportId: bigint; + features: readonly AdbFeature[]; +}; + +const patchedClients = new WeakSet(); + +function isMultiDeviceFeatureError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error); + return message.includes('more than one device/emulator'); +} + +async function resolveTransportId( + client: AdbServerClientType, + device: AdbServerClient.DeviceSelector, +): Promise { + if (device && 'transportId' in device) { + return device.transportId; + } + + if (device && 'serial' in device) { + const devices = await client.getDevices(); + return devices.find((info) => info.serial === device.serial)?.transportId; + } + + if (!device) { + const devices = await client.getDevices(); + if (devices.length !== 1) { + return undefined; + } + return devices[0]?.transportId; + } + + return undefined; +} + +async function getDeviceFeaturesByTransportId( + client: AdbServerClientType, + transportId: bigint, +): Promise { + const connection = await client.createConnection( + AdbServerClient.formatDeviceService({ transportId }, 'features'), + ); + try { + const featuresString = await connection.readString(); + const features = featuresString + ? (featuresString.split(',') as AdbFeature[]) + : []; + return { transportId, features }; + } finally { + await connection.dispose(); + } +} + +export function installAdbServerClientFeaturesFallback( + client: AdbServerClientType, +): void { + if (patchedClients.has(client)) { + return; + } + + const getDeviceFeatures = client.getDeviceFeatures.bind(client); + client.getDeviceFeatures = async (device) => { + try { + return await getDeviceFeatures(device); + } catch (error) { + if (!isMultiDeviceFeatureError(error)) { + throw error; + } + + let transportId: bigint | undefined; + try { + transportId = await resolveTransportId(client, device); + } catch { + throw error; + } + + if (transportId === undefined) { + throw error; + } + + return getDeviceFeaturesByTransportId(client, transportId); + } + }; + + patchedClients.add(client); +} diff --git a/packages/android/src/index.ts b/packages/android/src/index.ts index 693bae11c0..5eff37de92 100644 --- a/packages/android/src/index.ts +++ b/packages/android/src/index.ts @@ -9,3 +9,4 @@ export { } from './utils'; export type { AndroidConnectedDevice } from './utils'; export { ScrcpyDeviceAdapter } from './scrcpy-device-adapter'; +export { installAdbServerClientFeaturesFallback } from './adb-server-client-features-fallback'; diff --git a/packages/android/src/scrcpy-device-adapter.ts b/packages/android/src/scrcpy-device-adapter.ts index 5b8d0b6684..a5ceb57097 100644 --- a/packages/android/src/scrcpy-device-adapter.ts +++ b/packages/android/src/scrcpy-device-adapter.ts @@ -1,6 +1,7 @@ import type { Size } from '@midscene/core'; import { createImgBase64ByFormat } from '@midscene/shared/img'; import { getDebug } from '@midscene/shared/logger'; +import { installAdbServerClientFeaturesFallback } from './adb-server-client-features-fallback'; import type { ScrcpyScreenshotManager } from './scrcpy-manager'; import { DEFAULT_SCRCPY_CONFIG } from './scrcpy-manager'; @@ -111,6 +112,7 @@ export class ScrcpyDeviceAdapter { const adbClient = new AdbServerClient( new AdbServerNodeTcpConnector({ host: '127.0.0.1', port: 5037 }), ); + installAdbServerClientFeaturesFallback(adbClient); const adb = new Adb( await adbClient.createTransport({ serial: this.deviceId }), ); diff --git a/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts b/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts new file mode 100644 index 0000000000..661a86764a --- /dev/null +++ b/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts @@ -0,0 +1,78 @@ +import type { AdbServerClient } from '@yume-chan/adb'; +import { describe, expect, it, vi } from 'vitest'; +import { installAdbServerClientFeaturesFallback } from '../../src/adb-server-client-features-fallback'; + +const targetDevice: AdbServerClient.Device = { + serial: 'target-device', + state: 'device', + authenticating: false, + transportId: 42n, +}; + +function createConnection(featuresString = 'shell_v2,cmd') { + return { + readString: vi.fn().mockResolvedValue(featuresString), + dispose: vi.fn().mockResolvedValue(undefined), + }; +} + +describe('installAdbServerClientFeaturesFallback', () => { + it('falls back to a transport-id qualified features request for the multi-device ADB server bug', async () => { + const connection = createConnection(); + const client = { + getDeviceFeatures: vi + .fn() + .mockRejectedValue(new Error('more than one device/emulator')), + getDevices: vi.fn().mockResolvedValue([targetDevice]), + createConnection: vi.fn().mockResolvedValue(connection), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + + await expect( + client.getDeviceFeatures({ serial: 'target-device' }), + ).resolves.toEqual({ + transportId: 42n, + features: ['shell_v2', 'cmd'], + }); + expect(client.getDevices).toHaveBeenCalledTimes(1); + expect(client.createConnection).toHaveBeenCalledWith( + 'host-transport-id:42:features', + ); + expect(connection.dispose).toHaveBeenCalledTimes(1); + }); + + it('preserves the original error when the selector cannot be resolved deterministically', async () => { + const originalError = new Error('more than one device/emulator'); + const client = { + getDeviceFeatures: vi.fn().mockRejectedValue(originalError), + getDevices: vi.fn().mockResolvedValue([targetDevice]), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + + await expect(client.getDeviceFeatures({ usb: true })).rejects.toBe( + originalError, + ); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + + it('does not fallback for unrelated getDeviceFeatures errors', async () => { + const originalError = new Error('device offline'); + const client = { + getDeviceFeatures: vi.fn().mockRejectedValue(originalError), + getDevices: vi.fn(), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + + await expect( + client.getDeviceFeatures({ serial: 'target-device' }), + ).rejects.toBe(originalError); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(client.createConnection).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/android/tests/unit-test/scrcpy-adapter.test.ts b/packages/android/tests/unit-test/scrcpy-adapter.test.ts index d9a744327e..676ce003f5 100644 --- a/packages/android/tests/unit-test/scrcpy-adapter.test.ts +++ b/packages/android/tests/unit-test/scrcpy-adapter.test.ts @@ -3,12 +3,36 @@ import type { DevicePhysicalInfo } from '../../src/scrcpy-device-adapter'; import { ScrcpyDeviceAdapter } from '../../src/scrcpy-device-adapter'; import { DEFAULT_SCRCPY_CONFIG } from '../../src/scrcpy-manager'; +const adbMocks = vi.hoisted(() => { + const createClient = () => { + const originalGetDeviceFeatures = vi + .fn() + .mockResolvedValue({ transportId: 1n, features: [] }); + return { + originalGetDeviceFeatures, + getDeviceFeatures: originalGetDeviceFeatures, + getDevices: vi.fn().mockResolvedValue([]), + createConnection: vi.fn(), + createTransport: vi.fn().mockResolvedValue({}), + }; + }; + + let latestClient: ReturnType | undefined; + + return { + getLatestClient: () => latestClient, + mockAdb: vi.fn().mockImplementation(() => ({})), + mockAdbServerClient: vi.fn().mockImplementation(() => { + latestClient = createClient(); + return latestClient; + }), + }; +}); + // Mock @yume-chan packages (ESM-only, used via dynamic import in ensureManager) vi.mock('@yume-chan/adb', () => ({ - Adb: vi.fn().mockImplementation(() => ({})), - AdbServerClient: vi.fn().mockImplementation(() => ({ - createTransport: vi.fn().mockResolvedValue({}), - })), + Adb: adbMocks.mockAdb, + AdbServerClient: adbMocks.mockAdbServerClient, })); vi.mock('@yume-chan/adb-server-node-tcp', () => ({ @@ -254,6 +278,21 @@ describe('ScrcpyDeviceAdapter', () => { expect((adapter as any).manager).toBe(currentMockManager); }); + it('should install the multi-device features fallback before creating transport', async () => { + const adapter = new ScrcpyDeviceAdapter('device', { enabled: true }); + + await adapter.ensureManager(defaultDeviceInfo); + + const adbClient = adbMocks.getLatestClient(); + expect(adbClient).toBeDefined(); + expect(adbClient?.getDeviceFeatures).not.toBe( + adbClient?.originalGetDeviceFeatures, + ); + expect(adbClient?.createTransport).toHaveBeenCalledWith({ + serial: 'device', + }); + }); + it('should NOT cache manager when validateEnvironment fails', async () => { const adapter = new ScrcpyDeviceAdapter('device', { enabled: true }); currentMockManager.validateEnvironment.mockRejectedValue( From 50fe5f7ea21e386cee4666cd4b0b7e47d1a919a7 Mon Sep 17 00:00:00 2001 From: quanru Date: Mon, 15 Jun 2026 16:26:09 +0800 Subject: [PATCH 2/3] fix(android): address scrcpy fallback review --- .../android-playground/src/scrcpy-server.ts | 2 +- .../tests/unit/scrcpy-server.test.ts | 9 ++- packages/android/package.json | 5 ++ packages/android/rslib.config.ts | 2 + packages/android/src/index.ts | 1 - .../adb-server-client-features-fallback.ts | 18 +++-- packages/android/src/scrcpy-device-adapter.ts | 2 +- ...db-server-client-features-fallback.test.ts | 68 ++++++++++++++++++- 8 files changed, 96 insertions(+), 11 deletions(-) rename packages/android/src/{ => internal}/adb-server-client-features-fallback.ts (78%) diff --git a/packages/android-playground/src/scrcpy-server.ts b/packages/android-playground/src/scrcpy-server.ts index 46fbaa6cd0..b34140ed88 100644 --- a/packages/android-playground/src/scrcpy-server.ts +++ b/packages/android-playground/src/scrcpy-server.ts @@ -5,7 +5,7 @@ import type { Server as HttpServer } from 'node:http'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; -import { installAdbServerClientFeaturesFallback } from '@midscene/android'; +import { installAdbServerClientFeaturesFallback } from '@midscene/android/internal/adb-server-client-features-fallback'; import { SCRCPY_ADB_CONNECT_TIMEOUT_MS, SCRCPY_PREVIEW_METADATA_TIMEOUT_MS, diff --git a/packages/android-playground/tests/unit/scrcpy-server.test.ts b/packages/android-playground/tests/unit/scrcpy-server.test.ts index b83df3b2af..c91c1134b9 100644 --- a/packages/android-playground/tests/unit/scrcpy-server.test.ts +++ b/packages/android-playground/tests/unit/scrcpy-server.test.ts @@ -35,9 +35,12 @@ vi.mock('node:child_process', () => ({ exec: mockExec, })); -vi.mock('@midscene/android', () => ({ - installAdbServerClientFeaturesFallback: mockInstallFeaturesFallback, -})); +vi.mock( + '@midscene/android/internal/adb-server-client-features-fallback', + () => ({ + installAdbServerClientFeaturesFallback: mockInstallFeaturesFallback, + }), +); vi.mock('@yume-chan/adb', () => ({ Adb: vi.fn().mockImplementation(() => ({})), diff --git a/packages/android/package.json b/packages/android/package.json index 3acf702795..895117b82f 100644 --- a/packages/android/package.json +++ b/packages/android/package.json @@ -32,6 +32,11 @@ "import": "./dist/es/mcp-server.mjs", "require": "./dist/lib/mcp-server.js" }, + "./internal/adb-server-client-features-fallback": { + "types": "./dist/types/internal/adb-server-client-features-fallback.d.ts", + "import": "./dist/es/internal/adb-server-client-features-fallback.mjs", + "require": "./dist/lib/internal/adb-server-client-features-fallback.js" + }, "./package.json": "./package.json" }, "scripts": { diff --git a/packages/android/rslib.config.ts b/packages/android/rslib.config.ts index 50f42717c3..31f35b360b 100644 --- a/packages/android/rslib.config.ts +++ b/packages/android/rslib.config.ts @@ -38,6 +38,8 @@ export default defineConfig({ index: './src/index.ts', cli: './src/cli.ts', 'mcp-server': './src/mcp-server.ts', + 'internal/adb-server-client-features-fallback': + './src/internal/adb-server-client-features-fallback.ts', }, define: { __VERSION__: JSON.stringify(version), diff --git a/packages/android/src/index.ts b/packages/android/src/index.ts index 5eff37de92..693bae11c0 100644 --- a/packages/android/src/index.ts +++ b/packages/android/src/index.ts @@ -9,4 +9,3 @@ export { } from './utils'; export type { AndroidConnectedDevice } from './utils'; export { ScrcpyDeviceAdapter } from './scrcpy-device-adapter'; -export { installAdbServerClientFeaturesFallback } from './adb-server-client-features-fallback'; diff --git a/packages/android/src/adb-server-client-features-fallback.ts b/packages/android/src/internal/adb-server-client-features-fallback.ts similarity index 78% rename from packages/android/src/adb-server-client-features-fallback.ts rename to packages/android/src/internal/adb-server-client-features-fallback.ts index ef7d2298a9..ac50da7311 100644 --- a/packages/android/src/adb-server-client-features-fallback.ts +++ b/packages/android/src/internal/adb-server-client-features-fallback.ts @@ -11,9 +11,12 @@ type DeviceFeatures = { const patchedClients = new WeakSet(); +function getErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + function isMultiDeviceFeatureError(error: unknown): boolean { - const message = error instanceof Error ? error.message : String(error); - return message.includes('more than one device/emulator'); + return getErrorMessage(error).includes('more than one device/emulator'); } async function resolveTransportId( @@ -58,6 +61,10 @@ async function getDeviceFeaturesByTransportId( } } +/** + * @internal Work around ADB server feature requests that ignore serial selectors + * before yume-chan/adb includes a transport-id qualified features request. + */ export function installAdbServerClientFeaturesFallback( client: AdbServerClientType, ): void { @@ -77,8 +84,11 @@ export function installAdbServerClientFeaturesFallback( let transportId: bigint | undefined; try { transportId = await resolveTransportId(client, device); - } catch { - throw error; + } catch (resolveError) { + throw new Error( + `Failed to resolve transport ID for ADB features fallback after "${getErrorMessage(error)}": ${getErrorMessage(resolveError)}`, + { cause: resolveError }, + ); } if (transportId === undefined) { diff --git a/packages/android/src/scrcpy-device-adapter.ts b/packages/android/src/scrcpy-device-adapter.ts index a5ceb57097..6be3be0508 100644 --- a/packages/android/src/scrcpy-device-adapter.ts +++ b/packages/android/src/scrcpy-device-adapter.ts @@ -1,7 +1,7 @@ import type { Size } from '@midscene/core'; import { createImgBase64ByFormat } from '@midscene/shared/img'; import { getDebug } from '@midscene/shared/logger'; -import { installAdbServerClientFeaturesFallback } from './adb-server-client-features-fallback'; +import { installAdbServerClientFeaturesFallback } from './internal/adb-server-client-features-fallback'; import type { ScrcpyScreenshotManager } from './scrcpy-manager'; import { DEFAULT_SCRCPY_CONFIG } from './scrcpy-manager'; diff --git a/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts b/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts index 661a86764a..1161d95754 100644 --- a/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts +++ b/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts @@ -1,6 +1,6 @@ import type { AdbServerClient } from '@yume-chan/adb'; import { describe, expect, it, vi } from 'vitest'; -import { installAdbServerClientFeaturesFallback } from '../../src/adb-server-client-features-fallback'; +import { installAdbServerClientFeaturesFallback } from '../../src/internal/adb-server-client-features-fallback'; const targetDevice: AdbServerClient.Device = { serial: 'target-device', @@ -42,6 +42,44 @@ describe('installAdbServerClientFeaturesFallback', () => { expect(connection.dispose).toHaveBeenCalledTimes(1); }); + it('uses an existing transportId selector without resolving the device list', async () => { + const connection = createConnection('abb_exec'); + const client = { + getDeviceFeatures: vi + .fn() + .mockRejectedValue(new Error('more than one device/emulator')), + getDevices: vi.fn(), + createConnection: vi.fn().mockResolvedValue(connection), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + + await expect( + client.getDeviceFeatures({ transportId: 42n }), + ).resolves.toEqual({ + transportId: 42n, + features: ['abb_exec'], + }); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(client.createConnection).toHaveBeenCalledWith( + 'host-transport-id:42:features', + ); + }); + + it('does not wrap the same client more than once', () => { + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn(), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + const installedGetDeviceFeatures = client.getDeviceFeatures; + installAdbServerClientFeaturesFallback(client); + + expect(client.getDeviceFeatures).toBe(installedGetDeviceFeatures); + }); + it('preserves the original error when the selector cannot be resolved deterministically', async () => { const originalError = new Error('more than one device/emulator'); const client = { @@ -59,6 +97,34 @@ describe('installAdbServerClientFeaturesFallback', () => { expect(client.createConnection).not.toHaveBeenCalled(); }); + it('reports resolver failures while preserving the resolver error as the cause', async () => { + const resolveError = new Error('adb disconnected'); + const client = { + getDeviceFeatures: vi + .fn() + .mockRejectedValue(new Error('more than one device/emulator')), + getDevices: vi.fn().mockRejectedValue(resolveError), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientFeaturesFallback(client); + + let thrown: unknown; + try { + await client.getDeviceFeatures({ serial: 'target-device' }); + } catch (error) { + thrown = error; + } + + expect(thrown).toBeInstanceOf(Error); + expect((thrown as Error).message).toContain( + 'Failed to resolve transport ID for ADB features fallback', + ); + expect((thrown as Error).message).toContain('adb disconnected'); + expect((thrown as Error).cause).toBe(resolveError); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + it('does not fallback for unrelated getDeviceFeatures errors', async () => { const originalError = new Error('device offline'); const client = { From 88229fd9d269729f6863ad637182d73279970370 Mon Sep 17 00:00:00 2001 From: quanru Date: Mon, 15 Jun 2026 17:05:46 +0800 Subject: [PATCH 3/3] fix(android): use transport-id features requests --- .../android-playground/src/scrcpy-server.ts | 4 +- .../tests/unit/scrcpy-server.test.ts | 12 +- packages/android/package.json | 8 +- packages/android/rslib.config.ts | 4 +- ...db-server-client-transport-id-features.ts} | 51 +--- packages/android/src/scrcpy-device-adapter.ts | 4 +- ...db-server-client-features-fallback.test.ts | 144 ----------- ...erver-client-transport-id-features.test.ts | 230 ++++++++++++++++++ .../tests/unit-test/scrcpy-adapter.test.ts | 2 +- 9 files changed, 259 insertions(+), 200 deletions(-) rename packages/android/src/internal/{adb-server-client-features-fallback.ts => adb-server-client-transport-id-features.ts} (54%) delete mode 100644 packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts create mode 100644 packages/android/tests/unit-test/adb-server-client-transport-id-features.test.ts diff --git a/packages/android-playground/src/scrcpy-server.ts b/packages/android-playground/src/scrcpy-server.ts index b34140ed88..dd4b0ba578 100644 --- a/packages/android-playground/src/scrcpy-server.ts +++ b/packages/android-playground/src/scrcpy-server.ts @@ -5,7 +5,7 @@ import type { Server as HttpServer } from 'node:http'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { promisify } from 'node:util'; -import { installAdbServerClientFeaturesFallback } from '@midscene/android/internal/adb-server-client-features-fallback'; +import { installAdbServerClientTransportIdFeatures } from '@midscene/android/internal/adb-server-client-transport-id-features'; import { SCRCPY_ADB_CONNECT_TIMEOUT_MS, SCRCPY_PREVIEW_METADATA_TIMEOUT_MS, @@ -241,7 +241,7 @@ export default class ScrcpyServer { port: 5037, }), ); - installAdbServerClientFeaturesFallback(this.adbClient); + installAdbServerClientTransportIdFeatures(this.adbClient); await debugPage('success to initialize adb client'); } else { debugPage('use existing adb client'); diff --git a/packages/android-playground/tests/unit/scrcpy-server.test.ts b/packages/android-playground/tests/unit/scrcpy-server.test.ts index c91c1134b9..f534887cc8 100644 --- a/packages/android-playground/tests/unit/scrcpy-server.test.ts +++ b/packages/android-playground/tests/unit/scrcpy-server.test.ts @@ -10,7 +10,7 @@ const { mockCreateReadStream, mockOptionsCtor, mockExec, - mockInstallFeaturesFallback, + mockInstallTransportIdFeatures, mockAdbClient, mockAdbServerClient, mockAdbServerNodeTcpConnector, @@ -24,7 +24,7 @@ const { mockCreateReadStream: vi.fn(), mockOptionsCtor: vi.fn((options) => options), mockExec: vi.fn((_command, callback) => callback(null, '', '')), - mockInstallFeaturesFallback: vi.fn(), + mockInstallTransportIdFeatures: vi.fn(), mockAdbClient, mockAdbServerClient: vi.fn().mockImplementation(() => mockAdbClient), mockAdbServerNodeTcpConnector: vi.fn(), @@ -36,9 +36,9 @@ vi.mock('node:child_process', () => ({ })); vi.mock( - '@midscene/android/internal/adb-server-client-features-fallback', + '@midscene/android/internal/adb-server-client-transport-id-features', () => ({ - installAdbServerClientFeaturesFallback: mockInstallFeaturesFallback, + installAdbServerClientTransportIdFeatures: mockInstallTransportIdFeatures, }), ); @@ -129,7 +129,7 @@ describe('ScrcpyServer', () => { ]); }); - it('installs the multi-device features fallback when initializing the ADB client', async () => { + it('installs transport-id features handling when initializing the ADB client', async () => { const server = new ScrcpyServer(); await expect((server as any).getAdbClient()).resolves.toBe(mockAdbClient); @@ -143,7 +143,7 @@ describe('ScrcpyServer', () => { port: 5037, }); expect(mockAdbServerClient).toHaveBeenCalledTimes(1); - expect(mockInstallFeaturesFallback).toHaveBeenCalledWith(mockAdbClient); + expect(mockInstallTransportIdFeatures).toHaveBeenCalledWith(mockAdbClient); server.close(); }); diff --git a/packages/android/package.json b/packages/android/package.json index 895117b82f..54929f0ea4 100644 --- a/packages/android/package.json +++ b/packages/android/package.json @@ -32,10 +32,10 @@ "import": "./dist/es/mcp-server.mjs", "require": "./dist/lib/mcp-server.js" }, - "./internal/adb-server-client-features-fallback": { - "types": "./dist/types/internal/adb-server-client-features-fallback.d.ts", - "import": "./dist/es/internal/adb-server-client-features-fallback.mjs", - "require": "./dist/lib/internal/adb-server-client-features-fallback.js" + "./internal/adb-server-client-transport-id-features": { + "types": "./dist/types/internal/adb-server-client-transport-id-features.d.ts", + "import": "./dist/es/internal/adb-server-client-transport-id-features.mjs", + "require": "./dist/lib/internal/adb-server-client-transport-id-features.js" }, "./package.json": "./package.json" }, diff --git a/packages/android/rslib.config.ts b/packages/android/rslib.config.ts index 31f35b360b..624be428ab 100644 --- a/packages/android/rslib.config.ts +++ b/packages/android/rslib.config.ts @@ -38,8 +38,8 @@ export default defineConfig({ index: './src/index.ts', cli: './src/cli.ts', 'mcp-server': './src/mcp-server.ts', - 'internal/adb-server-client-features-fallback': - './src/internal/adb-server-client-features-fallback.ts', + 'internal/adb-server-client-transport-id-features': + './src/internal/adb-server-client-transport-id-features.ts', }, define: { __VERSION__: JSON.stringify(version), diff --git a/packages/android/src/internal/adb-server-client-features-fallback.ts b/packages/android/src/internal/adb-server-client-transport-id-features.ts similarity index 54% rename from packages/android/src/internal/adb-server-client-features-fallback.ts rename to packages/android/src/internal/adb-server-client-transport-id-features.ts index ac50da7311..6d30bf437a 100644 --- a/packages/android/src/internal/adb-server-client-features-fallback.ts +++ b/packages/android/src/internal/adb-server-client-transport-id-features.ts @@ -11,15 +11,7 @@ type DeviceFeatures = { const patchedClients = new WeakSet(); -function getErrorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} - -function isMultiDeviceFeatureError(error: unknown): boolean { - return getErrorMessage(error).includes('more than one device/emulator'); -} - -async function resolveTransportId( +async function resolveDeterministicTransportId( client: AdbServerClientType, device: AdbServerClient.DeviceSelector, ): Promise { @@ -32,14 +24,6 @@ async function resolveTransportId( return devices.find((info) => info.serial === device.serial)?.transportId; } - if (!device) { - const devices = await client.getDevices(); - if (devices.length !== 1) { - return undefined; - } - return devices[0]?.transportId; - } - return undefined; } @@ -62,10 +46,10 @@ async function getDeviceFeaturesByTransportId( } /** - * @internal Work around ADB server feature requests that ignore serial selectors - * before yume-chan/adb includes a transport-id qualified features request. + * @internal Prefer transport-id qualified feature requests for deterministic + * selectors before yume-chan/adb includes this behavior. */ -export function installAdbServerClientFeaturesFallback( +export function installAdbServerClientTransportIdFeatures( client: AdbServerClientType, ): void { if (patchedClients.has(client)) { @@ -74,29 +58,18 @@ export function installAdbServerClientFeaturesFallback( const getDeviceFeatures = client.getDeviceFeatures.bind(client); client.getDeviceFeatures = async (device) => { + let transportId: bigint | undefined; try { - return await getDeviceFeatures(device); - } catch (error) { - if (!isMultiDeviceFeatureError(error)) { - throw error; - } - - let transportId: bigint | undefined; - try { - transportId = await resolveTransportId(client, device); - } catch (resolveError) { - throw new Error( - `Failed to resolve transport ID for ADB features fallback after "${getErrorMessage(error)}": ${getErrorMessage(resolveError)}`, - { cause: resolveError }, - ); - } - - if (transportId === undefined) { - throw error; - } + transportId = await resolveDeterministicTransportId(client, device); + } catch { + return getDeviceFeatures(device); + } + if (transportId !== undefined) { return getDeviceFeaturesByTransportId(client, transportId); } + + return getDeviceFeatures(device); }; patchedClients.add(client); diff --git a/packages/android/src/scrcpy-device-adapter.ts b/packages/android/src/scrcpy-device-adapter.ts index 6be3be0508..48ba500bf5 100644 --- a/packages/android/src/scrcpy-device-adapter.ts +++ b/packages/android/src/scrcpy-device-adapter.ts @@ -1,7 +1,7 @@ import type { Size } from '@midscene/core'; import { createImgBase64ByFormat } from '@midscene/shared/img'; import { getDebug } from '@midscene/shared/logger'; -import { installAdbServerClientFeaturesFallback } from './internal/adb-server-client-features-fallback'; +import { installAdbServerClientTransportIdFeatures } from './internal/adb-server-client-transport-id-features'; import type { ScrcpyScreenshotManager } from './scrcpy-manager'; import { DEFAULT_SCRCPY_CONFIG } from './scrcpy-manager'; @@ -112,7 +112,7 @@ export class ScrcpyDeviceAdapter { const adbClient = new AdbServerClient( new AdbServerNodeTcpConnector({ host: '127.0.0.1', port: 5037 }), ); - installAdbServerClientFeaturesFallback(adbClient); + installAdbServerClientTransportIdFeatures(adbClient); const adb = new Adb( await adbClient.createTransport({ serial: this.deviceId }), ); diff --git a/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts b/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts deleted file mode 100644 index 1161d95754..0000000000 --- a/packages/android/tests/unit-test/adb-server-client-features-fallback.test.ts +++ /dev/null @@ -1,144 +0,0 @@ -import type { AdbServerClient } from '@yume-chan/adb'; -import { describe, expect, it, vi } from 'vitest'; -import { installAdbServerClientFeaturesFallback } from '../../src/internal/adb-server-client-features-fallback'; - -const targetDevice: AdbServerClient.Device = { - serial: 'target-device', - state: 'device', - authenticating: false, - transportId: 42n, -}; - -function createConnection(featuresString = 'shell_v2,cmd') { - return { - readString: vi.fn().mockResolvedValue(featuresString), - dispose: vi.fn().mockResolvedValue(undefined), - }; -} - -describe('installAdbServerClientFeaturesFallback', () => { - it('falls back to a transport-id qualified features request for the multi-device ADB server bug', async () => { - const connection = createConnection(); - const client = { - getDeviceFeatures: vi - .fn() - .mockRejectedValue(new Error('more than one device/emulator')), - getDevices: vi.fn().mockResolvedValue([targetDevice]), - createConnection: vi.fn().mockResolvedValue(connection), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - - await expect( - client.getDeviceFeatures({ serial: 'target-device' }), - ).resolves.toEqual({ - transportId: 42n, - features: ['shell_v2', 'cmd'], - }); - expect(client.getDevices).toHaveBeenCalledTimes(1); - expect(client.createConnection).toHaveBeenCalledWith( - 'host-transport-id:42:features', - ); - expect(connection.dispose).toHaveBeenCalledTimes(1); - }); - - it('uses an existing transportId selector without resolving the device list', async () => { - const connection = createConnection('abb_exec'); - const client = { - getDeviceFeatures: vi - .fn() - .mockRejectedValue(new Error('more than one device/emulator')), - getDevices: vi.fn(), - createConnection: vi.fn().mockResolvedValue(connection), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - - await expect( - client.getDeviceFeatures({ transportId: 42n }), - ).resolves.toEqual({ - transportId: 42n, - features: ['abb_exec'], - }); - expect(client.getDevices).not.toHaveBeenCalled(); - expect(client.createConnection).toHaveBeenCalledWith( - 'host-transport-id:42:features', - ); - }); - - it('does not wrap the same client more than once', () => { - const client = { - getDeviceFeatures: vi.fn(), - getDevices: vi.fn(), - createConnection: vi.fn(), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - const installedGetDeviceFeatures = client.getDeviceFeatures; - installAdbServerClientFeaturesFallback(client); - - expect(client.getDeviceFeatures).toBe(installedGetDeviceFeatures); - }); - - it('preserves the original error when the selector cannot be resolved deterministically', async () => { - const originalError = new Error('more than one device/emulator'); - const client = { - getDeviceFeatures: vi.fn().mockRejectedValue(originalError), - getDevices: vi.fn().mockResolvedValue([targetDevice]), - createConnection: vi.fn(), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - - await expect(client.getDeviceFeatures({ usb: true })).rejects.toBe( - originalError, - ); - expect(client.getDevices).not.toHaveBeenCalled(); - expect(client.createConnection).not.toHaveBeenCalled(); - }); - - it('reports resolver failures while preserving the resolver error as the cause', async () => { - const resolveError = new Error('adb disconnected'); - const client = { - getDeviceFeatures: vi - .fn() - .mockRejectedValue(new Error('more than one device/emulator')), - getDevices: vi.fn().mockRejectedValue(resolveError), - createConnection: vi.fn(), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - - let thrown: unknown; - try { - await client.getDeviceFeatures({ serial: 'target-device' }); - } catch (error) { - thrown = error; - } - - expect(thrown).toBeInstanceOf(Error); - expect((thrown as Error).message).toContain( - 'Failed to resolve transport ID for ADB features fallback', - ); - expect((thrown as Error).message).toContain('adb disconnected'); - expect((thrown as Error).cause).toBe(resolveError); - expect(client.createConnection).not.toHaveBeenCalled(); - }); - - it('does not fallback for unrelated getDeviceFeatures errors', async () => { - const originalError = new Error('device offline'); - const client = { - getDeviceFeatures: vi.fn().mockRejectedValue(originalError), - getDevices: vi.fn(), - createConnection: vi.fn(), - } as unknown as AdbServerClient; - - installAdbServerClientFeaturesFallback(client); - - await expect( - client.getDeviceFeatures({ serial: 'target-device' }), - ).rejects.toBe(originalError); - expect(client.getDevices).not.toHaveBeenCalled(); - expect(client.createConnection).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/android/tests/unit-test/adb-server-client-transport-id-features.test.ts b/packages/android/tests/unit-test/adb-server-client-transport-id-features.test.ts new file mode 100644 index 0000000000..05c8879f7a --- /dev/null +++ b/packages/android/tests/unit-test/adb-server-client-transport-id-features.test.ts @@ -0,0 +1,230 @@ +import type { AdbServerClient } from '@yume-chan/adb'; +import { describe, expect, it, vi } from 'vitest'; +import { installAdbServerClientTransportIdFeatures } from '../../src/internal/adb-server-client-transport-id-features'; + +const targetDevice: AdbServerClient.Device = { + serial: 'target-device', + state: 'device', + authenticating: false, + transportId: 42n, +}; + +function createConnection(featuresString = 'shell_v2,cmd') { + return { + readString: vi.fn().mockResolvedValue(featuresString), + dispose: vi.fn().mockResolvedValue(undefined), + }; +} + +describe('installAdbServerClientTransportIdFeatures', () => { + it('uses a transport-id qualified features request for serial selectors', async () => { + const connection = createConnection(); + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn().mockResolvedValue([targetDevice]), + createConnection: vi.fn().mockResolvedValue(connection), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect( + client.getDeviceFeatures({ serial: 'target-device' }), + ).resolves.toEqual({ + transportId: 42n, + features: ['shell_v2', 'cmd'], + }); + expect(client.getDevices).toHaveBeenCalledTimes(1); + expect(originalGetDeviceFeatures).not.toHaveBeenCalled(); + expect(client.createConnection).toHaveBeenCalledWith( + 'host-transport-id:42:features', + ); + expect(connection.dispose).toHaveBeenCalledTimes(1); + }); + + it('uses an existing transportId selector without resolving the device list', async () => { + const connection = createConnection('abb_exec'); + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn(), + createConnection: vi.fn().mockResolvedValue(connection), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect( + client.getDeviceFeatures({ transportId: 42n }), + ).resolves.toEqual({ + transportId: 42n, + features: ['abb_exec'], + }); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(originalGetDeviceFeatures).not.toHaveBeenCalled(); + expect(client.createConnection).toHaveBeenCalledWith( + 'host-transport-id:42:features', + ); + }); + + it('does not wrap the same client more than once', () => { + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn(), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientTransportIdFeatures(client); + const installedGetDeviceFeatures = client.getDeviceFeatures; + installAdbServerClientTransportIdFeatures(client); + + expect(client.getDeviceFeatures).toBe(installedGetDeviceFeatures); + }); + + it('delegates non-deterministic selectors to the original implementation', async () => { + const originalFeatures = { + transportId: 42n, + features: ['shell_v2'], + }; + const client = { + getDeviceFeatures: vi.fn().mockResolvedValue(originalFeatures), + getDevices: vi.fn().mockResolvedValue([targetDevice]), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect(client.getDeviceFeatures({ usb: true })).resolves.toBe( + originalFeatures, + ); + expect(originalGetDeviceFeatures).toHaveBeenCalledWith({ usb: true }); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + + it('delegates serial selectors when the serial cannot be resolved', async () => { + const originalFeatures = { + transportId: 7n, + features: ['cmd'], + }; + const client = { + getDeviceFeatures: vi.fn().mockResolvedValue(originalFeatures), + getDevices: vi.fn().mockResolvedValue([ + { + ...targetDevice, + serial: 'other-device', + }, + ]), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect( + client.getDeviceFeatures({ serial: 'target-device' }), + ).resolves.toBe(originalFeatures); + expect(client.getDevices).toHaveBeenCalledTimes(1); + expect(originalGetDeviceFeatures).toHaveBeenCalledWith({ + serial: 'target-device', + }); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + + it('delegates serial selectors when resolving devices fails', async () => { + const originalFeatures = { + transportId: 7n, + features: ['cmd'], + }; + const client = { + getDeviceFeatures: vi.fn().mockResolvedValue(originalFeatures), + getDevices: vi.fn().mockRejectedValue(new Error('adb disconnected')), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect( + client.getDeviceFeatures({ serial: 'target-device' }), + ).resolves.toBe(originalFeatures); + expect(originalGetDeviceFeatures).toHaveBeenCalledWith({ + serial: 'target-device', + }); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + + it('preserves original implementation errors for delegated selectors', async () => { + const originalError = new Error('device offline'); + const client = { + getDeviceFeatures: vi.fn().mockRejectedValue(originalError), + getDevices: vi.fn(), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + + installAdbServerClientTransportIdFeatures(client); + + await expect(client.getDeviceFeatures({ usb: true })).rejects.toBe( + originalError, + ); + expect(client.getDevices).not.toHaveBeenCalled(); + expect(client.createConnection).not.toHaveBeenCalled(); + }); + + it('propagates transport-id qualified feature request errors', async () => { + const transportError = new Error('transport closed'); + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn(), + createConnection: vi.fn().mockRejectedValue(transportError), + } as unknown as AdbServerClient; + + installAdbServerClientTransportIdFeatures(client); + + await expect(client.getDeviceFeatures({ transportId: 42n })).rejects.toBe( + transportError, + ); + expect(client.getDevices).not.toHaveBeenCalled(); + }); + + it('disposes the feature connection when reading features fails', async () => { + const readError = new Error('read failed'); + const connection = { + readString: vi.fn().mockRejectedValue(readError), + dispose: vi.fn().mockResolvedValue(undefined), + }; + const client = { + getDeviceFeatures: vi.fn(), + getDevices: vi.fn(), + createConnection: vi.fn().mockResolvedValue(connection), + } as unknown as AdbServerClient; + + installAdbServerClientTransportIdFeatures(client); + + await expect(client.getDeviceFeatures({ transportId: 42n })).rejects.toBe( + readError, + ); + expect(connection.dispose).toHaveBeenCalledTimes(1); + }); + + it('delegates undefined selectors to the original implementation', async () => { + const originalFeatures = { + transportId: 42n, + features: ['shell_v2'], + }; + const client = { + getDeviceFeatures: vi.fn().mockResolvedValue(originalFeatures), + getDevices: vi.fn(), + createConnection: vi.fn(), + } as unknown as AdbServerClient; + const originalGetDeviceFeatures = client.getDeviceFeatures; + + installAdbServerClientTransportIdFeatures(client); + + await expect(client.getDeviceFeatures(undefined)).resolves.toBe( + originalFeatures, + ); + expect(originalGetDeviceFeatures).toHaveBeenCalledWith(undefined); + expect(client.createConnection).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/android/tests/unit-test/scrcpy-adapter.test.ts b/packages/android/tests/unit-test/scrcpy-adapter.test.ts index 676ce003f5..38beb2484c 100644 --- a/packages/android/tests/unit-test/scrcpy-adapter.test.ts +++ b/packages/android/tests/unit-test/scrcpy-adapter.test.ts @@ -278,7 +278,7 @@ describe('ScrcpyDeviceAdapter', () => { expect((adapter as any).manager).toBe(currentMockManager); }); - it('should install the multi-device features fallback before creating transport', async () => { + it('should install transport-id features handling before creating transport', async () => { const adapter = new ScrcpyDeviceAdapter('device', { enabled: true }); await adapter.ensureManager(defaultDeviceInfo);