refactor(mcp): add registerTool wrapper to deduplicate tracing boilerplate#2559
Conversation
…plate
Introduce a ToolRegistrar pattern that wraps server.registerTool with
automatic withToolTracing, eliminating the need for each tool file to:
- Pass the tool name twice (registerTool + withToolTracing)
- Import McpServer, withToolTracing, and McpContext separately
- Manually wire up tracing in every handler
All 24 tool files now receive { context, registerTool } and call
registerTool(name, config, handler) directly.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
github-actions
Bot
added
the
review/tier-4
🔴 Tier 4 — CriticalTouches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD. Why this tier:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
Greptile SummaryThis PR centralizes MCP tool registration boilerplate into a
Confidence Score: 5/5Pure mechanical refactor — no logic, routing, or data-handling changes; tracing is applied identically to all 24 tool handlers via the new factory. Every tool file follows the exact same migration pattern: drop two imports, destructure the registrar, remove the withToolTracing wrapper call. The factory in registerTool.ts passes name and context to withToolTracing correctly, and the AnyZodObject binding is a sound workaround for the SDK generic constraint. No behavioral changes were introduced. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[createServer] --> B["createRegisterTool(server, context)"]
B --> C["registerTool fn bound to server + context"]
A --> D["registrar = { server, context, registerTool }"]
D --> E[sourcesTools]
D --> F[alertsTools]
D --> G[dashboardsTools]
D --> H[queryTools]
D --> I[savedSearchesTools]
D --> J[traceTools]
A --> K["dashboardPrompts(server, context) — no tracing wrapper"]
subgraph ToolFile["Inside each tool file"]
L["registerTool(name, config, handler)"]
L --> M["withToolTracing(name, context, handler)"]
M --> N["server.registerTool(name, config, traced)"]
end
C --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[createServer] --> B["createRegisterTool(server, context)"]
B --> C["registerTool fn bound to server + context"]
A --> D["registrar = { server, context, registerTool }"]
D --> E[sourcesTools]
D --> F[alertsTools]
D --> G[dashboardsTools]
D --> H[queryTools]
D --> I[savedSearchesTools]
D --> J[traceTools]
A --> K["dashboardPrompts(server, context) — no tracing wrapper"]
subgraph ToolFile["Inside each tool file"]
L["registerTool(name, config, handler)"]
L --> M["withToolTracing(name, context, handler)"]
M --> N["server.registerTool(name, config, traced)"]
end
C --> L
Reviews (3): Last reviewed commit: "Merge branch 'main' into brandon/brandon..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@pulpdrew PR for you - its actually fairly small, turn off whitespace diffing to view this one |
E2E Test Results✅ All tests passed • 225 passed • 3 skipped • 1450s
Tests ran across 4 shards in parallel. |
Move RegisterToolFn and ToolResult into tools/types.ts so the dependency flows one way (registerTool.ts → types.ts, tracing.ts → types.ts) with no cycle. Drop unused annotations field and ToolAnnotations import.
Deep Review✅ No critical issues found. This is a behavior-preserving mechanical refactor: tracing coverage is identical before and after (every tool was and still is wrapped by 🟡 P2 -- recommended
🔵 P3 nitpicks (1)
Reviewers (7): correctness, adversarial, kieran-typescript, api-contract, testing, maintainability, project-standards. Testing gaps:
|
2 participants
Summary
MCP tool registration required every tool file to independently import
McpServer,withToolTracing, andMcpContext, then pass the tool name string twice — once toserver.registerTool()and again towithToolTracing(). This was error-prone and noisy across 24 tool files.A new
ToolRegistrarpattern ({ server, context, registerTool }) centralizes tracing into acreateRegisterToolfactory. Tool files now destructure{ context, registerTool }and callregisterTool(name, config, handler)directly — tracing is applied automatically.What changed
registerTool.ts—createRegisterTool(server, context)factory that returns aRegisterToolFn. Wraps every handler withwithToolTracinginternally, binding the tool name once.ToolRegistrartype intypes.ts— replaces the old(server, context)function signature with({ server, context, registerTool }).tracing.ts—ToolResultis nowCallToolResult & { content: ... }so it's assignable to the SDK's return type without casts.withToolTracingreturn signature accepts the SDK'sextraparameter.withToolTracingwrapper call.Design decisions
The SDK's
server.registerToolinfers its ownInputArgsgeneric fromconfig.inputSchema, producing aToolCallbackwhose args type is computed via a conditionalBaseToolCallback. Our wrapper's generic (TSchema) can't unify with the SDK'sInputArgsacross the call boundary — they resolve identically at every call site but TypeScript can't prove it at the definition site. Rather than using a type assertion, the wrapper explicitly binds the SDK generic toAnyZodObjectso both sides resolve through the sameAnySchemabranch ofBaseToolCallback, making the assignment structurally sound with no casts.Prompt-related tools (
server.registerPrompt) keep their existing(server, context)signature since they don't usewithToolTracing.Changeset
No changeset — internal plumbing refactor with no user-facing changes.