From b67ce45a6a7df551955578e1455d1d47641cc77c Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 00:43:13 -0400 Subject: [PATCH 1/6] fix(agent): keep the task list across a same-session agent rebuild --- src/session/assemble-runtime.test.ts | 31 ++++++++++++++++++++++++++++ src/session/assemble-runtime.ts | 7 +++++++ 2 files changed, 38 insertions(+) diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index 4d1d9d15a..9bb4c043c 100644 --- a/src/session/assemble-runtime.test.ts +++ b/src/session/assemble-runtime.test.ts @@ -740,6 +740,37 @@ describe("assembleChatAgent", () => { ); }); + test("a same-session rebuild keeps the task list; a new session starts empty", async () => { + let workdir = "/session-a"; + let sessionId = "session-a"; + const directorHolder: { instance?: ChatDirector } = {}; + const tasks = [ + { id: "1", title: "wire the reload", status: "doing" as const }, + { id: "2", title: "write the test", status: "todo" as const }, + ]; + + await withAssembledDirector( + { + getRecords: () => [], + directorHolder, + wiring: { + getWorkdir: () => workdir, + getSessionId: () => sessionId, + }, + }, + async (buildAgent) => { + await buildAgent(); + directorHolder.instance?.restoreTasks(tasks); + await buildAgent(); + expect(directorHolder.instance?.getTasks()).toEqual(tasks); + workdir = "/session-b"; + sessionId = "session-b"; + await buildAgent(); + expect(directorHolder.instance?.getTasks()).toEqual([]); + }, + ); + }); + test("failed /clear rebuild then retry on a new identity does not inherit extraInstructions", async () => { let records: ManifestRecord[] = [ pruningExtrasRecord("keep the auth discussion"), diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 4edfa320b..b46d5705a 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -641,6 +641,12 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { const fromPrev = inheritFromPrev ? directorHolder.instance?.getCompactInstructions() : undefined; + // Reload and interrupt rebuild the director mid-session; the task list + // lives only in director memory, so it would otherwise read empty until + // the model calls manage_tasks again. + const tasksFromPrev = inheritFromPrev + ? directorHolder.instance?.getTasks() + : undefined; const d = createChatDirector( agentCtx.systemPrompt, wiring.computeAdvertised([...agentCtx.toolDefinitions]), @@ -653,6 +659,7 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { ); d.restoreCompactInstructions(stickyFromStore); d.restoreCompactInstructions(fromPrev); + if (tasksFromPrev !== undefined) d.restoreTasks(tasksFromPrev); if (wiring.clearDenials !== undefined) d.setClearDenials(wiring.clearDenials); directorHolder.instance = d; From bbcedbeceb8c2f2f4faf98428f0d8ecce40ece3b Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 00:44:14 -0400 Subject: [PATCH 2/6] fix(tui): hold agent reload while a rehydrated approval resumes --- docs/ARCHITECTURE.md | 2 +- src/tui/runner/exit.test.ts | 45 +++++++++++++++++++++++++++++++++---- src/tui/runner/exit.ts | 22 +++++++++++++++++- 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d9c244d18..0fc41c747 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. 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()`. + - **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 three 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`. A decision delivered to a reactor that rehydrated the parked approval from disk resumes with no run open, so the TUI also holds rebuilds from that `message.correlated` until the reactor announces a terminal (`connector.reply`, a run end, a fatal `reactor.error`, or `reactor.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 d409bf542..bfb04f00c 100644 --- a/src/tui/runner/exit.test.ts +++ b/src/tui/runner/exit.test.ts @@ -937,6 +937,15 @@ describe("reload-if-idle waits for the reactor's message run", () => { new Promise((resolve) => setTimeout(resolve, 0)); async function lifecycleWithOpenRun() { + const lifecycle = await idleLifecycle(); + // The operator's send opens the reactor's message run; an ask-tier call + // then parks it on the approval gate without closing it. + lifecycle.push(runStarted("run-1")); + await drainStream(); + return lifecycle; + } + + async function idleLifecycle() { const directorHolder: RunnerServices["directorHolder"] = {}; const live = pushableStreamAgent(); const { state, services } = stubSendLifecycle(live.agent); @@ -946,6 +955,7 @@ describe("reload-if-idle waits for the reactor's message run", () => { current: () => undefined, advanceToNextMessage: () => undefined, consumeTerminal: () => undefined, + consumeConnectorReply: () => undefined, reset: () => undefined, }, }); @@ -958,10 +968,6 @@ describe("reload-if-idle waits for the reactor's message run", () => { 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 }; } @@ -1010,6 +1016,37 @@ describe("reload-if-idle waits for the reactor's message run", () => { expect(state.fatalBuildError).toBeNull(); }); + test("an approval resumed with no open run defers the reload until its reply", async () => { + // A reactor that rehydrated a parked approval from disk resumes it on the + // correlated decision without opening a message run, so no run bracket + // covers the re-dispatched tool or the inference that follows it. + const { state, services, push, builds } = await idleLifecycle(); + push({ + type: "message.correlated", + seq: 3, + data: { message: {}, correlationId: "corr-1" }, + }); + await drainStream(); + state.pendingReload = true; + defined(state.reloadIfIdle, "reloadIfIdle")(); + await services.sessionOps.awaitTail(); + push({ + type: "tool.done", + seq: 4, + data: { call: { id: "c1", name: "mcp__x__y", arguments: {} } }, + }); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(true); + push({ type: "connector.reply", seq: 5, data: { content: "done" } }); + 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")(); diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index 4559bc410..b3f2b7265 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -369,6 +369,13 @@ export async function createRunLifecycle( // next inference execute inside it. Only reply/wait/done (or a fatal // abandon) close it. const openMessageRuns = new Set(); + // A reactor that rehydrated a parked approval from disk resumes it on the + // correlated decision without opening a message run, so no bracket covers + // the re-dispatched tool or the inference after it. Hold rebuilds from + // that correlation until the reactor reaches a terminal it announces: a + // reply, a run end, a fatal error, or shutdown. A bare `wait` terminal + // emits nothing, so the hold then lasts until the next run ends. + let resumingOutsideRun = false; const streamSink = ( event: Parameters[0], ): void => { @@ -376,6 +383,9 @@ export async function createRunLifecycle( if (event.type === "message.run.started") { openMessageRuns.add(event.data.messageRunId); } + if (event.type === "message.correlated" && openMessageRuns.size === 0) { + resumingOutsideRun = true; + } if (event.type === "message.received") { providerFailureAttempts.advanceToNextMessage(); } @@ -461,8 +471,17 @@ export async function createRunLifecycle( billingIdentityFromSource(event.data.source), ); } + if ( + event.type === "connector.reply" || + event.type === "reactor.done" || + (event.type === "reactor.error" && event.data.fatal) + ) { + resumingOutsideRun = false; + state.reloadIfIdle?.(); + } if (event.type === "message.run.ended") { openMessageRuns.delete(event.data.messageRunId); + resumingOutsideRun = false; state.reloadIfIdle?.(); } }; @@ -475,10 +494,11 @@ export async function createRunLifecycle( // 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(); + resumingOutsideRun = false; state.currentAgent = await services.buildAgent(); }; const agentBusy = (): boolean => - state.inFlight > 0 || openMessageRuns.size > 0; + state.inFlight > 0 || openMessageRuns.size > 0 || resumingOutsideRun; await buildLiveAgent("initial"); resyncIdleWithFleetFlag(services); From 5af4408621c5d319665cb240575b47202ebff368 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 00:46:13 -0400 Subject: [PATCH 3/6] fix(tui): rebuild the agent whenever the MCP tool set changes --- docs/ARCHITECTURE.md | 2 +- docs/MCP.md | 7 ++++ src/tui/runner/exit.test.ts | 69 +++++++++++++++++++++++++++++++++++++ src/tui/runner/exit.ts | 12 +++++-- src/tui/runner/mcp.ts | 9 +++-- src/tui/runner/session.ts | 4 --- src/tui/runner/state.ts | 4 ++- src/tui/runner/wiring.ts | 13 ++----- 8 files changed, 97 insertions(+), 23 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0fc41c747..904574cc6 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. Idle rebuild waits on three 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`. A decision delivered to a reactor that rehydrated the parked approval from disk resumes with no run open, so the TUI also holds rebuilds from that `message.correlated` until the reactor announces a terminal (`connector.reply`, a run end, a fatal `reactor.error`, or `reactor.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()`. + - **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 three 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`. A decision delivered to a reactor that rehydrated the parked approval from disk resumes with no run open, so the TUI also holds rebuilds from that `message.correlated` until the reactor announces a terminal (`connector.reply`, a run end, a fatal `reactor.error`, or `reactor.done`). Every agent build logs its reason (`initial`, `mcp-tools-changed`, `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/docs/MCP.md b/docs/MCP.md index 11e3813de..cb208cd70 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -128,6 +128,13 @@ file for that HTTP endpoint. Built-in Exa is disable-only — Alt+R notifies and does not open the confirm. While a local `.corbits/settings.json` `mcpServers` list is in effect, these actions edit that local file; add remains blocked. +The agent fixes the tool set it can authorize when it is built, so every +change to the connected tool set (startup connect, add, retry, enable, +disable, remove) rebuilds the TUI agent once no turn is running. A turn in +progress, including an approval being resumed, finishes on the agent that +started it; the rebuild follows, and the log records it as +`mcp-tools-changed`. + ## Dropped Transports Reconnect in the Background Stdio and HTTP do not share a redial. A stdio child that exits is respawned diff --git a/src/tui/runner/exit.test.ts b/src/tui/runner/exit.test.ts index bfb04f00c..bfcec2214 100644 --- a/src/tui/runner/exit.test.ts +++ b/src/tui/runner/exit.test.ts @@ -57,6 +57,8 @@ import { } from "./state.js"; import type { InboundMessage } from "@intx/types/runtime"; import { createCredentialRecoveryState } from "./credential-recovery.js"; +import { wireMcp } from "./mcp.js"; +import type { MCPConnectCallbacks } from "../../agent/tools.js"; import { createReconnectRecoveryState } from "./reconnect-recovery.js"; import type { ProviderFailureAttempt } from "../provider/failure-attempt.js"; @@ -1047,6 +1049,73 @@ describe("reload-if-idle waits for the reactor's message run", () => { expect(state.fatalBuildError).toBeNull(); }); + /** Late MCP connects that mount tools and report the change. */ + function wireLateMcp( + state: RunnerState, + services: RunnerServices, + ): ReturnType { + const connected = async ( + server: { name: string }, + callbacks: MCPConnectCallbacks, + ): Promise => { + callbacks.onStatus({ + name: server.name, + state: "connected", + tools: ["list_issues"], + }); + callbacks.onToolsChanged([]); + }; + Object.assign(services, { + mcpStates: new Map(), + mcpConnectController: new AbortController(), + computeAdvertised: () => [], + toolset: { + ...services.toolset, + connectMCPServer: connected, + retryMCPServer: connected, + }, + }); + state.configuredMcpEntries = []; + return wireMcp(state, services); + } + + const buildReasons = (info: ReturnType): unknown[] => + (info.mock.calls as unknown[][]) + .filter(([message]) => message === "building agent: {reason}") + .map(([, fields]) => (fields as { reason: unknown }).reason); + + test("an MCP server connected mid-run rebuilds once after the run ends, logging why", async () => { + const info = spyOn(getLogger([LOG_NAMESPACE_ROOT, "tui"]), "info"); + try { + const { state, services, push, builds } = await lifecycleWithOpenRun(); + const mcp = wireLateMcp(state, services); + mcp.connectLateMCPServer({ name: "linear", url: "https://l.test/mcp" }); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + push(runEnded("run-1")); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(2); + expect(buildReasons(info)).toEqual(["initial", "mcp-tools-changed"]); + } finally { + info.mockRestore(); + } + }); + + test("an MCP retry while idle rebuilds the agent", async () => { + const { state, services, builds } = await idleLifecycle(); + const mcp = wireLateMcp(state, services); + state.config = { + ...state.config, + mcpServers: [{ name: "linear", url: "https://l.test/mcp" }], + }; + await mcp.surface.retryServer("linear"); + await drainStream(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(2); + }); + 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")(); diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index b3f2b7265..58fe5246d 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -146,7 +146,11 @@ export async function closeAgentForRebuild( } } -type AgentBuildReason = "initial" | "reload" | "interrupt" | "session-rotation"; +type AgentBuildReason = + | "initial" + | "mcp-tools-changed" + | "interrupt" + | "session-rotation"; // Directors seed allowIdleWithFleet=true because fleet lanes may appear // mid-session. Rebuilds re-sync from the live count: the publisher only @@ -543,7 +547,7 @@ export async function createRunLifecycle( if (!closedCleanly) { throw new AgentContextLockError(state.workdir); } - await buildLiveAgent("reload"); + await buildLiveAgent("mcp-tools-changed"); resyncIdleWithFleetFlag(services); state.streamPromise = consumeStream( liveAgent(state).stream(), @@ -562,6 +566,10 @@ export async function createRunLifecycle( }); }; state.reloadIfIdle = reloadIfIdle; + state.requestToolSetRebuild = (): void => { + state.pendingReload = true; + reloadIfIdle(); + }; // Search loads top hits onto the next infer's tail; promote-on-execute // still declares a called name outside that prefix, at the cost of cache diff --git a/src/tui/runner/mcp.ts b/src/tui/runner/mcp.ts index 389b08c01..326433b52 100644 --- a/src/tui/runner/mcp.ts +++ b/src/tui/runner/mcp.ts @@ -59,11 +59,10 @@ export function wireMcp( void state.persistRunSnapshot?.("running"); } }, - // MCP tools register for dispatch but stay blind until tool_search promotes them. - onToolsChanged: (definitions) => - services.directorHolder.instance?.updateToolDefinitions( - services.computeAdvertised(definitions), - ), + // Connect, add, retry, enable, disable, and remove all land here. The + // reactor authorizes against the tool set the agent was built with, so a + // change reaches it only through a rebuild, never by editing the live one. + onToolsChanged: () => state.requestToolSetRebuild?.(), }; const connectLateMCPServer = (server: MCPServerConfig): void => { diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 86b5c46c8..d5ad07423 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -809,9 +809,6 @@ export async function assembleTUISession( // recorder keeps the in-flight cycle's text so an errored or interrupted // turn leaves its partial output in partial.jsonl instead of vanishing. const providerFailureAttempts = createProviderFailureAttemptTracker(); - // Tool count before any MCP server connects; a reload is only worthwhile if - // connecting actually added tools. - const baseToolCount = toolset.dynamicRunner.currentDefinitions().length; return { emitter, @@ -852,7 +849,6 @@ export async function assembleTUISession( correlationAcceptance, buildSessionSources, providerFailureAttempts, - baseToolCount, mcpStates, mcpConnectController, }; diff --git a/src/tui/runner/state.ts b/src/tui/runner/state.ts index 941769d05..286a1a200 100644 --- a/src/tui/runner/state.ts +++ b/src/tui/runner/state.ts @@ -190,7 +190,6 @@ export interface RunnerServices { providerFailureAttempts: ReturnType< typeof import("../provider/failure-attempt.js").createProviderFailureAttemptTracker >; - baseToolCount: number; mcpStates: Map; mcpConnectController: AbortController; } @@ -231,6 +230,8 @@ export interface RunnerState { // failed build poisons every later send instead of dispatching to a // closed agent. inFlight: number; + // Set when the connected MCP tool set changed: the agent fixes its tool set + // at build, so the change reaches it only through a rebuild once idle. pendingReload: boolean; fatalBuildError: Error | null; // Tracked live so compaction always summarizes with the current model @@ -293,6 +294,7 @@ export interface RunnerState { // because tests build partial states without session assembly. compactionLifecycle?: CompactionLifecycle; reloadIfIdle?: () => void; + requestToolSetRebuild?: () => void; systemNotice?: (text: string) => void; currentAttemptIdentity?: () => InferenceAttemptIdentity; handleSendFailure?: ( diff --git a/src/tui/runner/wiring.ts b/src/tui/runner/wiring.ts index c96ff275d..e818a6335 100644 --- a/src/tui/runner/wiring.ts +++ b/src/tui/runner/wiring.ts @@ -577,19 +577,12 @@ export function wirePostStartup( }); // Connect MCP after the TUI is up so auth surfaces as a copyable link, not - // a browser pop; tools land on the live runner and stay unadvertised until - // tool_search promotes them. On settle, reload-if-idle so construction-time - // maps match, then resume any persisted workflow. Aborted on exit. + // a browser pop; each server that mounts tools requests an agent rebuild + // through its callbacks. On settle, resume any persisted workflow. Aborted + // on exit. void services.toolset .connectMCP(mcpConnectCallbacks, services.mcpConnectController.signal) .then(async () => { - if ( - services.toolset.dynamicRunner.currentDefinitions().length > - services.baseToolCount - ) { - state.pendingReload = true; - state.reloadIfIdle?.(); - } // Capability map reflects connected servers; restore any persisted workflow. await services.workflowHost.resume(); }) From 7d858b736e06b673849c7201e17909f9b202c894 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 00:50:22 -0400 Subject: [PATCH 4/6] fix(agent): keep late MCP tools off the agent until it rebuilds The reactor authorizes against the tool definitions it snapshotted at build. A tool mounted later was still found by tool_search and could reach the wire, but an ask on it had no definition to build an approval prompt from and failed as a wiring defect. The TUI now pins the tool set at each build: search, the advertised wire, and authorization only see pinned tools, a late hit tells the model it becomes callable next turn, and a rebuild with nothing new to pin is skipped. --- docs/MCP.md | 8 +-- src/agent/dynamic-tool-runner.test.ts | 68 ++++++++++++++++++++++++ src/agent/dynamic-tool-runner.ts | 74 +++++++++++++++++++++++++++ src/agent/late-tool-authorize.ts | 26 ++++++++++ src/agent/tool-search.test.ts | 15 ++++++ src/agent/tool-search.ts | 18 ++++++- src/agent/tools.ts | 25 ++++++--- src/session/assemble-runtime.ts | 34 +++++++++--- src/tui/runner/exit.test.ts | 27 +++++++++- src/tui/runner/exit.ts | 16 +++++- src/tui/runner/session.ts | 5 +- src/tui/runner/settings.ts | 2 +- 12 files changed, 295 insertions(+), 23 deletions(-) create mode 100644 src/agent/late-tool-authorize.ts diff --git a/docs/MCP.md b/docs/MCP.md index cb208cd70..ab294bf40 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -90,9 +90,11 @@ present. The built-in Exa default is still injected beside local MCP unless a global or local `exa` entry disables or overrides it as described above. Tools from connected servers are not advertised to the model up front; they are -registered for dispatch as soon as the server connects (including later in the -same turn) and surfaced on demand through dynamic tool discovery -(`tool_search`). A match is a ranked handful of names plus capped +surfaced on demand through dynamic tool discovery (`tool_search`). In the TUI a +server that connects mid-turn joins at the agent rebuild after that turn: until +then `tool_search` reports its matching tools as callable next turn, and a call +to one is denied with the same hint rather than reaching a prompt that cannot +show it. A match is a ranked handful of names plus capped descriptions. The top five cards also include a compact input schema and are loaded onto the next inference's advertised tail (Codex/Pi deferred) and into `run.json`. Remaining hits stay name + description until called diff --git a/src/agent/dynamic-tool-runner.test.ts b/src/agent/dynamic-tool-runner.test.ts index 5d1edc103..8d4fbdd3f 100644 --- a/src/agent/dynamic-tool-runner.test.ts +++ b/src/agent/dynamic-tool-runner.test.ts @@ -374,3 +374,71 @@ describe("terminal control stripping", () => { expect(result.content).toBe("beforeredafter"); }); }); + +describe("agent tool set pin", () => { + const names = (runner: ReturnType) => + runner.agentDefinitions().map((definition) => definition.name); + + test("an unpinned runner exposes every registered tool", () => { + const runner = createDynamicToolRunner([stringTool("read_file", "core")]); + runner.addTools([stringTool("mcp__acme__do", "late")]); + expect(names(runner)).toEqual(["read_file", "mcp__acme__do"]); + expect(runner.isLateTool("mcp__acme__do")).toBe(false); + expect(runner.agentToolSetDrift()).toEqual({ + added: [], + removed: [], + changed: [], + }); + }); + + test("tools mounted after the pin stay hidden until the next pin", () => { + const runner = createDynamicToolRunner([ + stringTool("read_file", "core"), + stringTool("mcp__old__go", "old"), + ]); + runner.pinAgentToolSet(); + runner.addTools([stringTool("mcp__acme__do", "late")]); + runner.removeTools(["mcp__old__go"]); + expect(names(runner)).toEqual(["read_file"]); + expect(runner.isLateTool("mcp__acme__do")).toBe(true); + // Harness-namespaced spellings resolve to the same late tool. + expect(runner.isLateTool("default.mcp__acme__do")).toBe(true); + expect(runner.isLateTool("read_file")).toBe(false); + expect(runner.isLateTool("mcp__never__mounted")).toBe(false); + expect(runner.agentToolSetDrift()).toEqual({ + added: ["mcp__acme__do"], + removed: ["mcp__old__go"], + changed: [], + }); + + runner.pinAgentToolSet(); + expect(names(runner)).toEqual(["read_file", "mcp__acme__do"]); + expect(runner.isLateTool("mcp__acme__do")).toBe(false); + expect(runner.agentToolSetDrift()).toEqual({ + added: [], + removed: [], + changed: [], + }); + }); + + test("a remount under the same name with a new schema counts as drift", () => { + const runner = createDynamicToolRunner([stringTool("web_fetch", "a")]); + runner.pinAgentToolSet(); + runner.removeTools(["web_fetch"]); + runner.addTools([ + { + ...stringTool("web_fetch", "b"), + definition: { + name: "web_fetch", + description: "fetch through exa", + inputSchema: { type: "object", properties: {}, required: [] }, + }, + }, + ]); + expect(runner.agentToolSetDrift()).toEqual({ + added: [], + removed: [], + changed: ["web_fetch"], + }); + }); +}); diff --git a/src/agent/dynamic-tool-runner.ts b/src/agent/dynamic-tool-runner.ts index 4d42ec1e8..88be1d360 100644 --- a/src/agent/dynamic-tool-runner.ts +++ b/src/agent/dynamic-tool-runner.ts @@ -47,8 +47,32 @@ export type DynamicToolRunner = AgentToolRunner & { * director-side triggers. */ setOnUndeclaredCall(handler: (name: string) => void): void; + /** + * Record the current tool set as the one the agent being built can + * authorize. The reactor snapshots definitions at build, so a tool mounted + * later would reach dispatch with no approval snapshot. Until the next pin, + * `agentDefinitions` hides such tools and `isLateTool` names them. A runner + * that is never pinned exposes every registered tool. + */ + pinAgentToolSet(): void; + // Registered tools the pinned agent can authorize (all of them unpinned). + agentDefinitions(): ToolDefinition[]; + // A name that resolves to a registered tool the pinned agent lacks. + isLateTool(name: string): boolean; + // How the registry moved since the last pin; empty when never pinned. + agentToolSetDrift(): AgentToolSetDrift; }; +interface AgentToolSetDrift { + added: string[]; + removed: string[]; + changed: string[]; +} + +// Description and schema are what the approval snapshot and the wire carry. +const definitionShape = (definition: ToolDefinition): string => + `${definition.description ?? ""}|${JSON.stringify(definition.inputSchema ?? null)}`; + export function createDynamicToolRunner( initial: AgentTool[], watchdogConfig?: ToolWatchdogConfig, @@ -57,6 +81,9 @@ export function createDynamicToolRunner( let callGate: ((name: string) => boolean) | undefined; let isActivated: ((name: string) => boolean) | undefined; let onUndeclaredCall: ((name: string) => void) | undefined; + // Pinned while the agent's tool resolution walks the bundle, where the + // live-dispatch shim swaps the global Map; arrays and a Set stay clear of it. + let pinned: { names: Set; definitions: ToolDefinition[] } | undefined; const addTools = (tools: AgentTool[]): void => { const incoming = new Set(); @@ -78,6 +105,43 @@ export function createDynamicToolRunner( const currentDefinitions = (): ToolDefinition[] => [...byName.values()].map((t) => t.definition); + const agentDefinitions = (): ToolDefinition[] => { + const live = currentDefinitions(); + if (pinned === undefined) return live; + const names = pinned.names; + return live.filter((definition) => names.has(definition.name)); + }; + + const isLateTool = (name: string): boolean => { + if (pinned === undefined) return false; + const resolved = resolveRegisteredToolName(name, (n) => byName.has(n)); + return resolved !== undefined && !pinned.names.has(resolved); + }; + + const agentToolSetDrift = (): AgentToolSetDrift => { + if (pinned === undefined) return { added: [], removed: [], changed: [] }; + const pinnedShapes = new Map( + pinned.definitions.map((definition) => [ + definition.name, + definitionShape(definition), + ]), + ); + const live = currentDefinitions(); + const liveNames = new Set(live.map((definition) => definition.name)); + return { + added: live + .filter((definition) => !pinnedShapes.has(definition.name)) + .map((definition) => definition.name), + removed: [...pinnedShapes.keys()].filter((name) => !liveNames.has(name)), + changed: live + .filter((definition) => { + const before = pinnedShapes.get(definition.name); + return before !== undefined && before !== definitionShape(definition); + }) + .map((definition) => definition.name), + }; + }; + return { get definitions(): readonly ToolDefinition[] { return currentDefinitions(); @@ -85,6 +149,16 @@ export function createDynamicToolRunner( addTools, removeTools, currentDefinitions, + pinAgentToolSet(): void { + const definitions = currentDefinitions(); + pinned = { + names: new Set(definitions.map((definition) => definition.name)), + definitions, + }; + }, + agentDefinitions, + isLateTool, + agentToolSetDrift, setCallGate( isCallable: (name: string) => boolean, options?: { isActivated?: (name: string) => boolean }, diff --git a/src/agent/late-tool-authorize.ts b/src/agent/late-tool-authorize.ts new file mode 100644 index 000000000..e80b10994 --- /dev/null +++ b/src/agent/late-tool-authorize.ts @@ -0,0 +1,26 @@ +import type { AuthorizeFn } from "@intx/agent"; + +// The reactor authorizes against the definitions it snapshotted at build. A +// tool mounted later has none there, so an `ask` on it cannot build an +// approval prompt and an `allow` would run a tool the agent never offered. +// Deny it with a reason the model can act on; the rebuild that follows the +// current turn makes it callable. +export function denyLateTools( + authorize: AuthorizeFn, + isLateTool: (name: string) => boolean, +): AuthorizeFn { + return async (resource, action, context) => { + const name = resource.startsWith("tool:") ? resource.slice(5) : undefined; + if (name !== undefined && isLateTool(name)) { + return { + effect: "deny", + matchingGrants: [], + resolvedBy: null, + reason: + `${name} connected after this turn started and becomes ` + + `callable once the turn ends. Finish the turn without it.`, + }; + } + return authorize(resource, action, context); + }; +} diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index 496563d0f..bf380feb4 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -374,6 +374,21 @@ async function advanceAndFlush(ms: number): Promise { } describe("createToolSearchTool", () => { + test("a hit only among late tools names them for the next turn without promoting", async () => { + const promoted: string[][] = []; + const tool = createToolSearchTool({ + search: () => [], + searchLate: () => ["mcp__linear__save_issue"], + lookup: () => undefined, + promote: (names) => promoted.push(names), + }); + const out = await call(tool, { query: "linear issue" }); + expect(out).toContain("mcp__linear__save_issue"); + expect(out).toContain("callable once it ends"); + expect(listedToolNames(out)).toEqual([]); + expect(promoted).toEqual([]); + }); + test("lists matching cards and does not promote", async () => { const tool = createToolSearchTool({ search: (q) => index.search(q), diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 8b0d19295..49c8dc3df 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -362,6 +362,9 @@ export interface ToolSearchDeps { // handler re-races this so a stuck dependency (hung OAuth) cannot hang the // call. Omitted callers have none. awaitPendingConnections?: (timeoutMs?: number) => Promise; + // Registered tools the live agent was built without; they become callable + // after the turn ends, so a hit here is a "next turn" notice, not a card. + searchLate?: (query: string, limit: number) => string[]; // True when a reconnecting MCP server holds tools that could match // `query`, justifying one short extra wait for the redial to remount them. // Only transport-death reconnects qualify; needs-auth settles @@ -524,7 +527,11 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { ); names = deps.search(query, limit); } - if (names.length === 0 && (stillPending ?? 1) > 0) { + if ( + names.length === 0 && + (stillPending ?? 1) > 0 && + (deps.searchLate?.(query, limit) ?? []).length === 0 + ) { const detail = stillPending === undefined ? "a connector may still be starting up" @@ -534,6 +541,15 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { return `No tools matched "${query}" yet — ${detail}. Retry this search shortly.${skillBlock}`; } } + const late = + names.length === 0 ? (deps.searchLate?.(query, limit) ?? []) : []; + if (late.length > 0) { + return ( + `Tools matching "${query}" connected during this turn and become ` + + `callable once it ends: ${late.join(", ")}. Finish this turn ` + + `without them.${skillBlock}` + ); + } if (names.length === 0 && skillLines.length > 0) { return `No tools matched "${query}".${skillBlock}`; } diff --git a/src/agent/tools.ts b/src/agent/tools.ts index c950a3959..22a98521d 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -777,13 +777,25 @@ export async function createAgentToolset( }), ]; - // tool_search ranks over the live runner (set below). Top hits load onto the - // next infer via the session promoter; remaining cards stay - // name+description until call. + // tool_search ranks over the tools the live agent can authorize (set + // below). Top hits load onto the next infer via the session promoter; + // remaining cards stay name+description until call. Tools mounted after + // the agent was built only surface as a "next turn" notice. const runnerHolder: { current?: DynamicToolRunner } = {}; const promoterHolder: { current?: (names: string[]) => void } = {}; const toolIndex = createToolIndex( - () => runnerHolder.current?.currentDefinitions() ?? [], + () => runnerHolder.current?.agentDefinitions() ?? [], + advertisedBuiltIns, + args.toolSearchAllow, + ); + const lateToolIndex = createToolIndex( + () => { + const runner = runnerHolder.current; + if (runner === undefined) return []; + return runner + .currentDefinitions() + .filter((definition) => runner.isLateTool(definition.name)); + }, advertisedBuiltIns, args.toolSearchAllow, ); @@ -796,11 +808,10 @@ export async function createAgentToolset( baseTools.push( createToolSearchTool({ search: (query, limit) => toolIndex.search(query, limit), + searchLate: (query, limit) => lateToolIndex.search(query, limit), searchSkills: (query) => searchSkillCatalog(skills, query), lookup: (name) => - runnerHolder.current - ?.currentDefinitions() - .find((d) => d.name === name), + runnerHolder.current?.agentDefinitions().find((d) => d.name === name), promote: (names) => promoterHolder.current?.(names), // Misses wait briefly for in-flight MCP handshakes (bounded, so hung // OAuth cannot hang the call) and re-search before answering. diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index b46d5705a..35556302b 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -59,6 +59,7 @@ import { createChatDirector, type ChatDirector } from "../agent/director.js"; import { createDoomLoopCorrectiveNote } from "../agent/doom-loop-note.js"; import type { AgentToolset } from "../agent/tools.js"; import { createAgentWithLiveToolDispatch } from "../agent/live-tool-dispatch.js"; +import { denyLateTools } from "../agent/late-tool-authorize.js"; import { createSessionStores } from "./optimized-context-store.js"; import { getActiveRun } from "./active-run.js"; import { createAttachmentRehydrateTransform } from "./attachment-store.js"; @@ -530,6 +531,13 @@ export interface ChatAgentWiring { agentId: string; systemPrompt: string; getDynamicRunner: () => AgentToolset["dynamicRunner"]; + /** + * Pin the runner's tool set at each build so tools mounted afterwards stay + * hidden and unauthorizable until the next build. Set by hosts that + * rebuild when the tool set changes (the TUI); exec builds once and keeps + * every late tool reachable. + */ + pinToolSetAtBuild?: boolean; computeAdvertised: (all: readonly ToolDefinition[]) => ToolDefinition[]; inactivityTimeoutMs: number; totalTimeoutMs?: number | undefined; @@ -670,7 +678,11 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { const toolsFactory = defineTool({ id: wiring.toolsId, definitions: [], - factory: () => withAuthzParityDefinitions(wiring.getDynamicRunner()), + factory: () => { + const runner = wiring.getDynamicRunner(); + if (wiring.pinToolSetAtBuild === true) runner.pinAgentToolSet(); + return withAuthzParityDefinitions(runner); + }, }); const provider = wiring.getProvider(); @@ -750,6 +762,13 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { }, }; stickyFromStore = await stickyExtraInstructionsFromStore(storageForAgent); + const archivedAuthorize = + primaryArchive === undefined + ? wiring.authorize + : wrapAuthorizeWithEvidenceArchive( + wiring.authorize, + () => primaryArchive, + ); const agent = await createAgentWithLiveToolDispatch(agentDef, { sources: wiring.getSources(), defaultSource: wiring.getDefaultSource(), @@ -767,7 +786,7 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { doomLoopPolicy: "fail-run", doomLoopCorrectiveNote: createDoomLoopCorrectiveNote(() => wiring - .computeAdvertised(wiring.getDynamicRunner().currentDefinitions()) + .computeAdvertised(wiring.getDynamicRunner().agentDefinitions()) .map((def) => def.name), ), contextTransforms: [ @@ -794,12 +813,11 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { // finalize evidence admission after guards resolve, never scrubbing // exec args. authorize: - primaryArchive === undefined - ? wiring.authorize - : wrapAuthorizeWithEvidenceArchive( - wiring.authorize, - () => primaryArchive, - ), + wiring.pinToolSetAtBuild === true + ? denyLateTools(archivedAuthorize, (name) => + wiring.getDynamicRunner().isLateTool(name), + ) + : archivedAuthorize, directors: createDirectorRegistry({ factories: [chatDirectorDef.factory], defaultId: `${ID_PREFIX}/chat`, diff --git a/src/tui/runner/exit.test.ts b/src/tui/runner/exit.test.ts index bfcec2214..097a3143e 100644 --- a/src/tui/runner/exit.test.ts +++ b/src/tui/runner/exit.test.ts @@ -318,7 +318,18 @@ function stubSendLifecycle(agent: Agent): { emitter: new EventEmitter(), sessionOps: createSessionOperationQueue(), deliveryGeneration: createDeliveryGeneration(), - toolset: { setToolPromoter: () => undefined }, + toolset: { + setToolPromoter: () => undefined, + // A pending reload exists because MCP mounted something new. + dynamicRunner: { + agentDefinitions: () => [], + agentToolSetDrift: () => ({ + added: ["mcp__linear__save_issue"], + removed: [], + changed: [], + }), + }, + }, directorHolder: {}, subAgentSessions: { cancelAll: async () => [], list: () => [] }, activeRunHandle: { task: "", startedAt: 0, model: "" }, @@ -1103,6 +1114,20 @@ describe("reload-if-idle waits for the reactor's message run", () => { } }); + test("a rebuild request the previous build already absorbed is skipped", async () => { + const { state, services, builds } = await idleLifecycle(); + Object.assign(services.toolset, { + dynamicRunner: { + agentDefinitions: () => [], + agentToolSetDrift: () => ({ added: [], removed: [], changed: [] }), + }, + }); + defined(state.requestToolSetRebuild, "requestToolSetRebuild")(); + await services.sessionOps.awaitTail(); + expect(builds()).toBe(1); + expect(state.pendingReload).toBe(false); + }); + test("an MCP retry while idle rebuilds the agent", async () => { const { state, services, builds } = await idleLifecycle(); const mcp = wireLateMcp(state, services); diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index 58fe5246d..b61f48ab5 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -345,7 +345,7 @@ export async function createRunLifecycle( if (services.flushPromotions()) { services.directorHolder.instance?.updateToolDefinitions( services.computeAdvertised( - services.toolset.dynamicRunner.currentDefinitions(), + services.toolset.dynamicRunner.agentDefinitions(), ), ); } @@ -538,6 +538,20 @@ export async function createRunLifecycle( state.pendingReload = true; return; } + // Several connects can queue rebuilds before the first one runs; the + // first build pins them all, so the rest find nothing new. + const drift = services.toolset.dynamicRunner.agentToolSetDrift(); + if ( + drift.added.length === 0 && + drift.removed.length === 0 && + drift.changed.length === 0 + ) { + return; + } + tuiLogger.info( + "MCP tool set changed: added {added}, removed {removed}, changed {changed}", + { ...drift, sessionId: state.sessionId }, + ); try { const old = liveAgent(state); const closedCleanly = await closeAgentForRebuild(old, "reload"); diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index d5ad07423..1c2aaac57 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -709,6 +709,9 @@ export async function assembleTUISession( agentId: `${ID_PREFIX}/tui-agent`, systemPrompt, getDynamicRunner: () => toolset.dynamicRunner, + // MCP tool-set changes rebuild the agent (exit.ts reloadIfIdle), so each + // build pins what it can authorize. + pinToolSetAtBuild: true, computeAdvertised, inactivityTimeoutMs: config.inactivityTimeoutMs ?? 750_000, totalTimeoutMs: config.totalTimeoutMs, @@ -765,7 +768,7 @@ export async function assembleTUISession( pruneIdlePromotions, refreshAdvertised: () => { directorHolder.instance?.updateToolDefinitions( - computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + computeAdvertised(toolset.dynamicRunner.agentDefinitions()), ); }, persist: () => { diff --git a/src/tui/runner/settings.ts b/src/tui/runner/settings.ts index b23249d21..7c37b4dec 100644 --- a/src/tui/runner/settings.ts +++ b/src/tui/runner/settings.ts @@ -457,7 +457,7 @@ export async function wireSettings( refreshAdvertisedSchemas: () => { services.directorHolder.instance?.updateToolDefinitions( services.computeAdvertised( - services.toolset.dynamicRunner.currentDefinitions(), + services.toolset.dynamicRunner.agentDefinitions(), ), ); }, From 4be357d856091fa182a7cbf550a19d01733a27cb Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 01:05:35 -0400 Subject: [PATCH 5/6] test(e2e): cover approval for an MCP tool mounted after agent build --- e2e/integration-harness.ts | 126 +++++++++++------- e2e/mcp-late-approval.test.ts | 233 ++++++++++++++++++++++++++++++++++ 2 files changed, 312 insertions(+), 47 deletions(-) create mode 100644 e2e/mcp-late-approval.test.ts diff --git a/e2e/integration-harness.ts b/e2e/integration-harness.ts index e87cc9245..1d171aea4 100644 --- a/e2e/integration-harness.ts +++ b/e2e/integration-harness.ts @@ -35,6 +35,7 @@ import type { import { type } from "arktype"; import { createAgentWithLiveToolDispatch } from "../src/agent/live-tool-dispatch.js"; +import { denyLateTools } from "../src/agent/late-tool-authorize.js"; import { createChatDirector, type ChatDirector, @@ -97,6 +98,11 @@ export interface IntegrationSession { /** The live chat director — credential-recovery arming lives on it. */ chatDirector: ChatDirector; updateToolDefinitions: (definitions: ToolDefinition[]) => void; + /** + * Close the agent and build a fresh one over the same store and toolset, + * as the TUI does when the MCP tool set changes. + */ + rebuild: () => Promise; } export interface OpenIntegrationSessionOpts { @@ -114,6 +120,8 @@ export interface OpenIntegrationSessionOpts { contextTransforms?: ContextTransform[]; /** Override to pin the published createAgent snapshot (characterization). */ createAgentFn?: typeof createAgent; + /** Pin the tool set at each build, as the TUI does (see assembleChatAgent). */ + pinToolSetAtBuild?: boolean; /** * Tail-budget override for the production compactor; compaction tests pin * a tiny budget so calibrated growth volumes still fold. @@ -239,7 +247,11 @@ export async function openIntegrationSession( const toolsFactory = defineTool({ id: `${ID_PREFIX}/integration-tools`, definitions: [], - factory: () => toolset.dynamicRunner, + factory: () => { + if (opts.pinToolSetAtBuild === true) + toolset.dynamicRunner.pinAgentToolSet(); + return toolset.dynamicRunner; + }, }); const def = defineAgent({ @@ -308,54 +320,64 @@ export async function openIntegrationSession( }; authorize = wrapAuthorizeWithEvidenceArchive(baseAuthorize, () => archive); } - const innerAgent = await startAgent(def, { - sources, - defaultSource, - storage: storageForAgent, - workdir, - readCurrentMaterial: readSourceCredentialMaterial, - deps: { - ...harness.deps, - ...(opts.contextTransforms !== undefined - ? { contextTransforms: opts.contextTransforms } - : {}), - ...(opts.depsOverrides ?? {}), - }, - audit: noopAuditStore(), - authorize, - directors: createDirectorRegistry({ - factories: [chatDirectorDef.factory], - defaultId: `${ID_PREFIX}/chat`, - }), - ...(opts.compactionCompletion !== undefined && primaryArchive !== undefined - ? { - compactors: { - "pruning-compactor": createSessionPruningCompactor({ - // Absent compactionShape falls back to the shared production - // default inside createSessionPruningCompactor. - ...(opts.compactionShape !== undefined - ? { compactionShape: opts.compactionShape } - : {}), - summarize: createModelSummarizer({ - getSource: () => INTEGRATION_SOURCE, - deps: harness.deps, - complete: opts.compactionCompletion, - getArchive: () => evidenceArchiveHolder.current, + if (opts.pinToolSetAtBuild === true) { + authorize = denyLateTools(authorize, (name) => + toolset.dynamicRunner.isLateTool(name), + ); + } + const buildAgent = async (): Promise => { + const innerAgent = await startAgent(def, { + sources, + defaultSource, + storage: storageForAgent, + workdir, + readCurrentMaterial: readSourceCredentialMaterial, + deps: { + ...harness.deps, + ...(opts.contextTransforms !== undefined + ? { contextTransforms: opts.contextTransforms } + : {}), + ...(opts.depsOverrides ?? {}), + }, + audit: noopAuditStore(), + authorize, + directors: createDirectorRegistry({ + factories: [chatDirectorDef.factory], + defaultId: `${ID_PREFIX}/chat`, + }), + ...(opts.compactionCompletion !== undefined && + primaryArchive !== undefined + ? { + compactors: { + "pruning-compactor": createSessionPruningCompactor({ + // Absent compactionShape falls back to the shared production + // default inside createSessionPruningCompactor. + ...(opts.compactionShape !== undefined + ? { compactionShape: opts.compactionShape } + : {}), + summarize: createModelSummarizer({ + getSource: () => INTEGRATION_SOURCE, + deps: harness.deps, + complete: opts.compactionCompletion, + getArchive: () => evidenceArchiveHolder.current, + }), + readPriorHandoff: () => + tryReadPriorHandoffFile((key) => + storageForAgent.readBlob(key), + ), + wrapPruning: (pruning) => + wrapCompactorWithCompletenessGate(pruning, primaryArchive), }), - readPriorHandoff: () => - tryReadPriorHandoffFile((key) => storageForAgent.readBlob(key)), - wrapPruning: (pruning) => - wrapCompactorWithCompletenessGate(pruning, primaryArchive), - }), - }, - } - : {}), - closeTimeoutMs: 0, - }); - const agent = - primaryArchive === undefined + }, + } + : {}), + closeTimeoutMs: 0, + }); + return primaryArchive === undefined ? innerAgent : createPrimaryDeliveryAdmission(innerAgent, primaryArchive); + }; + const agent = await buildAgent(); const updateToolDefinitions = (definitions: ToolDefinition[]): void => { const director = directorHolder.current; @@ -368,7 +390,7 @@ export async function openIntegrationSession( throw new Error("chat director was not built during agent construction"); } - return { + const session: IntegrationSession = { harness, cwd, workdir, @@ -377,7 +399,17 @@ export async function openIntegrationSession( storage: storageForAgent, chatDirector, updateToolDefinitions, + rebuild: async () => { + await session.agent.close(); + session.agent = await buildAgent(); + const rebuilt = directorHolder.current; + if (rebuilt === undefined) { + throw new Error("chat director was not built during agent rebuild"); + } + session.chatDirector = rebuilt; + }, }; + return session; } export async function closeIntegrationSession( diff --git a/e2e/mcp-late-approval.test.ts b/e2e/mcp-late-approval.test.ts new file mode 100644 index 000000000..211368bfa --- /dev/null +++ b/e2e/mcp-late-approval.test.ts @@ -0,0 +1,233 @@ +import { describe, expect, test } from "bun:test"; +import type { ReactorEmittedEvent } from "@intx/inference"; + +import type { MCPClient } from "../src/mcp/client.js"; +import { mcpClientTools } from "../src/mcp/plugin.js"; +import { createPermissionGate } from "../src/permission/gate.js"; +import { createReactorAuthorize } from "../src/permission/reactor-authorize.js"; +import { + createApprovalResume, + resolveParkedCallIdFromStore, +} from "../src/session/approval-resume.js"; +import { + closeIntegrationSession, + openIntegrationSession, + runUntilDone, + runUntilSuspended, + toolDoneEvents, + type IntegrationSession, +} from "./integration-harness.js"; + +// A mutating MCP tool: classifyTool puts it on the ask tier. +const SAVE_ISSUE = "mcp__linear__save_issue"; + +function deferredApprovalGate() { + const asks: string[] = []; + let release: ((outcome: { allow: boolean }) => void) | undefined; + return { + asks, + gate: createPermissionGate({ + // Search is ask-tier outside auto mode; grant it so only the late + // tool's own authorization is under test. + approvals: [{ tool: "tool_search", pattern: "*" }], + interactive: true, + skipPermissions: false, + auto: false, + reactorGated: true, + requestApproval: (request) => { + asks.push(request.tool); + return new Promise((resolve) => { + release = resolve; + }); + }, + }), + approve: () => release?.({ allow: true }), + }; +} + +function linearClient(calls: Record[]): MCPClient { + return { + serverName: "linear", + tools: [ + { + name: "save_issue", + description: "create or update a linear issue", + inputSchema: { + type: "object", + properties: { title: { type: "string" } }, + }, + }, + ], + async callResult(_toolName, args) { + calls.push(args); + return { blocks: [{ type: "text", text: "SAVED-1" }], isError: false }; + }, + async close() { + return undefined; + }, + }; +} + +async function withLateLinear( + opts: { pinToolSetAtBuild: boolean }, + body: ( + session: IntegrationSession, + ctx: ReturnType, + calls: Record[], + ) => Promise, +): Promise { + const ctx = deferredApprovalGate(); + const session = await openIntegrationSession({ + permissionGate: ctx.gate, + authorize: createReactorAuthorize(ctx.gate), + pinToolSetAtBuild: opts.pinToolSetAtBuild, + }); + const calls: Record[] = []; + try { + // The server connects after the agent was built, as /mcp add, retry, + // enable, or a slow startup connect would. + session.toolset.dynamicRunner.addTools(mcpClientTools(linearClient(calls))); + await body(session, ctx, calls); + } finally { + await closeIntegrationSession(session); + } +} + +// tool.done carries only the result, and a blocked call never starts, so +// name the call ids from the assistant turns that requested them. +function toolDoneFor( + events: ReactorEmittedEvent[], + name: string, +): Extract | undefined { + const callIds = new Set( + events.flatMap((event) => + event.type === "inference.done" + ? event.data.turn.content.flatMap((block) => + block.type === "tool_call" && block.name === name ? [block.id] : [], + ) + : [], + ), + ); + return toolDoneEvents(events).find((event) => + callIds.has(event.data.result.callId), + ); +} + +const contentOf = ( + event: Extract | undefined, +): string => + typeof event?.data.result.content === "string" + ? event.data.result.content + : ""; + +async function waitForAsk(asks: string[]): Promise { + for (let i = 0; i < 100 && asks.length === 0; i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } +} + +describe("integration — late MCP tool approval", () => { + // Characterization of why a late tool needs a rebuild: the reactor + // authorizes against the definitions it snapshotted at build. + test.serial( + "without a rebuild an ask on a late MCP tool never reaches the operator", + async () => { + await withLateLinear( + { pinToolSetAtBuild: false }, + async (session, ctx, calls) => { + session.harness.scenario.replyOnce("anthropic", { + toolCalls: [{ name: SAVE_ISSUE, args: { title: "late" } }], + }); + session.harness.scenario.replyOnce("anthropic", { text: "gave up" }); + + const { events } = await runUntilDone(session, "file the issue"); + const done = toolDoneFor(events, SAVE_ISSUE); + expect(done?.data.result.isError).toBe(true); + expect(ctx.asks).toEqual([]); + expect(calls).toEqual([]); + }, + ); + }, + ); + + test.serial( + "a late MCP tool is not advertised or callable before the rebuild", + async () => { + await withLateLinear( + { pinToolSetAtBuild: true }, + async (session, ctx, calls) => { + session.harness.scenario.replyOnce("anthropic", { + toolCalls: [ + { name: "tool_search", args: { query: "linear save issue" } }, + ], + }); + session.harness.scenario.replyOnce("anthropic", { + toolCalls: [{ name: SAVE_ISSUE, args: { title: "late" } }], + }); + session.harness.scenario.replyOnce("anthropic", { + text: "next turn", + }); + + const { events } = await runUntilDone(session, "file the issue"); + const search = contentOf(toolDoneFor(events, "tool_search")); + expect(search).toContain("callable once it ends"); + expect(search).not.toContain("Matching tools"); + const call = toolDoneFor(events, SAVE_ISSUE); + expect(call?.data.result.isError).toBe(true); + expect(contentOf(call)).toContain("callable once the turn ends"); + expect(ctx.asks).toEqual([]); + expect(calls).toEqual([]); + }, + ); + }, + ); + + test.serial( + "after the rebuild a late mutating MCP tool asks, runs once on approval, and returns its result", + async () => { + await withLateLinear( + { pinToolSetAtBuild: true }, + async (session, ctx, calls) => { + await session.rebuild(); + session.harness.scenario.replyOnce("anthropic", { + toolCalls: [ + { name: "tool_search", args: { query: "linear save issue" } }, + ], + }); + session.harness.scenario.replyOnce("anthropic", { + toolCalls: [{ name: SAVE_ISSUE, args: { title: "late" } }], + }); + session.harness.scenario.replyOnce("anthropic", { text: "Saved." }); + + const turn = await runUntilSuspended(session, "file the issue"); + const { result } = turn; + expect(result.type).toBe("suspended"); + if (result.type !== "suspended") return; + expect(contentOf(toolDoneFor(turn.events, "tool_search"))).toContain( + SAVE_ISSUE, + ); + expect(result.approvalSnapshot?.name).toBe(SAVE_ISSUE); + expect(calls).toEqual([]); + + const resume = createApprovalResume({ + getAgent: () => session.agent, + resolveParkedCallId: (correlationId) => + resolveParkedCallIdFromStore(session.storage, correlationId), + gate: ctx.gate, + }); + const handling = resume.handle(result); + await waitForAsk(ctx.asks); + expect(ctx.asks).toEqual([SAVE_ISSUE]); + ctx.approve(); + expect(await handling).toBe(true); + + expect(await turn.reply()).toBe("Saved."); + expect(calls).toEqual([{ title: "late" }]); + const done = toolDoneFor(turn.events, SAVE_ISSUE); + expect(done?.data.result.isError).not.toBe(true); + expect(contentOf(done)).toContain("SAVED-1"); + }, + ); + }, + ); +}); From 230974d47a1274d71a334a47a2c82ff97e30e1e3 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Sat, 17 Oct 2026 01:05:48 -0400 Subject: [PATCH 6/6] docs(agent): name the callers that still need live tool dispatch --- src/agent/live-tool-dispatch.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/agent/live-tool-dispatch.ts b/src/agent/live-tool-dispatch.ts index 1995f1c16..df75f3e6e 100644 --- a/src/agent/live-tool-dispatch.ts +++ b/src/agent/live-tool-dispatch.ts @@ -8,11 +8,13 @@ import { import { createApprovalProvenanceGate } from "../session/approval-provenance.js"; // @intx/agent resolveTools snapshots `byName` from each bundle's definitions -// at createAgent and never consults a live getter. MCP tools arrive later via -// DynamicToolRunner.addTools (servers connect after the TUI is up; one -// OAuth-blocked server can also stall the post-connect reload that would -// rebuild the snapshot). A miss then returns `unknown tool` even though -// tool_search already listed the name from the live runner. +// at createAgent and never consults a live getter, so a miss returns +// `unknown tool` for a name only DynamicToolRunner can resolve. Two callers +// still depend on reaching it: exec builds once and mounts MCP tools after +// (it never rebuilds), and harness-namespaced spellings (`default.mcp__*`) +// resolve only through the runner's own name lookup. The TUI no longer does +// for late MCP tools: it pins the tool set per build and rebuilds when it +// changes (see pinToolSetAtBuild in session/assemble-runtime.ts). // // resolveTools is not exported. During its synchronous walk it does // `new Map()` for that snapshot; we install a Map whose get() falls back to