Merge pull request #890 from apoc/fix/mcp-tool-cache-stability

fix(coding-agent/mcp): stabilize tool ordering and skip redundant prompt rebuilds

packages/coding-agent/CHANGELOG.md

@@ -41,6 +41,10 @@
 
 - Fixed eval startup messaging to report `eval` as unavailable when Python is unreachable and JavaScript backend is disabled
 
+### 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

packages/coding-agent/src/mcp/manager.ts

@@ -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 {

packages/coding-agent/src/sdk.ts

@@ -428,6 +428,9 @@ function isCustomTool(tool: CustomTool | ToolDefinition): tool is CustomTool {
 
 const TOOL_DEFINITION_MARKER = Symbol("__isToolDefinition");
 
+/** Matches the truncation applied to per-server instructions inside `rebuildSystemPrompt`. */
+const MAX_MCP_INSTRUCTIONS_LENGTH = 4000;
+
 let sshCleanupRegistered = false;
 
 async function cleanupSshResources(): Promise<void> {
@@ -1337,7 +1340,6 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			const serverInstructions = mcpManager?.getServerInstructions();
 			let appendPrompt: string | undefined = memoryInstructions ?? undefined;
 			if (serverInstructions && serverInstructions.size > 0) {
-				const MAX_INSTRUCTIONS_LENGTH = 4000;
 				const parts: string[] = [];
 				if (appendPrompt) parts.push(appendPrompt);
 				parts.push(
@@ -1345,8 +1347,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 				);
 				for (const [srvName, srvInstructions] of serverInstructions) {
 					const truncated =
-						srvInstructions.length > MAX_INSTRUCTIONS_LENGTH
-							? `${srvInstructions.slice(0, MAX_INSTRUCTIONS_LENGTH)}\n[truncated]`
+						srvInstructions.length > MAX_MCP_INSTRUCTIONS_LENGTH
+							? `${srvInstructions.slice(0, MAX_MCP_INSTRUCTIONS_LENGTH)}\n[truncated]`
 							: srvInstructions;
 					parts.push(`### ${srvName}\n${truncated}`);
 				}
@@ -1626,6 +1628,20 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
 			onResponse,
 			convertToLlm: convertToLlmFinal,
 			rebuildSystemPrompt,
+			getMcpServerInstructions: mcpManager
+				? () => {
+						const raw = mcpManager.getServerInstructions();
+						if (!raw || raw.size === 0) return raw;
+						const out = new Map<string, string>();
+						for (const [name, text] of raw) {
+							out.set(
+								name,
+								text.length > MAX_MCP_INSTRUCTIONS_LENGTH ? text.slice(0, MAX_MCP_INSTRUCTIONS_LENGTH) : text,
+							);
+						}
+						return out;
+					}
+				: undefined,
 			mcpDiscoveryEnabled,
 			initialSelectedMCPToolNames,
 			defaultSelectedMCPToolNames,

packages/coding-agent/src/session/agent-session.ts

@@ -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,86 @@ 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, descriptions, and wire-visible names — all are
+	 *      rendered into the prompt body (see `system-prompt.md` `{{label}}: \`{{name}}\``
+	 *      and `toolPromptNames` in `buildSystemPrompt`). The wire name comes from
+	 *      `tool.customWireName` and overrides the internal name on the model wire
+	 *      (e.g. `edit` exposes itself as `apply_patch` to GPT-5 in apply_patch mode);
+	 *      a stale wire name would desync prompt guidance from actual tool routing.
+	 *   3. When MCP discovery is on, every registry tool's name+label+description+
+	 *      customWireName, 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.
+	 *
+	 * Settings-driven tool metadata is covered automatically: built-in tools that
+	 * depend on settings expose `description`/`label` via getters (see `TaskTool`,
+	 * `SearchToolBm25Tool`, `EditTool`), and the signature reads them live on every
+	 * call - so a settings flip that mutates the rendered string differs the signature
+	 * the next time `#applyActiveToolsByName` runs. Do not refactor `describeTool` to
+	 * cache per-tool strings without preserving this property.
+	 *
+	 * Inputs NOT covered: tool input schemas; memory instructions read from disk;
+	 * and SDK-init-time closure constants in `sdk.ts` (`repeatToolDescriptions`,
+	 * `eagerTasks`, `intentField`, `mcpDiscoveryEnabled`, `secretsEnabled`). The
+	 * closure-captured ones cannot change at runtime regardless of skip behavior.
+	 * For everything else, callers must explicitly call `refreshBaseSystemPrompt()`
+	 * after side-effecting changes; see e.g. the memory hooks and
+	 * `#syncEditToolModeAfterModelChange`.
+	 *
+	 * The current calendar date IS covered (appended as a segment) because
+	 * `buildSystemPrompt` injects it into the prompt body (`Today is '{{date}}'`).
+	 * Without this, a session spanning midnight with only tool-stable MCP
+	 * reconnects would keep yesterday's date indefinitely.
+	 */
+	#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 ?? ""}|${tool.customWireName ?? ""}`;
+		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");
+		}
+		const date = new Date().toISOString().slice(0, 10);
+		return `${nameSegment}\u0003${descriptionSegment}\u0005${registrySegment}\u0007${instructionsSegment}|${date}`;
 	}
 
 	/**

packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts

@@ -0,0 +1,463 @@
+import { afterEach, describe, expect, it, setSystemTime } 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);
+	});
+	it("rebuilds when an MCP tool's customWireName changes", async () => {
+		// `customWireName` overrides the model-facing tool name (e.g. `edit` exposes
+		// itself as `apply_patch` to GPT-5). The wire name is rendered into the prompt
+		// body via `toolPromptNames`, so a wire-name flip with the rest of the metadata
+		// constant would otherwise leave a stale system prompt that advertises the wrong
+		// callable name to the model. The signature must catch this.
+		let rebuildCount = 0;
+		const { session } = newSession(async toolNames => {
+			rebuildCount++;
+			return `tools:${toolNames.join(",")}`;
+		});
+
+		// Attach a custom wire name to the MCP tool. `applyToolProxy` forwards arbitrary
+		// properties from the underlying CustomTool to the wrapper, so the AgentTool the
+		// signature inspects exposes `customWireName` as if it were declared on the type.
+		const v1 = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
+		const v1WithWire = { ...v1, customWireName: "wire_v1" } as typeof v1 & { customWireName: string };
+		await session.refreshMCPTools([v1WithWire]);
+		expect(rebuildCount).toBe(1);
+
+		// Same wire name: skip.
+		await session.refreshMCPTools([v1WithWire]);
+		expect(rebuildCount).toBe(1);
+
+		// Wire name changes while name/label/description stay constant: must rebuild.
+		const v2WithWire = { ...v1, customWireName: "wire_v2" } as typeof v1 & { customWireName: string };
+		await session.refreshMCPTools([v2WithWire]);
+		expect(rebuildCount).toBe(2);
+
+		// Drop wire name entirely: must rebuild (signature must differ from `wire_v2`).
+		await session.refreshMCPTools([v1]);
+		expect(rebuildCount).toBe(3);
+	});
+
+	it("rebuilds when a tool's getter-based description reflects new settings state", async () => {
+		// Built-in tools whose prompt-rendered metadata depends on settings expose
+		// `description` via getters that re-evaluate on every access (TaskTool reads
+		// task.disabledAgents/maxConcurrency/isolation.mode/simple/async.enabled,
+		// SearchToolBm25Tool reads the discoverable MCP tool count, EditTool resolves
+		// through the current edit-mode definition). The signature reads `tool.description`
+		// live each call, so a settings flip that mutates the rendered string MUST differ
+		// the signature on the next `#applyActiveToolsByName`. Defending this contract
+		// against a future refactor that caches per-tool description strings.
+		let rebuildCount = 0;
+		const { session } = newSession(
+			async toolNames => {
+				rebuildCount++;
+				return `tools:${toolNames.join(",")}`;
+			},
+			// Discovery-on so the dynamic tool participates in the registry segment too,
+			// matching the production pattern where `TaskTool` and friends are always
+			// reachable via the registry.
+			{ mcpDiscoveryEnabled: true },
+		);
+
+		// Reuse the initially-active MCP name so the tool stays in the active list
+		// across refreshes - we want to defend the path where `tool.description` is read
+		// for the active descriptionSegment, not just the registrySegment.
+		const settingState = { disabled: "none" };
+		const dynamicTool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "placeholder");
+		Object.defineProperty(dynamicTool, "description", {
+			get: () => `dynamic disabled=${settingState.disabled}`,
+			enumerable: true,
+			configurable: true,
+		});
+
+		await session.refreshMCPTools([dynamicTool]);
+		const baseline = rebuildCount;
+		expect(baseline).toBeGreaterThanOrEqual(1);
+
+		// Same underlying state, same tool object identity: skip.
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline);
+
+		// Mutate the settings-backed state. The tool object identity does not change,
+		// but its `description` getter now returns a new string. The signature must
+		// pick this up live (no per-tool caching) and force a rebuild.
+		settingState.disabled = "plan,explore";
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline + 1);
+
+		// Same state again: skip.
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline + 1);
+	});
+	it("rebuilds when the calendar date rolls over between tool-stable MCP refreshes", async () => {
+		// `buildSystemPrompt` injects today's date into the prompt body.
+		// A session spanning midnight must not serve yesterday's date after an MCP
+		// reconnect that happens to bring an identical tool set.
+		setSystemTime(new Date("2025-01-01T23:59:58Z"));
+		try {
+			let rebuildCount = 0;
+			const { session } = newSession(async toolNames => {
+				rebuildCount++;
+				return `tools:${toolNames.join(",")}`;
+			});
+			const tool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
+
+			// First refresh: no signature yet, must rebuild.
+			await session.refreshMCPTools([tool]);
+			expect(rebuildCount).toBe(1);
+
+			// Same tools, same day: signature matches, skip.
+			await session.refreshMCPTools([tool]);
+			expect(rebuildCount).toBe(1);
+
+			// Advance past midnight.
+			setSystemTime(new Date("2025-01-02T00:00:01Z"));
+
+			// Same tools, new calendar day: date segment changed, must rebuild.
+			await session.refreshMCPTools([tool]);
+			expect(rebuildCount).toBe(2);
+
+			// Same tools, same new day: skip again.
+			await session.refreshMCPTools([tool]);
+			expect(rebuildCount).toBe(2);
+		} finally {
+			setSystemTime(); // restore real time
+		}
+	});
+	it("does not rebuild when MCP server instructions change only beyond the 4000-char truncation boundary", async () => {
+		// `rebuildSystemPrompt` (sdk.ts) truncates each server instruction to 4000 chars
+		// before embedding it. The `getMcpServerInstructions` callback must therefore
+		// return pre-truncated strings so the signature hashes exactly what the prompt
+		// builder uses. Changes beyond char 4000 cannot affect rendered prompt bytes
+		// and must NOT trigger a rebuild.
+		const prefix = "A".repeat(4000);
+		const instructions = new Map<string, string>([["nucleus", `${prefix}_tail_v1`]]);
+		let rebuildCount = 0;
+		const { session } = newSession(
+			async toolNames => {
+				rebuildCount++;
+				return `tools:${toolNames.join(",")}`;
+			},
+			{
+				getMcpServerInstructions: () => {
+					// Mirror what sdk.ts does: truncate to 4000 chars before returning.
+					const out = new Map<string, string>();
+					for (const [name, text] of instructions) {
+						out.set(name, text.length > 4000 ? text.slice(0, 4000) : text);
+					}
+					return out;
+				},
+			},
+		);
+		const tool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "Search");
+
+		await session.refreshMCPTools([tool]);
+		expect(rebuildCount).toBe(1);
+
+		// Mutate only the text beyond char 4000: truncated string is identical → skip.
+		instructions.set("nucleus", `${prefix}_tail_v2`);
+		await session.refreshMCPTools([tool]);
+		expect(rebuildCount).toBe(1);
+
+		// Mutate within the first 4000 chars: truncated string differs → rebuild.
+		instructions.set("nucleus", `${"B".repeat(4000)}_tail_v2`);
+		await session.refreshMCPTools([tool]);
+		expect(rebuildCount).toBe(2);
+	});
+});

packages/coding-agent/test/mcp-tool-ordering.test.ts

@@ -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"]);
+	});
+});