fix: harden iOS simulator recording cleanup

Author: thymikee

src/daemon/__tests__/recording-provider.test.ts

@@ -4,7 +4,7 @@ import { IOS_SIMULATOR } from '../../__tests__/test-utils/index.ts';
 
 const { runCmdBackgroundMock } = vi.hoisted(() => ({
   runCmdBackgroundMock: vi.fn(() => ({
-    child: { kill: () => true },
+    child: { kill: () => true, pid: 1234 },
     wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
   })),
 }));
@@ -31,6 +31,7 @@ test('local recording provider starts iOS simulator recordVideo through simctl',
   });
 
   assert.equal(result.child.kill('SIGINT'), true);
+  assert.equal(result.child.pid, 1234);
   assert.deepEqual(mockRunCmdBackground.mock.calls, [
     [
       'xcrun',

src/daemon/__tests__/request-recording-health.test.ts

@@ -53,7 +53,7 @@ test('raw iOS simulator recordings do not depend on runner health', () => {
   expect(session.recording?.invalidatedReason).toBeUndefined();
 });
 
-test('touch-overlay iOS simulator recordings are invalidated by runner restarts', () => {
+test('touch-overlay iOS simulator recordings tolerate runner restarts', () => {
   const session = makeIosSimulatorSession(true);
   mockGetRunnerSessionSnapshot.mockReturnValue({
     alive: true,
@@ -62,6 +62,30 @@ test('touch-overlay iOS simulator recordings are invalidated by runner restarts'
 
   refreshRecordingHealth(session);
 
+  expect(mockGetRunnerSessionSnapshot).not.toHaveBeenCalled();
+  expect(session.recording?.runnerSessionId).toBe('runner-before');
+  expect(session.recording?.invalidatedReason).toBeUndefined();
+});
+
+test('runner-backed iOS recordings still invalidate on runner restarts', () => {
+  const session = makeIosSimulatorSession(true);
+  session.device.kind = 'device';
+  session.recording = {
+    platform: 'ios-device-runner',
+    outPath: '/tmp/demo.mp4',
+    remotePath: '/tmp/demo.mp4',
+    startedAt: Date.now() - 1_000,
+    showTouches: true,
+    gestureEvents: [],
+    runnerSessionId: 'runner-before',
+  };
+  mockGetRunnerSessionSnapshot.mockReturnValue({
+    alive: true,
+    sessionId: 'runner-after',
+  });
+
+  refreshRecordingHealth(session);
+
   expect(mockGetRunnerSessionSnapshot).toHaveBeenCalledWith('sim-1');
   expect(session.recording?.invalidatedReason).toBe(
     'iOS runner session restarted during recording',

src/daemon/__tests__/request-router-recording-health.test.ts

@@ -7,17 +7,24 @@ vi.mock('../../core/dispatch.ts', async (importOriginal) => {
   return { ...actual, dispatchCommand: vi.fn(async () => ({})) };
 });
 
+vi.mock('../../platforms/ios/runner-client.ts', () => ({
+  getRunnerSessionSnapshot: vi.fn(),
+}));
+
 import { dispatchCommand } from '../../core/dispatch.ts';
+import { getRunnerSessionSnapshot } from '../../platforms/ios/runner-client.ts';
 import { createRequestHandler } from '../request-router.ts';
 import type { SessionState } from '../types.ts';
 import { LeaseRegistry } from '../lease-registry.ts';
 import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts';
 
 const mockDispatch = vi.mocked(dispatchCommand);
+const mockGetRunnerSessionSnapshot = vi.mocked(getRunnerSessionSnapshot);
 
 beforeEach(() => {
   mockDispatch.mockReset();
   mockDispatch.mockResolvedValue({});
+  mockGetRunnerSessionSnapshot.mockReset();
 });
 
 test('router blocks non-record commands when recording was invalidated', async () => {
@@ -72,3 +79,67 @@ test('router blocks non-record commands when recording was invalidated', async (
   expect(response.error.message).toBe('iOS runner session restarted during recording');
   expect(mockDispatch).not.toHaveBeenCalled();
 });
+
+test('router allows iOS simulator gestures during overlay recording after runner restart', async () => {
+  const sessionStore = makeSessionStore('agent-device-router-recording-health-');
+  const session: SessionState = {
+    name: 'default',
+    createdAt: Date.now(),
+    actions: [],
+    appBundleId: 'com.apple.Preferences',
+    device: {
+      platform: 'ios',
+      target: 'mobile',
+      id: 'sim-1',
+      name: 'iPhone 17 Pro',
+      kind: 'simulator',
+      booted: true,
+    },
+    recording: {
+      platform: 'ios',
+      outPath: '/tmp/demo.mp4',
+      startedAt: Date.now() - 1_000,
+      showTouches: true,
+      gestureEvents: [],
+      runnerSessionId: 'runner-before',
+      child: { kill: () => {} } as any,
+      wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
+    },
+  };
+  sessionStore.set('default', session);
+  mockGetRunnerSessionSnapshot.mockReturnValue({
+    alive: true,
+    sessionId: 'runner-after',
+  });
+  mockDispatch.mockResolvedValue({
+    action: 'pinch',
+    scale: 1.2,
+    x: 100,
+    y: 200,
+    durationMs: 280,
+  });
+
+  const handler = createRequestHandler({
+    logPath: path.join(os.tmpdir(), 'daemon.log'),
+    token: 'test-token',
+    sessionStore,
+    leaseRegistry: new LeaseRegistry(),
+    trackDownloadableArtifact: () => 'artifact-id',
+  });
+
+  const response = await handler({
+    token: 'test-token',
+    session: 'default',
+    command: 'pinch',
+    positionals: ['1.2', '100', '200'],
+    meta: { requestId: 'req-simulator-runner-restart' },
+  });
+
+  expect(response.ok).toBe(true);
+  expect(mockGetRunnerSessionSnapshot).not.toHaveBeenCalled();
+  expect(mockDispatch).toHaveBeenCalled();
+  const recording = sessionStore.get('default')?.recording;
+  expect(recording?.invalidatedReason).toBeUndefined();
+  expect(recording?.gestureEvents).toHaveLength(1);
+  expect(recording?.gestureEvents[0]?.kind).toBe('pinch');
+});

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

@@ -596,7 +596,105 @@ test('record stop leaves a short visual tail after iOS simulator gestures', asyn
   expect(kill).toHaveBeenCalledWith('SIGINT');
 });
 
-test('record stop escalates stale iOS simulator recordVideo processes', async () => {
+test('record start stores iOS simulator recorder pid for scoped cleanup', async () => {
+  const sessionStore = makeSessionStore();
+  const sessionName = 'ios-sim-recorder-pid';
+  sessionStore.set(
+    sessionName,
+    makeSession(sessionName, {
+      platform: 'ios',
+      id: 'sim-1',
+      name: 'Simulator',
+      kind: 'simulator',
+      booted: true,
+    }),
+  );
+  mockRunCmdBackground.mockImplementation(() => ({
+    child: { kill: () => {}, pid: 5151 } as any,
+    wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
+  }));
+
+  const response = await runRecordCommand({
+    sessionStore,
+    sessionName,
+    positionals: ['start', './sim-recorder-pid.mp4'],
+    flags: { hideTouches: true },
+  });
+
+  expect(response?.ok).toBe(true);
+  expect(sessionStore.get(sessionName)?.recording?.recorderPid).toBe(5151);
+});
+
+test('record stop prefers session-owned iOS recorder processes before path fallback', async () => {
+  vi.useFakeTimers();
+  const processKill = vi.spyOn(process, 'kill').mockImplementation(() => true);
+  const sessionStore = makeSessionStore();
+  const sessionName = 'ios-sim-owned-recorder';
+  const kill = vi.fn();
+  const session = makeSession(sessionName, {
+    platform: 'ios',
+    id: 'sim-1',
+    name: 'Simulator',
+    kind: 'simulator',
+    booted: true,
+  });
+  session.recording = {
+    platform: 'ios',
+    outPath: '/tmp/owned-recorder.mp4',
+    startedAt: Date.now(),
+    showTouches: true,
+    gestureEvents: [],
+    recorderPid: 1111,
+    child: { kill, pid: 1111 },
+    wait: new Promise(() => {}),
+  };
+  sessionStore.set(sessionName, session);
+  mockRunCmd.mockImplementation(async (cmd, args) => {
+    if (cmd === 'pgrep' && args[0] === '-P') {
+      expect(args).toEqual(['-P', '1111']);
+      return { stdout: '2222\n', stderr: '', exitCode: 0 };
+    }
+    if (cmd === 'pgrep' && args[0] === '-f') {
+      return { stdout: '3333\n', stderr: '', exitCode: 0 };
+    }
+    return { stdout: '', stderr: '', exitCode: 0 };
+  });
+
+  try {
+    const responsePromise = runRecordCommand({
+      sessionStore,
+      sessionName,
+      positionals: ['stop'],
+    });
+
+    await vi.advanceTimersByTimeAsync(12_000);
+    const response = await responsePromise;
+
+    expect(response?.ok).toBe(false);
+    expect((response as any).error?.message).toMatch(/did not exit/);
+    expect(kill.mock.calls.map((call) => call[0])).toEqual(['SIGINT', 'SIGTERM', 'SIGKILL']);
+    expect(mockRunCmd.mock.calls.map((call) => call[1])).toEqual([
+      ['-P', '1111'],
+      ['-P', '1111'],
+      ['-P', '1111'],
+    ]);
+    expect(processKill.mock.calls.map((call) => call[0])).toEqual([
+      1111, 2222, 1111, 2222, 1111, 2222,
+    ]);
+    expect(processKill.mock.calls.map((call) => call[1])).toEqual([
+      'SIGINT',
+      'SIGINT',
+      'SIGTERM',
+      'SIGTERM',
+      'SIGKILL',
+      'SIGKILL',
+    ]);
+  } finally {
+    processKill.mockRestore();
+  }
+});
+
+test('record stop falls back to path matching for stale iOS simulator recordVideo processes', async () => {
   vi.useFakeTimers();
   const processKill = vi.spyOn(process, 'kill').mockImplementation(() => true);
   const sessionStore = makeSessionStore();
@@ -1229,7 +1327,7 @@ test('record stop force-kills Android screenrecord when SIGINT fails but process
   ).toBe(true);
 });
 
-test('record stop reports invalidated recording after cleanup', async () => {
+test('record stop keeps iOS simulator video when touch overlay recording was invalidated', async () => {
   const sessionStore = makeSessionStore();
   const sessionName = 'ios-invalidated-recording';
   const session = makeSession(sessionName, {
@@ -1257,10 +1355,12 @@ test('record stop reports invalidated recording after cleanup', async () => {
     positionals: ['stop'],
   });
 
-  expect(response?.ok).toBe(false);
-  if (response?.ok === false) {
-    expect(response.error.code).toBe('COMMAND_FAILED');
-    expect(response.error.message).toBe('iOS runner session exited during recording');
+  expect(response?.ok).toBe(true);
+  if (response?.ok === true) {
+    expect(response.data?.outPath).toBe(path.resolve('./invalidated.mp4'));
+    expect(response.data?.overlayWarning).toBe(
+      'overlay unavailable: iOS runner session exited during recording',
+    );
   }
   expect(sessionStore.get(sessionName)?.recording).toBeUndefined();
 });