fix: quote session recovery commands

Author: thymikee

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<DaemonPaths, 'infoPath' | 'lockPath'>) {
   return `rm -f ${shellQuote(paths.infoPath)} ${shellQuote(paths.lockPath)}`;
 }
-
-function shellQuote(value: string): string {
-  return `'${value.replaceAll("'", "'\\''")}'`;
-}

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 <app> --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);

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: {

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';

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 },

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);
 

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<DaemonRequest['meta']>['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 <app>${sessionText} --platform ${lockPlatform} first if no session is active. `
     : `Run agent-device open <app>${sessionText} first if no session is active. `;

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}.`
   );
 }

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[];

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);
+}