fix(device): hard-fail device_screenshot on explicit-platform raw failure (closes #136 sub-issue 1) (#174)

* chore: gitignore codex-pair per-developer artifacts

codex-pair (ask-llm plugin) is opt-in via a .codex-pair-context.md marker
in the project root. The marker enables the PostToolUse hook; its content
is the per-file project context the reviewer uses.

Marker is per-developer (own model/threshold preferences, own rules) so
it does not belong in shared history. Same for the log, cache, and pause
sentinel directories — all developer-local.

See https://github.com/Lykhoyda/ask-llm marketplace plugin for details.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(device): hard-fail device_screenshot when platform: explicit and raw path fails

Closes #136 (sub-issue 1 — re-evidence on #60).

PR #142's "graceful degradation" — on raw screenshot failure, fall through to
runAgentDevice — was the regression vector. agent-device's --platform routing
is exactly the broken path PR #142 was created to avoid. The fallback re-
introduced the bug whenever the raw path tripped on transient state (the
user's session: OOM-unstable emulator → adb devices reports it as 'offline'
→ parseAdbDevicesEmu skips it → resolver returns null → fallback fires →
iOS screen captured for a platform=android request).

New behavior: when platform: is passed explicitly and the raw path fails,
return a structured failResult with code=SCREENSHOT_FAILED and
meta.reason=('no-device' | 'capture-failed') instead of trying agent-device.
The error message names the underlying CLI (xcrun simctl / adb) and the
sub-cause so the caller knows what to fix (boot a device vs retry a transient
state). Implicit-platform calls (platformExplicit=false) still route through
runAgentDevice — backward parity preserved by the existing
'platformExplicit=false → uses runAgentDevice (backward parity)' test.

Changes:
- src/tools/device-screenshot-raw.ts: tryRawScreenshot now returns a
  discriminated union {ok:true,path} | {ok:false,reason} so callers can
  distinguish resolver miss from capturer failure.
- src/tools/device-list.ts: captureAndResizeScreenshot returns failResult
  on explicit-platform raw failure; imports failResult.
- test/unit/gh-136-screenshot-raw-platform.test.js: inverted the existing
  'falls through' test to assert hard-fail; added a second hard-fail test
  for the capture-failed surface; tightened the three pre-existing
  resolver/capturer null-checks to deepEqual on the new union shape.

Verified: 1465/1465 cdp-bridge unit tests passing (+1 net new test).

Also included: scripts/cdp-bridge/dist/tools/device-permission.js — dist
was stale relative to source (PR #169's second escapeRegex site fix never
made it to dist). Rebuilt locally; no source change.

NOTE: codex-pair flagged an unrelated pre-existing issue in this file:
deriveScreenshotPath throws PathTraversalScreenshotError on a '..' path,
but no catch wraps the handler, so a bad path becomes an uncaught tool
exception. Filed as #136-followup, NOT addressed here.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: Lykhoyda

.gitignore

@@ -20,3 +20,9 @@ scripts/rn-fast-runner/**/DerivedData/
 
 # deepsec scanner workspace (local-only — config, data/<id>/INFO.md, scan output)
 .deepsec/
+
+# codex-pair (per-developer opt-in; marker presence enables the hook)
+.codex-pair-context.md
+.codex-pair-log.jsonl
+.codex-pair-cache/
+.codex-pair-state/

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

@@ -1,4 +1,5 @@
 import { runAgentDevice } from '../agent-device-wrapper.js';
+import { failResult } from '../utils.js';
 import { resizeWithSips } from './device-screenshot-resize.js';
 import { tryRawScreenshot } from './device-screenshot-raw.js';
 import { pathHasTraversal } from '../domain/path-safety.js';
@@ -146,18 +147,29 @@ export async function captureAndResizeScreenshot(args) {
     // regardless of which dispatch tier (fast-runner / daemon / CLI) responded.
     const argsWithPath = { ...args, path: requestedPath };
     const advisories = computeScreenshotAdvisories(args, requestedPath);
-    // GH #136 PR-A: explicit-platform raw path bypasses agent-device's --platform
-    // routing when both iOS sim and Android emu are booted. Falls through to
-    // runAgentDevice on any failure (resolution miss, command error) — graceful
-    // degradation preserves single-device behavior identically.
+    // GH #136 PR-B: when `platform:` is explicit, hard-fail instead of falling
+    // through to runAgentDevice. The original PR-A "graceful degradation" was
+    // backwards — if the caller explicitly asked for iOS or Android, silently
+    // capturing the other platform via agent-device's broken `--platform`
+    // routing defeats the entire purpose of passing the arg. Re-evidence on
+    // the user-reported regression: an OOM-unstable emulator leaves
+    // `adb devices` returning the emulator as `offline`, parseAdbDevicesEmu
+    // skips it, the fallback fires, iOS screen is returned.
     let result;
     if (args.platformExplicit && (args.platform === 'ios' || args.platform === 'android')) {
         const raw = await tryRawScreenshot(args.platform, requestedPath);
-        if (raw) {
+        if (raw.ok) {
             result = {
                 content: [{ type: 'text', text: JSON.stringify({ ok: true, data: { path: raw.path } }) }],
             };
         }
+        else {
+            const cli = args.platform === 'ios' ? 'xcrun simctl' : 'adb';
+            const hint = raw.reason === 'no-device'
+                ? `No booted ${args.platform === 'ios' ? 'iOS Simulator' : 'Android emulator'} detected by ${cli}. Boot one and retry; if your emulator is in 'offline' or 'unauthorized' state, restart it.`
+                : `Capture command failed (${cli}). The device may be transitioning state (booting, OOM, locked). Retry once it stabilizes.`;
+            return failResult(`device_screenshot platform=${args.platform} failed: ${hint}`, 'SCREENSHOT_FAILED', { platform: args.platform, reason: raw.reason });
+        }
     }
     if (!result) {
         result = await runAgentDeviceFn(buildScreenshotArgs(argsWithPath), { platform: args.platform ?? null });

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

@@ -122,7 +122,7 @@ async function androidQueryPermission(permission, appId) {
         if (stdout.includes('Unable to find package')) {
             return failResult(`Package "${appId}" not installed on device`);
         }
-        const re = new RegExp(`${androidKey.replace(/\./g, '\\.')}:.*granted=(true|false)`, 'i');
+        const re = new RegExp(`${escapeRegex(androidKey)}:.*granted=(true|false)`, 'i');
         const match = stdout.match(re);
         if (match) {
             return okResult({

scripts/cdp-bridge/dist/tools/device-screenshot-raw.js

@@ -186,7 +186,7 @@ export async function tryRawScreenshot(platform, path) {
     const capturer = platform === 'ios' ? iosCapturer : androidCapturer;
     const id = await resolver();
     if (!id)
-        return null;
+        return { ok: false, reason: 'no-device' };
     const ok = await capturer(id, path);
-    return ok ? { ok: true, path } : null;
+    return ok ? { ok: true, path } : { ok: false, reason: 'capture-failed' };
 }

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

@@ -1,5 +1,6 @@
 import type { CDPClient } from '../cdp-client.js';
 import { runAgentDevice } from '../agent-device-wrapper.js';
+import { failResult } from '../utils.js';
 import type { ToolResult } from '../utils.js';
 import { resizeWithSips, type ResizeResult, type ResizeOpts } from './device-screenshot-resize.js';
 import { tryRawScreenshot } from './device-screenshot-raw.js';
@@ -210,17 +211,31 @@ export async function captureAndResizeScreenshot(
   // regardless of which dispatch tier (fast-runner / daemon / CLI) responded.
   const argsWithPath = { ...args, path: requestedPath };
   const advisories = computeScreenshotAdvisories(args, requestedPath);
-  // GH #136 PR-A: explicit-platform raw path bypasses agent-device's --platform
-  // routing when both iOS sim and Android emu are booted. Falls through to
-  // runAgentDevice on any failure (resolution miss, command error) — graceful
-  // degradation preserves single-device behavior identically.
+  // GH #136 PR-B: when `platform:` is explicit, hard-fail instead of falling
+  // through to runAgentDevice. The original PR-A "graceful degradation" was
+  // backwards — if the caller explicitly asked for iOS or Android, silently
+  // capturing the other platform via agent-device's broken `--platform`
+  // routing defeats the entire purpose of passing the arg. Re-evidence on
+  // the user-reported regression: an OOM-unstable emulator leaves
+  // `adb devices` returning the emulator as `offline`, parseAdbDevicesEmu
+  // skips it, the fallback fires, iOS screen is returned.
   let result: ToolResult | undefined;
   if (args.platformExplicit && (args.platform === 'ios' || args.platform === 'android')) {
     const raw = await tryRawScreenshot(args.platform, requestedPath);
-    if (raw) {
+    if (raw.ok) {
       result = {
         content: [{ type: 'text' as const, text: JSON.stringify({ ok: true, data: { path: raw.path } }) }],
       };
+    } else {
+      const cli = args.platform === 'ios' ? 'xcrun simctl' : 'adb';
+      const hint = raw.reason === 'no-device'
+        ? `No booted ${args.platform === 'ios' ? 'iOS Simulator' : 'Android emulator'} detected by ${cli}. Boot one and retry; if your emulator is in 'offline' or 'unauthorized' state, restart it.`
+        : `Capture command failed (${cli}). The device may be transitioning state (booting, OOM, locked). Retry once it stabilizes.`;
+      return failResult(
+        `device_screenshot platform=${args.platform} failed: ${hint}`,
+        'SCREENSHOT_FAILED',
+        { platform: args.platform, reason: raw.reason },
+      );
     }
   }
   if (!result) {

scripts/cdp-bridge/src/tools/device-screenshot-raw.ts

@@ -201,19 +201,20 @@ export function _resetForTest(): void {
   androidCapturer = defaultAndroidCapturer;
 }
 
-export interface RawScreenshotResult {
-  ok: true;
-  path: string;
-}
+export type RawScreenshotFailureReason = 'no-device' | 'capture-failed';
+
+export type RawScreenshotResult =
+  | { ok: true; path: string }
+  | { ok: false; reason: RawScreenshotFailureReason };
 
 export async function tryRawScreenshot(
   platform: 'ios' | 'android',
   path: string,
-): Promise<RawScreenshotResult | null> {
+): Promise<RawScreenshotResult> {
   const resolver = platform === 'ios' ? iosResolver : androidResolver;
   const capturer = platform === 'ios' ? iosCapturer : androidCapturer;
   const id = await resolver();
-  if (!id) return null;
+  if (!id) return { ok: false, reason: 'no-device' };
   const ok = await capturer(id, path);
-  return ok ? { ok: true, path } : null;
+  return ok ? { ok: true, path } : { ok: false, reason: 'capture-failed' };
 }

scripts/cdp-bridge/test/unit/gh-136-screenshot-raw-platform.test.js

@@ -4,10 +4,14 @@
 // directly to disambiguate when both an iOS sim and an Android emu are booted.
 //
 // Tests cover (1) pure parsers for `xcrun simctl list -j devices booted` JSON
-// and `adb devices` stdout, (2) the `tryRawScreenshot` orchestrator branches,
+// and `adb devices` stdout, (2) the `tryRawScreenshot` orchestrator branches
+// (now returning a discriminated union `{ok:true,path}` | `{ok:false,reason}`),
 // and (3) the device-list `captureAndResizeScreenshot` plumbing — that the
-// raw path is taken iff `platformExplicit` is true, and falls through to
-// `runAgentDevice` on any failure (graceful degradation per spec).
+// raw path is taken iff `platformExplicit` is true, and **hard-fails with an
+// actionable SCREENSHOT_FAILED envelope** when raw fails (per PR-B; the
+// original PR-A graceful-fallback was the regression vector for #136).
+// Implicit-platform calls (platformExplicit=false) still route through
+// runAgentDevice — backward parity preserved.
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
 
@@ -122,7 +126,7 @@ test('tryRawScreenshot(ios): resolver returns UDID, capturer succeeds → envelo
   }
 });
 
-test('tryRawScreenshot(ios): resolver returns null → returns null (no capture attempt)', async () => {
+test('tryRawScreenshot(ios): resolver returns null → ok:false with reason no-device (no capture attempt)', async () => {
   const mod = await import(RAW_MOD);
   const { tryRawScreenshot, _setForTest, _resetForTest } = mod;
   let capturerCalled = false;
@@ -132,14 +136,14 @@ test('tryRawScreenshot(ios): resolver returns null → returns null (no capture
   });
   try {
     const result = await tryRawScreenshot('ios', '/tmp/shot.jpg');
-    assert.equal(result, null);
+    assert.deepEqual(result, { ok: false, reason: 'no-device' });
     assert.equal(capturerCalled, false);
   } finally {
     _resetForTest();
   }
 });
 
-test('tryRawScreenshot(ios): capturer fails → returns null', async () => {
+test('tryRawScreenshot(ios): capturer fails → ok:false with reason capture-failed', async () => {
   const mod = await import(RAW_MOD);
   const { tryRawScreenshot, _setForTest, _resetForTest } = mod;
   _setForTest({
@@ -148,7 +152,7 @@ test('tryRawScreenshot(ios): capturer fails → returns null', async () => {
   });
   try {
     const result = await tryRawScreenshot('ios', '/tmp/shot.jpg');
-    assert.equal(result, null);
+    assert.deepEqual(result, { ok: false, reason: 'capture-failed' });
   } finally {
     _resetForTest();
   }
@@ -171,7 +175,7 @@ test('tryRawScreenshot(android): resolver returns emu-id, capturer succeeds →
   }
 });
 
-test('tryRawScreenshot(android): capturer fails → returns null (mirrors iOS test for symmetry)', async () => {
+test('tryRawScreenshot(android): capturer fails → ok:false capture-failed (mirrors iOS for symmetry)', async () => {
   const mod = await import(RAW_MOD);
   const { tryRawScreenshot, _setForTest, _resetForTest } = mod;
   _setForTest({
@@ -180,7 +184,7 @@ test('tryRawScreenshot(android): capturer fails → returns null (mirrors iOS te
   });
   try {
     const result = await tryRawScreenshot('android', '/tmp/shot.png');
-    assert.equal(result, null);
+    assert.deepEqual(result, { ok: false, reason: 'capture-failed' });
   } finally {
     _resetForTest();
   }
@@ -254,31 +258,75 @@ test('captureAndResizeScreenshot: platformExplicit + android resolved → raw pa
   }
 });
 
-test('captureAndResizeScreenshot: platformExplicit + resolver fails → falls through to runAgentDevice', async () => {
+test('captureAndResizeScreenshot: platformExplicit + resolver miss → hard-fails (does NOT fall through to runAgentDevice)', async () => {
+  // GH #136 PR-B: the original PR-A behavior was to fall through to
+  // runAgentDevice on resolver miss. That was the regression vector — the
+  // fallback re-introduced the broken `--platform` routing. With an explicit
+  // platform, we must hard-fail with an actionable message instead.
   const raw = await import(RAW_MOD);
   const dl = await import(DEVICE_LIST_MOD);
   const { captureAndResizeScreenshot, _setRunAgentDeviceForTest, _resetRunAgentDeviceForTest } = dl;
   let runAgentDeviceCalled = false;
-  _setRunAgentDeviceForTest(async (args, opts) => {
+  _setRunAgentDeviceForTest(async () => {
     runAgentDeviceCalled = true;
-    // Mimic agent-device success envelope shape.
-    return {
-      content: [{ type: 'text', text: JSON.stringify({ ok: true, data: { path: '/tmp/fallback.jpg' } }) }],
-    };
+    return { content: [{ type: 'text', text: JSON.stringify({ ok: true, data: { path: '/tmp/wrong.jpg' } }) }] };
   });
   raw._setForTest({
-    iosResolver: async () => null, // resolver fails — should fall through
+    androidResolver: async () => null, // no booted Android emulator
+  });
+  try {
+    const result = await captureAndResizeScreenshot({
+      platform: 'android',
+      platformExplicit: true,
+      path: '/tmp/shot.png',
+      maxWidth: 0,
+    });
+    assert.equal(runAgentDeviceCalled, false, 'runAgentDevice MUST NOT be the fallback when platform is explicit');
+    assert.equal(result.isError, true);
+    const envelope = JSON.parse(result.content[0].text);
+    assert.equal(envelope.ok, false);
+    assert.equal(envelope.code, 'SCREENSHOT_FAILED');
+    assert.equal(envelope.meta.platform, 'android');
+    assert.equal(envelope.meta.reason, 'no-device');
+    // Error message names the platform and the underlying CLI so users know what to fix.
+    assert.match(envelope.error, /platform=android/);
+    assert.match(envelope.error, /adb/);
+  } finally {
+    _resetRunAgentDeviceForTest();
+    raw._resetForTest();
+  }
+});
+
+test('captureAndResizeScreenshot: platformExplicit + capture fails → hard-fails with capture-failed reason', async () => {
+  // Distinct from the resolver-miss case: device IS detected but the
+  // capture command itself failed (transient adb error, simctl crash,
+  // disk full, etc.). User-facing hint differs from "no device booted".
+  const raw = await import(RAW_MOD);
+  const dl = await import(DEVICE_LIST_MOD);
+  const { captureAndResizeScreenshot, _setRunAgentDeviceForTest, _resetRunAgentDeviceForTest } = dl;
+  let runAgentDeviceCalled = false;
+  _setRunAgentDeviceForTest(async () => {
+    runAgentDeviceCalled = true;
+    return { content: [{ type: 'text', text: JSON.stringify({ ok: true, data: { path: '/tmp/wrong.jpg' } }) }] };
+  });
+  raw._setForTest({
+    iosResolver: async () => 'UDID-IOS',
+    iosCapturer: async () => false,
   });
   try {
     const result = await captureAndResizeScreenshot({
       platform: 'ios',
       platformExplicit: true,
-      path: '/tmp/fallback.jpg',
+      path: '/tmp/shot.jpg',
       maxWidth: 0,
     });
-    assert.equal(runAgentDeviceCalled, true, 'runAgentDevice should be the fallback');
+    assert.equal(runAgentDeviceCalled, false);
+    assert.equal(result.isError, true);
     const envelope = JSON.parse(result.content[0].text);
-    assert.equal(envelope.ok, true);
+    assert.equal(envelope.ok, false);
+    assert.equal(envelope.code, 'SCREENSHOT_FAILED');
+    assert.equal(envelope.meta.reason, 'capture-failed');
+    assert.match(envelope.error, /xcrun simctl/);
   } finally {
     _resetRunAgentDeviceForTest();
     raw._resetForTest();