Skip to content

Commit 78196c5

Browse files
committed
fix(tui): auto-abandon resume overlay on reactor approval timeout
1 parent 445886d commit 78196c5

4 files changed

Lines changed: 160 additions & 4 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ tool call
408408
- **auto-shell-policy** — Constrains `run_shell` even when auto mode would otherwise rubber-stamp it. Before matching, `expandShellSubjects` peels `bash`/`sh`/`zsh -c`, `xargs` utility tails, and transparent prefixes (`env`, `nice`, `timeout`, …) so rules see the real payload; an unparseable wrapper (variable expansion or command substitution) sets an opaque flag that forces `ask`. Effects: `deny` blocks outright (file mutations through ad-hoc tooling — output redirection, `tee`, `sed -i`/`perl -i`, interpreter inline programs or heredocs — which must instead go through `write_file`/`edit_file`); `ask` declines to auto-allow and falls through to the operator prompt (recursive `rm`, dependency installs and remote runners: npm/yarn/pnpm/bun, pip, cargo, go, brew, npx/bunx, …, force or uncontained `git worktree` ops, shell that references a sensitive path such as `.env` or a private key, and opaque wrappers). Contained non-force `git worktree add`/`remove`/`prune` and read-only `list` auto-allow (sibling destinations like `../corbits-dispatch-wts/…` included; absolute outside, `~`, globs, and credential basenames still ask). Deny beats ask when multiple subjects match. Quoted spans are stripped before pattern matching so a quoted `>` or install word in an argument is not flagged, and program names are matched only in command position. Adding a table category is a one-line rule append in `AUTO_SHELL_RULES`.
409409
- **gate** — Evaluates a call: `skipPermissions` allows everything; `allow`-tier passes; for `ask`-tier, checks persisted approvals, otherwise requests operator approval. Shell security classifies each chain segment (`||` / `&&` / `|` / `;` / newlines), but the operator is prompted once for the full command block — any unapproved segment fails the whole block, and execution always runs the unsplit original. Safe pipeline tails and pure shell no-ops (`true` / `false` / `:` and bare control-flow keywords stranded by chain-splitting) skip without a prompt. In a non-interactive run an unresolved `ask` becomes a denial. In auto mode: non-shell built-ins in `AUTO_ALLOWED_TOOLS` (writes/edits/deletes, `manage_tasks`, `spawn_agent`, `wait_agents`, …) auto-allow when not path-restricted; for `run_shell` the gate consults the auto-shell policy — a `deny` rule fails the call, an `ask` rule skips the auto-allow shortcut and proceeds to the normal approval flow, and anything unmatched is auto-allowed. Paths outside the workspace on path-arg tools are denied at authorize time (the same sandbox path-escape enforces at execution, so the gate does not show an Accept overlay that cannot succeed). Writes under the in-workspace session state root (legacy `.agent-state`) still ask under auto mode. Under `--dangerously-skip-permissions` (forces this process) or `/yolo` (persists as the user-global default via `setSkipPermissions`), the gate auto-allows those same cases, and pre-gate sandboxes (path-escape, shell session cwd retention, `list_dir` / `delete_file` workspace bounds) honor `getSkipPermissions()` live so outside-workspace access is not hard-denied after the gate already allowed it — without rebuilding the plugin stack. Secret-guard path denies and authorization hard blocks still apply. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted.
410410
- **Reactor-gated sessions (main session; `reactorGated: true`).** The gate's decision logic lives in one `decide()` used by both consumers: `evaluate()` (the middleware path below, still used by sub-agents) and `authorizeCall()`, which expresses the decision as the vendored reactor's before-tool authz effect (`src/permission/reactor-authorize.ts` bridges it into `env.authorize`). An `ask` there suspends the call as a reactor `PendingOperation` keyed by a correlationId (persisted through the context store's existing `pendingOperations`); `send()` settles as `suspended` and `src/session/approval-resume.ts` rebuilds the operator request from the approval snapshot, resolves it through the same `requestApproval` seam the TUI overlay uses, and delivers the decision to the reactor on the correlationId signal channel — an approved decision grants a one-shot bypass and the exact parked call re-dispatches; a rejected one answers it with an error result. `inFlight` occupancy owns idle rebuild: the TUI stays busy across the overlay and waits until the correlated resume is accepted (`message.received` / `message.correlated`) or a generation bump `settleAll`s the waiter. Delivery generation owns session identity: interrupt, `/clear`, and `/new` abort the outstanding overlay, skip minting a grant, drop the decision, and surface an operator notice rather than delivering into a rebuilt agent. Under reactor gating the middleware/MCP `gateToolCall` is an execution backstop, not a second copy of `env.authorize`: it consumes the `authorizeCall` verdict only when id, name, and arguments match, and does not re-decide. Deny still blocks and does not call `next`; an `ask` or `allow` skips the middleware prompt so an approved re-dispatch never re-asks. A reused `codex-proxy` id cannot apply an outer `shell` allow to an inner `run_shell` deny. Inner posix runs whose outer tool is not `run_shell` (Codex `apply_patch` proxy) never pass `env.authorize`, so `gateToolCall` decides on that cache miss and still blocks a deny. The headless denial and the stricter chained-command hard-deny are preserved as deny effects (upstream `block`s) decided inside the same `decide()`.
411-
- **Approval resume identity.** Before opening the operator gate, resume captures the session generation and agent/store pair and resolves the correlation exactly once through `ContextStore.load().pendingOperations` to one approval operation's `suspendedCall.id`. Missing or duplicate mappings produce no gate or delivery; store errors propagate with registration cleanup. Every decision path first checks captured history for an exact-call approval timeout, and operator decisions check again after the gate. Identical tool names and arguments never establish identity. Cancellation during lookup cannot deliver a rejection. This suppresses observed exact timeouts, not all expired correlations: the reactor removes correlation state before the queued timeout result publishes, and expiration can also race the final history check or TUI delivery queue. Atomic stale-decision admission remains reactor-owned work tracked separately in CL-8000.
411+
- **Approval resume identity.** Before opening the operator gate, resume captures the session generation and agent/store pair and resolves the correlation exactly once through `ContextStore.load().pendingOperations` to one approval operation's `suspendedCall.id`. Missing or duplicate mappings produce no gate or delivery; store errors propagate with registration cleanup. Every decision path first checks captured history for an exact-call approval timeout, and operator decisions check again after the gate. While the overlay is open, resume watches history for that exact-call timeout and aborts the overlay signal so the gate auto-denies, occupancy (`inFlight`) unsticks, and no late decision is delivered. Identical tool names and arguments never establish identity. Cancellation during lookup cannot deliver a rejection. This suppresses observed exact timeouts, not all expired correlations: the reactor removes correlation state before the queued timeout result publishes, and expiration can also race the final history check or TUI delivery queue. Atomic stale-decision admission remains reactor-owned work tracked separately in CL-8000.
412412
- **Worker reactor ownership.** `workerPermissionGate` is a reactor-gated view over the parent's live permission gate: grants and policy are shared, not copied or toggled. Worker posix plugins and inherited MCP tools are bound to that view at worker start, so they take the reactor-gated `gateToolCall` path because the view reports `isReactorGated()` — they do not close over the parent's middleware-gated `isReactorGated()`. Deny still blocks; ask/allow skip the middleware prompt. `authorizeCall` on the view never emits `ask` — unresolved approvals become denials that name the permission subject, without invoking an approval callback or suspending, even with an interactive parent; the parent can obtain a grant and retry. Worker control-plane tools (`submit_result`, `ask_director`, and nested fleet verbs other than `spawn_agent`) allow without a parent grant. Authorization and tool execution run under the same async-local worker identity and cwd. Fleet authority remains an independent restriction, not an alternative permission grant.
413413

414414
- **matcher** — Approval pattern matching via `@intx/authz` `matchPattern` (`*` wildcards). Exact-command grants store a backslash before each metacharacter; those patterns match by equality after unescape (the package has no escape syntax).

‎src/session/approval-resume.ts‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,35 @@ function timeoutResult(
102102
);
103103
}
104104

