Skip to content

Commit 3fd9016

Browse files
committed
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 #2430
1 parent edbd96e commit 3fd9016

4 files changed

Lines changed: 178 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
## Unreleased
44

5+
- Fixed: `react-devtools` component reads (`errors`, `find`, `count`, `get`) now fail with
6+
`COMMAND_FAILED` when the DevTools daemon has zero connected apps, instead of rendering the
7+
daemon's empty tree as a result. `react-devtools errors` previously printed "No components with
8+
errors or warnings" with nothing attached, which reads as a passing check to an agent collecting
9+
evidence. Attachment is probed through `react-devtools status`: an unreachable daemon fails the
10+
read rather than starting an empty one on demand, a status without a parseable app count leaves
11+
the passthrough untouched, and `status`, `wait`, `start`, and `stop` are never gated.
512
- Added: `replay export` supports flows that switch apps and return, preserving each
613
`open <appId>` target as an explicit Maestro `launchApp.appId`.
714
- Added: `replay export` converts recorded `home` actions to Maestro `pressKey: Home`, allowing
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { afterEach, test, vi } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
4+
vi.mock('@agent-device/host-kit/command', () => ({
5+
runCmdStreaming: vi.fn(),
6+
}));
7+
8+
vi.mock('../client/client-react-devtools-companion.ts', () => ({
9+
ensureReactDevtoolsCompanion: vi.fn(),
10+
stopReactDevtoolsCompanion: vi.fn(),
11+
}));
12+
13+
import { runCmdStreaming } from '@agent-device/host-kit/command';
14+
import { AppError } from '@agent-device/kernel/errors';
15+
import { runReactDevtoolsCommand } from '../cli/commands/react-devtools.ts';
16+
17+
afterEach(() => {
18+
vi.clearAllMocks();
19+
});
20+
21+
function mockStatusOutput(connectedApps: number): void {
22+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({
23+
exitCode: 0,
24+
stdout: `Daemon: running (port 8097)\nApps: ${connectedApps} connected, 0 components\nUptime: 12s\n`,
25+
stderr: '',
26+
});
27+
}
28+
29+
async function captureError(args: string[]): Promise<unknown> {
30+
try {
31+
await runReactDevtoolsCommand(args, { cwd: '/tmp/project' });
32+
return null;
33+
} catch (error) {
34+
return error;
35+
}
36+
}
37+
38+
function passthroughArgs(callIndex: number): string[] {
39+
const args = vi.mocked(runCmdStreaming).mock.calls[callIndex]?.[1] ?? [];
40+
return args.slice(args.indexOf('agent-react-devtools') + 1);
41+
}
42+
43+
test('react-devtools errors fails instead of reporting a clean pass with no app attached', async () => {
44+
mockStatusOutput(0);
45+
46+
const error = await captureError(['errors']);
47+
48+
assert.ok(error instanceof AppError);
49+
assert.equal(error.code, 'COMMAND_FAILED');
50+
assert.equal(error.details?.connectedApps, 0);
51+
assert.equal(error.details?.subcommand, 'errors');
52+
assert.match(error.message, /0 apps connected/);
53+
assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1);
54+
assert.deepEqual(passthroughArgs(0), ['status']);
55+
});
56+
57+
for (const args of [['find', 'Button'], ['count'], ['get', 'tree']]) {
58+
test(`react-devtools ${args.join(' ')} fails with no app attached`, async () => {
59+
mockStatusOutput(0);
60+
61+
const error = await captureError(args);
62+
63+
assert.ok(error instanceof AppError);
64+
assert.equal(error.details?.subcommand, args[0]);
65+
});
66+
}
67+
68+
test('react-devtools errors passes through once an app is attached', async () => {
69+
mockStatusOutput(1);
70+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' });
71+
72+
const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' });
73+
74+
assert.equal(exitCode, 0);
75+
assert.deepEqual(passthroughArgs(1), ['errors']);
76+
});
77+
78+
test('react-devtools errors fails instead of starting an empty daemon to read', async () => {
79+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({
80+
exitCode: 1,
81+
stdout: 'Daemon is not running\n',
82+
stderr: '',
83+
});
84+
85+
const error = await captureError(['errors']);
86+
87+
assert.ok(error instanceof AppError);
88+
assert.match(error.message, /daemon is not running/);
89+
assert.equal(error.details?.connectedApps, null);
90+
assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1);
91+
});
92+
93+
test('react-devtools errors defers to the passthrough when status reports no app count', async () => {
94+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({
95+
exitCode: 0,
96+
stdout: 'Daemon: running (port 8097)\n',
97+
stderr: '',
98+
});
99+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' });
100+
101+
const exitCode = await runReactDevtoolsCommand(['errors'], { cwd: '/tmp/project' });
102+
103+
assert.equal(exitCode, 0);
104+
assert.deepEqual(passthroughArgs(1), ['errors']);
105+
});
106+
107+
for (const args of [['status'], ['wait', '--connected'], ['start'], ['stop']]) {
108+
test(`react-devtools ${args.join(' ')} runs without an attachment probe`, async () => {
109+
vi.mocked(runCmdStreaming).mockResolvedValueOnce({ exitCode: 0, stdout: '', stderr: '' });
110+
111+
await runReactDevtoolsCommand(args, { cwd: '/tmp/project' });
112+
113+
assert.equal(vi.mocked(runCmdStreaming).mock.calls.length, 1);
114+
assert.deepEqual(passthroughArgs(0), args);
115+
});
116+
}

