From 15594e37148a06450a861dd53b00257d61ff4cc2 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Sun, 6 Sep 2026 18:54:19 -0700 Subject: [PATCH 1/4] RFC: Reconcile ask-authz suspend with unified message bus (CL-5683) Documents the architectural decision to route ask-authz suspend through the unified message bus (Preact signals) rather than carrying forward the legacy 1:1 PermissionRequest / PermissionResponse model. --- docs/RFC-ask-authz-suspend.md | 131 ++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 docs/RFC-ask-authz-suspend.md diff --git a/docs/RFC-ask-authz-suspend.md b/docs/RFC-ask-authz-suspend.md new file mode 100644 index 000000000..41bbae3f3 --- /dev/null +++ b/docs/RFC-ask-authz-suspend.md @@ -0,0 +1,131 @@ +# RFC: Reconcile ask-authz suspend with unified message bus + +**Status:** Draft +**Author:** Corbits Code +**Ticket:** [CL-5683](https://linear.app/abklabs/issue/CL-5683/rfc-reconcile-ask-authz-suspend-with-unified-message-bus) +**Blocks:** CL-5699 (adopt reactor approval-suspend primitive) + +## Summary + +The current permission gate holds `resolve()` callbacks in memory for the +duration of operator interaction. This works in-process but has no durability, +no cross-process portability, and no integration with the `@intx/types` +`Channel` message bus that underlies the reactor. This RFC proposes a +migration path toward an explicit suspend/resume primitive that survives +process restarts and plugs into the unified message bus, aligning with the +reactor's approval-suspend primitive that CL-5699 will adopt. + +## Motivation + +### Current architecture + +The permission system is already unified in-process — both shell/file-tool +permissions and operator questions flow through typed gate events: + +- `PermissionGateEvent` (`src/tui/gate-events.ts:23-39`) — carries a + `PermissionRequest`, a `resolve(outcome)` callback, optional `timeoutMs`, + and an `AbortSignal`. +- `OperatorGateEvent` (`src/tui/gate-events.ts:4-21`) — carries a question, + options, a `resolve(result)` callback, optional `timeoutMs`, and an + `AbortSignal`. + +The overlay host (`src/tui/gate-wire.ts`) connects these events to the TUI, +holding the `resolve()` callback open until the operator interacts. A +reconciliation queue (`src/permission/queue.ts`) serializes concurrent requests +and routes grants through the approval store. + +### The gap + +The "suspend" is implicit: the `resolve()` closure sits in memory, pinned by +the gate-wire's pending queue. If the process dies, the suspend is lost — the +LLM's tool call returns a hung future with no recovery path. This is fine for +an interactive terminal session but blocks: + +1. **Process restart recovery.** A crashed session cannot resume a pending + operator approval. +2. **Cross-process portability.** The `@intx/types` `Channel` pattern lets + messages flow between the reactor and external surfaces (TUI, web, CI). + Permission requests cannot currently ride this bus because they are + closure-based, not message-based. +3. **Reactor approval-suspend primitive.** The upstream reactor now supports an + explicit `suspend()` / `resume()` cycle for gate interactions. Corbits Code + does not yet use it because the current closure-based approach predates it. + +## Design + +### Phase 1: Explicit suspend token (CL-5699 scope) + +Replace the bare `resolve()` closure with a `SuspendToken` that carries: + +```typescript +interface SuspendToken { + /** Unique ID for this permission interaction. */ + id: string; + /** Resume with the operator's decision. */ + resume(outcome: ApprovalOutcome | OperatorResult): void; + /** Cancel the interaction (auto-deny / auto-cancel). */ + cancel(reason: string): void; + /** Whether the interaction is still pending. */ + readonly pending: boolean; +} +``` + +The gate-wire creates a `SuspendToken` for each gate event, stores it in a +`Map`, and passes the token to the reactor's suspend +mechanism. The reactor calls `token.resume()` when the operator answers, and +`token.cancel()` on timeout or abort. + +This keeps the in-process behavior identical to today but gives the reactor an +explicit handle it can persist, serialize, or pass across process boundaries. + +### Phase 2: Message-bus integration (future, out of scope for v0.3.19) + +Once Phase 1 lands, permission requests can be serialized as `Channel` +messages. The reactor suspends with a token, the TUI overlay resumes it. +A future web or CI surface could do the same without in-process coupling. + +The serialized form would be: + +```typescript +interface SuspendMessage { + kind: "permission-suspend"; + token: string; // SuspendToken.id + request: PermissionRequest; +} +``` + +### Interaction with CL-7333 umbrella + +This RFC informs CL-5699 (adopt reactor's approval-suspend primitive) and +indirectly CL-5697 (vendor `@intx/inference` at HEAD, which ships the suspend +primitive in the reactor). No code changes in this RFC — it is a design doc. + +## Alternatives considered + +1. **Keep closure-based suspend, add try/catch wrapping.** Low effort but does + not solve durability or cross-process portability. The reactor's primitive + would go unused. + +2. **Serialize the full `PermissionRequest` into the context store.** Heavier + than needed — the token ID is sufficient to re-derive state from the queue. + The full request can be reconstructed from the queue's pending entries. + +3. **Skip Phase 1, jump straight to message-bus integration.** Risks a larger + blast radius; the Phase 1 token gives a clean seam for testing and + incremental migration. + +## Migration path + +1. CL-5683 (this RFC) — design complete. +2. CL-5699 — implement `SuspendToken`, wire gate-wire to use it, pass tokens + to reactor. Gate-wire behavior unchanged from the operator's perspective. +3. Monitor adoption; Phase 2 only when a cross-process surface requests it. + +## Open questions + +- Should `SuspendToken` be in `src/permission/` or `src/agent/`? The token is + created by the permission gate but consumed by the reactor. Leaning toward + `src/permission/suspend.ts` with a re-export from `src/agent/`. +- Should the token carry an optional serialized snapshot for process restart + recovery? Adds complexity; defer to Phase 2 unless a concrete use case + appears. From 1ba4b1490d492cdcd6182c0653ed3e9374a9918c Mon Sep 17 00:00:00 2001 From: Sawyer Date: Mon, 7 Sep 2026 10:50:48 -0700 Subject: [PATCH 2/4] Resolve the four CL-5683 decisions against the real suspend primitive The previous draft resolved none of the ticket's required decisions and designed against interfaces that do not exist: an invented suspend()/resume() token and a Channel bus absent from @intx/types. This rewrite resolves (a)-(d) explicitly, replaces the invented interfaces with the upstream suspend effect (gate, correlationId, persisted PendingOperation, signal-driven resume, approvedOnce bypass), and grounds every anchor in the vendored sources. Restart recovery for pending approvals is stated honestly as out of CL-5699 scope: the queue is an in-memory Map and the store persists grants only. --- docs/RFC-ask-authz-suspend.md | 332 +++++++++++++++++++++++----------- 1 file changed, 230 insertions(+), 102 deletions(-) diff --git a/docs/RFC-ask-authz-suspend.md b/docs/RFC-ask-authz-suspend.md index 41bbae3f3..42bb36ec1 100644 --- a/docs/RFC-ask-authz-suspend.md +++ b/docs/RFC-ask-authz-suspend.md @@ -7,125 +7,253 @@ ## Summary -The current permission gate holds `resolve()` callbacks in memory for the -duration of operator interaction. This works in-process but has no durability, -no cross-process portability, and no integration with the `@intx/types` -`Channel` message bus that underlies the reactor. This RFC proposes a -migration path toward an explicit suspend/resume primitive that survives -process restarts and plugs into the unified message bus, aligning with the -reactor's approval-suspend primitive that CL-5699 will adopt. +The current permission gate parks tool calls on in-memory `resolve()` +closures (`src/tui/gate-events.ts:23-39`) held open by the gate-wire +overlay. This RFC decides how Corbits Code adopts the upstream reactor's +approval-suspend primitive instead: a before-tool authz hook that returns +a `suspend` effect carrying an approval gate and a persisted +`PendingOperation`, with resumption driven by the reactor's signal +dispatch — not by any callback we invent. + +This RFC resolves the four decisions CL-5683 requires: + +- **(a)** The gate's pending-record bookkeeping maps onto the upstream + `PendingOperation`/`correlationId` flow by adopting the upstream + `correlationId` as the single identity for a parked call and reusing the + existing `pendingOperations` persistence in + `src/session/optimized-context-store.ts` — no parallel queue. +- **(b)** Director ask-handling consumes the reactor's suspend action and + `gate.cleared` resume dispatch; the director stops owning approval + queues. The wiring seam is `requestApproval` at + `src/session/assemble-runtime.ts:218,248`, not the director. +- **(c)** `src/permission/classify.ts` allow/ask tiering stays a + pre-filter above authz grants; it is not authz policy. +- **(d)** Headless denial and the stricter chained-command deny are + re-homed as `block` effects in the authz extension's before-tool hook, + where upstream already has a block channel. + +No code changes in this cut — design decision only. It blocks only +CL-5699. ## Motivation ### Current architecture -The permission system is already unified in-process — both shell/file-tool -permissions and operator questions flow through typed gate events: +- `PermissionGateEvent` (`src/tui/gate-events.ts:23-39`) carries a + `PermissionRequest`, a `resolve(outcome)` callback, optional + `timeoutMs`, and an `AbortSignal`. +- `OperatorGateEvent` (`src/tui/gate-events.ts:4-21`) is the question + analogue. +- The gate-wire overlay (`src/tui/gate-wire.ts`) connects these events to + the TUI and holds `resolve()` open until the operator interacts; it also + owns the pending-display overlay that serializes what the operator sees + (`src/tui/gate-wire.ts:235-273`). +- `src/permission/queue.ts` is an in-memory `Map` of concurrent pending + entries keyed by id (`src/permission/queue.ts:38-41`) used to reconcile + grants through the approval store; `src/permission/store.ts` persists + grants only, not pending requests. -- `PermissionGateEvent` (`src/tui/gate-events.ts:23-39`) — carries a - `PermissionRequest`, a `resolve(outcome)` callback, optional `timeoutMs`, - and an `AbortSignal`. -- `OperatorGateEvent` (`src/tui/gate-events.ts:4-21`) — carries a question, - options, a `resolve(result)` callback, optional `timeoutMs`, and an - `AbortSignal`. +### The gap -The overlay host (`src/tui/gate-wire.ts`) connects these events to the TUI, -holding the `resolve()` callback open until the operator interacts. A -reconciliation queue (`src/permission/queue.ts`) serializes concurrent requests -and routes grants through the approval store. +The "suspend" is implicit: the `resolve()` closure sits in memory, pinned +by the gate-wire's pending overlay. If the process dies, the suspend is +lost — the LLM's tool call returns a hung future with no recovery path. +Today there is **no durability for pending approvals**: the queue is an +in-memory Map and the store holds grants only. Nothing in this RFC claims +restart recovery for pending approvals until that changes (see the +durability decision under (a)). -### The gap +Meanwhile the upstream reactor (vendored at `vendor/intx-inference`) +already carries the primitive: + +- The before-tool authz hook returns, for an `ask` effect, + `{ type: "suspend", gate: { type: "approval", gateId, correlationId, + timeoutAt }, pendingOp }` (`authz-extension.ts:263-267` upstream; the + reactor persists `pendingOp`, minting `correlationId` at :223 and + `gateId = pending-${correlationId}` at :226). +- `DEFAULT_APPROVAL_TIMEOUT_MS = 3_600_000` (:36) — one hour. +- Resume is signal-driven dispatch in the reactor: a suspended call parks + on the reserved `signalName(correlationId)` channel (see the park-kind + prose in `packages/types/src/signals.ts:67` and `runtime.ts:663` + upstream), and a cleared gate enqueues a `reactor.gate.cleared` event + the director decides on (`reactor.ts:8-10, 68-72` upstream). +- A re-dispatch of an already-approved call bypasses the gate via a + delete-on-read in-memory `approvedOnce` token + (`authz-extension.ts:211-218`). + +Corbits Code does not use this yet because the closure-based approach +predates the vendoring. + +## Decisions + +### (a) Gate pending records map onto PendingOperation / correlationId + +**Decision:** adopt the upstream `correlationId` as the single identity of +a parked permission request. The gate's current per-request ids and +resolve closures are replaced by the upstream flow: the authz hook mints +`correlationId`, wraps the request into a `pendingOp` +(`PendingOperation` from `@intx/types/runtime`, with `approvalSnapshot` +and `suspendedCall`), and returns the `suspend` effect. The reactor +persists the operation; resume is addressed by `correlationId` via the +signal channel, not by holding a callback. + +Rationale: today `src/session/optimized-context-store.ts` already +persists `PendingOperation[]` from `@intx/types/runtime` +(`optimized-context-store.ts:9,178`), and upstream persists the operation +precisely so the id survives a restart (comment at +`authz-extension.ts:219-221`). Using that existing surface means the +gate's bookkeeping collapses into one identity and one store instead of a +parallel `Map` in the gate-wire. + +**Durability, stated honestly:** as of this RFC, restart recovery for +pending approvals is **not delivered and remains out of CL-5699 scope**. +`src/permission/queue.ts` is an in-memory `Map` and +`src/permission/store.ts` persists grants only, so a crashed session +still loses the pending approval. The mapping above is what makes +recovery *possible later* (the persisted `pendingOperations` plus +`correlationId`-addressed resume), but wiring snapshot-and-restore is +separate work and is not claimed by CL-5699. + +### (b) Director consumes suspend actions; requestApproval is the seam + +**Decision:** director ask-handling consumes the reactor's suspend action +and the `gate.cleared`-driven resume dispatch, and stops managing its own +approval queue. The `requestApproval` hook stays wired where it is today +— `src/session/assemble-runtime.ts:218` (the dependency declaration) and +`:248` (the wiring into the approval persist/log plumbing) — and its job +narrows to feeding the operator-facing surface. The director never sees +the gate itself; it sees outcomes only as tool-result text today +(`src/agent/director.ts:276` matches "Blocked by permission policy: +Operator declined:") and, after adoption, additionally sees the suspend +as a parked tool call and the clear as a `reactor.gate.cleared` event it +decides on. -The "suspend" is implicit: the `resolve()` closure sits in memory, pinned by -the gate-wire's pending queue. If the process dies, the suspend is lost — the -LLM's tool call returns a hung future with no recovery path. This is fine for -an interactive terminal session but blocks: - -1. **Process restart recovery.** A crashed session cannot resume a pending - operator approval. -2. **Cross-process portability.** The `@intx/types` `Channel` pattern lets - messages flow between the reactor and external surfaces (TUI, web, CI). - Permission requests cannot currently ride this bus because they are - closure-based, not message-based. -3. **Reactor approval-suspend primitive.** The upstream reactor now supports an - explicit `suspend()` / `resume()` cycle for gate interactions. Corbits Code - does not yet use it because the current closure-based approach predates it. - -## Design - -### Phase 1: Explicit suspend token (CL-5699 scope) - -Replace the bare `resolve()` closure with a `SuspendToken` that carries: - -```typescript -interface SuspendToken { - /** Unique ID for this permission interaction. */ - id: string; - /** Resume with the operator's decision. */ - resume(outcome: ApprovalOutcome | OperatorResult): void; - /** Cancel the interaction (auto-deny / auto-cancel). */ - cancel(reason: string): void; - /** Whether the interaction is still pending. */ - readonly pending: boolean; -} -``` - -The gate-wire creates a `SuspendToken` for each gate event, stores it in a -`Map`, and passes the token to the reactor's suspend -mechanism. The reactor calls `token.resume()` when the operator answers, and -`token.cancel()` on timeout or abort. - -This keeps the in-process behavior identical to today but gives the reactor an -explicit handle it can persist, serialize, or pass across process boundaries. - -### Phase 2: Message-bus integration (future, out of scope for v0.3.19) - -Once Phase 1 lands, permission requests can be serialized as `Channel` -messages. The reactor suspends with a token, the TUI overlay resumes it. -A future web or CI surface could do the same without in-process coupling. - -The serialized form would be: - -```typescript -interface SuspendMessage { - kind: "permission-suspend"; - token: string; // SuspendToken.id - request: PermissionRequest; -} -``` - -### Interaction with CL-7333 umbrella - -This RFC informs CL-5699 (adopt reactor's approval-suspend primitive) and -indirectly CL-5697 (vendor `@intx/inference` at HEAD, which ships the suspend -primitive in the reactor). No code changes in this RFC — it is a design doc. +Rationale: upstream deliberately separates "the call is parked" (reactor, +gate, persisted operation) from "the director decides what happens when +the gate clears" (resume dispatch reaches the director as a normal +event). Putting the queue in the director would duplicate the reactor's +park/bookkeeping role; putting it nowhere loses the operator surface. +The seam ownership follows the existing wiring: the session assembles the +gate dependencies, the reactor owns the suspend lifecycle, the director +only reacts to events. + +### (c) classify.ts tiering stays a pre-filter above authz grants + +**Decision:** `src/permission/classify.ts`'s allow/ask `Tier` +(`classify.ts:69`) remains a pre-filter that decides *whether and how* +the authz path is consulted; it does not become authz policy. Read-only +tools classify `allow` and short-circuit; everything else classifies +`ask` and flows through the authz grant path, where grants, denies, and +the suspend effect live. + +Rationale: the classifier encodes Corbits' tool-level defaults (which +tools are safe to auto-run) — knowledge that lives on our side of the +boundary and that upstream authz has no way to express. Authz grants +encode per-project operator intent (patterns, scopes, persistence). +Collapsing the two would either push Corbits tool defaults into +grant-matching (wrong layer, wrong persistence) or force the grant store +to re-implement tiering. Keeping the tier as a pre-filter preserves both +and gives the suspend primitive a clean trigger: `ask` tier is exactly +the condition under which the before-tool hook can return `suspend`. + +### (d) Headless denial and stricter command-deny re-home as block effects + +**Decision:** the two Corbits-only deny paths move into the authz +extension's before-tool hook as `block` effects, which upstream already +supports (`{ type: "block", reason }` is a first-class hook return in +`authz-extension.ts:205-208`): + +- **Headless denial** — today in `src/permission/gate.ts:592-600` and + `654-660` (note: `gate.ts` is 750 lines; the ticket's 625-line figure + is stale). When the run is non-interactive there is no operator to + approve, so instead of reaching the `ask` effect the hook returns + `block` with the existing denial reasons. +- **Stricter chained-command deny** — today + `src/permission/gate.ts:549-552`, owned by `preGrantGuardReason` + (`gate.ts:137-163`), which hard-denies chained shell commands whose + segments target restricted or sensitive paths even when a grant + exists. This stays a pre-grant guard but is expressed as a `block` + effect in the hook rather than gate-internal bookkeeping. + +Neither path has an upstream equivalent, so both are ours to carry; the +decision is only *where* they live. Putting them in the hook means the +gate's `ask` path is the only path that can suspend, and denial never +needs a parked operation, a correlation id, or a resume. + +Rationale: upstream's hook contract already distinguishes +allow / block / ask / suspend. Denial-without-interaction is exactly +`block`; approval-requiring is exactly `ask`→`suspend`. Re-homing keeps +`gate.ts`'s remaining job limited to interactive outcome routing while +the vendored reactor owns the lifecycle. + +## Transport: what exists, not `Channel` + +`@intx/types` has no `Channel`. An earlier draft of this RFC designed +a bus around one; that interface does not exist upstream or in our tree. +The real mechanisms are: + +- **Upstream:** the reserved signal channel — a suspended step parks on + `signalName(correlationId)` and resume is the reactor's signal-driven + dispatch (`packages/types/src/signals.ts:67`, + `packages/types/src/runtime.ts:663`, `reactor.ts:8-10, 68-72`). + This is the transport the suspend primitive is designed against, so it + is the one we adopt: gate clears are delivered by enqueueing a signal + on the correlation-id channel, and the reactor's existing resume + dispatch does the rest. +- **Ours:** the TUI-side runtime channels (`src/tui/runtime-channels`) + are EventEmitter-based — a display-plane mechanism, suited to + surfacing the pending operation to the operator, not to resuming a + parked reactor step. + +Decision: resume transport is the upstream signal channel (it is what +the reactor dispatches on); the EventEmitter runtime channels stay on +the display plane and carry the approval snapshot to the overlay. No new +message type is introduced. ## Alternatives considered -1. **Keep closure-based suspend, add try/catch wrapping.** Low effort but does - not solve durability or cross-process portability. The reactor's primitive - would go unused. +1. **Keep closure-based suspend.** Zero migration cost but leaves the + reactor's primitive unused, keeps `resolve()` pinned in memory, and + forfeits the persisted-`pendingOp` identity that any future restart + recovery needs. + +2. **Invent a `SuspendToken` with `resume()`/`cancel()` handed to the + reactor.** Rejected: corresponds to nothing upstream. The reactor's + resume is signal-driven dispatch; a callback-based token would be a + second resume mechanism racing the first. -2. **Serialize the full `PermissionRequest` into the context store.** Heavier - than needed — the token ID is sufficient to re-derive state from the queue. - The full request can be reconstructed from the queue's pending entries. +3. **Serialize the full `PermissionRequest` into the store "because the + queue can reconstruct it."** Rejected as stated: the queue is an + in-memory `Map` (`src/permission/queue.ts:38-41`), so after process + death there are no pending entries to reconstruct from, and + `src/permission/store.ts` persists grants only. Snapshot/restore of + pending operations is real future work, decided out of scope in (a). -3. **Skip Phase 1, jump straight to message-bus integration.** Risks a larger - blast radius; the Phase 1 token gives a clean seam for testing and - incremental migration. +4. **Skip adoption until a cross-process surface exists.** Rejected: the + closure-based path already breaks down on single-process restart, and + adoption is a prerequisite for CL-5699 regardless. ## Migration path -1. CL-5683 (this RFC) — design complete. -2. CL-5699 — implement `SuspendToken`, wire gate-wire to use it, pass tokens - to reactor. Gate-wire behavior unchanged from the operator's perspective. -3. Monitor adoption; Phase 2 only when a cross-process surface requests it. +1. CL-5683 (this RFC) — design complete; decisions (a)-(d) above. +2. CL-5699 — implement: route `ask`-tier calls through the authz hook's + suspend effect; persist `PendingOperation` via the existing + `optimized-context-store` surface; deliver gate clears on the + correlation-id signal channel; re-home the two deny paths as `block` + effects; narrow `requestApproval` to the operator-facing seam. + Gate-wire display behavior unchanged from the operator's perspective. + Explicitly out of scope: snapshot-and-restore of pending approvals + across restart. +3. Monitor adoption; revisit restart recovery as a separate ticket with + its own scope. ## Open questions -- Should `SuspendToken` be in `src/permission/` or `src/agent/`? The token is - created by the permission gate but consumed by the reactor. Leaning toward - `src/permission/suspend.ts` with a re-export from `src/agent/`. -- Should the token carry an optional serialized snapshot for process restart - recovery? Adds complexity; defer to Phase 2 unless a concrete use case - appears. +- Should the approval `approvalSnapshot` → TUI payload mapping live in + the gate-wire overlay or in a new adapter beside + `src/session/optimized-context-store.ts`? Leaning gate-wire, since it + already owns the pending-display overlay. +- Exact timeout policy: upstream defaults to one hour + (`DEFAULT_APPROVAL_TIMEOUT_MS`, `authz-extension.ts:36`); whether + unattended auto-continue runs should pass a shorter + `approvalTimeoutMs` per run. From 3721694618563cf8a5d2676472dd6d7589dfdb83 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Mon, 7 Sep 2026 20:19:59 -0700 Subject: [PATCH 3/4] Format the ask-authz suspend RFC with prettier --- docs/RFC-ask-authz-suspend.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/RFC-ask-authz-suspend.md b/docs/RFC-ask-authz-suspend.md index 42bb36ec1..5423f333e 100644 --- a/docs/RFC-ask-authz-suspend.md +++ b/docs/RFC-ask-authz-suspend.md @@ -68,7 +68,7 @@ already carries the primitive: - The before-tool authz hook returns, for an `ask` effect, `{ type: "suspend", gate: { type: "approval", gateId, correlationId, - timeoutAt }, pendingOp }` (`authz-extension.ts:263-267` upstream; the +timeoutAt }, pendingOp }` (`authz-extension.ts:263-267` upstream; the reactor persists `pendingOp`, minting `correlationId` at :223 and `gateId = pending-${correlationId}` at :226). - `DEFAULT_APPROVAL_TIMEOUT_MS = 3_600_000` (:36) — one hour. @@ -110,7 +110,7 @@ pending approvals is **not delivered and remains out of CL-5699 scope**. `src/permission/queue.ts` is an in-memory `Map` and `src/permission/store.ts` persists grants only, so a crashed session still loses the pending approval. The mapping above is what makes -recovery *possible later* (the persisted `pendingOperations` plus +recovery _possible later_ (the persisted `pendingOperations` plus `correlationId`-addressed resume), but wiring snapshot-and-restore is separate work and is not claimed by CL-5699. @@ -140,7 +140,7 @@ only reacts to events. ### (c) classify.ts tiering stays a pre-filter above authz grants **Decision:** `src/permission/classify.ts`'s allow/ask `Tier` -(`classify.ts:69`) remains a pre-filter that decides *whether and how* +(`classify.ts:69`) remains a pre-filter that decides _whether and how_ the authz path is consulted; it does not become authz policy. Read-only tools classify `allow` and short-circuit; everything else classifies `ask` and flows through the authz grant path, where grants, denies, and @@ -176,7 +176,7 @@ supports (`{ type: "block", reason }` is a first-class hook return in effect in the hook rather than gate-internal bookkeeping. Neither path has an upstream equivalent, so both are ours to carry; the -decision is only *where* they live. Putting them in the hook means the +decision is only _where_ they live. Putting them in the hook means the gate's `ask` path is the only path that can suspend, and denial never needs a parked operation, a correlation id, or a resume. From 9b910e19690bf535941b8592e2d128638b2f8dc0 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Mon, 7 Sep 2026 21:55:43 -0700 Subject: [PATCH 4/4] Anchor the suspend RFC's code cites to symbols Line-number cites into the runtime reflow as other branches merge; naming the owning functions and fields keeps the references findable. The TUI transport cite names modules that exist. --- docs/RFC-ask-authz-suspend.md | 57 +++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/docs/RFC-ask-authz-suspend.md b/docs/RFC-ask-authz-suspend.md index 5423f333e..4ad73a026 100644 --- a/docs/RFC-ask-authz-suspend.md +++ b/docs/RFC-ask-authz-suspend.md @@ -8,7 +8,8 @@ ## Summary The current permission gate parks tool calls on in-memory `resolve()` -closures (`src/tui/gate-events.ts:23-39`) held open by the gate-wire +closures (the `resolve` field of `PermissionGateEvent` in +`src/tui/gate-events.ts`) held open by the gate-wire overlay. This RFC decides how Corbits Code adopts the upstream reactor's approval-suspend primitive instead: a before-tool authz hook that returns a `suspend` effect carrying an approval gate and a persisted @@ -24,8 +25,10 @@ This RFC resolves the four decisions CL-5683 requires: `src/session/optimized-context-store.ts` — no parallel queue. - **(b)** Director ask-handling consumes the reactor's suspend action and `gate.cleared` resume dispatch; the director stops owning approval - queues. The wiring seam is `requestApproval` at - `src/session/assemble-runtime.ts:218,248`, not the director. + queues. The wiring seam is the `requestApproval` field of + `SessionGateArgs` and its pass into `createPermissionGate` in + `assembleSessionGate` (`src/session/assemble-runtime.ts`), + not the director. - **(c)** `src/permission/classify.ts` allow/ask tiering stays a pre-filter above authz grants; it is not authz policy. - **(d)** Headless denial and the stricter chained-command deny are @@ -39,17 +42,18 @@ CL-5699. ### Current architecture -- `PermissionGateEvent` (`src/tui/gate-events.ts:23-39`) carries a +- `PermissionGateEvent` (`src/tui/gate-events.ts`) carries a `PermissionRequest`, a `resolve(outcome)` callback, optional `timeoutMs`, and an `AbortSignal`. -- `OperatorGateEvent` (`src/tui/gate-events.ts:4-21`) is the question +- `OperatorGateEvent` (`src/tui/gate-events.ts`) is the question analogue. - The gate-wire overlay (`src/tui/gate-wire.ts`) connects these events to the TUI and holds `resolve()` open until the operator interacts; it also owns the pending-display overlay that serializes what the operator sees - (`src/tui/gate-wire.ts:235-273`). + (`wireGates` in `src/tui/gate-wire.ts`). - `src/permission/queue.ts` is an in-memory `Map` of concurrent pending - entries keyed by id (`src/permission/queue.ts:38-41`) used to reconcile + entries keyed by id (`createPermissionRequestQueue` in + `src/permission/queue.ts`) used to reconcile grants through the approval store; `src/permission/store.ts` persists grants only, not pending requests. @@ -99,7 +103,8 @@ signal channel, not by holding a callback. Rationale: today `src/session/optimized-context-store.ts` already persists `PendingOperation[]` from `@intx/types/runtime` -(`optimized-context-store.ts:9,178`), and upstream persists the operation +(the `pendingOperations` field of its `SessionMetadata`), and upstream +persists the operation precisely so the id survives a restart (comment at `authz-extension.ts:219-221`). Using that existing surface means the gate's bookkeeping collapses into one identity and one store instead of a @@ -119,12 +124,14 @@ separate work and is not claimed by CL-5699. **Decision:** director ask-handling consumes the reactor's suspend action and the `gate.cleared`-driven resume dispatch, and stops managing its own approval queue. The `requestApproval` hook stays wired where it is today -— `src/session/assemble-runtime.ts:218` (the dependency declaration) and -`:248` (the wiring into the approval persist/log plumbing) — and its job +— declared on `SessionGateArgs` and passed into `createPermissionGate` +by `assembleSessionGate` (both in `src/session/assemble-runtime.ts`) — +and its job narrows to feeding the operator-facing surface. The director never sees the gate itself; it sees outcomes only as tool-result text today -(`src/agent/director.ts:276` matches "Blocked by permission policy: -Operator declined:") and, after adoption, additionally sees the suspend +(`isOperatorDeclinedToolResult` in `src/agent/director.ts` matches +"Blocked by permission policy: Operator declined:") and, after adoption, +additionally sees the suspend as a parked tool call and the clear as a `reactor.gate.cleared` event it decides on. @@ -140,7 +147,7 @@ only reacts to events. ### (c) classify.ts tiering stays a pre-filter above authz grants **Decision:** `src/permission/classify.ts`'s allow/ask `Tier` -(`classify.ts:69`) remains a pre-filter that decides _whether and how_ +remains a pre-filter that decides _whether and how_ the authz path is consulted; it does not become authz policy. Read-only tools classify `allow` and short-circuit; everything else classifies `ask` and flows through the authz grant path, where grants, denies, and @@ -163,14 +170,15 @@ extension's before-tool hook as `block` effects, which upstream already supports (`{ type: "block", reason }` is a first-class hook return in `authz-extension.ts:205-208`): -- **Headless denial** — today in `src/permission/gate.ts:592-600` and - `654-660` (note: `gate.ts` is 750 lines; the ticket's 625-line figure - is stale). When the run is non-interactive there is no operator to +- **Headless denial** — today in the two `!interactive` deny branches of + `evaluate` in `src/permission/gate.ts`. When the run is non-interactive + there is no operator to approve, so instead of reaching the `ask` effect the hook returns `block` with the existing denial reasons. -- **Stricter chained-command deny** — today - `src/permission/gate.ts:549-552`, owned by `preGrantGuardReason` - (`gate.ts:137-163`), which hard-denies chained shell commands whose +- **Stricter chained-command deny** — today the + `runShellAuthzBlockReason` check in `evaluate`, owned by + `preGrantGuardReason` (both in `src/permission/gate.ts`), which + hard-denies chained shell commands whose segments target restricted or sensitive paths even when a grant exists. This stays a pre-grant guard but is expressed as a `block` effect in the hook rather than gate-internal bookkeeping. @@ -200,13 +208,15 @@ The real mechanisms are: is the one we adopt: gate clears are delivered by enqueueing a signal on the correlation-id channel, and the reactor's existing resume dispatch does the rest. -- **Ours:** the TUI-side runtime channels (`src/tui/runtime-channels`) - are EventEmitter-based — a display-plane mechanism, suited to +- **Ours:** the TUI gate wire (`src/tui/gate-wire.ts` and + `src/tui/gate-events.ts`, exercised end to end by the harness modules + under `tests/integration/`) is EventEmitter-based — a display-plane + mechanism, suited to surfacing the pending operation to the operator, not to resuming a parked reactor step. Decision: resume transport is the upstream signal channel (it is what -the reactor dispatches on); the EventEmitter runtime channels stay on +the reactor dispatches on); the EventEmitter gate-wire events stay on the display plane and carry the approval snapshot to the overlay. No new message type is introduced. @@ -224,7 +234,8 @@ message type is introduced. 3. **Serialize the full `PermissionRequest` into the store "because the queue can reconstruct it."** Rejected as stated: the queue is an - in-memory `Map` (`src/permission/queue.ts:38-41`), so after process + in-memory `Map` (`createPermissionRequestQueue` in + `src/permission/queue.ts`), so after process death there are no pending entries to reconstruct from, and `src/permission/store.ts` persists grants only. Snapshot/restore of pending operations is real future work, decided out of scope in (a).