fix(device-record): refuse multi-sim ambiguity, thread deviceId through script (closes #173 sub-issue 1)

scripts/cdp-bridge/dist/tools/device-record.js

@@ -11,6 +11,97 @@ const START_TIMEOUT_MS = 10_000;
 const STOP_TIMEOUT_MS = 60_000;
 const STATUS_TIMEOUT_MS = 5_000;
 const GIF_TIMEOUT_MS = 60_000;
+export function parseAllBootedIosDevices(jsonText) {
+    let data;
+    try {
+        data = JSON.parse(jsonText);
+    }
+    catch {
+        return [];
+    }
+    const runtimes = data?.devices;
+    if (!runtimes || typeof runtimes !== 'object')
+        return [];
+    const out = [];
+    for (const list of Object.values(runtimes)) {
+        if (!Array.isArray(list))
+            continue;
+        for (const device of list) {
+            if (device && device.state === 'Booted' && typeof device.udid === 'string' && device.udid.length > 0) {
+                out.push({ udid: device.udid, state: device.state, name: device.name });
+            }
+        }
+    }
+    return out;
+}
+export function parseAllAdbDevices(stdout) {
+    const out = [];
+    for (const raw of stdout.split('\n')) {
+        const line = raw.trim();
+        if (!line || line.startsWith('List of devices'))
+            continue;
+        // Match any serial — not just `emulator-NNNN` — so physical devices count
+        // toward multi-device ambiguity detection.
+        const m = line.match(/^(\S+)\s+(device|offline|unauthorized)\b/);
+        if (!m)
+            continue;
+        out.push({ serial: m[1], state: m[2] });
+    }
+    return out;
+}
+async function listBootedIosUdids() {
+    try {
+        const { stdout } = await execFileAsync('xcrun', ['simctl', 'list', '-j', 'devices', 'booted'], {
+            timeout: 5000,
+            maxBuffer: 1024 * 1024,
+        });
+        return parseAllBootedIosDevices(stdout);
+    }
+    catch {
+        return [];
+    }
+}
+async function listConnectedAndroidDevices() {
+    try {
+        const { stdout } = await execFileAsync('adb', ['devices'], {
+            timeout: 5000,
+            maxBuffer: 1024 * 1024,
+        });
+        return parseAllAdbDevices(stdout).filter((d) => d.state === 'device');
+    }
+    catch {
+        return [];
+    }
+}
+/**
+ * Pre-flight target resolution for `device_record start`. Returns the
+ * device id to use, or a structured ambiguity error listing the
+ * candidates the caller must pick from. Pure: takes the candidate list
+ * as input so the unit tests don't need to spawn xcrun/adb.
+ *
+ * Rules:
+ *   - 0 candidates → caller's NO_DEVICE path handles it (we don't fire here)
+ *   - 1 candidate  → auto-select, mark autoSelected: true
+ *   - >1 + explicit deviceId matches a candidate → use it
+ *   - >1 + explicit deviceId does NOT match → AMBIGUOUS with the full list
+ *     (so caller sees the exact valid ids — typos surface fast)
+ *   - >1 + no deviceId → AMBIGUOUS (the GH #173 bug fix surface)
+ */
+export function resolveTargetDevice(candidates, deviceId) {
+    // An explicit deviceId is authoritative regardless of candidate count.
+    // If the user said "record on X", we must record on X or refuse — silently
+    // picking a different device is the exact bug GH #173 reports.
+    if (deviceId) {
+        if (candidates.some((c) => c.id === deviceId)) {
+            return { ok: true, deviceId, autoSelected: false, totalAvailable: candidates.length };
+        }
+        return { ok: false, reason: 'AMBIGUOUS', candidates };
+    }
+    if (candidates.length === 1) {
+        return { ok: true, deviceId: candidates[0].id, autoSelected: true, totalAvailable: 1 };
+    }
+    return { ok: false, reason: 'AMBIGUOUS', candidates };
+}
 function getPluginRoot() {
     if (process.env.CLAUDE_PLUGIN_ROOT)
         return process.env.CLAUDE_PLUGIN_ROOT;
@@ -66,15 +157,46 @@ async function runStart(args) {
         return failResult(`Unknown platform: "${platform}". Expected ios or android.`);
     }
     const outputPath = args.outputPath ?? defaultOutputPath(platform);
+    // GH #173 sub-issue 1: pre-flight multi-device disambiguation. The shell
+    // script's `simctl io booted` / `adb devices` resolution picks
+    // non-deterministically when more than one device is booted/connected,
+    // and silently captures the wrong one. Refuse to start until the
+    // caller pins a target with `deviceId`.
+    const candidates = platform === 'ios'
+        ? (await listBootedIosUdids()).map((d) => ({ id: d.udid, label: d.name }))
+        : (await listConnectedAndroidDevices()).map((d) => ({ id: d.serial }));
+    if (candidates.length === 0) {
+        return failResult(platform === 'ios' ? 'No iOS simulator booted.' : 'No Android device connected.', { code: 'NO_DEVICE' });
+    }
+    const resolution = resolveTargetDevice(candidates, args.deviceId);
+    if (!resolution.ok) {
+        const list = resolution.candidates
+            .map((c) => `  - ${c.id}${c.label ? ` (${c.label})` : ''}`)
+            .join('\n');
+        const argName = platform === 'ios' ? 'UDID' : 'serial';
+        return failResult(`device_record: ${resolution.candidates.length} ${platform} ${argName === 'UDID' ? 'simulators booted' : 'devices connected'} — refusing to auto-pick to avoid recording the wrong device. ` +
+            `Pass deviceId=<${argName}> to disambiguate:\n${list}`, { code: 'DEVICE_AMBIGUOUS', platform, candidates: resolution.candidates });
+    }
+    const scriptArgs = ['start', platform, outputPath];
+    // Only forward an explicit id when we're picking from >1 candidate; the
+    // single-device case keeps the script's existing `booted`/auto path so
+    // we don't regress any environment where simctl's `booted` shorthand
+    // works differently than passing the literal UDID (defensive — both
+    // should be equivalent on Apple's side).
+    if (!resolution.autoSelected) {
+        scriptArgs.push(platform === 'ios' ? '--udid' : '--serial', resolution.deviceId);
+    }
     try {
-        const { stdout } = await execFileAsync(getRecordScript(), ['start', platform, outputPath], { timeout: START_TIMEOUT_MS });
+        const { stdout } = await execFileAsync(getRecordScript(), scriptArgs, { timeout: START_TIMEOUT_MS });
         const parsed = parseStartOutput(stdout);
         if (!parsed) {
             return failResult(`Recording started but could not parse PID/output. Raw: ${stdout.trim()}`);
         }
         return okResult({
             action: 'start',
             platform,
+            deviceId: resolution.deviceId,
+            autoSelected: resolution.autoSelected,
             output: parsed.output,
             pid: parsed.pid,
             note: 'Call device_record action=stop to finalize. Android caps at 180s; iOS has no inherent cap but xcrun simctl io may stall on long captures.',

scripts/cdp-bridge/src/index.ts

@@ -743,11 +743,12 @@ trackedTool(
 
 trackedTool(
   'device_record',
-  'Cross-platform screen recording for proof captures. Wraps xcrun simctl io recordVideo (iOS) and adb shell screenrecord (Android), auto-pulls Android files to the host, converts to MP4 with faststart via ffmpeg. Three actions: action="start" begins a background recording (returns pid + output path); action="stop" finalizes ALL active recordings (returns saved files; pass gif=true to also produce GIFs via ffmpeg); action="status" lists active recordings. Android caps at 180s per recording. iOS may stall on long captures via xcrun simctl. Session-less.',
+  'Cross-platform screen recording for proof captures. Wraps xcrun simctl io recordVideo (iOS) and adb shell screenrecord (Android), auto-pulls Android files to the host, converts to MP4 with faststart via ffmpeg. Three actions: action="start" begins a background recording (returns pid + output path + the deviceId actually used); action="stop" finalizes ALL active recordings (returns saved files; pass gif=true to also produce GIFs via ffmpeg); action="status" lists active recordings. Android caps at 180s per recording. iOS may stall on long captures via xcrun simctl. GH #173: when more than one simulator is booted (or more than one Android device connected), start refuses to auto-pick to avoid recording the wrong device — pass deviceId=<UDID|serial> to disambiguate; the response echoes the deviceId actually used so you can verify. Session-less.',
   {
     action: z.enum(['start', 'stop', 'status']).describe('start: begin recording. stop: finalize and save (all active recordings). status: list active recordings.'),
     platform: z.enum(['ios', 'android']).optional().describe('(start only) Force platform. Auto-detected from booted devices if omitted.'),
     outputPath: z.string().optional().describe('(start only) Absolute output path. Defaults to /tmp/rn-dev-agent-proof-<platform>-<timestamp>.mp4.'),
+    deviceId: z.string().optional().describe('(start only) Explicit target identifier (iOS UDID or Android serial). Required when more than one device of the same platform is booted/connected — without it, start fails with code=DEVICE_AMBIGUOUS and lists the candidates. Auto-selected when exactly one device is available.'),
     gif: z.boolean().optional().describe('(stop only) When true, also convert each saved recording to GIF via ffmpeg.'),
     gifPath: z.string().optional().describe('(stop only) Override GIF output path. Defaults to the recording path with .gif extension.'),
   },

scripts/cdp-bridge/src/tools/device-record.ts

@@ -21,6 +21,134 @@ export interface DeviceRecordArgs {
   outputPath?: string;
   gif?: boolean;
   gifPath?: string;
+  /**
+   * GH #173 (sub-issue 1): explicit target identifier for multi-device
+   * scenarios. iOS UDID for `simctl io <UDID> recordVideo`, Android
+   * serial for `adb -s <SERIAL> shell screenrecord`. Required when more
+   * than one device of the same platform is booted/connected — without
+   * it, `simctl io booted` and `adb devices` pick non-deterministically
+   * and silently capture the wrong device (the user's reported pain).
+   */
+  deviceId?: string;
+}
+
+interface SimctlDevice {
+  udid: string;
+  state: string;
+  name?: string;
+}
+interface SimctlListPayload {
+  devices?: Record<string, SimctlDevice[]>;
+}
+
+export function parseAllBootedIosDevices(jsonText: string): SimctlDevice[] {
+  let data: SimctlListPayload;
+  try {
+    data = JSON.parse(jsonText) as SimctlListPayload;
+  } catch {
+    return [];
+  }
+  const runtimes = data?.devices;
+  if (!runtimes || typeof runtimes !== 'object') return [];
+  const out: SimctlDevice[] = [];
+  for (const list of Object.values(runtimes)) {
+    if (!Array.isArray(list)) continue;
+    for (const device of list) {
+      if (device && device.state === 'Booted' && typeof device.udid === 'string' && device.udid.length > 0) {
+        out.push({ udid: device.udid, state: device.state, name: device.name });
+      }
+    }
+  }
+  return out;
+}
+
+export interface AdbDevice {
+  serial: string;
+  state: 'device' | 'offline' | 'unauthorized';
+}
+
+export function parseAllAdbDevices(stdout: string): AdbDevice[] {
+  const out: AdbDevice[] = [];
+  for (const raw of stdout.split('\n')) {
+    const line = raw.trim();
+    if (!line || line.startsWith('List of devices')) continue;
+    // Match any serial — not just `emulator-NNNN` — so physical devices count
+    // toward multi-device ambiguity detection.
+    const m = line.match(/^(\S+)\s+(device|offline|unauthorized)\b/);
+    if (!m) continue;
+    out.push({ serial: m[1], state: m[2] as 'device' | 'offline' | 'unauthorized' });
+  }
+  return out;
+}
+
+async function listBootedIosUdids(): Promise<SimctlDevice[]> {
+  try {
+    const { stdout } = await execFileAsync('xcrun', ['simctl', 'list', '-j', 'devices', 'booted'], {
+      timeout: 5000,
+      maxBuffer: 1024 * 1024,
+    });
+    return parseAllBootedIosDevices(stdout);
+  } catch {
+    return [];
+  }
+}
+
+async function listConnectedAndroidDevices(): Promise<AdbDevice[]> {
+  try {
+    const { stdout } = await execFileAsync('adb', ['devices'], {
+      timeout: 5000,
+      maxBuffer: 1024 * 1024,
+    });
+    return parseAllAdbDevices(stdout).filter((d) => d.state === 'device');
+  } catch {
+    return [];
+  }
+}
+
+export interface DeviceResolution {
+  ok: true;
+  deviceId: string;
+  autoSelected: boolean;
+  totalAvailable: number;
+}
+
+export interface DeviceResolutionAmbiguous {
+  ok: false;
+  reason: 'AMBIGUOUS';
+  candidates: Array<{ id: string; label?: string }>;
+}
+
+/**
+ * Pre-flight target resolution for `device_record start`. Returns the
+ * device id to use, or a structured ambiguity error listing the
+ * candidates the caller must pick from. Pure: takes the candidate list
+ * as input so the unit tests don't need to spawn xcrun/adb.
+ *
+ * Rules:
+ *   - 0 candidates → caller's NO_DEVICE path handles it (we don't fire here)
+ *   - 1 candidate  → auto-select, mark autoSelected: true
+ *   - >1 + explicit deviceId matches a candidate → use it
+ *   - >1 + explicit deviceId does NOT match → AMBIGUOUS with the full list
+ *     (so caller sees the exact valid ids — typos surface fast)
+ *   - >1 + no deviceId → AMBIGUOUS (the GH #173 bug fix surface)
+ */
+export function resolveTargetDevice(
+  candidates: Array<{ id: string; label?: string }>,
+  deviceId: string | undefined,
+): DeviceResolution | DeviceResolutionAmbiguous {
+  // An explicit deviceId is authoritative regardless of candidate count.
+  // If the user said "record on X", we must record on X or refuse — silently
+  // picking a different device is the exact bug GH #173 reports.
+  if (deviceId) {
+    if (candidates.some((c) => c.id === deviceId)) {
+      return { ok: true, deviceId, autoSelected: false, totalAvailable: candidates.length };
+    }
+    return { ok: false, reason: 'AMBIGUOUS', candidates };
+  }
+  if (candidates.length === 1) {
+    return { ok: true, deviceId: candidates[0].id, autoSelected: true, totalAvailable: 1 };
+  }
+  return { ok: false, reason: 'AMBIGUOUS', candidates };
 }
 
 function getPluginRoot(): string {
@@ -97,10 +225,49 @@ async function runStart(args: DeviceRecordArgs): Promise<ToolResult> {
   }
   const outputPath = args.outputPath ?? defaultOutputPath(platform);
 
+  // GH #173 sub-issue 1: pre-flight multi-device disambiguation. The shell
+  // script's `simctl io booted` / `adb devices` resolution picks
+  // non-deterministically when more than one device is booted/connected,
+  // and silently captures the wrong one. Refuse to start until the
+  // caller pins a target with `deviceId`.
+  const candidates = platform === 'ios'
+    ? (await listBootedIosUdids()).map((d) => ({ id: d.udid, label: d.name }))
+    : (await listConnectedAndroidDevices()).map((d) => ({ id: d.serial }));
+
+  if (candidates.length === 0) {
+    return failResult(
+      platform === 'ios' ? 'No iOS simulator booted.' : 'No Android device connected.',
+      { code: 'NO_DEVICE' },
+    );
+  }
+
+  const resolution = resolveTargetDevice(candidates, args.deviceId);
+  if (!resolution.ok) {
+    const list = resolution.candidates
+      .map((c) => `  - ${c.id}${c.label ? ` (${c.label})` : ''}`)
+      .join('\n');
+    const argName = platform === 'ios' ? 'UDID' : 'serial';
+    return failResult(
+      `device_record: ${resolution.candidates.length} ${platform} ${argName === 'UDID' ? 'simulators booted' : 'devices connected'} — refusing to auto-pick to avoid recording the wrong device. ` +
+      `Pass deviceId=<${argName}> to disambiguate:\n${list}`,
+      { code: 'DEVICE_AMBIGUOUS', platform, candidates: resolution.candidates },
+    );
+  }
+
+  const scriptArgs = ['start', platform, outputPath];
+  // Only forward an explicit id when we're picking from >1 candidate; the
+  // single-device case keeps the script's existing `booted`/auto path so
+  // we don't regress any environment where simctl's `booted` shorthand
+  // works differently than passing the literal UDID (defensive — both
+  // should be equivalent on Apple's side).
+  if (!resolution.autoSelected) {
+    scriptArgs.push(platform === 'ios' ? '--udid' : '--serial', resolution.deviceId);
+  }
+
   try {
     const { stdout } = await execFileAsync(
       getRecordScript(),
-      ['start', platform, outputPath],
+      scriptArgs,
       { timeout: START_TIMEOUT_MS },
     );
     const parsed = parseStartOutput(stdout);
@@ -110,6 +277,8 @@ async function runStart(args: DeviceRecordArgs): Promise<ToolResult> {
     return okResult({
       action: 'start',
       platform,
+      deviceId: resolution.deviceId,
+      autoSelected: resolution.autoSelected,
       output: parsed.output,
       pid: parsed.pid,
       note: 'Call device_record action=stop to finalize. Android caps at 180s; iOS has no inherent cap but xcrun simctl io may stall on long captures.',