Skip to content

Commit f6f6807

Browse files
committed
Hold the TUI busy across the approval overlay
1 parent 54e6b94 commit f6f6807

6 files changed

Lines changed: 93 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
3737
- Spawned workers enforce the parent permission gate. Unresolved worker
3838
approvals deny with a reason that names the permission subject so the parent
3939
can grant and retry, without hanging on operator approval.
40+
- Operator approval resume late-binds to the live agent and keeps the TUI
41+
busy across the overlay so a reload cannot drop the parked tool call.
4042

4143
### Changed
4244

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ tool call
382382
- **command** — Splits chained commands for security classification and derives command-shape approval scopes. Multi-segment chains only offer an exact-command persist pattern (a prefix like `npm *` must not cover `npm i && rm -rf /` later).
383383
- **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`.
384384
- **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 and writes under the session state root (`~/.corbits/projects/...` and 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.
385-
- **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. 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()`.
385+
- **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. The TUI stays busy across the overlay and late-binds deliver through the live agent / session queue. 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()`.
386386
- **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.
387387

388388
- **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/tui/runner/exit.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
hostOf,
3131
liveAgent,
3232
recordRunError,
33+
runWhileAgentBusy,
3334
type RunnerServices,
3435
type RunnerState,
3536
type SnapshotExtra,
@@ -339,15 +340,11 @@ export async function createRunLifecycle(
339340
services.emitter.emit("session.title", truncateSessionLabel(state.runTaskTitle));
340341
void persistRunSnapshot("running");
341342
}
342-
state.inFlight++;
343-
try {
343+
return await runWhileAgentBusy(state, async () => {
344344
await refreshCodexBeforeSend();
345345
await refreshXaiBeforeSend();
346346
return await liveAgent(state).send(content, opts);
347-
} finally {
348-
state.inFlight--;
349-
reloadIfIdle();
350-
}
347+
});
351348
},
352349
stream: () => liveAgent(state).stream(),
353350
deliver: (message) => {

src/tui/runner/state.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,19 @@ export function liveAgent(state: RunnerState): Agent {
254254
return agent;
255255
}
256256

257+
export async function runWhileAgentBusy<T>(
258+
state: Pick<RunnerState, "inFlight" | "reloadIfIdle">,
259+
op: () => Promise<T>,
260+
): Promise<T> {
261+
state.inFlight++;
262+
try {
263+
return await op();
264+
} finally {
265+
state.inFlight--;
266+
state.reloadIfIdle?.();
267+
}
268+
}
269+
257270
export function hostOf(state: RunnerState): RunnerHost {
258271
const host = state.host;
259272
if (host === undefined) {

src/tui/runner/submit.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { tuiSendFailureMessage } from "./send-failure-message.js";
3838
import type { ProviderFailureAttempt } from "../provider/failure-attempt.js";
3939
import type { Agent } from "@intx/agent";
4040
import { ASK_DIRECTOR_WAKE_PREFIX } from "../../subagent/fleet-report.js";
41-
import { hostOf, type RunnerServices, type RunnerState } from "./state.js";
41+
import { hostOf, runWhileAgentBusy, type RunnerServices, type RunnerState } from "./state.js";
4242
import { LOG_NAMESPACE_ROOT } from "../../branding.js";
4343

4444
const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]);
@@ -259,12 +259,14 @@ export function createSubmitPath(
259259
const attempt = live.attemptIdentity();
260260
const providerFailure = services.providerFailureAttempts.begin(attempt);
261261
try {
262-
const result = await live.agentProxy.send(message);
263-
// An ask-tier call parked on the reactor's approval gate settles the
264-
// send early; resolve the operator surface here and deliver the
265-
// decision on the correlationId signal channel so the parked run
266-
// resumes.
267-
await services.approvalResume.handle(result);
262+
await runWhileAgentBusy(state, async () => {
263+
const result = await live.agentProxy.send(message);
264+
// An ask-tier call parked on the reactor's approval gate settles the
265+
// send early; resolve the operator surface here and deliver the
266+
// decision on the correlationId signal channel so the parked run
267+
// resumes.
268+
await services.approvalResume.handle(result);
269+
});
268270
return true;
269271
} catch (error) {
270272
handleSendFailure(error, attempt, providerFailure);
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { describe, expect, test } from "bun:test";
2+
3+
import { runWhileAgentBusy, type RunnerState } from "../../../src/tui/runner/state.js";
4+
5+
function stubBusyState() {
6+
const rebuilds: number[] = [];
7+
const state: Pick<RunnerState, "inFlight" | "reloadIfIdle"> & { pendingReload: boolean } = {
8+
inFlight: 0,
9+
pendingReload: false,
10+
reloadIfIdle: () => {
11+
if (!state.pendingReload || state.inFlight > 0) return;
12+
state.pendingReload = false;
13+
rebuilds.push(state.inFlight);
14+
},
15+
};
16+
return { state, rebuilds };
17+
}
18+
19+
describe("runWhileAgentBusy vs pendingReload", () => {
20+
test("nested spans hold reloadIfIdle until the outer span finishes", async () => {
21+
const { state, rebuilds } = stubBusyState();
22+
23+
const result = await runWhileAgentBusy(state, async () => {
24+
return await runWhileAgentBusy(state, async () => {
25+
state.pendingReload = true;
26+
state.reloadIfIdle?.();
27+
return "suspended";
28+
});
29+
});
30+
31+
expect(result).toBe("suspended");
32+
expect(rebuilds).toEqual([0]);
33+
});
34+
35+
test("pendingReload during a deferred overlay-shaped outer op rebuilds only after resolve", async () => {
36+
const { state, rebuilds } = stubBusyState();
37+
let release: (() => void) | undefined;
38+
const deferred = new Promise<void>((resolve) => {
39+
release = resolve;
40+
});
41+
42+
const running = runWhileAgentBusy(state, async () => {
43+
state.pendingReload = true;
44+
state.reloadIfIdle?.();
45+
await deferred;
46+
return "ok";
47+
});
48+
49+
expect(rebuilds).toEqual([]);
50+
expect(state.inFlight).toBe(1);
51+
release?.();
52+
expect(await running).toBe("ok");
53+
expect(rebuilds).toEqual([0]);
54+
expect(state.inFlight).toBe(0);
55+
});
56+
57+
test("reloadIfIdle is a no-op when inFlight is already greater than zero", () => {
58+
const { state, rebuilds } = stubBusyState();
59+
state.inFlight = 2;
60+
state.pendingReload = true;
61+
state.reloadIfIdle?.();
62+
expect(rebuilds).toEqual([]);
63+
expect(state.pendingReload).toBe(true);
64+
});
65+
});

0 commit comments

Comments
 (0)