fix(coding-agent/mcp): stabilize tool ordering and skip redundant prompt rebuilds
Two cache-stability fixes for Anthropic prompt caching during MCP server
reconnects, which happen routinely (~5 min per server) in long sessions
due to SSE transport keepalive timeouts.
1) MCPManager: deterministic tool ordering
`#tools` is now sorted by name after every mutation. The previous
filter-out + push-to-end pattern in `#replaceServerTools` moved the
reconnecting server's tools to the end of the array, producing a new
byte order whenever the reconnect sequence differed from the initial
discovery sequence. With multiple healthy servers, each reconnect of
the non-last server flipped the order and invalidated the tools
cache breakpoint sent to Anthropic.
Sort applies in `discoverAndConnect` (initial population) and
`#replaceServerTools` (used by `reconnectServer` and
`refreshServerTools`). The comparator is character-code based,
locale-independent and deterministic. `sortMCPToolsByName` is
exported as a small generic helper and unit-tested.
2) AgentSession: skip system-prompt rebuild when inputs are unchanged
`#applyActiveToolsByName` (called from `refreshMCPTools` after every
reconnect) used to unconditionally call `rebuildSystemPrompt` and
`setSystemPrompt` even when the resulting prompt was byte-identical.
This wasted CPU on every flap and risked silent cache invalidation
if the rebuild path ever became non-deterministic.
Now `#applyActiveToolsByName` computes a stable signature of the
inputs `rebuildSystemPrompt` reads and skips the rebuild when the
signature matches the last successful one. The signature covers:
- active tool names in render order
- active tool labels and descriptions (rendered as `{{label}}:
\`{{name}}\`` in the prompt body)
- when MCP discovery is on, every registry tool's name + label +
description (the prompt summarizes discoverable-but-inactive
MCP tools)
- per-server MCP `instructions` text (embedded under "## MCP
Server Instructions" in the appended prompt; can change on
server upgrade while tool list stays identical)
Server instructions are read via a new optional
`getMcpServerInstructions` callback on `AgentSessionConfig`, wired
from the SDK as `() => mcpManager.getServerInstructions()`.
`refreshBaseSystemPrompt()` continues to rebuild unconditionally and
refreshes the cached signature, so explicit refreshes still pick up
ambient changes (edit-mode toggles, memory writes, etc.) that the
signature does not cover.
Signature inputs deliberately NOT covered: tool input schemas, memory
instructions read from disk, and other ambient state. Callers that
mutate those must call `refreshBaseSystemPrompt()` explicitly; existing
hooks (`#syncEditToolModeAfterModelChange`, memory hooks, `/clear`)
already do.
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- Stabilized MCP tool ordering so reconnects and refreshes no longer reorder the tools array sent to the model. Anthropic prompt caching is keyed on byte-identical tool definitions; previously, the order depended on connection sequence and a single MCP server reconnect could shuffle tools across servers and invalidate the tools cache breakpoint.
|
||||
- Skipped redundant system-prompt rebuilds in `AgentSession.refreshMCPTools` when the active tool set is unchanged. MCP transport flapping (e.g. routine 5-minute SSE reconnects) used to call `rebuildSystemPrompt` on every reconnect even though the resulting prompt was byte-identical, eating CPU and risking cache misses if the rebuild ever became non-deterministic. The applied-tool signature also covers `customWireName` so a wire-name flip with the rest of the tool metadata constant still forces a rebuild.
|
||||
|
||||
## [14.5.12] - 2026-04-30
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
@@ -78,6 +78,21 @@ function delay(ms: number): Promise<void> {
|
||||
return Bun.sleep(ms);
|
||||
}
|
||||
|
||||
/**
|
||||
* Stable, total ordering on MCP tools by name.
|
||||
*
|
||||
* Anthropic prompt caching keys on byte-identical tool definitions: any reorder
|
||||
* of the tools array invalidates the tools cache breakpoint and forces a full
|
||||
* prefix rebuild on the next request. MCP servers connect/reconnect at arbitrary
|
||||
* times, so the natural "insertion order" of `#tools` is non-deterministic.
|
||||
* Sorting after every mutation makes the array bytes independent of connection
|
||||
* sequence.
|
||||
*/
|
||||
export function sortMCPToolsByName<T extends { name: string }>(tools: T[]): T[] {
|
||||
tools.sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0));
|
||||
return tools;
|
||||
}
|
||||
|
||||
export function resolveSubscriptionPostAction(
|
||||
notificationsEnabled: boolean,
|
||||
currentEpoch: number,
|
||||
@@ -459,6 +474,10 @@ export class MCPManager {
|
||||
}
|
||||
}
|
||||
|
||||
// Stable sort by name so the order is independent of connection completion.
|
||||
// See `sortMCPToolsByName` for the cache-stability rationale.
|
||||
sortMCPToolsByName(allTools);
|
||||
|
||||
// Update cached tools
|
||||
this.#tools = allTools;
|
||||
allowBackgroundLogging = true;
|
||||
@@ -474,6 +493,9 @@ export class MCPManager {
|
||||
#replaceServerTools(name: string, tools: CustomTool<TSchema, MCPToolDetails>[]): void {
|
||||
this.#tools = this.#tools.filter(t => !t.name.startsWith(`mcp__${name}_`));
|
||||
this.#tools.push(...tools);
|
||||
// Stable sort by name so reconnect order does not perturb the array.
|
||||
// See `sortMCPToolsByName` for the cache-stability rationale.
|
||||
sortMCPToolsByName(this.#tools);
|
||||
}
|
||||
|
||||
#triggerNotificationRefresh(serverName: string, kind: "tools" | "resources" | "prompts"): void {
|
||||
|
||||
@@ -1612,6 +1612,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
||||
onResponse,
|
||||
convertToLlm: convertToLlmFinal,
|
||||
rebuildSystemPrompt,
|
||||
getMcpServerInstructions: mcpManager ? () => mcpManager.getServerInstructions() : undefined,
|
||||
mcpDiscoveryEnabled,
|
||||
initialSelectedMCPToolNames,
|
||||
defaultSelectedMCPToolNames,
|
||||
|
||||
@@ -244,6 +244,13 @@ export interface AgentSessionConfig {
|
||||
convertToLlm?: (messages: AgentMessage[]) => Message[] | Promise<Message[]>;
|
||||
/** System prompt builder that can consider tool availability */
|
||||
rebuildSystemPrompt?: (toolNames: string[], tools: Map<string, AgentTool>) => Promise<string>;
|
||||
/**
|
||||
* Optional accessor for live MCP server instructions. Read by the session's
|
||||
* `rebuildSystemPrompt`-skip optimization to detect server-side instruction
|
||||
* changes (e.g. an MCP server upgrade) that would otherwise pass the tool-set
|
||||
* signature comparison and silently keep a stale prompt cached.
|
||||
*/
|
||||
getMcpServerInstructions?: () => Map<string, string> | undefined;
|
||||
/** Enable hidden-by-default MCP tool discovery for this session. */
|
||||
mcpDiscoveryEnabled?: boolean;
|
||||
/** MCP tool names to activate for the current session when discovery mode is enabled. */
|
||||
@@ -511,7 +518,15 @@ export class AgentSession {
|
||||
#onResponse: SimpleStreamOptions["onResponse"] | undefined;
|
||||
#convertToLlm: (messages: AgentMessage[]) => Message[] | Promise<Message[]>;
|
||||
#rebuildSystemPrompt: ((toolNames: string[], tools: Map<string, AgentTool>) => Promise<string>) | undefined;
|
||||
#getMcpServerInstructions: (() => Map<string, string> | undefined) | undefined;
|
||||
#baseSystemPrompt: string;
|
||||
/**
|
||||
* Signature of the (toolNames, tool descriptions) tuple passed to the most
|
||||
* recent successful `rebuildSystemPrompt` call. Used to skip redundant rebuilds
|
||||
* when MCP servers reconnect without changing their tool definitions, which is
|
||||
* the dominant cause of prompt-cache invalidation in long sessions.
|
||||
*/
|
||||
#lastAppliedToolSignature: string | undefined;
|
||||
#mcpDiscoveryEnabled = false;
|
||||
#discoverableMCPTools = new Map<string, DiscoverableMCPTool>();
|
||||
#discoverableMCPSearchIndex: DiscoverableMCPSearchIndex | null = null;
|
||||
@@ -595,6 +610,7 @@ export class AgentSession {
|
||||
this.#onResponse = config.onResponse;
|
||||
this.#convertToLlm = config.convertToLlm ?? convertToLlm;
|
||||
this.#rebuildSystemPrompt = config.rebuildSystemPrompt;
|
||||
this.#getMcpServerInstructions = config.getMcpServerInstructions;
|
||||
this.#baseSystemPrompt = this.agent.state.systemPrompt;
|
||||
this.#mcpDiscoveryEnabled = config.mcpDiscoveryEnabled ?? false;
|
||||
this.#setDiscoverableMCPTools(this.#collectDiscoverableMCPToolsFromRegistry());
|
||||
@@ -2211,10 +2227,18 @@ export class AgentSession {
|
||||
}
|
||||
this.agent.setTools(tools);
|
||||
|
||||
// Rebuild base system prompt with new tool set
|
||||
// Rebuild base system prompt with new tool set, but only when the tool set
|
||||
// actually changed. MCP servers can reconnect at arbitrary times and call
|
||||
// `refreshMCPTools` -> `#applyActiveToolsByName` even though the resulting
|
||||
// tool list is byte-identical. Skipping the rebuild keeps the system prompt
|
||||
// stable, which is required for Anthropic prompt caching to keep hitting.
|
||||
if (this.#rebuildSystemPrompt) {
|
||||
this.#baseSystemPrompt = await this.#rebuildSystemPrompt(validToolNames, this.#toolRegistry);
|
||||
this.agent.setSystemPrompt(this.#baseSystemPrompt);
|
||||
const signature = this.#computeAppliedToolSignature(validToolNames, tools);
|
||||
if (signature !== this.#lastAppliedToolSignature) {
|
||||
this.#baseSystemPrompt = await this.#rebuildSystemPrompt(validToolNames, this.#toolRegistry);
|
||||
this.agent.setSystemPrompt(this.#baseSystemPrompt);
|
||||
this.#lastAppliedToolSignature = signature;
|
||||
}
|
||||
}
|
||||
if (options?.persistMCPSelection !== false) {
|
||||
this.#persistSelectedMCPToolNamesIfChanged(previousSelectedMCPToolNames);
|
||||
@@ -2256,6 +2280,65 @@ export class AgentSession {
|
||||
const activeToolNames = this.getActiveToolNames();
|
||||
this.#baseSystemPrompt = await this.#rebuildSystemPrompt(activeToolNames, this.#toolRegistry);
|
||||
this.agent.setSystemPrompt(this.#baseSystemPrompt);
|
||||
// Refresh the cached signature so a subsequent `#applyActiveToolsByName` with
|
||||
// the same tool set does not re-rebuild on top of the explicit refresh we
|
||||
// just performed (and conversely, a different set forces a fresh rebuild).
|
||||
const activeTools = activeToolNames
|
||||
.map(name => this.#toolRegistry.get(name))
|
||||
.filter((tool): tool is AgentTool => tool != null);
|
||||
this.#lastAppliedToolSignature = this.#computeAppliedToolSignature(activeToolNames, activeTools);
|
||||
}
|
||||
|
||||
/**
|
||||
* Compose a stable signature for the inputs that `rebuildSystemPrompt` reads.
|
||||
* Two calls producing identical signatures are guaranteed to produce identical
|
||||
* system prompt bytes, so the rebuild can be skipped.
|
||||
*
|
||||
* The signature covers:
|
||||
* 1. Active tool names in order (the prompt renders them in this order).
|
||||
* 2. Active tool labels and descriptions (both are echoed in the prompt body;
|
||||
* see `system-prompt.md` `{{label}}: \`{{name}}\`` rendering).
|
||||
* 3. When MCP discovery is on, every registry tool's name+label+description,
|
||||
* since `rebuildSystemPrompt` summarizes discoverable MCP tools that are
|
||||
* not in the active set.
|
||||
* 4. MCP server instructions text (per server), since `rebuildSystemPrompt`
|
||||
* embeds these in the appended prompt under "## MCP Server Instructions".
|
||||
* A server upgrade can change instructions while keeping tools identical.
|
||||
*
|
||||
* Inputs NOT covered: tool input schemas, memory instructions read from disk,
|
||||
* and any other ambient state the rebuild closure reads. Callers must explicitly
|
||||
* call `refreshBaseSystemPrompt()` after such side-effecting changes; see e.g.
|
||||
* the memory hooks and `#syncEditToolModeAfterModelChange`.
|
||||
*/
|
||||
#computeAppliedToolSignature(toolNames: string[], tools: AgentTool[]): string {
|
||||
// Order-preserving join: any reorder must produce a different signature so
|
||||
// the rebuild fires and the new tool list reaches the API.
|
||||
const nameSegment = toolNames.join("\u0001");
|
||||
const describeTool = (tool: AgentTool): string => `${tool.name}=${tool.label ?? ""}|${tool.description ?? ""}`;
|
||||
const descriptionSegment = tools.map(describeTool).join("\u0002");
|
||||
let registrySegment = "";
|
||||
if (this.#mcpDiscoveryEnabled) {
|
||||
// Registry iteration order is not load-bearing for the prompt content, so we
|
||||
// sort to keep the signature insensitive to incidental insertion order.
|
||||
const entries: string[] = [];
|
||||
for (const tool of this.#toolRegistry.values()) {
|
||||
entries.push(describeTool(tool));
|
||||
}
|
||||
entries.sort();
|
||||
registrySegment = entries.join("\u0004");
|
||||
}
|
||||
let instructionsSegment = "";
|
||||
const serverInstructions = this.#getMcpServerInstructions?.();
|
||||
if (serverInstructions && serverInstructions.size > 0) {
|
||||
// Sort by server name so transport flap order does not perturb the signature.
|
||||
const entries: string[] = [];
|
||||
for (const [server, instructions] of serverInstructions) {
|
||||
entries.push(`${server}=${instructions}`);
|
||||
}
|
||||
entries.sort();
|
||||
instructionsSegment = entries.join("\u0006");
|
||||
}
|
||||
return `${nameSegment}\u0003${descriptionSegment}\u0005${registrySegment}\u0007${instructionsSegment}`;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,303 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test";
|
||||
import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import type { Model } from "@oh-my-pi/pi-ai";
|
||||
import { Type } from "@sinclair/typebox";
|
||||
import { Settings } from "../src/config/settings";
|
||||
import type { CustomTool } from "../src/extensibility/custom-tools/types";
|
||||
import { AgentSession } from "../src/session/agent-session";
|
||||
import { SessionManager } from "../src/session/session-manager";
|
||||
|
||||
// Cache-stability invariant: when MCP servers reconnect with byte-identical tool
|
||||
// definitions, `refreshMCPTools` must not rebuild the system prompt. A rebuild
|
||||
// invalidates the Anthropic prompt-cache breakpoint placed on the system block
|
||||
// and forces a full prefix re-encode on the next request.
|
||||
|
||||
function createModel(): Model<"openai-responses"> {
|
||||
return {
|
||||
id: "mock",
|
||||
name: "mock",
|
||||
api: "openai-responses",
|
||||
provider: "openai",
|
||||
baseUrl: "https://example.invalid",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 8192,
|
||||
maxTokens: 2048,
|
||||
};
|
||||
}
|
||||
|
||||
function createBasicTool(name: string, label: string, description = `${label} tool`): AgentTool {
|
||||
return {
|
||||
name,
|
||||
label,
|
||||
description,
|
||||
parameters: Type.Object({ value: Type.String() }),
|
||||
strict: true,
|
||||
async execute() {
|
||||
return { content: [{ type: "text", text: `${name} executed` }] };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createMcpCustomTool(name: string, serverName: string, mcpToolName: string, description: string): CustomTool {
|
||||
return {
|
||||
name,
|
||||
label: `${serverName}/${mcpToolName}`,
|
||||
description,
|
||||
parameters: Type.Object({ q: Type.String() }),
|
||||
strict: true,
|
||||
mcpServerName: serverName,
|
||||
mcpToolName,
|
||||
async execute() {
|
||||
return { content: [{ type: "text", text: `${name} executed` }] };
|
||||
},
|
||||
} as CustomTool;
|
||||
}
|
||||
|
||||
describe("AgentSession refreshMCPTools rebuild skipping", () => {
|
||||
const sessions: AgentSession[] = [];
|
||||
|
||||
afterEach(async () => {
|
||||
for (const session of sessions.splice(0)) {
|
||||
await session.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
interface NewSessionOptions {
|
||||
mcpDiscoveryEnabled?: boolean;
|
||||
getMcpServerInstructions?: () => Map<string, string> | undefined;
|
||||
}
|
||||
|
||||
function newSession(
|
||||
rebuildSystemPrompt: (toolNames: string[]) => Promise<string>,
|
||||
options: NewSessionOptions = {},
|
||||
): {
|
||||
session: AgentSession;
|
||||
} {
|
||||
const readTool = createBasicTool("read", "Read");
|
||||
const initialMcp = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus");
|
||||
const toolRegistry = new Map<string, AgentTool>([
|
||||
[readTool.name, readTool],
|
||||
[initialMcp.name, initialMcp as unknown as AgentTool],
|
||||
]);
|
||||
const agent = new Agent({
|
||||
initialState: {
|
||||
model: createModel(),
|
||||
systemPrompt: "initial",
|
||||
tools: [readTool, initialMcp as unknown as AgentTool],
|
||||
messages: [],
|
||||
},
|
||||
});
|
||||
const session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings: Settings.isolated({ "compaction.enabled": false }),
|
||||
modelRegistry: {} as never,
|
||||
toolRegistry,
|
||||
rebuildSystemPrompt,
|
||||
mcpDiscoveryEnabled: options.mcpDiscoveryEnabled,
|
||||
getMcpServerInstructions: options.getMcpServerInstructions,
|
||||
});
|
||||
sessions.push(session);
|
||||
return { session };
|
||||
}
|
||||
|
||||
it("skips rebuild when an MCP refresh produces an identical tool set", async () => {
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
// The session constructor does not run rebuildSystemPrompt; baseline=0.
|
||||
expect(rebuildCount).toBe(0);
|
||||
|
||||
const initialMcp = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search nucleus");
|
||||
|
||||
// First refresh: no signature recorded yet, must rebuild.
|
||||
await session.refreshMCPTools([initialMcp]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
// Second refresh with byte-identical metadata: must NOT rebuild.
|
||||
await session.refreshMCPTools([initialMcp]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
// Third refresh, again identical: still no rebuild.
|
||||
await session.refreshMCPTools([initialMcp]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
});
|
||||
|
||||
it("rebuilds when an MCP tool's description changes", async () => {
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
|
||||
const v1 = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search v1");
|
||||
await session.refreshMCPTools([v1]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
const v2 = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search v2");
|
||||
await session.refreshMCPTools([v2]);
|
||||
expect(rebuildCount).toBe(2);
|
||||
});
|
||||
|
||||
it("rebuilds when the active tool list changes via setActiveToolsByName", async () => {
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
|
||||
const a = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
|
||||
const b = createMcpCustomTool("mcp__nucleus_explain", "nucleus", "explain", "Explain");
|
||||
|
||||
// Bring both into the registry; only `mcp__nucleus_search` becomes active per
|
||||
// the agent's initial tool set.
|
||||
await session.refreshMCPTools([a, b]);
|
||||
const baseline = rebuildCount;
|
||||
expect(baseline).toBeGreaterThanOrEqual(1);
|
||||
|
||||
// Activate the previously-inactive tool: the active list grew, signature must
|
||||
// change, rebuild must fire.
|
||||
await session.setActiveToolsByName(["read", "mcp__nucleus_search", "mcp__nucleus_explain"]);
|
||||
expect(rebuildCount).toBe(baseline + 1);
|
||||
|
||||
// Same list again: skip.
|
||||
await session.setActiveToolsByName(["read", "mcp__nucleus_search", "mcp__nucleus_explain"]);
|
||||
expect(rebuildCount).toBe(baseline + 1);
|
||||
|
||||
// Drop one: rebuild fires again.
|
||||
await session.setActiveToolsByName(["read", "mcp__nucleus_search"]);
|
||||
expect(rebuildCount).toBe(baseline + 2);
|
||||
});
|
||||
|
||||
it("does not skip when refreshBaseSystemPrompt is called explicitly", async () => {
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
|
||||
const tool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
// Explicit refresh must always rebuild (callers use it to pick up env-side changes
|
||||
// such as edit mode toggles, which are invisible to our tool signature).
|
||||
await session.refreshBaseSystemPrompt();
|
||||
expect(rebuildCount).toBe(2);
|
||||
|
||||
// Subsequent identical MCP refresh should still skip after the explicit refresh
|
||||
// freshens the cached signature.
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(2);
|
||||
});
|
||||
|
||||
it("ignores incidental insertion order in the refresh argument", async () => {
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
|
||||
const a = createMcpCustomTool("mcp__nucleus_a", "nucleus", "a", "A");
|
||||
const b = createMcpCustomTool("mcp__nucleus_b", "nucleus", "b", "B");
|
||||
|
||||
// `refreshMCPTools` re-derives the active tool set from the registry using the
|
||||
// session's existing ordering, so passing the same content in a different order
|
||||
// must not mutate the signature.
|
||||
await session.refreshMCPTools([a, b]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
await session.refreshMCPTools([b, a]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
});
|
||||
|
||||
it("rebuilds when an MCP tool's label changes", async () => {
|
||||
// Tool labels are rendered into the prompt body (`{{label}}: \`{{name}}\``),
|
||||
// so a label change \u2014 even with name and description constant \u2014 must force
|
||||
// a rebuild. Otherwise we'd serve a stale label after an MCP server upgrade.
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
});
|
||||
|
||||
const v1 = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
|
||||
// Override the auto-derived label so the test mutates only the label.
|
||||
const v1WithLabel = { ...v1, label: "old label" } as typeof v1;
|
||||
await session.refreshMCPTools([v1WithLabel]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
const v2WithLabel = { ...v1, label: "new label" } as typeof v1;
|
||||
await session.refreshMCPTools([v2WithLabel]);
|
||||
expect(rebuildCount).toBe(2);
|
||||
});
|
||||
|
||||
it("rebuilds when MCP server instructions text changes", async () => {
|
||||
// `rebuildSystemPrompt` embeds per-server `instructions` text into the appended
|
||||
// prompt. The signature must include this so a server upgrade that changes
|
||||
// instructions while keeping tools constant still triggers a rebuild.
|
||||
let rebuildCount = 0;
|
||||
const instructions = new Map<string, string>([["nucleus", "v1 instructions"]]);
|
||||
const { session } = newSession(
|
||||
async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
},
|
||||
{ getMcpServerInstructions: () => instructions },
|
||||
);
|
||||
|
||||
const tool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
// Same tools, same instructions: skip.
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(1);
|
||||
|
||||
// Mutate the live instructions map (callers return the live reference).
|
||||
instructions.set("nucleus", "v2 instructions");
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(2);
|
||||
|
||||
// Adding a new server's instructions also triggers rebuild.
|
||||
instructions.set("glean", "glean instructions");
|
||||
await session.refreshMCPTools([tool]);
|
||||
expect(rebuildCount).toBe(3);
|
||||
});
|
||||
|
||||
it("rebuilds when a discoverable (inactive) registry tool's metadata changes", async () => {
|
||||
// With MCP discovery on, the rebuilt prompt summarizes ALL discoverable MCP
|
||||
// tools \u2014 including ones not in the active set. The signature must capture the
|
||||
// full registry; otherwise a description change to a discoverable-but-inactive
|
||||
// tool would silently leave a stale summary in the cached prompt.
|
||||
let rebuildCount = 0;
|
||||
const { session } = newSession(
|
||||
async toolNames => {
|
||||
rebuildCount++;
|
||||
return `tools:${toolNames.join(",")}`;
|
||||
},
|
||||
{ mcpDiscoveryEnabled: true },
|
||||
);
|
||||
|
||||
const active = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
|
||||
const discoverable = createMcpCustomTool("mcp__nucleus_explain", "nucleus", "explain", "Explain v1");
|
||||
|
||||
await session.refreshMCPTools([active, discoverable]);
|
||||
const baseline = rebuildCount;
|
||||
expect(baseline).toBeGreaterThanOrEqual(1);
|
||||
|
||||
// Same registry: skip.
|
||||
await session.refreshMCPTools([active, discoverable]);
|
||||
expect(rebuildCount).toBe(baseline);
|
||||
|
||||
// Mutate the discoverable (NOT active) tool's description: signature must
|
||||
// differ via the registrySegment branch and force a rebuild.
|
||||
const discoverableV2 = createMcpCustomTool("mcp__nucleus_explain", "nucleus", "explain", "Explain v2");
|
||||
await session.refreshMCPTools([active, discoverableV2]);
|
||||
expect(rebuildCount).toBe(baseline + 1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,67 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { sortMCPToolsByName } from "../src/mcp/manager";
|
||||
|
||||
// `sortMCPToolsByName` is the cache-stability invariant: Anthropic prompt caching
|
||||
// keys on byte-identical tool definitions, so the tools array sent to the API
|
||||
// must be byte-stable across MCP server connect / reconnect / refresh cycles.
|
||||
// These tests defend that invariant directly because the production call sites
|
||||
// (initial discovery and `#replaceServerTools`) are heavy to exercise without
|
||||
// mock transports.
|
||||
|
||||
describe("sortMCPToolsByName", () => {
|
||||
it("orders tools lexicographically by name", () => {
|
||||
const tools = [
|
||||
{ name: "mcp__nucleus_searchCode" },
|
||||
{ name: "mcp__glean_chat" },
|
||||
{ name: "mcp__atlassian_get_issue" },
|
||||
];
|
||||
sortMCPToolsByName(tools);
|
||||
expect(tools.map(t => t.name)).toEqual([
|
||||
"mcp__atlassian_get_issue",
|
||||
"mcp__glean_chat",
|
||||
"mcp__nucleus_searchCode",
|
||||
]);
|
||||
});
|
||||
|
||||
it("produces identical output regardless of insertion order", () => {
|
||||
// Simulates the multi-server reconnect bug: depending on which MCP server
|
||||
// reconnected most recently, the same set of tools could land in different
|
||||
// orders in `#tools`. After sorting, the array bytes are identical.
|
||||
const orderA = sortMCPToolsByName([
|
||||
{ name: "mcp__nucleus_a" },
|
||||
{ name: "mcp__nucleus_b" },
|
||||
{ name: "mcp__glean_x" },
|
||||
{ name: "mcp__glean_y" },
|
||||
]);
|
||||
const orderB = sortMCPToolsByName([
|
||||
{ name: "mcp__glean_x" },
|
||||
{ name: "mcp__glean_y" },
|
||||
{ name: "mcp__nucleus_a" },
|
||||
{ name: "mcp__nucleus_b" },
|
||||
]);
|
||||
expect(orderA.map(t => t.name)).toEqual(orderB.map(t => t.name));
|
||||
});
|
||||
|
||||
it("mutates the input array in place and returns the same reference", () => {
|
||||
const tools = [{ name: "b" }, { name: "a" }];
|
||||
const result = sortMCPToolsByName(tools);
|
||||
expect(result).toBe(tools);
|
||||
expect(tools.map(t => t.name)).toEqual(["a", "b"]);
|
||||
});
|
||||
|
||||
it("preserves total order under repeated sorts", () => {
|
||||
// Reconnects re-sort an already-sorted array. The output must be byte-stable
|
||||
// across repeated sorts so the tools cache breakpoint keeps hitting; ES2019+
|
||||
// guarantees a stable sort, and MCP tool names are globally unique within a
|
||||
// session, so the comparator's strict total order yields one canonical result.
|
||||
const tools = sortMCPToolsByName([{ name: "c" }, { name: "a" }, { name: "b" }]);
|
||||
const before = tools.map(t => t.name);
|
||||
sortMCPToolsByName(tools);
|
||||
expect(tools.map(t => t.name)).toEqual(before);
|
||||
});
|
||||
|
||||
it("handles empty arrays and single-element arrays", () => {
|
||||
expect(sortMCPToolsByName([])).toEqual([]);
|
||||
expect(sortMCPToolsByName([{ name: "only" }]).map(t => t.name)).toEqual(["only"]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user