Skip to content

Commit f5d8535

Browse files
committed
feat(runner): record every parked gate and iterate the warm resume
Complete the plural approval path in run-turn: record every parkable gate into env.parkedApprovals (keyed by tool-call id), iterate opts.resume.decisions to answer each parked gate by its own permissionId on the live session, defer the orphan-sibling settle to the post-drain sweep (so a concurrent gate is not force-settled before its own permission request arrives), and wire the non-parkable-pause signal. Update the acp-interactions unit test to assert both concurrent gates emit their own card. Part of the concurrent-approvals change (#5373); sits on the deny-frame egress lane whose markToolCallDenied the live-resume deny path reuses. Claude-Session: https://claude.ai/code/session_01DnWRxU3dCJ11hgDidm26vW
1 parent 9620965 commit f5d8535

2 files changed

Lines changed: 116 additions & 123 deletions

File tree

services/runner/src/engines/sandbox_agent/run-turn.ts

Lines changed: 97 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
import {
2-
PendingApprovalLatch,
3-
permissionsFromRequest,
4-
} from "../../permission-plan.ts";
1+
import { permissionsFromRequest } from "../../permission-plan.ts";
52
import {
63
resolvePromptText,
74
type AgentRunRequest,
@@ -75,7 +72,7 @@ import { resolveRunUsage } from "./usage.ts";
7572

7673
/**
7774
* Run one turn against an acquired environment: start a fresh otel run, wire this turn's pause
78-
* controller / latch / decisions / responder into `env.currentTurn`, restart the tool relay,
75+
* controller / decisions / responder into `env.currentTurn`, restart the tool relay,
7976
* send the prompt, resolve usage, and finish + flush the trace. It does NOT tear down the
8077
* environment (the caller owns `env.destroy`). On a continuation the prompt is only the new user
8178
* text (`buildTurnText` does not run); on a cold turn it is `plan.turnText`, exactly as before.
@@ -98,11 +95,13 @@ export async function runTurn(
9895
// Reset the per-turn tool-call id record (the park folds the completed turn's ids into the
9996
// expected next-history fingerprint).
10097
env.lastTurnToolCallIds = [];
101-
// Reset the per-turn approval-park bookkeeping. A fresh turn starts with no parked gate; this
102-
// turn re-records it only if it pauses on a Claude ACP permission gate. (The dispatch has
103-
// already captured any prior park into `opts.resume` before calling us.)
98+
// Reset the per-turn approval-park bookkeeping. A fresh turn starts with no parked gates; this
99+
// turn re-records them only if it pauses on ACP permission gates. (The dispatch has already
100+
// captured any prior park into `opts.resume` before calling us.)
101+
env.parkedApprovals.clear();
104102
env.parkedApproval = undefined;
105103
env.approvalGateCount = 0;
104+
env.nonParkablePauseCount = 0;
106105
// Hoisted so the catch can flush a partial trace (mirroring the pre-split `otel?` handling —
107106
// a createOtel throw must still return `{ ok: false }`, not propagate raw) and the finally can
108107
// stop this turn's relay on EVERY exit path (a cleared sink must never orphan it).
@@ -171,23 +170,22 @@ export async function runTurn(
171170
});
172171

173172
const pause = new PendingApprovalPauseController(() => {
174-
// The sibling settle runs UNCONDITIONALLY, park mode or not: latch-loser tool calls
175-
// announced before the winning gate can never execute this turn, and skipping the settle
176-
// here would leave them as orphaned open parts whenever the dispatch later refuses the park
177-
// (multi-gate, pool full) — `env.destroy()` does not re-run it. The exclusion keeps the
178-
// gated (paused) call itself open, so the live resume is untouched.
179-
run.settleOpenToolCalls(
180-
(id) => pause.isPausedToolCall(id),
181-
TOOL_NOT_EXECUTED_PAUSED,
182-
);
183-
// Park mode: a parkable permission gate (Claude ACP or Pi ACP) recorded
184-
// `env.parkedApproval` BEFORE firing this pause (the onUserApprovalGate hook runs before
185-
// the single-pause latch). Keep the live session — the gated tool runs on the resume — so
186-
// skip ONLY the mcpAbort and the destroySession. The teardown is not lost: the dispatch
187-
// either parks the session or, if it decides not to (multi-gate, pool full), calls
188-
// `env.destroy()` which runs them. A non-parkable pause (keep-alive off, client tool)
189-
// never records `parkedApproval`, so it still tears down here exactly as today.
190-
if (opts.approvalParkMode && env.parkedApproval) return;
173+
// Do NOT force-settle open tool calls here, at first pause. With concurrent approvals a
174+
// second gated call may still be in flight (its permission request lands a tick after the
175+
// first gate pauses the turn), and settling it here would orphan a call that is about to
176+
// emit its own approval card. The orphan settle is deferred to the post-drain sweep below
177+
// (which runs on every paused turn, after `waitForEventDrain` lets every pending gate mark
178+
// its call paused) plus the in-band re-sweep in `handleUpdate` for a sibling announced after
179+
// the pause. Both exclude paused (gated) calls, so every carded gate stays open for its
180+
// resume and only a genuine orphan (announced, never gated) settles.
181+
// Park mode: at least one parkable permission gate (Claude ACP or Pi ACP) recorded into
182+
// `env.parkedApprovals` BEFORE firing this pause (the onUserApprovalGate hook runs as each
183+
// gate resolves). Keep the live session — the gated tools run on the resume — so skip ONLY
184+
// the mcpAbort and the destroySession. The teardown is not lost: the dispatch either parks
185+
// the session or, if it decides not to (mixed non-parkable set, pool full), calls
186+
// `env.destroy()` which runs them. A pause with no parkable gate (keep-alive off, client
187+
// tool only) records nothing, so it still tears down here exactly as today.
188+
if (opts.approvalParkMode && env.parkedApprovals.size > 0) return;
191189
// Abort any in-flight loopback `tools/call` (a paused Claude client tool) BEFORE the
192190
// session teardown, so its handler cannot write a result after the turn ends.
193191
env.mcpAbort.abort();
@@ -238,7 +236,7 @@ export async function runTurn(
238236
env.lastTurnToolCallIds.push(frame.toolCallId);
239237
}
240238
run.handleUpdate(update);
241-
// A sibling announced AFTER the pause won the latch can never execute; settle it
239+
// A non-gated sibling announced AFTER the pause can never execute this turn; settle it
242240
// immediately so the client never holds an orphaned part (idempotent re-sweep).
243241
if (pause.active) {
244242
run.settleOpenToolCalls(
@@ -265,7 +263,6 @@ export async function runTurn(
265263
extractClientToolOutputs(request),
266264
);
267265
const executionGrants = new ApprovedExecutionGrants();
268-
const latch = new PendingApprovalLatch();
269266
const responder =
270267
deps.responderFactory?.(request) ??
271268
new ApprovalResponder(permissionPlan, decisions, logger);
@@ -324,11 +321,13 @@ export async function runTurn(
324321
},
325322
run,
326323
responder,
327-
latch,
328324
serverPermissions,
329325
log: logger,
330326
onPause: () => pause.pause(),
331327
onPausedToolCall: (id) => pause.markPausedToolCall(id),
328+
onNonParkablePause: () => {
329+
env.nonParkablePauseCount += 1;
330+
},
332331
onCreateInteraction: recordPendingInteraction,
333332
onResolveInteraction: resolveInteractionToken,
334333
toolSpecsByName: specsByName,
@@ -355,27 +354,28 @@ export async function runTurn(
355354
// only a dialog-approved (or policy-allowed) call ever executes from the relay dir.
356355
onPiGateAllowed: (info) =>
357356
executionGrants.grant(info.toolName, info.args),
358-
// Record the parkable permission gate (only in keep-alive park mode) so the dispatch can
359-
// resume it live. Fires per pending gate (before the latch) so a parallel gate is counted;
360-
// the single-gate resume records only the FIRST gate's answer target. `info.gateType` names
361-
// the plane (Claude ACP vs Pi ACP) so the resume answers on the right one.
357+
// Record EVERY parkable permission gate (only in keep-alive park mode) so the dispatch can
358+
// resume each one live. Fires per pending gate, so parallel gated tool calls in one turn
359+
// all park, each keyed by its own tool-call id. `info.gateType` names the plane (Claude ACP
360+
// vs Pi ACP) so the resume answers on the right one. `approvalGateCount` counts every gate;
361+
// a gate that lacked a resumable id is counted but not recorded, so the dispatch can tell
362+
// "every gate is resumable" (count === map size) from "a gate cannot be resumed live".
362363
onUserApprovalGate: opts.approvalParkMode
363364
? (info) => {
364365
env.approvalGateCount += 1;
365-
if (
366-
env.approvalGateCount === 1 &&
367-
info.permissionId &&
368-
info.toolCallId
369-
) {
370-
env.parkedApproval = {
371-
gateType: info.gateType,
372-
permissionId: info.permissionId,
373-
toolCallId: info.toolCallId,
374-
toolName: info.toolName,
375-
args: info.args,
376-
interactionToken: info.interactionToken,
377-
};
378-
}
366+
if (!info.permissionId || !info.toolCallId) return;
367+
const record: ParkedApproval = {
368+
gateType: info.gateType,
369+
permissionId: info.permissionId,
370+
toolCallId: info.toolCallId,
371+
toolName: info.toolName,
372+
args: info.args,
373+
interactionToken: info.interactionToken,
374+
};
375+
env.parkedApprovals.set(info.toolCallId, record);
376+
// The first recorded gate is the per-turn representative for logging and the
377+
// per-turn-uniform validation reads (gate type, history, credentials).
378+
env.parkedApproval ??= record;
379379
}
380380
: undefined,
381381
});
@@ -385,10 +385,12 @@ export async function runTurn(
385385
env.clientToolRelayRef.current = buildClientToolRelay({
386386
responder,
387387
run,
388-
latch,
389388
pause,
390389
recordPendingInteraction,
391390
toolCallIndex: plan.isPi ? undefined : env.toolCallIndex,
391+
onNonParkablePause: () => {
392+
env.nonParkablePauseCount += 1;
393+
},
392394
log: logger,
393395
});
394396

@@ -445,43 +447,49 @@ export async function runTurn(
445447
let promptPromise: Promise<unknown>;
446448
if (opts.resume) {
447449
// The new (resume) turn owns streaming + tracing; the environment is already wired to route
448-
// continued events into this turn's sink (env.currentTurn was set above). Seed this run's
449-
// trace with the parked tool call so the completing `tool_call_update` closes it and the FE
450-
// approval part flips to output-available even if the adapter re-announces nothing. Then
451-
// answer the gate on the live session — the original prompt continues from here.
452-
run.handleUpdate({
453-
sessionUpdate: "tool_call",
454-
toolCallId: opts.resume.toolCallId,
455-
title: opts.resume.toolName,
456-
kind: opts.resume.toolName,
457-
rawInput: opts.resume.args,
458-
});
459-
promptPromise = Promise.resolve(opts.resume.promptPromise);
450+
// continued events into this turn's sink (env.currentTurn was set above). Every parked gate
451+
// of the turn is answered here, one `respondPermission` per gate keyed by its tool-call id.
452+
// All decisions share the ONE held prompt promise (one prompt per turn), so it is set once;
453+
// answering the last gate lets the original prompt continue from here.
454+
const decisions = opts.resume.decisions;
455+
promptPromise = Promise.resolve(decisions[0]?.promptPromise);
460456
promptPromise.catch(() => {});
461-
// A parked Pi dialog gate resumes on a FRESH turn whose relay and grant ledger are new;
462-
// grant the approved call here so the extension's execute record (written right after the
463-
// confirm resolves) passes the relay guard. Claude resumes grant too — harmlessly, no
464-
// guard consults it.
465-
if (opts.resume.reply === "once") {
466-
executionGrants.grant(opts.resume.toolName, opts.resume.args);
467-
}
468-
// A live-resume deny closes the seeded call as a failed tool call; flag it so the egress
469-
// projects `tool-output-denied` (a decline), mirroring the cold decision-map deny path.
470-
if (opts.resume.reply === "reject") {
471-
run.markToolCallDenied(opts.resume.toolCallId);
457+
for (const decision of decisions) {
458+
// Seed this run's trace with the parked tool call so the completing `tool_call_update`
459+
// closes it and the FE approval part flips to output-available even if the adapter
460+
// re-announces nothing.
461+
run.handleUpdate({
462+
sessionUpdate: "tool_call",
463+
toolCallId: decision.toolCallId,
464+
title: decision.toolName,
465+
kind: decision.toolName,
466+
rawInput: decision.args,
467+
});
468+
// A parked Pi dialog gate resumes on a FRESH turn whose relay and grant ledger are new;
469+
// grant the approved call here so the extension's execute record (written right after the
470+
// confirm resolves) passes the relay guard. Claude resumes grant too — harmlessly, no
471+
// guard consults it.
472+
if (decision.reply === "once") {
473+
executionGrants.grant(decision.toolName, decision.args);
474+
}
475+
// A live-resume deny closes the seeded call as a failed tool call; flag it so the egress
476+
// projects `tool-output-denied` (a decline), mirroring the cold decision-map deny path.
477+
if (decision.reply === "reject") {
478+
run.markToolCallDenied(decision.toolCallId);
479+
}
480+
// Answer this gate on the live session. Each parked gate holds its OWN pending
481+
// `respondPermission` on the harness, so answering them one by one settles each
482+
// independently — an approve and a deny in the same turn each land on the right call.
483+
await env.session.respondPermission(decision.permissionId, decision.reply);
484+
// The gate is answered: resolve its durable interaction row (the parked pending row the
485+
// cold path would otherwise resolve via its decision map). The fresh per-turn pause
486+
// controller starts with an EMPTY pausedToolCallIds set, so the resumed calls'
487+
// `tool_call_update` frames are no longer suppressed and stream through.
488+
resolveInteractionToken(decision.interactionToken);
489+
logger(
490+
`[keepalive] resume answered gate reply=${decision.reply} tool=${decision.toolName ?? "?"}`,
491+
);
472492
}
473-
await env.session.respondPermission(
474-
opts.resume.permissionId,
475-
opts.resume.reply,
476-
);
477-
// The gate is answered: resolve the durable interaction row (the parked pending row the cold
478-
// path would otherwise resolve via its decision map). The fresh per-turn pause controller
479-
// starts with an EMPTY pausedToolCallIds set, so the resumed call's `tool_call_update` frames
480-
// are no longer suppressed and stream through — the "clear pausedToolCallIds on resume" step.
481-
resolveInteractionToken(opts.resume.interactionToken);
482-
logger(
483-
`[keepalive] resume answered gate reply=${opts.resume.reply} tool=${opts.resume.toolName ?? "?"}`,
484-
);
485493
} else {
486494
promptPromise = Promise.resolve(
487495
env.session.prompt([{ type: "text", text: turnText }]),
@@ -511,13 +519,13 @@ export async function runTurn(
511519
);
512520
}
513521
const result = raced === PAUSED ? undefined : raced;
514-
// A parkable pause this turn: hand the still-pending prompt promise to the parked record so a
515-
// later resume can await the same continuation. (Set after the race so `promptPromise` exists.
516-
// The read is asserted because the onUserApprovalGate callback set the field via an async
517-
// mutation TS's flow analysis cannot see, so it would otherwise narrow the reset to `never`.)
518-
const parkedThisTurn = env.parkedApproval as ParkedApproval | undefined;
519-
if (opts.approvalParkMode && pause.active && parkedThisTurn) {
520-
parkedThisTurn.promptPromise = promptPromise;
522+
// A parkable pause this turn: hand the still-pending prompt promise to EVERY parked record so a
523+
// later resume can await the same continuation (there is one prompt per turn, so every gate
524+
// shares it). Set after the race so `promptPromise` exists.
525+
if (opts.approvalParkMode && pause.active && env.parkedApprovals.size > 0) {
526+
for (const record of env.parkedApprovals.values()) {
527+
record.promptPromise = promptPromise;
528+
}
521529
}
522530
await turn.toolRelay?.stop();
523531
logger(`prompt stopReason=${stopReason}`);

0 commit comments

Comments
 (0)