Files
oh-my-pi/packages/coding-agent/test/mcp-tool-ordering.test.ts
T
Miroslav Drbal b5ca55e79f 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.
2026-04-30 15:04:33 +02:00

68 lines
2.6 KiB
TypeScript

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