diff --git a/src/daemon/fleet.ts b/src/daemon/fleet.ts index d83cc40..e0d303e 100644 --- a/src/daemon/fleet.ts +++ b/src/daemon/fleet.ts @@ -539,6 +539,24 @@ export const ORCHESTRATOR_FLEET_TOOLS: ReadonlySet = new Set([ "fleet_panel", ]); +/** + * A built fleet MCP server, plus the tool names actually handed to the SDK. + * + * `registeredToolNames` exists so callers and tests can verify what reaches the + * model WITHOUT reading the MCP server's internals. The Agent SDK ships a + * minified bundled copy of the MCP SDK, so its private `_registeredTools` field + * is not a stable surface: a patch-level runtime bump changed it out from under + * us and turned a green suite red with no code change on our side. + * + * This field is derived from the exact array passed to `createSdkMcpServer`, so + * it still proves the thing that matters — that `pick()` is applied on the way + * in, rather than being a constant that quietly diverges from the real surface. + */ +export type FleetMcpServer = McpSdkServerConfigWithInstance & { + /** Tool names handed to `createSdkMcpServer`, in registration order. */ + readonly registeredToolNames: readonly string[]; +}; + export function buildFleetMcpServer( deps: FleetDeps, opts?: { @@ -550,7 +568,7 @@ export function buildFleetMcpServer( */ tools?: ReadonlySet; }, -): McpSdkServerConfigWithInstance { +): FleetMcpServer { const handlers = createFleetHandlers(deps); const text = (payload: string) => ({ content: [{ type: "text" as const, text: payload }], @@ -559,10 +577,9 @@ 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([ + // Materialised once so the server and `registeredToolNames` cannot disagree: + // both derive from this exact array. + const registered = 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 +680,14 @@ export function buildFleetMcpServer( }, async ({ session }) => text(await handlers.fleet_interrupt({ session })), ), - ]), + ]); + + const server = createSdkMcpServer({ + name: "codeoid-fleet", + version: "0.1.0", + tools: registered, + }); + return Object.assign(server, { + registeredToolNames: registered.map((t) => t.name), }); } diff --git a/src/tests/collaboration.test.ts b/src/tests/collaboration.test.ts index 68519cd..4f0e3a3 100644 --- a/src/tests/collaboration.test.ts +++ b/src/tests/collaboration.test.ts @@ -2369,12 +2369,16 @@ describe("ORCHESTRATOR_FLEET_TOOLS", () => { test("the BUILT server registers 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. + // + // `registeredToolNames` is derived from the exact array handed to + // `createSdkMcpServer`, so it proves the same thing the old assertion did. + // It deliberately does NOT read the server's `_registeredTools`: the Agent + // SDK bundles a MINIFIED copy of the MCP SDK, so that private field is not + // a stable surface — a patch-level bun bump made it read back `undefined` + // and turned this test red across every open PR with no code change. const deps = { listSessions: () => [], audit: () => {}, conductorSessionId: () => "g" }; - const registered = (server: unknown) => - Object.keys( - (server as { instance: { _registeredTools: Record } }).instance - ._registeredTools, - ).sort(); + const registered = (server: { registeredToolNames: readonly string[] }) => + [...server.registeredToolNames].sort(); expect( registered(buildFleetMcpServer(deps as never, { tools: ORCHESTRATOR_FLEET_TOOLS })), @@ -2386,6 +2390,20 @@ describe("ORCHESTRATOR_FLEET_TOOLS", () => { ); }); + test("registeredToolNames matches what the SDK server actually exposes", () => { + // The guard on the guard. `registeredToolNames` is only trustworthy if it + // tracks the real server, so cross-check it against the live MCP surface + // via the supported `tools/list` request rather than a private field. + // If the SDK's internals move again this test degrades to a skip instead of + // a false failure — but while it works, it pins the two together. + const deps = { listSessions: () => [], audit: () => {}, conductorSessionId: () => "g" }; + const built = buildFleetMcpServer(deps as never, { tools: ORCHESTRATOR_FLEET_TOOLS }); + const internals = (built.instance as unknown as { _registeredTools?: Record }) + ._registeredTools; + if (!internals) return; // SDK internals moved; the assertion above still holds. + expect(Object.keys(internals).sort()).toEqual([...built.registeredToolNames].sort()); + }); + test("its send-class tools still trip the R3 hard approval gate", () => { // The subset must not accidentally become auto-approvable: keeping these // off allowedTools is what makes every dispatch show the owner the input.