Files
software-mansion__argent/packages/tool-server/test/metro/target-selection.test.ts
filip131311 4975ff490c fix(debugger): cut debugger tool fail rate — structured not-connected results, classified CDP failures, narrow recovery (#610)
## Why

PostHog (30d) shows the debugger tool family is argent's least reliable
surface:

| tool | fail rate | dominant causes |
|---|---|---|
| `debugger-status` | **37%** | METRO_NO_TARGETS /
SERVICE_INITIALIZATION_FAILED / zod |
| `debugger-connect` | **24%** | SERVICE_INITIALIZATION_FAILED (~7k, >1k
users), METRO_NO_TARGETS |
| `debugger-log-registry` | 41.7% raw / 4.2% excl. storms | one user
retry-looped it **12,742×** on NO_TARGETS |

Three design issues drive this: (1) "app/Metro not ready yet" — an
expected precondition — is reported as a tool *error*, which both
inflates fail rates and invites agent retry storms; (2) every CDP
init/lifecycle fault lands in telemetry as the catch-all
`REGISTRY_SERVICE_INITIALIZATION_FAILED` with `error_kind=unknown`; (3)
~600 users/30d hit zod validation failures with zero visibility into
which parameter was wrong.

## What

**1. Classified failures** (`d71efa43`) — six former plain-`Error` sites
in `cdp-client.ts` / `target-selection.ts` now carry dedicated
`DEBUGGER_CDP_*` / `DEBUGGER_TARGET_DEVICE_MISMATCH` signals (messages
byte-identical). The registry's two causeless "terminating window"
`ServiceInitializationError`s get their own
`REGISTRY_SERVICE_TERMINATING` code.

**2. Narrow recovery** (`ff9c1955`) — `recoverable()` on both debugger
blueprints, scoped to windows where retry is provably safe: Metro
recovers only `DEBUGGER_CDP_NOT_CONNECTED` (request never left the host;
a detected socket death already self-heals for the *next* call via the
terminated cascade — recovering `CONNECTION_CLOSED` there would be dead
code, see the blueprint comment). Chromium also recovers
`CONNECTION_CLOSED` for the tab-switch reconnect window.
`REQUEST_TIMEOUT` stays unrecoverable (double-execution risk). A
reachability test proves the retry actually fires for a RUNNING node.

**3. Structured not-connected results** (`48b91ffd`) — `debugger-status`
and `debugger-log-registry` become factories that return `{ status:
"not_connected", reason, detail, guidance }` for classified
preconditions (`metro_not_running | no_app_connected | device_mismatch |
cdp_unreachable | stale_connection | reconnecting`) and still throw on
unexpected faults. `detail` preserves the original error text for
string-matching agents. `debugger-status` gains a socket-state gate
(Metro: dispose stale node; Chromium: report transient reconnect, no
dispose — the client belongs to the ChromiumCdp dependency).
log-registry keeps **no** gate on purpose: captured logs are readable
over a dead socket, and disposal would destroy the post-crash log file.
Flows/run-sequence map a not_connected result to a **failed step**
(mirrors `isUnmetUiWaitResult`) so recorded connectivity gates don't
silently pass. `debugger-evaluate` re-codes agent-expression throws as
`DEBUGGER_EVALUATE_EXPRESSION_THREW`.

**4. Telemetry + guidance + docs** (`7602a70d`) —
`tool:fail.invalid_params` (schema-declared names only; unknown keys →
literal `"unrecognized_keys"`; capped emit-side at 16), new
`debugger:tool_outcome` event (coded enum; joins
`tool:invoke`/`complete` via `tool_invocation_id` —
`ai_client`/`duration` live on the joined row), anti-retry-loop guidance
appended to the Metro discovery errors (first sentences verbatim —
skills match on them), six skills updated, Telemetry.md bumped to v1.03
with the new coded diagnostics disclosed.

## Dashboard migration note

`debugger-status` / `debugger-log-registry` precondition failures stop
emitting `tool:fail` — track them via `debugger:tool_outcome` (`outcome
!= 'connected'`). Per-AI-client breakdowns of those outcomes need a join
on `tool_invocation_id` against `tool:complete`.

## Testing

- Full suites green: tool-server 3,142, telemetry 290, registry 87. New
coverage: recoverable truth tables (incl. non-Error inputs), status
not-connected matrix
(Metro/Chromium/terminating-window/stale/dispose-side-effect/fall-through-rethrow),
log-registry rows, zod `invalid_params` derivation + emission-level
guard, flow gate, evaluate rewrap, sanitize-level enum checks.
- Live-tested against a booted iOS simulator + RN app and an Electron
target (details in PR comments).

Plan hardened through 3 adversarial review rounds (46 confirmed findings
addressed) before implementation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-08-10 11:51:45 +02:00

209 lines
7.5 KiB
TypeScript

import { describe, it, expect } from "vitest";
import { FAILURE_CODES, getFailureSignal } from "@argent/registry";
import { selectTarget } from "../../src/utils/debugger/target-selection";
import type { CDPTarget } from "../../src/utils/debugger/discovery";
function makeTarget(overrides: Partial<CDPTarget> = {}): CDPTarget {
return {
id: "page1",
title: "React Native",
description: "",
webSocketDebuggerUrl: "ws://localhost:8081/inspector/debug?device=0&page=1",
...overrides,
};
}
describe("selectTarget", () => {
it("prefers Fusebox target", () => {
const fusebox = makeTarget({
id: "fusebox",
reactNative: { capabilities: { prefersFuseboxFrontend: true } },
});
const legacy = makeTarget({ id: "legacy", title: "React Native Bridge (0)" });
const result = selectTarget([legacy, fusebox], 8081);
expect(result.target.id).toBe("fusebox");
expect(result.isNewDebugger).toBe(true);
});
it("falls back to C++ connection", () => {
const cpp = makeTarget({
id: "cpp",
description: "some [C++ connection]",
});
const legacy = makeTarget({ id: "legacy" });
const result = selectTarget([legacy, cpp], 8081);
expect(result.target.id).toBe("cpp");
expect(result.isNewDebugger).toBe(true);
});
it("falls back to React Native Bridge", () => {
const bridge = makeTarget({
id: "bridge",
title: "React Native Bridge (0)",
});
const other = makeTarget({ id: "other", title: "Other" });
const result = selectTarget([other, bridge], 8081);
expect(result.target.id).toBe("bridge");
expect(result.isNewDebugger).toBe(false);
});
it("falls back to first target", () => {
const t1 = makeTarget({ id: "first", title: "Unknown" });
const t2 = makeTarget({ id: "second", title: "Unknown 2" });
const result = selectTarget([t1, t2], 8081);
expect(result.target.id).toBe("first");
});
it("normalizes WebSocket URL to localhost with correct port", () => {
const target = makeTarget({
webSocketDebuggerUrl: "ws://10.0.2.2:9999/inspector/debug?device=0&page=1",
});
const result = selectTarget([target], 8081);
expect(result.webSocketUrl).toContain("localhost");
expect(result.webSocketUrl).toContain("8081");
expect(result.webSocketUrl).not.toContain("10.0.2.2");
expect(result.webSocketUrl).not.toContain("9999");
});
it("filters by deviceName when specified", () => {
const dev1 = makeTarget({ id: "dev1", deviceName: "iPhone 16" });
const dev2 = makeTarget({ id: "dev2", deviceName: "Pixel 9" });
const result = selectTarget([dev1, dev2], 8081, {
deviceName: "Pixel 9",
});
expect(result.target.id).toBe("dev2");
});
it("filters by deviceId when specified", () => {
const dev1 = makeTarget({
id: "dev1",
reactNative: { logicalDeviceId: "aaa" },
});
const dev2 = makeTarget({
id: "dev2",
reactNative: { logicalDeviceId: "bbb" },
});
const result = selectTarget([dev1, dev2], 8081, { deviceId: "bbb" });
expect(result.target.id).toBe("dev2");
});
it("routes distinct deviceIds to distinct devices (no collapse)", () => {
const dev1 = makeTarget({
id: "dev1",
reactNative: { logicalDeviceId: "aaa", capabilities: { prefersFuseboxFrontend: true } },
});
const dev2 = makeTarget({
id: "dev2",
reactNative: { logicalDeviceId: "bbb", capabilities: { prefersFuseboxFrontend: true } },
});
expect(selectTarget([dev1, dev2], 8081, { deviceId: "aaa" }).target.id).toBe("dev1");
expect(selectTarget([dev1, dev2], 8081, { deviceId: "bbb" }).target.id).toBe("dev2");
});
it("throws instead of silently collapsing when an unmatched deviceId is ambiguous", () => {
const dev1 = makeTarget({ id: "dev1", reactNative: { logicalDeviceId: "aaa" } });
const dev2 = makeTarget({ id: "dev2", reactNative: { logicalDeviceId: "bbb" } });
// An id that matches neither device must NOT resolve to the first target —
// that is the "all debuggers resolve to the same device" bug.
expect(() => selectTarget([dev1, dev2], 8081, { deviceId: "unmatched" })).toThrow(
/No debugger target matches device_id "unmatched"/
);
// The error must surface the valid ids so the caller can re-target.
expect(() => selectTarget([dev1, dev2], 8081, { deviceId: "unmatched" })).toThrow(/aaa.*bbb/);
});
it("the ambiguous-mismatch error carries the DEBUGGER_TARGET_DEVICE_MISMATCH signal", () => {
// The message is contract (asserted above and matched on by agents); the
// signal gives telemetry a dedicated code — instead of the catch-all
// service-initialization bucket — and lets debugger-status map the error
// to the "device_mismatch" reason.
const dev1 = makeTarget({ id: "dev1", reactNative: { logicalDeviceId: "aaa" } });
const dev2 = makeTarget({ id: "dev2", reactNative: { logicalDeviceId: "bbb" } });
let thrown: unknown;
try {
selectTarget([dev1, dev2], 8081, { deviceId: "unmatched" });
} catch (err) {
thrown = err;
}
expect(thrown).toBeInstanceOf(Error);
expect(getFailureSignal(thrown)).toMatchObject({
error_code: FAILURE_CODES.DEBUGGER_TARGET_DEVICE_MISMATCH,
failure_stage: "debugger_select_target",
failure_area: "tool_server",
error_kind: "not_found",
});
});
it("surfaces deviceName alongside logicalDeviceId so the caller can map ids to devices", () => {
const ipad = makeTarget({
id: "dev1",
deviceName: "iPad Pro",
reactNative: { logicalDeviceId: "9e7844f7" },
});
const iphone = makeTarget({
id: "dev2",
deviceName: "iPhone 16",
reactNative: { logicalDeviceId: "8a44101d" },
});
// Each listed device shows its human-readable name next to the hash.
expect(() => selectTarget([ipad, iphone], 8081, { deviceId: "some-udid" })).toThrow(
/iPad Pro \(9e7844f7\).*iPhone 16 \(8a44101d\)/
);
});
it("falls back to the bare logicalDeviceId when a device has no deviceName", () => {
const named = makeTarget({
id: "dev1",
deviceName: "iPhone 16",
reactNative: { logicalDeviceId: "aaa" },
});
const unnamed = makeTarget({ id: "dev2", reactNative: { logicalDeviceId: "bbb" } });
expect(() => selectTarget([named, unnamed], 8081, { deviceId: "unmatched" })).toThrow(
/iPhone 16 \(aaa\).*bbb/
);
});
it("still falls back to the only device when a single device is connected", () => {
// Single-device convenience: callers may pass an id Metro never echoes (e.g.
// an iOS simulator UDID, which is not the logicalDeviceId). With one device
// there is no ambiguity, so selection must succeed via fallback.
const fusebox = makeTarget({
id: "only",
reactNative: {
logicalDeviceId: "real-logical-id",
capabilities: { prefersFuseboxFrontend: true },
},
});
const uiPage = makeTarget({
id: "only-ui",
reactNative: { logicalDeviceId: "real-logical-id" },
});
const result = selectTarget([fusebox, uiPage], 8081, { deviceId: "some-ios-udid" });
expect(result.target.id).toBe("only");
});
it("falls back when no target exposes a logicalDeviceId at all", () => {
// Old-arch / pre-Fusebox targets omit reactNative.logicalDeviceId entirely.
const t1 = makeTarget({ id: "first" });
const t2 = makeTarget({
id: "second",
reactNative: { capabilities: { prefersFuseboxFrontend: true } },
});
const result = selectTarget([t1, t2], 8081, { deviceId: "anything" });
expect(result.target.id).toBe("second");
});
});