Repository navigation
Conversation
|
Size Report
Startup median (7 runs, lower is better):
|
|
Thanks for this. At e16c7c9 I found three code defects and no live evidence at head, so this is not ready to merge yet. All 22 checks pass, but CI cannot reach the Hermes/Metro route, and the unit fixture runs the injected source in node:vm without XMLHttpRequest. A failed fetch hides every later XHR to the same URL. The fetch wrapper takes a URL-keyed claim at line 135 and releases it only on a synchronous throw and on fulfilment. The rejection handler at line 188 never releases it, so that URL stays claimed and each later XHR to it records nothing. The claim is also held for the whole request, so a direct XHR concurrent with an in-flight fetch to the same URL is dropped. I reproduced this in node:vm with a hand-written fake XMLHttpRequest and a whatwg-fetch-style fetch: a failed fetch to /down, then an XHR to /down, gave no xhr entry. On an app that mixes fetch with axios, one network error or abort makes later axios calls to that URL invisible, and the dump looks clean. The rule should be: an XHR is attributed to fetch only when its send() runs synchronously inside a wrapped fetch call. Releasing the claim right after fetch.apply returns satisfies that. Since the PR body says live runs showed one entry per request without the guard, deleting the guard is also fine if a fresh run at head shows no double entries. A fixture test with fetch built over the fixture's XMLHttpRequest should cover a rejected fetch followed by an XHR to the same URL. It fails at this head. The dump may read and inject into another device's app. network-capture-dump.ts:95 calls selectReactNativeTarget with only the optional appId and no device filter. If one Metro serves several devices and the session's own app is not connected at that moment (closed, crashed, reloading), the same app id on another device is the only candidate. The dump then hooks that runtime and returns its traffic as this session's provider entries. That is wrong attribution, and it changes an app on a device this session does not own. The provider should read and inject only the runtime of the session's own app on the session's own device. Is passing the session device through NetworkTransportScope into RuntimeTargetFilter.device the right route, or should the selected target be checked against the session device and fail closed with a typed reason? Please also skip the provider when the session has no appBundleId, and add a two-device fixture where only the other device is connected and no entries come back. I did not run a two-device Metro, and I am not sure the Metro deviceName maps reliably to DeviceInfo.name on Android. Provider notes now suppress the inactive-log guidance. Both runtimes seed the fallback notes with provider.notes (Apple runtime.ts:30, Android runtime.ts:33) before appendLifecycleNote runs. That function only adds the 'Session app log stream is inactive…' note when the notes list is empty, so an arming note or a provider-unavailable note now hides it. On a React Native session with an inactive log stream, the arming dump and every dump with Metro unreachable lose the hint to run logs clear --restart. The new runtime tests use an active snapshot, so they cannot see this. Lifecycle notes should depend only on the app-log scan and its recovery. Build them first and add the provider notes after, or gate the inactive note on a recovered flag, in both runtimes. One Apple and one Android test with an inactive snapshot and an armed provider should expect both notes. This change installs code in a live app runtime over CDP, so it needs live proof at head. The live claims in the PR body have no output and no SHA, and they predate 922d57f, which rewrote the injected Hermes source (claim guard, recording after clone.text(), header caps, eviction accounting). After the fixes above, please run the examples/test-app network lab at the new head on an iOS simulator and an Android emulator, and paste the CLI output: (1) the first network dump shows source: app-log with the arming note; (2) after network-fetch, network-post and network-xhr, network dump --include all shows source: provider with exactly three entries (transport fetch, fetch, xhr), status 200, a [REDACTED] sensitive header and bounded bodies, and network-lab-status shows the app's own requests settled; (3) an immediate second dump repeats no entry and does not re-arm; (4) after a Metro reload the next dump re-arms (source: app-log plus the arming note) and a later dump still shows one entry per request; (5) a failed fetch followed by an XHR to the same URL reports the XHR. Not blocking, take or leave: providerFailureNote in capture-kit turns every provider failure into prose, so a typed outcome such as provider: { status: 'armed' | 'unavailable', reason? } on the app-log result would be easier to act on; redactNetworkHeaders repeats redactDiagnosticData's object path, so calling redactDiagnosticData(headers) in network-capture-entries.ts:20 would let you delete it; the early return in the SENSITIVE_ASSIGNMENT_RE replacer in redaction.ts cannot fire (I checked it in node against {"token":"abc"} and similar inputs), so it and its comment can go, and the new JSON rule also widens redaction for every redactDiagnosticData caller, which is worth a docs line; the fetch wrapper returns only after a second full body read, which is cheap with whatwg-fetch but would delay a streaming global fetch, so returning the response at once and recording in a detached chain is safer (I did not check whether current Expo SDKs replace globalThis.fetch); the fixture app defines no XMLHttpRequest, so instrumentXHR and the claim guard never run in any test, and the unreachable-server case asserts a message regex instead of details.reason; the docs in commands.md say 'every request' and 'names every request it observed', but capture covers only fetch and XMLHttpRequest after arming, the code accepts any apple/android device kind while debugging-profiling.md says simulator or emulator, and URL query strings are not redacted, which the docs should say; and the abort signal is checked once but not passed to the target list or the InspectorSession, so the CDP session can outlive the 12 s budget, and closing it on abort would fix that. Could this be smaller and better owned? Shipped code is about 880 lines, and the PR body names no rejected alternative. One shape: a single capture-kit owner, dumpSessionNetwork, walks one ordered source list (an armed in-app source, then the app log with platform recovery), with Apple and Android calling it and web as the provider-only case. Then the #3928 probe becomes another NetworkTransport resolved at the composition root from sessionId, so neither PR adds a NetworkDumpInput field or host slot. stateDir could also leave NetworkDumpInput if the composition root resolves it the way readMetroSessionRuntime does (I did not verify that the daemon's resolveDaemonStateDir agrees). Dropping the fetch-claim guard and redactNetworkHeaders would remove roughly 100 to 150 lines and one seam. The 320-line injected runtime looks like the needed core for React Native versions without a Network CDP domain. Subscribing to RN DevTools' built-in Network domain would need no injected code but would need a daemon-held CDP connection and drop older RN (from memory, not checked against RN source), and the PR body should state that trade-off. Before either PR lands, #3928 and this PR need a decision on which goes first, and ADR 0037 (or a new ADR) should name the source-selection owner, the source enum ('probe' | 'provider' | 'app-log'), and one opt-in policy, since today they differ on injecting into the app under test: explicit at open versus implicit on the first dump. Before merge, please fix the three defects above, post the live iOS and Android run at the new head, and settle source-selection ownership with #3928. |
|
All findings addressed at 1. A failed fetch hides every later XHR to the same URL (blocker). Fixed the way you sketched: the claim is released synchronously right after The second line in each pair is the XHR (no 2. The dump may read and inject into another device's app (blocker). Both routes you asked about, threaded through the existing seam: On your doubt about the name mapping: it is reliable enough to enforce. iOS Metro reports the simulator name verbatim ( 3. Provider notes suppress the inactive-log guidance (blocker). Both runtimes now build scan/recovery/lifecycle notes first and append provider notes last, so 4. Live proof at head (blocker). Full sequence on iOS simulator (1) First dump arms, answers from app log, arming note last: (2) After Exactly three entries, one per request, (3) Immediate second dump: (4) Cold app restart ( (5) Above in item 1, on both platforms. Non-blocking items — took almost all:
Arm at app open when Metro is bound: no, deliberately. Injecting on the first explicit On making the change smaller: I did not restructure to the single Gates at |
|
The earlier findings from e16c7c9 are fixed at a03ce9c, except for one that remains: device matching in the Metro runtime selection. CI is green (22 checks, none failing), and there are no conflicts. The device match treats a shared name as an identity (metro-inspector.ts#L180). runtime.ts#L34 adds Not blocking, take or leave: the docs sentence at debugging-profiling.md#L220 lists axios as a native HTTP stack outside the capture, but axios uses the XMLHttpRequest adapter that Could one exact device identity per platform, passed as the existing Evidence limits on my side: I read the code and did not run a multi-device Metro. The device-name finding relies on the author's live Android name, on The device-selection change with its live run is what must happen before merge, and then the ADR settles source-selection ownership with #3928. |
… device Review round on #3928 (thymikee, second pass). A refused open still changed the device first: the owner refusal ran inside invokeApplicationOpen, after the relaunch close (closeBeforeRelaunch on Limrun/webdriver), the runner stop, and the runtime-hint write. `open <app> --relaunch --capture-network` on a provider iOS simulator killed the running app and then failed. - One admission step: contracts `assertOpenAdmitted(binding, execution)` is the whole rule for `--launch-console` and `--capture-network`. It is the first call in openDirectApplication, openAndroidApplication and openAppleApplication, so an owner that cannot honor the option is refused with UNSUPPORTED_OPERATION before any device action. It replaces the nine `supportsLocalSimulatorLaunch` pass-through sites and the four stacked refusals in the canonical open; `invokeApplicationOpen` keeps only its positional, URL, and argument-shape checks. - Both refusals carry a typed reason (`launch-console-owner-unadmitted`, `network-capture-owner-unadmitted`), matching the sibling `provider-runtime-interactor-missing` posture. Their messages stay module constants of the module that raises them, clear of any function's JSDoc (the shared message home's facade must stay exhaustive, and a re-export no consumer imports is dead code). - The provider-shaped contract test now opens with `relaunch: true` against a `closeBeforeRelaunch: true` owner and asserts the typed code and reason and that neither open nor close ran. A second case pins the same typed refusal for a local binding on a non-simulator device. - A probe file at its 32 MB cap no longer reads as current traffic: the capture read reports `atSizeCap` (the reader owns the cap constant mirroring the probe's AdMaxFileBytes) and the probe-mode dump says the window stopped recording. The cap check reads the handle offset through `getOffset:error:` instead of the raising `offsetInFile`, per ADR 0037 rule 6. - ADR 0037 rule 1 now states admission-before-device-action, rule 6 the cap note and the offset API, and rule 5 fixes Apple dump precedence — probe, then provider, then app-log — as the answer to the #3930 Metro question. Live evidence re-run at this head on ios-sim-net: one open with --capture-network --launch-console produced source=probe with five distinct httpbin URLs, a 10-line console log, masked Authorization/Cookie with X-Trace-Id kept, zero secret occurrences in network.jsonl; `open --relaunch --capture-network` on the local simulator stays admitted (relaunch close, then capture, dump still probe); a non-admitted open refuses with UNSUPPORTED_OPERATION and reason network-capture-owner-unadmitted.
…licy Settles the source-selection question raised against #3930 and #3928 in one record: the platform runtime owns the order, the source enum is probe/provider/app-log, no capture installs unless an explicit operator command arms it, and the React Native provider injects only on the runtime whose device the platform named uniquely among attached devices.
|
Round 2 addressed at Device matching treats a shared name as an identity (blocker). Fixed with your rule and your ownership split. The alias list,
The fail-closed rule sits at the two owners that matter: Unit cases as requested: two same-model targets in Live run, two same-image Android emulators, one Metro, head (1) Both emulators attached, session A on 5554, its app closed/restarting state — A's dump fails closed and B is untouched: No injection happened on either device (the failure is before target listing; nothing was drained from B). Bonus corroboration from this run: Metro itself listed only one target for two attached same-image emulators — the shared device name collides in Metro's own device registry too, which is exactly why a shared name cannot be an identity. (2) Positive branch after shutting emulator-5554 down ( On your evidence limits: agreed on all three, and the clone-AVD run above is now pasted output at this head. On the Metro-reload docs sentence — fair doubt. My earlier claim was measured on this app only: after a Non-blocking items — all taken:
ADR (source-selection ownership with #3928): added Arm-at-open policy for this PR stays as stated in the round-1 reply, now also recorded in 0038. Gates at |
network dump answers a React Native app session from an in-app fetch/XMLHttpRequest capture installed over the session's dev-server debugger connection, instead of only parsing what the app wrote to the platform log. The capture is an ES5 runtime source installed into the app's JavaScript runtime on the first dump. It records method, full URL, status, duration, headers, and text bodies into a bounded ring buffer, drains newest-first, and survives repeated dumps without repeating entries. The host projects untrusted entries, redacts headers and bodies with the shared kernel rules, and adds JSON secret masking so a payload secret no longer passes through. When no dev-server binding exists or the provider cannot answer, the dump falls back to the app-log path and its notes say why. - contracts: NetworkTransportScope carries the session facts a provider transport needs; NetworkDumpResult reports its source - capture-kit: shared non-throwing provider prelude with a 12s budget - react-native: runtime source, CDP drain/stats orchestration, projection - root composition gates the transport on the session's Metro binding - CLI help and website docs describe arming, draining, and fallback
The route issues one plain fetch, one JSON POST, and one XMLHttpRequest per press, so a network dump can be verified against requests the harness just made. Live RN verification used it on an iOS simulator and an Android emulator.
Address review findings on the React Native network capture: - Eviction counts ride the drain result so buffered and dropped notes can never disagree with what a dump returned. - The in-app body cap takes the dump's payload budget; header values are capped at 512 characters across 64 entries at every capture seam. - JSON-quoted secrets with escaped quotes mask whole; the unquoted rule keeps masking 'api_key=value'; truncation markers count against the payload budget. - Drain JSON parsing fails with a typed reason instead of throwing raw. - whatwg-fetch-on-XHR double capture is suppressed by a synchronous URL claim held across the fetch() call. - The 2048 payload budget is one contract constant, shared by the daemon and the RN transport. - The test-app network lab is a registered route with README coverage.
The Apple and Android dumps read their fallback window with one capture-kit call instead of cloned lines, the projection splits its include branches into named helpers, and the capture backend name stops exporting what no consumer consumes.
- Release the fetch URL claim when fetch() returns: the claim marks only the synchronous window a whatwg-fetch XHR is built in, so a failed or in-flight fetch no longer hides later XHRs to the same URL. - Record fetch entries in a detached chain: the app gets its response at once even behind a streaming global fetch. - Hook only a runtime that reports the session's own device: the transport scope carries device aliases, selection narrows to positively-matching runtimes, and a mismatch or a session without an app id fails the provider closed with a typed reason instead of injecting into a stranger's app. - Build lifecycle and scan notes from the app-log source alone and append provider notes last, so an arming or unavailable note never hides the inactive-stream guidance. - Close the CDP socket on abort and thread the signal through the target list. - The fixture runs a real XMLHttpRequest shape with fetch polyfilled over it; new tests cover direct XHR capture, a failed fetch followed by an XHR to the same URL, and two-device attribution. - Drop redactNetworkHeaders for redactDiagnosticData and the dead JSON-form guard; type the unreachable-dev-server failure; state the capture's real scope, device kinds, and verbatim query strings in the docs.
…Android name An Android runtime reports its device as '<model> - <release> - API <level>', while the session knows the model from the inventory probe. Match the model as a prefix so a session's own Android runtime survives device attribution. Identification now requires a reported device name; a synthetic logical device id alone cannot match a session alias.
Live verification showed a reload does not wipe the JS global, so only an app process restart resets the capture and re-arms it.
A shared model or simulator name is not an identity: two emulators of one system image (or two same-name simulators) could match each other's runtimes, so one session could hook, drain, and report another device's traffic. The platform that owns the device inventory now composes the dev-server identity only when exactly one attached device answers to it — Android the composed '<model> - <release> - API <level>' after checking every attached serial, Apple the simulator display name after checking booted simulators — and an app-backed dump without one fails closed with metro-device-mismatch instead of guessing. This replaces the alias list, the model-prefix rule, and the requireDeviceMatch seam.
…licy Settles the source-selection question raised against #3930 and #3928 in one record: the platform runtime owns the order, the source enum is probe/provider/app-log, no capture installs unless an explicit operator command arms it, and the React Native provider injects only on the runtime whose device the platform named uniquely among attached devices.
…s listing The network provider's device-identity probe lists booted simulators before resolving the transport; the shared listing helper now recognizes that argv.
d70bf84 to
d0eda05
Compare
|
The earlier findings from a03ce9c are mostly addressed at d0eda05, but two identity checks still fail open and the iOS path has no live run. Apple: the identity probe in runtime.ts:33 lists booted simulators only in the session's own simulator set. Take session A on a default-set "iPhone 17 Pro" and session B on a tenant-set simulator with the same name, both using one Metro. Each sees one such simulator in its own set, so each gets the identity. When A's app is closed, A's dump can select, inject into and drain B's runtime. This is the same cross-device drain as before, and it contradicts the "exactly one device attached to this host" wording in ADR 0038. Rule: grant the identity only when no other booted simulator on the host carries the name, in any set, and a physical device that could report the same name also counts. If the platform cannot check every set, fail closed. If a host-wide check is not feasible, record the per-set limit as an accepted risk in the ADR and docs. Please add a unit case with a same-named booted simulator in another set that expects no identity. Android: the collision loop in runtime.ts:99 treats an unknown device as unique. If getprop fails, times out or prints nothing on another serial, Evidence: the Apple route changed from The Android two-emulator and single-emulator runs cover the Android route. I read the code only and did not run Metro, simulators or emulators. The Apple finding assumes RN's iOS inspector reports the simulator display name as deviceName and that two simulator sets can share one Metro server. The Apple "Smoke Tests" check is still running, and I cannot yet tell whether it overlaps with the network dump change. Before merge, both probes must fail closed on anything unknown and the iOS provider run must be posted. Not blocking, and fine to take or leave: the identity probe runs before the transport is resolved, so native non-RN sessions pay 1 + N adb calls without a timeout, or a 4 s simctl listing, outside the capture budget, and it could be resolved lazily after the transport declares a dump; and the new adb and simctl parsing duplicates |
…device The identity probe granted a name while devices it could not see remained possible. Android now lists every serial adb reports, in any state, and withholds the identity when any serial's properties cannot be read. Apple now lists booted simulators in every set the host can name and the connected physical devices devicectl reports, and fails closed when either answer is unreadable or the daemon's configured set contradicts the device. The probe runs lazily inside a dump, so a session whose transport declares no provider dump never pays for it.
…the Apple identity devicectl registers the host's booted simulators as connected devices under their own udids, so the identity probe that lists connected devices to rule a simulator name in among physical devices sees the session simulator itself. The live run caught it: the first dump failed closed on this host because the session's own booted simulator answered as a connected other device. The probe now compares the device identity across CoreDevice's registrations and skips only the session's own udid, so a same-named booted twin is still counted.
|
Round 3 (at Blocker 1 — Apple: the name must be unique across every set and against physical devices
The live run found a collision this rule exposes that the old probe could not see: CoreDevice registers booted simulators themselves as connected devices ( Unit cases ( Blocker 2 — Android: every listed serial, any state, must be read
Test: second serial Live iOS run at the final headEnvironment: simulator Dump 1 — arming note ( Dump 2 — Drain-once, immediately after: Two booted simulators sharing the name → fail closed. Created a second simulator named Shutting the twin down and re-running returned Non-blocking notes — both taken
Validation at |
|
Thanks for the update. The earlier findings from d0eda05 are addressed in 3271f87, but one cross-device case is still open. All 22 checks pass and there are no conflicts. A simulator session can still take the identity when a same-named physical iPhone is paired but idle. Not blocking, and you can take or leave these: (1) Could each platform's inventory owner provide one fail-closed census of the host's devices, so that the network identity reduces to "the census names I did not run devicectl, simctl, Metro, a simulator or the unit tests. The paired-but-idle finding assumes devicectl reports a Before merge, count every devicectl-listed physical device regardless of tunnel state, add the disconnected-but-paired unit case, and post one iOS simulator dump at the new head showing |
Summary
network dumpon a React Native session with a Metro dev-server binding now installs a JSfetch/XMLHttpRequestinterceptor into the app runtime over the Metro inspector (CDPRuntime.evaluate) and reports real traffic: method, full URL, status, duration, headers, and bounded bodies. Previously it only parsed app-log lines, which on RN name almost nothing.The first dump arms the hook and answers that window from the app log (
source=app-logplus an arming note); later dumps answer from the in-app ring buffer (source=provider, drain-once, newest-first, 200 entries). No Metro binding, an unreachable dev server, several connected apps, or a session device the host cannot name uniquely keeps the old log path with a typed note. The provider injects only into the runtime whose reported device equals the platform-composed Metro device identity — a name that marks exactly one device this host can see (Android reads everyadb devicesserial in any state and withholds the name when any read fails; Apple checks booted simulators in every simulator set the host can name plus CoreDevice's connected devices, and ADR 0038 records the one residue it cannot see: a set directory the host was never told about); anything weaker fails closed withmetro-device-mismatchbefore any injection. The identity probe resolves lazily inside a dump, so only a session whose transport declared a provider dump pays for it. A session without an app never reaches the provider at all. Bodies/headers are capped and redacted with the shared kernel rules. Provider preference lives inside the Apple/Android runtimes via the existingNetworkTransportslot, sonetwork dumpstays always-available.docs/adr/0038records the source owner, theprobe | provider | app-logenum, and the lazy-arming opt-in policy shared with #3928.Rejected alternatives
network dump, so nothing is installed in the app under test until a command asks for network data. Open-path arming needs an attach-retry policy for cold-start races and pre-empts the feat(ios): capture every URLSession request with open --capture-network #3928/ADR injection-policy decision.Validation
pnpm check:affected --runat head3271f8738e: all runnable checks passed (exactCommit=true, basec48d27a819, receipt.tmp/validation/run-DWkJRo/receipt.json);fallow audit --base c48d27a819passes on changed files.source=providercapture, drain-once, then a second booted same-named simulator flips the dump tosource=app-logwith the mismatch note, and shutting the twin restores it. Verbatim outputs in the round-3 reply.metroDeviceIdentity(device)is platform-owned — Android composes<model> - <release> - API <level>only when unique acrossadb devices, Apple passes the simulator name only when unique among booted simulators; anything weaker fails closed withmetro-device-mismatchbefore any injection. Live on two same-image emulators (bothsdk_gphone64_arm64): shared model fails closed with no injection on either device; one attached device arms and captures the full sequence. Verbatim outputs in the round-2 reply.rn-net2 ios, final head — arming dump (source=app-log+ arming note), thensource=providerdumps with full URL/status/durationMs/headers/bodies for the network lab's fetch/POST/XHR (httpbin),x-api-keyshown as[REDACTED]; drain-once verified; cold restart re-arms; a failed fetch followed by an XHR to the same URL reports both entries.ReactNative_API_35— same sequence; entries carry theokhttp/4.12.0User-Agent.matchedLines=0.🤖 Generated with Claude Code