mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
filip131311
7e7f7f73b6
Fixes #618. ## The reported symptom is the mildest part `debugger-connect` returns a Metro `logicalDeviceId`, 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, and `classifyDevice` decides 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: ``` ios ← 1E273101-2926-4A76-88D0-544C7EA5C2FD (real UDID) android ← bb2e9dd6a3f14c8e9d0f1a2b3c4d5e6f (Metro logicalDeviceId) android ← "" (empty string) android ← "totally-bogus" ``` Because that id names **no existing session**, `services()` builds `NativeProfilerSession:<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-start` dispatches the same misclassified id down the Android path, where `platforms/android.ts` hands it to **adb as a serial**. - `react-profiler-status`, `-fiber-tree` and `-renders` build 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 #528 exists to prevent. ## The fix `device_id` resolves 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 to `services()` and `execute()` (`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 to `adb` and `simctl`. Canonicalizing inside `services()` 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: ``` input JSON schema → {"type":"string","minLength":1,"description":"…"} parse(LOGICAL) → REAL-UDID double-parse → REAL-UDID (http.ts parses, then invokeTool parses again) parse("") → rejected ``` **`classifyDevice` is 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 are `8a44101d` for a logical id and `HT82A0203045` for 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-devices` as 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 keeping `string`/`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: - The remaining 16 Metro-family tools carrying `device_id` (24 in total across the registry). - Seven tool descriptions that say *"Device logicalDeviceId from debugger-connect"* — stale, since `react-profiler-start`/`-stop` both canonicalize before building their URN and cache under the canonical id, so the session is keyed by the list-devices id either way. - **The skill docs, which drive the agent harder than any zod description**: `argent-metro-debugger/SKILL.md:26` calls `device_id` *"a.k.a. logicalDeviceId"*, `:34` says to *"use it as the device_id for every subsequent debugger call"*, and `references/failure-scenarios.md:10` repeats that after a crash — which is exactly the alias-forgotten path. Also noted for a follow-up: `platforms/android.ts` uses `params.device_id` as the adb serial in some places and `api.deviceId` in 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. --- > **Stacked on #663** (`filip/profiler-component-name-resolution`). Both add an import after `readCommitTree` in `profiler/query/profiler-commit-query.ts`. Rebased on #663, so review only the top commit. GitHub retargets it to `main` when #663 merges.