diff --git a/src/agent/director.test.ts b/src/agent/director.test.ts index 8113bf217..fc0c2df95 100644 --- a/src/agent/director.test.ts +++ b/src/agent/director.test.ts @@ -5,7 +5,7 @@ import type { ReactorInboundEvent, ReactorState, } from "@intx/types/runtime"; -import { createChatDirector } from "./director.js"; +import { createChatDirector, toolSetDigest } from "./director.js"; const mockState: ReactorState = { turns: [] } as unknown as ReactorState; @@ -100,6 +100,36 @@ async function runToolOnlyStreak( return last; } +describe("toolSetDigest", () => { + const base = { + name: "read_file", + description: "read a file", + inputSchema: { type: "object" }, + }; + + test("identical sets share a digest", () => { + expect(toolSetDigest([{ ...base }])).toBe(toolSetDigest([{ ...base }])); + }); + + // The digest gates the tool-set-changed log line, and the serialized tools + // array is the head of the provider's cached prompt prefix — an + // inputSchema-only change reshapes the wire bytes, so it must move the + // digest or the cache bust goes unlogged. + test("an inputSchema-only change alters the digest", () => { + const before = [{ ...base }]; + const after = [ + { + ...base, + inputSchema: { + type: "object", + properties: { path: { type: "string" } }, + }, + }, + ]; + expect(toolSetDigest(after)).not.toBe(toolSetDigest(before)); + }); +}); + describe("ChatDirector tool-only loop protection", () => { const providerlessPolicy = { providerName: "test-provider" }; diff --git a/src/agent/director.ts b/src/agent/director.ts index bf548acba..628c75a12 100644 --- a/src/agent/director.ts +++ b/src/agent/director.ts @@ -2,6 +2,7 @@ import { DefaultDirector, type ExtendedInferenceOptions, } from "@intx/inference"; +import { createHash } from "node:crypto"; import { getLogger } from "@intx/log"; import type { ReactorDirector, @@ -48,6 +49,29 @@ import { const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]); +// The serialized `tools` array is the head of the provider's cached prompt +// prefix, ahead of the system prompt. Measured on OpenCode Go Responses, a warm +// session holds 99.3% cached and ANY change to that array — a mount, a +// description edit, or a pure reorder of an unchanged set — drops the next turn +// to 2-4%. Appending at the end is not cheaper than prepending: 4.5% versus +// 2.1%, both full misses. +// +// `advertisedTools` (src/agent/tool-search.ts) already keeps this array +// deterministic, so the array should only ever change when a genuine discovery +// grows it. This digest is here to prove that, because prefix churn is +// otherwise invisible — it shows up only as a billing and latency spike a turn +// later. Hashed rather than logged verbatim: MCP tool descriptions are +// arbitrary-length, server-supplied text and do not belong in the log stream. +export function toolSetDigest(tools: readonly ToolDefinition[]): string { + const shape = tools + .map( + (t) => + `${t.name}:${t.description ?? ""}:${JSON.stringify(t.inputSchema ?? null)}`, + ) + .join("|"); + return createHash("sha256").update(shape).digest("hex").slice(0, 12); +} + function isInternalRecoveryAbort( event: Extract, ): boolean { @@ -521,7 +545,11 @@ class ChatDirectorImpl extends DefaultDirector { } updateToolDefinitions(toolDefinitions: ToolDefinition[]): void { + const before = toolSetDigest(this._toolDefinitions); + const after = toolSetDigest(toolDefinitions); this._toolDefinitions = toolDefinitions; + if (before === after) return; + logger.debug`tool-set-changed count=${String(this._toolDefinitions.length)} before=${before} after=${after}`; } getTasks(): Task[] { diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index e51ebdb6c..1d62f274a 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -8,7 +8,8 @@ import { sessionModeEnablesSubAgents } from "../config/session-mode.js"; // Tools whose full schema is always advertised to the model. Everything else is // registered but discovered on demand via tool_search, which promotes matches -// onto the advertised set and the call gate. Shared by the system prompt and +// onto the call gate at once; the full schema joins the wire set at the next +// cache-safe boundary. Shared by the system prompt and // the advertised-set gate so the two never drift. // // `present` is deliberately absent: most sessions never render a view, and at @@ -142,10 +143,11 @@ export const ADVERTISED_TOOL_NAMES: readonly string[] = [ // Project the live tool registry onto the advertised set: the fixed built-in // prefix (its order never changes — this is what keeps the provider cache -// prefix stable) followed by session-activated tools (MCP or otherwise) in -// first-activation order. The wire array is byte-stable turn to turn until a -// discovery appends a new name, at which point it grows once and then holds -// steady again. `activated` is expected to already be deduped/ordered (see +// prefix stable) followed by wire-committed tools (MCP or otherwise) in +// first-commit order. The wire array is byte-stable turn to turn: callers pass +// only names committed via flushPromotions at a cache-safe boundary, never the +// live activation list, so a mid-session discovery cannot append here. +// `activated` is expected to already be deduped/ordered (see // `createActivatedToolTracker`), but names are deduped again here defensively // so a caller passing raw matches still can't reorder or duplicate an entry. export function advertisedTools( @@ -209,7 +211,7 @@ export function createActivatedToolTracker(): ActivatedToolTracker { export const toolSearchDefinition: ToolDefinition = { name: "tool_search", description: - "Discover callable tools by capability. Most tools — MCP servers, present, and other integrations — are not advertised until this search promotes them onto the wire. Core tools (read_file, run_shell, web_fetch, web_search, spawn_agent, …) are already on the wire — do not tool_search for them. wait_agents is mounted on exec-primary runs only, so it is not on the wire elsewhere and this search cannot promote it there. Call this with a short description of what you need (e.g. 'issue tracker', 'render layout', 'granola notes') to get matching tools' names, descriptions, and input schemas. Matched tools are promoted and callable on return — invoke them directly, no separate load step.", + "Discover callable tools by capability. Most tools — MCP servers, present, and other integrations — are not callable until this search promotes them. Core tools (read_file, run_shell, web_fetch, web_search, spawn_agent, …) are already on the wire — do not tool_search for them. wait_agents is mounted on exec-primary runs only, so it is not on the wire elsewhere and this search cannot promote it there. Call this with a short description of what you need (e.g. 'issue tracker', 'render layout', 'granola notes') to get matching tools' names, descriptions, and input schemas. Matched tools are promoted and callable on return — invoke them directly, no separate load step.", inputSchema: { type: "object", properties: { @@ -283,9 +285,9 @@ export function createToolIndex( export interface ToolSearchDeps { search: (query: string) => string[]; lookup: (name: string) => ToolDefinition | undefined; - // Promote matches onto the advertised set and the call gate so the model can - // invoke them this turn. The next inference also declares them on the wire - // for strict providers. + // Promote matches onto the call gate so the model can invoke them this turn + // from the result card's schema. Wire declaration follows at the next + // cache-safe boundary (compaction fold), never mid-thread. promote: (names: string[]) => void; // Resolves to the remaining in-flight MCP handshake count after waiting up // to `timeoutMs`. The toolset bounds its own wait; the handler re-races @@ -374,12 +376,12 @@ export function createToolSearchTool(deps: ToolSearchDeps): AgentTool { if (names.length === 0) { return `No tools matched "${query}". Try different keywords describing the capability.`; } - // Matches are promoted into the advertised set so the next inference - // declares them on the wire — required for strict providers (e.g. the grok - // Responses API) where a model cannot call a tool that was never declared. - // The tool result below still carries name, description, AND input schema - // so the model can shape arguments this same turn, before the promoted - // definition round-trips through the next infer call. + // Matches open on the call gate at once; the full schema joins the wire + // declarations at the next cache-safe boundary (compaction fold), never + // mid-thread, so the provider's cached prefix stays byte-stable. The + // tool result below still carries name, description, AND input schema + // so the model can shape arguments and call this same turn, before the + // promoted definition is declared on the wire. deps.promote(names); const blocks = names.map((name) => renderToolCard(deps.lookup(name), name), diff --git a/src/director.test.ts b/src/director.test.ts index 7eeaf2568..1be80a8d8 100644 --- a/src/director.test.ts +++ b/src/director.test.ts @@ -1,10 +1,7 @@ import { describe, test, expect } from "bun:test"; import { createChatDirector, askOperatorDefinition } from "./agent/director.js"; import { createAgentToolset } from "./agent/tools.js"; -import { - advertisedTools, - createActivatedToolTracker, -} from "./agent/tool-search.js"; +import { createAdvertisedToolset } from "./session/assemble-runtime.js"; import { createPermissionGate } from "./permission/gate.js"; import { COMPACTOR_KEEP_RECENT_TURNS, @@ -1145,6 +1142,93 @@ describe("updateToolDefinitions rewrites infer tools", () => { expect(JSON.stringify(after)).toBe(JSON.stringify(before)); }); + // CL-7868 (direction A): the provider cache is a prefix cache keyed on the + // tools array, so a tool_search turn that promotes a genuinely new tool must + // not reshape the wire set mid-session. The call gate opens (the model + // invokes the tool from the search result's schema) while the advertised + // array holds steady; the growth event lands at the next cache-safe + // boundary (compaction fold), appended after the untouched fixed prefix. + test("a tool_search turn promoting a genuinely new tool leaves the wire byte-identical until the fold commits it", async () => { + const linearTool = { + name: "mcp__linear__list_issues", + description: "list issues", + inputSchema: { type: "object", properties: {}, required: [] }, + }; + const toolset = await createAgentToolset({ + cwd: process.cwd(), + permissionGate: createPermissionGate({ + approvals: [], + interactive: false, + skipPermissions: true, + reactorGated: false, + }), + onOperatorGate: async () => ({ kind: "cancel" }), + }); + toolset.dynamicRunner.addTools([ + { kind: "string", definition: linearTool, handler: async () => "ok" }, + ]); + + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "openai", model: "gpt-5" }), + }); + const director = createChatDirector( + "base-prompt", + advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + { onTasksChange: () => undefined }, + ); + + const before = await firstInferTools( + director, + makeMessageReceivedEvent("hello"), + ); + + // tool_search matched a genuinely new tool: the runner opens the call + // gate (activation); the per-turn wire recompute deliberately ignores it. + expect(advertised.activated.activate(["mcp__linear__list_issues"])).toBe( + true, + ); + director.updateToolDefinitions( + advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + ); + + // Mid-session the wire is byte-identical — the hot prefix never grows — + // while the gate is open so the model can invoke the match from the + // result card's schema. + const after = await firstInferTools( + director, + makeMessageReceivedEvent("continue"), + ); + expect(JSON.stringify(after)).toBe(JSON.stringify(before)); + expect(advertised.isAdvertised("mcp__linear__list_issues")).toBe(true); + + // Cache-safe boundary (compaction fold): the pending promotion commits and + // the next turn declares it after the untouched fixed prefix. + expect(advertised.flushPromotions()).toBe(true); + director.updateToolDefinitions( + advertised.computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + ); + const folded = await firstInferTools( + director, + makeMessageReceivedEvent("after fold"), + ); + const foldedNames = (folded as { name: string }[]).map((t) => t.name); + expect(foldedNames).toContain("mcp__linear__list_issues"); + const beforeNames = (before as { name: string }[]) + .map((t) => t.name) + .filter((n) => n !== "submit_output"); + const foldedPrefix = foldedNames.filter( + (n) => n !== "submit_output" && n !== "mcp__linear__list_issues", + ); + expect(foldedPrefix).toEqual(beforeNames); + expect(foldedNames.indexOf("mcp__linear__list_issues")).toBe( + beforeNames.length, + ); + + await toolset.dispose(); + }); + // submit_output is always on the wire so a workflow going active never grows // the array and busts the provider cache prefix. test("submit_output is advertised even with no active workflow", async () => { @@ -1165,12 +1249,13 @@ describe("updateToolDefinitions rewrites infer tools", () => { expect(inferToolNames(inferAction)).toContain("submit_output"); }); - // End-to-end: tool_search matches an MCP tool, the runner's promote wiring - // (mirrored here via createActivatedToolTracker + updateToolDefinitions) grows - // the wire set once with the tool's full definition, and it then holds steady. - // On a strict provider, a model can only call a tool - // that was actually declared on the wire, so promotion must land here. - test("a tool_search match is on the wire on the next turn, then the array holds stable", async () => { + // End-to-end: tool_search matches an MCP tool; the runner's gate-only promote + // wiring (mirrored here via createAdvertisedToolset + updateToolDefinitions) + // keeps it off the wire the next turn — the hot prefix stays byte-identical + // while the gate lets the model call it from the result card. The fold + // commits the pending promotion with the tool's full definition, appended + // after the untouched fixed prefix, and the array then holds steady. + test("a tool_search match stays off the wire until the fold, then holds stable", async () => { const linearTool = { name: "mcp__linear__list_issues", description: "list issues", @@ -1190,10 +1275,14 @@ describe("updateToolDefinitions rewrites infer tools", () => { { kind: "string", definition: linearTool, handler: async () => "ok" }, ]); - const activated = createActivatedToolTracker(); + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "openai", model: "gpt-5" }), + }); const computeAdvertised = ( all: ReturnType, - ) => advertisedTools(all, activated.list()); + ) => advertised.computeAdvertised(all); const director = createChatDirector( "base-prompt", computeAdvertised(toolset.dynamicRunner.currentDefinitions()), @@ -1209,17 +1298,34 @@ describe("updateToolDefinitions rewrites infer tools", () => { expect(beforeNames).not.toContain("mcp__linear__list_issues"); const beforeJson = JSON.stringify(before); - // Simulate the runner's promoteTools: tool_search matched this tool, so it - // is activated and the director's tool set is updated for the next infer. - activated.activate(["mcp__linear__list_issues"]); + // Simulate the runner's gate-only promoter plus its boundary wire push: + // the gate opens but the wire recompute is byte-identical until the fold. + advertised.activated.activate(["mcp__linear__list_issues"]); director.updateToolDefinitions( computeAdvertised(toolset.dynamicRunner.currentDefinitions()), ); - const after = await firstInferTools( + const gated = await firstInferTools( director, makeMessageReceivedEvent("continue"), ); + expect((gated as { name: string }[]).map((t) => t.name)).not.toContain( + "mcp__linear__list_issues", + ); + expect(JSON.stringify(gated)).toBe(beforeJson); + + // Simulate the compaction fold: commit the pending promotion, push the + // refreshed set, and the tool is declared — appended after the fixed + // built-in prefix, which survives untouched ahead of it. + advertised.flushPromotions(); + director.updateToolDefinitions( + computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + ); + + const after = await firstInferTools( + director, + makeMessageReceivedEvent("after fold"), + ); const afterTools = after as { name: string }[]; const afterNames = afterTools.map((t) => t.name); expect(afterNames).toContain("mcp__linear__list_issues"); diff --git a/src/exec/runner.test.ts b/src/exec/runner.test.ts index 106a182d6..a98fd8de3 100644 --- a/src/exec/runner.test.ts +++ b/src/exec/runner.test.ts @@ -63,19 +63,15 @@ describe("exec director allowlist", () => { test("promote cannot make an outside-allow tool callable under explorer", () => { const overlay = resolveExecDirectorOverlay("explorer"); expect(isExecOverlayToolAllowed(overlay, OUTSIDE_ALLOW)).toBe(false); - const { activated, computeAdvertised, isAdvertised } = - createAdvertisedToolset({ - sessionMode: "orchestrator", - toolAvailability: { languageServerAvailable: true }, - getProvider: () => ({ providerName: "test", model: "test-model" }), - builtInPrefix: overlay.advertisedAllow, - }); + const { activated, isAdvertised } = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: true }, + getProvider: () => ({ providerName: "test", model: "test-model" }), + builtInPrefix: overlay.advertisedAllow, + }); const promote = createExecToolPromoter({ activate: (names) => activated.activate(names), isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), - currentDefinitions: () => [], - computeAdvertised, - updateDirectorTools: () => undefined, }); promote([OUTSIDE_ALLOW]); expect(activated.has(OUTSIDE_ALLOW)).toBe(false); @@ -85,28 +81,39 @@ describe("exec director allowlist", () => { ).toBe(false); }); - test("the promoter gates allow itself — a raw activate caller gets no bypass", () => { + test("the promoter gates allow itself and leaves the wire set for the fold", () => { const overlay = resolveExecDirectorOverlay("explorer"); - const { activated, computeAdvertised } = createAdvertisedToolset({ - sessionMode: "orchestrator", - toolAvailability: { languageServerAvailable: true }, - getProvider: () => ({ providerName: "test", model: "test-model" }), - builtInPrefix: overlay.advertisedAllow, - }); - let advertisedCount = 0; + const { activated, computeAdvertised, flushPromotions } = + createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: true }, + getProvider: () => ({ providerName: "test", model: "test-model" }), + builtInPrefix: overlay.advertisedAllow, + }); const promote = createExecToolPromoter({ activate: (names) => activated.activate(names), isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), - currentDefinitions: () => [], - computeAdvertised, - updateDirectorTools: () => { - advertisedCount += 1; - }, }); + const registry = [ + { + name: "read_file", + description: "read a file", + inputSchema: { type: "object", properties: {} }, + }, + ]; + const before = JSON.stringify(computeAdvertised(registry)); + // A raw activate caller gets no bypass: outside-allow names never open the + // gate, while the allowed match opens it at once. promote([OUTSIDE_ALLOW, "read_file"]); expect(activated.has(OUTSIDE_ALLOW)).toBe(false); expect(activated.has("read_file")).toBe(true); - expect(advertisedCount).toBe(1); + // Gate-only: the wire recompute ignores the fresh activation, so the + // provider's cached prefix holds until the fold commits it. + expect(JSON.stringify(computeAdvertised(registry))).toBe(before); + expect(flushPromotions()).toBe(true); + expect(computeAdvertised(registry).map((d) => d.name)).toContain( + "read_file", + ); }); test("skywalker overlay leaves every tool allowed", () => { diff --git a/src/exec/runner.ts b/src/exec/runner.ts index eb53ade0d..b1dc3e0e5 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -48,7 +48,6 @@ import type { ContextStore, InferenceSource, InboundMessage, - ToolDefinition, } from "@intx/types/runtime"; import { OPERATOR_ORIGINATED_FLAG } from "../agent/message-provenance.js"; import { loadAgentProfiles } from "../agent/profiles.js"; @@ -403,14 +402,14 @@ export function createExecToolCallGate( export function createExecToolPromoter(args: { activate: (names: readonly string[]) => boolean; isAllowed: (name: string) => boolean; - currentDefinitions: () => readonly ToolDefinition[]; - computeAdvertised: (all: readonly ToolDefinition[]) => ToolDefinition[]; - updateDirectorTools: (defs: ToolDefinition[]) => void; persist?: () => void; }): (names: string[]) => void { return (names) => { + // Gate-only, like the TUI promoteTools: activation lets the model invoke + // the match from the result card's schema at once, while the schema joins + // the wire set at the next cache-safe boundary (compaction fold) so the + // provider's cached prefix never grows mid-thread (CL-7868). if (!args.activate(names.filter((name) => args.isAllowed(name)))) return; - args.updateDirectorTools(args.computeAdvertised(args.currentDefinitions())); args.persist?.(); }; } @@ -818,6 +817,7 @@ export async function runExec(config: Config): Promise { activated: activatedToolNames, computeAdvertised, isAdvertised, + flushPromotions, } = createAdvertisedToolset({ sessionMode, toolAvailability, @@ -879,6 +879,17 @@ export async function runExec(config: Config): Promise { return tools.length > 0 ? { activatedTools: tools } : undefined; }, telemetry: liveTelemetry, + onFolded: () => { + // A fold restarts the provider's cached prefix anyway: commit + // mid-session promotions so the next turn declares them. + if (flushPromotions()) { + directorHolder.instance?.updateToolDefinitions( + computeAdvertised( + agentToolset.dynamicRunner.currentDefinitions(), + ), + ); + } + }, }), onBuilt: (agent, storage) => { currentAgent = agent; @@ -895,11 +906,6 @@ export async function runExec(config: Config): Promise { createExecToolPromoter({ activate: (names) => activatedToolNames.activate(names), isAllowed: (name) => isExecOverlayToolAllowed(overlay, name), - currentDefinitions: () => - agentToolset.dynamicRunner.currentDefinitions(), - computeAdvertised, - updateDirectorTools: (defs) => - directorHolder.instance?.updateToolDefinitions(defs), persist: () => { void persist("running"); }, diff --git a/src/session/assemble-runtime.test.ts b/src/session/assemble-runtime.test.ts index 00cbe6e20..8b96b87df 100644 --- a/src/session/assemble-runtime.test.ts +++ b/src/session/assemble-runtime.test.ts @@ -47,15 +47,46 @@ describe("createAdvertisedToolset", () => { expect(names).not.toContain("mystery_tool"); }); - test("appends activated tools after the prefix, in activation order", () => { - const { activated, computeAdvertised } = createAdvertisedToolset(wiring()); + // CL-7868 (direction A): mid-session activation opens the call gate but must + // not reshape the wire set, so computeAdvertised ignores it until a + // cache-safe boundary commits it via flushPromotions. + test("activation alone leaves the wire set untouched until flushPromotions commits it", () => { + const { activated, computeAdvertised, flushPromotions } = + createAdvertisedToolset(wiring()); + const registry = [def("read_file"), def("mystery_tool")]; + const wireBefore = JSON.stringify(computeAdvertised(registry)); expect(activated.activate(["mystery_tool"])).toBe(true); + // Byte-identical adapter input across turns differing only in activation: + // the provider's serialized tools payload cannot drift mid-session. + expect(JSON.stringify(computeAdvertised(registry))).toBe(wireBefore); + expect(flushPromotions()).toBe(true); + const names = computeAdvertised(registry).map((d) => d.name); + expect(names[names.length - 1]).toBe("mystery_tool"); + expect(names.slice(0, -1)).not.toContain("mystery_tool"); + // A second flush with nothing pending is a no-op so the array holds steady. + expect(flushPromotions()).toBe(false); + }); + + test("flushPromotions commits pending activations in order and resume re-arms them", () => { + const { activated, computeAdvertised, flushPromotions } = + createAdvertisedToolset(wiring()); + expect(flushPromotions()).toBe(false); + expect(activated.activate(["tool_b", "tool_a"])).toBe(true); + expect(flushPromotions()).toBe(true); const names = computeAdvertised([ def("read_file"), - def("mystery_tool"), + def("tool_a"), + def("tool_b"), ]).map((d) => d.name); - expect(names[names.length - 1]).toBe("mystery_tool"); - expect(names.slice(0, -1)).not.toContain("mystery_tool"); + expect(names.slice(-2)).toEqual(["tool_b", "tool_a"]); + // Rotation clears the gate and the wire snapshot; session start replays the + // restored names (activate) at a cache-safe boundary (flush), re-arming + // both while the pending edge stays empty afterwards. + activated.clear(); + expect(flushPromotions()).toBe(false); + expect(activated.activate(["tool_a", "tool_b"])).toBe(true); + expect(flushPromotions()).toBe(true); + expect(flushPromotions()).toBe(false); }); test("honors an explicit built-in prefix", () => { diff --git a/src/session/assemble-runtime.ts b/src/session/assemble-runtime.ts index 9f939a4cd..2e80296fd 100644 --- a/src/session/assemble-runtime.ts +++ b/src/session/assemble-runtime.ts @@ -344,6 +344,12 @@ export interface AdvertisedToolset { // so a registered-but-unadvertised call surfaces as a tool_search error // instead of silently dispatching. isAdvertised: (name: string) => boolean; + // Commit activated-but-unadvertised names onto the wire set, in activation + // order. Returns whether the wire set actually grew. Call only at a + // cache-safe boundary (session start/resume, rotation, compaction): the + // serialized tools array heads the provider's cached prefix, so growing it + // mid-thread re-prefills the whole request. See CL-7868. + flushPromotions: () => boolean; } /** @@ -351,6 +357,13 @@ export interface AdvertisedToolset { * wire. The provider identity is read per call so a live model switch * re-gates without rebuilding the agent. * + * Activation and advertisement are split on purpose. Activating a name opens + * the call gate at once (isAdvertised flips, so the model can invoke the tool + * from the tool_search result card's schema), but the newly promoted schema + * stays off the wire array until flushPromotions commits it at the next + * cache-safe boundary. Mid-session promotion therefore never reshapes the + * provider's cached prefix. + * * `pinnedTools` (local settings) merge into the prefix — advertised from the * first turn and exempt from activation state, so a resume needs no * tool_search round-trip for the project's hottest integrations. @@ -370,6 +383,24 @@ export function createAdvertisedToolset(args: { ...(args.pinnedTools ?? []).filter((name) => !builtIn.includes(name)), ]; const activated = createActivatedToolTracker(); + // Wire-committed activations. activate() opens the call gate (see + // isAdvertised) at once, but names join this snapshot only via + // flushPromotions at a cache-safe boundary — the serialized tools array + // heads the provider's cached prefix, so growing it mid-thread re-prefills + // the whole request. clear() resets both: a rotated session restarts at the + // prefix (see newSession). + let wireActivated: string[] = []; + const wireActivatedSet = new Set(); + const advertised: ActivatedToolTracker = { + activate: (names) => activated.activate(names), + has: (name) => activated.has(name), + list: () => activated.list(), + clear: () => { + activated.clear(); + wireActivated = []; + wireActivatedSet.clear(); + }, + }; // Advertise then family-gate wire schemas (kimi gets a non-recursive present). // The primary session is always the orchestrator (SessionMode is the single // literal "orchestrator"), so orchestrator: true is passed directly instead @@ -396,8 +427,11 @@ export function createAdvertisedToolset(args: { denied.length === 0 ? prefix : prefix.filter((name) => !denied.includes(name)); + // The wire carries the fixed prefix plus wire-committed activations only: + // fresh activations open the call gate (isAdvertised) at once but stay off + // this array until flushPromotions commits them at a cache-safe boundary. return normalizeToolDefinitionsForProvider( - advertisedTools(all, activated.list(), gatedPrefix), + advertisedTools(all, wireActivated, gatedPrefix), { ...provider, }, @@ -407,7 +441,22 @@ export function createAdvertisedToolset(args: { if (deniedFor(args.getProvider()).includes(name)) return false; return prefix.includes(name) || activated.has(name); }; - return { activated, computeAdvertised, isAdvertised }; + const flushPromotions = (): boolean => { + let grew = false; + for (const name of activated.list()) { + if (wireActivatedSet.has(name)) continue; + wireActivatedSet.add(name); + wireActivated.push(name); + grew = true; + } + return grew; + }; + return { + activated: advertised, + computeAdvertised, + isAdvertised, + flushPromotions, + }; } // --------------------------------------------------------------------------- @@ -421,8 +470,6 @@ export interface ChatAgentWiring { getDynamicRunner: () => AgentToolset["dynamicRunner"]; computeAdvertised: (all: readonly ToolDefinition[]) => ToolDefinition[]; activateTools: (names: readonly string[]) => boolean; - /** Fires after the standard activate + director-update handling. */ - onToolsPromoted?: () => void; inactivityTimeoutMs: number; totalTimeoutMs?: number | undefined; onTasksChange: (tasks: Task[]) => void; @@ -483,13 +530,12 @@ export function assembleChatAgent(wiring: ChatAgentWiring): AssembledChatAgent { wiring.computeAdvertised([...agentCtx.toolDefinitions]), { onActivateTools: (names) => { - if (!wiring.activateTools(names)) return; - directorHolder.instance?.updateToolDefinitions( - wiring.computeAdvertised( - wiring.getDynamicRunner().currentDefinitions(), - ), - ); - wiring.onToolsPromoted?.(); + // Gate-only: activation lets the model invoke the tool from the + // tool_search result card's schema at once. The schema itself + // stays off the wire array until flushPromotions commits it at a + // cache-safe boundary, so mid-session promotion never reshapes + // the provider's cached prefix (CL-7868). + wiring.activateTools(names); }, inactivityTimeoutMs: wiring.inactivityTimeoutMs, totalTimeoutMs: wiring.totalTimeoutMs, diff --git a/src/session/state.ts b/src/session/state.ts index 4f8629a51..13a372245 100644 --- a/src/session/state.ts +++ b/src/session/state.ts @@ -46,7 +46,7 @@ const RunStateSchema = type({ // MCP servers connected during the session, with the tool count each // contributed. Empty until the first server finishes connecting. "mcpServers?": ConnectedMcpServerSchema.array(), - // Tool names promoted onto the wire via tool_search this session. Persisted + // Tool names promoted via tool_search this session. Persisted // so a resume can re-activate them before the first post-resume inference — // the transcript still tells the model they are callable. "activatedTools?": "string[]", diff --git a/src/tui/runner/exit.ts b/src/tui/runner/exit.ts index 0738872bd..795e02cca 100644 --- a/src/tui/runner/exit.ts +++ b/src/tui/runner/exit.ts @@ -359,24 +359,19 @@ export async function createRunLifecycle( }; state.reloadIfIdle = reloadIfIdle; - // tool_search (and contextual triggers, e.g. the lsp hint) promote tools into - // the advertised set. Advertising takes effect on the next infer; a reload is - // scheduled so a newly connected MCP tool also becomes dispatchable after a - // rebuild (built-in tools are already dispatchable, so promoting them alone - // needs no reload, but the reload is a cheap no-op in that case). + // tool_search (and contextual triggers, e.g. the lsp hint) promote tools by + // opening the call gate: the model invokes the match from the result card's + // schema on the very next turn. The schema itself joins the wire set at the + // next cache-safe boundary (compaction fold), never mid-thread — the + // serialized tools array heads the provider's cached prefix (CL-7868). No + // reload is scheduled: dispatchability comes from the live call gate, not + // the rebuilt agent. const promoteTools = (names: string[]): void => { if (!services.activatedToolNames.activate(names)) return; - services.directorHolder.instance?.updateToolDefinitions( - services.computeAdvertised( - services.toolset.dynamicRunner.currentDefinitions(), - ), - ); // Activation is model-visible contract — persist it now so a crash or // restart before the next turn boundary does not strand the transcript's // "these tools are available" record. void persistRunSnapshot("running"); - state.pendingReload = true; - reloadIfIdle(); }; services.toolset.setToolPromoter(promoteTools); @@ -400,7 +395,7 @@ export async function createRunLifecycle( // as a CodexAuthError naming the profile and rejects the send. // // The source is pushed on every send, not only when the token changed: an - // agent rebuild (tool promotion, interrupt, /clear) reseeds the source from + // agent rebuild (interrupt, /clear) reseeds the source from // the original login-time token, so unconditionally re-pushing the live token // is what keeps the rebuilt agent from sending a stale credential. const refreshCodexBeforeSend = async (): Promise => { diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index c6326a8cd..200e4e578 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -460,13 +460,16 @@ export async function assembleTUISession( }); workflowHostHolder.instance = workflowHost; - // Dynamic tool discovery: only the fixed built-in prefix plus activated - // tools reach the wire, so the provider cache prefix holds steady; MCP - // tools must be promoted here before the model can invoke them. + // Dynamic tool discovery: only the fixed built-in prefix plus + // wire-committed activations reach the wire, so the provider cache prefix + // holds steady; MCP tools must be promoted here before the model can invoke + // them, and their schemas join the wire at a cache-safe boundary (below, + // and on compaction folds). const { activated: activatedToolNames, computeAdvertised, isAdvertised, + flushPromotions, } = createAdvertisedToolset({ sessionMode: liveSessionMode, toolAvailability, @@ -476,8 +479,11 @@ export async function assembleTUISession( : {}), }); // Re-activate the prior run's promoted tools before the first build so the - // post-resume wire matches the transcript the model still sees. + // post-resume wire matches the transcript the model still sees. Session + // start is a cache-safe boundary: commit them to the wire now so the first + // turn already declares them. activatedToolNames.activate(start.resumeSeed.activatedTools); + flushPromotions(); // A registered tool the wire never advertised must error toward tool_search // instead of dispatching blind — the transcript would otherwise claim a call // the next infer does not declare. submit_output rides every infer via the @@ -626,10 +632,6 @@ export async function assembleTUISession( // without rebuilding the agent (aligned with transcript stamp). getProviderId: () => state.config.providerName, directorHolder, - onToolsPromoted: () => { - state.pendingReload = true; - state.reloadIfIdle?.(); - }, getWorkdir: () => state.workdir, getSessionId: () => state.sessionId, authorize: createReactorAuthorize(permissionGate), @@ -646,7 +648,17 @@ export async function assembleTUISession( summaryContext, telemetry: liveTelemetry, // Main-session folds only — exec runner and subagents stay silent. - onFolded: (info) => emitter.emit("compaction", info), + onFolded: (info) => { + // A fold restarts the provider's cached prefix anyway, so this is + // the cache-safe moment to commit mid-session promotions: the next + // turn declares the newly callable tools' schemas. + if (flushPromotions()) { + directorHolder.instance?.updateToolDefinitions( + computeAdvertised(toolset.dynamicRunner.currentDefinitions()), + ); + } + emitter.emit("compaction", info); + }, }), onBuilt: (agent, storage) => { state.currentAgent = agent; diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index d705b7c19..5ad6f8983 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -773,7 +773,7 @@ describe("exec tool call gate and promoter", () => { stringTool(shellDefinition.name, "sh", "run a shell command"), stringTool(updatePlanDefinition.name, "planned", "update the plan"), ]); - const { activated, isAdvertised, computeAdvertised } = + const { activated, isAdvertised, computeAdvertised, flushPromotions } = createAdvertisedToolset({ sessionMode: "orchestrator", toolAvailability: { languageServerAvailable: false }, @@ -781,15 +781,9 @@ describe("exec tool call gate and promoter", () => { }); runner.setCallGate(createExecToolCallGate(isAdvertised, { isCodex })); let persistCount = 0; - const directorNames: string[][] = []; const promote = createExecToolPromoter({ activate: (names) => activated.activate(names), isAllowed: () => true, - currentDefinitions: () => runner.currentDefinitions(), - computeAdvertised, - updateDirectorTools: (defs) => { - directorNames.push(defs.map((d) => d.name)); - }, persist: () => { persistCount += 1; }, @@ -801,7 +795,13 @@ describe("exec tool call gate and promoter", () => { runner.currentDefinitions().find((d) => d.name === name), promote, }); - return { runner, persistCount: () => persistCount, directorNames, search }; + return { + runner, + persistCount: () => persistCount, + computeAdvertised, + flushPromotions, + search, + }; } async function dispatch( @@ -815,20 +815,32 @@ describe("exec tool call gate and promoter", () => { } test("tool_search then MCP dispatch with the gate on", async () => { - const { runner, search, persistCount, directorNames } = + const { runner, search, persistCount, computeAdvertised, flushPromotions } = wireExecDiscovery(false); const blocked = await dispatch(runner, "mcp__linear__save_issue"); expect(blocked.isError).toBe(true); expect(blocked.content).toContain("tool_search"); + const wireBefore = JSON.stringify( + computeAdvertised(runner.currentDefinitions()), + ); if (search.kind !== "string") throw new Error("expected string tool"); await search.handler({ query: "linear" }, new AbortController().signal); expect(persistCount()).toBe(1); - expect(directorNames.at(-1)).toContain("mcp__linear__save_issue"); + // Gate-only promotion: the tool dispatches now, but the wire set holds + // steady until a cache-safe boundary commits it. const allowed = await dispatch(runner, "mcp__linear__save_issue"); expect(allowed.content).toBe("saved"); expect(allowed.isError).toBeUndefined(); + expect(JSON.stringify(computeAdvertised(runner.currentDefinitions()))).toBe( + wireBefore, + ); + + expect(flushPromotions()).toBe(true); + expect( + computeAdvertised(runner.currentDefinitions()).map((d) => d.name), + ).toContain("mcp__linear__save_issue"); }); test("present and plugin names pass the gate after tool_search promote", async () => { diff --git a/tests/unit/openai-responses-adapter.test.ts b/tests/unit/openai-responses-adapter.test.ts index 9be47495c..2b2cc1c60 100644 --- a/tests/unit/openai-responses-adapter.test.ts +++ b/tests/unit/openai-responses-adapter.test.ts @@ -3,12 +3,14 @@ import { createOpenAIResponsesAdapter, OPENAI_SESSION_ID_OPTION, } from "../../src/provider/openai-responses.js"; +import { createAdvertisedToolset } from "../../src/session/assemble-runtime.js"; import { OPENCODE_SESSION_ID_OPTION } from "../../src/provider/opencode-session.js"; import { BEARER_CREDENTIAL_SENTINEL } from "@intx/inference"; import type { ConversationTurn, InferenceOptions, LastCycleSource, + ToolDefinition, } from "@intx/types/runtime"; const SOURCE: LastCycleSource = { @@ -70,6 +72,43 @@ describe("openai-responses buildRequest", () => { }); }); +describe("openai-responses promotion cache safety", () => { + function def(name: string): ToolDefinition { + return { + name, + description: `${name} tool`, + inputSchema: { type: "object", properties: {} }, + }; + } + + // CL-7868: the tools array is the head of the provider's cached prefix, so + // a mid-session activation must not change the serialized request body — + // the turns differ only in activated tools. + test("activating a tool mid-session leaves the serialized wire body byte-identical", () => { + const advertised = createAdvertisedToolset({ + sessionMode: "orchestrator", + toolAvailability: { languageServerAvailable: false }, + getProvider: () => ({ providerName: "openai", model: "gpt-5.6-luna" }), + }); + const defs = [ + def("read_file"), + def("write_file"), + def("tool_search"), + def("mcp__linear__list_issues"), + ]; + const bodyFor = (tools: ToolDefinition[]): string => + adapter().buildRequest([userTurn("hi")], "gpt-5.6-luna", { tools }).body; + const before = bodyFor(advertised.computeAdvertised(defs)); + expect(JSON.parse(before)).toHaveProperty("tools"); + + advertised.activated.activate(["mcp__linear__list_issues"]); + + const after = bodyFor(advertised.computeAdvertised(defs)); + expect(after).toBe(before); + expect(advertised.isAdvertised("mcp__linear__list_issues")).toBe(true); + }); +}); + describe("openai-responses x-opencode-session header", () => { test("sets the header from the opencode session id without leaking it into the body", () => { const req = adapter().buildRequest([userTurn("hi")], "gpt-5.6-luna", {