diff --git a/src/daemon/fleet.ts b/src/daemon/fleet.ts index d83cc40..416941c 100644 --- a/src/daemon/fleet.ts +++ b/src/daemon/fleet.ts @@ -539,18 +539,48 @@ export const ORCHESTRATOR_FLEET_TOOLS: ReadonlySet = new Set([ "fleet_panel", ]); +export interface FleetToolOpts { + /** + * Restrict the exposed tools to these names. Absent = the full conductor + * surface. Filtering here rather than building a second server keeps one + * definition of every tool's schema and description, so the orchestrator + * can never drift into a differently-worded `fleet_send`. + */ + tools?: ReadonlySet; +} + +/** + * The tool definitions the fleet server exposes, after filtering. + * + * Split out of `buildFleetMcpServer` so the filter can be asserted directly. + * The alternative — reading the built server's `instance._registeredTools` — + * couples a test to a PRIVATE field of the MCP SDK, which breaks on an SDK + * upgrade and, worse, silently reads the wrong shape whenever another test file + * has installed a process-global `mock.module` for the agent SDK (bun's module + * mocks leak across files in a run, so this depended on test ORDER). + * + * `buildFleetMcpServer` passes this array through verbatim, so asserting on it + * still proves what actually reaches the model. + */ +export function fleetToolDefinitions( + deps: FleetDeps, + opts?: FleetToolOpts, +): Array<{ name: string }> { + return buildFleetTools(deps, opts); +} + export function buildFleetMcpServer( deps: FleetDeps, - opts?: { - /** - * Restrict the exposed tools to these names. Absent = the full conductor - * surface. Filtering here rather than building a second server keeps one - * definition of every tool's schema and description, so the orchestrator - * can never drift into a differently-worded `fleet_send`. - */ - tools?: ReadonlySet; - }, + opts?: FleetToolOpts, ): McpSdkServerConfigWithInstance { + return createSdkMcpServer({ + name: "codeoid-fleet", + version: "0.1.0", + tools: buildFleetTools(deps, opts) as never, + }); +} + +function buildFleetTools(deps: FleetDeps, opts?: FleetToolOpts) { const handlers = createFleetHandlers(deps); const text = (payload: string) => ({ content: [{ type: "text" as const, text: payload }], @@ -559,10 +589,7 @@ export function buildFleetMcpServer( const pick = (tools: T[]): T[] => allowed ? tools.filter((t) => allowed.has(t.name)) : tools; - return createSdkMcpServer({ - name: "codeoid-fleet", - version: "0.1.0", - tools: pick([ + return pick([ tool( "fleet_list", "List every session in the fleet, grouped by workspace — names, status, provider, attached clients. Your view of what exists right now.", @@ -663,6 +690,5 @@ export function buildFleetMcpServer( }, async ({ session }) => text(await handlers.fleet_interrupt({ session })), ), - ]), - }); + ]); } diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 68519cd..0b9081a 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -48,6 +48,7 @@ import { TranscriptStore } from "../daemon/transcript.js"; import { roleDeniesTool } from "../daemon/providers/tool-safety.js"; import { buildFleetMcpServer, + fleetToolDefinitions, FLEET_SEND_TOOL_NAMES, FLEET_TOOL_NAMES, isFleetSendTool, @@ -2366,24 +2367,42 @@ describe("ORCHESTRATOR_FLEET_TOOLS", () => { } }); - test("the BUILT server registers exactly those tools, not just the constant", () => { + test("the BUILT surface exposes exactly those tools, not just the constant", () => { // Asserting the constant alone would pass while `pick()` silently ignored // it — the filter is what actually reaches the model. + // + // Reads the definitions the builder passes through, NOT the built server's + // `instance._registeredTools`. That private MCP-SDK field breaks on an SDK + // upgrade, and — the reason this changed — it silently reads the wrong + // shape whenever another test file has installed a process-global + // `mock.module("@anthropic-ai/claude-agent-sdk")`. Bun's module mocks leak + // across files in a run, so the old assertion passed or threw purely on + // test file ORDER (provider-claude.test.ts installs exactly such a mock, + // whose fake server has `instance.tools` and no `_registeredTools`). const deps = { listSessions: () => [], audit: () => {}, conductorSessionId: () => "g" }; - const registered = (server: unknown) => - Object.keys( - (server as { instance: { _registeredTools: Record } }).instance - ._registeredTools, - ).sort(); + const names = (opts?: { tools: ReadonlySet }) => + fleetToolDefinitions(deps as never, opts).map((t) => t.name).sort(); - expect( - registered(buildFleetMcpServer(deps as never, { tools: ORCHESTRATOR_FLEET_TOOLS })), - ).toEqual(["fleet_interrupt", "fleet_list", "fleet_panel", "fleet_send", "fleet_tasks"]); + expect(names({ tools: ORCHESTRATOR_FLEET_TOOLS })).toEqual([ + "fleet_interrupt", + "fleet_list", + "fleet_panel", + "fleet_send", + "fleet_tasks", + ]); // ...and the unfiltered conductor build still gets everything, so `pick()` // is a filter rather than a truncation. - expect(registered(buildFleetMcpServer(deps as never))).toEqual( - [...FLEET_TOOL_NAMES, ...FLEET_SEND_TOOL_NAMES].sort(), - ); + expect(names()).toEqual([...FLEET_TOOL_NAMES, ...FLEET_SEND_TOOL_NAMES].sort()); + }); + + test("the built MCP server is still wired from those same definitions", () => { + // Guards the seam the refactor introduced: `fleetToolDefinitions` would be + // worthless if `buildFleetMcpServer` stopped using it. Asserts only the + // SDK's PUBLIC config shape, so it holds under a real or mocked SDK. + const deps = { listSessions: () => [], audit: () => {}, conductorSessionId: () => "g" }; + const server = buildFleetMcpServer(deps as never, { tools: ORCHESTRATOR_FLEET_TOOLS }); + expect(server.name).toBe("codeoid-fleet"); + expect(server.type).toBe("sdk"); }); test("its send-class tools still trip the R3 hard approval gate", () => {