diff --git a/packages/android-playground/src/scrcpy-server.ts b/packages/android-playground/src/scrcpy-server.ts index d0f9248956..dd4b0ba578 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 { installAdbServerClientTransportIdFeatures } from '@midscene/android/internal/adb-server-client-transport-id-features'; import { SCRCPY_ADB_CONNECT_TIMEOUT_MS, SCRCPY_PREVIEW_METADATA_TIMEOUT_MS, @@ -240,6 +241,7 @@ export default class ScrcpyServer { port: 5037, }), ); + 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 fd60852ef8..f534887cc8 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,46 @@ const { mockReadableFrom, mockCreateReadStream, mockOptionsCtor, -} = vi.hoisted(() => ({ - mockPushServer: vi.fn(), - mockStart: vi.fn(), - mockReadableFrom: vi.fn(), - mockCreateReadStream: vi.fn(), - mockOptionsCtor: vi.fn((options) => options), + mockExec, + mockInstallTransportIdFeatures, + 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, '', '')), + mockInstallTransportIdFeatures: vi.fn(), + mockAdbClient, + mockAdbServerClient: vi.fn().mockImplementation(() => mockAdbClient), + mockAdbServerNodeTcpConnector: vi.fn(), + }; +}); + +vi.mock('node:child_process', () => ({ + exec: mockExec, +})); + +vi.mock( + '@midscene/android/internal/adb-server-client-transport-id-features', + () => ({ + installAdbServerClientTransportIdFeatures: mockInstallTransportIdFeatures, + }), +); + +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 +78,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 +129,25 @@ describe('ScrcpyServer', () => { ]); }); + 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); + + expect(mockExec).toHaveBeenCalledWith( + 'adb start-server', + expect.any(Function), + ); + expect(mockAdbServerNodeTcpConnector).toHaveBeenCalledWith({ + host: '127.0.0.1', + port: 5037, + }); + expect(mockAdbServerClient).toHaveBeenCalledTimes(1); + expect(mockInstallTransportIdFeatures).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/package.json b/packages/android/package.json index 3acf702795..54929f0ea4 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-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" }, "scripts": { diff --git a/packages/android/rslib.config.ts b/packages/android/rslib.config.ts index 50f42717c3..624be428ab 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-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-transport-id-features.ts b/packages/android/src/internal/adb-server-client-transport-id-features.ts new file mode 100644 index 0000000000..6d30bf437a --- /dev/null +++ b/packages/android/src/internal/adb-server-client-transport-id-features.ts @@ -0,0 +1,76 @@ +import { + type AdbFeature, + AdbServerClient, + type AdbServerClient as AdbServerClientType, +} from '@yume-chan/adb'; + +type DeviceFeatures = { + transportId: bigint; + features: readonly AdbFeature[]; +}; + +const patchedClients = new WeakSet(); + +async function resolveDeterministicTransportId( + 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; + } + + 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(); + } +} + +/** + * @internal Prefer transport-id qualified feature requests for deterministic + * selectors before yume-chan/adb includes this behavior. + */ +export function installAdbServerClientTransportIdFeatures( + client: AdbServerClientType, +): void { + if (patchedClients.has(client)) { + return; + } + + const getDeviceFeatures = client.getDeviceFeatures.bind(client); + client.getDeviceFeatures = async (device) => { + let transportId: bigint | undefined; + try { + 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 5b8d0b6684..48ba500bf5 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 { installAdbServerClientTransportIdFeatures } from './internal/adb-server-client-transport-id-features'; 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 }), ); + installAdbServerClientTransportIdFeatures(adbClient); const adb = new Adb( await adbClient.createTransport({ serial: this.deviceId }), ); 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 d9a744327e..38beb2484c 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 transport-id features handling 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(