Skip to content

fix(sdk): instrument Skybridge's registerTool(config, cb)#15

Open
marselsel wants to merge 1 commit into
mainfrom
fix/skybridge-register-tool
Open

fix(sdk): instrument Skybridge's registerTool(config, cb)#15
marselsel wants to merge 1 commit into
mainfrom
fix/skybridge-register-tool

Conversation

@marselsel

Copy link
Copy Markdown
Collaborator

Problem

createProxy's registerTool interceptor only understands the MCP-SDK signature registerTool(name, config, cb) (name = string args[0], callback = args[2]). Skybridge's McpServer uses registerTool(config, cb) — the name lives on config.name and the callback is args[1]. So against a Skybridge server the interceptor wraps nothing: tools emit a tool_discovery named "unknown" and produce zero tool_call / connection events.

This affects every Skybridge-based MCP app (any skybridge version — 1.1.0 still uses the same 2-arg (config, cb) signature).

Fix

Find the callback by type instead of a fixed index, and derive the tool name from the string arg or the config object's name. ~15 lines in proxy.ts.

  • MCP-SDK path unchanged — same name, same wrapped callback (backward compatible).
  • Skybridge path now instrumentedtool_discovery (correct name), connection, tool_call all fire.
  • The fluent-chain handling (chainable) already supported Skybridge's return this; this completes it.

Verification

  • Added regression tests (proxy.test.ts → "Skybridge registerTool(config, cb)") covering name derivation, handler wrapping → tool_call, and fluent chaining.
  • Verified empirically against @yavio/sdk@0.1.6 with skybridge 0.36 and 1.1: a Skybridge registerTool(config, cb) emits tool_discovery (correct name), connection, and tool_call.

Context: found while instrumenting a Skybridge MCP app. We're currently shipping this as a local pnpm patch; this PR would let consumers drop it. Opening for review — no rush to merge.

🤖 Generated with Claude Code

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) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Coverage Report for sdk

Status Category Percentage Covered / Total
🔵 Lines 92.59% 1538 / 1661
🔵 Statements 92.59% 1538 / 1661
🔵 Functions 94.64% 106 / 112
🔵 Branches 81.95% 445 / 543
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/sdk/src/server/proxy.ts 91.14% 74.8% 100% 91.14% 45-46, 120, 314-315, 349-350, 377, 421, 426, 484-485, 496-497, 519-520, 530-534, 536-546, 565-566, 569-570, 581-582
Generated in workflow #57 for commit 6d8474a by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant