From fee6b76da8c7c67bc2584e44f9c77199c97941e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 24 Jun 2026 17:53:44 +0200 Subject: [PATCH] fix: clean up Android snapshot helper sessions --- src/core/interactors/android.ts | 3 + .../__tests__/session-close-shutdown.test.ts | 61 ++++++ src/daemon/handlers/session-close.ts | 8 + .../android/__tests__/snapshot.test.ts | 200 +++++++++++++++++- .../android/snapshot-helper-session.ts | 13 ++ src/platforms/android/snapshot-helper.ts | 2 + src/platforms/android/snapshot.ts | 34 +-- 7 files changed, 303 insertions(+), 18 deletions(-) diff --git a/src/core/interactors/android.ts b/src/core/interactors/android.ts index 7cadf1d6f..7ad19df09 100644 --- a/src/core/interactors/android.ts +++ b/src/core/interactors/android.ts @@ -71,6 +71,9 @@ export function createAndroidInteractor(device: DeviceInfo): Interactor { depth: options?.depth, scope: options?.scope, raw: options?.raw, + // appBundleId is present for app-backed daemon sessions; keep the helper warm there, + // but release it after standalone device snapshots so UiAutomation is not squatted. + helperSessionScope: options?.appBundleId ? 'daemon-session' : 'command', }), { backend: 'android' }, ); diff --git a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts index 5611ab571..9e1e23420 100644 --- a/src/daemon/handlers/__tests__/session-close-shutdown.test.ts +++ b/src/daemon/handlers/__tests__/session-close-shutdown.test.ts @@ -26,6 +26,11 @@ vi.mock('../../../platforms/android/perf.ts', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, cleanupAndroidNativePerfSession: vi.fn(async () => {}) }; }); +vi.mock('../../../platforms/android/snapshot-helper.ts', async (importOriginal) => { + const actual = + await importOriginal(); + return { ...actual, stopAndroidSnapshotHelperSessionForDevice: vi.fn(async () => {}) }; +}); vi.mock('../../../platforms/ios/macos-helper.ts', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, runMacOsAlertAction: vi.fn(async () => {}) }; @@ -50,6 +55,7 @@ import { runCmd } from '../../../utils/exec.ts'; import { dispatchCommand } from '../../../core/dispatch.ts'; import { cleanupAppleXctracePerfCapture } from '../../../platforms/ios/perf-xctrace.ts'; import { cleanupAndroidNativePerfSession } from '../../../platforms/android/perf.ts'; +import { stopAndroidSnapshotHelperSessionForDevice } from '../../../platforms/android/snapshot-helper.ts'; import { WEB_DESKTOP_DEVICE } from '../../../__tests__/test-utils/index.ts'; const mockShutdownSimulator = vi.mocked(shutdownSimulator); @@ -57,6 +63,9 @@ const mockRunCmd = vi.mocked(runCmd); const mockDispatchCommand = vi.mocked(dispatchCommand); const mockCleanupAppleXctracePerfCapture = vi.mocked(cleanupAppleXctracePerfCapture); const mockCleanupAndroidNativePerfSession = vi.mocked(cleanupAndroidNativePerfSession); +const mockStopAndroidSnapshotHelperSessionForDevice = vi.mocked( + stopAndroidSnapshotHelperSessionForDevice, +); const noopInvoke = async (_req: DaemonRequest): Promise => ({ ok: true, data: {} }); @@ -176,6 +185,40 @@ test('close --shutdown calls shutdownAndroidEmulator for Android emulator and in } }); +test('close stops Android snapshot helper session before deleting session', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'android-snapshot-helper-session'; + const device: SessionState['device'] = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel_9_API_35', + kind: 'emulator', + booted: true, + }; + sessionStore.set(sessionName, { + ...makeSession(sessionName, device), + appBundleId: 'com.example.app', + }); + + const response = await handleSessionCommands({ + req: { + token: 't', + session: sessionName, + command: 'close', + positionals: [], + flags: {}, + }, + sessionName, + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + + expect(response?.ok).toBe(true); + expect(mockStopAndroidSnapshotHelperSessionForDevice).toHaveBeenCalledWith(device); + expect(sessionStore.get(sessionName)).toBeUndefined(); +}); + test('close --shutdown is ignored for non-simulator iOS devices', async () => { const sessionStore = makeSessionStore(); const sessionName = 'ios-device-shutdown-session'; @@ -411,6 +454,24 @@ test('daemon session teardown stops active Android native perf capture', async ( expect(session.nativePerf?.android).toBeUndefined(); }); +test('daemon session teardown stops Android snapshot helper session', async () => { + const sessionName = 'android-snapshot-helper-teardown-session'; + const session = { + ...makeSession(sessionName, { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', + booted: true, + }), + appBundleId: 'com.example.app', + } as SessionState; + + await teardownSessionResources(session, sessionName); + + expect(mockStopAndroidSnapshotHelperSessionForDevice).toHaveBeenCalledWith(session.device); +}); + test('close --shutdown is ignored for Android devices', async () => { const sessionStore = makeSessionStore(); const sessionName = 'android-device-shutdown-session'; diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index 175bb855d..e42430216 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -9,6 +9,7 @@ import { stopAppLog } from '../app-log.ts'; import { stopIosRunnerSession } from '../../platforms/ios/runner-client.ts'; import { cleanupAppleXctracePerfCapture } from '../../platforms/ios/perf-xctrace.ts'; import { cleanupAndroidNativePerfSession } from '../../platforms/android/perf.ts'; +import { stopAndroidSnapshotHelperSessionForDevice } from '../../platforms/android/snapshot-helper.ts'; import { clearRuntimeHintsFromApp, hasRuntimeTransportHints } from '../runtime-hints.ts'; import { cleanupRetainedMaterializedPathsForSession } from '../materialized-path-registry.ts'; import { @@ -80,6 +81,11 @@ async function stopSessionAndroidNativePerfCapture(session: SessionState): Promi session.nativePerf = { ...(session.nativePerf ?? {}), android: undefined }; } +async function stopSessionAndroidSnapshotHelper(session: SessionState): Promise { + if (session.device.platform !== 'android') return; + await stopAndroidSnapshotHelperSessionForDevice(session.device); +} + export async function teardownSessionResources( session: SessionState, sessionName: string, @@ -89,6 +95,7 @@ export async function teardownSessionResources( } await stopSessionApplePerfCapture(session); await stopSessionAndroidNativePerfCapture(session); + await stopSessionAndroidSnapshotHelper(session); if (isApplePlatform(session.device.platform)) { await stopAppleRunnerForClose(session); } @@ -111,6 +118,7 @@ export async function handleCloseCommand(params: { } await stopSessionApplePerfCapture(session); await stopSessionAndroidNativePerfCapture(session); + await stopSessionAndroidSnapshotHelper(session); if (shouldDispatchPlatformClose(req, session)) { if (shouldStopAppleRunnerBeforeTargetedClose(session)) { await stopAppleRunnerForClose(session); diff --git a/src/platforms/android/__tests__/snapshot.test.ts b/src/platforms/android/__tests__/snapshot.test.ts index d80a57e28..2adcc3ef9 100644 --- a/src/platforms/android/__tests__/snapshot.test.ts +++ b/src/platforms/android/__tests__/snapshot.test.ts @@ -1,8 +1,11 @@ -import { beforeEach, test, vi } from 'vitest'; +import { afterEach, beforeEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; +import { EventEmitter } from 'node:events'; import { promises as fs } from 'node:fs'; +import net from 'node:net'; import os from 'node:os'; import path from 'node:path'; +import { PassThrough } from 'node:stream'; vi.mock('../../../utils/exec.ts', async (importOriginal) => { const actual = await importOriginal(); @@ -22,11 +25,16 @@ import { AppError } from '../../../utils/errors.ts'; import { runCmd } from '../../../utils/exec.ts'; import { sleep } from '../adb.ts'; import { + resetAndroidSnapshotHelperSessions, resetAndroidSnapshotHelperInstallCache, type AndroidAdbExecutor, type AndroidSnapshotHelperManifest, } from '../snapshot-helper.ts'; -import { withAndroidAdbProvider, type AndroidAdbProvider } from '../adb-executor.ts'; +import { + withAndroidAdbProvider, + type AndroidAdbProcess, + type AndroidAdbProvider, +} from '../adb-executor.ts'; const VALID_PNG = Buffer.from( 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+b9xkAAAAASUVORK5CYII=', @@ -94,7 +102,123 @@ function createHelperAdb( }; } -beforeEach(() => { +function createPersistentSnapshotHelperProvider(options: { + calls: string[][]; + spawnArgs: string[][]; + killedProcesses: FakeAndroidProcess[]; +}): AndroidAdbProvider { + return { + exec: async (args) => { + options.calls.push(args); + if (args.includes('--show-versioncode')) return installedHelperProbe; + if (args[0] === 'forward') return { exitCode: 0, stdout: '', stderr: '' }; + if (args[0] === 'shell' && args[1] === 'am' && args[2] === 'force-stop') { + return { exitCode: 0, stdout: '', stderr: '' }; + } + throw new Error(`unexpected persistent helper adb args: ${args.join(' ')}`); + }, + spawn: (args) => { + options.spawnArgs.push(args); + const process = new FakeAndroidProcess(); + const port = readSessionPort(args); + let snapshotCount = 0; + const server = net.createServer((socket) => { + socket.once('data', (chunk) => { + const command = chunk.toString('utf8').trim(); + const [, requestId = ''] = command.split(/\s+/, 2); + if (command.startsWith('quit')) { + socket.end(sessionResponse({ requestId, body: '' })); + return; + } + snapshotCount += 1; + const body = ``; + socket.end( + sessionResponse({ + requestId, + body, + metadata: { + waitForIdleTimeoutMs: '500', + waitForIdleQuietMs: '100', + timeoutMs: '5000', + maxDepth: '128', + maxNodes: '5000', + rootPresent: 'true', + captureMode: 'interactive-windows', + windowCount: '1', + nodeCount: '1', + truncated: 'false', + elapsedMs: '8', + }, + }), + ); + }); + }); + server.listen(port, '127.0.0.1', () => { + process.stdout.write( + [ + 'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1', + 'INSTRUMENTATION_STATUS: sessionReady=true', + 'INSTRUMENTATION_STATUS_CODE: 2', + '', + ].join('\n'), + ); + }); + process.onKill = () => { + options.killedProcesses.push(process); + server.close(() => process.emitExit(0, null)); + }; + return process; + }, + }; +} + +function sessionResponse(params: { + requestId: string; + body: string; + metadata?: Record; +}): string { + const headers = { + agentDeviceProtocol: 'android-snapshot-helper-v1', + helperApiVersion: '1', + outputFormat: 'uiautomator-xml', + requestId: params.requestId, + ok: 'true', + byteLength: String(Buffer.byteLength(params.body, 'utf8')), + ...params.metadata, + }; + return `${Object.entries(headers) + .map(([key, value]) => `${key}=${value}`) + .join('\n')}\n\n${params.body}`; +} + +function readSessionPort(args: string[]): number { + const index = args.indexOf('sessionPort'); + assert.notEqual(index, -1); + return Number(args[index + 1]); +} + +class FakeAndroidProcess extends EventEmitter implements AndroidAdbProcess { + stdin = new PassThrough(); + stdout = new PassThrough(); + stderr = new PassThrough(); + killed = false; + onKill: (() => void) | undefined; + + kill(): boolean { + if (this.killed) return true; + this.killed = true; + this.onKill?.(); + return true; + } + + emitExit(code: number | null, signal: NodeJS.Signals | null): void { + this.emit('exit', code, signal); + this.emit('close', code, signal); + } +} + +beforeEach(async () => { + await resetAndroidSnapshotHelperSessions(); resetAndroidSnapshotHelperInstallCache(); mockRunCmd.mockReset(); mockSleep.mockReset(); @@ -107,6 +231,10 @@ beforeEach(() => { }); }); +afterEach(async () => { + await resetAndroidSnapshotHelperSessions(); +}); + test('screenshotAndroid waits for transient UI to settle before capture', async () => { const events: string[] = []; const outPath = path.join(os.tmpdir(), `agent-device-android-screenshot-${Date.now()}.png`); @@ -496,6 +624,72 @@ test('snapshotAndroid resolves helper adb through scoped provider', async () => assert.equal(mockRunCmd.mock.calls.length, 0); }); +test('snapshotAndroid stops command-scoped persistent helper session after capture', async () => { + const adbCalls: string[][] = []; + const spawnArgs: string[][] = []; + const killedProcesses: FakeAndroidProcess[] = []; + const provider = createPersistentSnapshotHelperProvider({ + calls: adbCalls, + spawnArgs, + killedProcesses, + }); + + const result = await snapshotAndroid(device, { + helperAdb: provider, + helperArtifact, + }); + + assert.equal(result.nodes[0]?.label, 'persistent helper snapshot 1'); + assert.equal(result.androidSnapshot.helperTransport, 'persistent-session'); + assert.equal(result.androidSnapshot.helperSessionReused, false); + assert.equal(spawnArgs.length, 1); + assert.equal(killedProcesses.length, 1); + assert.equal( + adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'), + true, + ); +}); + +test('snapshotAndroid keeps daemon-session helper alive for reuse until session cleanup', async () => { + const adbCalls: string[][] = []; + const spawnArgs: string[][] = []; + const killedProcesses: FakeAndroidProcess[] = []; + const provider = createPersistentSnapshotHelperProvider({ + calls: adbCalls, + spawnArgs, + killedProcesses, + }); + + const first = await snapshotAndroid(device, { + helperAdb: provider, + helperArtifact, + helperSessionScope: 'daemon-session', + }); + const second = await snapshotAndroid(device, { + helperAdb: provider, + helperArtifact, + helperSessionScope: 'daemon-session', + }); + + assert.equal(first.androidSnapshot.helperSessionReused, false); + assert.equal(second.androidSnapshot.helperSessionReused, true); + assert.equal(second.nodes[0]?.label, 'persistent helper snapshot 2'); + assert.equal(spawnArgs.length, 1); + assert.equal(killedProcesses.length, 0); + assert.equal( + adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'), + false, + ); + + await resetAndroidSnapshotHelperSessions(); + + assert.equal(killedProcesses.length, 1); + assert.equal( + adbCalls.some((args) => args[0] === 'forward' && args[1] === '--remove'), + true, + ); +}); + test('snapshotAndroid falls back to stock uiautomator when helper fails', async () => { const adbCalls: string[][] = []; const stockXml = diff --git a/src/platforms/android/snapshot-helper-session.ts b/src/platforms/android/snapshot-helper-session.ts index a3b53d197..39bb57e60 100644 --- a/src/platforms/android/snapshot-helper-session.ts +++ b/src/platforms/android/snapshot-helper-session.ts @@ -1,5 +1,6 @@ import net from 'node:net'; import type { AndroidAdbProcess } from './adb-executor.ts'; +import type { DeviceInfo } from '../../utils/device.ts'; import { AppError } from '../../utils/errors.ts'; import { emitDiagnostic } from '../../utils/diagnostics.ts'; import { @@ -124,6 +125,18 @@ export async function stopAndroidSnapshotHelperSession(deviceKey: string): Promi }); } +export async function stopAndroidSnapshotHelperSessionForDevice( + device: Pick, +): Promise { + await stopAndroidSnapshotHelperSession(getAndroidSnapshotHelperSessionDeviceKey(device)); +} + +export function getAndroidSnapshotHelperSessionDeviceKey( + device: Pick, +): string { + return `${device.platform}:${device.id}`; +} + export async function resetAndroidSnapshotHelperSessions(): Promise { await Promise.all( [...sessions.keys()].map((deviceKey) => stopAndroidSnapshotHelperSession(deviceKey)), diff --git a/src/platforms/android/snapshot-helper.ts b/src/platforms/android/snapshot-helper.ts index ec24da245..ad7aea2ae 100644 --- a/src/platforms/android/snapshot-helper.ts +++ b/src/platforms/android/snapshot-helper.ts @@ -10,8 +10,10 @@ export { } from './snapshot-helper-capture.ts'; export { captureAndroidSnapshotWithHelperSession, + getAndroidSnapshotHelperSessionDeviceKey, resetAndroidSnapshotHelperSessions, stopAndroidSnapshotHelperSession, + stopAndroidSnapshotHelperSessionForDevice, } from './snapshot-helper-session.ts'; export { ensureAndroidSnapshotHelper, diff --git a/src/platforms/android/snapshot.ts b/src/platforms/android/snapshot.ts index 49398449e..e2d0eba97 100644 --- a/src/platforms/android/snapshot.ts +++ b/src/platforms/android/snapshot.ts @@ -34,6 +34,7 @@ import { ANDROID_SNAPSHOT_HELPER_WAIT_FOR_IDLE_TIMEOUT_MS, ensureAndroidSnapshotHelper, forgetAndroidSnapshotHelperInstall, + getAndroidSnapshotHelperSessionDeviceKey, parseAndroidSnapshotHelperManifest, stopAndroidSnapshotHelperSession, type AndroidAdbExecutor, @@ -63,6 +64,7 @@ const RETRYABLE_ADB_STDERR_PATTERNS = [ export type AndroidSnapshotOptions = SnapshotOptions & { helperArtifact?: AndroidSnapshotHelperArtifact; helperInstallPolicy?: AndroidSnapshotHelperInstallPolicy; + helperSessionScope?: 'command' | 'daemon-session'; helperAdb?: AndroidAdbExecutor | AndroidAdbProvider; helperWaitForIdleTimeoutMs?: number; includeHiddenContentHints?: boolean; @@ -207,8 +209,9 @@ async function captureAndroidUiHierarchyWithHelper( adb: AndroidAdbExecutor, artifact: AndroidSnapshotHelperArtifact, ): Promise<{ xml: string; metadata: AndroidSnapshotBackendMetadata }> { - const helperDeviceKey = getAndroidSnapshotHelperDeviceKey(device); + const helperDeviceKey = getAndroidSnapshotHelperSessionDeviceKey(device); const adbProvider = resolveAndroidAdbProvider(device, options.helperAdb); + const commandScopedHelperSession = options.helperSessionScope !== 'daemon-session'; try { const install = await installAndroidSnapshotHelper( options, @@ -220,13 +223,13 @@ async function captureAndroidUiHierarchyWithHelper( if (install.installed) { await stopAndroidSnapshotHelperSession(helperDeviceKey); } - const capture = await captureAndroidUiHierarchyFromHelper( + const capture = await captureAndroidUiHierarchyFromHelper({ options, adb, adbProvider, artifact, helperDeviceKey, - ); + }); return formatAndroidHelperCaptureResult(capture, artifact, install.reason); } catch (error) { return await recoverAndroidHelperCaptureFailure({ @@ -236,6 +239,10 @@ async function captureAndroidUiHierarchyWithHelper( device, adb, }); + } finally { + if (commandScopedHelperSession) { + await stopAndroidSnapshotHelperSession(helperDeviceKey); + } } } @@ -276,17 +283,18 @@ async function installAndroidSnapshotHelper( return install; } -async function captureAndroidUiHierarchyFromHelper( - options: AndroidSnapshotOptions, - adb: AndroidAdbExecutor, - adbProvider: AndroidAdbProvider, - artifact: AndroidSnapshotHelperArtifact, - deviceKey: string, -): Promise { +async function captureAndroidUiHierarchyFromHelper(params: { + options: AndroidSnapshotOptions; + adb: AndroidAdbExecutor; + adbProvider: AndroidAdbProvider; + artifact: AndroidSnapshotHelperArtifact; + helperDeviceKey: string; +}): Promise { + const { options, adb, adbProvider, artifact, helperDeviceKey } = params; const captureOptions = { adb, adbProvider, - deviceKey, + deviceKey: helperDeviceKey, helperVersion: artifact.manifest.version, helperVersionCode: artifact.manifest.versionCode, packageName: artifact.manifest.packageName, @@ -545,10 +553,6 @@ function enrichStockSnapshotFailureWithHelperReason( ); } -function getAndroidSnapshotHelperDeviceKey(device: DeviceInfo): string { - return `${device.platform}:${device.id}`; -} - async function deriveScrollableContentHintsIfNeeded( device: DeviceInfo, nodes: RawSnapshotNode[],