fix(profiler): resolve a Metro logicalDeviceId instead of calling it an Android device - #687
Open
filip131311 wants to merge 1 commit into
Open
Conversation
…an Android device `debugger-connect` returns a Metro logicalDeviceId, and when several devices share one Metro the debugger tools instruct the user to pass it. Profiler sessions, though, are keyed by the list-devices id, and platform is decided purely from an id's shape — an opaque logical id matches nothing, so it fell through to "android". The reported symptom was the mildest part. Because the id named no existing session, the registry minted a brand-new one on demand, frozen to android and holding no capture. So an iOS user was not merely told the wrong word: the tool reported on a session that had never recorded anything. Correcting only the platform string would still have been wrong, since the real data sits under the UDID. It reached further than the message, too. The same misclassification sends native-profiler-start down the Android path, where the id is handed to adb as a serial, and three react-profiler tools build session URNs with no canonicalization at all — opening a second CDP connection to a device that already has one, which is what the alias map was introduced to prevent. The device_id parameter now resolves through the alias map in the schema itself. That placement is the point: the registry parses params once and hands the same object to services() and to execute(), so one transform covers URN construction, the platform branch, and every id forwarded to a platform impl — including the ones passed to adb and simctl. Canonicalizing inside services() alone would let those disagree with each other. classifyDevice is deliberately untouched. Its android fallback is load-bearing: a physical Android serial is an arbitrary manufacturer string with no matchable shape, so any rule strict enough to reject a logicalDeviceId rejects real hardware too. There is a test pinning that. Canonicalization alone would not have been enough. Aliases live only as long as the debugger connection — they are dropped on dispose — so a user who profiles, lets the app reload, and then asks for a report has nothing to resolve through. That is the ordinary case, not an edge one. So a session with no capture state at all now reports that without naming a platform, and points at list-devices as the id to use. The platform-specific messages are kept for the states that actually prove a platform, so an Android user who started but never stopped still gets the Android instruction. An empty device_id is also rejected rather than resolving to a plausible-looking Android device. Scoped to the eight profiler tools that route to a native session, which is what closes the reported bug. The remaining Metro-family tools and the tool and skill docs that tell the agent to reuse a logicalDeviceId as device_id are a separate change, since that one rewrites agent-facing semantics rather than fixing a defect. Fixes #618
filip131311
force-pushed
the
filip/device-id-classification
branch
from
August 3, 2026 07:46
386c9cc to
002f80f
Compare
filip131311
changed the base branch from
main
to
filip/profiler-component-name-resolution
August 3, 2026 07:46
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.
Fixes #618.
The reported symptom is the mildest part
debugger-connectreturns a MetrologicalDeviceId, and when several devices share one Metro the debugger tools instruct the user to pass it (#522). But profiler sessions are keyed by the list-devices id, andclassifyDevicedecides platform purely from an id's shape — an opaque logical id matches nothing, so it falls through to"android"(utils/device-info.ts:52). Reproduced against the built code:Because that id names no existing session,
services()buildsNativeProfilerSession:<logicalId>, and the registry mints services on demand — so it resolves to a brand-new session, frozen to android, holding no capture. The user isn't told the wrong word; the tool is reporting on a session that never recorded anything. Fixing only the platform string would still be wrong, since the data sits under the UDID.It reaches past this one message:
native-profiler-startdispatches the same misclassified id down the Android path, whereplatforms/android.tshands it to adb as a serial.react-profiler-status,-fiber-treeand-rendersbuild session URNs with no canonicalization, so following the debugger tools' advice opens a second CDP connection to a device that already has one — precisely what the alias map in fix(debugger): collapse a device's two ids onto one debugger instance #528 exists to prevent.The fix
device_idresolves through the alias map in the schema itself. That placement is the whole point: the registry parses params exactly once and hands the same object toservices()andexecute()(registry.ts:125-153), so one transform covers URN construction, the platform branch, and every id the tool forwards to a platform impl — including the ones passed toadbandsimctl. Canonicalizing insideservices()alone would leave those disagreeing with each other, which is how the bug got past the places that did canonicalize.Verified on the repo's zod 4.4.3 that this is invisible to callers:
classifyDeviceis deliberately untouched. Its android fallback is load-bearing — a physical Android serial is an arbitrary manufacturer string with no matchable shape (the fixtures in-tree are8a44101dfor a logical id andHT82A0203045for a real serial; both are opaque alphanumerics, so any rule strict enough to reject one rejects the other). There is a test pinning that.Canonicalization alone would not have been enough
Aliases are learned on debugger connect (
js-runtime-debugger.ts:267) and forgotten on dispose (:287). A user who profiles, lets the app reload, and then asks for a report has nothing left to resolve through — so "alias unknown" is the ordinary state, not an edge case.So a session with no capture state at all now reports that without naming a platform, and points at
list-devicesas the id to use. The platform-specific messages are kept for the states that actually prove a platform — an Android user who started but never stopped still gets "No Android trace loaded", and that existing test fixture required no change.Tests
12 new cases: alias resolution, the URN/platform actually produced by
services(), pass-through once the alias is forgotten, the published schema keepingstring/minLength/description while hiding the transform, empty-id rejection, and the neutral gate firing for a never-captured session regardless of which platform the shape guessed — while the Android-specific message survives when a trace proves the platform.Three of them exist to stop a future "tidy-up" reintroducing this: a physical Android serial, a wireless-debugging address, and an emulator serial must all still classify as they do today.
Mutation-verified: removing the canonicalization fails 2; removing the neutral gate fails 2.
Full tool-server suite green — 3098 passed / 297 files.
Scope
Deliberately limited to the eight profiler tools that route to a
NativeProfilerSession, which is what closes this bug.Not in this PR, because it rewrites agent-facing semantics rather than fixing a defect, and a revert of the doc churn shouldn't revert the fix:
device_id(24 in total across the registry).react-profiler-start/-stopboth canonicalize before building their URN and cache under the canonical id, so the session is keyed by the list-devices id either way.argent-metro-debugger/SKILL.md:26callsdevice_id"a.k.a. logicalDeviceId",:34says to "use it as the device_id for every subsequent debugger call", andreferences/failure-scenarios.md:10repeats that after a crash — which is exactly the alias-forgotten path.Also noted for a follow-up:
platforms/android.tsusesparams.device_idas the adb serial in some places andapi.deviceIdin others; that divergence closes by construction once the family change lands. And a bogus android session minted before this fix can linger in a long-running server — inert, and new calls no longer reach it.