Skip to content

Commit 9620965

Browse files
committed
feat(runner): allow multiple simultaneous approval requests in one turn
Remove the one-approval-per-turn latch (PendingApprovalLatch) so each gated tool call in a turn emits its own approval card; pluralize the parked-approval record (a Map keyed by tool-call id) and the warm keep-alive resume input (a list of decisions), settling each parked gate by its own tool-call id on a live resume; defer orphan-sibling settling to the post-drain sweep so a concurrent gate is not force-settled before its own permission request arrives. Add runner unit tests (two cards emit, neither force-settled; multi-gate warm park + resume; deny-one-approve-one; mixed/unresumable sets stay cold), SDK vercel adapter plural egress/ingress tests, and a playground all-settled resume test. Closes #5373. Claude-Session: https://claude.ai/code/session_01DnWRxU3dCJ11hgDidm26vW
1 parent 1acfe7c commit 9620965

15 files changed

Lines changed: 686 additions & 192 deletions

sdks/python/oss/tests/pytest/unit/agents/adapters/test_vercel_stream_park.py

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,87 @@ async def test_parked_run_emits_approval_then_finish() -> None:
8484
assert finish["finishReason"] == "other"
8585

8686

87+
def _two_concurrent_gates_run() -> AgentStream:
88+
"""One turn that raises TWO approval gates at once (concurrent approvals): two distinct tool
89+
calls, each followed by its own ``user_approval`` request, then the terminal ``paused`` result.
90+
The egress must surface one ``tool-approval-request`` per gate, each keyed to its own tool-call
91+
id, and still drain to a single clean finish."""
92+
return AgentStream(
93+
_records(
94+
[
95+
{
96+
"kind": "event",
97+
"event": {
98+
"type": "tool_call",
99+
"id": "tool-1",
100+
"name": "github__get_user",
101+
"input": {},
102+
},
103+
},
104+
{
105+
"kind": "event",
106+
"event": {
107+
"type": "tool_call",
108+
"id": "tool-2",
109+
"name": "github__list_repos",
110+
"input": {},
111+
},
112+
},
113+
{
114+
"kind": "event",
115+
"event": {
116+
"type": "interaction_request",
117+
"id": "perm-1",
118+
"kind": "user_approval",
119+
"payload": {"toolCallId": "tool-1"},
120+
},
121+
},
122+
{
123+
"kind": "event",
124+
"event": {
125+
"type": "interaction_request",
126+
"id": "perm-2",
127+
"kind": "user_approval",
128+
"payload": {"toolCallId": "tool-2"},
129+
},
130+
},
131+
{"kind": "event", "event": {"type": "done", "stopReason": "paused"}},
132+
{
133+
"kind": "result",
134+
"result": {
135+
"ok": True,
136+
"output": "",
137+
"stopReason": "paused",
138+
"sessionId": "conv-1",
139+
"traceId": "trace-1",
140+
},
141+
},
142+
]
143+
)
144+
)
145+
146+
147+
@pytest.mark.asyncio
148+
async def test_concurrent_gates_emit_one_approval_request_each() -> None:
149+
"""Two ``user_approval`` events in one turn yield TWO ``tool-approval-request`` frames — one
150+
per event — each carrying its own approvalId and tool-call id, so the FE can prompt for both
151+
gates independently. Pins the plural side of the single-gate contract above."""
152+
parts = [
153+
part async for part in agent_run_to_vercel_parts(_two_concurrent_gates_run())
154+
]
155+
156+
approvals = [p for p in parts if p["type"] == "tool-approval-request"]
157+
assert len(approvals) == 2
158+
# One frame per event, each keyed to its own gate (perm/tool pair), order preserved.
159+
assert [(a["approvalId"], a["toolCallId"]) for a in approvals] == [
160+
("perm-1", "tool-1"),
161+
("perm-2", "tool-2"),
162+
]
163+
# Both gates share the turn but the stream still drains to a single clean finish (F-040).
164+
assert parts[-1]["type"] == "finish"
165+
assert [p.get("type") for p in parts].count("finish") == 1
166+
167+
87168
def _parked_run_with_real_args() -> AgentStream:
88169
"""A parked turn where the runner first surfaced the tool call with EMPTY input, then the
89170
approval request carries the REAL args on ``payload.toolCall.rawInput`` (the cold-replay

sdks/python/oss/tests/pytest/unit/agents/test_ui_messages.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,47 @@ def test_inline_approval_responded_approve_emits_approved_envelope(self):
240240
assert result.tool_call_id == "call_2"
241241
assert result.output == {"approved": True}
242242

243+
def test_concurrent_inline_approvals_each_emit_their_own_result(self):
244+
# Concurrent approvals: the browser round-trips TWO tool parts in one turn, each with its
245+
# own inline decision (`state: approval-responded`, `approval.approved`) — one approve, one
246+
# deny. The ingress must emit one `{approved}` tool_result per part, keyed by its own
247+
# toolCallId, preserving each decision independently (neither gate clobbers the other).
248+
msgs = vercel_ui_messages_to_messages(
249+
[
250+
{
251+
"id": "m10",
252+
"role": "assistant",
253+
"parts": [
254+
{
255+
"type": "tool-deleteFile",
256+
"toolCallId": "call_1",
257+
"state": "approval-responded",
258+
"input": {"path": "/a"},
259+
"approval": {"id": "perm_1", "approved": True},
260+
},
261+
{
262+
"type": "tool-writeFile",
263+
"toolCallId": "call_2",
264+
"state": "approval-responded",
265+
"input": {"path": "/b"},
266+
"approval": {"id": "perm_2", "approved": False},
267+
},
268+
],
269+
}
270+
]
271+
)
272+
results = [b for b in msgs[0].content if b.type == "tool_result"]
273+
assert len(results) == 2
274+
# Each decision lands on its own toolCallId, in order, approve and deny preserved.
275+
assert (results[0].tool_call_id, results[0].output) == (
276+
"call_1",
277+
{"approved": True},
278+
)
279+
assert (results[1].tool_call_id, results[1].output) == (
280+
"call_2",
281+
{"approved": False},
282+
)
283+
243284
def test_inline_output_denied_state_emits_denied_envelope(self):
244285
# The AI SDK terminal deny state has no `approval.approved` flag; `output-denied`
245286
# itself means denied, so the ingress still emits `{approved: False}`.

services/runner/src/engines/sandbox_agent/acp-interactions.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
} from "../../responder.ts";
1212
import {
1313
piBuiltinIdentity,
14-
PendingApprovalLatch,
1514
type GateDescriptor,
1615
} from "../../permission-plan.ts";
1716
import {
@@ -44,7 +43,6 @@ export interface AttachPermissionResponderInput {
4443
markToolCallDenied?: (toolCallId: string | undefined) => void;
4544
};
4645
responder: Responder;
47-
latch: PendingApprovalLatch;
4846
serverPermissions?: ReadonlyMap<string, ToolPermission>;
4947
/**
5048
* Called when a gate pauses the turn. The orchestration loop uses this to end the turn
@@ -54,6 +52,13 @@ export interface AttachPermissionResponderInput {
5452
log?: (msg: string) => void;
5553
/** Called with the ACP tool-call id when a gate pauses the turn. */
5654
onPausedToolCall?: (id: string) => void;
55+
/**
56+
* Called when a NON-parkable pause happens this turn (a client-tool ACP gate, which cannot be
57+
* answered across a turn boundary on the live session). The keep-alive dispatch reads this to
58+
* keep a turn that mixes an approval gate with a client-tool pause on the cold path, since only
59+
* the cold path can multiplex that mixed set. An approval gate does NOT fire it.
60+
*/
61+
onNonParkablePause?: () => void;
5762
/** Called on pause to record the pending gate as an interaction (fire-and-forget). */
5863
onCreateInteraction?: (
5964
token: string,
@@ -65,10 +70,10 @@ export interface AttachPermissionResponderInput {
6570
onResolveInteraction?: (token: string) => void;
6671
/**
6772
* Fires for EVERY parkable permission gate (a Claude ACP gate or a Pi ACP gate) that
68-
* resolves to pendingApproval, BEFORE the single-pause latch. Keep-alive uses it to record
69-
* the parked permission id / tool-call id (for a live resume via `respondPermission`) and to
70-
* count how many gates are pending this turn (a multi-gate pause does not park). It never
71-
* fires for a client-tool gate (`pauseClientTool`), which stays on the cold path.
73+
* resolves to pendingApproval. Keep-alive uses it to record every parked permission id /
74+
* tool-call id (for a live resume via `respondPermission`, keyed by tool-call id) and to count
75+
* how many gates are pending this turn. It never fires for a client-tool gate
76+
* (`pauseClientTool`), which fires `onNonParkablePause` instead and stays on the cold path.
7277
*/
7378
onUserApprovalGate?: (info: {
7479
permissionId: string;
@@ -109,11 +114,11 @@ export function attachPermissionResponder({
109114
session,
110115
run,
111116
responder,
112-
latch,
113117
serverPermissions = new Map(),
114118
onPause,
115119
log,
116120
onPausedToolCall,
121+
onNonParkablePause,
117122
onCreateInteraction,
118123
onResolveInteraction,
119124
onUserApprovalGate,
@@ -155,19 +160,18 @@ export function attachPermissionResponder({
155160
gate: GateDescriptor,
156161
gateType: ParkedApprovalGateType,
157162
): void => {
158-
// Signal the parkable gate BEFORE the latch so a keep-alive resume can record the pending
159-
// permission id and the multi-gate detector counts every pending gate (not just the first).
160-
const gateToolCallId = stringValue(req?.toolCall?.toolCallId);
163+
// Signal the parkable gate so a keep-alive resume can record the pending permission id and
164+
// count every pending gate. Each gate emits its own card: there is no per-turn cap, so N
165+
// gated calls in one turn all render and all park (the plural approval path).
166+
const toolCallId = stringValue(req?.toolCall?.toolCallId);
161167
onUserApprovalGate?.({
162168
permissionId: id,
163-
toolCallId: gateToolCallId ?? "",
169+
toolCallId: toolCallId ?? "",
164170
toolName: gate.toolName,
165171
args: gate.args,
166-
interactionToken: interactionEventId(id, gateToolCallId),
172+
interactionToken: interactionEventId(id, toolCallId),
167173
gateType,
168174
});
169-
if (!latch.tryAcquire()) return;
170-
const toolCallId = stringValue(req?.toolCall?.toolCallId);
171175
const eventId = interactionEventId(id, toolCallId);
172176
if (toolCallId) onPausedToolCall?.(toolCallId);
173177
run.emitEvent({
@@ -192,7 +196,9 @@ export function attachPermissionResponder({
192196
gate: GateDescriptor,
193197
spec: ResolvedToolSpec,
194198
): void => {
195-
if (!latch.tryAcquire()) return;
199+
// A client-tool ACP pause cannot be answered on the live session across a turn boundary, so
200+
// flag the turn non-parkable: a turn that mixes this with an approval gate stays cold.
201+
onNonParkablePause?.();
196202
const toolCallId = stringValue(req?.toolCall?.toolCallId);
197203
const eventId = interactionEventId(id, toolCallId);
198204
if (toolCallId) onPausedToolCall?.(toolCallId);

services/runner/src/engines/sandbox_agent/client-tools.ts

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* `tools/call` handler (`tools/tool-mcp-http.ts`).
1212
*
1313
* Both consume the `ClientToolRelay` built here, so the pause decision (the responder's verdict
14-
* ladder), the single `client_tool` payload shape, the pause-latch bookkeeping, and the
14+
* ladder), the single `client_tool` payload shape, the pause bookkeeping, and the
1515
* ACP-tool-call correlation live in ONE place instead of being re-derived per path.
1616
*/
1717
import type { AgentEvent, RenderHint } from "../../protocol.ts";
@@ -200,11 +200,6 @@ export function emitClientToolInteraction(
200200
});
201201
}
202202

203-
/** The one-pause-per-turn latch surface the seam needs (see `PendingApprovalLatch`). */
204-
interface LatchLike {
205-
tryAcquire(): boolean;
206-
}
207-
208203
/** The pause-controller surface the seam needs (see `PendingApprovalPauseController`). */
209204
interface PauseLike {
210205
markPausedToolCall(toolCallId: string): void;
@@ -214,12 +209,14 @@ interface PauseLike {
214209
export interface BuildClientToolRelayInput {
215210
responder: Responder;
216211
run: EmitRun;
217-
/** The approval-dialog latch. Client tools do NOT gate on it — each pending client tool parks
218-
* its own widget, and only the approval path (`acp-interactions.ts`) enforces one dialog per
219-
* turn. Kept in the shared input so the wiring stays symmetric; ignored here. */
220-
latch?: LatchLike;
221212
/** The turn-ender: `pause()` cancels the prompt; `markPausedToolCall` suppresses late frames. */
222213
pause: PauseLike;
214+
/**
215+
* Flag the turn non-parkable when a browser-fulfilled client tool pauses: it cannot be answered
216+
* on the live session across a turn boundary, so a turn that mixes it with an approval gate must
217+
* stay on the cold path. Fires once per pending client tool (idempotent counter on the caller).
218+
*/
219+
onNonParkablePause?: () => void;
223220
/** Seeds the durable interactions plane for the pending call (fire-and-forget). */
224221
recordPendingInteraction: (
225222
token: string,
@@ -247,6 +244,7 @@ export function buildClientToolRelay({
247244
pause,
248245
recordPendingInteraction,
249246
toolCallIndex,
247+
onNonParkablePause,
250248
log = () => {},
251249
}: BuildClientToolRelayInput): ClientToolRelay {
252250
return {
@@ -281,12 +279,15 @@ export function buildClientToolRelay({
281279
const correlatedId =
282280
toolCallIndex?.lookup(request.toolName, request.input) ??
283281
request.toolCallId;
284-
// Every pending client tool parks its OWN widget. Unlike an approval DIALOG (one at a time,
285-
// gated by the latch), browser-fulfilled client tools are independent surfaces — an agent may
286-
// request several connections in one turn, and each must render. The turn still ends exactly
287-
// once: `pause.pause()` (via `onPause`) is idempotent, so N emits pause the turn once, and
288-
// `markPausedToolCall` keeps each parked call from being force-settled as a deferred sibling.
282+
// Every pending client tool parks its OWN widget: browser-fulfilled client tools are
283+
// independent surfaces — an agent may request several connections in one turn, and each must
284+
// render. Approval gates now behave the same way (no per-turn cap). The turn still ends
285+
// exactly once: `pause.pause()` (via `onPause`) is idempotent, so N emits pause the turn
286+
// once, and `markPausedToolCall` keeps each parked call from being force-settled as a sibling.
289287
pause.markPausedToolCall(correlatedId);
288+
// A client-tool pause keeps the whole turn on the cold path: the warm resume cannot answer
289+
// a browser-fulfilled call, so a turn that also holds an approval gate must not park.
290+
onNonParkablePause?.();
290291
emitClientToolInteraction(run, {
291292
id: request.id,
292293
toolCallId: correlatedId,

services/runner/src/engines/sandbox_agent/environment-setup.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,8 +347,10 @@ export async function prepareEnvironmentSetup(
347347
closeToolMcp: undefined,
348348
currentTurn: undefined,
349349
lastTurnToolCallIds: [],
350+
parkedApprovals: new Map(),
350351
parkedApproval: undefined,
351352
approvalGateCount: 0,
353+
nonParkablePauseCount: 0,
352354
destroyed: false,
353355
destroy: async () => {},
354356
clearTurn: () => {},

services/runner/src/engines/sandbox_agent/runtime-contracts.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,14 @@ export interface RunTurnOptions {
156156
* keep-alive turn.
157157
*/
158158
approvalParkMode?: boolean;
159-
/** A live approval resume: answer the parked gate and stream the continued prompt's events. */
160-
resume?: ResumeApprovalInput;
159+
/**
160+
* A live approval resume: answer every parked gate of the turn (keyed by tool-call id) and
161+
* stream the continued prompt's events. A turn can hold more than one parked gate, so the
162+
* resume carries a LIST — the server builds it from the inbound approval replies, one decision
163+
* per parked gate. All decisions share the one held prompt promise (there is one prompt per
164+
* turn); `runTurn` answers each gate then awaits that single continuation.
165+
*/
166+
resume?: { decisions: ResumeApprovalInput[] };
161167
}
162168

163169
/**
@@ -222,17 +228,31 @@ export interface SessionEnvironment {
222228
*/
223229
lastTurnToolCallIds: string[];
224230
/**
225-
* The Claude ACP permission gate the LAST turn paused on, or undefined. Set only for a harness
226-
* ACP permission gate, reset at each turn start; the dispatch reads it after a paused turn to
227-
* decide whether to park in `awaiting_approval` and, on the next request, how to resume.
231+
* Every parkable ACP permission gate the LAST turn paused on, keyed by the gated tool-call id
232+
* (reset at each turn start). This is the source of truth the warm resume iterates: a turn can
233+
* hold more than one gate (parallel gated tool calls), and each is answered by its own
234+
* `permissionId` on the live session. Empty when no parkable gate paused the turn.
235+
*/
236+
parkedApprovals: Map<string, ParkedApproval>;
237+
/**
238+
* The FIRST parked gate this turn, a convenience for per-turn-uniform reads (logging, the
239+
* gate-type check, the shared history/credential validation). Undefined when the map is empty.
240+
* The multi-answer resume and the all-parkable park check read `parkedApprovals`, not this.
228241
*/
229242
parkedApproval?: ParkedApproval;
230243
/**
231-
* How many Claude ACP permission gates resolved to pendingApproval THIS turn (reset at turn
232-
* start). More than one means parallel gates the single-gate resume cannot answer, so the
233-
* dispatch does not park (tears down cold as today).
244+
* How many ACP permission gates resolved to pendingApproval THIS turn (reset at turn start).
245+
* Equals `parkedApprovals.size` when every gate carried a resumable tool-call id; a larger
246+
* count means a gate lacked an id and cannot be resumed live, so the dispatch stays cold.
234247
*/
235248
approvalGateCount: number;
249+
/**
250+
* How many NON-parkable pauses happened this turn (a client-tool ACP gate or a browser-fulfilled
251+
* relay/MCP client tool), reset at turn start. Non-zero means the turn mixes an unanswerable
252+
* client-tool pause into the set, so the whole turn stays on the cold path (only cold can
253+
* multiplex a mixed set today).
254+
*/
255+
nonParkablePauseCount: number;
236256
destroyed: boolean;
237257
/** Complete, idempotent teardown selected from the typed teardown reason. */
238258
destroy: (opts?: { reason?: TeardownReason }) => Promise<void>;

services/runner/src/engines/sandbox_agent/runtime-policy.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,24 @@ export function shouldSuppressPausedToolCallUpdate(
3939
if (kind !== "tool_call" && kind !== "tool_call_update") return false;
4040
const toolCallId =
4141
typeof frame?.toolCallId === "string" ? frame.toolCallId : undefined;
42-
return pause.isPausedToolCall(toolCallId);
42+
// F-024: a paused (gated) tool call's later frames are teardown artifacts and never reach the
43+
// stream.
44+
if (pause.isPausedToolCall(toolCallId)) return true;
45+
// Once the turn is pausing, a FAILED update for any other open call is a managed-cancel
46+
// artifact (a sibling that will not run this turn), so suppress it and let the deterministic
47+
// post-drain sweep settle that call as TOOL_NOT_EXECUTED_PAUSED — the human sees "paused", not a
48+
// spurious cancellation error. A `completed` update is NOT suppressed: an auto-allowed sibling
49+
// that legitimately finishes while another gate holds the (kept-alive) warm session must show
50+
// its real result, not be overwritten as paused. Announcements (`tool_call`) are never
51+
// suppressed: the call must be recorded so the sweep knows which calls to settle. (Removing the
52+
// eager first-pause settle is what lets concurrent gates each mark their own call paused before
53+
// the sweep runs; this narrow suppression only masks the cancel artifact.) Residual: a genuine
54+
// mid-pause tool error also arrives as `failed` and reads as paused here — acceptable, since the
55+
// paused result invites a retry and the runner has no adapter provenance to tell the two apart.
56+
if (kind === "tool_call_update" && pause.active && frame?.status === "failed") {
57+
return true;
58+
}
59+
return false;
4360
}
4461

4562
const CLAUDE_STRICT_DEPLOYMENTS = new Set([

0 commit comments

Comments
 (0)