fix(record): keep the daemon alive when a record request times out (#3199)

* fix(record): keep the daemon alive when a record request times out

A record stop export routinely outlasts the 90s client envelope on long
recordings. The default reset-daemon policy SIGKILLed the daemon
mid-export, leaving the session's screen-recording manifest open with no
owner. A record-only session holds no device claim, so nothing reconciled
it, and every later record start on the device refused with
cleanup-unconfirmed until that exact session ran record stop.

record now preserves the daemon on timeout: the export finishes, a retried
record stop serves it, and the next record start is admitted. The local
timeout hint now names that retry, as the remote one already did.

* fix(record): say a timed-out record stop may still be exporting

A record stop that times out while still queued for the device lock is
dropped at lock entry, so no export ran. The timeout hint now says the
daemon may still be exporting and keeps the retry instruction. The docs
no longer call the in-progress export typical only for a remote daemon,
since a local daemon now survives the timeout too.

* fix(record): keep the Apple runner alive when a record request times out

The client's timeout cleanup pkills every Apple runner xcodebuild on the
host. On a physical iOS device or macOS the runner is the recorder, so a
timed-out record stop could kill the export the preserved daemon was
still finishing. record now skips that sweep. The daemon still cancels
its own runner work for a timed-out request through the request signal,
and the exclusion is keyed on the command, never on the declared
platform.

Author: pvedula7

packages/command-registry/src/registry.ts

@@ -1234,7 +1234,10 @@ export const RAW_COMMAND_DESCRIPTORS = [
       allowSessionlessDefaultDevice: isRecordingStartRequest,
     },
     platformExecution: { kind: 'device-runtime', uses: screenRecordingRuntimePlanUses },
-    timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
+    // A `record stop` export can outlast the request envelope. Resetting the daemon mid-export
+    // left its recording manifest open with no owner, and every later `record start` on the
+    // device refused until that exact session ran `record stop`.
+    timeoutPolicy: PRESERVE_DAEMON_TIMEOUT_POLICY,
     batchable: true,
   },
   {

src/__tests__/command-descriptor-timeout-policy.test.ts

@@ -71,6 +71,9 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => {
   // sessions the daemon owns, so a client-side timeout must not SIGKILL the
   // daemon mid-create/mid-release and orphan them (and every other provider
   // session held).
+  // record joined because a `record stop` export can outlast the envelope: a
+  // reset mid-export left an ownerless open manifest that refused every later
+  // `record start` on the device.
   const preserving = commandDescriptors
     .filter((descriptor) => descriptor.timeoutPolicy.onTimeout === 'preserve-daemon')
     .map((descriptor) => descriptor.name);
@@ -87,6 +90,7 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => {
     'lease_release',
     'longpress',
     'press',
+    'record',
     'scroll',
     'snapshot',
     'type',

src/daemon-client/__tests__/daemon-client-timeout-route.test.ts

@@ -19,8 +19,9 @@
 // (the "unknown-session" case) is the common route the original bug misled.
 // A design that skips the pkill sweep based on the declared flag alone would
 // skip real cleanup in the rebound case — the dangerous direction. This test
-// proves the sweep always fires for local timeouts, and that the HINT text
-// (not the cleanup) is what carries the platform-evidence gating.
+// proves the sweep fires for every local timeout except `record` (excluded by
+// command, never by platform), and that the HINT text (not the cleanup) is
+// what carries the platform-evidence gating.
 
 import net from 'node:net';
 import http from 'node:http';
@@ -308,3 +309,41 @@ test('a refused timeout fallback preserves the timeout without an unhandled reje
     server.close();
   }
 });
+
+test('a local record stop timeout leaves the exporting daemon and runner alive and names the retry', async () => {
+  // A match would terminate the runner, which is the recorder on a physical iOS device or macOS.
+  mockRunCmdSync.mockReturnValue({ exitCode: 0, stdout: '', stderr: '' });
+  mockIsDaemon.mockReturnValue(true);
+  const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
+  const { server, port } = await startHangingSocketServer();
+  try {
+    await assert.rejects(
+      sendRequest(
+        { port, pid: 7, token: 'test-token', processStartTime: 'start' },
+        {
+          ...buildRequest('ios'),
+          session: 'e2e-ios-0',
+          command: 'record',
+          positionals: ['stop'],
+        },
+        'socket',
+        dummyStatePaths(),
+        TIMEOUT_MS,
+      ),
+      (error: unknown) => {
+        assert.ok(error instanceof AppError);
+        assert.equal(error.details?.reason, 'daemon_transport_timeout');
+        assert.match(
+          error.details?.hint as string,
+          /^The daemon may still be exporting the recording\. Run agent-device record stop --session e2e-ios-0 again/,
+        );
+        return true;
+      },
+    );
+  } finally {
+    server.close();
+  }
+  assert.equal(mockRunCmdSync.mock.calls.length, 0);
+  assert.equal(kill.mock.calls.length, 0);
+  assert.equal(mockStop.mock.calls.length, 0);
+});

src/daemon-client/__tests__/daemon-client-timeout.test.ts

@@ -97,7 +97,7 @@ test('request timeout hint only names Apple runner cleanup on actual evidence',
   );
 });
 
-test('a timed-out remote recording names the retry that returns the export', () => {
+test('a timed-out record stop on a surviving daemon names the retry that returns the export', () => {
   assert.equal(
     resolveRequestTimeoutHint({
       remote: true,
@@ -107,7 +107,7 @@ test('a timed-out remote recording names the retry that returns the export', ()
       action: 'stop',
       session: 'recording',
     }),
-    'The remote daemon is still exporting the recording. Run agent-device record stop --session recording again to wait for that export and receive the completed recording.',
+    'The remote daemon may still be exporting the recording. Run agent-device record stop --session recording again to wait for that export and receive the completed recording.',
   );
   assert.equal(
     resolveRequestTimeoutHint({
@@ -117,9 +117,21 @@ test('a timed-out remote recording names the retry that returns the export', ()
       appleCleanupEvidence: false,
       action: 'stop',
     }),
-    'The remote daemon is still exporting the recording. Run agent-device record stop again to wait for that export and receive the completed recording.',
+    'The remote daemon may still be exporting the recording. Run agent-device record stop again to wait for that export and receive the completed recording.',
   );
-  // A local timeout resets the daemon mid-export, so no keep-exporting promise is made.
+  // A local daemon preserved across the timeout may still be exporting too.
+  assert.equal(
+    resolveRequestTimeoutHint({
+      remote: false,
+      resetDaemon: false,
+      command: 'record',
+      appleCleanupEvidence: true,
+      action: 'stop',
+      session: 'recording',
+    }),
+    'The daemon may still be exporting the recording. Run agent-device record stop --session recording again to wait for that export and receive the completed recording.',
+  );
+  // A reset daemon is no longer exporting, so no keep-exporting promise is made.
   assert.equal(
     resolveRequestTimeoutHint({
       remote: false,

src/daemon-client/daemon-client-timeout.ts

@@ -56,8 +56,8 @@ export function handleRequestTimeout(
 ): AppError {
   const { info, statePaths, remote, timeoutMs, requestId, command, platform, session, action } =
     params;
-  // Cleanup eligibility stays UNCONDITIONAL for every local (non-remote)
-  // timeout, on purpose: the request's declared --platform is not
+  // Cleanup eligibility never depends on the declared platform, on purpose:
+  // the request's declared --platform is not
   // authoritative for session-bound execution. An existing session's real
   // device platform can silently override a conflicting declared selector
   // (`applyStripLockPolicy` in request-lock-policy.ts, reached via
@@ -67,7 +67,10 @@ export function handleRequestTimeout(
   // Apple-process-name-specific, so sweeping them on a non-Apple host or
   // session matches nothing and costs a few no-op subprocess spawns, never
   // a wrong skip.
-  const cleanup = remote ? { terminated: 0 } : cleanupTimedOutIosRunnerBuilds();
+  // `record` is excluded by command, which is authoritative: on a physical iOS device or macOS the
+  // runner is the recorder, and the sweep would kill the export the preserved daemon is finishing.
+  const sweepRunnerBuilds = !remote && command !== PUBLIC_COMMANDS.record;
+  const cleanup = sweepRunnerBuilds ? cleanupTimedOutIosRunnerBuilds() : { terminated: 0 };
   const resetDaemon = !remote && shouldResetDaemonAfterRequestTimeout(command);
   const daemonReset = resetDaemon
     ? resetDaemonAfterTimeout(info, statePaths)
@@ -136,15 +139,15 @@ export function resolveRequestTimeoutHint(params: {
   session?: string;
 }): string {
   const { remote, resetDaemon, command, appleCleanupEvidence, session, action } = params;
+  // A daemon that survives this client window may still be exporting a `record stop` that ran out
+  // of time (a stop still queued for the device lock is dropped before any export starts), and a
+  // finished file stays retrievable by asking again. A reset daemon makes no such promise.
+  if (!resetDaemon && command === PUBLIC_COMMANDS.record && action === 'stop') {
+    return `The ${remote ? 'remote ' : ''}daemon may still be exporting the recording. Run agent-device record stop${
+      session ? ` --session ${session}` : ''
+    } again to wait for that export and receive the completed recording.`;
+  }
   if (remote) {
-    // A remote daemon survives this client window, so a `record stop` that ran out of time is still
-    // exporting there and its finished file stays retrievable by asking again. A local timeout
-    // resets the daemon mid-export, where that promise would be false.
-    if (command === PUBLIC_COMMANDS.record && action === 'stop') {
-      return `The remote daemon is still exporting the recording. Run agent-device record stop${
-        session ? ` --session ${session}` : ''
-      } again to wait for that export and receive the completed recording.`;
-    }
     return 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.';
   }
   if (!resetDaemon) {

website/docs/docs/commands.md

@@ -1093,7 +1093,7 @@ agent-device record stop                # Stop active recording
 - Android uses `adb shell screenrecord`, which has a 180s platform limit. `record start` publishes a durable device manifest. Longer recordings are split into MP4 chunks while the daemon stays alive; after daemon restart, `record stop` recovers only manifest-owned chunks and warns when gesture overlay telemetry was lost.
 - Android `screenrecord` encodes a frame only when the screen changes, so a clip ends at the last frame the recorder encoded instead of at `record stop`: a window that ends on an unchanged screen yields a shorter video, while every on-screen change inside the window stays at its real offset in it. `record stop` reports `durationMs` as host wall clock from `record start` until the export finished, and when the video can be measured it also reports `capturedDurationMs` and warns with how much of the window that video covers.
 - Limrun iOS and Android direct sessions record the whole simulator or emulator screen through the provider's server-side recorder, so every `--scope` captures the same frame and `--fps` and `--hide-touches` are refused with `INVALID_ARGS` before any device work. `record stop` asks the instance to stop once and then downloads the served MP4 to the output path; that download is bounded to end inside the request window, so a slow or dropped transfer ends typed, leaves no file behind, and is retried by the next `record stop` from the same URL while the instance lives. Nothing survives a daemon restart: the recording is `unreattachable` and the instance disposes the file when the lease is released.
-- `record stop` is safe to repeat. When its request window ends while the daemon is still exporting — typical for a long touch-overlay burn-in on a remote daemon — the export keeps running there, and a second `record stop` in the same session returns that completed recording, including the caller-side output path, without starting another recording. A finished recording whose video file is already gone reports `no active recording`.
+- `record stop` is safe to repeat. When its request window ends while the daemon is still exporting — typical for a long touch-overlay burn-in, on a local or remote daemon — the export keeps running there, and a second `record stop` in the same session returns that completed recording, including the caller-side output path, without starting another recording. A finished recording whose video file is already gone reports `no active recording`.
 - A recording answers two independent questions, and `record stop` reports both: whether a playable export exists, and whether the recorder stopped. `recorder` is `confirmed` when the recorder exited or acknowledged a stop meant for this recording, and `lost` when the session holding it died — an Apple recording invalidated by a runner restart. `nativePathDisposition` says what became of the artifact path the recorder itself writes to: `retirable` while that file still sits there owed a removal, and `retired` once its removal was verified. Both are optional disclosures — the export is served either way, a replay of a recording stopped before they existed omits them, and so does a backend whose recorder writes the served file itself. The vocabulary is deliberately wider than today's answers: `recorder: unconfirmed` (a probe that could not be read, or no exit inside the stop budget), the identity-mismatch reasons under `lost`, and `nativePathDisposition: pending` are declared by [ADR 0024](https://github.com/callstack/agent-device/blob/main/docs/adr/0024-screen-recording-provable-signal.md) for the steps that gain those probes, and no stop reports them yet.
 
 - `record contact-sheet <video.mp4> [--out <sheet.png>]` turns a recording you already exported into one PNG: the frames where the screen visibly changed, laid out in a grid and each labeled with its elapsed time (`HH:MM:SS.mmm`) on the clip timeline. It is how an agent reads a recording it cannot play. It reads the file you pass — no session, no device, no daemon — so it can rebuild an old take and the sheet can only describe screens that export contains. Any backend that exports MP4 works (Apple, Android, HarmonyOS, Limrun); a WebM recording from the web backend is refused with `details.reason: contact_sheet_container_unsupported`. The default output is `<recording>.contact-sheet.png`, and `--out` is refused when it points back at the recording itself, because a sheet is derived from a take and never replaces it. `--json` reports each cell's time and changed-pixel share alongside `sampledFrameCount`, `decodedFrameCount`, and `skippedSampleCount`.