105+
function sleep(ms: number, signal: AbortSignal): Promise<void> {
106+
if (signal.aborted) return Promise.resolve();
107+
return new Promise((resolve) => {
108+
const timer = setTimeout(resolve, ms);
109+
signal.addEventListener(
110+
"abort",
111+
() => {
112+
clearTimeout(timer);
113+
resolve();
114+
},
115+
{ once: true },
116+
);
117+
});
118+
}
119+
120+
async function watchParkedTimeout(
121+
history: () => ReturnType<Agent["history"]>,
122+
parkedCallId: string,
123+
signal: AbortSignal,
124+
pollMs: number,
125+
): Promise<boolean> {
126+
while (!signal.aborted) {
127+
await sleep(pollMs, signal);
128+
if (signal.aborted) return false;
129+
if (timeoutResult(await history(), parkedCallId)) return true;
130+
}
131+
return false;
132+
}
133+
105134
function decisionMessage(
106135
correlationId: string,
107136
outcome: "approved" | "rejected",
@@ -137,6 +166,8 @@ export function createApprovalResume(args: {
137166
captureGeneration?: () => () => boolean;
138167
onDropped?: (text: string) => void;
139168
registerParkedCancel?: (cancel: (() => void) | undefined) => void;
169+
registerOverlayAbort?: (controller: AbortController | undefined) => void;
170+
parkedTimeoutPollMs?: number;
140171
resolveParkedCallId: (
141172
correlationId: string,
142173
) => string | undefined | Promise<string | undefined>;
@@ -179,6 +210,7 @@ export function createApprovalResume(args: {
179210
cancelParked();
180211
};
181212
args.registerParkedCancel?.(cancelParked);
213+
let overlayAbort: AbortController | undefined;
182214
try {
183215
// The resolver captures the paired store synchronously before its first await.
184216
const parkedCallId = await args.resolveParkedCallId(correlationId);
@@ -222,14 +254,38 @@ export function createApprovalResume(args: {
222254
);
223255
return true;
224256
}
257+
258+
overlayAbort = new AbortController();
259+
args.registerOverlayAbort?.(overlayAbort);
260+
const timeoutWatch = watchParkedTimeout(
261+
() => parkedAgent.history(),
262+
parkedCallId,
263+
overlayAbort.signal,
264+
args.parkedTimeoutPollMs ?? 250,
265+
).then((timedOut) => {
266+
if (
267+
timedOut &&
268+
overlayAbort !== undefined &&
269+
!overlayAbort.signal.aborted
270+
) {
271+
overlayAbort.abort(APPROVAL_TIMEOUT_RESULT_TEXT);
272+
}
273+
return timedOut;
274+
});
275+
225276
const outcome = await args.gate.resolveSuspended(request, stillCurrent);
277+
if (!overlayAbort.signal.aborted) overlayAbort.abort();
278+
const timedOutDuringOverlay =
279+
overlayAbort.signal.reason === APPROVAL_TIMEOUT_RESULT_TEXT ||
280+
(await timeoutWatch);
226281
if (!stillCurrent()) {
227282
dropParked();
228283
return true;
229284
}
230285
args.registerParkedCancel?.(undefined);
231286
const history = await parkedAgent.history();
232-
const timedOut = timeoutResult(history, parkedCallId);
287+
const timedOut =
288+
timedOutDuringOverlay || timeoutResult(history, parkedCallId);
233289
if (timedOut) canReject = false;
234290
if (!stillCurrent()) {
235291
dropParked();
@@ -248,6 +304,10 @@ export function createApprovalResume(args: {
248304
);
249305
return true;
250306
} finally {
307+
if (overlayAbort !== undefined && !overlayAbort.signal.aborted) {
308+
overlayAbort.abort();
309+
}
310+
args.registerOverlayAbort?.(undefined);
251311
args.registerParkedCancel?.(undefined);
252312
}
253313
};

‎src/tui/runner/session.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,9 @@ export async function assembleTUISession(
171171

172172
const correlationAcceptance = createCorrelationAcceptance();
173173
const parkedApprovalCancel = { fn: undefined as (() => void) | undefined };
174+
const parkedOverlayAbort = {
175+
controller: undefined as AbortController | undefined,
176+
};
174177
const deliveryGeneration = createDeliveryGeneration(() => {
175178
parkedApprovalCancel.fn?.();
176179
correlationAcceptance.settleAll();
@@ -185,7 +188,13 @@ export async function assembleTUISession(
185188
requestApproval: createGateRequestApproval({
186189
emitGate: (event) => emitter.emit("permission.gate", event),
187190
approvalTimeout,
188-
identitySignal: () => deliveryGeneration.signal(),
191+
identitySignal: () => {
192+
const identity = deliveryGeneration.signal();
193+
const parked = parkedOverlayAbort.controller?.signal;
194+
return parked === undefined
195+
? identity
196+
: AbortSignal.any([identity, parked]);
197+
},
189198
}),
190199
getActiveProviderModel: () =>
191200
`${state.config.providerName}:${state.config.model}`,
@@ -526,6 +535,9 @@ export async function assembleTUISession(
526535
registerParkedCancel: (cancel) => {
527536
parkedApprovalCancel.fn = cancel;
528537
},
538+
registerOverlayAbort: (controller) => {
539+
parkedOverlayAbort.controller = controller;
540+
},
529541
deliver: (message, stillCurrent) => {
530542
return sessionOps.enqueue(async () => {
531543
if (!stillCurrent()) return;

‎tests/unit/approval-resume.test.ts‎

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { describe, expect, test } from "bun:test";
33
import { AgentClosedError, type SendResult } from "@intx/agent";
44
import type { ConversationTurn, InboundMessage } from "@intx/types/runtime";
55

6+
import { APPROVAL_TIMEOUT_RESULT_TEXT } from "../../src/permission/decline-markers.js";
67
import {
78
createPermissionGate,
89
type PermissionGate,
@@ -51,7 +52,7 @@ function approvalTimedOutTurn(): ConversationTurn {
5152
{
5253
type: "tool_result",
5354
callId: "call-ask",
54-
content: [{ type: "text", text: "approval timed out" }],
55+
content: [{ type: "text", text: APPROVAL_TIMEOUT_RESULT_TEXT }],
5556
},
5657
],
5758
timestamp: 0,
@@ -752,3 +753,86 @@ describe("approval resume occupancy until correlation", () => {
752753
expect(state.inFlight).toBe(0);
753754
});
754755
});
756+
757+
describe("approval resume overlay on reactor timeout", () => {
758+
test("auto-abandons the overlay and unsticks occupancy when the parked call times out", async () => {
759+
const parkedOverlayAbort = {
760+
controller: undefined as AbortController | undefined,
761+
};
762+
const generation = createDeliveryGeneration();
763+
const turns = [userTurn()];
764+
let overlay: PermissionGateEvent | undefined;
765+
let overlayReady: (() => void) | undefined;
766+
const waitForOverlay = new Promise<void>((resolve) => {
767+
overlayReady = resolve;
768+
});
769+
const requestApproval = createGateRequestApproval({
770+
emitGate: (event) => {
771+
overlay = event;
772+
overlayReady?.();
773+
event.signal?.addEventListener(
774+
"abort",
775+
() => {
776+
event.resolve({
777+
allow: false,
778+
message:
779+
typeof event.signal?.reason === "string"
780+
? event.signal.reason
781+
: "aborted",
782+
});
783+
},
784+
{ once: true },
785+
);
786+
return true;
787+
},
788+
approvalTimeout: () => undefined,
789+
identitySignal: () => {
790+
const identity = generation.signal();
791+
const parked = parkedOverlayAbort.controller?.signal;
792+
if (parked === undefined) return identity;
793+
return AbortSignal.any([identity, parked]);
794+
},
795+
});
796+
const gate = createPermissionGate({
797+
approvals: [],
798+
interactive: true,
799+
skipPermissions: false,
800+
reactorGated: true,
801+
requestApproval,
802+
});
803+
const delivered: unknown[] = [];
804+
const resume = createApprovalResume({
805+
resolveParkedCallId: () => "call-ask",
806+
getAgent: () => ({
807+
deliver: (message: unknown) => delivered.push(message),
808+
history: async () => turns,
809+
}),
810+
captureGeneration: generation.capture,
811+
registerOverlayAbort: (controller) => {
812+
parkedOverlayAbort.controller = controller;
813+
},
814+
parkedTimeoutPollMs: 5,
815+
gate,
816+
});
817+
const state = {
818+
inFlight: 0,
819+
pendingReload: false,
820+
reloadIfIdle: () => undefined,
821+
};
822+
823+
const running = runWhileAgentBusy(state, async () => {
824+
await resume.handle(SUSPENDED);
825+
});
826+
await waitForOverlay;
827+
expect(state.inFlight).toBe(1);
828+
expect(overlay?.signal?.aborted).toBe(false);
829+
830+
turns.push(approvalTimedOutTurn());
831+
await running;
832+
833+
expect(overlay?.signal?.aborted).toBe(true);
834+
expect(overlay?.signal?.reason).toBe(APPROVAL_TIMEOUT_RESULT_TEXT);
835+
expect(delivered).toEqual([]);
836+
expect(state.inFlight).toBe(0);
837+
});
838+
});

0 commit comments

Comments
 (0)