|
| 1 | +# Fix MCP Streamable HTTP Compliance for Codex |
| 2 | + |
| 3 | +**Created**: 2026-04-03 |
| 4 | +**Priority**: High |
| 5 | +**Classification**: `bug` |
| 6 | + |
| 7 | +## Problem |
| 8 | + |
| 9 | +SAM's `/mcp` endpoint treats all JSON-RPC messages as requests. When Codex sends `notifications/initialized` (a notification with no `id` field), SAM returns a JSON-RPC error instead of `202 Accepted` with no body. Codex's strict Rust RMCP client closes the transport immediately. |
| 10 | + |
| 11 | +## Research Findings |
| 12 | + |
| 13 | +- **Root cause**: `apps/api/src/routes/mcp/index.ts` line 184 — the `default` case in the method switch returns `Method not found` for notifications like `notifications/initialized` |
| 14 | +- **JSON-RPC notifications** have no `id` field (per spec, `id` is undefined, not null) |
| 15 | +- **MCP Streamable HTTP spec** requires notifications to get `202 Accepted` with no body |
| 16 | +- **GET and DELETE** on `/mcp` should return `405 Method Not Allowed` per spec |
| 17 | +- **Existing test patterns**: `apps/api/tests/unit/routes/mcp.test.ts` uses `mcpRequest()` helper with Hono app mock |
| 18 | +- Claude Code and Vibe work because they use different integration paths (ACP SDK / tolerant client) |
| 19 | + |
| 20 | +## Implementation Checklist |
| 21 | + |
| 22 | +### A. Notification detection + 202 response |
| 23 | +- [ ] After parsing JSON-RPC body and validating `jsonrpc: '2.0'`, detect notifications where `rpc.id` is `undefined` |
| 24 | +- [ ] Return `202 Accepted` with no body for all notifications (before the method switch) |
| 25 | + |
| 26 | +### B. GET and DELETE 405 handlers |
| 27 | +- [ ] Add `mcpRoutes.get('/', ...)` returning 405 Method Not Allowed |
| 28 | +- [ ] Add `mcpRoutes.delete('/', ...)` returning 405 Method Not Allowed |
| 29 | + |
| 30 | +### C. Tests |
| 31 | +- [ ] Test that POST with no `id` field returns 202 with empty body |
| 32 | +- [ ] Test that `notifications/initialized` returns 202 |
| 33 | +- [ ] Test that unknown notifications return 202 (not JSON-RPC error) |
| 34 | +- [ ] Test that GET `/mcp` returns 405 |
| 35 | +- [ ] Test that DELETE `/mcp` returns 405 |
| 36 | +- [ ] Test full Codex lifecycle: initialize (200) → notifications/initialized (202) → tools/list (200) → tools/call (200) |
| 37 | +- [ ] Regression: existing initialize, tools/list, tools/call, ping still return 200 with JSON-RPC responses |
| 38 | + |
| 39 | +## Acceptance Criteria |
| 40 | + |
| 41 | +- [ ] `notifications/initialized` POST returns 202 with no body |
| 42 | +- [ ] All notifications (no `id` field) return 202 |
| 43 | +- [ ] GET `/mcp` returns 405 |
| 44 | +- [ ] DELETE `/mcp` returns 405 |
| 45 | +- [ ] Existing MCP requests (initialize, tools/list, tools/call, ping) still work |
| 46 | +- [ ] Automated tests cover all lifecycle steps |
| 47 | + |
| 48 | +## References |
| 49 | + |
| 50 | +- `apps/api/src/routes/mcp/index.ts` |
| 51 | +- `apps/api/src/routes/mcp/_helpers.ts` |
| 52 | +- `tasks/backlog/2026-04-02-fix-codex-mcp-streamable-http-compliance.md` |
0 commit comments