feat: HTTP transport mode (--http flag)#78
Conversation
Adds `--http` flag (and `CODEBASE_CONTEXT_HTTP=1` env var) to run the MCP server as a long-lived HTTP process using StreamableHTTPServerTransport. - src/server/factory.ts: server factory so both stdio and HTTP can create fresh Server instances sharing the same module-level state - src/server/http.ts: full HTTP server with per-session Server+Transport, session map, idle timeout reaper, and graceful shutdown - src/index.ts: export registerHandlers, add startHttp() entry point, port flag parsing (--port / CODEBASE_CONTEXT_PORT, default 3100), fix resolveRootPath() to skip --http/--port flags Multi-session support: each client gets its own Server+Transport while sharing project state. stdio mode unchanged; 312/312 tests pass.
Greptile SummaryThis PR adds a Key changes:
Issues found:
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| src/server/http.ts | New HTTP transport layer. Session lifecycle, timeout reaper, and graceful shutdown are well-structured; however the async request handler has no top-level try/catch (unhandled rejections crash the process), the DELETE path leaks sessions if transport.close() throws, and orphaned server+transport pairs can occur if the SDK omits a session ID. |
| src/server/factory.ts | Thin factory for MCP Server instantiation; clean and correct. Optional registerHandlers callback is a good decoupling mechanism. |
| src/index.ts | Handler logic correctly extracted into exported registerHandlers(), resolveRootPath() fixed to skip --http/--port flags, and startHttp() entry point mirrors main() semantics. Port parsing is safe (NaN-guarded). No issues found. |
| .gitignore | Adds .agents/ to gitignore; minor housekeeping. |
Sequence Diagram
sequenceDiagram
participant C as MCP Client
participant H as HTTP Server (http.ts)
participant F as factory.ts
participant S as MCP Server (SDK)
participant I as index.ts (module state)
C->>H: POST /mcp (no session ID)
H->>F: createServer(options, registerHandlers)
F->>S: new Server(name, version, capabilities)
F->>I: registerHandlers(server)
F-->>H: { server, transport }
H->>S: server.connect(transport)
H->>S: transport.handleRequest(req, res)
S-->>H: sessionId assigned
H->>H: sessions.set(sessionId, entry)
H->>I: onSessionReady(server)
S-->>C: HTTP 200 + Mcp-Session-Id header
C->>H: POST /mcp (Mcp-Session-Id: xyz)
H->>H: sessions.get(sessionId) ✓
H->>S: transport.handleRequest(req, res)
S-->>C: tool/resource response
C->>H: GET /mcp (Mcp-Session-Id: xyz)
H->>S: transport.handleRequest(req, res)
S-->>C: SSE stream (server-initiated messages)
C->>H: DELETE /mcp (Mcp-Session-Id: xyz)
H->>S: transport.close()
H->>H: sessions.delete(sessionId)
H-->>C: 200 session_closed
Note over H: Idle reaper (every 60s)<br/>evicts sessions idle > 30min
Note over H: SIGINT/SIGTERM<br/>→ close all sessions<br/>→ httpServer.close()
Reviews (1): Last reviewed commit: "style: apply prettier formatting" | Re-trigger Greptile
| const httpServer = createHttpServer(async (req: IncomingMessage, res: ServerResponse) => { | ||
| const url = new URL(req.url ?? '/', `http://${host}:${port}`); | ||
|
|
||
| // Health check on root | ||
| if (url.pathname === '/' && req.method === 'GET') { | ||
| res.writeHead(200, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ status: 'ok', sessions: sessions.size })); | ||
| return; | ||
| } | ||
|
|
||
| // Only handle /mcp | ||
| if (url.pathname !== '/mcp') { | ||
| res.writeHead(404, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Not found' })); | ||
| return; | ||
| } | ||
|
|
||
| const method = req.method?.toUpperCase(); | ||
|
|
||
| if (method === 'POST') { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
|
|
||
| if (sessionId && sessions.has(sessionId)) { | ||
| // Existing session — delegate to its transport | ||
| touchSession(sessionId); | ||
| const session = sessions.get(sessionId)!; | ||
| await session.transport.handleRequest(req, res); | ||
| return; | ||
| } | ||
|
|
||
| if (sessionId && !sessions.has(sessionId)) { | ||
| // Unknown session ID — 404 | ||
| res.writeHead(404, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Session not found' })); | ||
| return; | ||
| } | ||
|
|
||
| // No session ID — new initialization request | ||
| const { server: mcpServer, transport } = createSessionServer(); | ||
|
|
||
| // Connect server to transport | ||
| await mcpServer.connect(transport); | ||
|
|
||
| // Register onclose before handleRequest so cleanup always fires. | ||
| transport.onclose = () => { | ||
| const sid = transport.sessionId; | ||
| if (sid && sessions.has(sid)) { | ||
| sessions.delete(sid); | ||
| console.error(`[HTTP] Session ${sid} disconnected`); | ||
| } | ||
| }; | ||
|
|
||
| // Handle the init request — this generates the session ID | ||
| await transport.handleRequest(req, res); | ||
|
|
||
| // After handleRequest, the session ID is available | ||
| const newSessionId = transport.sessionId; | ||
| if (newSessionId) { | ||
| sessions.set(newSessionId, { | ||
| server: mcpServer, | ||
| transport, | ||
| lastActivity: Date.now() | ||
| }); | ||
| console.error(`[HTTP] Session ${newSessionId} connected`); | ||
|
|
||
| // Notify caller so they can set up per-session handlers (roots refresh, etc.) | ||
| options.onSessionReady?.(mcpServer); | ||
| } | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| if (method === 'GET') { | ||
| // SSE streaming for server-initiated messages | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
| if (!sessionId || !sessions.has(sessionId)) { | ||
| res.writeHead(400, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Missing or invalid session ID' })); | ||
| return; | ||
| } | ||
|
|
||
| touchSession(sessionId); | ||
| const session = sessions.get(sessionId)!; | ||
| await session.transport.handleRequest(req, res); | ||
| return; | ||
| } | ||
|
|
||
| if (method === 'DELETE') { | ||
| const sessionId = req.headers['mcp-session-id'] as string | undefined; | ||
| if (!sessionId || !sessions.has(sessionId)) { | ||
| res.writeHead(404, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Session not found' })); | ||
| return; | ||
| } | ||
|
|
||
| const session = sessions.get(sessionId)!; | ||
| await session.transport.close(); | ||
| sessions.delete(sessionId); | ||
| console.error(`[HTTP] Session ${sessionId} closed by client`); | ||
| res.writeHead(200, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ status: 'session_closed' })); | ||
| return; | ||
| } | ||
|
|
||
| // Method not allowed | ||
| res.writeHead(405, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify({ error: 'Method not allowed' })); | ||
| }); |
There was a problem hiding this comment.
Unhandled async errors crash the server
The async callback passed to createHttpServer() has no top-level try/catch. Node.js does not automatically catch promise rejections thrown from HTTP request listeners — they surface as unhandled rejections, which in modern Node.js will terminate the process.
Any of the await calls inside (mcpServer.connect(transport), transport.handleRequest(req, res), session.transport.close()) can throw: e.g. a malformed MCP init message, a transport-level I/O error, or an SDK-internal assertion. When that happens the response is never sent (client hangs) and the process crashes.
A minimal fix is a top-level try/catch that writes a 500 before rethrowing (or absorbing):
const httpServer = createHttpServer(async (req: IncomingMessage, res: ServerResponse) => {
try {
// ... all existing routing logic ...
} catch (err) {
console.error('[HTTP] Unhandled request error:', err);
if (!res.headersSent) {
res.writeHead(500, { 'Content-Type': 'application/json' });
res.end(JSON.stringify({ error: 'Internal server error' }));
}
}
});
src/server/http.ts
Outdated
| const session = sessions.get(sessionId)!; | ||
| await session.transport.close(); | ||
| sessions.delete(sessionId); |
There was a problem hiding this comment.
Session leaked if
transport.close() throws in DELETE
If session.transport.close() rejects, execution jumps past sessions.delete(sessionId), leaving the entry in the map permanently. Subsequent DELETE or GET requests for this session ID will hit the stale entry and attempt another close() call, creating a resource leak.
A try/finally guarantees the map is cleaned up regardless:
| const session = sessions.get(sessionId)!; | |
| await session.transport.close(); | |
| sessions.delete(sessionId); | |
| const session = sessions.get(sessionId)!; | |
| try { | |
| await session.transport.close(); | |
| } finally { | |
| sessions.delete(sessionId); | |
| } | |
| console.error(`[HTTP] Session ${sessionId} closed by client`); |
src/server/http.ts
Outdated
| // No session ID — new initialization request | ||
| const { server: mcpServer, transport } = createSessionServer(); | ||
|
|
||
| // Connect server to transport | ||
| await mcpServer.connect(transport); | ||
|
|
||
| // Register onclose before handleRequest so cleanup always fires. | ||
| transport.onclose = () => { | ||
| const sid = transport.sessionId; | ||
| if (sid && sessions.has(sid)) { | ||
| sessions.delete(sid); | ||
| console.error(`[HTTP] Session ${sid} disconnected`); | ||
| } | ||
| }; | ||
|
|
||
| // Handle the init request — this generates the session ID | ||
| await transport.handleRequest(req, res); | ||
|
|
||
| // After handleRequest, the session ID is available | ||
| const newSessionId = transport.sessionId; | ||
| if (newSessionId) { | ||
| sessions.set(newSessionId, { | ||
| server: mcpServer, | ||
| transport, | ||
| lastActivity: Date.now() | ||
| }); | ||
| console.error(`[HTTP] Session ${newSessionId} connected`); | ||
|
|
||
| // Notify caller so they can set up per-session handlers (roots refresh, etc.) | ||
| options.onSessionReady?.(mcpServer); | ||
| } | ||
|
|
||
| return; |
There was a problem hiding this comment.
Orphaned server+transport if
transport.sessionId is null after init
If handleRequest completes but the SDK has not populated transport.sessionId (e.g. the init request was malformed and the SDK rejected it silently), the if (newSessionId) guard is false. The already-connected mcpServer+transport pair is then quietly abandoned: it is never stored in sessions, never closed, and never cleaned up.
While the MCP SDK currently always sets a session ID on a successful initialize, tying cleanup entirely to the SDK's internal behaviour makes this code brittle. A defensive close on the !newSessionId path would prevent a silent resource leak:
const newSessionId = transport.sessionId;
if (newSessionId) {
sessions.set(newSessionId, { server: mcpServer, transport, lastActivity: Date.now() });
console.error(`[HTTP] Session ${newSessionId} connected`);
options.onSessionReady?.(mcpServer);
} else {
// SDK did not assign a session ID — tear down the orphaned pair.
await transport.close().catch(() => { /* best effort */ });
}There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0b036f010
ℹ️ 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".
| // Per-session roots change handler | ||
| sessionServer.setNotificationHandler(RootsListChangedNotificationSchema, async () => { | ||
| try { | ||
| await refreshKnownRootsFromClient(); |
There was a problem hiding this comment.
Refresh roots from the active HTTP session server
In HTTP mode this notification path calls refreshKnownRootsFromClient(), but that helper uses the module-level server.listRoots() instance that is only connected in stdio mode. For --http sessions, listRoots() therefore falls into the fallback branch and never ingests the client’s actual roots, so root-scoped project discovery/selection can silently fail for HTTP clients even after roots/list_changed notifications.
Useful? React with 👍 / 👎.
…ansport Three issues flagged in code review: 1. Wrap entire request handler in try/catch — any await that throws (malformed init, SDK I/O error) previously caused an unhandled promise rejection that could crash the long-lived server process. 2. DELETE handler: use try/finally so sessions.delete() always runs even if transport.close() rejects, preventing session map leaks. 3. Init path: if transport.sessionId is null after handleRequest (malformed request), close the orphaned Server+Transport pair immediately rather than silently abandoning it.
Summary
--httpflag to run codebase-context as a long-lived HTTP MCP server (StreamableHTTP transport) alongside the existing stdio mode--portflag orCODEBASE_CONTEXT_PORTenv var (default 3100)Server+Transportinstance per session; all sessions share module-level project stateNew files
src/server/factory.ts— server factory that decouplesServerinstantiation from handler registrationsrc/server/http.ts— HTTP server: session map, timeout reaper, full request routing,handle.close()Changes to
src/index.tsregisterHandlers()so HTTP sessions can reuse all tool/resource handlersstartHttp()entry point — mirrorsmain()but without stdin/ppid guardsresolveRootPath()to skip--http/--portflags when finding the bootstrap root--http→startHttp(), else →main()(stdio unchanged)Test plan
pnpm run test— 312/312 pass, no regressionsnode dist/index.js --http— server starts, logslistening on http://127.0.0.1:3100/mcpserverInfo, session ID header presentecho initialize | node dist/index.js .responds correctly