src/cli/commands/react-devtools.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,59 @@ export function buildReactDevtoolsNpmExecArgs(args: string[]): string[] {
5555
];
5656
}
5757

58+
/**
59+
* Subcommands that answer a question about an attached app's React tree. The
60+
* passthrough starts a daemon on demand and answers them from its empty tree,
61+
* so `errors` reports the same "nothing found" as a healthy app with nothing
62+
* wrong. Gating them on attachment keeps a failed observation from reading as
63+
* a negative one.
64+
*/
65+
const COMPONENT_READ_COMMANDS = new Set(['errors', 'find', 'count', 'get']);
66+
67+
// The pinned passthrough has no machine-readable status, so the connected-app
68+
// count is read off its `status` rendering. A status the probe cannot parse
69+
// means unknown and lets the read through; a status it cannot obtain means no
70+
// daemon is reachable, which no component read can observe around.
71+
const CONNECTED_APPS_PATTERN = /^Apps: (\d+) connected/m;
72+
73+
type Attachment = number | 'no-daemon' | 'unknown';
74+
75+
async function readAttachment(cwd: string, env: NodeJS.ProcessEnv): Promise<Attachment> {
76+
const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(['status']), {
77+
cwd,
78+
env,
79+
allowFailure: true,
80+
});
81+
if (result.exitCode !== 0) return 'no-daemon';
82+
const match = CONNECTED_APPS_PATTERN.exec(result.stdout);
83+
return match ? Number(match[1]) : 'unknown';
84+
}
85+
86+
async function assertComponentReadCanObserve(
87+
args: string[],
88+
cwd: string,
89+
env: NodeJS.ProcessEnv,
90+
): Promise<void> {
91+
const subcommand = args[0] ?? '';
92+
if (!COMPONENT_READ_COMMANDS.has(subcommand)) return;
93+
const attachment = await readAttachment(cwd, env);
94+
if (attachment === 'unknown') return;
95+
if (typeof attachment === 'number' && attachment > 0) return;
96+
throw new AppError(
97+
'COMMAND_FAILED',
98+
`react-devtools ${subcommand} observed nothing: ${
99+
attachment === 'no-daemon'
100+
? 'the React DevTools daemon is not running'
101+
: 'the React DevTools daemon has 0 apps connected'
102+
}.`,
103+
{
104+
subcommand,
105+
connectedApps: attachment === 'no-daemon' ? null : attachment,
106+
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.',
107+
},
108+
);
109+
}
110+
58111
function isRemoteIosBridgeBackend(leaseBackend: CliFlags['leaseBackend']): boolean {
59112
return leaseBackend === 'ios-instance';
60113
}
@@ -186,6 +239,7 @@ export async function runReactDevtoolsCommand(
186239
if (shouldConfigureDirectReverse(args, options)) {
187240
await options.configureDirectPortReverse?.();
188241
}
242+
await assertComponentReadCanObserve(args, cwd, env);
189243
const result = await runCmdStreaming('npm', buildReactDevtoolsNpmExecArgs(args), {
190244
cwd,
191245
env,

website/docs/docs/commands.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,7 @@ agent-device react-devtools profile report @c5
853853
854854
- `react-devtools` dynamically runs pinned `agent-react-devtools@0.4.0` through npm and passes arguments through 1:1.
855855
- The first run may download the pinned package from npm; later runs can reuse the npm cache.
856+
- 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.
856857
- `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.
857858
- Use it when a React Native workflow needs component hierarchy, props, state, hooks, render causes, slow components, or re-render counts.
858859
- 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`.

0 commit comments

Comments
 (0)