From ae2579b1c98f5d5d6fb09e4fdf731ba4acad03ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 29 Apr 2026 17:45:27 -0400 Subject: [PATCH] fix: centralize Android adb execution --- .fallowrc.json | 1 + package.json | 4 + rslib.config.ts | 1 + src/android-adb.ts | 11 ++ src/cli/__tests__/auth-session.test.ts | 10 +- ...t-router-android-open-adb-provider.test.ts | 64 ++++++++ .../request-router-android-perf.test.ts | 124 +++++++++++++++ src/daemon/app-log-android.ts | 6 +- src/daemon/handlers/session-observability.ts | 9 +- src/daemon/handlers/session-perf.ts | 22 ++- src/daemon/handlers/session.ts | 5 +- src/daemon/request-router.ts | 143 +++++++++++++----- .../android/__tests__/adb-executor.test.ts | 58 +++++++ .../__tests__/adb-provider-scope.test.ts | 46 ++++++ src/platforms/android/adb-executor.ts | 107 +++++++++++++ src/platforms/android/perf-frame.ts | 32 ++-- src/platforms/android/perf.ts | 30 ++-- .../android/snapshot-helper-types.ts | 13 +- src/platforms/android/snapshot.ts | 5 +- src/utils/exec.ts | 27 +++- 20 files changed, 620 insertions(+), 98 deletions(-) create mode 100644 src/android-adb.ts create mode 100644 src/daemon/__tests__/request-router-android-open-adb-provider.test.ts create mode 100644 src/daemon/__tests__/request-router-android-perf.test.ts create mode 100644 src/platforms/android/__tests__/adb-executor.test.ts create mode 100644 src/platforms/android/__tests__/adb-provider-scope.test.ts create mode 100644 src/platforms/android/adb-executor.ts diff --git a/.fallowrc.json b/.fallowrc.json index 585d198f4..0328217c1 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -9,6 +9,7 @@ "src/remote-config.ts", "src/install-source.ts", "src/android-apps.ts", + "src/android-adb.ts", "src/android-snapshot-helper.ts", "src/contracts.ts", "src/selectors.ts", diff --git a/package.json b/package.json index 78eb7f7e5..ff186be01 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,10 @@ "import": "./dist/src/android-apps.js", "types": "./dist/src/android-apps.d.ts" }, + "./android-adb": { + "import": "./dist/src/android-adb.js", + "types": "./dist/src/android-adb.d.ts" + }, "./android-snapshot-helper": { "import": "./dist/src/android-snapshot-helper.js", "types": "./dist/src/android-snapshot-helper.d.ts" diff --git a/rslib.config.ts b/rslib.config.ts index 1713d9307..cbba6ff18 100644 --- a/rslib.config.ts +++ b/rslib.config.ts @@ -24,6 +24,7 @@ export default defineConfig({ 'remote-config': 'src/remote-config.ts', 'install-source': 'src/install-source.ts', 'android-apps': 'src/android-apps.ts', + 'android-adb': 'src/android-adb.ts', 'android-snapshot-helper': 'src/android-snapshot-helper.ts', contracts: 'src/contracts.ts', selectors: 'src/selectors.ts', diff --git a/src/android-adb.ts b/src/android-adb.ts new file mode 100644 index 000000000..a55f72e69 --- /dev/null +++ b/src/android-adb.ts @@ -0,0 +1,11 @@ +export { + createDeviceAdbExecutor, + resolveAndroidAdbExecutor, + spawnAndroidAdbBySerial, + withAndroidAdbProvider, + type AndroidAdbExecutor, + type AndroidAdbExecutorOptions, + type AndroidAdbExecutorResult, + type AndroidAdbProvider, + type AndroidAdbSpawner, +} from './platforms/android/adb-executor.ts'; diff --git a/src/cli/__tests__/auth-session.test.ts b/src/cli/__tests__/auth-session.test.ts index 778fb6be2..56e8cb07a 100644 --- a/src/cli/__tests__/auth-session.test.ts +++ b/src/cli/__tests__/auth-session.test.ts @@ -201,6 +201,7 @@ test('device login opens browser, stores CLI session, and returns agent token', const opened: string[] = []; let stderr = ''; const requests: string[] = []; + const bodies: unknown[] = []; const login = await loginWithDeviceAuth({ stateDir: tempRoot, @@ -218,8 +219,9 @@ test('device login opens browser, stores CLI session, and returns agent token', openBrowser: async (url) => { opened.push(url); }, - fetch: async (url) => { + fetch: async (url, init) => { requests.push(String(url)); + bodies.push(JSON.parse(String(init?.body))); if (String(url).endsWith('/api/control-plane/device-auth/start')) { return jsonResponse({ deviceCode: 'device-secret', @@ -255,6 +257,12 @@ test('device login opens browser, stores CLI session, and returns agent token', 'https://cloud.example/api/control-plane/device-auth/start', 'https://cloud.example/api/control-plane/device-auth/poll', ]); + assert.deepEqual(bodies[0], { + client: 'agent-device', + tenant: 'acme', + runId: 'run-123', + daemonBaseUrl: 'https://daemon.example', + }); assert.equal(readCliSession({ stateDir: tempRoot })?.refreshCredential, 'adc_refresh_login'); if (process.platform !== 'win32') { const mode = fs.statSync(resolveCliSessionPath(tempRoot)).mode & 0o777; diff --git a/src/daemon/__tests__/request-router-android-open-adb-provider.test.ts b/src/daemon/__tests__/request-router-android-open-adb-provider.test.ts new file mode 100644 index 000000000..9d4949ffb --- /dev/null +++ b/src/daemon/__tests__/request-router-android-open-adb-provider.test.ts @@ -0,0 +1,64 @@ +import { expect, test, vi } from 'vitest'; + +vi.mock('../../core/dispatch.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dispatchCommand: vi.fn(), + resolveTargetDevice: vi.fn(), + }; +}); +vi.mock('../device-ready.ts', () => ({ ensureDeviceReady: vi.fn(async () => {}) })); + +import { dispatchCommand, resolveTargetDevice } from '../../core/dispatch.ts'; +import { runCmd } from '../../utils/exec.ts'; +import type { DeviceInfo } from '../../utils/device.ts'; +import { createRequestHandler } from '../request-router.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; + +const androidDevice: DeviceInfo = { + platform: 'android', + id: 'remote-android-1', + name: 'Remote Android', + kind: 'device', + target: 'mobile', + booted: true, +}; + +test('router scopes first Android open request through injected adb provider', async () => { + vi.mocked(resolveTargetDevice).mockResolvedValue(androidDevice); + vi.mocked(dispatchCommand).mockImplementationOnce(async (device) => { + await runCmd('adb', ['-s', device.id, 'shell', 'am', 'start', 'com.example.app']); + return {}; + }); + const adbCalls: string[][] = []; + const providerCalls: Array<{ device: DeviceInfo; hasSession: boolean }> = []; + + const handler = createRequestHandler({ + logPath: '/tmp/daemon.log', + token: 'token', + sessionStore: makeSessionStore('agent-device-router-open-adb-provider-'), + leaseRegistry: new LeaseRegistry(), + androidAdbProvider: ({ device, session }) => { + providerCalls.push({ device, hasSession: Boolean(session) }); + return async (args) => { + adbCalls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }; + }, + trackDownloadableArtifact: () => 'artifact-id', + }); + + const response = await handler({ + token: 'token', + session: 'default', + command: 'open', + positionals: ['com.example.app'], + flags: { platform: 'android' }, + }); + + expect(response.ok).toBe(true); + expect(providerCalls).toEqual([{ device: androidDevice, hasSession: false }]); + expect(adbCalls).toContainEqual(['shell', 'am', 'start', 'com.example.app']); +}); diff --git a/src/daemon/__tests__/request-router-android-perf.test.ts b/src/daemon/__tests__/request-router-android-perf.test.ts new file mode 100644 index 000000000..6db081358 --- /dev/null +++ b/src/daemon/__tests__/request-router-android-perf.test.ts @@ -0,0 +1,124 @@ +import { expect, test } from 'vitest'; +import { createRequestHandler } from '../request-router.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { SessionStore } from '../session-store.ts'; +import { AppError } from '../../utils/errors.ts'; +import type { + AndroidAdbExecutor, + AndroidAdbProvider, +} from '../../platforms/android/adb-executor.ts'; + +function makeAndroidSessionStore(name: string): SessionStore { + const sessionStore = new SessionStore(`/tmp/${name}`); + sessionStore.set('default', { + name: 'default', + createdAt: Date.now(), + device: { + platform: 'android', + id: 'remote-android-1', + name: 'Remote Android', + kind: 'device', + booted: true, + }, + appBundleId: 'com.example.app', + actions: [], + }); + return sessionStore; +} + +function makeHandler(sessionStore: SessionStore, androidAdbProvider: () => AndroidAdbProvider) { + return createRequestHandler({ + logPath: '/tmp/daemon.log', + token: 'token', + sessionStore, + leaseRegistry: new LeaseRegistry(), + androidAdbProvider, + trackDownloadableArtifact: () => 'artifact-id', + }); +} + +test('request handler routes Android perf through injected adb executor', async () => { + const sessionStore = makeAndroidSessionStore('agent-device-request-router-perf-test'); + const adbCalls: string[][] = []; + const adb: AndroidAdbExecutor = async (args) => { + adbCalls.push(args); + if (args.includes('meminfo')) { + return { stdout: 'TOTAL PSS: 100 TOTAL RSS: 200', stderr: '', exitCode: 0 }; + } + if (args.includes('cpuinfo')) { + return { + stdout: '3.0% 1234/com.example.app: 2.0% user + 1.0% kernel', + stderr: '', + exitCode: 0, + }; + } + return { + stdout: ['Total frames rendered: 4', 'Janky frames: 1 (25.00%)'].join('\n'), + stderr: '', + exitCode: 0, + }; + }; + const handler = makeHandler(sessionStore, () => ({ exec: adb })); + + const response = await handler({ + token: 'token', + session: 'default', + command: 'perf', + positionals: [], + flags: {}, + }); + + expect(response.ok).toBe(true); + if (!response.ok) throw new Error('Expected perf response to succeed'); + expect((response.data?.metrics as Record)?.fps?.droppedFramePercent).toBe(25); + expect(adbCalls).toContainEqual(['shell', 'dumpsys', 'gfxinfo', 'com.example.app', 'reset']); +}); + +test('request handler reports injected Android adb failures per perf metric', async () => { + const sessionStore = makeAndroidSessionStore('agent-device-request-router-perf-unavailable-test'); + const adb: AndroidAdbExecutor = async () => { + throw new AppError('COMMAND_FAILED', 'Remote Android ADB executor is unavailable'); + }; + const handler = makeHandler(sessionStore, () => ({ exec: adb })); + + const response = await handler({ + token: 'token', + session: 'default', + command: 'perf', + positionals: [], + flags: {}, + }); + + expect(response.ok).toBe(true); + if (!response.ok) throw new Error('Expected perf response to succeed'); + const metrics = response.data?.metrics as Record; + for (const metricName of ['memory', 'cpu', 'fps']) { + const metric = metrics[metricName]; + expect(metric.available).toBe(false); + expect(metric.reason).toBe('Remote Android ADB executor is unavailable'); + expect(metric.error.details.metric).toBe(metricName); + } +}); + +test('request handler scopes generic Android commands through injected adb provider', async () => { + const sessionStore = makeAndroidSessionStore('agent-device-request-router-adb-provider-test'); + const adbCalls: string[][] = []; + const adbProvider: AndroidAdbProvider = { + exec: async (args) => { + adbCalls.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + }; + const handler = makeHandler(sessionStore, () => adbProvider); + + const response = await handler({ + token: 'token', + session: 'default', + command: 'press', + positionals: ['10', '20'], + flags: {}, + }); + + expect(response.ok).toBe(true); + expect(adbCalls).toContainEqual(['shell', 'input', 'tap', '10', '20']); +}); diff --git a/src/daemon/app-log-android.ts b/src/daemon/app-log-android.ts index 415a371a5..537a5c162 100644 --- a/src/daemon/app-log-android.ts +++ b/src/daemon/app-log-android.ts @@ -1,5 +1,5 @@ -import { spawn } from 'node:child_process'; import fs from 'node:fs'; +import { spawnAndroidAdbBySerial } from '../platforms/android/adb-executor.ts'; import { AppError } from '../utils/errors.ts'; import { runCmd } from '../utils/exec.ts'; import { @@ -70,7 +70,7 @@ export async function startAndroidAppLog( ): Promise { let state: AppLogState = 'recovering'; let stopped = false; - let activeChild: ReturnType | undefined; + let activeChild: ReturnType | undefined; let activeWait: ReturnType | undefined; const wait = (async () => { @@ -82,7 +82,7 @@ export async function startAndroidAppLog( await sleep(1_000); continue; } - const child = spawn('adb', ['-s', deviceId, 'logcat', '-v', 'time', '--pid', pid], { + const child = spawnAndroidAdbBySerial(deviceId, ['logcat', '-v', 'time', '--pid', pid], { stdio: ['ignore', 'pipe', 'pipe'], }); activeChild = child; diff --git a/src/daemon/handlers/session-observability.ts b/src/daemon/handlers/session-observability.ts index 305ffaf08..93d4070c0 100644 --- a/src/daemon/handlers/session-observability.ts +++ b/src/daemon/handlers/session-observability.ts @@ -1,5 +1,6 @@ import { isCommandSupportedOnDevice } from '../../core/capabilities.ts'; import { normalizeError } from '../../utils/errors.ts'; +import type { AndroidAdbExecutor } from '../../platforms/android/adb-executor.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; import { SessionStore } from '../session-store.ts'; import { @@ -27,6 +28,7 @@ type ObservabilityParams = { req: DaemonRequest; sessionName: string; sessionStore: SessionStore; + androidAdbExecutor?: AndroidAdbExecutor; }; function resolveSessionLogBackendLabel( @@ -67,14 +69,17 @@ export async function handleSessionObservabilityCommands( // --------------------------------------------------------------------------- async function handlePerfCommand(params: ObservabilityParams): Promise { - const { sessionName, sessionStore } = params; + const { sessionName, sessionStore, androidAdbExecutor } = params; const session = sessionStore.get(sessionName); if (!session) { return errorResponse('SESSION_NOT_FOUND', 'perf requires an active session. Run open first.'); } try { - return { ok: true, data: await buildPerfResponseData(session) }; + return { + ok: true, + data: await buildPerfResponseData(session, { androidAdb: androidAdbExecutor }), + }; } catch (error) { return { ok: false, error: normalizeError(error) }; } diff --git a/src/daemon/handlers/session-perf.ts b/src/daemon/handlers/session-perf.ts index 7c3bf2d6d..530d24632 100644 --- a/src/daemon/handlers/session-perf.ts +++ b/src/daemon/handlers/session-perf.ts @@ -1,5 +1,6 @@ import type { SessionAction, SessionState } from '../types.ts'; import { normalizeError } from '../../utils/errors.ts'; +import type { AndroidAdbExecutor } from '../../platforms/android/adb-executor.ts'; import { ANDROID_CPU_SAMPLE_DESCRIPTION, ANDROID_CPU_SAMPLE_METHOD, @@ -32,6 +33,9 @@ type PerfResponseData = { metrics: Record; sampling: Record; }; +type BuildPerfResponseOptions = { + androidAdb?: AndroidAdbExecutor; +}; const RELATED_PERF_ACTION_LIMIT = 12; @@ -70,6 +74,7 @@ function readStartupPerfSamples(actions: SessionAction[]): StartupPerfSample[] { export async function buildPerfResponseData( session: SessionState, + options: BuildPerfResponseOptions = {}, ): Promise> { const response = buildBasePerfResponse(session); @@ -83,7 +88,7 @@ export async function buildPerfResponseData( } if (session.device.platform === 'android') { - await applyAndroidPerfMetrics(response, session); + await applyAndroidPerfMetrics(response, session, options); return response; } @@ -149,8 +154,9 @@ function applyMissingAppPerfMetrics(response: PerfResponseData, session: Session async function applyAndroidPerfMetrics( response: PerfResponseData, session: SessionState, + options: BuildPerfResponseOptions, ): Promise { - const results = await sampleAndroidPerfResults(session); + const results = await sampleAndroidPerfResults(session, options); response.metrics.memory = buildMetricResult(results.memory); response.metrics.cpu = buildMetricResult(results.cpu); response.metrics.fps = enrichFrameMetricWithSessionContext( @@ -210,16 +216,20 @@ function buildPlatformSamplingMetadata(session: SessionState): Record { const appBundleId = session.appBundleId as string; + const androidPerfOptions = { adb: options.androidAdb }; const [memory, cpu, fps] = await Promise.allSettled([ - sampleAndroidMemoryPerf(session.device, appBundleId), - sampleAndroidCpuPerf(session.device, appBundleId), - sampleAndroidFramePerf(session.device, appBundleId), + sampleAndroidMemoryPerf(session.device, appBundleId, androidPerfOptions), + sampleAndroidCpuPerf(session.device, appBundleId, androidPerfOptions), + sampleAndroidFramePerf(session.device, appBundleId, androidPerfOptions), ]); return { memory, cpu, fps }; } diff --git a/src/daemon/handlers/session.ts b/src/daemon/handlers/session.ts index 1e96a3d1c..dcec30989 100644 --- a/src/daemon/handlers/session.ts +++ b/src/daemon/handlers/session.ts @@ -1,6 +1,7 @@ import { dispatchCommand } from '../../core/dispatch.ts'; import { isCommandSupportedOnDevice } from '../../core/capabilities.ts'; import { resolvePayloadInput } from '../../utils/payload-input.ts'; +import type { AndroidAdbExecutor } from '../../platforms/android/adb-executor.ts'; import type { DeviceInfo } from '../../utils/device.ts'; import { normalizePlatformSelector } from '../../utils/device.ts'; import type { DaemonRequest, DaemonResponse, SessionState } from '../types.ts'; @@ -139,8 +140,9 @@ export async function handleSessionCommands(params: { logPath: string; sessionStore: SessionStore; invoke: (req: DaemonRequest) => Promise; + androidAdbExecutor?: AndroidAdbExecutor; }): Promise { - const { req, sessionName, logPath, sessionStore, invoke } = params; + const { req, sessionName, logPath, sessionStore, invoke, androidAdbExecutor } = params; if (INVENTORY_COMMANDS.has(req.command)) { return await handleSessionInventoryCommands({ @@ -203,6 +205,7 @@ export async function handleSessionCommands(params: { req, sessionName, sessionStore, + androidAdbExecutor, }); } diff --git a/src/daemon/request-router.ts b/src/daemon/request-router.ts index e913d219b..90fd7e44e 100644 --- a/src/daemon/request-router.ts +++ b/src/daemon/request-router.ts @@ -1,5 +1,10 @@ import path from 'node:path'; import type { CommandFlags } from '../core/dispatch.ts'; +import { + withAndroidAdbProvider, + type AndroidAdbExecutor, + type AndroidAdbProvider, +} from '../platforms/android/adb-executor.ts'; import { dispatchCommand, resolveTargetDevice, @@ -7,6 +12,7 @@ import { } from '../core/dispatch.ts'; import { isCommandSupportedOnDevice } from '../core/capabilities.ts'; import { AppError, normalizeError, toAppErrorCode } from '../utils/errors.ts'; +import type { DeviceInfo } from '../utils/device.ts'; import type { DaemonArtifact, DaemonRequest, @@ -80,6 +86,12 @@ const leaseAdmissionExemptCommands = new Set([ const sessionExecutionExemptCommands = new Set(leaseAdmissionExemptCommands); const sessionExecutionLocks = new Map>(); +type AndroidAdbProviderResolver = (params: { + req: DaemonRequest; + device: DeviceInfo; + session?: SessionState; +}) => AndroidAdbProvider | AndroidAdbExecutor | undefined; + // --------------------------------------------------------------------------- // Request preparation helpers // --------------------------------------------------------------------------- @@ -290,6 +302,37 @@ async function resolveExecutionLockKey( return `session:${sessionName}`; } +async function resolveAndroidAdbProviderDevice( + req: DaemonRequest, + existingSession: SessionState | undefined, +): Promise { + if (existingSession) { + return existingSession.device.platform === 'android' ? existingSession.device : undefined; + } + if (req.command !== 'open' && !hasExplicitDeviceSelector(req.flags)) { + return undefined; + } + const device = await resolveTargetDevice(req.flags ?? {}); + return device.platform === 'android' ? device : undefined; +} + +async function resolveScopedAndroidAdbProvider(params: { + req: DaemonRequest; + existingSession: SessionState | undefined; + androidAdbProvider?: AndroidAdbProviderResolver; +}): Promise<{ + provider?: AndroidAdbProvider | AndroidAdbExecutor; + executor?: AndroidAdbExecutor; +}> { + const { req, existingSession, androidAdbProvider } = params; + if (!androidAdbProvider) return {}; + const device = await resolveAndroidAdbProviderDevice(req, existingSession); + if (!device) return {}; + const provider = androidAdbProvider({ req, device, session: existingSession }); + const executor = typeof provider === 'function' ? provider : provider?.exec; + return { provider, executor }; +} + // --------------------------------------------------------------------------- // Specialized handler chain // --------------------------------------------------------------------------- @@ -301,14 +344,23 @@ async function runHandlerChain(params: { sessionStore: SessionStore; leaseRegistry: LeaseRegistry; invoke: (req: DaemonRequest) => Promise; + androidAdbExecutor?: AndroidAdbExecutor; contextFromFlags: ( flags: CommandFlags | undefined, appBundleId?: string, traceLogPath?: string, ) => DaemonCommandContext; }): Promise { - const { req, sessionName, logPath, sessionStore, leaseRegistry, invoke, contextFromFlags } = - params; + const { + req, + sessionName, + logPath, + sessionStore, + leaseRegistry, + invoke, + androidAdbExecutor, + contextFromFlags, + } = params; const leaseResponse = await handleLeaseCommands({ req, leaseRegistry }); if (leaseResponse) return leaseResponse; @@ -319,6 +371,7 @@ async function runHandlerChain(params: { logPath, sessionStore, invoke, + androidAdbExecutor, }); if (sessionResponse) return sessionResponse; @@ -549,6 +602,7 @@ export type RequestRouterDeps = { token: string; sessionStore: SessionStore; leaseRegistry: LeaseRegistry; + androidAdbProvider?: AndroidAdbProviderResolver; trackDownloadableArtifact: (opts: { artifactPath: string; tenantId?: string; @@ -559,7 +613,14 @@ export type RequestRouterDeps = { export function createRequestHandler( deps: RequestRouterDeps, ): (req: DaemonRequest) => Promise { - const { logPath, token, sessionStore, leaseRegistry, trackDownloadableArtifact } = deps; + const { + logPath, + token, + sessionStore, + leaseRegistry, + androidAdbProvider, + trackDownloadableArtifact, + } = deps; async function handleRequest(req: DaemonRequest): Promise { const debug = Boolean(req.meta?.debug || req.flags?.verbose); @@ -641,43 +702,53 @@ export function createRequestHandler( assertSessionSelectorMatches(existingSession, lockedReq.flags); } - // Phase 1: Try specialized handler chain - const handlerResponse = await runHandlerChain({ + const requestAdb = await resolveScopedAndroidAdbProvider({ req: lockedReq, - sessionName, - logPath, - sessionStore, - leaseRegistry, - invoke: handleRequest, - contextFromFlags: (flags, appBundleId, traceLogPath) => - ({ - ...contextFromFlags(logPath, flags, appBundleId, traceLogPath), - surface: sessionStore.get(sessionName)?.surface, - }) satisfies DaemonCommandContext, + existingSession, + androidAdbProvider, }); - if (handlerResponse) return finalize(handlerResponse); - - // Phase 2: Require active session for generic dispatch - const session = sessionStore.get(sessionName); - if (!session) { - return finalize({ - ok: false, - error: { - code: 'SESSION_NOT_FOUND', - message: 'No active session. Run open first.', - }, + return await withAndroidAdbProvider(requestAdb.provider, async () => { + // The ADB provider is scoped to this single locked request; handlers may re-read + // the session state, but all device-scoped adb calls in this request share it. + // Phase 1: Try specialized handler chain + const handlerResponse = await runHandlerChain({ + req: lockedReq, + sessionName, + logPath, + sessionStore, + leaseRegistry, + invoke: handleRequest, + androidAdbExecutor: requestAdb.executor, + contextFromFlags: (flags, appBundleId, traceLogPath) => + ({ + ...contextFromFlags(logPath, flags, appBundleId, traceLogPath), + surface: sessionStore.get(sessionName)?.surface, + }) satisfies DaemonCommandContext, }); - } - - // Phase 3: Dispatch command directly to device - const dispatchResponse = await dispatchGenericCommand({ - req: lockedReq, - session, - sessionName, - logPath, - sessionStore, + if (handlerResponse) return finalize(handlerResponse); + + // Phase 2: Require active session for generic dispatch + const session = sessionStore.get(sessionName); + if (!session) { + return finalize({ + ok: false, + error: { + code: 'SESSION_NOT_FOUND', + message: 'No active session. Run open first.', + }, + }); + } + + // Phase 3: Dispatch command directly to device + const dispatchResponse = await dispatchGenericCommand({ + req: lockedReq, + session, + sessionName, + logPath, + sessionStore, + }); + return finalize(dispatchResponse); }); - return finalize(dispatchResponse); }; if (!executionLockKey) { diff --git a/src/platforms/android/__tests__/adb-executor.test.ts b/src/platforms/android/__tests__/adb-executor.test.ts new file mode 100644 index 000000000..8fd426a77 --- /dev/null +++ b/src/platforms/android/__tests__/adb-executor.test.ts @@ -0,0 +1,58 @@ +import assert from 'node:assert/strict'; +import { test, vi } from 'vitest'; + +vi.mock('../../../utils/exec.ts', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + runCmd: vi.fn(async () => ({ stdout: 'ok', stderr: '', exitCode: 0 })), + }; +}); + +import { createDeviceAdbExecutor, withAndroidAdbProvider } from '../adb-executor.ts'; +import { runCmd } from '../../../utils/exec.ts'; + +const mockRunCmd = vi.mocked(runCmd); + +test('createDeviceAdbExecutor routes local commands through adb with the device serial', async () => { + const adb = createDeviceAdbExecutor({ + platform: 'android', + id: 'emulator-5554', + name: 'Pixel Emulator', + kind: 'emulator', + booted: true, + }); + + const result = await adb(['shell', 'getprop', 'sys.boot_completed'], { timeoutMs: 1000 }); + + assert.deepEqual(result, { stdout: 'ok', stderr: '', exitCode: 0 }); + assert.deepEqual(mockRunCmd.mock.calls, [ + ['adb', ['-s', 'emulator-5554', 'shell', 'getprop', 'sys.boot_completed'], { timeoutMs: 1000 }], + ]); +}); + +test('createDeviceAdbExecutor remains a local adb executor inside provider scopes', async () => { + mockRunCmd.mockClear(); + const providerCalls: string[][] = []; + const adb = createDeviceAdbExecutor({ + platform: 'android', + id: 'emulator-5554', + name: 'Pixel Emulator', + kind: 'emulator', + booted: true, + }); + + const result = await withAndroidAdbProvider( + async (args) => { + providerCalls.push(args); + return { stdout: 'provider', stderr: '', exitCode: 0 }; + }, + async () => await adb(['shell', 'echo', 'local']), + ); + + assert.equal(result.stdout, 'ok'); + assert.deepEqual(providerCalls, []); + assert.deepEqual(mockRunCmd.mock.calls, [ + ['adb', ['-s', 'emulator-5554', 'shell', 'echo', 'local'], undefined], + ]); +}); diff --git a/src/platforms/android/__tests__/adb-provider-scope.test.ts b/src/platforms/android/__tests__/adb-provider-scope.test.ts new file mode 100644 index 000000000..6725f95f4 --- /dev/null +++ b/src/platforms/android/__tests__/adb-provider-scope.test.ts @@ -0,0 +1,46 @@ +import assert from 'node:assert/strict'; +import type { ChildProcess } from 'node:child_process'; +import { test } from 'vitest'; +import { runCmd } from '../../../utils/exec.ts'; +import { spawnAndroidAdbBySerial, withAndroidAdbProvider } from '../adb-executor.ts'; + +test('withAndroidAdbProvider intercepts scoped adb commands with a device serial', async () => { + const calls: string[][] = []; + + const result = await withAndroidAdbProvider( + async (args, options) => { + calls.push(args); + return { + stdout: options?.allowFailure ? 'allowed' : 'ok', + stderr: '', + exitCode: 0, + }; + }, + async () => + await runCmd('adb', ['-s', 'emulator-5554', 'shell', 'echo', 'ok'], { + allowFailure: true, + }), + ); + + assert.equal(result.stdout, 'allowed'); + assert.deepEqual(calls, [['shell', 'echo', 'ok']]); +}); + +test('spawnAndroidAdbBySerial uses the scoped provider spawner', async () => { + const child = { pid: 123 } as ChildProcess; + const calls: string[][] = []; + + const result = await withAndroidAdbProvider( + { + exec: async () => ({ stdout: '', stderr: '', exitCode: 0 }), + spawn: (args) => { + calls.push(args); + return child; + }, + }, + async () => spawnAndroidAdbBySerial('emulator-5554', ['logcat', '-v', 'time']), + ); + + assert.equal(result, child); + assert.deepEqual(calls, [['logcat', '-v', 'time']]); +}); diff --git a/src/platforms/android/adb-executor.ts b/src/platforms/android/adb-executor.ts new file mode 100644 index 000000000..e34e126f8 --- /dev/null +++ b/src/platforms/android/adb-executor.ts @@ -0,0 +1,107 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; +import { spawn, type ChildProcess, type SpawnOptions } from 'node:child_process'; +import type { DeviceInfo } from '../../utils/device.ts'; +import { + runCmd, + withCommandExecutorOverride, + withoutCommandExecutorOverride, + type CommandExecutorOverride, + type ExecOptions, + type ExecResult, +} from '../../utils/exec.ts'; + +export type AndroidAdbExecutorOptions = Pick< + ExecOptions, + 'allowFailure' | 'timeoutMs' | 'binaryStdout' | 'stdin' | 'signal' +>; + +export type AndroidAdbExecutorResult = Pick< + ExecResult, + 'exitCode' | 'stdout' | 'stderr' | 'stdoutBuffer' +>; + +/** + * Runs device-scoped adb arguments after the device serial has already been selected. + * Implementations must be safe to call concurrently for one request. + */ +export type AndroidAdbExecutor = ( + args: string[], + options?: AndroidAdbExecutorOptions, +) => Promise; + +export type AndroidAdbSpawner = (args: string[], options?: SpawnOptions) => ChildProcess; + +export type AndroidAdbProvider = { + exec: AndroidAdbExecutor; + spawn?: AndroidAdbSpawner; +}; + +const androidAdbProviderScope = new AsyncLocalStorage(); + +export function createDeviceAdbExecutor(device: DeviceInfo): AndroidAdbExecutor { + return createSerialAdbExecutor(device.id); +} + +function createSerialAdbExecutor(serial: string): AndroidAdbExecutor { + return async (args, options) => + // Local adb execution must escape any active provider scope to avoid routing + // tunnel-backed providers back into themselves when they shell out to adb. + await withoutCommandExecutorOverride( + async () => await runCmd('adb', ['-s', serial, ...args], options), + ); +} + +function createSerialAdbSpawner(serial: string): AndroidAdbSpawner { + return (args, options) => spawn('adb', ['-s', serial, ...args], options ?? {}); +} + +export function resolveAndroidAdbExecutor( + device: DeviceInfo, + executor?: AndroidAdbExecutor, +): AndroidAdbExecutor { + return executor ?? androidAdbProviderScope.getStore()?.exec ?? createDeviceAdbExecutor(device); +} + +export function spawnAndroidAdbBySerial( + serial: string, + args: string[], + options?: SpawnOptions, +): ChildProcess { + return resolveAndroidSerialAdbSpawner(serial)(args, options); +} + +export async function withAndroidAdbProvider( + provider: AndroidAdbProvider | AndroidAdbExecutor | undefined, + fn: () => Promise, +): Promise { + if (!provider) return await fn(); + const normalized = typeof provider === 'function' ? { exec: provider } : provider; + const override = createAndroidCommandExecutorOverride(normalized); + return await androidAdbProviderScope.run( + normalized, + async () => await withCommandExecutorOverride(override, fn), + ); +} + +function createAndroidCommandExecutorOverride( + provider: AndroidAdbProvider, +): CommandExecutorOverride { + return (cmd, args, options) => { + if (cmd !== 'adb') return undefined; + const providerArgs = stripAdbSerialArgs(args); + if (!providerArgs) return undefined; + return withoutCommandExecutorOverride(async () => await provider.exec(providerArgs, options)); + }; +} + +function stripAdbSerialArgs(args: string[]): string[] | undefined { + // The provider scope only owns normalized device-scoped adb calls produced by + // this repo's adbArgs helpers: adb -s . Global commands + // such as adb devices/version and host-preconfigured invocations stay local. + if (args[0] !== '-s' || !args[1]) return undefined; + return args.slice(2); +} + +function resolveAndroidSerialAdbSpawner(serial: string): AndroidAdbSpawner { + return androidAdbProviderScope.getStore()?.spawn ?? createSerialAdbSpawner(serial); +} diff --git a/src/platforms/android/perf-frame.ts b/src/platforms/android/perf-frame.ts index 4d7cb8dba..0723e47a4 100644 --- a/src/platforms/android/perf-frame.ts +++ b/src/platforms/android/perf-frame.ts @@ -1,7 +1,6 @@ import type { DeviceInfo } from '../../utils/device.ts'; import { AppError } from '../../utils/errors.ts'; -import { runCmd } from '../../utils/exec.ts'; -import { adbArgs } from './adb.ts'; +import { resolveAndroidAdbExecutor, type AndroidAdbExecutor } from './adb-executor.ts'; import { parseAndroidFramePerfSample, type AndroidFramePerfSample } from './perf-frame-parser.ts'; export { @@ -15,22 +14,26 @@ export { const ANDROID_FRAME_PERF_TIMEOUT_MS = 15_000; const ANDROID_FRAME_RESET_TIMEOUT_MS = 3_000; +export type AndroidFramePerfOptions = { + adb?: AndroidAdbExecutor; +}; + export async function sampleAndroidFramePerf( device: DeviceInfo, packageName: string, + options: AndroidFramePerfOptions = {}, ): Promise { + const adb = resolveAndroidAdbExecutor(device, options.adb); try { - const result = await runCmd( - 'adb', - adbArgs(device, ['shell', 'dumpsys', 'gfxinfo', packageName, 'framestats']), - { timeoutMs: ANDROID_FRAME_PERF_TIMEOUT_MS }, - ); + const result = await adb(['shell', 'dumpsys', 'gfxinfo', packageName, 'framestats'], { + timeoutMs: ANDROID_FRAME_PERF_TIMEOUT_MS, + }); const sample = parseAndroidFramePerfSample( result.stdout, packageName, new Date().toISOString(), ); - await resetAndroidFramePerfStats(device, packageName); + await resetAndroidFramePerfStats(device, packageName, options); return sample; } catch (error) { throw annotateAndroidFramePerfSamplingError(packageName, error); @@ -40,9 +43,11 @@ export async function sampleAndroidFramePerf( export async function resetAndroidFramePerfStats( device: DeviceInfo, packageName: string, + options: AndroidFramePerfOptions = {}, ): Promise { + const adb = resolveAndroidAdbExecutor(device, options.adb); try { - await runCmd('adb', adbArgs(device, ['shell', 'dumpsys', 'gfxinfo', packageName, 'reset']), { + await adb(['shell', 'dumpsys', 'gfxinfo', packageName, 'reset'], { allowFailure: true, timeoutMs: ANDROID_FRAME_RESET_TIMEOUT_MS, }); @@ -52,10 +57,7 @@ export async function resetAndroidFramePerfStats( } function annotateAndroidFramePerfSamplingError(packageName: string, error: unknown): AppError { - if ( - error instanceof AppError && - (error.code === 'TOOL_MISSING' || error.code === 'COMMAND_FAILED') - ) { + if (error instanceof AppError) { return new AppError( error.code, error.message, @@ -68,10 +70,6 @@ function annotateAndroidFramePerfSamplingError(packageName: string, error: unkno ); } - if (error instanceof AppError) { - return error; - } - return new AppError( 'COMMAND_FAILED', `Failed to sample Android fps for ${packageName}`, diff --git a/src/platforms/android/perf.ts b/src/platforms/android/perf.ts index cf62f55d7..7d875b6d3 100644 --- a/src/platforms/android/perf.ts +++ b/src/platforms/android/perf.ts @@ -1,7 +1,6 @@ import type { DeviceInfo } from '../../utils/device.ts'; import { AppError } from '../../utils/errors.ts'; -import { runCmd } from '../../utils/exec.ts'; -import { adbArgs } from './adb.ts'; +import { resolveAndroidAdbExecutor, type AndroidAdbExecutor } from './adb-executor.ts'; import { roundPercent } from '../perf-utils.ts'; export { ANDROID_FRAME_SAMPLE_DESCRIPTION, @@ -22,6 +21,10 @@ export const ANDROID_MEMORY_SAMPLE_DESCRIPTION = const ANDROID_PERF_TIMEOUT_MS = 15_000; +export type AndroidPerfOptions = { + adb?: AndroidAdbExecutor; +}; + export type AndroidCpuPerfSample = { usagePercent: number; measuredAt: string; @@ -39,9 +42,11 @@ export type AndroidMemoryPerfSample = { export async function sampleAndroidCpuPerf( device: DeviceInfo, packageName: string, + options: AndroidPerfOptions = {}, ): Promise { + const adb = resolveAndroidAdbExecutor(device, options.adb); try { - const result = await runCmd('adb', adbArgs(device, ['shell', 'dumpsys', 'cpuinfo']), { + const result = await adb(['shell', 'dumpsys', 'cpuinfo'], { timeoutMs: ANDROID_PERF_TIMEOUT_MS, }); return parseAndroidCpuInfoSample(result.stdout, packageName, new Date().toISOString()); @@ -53,13 +58,13 @@ export async function sampleAndroidCpuPerf( export async function sampleAndroidMemoryPerf( device: DeviceInfo, packageName: string, + options: AndroidPerfOptions = {}, ): Promise { + const adb = resolveAndroidAdbExecutor(device, options.adb); try { - const result = await runCmd( - 'adb', - adbArgs(device, ['shell', 'dumpsys', 'meminfo', packageName]), - { timeoutMs: ANDROID_PERF_TIMEOUT_MS }, - ); + const result = await adb(['shell', 'dumpsys', 'meminfo', packageName], { + timeoutMs: ANDROID_PERF_TIMEOUT_MS, + }); return parseAndroidMemInfoSample(result.stdout, packageName, new Date().toISOString()); } catch (error) { throw annotateAndroidPerfSamplingError('memory', packageName, error); @@ -141,10 +146,7 @@ function annotateAndroidPerfSamplingError( packageName: string, error: unknown, ): AppError { - if ( - error instanceof AppError && - (error.code === 'TOOL_MISSING' || error.code === 'COMMAND_FAILED') - ) { + if (error instanceof AppError) { return new AppError( error.code, error.message, @@ -157,10 +159,6 @@ function annotateAndroidPerfSamplingError( ); } - if (error instanceof AppError) { - return error; - } - return new AppError( 'COMMAND_FAILED', `Failed to sample Android ${metric} for ${packageName}`, diff --git a/src/platforms/android/snapshot-helper-types.ts b/src/platforms/android/snapshot-helper-types.ts index d284acbcc..fede2b5ae 100644 --- a/src/platforms/android/snapshot-helper-types.ts +++ b/src/platforms/android/snapshot-helper-types.ts @@ -1,4 +1,5 @@ import type { RawSnapshotNode } from '../../utils/snapshot.ts'; +import type { AndroidAdbExecutor } from './adb-executor.ts'; import type { AndroidSnapshotAnalysis } from './ui-hierarchy.ts'; import type { AndroidSnapshotBackendMetadata } from './snapshot-types.ts'; @@ -11,17 +12,7 @@ export const ANDROID_SNAPSHOT_HELPER_OUTPUT_FORMAT = 'uiautomator-xml'; export const ANDROID_SNAPSHOT_HELPER_WAIT_FOR_IDLE_TIMEOUT_MS = 500; export const ANDROID_SNAPSHOT_HELPER_COMMAND_OVERHEAD_MS = 5_000; -export type AndroidAdbExecutor = ( - args: string[], - options?: { - allowFailure?: boolean; - timeoutMs?: number; - }, -) => Promise<{ - exitCode: number; - stdout: string; - stderr: string; -}>; +export type { AndroidAdbExecutor } from './adb-executor.ts'; export type AndroidSnapshotHelperManifest = { name: 'android-snapshot-helper'; diff --git a/src/platforms/android/snapshot.ts b/src/platforms/android/snapshot.ts index f1d492ef2..3f53f6270 100644 --- a/src/platforms/android/snapshot.ts +++ b/src/platforms/android/snapshot.ts @@ -22,6 +22,7 @@ import { type AndroidUiHierarchy, } from './ui-hierarchy.ts'; import { adbArgs } from './adb.ts'; +import { createDeviceAdbExecutor } from './adb-executor.ts'; import { deriveAndroidScrollableContentHints } from './scroll-hints.ts'; import { captureAndroidSnapshotWithHelper, @@ -188,10 +189,6 @@ async function captureStockUiHierarchy( }; } -function createDeviceAdbExecutor(device: DeviceInfo): AndroidAdbExecutor { - return async (args, options) => await runCmd('adb', adbArgs(device, args), options); -} - function getAndroidSnapshotHelperDeviceKey(device: DeviceInfo): string { // Emulator serials are port-based and can be reused after restart; capture failure invalidates // this key before falling back so stale process-local entries self-heal on the next snapshot. diff --git a/src/utils/exec.ts b/src/utils/exec.ts index dea542a96..d62f224ed 100644 --- a/src/utils/exec.ts +++ b/src/utils/exec.ts @@ -1,3 +1,4 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; import { constants } from 'node:fs'; import { access, stat } from 'node:fs/promises'; import path from 'node:path'; @@ -13,7 +14,7 @@ export type ExecResult = { stdoutBuffer?: Buffer; }; -type ExecOptions = { +export type ExecOptions = { cwd?: string; env?: NodeJS.ProcessEnv; allowFailure?: boolean; @@ -41,12 +42,34 @@ type ExecDetachedOptions = ExecOptions & { const BARE_COMMAND_RE = /^[A-Za-z0-9][A-Za-z0-9._+-]*$/; const WINDOWS_PATH_EXTENSIONS = ['.com', '.exe', '.bat', '.cmd']; +export type CommandExecutorOverride = ( + cmd: string, + args: string[], + options: ExecOptions, +) => Promise | undefined; + +const commandExecutorOverrideScope = new AsyncLocalStorage(); + +export async function withCommandExecutorOverride( + override: CommandExecutorOverride | undefined, + fn: () => Promise, +): Promise { + if (!override) return await fn(); + return await commandExecutorOverrideScope.run(override, fn); +} + +// Used by local executors to bypass the active override for intentional host command execution. +export async function withoutCommandExecutorOverride(fn: () => Promise): Promise { + return await commandExecutorOverrideScope.run(undefined, fn); +} export async function runCmd( cmd: string, args: string[], options: ExecOptions = {}, ): Promise { + const overrideResult = commandExecutorOverrideScope.getStore()?.(cmd, args, options); + if (overrideResult) return await overrideResult; return await runSpawnedCommand(cmd, args, options); } @@ -55,6 +78,8 @@ export async function runCmdStreaming( args: string[], options: ExecStreamOptions = {}, ): Promise { + const overrideResult = commandExecutorOverrideScope.getStore()?.(cmd, args, options); + if (overrideResult) return await overrideResult; return await runSpawnedCommand(cmd, args, options); }