Skip to content

Commit 9d2eb9b

Browse files
committed
fix: fail tree observation without an attached app and replace re-flushed roots
Observation commands (get tree, get component, find, count, errors, profile start) returned an empty result with exit 0 when no React app was attached, so "No components with errors or warnings" was indistinguishable from a check that never ran. They now fail with a typed `no-app-attached` reason and a message that says whether an app ever connected. When another React DevTools backend attaches to the same app (React Native DevTools opening, another agent), react-devtools-core creates a fresh renderer interface with a new fiber-ID space and flushes the whole tree through the hook-wide operations channel every agent subscribes to. Later commits reach only that new interface, so the daemon's existing root is frozen, not merely duplicated. The bridge now asks the tree to reconcile a second root on the same connection: a root that structurally duplicates an older root of the same renderer replaces it; a genuinely different root is kept. Reported through callstack/agent-device#2430.
1 parent f22798b commit 9d2eb9b

13 files changed

Lines changed: 275 additions & 28 deletions

File tree

.changeset/brave-owls-observe.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'agent-react-devtools': minor
3+
---
4+
5+
Tree observation commands (`get tree`, `get component`, `find`, `count`,
6+
`errors`, `profile start`) now fail with a typed `no-app-attached` reason and a
7+
non-zero exit code when no React app is attached, instead of reporting an empty
8+
result that reads as a clean pass. When another React DevTools backend attaches
9+
to the same app (React Native DevTools opening, another agent) and re-flushes the
10+
tree under a fresh fiber-ID space, the daemon now replaces its frozen copy
11+
instead of counting every component twice.

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ Components with errors or warnings are annotated in tree and search output:
152152
@c5 [fn] Form ⚠2 ✗1
153153
```
154154

155-
Use the `errors` command to list only components with issues:
155+
Use the `errors` command to list only components with issues. When no app is attached, it exits 1 with `No React app is attached` rather than reporting a clean tree:
156156

157157
```sh
158158
agent-react-devtools errors

packages/agent-react-devtools/skills/react-devtools/SKILL.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,8 @@ agent-react-devtools status # Should show 1 connected app
167167
## Important Rules
168168

169169
- **Labels reset** when the app reloads or components unmount/remount. After a reload, use `wait --connected` then re-check with `get tree` or `find`.
170-
- **`status` first** — if status shows 0 connected apps, the React app is not connected. The user may need to run `npx agent-react-devtools init` in their project first.
170+
- **`status` first** — if status shows 0 connected apps, the React app is not connected. The user may need to run `npx agent-react-devtools init` in their project first. Tree observation commands exit 1 with `No React app is attached` in that state; treat that as "nothing was observed", not as a clean result.
171+
- **Another DevTools attaching reassigns IDs** — when React Native DevTools (or another agent) attaches to the same app, React re-flushes the tree under new IDs and the daemon replaces its copy. Re-run `get tree` or `find` before reusing earlier `@cN` labels.
171172
- **Headed browser required** — if using `agent-browser`, always use `--headed` mode. Headless Chromium does not properly load the devtools connect script.
172173
- **Profile while interacting** — profiling only captures renders that happen between `profile start` and `profile stop`. Make sure the relevant interaction happens during that window.
173174
- **Use `--depth`** on large trees — a deep tree can produce a lot of output. Start with `--depth 3` or `--depth 4` and go deeper only on the subtree you care about.

packages/agent-react-devtools/skills/react-devtools/references/commands.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ Output example:
6565

6666
`⚠N` = N warnings, `✗N` = N errors. Returns "No components with errors or warnings" when everything is clean.
6767

68+
When no React app is attached, this and every other tree observation command (`get tree`, `get component`, `find`, `count`, `profile start`) exits 1 with `No React app is attached to the DevTools daemon ...` instead of an empty result, so a missing app can never read as a clean pass.
69+
6870
Error/warning annotations also appear in `get tree`, `get component`, and `find` output when counts are non-zero.
6971

7072
## Profiling

