Skip to content

Commit f2f48a8

Browse files
pvedula7pvedula7
committed
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.
1 parent d396f3b commit f2f48a8

5 files changed

Lines changed: 66 additions & 11 deletions

File tree

‎packages/command-registry/src/registry.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1234,7 +1234,10 @@ export const RAW_COMMAND_DESCRIPTORS = [
12341234
allowSessionlessDefaultDevice: isRecordingStartRequest,
12351235
},
12361236
platformExecution: { kind: 'device-runtime', uses: screenRecordingRuntimePlanUses },
1237-
timeoutPolicy: DEFAULT_TIMEOUT_POLICY,
1237+
// A `record stop` export can outlast the request envelope. Resetting the daemon mid-export
1238+
// left its recording manifest open with no owner, and every later `record start` on the
1239+
// device refused until that exact session ran `record stop`.
1240+
timeoutPolicy: PRESERVE_DAEMON_TIMEOUT_POLICY,
12381241
batchable: true,
12391242
},
12401243
{

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => {
7171
// sessions the daemon owns, so a client-side timeout must not SIGKILL the
7272
// daemon mid-create/mid-release and orphan them (and every other provider
7373
// session held).
74+
// record joined because a `record stop` export can outlast the envelope: a
75+
// reset mid-export left an ownerless open manifest that refused every later
76+
// `record start` on the device.
7477
const preserving = commandDescriptors
7578
.filter((descriptor) => descriptor.timeoutPolicy.onTimeout === 'preserve-daemon')
7679
.map((descriptor) => descriptor.name);
@@ -87,6 +90,7 @@ test('daemon-preserving timeout commands are a bounded, reviewed set', () => {
8790
'lease_release',
8891
'longpress',
8992
'press',
93+
'record',
9094
'scroll',
9195
'snapshot',
9296
'type',

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,3 +308,39 @@ test('a refused timeout fallback preserves the timeout without an unhandled reje
308308
server.close();
309309
}
310310
});
311+
312+
test('a local record stop timeout keeps the exporting daemon alive and names the retry', async () => {
313+
mockRunCmdSync.mockReturnValue({ exitCode: 1, stdout: '', stderr: '' });
314+
mockIsDaemon.mockReturnValue(true);
315+
const kill = vi.spyOn(process, 'kill').mockImplementation(() => true);
316+
const { server, port } = await startHangingSocketServer();
317+
try {
318+
await assert.rejects(
319+
sendRequest(
320+
{ port, pid: 7, token: 'test-token', processStartTime: 'start' },
321+
{
322+
...buildRequest('ios'),
323+
session: 'e2e-ios-0',
324+
command: 'record',
325+
positionals: ['stop'],
326+
},
327+
'socket',
328+
dummyStatePaths(),
329+
TIMEOUT_MS,
330+
),
331+
(error: unknown) => {
332+
assert.ok(error instanceof AppError);
333+
assert.equal(error.details?.reason, 'daemon_transport_timeout');
334+
assert.match(
335+
error.details?.hint as string,
336+
/^The daemon is still exporting the recording\. Run agent-device record stop --session e2e-ios-0 again/,
337+
);
338+
return true;
339+
},
340+
);
341+
} finally {
342+
server.close();
343+
}
344+
assert.equal(kill.mock.calls.length, 0);
345+
assert.equal(mockStop.mock.calls.length, 0);
346+
});

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ test('request timeout hint only names Apple runner cleanup on actual evidence',
9797
);
9898
});
9999

100-
test('a timed-out remote recording names the retry that returns the export', () => {
100+
test('a timed-out record stop on a surviving daemon names the retry that returns the export', () => {
101101
assert.equal(
102102
resolveRequestTimeoutHint({
103103
remote: true,
@@ -119,7 +119,19 @@ test('a timed-out remote recording names the retry that returns the export', ()
119119
}),
120120
'The remote daemon is still exporting the recording. Run agent-device record stop again to wait for that export and receive the completed recording.',
121121
);
122-
// A local timeout resets the daemon mid-export, so no keep-exporting promise is made.
122+
// A local daemon preserved across the timeout is still exporting too.
123+
assert.equal(
124+
resolveRequestTimeoutHint({
125+
remote: false,
126+
resetDaemon: false,
127+
command: 'record',
128+
appleCleanupEvidence: true,
129+
action: 'stop',
130+
session: 'recording',
131+
}),
132+
'The daemon is still exporting the recording. Run agent-device record stop --session recording again to wait for that export and receive the completed recording.',
133+
);
134+
// A reset daemon is no longer exporting, so no keep-exporting promise is made.
123135
assert.equal(
124136
resolveRequestTimeoutHint({
125137
remote: false,

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,15 +136,15 @@ export function resolveRequestTimeoutHint(params: {
136136
session?: string;
137137
}): string {
138138
const { remote, resetDaemon, command, appleCleanupEvidence, session, action } = params;
139+
// A daemon that survives this client window is still exporting a `record stop` that ran out of
140+
// time, and its finished file stays retrievable by asking again. A reset daemon makes no such
141+
// promise.
142+
if (!resetDaemon && command === PUBLIC_COMMANDS.record && action === 'stop') {
143+
return `The ${remote ? 'remote ' : ''}daemon is still exporting the recording. Run agent-device record stop${
144+
session ? ` --session ${session}` : ''
145+
} again to wait for that export and receive the completed recording.`;
146+
}
139147
if (remote) {
140-
// A remote daemon survives this client window, so a `record stop` that ran out of time is still
141-
// exporting there and its finished file stays retrievable by asking again. A local timeout
142-
// resets the daemon mid-export, where that promise would be false.
143-
if (command === PUBLIC_COMMANDS.record && action === 'stop') {
144-
return `The remote daemon is still exporting the recording. Run agent-device record stop${
145-
session ? ` --session ${session}` : ''
146-
} again to wait for that export and receive the completed recording.`;
147-
}
148148
return 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.';
149149
}
150150
if (!resetDaemon) {

0 commit comments

Comments
 (0)