Skip to content

Commit efa2b1a

Browse files
Merge pull request #1077 from corbitsdev/cl-7953-leftovers-0-3-27
fix(approval): exact parked-call identity, typed discard, and retry re-use
2 parents 74384f3 + 6835e75 commit efa2b1a

21 files changed

Lines changed: 1506 additions & 420 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,21 @@ The reactor (from `@intx/agent`) drives a single agent turn-by-turn. Each turn i
1414

1515
This repeats until the director emits `capabilities.done()`.
1616

17+
Explicit approval messages use `headers.interchangeType` (`approval.granted` or
18+
`approval.denied`) as intent, not authentication. The reactor accepts one only
19+
while its exact pending operation and matching gate remain live, before both
20+
epoch deadlines (equality expires), with a body agreeing with the header. Typed
21+
messages without an eligible suspended call, expired/consumed decisions and
22+
concurrent duplicates are discarded before body parsing, never ordinary input.
23+
Validator false/throw also discards typed decisions but leaves a still-live
24+
operation retryable; untyped and nonapproval response routing is unchanged.
25+
After validation and synchronous grant callbacks, the reactor rechecks ownership,
26+
deadlines and lifecycle, then consumes the operation and enqueues its continuation
27+
before publishing `message.correlated`. The claim is released on every settled
28+
exit; a never-settling validator retains it. Extension side effects are not rolled
29+
back. Discards produce no acceptance receipt: the TUI's existing bounded 30-second
30+
acceptance wait remains unchanged.
31+
1732
### Reactor Events (Partial)
1833

1934
This table is not the full reactor/stream event vocabulary. `tool.done` is
@@ -393,6 +408,7 @@ tool call
393408
- **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`.
394409
- **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.
395410
- **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.
396412
- **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.
397413

398414
- **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/exec/runner.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,10 @@ import {
110110
loadSessionChatPrompt,
111111
skillDirsFromEnabledPlugins,
112112
} from "../session/runtime-assembly.js";
113-
import { createApprovalResume } from "../session/approval-resume.js";
113+
import {
114+
createApprovalResume,
115+
resolveParkedCallIdFromStore,
116+
} from "../session/approval-resume.js";
114117
import { createReactorAuthorize } from "../permission/reactor-authorize.js";
115118
import {
116119
assembleChatAgent,
@@ -932,6 +935,9 @@ export async function runExec(config: Config): Promise<ExecResult> {
932935
agent = currentAgent;
933936
// Local non-null handle after init (currentAgent stays mutable for compaction deliver).
934937
const activeAgent = currentAgent;
938+
const activeStorage = currentStorage;
939+
if (activeStorage === null)
940+
throw new Error("approval resume: no context store");
935941

936942
if (agentToolset.connectMCP !== undefined) {
937943
await agentToolset
@@ -1051,10 +1057,10 @@ export async function runExec(config: Config): Promise<ExecResult> {
10511057
const sendResult = await activeAgent.send(operatorTaskMessage(task));
10521058
// A suspension must not park silently in exec: the approval resume owns
10531059
// the terminal prompt flow and delivers the decision to the reactor.
1054-
// No resolveParkedCallId: the vendored reactor exposes no
1055-
// correlationId-to-call lookup, so the history heuristic is the path.
10561060
await createApprovalResume({
10571061
getAgent: () => activeAgent,
1062+
resolveParkedCallId: (correlationId) =>
1063+
resolveParkedCallIdFromStore(activeStorage, correlationId),
10581064
gate: permissionGate,
10591065
}).handle(sendResult);
10601066
sendCompleted = true;

0 commit comments

Comments
 (0)