packages/agent-react-devtools/src/__tests__/component-tree.test.ts

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,3 +492,68 @@ describe('ComponentTree', () => {
492492
});
493493
});
494494
});
495+
496+
describe('ComponentTree.reconcileReflushedRoot', () => {
497+
const ROOT_OP = (id: number) => [1, id, 11, 0, 1, 0, 0];
498+
499+
function fullTree(rendererId: number, rootId: number, base: number): number[] {
500+
return buildOps(rendererId, rootId, ['App', 'Header', 'Item'], (strId) => [
501+
...ROOT_OP(rootId),
502+
...addOp(base + 1, 5, rootId, strId('App')),
503+
...addOp(base + 2, 8, base + 1, strId('Header')),
504+
...addOp(base + 3, 5, base + 1, strId('Item')),
505+
...addOp(base + 4, 5, base + 1, strId('Item')),
506+
]);
507+
}
508+
509+
it('drops the older root when a new root of the same renderer duplicates it', () => {
510+
const tree = new ComponentTree();
511+
tree.applyOperations(fullTree(1, 100, 0));
512+
expect(tree.getComponentCount()).toBe(5);
513+
514+
// Another backend attached: same fibers re-flushed under a fresh ID space
515+
tree.applyOperations(fullTree(1, 500, 1000));
516+
expect(tree.getComponentCount()).toBe(10);
517+
518+
expect(tree.reconcileReflushedRoot(500)).toBe(100);
519+
expect(tree.getComponentCount()).toBe(5);
520+
expect(tree.getRootIds()).toEqual([500]);
521+
expect(tree.getNode(1)).toBeUndefined();
522+
expect(tree.getNode(1001)?.displayName).toBe('App');
523+
expect(tree.findByName('Item', true)).toHaveLength(2);
524+
});
525+
526+
it('keeps a genuine second root whose structure differs', () => {
527+
const tree = new ComponentTree();
528+
tree.applyOperations(fullTree(1, 100, 0));
529+
tree.applyOperations(
530+
buildOps(1, 500, ['Sidebar'], (strId) => [
531+
...ROOT_OP(500),
532+
...addOp(1001, 5, 500, strId('Sidebar')),
533+
]),
534+
);
535+
536+
expect(tree.reconcileReflushedRoot(500)).toBeNull();
537+
expect(tree.getRootIds()).toEqual([100, 500]);
538+
expect(tree.getComponentCount()).toBe(7);
539+
});
540+
541+
it('never matches roots across renderers', () => {
542+
const tree = new ComponentTree();
543+
tree.applyOperations(fullTree(1, 100, 0));
544+
tree.applyOperations(fullTree(2, 500, 1000));
545+
546+
expect(tree.reconcileReflushedRoot(500)).toBeNull();
547+
expect(tree.getComponentCount()).toBe(10);
548+
});
549+
550+
it('only considers roots added before the reflushed one', () => {
551+
const tree = new ComponentTree();
552+
tree.applyOperations(fullTree(1, 100, 0));
553+
tree.applyOperations(fullTree(1, 500, 1000));
554+
555+
// Asking about the older root must not delete the newer, live one
556+
expect(tree.reconcileReflushedRoot(100)).toBeNull();
557+
expect(tree.getRootIds()).toEqual([100, 500]);
558+
});
559+
});

packages/agent-react-devtools/src/component-tree.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -547,6 +547,41 @@ export class ComponentTree {
547547
this.removeNode(rootId);
548548
}
549549

