Skip to content

Commit b81fdf6

Browse files
committed
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.
1 parent e6cb5d9 commit b81fdf6

5 files changed

Lines changed: 93 additions & 22 deletions

File tree

‎src/perf/active-turn.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Process-wide open-turn id for nesting permission.wait / subagent outside the
3+
* reactor observer. Single-primary assumption: one run-sink observer owns the
4+
* slot. `clear()` and observer `reset()`/`closeTurn` null it.
5+
*/
6+
7+
let activeTurnId: string | null = null;
8+
9+
export function getActiveTurnId(): string | null {
10+
return activeTurnId;
11+
}
12+
13+
export function setActiveTurnId(id: string | null): void {
14+
activeTurnId = id;
15+
}
16+
17+
export function clearActiveTurnId(): void {
18+
activeTurnId = null;
19+
}

‎src/perf/index.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import { isOpaqueId, sanitizeTags, type PerfTags } from "./sanitize.js";
14+
import { clearActiveTurnId } from "./active-turn.js";
1415

1516
export {
1617
sanitizeTags,
@@ -244,4 +245,6 @@ export function clear(): void {
244245
ringWrite = 0;
245246
ringCount = 0;
246247
nextId = 0;
248+
clearActiveTurnId();
247249
}
250+

‎src/perf/permission-subagent-spans.test.ts‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type { ReactorEmittedEvent } from "@intx/inference";
66
import { createPermissionGate } from "../permission/gate.js";
77
import { createTaskTool } from "../subagent/task-tool.js";
88
import { clear, snapshot, type PerfSpan } from "./index.js";
9-
import { createPerfReactorObserver } from "./reactor-spans.js";
9+
import { createPerfReactorObserver, currentTurnId } from "./reactor-spans.js";
1010

1111
afterEach(() => {
1212
clear();
@@ -93,6 +93,36 @@ describe("permission.wait spans", () => {
9393
expect(waits[0]!.tags).toEqual({ tool_id: "write_file", decision: "allow" });
9494
});
9595

96+
test("closes permission.wait when requestApproval throws", async () => {
97+
const gate = createPermissionGate({
98+
approvals: [],
99+
interactive: true,
100+
skipPermissions: false,
101+
requestApproval: async () => {
102+
throw new Error("ui aborted");
103+
},
104+
});
105+
106+
await expect(gate.evaluate(shellCall("curl example.com"))).rejects.toThrow(
107+
"ui aborted",
108+
);
109+
110+
const waits = byName(completed(snapshot()), "permission.wait");
111+
expect(waits).toHaveLength(1);
112+
expect(waits[0]!.endNs).toBeDefined();
113+
expect(waits[0]!.tags?.tool_id).toBe("run_shell");
114+
// No decision tag when approval never returned.
115+
expect(waits[0]!.tags?.decision).toBeUndefined();
116+
});
117+
118+
test("clear() nulls process-wide currentTurnId", () => {
119+
const obs = createPerfReactorObserver();
120+
obs.observe(event("inference.start", { model: "m" }));
121+
expect(currentTurnId()).not.toBeNull();
122+
clear();
123+
expect(currentTurnId()).toBeNull();
124+
});
125+
96126
test("does not open a span when a grant auto-approves", async () => {
97127
let asked = 0;
98128
const gate = createPermissionGate({

‎src/perf/reactor-spans.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121

2222
import type { ReactorEmittedEvent } from "@intx/inference";
2323
import { end, start } from "./index.js";
24+
import {
25+
getActiveTurnId,
26+
setActiveTurnId,
27+
} from "./active-turn.js";
2428

2529
/** Content-bearing events that end TTFT and open the stream phase. */
2630
const FIRST_TOKEN_TYPES: ReadonlySet<string> = new Set([
@@ -49,17 +53,10 @@ export type PerfReactorObserver = {
4953
/**
5054
* Process-wide open-turn id from the most recently active reactor observer.
5155
* Permission-wait and subagent spans nest under this when present.
52-
* Tests that open turns should clear() PerfTrace and reset observers between cases.
56+
* Owned by `active-turn.ts`; cleared on observer close/reset and PerfTrace clear().
5357
*/
54-
let activeTurnId: string | null = null;
55-
56-
/** Current open turn span id (for nesting permission.wait / subagent outside the observer). */
5758
export function currentTurnId(): string | null {
58-
return activeTurnId;
59-
}
60-
61-
function setActiveTurnId(id: string | null): void {
62-
activeTurnId = id;
59+
return getActiveTurnId();
6360
}
6461

6562
type ObserverState = {
@@ -172,7 +169,7 @@ export function createPerfReactorObserver(): PerfReactorObserver {
172169
function closeTurn(): void {
173170
closeOpenTools();
174171
endIfOpen(state.turnId);
175-
if (state.turnId !== null && activeTurnId === state.turnId) {
172+
if (state.turnId !== null && getActiveTurnId() === state.turnId) {
176173
setActiveTurnId(null);
177174
}
178175
state.turnId = null;

‎src/permission/gate.ts‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -466,11 +466,20 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
466466
...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}),
467467
tags: { tool_id: request.tool },
468468
});
469-
const outcome = await requestApproval(requestForOperator);
470-
end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" });
471-
if (!outcome.allow) {
469+
let outcome: ApprovalOutcome | undefined;
470+
try {
471+
outcome = await requestApproval(requestForOperator);
472+
} finally {
473+
end(
474+
waitSpanId,
475+
outcome !== undefined
476+
? { decision: outcome.allow ? "allow" : "deny" }
477+
: undefined,
478+
);
479+
}
480+
if (outcome === undefined || !outcome.allow) {
472481
const suffix =
473-
outcome.message !== undefined && outcome.message.length > 0
482+
outcome?.message !== undefined && outcome.message.length > 0
474483
? ` — ${outcome.message}`
475484
: "";
476485
return {
@@ -509,13 +518,26 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
509518
...(turnId !== null && turnId.length > 0 ? { parentId: turnId } : {}),
510519
tags: { tool_id: request.tool },
511520
});
512-
const outcome = await requestApproval(request);
513-
end(waitSpanId, { decision: outcome.allow ? "allow" : "deny" });
514-
if (!outcome.allow) {
515-
const suffix = outcome.message !== undefined && outcome.message.length > 0
516-
? ` — ${outcome.message}`
517-
: "";
518-
return { allowed: false, reason: `Operator declined: ${request.action} (${request.subject})${suffix}` };
521+
let outcome: ApprovalOutcome | undefined;
522+
try {
523+
outcome = await requestApproval(request);
524+
} finally {
525+
end(
526+
waitSpanId,
527+
outcome !== undefined
528+
? { decision: outcome.allow ? "allow" : "deny" }
529+
: undefined,
530+
);
531+
}
532+
if (outcome === undefined || !outcome.allow) {
533+
const suffix =
534+
outcome?.message !== undefined && outcome.message.length > 0
535+
? ` — ${outcome.message}`
536+
: "";
537+
return {
538+
allowed: false,
539+
reason: `Operator declined: ${request.action} (${request.subject})${suffix}`,
540+
};
519541
}
520542
mintGrant(request.tool, outcome);
521543
}

0 commit comments

Comments
 (0)