Skip to content

Commit df0b168

Browse files
Resume operator approval on the live agent (#845)
* Late-bind operator approval to the live agent * Hold the TUI busy across the approval overlay * Capture approval generation at overlay start /clear during the permission overlay bumped generation after handle started, but the TUI recaptured at deliver time so an accept still injected the approved decision into the empty session. * Drop overlay approval after interrupt rebuilds the agent Interrupt rebuilt the agent while the permission overlay stayed open, so a later accept or decline still saw a current generation and delivered into the new agent. Bump delivery generation on interrupt the same way session rotation does. * Assert interrupt bumps delivery generation before rebuild * Drop operator approval when session identity changes * Reject parked approval on interrupt and hold occupancy until tool start
1 parent 8685cbd commit df0b168

20 files changed

Lines changed: 1311 additions & 100 deletions

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
4242
- A session falls back to the next resolvable provider when the selected
4343
default or project-local provider is missing or incomplete. `--provider`
4444
still errors.
45+
- Operator approval resume late-binds to the live agent and keeps the TUI
46+
busy across the overlay so a reload cannot drop the parked tool call.
47+
- Operator approval drops on session identity change. inFlight occupancy
48+
owns idle rebuild; delivery generation owns session identity, so interrupt,
49+
/clear, and /new abort the outstanding overlay, skip minting a grant, and
50+
notify the operator instead of delivering into a rebuilt agent.
4551

4652
### Changed
4753

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. `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()`.
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/permission/gate.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@ function classifyOutcome(outcome: ApprovalOutcome | undefined): ApprovalOutcomeK
6767
if (!outcome.allow) {
6868
const message = outcome.message ?? "";
6969
if (message.includes("timed out")) return "timeout";
70-
if (message.includes("no longer running")) return "abort";
70+
if (message.includes("no longer running") || message.includes("identity changed")) {
71+
return "abort";
72+
}
7173
return "deny";
7274
}
7375
return outcome.persist !== undefined ? "allow-with-scope" : "allow-once";
@@ -325,8 +327,12 @@ export interface PermissionGate {
325327
// run_shell, colliding reused ids, and tests).
326328
executionVerdict: (call: ToolCall) => Promise<AuthorizeVerdict>;
327329
// Resolve a suspended reactor approval against the operator (and mint the
328-
// outcome's grant). Returns undefined when no outcome arrived.
329-
resolveSuspended: (request: PermissionRequest) => Promise<ApprovalOutcome | undefined>;
330+
// outcome's grant when the session identity is still current). Returns
331+
// undefined when no outcome arrived.
332+
resolveSuspended: (
333+
request: PermissionRequest,
334+
stillCurrent?: () => boolean,
335+
) => Promise<ApprovalOutcome | undefined>;
330336
// True when this gate's decisions go through env.authorize (authorizeCall)
331337
// rather than evaluate() in the tool-runner middleware. Under reactor gating,
332338
// gateToolCall is an execution backstop: it consumes a matching cached
@@ -724,7 +730,10 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
724730
// span, await the requestApproval seam, settle the log/span, and mint any
725731
// grant the outcome carries (never for secret-path shell). Returns undefined
726732
// when no outcome arrived (timeout/abort auto-deny paths).
727-
const resolveInteractiveAsk = async (decision: Extract<GateDecision, { kind: "ask" }>) => {
733+
const resolveInteractiveAsk = async (
734+
decision: Extract<GateDecision, { kind: "ask" }>,
735+
stillCurrent?: () => boolean,
736+
) => {
728737
const { request, anySecret, segmentCount } = decision;
729738
const askRule = anySecret ? "sensitive-path" : undefined;
730739
const ask = approvalLog.ask({
@@ -752,7 +761,7 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
752761
finishApprovalWait(telemetry, waitSpanId, request.tool, outcome);
753762
ask.settle(classifyOutcome(outcome));
754763
}
755-
if (outcome !== undefined && outcome.allow && !anySecret) {
764+
if (outcome !== undefined && outcome.allow && !anySecret && (stillCurrent?.() ?? true)) {
756765
mintGrant(request.tool, outcome);
757766
}
758767
return outcome;
@@ -823,18 +832,21 @@ export function createPermissionGate(options: PermissionGateOptions): Permission
823832
// Resolve a suspended reactor approval once the operator answers. The
824833
// request is the one authorizeCall built at decision time, so the ask log,
825834
// wait span, and grant minting are identical to the middleware path.
826-
const resolveSuspended = (request: PermissionRequest) => {
835+
const resolveSuspended = (request: PermissionRequest, stillCurrent?: () => boolean) => {
827836
const anySecret =
828837
request.tool === "run_shell" && commandReferencesSensitivePath(request.subject) !== undefined;
829-
return resolveInteractiveAsk({
830-
kind: "ask",
831-
request,
832-
anySecret,
833-
segmentCount:
834-
request.tool === "run_shell"
835-
? splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s)).length
836-
: 0,
837-
});
838+
return resolveInteractiveAsk(
839+
{
840+
kind: "ask",
841+
request,
842+
anySecret,
843+
segmentCount:
844+
request.tool === "run_shell"
845+
? splitChainedCommand(request.subject).filter((s) => !isShellCommentOnly(s)).length
846+
: 0,
847+
},
848+
stillCurrent,
849+
);
838850
};
839851

840852
const reset = (): void => {

src/session/approval-resume.ts

Lines changed: 97 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import type { PermissionRequest } from "../permission/types.js";
2626

2727
const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-resume"]);
2828

29+
export const APPROVAL_DROPPED_NOTICE = "Approval dropped because the session changed.";
30+
2931
const ApprovalSnapshotShape = type({
3032
name: "string",
3133
"arguments?": "Record<string, unknown>",
@@ -108,52 +110,115 @@ function decisionMessage(
108110
}
109111

110112
export function createApprovalResume(args: {
111-
// Late-bound: the live agent is read at handle() time so rebuilds
112-
// (/clear, model switch) deliver through the current instance.
113+
// Live agent at history/deliver time. TUI occupancy holds this identity
114+
// until the correlated resume is accepted; a generation bump aborts the
115+
// gate rather than retargeting a rebuilt agent.
113116
getAgent: () => Pick<Agent, "deliver" | "history"> | undefined;
117+
// TUI session queue. When present, each decision is awaited through this
118+
// seam; exec omits it and uses getAgent().deliver.
119+
deliver?: (message: InboundMessage, stillCurrent: () => boolean) => void | Promise<void>;
120+
// TUI: capture at handle() start so interrupt, /clear, or /new during the
121+
// overlay aborts the gate and drops the decision. Exec omits this.
122+
captureGeneration?: () => () => boolean;
123+
// TUI: operator-visible notice when an overlay decision is dropped after
124+
// a generation bump.
125+
onDropped?: (text: string) => void;
126+
// TUI: interrupt/clear bump this to reject the parked call on the old
127+
// agent before close/rebuild. Cleared when handle returns.
128+
registerParkedCancel?: (cancel: (() => void) | undefined) => void;
114129
gate: PermissionGate;
115130
}): ApprovalResume {
116131
const { getAgent, gate } = args;
132+
133+
const requireAgent = (): Pick<Agent, "deliver" | "history"> => {
134+
const agent = getAgent();
135+
if (agent === undefined) {
136+
throw new Error("approval resume: no live agent");
137+
}
138+
return agent;
139+
};
140+
117141
return {
118142
handle: async (result) => {
119143
if (result.type !== "suspended") return false;
120-
const agent = getAgent();
121-
if (agent === undefined) return true;
144+
const stillCurrent = args.captureGeneration?.() ?? (() => true);
145+
const parkedAgent = requireAgent();
122146
const { correlationId, approvalSnapshot } = result;
123147

124-
// Turn-count watermark for the settled guard below: a "approval timed
125-
// out" tool result appended after this point means the reactor settled
126-
// this very correlation before our decision lands.
127-
const turnsAtSuspend = (await agent.history()).length;
148+
let cancelled = false;
149+
const cancelParked = (): void => {
150+
if (cancelled) return;
151+
cancelled = true;
152+
parkedAgent.deliver(decisionMessage(correlationId, "rejected", APPROVAL_DROPPED_NOTICE));
153+
};
154+
args.registerParkedCancel?.(cancelParked);
128155

129-
if (approvalSnapshot === undefined) {
130-
// A suspension without a snapshot cannot be surfaced; fail closed by
131-
// rejecting the parked call so the run does not hang on an invisible
132-
// gate.
133-
agent.deliver(decisionMessage(correlationId, "rejected", "approval surface unavailable"));
134-
return true;
135-
}
156+
const dropParked = (): void => {
157+
args.onDropped?.(APPROVAL_DROPPED_NOTICE);
158+
cancelParked();
159+
};
136160

137-
const request = requestFromApprovalSnapshot(approvalSnapshot, correlationId);
138-
if (request === null) {
139-
agent.deliver(decisionMessage(correlationId, "rejected", "approval surface unavailable"));
140-
return true;
141-
}
161+
try {
162+
const deliverDecision = async (message: InboundMessage): Promise<void> => {
163+
if (!stillCurrent()) return;
164+
if (args.deliver !== undefined) {
165+
await args.deliver(message, stillCurrent);
166+
return;
167+
}
168+
requireAgent().deliver(message);
169+
};
142170

143-
const outcome = await gate.resolveSuspended(request);
144-
if (settledAfterSuspend(await agent.history(), turnsAtSuspend)) {
145-
// The reactor already answered the parked call (its approval timeout
146-
// fired while the surface was still up). Delivering now would append
147-
// the raw decision JSON as an uncorrelated user turn — drop and log.
148-
logger.warn`late approval decision dropped correlation=${correlationId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
149-
return true;
150-
}
151-
if (outcome === undefined || !outcome.allow) {
152-
agent.deliver(decisionMessage(correlationId, "rejected", outcome?.message));
171+
// Turn-count watermark for the settled guard below: a "approval timed
172+
// out" tool result appended after this point means the reactor settled
173+
// this very correlation before our decision lands.
174+
const turnsAtSuspend = (await parkedAgent.history()).length;
175+
if (!stillCurrent()) {
176+
dropParked();
177+
return true;
178+
}
179+
180+
if (approvalSnapshot === undefined) {
181+
// A suspension without a snapshot cannot be surfaced; fail closed by
182+
// rejecting the parked call so the run does not hang on an invisible
183+
// gate.
184+
args.registerParkedCancel?.(undefined);
185+
await deliverDecision(
186+
decisionMessage(correlationId, "rejected", "approval surface unavailable"),
187+
);
188+
return true;
189+
}
190+
191+
const request = requestFromApprovalSnapshot(approvalSnapshot, correlationId);
192+
if (request === null) {
193+
args.registerParkedCancel?.(undefined);
194+
await deliverDecision(
195+
decisionMessage(correlationId, "rejected", "approval surface unavailable"),
196+
);
197+
return true;
198+
}
199+
200+
const outcome = await gate.resolveSuspended(request, stillCurrent);
201+
if (!stillCurrent()) {
202+
dropParked();
203+
return true;
204+
}
205+
args.registerParkedCancel?.(undefined);
206+
if (settledAfterSuspend(await requireAgent().history(), turnsAtSuspend)) {
207+
// The reactor already answered the parked call (its approval timeout
208+
// fired while the surface was still up). Delivering now would append
209+
// the raw decision JSON as an uncorrelated user turn — drop and log.
210+
logger.warn`late approval decision dropped correlation=${correlationId} outcome=${outcome?.allow === true ? "approved" : "rejected"}`;
211+
return true;
212+
}
213+
if (outcome === undefined || !outcome.allow) {
214+
await deliverDecision(decisionMessage(correlationId, "rejected", outcome?.message));
215+
return true;
216+
}
217+
await deliverDecision(decisionMessage(correlationId, "approved"));
153218
return true;
219+
} finally {
220+
args.registerParkedCancel?.(undefined);
154221
}
155-
agent.deliver(decisionMessage(correlationId, "approved"));
156-
return true;
157222
},
158223
};
159224
}

0 commit comments

Comments
 (0)