Skip to content

Commit b27aea3

Browse files
committed
fix: improve session recovery guidance
1 parent 6247a3d commit b27aea3

16 files changed

Lines changed: 469 additions & 17 deletions

src/daemon-client.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,11 +1765,23 @@ export function resolveDaemonStartupHint(
17651765
process.env.AGENT_DEVICE_STATE_DIR,
17661766
),
17671767
): string {
1768+
const cleanupCommand = buildDaemonMetadataCleanupCommand(paths);
17681769
if (state.hasLock && !state.hasInfo) {
1769-
return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.lockPath} still exists without ${paths.infoPath}. Retry with --debug; if this persists, remove ${paths.lockPath} after confirming no agent-device daemon process is running.`;
1770+
return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.lockPath} still exists without ${paths.infoPath}. Retry with --debug; if this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`;
17701771
}
17711772
if (state.hasLock && state.hasInfo) {
1772-
return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.infoPath} and ${paths.lockPath} still remain. Retry with --debug; if this persists, remove both files after confirming no agent-device daemon process is running.`;
1773+
return `agent-device attempted to clean stale daemon metadata automatically, but ${paths.infoPath} and ${paths.lockPath} still remain. Retry with --debug; if this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`;
17731774
}
1774-
return `agent-device did not observe reachable daemon metadata after retrying. Stale metadata was cleaned automatically when safe; retry with --debug and check daemon diagnostics logs.`;
1775+
if (state.hasInfo) {
1776+
return `agent-device did not observe reachable daemon metadata after retrying, and ${paths.infoPath} still remains. Stale metadata was cleaned automatically when safe; retry with --debug. If this persists after confirming no agent-device daemon process is running, run: ${cleanupCommand}`;
1777+
}
1778+
return `agent-device did not observe reachable daemon metadata after retrying. Stale metadata was cleaned automatically when safe; retry with --debug and check daemon diagnostics logs. If stale metadata returns after confirming no agent-device daemon process is running, run: ${cleanupCommand}`;
1779+
}
1780+
1781+
function buildDaemonMetadataCleanupCommand(paths: Pick<DaemonPaths, 'infoPath' | 'lockPath'>) {
1782+
return `rm -f ${shellQuote(paths.infoPath)} ${shellQuote(paths.lockPath)}`;
1783+
}
1784+
1785+
function shellQuote(value: string): string {
1786+
return `'${value.replaceAll("'", "'\\''")}'`;
17751787
}

src/daemon/__tests__/request-execution-scope.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ test('prepareLockedRequestScope preserves existing-session selector validation',
8888
sessionStore,
8989
trackDownloadableArtifact: () => 'artifact-id',
9090
}),
91-
).toThrow(/cannot be used with --platform=ios/i);
91+
).toThrow(/already bound to android device "Pixel" \(emulator-5554\).*--platform=ios/i);
9292
});
9393

9494
test('prepareLockedRequestScope blocks commands for invalidated recordings before handlers run', async () => {
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { test, expect } from 'vitest';
2+
import { finalizeDaemonResponse } from '../request-finalization.ts';
3+
import type { DaemonRequest, DaemonResponse } from '../types.ts';
4+
5+
test('finalizeDaemonResponse preserves handler error hints from details', () => {
6+
const req: DaemonRequest = {
7+
token: 'token',
8+
session: 'default',
9+
command: 'open',
10+
positionals: [],
11+
flags: {},
12+
};
13+
const response: DaemonResponse = {
14+
ok: false,
15+
error: {
16+
code: 'DEVICE_IN_USE',
17+
message: 'Device is already in use by session "default".',
18+
details: {
19+
session: 'default',
20+
hint: 'Run agent-device session list and reuse --session default.',
21+
},
22+
},
23+
};
24+
25+
const finalized = finalizeDaemonResponse(req, response, () => 'artifact-id');
26+
27+
expect(finalized.ok).toBe(false);
28+
if (!finalized.ok) {
29+
expect(finalized.error.hint).toBe('Run agent-device session list and reuse --session default.');
30+
}
31+
});

src/daemon/__tests__/request-lock-policy.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { test } from 'vitest';
22
import assert from 'node:assert/strict';
33
import { applyRequestLockPolicy } from '../request-lock-policy.ts';
44
import type { SessionState } from '../types.ts';
5+
import { AppError } from '../../utils/errors.ts';
56

67
const IOS_SESSION: SessionState = {
78
name: 'qa-ios',
@@ -32,6 +33,21 @@ const ANDROID_SESSION: SessionState = {
3233
},
3334
};
3435

36+
const RECORDING_SESSION: SessionState = {
37+
...ANDROID_SESSION,
38+
name: 'default',
39+
recordingSession: true,
40+
recording: {
41+
platform: 'android',
42+
outPath: '/tmp/recording.mp4',
43+
remotePath: '/sdcard/recording.mp4',
44+
remotePid: '1234',
45+
startedAt: Date.now(),
46+
showTouches: false,
47+
gestureEvents: [],
48+
},
49+
};
50+
3551
test('rejects fresh-session selector conflicts under request lock policy', () => {
3652
assert.throws(
3753
() =>
@@ -52,6 +68,37 @@ test('rejects fresh-session selector conflicts under request lock policy', () =>
5268
);
5369
});
5470

71+
test('reject lock policy explains fresh-session recovery commands', () => {
72+
assert.throws(
73+
() =>
74+
applyRequestLockPolicy({
75+
token: 'token',
76+
session: 'qa-ios',
77+
command: 'snapshot',
78+
positionals: [],
79+
flags: {
80+
device: 'Pixel 9',
81+
},
82+
meta: {
83+
lockPolicy: 'reject',
84+
lockPlatform: 'ios',
85+
},
86+
}),
87+
(error) => {
88+
assert.ok(error instanceof AppError);
89+
assert.match(error.message, /snapshot is using a bound-session lock for ios/i);
90+
assert.match(error.message, /--device=Pixel 9/i);
91+
assert.match(error.details?.hint ?? '', /Remove conflicting device selectors/i);
92+
assert.match(
93+
error.details?.hint ?? '',
94+
/agent-device open <app> --session qa-ios --platform ios/i,
95+
);
96+
assert.match(error.details?.hint ?? '', /agent-device session list/i);
97+
return true;
98+
},
99+
);
100+
});
101+
55102
test('allows open to choose a fresh-session target under request lock policy', () => {
56103
const req = applyRequestLockPolicy({
57104
token: 'token',
@@ -118,6 +165,66 @@ test('rejects existing-session selector conflicts under request lock policy', ()
118165
);
119166
});
120167

168+
test('reject lock policy explains existing-session recovery commands', () => {
169+
assert.throws(
170+
() =>
171+
applyRequestLockPolicy(
172+
{
173+
token: 'token',
174+
session: 'qa-ios',
175+
command: 'snapshot',
176+
positionals: [],
177+
flags: {
178+
serial: 'emulator-5554',
179+
},
180+
meta: {
181+
lockPolicy: 'reject',
182+
},
183+
},
184+
IOS_SESSION,
185+
),
186+
(error) => {
187+
assert.ok(error instanceof AppError);
188+
assert.match(error.message, /locked to session "qa-ios"/i);
189+
assert.match(error.message, /ios device "iPhone 16" \(SIM-001\)/i);
190+
assert.match(error.message, /--serial=emulator-5554/i);
191+
assert.match(error.details?.hint ?? '', /agent-device session list/i);
192+
assert.match(error.details?.hint ?? '', /--session qa-ios/i);
193+
assert.match(error.details?.hint ?? '', /agent-device close --session qa-ios/i);
194+
return true;
195+
},
196+
);
197+
});
198+
199+
test('reject lock policy explains recording-session recovery commands', () => {
200+
assert.throws(
201+
() =>
202+
applyRequestLockPolicy(
203+
{
204+
token: 'token',
205+
session: 'default',
206+
command: 'snapshot',
207+
positionals: [],
208+
flags: {
209+
device: 'Pixel 8',
210+
},
211+
meta: {
212+
lockPolicy: 'reject',
213+
},
214+
},
215+
RECORDING_SESSION,
216+
),
217+
(error) => {
218+
assert.ok(error instanceof AppError);
219+
assert.match(error.message, /locked to session "default"/i);
220+
assert.match(error.details?.hint ?? '', /recording session "default"/i);
221+
assert.match(error.details?.hint ?? '', /agent-device record stop --session default/i);
222+
assert.match(error.details?.hint ?? '', /agent-device close --session default/i);
223+
return true;
224+
},
225+
);
226+
});
227+
121228
test('allows inventory commands to use explicit Apple selectors under another lock platform', () => {
122229
const req = applyRequestLockPolicy({
123230
token: 'token',

src/daemon/__tests__/request-router-lock-policy.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ test('direct daemon requests cannot bypass reject lock policy for existing sessi
6767
if (!response.ok) {
6868
expect(response.error.code).toBe('INVALID_ARGS');
6969
expect(response.error.message).toMatch(/--udid=SIM-999/i);
70+
expect(response.error.hint).toMatch(/agent-device session list/i);
71+
expect(response.error.hint).toMatch(/agent-device close --session qa-ios/i);
7072
}
7173
});
7274

@@ -108,6 +110,8 @@ test('batch steps cannot bypass reject lock policy on nested direct requests', a
108110
expect(response.error.code).toBe('INVALID_ARGS');
109111
expect(response.error.message).toMatch(/Batch failed at step 1/i);
110112
expect(response.error.message).toMatch(/--serial=emulator-5554/i);
113+
expect(response.error.hint).toMatch(/agent-device session list/i);
114+
expect(response.error.hint).toMatch(/agent-device close --session qa-ios/i);
111115
}
112116
});
113117

src/daemon/__tests__/session-selector.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@ test('rejects mismatched platform selector', () => {
4343
);
4444
});
4545

46+
test('selector mismatch explains session recovery commands', () => {
47+
const session = makeSession();
48+
assert.throws(
49+
() => assertSessionSelectorMatches(session, { platform: 'ios' }),
50+
(err: unknown) => {
51+
assert.ok(err instanceof AppError);
52+
assert.equal(err.code, 'INVALID_ARGS');
53+
assert.match(err.message, /Session "default" is already bound to android device "Pixel 9"/i);
54+
assert.match(err.message, /--platform=ios/i);
55+
assert.match(err.details?.hint ?? '', /agent-device session list/i);
56+
assert.match(err.details?.hint ?? '', /--session default/i);
57+
assert.match(err.details?.hint ?? '', /agent-device close --session default/i);
58+
return true;
59+
},
60+
);
61+
});
62+
4663
test('accepts --platform apple alias for ios sessions', () => {
4764
const session = makeSession({
4865
device: {

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

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,94 @@ test('record stop derives telemetry artifact local path from client outPath', as
226226
expect((responseStop as any).data?.telemetryPath).toBe(deriveRecordingTelemetryPath(finalOut));
227227
});
228228

229+
test('record stop releases session created only for recording', async () => {
230+
const sessionStore = makeSessionStore();
231+
const sessionName = 'record-only-session';
232+
const session = makeSession(sessionName, {
233+
platform: 'ios',
234+
id: 'ios-sim-1',
235+
name: 'iPhone 16',
236+
kind: 'simulator',
237+
booted: true,
238+
});
239+
session.recordingSession = true;
240+
session.recording = {
241+
platform: 'ios',
242+
child: { kill: vi.fn(), pid: 123 },
243+
wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
244+
outPath: path.join(os.tmpdir(), 'record-only.mp4'),
245+
startedAt: Date.now(),
246+
showTouches: false,
247+
gestureEvents: [],
248+
};
249+
sessionStore.set(sessionName, session);
250+
251+
const response = await runRecordCommand({
252+
sessionStore,
253+
sessionName,
254+
positionals: ['stop'],
255+
});
256+
257+
expect(response?.ok).toBe(true);
258+
expect(sessionStore.get(sessionName)).toBeUndefined();
259+
});
260+
261+
test('record stop releases record-only session even when recording state is stale', async () => {
262+
const sessionStore = makeSessionStore();
263+
const sessionName = 'stale-record-only-session';
264+
const session = makeSession(sessionName, {
265+
platform: 'ios',
266+
id: 'ios-sim-1',
267+
name: 'iPhone 16',
268+
kind: 'simulator',
269+
booted: true,
270+
});
271+
session.recordingSession = true;
272+
sessionStore.set(sessionName, session);
273+
274+
const response = await runRecordCommand({
275+
sessionStore,
276+
sessionName,
277+
positionals: ['stop'],
278+
});
279+
280+
expect(response?.ok).toBe(false);
281+
expect(sessionStore.get(sessionName)).toBeUndefined();
282+
});
283+
284+
test('record stop keeps normal app session open after recording', async () => {
285+
const sessionStore = makeSessionStore();
286+
const sessionName = 'app-session';
287+
const session = makeSession(sessionName, {
288+
platform: 'ios',
289+
id: 'ios-sim-1',
290+
name: 'iPhone 16',
291+
kind: 'simulator',
292+
booted: true,
293+
});
294+
session.appBundleId = 'com.apple.Preferences';
295+
session.recording = {
296+
platform: 'ios',
297+
child: { kill: vi.fn(), pid: 123 },
298+
wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }),
299+
outPath: path.join(os.tmpdir(), 'app-session.mp4'),
300+
startedAt: Date.now(),
301+
showTouches: false,
302+
gestureEvents: [],
303+
};
304+
sessionStore.set(sessionName, session);
305+
306+
const response = await runRecordCommand({
307+
sessionStore,
308+
sessionName,
309+
positionals: ['stop'],
310+
});
311+
312+
expect(response?.ok).toBe(true);
313+
expect(sessionStore.get(sessionName)).toBe(session);
314+
expect(sessionStore.get(sessionName)?.recording).toBeUndefined();
315+
});
316+
229317
test('record start resolves relative output path from request cwd', async () => {
230318
const sessionStore = makeSessionStore();
231319
const sessionName = 'ios-device-cwd';

0 commit comments

Comments
 (0)