fix: fail tree observation without an attached app and replace re-flushed roots - #59
Open
thymikee wants to merge 1 commit into
Open
fix: fail tree observation without an attached app and replace re-flushed roots#59thymikee wants to merge 1 commit into
thymikee wants to merge 1 commit into
Conversation
…shed 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this?
Two daemon-side fixes reported through callstack/agent-device#2430, where
agent-react-devtoolsruns behindagent-device react-devtoolsas an agent-facing verification step.get tree,get component,find,count,errorsandprofile startreturned an empty result with exit 0 when zero apps were connected. The response now carriesreason: 'no-app-attached'and the CLI exits 1 with a message that says whether an app ever connected, and when it disconnected.Why?
1. With 149 components attached and with nothing attached at all,
errorsprinted the byte-identicalNo components with errors or warningsand exited 0. Read commands also callensureDaemon(), so with the daemon down they silently started one and still reported a clean pass. An agent cannot tell "no problems" from "nothing was observed", andwait --connectedwas the only command that behaved.2. When React Native DevTools opens (or any other client, such as a CDP session, attaches) while the daemon is connected,
react-devtools-corerunsinitBackendagain:attach()creates a new renderer interface with a fresh fiber-ID space,hook.rendererInterfaces.set(id, …)replaces the old one, andflushInitialOperationsemits on the hook-wideoperationschannel that every agent subscribes to (sub('operations', agent.onHookOperations)). The daemon receives exactly one message: a full tree under a new root id, with norendererAttachedorbackendInitialized. Measured on a real React Native 0.87.1 app with #58 applied:countwent 149 → 298 after one attach, 1075 → 6450 after five on an Expo app, andfind Appreturned one hit per attach. Because commits dispatch throughrendererInterfaces.get(rendererID), the daemon's original root never updates again, so the fix has to replace it.How does it work?
daemon.ts:requireAttachedApp()guards the observation commands and supersedes the oldget-treehint, which only fired when an app had previously connected.component-tree.ts:reconcileReflushedRoot(rootId)drops an older root of the same renderer whose structure (type, display name, key, child order) matches. A structurally different second root is kept, so genuine multi-root apps are unaffected.devtools-bridge.ts: runs the reconciliation only when a connection holds more than one root, so single-root apps pay nothing per operations batch.types.ts:IpcFailureReasonso callers key on a typed reason, not error text.Verification
ComponentTree.reconcileReflushedRootcases. e2e: five new no-app CLI cases, two reflush cases, and the disconnect test now expects the typed failure.daemon-auto-restartprobed liveness withget treeand no app; it now usesprofile slow, which still goes throughensureDaemon(). 129 unit and 36 e2e protocol tests pass.react-native@0.87.1app with feat: restore React Native DevTools connections #58 merged on top: attach at 149 components, two foreign attaches via Metro's inspector proxy leavecountat 149 andfind App --exactat one result; killing the app makeserrorsexit 1 withapp disconnected 3s ago, waiting for reconnect.Relation to #58
Independent of the transport. #58 restores attachment on React Native 0.87+; this PR makes what the daemon reports trustworthy once attached. Merging both leaves one trivial docs conflict in
SKILL.md; I will rebase whichever lands second.