fix(record): finalize active recording on session/daemon teardown

src/daemon/handlers/__tests__/record-trace.test.ts

@@ -54,6 +54,7 @@ vi.mock('../../device-ready.ts', () => ({
 }));
 
 import { handleRecordTraceCommands } from '../record-trace.ts';
+import { stopSessionRecordingForTeardown } from '../record-trace-recording.ts';
 import { deriveRecordingTelemetryPath } from '../../recording-telemetry.ts';
 import { SessionStore } from '../../session-store.ts';
 import type { SessionState } from '../../types.ts';
@@ -986,6 +987,46 @@ test('record stop leaves a short visual tail after iOS simulator gestures', asyn
   expect(kill).toHaveBeenCalledWith('SIGINT');
 });
 
+test('stopSessionRecordingForTeardown finalizes an active iOS simulator recording and clears session state', async () => {
+  const outPath = path.join(os.tmpdir(), `agent-device-teardown-${Date.now()}.mp4`);
+  fs.writeFileSync(outPath, 'recorded-bytes');
+  const session = makeIosSimulatorRecordingSession('ios-sim-teardown', { outPath });
+  const recording = session.recording;
+  const kill = recording?.platform === 'ios' ? recording.child.kill : undefined;
+
+  await stopSessionRecordingForTeardown(session);
+
+  // Teardown must SIGINT the recorder (which finalizes the mp4) rather than
+  // orphaning the simctl child, and must detach the recording from the session.
+  expect(kill).toHaveBeenCalledWith('SIGINT');
+  expect(session.recording).toBeUndefined();
+});
+
+test('stopSessionRecordingForTeardown is a no-op when the session has no active recording', async () => {
+  const session = makeIosSimulatorSession('ios-sim-teardown-no-recording');
+
+  await expect(stopSessionRecordingForTeardown(session)).resolves.toBeUndefined();
+
+  expect(session.recording).toBeUndefined();
+});
+
+test('stopSessionRecordingForTeardown rethrows a typed stop failure for the cleanup-failure channel', async () => {
+  const session = makeIosSimulatorRecordingSession('ios-sim-teardown-stop-failure', {
+    startedAt: Date.now() - 5_000,
+  });
+  const recording = session.recording;
+  if (recording?.platform === 'ios') {
+    recording.wait = Promise.resolve({ stdout: '', stderr: 'recorder crashed', exitCode: 1 });
+  }
+
+  await expect(stopSessionRecordingForTeardown(session)).rejects.toThrow(
+    /failed to stop recording/,
+  );
+
+  // The recording is still detached so a retry cannot double-stop the recorder.
+  expect(session.recording).toBeUndefined();
+});
+
 test('record stop reports too-short iOS simulator recordings without leaving invalid output', async () => {
   const sessionStore = makeSessionStore();
   const sessionName = 'ios-sim-too-short';

src/daemon/handlers/__tests__/session-close-shutdown.test.ts

@@ -39,6 +39,10 @@ vi.mock('../../../platforms/apple/os/macos/helper.ts', async (importOriginal) =>
     await importOriginal<typeof import('../../../platforms/apple/os/macos/helper.ts')>();
   return { ...actual, runMacOsAlertAction: vi.fn(async () => {}) };
 });
+vi.mock('../../../utils/video.ts', () => ({
+  waitForStableFile: vi.fn(async () => {}),
+  isPlayableVideo: vi.fn(async () => true),
+}));
 vi.mock('../../runtime-hints.ts', async (importOriginal) => {
   const actual = await importOriginal<typeof import('../../runtime-hints.ts')>();
   return { ...actual, clearRuntimeHintsFromApp: vi.fn(async () => {}) };
@@ -91,6 +95,40 @@ function makeSession(name: string, device: SessionState['device']): SessionState
   };
 }
 
+function makeIosSimulatorRecordingSession(
+  name: string,
+  options: { recorderExitCode?: number } = {},
+): SessionState {
+  const session = makeSession(name, {
+    platform: 'apple',
+    id: 'sim-udid-recording',
+    name: 'iPhone 15',
+    kind: 'simulator',
+    booted: true,
+  });
+  session.appBundleId = 'com.example.app';
+  session.recording = {
+    platform: 'ios',
+    outPath: path.join(os.tmpdir(), `${name}.mp4`),
+    startedAt: Date.now() - 5_000,
+    showTouches: false,
+    gestureEvents: [],
+    child: { kill: vi.fn(), pid: 4242 },
+    wait: Promise.resolve({
+      stdout: '',
+      stderr: options.recorderExitCode ? 'recorder crashed' : '',
+      exitCode: options.recorderExitCode ?? 0,
+    }),
+  };
+  return session;
+}
+
+function recordingKillMock(session: SessionState): ReturnType<typeof vi.fn> {
+  const recording = session.recording;
+  if (recording?.platform !== 'ios') throw new Error('expected an iOS simulator recording');
+  return recording.child.kill as ReturnType<typeof vi.fn>;
+}
+
 beforeEach(() => {
   vi.clearAllMocks();
   setActiveProviderDeviceRuntimes([]);
@@ -393,6 +431,91 @@ test('daemon session teardown stops active Apple xctrace perf capture', async ()
   expect(session.applePerf?.active).toBeUndefined();
 });
 
+test('close finalizes an active iOS simulator recording before deleting the session', async () => {
+  const sessionStore = makeSessionStore();
+  const sessionName = 'ios-active-recording-close-session';
+  const session = makeIosSimulatorRecordingSession(sessionName);
+  const kill = recordingKillMock(session);
+  sessionStore.set(sessionName, session);
+
+  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);
+  // The recorder was signaled (SIGINT finalizes the simctl mp4), the recording
+  // was detached, and the session was deleted — no orphaned recordVideo child.
+  expect(kill).toHaveBeenCalledWith('SIGINT');
+  expect(session.recording).toBeUndefined();
+  expect(sessionStore.get(sessionName)).toBeUndefined();
+  // An active recording at close time still defeats iOS runner retention even
+  // though the recording is finalized (and cleared) before the retention step.
+  expect(mockStopIosRunnerSession).toHaveBeenCalledWith(session.device.id);
+});
+
+test('close surfaces a recording finalization failure through the cleanup-failure channel', async () => {
+  const sessionStore = makeSessionStore();
+  const sessionName = 'ios-recording-close-failure-session';
+  const session = makeIosSimulatorRecordingSession(sessionName, { recorderExitCode: 1 });
+  const kill = recordingKillMock(session);
+  sessionStore.set(sessionName, session);
+
+  await expect(
+    handleSessionCommands({
+      req: {
+        token: 't',
+        session: sessionName,
+        command: 'close',
+        positionals: [],
+        flags: {},
+      },
+      sessionName,
+      logPath: path.join(os.tmpdir(), 'daemon.log'),
+      sessionStore,
+      invoke: noopInvoke,
+    }),
+  ).rejects.toThrow(/recording: .*failed to stop recording/);
+
+  // Cleanup failure is reported, later cleanup still ran, session still deleted.
+  expect(kill).toHaveBeenCalledWith('SIGINT');
+  expect(mockStopIosRunnerSession).toHaveBeenCalledWith(session.device.id);
+  expect(sessionStore.get(sessionName)).toBeUndefined();
+});
+
+test('daemon session teardown finalizes an active iOS simulator recording', async () => {
+  const sessionName = 'ios-active-recording-teardown-session';
+  const session = makeIosSimulatorRecordingSession(sessionName);
+  const kill = recordingKillMock(session);
+
+  await teardownSessionResources(session, sessionName);
+
+  expect(kill).toHaveBeenCalledWith('SIGINT');
+  expect(session.recording).toBeUndefined();
+});
+
+test('daemon session teardown surfaces a recording finalization failure', async () => {
+  const sessionName = 'ios-recording-teardown-failure-session';
+  const session = makeIosSimulatorRecordingSession(sessionName, { recorderExitCode: 1 });
+  const kill = recordingKillMock(session);
+
+  await expect(teardownSessionResources(session, sessionName)).rejects.toThrow(
+    /recording: .*failed to stop recording/,
+  );
+
+  expect(kill).toHaveBeenCalledWith('SIGINT');
+  expect(session.recording).toBeUndefined();
+});
+
 test('close stops active Android native perf capture before deleting session', async () => {
   const sessionStore = makeSessionStore();
   const sessionName = 'android-active-native-perf-session';

src/daemon/handlers/record-trace-ios-simulator.ts

@@ -10,6 +10,18 @@ export const IOS_SIMULATOR_RECORDING_STOP_TIMEOUT_MS = 5_000;
 
 const IOS_SIMULATOR_RECORDING_FORCE_STOP_TIMEOUT_MS = 2_000;
 
+/**
+ * Worst-case wall-clock time for {@link stopIosSimulatorRecordingProcess} to
+ * conclude: the direct child-handle SIGINT wait plus the three escalating
+ * PID-based retries (SIGINT, SIGTERM, SIGKILL). Any teardown path that bounds
+ * recording finalization with its own timeout (daemon shutdown's per-session
+ * teardown race) must budget at least this long, or a recorder stuck past the
+ * direct-handle wait is abandoned mid-escalation and the simctl child orphans
+ * with an unfinalized 0-byte mp4.
+ */
+export const IOS_SIMULATOR_RECORDING_STOP_ESCALATION_BUDGET_MS =
+  IOS_SIMULATOR_RECORDING_STOP_TIMEOUT_MS + 3 * IOS_SIMULATOR_RECORDING_FORCE_STOP_TIMEOUT_MS;
+
 type IosSimulatorRecording = Extract<NonNullable<SessionState['recording']>, { platform: 'ios' }>;
 
 export async function stopIosSimulatorRecordingProcess(params: {

src/daemon/handlers/record-trace-recording.ts

@@ -1,5 +1,6 @@
 import fs from 'node:fs';
 import path from 'node:path';
+import { AppError, toAppErrorCode } from '../../kernel/errors.ts';
 import { sleep } from '../../utils/timeouts.ts';
 import { resolveTargetDevice } from '../../core/dispatch.ts';
 import { ensureDeviceReady } from '../device-ready.ts';
@@ -521,6 +522,55 @@ async function releaseRecordOnlySession(
   sessionStore.delete(sessionName);
 }
 
+/**
+ * Best-effort finalization of a session's still-active recording during
+ * teardown (session close or daemon shutdown). The normal `test --record-video`
+ * and `record stop` flows stop the recorder explicitly, but a session torn down
+ * while a recording is still active — e.g. the daemon is signalled/reaped or the
+ * session is closed before an explicit stop — otherwise leaks its recorder
+ * process. On the iOS simulator the `simctl io … recordVideo` child then
+ * reparents to launchd (PPID 1) and, because simctl only finalizes the mp4 on
+ * SIGINT, leaves a 0-byte file that also holds the device's single host
+ * recording slot (later attempts fail with "Host recording is already in
+ * progress"). Routing through the normal {@link stopActiveRecording} path sends
+ * SIGINT to the recorder and awaits the finalized file on every platform.
+ *
+ * The recording is detached from the session first so a late explicit
+ * `record stop` (or a second teardown pass) cannot double-stop the same
+ * recorder. A typed stop failure (the recorder could not be finalized) is
+ * rethrown as an {@link AppError} so both callers' isolated cleanup channels
+ * (`runIsolatedSessionCleanup` / `attemptCleanup`) record it as a `recording`
+ * cleanup failure instead of silently reporting successful cleanup; later
+ * cleanup steps still run because those channels isolate per-step failures.
+ */
+export async function stopSessionRecordingForTeardown(
+  session: SessionState,
+  logPath?: string,
+): Promise<void> {
+  const recording = session.recording;
+  if (!recording) return;
+  session.recording = undefined;
+  const req: DaemonRequest = {
+    token: '',
+    session: session.name,
+    command: 'record',
+    positionals: ['stop'],
+    flags: {},
+  };
+  const stopFailure = await stopActiveRecording({
+    req,
+    activeSession: session,
+    device: session.device,
+    logPath,
+    deps: buildRecordTraceDeps(),
+    recording,
+    stopRequestedAt: Date.now(),
+  });
+  if (stopFailure && stopFailure.ok === false) {
+    throw new AppError(toAppErrorCode(stopFailure.error.code), stopFailure.error.message);
+  }
+}
+
 // --- Main command handler ---
 
 export async function handleRecordCommand(params: {

src/daemon/handlers/session-close.ts

@@ -24,6 +24,7 @@ import {
 import { errorResponse } from './response.ts';
 import { expireRefFrame } from '../ref-frame.ts';
 import { recordSessionAction } from './handler-utils.ts';
+import { stopSessionRecordingForTeardown } from './record-trace-recording.ts';
 import type { LeaseRegistry } from '../lease-registry.ts';
 import { releaseSessionLease } from '../lease-lifecycle.ts';
 import type { LeaseLifecycleProvider } from './lease.ts';
@@ -195,6 +196,10 @@ async function runSessionCloseTeardown(params: {
       cleanupFailures.push({ step, error });
     }
   };
+  // Decide runner retention from the PRE-teardown state: an active recording
+  // defeats retention, and `stopBestEffortSessionResources` below finalizes (and
+  // clears) that recording, so the decision must be captured before it runs.
+  const retainAppleRunner = shouldRetainAppleRunnerAfterClose(req, session);
   await stopBestEffortSessionResources(session, sessionStore, attemptCleanup);
   // The targeted platform close is the primary operation, not best-effort cleanup:
   // its AppError (code/details/hint) is preserved and returned for the caller to
@@ -203,7 +208,7 @@ async function runSessionCloseTeardown(params: {
   const platformCloseError = params.skipPlatformClose
     ? undefined
     : await dispatchTargetedPlatformClose({ req, session, logPath });
-  await stopOrRetainAppleRunnerAfterClose(req, session, attemptCleanup);
+  await stopOrRetainAppleRunnerAfterClose(retainAppleRunner, session, attemptCleanup);
   await clearSessionRuntimeHints(session, sessionStore, sessionName);
   // ADR 0012 decision 6 (BLOCKER 2): a repair-armed session already recorded its
   // finalize `close` and committed (or aborted) its healed `.ad` BEFORE this
@@ -237,6 +242,12 @@ async function stopBestEffortSessionResources(
   sessionStore: SessionStore,
   attemptCleanup: CleanupRunner,
 ): Promise<void> {
+  // Finalize any still-active recording first so closing a session mid-recording
+  // (without an explicit `record stop`) cannot leak the recorder process; must
+  // run before the Apple runner is stopped below since overlay finalization
+  // consults the runner. `shouldRetainAppleRunnerAfterClose` then observes the
+  // now-cleared `session.recording`.
+  await attemptCleanup('recording', () => stopSessionRecordingForTeardown(session));
   await attemptCleanup('app_log', () => stopSessionAppLog(session));
   await attemptCleanup('audio_probe', async () => {
     await stopSessionAudioProbe(session, 'session-close');
@@ -379,12 +390,12 @@ async function clearSessionRuntimeHints(
 }
 
 async function stopOrRetainAppleRunnerAfterClose(
-  req: DaemonRequest,
+  retainAppleRunner: boolean,
   session: SessionState,
   attemptCleanup: CleanupRunner,
 ): Promise<void> {
   if (!isApplePlatform(session.device.platform)) return;
-  if (!shouldRetainAppleRunnerAfterClose(req, session)) {
+  if (!retainAppleRunner) {
     // The targeted close path stops before dispatch to avoid runner/app races.
     // Stop again here for idempotent cleanup, and keep cleanup-sensitive closes explicit.
     await attemptCleanup('apple_runner', () => stopAppleRunnerForClose(session));