diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b82c46f02..d9c244d18 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -415,7 +415,7 @@ tool call - **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). - **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`. - **gate** — Evaluates a call in order: catastrophic authorization denial, `skipPermissions`, then the normal tier, grant, and auto policies. Catastrophic commands remain denied under every mode. Skip mode, enabled for the process by `--dangerously-skip-permissions` or `--yolo` and persisted to the active settings source by TUI `/yolo` (machine-wide only for the default user-global source, not an explicit `--config` source), allows the call before normal policy evaluation. Therefore `--auto --yolo` behaves as yolo: auto-mode file-mutation asks and denials do not run. Otherwise, `allow`-tier passes; for `ask`-tier, the gate checks persisted approvals and then 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, `web_fetch`, `web_search`, `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 process-only `--dangerously-skip-permissions` / `--yolo` or TUI `/yolo` persisted in the active settings source (machine-wide only for the default user-global source), 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. Static secret-guard denies for path-keyed tools and authorization hard blocks still apply. Sensitive shell references require approval in normal and auto modes, but skip mode allows them without approval after catastrophic authorization checks. Mutating MCP and unknown built-ins are not blanket-allowed outside skip. Newly granted scopes are appended in memory and persisted. - - **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()`. + - **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. Idle rebuild waits on two signals. `inFlight` occupancy keeps the TUI busy across the overlay until the correlated resume is accepted (`message.received` / `message.correlated`) or a generation bump `settleAll`s the waiter. The reactor's message-run bracket (`message.run.started` to `message.run.ended`) stays open while the parked run suspends and while the resumed tool and next inference execute, so a deferred reload (for example, MCP tools landing while the overlay is up) runs only after the run reaches `reply`, `wait`, or `done`. Every agent build logs its reason (`initial`, `reload`, `interrupt`, `session-rotation`). 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()`. - **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. - **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. diff --git a/src/tui/runner/exit.test.ts b/src/tui/runner/exit.test.ts index 7427f3f0e..ebaa3a614 100644 --- a/src/tui/runner/exit.test.ts +++ b/src/tui/runner/exit.test.ts @@ -50,7 +50,11 @@ import { createCompactionLifecycle, } from "../../session/compaction-lifecycle.js"; import { COMPACTION_CONTINUATION_EVENT } from "../../agent/compaction.js"; -import type { RunnerServices, RunnerState } from "./state.js"; +import { + runWhileAgentBusy, + type RunnerServices, + type RunnerState, +} from "./state.js"; import type { InboundMessage } from "@intx/types/runtime"; import { createCredentialRecoveryState } from "./credential-recovery.js"; import { createReconnectRecoveryState } from "./reconnect-recovery.js"; @@ -873,6 +877,190 @@ describe("rebuild re-syncs idle-with-fleet while drained", () => { }); }); +describe("reload-if-idle waits for the reactor's message run", () => { + /** + * An agent whose stream the test feeds one event at a time; close() ends + * the stream so a rebuild's drain settles. + */ + function pushableStreamAgent(): { + agent: Agent; + push: (event: unknown) => void; + } { + const queued: unknown[] = []; + let wake: (() => void) | undefined; + let closed = false; + const signal = (): void => { + const resolve = wake; + wake = undefined; + resolve?.(); + }; + const agent: Agent = { + ...recordingAgent([]), + stream: async function* stream() { + while (!closed) { + const next = queued.shift(); + if (next !== undefined) { + yield next as never; + continue; + } + await new Promise((resolve) => { + wake = resolve; + }); + } + }, + close: async () => { + closed = true; + signal(); + }, + }; + return { + agent, + push: (event) => { + queued.push(event); + signal(); + }, + }; + } + + const runStarted = (messageRunId: string) => ({ + type: "message.run.started", + seq: 1, + data: { messageRunId, receivedAt: 0 }, + }); + const runEnded = (messageRunId: string) => ({ + type: "message.run.ended", + seq: 2, + data: { messageRunId, status: "completed" }, + }); + + const drainStream = (): Promise => + new Promise((resolve) => setTimeout(resolve, 0)); + + async function lifecycleWithOpenRun() { + const directorHolder: RunnerServices["directorHolder"] = {}; + const live = pushableStreamAgent(); + const { state, services } = stubSendLifecycle(live.agent); + wireRebuildServices(services, directorHolder, live.agent); + Object.assign(services, { + providerFailureAttempts: { + current: () => undefined, + advanceToNextMessage: () => undefined, + consumeTerminal: () => undefined, + reset: () => undefined, + }, + }); + let builds = 0; + services.buildAgent = (async () => { + builds += 1; + directorHolder.instance = createChatDirector("base", [], { + allowIdleWithFleet: true, + }); + return builds === 1 ? live.agent : recordingAgent([]); + }) as unknown as RunnerServices["buildAgent"]; + await createRunLifecycle(state, services); + // The operator's send opens the reactor's message run; an ask-tier call + // then parks it on the approval gate without closing it. + live.push(runStarted("run-1")); + await drainStream(); + return { state, services, push: live.push, builds: () => builds }; + } + + test("an approval-resumed run with a pending reload rebuilds only after the run ends", async () => { + const { state, services, push, builds } = await lifecycleWithOpenRun(); + expect(builds()).toBe(1); + // The submit path wraps send() and approvalResume.handle(); handle + // resolves once the decision is handed over, while the resumed run is + // still executing. MCP connect lands while the prompt is up. + await runWhileAgentBusy(state, async () => { + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + }); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(true); + // The resumed run reaches its terminal: the deferred reload runs now. + push(runEnded("run-1")); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(2); + expect(state.pendingReload).toBe(false); + expect(state.fatalBuildError).toBeNull(); + }); + + test("a run that ends into an immediately following run rebuilds only after the successor ends", async () => { + const { state, services, push, builds } = await lifecycleWithOpenRun(); + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + // An uncorrelated deliver dequeued while run-1 is still open: the + // reactor closes run-1 and opens run-2 with no await between them, so + // both land in the stream in one burst. + push(runEnded("run-1")); + push(runStarted("run-2")); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(true); + push(runEnded("run-2")); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(2); + expect(state.pendingReload).toBe(false); + expect(state.fatalBuildError).toBeNull(); + }); + + test("an interrupt that kills an open run does not strand a later reload", async () => { + const { state, services, builds } = await lifecycleWithOpenRun(); + defined(state.interrupt, "interrupt")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(2); + // The interrupted run never emits message.run.ended; the rebuilt agent + // is idle, so a reload requested now is not held behind it. + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(3); + }); + + test("a pending reload held for an open run does not survive interrupt", async () => { + const { state, services, builds } = await lifecycleWithOpenRun(); + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(true); + defined(state.interrupt, "interrupt")(); + await services.sessionOps.awaitTail(); + expect(state.pendingReload).toBe(false); + expect(builds()).toBe(2); + await runWhileAgentBusy(state, async () => undefined); + expect(builds()).toBe(2); + expect(state.pendingReload).toBe(false); + }); + + test("a pending reload held for an open run does not survive session rotation", async () => { + const { state, services, builds } = await lifecycleWithOpenRun(); + const dirs = stubRotationDirs(); + try { + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(true); + defined(state.newSession, "newSession")(); + await services.sessionOps.awaitTail(); + expect(state.pendingReload).toBe(false); + expect(builds()).toBe(2); + await runWhileAgentBusy(state, async () => undefined); + expect(builds()).toBe(2); + expect(state.pendingReload).toBe(false); + } finally { + for (const spy of dirs) spy.mockRestore(); + } + }); +}); + // An interrupt can hit close() mid-teardown and leak the workdir lock (see // closeAgentForRebuild's doc in exit.ts). These tests cover the two helpers // the runner routes every rebuild failure through, so that failure is diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index 4b8e4f123..8e26c2095 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -146,6 +146,8 @@ export async function closeAgentForRebuild( } } +type AgentBuildReason = "initial" | "reload" | "interrupt" | "session-rotation"; + // Directors seed allowIdleWithFleet=true because fleet lanes may appear // mid-session. Rebuilds re-sync from the live count: the publisher only // fires on a count change, so a session that never sees 0→1→0 keeps the seed. @@ -361,10 +363,19 @@ export async function createRunLifecycle( // Consume-once gate for the compaction continuation emit: a replayed // duplicate of an already-answered emission must not re-deliver. const continuationGate = createContinuationGate(); + // The reactor's per-message run bracket is the idle signal for rebuilds: + // a suspended send settles, and approvalResume.handle resolves at decision + // hand-over, while the parked run is still open and the resumed tool and + // next inference execute inside it. Only reply/wait/done (or a fatal + // abandon) close it. + const openMessageRuns = new Set(); const streamSink = ( event: Parameters[0], ): void => { let eventForSink = event; + if (event.type === "message.run.started") { + openMessageRuns.add(event.data.messageRunId); + } if (event.type === "message.received") { providerFailureAttempts.advanceToNextMessage(); } @@ -450,9 +461,33 @@ export async function createRunLifecycle( billingIdentityFromSource(event.data.source), ); } + if (event.type === "message.run.ended") { + openMessageRuns.delete(event.data.messageRunId); + state.reloadIfIdle?.(); + } }; - state.currentAgent = await services.buildAgent(); + const buildLiveAgent = async (reason: AgentBuildReason): Promise => { + tuiLogger.info("building agent: {reason}", { + reason, + sessionId: state.sessionId, + }); + // The outgoing agent's stream has drained; a run it left open (closed + // mid-run by interrupt or rotation) can never emit its end. + openMessageRuns.clear(); + // Those rebuilds already construct the agent a deferred reload was + // waiting on. Clear before the await so a connect that lands during + // buildAgent can still mark tools this build did not see. Initial and + // reload builds leave the flag to reloadIfIdle. + if (reason === "interrupt" || reason === "session-rotation") { + state.pendingReload = false; + } + state.currentAgent = await services.buildAgent(); + }; + const agentBusy = (): boolean => + state.inFlight > 0 || openMessageRuns.size > 0; + + await buildLiveAgent("initial"); resyncIdleWithFleetFlag(services); await persistRunSnapshot("running"); void resolveSessionLabel( @@ -471,9 +506,21 @@ export async function createRunLifecycle( const enqueueOp = services.sessionOps.enqueue; const reloadIfIdle = (): void => { - if (!state.pendingReload || state.inFlight > 0) return; + if (!state.pendingReload || agentBusy()) return; state.pendingReload = false; void enqueueOp(async () => { + // When the reactor dequeues a message it closes the previous run and + // opens the next without leaving the microtask queue, and the stream + // hands buffered events to the sink through resolved promises. Waiting + // one macrotask lets the sink apply a successor's message.run.started + // that is already on its way, so the check below sees it. + await new Promise((resolve) => setImmediate(resolve)); + // A delivery queued ahead of this op may have started a run since the + // check above; re-defer to that run's end instead of killing it. + if (agentBusy()) { + state.pendingReload = true; + return; + } try { const old = liveAgent(state); const closedCleanly = await closeAgentForRebuild(old, "reload"); @@ -483,7 +530,7 @@ export async function createRunLifecycle( if (!closedCleanly) { throw new AgentContextLockError(state.workdir); } - state.currentAgent = await services.buildAgent(); + await buildLiveAgent("reload"); resyncIdleWithFleetFlag(services); state.streamPromise = consumeStream( liveAgent(state).stream(), @@ -636,7 +683,7 @@ export async function createRunLifecycle( if (!closedCleanly) { throw new AgentContextLockError(state.workdir); } - state.currentAgent = await services.buildAgent(); + await buildLiveAgent("interrupt"); resyncIdleWithFleetFlag(services); services.cycleRecorder.reset(); state.streamPromise = consumeStream( @@ -742,7 +789,7 @@ export async function createRunLifecycle( // The rotated session's transcript never recorded the activations, so // its wire starts at the prefix and its run.json does not inherit them. services.activatedToolNames.clear(); - state.currentAgent = await services.buildAgent(); + await buildLiveAgent("session-rotation"); resyncIdleWithFleetFlag(services); services.cycleRecorder.reset(); state.streamPromise = consumeStream(