mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
4975ff490c
## 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)
107 lines
4.2 KiB
TypeScript
107 lines
4.2 KiB
TypeScript
import { describe, it, expect, afterEach, vi } from "vitest";
|
|
import { Registry, ServiceState, FAILURE_CODES, FailureError } from "@argent/registry";
|
|
import {
|
|
jsRuntimeDebuggerBlueprint,
|
|
JS_RUNTIME_DEBUGGER_NAMESPACE,
|
|
type JsRuntimeDebuggerApi,
|
|
} from "../../src/blueprints/js-runtime-debugger";
|
|
import { debuggerEvaluateTool } from "../../src/tools/debugger/debugger-evaluate";
|
|
import { startMockMetroCdp, type MockMetroCdp } from "./metro-cdp-harness";
|
|
|
|
/**
|
|
* Reachability proof for the Metro blueprint's dispose-and-retry recovery
|
|
* (Registry._recoverFailedServices consults recoverable() only for a node
|
|
* still RUNNING).
|
|
*
|
|
* On this blueprint a DETECTED socket death tears the node down via the
|
|
* terminated cascade before the failing call's catch runs, so the only
|
|
* reachable window is the send() guard rejecting (NOT_CONNECTED) while the
|
|
* WebSocket is CLOSING but the close event has not dispatched — the request
|
|
* provably never left the host. The test forges that state by stubbing the
|
|
* cached client's send to reject with the classified NOT_CONNECTED error while
|
|
* the node provably stays RUNNING (no cascade fires), then asserts exactly one
|
|
* dispose-and-retry: the factory runs a second time and the call succeeds.
|
|
*/
|
|
|
|
const NOT_CONNECTED = () =>
|
|
new FailureError("CDP not connected", {
|
|
error_code: FAILURE_CODES.DEBUGGER_CDP_NOT_CONNECTED,
|
|
failure_stage: "debugger_cdp_send",
|
|
failure_area: "tool_server",
|
|
error_kind: "network",
|
|
});
|
|
|
|
const CONNECTION_CLOSED = () =>
|
|
new FailureError("CDP connection closed", {
|
|
error_code: FAILURE_CODES.DEBUGGER_CDP_CONNECTION_CLOSED,
|
|
failure_stage: "debugger_cdp_lifecycle",
|
|
failure_area: "tool_server",
|
|
error_kind: "network",
|
|
});
|
|
|
|
let metro: MockMetroCdp | null = null;
|
|
let registry: Registry | null = null;
|
|
|
|
afterEach(async () => {
|
|
await registry?.dispose();
|
|
await metro?.close();
|
|
registry = null;
|
|
metro = null;
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
async function setup() {
|
|
metro = await startMockMetroCdp();
|
|
const factorySpy = vi.fn(jsRuntimeDebuggerBlueprint.factory);
|
|
registry = new Registry();
|
|
registry.registerBlueprint({ ...jsRuntimeDebuggerBlueprint, factory: factorySpy });
|
|
registry.registerTool(debuggerEvaluateTool);
|
|
const urn = `${JS_RUNTIME_DEBUGGER_NAMESPACE}:${metro.port}:mock-device`;
|
|
const api = await registry.resolveService<JsRuntimeDebuggerApi>(urn);
|
|
expect(registry.getServiceState(urn)).toBe(ServiceState.RUNNING);
|
|
expect(factorySpy).toHaveBeenCalledTimes(1);
|
|
return { registry, metro, urn, api, factorySpy };
|
|
}
|
|
|
|
describe("registry recovery reachability (Metro debugger)", () => {
|
|
it("NOT_CONNECTED while RUNNING → exactly one dispose-and-retry, and the retry succeeds", async () => {
|
|
const { registry, metro, urn, api, factorySpy } = await setup();
|
|
|
|
// Forge the send-guard rejection on the CACHED client; the node stays
|
|
// RUNNING because no socket event (and hence no terminated cascade) fires.
|
|
vi.spyOn(api.cdp, "send").mockRejectedValue(NOT_CONNECTED());
|
|
|
|
const result = (await registry.invokeTool("debugger-evaluate", {
|
|
port: metro.port,
|
|
device_id: "mock-device",
|
|
expression: "1 + 1",
|
|
})) as { result: unknown };
|
|
|
|
// The retry ran against a FRESH instance (factory re-invoked exactly once
|
|
// more) whose unstubbed client got the mock server's real answer.
|
|
expect(result.result).toBe("mock");
|
|
expect(factorySpy).toHaveBeenCalledTimes(2);
|
|
expect(registry.getServiceState(urn)).toBe(ServiceState.RUNNING);
|
|
});
|
|
|
|
it("CONNECTION_CLOSED while RUNNING is NOT recovered — the call fails once, no retry", async () => {
|
|
// Double-execution guard: a request rejected by cleanup() was already
|
|
// delivered and may have taken effect, so the Metro blueprint must not
|
|
// dispose-and-retry on it even in the (test-forged) RUNNING state.
|
|
const { registry, metro, urn, api, factorySpy } = await setup();
|
|
|
|
vi.spyOn(api.cdp, "send").mockRejectedValue(CONNECTION_CLOSED());
|
|
|
|
await expect(
|
|
registry.invokeTool("debugger-evaluate", {
|
|
port: metro.port,
|
|
device_id: "mock-device",
|
|
expression: "1 + 1",
|
|
})
|
|
).rejects.toThrow(/CDP connection closed/);
|
|
|
|
expect(factorySpy).toHaveBeenCalledTimes(1);
|
|
expect(registry.getServiceState(urn)).toBe(ServiceState.RUNNING);
|
|
});
|
|
});
|