From b27aea371772bfbd0417e0bbcdee10ef6188d683 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 09:59:03 -0700 Subject: [PATCH 1/9] fix: improve session recovery guidance --- src/daemon-client.ts | 18 ++- .../__tests__/request-execution-scope.test.ts | 2 +- .../__tests__/request-finalization.test.ts | 31 +++++ .../__tests__/request-lock-policy.test.ts | 107 ++++++++++++++++++ .../request-router-lock-policy.test.ts | 4 + src/daemon/__tests__/session-selector.test.ts | 17 +++ .../handlers/__tests__/record-trace.test.ts | 88 ++++++++++++++ src/daemon/handlers/__tests__/session.test.ts | 56 +++++++++ src/daemon/handlers/record-trace-recording.ts | 17 +++ src/daemon/handlers/session-open.ts | 3 +- src/daemon/request-finalization.ts | 6 +- src/daemon/request-lock-policy.ts | 45 +++++++- src/daemon/session-recovery-hints.ts | 58 ++++++++++ src/daemon/session-selector.ts | 15 ++- src/daemon/types.ts | 1 + src/utils/__tests__/daemon-client.test.ts | 18 ++- 16 files changed, 469 insertions(+), 17 deletions(-) create mode 100644 src/daemon/__tests__/request-finalization.test.ts create mode 100644 src/daemon/session-recovery-hints.ts diff --git a/src/daemon-client.ts b/src/daemon-client.ts index 0af33e5f0..dec127437 100644 --- a/src/daemon-client.ts +++ b/src/daemon-client.ts @@ -1765,11 +1765,23 @@ export function resolveDaemonStartupHint( process.env.AGENT_DEVICE_STATE_DIR, ), ): string { + const cleanupCommand = buildDaemonMetadataCleanupCommand(paths); if (state.hasLock && !state.hasInfo) { - 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.`; + 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}`; } if (state.hasLock && state.hasInfo) { - 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.`; + 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}`; } - 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 (state.hasInfo) { + 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}`; + } + 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}`; +} + +function buildDaemonMetadataCleanupCommand(paths: Pick) { + return `rm -f ${shellQuote(paths.infoPath)} ${shellQuote(paths.lockPath)}`; +} + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'`; } diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index fd2052c43..e20e98eef 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -88,7 +88,7 @@ test('prepareLockedRequestScope preserves existing-session selector validation', sessionStore, trackDownloadableArtifact: () => 'artifact-id', }), - ).toThrow(/cannot be used with --platform=ios/i); + ).toThrow(/already bound to android device "Pixel" \(emulator-5554\).*--platform=ios/i); }); test('prepareLockedRequestScope blocks commands for invalidated recordings before handlers run', async () => { diff --git a/src/daemon/__tests__/request-finalization.test.ts b/src/daemon/__tests__/request-finalization.test.ts new file mode 100644 index 000000000..8eeba9a37 --- /dev/null +++ b/src/daemon/__tests__/request-finalization.test.ts @@ -0,0 +1,31 @@ +import { test, expect } from 'vitest'; +import { finalizeDaemonResponse } from '../request-finalization.ts'; +import type { DaemonRequest, DaemonResponse } from '../types.ts'; + +test('finalizeDaemonResponse preserves handler error hints from details', () => { + const req: DaemonRequest = { + token: 'token', + session: 'default', + command: 'open', + positionals: [], + flags: {}, + }; + const response: DaemonResponse = { + ok: false, + error: { + code: 'DEVICE_IN_USE', + message: 'Device is already in use by session "default".', + details: { + session: 'default', + hint: 'Run agent-device session list and reuse --session default.', + }, + }, + }; + + const finalized = finalizeDaemonResponse(req, response, () => 'artifact-id'); + + expect(finalized.ok).toBe(false); + if (!finalized.ok) { + expect(finalized.error.hint).toBe('Run agent-device session list and reuse --session default.'); + } +}); diff --git a/src/daemon/__tests__/request-lock-policy.test.ts b/src/daemon/__tests__/request-lock-policy.test.ts index d6b54505d..0844805ec 100644 --- a/src/daemon/__tests__/request-lock-policy.test.ts +++ b/src/daemon/__tests__/request-lock-policy.test.ts @@ -2,6 +2,7 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { applyRequestLockPolicy } from '../request-lock-policy.ts'; import type { SessionState } from '../types.ts'; +import { AppError } from '../../utils/errors.ts'; const IOS_SESSION: SessionState = { name: 'qa-ios', @@ -32,6 +33,21 @@ const ANDROID_SESSION: SessionState = { }, }; +const RECORDING_SESSION: SessionState = { + ...ANDROID_SESSION, + name: 'default', + recordingSession: true, + recording: { + platform: 'android', + outPath: '/tmp/recording.mp4', + remotePath: '/sdcard/recording.mp4', + remotePid: '1234', + startedAt: Date.now(), + showTouches: false, + gestureEvents: [], + }, +}; + test('rejects fresh-session selector conflicts under request lock policy', () => { assert.throws( () => @@ -52,6 +68,37 @@ test('rejects fresh-session selector conflicts under request lock policy', () => ); }); +test('reject lock policy explains fresh-session recovery commands', () => { + assert.throws( + () => + applyRequestLockPolicy({ + token: 'token', + session: 'qa-ios', + command: 'snapshot', + positionals: [], + flags: { + device: 'Pixel 9', + }, + meta: { + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }), + (error) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /snapshot is using a bound-session lock for ios/i); + assert.match(error.message, /--device=Pixel 9/i); + assert.match(error.details?.hint ?? '', /Remove conflicting device selectors/i); + assert.match( + error.details?.hint ?? '', + /agent-device open --session qa-ios --platform ios/i, + ); + assert.match(error.details?.hint ?? '', /agent-device session list/i); + return true; + }, + ); +}); + test('allows open to choose a fresh-session target under request lock policy', () => { const req = applyRequestLockPolicy({ token: 'token', @@ -118,6 +165,66 @@ test('rejects existing-session selector conflicts under request lock policy', () ); }); +test('reject lock policy explains existing-session recovery commands', () => { + assert.throws( + () => + applyRequestLockPolicy( + { + token: 'token', + session: 'qa-ios', + command: 'snapshot', + positionals: [], + flags: { + serial: 'emulator-5554', + }, + meta: { + lockPolicy: 'reject', + }, + }, + IOS_SESSION, + ), + (error) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /locked to session "qa-ios"/i); + assert.match(error.message, /ios device "iPhone 16" \(SIM-001\)/i); + assert.match(error.message, /--serial=emulator-5554/i); + assert.match(error.details?.hint ?? '', /agent-device session list/i); + assert.match(error.details?.hint ?? '', /--session qa-ios/i); + assert.match(error.details?.hint ?? '', /agent-device close --session qa-ios/i); + return true; + }, + ); +}); + +test('reject lock policy explains recording-session recovery commands', () => { + assert.throws( + () => + applyRequestLockPolicy( + { + token: 'token', + session: 'default', + command: 'snapshot', + positionals: [], + flags: { + device: 'Pixel 8', + }, + meta: { + lockPolicy: 'reject', + }, + }, + RECORDING_SESSION, + ), + (error) => { + assert.ok(error instanceof AppError); + assert.match(error.message, /locked to session "default"/i); + assert.match(error.details?.hint ?? '', /recording session "default"/i); + assert.match(error.details?.hint ?? '', /agent-device record stop --session default/i); + assert.match(error.details?.hint ?? '', /agent-device close --session default/i); + return true; + }, + ); +}); + test('allows inventory commands to use explicit Apple selectors under another lock platform', () => { const req = applyRequestLockPolicy({ token: 'token', diff --git a/src/daemon/__tests__/request-router-lock-policy.test.ts b/src/daemon/__tests__/request-router-lock-policy.test.ts index 36f676b60..3070fb1fb 100644 --- a/src/daemon/__tests__/request-router-lock-policy.test.ts +++ b/src/daemon/__tests__/request-router-lock-policy.test.ts @@ -67,6 +67,8 @@ test('direct daemon requests cannot bypass reject lock policy for existing sessi if (!response.ok) { expect(response.error.code).toBe('INVALID_ARGS'); expect(response.error.message).toMatch(/--udid=SIM-999/i); + expect(response.error.hint).toMatch(/agent-device session list/i); + expect(response.error.hint).toMatch(/agent-device close --session qa-ios/i); } }); @@ -108,6 +110,8 @@ test('batch steps cannot bypass reject lock policy on nested direct requests', a expect(response.error.code).toBe('INVALID_ARGS'); expect(response.error.message).toMatch(/Batch failed at step 1/i); expect(response.error.message).toMatch(/--serial=emulator-5554/i); + expect(response.error.hint).toMatch(/agent-device session list/i); + expect(response.error.hint).toMatch(/agent-device close --session qa-ios/i); } }); diff --git a/src/daemon/__tests__/session-selector.test.ts b/src/daemon/__tests__/session-selector.test.ts index c43b4a4ab..f65fe4d31 100644 --- a/src/daemon/__tests__/session-selector.test.ts +++ b/src/daemon/__tests__/session-selector.test.ts @@ -43,6 +43,23 @@ test('rejects mismatched platform selector', () => { ); }); +test('selector mismatch explains session recovery commands', () => { + const session = makeSession(); + assert.throws( + () => assertSessionSelectorMatches(session, { platform: 'ios' }), + (err: unknown) => { + assert.ok(err instanceof AppError); + assert.equal(err.code, 'INVALID_ARGS'); + assert.match(err.message, /Session "default" is already bound to android device "Pixel 9"/i); + assert.match(err.message, /--platform=ios/i); + assert.match(err.details?.hint ?? '', /agent-device session list/i); + assert.match(err.details?.hint ?? '', /--session default/i); + assert.match(err.details?.hint ?? '', /agent-device close --session default/i); + return true; + }, + ); +}); + test('accepts --platform apple alias for ios sessions', () => { const session = makeSession({ device: { diff --git a/src/daemon/handlers/__tests__/record-trace.test.ts b/src/daemon/handlers/__tests__/record-trace.test.ts index 977bcb005..d56a3134d 100644 --- a/src/daemon/handlers/__tests__/record-trace.test.ts +++ b/src/daemon/handlers/__tests__/record-trace.test.ts @@ -226,6 +226,94 @@ test('record stop derives telemetry artifact local path from client outPath', as expect((responseStop as any).data?.telemetryPath).toBe(deriveRecordingTelemetryPath(finalOut)); }); +test('record stop releases session created only for recording', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'record-only-session'; + const session = makeSession(sessionName, { + platform: 'ios', + id: 'ios-sim-1', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + }); + session.recordingSession = true; + session.recording = { + platform: 'ios', + child: { kill: vi.fn(), pid: 123 }, + wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + outPath: path.join(os.tmpdir(), 'record-only.mp4'), + startedAt: Date.now(), + showTouches: false, + gestureEvents: [], + }; + sessionStore.set(sessionName, session); + + const response = await runRecordCommand({ + sessionStore, + sessionName, + positionals: ['stop'], + }); + + expect(response?.ok).toBe(true); + expect(sessionStore.get(sessionName)).toBeUndefined(); +}); + +test('record stop releases record-only session even when recording state is stale', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'stale-record-only-session'; + const session = makeSession(sessionName, { + platform: 'ios', + id: 'ios-sim-1', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + }); + session.recordingSession = true; + sessionStore.set(sessionName, session); + + const response = await runRecordCommand({ + sessionStore, + sessionName, + positionals: ['stop'], + }); + + expect(response?.ok).toBe(false); + expect(sessionStore.get(sessionName)).toBeUndefined(); +}); + +test('record stop keeps normal app session open after recording', async () => { + const sessionStore = makeSessionStore(); + const sessionName = 'app-session'; + const session = makeSession(sessionName, { + platform: 'ios', + id: 'ios-sim-1', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + }); + session.appBundleId = 'com.apple.Preferences'; + session.recording = { + platform: 'ios', + child: { kill: vi.fn(), pid: 123 }, + wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + outPath: path.join(os.tmpdir(), 'app-session.mp4'), + startedAt: Date.now(), + showTouches: false, + gestureEvents: [], + }; + sessionStore.set(sessionName, session); + + const response = await runRecordCommand({ + sessionStore, + sessionName, + positionals: ['stop'], + }); + + expect(response?.ok).toBe(true); + expect(sessionStore.get(sessionName)).toBe(session); + expect(sessionStore.get(sessionName)?.recording).toBeUndefined(); +}); + test('record start resolves relative output path from request cwd', async () => { const sessionStore = makeSessionStore(); const sessionName = 'ios-device-cwd'; diff --git a/src/daemon/handlers/__tests__/session.test.ts b/src/daemon/handlers/__tests__/session.test.ts index 0a2dbade5..4b51541cc 100644 --- a/src/daemon/handlers/__tests__/session.test.ts +++ b/src/daemon/handlers/__tests__/session.test.ts @@ -3111,6 +3111,62 @@ test('open on in-use device returns DEVICE_IN_USE before readiness checks', asyn expect(response.error.code).toBe('DEVICE_IN_USE'); expect(response.error.details?.hint).toContain('agent-device session list'); expect(response.error.details?.hint).toContain('--session busy-session'); + expect(response.error.details?.hint).toContain('agent-device close --session busy-session'); + } + expect(mockEnsureDeviceReady).not.toHaveBeenCalled(); +}); + +test('open on device owned by recording session returns recording recovery hint', async () => { + const sessionStore = makeSessionStore(); + const recordingSession = makeSession('default', { + platform: 'ios', + id: 'ios-device-1', + name: 'iPhone Device', + kind: 'device', + booted: true, + }); + recordingSession.recordingSession = true; + recordingSession.recording = { + platform: 'ios', + child: { kill: vi.fn(), pid: 123 }, + wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + outPath: '/tmp/recording.mp4', + startedAt: Date.now(), + showTouches: false, + gestureEvents: [], + }; + sessionStore.set('default', recordingSession); + + mockResolveTargetDevice.mockResolvedValue({ + platform: 'ios', + id: 'ios-device-1', + name: 'iPhone Device', + kind: 'device', + booted: true, + }); + + const response = await handleSessionCommands({ + req: { + token: 't', + session: 'test-attempt', + command: 'open', + positionals: ['settings'], + flags: { platform: 'ios' }, + }, + sessionName: 'test-attempt', + logPath: path.join(os.tmpdir(), 'daemon.log'), + sessionStore, + invoke: noopInvoke, + }); + + expect(response).toBeTruthy(); + expect(response?.ok).toBe(false); + if (response && !response.ok) { + expect(response.error.code).toBe('DEVICE_IN_USE'); + expect(response.error.details?.hint).toContain('Recording session "default" owns this device'); + expect(response.error.details?.hint).toContain('agent-device record stop --session default'); + expect(response.error.details?.hint).toContain('agent-device close --session default'); + expect(response.error.details?.hint).toContain('agent-device session list'); } expect(mockEnsureDeviceReady).not.toHaveBeenCalled(); }); diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index d7c3700c8..3e6c428be 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -529,6 +529,20 @@ function deriveClientTelemetryPath( return deriveRecordingTelemetryPath(recording.clientOutPath); } +function releaseRecordOnlySession( + sessionStore: SessionStore, + session: SessionState, + options: { writeLog?: boolean } = {}, +): void { + if (!session.recordingSession) { + return; + } + if (options.writeLog) { + sessionStore.writeSessionLog(session); + } + sessionStore.delete(session.name); +} + // --- Main command handler --- export async function handleRecordCommand(params: { @@ -551,6 +565,7 @@ export async function handleRecordCommand(params: { name: sessionName, device, createdAt: Date.now(), + recordingSession: true, actions: [], } satisfies SessionState); @@ -565,6 +580,7 @@ export async function handleRecordCommand(params: { const response = await stopRecording({ req, activeSession, device, logPath, deps }); if (!response.ok) { + releaseRecordOnlySession(sessionStore, activeSession); return response; } @@ -578,5 +594,6 @@ export async function handleRecordCommand(params: { showTouches: response.data?.showTouches, }, }); + releaseRecordOnlySession(sessionStore, activeSession, { writeLog: true }); return response; } diff --git a/src/daemon/handlers/session-open.ts b/src/daemon/handlers/session-open.ts index 03fe3b302..cbb0b7838 100644 --- a/src/daemon/handlers/session-open.ts +++ b/src/daemon/handlers/session-open.ts @@ -32,6 +32,7 @@ import { validateResolvedOpenRequest, } from './session-open-prepare.ts'; import { errorResponse } from './response.ts'; +import { buildSessionRecoveryHint } from '../session-recovery-hints.ts'; const firstSessionOpenLocks = new Map>(); @@ -387,7 +388,7 @@ export async function handleOpenCommand(params: { session: inUse.name, deviceId: device.id, deviceName: device.name, - hint: `Run agent-device session list and reuse --session ${inUse.name}, or close that session before opening a new one on this device.`, + hint: buildSessionRecoveryHint(inUse, 'device-in-use'), }, ); } diff --git a/src/daemon/request-finalization.ts b/src/daemon/request-finalization.ts index 02df86315..5668b8d7d 100644 --- a/src/daemon/request-finalization.ts +++ b/src/daemon/request-finalization.ts @@ -26,7 +26,11 @@ export function finalizeDaemonResponse( const normalizedError = normalizeError( new AppError(toAppErrorCode(response.error.code), response.error.message, { ...(response.error.details ?? {}), - hint: response.error.hint, + hint: + response.error.hint ?? + (typeof response.error.details?.hint === 'string' + ? response.error.details.hint + : undefined), diagnosticId: response.error.diagnosticId, logPath: response.error.logPath, }), diff --git a/src/daemon/request-lock-policy.ts b/src/daemon/request-lock-policy.ts index 85bdc6e23..9f47077b5 100644 --- a/src/daemon/request-lock-policy.ts +++ b/src/daemon/request-lock-policy.ts @@ -9,6 +9,7 @@ import { type SessionSelectorConflictKey, } from './session-selector.ts'; import { isApplePlatform, normalizePlatformSelector } from '../utils/device.ts'; +import { buildSessionRecoveryHint, describeSessionDevice } from './session-recovery-hints.ts'; type LockPlatform = NonNullable['lockPlatform']; @@ -66,8 +67,48 @@ export function applyRequestLockPolicy( throw new AppError( 'INVALID_ARGS', - `${req.command} cannot override session lock policy with ${conflicts.map(formatSessionSelectorConflict).join(', ')}. ` + - 'Unset those selectors or remove the request lock policy.', + buildLockPolicyConflictMessage(req, conflicts, existingSession), + { + session: req.session, + conflicts: conflicts.map(formatSessionSelectorConflict), + hint: buildLockPolicyConflictHint(req, existingSession), + }, + ); +} + +function buildLockPolicyConflictMessage( + req: DaemonRequest, + conflicts: SessionSelectorConflict[], + existingSession: SessionState | undefined, +): string { + const conflictList = conflicts.map(formatSessionSelectorConflict).join(', '); + if (existingSession) { + return ( + `${req.command} is locked to session "${existingSession.name}" on ${describeSessionDevice(existingSession)}, ` + + `but this request selected ${conflictList}.` + ); + } + const lockPlatform = req.meta?.lockPlatform; + const platformText = lockPlatform ? ` for ${lockPlatform}` : ''; + return `${req.command} is using a bound-session lock${platformText}, but this request selected ${conflictList}.`; +} + +function buildLockPolicyConflictHint( + req: DaemonRequest, + existingSession: SessionState | undefined, +): string { + if (existingSession) { + return buildSessionRecoveryHint(existingSession, 'selector-conflict'); + } + const lockPlatform = req.meta?.lockPlatform; + const sessionText = req.session ? ` --session ${req.session}` : ''; + const openText = lockPlatform + ? `Run agent-device open ${sessionText} --platform ${lockPlatform} first if no session is active. ` + : `Run agent-device open ${sessionText} first if no session is active. `; + return ( + `Remove conflicting device selectors from this command, or use --session-lock strip to let agent-device ignore them. ` + + openText + + `Run agent-device session list to inspect active sessions.` ); } diff --git a/src/daemon/session-recovery-hints.ts b/src/daemon/session-recovery-hints.ts new file mode 100644 index 000000000..bf0a0f47f --- /dev/null +++ b/src/daemon/session-recovery-hints.ts @@ -0,0 +1,58 @@ +import type { SessionState } from './types.ts'; + +export type SessionRecoveryContext = 'device-in-use' | 'selector-conflict'; + +export function describeSessionDevice(session: SessionState): string { + const platform = session.device.platform; + const name = session.device.name.trim(); + const id = session.device.id; + return `${platform} device "${name}" (${id})`; +} + +export function buildSessionRecoveryHint( + session: SessionState, + context: SessionRecoveryContext, +): string { + if (session.recording) { + return buildRecordingSessionRecoveryHint(session, context); + } + return buildOpenSessionRecoveryHint(session, context); +} + +function buildRecordingSessionRecoveryHint( + session: SessionState, + context: SessionRecoveryContext, +): string { + const closeCommand = `agent-device close --session ${session.name}`; + const reuseText = + context === 'selector-conflict' + ? `To keep using this device, rerun the command with --session ${session.name} and remove conflicting device selectors.` + : `To keep using this device, reuse --session ${session.name} for commands that should attach to the recording session.`; + + return ( + `Recording session "${session.name}" owns this device. ` + + `Run agent-device record stop --session ${session.name}; if the session still appears in agent-device session list, run ${closeCommand}. ` + + `${reuseText} ` + + `Run agent-device session list to inspect active sessions.` + ); +} + +function buildOpenSessionRecoveryHint( + session: SessionState, + context: SessionRecoveryContext, +): string { + const closeCommand = `agent-device close --session ${session.name}`; + if (context === 'selector-conflict') { + return ( + `Run agent-device session list to inspect active sessions. ` + + `To reuse this device, rerun the command with --session ${session.name} and remove conflicting device selectors. ` + + `To switch devices, first run ${closeCommand}, then open the desired device with a different --session name.` + ); + } + + return ( + `Run agent-device session list to inspect active sessions. ` + + `To reuse this device, rerun the command with --session ${session.name}. ` + + `To open a new session on this device, first run ${closeCommand}.` + ); +} diff --git a/src/daemon/session-selector.ts b/src/daemon/session-selector.ts index e33b885c8..ef586cd56 100644 --- a/src/daemon/session-selector.ts +++ b/src/daemon/session-selector.ts @@ -3,6 +3,7 @@ import type { CommandFlags } from '../core/dispatch.ts'; import type { SessionState } from './types.ts'; import { matchesPlatformSelector, normalizePlatformSelector } from '../utils/device.ts'; import { parseSerialAllowlist } from '../utils/device-isolation.ts'; +import { buildSessionRecoveryHint, describeSessionDevice } from './session-recovery-hints.ts'; export type SessionSelectorConflictKey = | 'platform' @@ -24,7 +25,12 @@ export function assertSessionSelectorMatches(session: SessionState, flags?: Comm throw new AppError( 'INVALID_ARGS', - `Session "${session.name}" is bound to ${describeDevice(session)} and cannot be used with ${mismatches.map(formatSessionSelectorConflict).join(', ')}. Use a different --session name or close this session first.`, + `Session "${session.name}" is already bound to ${describeSessionDevice(session)}, but this request selected ${mismatches.map(formatSessionSelectorConflict).join(', ')}.`, + { + session: session.name, + conflicts: mismatches.map(formatSessionSelectorConflict), + hint: buildSessionRecoveryHint(session, 'selector-conflict'), + }, ); } @@ -83,13 +89,6 @@ export function formatSessionSelectorConflict(conflict: SessionSelectorConflict) return `${flagNameForConflictKey(conflict.key)}=${conflict.value}`; } -function describeDevice(session: SessionState): string { - const platform = session.device.platform; - const name = session.device.name.trim(); - const id = session.device.id; - return `${platform} device "${name}" (${id})`; -} - function flagNameForConflictKey(key: SessionSelectorConflictKey): string { switch (key) { case 'iosSimulatorDeviceSet': diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 3a5267918..11e336832 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -214,6 +214,7 @@ export type SessionState = { outPath: string; startedAt: number; }; + recordingSession?: boolean; recordSession?: boolean; saveScriptPath?: string; actions: SessionAction[]; diff --git a/src/utils/__tests__/daemon-client.test.ts b/src/utils/__tests__/daemon-client.test.ts index a3bff4a3a..1cba05c43 100644 --- a/src/utils/__tests__/daemon-client.test.ts +++ b/src/utils/__tests__/daemon-client.test.ts @@ -139,17 +139,20 @@ test('resolveDaemonStartupHint prefers stale lock guidance when lock exists with const hint = resolveDaemonStartupHint({ hasInfo: false, hasLock: true }); assert.match(hint, /daemon\.lock/i); assert.match(hint, /automatically/i); + assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); }); test('resolveDaemonStartupHint covers stale info+lock pair', () => { const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: true }); assert.match(hint, /daemon\.json/i); assert.match(hint, /daemon\.lock/i); + assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); }); test('resolveDaemonStartupHint falls back to daemon.json guidance', () => { const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: false }); - assert.match(hint, /cleaned automatically/i); + assert.match(hint, /daemon\.json/i); + assert.match(hint, /rm -f '.+daemon\.json' '.+daemon\.lock'/); }); test('resolveDaemonStartupHint includes configured state directory paths', () => { @@ -157,6 +160,19 @@ test('resolveDaemonStartupHint includes configured state directory paths', () => const hint = resolveDaemonStartupHint({ hasInfo: false, hasLock: true }, paths); assert.match(hint, /\/tmp\/ad-custom-state\/daemon\.lock/); assert.match(hint, /\/tmp\/ad-custom-state\/daemon\.json/); + assert.match( + hint, + /rm -f '\/tmp\/ad-custom-state\/daemon\.json' '\/tmp\/ad-custom-state\/daemon\.lock'/, + ); +}); + +test('resolveDaemonStartupHint shell-quotes cleanup paths', () => { + const paths = resolveDaemonPaths("/tmp/ad custom's state"); + const hint = resolveDaemonStartupHint({ hasInfo: true, hasLock: true }, paths); + assert.match( + hint, + /rm -f '\/tmp\/ad custom'\\''s state\/daemon\.json' '\/tmp\/ad custom'\\''s state\/daemon\.lock'/, + ); }); test('snapshot request timeout preserves daemon metadata for follow-up evidence commands', () => { From 8990f43c05fedd5df3e56bffce32f0479a8441fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 10:53:38 -0700 Subject: [PATCH 2/9] fix: quote session recovery commands --- src/daemon-client.ts | 5 +-- .../__tests__/request-lock-policy.test.ts | 33 +++++++++++++-- src/daemon/__tests__/session-selector.test.ts | 16 +++++++ .../handlers/__tests__/record-trace.test.ts | 42 ++++++++++++++++++- src/daemon/handlers/__tests__/session.test.ts | 2 +- src/daemon/handlers/record-trace-recording.ts | 4 +- src/daemon/request-lock-policy.ts | 5 ++- src/daemon/session-recovery-hints.ts | 19 +++++---- src/daemon/types.ts | 2 +- src/utils/shell-quote.ts | 9 ++++ 10 files changed, 115 insertions(+), 22 deletions(-) create mode 100644 src/utils/shell-quote.ts diff --git a/src/daemon-client.ts b/src/daemon-client.ts index dec127437..478056dc2 100644 --- a/src/daemon-client.ts +++ b/src/daemon-client.ts @@ -29,6 +29,7 @@ import { import { uploadArtifact } from './upload-client.ts'; import { computeDaemonCodeSignature } from './daemon/code-signature.ts'; import { PUBLIC_COMMANDS } from './command-catalog.ts'; +import { shellQuote } from './utils/shell-quote.ts'; import { readDaemonHttpProgressResponse, shouldReadDaemonProgressStream, @@ -1781,7 +1782,3 @@ export function resolveDaemonStartupHint( function buildDaemonMetadataCleanupCommand(paths: Pick) { return `rm -f ${shellQuote(paths.infoPath)} ${shellQuote(paths.lockPath)}`; } - -function shellQuote(value: string): string { - return `'${value.replaceAll("'", "'\\''")}'`; -} diff --git a/src/daemon/__tests__/request-lock-policy.test.ts b/src/daemon/__tests__/request-lock-policy.test.ts index 0844805ec..50c858dc6 100644 --- a/src/daemon/__tests__/request-lock-policy.test.ts +++ b/src/daemon/__tests__/request-lock-policy.test.ts @@ -36,7 +36,7 @@ const ANDROID_SESSION: SessionState = { const RECORDING_SESSION: SessionState = { ...ANDROID_SESSION, name: 'default', - recordingSession: true, + recordOnlySession: true, recording: { platform: 'android', outPath: '/tmp/recording.mp4', @@ -99,6 +99,33 @@ test('reject lock policy explains fresh-session recovery commands', () => { ); }); +test('reject lock policy quotes unsafe fresh session names in recovery commands', () => { + assert.throws( + () => + applyRequestLockPolicy({ + token: 'token', + session: "qa ios; echo 'oops'", + command: 'snapshot', + positionals: [], + flags: { + device: 'Pixel 9', + }, + meta: { + lockPolicy: 'reject', + lockPlatform: 'ios', + }, + }), + (error) => { + assert.ok(error instanceof AppError); + assert.match( + error.details?.hint ?? '', + /agent-device open --session 'qa ios; echo '\\''oops'\\''' --platform ios/i, + ); + return true; + }, + ); +}); + test('allows open to choose a fresh-session target under request lock policy', () => { const req = applyRequestLockPolicy({ token: 'token', @@ -185,7 +212,7 @@ test('reject lock policy explains existing-session recovery commands', () => { ), (error) => { assert.ok(error instanceof AppError); - assert.match(error.message, /locked to session "qa-ios"/i); + assert.match(error.message, /already bound to session "qa-ios"/i); assert.match(error.message, /ios device "iPhone 16" \(SIM-001\)/i); assert.match(error.message, /--serial=emulator-5554/i); assert.match(error.details?.hint ?? '', /agent-device session list/i); @@ -216,7 +243,7 @@ test('reject lock policy explains recording-session recovery commands', () => { ), (error) => { assert.ok(error instanceof AppError); - assert.match(error.message, /locked to session "default"/i); + assert.match(error.message, /already bound to session "default"/i); assert.match(error.details?.hint ?? '', /recording session "default"/i); assert.match(error.details?.hint ?? '', /agent-device record stop --session default/i); assert.match(error.details?.hint ?? '', /agent-device close --session default/i); diff --git a/src/daemon/__tests__/session-selector.test.ts b/src/daemon/__tests__/session-selector.test.ts index f65fe4d31..edb862f60 100644 --- a/src/daemon/__tests__/session-selector.test.ts +++ b/src/daemon/__tests__/session-selector.test.ts @@ -60,6 +60,22 @@ test('selector mismatch explains session recovery commands', () => { ); }); +test('selector mismatch quotes unsafe session names in recovery commands', () => { + const session = makeSession({ name: "default session; echo 'oops'" }); + assert.throws( + () => assertSessionSelectorMatches(session, { platform: 'ios' }), + (err: unknown) => { + assert.ok(err instanceof AppError); + assert.match(err.details?.hint ?? '', /--session 'default session; echo '\\''oops'\\'''/); + assert.match( + err.details?.hint ?? '', + /agent-device close --session 'default session; echo '\\''oops'\\'''/, + ); + return true; + }, + ); +}); + test('accepts --platform apple alias for ios sessions', () => { const session = makeSession({ device: { diff --git a/src/daemon/handlers/__tests__/record-trace.test.ts b/src/daemon/handlers/__tests__/record-trace.test.ts index d56a3134d..79ea70282 100644 --- a/src/daemon/handlers/__tests__/record-trace.test.ts +++ b/src/daemon/handlers/__tests__/record-trace.test.ts @@ -236,7 +236,7 @@ test('record stop releases session created only for recording', async () => { kind: 'simulator', booted: true, }); - session.recordingSession = true; + session.recordOnlySession = true; session.recording = { platform: 'ios', child: { kill: vi.fn(), pid: 123 }, @@ -268,7 +268,7 @@ test('record stop releases record-only session even when recording state is stal kind: 'simulator', booted: true, }); - session.recordingSession = true; + session.recordOnlySession = true; sessionStore.set(sessionName, session); const response = await runRecordCommand({ @@ -314,6 +314,44 @@ test('record stop keeps normal app session open after recording', async () => { expect(sessionStore.get(sessionName)?.recording).toBeUndefined(); }); +test('record stop keeps normal app session open when stop validation fails', async () => { + vi.useFakeTimers(); + vi.setSystemTime(20_000); + const sessionStore = makeSessionStore(); + const sessionName = 'app-session-failed-stop'; + const outPath = path.join(os.tmpdir(), 'app-session-failed-stop.mp4'); + fs.writeFileSync(outPath, 'not playable'); + const session = makeSession(sessionName, { + platform: 'ios', + id: 'ios-sim-1', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + }); + session.appBundleId = 'com.apple.Preferences'; + session.recording = { + platform: 'ios', + child: { kill: vi.fn(), pid: 123 }, + wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + outPath, + startedAt: Date.now() - 500, + showTouches: false, + gestureEvents: [], + }; + sessionStore.set(sessionName, session); + mockIsPlayableVideo.mockImplementation(async () => false); + + const response = await runRecordCommand({ + sessionStore, + sessionName, + positionals: ['stop'], + }); + + expect(response?.ok).toBe(false); + expect(sessionStore.get(sessionName)).toBe(session); + expect(sessionStore.get(sessionName)?.recording).toBeUndefined(); +}); + test('record start resolves relative output path from request cwd', async () => { const sessionStore = makeSessionStore(); const sessionName = 'ios-device-cwd'; diff --git a/src/daemon/handlers/__tests__/session.test.ts b/src/daemon/handlers/__tests__/session.test.ts index 4b51541cc..3ecedc460 100644 --- a/src/daemon/handlers/__tests__/session.test.ts +++ b/src/daemon/handlers/__tests__/session.test.ts @@ -3125,7 +3125,7 @@ test('open on device owned by recording session returns recording recovery hint' kind: 'device', booted: true, }); - recordingSession.recordingSession = true; + recordingSession.recordOnlySession = true; recordingSession.recording = { platform: 'ios', child: { kill: vi.fn(), pid: 123 }, diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 3e6c428be..7dc54131c 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -534,7 +534,7 @@ function releaseRecordOnlySession( session: SessionState, options: { writeLog?: boolean } = {}, ): void { - if (!session.recordingSession) { + if (!session.recordOnlySession) { return; } if (options.writeLog) { @@ -565,7 +565,7 @@ export async function handleRecordCommand(params: { name: sessionName, device, createdAt: Date.now(), - recordingSession: true, + recordOnlySession: true, actions: [], } satisfies SessionState); diff --git a/src/daemon/request-lock-policy.ts b/src/daemon/request-lock-policy.ts index 9f47077b5..7aaa8867b 100644 --- a/src/daemon/request-lock-policy.ts +++ b/src/daemon/request-lock-policy.ts @@ -10,6 +10,7 @@ import { } from './session-selector.ts'; import { isApplePlatform, normalizePlatformSelector } from '../utils/device.ts'; import { buildSessionRecoveryHint, describeSessionDevice } from './session-recovery-hints.ts'; +import { shellQuoteIfNeeded } from '../utils/shell-quote.ts'; type LockPlatform = NonNullable['lockPlatform']; @@ -84,7 +85,7 @@ function buildLockPolicyConflictMessage( const conflictList = conflicts.map(formatSessionSelectorConflict).join(', '); if (existingSession) { return ( - `${req.command} is locked to session "${existingSession.name}" on ${describeSessionDevice(existingSession)}, ` + + `${req.command} is already bound to session "${existingSession.name}" on ${describeSessionDevice(existingSession)}, ` + `but this request selected ${conflictList}.` ); } @@ -101,7 +102,7 @@ function buildLockPolicyConflictHint( return buildSessionRecoveryHint(existingSession, 'selector-conflict'); } const lockPlatform = req.meta?.lockPlatform; - const sessionText = req.session ? ` --session ${req.session}` : ''; + const sessionText = req.session ? ` --session ${shellQuoteIfNeeded(req.session)}` : ''; const openText = lockPlatform ? `Run agent-device open ${sessionText} --platform ${lockPlatform} first if no session is active. ` : `Run agent-device open ${sessionText} first if no session is active. `; diff --git a/src/daemon/session-recovery-hints.ts b/src/daemon/session-recovery-hints.ts index bf0a0f47f..a7d6ccd2b 100644 --- a/src/daemon/session-recovery-hints.ts +++ b/src/daemon/session-recovery-hints.ts @@ -1,4 +1,5 @@ import type { SessionState } from './types.ts'; +import { shellQuoteIfNeeded } from '../utils/shell-quote.ts'; export type SessionRecoveryContext = 'device-in-use' | 'selector-conflict'; @@ -13,6 +14,7 @@ export function buildSessionRecoveryHint( session: SessionState, context: SessionRecoveryContext, ): string { + // Active recording state controls user recovery text; record-only ownership controls cleanup. if (session.recording) { return buildRecordingSessionRecoveryHint(session, context); } @@ -23,15 +25,17 @@ function buildRecordingSessionRecoveryHint( session: SessionState, context: SessionRecoveryContext, ): string { - const closeCommand = `agent-device close --session ${session.name}`; + const sessionArg = shellQuoteIfNeeded(session.name); + const closeCommand = `agent-device close --session ${sessionArg}`; + const recordStopCommand = `agent-device record stop --session ${sessionArg}`; const reuseText = context === 'selector-conflict' - ? `To keep using this device, rerun the command with --session ${session.name} and remove conflicting device selectors.` - : `To keep using this device, reuse --session ${session.name} for commands that should attach to the recording session.`; + ? `To keep using this device, rerun the command with --session ${sessionArg} and remove conflicting device selectors.` + : `To keep using this device, reuse --session ${sessionArg} for commands that should attach to the recording session.`; return ( `Recording session "${session.name}" owns this device. ` + - `Run agent-device record stop --session ${session.name}; if the session still appears in agent-device session list, run ${closeCommand}. ` + + `Run ${recordStopCommand}; if the session still appears in agent-device session list, run ${closeCommand}. ` + `${reuseText} ` + `Run agent-device session list to inspect active sessions.` ); @@ -41,18 +45,19 @@ function buildOpenSessionRecoveryHint( session: SessionState, context: SessionRecoveryContext, ): string { - const closeCommand = `agent-device close --session ${session.name}`; + const sessionArg = shellQuoteIfNeeded(session.name); + const closeCommand = `agent-device close --session ${sessionArg}`; if (context === 'selector-conflict') { return ( `Run agent-device session list to inspect active sessions. ` + - `To reuse this device, rerun the command with --session ${session.name} and remove conflicting device selectors. ` + + `To reuse this device, rerun the command with --session ${sessionArg} and remove conflicting device selectors. ` + `To switch devices, first run ${closeCommand}, then open the desired device with a different --session name.` ); } return ( `Run agent-device session list to inspect active sessions. ` + - `To reuse this device, rerun the command with --session ${session.name}. ` + + `To reuse this device, rerun the command with --session ${sessionArg}. ` + `To open a new session on this device, first run ${closeCommand}.` ); } diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 11e336832..264b959c5 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -214,7 +214,7 @@ export type SessionState = { outPath: string; startedAt: number; }; - recordingSession?: boolean; + recordOnlySession?: boolean; recordSession?: boolean; saveScriptPath?: string; actions: SessionAction[]; diff --git a/src/utils/shell-quote.ts b/src/utils/shell-quote.ts new file mode 100644 index 000000000..a2122a615 --- /dev/null +++ b/src/utils/shell-quote.ts @@ -0,0 +1,9 @@ +const SAFE_SHELL_ARG = /^[A-Za-z0-9_@%+=:,./-]+$/; + +export function shellQuote(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'`; +} + +export function shellQuoteIfNeeded(value: string): string { + return SAFE_SHELL_ARG.test(value) ? value : shellQuote(value); +} From 0e9d816298ac52d24052a354d8fa5cb6442d7b89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 11:07:33 -0700 Subject: [PATCH 3/9] test: simplify record session cleanup fixtures --- .../handlers/__tests__/record-trace.test.ts | 100 ++++++++---------- 1 file changed, 45 insertions(+), 55 deletions(-) diff --git a/src/daemon/handlers/__tests__/record-trace.test.ts b/src/daemon/handlers/__tests__/record-trace.test.ts index 79ea70282..40864a0bf 100644 --- a/src/daemon/handlers/__tests__/record-trace.test.ts +++ b/src/daemon/handlers/__tests__/record-trace.test.ts @@ -115,6 +115,44 @@ function makeIosDeviceSession(name: string, appBundleId?: string): SessionState return session; } +function makeIosSimulatorSession(name: string): SessionState { + return makeSession(name, { + platform: 'ios', + id: 'ios-sim-1', + name: 'iPhone 16', + kind: 'simulator', + booted: true, + }); +} + +function makeIosSimulatorRecordingSession( + name: string, + options: { + appBundleId?: string; + outPath?: string; + recordOnlySession?: boolean; + startedAt?: number; + } = {}, +): SessionState { + const session = makeIosSimulatorSession(name); + if (options.appBundleId) { + session.appBundleId = options.appBundleId; + } + if (options.recordOnlySession) { + session.recordOnlySession = true; + } + session.recording = { + platform: 'ios', + child: { kill: vi.fn(), pid: 123 }, + wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + outPath: options.outPath ?? path.join(os.tmpdir(), `${name}.mp4`), + startedAt: options.startedAt ?? Date.now(), + showTouches: false, + gestureEvents: [], + }; + return session; +} + async function runRecordCommand(params: { sessionStore: SessionStore; sessionName: string; @@ -229,23 +267,7 @@ test('record stop derives telemetry artifact local path from client outPath', as test('record stop releases session created only for recording', async () => { const sessionStore = makeSessionStore(); const sessionName = 'record-only-session'; - const session = makeSession(sessionName, { - platform: 'ios', - id: 'ios-sim-1', - name: 'iPhone 16', - kind: 'simulator', - booted: true, - }); - session.recordOnlySession = true; - session.recording = { - platform: 'ios', - child: { kill: vi.fn(), pid: 123 }, - wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), - outPath: path.join(os.tmpdir(), 'record-only.mp4'), - startedAt: Date.now(), - showTouches: false, - gestureEvents: [], - }; + const session = makeIosSimulatorRecordingSession(sessionName, { recordOnlySession: true }); sessionStore.set(sessionName, session); const response = await runRecordCommand({ @@ -261,13 +283,7 @@ test('record stop releases session created only for recording', async () => { test('record stop releases record-only session even when recording state is stale', async () => { const sessionStore = makeSessionStore(); const sessionName = 'stale-record-only-session'; - const session = makeSession(sessionName, { - platform: 'ios', - id: 'ios-sim-1', - name: 'iPhone 16', - kind: 'simulator', - booted: true, - }); + const session = makeIosSimulatorSession(sessionName); session.recordOnlySession = true; sessionStore.set(sessionName, session); @@ -284,23 +300,9 @@ test('record stop releases record-only session even when recording state is stal test('record stop keeps normal app session open after recording', async () => { const sessionStore = makeSessionStore(); const sessionName = 'app-session'; - const session = makeSession(sessionName, { - platform: 'ios', - id: 'ios-sim-1', - name: 'iPhone 16', - kind: 'simulator', - booted: true, + const session = makeIosSimulatorRecordingSession(sessionName, { + appBundleId: 'com.apple.Preferences', }); - session.appBundleId = 'com.apple.Preferences'; - session.recording = { - platform: 'ios', - child: { kill: vi.fn(), pid: 123 }, - wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), - outPath: path.join(os.tmpdir(), 'app-session.mp4'), - startedAt: Date.now(), - showTouches: false, - gestureEvents: [], - }; sessionStore.set(sessionName, session); const response = await runRecordCommand({ @@ -321,23 +323,11 @@ test('record stop keeps normal app session open when stop validation fails', asy const sessionName = 'app-session-failed-stop'; const outPath = path.join(os.tmpdir(), 'app-session-failed-stop.mp4'); fs.writeFileSync(outPath, 'not playable'); - const session = makeSession(sessionName, { - platform: 'ios', - id: 'ios-sim-1', - name: 'iPhone 16', - kind: 'simulator', - booted: true, - }); - session.appBundleId = 'com.apple.Preferences'; - session.recording = { - platform: 'ios', - child: { kill: vi.fn(), pid: 123 }, - wait: Promise.resolve({ stdout: '', stderr: '', exitCode: 0 }), + const session = makeIosSimulatorRecordingSession(sessionName, { + appBundleId: 'com.apple.Preferences', outPath, startedAt: Date.now() - 500, - showTouches: false, - gestureEvents: [], - }; + }); sessionStore.set(sessionName, session); mockIsPlayableVideo.mockImplementation(async () => false); From 93799442a80fec187353c4d8ed115989c1003bc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 11:58:59 -0700 Subject: [PATCH 4/9] test: cover recovery hint edge cases --- .../__tests__/request-finalization.test.ts | 28 +++++++++++++++++++ src/daemon/types.ts | 1 + src/utils/__tests__/shell-quote.test.ts | 21 ++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 src/utils/__tests__/shell-quote.test.ts diff --git a/src/daemon/__tests__/request-finalization.test.ts b/src/daemon/__tests__/request-finalization.test.ts index 8eeba9a37..029a997fc 100644 --- a/src/daemon/__tests__/request-finalization.test.ts +++ b/src/daemon/__tests__/request-finalization.test.ts @@ -29,3 +29,31 @@ test('finalizeDaemonResponse preserves handler error hints from details', () => expect(finalized.error.hint).toBe('Run agent-device session list and reuse --session default.'); } }); + +test('finalizeDaemonResponse prefers top-level error hint over details hint', () => { + const req: DaemonRequest = { + token: 'token', + session: 'default', + command: 'open', + positionals: [], + flags: {}, + }; + const response: DaemonResponse = { + ok: false, + error: { + code: 'DEVICE_IN_USE', + message: 'Device is already in use by session "default".', + hint: 'Use the top-level hint.', + details: { + hint: 'Use the details hint.', + }, + }, + }; + + const finalized = finalizeDaemonResponse(req, response, () => 'artifact-id'); + + expect(finalized.ok).toBe(false); + if (!finalized.ok) { + expect(finalized.error.hint).toBe('Use the top-level hint.'); + } +}); diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 264b959c5..39a0477cb 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -214,6 +214,7 @@ export type SessionState = { outPath: string; startedAt: number; }; + /** Session was created by record start and should be released when recording stops. */ recordOnlySession?: boolean; recordSession?: boolean; saveScriptPath?: string; diff --git a/src/utils/__tests__/shell-quote.test.ts b/src/utils/__tests__/shell-quote.test.ts new file mode 100644 index 000000000..ed0cfd1aa --- /dev/null +++ b/src/utils/__tests__/shell-quote.test.ts @@ -0,0 +1,21 @@ +import { test } from 'vitest'; +import assert from 'node:assert/strict'; +import { shellQuote, shellQuoteIfNeeded } from '../shell-quote.ts'; + +test('shellQuote single-quotes and escapes POSIX shell arguments', () => { + assert.equal(shellQuote('plain'), "'plain'"); + assert.equal(shellQuote(''), "''"); + assert.equal(shellQuote("qa ios; echo 'oops'"), "'qa ios; echo '\\''oops'\\'''"); +}); + +test('shellQuoteIfNeeded keeps safe command arguments readable', () => { + assert.equal(shellQuoteIfNeeded('default'), 'default'); + assert.equal(shellQuoteIfNeeded('qa-ios_1.2'), 'qa-ios_1.2'); +}); + +test('shellQuoteIfNeeded quotes unsafe command arguments', () => { + assert.equal(shellQuoteIfNeeded('my session'), "'my session'"); + assert.equal(shellQuoteIfNeeded(''), "''"); + assert.equal(shellQuoteIfNeeded('café'), "'café'"); + assert.equal(shellQuoteIfNeeded("qa ios; echo 'oops'"), "'qa ios; echo '\\''oops'\\'''"); +}); From 219a12bead21f99efeaa0369476c0844a536a6c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 12:01:53 -0700 Subject: [PATCH 5/9] test: trim duplicate recovery quoting coverage --- .../__tests__/request-lock-policy.test.ts | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/src/daemon/__tests__/request-lock-policy.test.ts b/src/daemon/__tests__/request-lock-policy.test.ts index 50c858dc6..cf39341cb 100644 --- a/src/daemon/__tests__/request-lock-policy.test.ts +++ b/src/daemon/__tests__/request-lock-policy.test.ts @@ -99,33 +99,6 @@ test('reject lock policy explains fresh-session recovery commands', () => { ); }); -test('reject lock policy quotes unsafe fresh session names in recovery commands', () => { - assert.throws( - () => - applyRequestLockPolicy({ - token: 'token', - session: "qa ios; echo 'oops'", - command: 'snapshot', - positionals: [], - flags: { - device: 'Pixel 9', - }, - meta: { - lockPolicy: 'reject', - lockPlatform: 'ios', - }, - }), - (error) => { - assert.ok(error instanceof AppError); - assert.match( - error.details?.hint ?? '', - /agent-device open --session 'qa ios; echo '\\''oops'\\''' --platform ios/i, - ); - return true; - }, - ); -}); - test('allows open to choose a fresh-session target under request lock policy', () => { const req = applyRequestLockPolicy({ token: 'token', From a91318c53858960bf2e667087d66d6f0f8e33799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 12:43:33 -0700 Subject: [PATCH 6/9] fix: scope implicit sessions by caller workspace --- src/cli.ts | 1 + src/client-normalizers.ts | 1 + src/client-types.ts | 2 + src/contracts.ts | 2 + src/daemon-client.ts | 1 + .../request-router-replay-scope.test.ts | 4 +- .../request-router-screenshot.test.ts | 6 +- src/daemon/__tests__/session-routing.test.ts | 103 +++++++++++++++++- src/daemon/handlers/record-trace-recording.ts | 11 +- src/daemon/handlers/session-close.ts | 8 +- src/daemon/handlers/session-inventory.ts | 33 +++--- src/daemon/handlers/session-open-surface.ts | 14 ++- src/daemon/handlers/session-open.ts | 21 +++- src/daemon/session-routing.ts | 89 +++++++++++++-- src/daemon/types.ts | 4 + src/utils/__tests__/args.test.ts | 6 +- src/utils/cli-help.ts | 32 +++--- 17 files changed, 277 insertions(+), 61 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index efc2b9ea0..f86815347 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -231,6 +231,7 @@ export async function runCli(argv: string[], deps: CliDeps = DEFAULT_CLI_DEPS): lockPolicy: binding.lockPolicy, lockPlatform: binding.defaultPlatform, cwd: process.cwd(), + sessionExplicit: currentFlags.session !== undefined, debug: debugOutputEnabled, }); let parsedBatchSteps: BatchStep[] | undefined; diff --git a/src/client-normalizers.ts b/src/client-normalizers.ts index 4c6eeebdd..4e04a1609 100644 --- a/src/client-normalizers.ts +++ b/src/client-normalizers.ts @@ -333,6 +333,7 @@ export function buildMeta(options: InternalRequestOptions): DaemonRequest['meta' return stripUndefined({ requestId: options.requestId, cwd: options.cwd, + sessionExplicit: options.sessionExplicit ?? options.session !== undefined, debug: options.debug, lockPolicy: options.lockPolicy, lockPlatform: options.lockPlatform, diff --git a/src/client-types.ts b/src/client-types.ts index d22c4ed5e..0385ecd69 100644 --- a/src/client-types.ts +++ b/src/client-types.ts @@ -57,6 +57,7 @@ export type AgentDeviceClientConfig = { leaseBackend?: LeaseBackend; runtime?: SessionRuntimeHints; cwd?: string; + sessionExplicit?: boolean; debug?: boolean; }; @@ -76,6 +77,7 @@ export type AgentDeviceRequestOverrides = Pick< | 'leaseId' | 'leaseBackend' | 'cwd' + | 'sessionExplicit' | 'debug' >; diff --git a/src/contracts.ts b/src/contracts.ts index a870922a4..cae5c4efe 100644 --- a/src/contracts.ts +++ b/src/contracts.ts @@ -43,6 +43,7 @@ export type DaemonRequestMeta = { requestId?: string; debug?: boolean; cwd?: string; + sessionExplicit?: boolean; tenantId?: string; runId?: string; leaseId?: string; @@ -348,6 +349,7 @@ export const daemonCommandRequestSchema = schema((input, path) => requestId: optionalString(meta, 'requestId', `${path}.meta`), debug: optionalBoolean(meta, 'debug', `${path}.meta`), cwd: optionalString(meta, 'cwd', `${path}.meta`), + sessionExplicit: optionalBoolean(meta, 'sessionExplicit', `${path}.meta`), tenantId: optionalString(meta, 'tenantId', `${path}.meta`), runId: optionalString(meta, 'runId', `${path}.meta`), leaseId: optionalString(meta, 'leaseId', `${path}.meta`), diff --git a/src/daemon-client.ts b/src/daemon-client.ts index 478056dc2..bc19780c3 100644 --- a/src/daemon-client.ts +++ b/src/daemon-client.ts @@ -156,6 +156,7 @@ export async function sendToDaemon(req: Omit): Promise { session: 'default', command: 'screenshot', positionals: [absolutePath], - meta: { cwd: '/some/other/dir', requestId: 'req-2' }, + meta: { cwd: '/some/other/dir', requestId: 'req-2', sessionExplicit: true }, }); expect(capturedPath).toBe(absolutePath); @@ -359,7 +359,7 @@ test('screenshot resolves --out flag path against request cwd', async () => { command: 'screenshot', positionals: [], flags: { out: 'evidence/test.png' }, - meta: { cwd: callerCwd, requestId: 'req-3' }, + meta: { cwd: callerCwd, requestId: 'req-3', sessionExplicit: true }, }); expect(capturedOut).toBeTruthy(); diff --git a/src/daemon/__tests__/session-routing.test.ts b/src/daemon/__tests__/session-routing.test.ts index 660d6dfdf..454ec0d7e 100644 --- a/src/daemon/__tests__/session-routing.test.ts +++ b/src/daemon/__tests__/session-routing.test.ts @@ -4,7 +4,11 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { SessionStore } from '../session-store.ts'; -import { resolveEffectiveSessionName } from '../session-routing.ts'; +import { + resolveEffectiveSessionName, + resolveImplicitSessionScope, + sessionMatchesScope, +} from '../session-routing.ts'; import type { SessionState } from '../types.ts'; function makeSession(name: string): SessionState { @@ -30,9 +34,13 @@ function makeStore(t: TestContext): SessionStore { return new SessionStore(path.join(root, 'sessions')); } -test('reuses lone active session for implicit default session', (t) => { +test('does not reuse lone active session for implicit default session from another scope', (t) => { const store = makeStore(t); store.set('android', makeSession('android')); + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cwd-scope-')); + t.onTestFinished(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + }); const resolved = resolveEffectiveSessionName( { @@ -41,9 +49,98 @@ test('reuses lone active session for implicit default session', (t) => { command: 'open', positionals: ['com.google.android.apps.maps'], flags: {}, + meta: { cwd }, + }, + store, + ); + + assert.match(resolved, /^cwd:[a-f0-9]{16}:default$/); + assert.notEqual(resolved, 'android'); +}); + +test('uses git worktree root for implicit default session scope', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cwd-scope-')); + const nested = path.join(root, 'packages', 'app'); + fs.mkdirSync(path.join(root, '.git')); + fs.mkdirSync(nested, { recursive: true }); + t.onTestFinished(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + const store = makeStore(t); + const fromRoot = resolveEffectiveSessionName( + { + token: 't', + session: 'default', + command: 'snapshot', + positionals: [], + flags: {}, + meta: { cwd: root }, + }, + store, + ); + const fromNested = resolveEffectiveSessionName( + { + token: 't', + session: 'default', + command: 'snapshot', + positionals: [], + flags: {}, + meta: { cwd: nested }, }, store, ); - assert.equal(resolved, 'android'); + assert.equal(fromNested, fromRoot); +}); + +test('keeps explicitly configured default session global', (t) => { + const store = makeStore(t); + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cwd-scope-')); + t.onTestFinished(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + }); + + const resolved = resolveEffectiveSessionName( + { + token: 't', + session: 'default', + command: 'snapshot', + positionals: [], + flags: {}, + meta: { cwd, sessionExplicit: true }, + }, + store, + ); + + assert.equal(resolved, 'default'); +}); + +test('matches sessions only within the same implicit scope', (t) => { + const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cwd-scope-')); + t.onTestFinished(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + }); + const req = { + token: 't', + session: 'default', + command: 'session_list', + positionals: [], + flags: {}, + meta: { cwd }, + }; + const scope = resolveImplicitSessionScope(req); + assert.ok(scope); + + assert.equal( + sessionMatchesScope({ ...makeSession('default'), sessionScope: scope }, scope), + true, + ); + assert.equal( + sessionMatchesScope( + { ...makeSession('default'), sessionScope: { kind: 'cwd', id: 'other' } }, + scope, + ), + false, + ); }); diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 7dc54131c..389e3c998 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -40,6 +40,7 @@ import { IOS_SIMULATOR_RECORDING_STOP_TIMEOUT_MS, stopIosSimulatorRecordingProcess, } from './record-trace-ios-simulator.ts'; +import { resolveImplicitSessionScope, resolvePublicSessionName } from '../session-routing.ts'; const IOS_DEVICE_RECORD_MIN_FPS = 1; const IOS_DEVICE_RECORD_MAX_FPS = 120; @@ -531,6 +532,7 @@ function deriveClientTelemetryPath( function releaseRecordOnlySession( sessionStore: SessionStore, + sessionName: string, session: SessionState, options: { writeLog?: boolean } = {}, ): void { @@ -540,7 +542,7 @@ function releaseRecordOnlySession( if (options.writeLog) { sessionStore.writeSessionLog(session); } - sessionStore.delete(session.name); + sessionStore.delete(sessionName); } // --- Main command handler --- @@ -562,7 +564,8 @@ export async function handleRecordCommand(params: { const activeSession = session ?? ({ - name: sessionName, + name: resolvePublicSessionName(req), + sessionScope: resolveImplicitSessionScope(req), device, createdAt: Date.now(), recordOnlySession: true, @@ -580,7 +583,7 @@ export async function handleRecordCommand(params: { const response = await stopRecording({ req, activeSession, device, logPath, deps }); if (!response.ok) { - releaseRecordOnlySession(sessionStore, activeSession); + releaseRecordOnlySession(sessionStore, sessionName, activeSession); return response; } @@ -594,6 +597,6 @@ export async function handleRecordCommand(params: { showTouches: response.data?.showTouches, }, }); - releaseRecordOnlySession(sessionStore, activeSession, { writeLog: true }); + releaseRecordOnlySession(sessionStore, sessionName, activeSession, { writeLog: true }); return response; } diff --git a/src/daemon/handlers/session-close.ts b/src/daemon/handlers/session-close.ts index 3f1c1037a..4aa5f76ce 100644 --- a/src/daemon/handlers/session-close.ts +++ b/src/daemon/handlers/session-close.ts @@ -166,7 +166,7 @@ export async function handleCloseCommand(params: { command: 'close', positionals: req.positionals ?? [], flags: req.flags ?? {}, - result: { session: sessionName, ...successText(`Closed: ${sessionName}`) }, + result: { session: session.name, ...successText(`Closed: ${session.name}`) }, }); if (req.flags?.saveScript) { session.recordSession = true; @@ -182,12 +182,12 @@ export async function handleCloseCommand(params: { return { ok: true, data: withSuccessText( - { session: sessionName, shutdown: shutdownResult }, - `Closed: ${sessionName}`, + { session: session.name, shutdown: shutdownResult }, + `Closed: ${session.name}`, ), }; } - return { ok: true, data: { session: sessionName, ...successText(`Closed: ${sessionName}`) } }; + return { ok: true, data: { session: session.name, ...successText(`Closed: ${session.name}`) } }; } async function closeWithoutSession(req: DaemonRequest, logPath: string): Promise { diff --git a/src/daemon/handlers/session-inventory.ts b/src/daemon/handlers/session-inventory.ts index 8eff9ef9a..578c4d634 100644 --- a/src/daemon/handlers/session-inventory.ts +++ b/src/daemon/handlers/session-inventory.ts @@ -18,6 +18,7 @@ import { listAndroidApps } from '../../platforms/android/app-lifecycle.ts'; import { listIosApps } from '../../platforms/ios/apps.ts'; import { requireSessionOrExplicitSelector, resolveCommandDevice } from './session-device-utils.ts'; import { errorResponse } from './response.ts'; +import { resolveImplicitSessionScope, sessionMatchesScope } from '../session-routing.ts'; export async function handleSessionInventoryCommands(params: { req: DaemonRequest; @@ -27,23 +28,27 @@ export async function handleSessionInventoryCommands(params: { const { req, sessionName, sessionStore } = params; if (req.command === 'session_list') { + const scope = resolveImplicitSessionScope(req); return { ok: true, data: { - sessions: sessionStore.toArray().map((session) => ({ - name: session.name, - platform: session.device.platform, - target: session.device.target ?? 'mobile', - surface: session.surface ?? 'app', - device: session.device.name, - id: session.device.id, - device_id: session.device.id, - createdAt: session.createdAt, - ...(session.device.platform === 'ios' && { - device_udid: session.device.id, - ios_simulator_device_set: session.device.simulatorSetPath ?? null, - }), - })), + sessions: sessionStore + .toArray() + .filter((session) => sessionMatchesScope(session, scope)) + .map((session) => ({ + name: session.name, + platform: session.device.platform, + target: session.device.target ?? 'mobile', + surface: session.surface ?? 'app', + device: session.device.name, + id: session.device.id, + device_id: session.device.id, + createdAt: session.createdAt, + ...(session.device.platform === 'ios' && { + device_udid: session.device.id, + ios_simulator_device_set: session.device.simulatorSetPath ?? null, + }), + })), }, }; } diff --git a/src/daemon/handlers/session-open-surface.ts b/src/daemon/handlers/session-open-surface.ts index d584699c8..5cb19ae03 100644 --- a/src/daemon/handlers/session-open-surface.ts +++ b/src/daemon/handlers/session-open-surface.ts @@ -59,14 +59,23 @@ export function buildOpenResult(params: { export function buildNextOpenSession(params: { existingSession?: SessionState; sessionName: string; + sessionScope?: SessionState['sessionScope']; device: DeviceInfo; surface: SessionSurface; appBundleId?: string; appName?: string; saveScript: boolean; }): SessionState { - const { existingSession, sessionName, device, surface, appBundleId, appName, saveScript } = - params; + const { + existingSession, + sessionName, + sessionScope, + device, + surface, + appBundleId, + appName, + saveScript, + } = params; if (existingSession) { return { ...existingSession, @@ -80,6 +89,7 @@ export function buildNextOpenSession(params: { } return { name: sessionName, + sessionScope, device, createdAt: Date.now(), surface, diff --git a/src/daemon/handlers/session-open.ts b/src/daemon/handlers/session-open.ts index cbb0b7838..da6eaae71 100644 --- a/src/daemon/handlers/session-open.ts +++ b/src/daemon/handlers/session-open.ts @@ -33,6 +33,11 @@ import { } from './session-open-prepare.ts'; import { errorResponse } from './response.ts'; import { buildSessionRecoveryHint } from '../session-recovery-hints.ts'; +import { + isImplicitSessionScopeConflict, + resolveImplicitSessionScope, + resolvePublicSessionName, +} from '../session-routing.ts'; const firstSessionOpenLocks = new Map>(); @@ -237,7 +242,8 @@ async function completeOpenCommand(params: { } const nextSession = buildNextOpenSession({ existingSession, - sessionName, + sessionName: existingSession?.name ?? resolvePublicSessionName(req), + sessionScope: existingSession?.sessionScope ?? resolveImplicitSessionScope(req), device, surface, appBundleId: sessionAppBundleId, @@ -249,7 +255,7 @@ async function completeOpenCommand(params: { } timing.totalDurationMs = Math.max(0, Date.now() - openCommandStartedAtMs); const openResult = buildOpenResult({ - sessionName, + sessionName: nextSession.name, appName, appBundleId: sessionAppBundleId, surface, @@ -381,6 +387,17 @@ export async function handleOpenCommand(params: { .toArray() .find((activeSession) => activeSession.device.id === device.id); if (inUse) { + if (isImplicitSessionScopeConflict(req, inUse)) { + return errorResponse( + 'DEVICE_IN_USE', + 'Device is already in use by another workspace session.', + { + deviceId: device.id, + deviceName: device.name, + hint: 'Use a different device selector, wait for the other workspace to close its session, or run agent-device devices to choose another target.', + }, + ); + } return errorResponse( 'DEVICE_IN_USE', `Device is already in use by session "${inUse.name}".`, diff --git a/src/daemon/session-routing.ts b/src/daemon/session-routing.ts index 5925a4ebc..be99cb7af 100644 --- a/src/daemon/session-routing.ts +++ b/src/daemon/session-routing.ts @@ -1,23 +1,96 @@ -import type { DaemonRequest } from './types.ts'; +import crypto from 'node:crypto'; +import fs from 'node:fs'; +import path from 'node:path'; +import type { DaemonRequest, SessionState } from './types.ts'; import { SessionStore } from './session-store.ts'; import type { CommandFlags } from '../core/dispatch.ts'; +const DEFAULT_SESSION_NAME = 'default'; +const IMPLICIT_SESSION_KEY_PREFIX = 'cwd'; + export function resolveEffectiveSessionName( req: DaemonRequest, - sessionStore: SessionStore, + _sessionStore: SessionStore, ): string { - const requested = req.session || 'default'; + const requested = req.session || DEFAULT_SESSION_NAME; if (hasExplicitSessionFlag(req)) return requested; + const scope = resolveImplicitSessionScope(req); + if (scope) return formatScopedSessionName(scope.id, requested); if (requested !== 'default') return requested; - if (sessionStore.has(requested)) return requested; - - const sessions = sessionStore.toArray(); - const session = sessions[0]; - if (session !== undefined && sessions.length === 1) return session.name; return requested; } +export function resolvePublicSessionName(req: DaemonRequest): string { + return req.session || DEFAULT_SESSION_NAME; +} + +export function resolveImplicitSessionScope( + req: DaemonRequest, +): SessionState['sessionScope'] | undefined { + if (req.meta?.sessionExplicit === true) return undefined; + if ((req.session || DEFAULT_SESSION_NAME) !== DEFAULT_SESSION_NAME) return undefined; + if (req.meta?.sessionIsolation === 'tenant' || req.flags?.sessionIsolation === 'tenant') { + return undefined; + } + const scopeRoot = resolveCallerScopeRoot(req.meta?.cwd); + if (!scopeRoot) return undefined; + return { + kind: 'cwd', + id: hashScopeRoot(scopeRoot), + }; +} + +export function sessionMatchesScope( + session: SessionState, + scope: SessionState['sessionScope'] | undefined, +): boolean { + if (!scope) return true; + return session.sessionScope?.kind === scope.kind && session.sessionScope.id === scope.id; +} + +export function isImplicitSessionScopeConflict(req: DaemonRequest, session: SessionState): boolean { + const scope = resolveImplicitSessionScope(req); + if (!scope || !session.sessionScope) return false; + return !sessionMatchesScope(session, scope); +} + function hasExplicitSessionFlag(req: DaemonRequest): boolean { + if (req.meta?.sessionExplicit === true) return true; const value = (req.flags as CommandFlags | undefined)?.session; return typeof value === 'string' && value.trim().length > 0; } + +function formatScopedSessionName(scopeId: string, sessionName: string): string { + return `${IMPLICIT_SESSION_KEY_PREFIX}:${scopeId}:${sessionName}`; +} + +function hashScopeRoot(scopeRoot: string): string { + return crypto.createHash('sha256').update(scopeRoot).digest('hex').slice(0, 16); +} + +function resolveCallerScopeRoot(rawCwd: string | undefined): string | undefined { + if (!rawCwd || rawCwd.trim().length === 0) return undefined; + const cwd = resolveExistingPath(rawCwd); + return findGitWorktreeRoot(cwd) ?? cwd; +} + +function resolveExistingPath(rawPath: string): string { + const resolved = path.resolve(rawPath); + try { + return fs.realpathSync.native(resolved); + } catch { + return resolved; + } +} + +function findGitWorktreeRoot(startDir: string): string | undefined { + let current = startDir; + while (true) { + if (fs.existsSync(path.join(current, '.git'))) { + return current; + } + const parent = path.dirname(current); + if (parent === current) return undefined; + current = parent; + } +} diff --git a/src/daemon/types.ts b/src/daemon/types.ts index 39a0477cb..2e65f0cf0 100644 --- a/src/daemon/types.ts +++ b/src/daemon/types.ts @@ -199,6 +199,10 @@ type SessionRecordingProcessChild = Pick ""/); assert.match(usageText, /Android IME capture: if fill says input was captured/); - assert.match(usageText, /Run mutating commands serially against one session/); - assert.match(usageText, /run session list and reuse the active session name/); + assert.match(usageText, /Implicit default sessions are scoped to the current worktree/); + assert.match(usageText, /Run mutating commands serially within one session/); assert.match(usageText, /After mutation: refs are stale/); assert.match(usageText, /use its selector directly; otherwise refresh with snapshot -i/); assert.match(usageText, /app-owned back uses back/); @@ -1053,7 +1053,7 @@ test('usageForCommand resolves workflow help topic', () => { assert.match(help, /do not plan fill ""/); assert.match(help, /prefer keyboard dismiss before manually pressing visible Done/); assert.match(help, /UNSUPPORTED_OPERATION/); - assert.match(help, /Stateful commands against one --session must run serially/); + assert.match(help, /Stateful commands within one session must run serially/); assert.match( help, /Do not run open\/press\/fill\/type\/scroll\/back\/alert\/replay\/batch\/close commands in parallel/, diff --git a/src/utils/cli-help.ts b/src/utils/cli-help.ts index f80003232..3297254a1 100644 --- a/src/utils/cli-help.ts +++ b/src/utils/cli-help.ts @@ -43,8 +43,8 @@ const AGENT_QUICKSTART_LINES = [ 'Text: fill \'id="field-email"\' "qa@example.com" replaces; type appends after press.', 'Clearing text: do not use fill ""; use a visible clear/reset control or report that clearing is unsupported.', 'Android IME capture: if fill says input was captured by the keyboard/IME, inspect keyboard state and switch/disable handwriting before retrying; do not loop fill/type.', - 'Run mutating commands serially against one session; parallelize only read-only commands or separate sessions.', - 'Before taking over a shared device, run session list and reuse the active session name when one already owns the device.', + 'Implicit default sessions are scoped to the current worktree; use --session only when intentionally sharing a named session.', + 'Run mutating commands serially within one session; parallelize only read-only commands or separate sessions/devices.', 'Clipboard limits: iOS Allow Paste cannot be automated through XCUITest; prefill with clipboard write. Android non-ASCII should use fill/type, not raw adb input.', 'After mutation: refs are stale. If the next target is known, use its selector directly; otherwise refresh with snapshot -i, scoped with -s when a stable container is known.', 'Raw coordinates are fallback-only: use snapshot -i -c --json rects when iOS refs no-op or child refs are missing.', @@ -160,7 +160,7 @@ Text entry: Android text entry is owned by agent-device: provider-native text injection when available, then chunk-safe ASCII shell input. Do not switch to raw adb, clipboard, or paste as an agent fallback. If non-ASCII is unsupported in the current backend, report the tool/device gap. Session ordering: - Stateful commands against one --session must run serially. Do not run open/press/fill/type/scroll/back/alert/replay/batch/close commands in parallel against the same session. + Stateful commands within one session must run serially. Do not run open/press/fill/type/scroll/back/alert/replay/batch/close commands in parallel against the same session. It is fine to parallelize independent read-only collection or commands that use different sessions/devices. Read-only and waits: @@ -400,7 +400,7 @@ React DevTools routing: If React DevTools cannot connect, report status and continue with logs, network, perf, screenshot, and trace evidence instead of blocking the whole flow. Slow-flow investigation: - Keep one named session, start with session list, open, and snapshot -i. + Keep one session, open the app, and snapshot -i. Use help react-devtools for the narrow React profile window. Use help debugging for logs clear --restart, logs mark, network dump --include headers, perf --json, traces, and runtime failure evidence. For 15-20s async work, use wait with the exact expected text or selector instead of repeated snapshots. @@ -482,7 +482,7 @@ Goal: Loop: 1. Identify target app/platform; ask only if missing. - 2. Create output dirs and open a named session. If auth or OTP is required, sign in or ask the user for the code. + 2. Create output dirs and open the app. If auth or OTP is required, sign in or ask the user for the code. 3. Capture baseline snapshot -i and screenshot. 4. Map top-level navigation, then exercise primary flows and edge states. 5. For each issue, capture evidence and write the finding immediately, then continue. @@ -499,17 +499,17 @@ Coverage: Evidence commands: mkdir -p ./dogfood-output/screenshots ./dogfood-output/videos ./dogfood-output/traces - agent-device --session qa open --platform ios - agent-device --session qa snapshot -i - agent-device --session qa screenshot ./dogfood-output/screenshots/initial.png - agent-device --session qa screenshot ./dogfood-output/screenshots/issue-001.png --overlay-refs - agent-device --session qa logs clear --restart - agent-device --session qa logs mark "issue-001 repro" - agent-device --session qa logs path - agent-device --session qa record start ./dogfood-output/videos/issue-001.mp4 - agent-device --session qa record start ./dogfood-output/videos/benchmark.mp4 --hide-touches - agent-device --session qa record stop - agent-device --session qa close + agent-device open --platform ios + agent-device snapshot -i + agent-device screenshot ./dogfood-output/screenshots/initial.png + agent-device screenshot ./dogfood-output/screenshots/issue-001.png --overlay-refs + agent-device logs clear --restart + agent-device logs mark "issue-001 repro" + agent-device logs path + agent-device record start ./dogfood-output/videos/issue-001.mp4 + agent-device record start ./dogfood-output/videos/benchmark.mp4 --hide-touches + agent-device record stop + agent-device close Evidence rules: Interactive/behavioral issues need step screenshots and usually a repro video. From 98b91653b76105d4feb3a7710e2c6ee0152927f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 13:18:23 -0700 Subject: [PATCH 7/9] fix: surface session state directory --- skills/dogfood/SKILL.md | 2 +- src/cli.ts | 3 +- src/client-normalizers.ts | 2 +- src/client-shared.ts | 1 + src/client-types.ts | 3 +- src/client.ts | 1 + src/commands/__tests__/client-output.test.ts | 34 ++++++++++++++++++- src/commands/client-output.ts | 9 ++++- .../__tests__/request-router-open.test.ts | 18 ++++++++++ src/daemon/handlers/record-trace-recording.ts | 2 ++ src/daemon/handlers/session-open-surface.ts | 3 ++ src/daemon/handlers/session-open.ts | 2 ++ src/daemon/session-store.ts | 15 ++++++-- src/utils/__tests__/args.test.ts | 4 +-- src/utils/cli-help.ts | 4 +-- .../suites/agent-device-smoke-suite.ts | 12 ++----- website/docs/docs/commands.md | 10 +++--- website/docs/docs/configuration.md | 4 +-- website/docs/docs/sessions.md | 10 ++++-- 19 files changed, 105 insertions(+), 34 deletions(-) diff --git a/skills/dogfood/SKILL.md b/skills/dogfood/SKILL.md index 2cab72a10..4e2b91177 100644 --- a/skills/dogfood/SKILL.md +++ b/skills/dogfood/SKILL.md @@ -22,6 +22,6 @@ Read current CLI guidance: agent-device help dogfood ``` -Loop: open named session -> snapshot -i + screenshot -> explore flows -> capture evidence per issue -> close. +Loop: open app -> snapshot -i + screenshot -> explore flows -> capture evidence per issue -> close. Target app is required; infer platform or ask. Findings must come from runtime behavior, not source reads. Let `help dogfood` provide exact report shape, evidence commands, and current workflow guidance. diff --git a/src/cli.ts b/src/cli.ts index f86815347..e2110d2fa 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -215,7 +215,7 @@ export async function runCli(argv: string[], deps: CliDeps = DEFAULT_CLI_DEPS): currentFlags: CliFlags, runtime: SessionRuntimeHints | undefined, ): AgentDeviceClientConfig => ({ - session: currentFlags.session ?? sessionName, + session: currentFlags.session, requestId, stateDir: currentFlags.stateDir, daemonBaseUrl: currentFlags.daemonBaseUrl, @@ -231,7 +231,6 @@ export async function runCli(argv: string[], deps: CliDeps = DEFAULT_CLI_DEPS): lockPolicy: binding.lockPolicy, lockPlatform: binding.defaultPlatform, cwd: process.cwd(), - sessionExplicit: currentFlags.session !== undefined, debug: debugOutputEnabled, }); let parsedBatchSteps: BatchStep[] | undefined; diff --git a/src/client-normalizers.ts b/src/client-normalizers.ts index 4e04a1609..e77b0089d 100644 --- a/src/client-normalizers.ts +++ b/src/client-normalizers.ts @@ -333,7 +333,7 @@ export function buildMeta(options: InternalRequestOptions): DaemonRequest['meta' return stripUndefined({ requestId: options.requestId, cwd: options.cwd, - sessionExplicit: options.sessionExplicit ?? options.session !== undefined, + sessionExplicit: options.session !== undefined, debug: options.debug, lockPolicy: options.lockPolicy, lockPlatform: options.lockPlatform, diff --git a/src/client-shared.ts b/src/client-shared.ts index dd354dac0..8cd2675f3 100644 --- a/src/client-shared.ts +++ b/src/client-shared.ts @@ -140,6 +140,7 @@ export function serializeOpenResult(result: AppOpenResult): Record; @@ -186,6 +184,7 @@ export type AppOpenOptions = AgentDeviceRequestOverrides & export type AppOpenResult = { session: string; + sessionStateDir?: string; appName?: string; appBundleId?: string; appId?: string; diff --git a/src/client.ts b/src/client.ts index 1bf2abe79..8cc83b6ac 100644 --- a/src/client.ts +++ b/src/client.ts @@ -151,6 +151,7 @@ export function createAgentDeviceClient( const appId = appBundleId; return { session, + sessionStateDir: readOptionalString(data, 'sessionStateDir'), appName: readOptionalString(data, 'appName'), appBundleId, appId, diff --git a/src/commands/__tests__/client-output.test.ts b/src/commands/__tests__/client-output.test.ts index 66c2600d6..e11c3cabf 100644 --- a/src/commands/__tests__/client-output.test.ts +++ b/src/commands/__tests__/client-output.test.ts @@ -1,7 +1,39 @@ import { describe, expect, test } from 'vitest'; -import { recordCliOutput } from '../client-output.ts'; +import { openCliOutput, recordCliOutput } from '../client-output.ts'; + +describe('openCliOutput', () => { + test('prints session state directory on a second line', () => { + const output = openCliOutput({ + session: 'default', + sessionStateDir: '/tmp/agent-device/sessions/cwd_123_default', + identifiers: { session: 'default' }, + }); + + expect(output.text).toBe( + ['Opened: default', 'Session state: /tmp/agent-device/sessions/cwd_123_default'].join('\n'), + ); + expect(output.data).toMatchObject({ + session: 'default', + sessionStateDir: '/tmp/agent-device/sessions/cwd_123_default', + }); + }); +}); describe('recordCliOutput', () => { + test('prints session state directory for record-created sessions', () => { + const output = recordCliOutput({ + recording: 'started', + outPath: '/tmp/recording.mp4', + sessionStateDir: '/tmp/agent-device/sessions/cwd_123_default', + }); + + expect(output.text).toBe( + ['/tmp/recording.mp4', 'Session state: /tmp/agent-device/sessions/cwd_123_default'].join( + '\n', + ), + ); + }); + test('prints chunked Android recording paths clearly for human stdout', () => { const output = recordCliOutput({ recording: 'stopped', diff --git a/src/commands/client-output.ts b/src/commands/client-output.ts index ba4698cc3..d4732d8f1 100644 --- a/src/commands/client-output.ts +++ b/src/commands/client-output.ts @@ -56,7 +56,12 @@ export function sessionCliOutput(result: { sessions: AgentDeviceSession[] }): Cl } export function openCliOutput(result: AppOpenResult): CliOutput { - return messageOutput(serializeOpenResult(result)); + const data = serializeOpenResult(result); + const lines = [readCommandMessage(data)].filter((line): line is string => Boolean(line)); + if (typeof data.sessionStateDir === 'string') { + lines.push(`Session state: ${data.sessionStateDir}`); + } + return { data, text: lines.join('\n') || null }; } export function closeCliOutput(result: AppCloseResult | SessionCloseResult): CliOutput { @@ -207,6 +212,8 @@ function defaultCommandCliOutput(result: CommandRequestResult): CliOutput { function formatRecordSingleOutput(data: Record, outPath: string): string { const lines: string[] = []; if (outPath) lines.push(outPath); + if (typeof data.sessionStateDir === 'string') + lines.push(`Session state: ${data.sessionStateDir}`); if (typeof data.warning === 'string') lines.push(`Warning: ${data.warning}`); if (typeof data.overlayWarning === 'string') lines.push(`Overlay warning: ${data.overlayWarning}`); diff --git a/src/daemon/__tests__/request-router-open.test.ts b/src/daemon/__tests__/request-router-open.test.ts index 59bbb2322..26b6e7f30 100644 --- a/src/daemon/__tests__/request-router-open.test.ts +++ b/src/daemon/__tests__/request-router-open.test.ts @@ -1,4 +1,5 @@ import { test, expect, vi, beforeEach } from 'vitest'; +import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { getResolveTargetDeviceMock } from './request-router-dispatch-mocks.ts'; @@ -57,6 +58,23 @@ beforeEach(() => { mockEnsureDeviceReady.mockResolvedValue(undefined); }); +test('open returns and creates the session state directory', async () => { + const sessionStore = makeSessionStore('agent-device-router-open-'); + const device = makeIosDevice('SIM-STATE'); + mockResolveTargetDevice.mockResolvedValue(device); + + const handler = createOpenHandler(sessionStore); + + const response = await handler(openRequest('session-a', { platform: 'ios' }, 'req-open-state')); + + expect(response.ok).toBe(true); + if (response.ok) { + expect(response.data?.session).toBe('session-a'); + expect(response.data?.sessionStateDir).toEqual(expect.stringContaining('session-a')); + expect(fs.existsSync(String(response.data?.sessionStateDir))).toBe(true); + } +}); + test('router serializes same-device open requests before first session creation finishes', async () => { const sessionStore = makeSessionStore('agent-device-router-open-'); const sameDevice = makeIosDevice('SIM-001'); diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 389e3c998..846659b17 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -286,6 +286,7 @@ async function startRecording(params: { activeSession.recording = recording; sessionStore.set(sessionName, activeSession); + const sessionStateDir = sessionStore.ensureSessionDir(sessionName); sessionStore.recordAction(activeSession, { command: req.command, positionals: req.positionals ?? [], @@ -298,6 +299,7 @@ async function startRecording(params: { data: { recording: 'started', outPath: recording.clientOutPath ?? outPath, + sessionStateDir, showTouches: recording.showTouches, }, }; diff --git a/src/daemon/handlers/session-open-surface.ts b/src/daemon/handlers/session-open-surface.ts index 5cb19ae03..6c8350c65 100644 --- a/src/daemon/handlers/session-open-surface.ts +++ b/src/daemon/handlers/session-open-surface.ts @@ -8,6 +8,7 @@ import type { StartupPerfSample } from './session-startup-metrics.ts'; export function buildOpenResult(params: { sessionName: string; + sessionStateDir?: string; appName?: string; appBundleId?: string; surface: SessionSurface; @@ -19,6 +20,7 @@ export function buildOpenResult(params: { }): Record { const { sessionName, + sessionStateDir, appName, appBundleId, surface, @@ -29,6 +31,7 @@ export function buildOpenResult(params: { runtimeHintCount, } = params; const result: Record = { session: sessionName, surface }; + if (sessionStateDir) result.sessionStateDir = sessionStateDir; if (appName) result.appName = appName; if (appBundleId) result.appBundleId = appBundleId; if (startup) result.startup = startup; diff --git a/src/daemon/handlers/session-open.ts b/src/daemon/handlers/session-open.ts index da6eaae71..1efc757ad 100644 --- a/src/daemon/handlers/session-open.ts +++ b/src/daemon/handlers/session-open.ts @@ -253,9 +253,11 @@ async function completeOpenCommand(params: { if (req.runtime !== undefined) { setSessionRuntimeHintsForOpen(sessionStore, sessionName, runtime); } + const sessionStateDir = sessionStore.ensureSessionDir(sessionName); timing.totalDurationMs = Math.max(0, Date.now() - openCommandStartedAtMs); const openResult = buildOpenResult({ sessionName: nextSession.name, + sessionStateDir, appName, appBundleId: sessionAppBundleId, surface, diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index b3c0bcf4a..9b8aa4a8c 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -1,4 +1,5 @@ import path from 'node:path'; +import fs from 'node:fs'; import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { SessionRuntimeHints, SessionState } from './types.ts'; import { recordActionEntry, type RecordActionEntry } from './session-action-recorder.ts'; @@ -74,13 +75,23 @@ export class SessionStore { return path.join(this.sessionsDir, `${safeName}-${timestamp}.trace.log`); } + resolveSessionDir(sessionName: string): string { + return path.join(this.sessionsDir, safeSessionName(sessionName)); + } + + ensureSessionDir(sessionName: string): string { + const sessionDir = this.resolveSessionDir(sessionName); + fs.mkdirSync(sessionDir, { recursive: true }); + return sessionDir; + } + /** Path to session-scoped app log file. Agent can grep this for token-efficient debugging. */ resolveAppLogPath(sessionName: string): string { - return path.join(this.sessionsDir, safeSessionName(sessionName), 'app.log'); + return path.join(this.resolveSessionDir(sessionName), 'app.log'); } resolveAppLogPidPath(sessionName: string): string { - return path.join(this.sessionsDir, safeSessionName(sessionName), 'app-log.pid'); + return path.join(this.resolveSessionDir(sessionName), 'app-log.pid'); } static expandHome(filePath: string, cwd?: string): string { diff --git a/src/utils/__tests__/args.test.ts b/src/utils/__tests__/args.test.ts index 1d0ced395..4a5697d5d 100644 --- a/src/utils/__tests__/args.test.ts +++ b/src/utils/__tests__/args.test.ts @@ -986,7 +986,7 @@ test('usage includes agent workflows, config, environment, and examples footers' /Use --config or AGENT_DEVICE_CONFIG to load one explicit config file\./, ); assert.match(usageText, /Environment:/); - assert.match(usageText, /AGENT_DEVICE_SESSION\s+Default session name/); + assert.match(usageText, /AGENT_DEVICE_SESSION\s+Explicit session name/); assert.match(usageText, /AGENT_DEVICE_PLATFORM\s+Default platform binding/); assert.match(usageText, /AGENT_DEVICE_SESSION_LOCK\s+Bound-session conflict mode/); assert.match(usageText, /AGENT_DEVICE_DAEMON_BASE_URL\s+Connect to remote daemon/); @@ -1176,7 +1176,7 @@ test('usageForCommand resolves react-devtools help topic', () => { assert.match(help, /Do not write agent-devtools/); assert.match(help, /agent-device network dump --include headers/); assert.match(help, /@c refs reset after reload\/remount/); - assert.match(help, /isolated --state-dir/); + assert.match(help, /use separate sessions\/devices/); assert.match(help, /local service tunnel/); assert.match(help, /Remote iOS apps attempt the legacy React DevTools websocket/); }); diff --git a/src/utils/cli-help.ts b/src/utils/cli-help.ts index 3297254a1..3fe3751d0 100644 --- a/src/utils/cli-help.ts +++ b/src/utils/cli-help.ts @@ -62,7 +62,7 @@ const CONFIGURATION_LINES = [ ] as const; const ENVIRONMENT_LINES = [ - { label: 'AGENT_DEVICE_SESSION', description: 'Default session name' }, + { label: 'AGENT_DEVICE_SESSION', description: 'Explicit session name' }, { label: 'AGENT_DEVICE_PLATFORM', description: 'Default platform binding' }, { label: 'AGENT_DEVICE_SESSION_LOCK', description: 'Bound-session conflict mode' }, { label: 'AGENT_DEVICE_DAEMON_BASE_URL', description: 'Connect to remote daemon' }, @@ -340,7 +340,7 @@ Rules: Keep the profile window narrow; unrelated navigation makes render data noisy. Do not repeatedly raise broad profile slow limits such as --limit 50, --limit 200, or --limit 500. Drill into a specific @c ref with profile report unless you have a specific target that needs more rows. For network evidence, use agent-device network dump --include headers; headers is not a positional argument. - For cross-platform validation with explicit device selectors, prefer isolated --state-dir and restart react-devtools between platforms. + For cross-platform validation with explicit device selectors, use separate sessions/devices and restart react-devtools between platforms. Remote Android and iOS bridge runs normally through agent-device react-devtools; the CLI keeps the needed local service tunnel alive until agent-device react-devtools stop or disconnect. Expo support depends on the SDK's bundled React Native runtime. Remote iOS apps attempt the legacy React DevTools websocket during JavaScript startup. If the app was already open before react-devtools start, run open --platform ios --relaunch, then wait --connected. diff --git a/test/skillgym/suites/agent-device-smoke-suite.ts b/test/skillgym/suites/agent-device-smoke-suite.ts index 169e6e2db..d087c22f6 100644 --- a/test/skillgym/suites/agent-device-smoke-suite.ts +++ b/test/skillgym/suites/agent-device-smoke-suite.ts @@ -914,15 +914,9 @@ const SKILL_GUIDANCE_CASES: Case[] = [ 'Target app display name is known: Agent Device Tester', 'Package id is unknown', 'No app session is open yet', - 'Session name: discovery', - ], - task: 'Plan the bootstrap commands to discover the correct Android device and app identifier, then open the discovered app in the named session.', - outputs: [ - plannedCommand('devices'), - plannedCommand('apps'), - plannedCommand('open'), - /--session/i, ], + task: 'Plan the bootstrap commands to discover the correct Android device and app identifier, then open the discovered app.', + outputs: [plannedCommand('devices'), plannedCommand('apps'), plannedCommand('open')], forbiddenOutputs: [/com\.agent\.device\.tester/i, /com\.example/i], }), makeCase({ @@ -1394,12 +1388,10 @@ const SKILL_GUIDANCE_CASES: Case[] = [ 'Expected async result selector: id="diagnostics-error"', 'The diagnostics request can take 5-10 seconds', 'Need React component offenders and network evidence', - 'Before beginning the shared-device flow, run session list to discover/reuse any active session', 'Open Agent Device Tester on Android and take snapshot -i before interacting', ], task: 'Plan commands for a focused React Native performance run around the Settings diagnostics load flow, including debug markers, async verification, slow/rerender output, and network headers.', outputs: [ - plannedCommand('session list'), plannedCommand('open'), /snapshot -i/i, /logs (?:clear --restart|start)/i, diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 7f81ed7d4..be5625396 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -70,10 +70,10 @@ agent-device app-switcher - `rotate ` forces a mobile device into `portrait`, `portrait-upside-down`, `landscape-left`, or `landscape-right`. - `rotate` is supported on iOS and Android mobile targets. macOS and tvOS do not expose it. - On iOS devices, `http(s)://` URLs open in Safari when no app is active. Custom scheme URLs require an active app in the session. -- `AGENT_DEVICE_SESSION` and `AGENT_DEVICE_PLATFORM` can pre-bind a default session/platform for CLI automation runs, so normal commands (`open`, `snapshot`, `press`, `fill`, `screenshot`, `devices`, and `batch`) do not need those flags repeated on every call. +- Commands that omit `--session` use an implicit `default` session scoped to the caller's current git worktree or working directory. This keeps independent local agents from accidentally attaching to each other's default session. +- `--session ` or `AGENT_DEVICE_SESSION` opt into an explicitly named session when a script intentionally wants to share or reuse that session name. - A configured `AGENT_DEVICE_SESSION` implies bound-session lock mode by default. The CLI forwards that policy to the daemon, which enforces the same conflict handling for CLI, typed client, and direct RPC requests. -- `--session-lock reject|strip` sets the lock policy for a single CLI invocation, including nested batch steps. -- `AGENT_DEVICE_SESSION_LOCK=reject|strip` sets the default lock policy for bound-session automation runs. Older lock aliases remain accepted for compatibility, but new automation should use `--session-lock` or `AGENT_DEVICE_SESSION_LOCK`. +- `--session-lock reject|strip` and `AGENT_DEVICE_SESSION_LOCK=reject|strip` remain available for explicit named-session automation. Older lock aliases remain accepted for compatibility. - Direct RPC callers can pass `meta.lockPolicy` and optional `meta.lockPlatform` on `agent_device.command` requests for the same daemon-enforced behavior. - In `batch`, steps that omit `platform` still inherit the parent batch `--platform`; lock-mode defaults do not override that parent setting. - Tenant-scoped daemon runs can pass `--tenant`, `--session-isolation tenant`, `--run-id`, and `--lease-id` to enforce lease admission. @@ -617,7 +617,7 @@ agent-device react-devtools profile report @c5 - For Android and iOS sessions connected through a remote bridge profile, `react-devtools` registers a lease-scoped companion tunnel to the sandbox-local DevTools daemon at `127.0.0.1:8097`. Android bridge profiles use the bridge-owned remote `adb reverse` mapping; iOS bridge profiles use the bridge-owned wildcard Metro host tunnel. The CLI keeps the companion alive until `agent-device react-devtools stop` or `agent-device disconnect`. - For remote iOS bridge sessions, open the app once to create the bridge session, run `agent-device react-devtools start`, then relaunch the same bundle id with `agent-device open --platform ios --relaunch` before `wait --connected`. React Native attempts the legacy DevTools websocket during JavaScript startup, so starting DevTools after the first launch can miss that connection attempt. - Remote bridge React DevTools assumes the React Native-bundled DevTools behavior in React Native 0.83+. Older browser/Chromium DevTools workflows are not assumed to exist inside remote sandboxes. Expo projects should be verified against the SDK's bundled React Native version before relying on this path; this release does not claim a separately verified Expo SDK version. -- For cross-platform validation with explicit target selectors, prefer an isolated `--state-dir` over separate named sessions. Named sessions enable bound-session locks during setup. Restart `react-devtools` between iOS and Android runs. +- For cross-platform validation with explicit target selectors, use separate sessions/devices and restart `react-devtools` between iOS and Android runs. ## Metro reload @@ -808,7 +808,7 @@ agent-device session list agent-device session list --json ``` -- `session list` shows active daemon sessions and their tracked device/app context. +- `session list` shows active daemon sessions for the caller's implicit workspace scope, or the explicitly named session scope when `--session` / `AGENT_DEVICE_SESSION` is configured. - Use `--json` when you want to inspect or script against the raw session metadata. ## iOS device prerequisites diff --git a/website/docs/docs/configuration.md b/website/docs/docs/configuration.md index b1fa0cb46..23ced9640 100644 --- a/website/docs/docs/configuration.md +++ b/website/docs/docs/configuration.md @@ -86,10 +86,10 @@ Command-specific defaults are supported too, for example `snapshotDepth`, `snaps Use a numeric `artifact` value for an artifact ID. Use a string `artifact` value for an artifact name. -Bound-session defaults use the same config and env mapping too: +Explicit named-session lock defaults use the same config and env mapping too: - `sessionLock` -> `AGENT_DEVICE_SESSION_LOCK` -Older bound-session lock config aliases remain accepted for compatibility, but new configs and automation should use `sessionLock`, `--session-lock`, or `AGENT_DEVICE_SESSION_LOCK`. +Older bound-session lock config aliases remain accepted for compatibility. Most local automation can omit this because implicit `default` sessions are workspace-scoped; use `sessionLock`, `--session-lock`, or `AGENT_DEVICE_SESSION_LOCK` when intentionally running an explicitly named session. ## Supported environment variables diff --git a/website/docs/docs/sessions.md b/website/docs/docs/sessions.md index 1e70254ac..a09de37c6 100644 --- a/website/docs/docs/sessions.md +++ b/website/docs/docs/sessions.md @@ -9,11 +9,15 @@ Sessions keep device state and snapshots consistent across commands. ```bash agent-device open Settings --platform ios agent-device session list -agent-device open Contacts # change app while reusing the default session +agent-device open Contacts # change app in this workspace's default session agent-device close ``` -Open another session independently (for parallel work): +The implicit `default` session is scoped to the caller's git worktree or current working directory. +Independent agents in different worktrees do not attach to each other's default session. +When a session is established, human output includes a `Session state: ` line and JSON output includes `sessionStateDir`; this is the per-session artifact directory that can be inspected or removed after the run. + +Open an explicitly named session only when you intentionally want a shared/reusable handle: ```bash agent-device open Contacts --platform ios --session my-session @@ -35,6 +39,6 @@ Notes: - On iOS devices, `http(s)://` URLs open in Safari when no app is active. Custom scheme URLs require an active app in the session. - On iOS, `appstate` is session-scoped and requires a matching active session on the target device. - For remote `connect --remote-config` sessions, see [Commands](/docs/commands#remote-metro-workflow). -- Use `--session ` to run multiple sessions in parallel. Do not parallelize mutating commands against the same session; serialize stateful actions such as open, press, fill, type, scroll, back, alert, replay, batch, and close. +- Use `--session ` for intentional named-session sharing. Do not parallelize mutating commands against the same session; serialize stateful actions such as open, press, fill, type, scroll, back, alert, replay, batch, and close. For replay scripts and deterministic E2E guidance, see [Replay & E2E](/docs/replay-e2e). From 794ba07b3bf31491b6fd09f463f00ad61ffa4edc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 13:33:40 -0700 Subject: [PATCH 8/9] fix: remove unused session store check --- src/daemon/handlers/__tests__/session.test.ts | 2 +- src/daemon/handlers/session-open.ts | 7 ++----- src/daemon/session-store.ts | 4 ---- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/daemon/handlers/__tests__/session.test.ts b/src/daemon/handlers/__tests__/session.test.ts index 3ecedc460..7bbfcde08 100644 --- a/src/daemon/handlers/__tests__/session.test.ts +++ b/src/daemon/handlers/__tests__/session.test.ts @@ -4521,7 +4521,7 @@ test('open does not retain a session when the request was canceled before comple expect(response.error.code).toBe('COMMAND_FAILED'); expect(response.error.message).toBe('request canceled'); } - expect(sessionStore.has('default')).toBe(false); + expect(sessionStore.get('default')).toBeUndefined(); } finally { clearRequestCanceled(requestId); } diff --git a/src/daemon/handlers/session-open.ts b/src/daemon/handlers/session-open.ts index 1efc757ad..aacba7e7c 100644 --- a/src/daemon/handlers/session-open.ts +++ b/src/daemon/handlers/session-open.ts @@ -286,11 +286,8 @@ export async function handleOpenCommand(params: { }): Promise { const { req, sessionName, logPath, sessionStore } = params; - if (sessionStore.has(sessionName)) { - const session = sessionStore.get(sessionName); - if (!session) { - return errorResponse('SESSION_NOT_FOUND', `Session "${sessionName}" not found.`); - } + const session = sessionStore.get(sessionName); + if (session) { const shouldRelaunch = req.flags?.relaunch === true; const requestedOpenTarget = req.positionals?.[0]; const openTarget = requestedOpenTarget ?? (shouldRelaunch ? session.appName : undefined); diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index 9b8aa4a8c..3df94f4b8 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -21,10 +21,6 @@ export class SessionStore { return this.sessions.get(name); } - has(name: string): boolean { - return this.sessions.has(name); - } - set(name: string, session: SessionState): void { this.sessions.set(name, session); } From 4f6735a03a5e809ddfbf39d16d6c4ae56ef154c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 2 Jun 2026 13:48:50 -0700 Subject: [PATCH 9/9] test: trim duplicate session recovery coverage --- .../__tests__/request-finalization.test.ts | 28 ----- .../__tests__/request-lock-policy.test.ts | 107 ------------------ src/daemon/__tests__/session-routing.test.ts | 35 +----- src/daemon/__tests__/session-selector.test.ts | 16 --- .../handlers/__tests__/record-trace.test.ts | 36 ------ 5 files changed, 1 insertion(+), 221 deletions(-) diff --git a/src/daemon/__tests__/request-finalization.test.ts b/src/daemon/__tests__/request-finalization.test.ts index 029a997fc..8eeba9a37 100644 --- a/src/daemon/__tests__/request-finalization.test.ts +++ b/src/daemon/__tests__/request-finalization.test.ts @@ -29,31 +29,3 @@ test('finalizeDaemonResponse preserves handler error hints from details', () => expect(finalized.error.hint).toBe('Run agent-device session list and reuse --session default.'); } }); - -test('finalizeDaemonResponse prefers top-level error hint over details hint', () => { - const req: DaemonRequest = { - token: 'token', - session: 'default', - command: 'open', - positionals: [], - flags: {}, - }; - const response: DaemonResponse = { - ok: false, - error: { - code: 'DEVICE_IN_USE', - message: 'Device is already in use by session "default".', - hint: 'Use the top-level hint.', - details: { - hint: 'Use the details hint.', - }, - }, - }; - - const finalized = finalizeDaemonResponse(req, response, () => 'artifact-id'); - - expect(finalized.ok).toBe(false); - if (!finalized.ok) { - expect(finalized.error.hint).toBe('Use the top-level hint.'); - } -}); diff --git a/src/daemon/__tests__/request-lock-policy.test.ts b/src/daemon/__tests__/request-lock-policy.test.ts index cf39341cb..d6b54505d 100644 --- a/src/daemon/__tests__/request-lock-policy.test.ts +++ b/src/daemon/__tests__/request-lock-policy.test.ts @@ -2,7 +2,6 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { applyRequestLockPolicy } from '../request-lock-policy.ts'; import type { SessionState } from '../types.ts'; -import { AppError } from '../../utils/errors.ts'; const IOS_SESSION: SessionState = { name: 'qa-ios', @@ -33,21 +32,6 @@ const ANDROID_SESSION: SessionState = { }, }; -const RECORDING_SESSION: SessionState = { - ...ANDROID_SESSION, - name: 'default', - recordOnlySession: true, - recording: { - platform: 'android', - outPath: '/tmp/recording.mp4', - remotePath: '/sdcard/recording.mp4', - remotePid: '1234', - startedAt: Date.now(), - showTouches: false, - gestureEvents: [], - }, -}; - test('rejects fresh-session selector conflicts under request lock policy', () => { assert.throws( () => @@ -68,37 +52,6 @@ test('rejects fresh-session selector conflicts under request lock policy', () => ); }); -test('reject lock policy explains fresh-session recovery commands', () => { - assert.throws( - () => - applyRequestLockPolicy({ - token: 'token', - session: 'qa-ios', - command: 'snapshot', - positionals: [], - flags: { - device: 'Pixel 9', - }, - meta: { - lockPolicy: 'reject', - lockPlatform: 'ios', - }, - }), - (error) => { - assert.ok(error instanceof AppError); - assert.match(error.message, /snapshot is using a bound-session lock for ios/i); - assert.match(error.message, /--device=Pixel 9/i); - assert.match(error.details?.hint ?? '', /Remove conflicting device selectors/i); - assert.match( - error.details?.hint ?? '', - /agent-device open --session qa-ios --platform ios/i, - ); - assert.match(error.details?.hint ?? '', /agent-device session list/i); - return true; - }, - ); -}); - test('allows open to choose a fresh-session target under request lock policy', () => { const req = applyRequestLockPolicy({ token: 'token', @@ -165,66 +118,6 @@ test('rejects existing-session selector conflicts under request lock policy', () ); }); -test('reject lock policy explains existing-session recovery commands', () => { - assert.throws( - () => - applyRequestLockPolicy( - { - token: 'token', - session: 'qa-ios', - command: 'snapshot', - positionals: [], - flags: { - serial: 'emulator-5554', - }, - meta: { - lockPolicy: 'reject', - }, - }, - IOS_SESSION, - ), - (error) => { - assert.ok(error instanceof AppError); - assert.match(error.message, /already bound to session "qa-ios"/i); - assert.match(error.message, /ios device "iPhone 16" \(SIM-001\)/i); - assert.match(error.message, /--serial=emulator-5554/i); - assert.match(error.details?.hint ?? '', /agent-device session list/i); - assert.match(error.details?.hint ?? '', /--session qa-ios/i); - assert.match(error.details?.hint ?? '', /agent-device close --session qa-ios/i); - return true; - }, - ); -}); - -test('reject lock policy explains recording-session recovery commands', () => { - assert.throws( - () => - applyRequestLockPolicy( - { - token: 'token', - session: 'default', - command: 'snapshot', - positionals: [], - flags: { - device: 'Pixel 8', - }, - meta: { - lockPolicy: 'reject', - }, - }, - RECORDING_SESSION, - ), - (error) => { - assert.ok(error instanceof AppError); - assert.match(error.message, /already bound to session "default"/i); - assert.match(error.details?.hint ?? '', /recording session "default"/i); - assert.match(error.details?.hint ?? '', /agent-device record stop --session default/i); - assert.match(error.details?.hint ?? '', /agent-device close --session default/i); - return true; - }, - ); -}); - test('allows inventory commands to use explicit Apple selectors under another lock platform', () => { const req = applyRequestLockPolicy({ token: 'token', diff --git a/src/daemon/__tests__/session-routing.test.ts b/src/daemon/__tests__/session-routing.test.ts index 454ec0d7e..cc37ae64f 100644 --- a/src/daemon/__tests__/session-routing.test.ts +++ b/src/daemon/__tests__/session-routing.test.ts @@ -4,11 +4,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { SessionStore } from '../session-store.ts'; -import { - resolveEffectiveSessionName, - resolveImplicitSessionScope, - sessionMatchesScope, -} from '../session-routing.ts'; +import { resolveEffectiveSessionName } from '../session-routing.ts'; import type { SessionState } from '../types.ts'; function makeSession(name: string): SessionState { @@ -115,32 +111,3 @@ test('keeps explicitly configured default session global', (t) => { assert.equal(resolved, 'default'); }); - -test('matches sessions only within the same implicit scope', (t) => { - const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'agent-device-cwd-scope-')); - t.onTestFinished(() => { - fs.rmSync(cwd, { recursive: true, force: true }); - }); - const req = { - token: 't', - session: 'default', - command: 'session_list', - positionals: [], - flags: {}, - meta: { cwd }, - }; - const scope = resolveImplicitSessionScope(req); - assert.ok(scope); - - assert.equal( - sessionMatchesScope({ ...makeSession('default'), sessionScope: scope }, scope), - true, - ); - assert.equal( - sessionMatchesScope( - { ...makeSession('default'), sessionScope: { kind: 'cwd', id: 'other' } }, - scope, - ), - false, - ); -}); diff --git a/src/daemon/__tests__/session-selector.test.ts b/src/daemon/__tests__/session-selector.test.ts index edb862f60..f65fe4d31 100644 --- a/src/daemon/__tests__/session-selector.test.ts +++ b/src/daemon/__tests__/session-selector.test.ts @@ -60,22 +60,6 @@ test('selector mismatch explains session recovery commands', () => { ); }); -test('selector mismatch quotes unsafe session names in recovery commands', () => { - const session = makeSession({ name: "default session; echo 'oops'" }); - assert.throws( - () => assertSessionSelectorMatches(session, { platform: 'ios' }), - (err: unknown) => { - assert.ok(err instanceof AppError); - assert.match(err.details?.hint ?? '', /--session 'default session; echo '\\''oops'\\'''/); - assert.match( - err.details?.hint ?? '', - /agent-device close --session 'default session; echo '\\''oops'\\'''/, - ); - return true; - }, - ); -}); - test('accepts --platform apple alias for ios sessions', () => { const session = makeSession({ device: { diff --git a/src/daemon/handlers/__tests__/record-trace.test.ts b/src/daemon/handlers/__tests__/record-trace.test.ts index 40864a0bf..267e0302e 100644 --- a/src/daemon/handlers/__tests__/record-trace.test.ts +++ b/src/daemon/handlers/__tests__/record-trace.test.ts @@ -280,42 +280,6 @@ test('record stop releases session created only for recording', async () => { expect(sessionStore.get(sessionName)).toBeUndefined(); }); -test('record stop releases record-only session even when recording state is stale', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'stale-record-only-session'; - const session = makeIosSimulatorSession(sessionName); - session.recordOnlySession = true; - sessionStore.set(sessionName, session); - - const response = await runRecordCommand({ - sessionStore, - sessionName, - positionals: ['stop'], - }); - - expect(response?.ok).toBe(false); - expect(sessionStore.get(sessionName)).toBeUndefined(); -}); - -test('record stop keeps normal app session open after recording', async () => { - const sessionStore = makeSessionStore(); - const sessionName = 'app-session'; - const session = makeIosSimulatorRecordingSession(sessionName, { - appBundleId: 'com.apple.Preferences', - }); - sessionStore.set(sessionName, session); - - const response = await runRecordCommand({ - sessionStore, - sessionName, - positionals: ['stop'], - }); - - expect(response?.ok).toBe(true); - expect(sessionStore.get(sessionName)).toBe(session); - expect(sessionStore.get(sessionName)?.recording).toBeUndefined(); -}); - test('record stop keeps normal app session open when stop validation fails', async () => { vi.useFakeTimers(); vi.setSystemTime(20_000);