feat: FastMCP Stabilization & SSE Protocol Realignment (CM-46) - #135
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a significant architectural refactor of the CyberMem MCP server, migrating from a custom MCP SDK implementation to the FastMCP framework. The changes aim to stabilize the SSE transport layer and realign with the MCP protocol's httpStream handshake pattern while maintaining backward compatibility through the tool-based API.
Changes:
- Complete rewrite of MCP server implementation using FastMCP framework (packages/mcp/src/index.ts reduced from ~600 to ~192 lines)
- Database schema migration renaming 'operation' column to 'tool' across stats and audit log tables
- E2E test suite migrated from direct HTTP API calls to proper MCP SDK client with SSE transport
- Dashboard and metrics API updated to use 'tool' terminology instead of 'operation'
- Docker configuration changes: Ollama service no longer profile-gated, new OLLAMA_URL environment variable, updated Traefik routing
- Auth-sidecar security improvements: placeholder token rejection, Node.js 24 upgrade
- Build process enhancement: automatic shebang injection for executable compatibility
Reviewed changes
Copilot reviewed 12 out of 20 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mcp/src/index.ts | Complete FastMCP migration with simplified tool registration and httpStream transport |
| packages/mcp/package.json | Added fastmcp dependency and postbuild script for shebang injection |
| packages/mcp/e2e/api.spec.ts | Migrated tests to use MCP Client SDK instead of direct HTTP calls |
| packages/mcp/e2e/sse_transport.spec.ts | Updated SSE transport tests with increased chunk reading (3→10 iterations) |
| packages/mcp/e2e/sse_transport_multi.spec.ts | Enhanced multi-session test resilience with extended chunk reading |
| packages/dashboard/components/dashboard/logs/log-viewer.tsx | Updated UI to display 'Tool' instead of 'Operation' |
| packages/dashboard/app/api/metrics/route.ts | Updated queries to use 'tool' column with specific tool names (add_memory, query_memory, etc.) |
| packages/dashboard/app/api/audit-logs/route.ts | Updated log processing to map tool names to friendly labels |
| packages/cli/templates/docker-compose.yml | Added OLLAMA_URL env var, ollama dependency, removed ollama profile |
| packages/cli/dist/templates/docker-compose.yml | Updated Traefik routing to include /message endpoint |
| packages/cli/dist/templates/auth-sidecar/server.js | Enhanced security with insecure placeholder detection |
| packages/cli/dist/templates/auth-sidecar/Dockerfile | Upgraded to Node.js 24 |
| packages/cli/dist/templates/ansible/playbooks/deploy-cybermem.yml | Added no_log for sensitive operations, increased health check retries |
| packages/cli/src/commands/install.ts | Updated installation instructions to point to root URL instead of /client-setup |
| GEMINI.md | Added constraint against changing binary names without approval |
| .gitignore | Generalized ignore patterns using wildcards for better maintainability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix e2e global-setup readiness check (use /mcp listTools) - Add validation to update_memory tool - Implement robust migration for 'tool' column - Add descriptions to all tools - Add try-catch logging to tool execution - Add dashboard API fallback for legacy schemas - Replace shell postbuild with portable Node.js script
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 25 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix postbuild script path resolution - Add robust migration logic for cybermem_stats tool column
…ice to unblock CI startup
…lth (FastMCP post-migration fix)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 26 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ORRUPT - Rewrite mcpRpc helper in api.spec.ts to use Node fetch with SSE stream parsing instead of Playwright request.post() - Fix operation→tool property rename in api.spec.ts assertions and ui.spec.ts mock data - Add MCP container restart step in global-setup.ts after DB reset to prevent SQLITE_CORRUPT from stale WAL/journal files - Update legacy /add references to /mcp in test attachments - Set stateless: true in MCP server httpStream transport All 17 E2E tests pass locally.
The MCP Streamable HTTP transport requires mcp-session-id headers for the handshake protocol. stateless: true in FastMCP suppresses session ID generation, causing E2E tests to fail with 'No mcp-session-id received'. Setting stateless: false enables proper session management.
- Extract FastMCPHandshakeTransport to shared e2e/utils/ module - Replace hardcoded 200ms/500ms delay with event-driven stream readiness - Make database migration errors fatal (process.exit(1)) - Add MCP JSON-RPC protocol handshake verification to global-setup - Remove unused addUrl variable from global-setup
Feature Decomposition
Requirements
FastMCPserver with session tracking enabled (stateless: false).enableJsonResponse: truefor tool call compatibility.FastMCPauthentication forX-Client-Nameidentification.httpStreamhandshake protocol.Existing Patterns
packages/mcp/e2e/api.spec.tsEdge Cases
Verification
Stabilized the MCP core and verified with the updated E2E suite.
Automated Tests
npm run test:e2e(Local)sse_transport.spec.ts(Handshake verified)sse_transport_multi.spec.ts(100% pass)Environments Verified
Visual Proof (Screenshots/Videos)
📋 Linear Issue: CM-46