From 6d8474a7f0cd9b60918102937563f99983bb26c1 Mon Sep 17 00:00:00 2001 From: marselsel Date: Wed, 17 Jun 2026 13:58:53 +0000 Subject: [PATCH] fix(sdk): instrument Skybridge's registerTool(config, cb) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The proxy's `registerTool` interceptor only handled the MCP-SDK `registerTool(name, config, cb)` signature — name from a string `args[0]`, callback from `args[2]`. Skybridge's `McpServer` uses `registerTool(config, cb)` (name on `config.name`, callback at `args[1]`), so the interceptor wrapped nothing: Skybridge-registered tools emitted a `tool_discovery` named "unknown" and produced **no `tool_call` or `connection` events**. Locate the callback by type and derive the name from the string arg or the config object's `name`, so both signatures are instrumented. The MCP-SDK path is unchanged (same name, same wrapped callback); this only adds the Skybridge case. The fluent-chain handling already supported Skybridge — this completes it. Verified empirically against @yavio/sdk@0.1.6 + skybridge 0.36 and 1.1: a Skybridge `registerTool(config, cb)` now emits tool_discovery (correct name), connection, and tool_call. Adds regression tests for the 2-arg form. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/sdk/src/__tests__/proxy.test.ts | 80 ++++++++++++++++++++++++ packages/sdk/src/server/proxy.ts | 37 +++++++---- 2 files changed, 104 insertions(+), 13 deletions(-) diff --git a/packages/sdk/src/__tests__/proxy.test.ts b/packages/sdk/src/__tests__/proxy.test.ts index 98c34fe..1b6cd70 100644 --- a/packages/sdk/src/__tests__/proxy.test.ts +++ b/packages/sdk/src/__tests__/proxy.test.ts @@ -892,3 +892,83 @@ describe("createProxy — session reuse", () => { expect(firstSessionId).toMatch(/^ses_/); }); }); + +describe("createProxy — Skybridge registerTool(config, cb)", () => { + beforeEach(() => { + _resetGlobalState(); + mockedMint.mockResolvedValue(null as never); + }); + + // Skybridge's McpServer uses `registerTool(config, cb)` (the name lives on + // `config.name`) and returns `this` for chaining — unlike the MCP SDK's + // `registerTool(name, config, cb)`. The proxy must instrument both. + function createSkybridgeMock() { + const mock: Record = { + stored: null, + tool() { + return mock; + }, + registerTool(...args: unknown[]) { + mock.stored = args.find((a) => typeof a === "function"); + return mock; // fluent — returns the server itself + }, + connect: async () => {}, + server: { getClientVersion: () => ({ name: "test-client" }) }, + }; + return mock; + } + + it("derives the tool name from config.name (not 'unknown')", () => { + const mock = createSkybridgeMock(); + const transport = createMockTransport(); + const proxy = createProxy(mock as never, testConfig, transport, "0.0.1"); + + proxy.registerTool( + { name: "compare_prices", description: "Compare prices", inputSchema: { q: {} } } as never, + (() => ({ content: [{ type: "text", text: "ok" }] })) as never, + ); + + const discovery = transport.sent + .flat() + .find((e) => (e as Record).event_type === "tool_discovery") as + | Record + | undefined; + expect(discovery?.tool_name).toBe("compare_prices"); + }); + + it("wraps the handler so invoking it emits a tool_call event", async () => { + const mock = createSkybridgeMock(); + const transport = createMockTransport(); + const proxy = createProxy(mock as never, testConfig, transport, "0.0.1"); + + proxy.registerTool( + { name: "compare_prices", description: "d" } as never, + (async () => ({ content: [{ type: "text", text: "ok" }] })) as never, + ); + + const handler = mock.stored as (...a: unknown[]) => Promise; + expect(typeof handler).toBe("function"); + await handler({ q: "x" }, { sessionId: "s1", _meta: {} }); + + const call = transport.sent + .flat() + .find((e) => (e as Record).event_type === "tool_call") as + | Record + | undefined; + expect(call?.event_name).toBe("compare_prices"); + }); + + it("keeps the fluent chain intercepted across registrations", () => { + const mock = createSkybridgeMock(); + const transport = createMockTransport(); + const proxy = createProxy(mock as never, testConfig, transport, "0.0.1"); + + const ret = proxy.registerTool( + { name: "a" } as never, + (() => ({ content: [] })) as never, + ); + // Skybridge returns `this`; the proxy must hand back the proxy so the next + // chained registration stays instrumented. + expect(ret).toBe(proxy); + }); +}); diff --git a/packages/sdk/src/server/proxy.ts b/packages/sdk/src/server/proxy.ts index d3259d0..588c269 100644 --- a/packages/sdk/src/server/proxy.ts +++ b/packages/sdk/src/server/proxy.ts @@ -360,12 +360,25 @@ export function createProxy( if (prop === "registerTool") { return (...args: unknown[]) => { - // registerTool(name: string, config: object, cb: Function) - // Callback is always the 3rd argument (index 2) - const toolName = typeof args[0] === "string" ? args[0] : "unknown"; - if (args.length >= 3 && typeof args[2] === "function") { - const originalCb = args[2] as (...cbArgs: unknown[]) => unknown; - args[2] = wrapToolCallback( + // Two registerTool signatures are supported: + // - MCP SDK: registerTool(name: string, config: object, cb) + // - Skybridge: registerTool(config: object, cb) // name is config.name + // Locate the callback by type rather than a fixed index, and derive + // the tool name from the string arg or the config object's `name`. + const cbIndex = args.findIndex((a) => typeof a === "function"); + const configObj = args.find( + (a, i) => i !== cbIndex && a !== null && typeof a === "object", + ) as Record | undefined; + const toolName = + typeof args[0] === "string" + ? args[0] + : typeof configObj?.name === "string" + ? configObj.name + : "unknown"; + + if (cbIndex !== -1) { + const originalCb = args[cbIndex] as (...cbArgs: unknown[]) => unknown; + args[cbIndex] = wrapToolCallback( originalCb, toolName, resolveSession, @@ -384,14 +397,12 @@ export function createProxy( emittedToolDiscoveries.add(toolName); let description: string | undefined; let inputSchema: Record | undefined; - const configArg = args[1]; - if (configArg && typeof configArg === "object") { - const obj = configArg as Record; - if (typeof obj.description === "string") { - description = obj.description; + if (configObj) { + if (typeof configObj.description === "string") { + description = configObj.description; } - if (obj.inputSchema && typeof obj.inputSchema === "object") { - inputSchema = obj.inputSchema as Record; + if (configObj.inputSchema && typeof configObj.inputSchema === "object") { + inputSchema = configObj.inputSchema as Record; } } emitDiscovery(toolName, description, inputSchema);