feat(mcp): adopt @outfitter/mcp with createMcpServer() and defineTool()#171
feat(mcp): adopt @outfitter/mcp with createMcpServer() and defineTool()#171galligan wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Replace direct @modelcontextprotocol/sdk usage with @outfitter/mcp framework: - createMcpServer() + connectStdio() replace McpServer + StdioServerTransport - defineTool() with TOOL_ANNOTATIONS replaces server.tool() registration - adaptHandler() bridges Error -> OutfitterError for Result-based handlers - All 6 tools (fw_query, fw_status, fw_doctor, fw_help, fw_pr, fw_fb) converted - Auth-gated write tool pattern preserved with notifyToolsChanged() - Integration tests updated to use createSdkServer() for InMemoryTransport 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
galligan
force-pushed
the
fire-11/mcp-framework
branch
from
c31cf2c to
0e54a57
Compare
galligan
force-pushed
the
fire-10/cli-output
branch
from
9244256 to
056e9bc
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e54a578d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Returns both the server and the auth gate state for lifecycle management. | ||
| */ | ||
| function initializeServer(): { server: McpServer; state: AuthGateState } { | ||
| const server = createMcpServer({ name: "firewatch", version: mcpVersion }); |
There was a problem hiding this comment.
The migrated initialization now calls createMcpServer with only { name, version }, which drops the server-level instructions string that was previously advertised to clients. In this codebase, docs/development/mcp-context-management.md explicitly treats server instructions as critical for tool-search routing; removing them means models are less likely to discover and invoke Firewatch tools unless the user gives highly explicit prompts, so behavior regresses even though tool registration still succeeds. Please pass the prior instructions text (or equivalent metadata field in @outfitter/mcp) when creating the server.
Useful? React with 👍 / 👎.
1 participant
Summary
McpServer/StdioServerTransportwithcreateMcpServer()/connectStdio()defineTool()withTOOL_ANNOTATIONSpresets (readOnly, destructive)adaptHandler()bridges domain errors toOutfitterErrorfor JSON-RPC code mappingconnectStdio()with autotools/list_changed@modelcontextprotocol/sdkretained as devDependency for types (FIRE-11)Test plan
bun run checkpassesbun testpasses (298 tests)bun apps/mcp/bin/fw-mcp.tsstarts without error🤘🏻 In-collaboration-with: Claude Code
Greptile Summary
Migrated the MCP server from direct
@modelcontextprotocol/sdkusage to the@outfitter/mcpabstraction layer. The class-basedFirewatchMCPServeris replaced with functional composition usingcreateMcpServer(),defineTool(), andadaptHandler(). All 6 tools now use standardizedTOOL_ANNOTATIONSpresets for readOnly/write/destructive categories, and error handling is unified throughadaptHandler()which bridges domain errors toResult<T, OutfitterError>for proper JSON-RPC error code mapping. The SDK is retained as a devDependency for type compatibility (FIRE-11).Key changes:
*ParamsShapeto*ParamsSchemafor consistencyconnectStdio()with autonotifyToolsChanged()createSdkServer()wrapper for SDK compatibilityConfidence Score: 5/5
Important Files Changed
@modelcontextprotocol/sdkto devDependencies, added new@outfitter/*dependencies for the MCP refactorFirewatchMCPServerto functional approach withcreateMcpServer()anddefineTool(), usingadaptHandler()to bridge domain errors to Result typescreateSdkServer()wrapper for SDK compatibility with InMemoryTransportFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[run] --> B[initializeServer] B --> C[createMcpServer] B --> D[Create AuthGateState] B --> E[Register Base Tools] E --> E1[defineQueryTool] E --> E2[defineStatusTool] E --> E3[defineDoctorTool] E --> E4[defineHelpTool] E1 --> F[defineTool + adaptHandler] E2 --> F E3 --> F E4 --> F A --> G[connectStdio] A --> H[verifyAuthAndEnableWriteTools] H --> I{Auth Success?} I -->|Yes| J[registerWriteTools] I -->|No| K[Return Error] J --> J1[definePrTool] J --> J2[defineFbTool] J1 --> L[defineTool + adaptHandler] J2 --> L J --> M[notifyToolsChanged] F --> N[Result.ok/Result.err] L --> NLast reviewed commit: 0e54a57