From 1652e2c6c2c2da29f5c49c08de94a3e64e6e4394 Mon Sep 17 00:00:00 2001 From: Gabe Debes Date: Fri, 9 Oct 2026 01:20:19 -0700 Subject: [PATCH 1/5] [build-tools] Separate agent-device and preview logs --- CHANGELOG.md | 1 + ...tDeviceRemoteSession-orchestration.test.ts | 22 ++ .../startAgentDeviceRemoteSession.test.ts | 47 ++- .../startAgentDeviceRemoteSession.ts | 27 +- .../utils/__tests__/deviceSessionHost.test.ts | 281 ++++++++++++++++-- .../src/steps/utils/deviceSessionHost.ts | 86 ++++-- .../src/utils/__tests__/logPhase.test.ts | 29 +- packages/build-tools/src/utils/logPhase.ts | 37 ++- 8 files changed, 461 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0361b9213..4944037fd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This is the log of notable changes to EAS CLI and related packages. ### 🎉 New features +- [build-tools] Give agent-device and its simulator preview separate session logs. - [build-tools] Let the MCP server read text and image files from sandboxes. ([#4555](https://github.com/expo/eas-cli/pull/4555) by [@AHGIJMKLKKZNPJKQR](https://github.com/AHGIJMKLKKZNPJKQR)) - [eas-cli][local-build-plugin] Deprecate Node.js 20, and print a warning when running on it. A future release will require Node.js 22 or newer. Development and CI now use Node.js 22. ([#4579](https://github.com/expo/eas-cli/pull/4579) by [@douglowder](https://github.com/douglowder)) - [build-tools] Add `eas/run_agent`, a step that runs Claude Code for an agent run and connects it to the Expo MCP server. ([#4552](https://github.com/expo/eas-cli/pull/4552) by [@hSATAC](https://github.com/hSATAC)) diff --git a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession-orchestration.test.ts b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession-orchestration.test.ts index 35778d2545..88c80bbe86 100644 --- a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession-orchestration.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession-orchestration.test.ts @@ -19,6 +19,7 @@ import { waitForDeviceRunSessionStoppedAsync, waitForFileAsync, } from '../../utils/remoteDeviceRunSession'; +import { createProcessOutput } from '../../utils/processOutput'; import { type StartupTasks, createStartupTasks } from '../../utils/startupTasks'; import { runAgentDeviceRemoteSessionAsync } from '../startAgentDeviceRemoteSession'; @@ -163,6 +164,27 @@ describe('runAgentDeviceRemoteSessionAsync orchestration', () => { expect(mockDaemonStopAsync).toHaveBeenCalledTimes(1); }); + it('redacts learned daemon credentials in arbitrary output and retained diagnostics', async () => { + const logger = { info: jest.fn(), warn: jest.fn(), child: jest.fn().mockReturnThis() }; + const token = 'daemon secret/+'; + jest.mocked(waitForFileAsync).mockResolvedValue({ port: 5678, token }); + jest.mocked(startNgrokTunnelAsync).mockImplementation(async () => { + const options = jest.mocked(spawnDetached).mock.calls[0][0]; + expect(options.secrets).toContain(token); + const output = createProcessOutput(options.logger, options.secrets); + output.stdout.append('unrecognized value: daemon sec'); + output.stdout.append('ret/+\n'); + output.stderr.append(`escaped value: ${encodeURIComponent(token)}\n`); + expect(output.getOutput()).toBe( + 'unrecognized value: [REDACTED]\nescaped value: [REDACTED]\n' + ); + return { url: 'https://daemon.test', subdomainId: 'daemon', stopAsync: mockTunnelStopAsync }; + }); + await runAsync(logger, BuildRuntimePlatform.DARWIN); + expect(JSON.stringify(logger.info.mock.calls)).not.toContain(token); + expect(JSON.stringify(logger.info.mock.calls)).not.toContain(encodeURIComponent(token)); + }); + it('hands network capture to serve-sim', async () => { const logger = { info: jest.fn(), warn: jest.fn(), child: jest.fn().mockReturnThis() }; diff --git a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts index 47c96bf27b..de8ae6e83a 100644 --- a/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/startAgentDeviceRemoteSession.test.ts @@ -93,6 +93,28 @@ describe(startAgentDeviceDaemonAsync, () => { }) as never); }); + it.each([false, true])( + 'shares the daemon secret list through git fallback=%s', + async fallback => { + if (fallback) { + jest.mocked(spawn).mockResolvedValue({ stdout: '' } as never); + } + const secrets: string[] = []; + const handle = await startAgentDeviceDaemonAsync({ + packageVersion: '1.2.3', + env: {}, + logger, + secrets, + }); + const options = jest.mocked(spawnDetached).mock.calls[0][0]; + expect(options.command).toBe(fallback ? 'bun' : 'node'); + expect(options.secrets).toBe(secrets); + secrets.push('learned-daemon-token'); + expect(options.secrets).toContain('learned-daemon-token'); + await handle.stopAsync(); + } + ); + it('installs with bun add by default and launches the published daemon', async () => { const handle = await startAgentDeviceDaemonAsync({ packageVersion: '1.2.3', @@ -108,6 +130,8 @@ describe(startAgentDeviceDaemonAsync, () => { const addCwd = jest.mocked(spawn).mock.calls[0][2]?.cwd as string; expect(spawnDetached).toHaveBeenCalledWith({ command: 'node', + logger, + secrets: undefined, args: [path.join(addCwd, 'node_modules/agent-device/dist/src/internal/daemon.js')], env: expect.objectContaining({ AGENT_DEVICE_DAEMON_SERVER_MODE: 'http', @@ -121,6 +145,20 @@ describe(startAgentDeviceDaemonAsync, () => { await expect(fs.promises.access(addCwd)).rejects.toThrow(); }); + it('removes the install directory when the daemon cannot be stopped', async () => { + const stopError = new Error('drain timed out'); + stopAsync.mockRejectedValueOnce(stopError); + const handle = await startAgentDeviceDaemonAsync({ + packageVersion: '1.2.3', + env: {}, + logger, + }); + const addCwd = jest.mocked(spawn).mock.calls[0][2]?.cwd as string; + + await expect(handle.stopAsync()).rejects.toBe(stopError); + await expect(fs.promises.access(addCwd)).rejects.toThrow(); + }); + it('installs with npm when EAS_OVERRIDE_PACKAGE_MANAGER is npm', async () => { await startAgentDeviceDaemonAsync({ packageVersion: undefined, @@ -136,10 +174,15 @@ describe(startAgentDeviceDaemonAsync, () => { }); it('kills the install and does not fall back to git when aborted', async () => { + let installStarted!: () => void; + const started = new Promise(resolve => { + installStarted = resolve; + }); jest.mocked(spawn).mockImplementation( ((_command: string, _args: string[], options?: { signal?: AbortSignal }) => new Promise((_resolve, reject) => { options!.signal!.addEventListener('abort', () => reject(options!.signal!.reason)); + installStarted(); })) as never ); const controller = new AbortController(); @@ -151,7 +194,7 @@ describe(startAgentDeviceDaemonAsync, () => { logger, signal: controller.signal, }); - await new Promise(resolve => setImmediate(resolve)); + await started; controller.abort(failure); await expect(daemon).rejects.toBe(failure); @@ -199,6 +242,8 @@ describe(startAgentDeviceDaemonAsync, () => { ); expect(spawnDetached).toHaveBeenCalledWith({ command: 'bun', + logger, + secrets: undefined, args: ['run', 'src/daemon.ts'], cwd: '/tmp/agent-device-src', env: expect.objectContaining({ diff --git a/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts b/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts index 5afc4faf46..4a4de23b8e 100644 --- a/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts +++ b/packages/build-tools/src/steps/functions/startAgentDeviceRemoteSession.ts @@ -121,6 +121,7 @@ export async function runAgentDeviceRemoteSessionAsync( let sessionHost: DeviceSessionHost | undefined; let eventCollection: Awaited> | undefined; let sessionFailed = false; + const daemonSecrets = [ngrokAuthtoken]; // Each task stores what it started, so the teardown below can stop it even when // another task failed first. @@ -131,12 +132,14 @@ export async function runAgentDeviceRemoteSessionAsync( env, logger: taskLogger, signal: tasks.signal, + secrets: daemonSecrets, }); taskLogger.info(`Waiting for daemon credentials at ${DAEMON_JSON_PATH}.`); // The daemon is stored above, so the teardown stops it. The wait itself is bounded // (STARTUP_TIMEOUT_MS) and only reads a file, so it may finish in the background. const daemonInfo = await tasks.untilAborted(waitForDaemonInfoAsync({ daemonProcess })); + daemonSecrets.push(daemonInfo.token); taskLogger.info(`Daemon is listening on port ${daemonInfo.port}; loaded auth token.`); tasks.signal.throwIfAborted(); @@ -155,6 +158,7 @@ export async function runAgentDeviceRemoteSessionAsync( await tasks.untilAborted(device.booted); tasks.signal.throwIfAborted(); sessionHost = await startDeviceSessionHostAsync(ctx, { + separateLogPhase: true, runtimePlatform, env, logger: taskLogger, @@ -267,12 +271,14 @@ export async function startAgentDeviceDaemonAsync({ env, logger, signal, + secrets, }: { packageVersion: string | undefined; env: BuildStepEnv; logger: bunyan; /** Kills the install and stops before the daemon starts, when aborted. No git fallback then. */ signal?: AbortSignal; + secrets?: string[]; }): Promise { const packageSpec = createAgentDevicePackageSpec(packageVersion); const packageManager = resolveConfiguredPackageManager(env, PackageManager.BUN); @@ -298,12 +304,17 @@ export async function startAgentDeviceDaemonAsync({ command: 'node', args: [daemonPath], env: { ...env, ...AGENT_DEVICE_DAEMON_ENV }, + logger, + secrets, }); return { ...daemonProcess, stopAsync: async () => { - await daemonProcess.stopAsync(); - await fs.promises.rm(installDir, { recursive: true, force: true }); + try { + await daemonProcess.stopAsync(); + } finally { + await fs.promises.rm(installDir, { recursive: true, force: true }); + } }, }; } catch (err) { @@ -334,7 +345,13 @@ export async function startAgentDeviceDaemonAsync({ logger.warn( `Failed to start daemon from ${packageSpec} via ${packageManager}; falling back to git clone: ${error.message}` ); - return await startAgentDeviceDaemonFromGitAsync({ packageVersion, env, logger, signal }); + return await startAgentDeviceDaemonFromGitAsync({ + packageVersion, + env, + logger, + signal, + secrets, + }); } } @@ -365,11 +382,13 @@ async function startAgentDeviceDaemonFromGitAsync({ env, logger, signal, + secrets, }: { packageVersion: string | undefined; env: BuildStepEnv; logger: bunyan; signal?: AbortSignal; + secrets?: string[]; }): Promise { logger.info( packageVersion @@ -396,6 +415,8 @@ async function startAgentDeviceDaemonFromGitAsync({ args: ['run', 'src/daemon.ts'], cwd: SRC_DIR, env: { ...env, ...AGENT_DEVICE_DAEMON_ENV }, + logger, + secrets, }); } diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts index 0e4bc6c7a9..26e9abdce3 100644 --- a/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts @@ -1,3 +1,4 @@ +import { BuildPhase, BuildPhaseResult, LogMarker } from '@expo/eas-build-job'; import type { bunyan } from '@expo/logger'; import { BuildRuntimePlatform, type BuildStepEnv } from '@expo/steps'; import * as ngrok from '@ngrok/ngrok'; @@ -22,6 +23,7 @@ import { fetchWebPreviewTurnArgsAsync, spawnDetached, } from '../remoteDeviceRunSession'; +import * as screenshotCollector from '../deviceRunSessionScreenshots'; jest.mock('@ngrok/ngrok'); jest.mock('../deviceRunSessionArtifacts'); @@ -62,17 +64,22 @@ const env = { NGROK_AUTHTOKEN: 'token', } as BuildStepEnv; const ctx = {} as CustomBuildContext; -const logger = { info: jest.fn(), warn: jest.fn() } as unknown as bunyan; +const logger = { + info: jest.fn(), + warn: jest.fn(), + child: jest.fn().mockReturnThis(), +} as unknown as bunyan; const stopServer = jest.fn(); const closeTunnel = jest.fn(); const directories: string[] = []; const baseDomain = 'preview.example.test'; -async function startHostAsync() { +async function startHostAsync(separateLogPhase = true) { return await startDeviceSessionHostAsync(ctx, { runtimePlatform: BuildRuntimePlatform.LINUX, env, logger, + separateLogPhase, timeoutMs: 10_000, }); } @@ -374,6 +381,10 @@ it('finishes despite a stalled tunnel close', async () => { await finishing; expect(stopServer).toHaveBeenCalledTimes(1); expect(logger.warn).toHaveBeenCalledWith(expect.anything(), expect.stringContaining('deadline')); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); closing.resolve(); }); @@ -466,6 +477,7 @@ it('leaves iOS recording to its existing build steps', async () => { runtimePlatform: BuildRuntimePlatform.DARWIN, env, logger, + separateLogPhase: true, timeoutMs: 10_000, }); const options = jest.mocked(spawnDetached).mock.calls[0][0]; @@ -477,6 +489,237 @@ it('leaves iOS recording to its existing build steps', async () => { expect(stopServer).toHaveBeenCalledTimes(1); }); +const turnArgs = [ + '--turn-url', + 'turn:turn.example.test:3478', + '--turn-username', + 'turn-user', + '--turn-credential', + 'turn-secret', +]; + +it('passes the TURN credential and the Android control token to the host as secrets', async () => { + jest.mocked(fetchWebPreviewTurnArgsAsync).mockResolvedValueOnce(turnArgs); + const host = await startHostAsync(); + const options = jest.mocked(spawnDetached).mock.calls[0][0]; + const controlToken = options.env.EXPO_DEVICE_HUB_RECORDING_CONTROL_TOKEN; + expect(controlToken).toMatch(/^[a-f0-9]{64}$/); + expect(options.secrets).toEqual(['turn-secret', controlToken]); + await host.finishAsync(); +}); + +it('adds the iOS preview token to the host secrets once serve-sim is ready', async () => { + jest.mocked(fetchWebPreviewTurnArgsAsync).mockResolvedValueOnce(turnArgs); + let secretsAtSpawn: string[] | undefined; + jest.mocked(spawnDetached).mockImplementationOnce(options => { + secretsAtSpawn = [...(options.secrets ?? [])]; + return { + pid: undefined, + getOutput: () => '', + getExitError: () => undefined, + stopAsync: stopServer, + }; + }); + const host = await startDeviceSessionHostAsync(ctx, { + runtimePlatform: BuildRuntimePlatform.DARWIN, + env, + logger, + separateLogPhase: true, + timeoutMs: 10_000, + }); + expect(secretsAtSpawn).toEqual(['turn-secret']); + // Host output is redacted against this array, so the token must land in the one passed at spawn. + expect(jest.mocked(spawnDetached).mock.calls[0][0].secrets).toEqual([ + 'turn-secret', + 'preview-token', + ]); + expect((await host.openPreviewAsync({ baseDomain })).previewToken).toBe('preview-token'); + await host.finishAsync(); +}); + +it('keeps the preview phase open until shutdown output has been logged', async () => { + const previewLogger = { info: jest.fn(), warn: jest.fn() } as unknown as bunyan; + jest.mocked(logger.child).mockReturnValueOnce(previewLogger); + const host = await startHostAsync(); + expect(logger.child).toHaveBeenCalledWith({ + phase: BuildPhase.CUSTOM, + buildStepId: expect.stringMatching(/^step-\d{3,}$/), + buildStepDisplayName: 'Simulator preview', + }); + expect(spawnDetached).toHaveBeenCalledWith(expect.objectContaining({ logger: previewLogger })); + expect(previewLogger.info).not.toHaveBeenCalledWith( + expect.objectContaining({ marker: LogMarker.END_PHASE }), + expect.anything() + ); + stopServer.mockImplementationOnce(async () => previewLogger.info('shutdown output')); + await host.finishAsync(); + await host.finishAsync(); + const calls = jest.mocked(previewLogger.info).mock.calls; + const endings = calls.filter(([fields]) => fields?.marker === LogMarker.END_PHASE); + expect(endings).toEqual([ + [ + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.SUCCESS }, + 'End phase: Simulator preview', + ], + ]); + expect(calls.findIndex(([message]) => message === 'shutdown output')).toBeLessThan( + calls.length - 1 + ); +}); + +it('ends the preview phase before waiting for screenshot uploads', async () => { + const previewLogger = { info: jest.fn(), warn: jest.fn() } as unknown as bunyan; + jest.mocked(logger.child).mockReturnValueOnce(previewLogger); + const uploading = deferred(); + const uploaded = deferred(); + const finishScreenshots = jest.fn(async () => { + uploading.resolve(); + await uploaded.promise; + }); + const collector = jest + .spyOn(screenshotCollector, 'startDeviceRunSessionScreenshotsAsync') + .mockResolvedValueOnce({ directory: '/tmp/screenshots', finishAsync: finishScreenshots }); + try { + const host = await startHostAsync(); + const finishing = host.finishAsync(); + let finished = false; + void finishing.then(() => { + finished = true; + }); + await uploading.promise; + expect(stopServer).toHaveBeenCalledTimes(1); + expect(previewLogger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.SUCCESS }, + 'End phase: Simulator preview' + ); + expect(finished).toBe(false); + uploaded.resolve(); + await finishing; + expect(collector).toHaveBeenCalledWith(ctx, expect.objectContaining({ logger })); + } finally { + uploaded.resolve(); + collector.mockRestore(); + } +}); + +it('ends the preview phase with failure when spawning fails', async () => { + jest.mocked(spawnDetached).mockImplementationOnce(() => { + throw new Error('could not spawn'); + }); + await expect(startHostAsync()).rejects.toThrow('could not spawn'); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); +}); + +it('ends the preview phase with failure when the host cannot stop', async () => { + const previewLogger = { info: jest.fn(), warn: jest.fn() } as unknown as bunyan; + jest.mocked(logger.child).mockReturnValueOnce(previewLogger); + const host = await startHostAsync(); + stopServer.mockRejectedValueOnce(new Error('could not stop')); + await host.finishAsync(); + expect(previewLogger.warn).toHaveBeenCalledWith( + expect.anything(), + 'Could not stop the expo-device-hub session host.' + ); + expect(logger.warn).not.toHaveBeenCalledWith( + expect.anything(), + 'Could not stop the expo-device-hub session host.' + ); + expect(previewLogger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); +}); + +it.each([ + ['exited during the session', true, BuildPhaseResult.FAIL], + ['exited only when stopped', false, BuildPhaseResult.SUCCESS], +])('finishes cleanup and uploads when the host %s', async (_description, exitedEarly, result) => { + let exitError: Error | undefined; + jest.mocked(spawnDetached).mockImplementationOnce(options => { + directories.push(options.args[options.args.indexOf('--android-recording-directory') + 1]); + return { + pid: undefined, + getOutput: () => '', + getExitError: () => exitError, + stopAsync: stopServer, + }; + }); + stopServer.mockImplementationOnce(async () => { + exitError ??= new Error('Process exited with signal SIGTERM.'); + }); + const host = await startHostAsync(); + await host.openPreviewAsync({ baseDomain }); + const directory = directories[0]; + await writeRecordingDescriptorAsync(directory); + jest.mocked(uploadDeviceRunSessionScreenRecordingsAsync).mockResolvedValueOnce(true); + const screenshotDirectory = + jest.mocked(spawnDetached).mock.calls[0][0].env.EXPO_DEVICE_HUB_SCREENSHOT_DIRECTORY; + if (!screenshotDirectory) { + throw new Error('Missing screenshot artifact directory'); + } + if (exitedEarly) { + exitError = new Error('Process exited with code 1.'); + } + await host.finishAsync(); + expect(closeTunnel).toHaveBeenCalledTimes(1); + expect(stopServer).toHaveBeenCalledTimes(1); + expect(uploadDeviceRunSessionScreenRecordingsAsync).toHaveBeenCalledTimes(1); + await expect(access(directory)).rejects.toMatchObject({ code: 'ENOENT' }); + await expect(access(screenshotDirectory)).rejects.toMatchObject({ code: 'ENOENT' }); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result }, + 'End phase: Simulator preview' + ); +}); + +it('ends the preview phase with failure when the tunnel cannot close', async () => { + const host = await startHostAsync(); + await host.openPreviewAsync({ baseDomain }); + closeTunnel.mockRejectedValue(new Error('tunnel close failure')); + jest.useFakeTimers(); + const finishing = host.finishAsync(); + await jest.advanceTimersByTimeAsync(4_000); + await finishing; + expect(stopServer).toHaveBeenCalledTimes(1); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); + closeTunnel.mockResolvedValue(undefined); +}); + +it('ends the preview phase with failure when startup never provides a token', async () => { + const metrics = jest.requireMock('../serveSimMetricsRecorder'); + metrics.readServeSimServersAsync.mockResolvedValueOnce([]); + await expect( + startDeviceSessionHostAsync(ctx, { + runtimePlatform: BuildRuntimePlatform.DARWIN, + env, + logger, + separateLogPhase: true, + timeoutMs: 10_000, + }) + ).rejects.toThrow('wrote no session token'); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); +}); + +it('ends the preview phase with failure if opening the tunnel fails', async () => { + const host = await startHostAsync(); + jest.mocked(ngrok.forward).mockRejectedValueOnce(new Error('tunnel unavailable')); + await expect(host.openPreviewAsync({ baseDomain })).rejects.toThrow('tunnel unavailable'); + await host.finishAsync(); + expect(logger.info).toHaveBeenCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, + 'End phase: Simulator preview' + ); +}); + it.each([BuildRuntimePlatform.LINUX, BuildRuntimePlatform.DARWIN])( 'uploads screenshots and flushes them exactly once when the %s host finishes', async runtimePlatform => { @@ -551,6 +794,7 @@ it('does not delay macOS readiness on encoder setup or start the preview after f runtimePlatform: BuildRuntimePlatform.DARWIN, env, logger, + separateLogPhase: true, timeoutMs: 10_000, }); await host.finishAsync(); @@ -568,6 +812,7 @@ it('keeps the session running when encoder setup for the preview fails', async ( runtimePlatform: BuildRuntimePlatform.DARWIN, env, logger, + separateLogPhase: true, timeoutMs: 10_000, }); await new Promise(resolve => setImmediate(resolve)); @@ -579,38 +824,8 @@ it('keeps the session running when encoder setup for the preview fails', async ( await host.finishAsync(); }); -it('adds the iOS preview token to the host secrets once serve-sim is ready', async () => { - jest - .mocked(fetchWebPreviewTurnArgsAsync) - .mockResolvedValueOnce(['--turn-credential', 'turn-secret']); - let secretsAtSpawn: string[] | undefined; - jest.mocked(spawnDetached).mockImplementationOnce(options => { - secretsAtSpawn = [...(options.secrets ?? [])]; - return { - pid: undefined, - getOutput: () => '', - getExitError: () => undefined, - stopAsync: stopServer, - }; - }); - const host = await startDeviceSessionHostAsync(ctx, { - runtimePlatform: BuildRuntimePlatform.DARWIN, - env, - logger, - timeoutMs: 10_000, - }); - expect(secretsAtSpawn).toEqual(['turn-secret']); - // Host output is redacted against this array, so the token must land in the one passed at spawn. - expect(jest.mocked(spawnDetached).mock.calls[0][0].secrets).toEqual([ - 'turn-secret', - 'preview-token', - ]); - expect((await host.openPreviewAsync({ baseDomain })).previewToken).toBe('preview-token'); - await host.finishAsync(); -}); - it('uses the existing step logger for startup and shutdown output', async () => { - const host = await startHostAsync(); + const host = await startHostAsync(false); const processLogger = jest.mocked(spawnDetached).mock.calls[0][0].logger; expect(processLogger).toBe(logger); stopServer.mockImplementationOnce(async () => processLogger?.info('last host output')); diff --git a/packages/build-tools/src/steps/utils/deviceSessionHost.ts b/packages/build-tools/src/steps/utils/deviceSessionHost.ts index 145a89c6a2..b1a0ff591d 100644 --- a/packages/build-tools/src/steps/utils/deviceSessionHost.ts +++ b/packages/build-tools/src/steps/utils/deviceSessionHost.ts @@ -1,4 +1,4 @@ -import { SystemError, UserError } from '@expo/eas-build-job'; +import { BuildPhaseResult, SystemError, UserError } from '@expo/eas-build-job'; import type { bunyan } from '@expo/logger'; import { BuildRuntimePlatform, type BuildStepEnv } from '@expo/steps'; import { randomBytes } from 'node:crypto'; @@ -45,6 +45,7 @@ import { SERVE_SIM_STOP_GRACE_PERIOD_MS, } from './IosSimulatorRecordingUtils'; import { SERVE_SIM_STATE_DIR, readServeSimServersAsync } from './serveSimMetricsRecorder'; +import { startLogPhase } from '../../utils/logPhase'; const WEB_PREVIEW_HOST = '127.0.0.1'; const SERVE_SIM_PACKAGE_NAME = '@expo/serve-sim'; @@ -273,7 +274,43 @@ type AndroidSessionRecording = { env: BuildStepEnv; }; +type DeviceSessionHostOptions = { + runtimePlatform: BuildRuntimePlatform; + env: BuildStepEnv; + logger: bunyan; + timeoutMs: number; + separateLogPhase?: boolean; + packageVersion?: string; + networkCapture?: boolean; + networkCaptureFields?: string[]; +} & ServeSimLaunchOptions; + export async function startDeviceSessionHostAsync( + ctx: CustomBuildContext, + options: DeviceSessionHostOptions +): Promise { + if (!options.separateLogPhase) { + return await startDeviceSessionHostInternalAsync(ctx, options, () => {}, options.logger); + } + const phase = startLogPhase(options.logger, 'Simulator preview'); + let ready = false; + try { + const host = await startDeviceSessionHostInternalAsync( + ctx, + { ...options, logger: phase.logger }, + successful => + phase.end(ready && successful ? BuildPhaseResult.SUCCESS : BuildPhaseResult.FAIL), + options.logger + ); + ready = true; + return host; + } catch (error) { + phase.end(BuildPhaseResult.FAIL); + throw error; + } +} + +async function startDeviceSessionHostInternalAsync( ctx: CustomBuildContext, { runtimePlatform, @@ -286,15 +323,9 @@ export async function startDeviceSessionHostAsync( openUrl, networkCapture = false, networkCaptureFields = [], - }: { - runtimePlatform: BuildRuntimePlatform; - env: BuildStepEnv; - logger: bunyan; - timeoutMs: number; - packageVersion?: string; - networkCapture?: boolean; - networkCaptureFields?: string[]; - } & ServeSimLaunchOptions + }: DeviceSessionHostOptions, + onFinished: (successful: boolean) => void, + artifactLogger: bunyan ): Promise { const isAndroid = runtimePlatform === BuildRuntimePlatform.LINUX; // Unreachable from the step functions, which reject a non-Darwin launch while parsing. @@ -362,7 +393,7 @@ export async function startDeviceSessionHostAsync( ]; const screenshots = await startDeviceRunSessionScreenshotsAsync(ctx, { deviceRunSessionId: getDeviceRunSessionIdOrThrow(env), - logger, + logger: artifactLogger, }); let previewServer: DetachedProcessHandle; try { @@ -392,6 +423,7 @@ export async function startDeviceSessionHostAsync( let finishTask: Promise | null = null; let hostReady = false; let sessionPreview: ReturnType | null = null; + let previewFailed = false; const host: DeviceSessionHost = { openPreviewAsync({ baseDomain }) { @@ -420,6 +452,7 @@ export async function startDeviceSessionHostAsync( throw new SystemError('Session host finalized while the preview was opening.'); } let closeTask: Promise | null = null; + previewFailed = false; return { previewPageUrl, apiUrl: tunnel.url, @@ -440,6 +473,7 @@ export async function startDeviceSessionHostAsync( previewTask = opening; // A failed tunnel may be retried without restarting capture. void opening.catch(() => { + previewFailed = true; if (previewTask === opening) { previewTask = null; } @@ -457,6 +491,11 @@ export async function startDeviceSessionHostAsync( // A host that never answered /readyz has nothing to finalize or upload. recording: hostReady ? recording : null, logger, + artifactLogger, + onStopped: successful => onFinished(successful && !previewFailed), + }).catch(err => { + artifactLogger.warn({ err }, 'Could not finish the simulator preview.'); + onFinished(false); })); }, }; @@ -523,6 +562,8 @@ async function finishDeviceSessionHostAsync( port, recording, logger, + artifactLogger, + onStopped, }: { previewTask: Promise | null; stopSessionPreviewAsync: () => Promise; @@ -532,10 +573,13 @@ async function finishDeviceSessionHostAsync( port: number; recording: AndroidSessionRecording | null; logger: bunyan; + artifactLogger: bunyan; + onStopped: (successful: boolean) => void; } ): Promise { // Stop capturing before the host stops, so the last thumbnail shows the session, not shutdown. await stopSessionPreviewAsync(); + const hostExited = previewServer.getExitError() !== undefined; // Native ngrok operations have no scoped cancellation. Retire a late listener too. const retirePreview = withDeviceRunSessionTimeoutAsync( { name: 'Preview tunnel retirement', timeoutMs: 5_000 }, @@ -544,9 +588,13 @@ async function finishDeviceSessionHostAsync( const preview = await previewTask?.catch(() => null); await preview?.closeAsync(); } - ).catch(err => { - logger.warn({ err }, `Could not close the ${serverName} preview tunnel within its deadline.`); - }); + ).then( + () => true, + err => { + logger.warn({ err }, `Could not close the ${serverName} preview tunnel within its deadline.`); + return false; + } + ); let finalization: AndroidRecordingFinalization | null = null; if (recording) { // stopAsync signals the whole process group, including capture's encoder. @@ -567,17 +615,21 @@ async function finishDeviceSessionHostAsync( } catch (err) { logger.warn({ err }, `Could not stop the ${serverName} session host.`); } - await retirePreview; + const previewRetired = await retirePreview; + onStopped(hostStopped && !hostExited && previewRetired); await screenshots.finishAsync(hostStopped); // A Hub that never recorded has logged its reason and left nothing to upload. const captured = finalization !== 'not-recording'; let uploaded = false; if (recording && captured && hostStopped) { - uploaded = await uploadFinishedAndroidRecordingAsync(ctx, { recording, logger }); + uploaded = await uploadFinishedAndroidRecordingAsync(ctx, { + recording, + logger: artifactLogger, + }); } if (recording && captured && (finalization === 'failed' || !uploaded)) { // The Hub reports capture failures on stderr; the stop route answers with only a summary. - logger.warn( + artifactLogger.warn( { hostOutput: previewServer.getOutput().slice(-HOST_OUTPUT_TAIL_CHARS) || '' }, 'Session host output around the recording failure.' ); diff --git a/packages/build-tools/src/utils/__tests__/logPhase.test.ts b/packages/build-tools/src/utils/__tests__/logPhase.test.ts index 7cee93c8f9..2869ffffe6 100644 --- a/packages/build-tools/src/utils/__tests__/logPhase.test.ts +++ b/packages/build-tools/src/utils/__tests__/logPhase.test.ts @@ -1,6 +1,6 @@ import { BuildPhase, BuildPhaseResult, LogMarker } from '@expo/eas-build-job'; -import { withLogPhaseAsync } from '../logPhase'; +import { startLogPhase, withLogPhaseAsync } from '../logPhase'; describe(withLogPhaseAsync.name, () => { const phaseLogger = { info: jest.fn() }; @@ -49,3 +49,30 @@ describe(withLogPhaseAsync.name, () => { ); }); }); + +it('keeps concurrent phases separate and ends each only once', () => { + const first = { info: jest.fn() }; + const second = { info: jest.fn() }; + const logger = { child: jest.fn().mockReturnValueOnce(first).mockReturnValueOnce(second) }; + const tools = startLogPhase(logger as any, 'Argent session'); + const preview = startLogPhase(logger as any, 'Simulator preview'); + expect(logger.child.mock.calls[0][0].buildStepId).not.toBe( + logger.child.mock.calls[1][0].buildStepId + ); + tools.logger.info('tool output'); + preview.logger.info('preview output'); + preview.end(BuildPhaseResult.FAIL); + preview.end(BuildPhaseResult.SUCCESS); + expect( + second.info.mock.calls.filter(([fields]) => fields.marker === LogMarker.END_PHASE) + ).toHaveLength(1); + expect(first.info).not.toHaveBeenCalledWith( + expect.objectContaining({ marker: LogMarker.END_PHASE }), + expect.anything() + ); + tools.end(BuildPhaseResult.SUCCESS); + expect(first.info).toHaveBeenLastCalledWith( + { marker: LogMarker.END_PHASE, result: BuildPhaseResult.SUCCESS }, + 'End phase: Argent session' + ); +}); diff --git a/packages/build-tools/src/utils/logPhase.ts b/packages/build-tools/src/utils/logPhase.ts index f79bb5133e..75d54158a7 100644 --- a/packages/build-tools/src/utils/logPhase.ts +++ b/packages/build-tools/src/utils/logPhase.ts @@ -2,29 +2,38 @@ import { BuildPhase, BuildPhaseResult, LogMarker } from '@expo/eas-build-job'; import { type bunyan } from '@expo/logger'; import { BuildStep } from '@expo/steps'; -export async function withLogPhaseAsync( - logger: bunyan, - name: string, - fn: (logger: bunyan) => Promise -): Promise { +export function startLogPhase(logger: bunyan, name: string) { const phaseLogger = logger.child({ phase: BuildPhase.CUSTOM, buildStepId: BuildStep.getNewId(), buildStepDisplayName: name, }); phaseLogger.info({ marker: LogMarker.START_PHASE }, `Start phase: ${name}`); + let ended = false; + return { + logger: phaseLogger, + end(result: BuildPhaseResult): void { + if (ended) { + return; + } + ended = true; + phaseLogger.info({ marker: LogMarker.END_PHASE, result }, `End phase: ${name}`); + }, + }; +} + +export async function withLogPhaseAsync( + logger: bunyan, + name: string, + fn: (logger: bunyan) => Promise +): Promise { + const phase = startLogPhase(logger, name); try { - const result = await fn(phaseLogger); - phaseLogger.info( - { marker: LogMarker.END_PHASE, result: BuildPhaseResult.SUCCESS }, - `End phase: ${name}` - ); + const result = await fn(phase.logger); + phase.end(BuildPhaseResult.SUCCESS); return result; } catch (error) { - phaseLogger.info( - { marker: LogMarker.END_PHASE, result: BuildPhaseResult.FAIL }, - `End phase: ${name}` - ); + phase.end(BuildPhaseResult.FAIL); throw error; } } From af8874cf95688a438405260ff684dd8db44fbc68 Mon Sep 17 00:00:00 2001 From: Gabe Debes Date: Fri, 9 Oct 2026 16:12:26 -0700 Subject: [PATCH 2/5] [build-tools] Link agent-device changelog entry Co-Authored-By: GPT-6.1 Sol --- .changeset/serve-sim-pr-4585.md | 5 +++++ CHANGELOG.md | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 .changeset/serve-sim-pr-4585.md diff --git a/.changeset/serve-sim-pr-4585.md b/.changeset/serve-sim-pr-4585.md new file mode 100644 index 0000000000..e8f56b33a9 --- /dev/null +++ b/.changeset/serve-sim-pr-4585.md @@ -0,0 +1,5 @@ +--- +'@expo/build-tools': minor +--- + +Give agent-device and its simulator preview separate session logs. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4944037fd1..d0361b9213 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,6 @@ This is the log of notable changes to EAS CLI and related packages. ### 🎉 New features -- [build-tools] Give agent-device and its simulator preview separate session logs. - [build-tools] Let the MCP server read text and image files from sandboxes. ([#4555](https://github.com/expo/eas-cli/pull/4555) by [@AHGIJMKLKKZNPJKQR](https://github.com/AHGIJMKLKKZNPJKQR)) - [eas-cli][local-build-plugin] Deprecate Node.js 20, and print a warning when running on it. A future release will require Node.js 22 or newer. Development and CI now use Node.js 22. ([#4579](https://github.com/expo/eas-cli/pull/4579) by [@douglowder](https://github.com/douglowder)) - [build-tools] Add `eas/run_agent`, a step that runs Claude Code for an agent run and connects it to the Expo MCP server. ([#4552](https://github.com/expo/eas-cli/pull/4552) by [@hSATAC](https://github.com/hSATAC)) From 594435a62b20dd9b4530ef02dcf3ce93a2124b71 Mon Sep 17 00:00:00 2001 From: Gabe Debes Date: Wed, 14 Oct 2026 21:34:29 -0700 Subject: [PATCH 3/5] [build-tools] Drain preview encoder setup before ending logs --- .../deviceRunSessionPreview.process.test.ts | 137 ++++++++++++++++++ .../__tests__/deviceRunSessionPreview.test.ts | 102 ++++++++++++- .../utils/__tests__/deviceSessionHost.test.ts | 70 ++++++--- .../steps/utils/deviceRunSessionPreview.ts | 42 +++++- .../src/steps/utils/deviceSessionHost.ts | 20 ++- 5 files changed, 337 insertions(+), 34 deletions(-) create mode 100644 packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts new file mode 100644 index 0000000000..6c3fe3140a --- /dev/null +++ b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts @@ -0,0 +1,137 @@ +import * as turtleSpawn from '@expo/turtle-spawn'; +import { execFileSync } from 'node:child_process'; +import { getEventListeners } from 'node:events'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { ensureMacosPreviewEncoderInstalledAsync } from '../deviceRunSessionPreview'; + +jest.unmock('node:fs'); +jest.unmock('node:fs/promises'); +jest.unmock('@expo/turtle-spawn'); +jest.unmock('@expo/logger'); + +const sleep = (ms: number): Promise => new Promise(resolve => setTimeout(resolve, ms)); + +async function waitUntil(predicate: () => boolean): Promise { + const deadline = Date.now() + 3_000; + while (!predicate()) { + if (Date.now() >= deadline) { + throw new Error('Encoder subprocess fixture did not reach the expected state'); + } + await sleep(20); + } +} + +function isRunning(pid: number): boolean { + try { + const state = execFileSync('ps', ['-p', String(pid), '-o', 'stat=']) + .toString() + .trim(); + return state.length > 0 && !state.startsWith('Z'); + } catch { + return false; + } +} + +it('aborts the installer group and drains inherited output before setup settles', async () => { + const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'preview-encoder-abort-')); + const controller = new AbortController(); + const logger = { child: () => logger, info: jest.fn() }; + const pidFiles = ['parent', 'child', 'grandchild'].map(role => path.join(root, `${role}.pid`)); + const spawn = jest.spyOn(turtleSpawn, 'default'); + let preparation: Promise | undefined; + let deadline: NodeJS.Timeout | undefined; + try { + await fs.promises.writeFile( + path.join(root, 'cwebp'), + `#!${process.execPath} +process.exit(1); +`, + { + mode: 0o700, + } + ); + await fs.promises.writeFile( + path.join(root, 'brew'), + `#!${process.execPath} +const fs = require('node:fs'); +const path = require('node:path'); +const {spawn} = require('node:child_process'); +const role = process.argv[2] === 'install' ? 'parent' : process.argv[2]; +const pidFile = path.join(__dirname, role + '.pid'); +fs.writeFileSync(pidFile + '.tmp', String(process.pid)); +fs.renameSync(pidFile + '.tmp', pidFile); +if (role !== 'grandchild') { + spawn(process.execPath, [__filename, role === 'parent' ? 'child' : 'grandchild'], {stdio: 'inherit'}); +} +process.stdout.write(role + '-ready\\n'); +setInterval(() => {}, 1000); +`, + { mode: 0o700 } + ); + preparation = ensureMacosPreviewEncoderInstalledAsync({ + env: { PATH: root }, + logger: logger as never, + signal: controller.signal, + }).then( + () => ({ ok: true }), + error => ({ error }) + ); + await waitUntil(() => pidFiles.every(file => fs.existsSync(file))); + await expect(spawn.mock.results[0].value).rejects.toMatchObject({ status: 1 }); + const pids = pidFiles.map(file => Number(fs.readFileSync(file, 'utf8'))); + expect(pids.every(isRunning)).toBe(true); + const installer = spawn.mock.results[1].value as ReturnType; + let closed = false; + let drained = false; + installer.child.once('close', () => { + closed = true; + }); + installer.child.stdout!.once('end', () => { + drained = true; + }); + controller.abort(new Error('preview phase ended')); + const result = await Promise.race([ + preparation, + new Promise((_, reject) => { + deadline = setTimeout( + () => reject(new Error('Encoder cancellation did not close inherited output pipes')), + 3_000 + ); + }), + ]); + expect(result).toEqual({ error: expect.objectContaining({ signal: 'SIGKILL' }) }); + expect(closed).toBe(true); + expect(drained).toBe(true); + await waitUntil(() => pids.every(pid => !isRunning(pid))); + expect(getEventListeners(controller.signal, 'abort')).toEqual([]); + expect(spawn.mock.calls.map(([command]) => command)).toEqual(['cwebp', 'brew']); + expect(spawn.mock.calls.every(([, , options]) => options?.detached && !options.signal)).toBe( + true + ); + } finally { + clearTimeout(deadline); + controller.abort(); + for (const file of pidFiles) { + if (fs.existsSync(file)) { + const pid = Number(fs.readFileSync(file, 'utf8')); + if (!Number.isSafeInteger(pid) || pid <= 0) { + continue; + } + try { + process.kill(-pid, 'SIGKILL'); + } catch {} + if (isRunning(pid)) { + try { + process.kill(pid, 'SIGKILL'); + } catch {} + } + } + } + await preparation; + spawn.mockRestore(); + await fs.promises.rm(root, { recursive: true, force: true }); + } +}, 10_000); diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.test.ts index 9e4a713ea0..f7e2011adf 100644 --- a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.test.ts @@ -2,12 +2,14 @@ import { type bunyan } from '@expo/logger'; import { BuildRuntimePlatform } from '@expo/steps'; import spawn from '@expo/turtle-spawn'; import { Client, fetchExchange } from '@urql/core'; +import { getEventListeners } from 'node:events'; import { readFile, readdir, writeFile } from 'node:fs/promises'; import { Readable } from 'node:stream'; import os from 'node:os'; import fetch from 'node-fetch'; import { type CustomBuildContext } from '../../../customBuildContext'; +import * as processes from '../../../utils/processes'; import { captureDeviceRunSessionPreviewAsync, ensureMacosPreviewEncoderInstalledAsync, @@ -383,18 +385,106 @@ describe(ensureMacosPreviewEncoderInstalledAsync, () => { [ 'brew', ['install', 'webp'], - { env: { HOME: '/Users/expo', HOMEBREW_NO_AUTO_UPDATE: '1' }, logger }, + { + env: { HOME: '/Users/expo', HOMEBREW_NO_AUTO_UPDATE: '1' }, + logger, + detached: true, + }, ], ]); } } ); - it('rejects when webp cannot be installed', async () => { + it('does not start a command when setup was already canceled', async () => { jest.mocked(spawn).mockReset(); - jest.mocked(spawn).mockRejectedValue(new Error('brew failed')); - await expect(ensureMacosPreviewEncoderInstalledAsync({ env: {}, logger })).rejects.toThrow( - 'brew failed' - ); + const controller = new AbortController(); + const reason = new Error('session ended'); + controller.abort(reason); + await expect( + ensureMacosPreviewEncoderInstalledAsync({ env: {}, logger, signal: controller.signal }) + ).rejects.toBe(reason); + expect(spawn).not.toHaveBeenCalled(); + expect(getEventListeners(controller.signal, 'abort')).toEqual([]); + }); + + it.each(['cwebp', 'brew'])( + 'waits for canceled %s to close and never starts another command', + async command => { + jest.mocked(spawn).mockReset(); + const controller = new AbortController(); + const child = { pid: 12345, kill: jest.fn() }; + const killed = jest.spyOn(processes, 'killProcessGroup').mockImplementation(() => {}); + const closeError = new Error(`${command} closed after cancellation`); + let entered!: () => void; + const commandEntered = new Promise(resolve => { + entered = resolve; + }); + let close!: (error: Error) => void; + jest.mocked(spawn).mockImplementation(current => { + if (current !== command) { + return Promise.reject(new Error('cwebp missing')) as ReturnType; + } + entered(); + return Object.assign( + new Promise((_, reject) => { + close = reject; + }), + { child } + ) as unknown as ReturnType; + }); + let settled = false; + const result = ensureMacosPreviewEncoderInstalledAsync({ + env: {}, + logger, + signal: controller.signal, + }).then( + () => { + settled = true; + }, + error => { + settled = true; + return error; + } + ); + try { + await commandEntered; + controller.abort(new Error('session ended')); + await Promise.resolve(); + expect(killed).toHaveBeenCalledWith(child, 'SIGKILL'); + expect(settled).toBe(false); + expect(jest.mocked(spawn).mock.calls.at(-1)).toEqual([ + command, + command === 'cwebp' ? ['-version'] : ['install', 'webp'], + expect.objectContaining({ detached: true }), + ]); + expect(jest.mocked(spawn).mock.calls.every(([, , options]) => !options?.signal)).toBe(true); + } finally { + close(closeError); + await result; + killed.mockRestore(); + } + expect(spawn).toHaveBeenCalledTimes(command === 'cwebp' ? 1 : 2); + expect(getEventListeners(controller.signal, 'abort')).toEqual([]); + } + ); + + it('rejects an installation failure and removes its abort listeners', async () => { + jest.mocked(spawn).mockReset(); + const controller = new AbortController(); + const failure = new Error('brew failed'); + jest.mocked(spawn).mockRejectedValue(failure); + await expect( + ensureMacosPreviewEncoderInstalledAsync({ env: {}, logger, signal: controller.signal }) + ).rejects.toBe(failure); + expect(getEventListeners(controller.signal, 'abort')).toEqual([]); + }); + + it('removes its abort listener after a successful probe', async () => { + jest.mocked(spawn).mockReset(); + jest.mocked(spawn).mockResolvedValue({ stdout: '', stderr: '' } as never); + const controller = new AbortController(); + await ensureMacosPreviewEncoderInstalledAsync({ env: {}, logger, signal: controller.signal }); + expect(getEventListeners(controller.signal, 'abort')).toEqual([]); }); }); diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts index 26e9abdce3..7eb2d7c5ee 100644 --- a/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/deviceSessionHost.test.ts @@ -53,10 +53,12 @@ jest.mock('../deviceRunSessionScreenRecordings', () => ({ function deferred() { let resolve!: (value: T) => void; - const promise = new Promise(r => { + let reject!: (reason: Error) => void; + const promise = new Promise((r, j) => { resolve = r; + reject = j; }); - return { promise, resolve }; + return { promise, resolve, reject }; } const env = { @@ -787,22 +789,54 @@ it.each([BuildRuntimePlatform.LINUX, BuildRuntimePlatform.DARWIN])( } ); -it('does not delay macOS readiness on encoder setup or start the preview after finish', async () => { - const install = deferred(); - jest.mocked(ensureMacosPreviewEncoderInstalledAsync).mockReturnValueOnce(install.promise); - const host = await startDeviceSessionHostAsync(ctx, { - runtimePlatform: BuildRuntimePlatform.DARWIN, - env, - logger, - separateLogPhase: true, - timeoutMs: 10_000, - }); - await host.finishAsync(); - install.resolve(); - await new Promise(resolve => setImmediate(resolve)); - expect(startDeviceRunSessionPreview).not.toHaveBeenCalled(); - expect(stopServer).toHaveBeenCalledTimes(1); -}); +it.each(['success', 'failure'])( + 'keeps macOS readiness nonblocking and drains encoder setup %s before ending the phase', + async result => { + const install = deferred(); + jest.mocked(ensureMacosPreviewEncoderInstalledAsync).mockReturnValueOnce(install.promise); + const host = await startDeviceSessionHostAsync(ctx, { + runtimePlatform: BuildRuntimePlatform.DARWIN, + env, + logger, + separateLogPhase: true, + timeoutMs: 10_000, + }); + const setupLogger = jest.mocked(ensureMacosPreviewEncoderInstalledAsync).mock.calls[0][0] + .logger; + const finishing = host.finishAsync(); + expect(host.finishAsync()).toBe(finishing); + try { + await new Promise(resolve => setImmediate(resolve)); + expect(logger.info).not.toHaveBeenCalledWith( + expect.objectContaining({ marker: LogMarker.END_PHASE }), + expect.any(String) + ); + expect( + jest.mocked(ensureMacosPreviewEncoderInstalledAsync).mock.calls[0][0].signal?.aborted + ).toBe(true); + setupLogger.info('last encoder output'); + if (result === 'failure') { + install.reject(new Error('encoder stopped')); + } else { + install.resolve(); + } + await finishing; + const calls = jest.mocked(logger.info).mock.calls; + expect(calls.findIndex(([message]) => message === 'last encoder output')).toBeLessThan( + calls.findIndex(([record]) => record?.marker === LogMarker.END_PHASE) + ); + expect(logger.warn).not.toHaveBeenCalledWith( + expect.anything(), + 'Could not start refreshing the session preview.' + ); + expect(startDeviceRunSessionPreview).not.toHaveBeenCalled(); + expect(stopServer).toHaveBeenCalledTimes(1); + } finally { + install.resolve(); + await finishing; + } + } +); it('keeps the session running when encoder setup for the preview fails', async () => { jest diff --git a/packages/build-tools/src/steps/utils/deviceRunSessionPreview.ts b/packages/build-tools/src/steps/utils/deviceRunSessionPreview.ts index 9a8bb1c123..9cbf1eb527 100644 --- a/packages/build-tools/src/steps/utils/deviceRunSessionPreview.ts +++ b/packages/build-tools/src/steps/utils/deviceRunSessionPreview.ts @@ -1,7 +1,7 @@ import { type bunyan } from '@expo/logger'; import { asyncResult } from '@expo/results'; import { BuildRuntimePlatform, type BuildStepEnv } from '@expo/steps'; -import spawn from '@expo/turtle-spawn'; +import spawn, { type SpawnOptions } from '@expo/turtle-spawn'; import { graphql } from 'gql.tada'; import fetch from 'node-fetch'; import { createWriteStream } from 'node:fs'; @@ -11,6 +11,7 @@ import os from 'node:os'; import path from 'node:path'; import { type CustomBuildContext } from '../../customBuildContext'; +import { killProcessGroup } from '../../utils/processes'; const PREVIEW_INTERVAL_MS = 60_000; const PREVIEW_MAX_DIMENSION = 320; @@ -146,18 +147,47 @@ export function startDeviceRunSessionPreview({ export async function ensureMacosPreviewEncoderInstalledAsync({ env, logger, + signal, }: { env: BuildStepEnv; logger: bunyan; + signal?: AbortSignal; }): Promise { - if ((await asyncResult(spawn('cwebp', ['-version'], { env, stdio: 'pipe' }))).ok) { + const probe = await asyncResult( + runEncoderCommandAsync('cwebp', ['-version'], { env, stdio: 'pipe' }, signal) + ); + if (probe.ok) { return; } + signal?.throwIfAborted(); logger.info('Installing webp with Homebrew for the session preview.'); - await spawn('brew', ['install', 'webp'], { - env: { ...env, HOMEBREW_NO_AUTO_UPDATE: '1' }, - logger, - }); + await runEncoderCommandAsync( + 'brew', + ['install', 'webp'], + { env: { ...env, HOMEBREW_NO_AUTO_UPDATE: '1' }, logger }, + signal + ); +} + +async function runEncoderCommandAsync( + command: string, + args: string[], + options: SpawnOptions, + signal?: AbortSignal +): Promise { + signal?.throwIfAborted(); + const task = spawn(command, args, { ...options, detached: true }); + const abort = (): void => killProcessGroup(task.child, 'SIGKILL'); + signal?.addEventListener('abort', abort, { once: true }); + if (signal?.aborted) { + abort(); + } + try { + await task; + signal?.throwIfAborted(); + } finally { + signal?.removeEventListener('abort', abort); + } } export async function captureDeviceRunSessionPreviewAsync({ diff --git a/packages/build-tools/src/steps/utils/deviceSessionHost.ts b/packages/build-tools/src/steps/utils/deviceSessionHost.ts index b1a0ff591d..60a0d1155c 100644 --- a/packages/build-tools/src/steps/utils/deviceSessionHost.ts +++ b/packages/build-tools/src/steps/utils/deviceSessionHost.ts @@ -423,6 +423,8 @@ async function startDeviceSessionHostInternalAsync( let finishTask: Promise | null = null; let hostReady = false; let sessionPreview: ReturnType | null = null; + const previewSetupController = new AbortController(); + let previewSetupTask: Promise | undefined; let previewFailed = false; const host: DeviceSessionHost = { @@ -483,7 +485,11 @@ async function startDeviceSessionHostInternalAsync( finishAsync() { return (finishTask ??= finishDeviceSessionHostAsync(ctx, { previewTask, - stopSessionPreviewAsync: async () => await sessionPreview?.stopAsync(), + stopSessionPreviewAsync: async () => { + previewSetupController.abort(); + await previewSetupTask; + await sessionPreview?.stopAsync(); + }, previewServer, screenshots, serverName, @@ -523,9 +529,13 @@ async function startDeviceSessionHostInternalAsync( } // Android installed FFmpeg before launching the host. The optional thumbnail must not delay // readiness on macOS, so it starts after its encoder is installed there. - void (async () => { + previewSetupTask = (async () => { if (!isAndroid) { - await ensureMacosPreviewEncoderInstalledAsync({ env, logger }); + await ensureMacosPreviewEncoderInstalledAsync({ + env, + logger, + signal: previewSetupController.signal, + }); } if (!finishTask) { sessionPreview = startDeviceRunSessionPreview({ @@ -542,7 +552,9 @@ async function startDeviceSessionHostInternalAsync( }); } })().catch(err => { - logger.warn({ err }, 'Could not start refreshing the session preview.'); + if (!previewSetupController.signal.aborted) { + logger.warn({ err }, 'Could not start refreshing the session preview.'); + } }); return host; } catch (error) { From b6073728337408f525db5802c76680cfeda907f4 Mon Sep 17 00:00:00 2001 From: Gabe Debes Date: Wed, 14 Oct 2026 21:52:06 -0700 Subject: [PATCH 4/5] [build-tools] Move preview encoder scripts into test fixtures --- .../deviceRunSessionPreview.process.test.ts | 38 +++++-------------- .../fixtures/preview-encoder/brew.cjs | 18 +++++++++ .../fixtures/preview-encoder/cwebp.cjs | 2 + 3 files changed, 30 insertions(+), 28 deletions(-) create mode 100644 packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs create mode 100644 packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts index 6c3fe3140a..cff763a827 100644 --- a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts @@ -35,7 +35,7 @@ function isRunning(pid: number): boolean { } } -it('aborts the installer group and drains inherited output before setup settles', async () => { +it('stops the installer and its children before finishing encoder setup', async () => { const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'preview-encoder-abort-')); const controller = new AbortController(); const logger = { child: () => logger, info: jest.fn() }; @@ -44,33 +44,15 @@ it('aborts the installer group and drains inherited output before setup settles' let preparation: Promise | undefined; let deadline: NodeJS.Timeout | undefined; try { - await fs.promises.writeFile( - path.join(root, 'cwebp'), - `#!${process.execPath} -process.exit(1); -`, - { - mode: 0o700, - } - ); - await fs.promises.writeFile( - path.join(root, 'brew'), - `#!${process.execPath} -const fs = require('node:fs'); -const path = require('node:path'); -const {spawn} = require('node:child_process'); -const role = process.argv[2] === 'install' ? 'parent' : process.argv[2]; -const pidFile = path.join(__dirname, role + '.pid'); -fs.writeFileSync(pidFile + '.tmp', String(process.pid)); -fs.renameSync(pidFile + '.tmp', pidFile); -if (role !== 'grandchild') { - spawn(process.execPath, [__filename, role === 'parent' ? 'child' : 'grandchild'], {stdio: 'inherit'}); -} -process.stdout.write(role + '-ready\\n'); -setInterval(() => {}, 1000); -`, - { mode: 0o700 } - ); + await fs.promises.symlink(process.execPath, path.join(root, 'node')); + for (const command of ['cwebp', 'brew']) { + const target = path.join(root, command); + await fs.promises.copyFile( + path.join(__dirname, 'fixtures', 'preview-encoder', `${command}.cjs`), + target + ); + await fs.promises.chmod(target, 0o700); + } preparation = ensureMacosPreviewEncoderInstalledAsync({ env: { PATH: root }, logger: logger as never, diff --git a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs new file mode 100644 index 0000000000..390ae7bc52 --- /dev/null +++ b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs @@ -0,0 +1,18 @@ +#!/usr/bin/env node +const { spawn } = require('node:child_process'); +const fs = require('node:fs'); +const path = require('node:path'); + +const role = process.argv[2] === 'install' ? 'parent' : process.argv[2]; +const pidFile = path.join(__dirname, `${role}.pid`); +fs.writeFileSync(`${pidFile}.tmp`, String(process.pid)); +fs.renameSync(`${pidFile}.tmp`, pidFile); + +if (role !== 'grandchild') { + spawn(process.execPath, [__filename, role === 'parent' ? 'child' : 'grandchild'], { + stdio: 'inherit', + }); +} + +process.stdout.write(`${role}-ready\n`); +setInterval(() => {}, 1000); diff --git a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs new file mode 100644 index 0000000000..a88ba1a6bb --- /dev/null +++ b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs @@ -0,0 +1,2 @@ +#!/usr/bin/env node +process.exit(1); From 3e8173f9acdcf791ffcd58cca15df97fb3416ce5 Mon Sep 17 00:00:00 2001 From: Gabe Debes Date: Wed, 14 Oct 2026 21:59:04 -0700 Subject: [PATCH 5/5] [build-tools] Keep encoder shutdown tests focused --- .../deviceRunSessionPreview.process.test.ts | 119 ------------------ .../fixtures/preview-encoder/brew.cjs | 18 --- .../fixtures/preview-encoder/cwebp.cjs | 2 - 3 files changed, 139 deletions(-) delete mode 100644 packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts delete mode 100644 packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs delete mode 100644 packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs diff --git a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts b/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts deleted file mode 100644 index cff763a827..0000000000 --- a/packages/build-tools/src/steps/utils/__tests__/deviceRunSessionPreview.process.test.ts +++ /dev/null @@ -1,119 +0,0 @@ -import * as turtleSpawn from '@expo/turtle-spawn'; -import { execFileSync } from 'node:child_process'; -import { getEventListeners } from 'node:events'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; - -import { ensureMacosPreviewEncoderInstalledAsync } from '../deviceRunSessionPreview'; - -jest.unmock('node:fs'); -jest.unmock('node:fs/promises'); -jest.unmock('@expo/turtle-spawn'); -jest.unmock('@expo/logger'); - -const sleep = (ms: number): Promise => new Promise(resolve => setTimeout(resolve, ms)); - -async function waitUntil(predicate: () => boolean): Promise { - const deadline = Date.now() + 3_000; - while (!predicate()) { - if (Date.now() >= deadline) { - throw new Error('Encoder subprocess fixture did not reach the expected state'); - } - await sleep(20); - } -} - -function isRunning(pid: number): boolean { - try { - const state = execFileSync('ps', ['-p', String(pid), '-o', 'stat=']) - .toString() - .trim(); - return state.length > 0 && !state.startsWith('Z'); - } catch { - return false; - } -} - -it('stops the installer and its children before finishing encoder setup', async () => { - const root = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'preview-encoder-abort-')); - const controller = new AbortController(); - const logger = { child: () => logger, info: jest.fn() }; - const pidFiles = ['parent', 'child', 'grandchild'].map(role => path.join(root, `${role}.pid`)); - const spawn = jest.spyOn(turtleSpawn, 'default'); - let preparation: Promise | undefined; - let deadline: NodeJS.Timeout | undefined; - try { - await fs.promises.symlink(process.execPath, path.join(root, 'node')); - for (const command of ['cwebp', 'brew']) { - const target = path.join(root, command); - await fs.promises.copyFile( - path.join(__dirname, 'fixtures', 'preview-encoder', `${command}.cjs`), - target - ); - await fs.promises.chmod(target, 0o700); - } - preparation = ensureMacosPreviewEncoderInstalledAsync({ - env: { PATH: root }, - logger: logger as never, - signal: controller.signal, - }).then( - () => ({ ok: true }), - error => ({ error }) - ); - await waitUntil(() => pidFiles.every(file => fs.existsSync(file))); - await expect(spawn.mock.results[0].value).rejects.toMatchObject({ status: 1 }); - const pids = pidFiles.map(file => Number(fs.readFileSync(file, 'utf8'))); - expect(pids.every(isRunning)).toBe(true); - const installer = spawn.mock.results[1].value as ReturnType; - let closed = false; - let drained = false; - installer.child.once('close', () => { - closed = true; - }); - installer.child.stdout!.once('end', () => { - drained = true; - }); - controller.abort(new Error('preview phase ended')); - const result = await Promise.race([ - preparation, - new Promise((_, reject) => { - deadline = setTimeout( - () => reject(new Error('Encoder cancellation did not close inherited output pipes')), - 3_000 - ); - }), - ]); - expect(result).toEqual({ error: expect.objectContaining({ signal: 'SIGKILL' }) }); - expect(closed).toBe(true); - expect(drained).toBe(true); - await waitUntil(() => pids.every(pid => !isRunning(pid))); - expect(getEventListeners(controller.signal, 'abort')).toEqual([]); - expect(spawn.mock.calls.map(([command]) => command)).toEqual(['cwebp', 'brew']); - expect(spawn.mock.calls.every(([, , options]) => options?.detached && !options.signal)).toBe( - true - ); - } finally { - clearTimeout(deadline); - controller.abort(); - for (const file of pidFiles) { - if (fs.existsSync(file)) { - const pid = Number(fs.readFileSync(file, 'utf8')); - if (!Number.isSafeInteger(pid) || pid <= 0) { - continue; - } - try { - process.kill(-pid, 'SIGKILL'); - } catch {} - if (isRunning(pid)) { - try { - process.kill(pid, 'SIGKILL'); - } catch {} - } - } - } - await preparation; - spawn.mockRestore(); - await fs.promises.rm(root, { recursive: true, force: true }); - } -}, 10_000); diff --git a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs deleted file mode 100644 index 390ae7bc52..0000000000 --- a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/brew.cjs +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env node -const { spawn } = require('node:child_process'); -const fs = require('node:fs'); -const path = require('node:path'); - -const role = process.argv[2] === 'install' ? 'parent' : process.argv[2]; -const pidFile = path.join(__dirname, `${role}.pid`); -fs.writeFileSync(`${pidFile}.tmp`, String(process.pid)); -fs.renameSync(`${pidFile}.tmp`, pidFile); - -if (role !== 'grandchild') { - spawn(process.execPath, [__filename, role === 'parent' ? 'child' : 'grandchild'], { - stdio: 'inherit', - }); -} - -process.stdout.write(`${role}-ready\n`); -setInterval(() => {}, 1000); diff --git a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs b/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs deleted file mode 100644 index a88ba1a6bb..0000000000 --- a/packages/build-tools/src/steps/utils/__tests__/fixtures/preview-encoder/cwebp.cjs +++ /dev/null @@ -1,2 +0,0 @@ -#!/usr/bin/env node -process.exit(1);