From 7df893de9f71f426fef55bbaaa229ab644a850dc Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 04:01:09 -0700 Subject: [PATCH 1/6] Add permission.wait and subagent spans for attribution --- src/perf/index.test.ts | 8 + src/perf/index.ts | 1 + src/perf/permission-subagent-spans.test.ts | 248 +++++++++++++++++++++ src/perf/reactor-spans.ts | 39 +++- src/perf/sanitize.ts | 15 ++ src/permission/gate.ts | 14 ++ src/subagent/task-tool.ts | 14 +- 7 files changed, 337 insertions(+), 2 deletions(-) create mode 100644 src/perf/permission-subagent-spans.test.ts diff --git a/src/perf/index.test.ts b/src/perf/index.test.ts index d226e3a21..7a688f75c 100644 --- a/src/perf/index.test.ts +++ b/src/perf/index.test.ts @@ -243,6 +243,7 @@ describe("sanitizeTags", () => { provider_id: "openai", model_id: "gpt-5.4", transport: "ws", + decision: "allow", duration_ms: 12.5, bytes: 1024, payload_bytes: 2048, @@ -257,6 +258,7 @@ describe("sanitizeTags", () => { provider_id: "openai", model_id: "gpt-5.4", transport: "ws", + decision: "allow", duration_ms: 12.5, bytes: 1024, payload_bytes: 2048, @@ -269,6 +271,12 @@ describe("sanitizeTags", () => { }); }); + test("keeps decision allow/deny and strips free-text decisions", () => { + expect(sanitizeTags({ decision: "allow" })).toEqual({ decision: "allow" }); + expect(sanitizeTags({ decision: "deny" })).toEqual({ decision: "deny" }); + expect(sanitizeTags({ decision: "maybe" })).toBeUndefined(); + }); + test("strips free-text, paths, prompts, and unknown keys", () => { const tags = sanitizeTags({ prompt: "system: you are a helpful assistant", diff --git a/src/perf/index.ts b/src/perf/index.ts index 0bd5f62b1..9f0052963 100644 --- a/src/perf/index.ts +++ b/src/perf/index.ts @@ -18,6 +18,7 @@ export { OPAQUE_ID_RE, ALLOWED_TAG_KEYS, type AllowedTagKey, + type DecisionKind, type PerfTags, type TransportKind, } from "./sanitize.js"; diff --git a/src/perf/permission-subagent-spans.test.ts b/src/perf/permission-subagent-spans.test.ts new file mode 100644 index 000000000..3f597a8be --- /dev/null +++ b/src/perf/permission-subagent-spans.test.ts @@ -0,0 +1,248 @@ +/** + * CL-5170: permission.wait and subagent spans at the ask gate and task fleet. + */ +import { afterEach, describe, expect, test } from "bun:test"; +import type { ReactorEmittedEvent } from "@intx/inference"; +import { createPermissionGate } from "../permission/gate.js"; +import { createTaskTool } from "../subagent/task-tool.js"; +import { clear, snapshot, type PerfSpan } from "./index.js"; +import { createPerfReactorObserver } from "./reactor-spans.js"; + +afterEach(() => { + clear(); +}); + +function byName(spans: PerfSpan[], name: string): PerfSpan[] { + return spans.filter((s) => s.name === name); +} + +function completed(spans: PerfSpan[]): PerfSpan[] { + return spans.filter((s) => s.endNs !== undefined); +} + +function event(type: string, data: unknown = {}): ReactorEmittedEvent { + return { type, seq: 1, data } as ReactorEmittedEvent; +} + +const shellCall = (command: string) => + ({ id: "c1", name: "run_shell", arguments: { command } }) as const; + +const provider = { + providerName: "test-provider", + baseURL: "http://localhost", + model: "test-model", +}; + +const skipGate = createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, +}); + +describe("permission.wait spans", () => { + test("records allow decision when operator approves a shell ask", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: true }), + }); + + const verdict = await gate.evaluate(shellCall("curl example.com")); + expect(verdict.allowed).toBe(true); + + const waits = byName(completed(snapshot()), "permission.wait"); + expect(waits).toHaveLength(1); + expect(waits[0]!.tags).toEqual({ tool_id: "run_shell", decision: "allow" }); + }); + + test("records deny decision when operator declines", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: false, message: "nope" }), + }); + + const verdict = await gate.evaluate(shellCall("curl example.com")); + expect(verdict.allowed).toBe(false); + + const waits = byName(completed(snapshot()), "permission.wait"); + expect(waits).toHaveLength(1); + expect(waits[0]!.tags?.decision).toBe("deny"); + expect(waits[0]!.tags?.tool_id).toBe("run_shell"); + }); + + test("records path-arg tool ask as permission.wait", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: true }), + }); + + const verdict = await gate.evaluate({ + id: "c2", + name: "write_file", + arguments: { path: "src/a.ts", content: "x" }, + }); + expect(verdict.allowed).toBe(true); + + const waits = byName(completed(snapshot()), "permission.wait"); + expect(waits).toHaveLength(1); + expect(waits[0]!.tags).toEqual({ tool_id: "write_file", decision: "allow" }); + }); + + test("does not open a span when a grant auto-approves", async () => { + let asked = 0; + const gate = createPermissionGate({ + approvals: [{ tool: "run_shell", pattern: "npm *" }], + interactive: true, + skipPermissions: false, + requestApproval: async () => { + asked += 1; + return { allow: true }; + }, + }); + + const verdict = await gate.evaluate(shellCall("npm test")); + expect(verdict.allowed).toBe(true); + expect(asked).toBe(0); + expect(byName(snapshot(), "permission.wait")).toHaveLength(0); + }); + + test("nests under the open turn when a reactor turn is active", async () => { + const obs = createPerfReactorObserver(); + obs.observe(event("inference.start", { model: "m" })); + obs.observe( + event("inference.done", { + turn: { + role: "assistant", + content: [{ type: "tool_call", id: "t1", name: "run_shell", arguments: {} }], + model: "m", + timestamp: 0, + }, + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 }, + source: { provider: "p", model: "m" }, + }), + ); + const turnId = obs.currentTurnId(); + expect(turnId).not.toBeNull(); + + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: true }), + }); + await gate.evaluate(shellCall("curl x")); + + const wait = byName(completed(snapshot()), "permission.wait")[0]!; + expect(wait.parentId).toBe(turnId!); + + obs.reset(); + }); +}); + +describe("subagent spans", () => { + test("records a completed subagent span around run()", async () => { + let runEntered = false; + const tool = createTaskTool({ + permissionGate: skipGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.corbits", + provider, + run: async () => { + runEntered = true; + // Span must still be open while the child runs. + const open = snapshot().filter((s) => s.name === "subagent" && s.endNs === undefined); + expect(open).toHaveLength(1); + expect(open[0]!.tags?.subagent_id).toBe("call-sa-1"); + return "## Summary\n\nok\n"; + }, + }); + if (tool.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "call-sa-1", name: "task", arguments: { description: "Job", prompt: "Do it" } }, + new AbortController().signal, + ); + expect(runEntered).toBe(true); + expect(typeof result.content === "string" ? result.content : "").toContain("ok"); + + const agents = byName(completed(snapshot()), "subagent"); + expect(agents).toHaveLength(1); + expect(agents[0]!.tags?.subagent_id).toBe("call-sa-1"); + expect(agents[0]!.endNs).toBeDefined(); + expect(agents[0]!.endNs! >= agents[0]!.startNs).toBe(true); + }); + + test("nests under the open turn with turn_id tag for fanout rollup", async () => { + const obs = createPerfReactorObserver(); + obs.observe(event("inference.start", { model: "m" })); + obs.observe( + event("inference.done", { + turn: { + role: "assistant", + content: [{ type: "tool_call", id: "task-1", name: "task", arguments: {} }], + model: "m", + timestamp: 0, + }, + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 }, + source: { provider: "p", model: "m" }, + }), + ); + const turnId = obs.currentTurnId(); + expect(turnId).not.toBeNull(); + + const tool = createTaskTool({ + permissionGate: skipGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.corbits", + provider, + run: async () => "## Summary\n\nchild done\n", + }); + if (tool.kind !== "full") throw new Error("expected full tool"); + + await tool.handler( + { id: "call-child", name: "task", arguments: { description: "Child", prompt: "Work" } }, + new AbortController().signal, + ); + + const agent = byName(completed(snapshot()), "subagent")[0]!; + expect(agent.parentId).toBe(turnId!); + expect(agent.tags?.subagent_id).toBe("call-child"); + expect(agent.tags?.turn_id).toBe(turnId!); + + // Wall time under the child is attributable via parentId (fanout rollup). + const turn = byName(snapshot(), "turn").find((s) => s.id === turnId); + expect(turn).toBeDefined(); + expect(agent.startNs >= turn!.startNs).toBe(true); + + obs.reset(); + }); + + test("closes the span when run() rejects", async () => { + const tool = createTaskTool({ + permissionGate: skipGate, + cwd: "/repo", + getWorkdirBase: () => "/repo/.corbits", + provider, + run: async () => { + throw new Error("boom"); + }, + }); + if (tool.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { id: "call-fail", name: "task", arguments: { description: "Fail", prompt: "Work" } }, + new AbortController().signal, + ); + expect(typeof result.content === "string" ? result.content : "").toContain("Error:"); + + const agents = byName(completed(snapshot()), "subagent"); + expect(agents).toHaveLength(1); + expect(agents[0]!.tags?.subagent_id).toBe("call-fail"); + expect(agents[0]!.endNs).toBeDefined(); + }); +}); diff --git a/src/perf/reactor-spans.ts b/src/perf/reactor-spans.ts index fcefd0d3f..2e6e5d379 100644 --- a/src/perf/reactor-spans.ts +++ b/src/perf/reactor-spans.ts @@ -7,6 +7,16 @@ * inference.ttft (start → first content-bearing delta) * inference.stream (first delta → inference.done) * tool (per invocation) + * permission.wait (operator ask; diagnostic nested category — wall time + * overlaps tool; exclusive attribution already excludes + * nested categories, so double-count is intentional) + * subagent (task fleet child wall) + * + * Single-primary assumption: the session run-sink owns one + * `createPerfReactorObserver`. Process-wide `currentTurnId()` is published only + * by that primary so permission.wait / subagent can nest outside the observer. + * Do not create concurrent observers that also call ensureTurn — they would + * overwrite the slot. Tests call `clear()` (and observer `reset()`) between cases. */ import type { ReactorEmittedEvent } from "@intx/inference"; @@ -32,8 +42,26 @@ const FIRST_TOKEN_TYPES: ReadonlySet = new Set([ export type PerfReactorObserver = { observe(event: ReactorEmittedEvent): void; reset(): void; + /** Opaque PerfTrace id of the open turn span, or null when no turn is open. */ + currentTurnId(): string | null; }; +/** + * Process-wide open-turn id from the most recently active reactor observer. + * Permission-wait and subagent spans nest under this when present. + * Tests that open turns should clear() PerfTrace and reset observers between cases. + */ +let activeTurnId: string | null = null; + +/** Current open turn span id (for nesting permission.wait / subagent outside the observer). */ +export function currentTurnId(): string | null { + return activeTurnId; +} + +function setActiveTurnId(id: string | null): void { + activeTurnId = id; +} + type ObserverState = { turnId: string | null; inferenceId: string | null; @@ -139,10 +167,14 @@ export function createPerfReactorObserver(): PerfReactorObserver { /** * Single exit for ending a turn: close orphan tool spans, then the turn. * Inference tree must already be closed (or will be via abandonTurn). + * Clears the process-wide active turn when this observer owns it. */ function closeTurn(): void { closeOpenTools(); endIfOpen(state.turnId); + if (state.turnId !== null && activeTurnId === state.turnId) { + setActiveTurnId(null); + } state.turnId = null; state.pendingTools = 0; } @@ -156,6 +188,7 @@ export function createPerfReactorObserver(): PerfReactorObserver { function ensureTurn(): string { if (state.turnId === null) { state.turnId = start("turn"); + setActiveTurnId(state.turnId); } return state.turnId; } @@ -257,5 +290,9 @@ export function createPerfReactorObserver(): PerfReactorObserver { state = emptyState(); } - return { observe, reset }; + return { + observe, + reset, + currentTurnId: () => state.turnId, + }; } diff --git a/src/perf/sanitize.ts b/src/perf/sanitize.ts index 2a08d2ac8..79edf4d4f 100644 --- a/src/perf/sanitize.ts +++ b/src/perf/sanitize.ts @@ -8,11 +8,15 @@ export type TransportKind = "http_sse" | "ws"; +/** Permission-wait decision enum (allowlisted; never free text). */ +export type DecisionKind = "allow" | "deny"; + /** Tag keys that may appear on a span. Everything else is stripped. */ export const ALLOWED_TAG_KEYS = [ "provider_id", "model_id", "transport", + "decision", "duration_ms", "duration_ns", "bytes", @@ -31,6 +35,7 @@ export type PerfTags = Partial<{ provider_id: string; model_id: string; transport: TransportKind; + decision: DecisionKind; duration_ms: number; duration_ns: number; bytes: number; @@ -47,6 +52,8 @@ const ALLOWED_KEY_SET: ReadonlySet = new Set(ALLOWED_TAG_KEYS); const TRANSPORT_VALUES: ReadonlySet = new Set(["http_sse", "ws"]); +const DECISION_VALUES: ReadonlySet = new Set(["allow", "deny"]); + /** Numeric tag keys — only finite numbers are kept. */ const NUMERIC_KEYS: ReadonlySet = new Set([ "duration_ms", @@ -106,6 +113,14 @@ export function sanitizeTags(tags: Record | undefined | null): continue; } + if (allowedKey === "decision") { + if (typeof value === "string" && DECISION_VALUES.has(value)) { + out.decision = value as DecisionKind; + kept += 1; + } + continue; + } + if (NUMERIC_KEYS.has(allowedKey)) { if (isFiniteNumber(value)) { (out as Record)[allowedKey] = value; diff --git a/src/permission/gate.ts b/src/permission/gate.ts index f492cd37d..e43a619ec 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -25,6 +25,8 @@ import { type McpToolPermissionRegistry, } from "../mcp/tool-permissions.js"; import type { MCPClient } from "../mcp/client.js"; +import { end, start } from "../perf/index.js"; +import { currentTurnId } from "../perf/reactor-spans.js"; export type GateVerdict = { allowed: true } | { allowed: false; reason: string }; @@ -459,7 +461,13 @@ export function createPermissionGate(options: PermissionGateOptions): Permission // to mint a grant for one even if a persist scope somehow arrived. const isMegaChain = segments.length >= MEGA_CHAIN_SEGMENT_THRESHOLD; const requestForOperator = anySecret ? { ...request, scopes: [] } : request; + const turnId = currentTurnId(); + const waitSpanId = start("permission.wait", { + ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), + tags: { tool_id: request.tool }, + }); const outcome = await requestApproval(requestForOperator); + end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" }); if (!outcome.allow) { const suffix = outcome.message !== undefined && outcome.message.length > 0 @@ -496,7 +504,13 @@ export function createPermissionGate(options: PermissionGateOptions): Permission }; } + const turnId = currentTurnId(); + const waitSpanId = start("permission.wait", { + ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), + tags: { tool_id: request.tool }, + }); const outcome = await requestApproval(request); + end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" }); if (!outcome.allow) { const suffix = outcome.message !== undefined && outcome.message.length > 0 ? ` — ${outcome.message}` diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index a87b00e5a..16e660323 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -42,6 +42,8 @@ import { import { isSubAgentCancelError } from "./dispose.js"; import { cleanupSubAgentWorktree, createSubAgentWorktree, WorktreeError } from "./worktree.js"; import { generateSessionId } from "../session/index.js"; +import { end, start } from "../perf/index.js"; +import { currentTurnId } from "../perf/reactor-spans.js"; import { join } from "node:path"; import type { NestedDispatchDeps, @@ -591,7 +593,17 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { maxTurns: resolvedMaxTurns, ...(deps.deadlineMs !== undefined ? { deadlineMs: deps.deadlineMs } : {}), }; - const result = await run(params); + const turnId = currentTurnId(); + const subagentSpanId = start("subagent", { + ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), + tags: { + subagent_id: call.id, + ...(turnId !== null && turnId.length > 0 ? { turn_id: turnId } : {}), + }, + }); + const result = await run(params).finally(() => { + end(subagentSpanId); + }); // Operator cancel may race after run resolves. Keep strip status cancelled // when requested, but never discard a returned body (including salvage). const wasCancelled = From 72edd82f56de0a6df7c72f4bc14acbe5a6828df7 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 09:38:08 -0700 Subject: [PATCH 2/6] Close permission.wait on abort and clear active turn Always end the wait span in finally when requestApproval throws. Move the process-wide turn id into active-turn so clear() and turn close both null the nesting slot used by gate and task spans. --- src/perf/active-turn.ts | 19 ++++++++++ src/perf/index.ts | 3 ++ src/perf/permission-subagent-spans.test.ts | 32 +++++++++++++++- src/perf/reactor-spans.ts | 17 ++++----- src/permission/gate.ts | 44 ++++++++++++++++------ 5 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 src/perf/active-turn.ts diff --git a/src/perf/active-turn.ts b/src/perf/active-turn.ts new file mode 100644 index 000000000..e4c830587 --- /dev/null +++ b/src/perf/active-turn.ts @@ -0,0 +1,19 @@ +/** + * Process-wide open-turn id for nesting permission.wait / subagent outside the + * reactor observer. Single-primary assumption: one run-sink observer owns the + * slot. `clear()` and observer `reset()`/`closeTurn` null it. + */ + +let activeTurnId: string | null = null; + +export function getActiveTurnId(): string | null { + return activeTurnId; +} + +export function setActiveTurnId(id: string | null): void { + activeTurnId = id; +} + +export function clearActiveTurnId(): void { + activeTurnId = null; +} diff --git a/src/perf/index.ts b/src/perf/index.ts index 9f0052963..74159a5a7 100644 --- a/src/perf/index.ts +++ b/src/perf/index.ts @@ -11,6 +11,7 @@ */ import { isOpaqueId, sanitizeTags, type PerfTags } from "./sanitize.js"; +import { clearActiveTurnId } from "./active-turn.js"; export { sanitizeTags, @@ -244,4 +245,6 @@ export function clear(): void { ringWrite = 0; ringCount = 0; nextId = 0; + clearActiveTurnId(); } + diff --git a/src/perf/permission-subagent-spans.test.ts b/src/perf/permission-subagent-spans.test.ts index 3f597a8be..0d0682841 100644 --- a/src/perf/permission-subagent-spans.test.ts +++ b/src/perf/permission-subagent-spans.test.ts @@ -6,7 +6,7 @@ import type { ReactorEmittedEvent } from "@intx/inference"; import { createPermissionGate } from "../permission/gate.js"; import { createTaskTool } from "../subagent/task-tool.js"; import { clear, snapshot, type PerfSpan } from "./index.js"; -import { createPerfReactorObserver } from "./reactor-spans.js"; +import { createPerfReactorObserver, currentTurnId } from "./reactor-spans.js"; afterEach(() => { clear(); @@ -93,6 +93,36 @@ describe("permission.wait spans", () => { expect(waits[0]!.tags).toEqual({ tool_id: "write_file", decision: "allow" }); }); + test("closes permission.wait when requestApproval throws", async () => { + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => { + throw new Error("ui aborted"); + }, + }); + + await expect(gate.evaluate(shellCall("curl example.com"))).rejects.toThrow( + "ui aborted", + ); + + const waits = byName(completed(snapshot()), "permission.wait"); + expect(waits).toHaveLength(1); + expect(waits[0]!.endNs).toBeDefined(); + expect(waits[0]!.tags?.tool_id).toBe("run_shell"); + // No decision tag when approval never returned. + expect(waits[0]!.tags?.decision).toBeUndefined(); + }); + + test("clear() nulls process-wide currentTurnId", () => { + const obs = createPerfReactorObserver(); + obs.observe(event("inference.start", { model: "m" })); + expect(currentTurnId()).not.toBeNull(); + clear(); + expect(currentTurnId()).toBeNull(); + }); + test("does not open a span when a grant auto-approves", async () => { let asked = 0; const gate = createPermissionGate({ diff --git a/src/perf/reactor-spans.ts b/src/perf/reactor-spans.ts index 2e6e5d379..b15785bb5 100644 --- a/src/perf/reactor-spans.ts +++ b/src/perf/reactor-spans.ts @@ -21,6 +21,10 @@ import type { ReactorEmittedEvent } from "@intx/inference"; import { end, start } from "./index.js"; +import { + getActiveTurnId, + setActiveTurnId, +} from "./active-turn.js"; /** Content-bearing events that end TTFT and open the stream phase. */ const FIRST_TOKEN_TYPES: ReadonlySet = new Set([ @@ -49,17 +53,10 @@ export type PerfReactorObserver = { /** * Process-wide open-turn id from the most recently active reactor observer. * Permission-wait and subagent spans nest under this when present. - * Tests that open turns should clear() PerfTrace and reset observers between cases. + * Owned by `active-turn.ts`; cleared on observer close/reset and PerfTrace clear(). */ -let activeTurnId: string | null = null; - -/** Current open turn span id (for nesting permission.wait / subagent outside the observer). */ export function currentTurnId(): string | null { - return activeTurnId; -} - -function setActiveTurnId(id: string | null): void { - activeTurnId = id; + return getActiveTurnId(); } type ObserverState = { @@ -172,7 +169,7 @@ export function createPerfReactorObserver(): PerfReactorObserver { function closeTurn(): void { closeOpenTools(); endIfOpen(state.turnId); - if (state.turnId !== null && activeTurnId === state.turnId) { + if (state.turnId !== null && getActiveTurnId() === state.turnId) { setActiveTurnId(null); } state.turnId = null; diff --git a/src/permission/gate.ts b/src/permission/gate.ts index e43a619ec..3ea670f05 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -466,11 +466,20 @@ export function createPermissionGate(options: PermissionGateOptions): Permission ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), tags: { tool_id: request.tool }, }); - const outcome = await requestApproval(requestForOperator); - end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" }); - if (!outcome.allow) { + let outcome: ApprovalOutcome | undefined; + try { + outcome = await requestApproval(requestForOperator); + } finally { + end( + waitSpanId, + outcome !== undefined + ? { decision: outcome.allow ? "allow" : "deny" } + : undefined, + ); + } + if (outcome === undefined || !outcome.allow) { const suffix = - outcome.message !== undefined && outcome.message.length > 0 + outcome?.message !== undefined && outcome.message.length > 0 ? ` — ${outcome.message}` : ""; return { @@ -509,13 +518,26 @@ export function createPermissionGate(options: PermissionGateOptions): Permission ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), tags: { tool_id: request.tool }, }); - const outcome = await requestApproval(request); - end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" }); - if (!outcome.allow) { - const suffix = outcome.message !== undefined && outcome.message.length > 0 - ? ` — ${outcome.message}` - : ""; - return { allowed: false, reason: `Operator declined: ${request.action} (${request.subject})${suffix}` }; + let outcome: ApprovalOutcome | undefined; + try { + outcome = await requestApproval(request); + } finally { + end( + waitSpanId, + outcome !== undefined + ? { decision: outcome.allow ? "allow" : "deny" } + : undefined, + ); + } + if (outcome === undefined || !outcome.allow) { + const suffix = + outcome?.message !== undefined && outcome.message.length > 0 + ? ` — ${outcome.message}` + : ""; + return { + allowed: false, + reason: `Operator declined: ${request.action} (${request.subject})${suffix}`, + }; } mintGrant(request.tool, outcome); } From 802acb2abe0119c23bb9bbcb16b6acb3bc831cc1 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 09:45:24 -0700 Subject: [PATCH 3/6] Prove permission.wait tags never carry free-text reason/prompt Gate path with operator message free text must only land tool_id + allow/deny enums on the span; reason stays operator-facing only. --- src/perf/permission-subagent-spans.test.ts | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/perf/permission-subagent-spans.test.ts b/src/perf/permission-subagent-spans.test.ts index 0d0682841..98a0043ec 100644 --- a/src/perf/permission-subagent-spans.test.ts +++ b/src/perf/permission-subagent-spans.test.ts @@ -73,6 +73,43 @@ describe("permission.wait spans", () => { expect(waits[0]!.tags?.tool_id).toBe("run_shell"); }); + test("permission.wait tags never include free-text reason/prompt — only tool_id + decision", async () => { + // Gate path that would surface operator free text in the verdict reason, but + // must never land free-text keys on the span (sanitizeTags + gate only pass + // tool_id + allow/deny enums). + const freeText = + "please do not store this prompt: /Users/me/secret/key.pem and system: you are"; + const gate = createPermissionGate({ + approvals: [], + interactive: true, + skipPermissions: false, + requestApproval: async () => ({ allow: false, message: freeText }), + }); + + const verdict = await gate.evaluate(shellCall("curl example.com")); + expect(verdict.allowed).toBe(false); + // Free text reaches the operator-facing reason only (not span tags). + expect( + !verdict.allowed && "reason" in verdict ? verdict.reason : "", + ).toContain(freeText); + + const waits = byName(completed(snapshot()), "permission.wait"); + expect(waits).toHaveLength(1); + const tags = waits[0]!.tags ?? {}; + // Allowlist: only tool_id + decision enums on permission.wait. + expect(Object.keys(tags).sort()).toEqual(["decision", "tool_id"]); + expect(tags).toEqual({ tool_id: "run_shell", decision: "deny" }); + // Explicit privacy fence: no free-text keys, and free text never appears in values. + for (const key of ["reason", "prompt", "message", "path", "error", "action", "subject"] as const) { + expect(Object.hasOwn(tags, key)).toBe(false); + } + for (const value of Object.values(tags)) { + expect(String(value)).not.toContain(freeText); + expect(String(value)).not.toContain("secret"); + expect(String(value)).not.toContain("system:"); + } + }); + test("records path-arg tool ask as permission.wait", async () => { const gate = createPermissionGate({ approvals: [], From ac9f2de6a9895bda927f26896588e266a005e391 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 10:00:01 -0700 Subject: [PATCH 4/6] Document single-primary active-turn nesting limit Concurrent multi-observer setups overwrite the process-wide turn parent used by permission.wait and subagent spans. --- src/perf/active-turn.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/perf/active-turn.ts b/src/perf/active-turn.ts index e4c830587..5a631e494 100644 --- a/src/perf/active-turn.ts +++ b/src/perf/active-turn.ts @@ -1,7 +1,10 @@ /** * Process-wide open-turn id for nesting permission.wait / subagent outside the - * reactor observer. Single-primary assumption: one run-sink observer owns the - * slot. `clear()` and observer `reset()`/`closeTurn` null it. + * reactor observer. + * + * Single-primary assumption: one run-sink observer owns the slot. A second + * concurrent observer overwrites the parent used by gate/task spans — not + * supported. `clear()`, observer `reset()`, and `closeTurn` null the slot. */ let activeTurnId: string | null = null; From cd459a18c13dc2bf10728f5b04d811a6347c3c9d Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 15:31:00 -0700 Subject: [PATCH 5/6] Wrap subagent span around full child wall Start the exclusive subagent span before worktree setup and end it after teardown so attribution includes prepare and cleanup, including prepare failures that never reach run(). --- src/perf/permission-subagent-spans.test.ts | 34 +++ src/subagent/task-tool.ts | 253 +++++++++++---------- 2 files changed, 162 insertions(+), 125 deletions(-) diff --git a/src/perf/permission-subagent-spans.test.ts b/src/perf/permission-subagent-spans.test.ts index 98a0043ec..1bd888f36 100644 --- a/src/perf/permission-subagent-spans.test.ts +++ b/src/perf/permission-subagent-spans.test.ts @@ -312,4 +312,38 @@ describe("subagent spans", () => { expect(agents[0]!.tags?.subagent_id).toBe("call-fail"); expect(agents[0]!.endNs).toBeDefined(); }); + + test("opens and closes subagent span when worktree setup fails before run", async () => { + let runEntered = false; + const tool = createTaskTool({ + permissionGate: skipGate, + // Not a git repo — createSubAgentWorktree fails before run. + cwd: "/tmp/not-a-git-repo-for-subagent-span", + getWorkdirBase: () => "/tmp/not-a-git-repo-for-subagent-span/.corbits", + provider, + useWorktree: true, + run: async () => { + runEntered = true; + return "## Summary\n\nshould not run\n"; + }, + }); + if (tool.kind !== "full") throw new Error("expected full tool"); + + const result = await tool.handler( + { + id: "call-wt-fail", + name: "task", + arguments: { description: "Worktree fail", prompt: "Work" }, + }, + new AbortController().signal, + ); + expect(runEntered).toBe(false); + expect(typeof result.content === "string" ? result.content : "").toContain("Error:"); + + const agents = byName(completed(snapshot()), "subagent"); + expect(agents).toHaveLength(1); + expect(agents[0]!.tags?.subagent_id).toBe("call-wt-fail"); + expect(agents[0]!.endNs).toBeDefined(); + }); }); + diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index 16e660323..c927d66c3 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -526,144 +526,147 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { let worktreeCwd: string | undefined; let worktreeStashBaseline: readonly string[] | null = []; let worktreeHeadAtCreate: string | undefined; - if (deps.useWorktree === true) { - const worktreePath = join(deps.getWorkdirBase(), "worktrees", generateSessionId()); - try { - const worktree = await createSubAgentWorktree(deps.cwd, worktreePath); - worktreeCwd = worktree.path; - worktreeStashBaseline = worktree.stashBaseline; - worktreeHeadAtCreate = worktree.headAtCreate; - } catch (err) { - // Admit already happened and the strip session may be "running" — - // release the ledger slot and fail the session so a worktree setup - // error never burns turn-budget budget or leaves a ghost row. - const message = - err instanceof WorktreeError - ? err.message - : `sub-agent worktree setup failed: ${err instanceof Error ? err.message : String(err)}`; - briefLedger.release(fingerprint); - if (session !== undefined) deps.sessions?.fail(session.id, message); - signal.removeEventListener("abort", onParentAbort); - return taskToolResult(call.id, `Error: ${message}`); - } - } - // Cleanup runs once the sub-agent's report is ready, regardless of - // outcome, so a cancelled or failed run's worktree is still reclaimed - // (or preserved with a notice) rather than leaked. - const finishWithWorktree = async (result: ToolResult): Promise => { - if (worktreeCwd === undefined) return result; - const cleanup = await cleanupSubAgentWorktree(deps.cwd, worktreeCwd, { - stashBaseline: worktreeStashBaseline, - ...(worktreeHeadAtCreate !== undefined ? { headAtCreate: worktreeHeadAtCreate } : {}), - }); - if (cleanup.status === "preserved") { - return { ...result, content: `${result.content}\n\n${cleanup.notice}` }; - } - return result; - }; - + // Full child wall: worktree setup → run → teardown (CL-5170 exclusive share). + const turnId = currentTurnId(); + const subagentSpanId = start("subagent", { + ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), + tags: { + subagent_id: call.id, + ...(turnId !== null && turnId.length > 0 ? { turn_id: turnId } : {}), + }, + }); try { - const params: RunSubAgentParams = { - ...sandbox, - cwd: worktreeCwd ?? deps.cwd, - workdirBase: deps.getWorkdirBase(), - provider, - ...(tier !== undefined ? { tier } : {}), - ...(settings !== undefined ? { settings } : {}), - ...(catalog !== undefined ? { catalog } : {}), - description, - ...(context !== undefined && context.length > 0 ? { context } : {}), - prompt, - ...(goals.length > 0 ? { goals } : {}), - ...(intent !== undefined ? { intent } : {}), - ...(successCriteria.length > 0 ? { successCriteria } : {}), - ...(doNot.length > 0 ? { doNot } : {}), - ...(reportFocus !== undefined && reportFocus.length > 0 ? { reportFocus } : {}), - signal: childCtl.signal, - ...(recordEvent !== undefined ? { onEvent: recordEvent } : {}), - ...(deps.onProgress !== undefined ? { onProgress: deps.onProgress } : {}), - ...(capabilities !== undefined ? { capabilities } : {}), - ...(systemPromptRole !== undefined ? { systemPromptRole } : {}), - ...(orchestrator - ? { orchestrator: true, nestedDispatch: nestedDispatch! } - : {}), - // Nested workers (installed by an orchestrator that already holds a - // slot) reuse the parent slot rather than acquiring their own. - ...(deps.allowOrchestrator === false ? { nested: true } : {}), - maxTurns: resolvedMaxTurns, - ...(deps.deadlineMs !== undefined ? { deadlineMs: deps.deadlineMs } : {}), - }; - const turnId = currentTurnId(); - const subagentSpanId = start("subagent", { - ...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}), - tags: { - subagent_id: call.id, - ...(turnId !== null && turnId.length > 0 ? { turn_id: turnId } : {}), - }, - }); - const result = await run(params).finally(() => { - end(subagentSpanId); - }); - // Operator cancel may race after run resolves. Keep strip status cancelled - // when requested, but never discard a returned body (including salvage). - const wasCancelled = - childCtl.signal.aborted || - (session !== undefined && - deps.sessions?.get(session.id)?.status === "cancelled"); - const salvage = classifyBriefSalvage(result); - briefLedger.recordOutcome(fingerprint, salvage); - const hintOptions = { - dispatchCount, - turnBudgetStopAfterDispatches: TURN_BUDGET_STOP_AFTER_DISPATCHES, + if (deps.useWorktree === true) { + const worktreePath = join(deps.getWorkdirBase(), "worktrees", generateSessionId()); + try { + const worktree = await createSubAgentWorktree(deps.cwd, worktreePath); + worktreeCwd = worktree.path; + worktreeStashBaseline = worktree.stashBaseline; + worktreeHeadAtCreate = worktree.headAtCreate; + } catch (err) { + // Admit already happened and the strip session may be "running" — + // release the ledger slot and fail the session so a worktree setup + // error never burns turn-budget budget or leaves a ghost row. + const message = + err instanceof WorktreeError + ? err.message + : `sub-agent worktree setup failed: ${err instanceof Error ? err.message : String(err)}`; + briefLedger.release(fingerprint); + if (session !== undefined) deps.sessions?.fail(session.id, message); + signal.removeEventListener("abort", onParentAbort); + return taskToolResult(call.id, `Error: ${message}`); + } + } + // Cleanup runs once the sub-agent's report is ready, regardless of + // outcome, so a cancelled or failed run's worktree is still reclaimed + // (or preserved with a notice) rather than leaked. + const finishWithWorktree = async (result: ToolResult): Promise => { + if (worktreeCwd === undefined) return result; + const cleanup = await cleanupSubAgentWorktree(deps.cwd, worktreeCwd, { + stashBaseline: worktreeStashBaseline, + ...(worktreeHeadAtCreate !== undefined ? { headAtCreate: worktreeHeadAtCreate } : {}), + }); + if (cleanup.status === "preserved") { + return { ...result, content: `${result.content}\n\n${cleanup.notice}` }; + } + return result; }; - if (wasCancelled) { - if ( - session !== undefined && - deps.sessions?.get(session.id)?.status === "running" - ) { - deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); + + try { + const params: RunSubAgentParams = { + ...sandbox, + cwd: worktreeCwd ?? deps.cwd, + workdirBase: deps.getWorkdirBase(), + provider, + ...(tier !== undefined ? { tier } : {}), + ...(settings !== undefined ? { settings } : {}), + ...(catalog !== undefined ? { catalog } : {}), + description, + ...(context !== undefined && context.length > 0 ? { context } : {}), + prompt, + ...(goals.length > 0 ? { goals } : {}), + ...(intent !== undefined ? { intent } : {}), + ...(successCriteria.length > 0 ? { successCriteria } : {}), + ...(doNot.length > 0 ? { doNot } : {}), + ...(reportFocus !== undefined && reportFocus.length > 0 ? { reportFocus } : {}), + signal: childCtl.signal, + ...(recordEvent !== undefined ? { onEvent: recordEvent } : {}), + ...(deps.onProgress !== undefined ? { onProgress: deps.onProgress } : {}), + ...(capabilities !== undefined ? { capabilities } : {}), + ...(systemPromptRole !== undefined ? { systemPromptRole } : {}), + ...(orchestrator + ? { orchestrator: true, nestedDispatch: nestedDispatch! } + : {}), + // Nested workers (installed by an orchestrator that already holds a + // slot) reuse the parent slot rather than acquiring their own. + ...(deps.allowOrchestrator === false ? { nested: true } : {}), + maxTurns: resolvedMaxTurns, + ...(deps.deadlineMs !== undefined ? { deadlineMs: deps.deadlineMs } : {}), + }; + const result = await run(params); + // Operator cancel may race after run resolves. Keep strip status cancelled + // when requested, but never discard a returned body (including salvage). + const wasCancelled = + childCtl.signal.aborted || + (session !== undefined && + deps.sessions?.get(session.id)?.status === "cancelled"); + const salvage = classifyBriefSalvage(result); + briefLedger.recordOutcome(fingerprint, salvage); + const hintOptions = { + dispatchCount, + turnBudgetStopAfterDispatches: TURN_BUDGET_STOP_AFTER_DISPATCHES, + }; + if (wasCancelled) { + if ( + session !== undefined && + deps.sessions?.get(session.id)?.status === "running" + ) { + deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); + } + const reported = appendSubAgentParentHints(result, hintOptions); + return finishWithWorktree( + taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`), + ); } + if (session !== undefined) deps.sessions?.complete(session.id, result); const reported = appendSubAgentParentHints(result, hintOptions); return finishWithWorktree( taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`), ); - } - if (session !== undefined) deps.sessions?.complete(session.id, result); - const reported = appendSubAgentParentHints(result, hintOptions); - return finishWithWorktree( - taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`), - ); - } catch (err) { - if ( - isSubAgentCancelError(err, childCtl.signal) || - (session !== undefined && - deps.sessions?.get(session.id)?.status === "cancelled") - ) { - briefLedger.recordOutcome(fingerprint, "cancelled"); + } catch (err) { if ( - session !== undefined && - deps.sessions?.get(session.id)?.status === "running" + isSubAgentCancelError(err, childCtl.signal) || + (session !== undefined && + deps.sessions?.get(session.id)?.status === "cancelled") ) { - deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); + briefLedger.recordOutcome(fingerprint, "cancelled"); + if ( + session !== undefined && + deps.sessions?.get(session.id)?.status === "running" + ) { + deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); + } + return finishWithWorktree(taskToolResult(call.id, cancelledSubAgentMessage(description))); } - return finishWithWorktree(taskToolResult(call.id, cancelledSubAgentMessage(description))); + // Run never produced a body — undo the admit so turn-budget retry budget + // is not burned by auth/provider crashes. + briefLedger.release(fingerprint); + const authMessage = formatSubAgentTaskAuthFailureMessage(description, err); + const message = + authMessage !== null + ? `Error: ${authMessage}` + : `Error: sub-agent "${description}" failed: ${err instanceof Error ? err.message : String(err)}`; + const sessionError = err instanceof Error ? err.message : String(err); + // fail() prefixes "Error:" on the transcript report entry — pass bare text. + const failReason = authMessage ?? sessionError; + if (session !== undefined) deps.sessions?.fail(session.id, failReason); + return finishWithWorktree(taskToolResult(call.id, message)); + } finally { + signal.removeEventListener("abort", onParentAbort); } - // Run never produced a body — undo the admit so turn-budget retry budget - // is not burned by auth/provider crashes. - briefLedger.release(fingerprint); - const authMessage = formatSubAgentTaskAuthFailureMessage(description, err); - const message = - authMessage !== null - ? `Error: ${authMessage}` - : `Error: sub-agent "${description}" failed: ${err instanceof Error ? err.message : String(err)}`; - const sessionError = err instanceof Error ? err.message : String(err); - // fail() prefixes "Error:" on the transcript report entry — pass bare text. - const failReason = authMessage ?? sessionError; - if (session !== undefined) deps.sessions?.fail(session.id, failReason); - return finishWithWorktree(taskToolResult(call.id, message)); } finally { - signal.removeEventListener("abort", onParentAbort); + end(subagentSpanId); } }, }); From b01d246769927b0c5c839ef8cca6b05aa8326194 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Tue, 4 Aug 2026 15:34:08 -0700 Subject: [PATCH 6/6] Await worktree cleanup before ending subagent span Bare return of finishWithWorktree ran the outer finally before teardown settled. return await keeps cleanup inside the exclusive child wall. --- src/subagent/task-tool.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/subagent/task-tool.ts b/src/subagent/task-tool.ts index c927d66c3..234264da3 100644 --- a/src/subagent/task-tool.ts +++ b/src/subagent/task-tool.ts @@ -624,13 +624,13 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); } const reported = appendSubAgentParentHints(result, hintOptions); - return finishWithWorktree( + return await finishWithWorktree( taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`), ); } if (session !== undefined) deps.sessions?.complete(session.id, result); const reported = appendSubAgentParentHints(result, hintOptions); - return finishWithWorktree( + return await finishWithWorktree( taskToolResult(call.id, `Sub-agent "${description}" reported:\n\n${reported}`), ); @@ -647,7 +647,7 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { ) { deps.sessions.cancel(session.id, cancelReason(childCtl.signal)); } - return finishWithWorktree(taskToolResult(call.id, cancelledSubAgentMessage(description))); + return await finishWithWorktree(taskToolResult(call.id, cancelledSubAgentMessage(description))); } // Run never produced a body — undo the admit so turn-budget retry budget // is not burned by auth/provider crashes. @@ -661,10 +661,11 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool { // fail() prefixes "Error:" on the transcript report entry — pass bare text. const failReason = authMessage ?? sessionError; if (session !== undefined) deps.sessions?.fail(session.id, failReason); - return finishWithWorktree(taskToolResult(call.id, message)); + return await finishWithWorktree(taskToolResult(call.id, message)); } finally { signal.removeEventListener("abort", onParentAbort); } + } finally { end(subagentSpanId); }