From 580d50d82c2515419e471528e621d885860e783f Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Wed, 9 Sep 2026 18:09:11 -0400 Subject: [PATCH 1/2] fix(cli): fail react-devtools component reads that cannot observe an app `react-devtools errors` printed "No components with errors or warnings" with nothing attached, so a check that was never performed rendered identically to a check that passed. The passthrough starts a daemon on demand and answers component reads from its empty tree, which makes the vacuous pass reachable even with no daemon running. Gate `errors`, `find`, `count`, and `get` on attachment, probed through the passthrough's own `status`. An unreachable daemon or a parsed zero connected apps fails the read with COMMAND_FAILED and the connected-app count in details; a status without a parseable count leaves the passthrough untouched. `status`, `wait`, `start`, and `stop` are never gated. Fixes callstack/agent-device#2430 --- CHANGELOG.md | 7 ++ .../cli-react-devtools-attachment.test.ts | 116 ++++++++++++++++++ src/cli/commands/react-devtools.ts | 54 ++++++++ website/docs/docs/commands.md | 1 + 4 files changed, 178 insertions(+) create mode 100644 src/__tests__/cli-react-devtools-attachment.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 77cf6cc336..2e890e4f59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ disclosing that through `truncated`/`effectiveDepth` as it does unscoped. - Fixed: repeated unfiltered Android snapshots stay compact when identical element bounds arrive with a different property order. Changes to the bounds still re-emit the tree. +- Fixed: `react-devtools` component reads (`errors`, `find`, `count`, `get`) now fail with + `COMMAND_FAILED` when the DevTools daemon has zero connected apps, instead of rendering the + daemon's empty tree as a result. `react-devtools errors` previously printed "No components with + errors or warnings" with nothing attached, which reads as a passing check to an agent collecting + evidence. Attachment is probed through `react-devtools status`: an unreachable daemon fails the + read rather than starting an empty one on demand, a status without a parseable app count leaves + the passthrough untouched, and `status`, `wait`, `start`, and `stop` are never gated. - Added: `replay export` supports flows that switch apps and return, preserving each `open ` target as an explicit Maestro `launchApp.appId`. - Added: `replay export` converts recorded `home` actions to Maestro `pressKey: Home`, allowing diff --git a/src/__tests__/cli-react-devtools-attachment.test.ts b/src/__tests__/cli-react-devtools-attachment.test.ts new file mode 100644 index 0000000000..dfcb1f1b57 --- /dev/null +++ b/src/__tests__/cli-react-devtools-attachment.test.ts @@ -0,0 +1,116 @@ +import { afterEach, test, vi } from 'vitest'; +import assert from 'node:assert/strict'; + +vi.mock('@agent-device/host-kit/command', () => ({ + runCmdStreaming: vi.fn(), +})); + +vi.mock('../client/client-react-devtools-companion.ts', () => ({ + ensureReactDevtoolsCompanion: vi.fn(), + stopReactDevtoolsCompanion: vi.fn(), +})); + +import { runCmdStreaming } from '@agent-device/host-kit/command'; +import { AppError } from '@agent-device/kernel/errors'; +import { runReactDevtoolsCommand } from '../cli/commands/react-devtools.ts'; + +afterEach(() => { + vi.clearAllMocks(); +}); + +function mockStatusOutput(connectedApps: number): void { + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ + exitCode: 0, + stdout: `Daemon: running (port 8097)\nApps: ${connectedApps} connected, 0 components\nUptime: 12s\n`, + stderr: '', + }); +} + +async function captureError(args: string[]): Promise { + try { + await runReactDevtoolsCommand(args, { cwd: '/tmp/project' }); + return null; + } catch (error) { + return error; + } +} + +function passthroughArgs(callIndex: number): string[] { + const args = vi.mocked(runCmdStreaming).mock.calls[callIndex]?.[1] ?? []; + return args.slice(args.indexOf('agent-react-devtools') + 1); +} + +test('react-devtools errors fails instead of reporting a clean pass with no app attached', async () => { + mockStatusOutput(0); + + const error = await captureError(['errors']); + + assert.ok(error instanceof AppError); + assert.equal(error.code, 'COMMAND_FAILED'); + assert.equal(error.details?.connectedApps, 0); + assert.equal(error.details?.subcommand, 'errors'); + assert.match(error.message, /0 apps connected/); + assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); + assert.deepEqual(passthroughArgs(0), ['status']); +}); + +for (const args of [['find', 'Button'], ['count'], ['get', 'tree']]) { + test(`react-devtools ${args.join(' ')} fails with no app attached`, async () => { + mockStatusOutput(0); + + const error = await captureError(args); + + assert.ok(error instanceof AppError); + assert.equal(error.details?.subcommand, args[0]); + }); +} + +test('react-devtools errors passes through once an app is attached', async () => { + mockStatusOutput(1); + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); + + const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' }); + + assert.equal(exitCode, 0); + assert.deepEqual(passthroughArgs(1), ['errors']); +}); + +test('react-devtools errors fails instead of starting an empty daemon to read', async () => { + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ + exitCode: 1, + stdout: 'Daemon is not running\n', + stderr: '', + }); + + const error = await captureError(['errors']); + + assert.ok(error instanceof AppError); + assert.match(error.message, /daemon is not running/); + assert.equal(error.details?.connectedApps, null); + assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); +}); + +test('react-devtools errors defers to the passthrough when status reports no app count', async () => { + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ + exitCode: 0, + stdout: 'Daemon: running (port 8097)\n', + stderr: '', + }); + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); + + const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' }); + + assert.equal(exitCode, 0); + assert.deepEqual(passthroughArgs(1), ['errors']); +}); + +for (const args of [['status'], ['wait', '--connected'], ['start'], ['stop']]) { + test(`react-devtools ${args.join(' ')} runs without an attachment probe`, async () => { + vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); + + await runReactDevtoolsCommand(args, { cwd: '/tmp/project' }); + + assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); + assert.deepEqual(passthroughArgs(0), args); + }); +} diff --git a/src/cli/commands/react-devtools.ts b/src/cli/commands/react-devtools.ts index 149ebd1be3..8c72401d9d 100644 --- a/src/cli/commands/react-devtools.ts +++ b/src/cli/commands/react-devtools.ts @@ -55,6 +55,59 @@ export function buildReactDevtoolsNpmExecArgs(args: string[]): string[] { ]; } +/** + * Subcommands that answer a question about an attached app's React tree. The + * passthrough starts a daemon on demand and answers them from its empty tree, + * so `errors` reports the same "nothing found" as a healthy app with nothing + * wrong. Gating them on attachment keeps a failed observation from reading as + * a negative one. + */ +const COMPONENT_READ_COMMANDS = new Set(['errors', 'find', 'count', 'get']); + +// The pinned passthrough has no machine-readable status, so the connected-app +// count is read off its `status` rendering. A status the probe cannot parse +// means unknown and lets the read through; a status it cannot obtain means no +// daemon is reachable, which no component read can observe around. +const CONNECTED_APPS_PATTERN = /^Apps: (\d+) connected/m; + +type Attachment = number | 'no-daemon' | 'unknown'; + +async function readAttachment(cwd: string, env: NodeJS.ProcessEnv): Promise { + const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(['status']), { + cwd, + env, + allowFailure: true, + }); + if (result.exitCode !== 0) return 'no-daemon'; + const match = CONNECTED_APPS_PATTERN.exec(result.stdout); + return match ? Number(match[1]) : 'unknown'; +} + +async function assertComponentReadCanObserve( + args: string[], + cwd: string, + env: NodeJS.ProcessEnv, +): Promise { + const subcommand = args[0] ?? ''; + if (!COMPONENT_READ_COMMANDS.has(subcommand)) return; + const attachment = await readAttachment(cwd, env); + if (attachment === 'unknown') return; + if (typeof attachment === 'number' && attachment > 0) return; + throw new AppError( + 'COMMAND_FAILED', + `react-devtools ${subcommand} observed nothing: ${ + attachment === 'no-daemon' + ? 'the React DevTools daemon is not running' + : 'the React DevTools daemon has 0 apps connected' + }.`, + { + subcommand, + connectedApps: attachment === 'no-daemon' ? null : attachment, + hint: 'Attach an app first: `agent-device react-devtools wait --connected` blocks until one connects or reconnects. If none ever attaches, run `agent-device react-devtools start` and launch or relaunch the app.', + }, + ); +} + function isRemoteIosBridgeBackend(leaseBackend: CliFlags['leaseBackend']): boolean { return leaseBackend === 'ios-instance'; } @@ -186,6 +239,7 @@ export async function runReactDevtoolsCommand( if (shouldConfigureDirectReverse(args, options)) { await options.configureDirectPortReverse?.(); } + await assertComponentReadCanObserve(args, cwd, env); const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(args), { cwd, env, diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index cb8ac9899f..29db9ddcff 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -852,6 +852,7 @@ agent-device react-devtools profile report @c5 - `react-devtools` dynamically runs pinned `agent-react-devtools@0.4.0` through npm and passes arguments through 1:1. - The first run may download the pinned package from npm; later runs can reuse the npm cache. +- Component reads (`errors`, `find`, `count`, `get`) fail with `COMMAND_FAILED` when the DevTools daemon is not running or reports zero connected apps, so an unobservable tree cannot be mistaken for an empty one. Use `react-devtools start` and `react-devtools wait --connected` to establish attachment first. - `agent-device` global flags work before or after `react-devtools`. Use `--` before downstream flags only when they intentionally share an `agent-device` global flag name. - Use it when a React Native workflow needs component hierarchy, props, state, hooks, render causes, slow components, or re-render counts. - For profiling, keep the window narrow and make one bounded first-pass survey: use the `profile stop` summary, run `profile slow --limit 5` and `profile rerenders --limit 5` once, add `profile timeline --limit 20` only when commit timing matters, then drill into a specific `@c` ref with `profile report`. From 9f728f3ee36f384f14adbb89b78dd4afeca677bc Mon Sep 17 00:00:00 2001 From: Brad Anderson Date: Thu, 10 Sep 2026 10:32:56 -0400 Subject: [PATCH 2/2] revert(cli): drop the react-devtools status probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe cannot guarantee the read that follows it observed an app, and its unparseable-status branch let the vacuous result through — fail-open by default in the one place a false clean is the bug. Attachment is checked in the operation that reads the component tree instead, upstream in callstackincubator/agent-react-devtools#60, which this repo picks up through the existing 1:1 passthrough once the pin bumps. --- CHANGELOG.md | 7 -- .../cli-react-devtools-attachment.test.ts | 116 ------------------ src/cli/commands/react-devtools.ts | 54 -------- website/docs/docs/commands.md | 1 - 4 files changed, 178 deletions(-) delete mode 100644 src/__tests__/cli-react-devtools-attachment.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e890e4f59..77cf6cc336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,13 +13,6 @@ disclosing that through `truncated`/`effectiveDepth` as it does unscoped. - Fixed: repeated unfiltered Android snapshots stay compact when identical element bounds arrive with a different property order. Changes to the bounds still re-emit the tree. -- Fixed: `react-devtools` component reads (`errors`, `find`, `count`, `get`) now fail with - `COMMAND_FAILED` when the DevTools daemon has zero connected apps, instead of rendering the - daemon's empty tree as a result. `react-devtools errors` previously printed "No components with - errors or warnings" with nothing attached, which reads as a passing check to an agent collecting - evidence. Attachment is probed through `react-devtools status`: an unreachable daemon fails the - read rather than starting an empty one on demand, a status without a parseable app count leaves - the passthrough untouched, and `status`, `wait`, `start`, and `stop` are never gated. - Added: `replay export` supports flows that switch apps and return, preserving each `open ` target as an explicit Maestro `launchApp.appId`. - Added: `replay export` converts recorded `home` actions to Maestro `pressKey: Home`, allowing diff --git a/src/__tests__/cli-react-devtools-attachment.test.ts b/src/__tests__/cli-react-devtools-attachment.test.ts deleted file mode 100644 index dfcb1f1b57..0000000000 --- a/src/__tests__/cli-react-devtools-attachment.test.ts +++ /dev/null @@ -1,116 +0,0 @@ -import { afterEach, test, vi } from 'vitest'; -import assert from 'node:assert/strict'; - -vi.mock('@agent-device/host-kit/command', () => ({ - runCmdStreaming: vi.fn(), -})); - -vi.mock('../client/client-react-devtools-companion.ts', () => ({ - ensureReactDevtoolsCompanion: vi.fn(), - stopReactDevtoolsCompanion: vi.fn(), -})); - -import { runCmdStreaming } from '@agent-device/host-kit/command'; -import { AppError } from '@agent-device/kernel/errors'; -import { runReactDevtoolsCommand } from '../cli/commands/react-devtools.ts'; - -afterEach(() => { - vi.clearAllMocks(); -}); - -function mockStatusOutput(connectedApps: number): void { - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ - exitCode: 0, - stdout: `Daemon: running (port 8097)\nApps: ${connectedApps} connected, 0 components\nUptime: 12s\n`, - stderr: '', - }); -} - -async function captureError(args: string[]): Promise { - try { - await runReactDevtoolsCommand(args, { cwd: '/tmp/project' }); - return null; - } catch (error) { - return error; - } -} - -function passthroughArgs(callIndex: number): string[] { - const args = vi.mocked(runCmdStreaming).mock.calls[callIndex]?.[1] ?? []; - return args.slice(args.indexOf('agent-react-devtools') + 1); -} - -test('react-devtools errors fails instead of reporting a clean pass with no app attached', async () => { - mockStatusOutput(0); - - const error = await captureError(['errors']); - - assert.ok(error instanceof AppError); - assert.equal(error.code, 'COMMAND_FAILED'); - assert.equal(error.details?.connectedApps, 0); - assert.equal(error.details?.subcommand, 'errors'); - assert.match(error.message, /0 apps connected/); - assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); - assert.deepEqual(passthroughArgs(0), ['status']); -}); - -for (const args of [['find', 'Button'], ['count'], ['get', 'tree']]) { - test(`react-devtools ${args.join(' ')} fails with no app attached`, async () => { - mockStatusOutput(0); - - const error = await captureError(args); - - assert.ok(error instanceof AppError); - assert.equal(error.details?.subcommand, args[0]); - }); -} - -test('react-devtools errors passes through once an app is attached', async () => { - mockStatusOutput(1); - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); - - const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' }); - - assert.equal(exitCode, 0); - assert.deepEqual(passthroughArgs(1), ['errors']); -}); - -test('react-devtools errors fails instead of starting an empty daemon to read', async () => { - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ - exitCode: 1, - stdout: 'Daemon is not running\n', - stderr: '', - }); - - const error = await captureError(['errors']); - - assert.ok(error instanceof AppError); - assert.match(error.message, /daemon is not running/); - assert.equal(error.details?.connectedApps, null); - assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); -}); - -test('react-devtools errors defers to the passthrough when status reports no app count', async () => { - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ - exitCode: 0, - stdout: 'Daemon: running (port 8097)\n', - stderr: '', - }); - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); - - const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' }); - - assert.equal(exitCode, 0); - assert.deepEqual(passthroughArgs(1), ['errors']); -}); - -for (const args of [['status'], ['wait', '--connected'], ['start'], ['stop']]) { - test(`react-devtools ${args.join(' ')} runs without an attachment probe`, async () => { - vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' }); - - await runReactDevtoolsCommand(args, { cwd: '/tmp/project' }); - - assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1); - assert.deepEqual(passthroughArgs(0), args); - }); -} diff --git a/src/cli/commands/react-devtools.ts b/src/cli/commands/react-devtools.ts index 8c72401d9d..149ebd1be3 100644 --- a/src/cli/commands/react-devtools.ts +++ b/src/cli/commands/react-devtools.ts @@ -55,59 +55,6 @@ export function buildReactDevtoolsNpmExecArgs(args: string[]): string[] { ]; } -/** - * Subcommands that answer a question about an attached app's React tree. The - * passthrough starts a daemon on demand and answers them from its empty tree, - * so `errors` reports the same "nothing found" as a healthy app with nothing - * wrong. Gating them on attachment keeps a failed observation from reading as - * a negative one. - */ -const COMPONENT_READ_COMMANDS = new Set(['errors', 'find', 'count', 'get']); - -// The pinned passthrough has no machine-readable status, so the connected-app -// count is read off its `status` rendering. A status the probe cannot parse -// means unknown and lets the read through; a status it cannot obtain means no -// daemon is reachable, which no component read can observe around. -const CONNECTED_APPS_PATTERN = /^Apps: (\d+) connected/m; - -type Attachment = number | 'no-daemon' | 'unknown'; - -async function readAttachment(cwd: string, env: NodeJS.ProcessEnv): Promise { - const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(['status']), { - cwd, - env, - allowFailure: true, - }); - if (result.exitCode !== 0) return 'no-daemon'; - const match = CONNECTED_APPS_PATTERN.exec(result.stdout); - return match ? Number(match[1]) : 'unknown'; -} - -async function assertComponentReadCanObserve( - args: string[], - cwd: string, - env: NodeJS.ProcessEnv, -): Promise { - const subcommand = args[0] ?? ''; - if (!COMPONENT_READ_COMMANDS.has(subcommand)) return; - const attachment = await readAttachment(cwd, env); - if (attachment === 'unknown') return; - if (typeof attachment === 'number' && attachment > 0) return; - throw new AppError( - 'COMMAND_FAILED', - `react-devtools ${subcommand} observed nothing: ${ - attachment === 'no-daemon' - ? 'the React DevTools daemon is not running' - : 'the React DevTools daemon has 0 apps connected' - }.`, - { - subcommand, - connectedApps: attachment === 'no-daemon' ? null : attachment, - hint: 'Attach an app first: `agent-device react-devtools wait --connected` blocks until one connects or reconnects. If none ever attaches, run `agent-device react-devtools start` and launch or relaunch the app.', - }, - ); -} - function isRemoteIosBridgeBackend(leaseBackend: CliFlags['leaseBackend']): boolean { return leaseBackend === 'ios-instance'; } @@ -239,7 +186,6 @@ export async function runReactDevtoolsCommand( if (shouldConfigureDirectReverse(args, options)) { await options.configureDirectPortReverse?.(); } - await assertComponentReadCanObserve(args, cwd, env); const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(args), { cwd, env, diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 29db9ddcff..cb8ac9899f 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -852,7 +852,6 @@ agent-device react-devtools profile report @c5 - `react-devtools` dynamically runs pinned `agent-react-devtools@0.4.0` through npm and passes arguments through 1:1. - The first run may download the pinned package from npm; later runs can reuse the npm cache. -- Component reads (`errors`, `find`, `count`, `get`) fail with `COMMAND_FAILED` when the DevTools daemon is not running or reports zero connected apps, so an unobservable tree cannot be mistaken for an empty one. Use `react-devtools start` and `react-devtools wait --connected` to establish attachment first. - `agent-device` global flags work before or after `react-devtools`. Use `--` before downstream flags only when they intentionally share an `agent-device` global flag name. - Use it when a React Native workflow needs component hierarchy, props, state, hooks, render causes, slow components, or re-render counts. - For profiling, keep the window narrow and make one bounded first-pass survey: use the `profile stop` summary, run `profile slow --limit 5` and `profile rerenders --limit 5` once, add `profile timeline --limit 20` only when commit timing matters, then drill into a specific `@c` ref with `profile report`.