refactor: route process spawning through exec helpers (#504)

Author: thymikee

AGENTS.md

@@ -102,7 +102,7 @@ A new snapshot/command flag touches up to 7 files in a fixed order. Follow this
 Command-only flags (like `find --first`) that don't flow to the platform layer only need steps 1 and the handler file.
 
 ## Hard Rules
-- Use `runCmd`/`runCmdSync` from `src/utils/exec.ts` for process execution.
+- Use process helpers from `src/utils/exec.ts` for TypeScript process execution: `runCmd`, `runCmdStreaming`, `runCmdSync`, `runCmdBackground`, and `runCmdDetached`. Do not import raw `spawn`/`spawnSync` outside `src/utils/exec.ts`; add or extend an exec helper instead. Plain `.mjs` packaging fixtures that cannot import TypeScript helpers should keep child-process usage local and prefer `execFile`/`execFileSync` over spawn.
 - Use daemon session flow for interactions (`open` before interactions, `close` after).
 - Use `keyboard dismiss` for iOS keyboard dismissal; it may tap safe native controls such as `Done` but must not fall back to system back navigation.
 - Do not remove shared snapshot/session model behavior without full migration.

src/daemon/__tests__/app-log-android.test.ts

@@ -6,25 +6,24 @@ import os from 'node:os';
 import path from 'node:path';
 import { PassThrough } from 'node:stream';
 
-vi.mock('node:child_process', () => ({ spawn: vi.fn() }));
 vi.mock('../../utils/exec.ts', async (importOriginal) => {
   const actual = await importOriginal<typeof import('../../utils/exec.ts')>();
   return {
     ...actual,
     runCmd: vi.fn(async () => ({ stdout: '', stderr: '', exitCode: 0 })),
+    runCmdBackground: vi.fn(),
   };
 });
 vi.mock('../app-log-stream.ts', async (importOriginal) => {
   const actual = await importOriginal<typeof import('../app-log-stream.ts')>();
   return { ...actual, sleep: vi.fn(async () => {}) };
 });
 
-import { spawn } from 'node:child_process';
-import { runCmd } from '../../utils/exec.ts';
+import { runCmd, runCmdBackground } from '../../utils/exec.ts';
 import { readRecentAndroidLogcatForPackage, startAndroidAppLog } from '../app-log-android.ts';
 
-const mockSpawn = vi.mocked(spawn);
 const mockRunCmd = vi.mocked(runCmd);
+const mockRunCmdBackground = vi.mocked(runCmdBackground);
 
 type MockChild = EventEmitter & {
   stdout: PassThrough;
@@ -51,6 +50,15 @@ function makeMockChild(pid?: number): MockChild {
   return child;
 }
 
+function mockBackgroundChild(child: MockChild): ReturnType<typeof runCmdBackground> {
+  return {
+    child: child as unknown as ReturnType<typeof runCmdBackground>['child'],
+    wait: new Promise((resolve) => {
+      child.once('close', (code) => resolve({ stdout: '', stderr: '', exitCode: code ?? 0 }));
+    }),
+  };
+}
+
 test('startAndroidAppLog returns to active state after a successful reattach', async () => {
   const logDir = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-android-log-'));
   const stream = fs.createWriteStream(path.join(logDir, 'app.log'));
@@ -71,25 +79,25 @@ test('startAndroidAppLog returns to active state after a successful reattach', a
     return { stdout: '', stderr: '', exitCode: 0 };
   });
 
-  mockSpawn.mockReset();
+  mockRunCmdBackground.mockReset();
   let spawnCount = 0;
-  mockSpawn.mockImplementation(() => {
+  mockRunCmdBackground.mockImplementation(() => {
     spawnCount += 1;
     if (spawnCount === 1) {
-      return firstChild as unknown as ReturnType<typeof spawn>;
+      return mockBackgroundChild(firstChild);
     }
-    return secondChild as unknown as ReturnType<typeof spawn>;
+    return mockBackgroundChild(secondChild);
   });
 
   const appLog = await startAndroidAppLog('emulator-5554', 'com.example.app', stream, []);
   await vi.waitFor(() => {
-    expect(mockSpawn).toHaveBeenCalledTimes(1);
+    expect(mockRunCmdBackground).toHaveBeenCalledTimes(1);
   });
   assert.equal(appLog.getState(), 'active');
 
   firstChild.emit('close', 1);
   await vi.waitFor(() => {
-    expect(mockSpawn).toHaveBeenCalledTimes(2);
+    expect(mockRunCmdBackground).toHaveBeenCalledTimes(2);
   });
   assert.equal(appLog.getState(), 'active');
 
@@ -110,12 +118,12 @@ test('startAndroidAppLog reports active for provider streams without host pid',
     return { stdout: '', stderr: '', exitCode: 0 };
   });
 
-  mockSpawn.mockReset();
-  mockSpawn.mockImplementation(() => child as unknown as ReturnType<typeof spawn>);
+  mockRunCmdBackground.mockReset();
+  mockRunCmdBackground.mockImplementation(() => mockBackgroundChild(child));
 
   const appLog = await startAndroidAppLog('emulator-5554', 'com.example.app', stream, []);
   await vi.waitFor(() => {
-    expect(mockSpawn).toHaveBeenCalledTimes(1);
+    expect(mockRunCmdBackground).toHaveBeenCalledTimes(1);
   });
 
   assert.equal(appLog.getState(), 'active');

src/daemon/app-log-ios.ts

@@ -1,7 +1,6 @@
-import { spawn } from 'node:child_process';
 import fs from 'node:fs';
 import { buildSimctlArgs } from '../platforms/ios/simctl.ts';
-import { runCmd } from '../utils/exec.ts';
+import { runCmd, runCmdBackground } from '../utils/exec.ts';
 import { clearPidFile, writePidFile, type AppLogResult } from './app-log-process.ts';
 import { attachChildToStream, createLineWriter, waitForChildExit } from './app-log-stream.ts';
 
@@ -96,38 +95,14 @@ export async function startIosSimulatorAppLog(
   simulatorSetPath?: string,
   pidPath?: string,
 ): Promise<AppLogResult> {
-  let state: 'active' | 'failed' = 'active';
-  const child = spawn(
-    'xcrun',
-    buildIosSimulatorLogStreamArgs({ deviceId, appBundleId, simulatorSetPath }),
-    {
-      stdio: ['ignore', 'pipe', 'pipe'],
-    },
-  );
-  const writer = createLineWriter(stream, { redactionPatterns });
-  if (typeof child.pid === 'number') {
-    writePidFile(pidPath, child.pid);
-  }
-  const wait = attachChildToStream(child, stream, { endStreamOnClose: true, writer }).then(
-    (result) => {
-      if (result.exitCode !== 0) state = 'failed';
-      clearPidFile(pidPath);
-      return result;
-    },
-  );
-  return {
+  return startAppleAppLogStream({
     backend: 'ios-simulator',
-    getState: () => state,
-    startedAt: Date.now(),
-    wait,
-    stop: async () => {
-      if (!child.killed) child.kill('SIGINT');
-      await waitForChildExit(wait);
-      if (!child.killed) child.kill('SIGKILL');
-      await waitForChildExit(wait);
-      clearPidFile(pidPath);
-    },
-  };
+    cmd: 'xcrun',
+    args: buildIosSimulatorLogStreamArgs({ deviceId, appBundleId, simulatorSetPath }),
+    stream,
+    redactionPatterns,
+    pidPath,
+  });
 }
 
 export async function startMacOsAppLog(
@@ -136,38 +111,14 @@ export async function startMacOsAppLog(
   redactionPatterns: RegExp[],
   pidPath?: string,
 ): Promise<AppLogResult> {
-  let state: 'active' | 'failed' = 'active';
-  const child = spawn(
-    'log',
-    ['stream', '--style', 'compact', '--predicate', buildAppleLogPredicate(appBundleId)],
-    {
-      stdio: ['ignore', 'pipe', 'pipe'],
-    },
-  );
-  const writer = createLineWriter(stream, { redactionPatterns });
-  if (typeof child.pid === 'number') {
-    writePidFile(pidPath, child.pid);
-  }
-  const wait = attachChildToStream(child, stream, { endStreamOnClose: true, writer }).then(
-    (result) => {
-      if (result.exitCode !== 0) state = 'failed';
-      clearPidFile(pidPath);
-      return result;
-    },
-  );
-  return {
+  return startAppleAppLogStream({
     backend: 'macos',
-    getState: () => state,
-    startedAt: Date.now(),
-    wait,
-    stop: async () => {
-      if (!child.killed) child.kill('SIGINT');
-      await waitForChildExit(wait);
-      if (!child.killed) child.kill('SIGKILL');
-      await waitForChildExit(wait);
-      clearPidFile(pidPath);
-    },
-  };
+    cmd: 'log',
+    args: ['stream', '--style', 'compact', '--predicate', buildAppleLogPredicate(appBundleId)],
+    stream,
+    redactionPatterns,
+    pidPath,
+  });
 }
 
 export async function startIosDeviceAppLog(
@@ -176,23 +127,52 @@ export async function startIosDeviceAppLog(
   redactionPatterns: RegExp[],
   pidPath?: string,
 ): Promise<AppLogResult> {
+  return startAppleAppLogStream({
+    backend: 'ios-device',
+    cmd: 'xcrun',
+    args: buildIosDeviceLogStreamArgs(deviceId),
+    stream,
+    redactionPatterns,
+    pidPath,
+  });
+}
+
+function startAppleAppLogStream(params: {
+  backend: AppLogResult['backend'];
+  cmd: string;
+  args: string[];
+  stream: fs.WriteStream;
+  redactionPatterns: RegExp[];
+  pidPath?: string;
+}): AppLogResult {
   let state: 'active' | 'failed' = 'active';
-  const child = spawn('xcrun', buildIosDeviceLogStreamArgs(deviceId), {
-    stdio: ['ignore', 'pipe', 'pipe'],
+  const background = runCmdBackground(params.cmd, params.args, {
+    allowFailure: true,
+    captureOutput: false,
   });
-  const writer = createLineWriter(stream, { redactionPatterns });
+  void background.wait.catch(() => {});
+  const child = background.child;
+  const writer = createLineWriter(params.stream, { redactionPatterns: params.redactionPatterns });
   if (typeof child.pid === 'number') {
-    writePidFile(pidPath, child.pid);
+    writePidFile(params.pidPath, child.pid);
   }
-  const wait = attachChildToStream(child, stream, { endStreamOnClose: true, writer }).then(
+  const wait = attachChildToStream(child, params.stream, {
+    endStreamOnClose: true,
+    writer,
+  }).then(
     (result) => {
       if (result.exitCode !== 0) state = 'failed';
-      clearPidFile(pidPath);
+      clearPidFile(params.pidPath);
       return result;
     },
+    (error: unknown) => {
+      state = 'failed';
+      clearPidFile(params.pidPath);
+      throw error;
+    },
   );
   return {
-    backend: 'ios-device',
+    backend: params.backend,
     getState: () => state,
     startedAt: Date.now(),
     wait,
@@ -201,7 +181,7 @@ export async function startIosDeviceAppLog(
       await waitForChildExit(wait);
       if (!child.killed) child.kill('SIGKILL');
       await waitForChildExit(wait);
-      clearPidFile(pidPath);
+      clearPidFile(params.pidPath);
     },
   };
 }

src/daemon/types.ts

@@ -12,7 +12,7 @@ export type { DaemonLockPolicy } from '../contracts.ts';
 import type { CommandFlags } from '../core/dispatch.ts';
 import type { SessionSurface } from '../core/session-surface.ts';
 import type { DeviceInfo, Platform, PlatformSelector } from '../utils/device.ts';
-import type { ExecResult } from '../utils/exec.ts';
+import type { ExecBackgroundResult, ExecResult } from '../utils/exec.ts';
 import type { SnapshotState } from '../utils/snapshot.ts';
 import type { AppLogState } from './app-log-process.ts';
 
@@ -177,7 +177,7 @@ export type SessionState = {
   recording?:
     | (SessionRecordingBase & {
         platform: 'ios';
-        child: ReturnType<typeof import('node:child_process').spawn>;
+        child: ExecBackgroundResult['child'];
         wait: Promise<ExecResult>;
         remotePath?: string;
       })

src/platforms/android/__tests__/adb-executor.test.ts

@@ -1,23 +1,19 @@
 import assert from 'node:assert/strict';
 import { test, vi } from 'vitest';
 
-const { spawnMock } = vi.hoisted(() => ({
-  spawnMock: vi.fn(() => ({})),
+const { runCmdBackgroundMock } = vi.hoisted(() => ({
+  runCmdBackgroundMock: vi.fn(() => ({
+    child: {},
+    wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
+  })),
 }));
 
-vi.mock('node:child_process', async (importOriginal) => {
-  const actual = await importOriginal<typeof import('node:child_process')>();
-  return {
-    ...actual,
-    spawn: spawnMock,
-  };
-});
-
 vi.mock('../../../utils/exec.ts', async (importOriginal) => {
   const actual = await importOriginal<typeof import('../../../utils/exec.ts')>();
   return {
     ...actual,
     runCmd: vi.fn(async () => ({ stdout: 'ok', stderr: '', exitCode: 0 })),
+    runCmdBackground: runCmdBackgroundMock,
   };
 });
 
@@ -31,9 +27,10 @@ import {
   resolveAndroidAdbProvider,
   withAndroidAdbProvider,
 } from '../adb-executor.ts';
-import { runCmd } from '../../../utils/exec.ts';
+import { runCmd, runCmdBackground } from '../../../utils/exec.ts';
 
 const mockRunCmd = vi.mocked(runCmd);
+const mockRunCmdBackground = vi.mocked(runCmdBackground);
 
 test('createDeviceAdbExecutor routes local commands through adb with the device serial', async () => {
   const adb = createDeviceAdbExecutor({
@@ -117,6 +114,7 @@ test('scoped provider only resolves for the matching device serial', async () =>
 
 test('createLocalAndroidAdbProvider exposes exec, spawn, and reverse over local adb', async () => {
   mockRunCmd.mockClear();
+  mockRunCmdBackground.mockClear();
   const provider = createLocalAndroidAdbProvider({
     platform: 'android',
     id: 'emulator-5554',
@@ -130,11 +128,11 @@ test('createLocalAndroidAdbProvider exposes exec, spawn, and reverse over local
   await provider.reverse?.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'session-a' });
   await provider.reverse?.removeAllOwned('session-a');
 
-  assert.deepEqual(spawnMock.mock.calls, [
+  assert.deepEqual(mockRunCmdBackground.mock.calls, [
     [
       'adb',
       ['-s', 'emulator-5554', 'logcat'],
-      { stdio: ['ignore', 'pipe', 'pipe'], windowsHide: true },
+      { stdio: ['ignore', 'pipe', 'pipe'], allowFailure: true, captureOutput: false },
     ],
   ]);
   assert.deepEqual(

src/platforms/android/__tests__/adb-provider-scope.test.ts

@@ -2,10 +2,13 @@ import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
-import type { ChildProcess } from 'node:child_process';
 import { test } from 'vitest';
 import { runCmd } from '../../../utils/exec.ts';
-import { resolveAndroidAdbProvider, withAndroidAdbProvider } from '../adb-executor.ts';
+import {
+  resolveAndroidAdbProvider,
+  withAndroidAdbProvider,
+  type AndroidAdbProcess,
+} from '../adb-executor.ts';
 
 const device = {
   platform: 'android',
@@ -66,7 +69,7 @@ test('withAndroidAdbProvider ignores adb commands for another serial', async ()
 });
 
 test('resolveAndroidAdbProvider uses the scoped provider spawner', async () => {
-  const child = { pid: 123 } as ChildProcess;
+  const child = { pid: 123 } as AndroidAdbProcess;
   const calls: string[][] = [];
 
   const result = await withAndroidAdbProvider(