feat(agent): V8 inspector (CDP) tools for debugging/profiling workers (#626)

[kriszyp]

Summary

Adds the operator-only V8 inspector (CDP) tools from the #626 design — the last operator tool not yet built. The built-in agent runs on the main thread; these tools attach over the Chrome DevTools Protocol to worker thread inspector ports so it can evaluate expressions, set breakpoints/logpoints, and CPU-profile a worker without stalling the thread it runs on.

Stacked on #1545 (registry tools). Review/merge that first; this diff is just the inspector surface. Base branch: kris/agent-registry-tools.

Tools (agent/tools/inspectorTool.ts)

Tool	Purpose	Gate
inspector_attach	Attach + enable Runtime/Debugger on a worker	—
inspector_evaluate	Evaluate JS in a worker, return the value	destructive (arbitrary in-worker code)
inspector_set_breakpoint	Breakpoint by URL+line (0-based); a hit pauses the worker	destructive (can pause a live worker)
inspector_set_logpoint	Non-pausing logpoint (breakpoint whose condition console.logs and returns false)	—
inspector_profile_cpu	Record a CPU profile for durationMs, return hottest functions by self time	—

• CDP client: a minimal id-correlated request/response client over ws (already a direct dep), one reused connection per worker debug port, evicted on socket close.
• Port scheme: worker debug port = threads_debug_startingPort + workerIndex, mirroring server/threads/threadServer.js. Attaching requires threads_debug: true and a configured threads_debug_startingPort.
• Profile size: the raw CDP profile is megabytes; summarizeProfile reduces it to the top-N functions by self time so the LLM observation stays small.
• Deps injected (debug config + live worker count) so the tool is unit-testable without a server boot.

Safety envelope (resolvePort)

The security-relevant part. Every tool call must clear:

• threads_debug enabled and threads_debug_startingPort configured (else a clear error telling the operator what to set);
• workerIndex an integer, in range against the live worker pool;
• workerIndex >= 0 — the main thread is where the agent itself runs; a self-attach + breakpoint would deadlock it. This is the operator-agent counterpart to the app-developer agent (Add agent-loop orchestration / toolMode: 'auto' to scope.models #612), which runs on a worker and must never attach to itself.

evaluate and set_breakpoint are marked destructive, so they route through the loop's approval gate unless autoApprove is set.

Tests (unitTests/agent/inspectorTool.test.js)

• Safety envelope — debug-disabled, no starting port, main-thread rejection, out-of-range index, destructive flags. Pure, no network.
• summarizeProfile — self-time ranking + topN cap on a synthetic profile.
• Live CDP round-trip — opens a real inspector on an ephemeral port and drives attach + evaluate (40+2 → 42, exception → thrown) + profile_cpu against it. This process stands in for a worker (same protocol); it's never breakpointed (that would pause the test — the very reason main-thread attach is banned).

Full agent unit suite: 60 passing. npm run build clean.

Operator setup note (for docs)

To let the agent debug workers, operators set threads_debug: true and a sensible threads_debug_startingPort before asking the agent to attach.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces V8 inspector (CDP) tools for the built-in agent, enabling operators to attach to worker threads, evaluate expressions, set breakpoints/logpoints, and record CPU profiles. The review feedback highlights several critical security and reliability improvements: marking inspector_set_logpoint as destructive to prevent unauthorized arbitrary code execution, caching connection promises to avoid concurrent connection race conditions, fixing a memory leak in the sleep function's abort listener, and aligning the THREADS_DEBUG default check with the server's behavior.

[gemini-code-assist]

To prevent concurrent connection race conditions where multiple tool calls concurrently attempt to connect to the same port, store the Promise<CdpSession> in the sessions map instead of the resolved CdpSession.

Suggested change

	const sessions = new Map<number, CdpSession>();
	const sessions = new Map<number, Promise<CdpSession>>();

[gemini-code-assist]

Refactor sessionFor to use and cache the connection promise. This prevents duplicate WebSocket connections from being opened if multiple tool calls are made concurrently before the connection is established.

async function sessionFor(port: number, host: string, signal?: AbortSignal): Promise<CdpSession> {
	let sessionPromise = sessions.get(port);
	if (!sessionPromise) {
		sessionPromise = (async () => {
			try {
				const { wsUrl, title } = await fetchWebSocketUrl(host, port, signal);
				const session = await openCdp(wsUrl, () => sessions.delete(port));
				session.title = title;
				return session;
			} catch (err) {
				sessions.delete(port);
				throw err;
			}
		})();
		sessions.set(port, sessionPromise);
	}
	return sessionPromise;
}

[gemini-code-assist]

Update _closeInspectorSessions to handle the promise-based session cache synchronously by calling .then() on the cached promises.

Suggested change

	export function _closeInspectorSessions(): void {
	for (const session of sessions.values()) session.close();
	sessions.clear();
	}
	export function _closeInspectorSessions(): void {
	for (const sessionPromise of sessions.values()) {
	sessionPromise.then(
	(session) => session.close(),
	() => {}
	);
	}
	sessions.clear();
	}

[gemini-code-assist]

Security Vulnerability: inspector_set_logpoint allows arbitrary JavaScript code execution in the worker thread via the logExpression parameter (which is evaluated as part of the breakpoint condition). This can be used to bypass the approval gate for destructive actions (like inspector_evaluate). It must be marked as destructive: true to ensure it is routed through the operator approval gate.

			},
		},
		destructive: true, // arbitrary in-worker code execution via logExpression
		handler: async (args: any, ctx: AgentToolContext) => {

[gemini-code-assist]

The default value for THREADS_DEBUG is inconsistent with server/threads/threadServer.js. In threadServer.js, debugging is disabled by default if not set (falsy). However, using !== false here causes debugEnabled to default to true when THREADS_DEBUG is undefined. This should be updated to explicitly check for true or truthiness to match the server's behavior.

Suggested change

	const inspectorTools = buildInspectorTools({
	debugEnabled: env.get(CONFIG_PARAMS.THREADS_DEBUG) !== false,
	startingPort: (env.get(CONFIG_PARAMS.THREADS_DEBUG_STARTINGPORT) as number | undefined) ?? undefined,
	host: (env.get(CONFIG_PARAMS.THREADS_DEBUG_HOST) as string | undefined) ?? '127.0.0.1',
	getWorkerCount: () => workers.length,
	});
	const inspectorTools = buildInspectorTools({
	debugEnabled: env.get(CONFIG_PARAMS.THREADS_DEBUG) === true,
	startingPort: (env.get(CONFIG_PARAMS.THREADS_DEBUG_STARTINGPORT) as number | undefined) ?? undefined,
	host: (env.get(CONFIG_PARAMS.THREADS_DEBUG_HOST) as string | undefined) ?? '127.0.0.1',
	getWorkerCount: () => workers.length,
	});

[gemini-code-assist]

There is a memory leak in the sleep function. If the sleep timer fires and resolves normally, the abort event listener remains attached to the signal. For long-lived signals, this prevents the promise and its closures from being garbage collected. The event listener must be removed when the promise settles.

Suggested change

	function sleep(ms: number, signal?: AbortSignal): Promise<void> {
	return new Promise((resolve, reject) => {
	if (signal?.aborted) return reject(new Error('aborted'));
	const timer = setTimeout(resolve, ms);
	signal?.addEventListener(
	'abort',
	() => {
	clearTimeout(timer);
	reject(new Error('aborted'));
	},
	{ once: true }
	);
	});
	}
	function sleep(ms: number, signal?: AbortSignal): Promise<void> {
	return new Promise((resolve, reject) => {
	if (signal?.aborted) return reject(new Error('aborted'));
	let timer: NodeJS.Timeout;
	const onAbort = () => {
	clearTimeout(timer);
	reject(new Error('aborted'));
	};
	timer = setTimeout(() => {
	signal?.removeEventListener('abort', onAbort);
	resolve();
	}, ms);
	signal?.addEventListener('abort', onAbort, { once: true });
	});
	}

[gemini-code-assist]

Update the destructiveness assertions to verify that inspector_set_logpoint is correctly marked as destructive.

Suggested change

	it('marks evaluate and set_breakpoint destructive, others not', () => {
	const tools = toolMap(baseDeps);
	assert.equal(tools.get('inspector_evaluate').destructive, true);
	assert.equal(tools.get('inspector_set_breakpoint').destructive, true);
	assert.ok(!tools.get('inspector_attach').destructive);
	assert.ok(!tools.get('inspector_set_logpoint').destructive);
	assert.ok(!tools.get('inspector_profile_cpu').destructive);
	});
	it('marks evaluate, set_breakpoint, and set_logpoint destructive, others not', () => {
	const tools = toolMap(baseDeps);
	assert.equal(tools.get('inspector_evaluate').destructive, true);
	assert.equal(tools.get('inspector_set_breakpoint').destructive, true);
	assert.equal(tools.get('inspector_set_logpoint').destructive, true);
	assert.ok(!tools.get('inspector_attach').destructive);
	assert.ok(!tools.get('inspector_profile_cpu').destructive);
	});

[claude]

Reviewed; no blockers found.

[dawsontoth]

  1) agent/inspectorTool — live CDP round-trip
       "before all" hook for "attaches and evaluates an expression in the target":
     Error [ERR_INSPECTOR_ALREADY_ACTIVATED]: Inspector is already activated. Close it with inspector.close() before activating it again.
      at Object.inspectorOpen [as open] (node:inspector:174:11)
      at Context.<anonymous> (unitTests/agent/inspectorTool.test.js:97:13)
      at process.processImmediate (node:internal/timers:484:21)
      ```

[kriszyp]

Cross-model review (Gemini + Codex + Harper-domain) run before marking ready. Gemini surfaced real CDP lifecycle/concurrency hazards — all fixed in the latest commit (see fix(agent): harden inspector CDP client):

• Blockers: openCdp hang on pre-open close; session-cache eviction race + concurrent-open dupes; breakpoint could wedge a worker (now auto-resume + stack-snapshot log); logpoint condition-string injection (now JSON-encoded + eval'd, marked destructive).
• Significant: per-call abort signal + 30s timeout on CDP calls; strict workerIndex parsing (no ""/null→0 coercion); teardown handles in-flight opens.
• Suggestions: Profiler.disable on cleanup; port>65535 guard.

Codex leg returned no structured findings. 61 agent unit tests green (incl. a live CDP round-trip).

[claude]

Suggestion (non-blocking): use plain assert instead of node:assert/strict — AGENTS.md explicitly calls out that strict mode's deep-equality and coercion rules cause more friction and surprising failures than they prevent, and that plain assert is the house style.

Suggested change

	* enabled, a starting port configured, and workerIndex in range and NOT the
	const assert = require('node:assert');