550+
/**
551+
* A second React DevTools backend attaching to the same app (React Native
552+
* DevTools opening, another agent) re-flushes the whole tree through the
553+
* shared hook under a fresh fiber-ID space, and later commits reach only that
554+
* new root. The copy this tree already holds is therefore frozen, not merely
555+
* duplicated. When `rootId` structurally duplicates an older root of the same
556+
* renderer, drop the older root and return its id.
557+
*/
558+
reconcileReflushedRoot(rootId: number): number | null {
559+
const root = this.nodes.get(rootId);
560+
if (!root) return null;
561+
for (const olderId of this.roots) {
562+
if (olderId === rootId) break;
563+
const older = this.nodes.get(olderId);
564+
if (!older || older.rendererId !== root.rendererId) continue;
565+
if (this.subtreesMatch(olderId, rootId)) {
566+
this.removeNode(olderId);
567+
return olderId;
568+
}
569+
}
570+
return null;
571+
}
572+
573+
private subtreesMatch(a: number, b: number): boolean {
574+
const x = this.nodes.get(a);
575+
const y = this.nodes.get(b);
576+
if (!x || !y) return false;
577+
if (x.type !== y.type || x.displayName !== y.displayName || x.key !== y.key) return false;
578+
if (x.children.length !== y.children.length) return false;
579+
for (let i = 0; i < x.children.length; i++) {
580+
if (!this.subtreesMatch(x.children[i], y.children[i])) return false;
581+
}
582+
return true;
583+
}
584+
550585
/**
551586
* Look up the @cN label for a given component ID.
552587
* Returns undefined if the ID has no label assigned.

packages/agent-react-devtools/src/daemon.ts

Lines changed: 39 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,30 @@ class Daemon {
139139
});
140140
}
141141

142+
/**
143+
* Commands that observe the live component tree must fail when no app is
144+
* attached. An empty tree is not evidence of anything: reporting it as a
145+
* result lets "no components with errors" stand in for "nothing was observed".
146+
*/
147+
private requireAttachedApp(): IpcResponse | null {
148+
const health = this.bridge.getConnectionHealth();
149+
if (health.connectedApps > 0) return null;
150+
151+
const state = health.lastDisconnectAt !== null
152+
? `app disconnected ${Math.round((Date.now() - health.lastDisconnectAt) / 1000)}s ago, waiting for reconnect`
153+
: health.hasEverConnected
154+
? 'app disconnected'
155+
: 'no app has connected since the daemon started';
156+
return {
157+
ok: false,
158+
reason: 'no-app-attached',
159+
error:
160+
`No React app is attached to the DevTools daemon on port ${this.port} (${state}). ` +
161+
'Start the app in development mode, then run `agent-react-devtools wait --connected`. ' +
162+
'React Native 0.87+ apps also need `agent-react-devtools init`.',
163+
};
164+
}
165+
142166
private async handleCommand(cmd: IpcCommand, conn: net.Socket): Promise<IpcResponse> {
143167
try {
144168
switch (cmd.type) {
@@ -160,6 +184,8 @@ class Daemon {
160184
};
161185

162186
case 'get-tree': {
187+
const detached = this.requireAttachedApp();
188+
if (detached) return detached;
163189
let resolvedRoot: number | undefined;
164190
if (cmd.root !== undefined) {
165191
resolvedRoot = this.tree.resolveId(cmd.root);
@@ -178,21 +204,12 @@ class Daemon {
178204
if (resolvedRoot !== undefined && treeData.length === 0) {
179205
return { ok: false, error: `Component ${cmd.root} not found` };
180206
}
181-
const response: IpcResponse = {
182-
ok: true,
183-
data: { nodes: treeData, totalCount },
184-
};
185-
if (treeData.length === 0) {
186-
const health = this.bridge.getConnectionHealth();
187-
if (health.hasEverConnected && health.connectedApps === 0 && health.lastDisconnectAt !== null) {
188-
const ago = Math.round((Date.now() - health.lastDisconnectAt) / 1000);
189-
response.hint = `app disconnected ${ago}s ago, waiting for reconnect...`;
190-
}
191-
}
192-
return response;
207+
return { ok: true, data: { nodes: treeData, totalCount } };
193208
}
194209

195210
case 'get-component': {
211+
const detachedForComponent = this.requireAttachedApp();
212+
if (detachedForComponent) return detachedForComponent;
196213
const resolvedId = this.tree.resolveId(cmd.id);
197214
if (resolvedId === undefined) {
198215
return { ok: false, error: `Component ${cmd.id} not found` };
@@ -212,25 +229,30 @@ class Daemon {
212229
}
213230

214231
case 'find':
215-
return {
232+
return this.requireAttachedApp() ?? {
216233
ok: true,
217234
data: this.tree.findByName(cmd.name, cmd.exact),
218235
};
219236

220237
case 'count':
221-
return {
238+
return this.requireAttachedApp() ?? {
222239
ok: true,
223240
data: this.tree.getCountByType(),
224241
};
225242

226-
case 'errors':
243+
case 'errors': {
244+
const detachedForErrors = this.requireAttachedApp();
245+
if (detachedForErrors) return detachedForErrors;
227246
this.tree.getTree();
228247
return {
229248
ok: true,
230249
data: this.tree.getComponentsWithErrorsOrWarnings(),
231250
};
251+
}
232252

233-
case 'profile-start':
253+
case 'profile-start': {
254+
const detachedForProfile = this.requireAttachedApp();
255+
if (detachedForProfile) return detachedForProfile;
234256
this.profiler.start(cmd.name);
235257
// Snapshot existing component names so they survive unmounts
236258
for (const id of this.tree.getAllNodeIds()) {
@@ -239,6 +261,7 @@ class Daemon {
239261
}
240262
this.bridge.startProfiling();
241263
return { ok: true, data: 'Profiling started' };
264+
}
242265

243266
case 'profile-stop': {
244267
await this.bridge.stopProfilingAndCollect();

packages/agent-react-devtools/src/devtools-bridge.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,14 @@ export class DevToolsBridge {
257257
}
258258
const added = this.tree.applyOperations(operations);
259259

260+
// A second root on one connection is either a genuine multi-root app or a
261+
// re-flush from another DevTools backend attaching; the tree decides by structure.
262+
const roots = operations.length >= 2 ? this.connectionRoots.get(ws) : undefined;
263+
if (roots && roots.size > 1) {
264+
const replaced = this.tree.reconcileReflushedRoot(operations[1]);
265+
if (replaced !== null) roots.delete(replaced);
266+
}
267+
260268
// Cache display names during profiling so unmounted components are still identifiable
261269
if (this.profiler.isActive()) {
262270
for (const node of added) {

packages/agent-react-devtools/src/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,15 @@ export type IpcCommand =
196196
| { type: 'wait'; condition: 'connected'; timeout?: number }
197197
| { type: 'wait'; condition: 'component'; name: string; timeout?: number };
198198

199+
/** Machine-readable failure reasons, for callers that must not key on error text. */
200+
export type IpcFailureReason = 'no-app-attached';
201+
199202
export interface IpcResponse {
200203
ok: boolean;
201204
data?: unknown;
202205
error?: string;
206+
/** Set alongside `error` when the failure has a typed cause */
207+
reason?: IpcFailureReason;
203208
/** The @cN label, passed through when commands use label-based IDs */
204209
label?: string;
205210
/** Contextual hint for empty or stale results */

packages/e2e-tests/src/cli-commands.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,102 @@ describe('CLI commands (e2e)', () => {
8383
expect(result.stdout).toContain('Usage:');
8484
});
8585
});
86+
87+
describe('CLI commands without an attached app (e2e)', () => {
88+
let stateDir: string;
89+
let port: number;
90+
let daemon: ChildProcess | null = null;
91+
92+
beforeEach(async () => {
93+
stateDir = createTempStateDir();
94+
port = getTestPort();
95+
daemon = startDaemon(port, stateDir);
96+
await waitForDaemon(stateDir);
97+
});
98+
99+
afterEach(async () => {
100+
await stopDaemon(daemon, stateDir);
101+
daemon = null;
102+
});
103+
104+
for (const args of [['errors'], ['count'], ['get', 'tree'], ['find', 'App'], ['profile', 'start']]) {
105+
it(`should fail \`${args.join(' ')}\` instead of reporting an empty result`, async () => {
106+
const result = await runCli(args, stateDir);
107+
expect(result.exitCode).toBe(1);
108+
expect(result.stderr).toContain('No React app is attached');
109+
expect(result.stderr).toContain(String(port));
110+
expect(result.stdout).not.toContain('No components');
111+
expect(result.stdout).not.toContain('0 components');
112+
});
113+
}
114+
115+
it('should still report status and honour wait timeouts', async () => {
116+
const status = await runCli(['status'], stateDir);
117+
expect(status.exitCode).toBe(0);
118+
expect(status.stdout).toContain('0 connected');
119+
120+
const wait = await runCli(['wait', '--connected', '--timeout', '1'], stateDir);
121+
expect(wait.exitCode).toBe(1);
122+
});
123+
});
124+
125+
describe('CLI commands when another DevTools backend attaches (e2e)', () => {
126+
let stateDir: string;
127+
let port: number;
128+
let daemon: ChildProcess | null = null;
129+
let ws: WebSocket | null = null;
130+
131+
const fullTree = (rootId: number, base: number) =>
132+
buildOperations(1, rootId, (s) => [
133+
rootOp(rootId),
134+
addOp(base + 1, ELEMENT_TYPE_FUNCTION, rootId, s('App')),
135+
addOp(base + 2, ELEMENT_TYPE_MEMO, base + 1, s('Header')),
136+
addOp(base + 3, ELEMENT_TYPE_FUNCTION, base + 1, s('UserProfile')),
137+
addOp(base + 4, ELEMENT_TYPE_HOST, base + 1, s('div')),
138+
]);
139+
140+
beforeEach(async () => {
141+
stateDir = createTempStateDir();
142+
port = getTestPort();
143+
daemon = startDaemon(port, stateDir);
144+
await waitForDaemon(stateDir);
145+
ws = await connectMockApp(port);
146+
sendOperations(ws!, fullTree(100, 0));
147+
await sleep(300);
148+
});
149+
150+
afterEach(async () => {
151+
if (ws && ws.readyState === WebSocket.OPEN) ws.close();
152+
await stopDaemon(daemon, stateDir);
153+
daemon = null;
154+
ws = null;
155+
});
156+
157+
it('should replace the frozen tree instead of counting it twice', async () => {
158+
// React Native DevTools (or another agent) attaching re-flushes the same
159+
// tree through the shared hook under a fresh fiber-ID space.
160+
sendOperations(ws!, fullTree(500, 1000));
161+
await sleep(300);
162+
163+
const count = await runCli(['count'], stateDir);
164+
expect(count.stdout).toContain('5 components');
165+
166+
const found = await runCli(['find', 'UserProfile', '--exact'], stateDir);
167+
expect(found.stdout.trim().split('\n')).toHaveLength(1);
168+
expect(found.stdout).toContain('id:1003');
169+
});
170+
171+
it('should keep a genuinely different second root', async () => {
172+
sendOperations(
173+
ws!,
174+
buildOperations(1, 500, (s) => [
175+
rootOp(500),
176+
addOp(1001, ELEMENT_TYPE_FUNCTION, 500, s('Sidebar')),
177+
]),
178+
);
179+
await sleep(300);
180+
181+
const count = await runCli(['count'], stateDir);
182+
expect(count.stdout).toContain('7 components');
183+
});
184+
});

0 commit comments

Comments
 (0)