Skip to content

feat: HTTP transport mode (--http flag)#78

Merged
PatrickSys merged 3 commits intomasterfrom
feat/http-transport
Mar 25, 2026
Merged

feat: HTTP transport mode (--http flag)#78
PatrickSys merged 3 commits intomasterfrom
feat/http-transport

Conversation

@PatrickSys
Copy link
Owner

Summary

  • Adds --http flag to run codebase-context as a long-lived HTTP MCP server (StreamableHTTP transport) alongside the existing stdio mode
  • Port configurable via --port flag or CODEBASE_CONTEXT_PORT env var (default 3100)
  • Each MCP client gets its own Server+Transport instance per session; all sessions share module-level project state
  • Idle session timeout reaper + graceful SIGINT/SIGTERM shutdown

New files

  • src/server/factory.ts — server factory that decouples Server instantiation from handler registration
  • src/server/http.ts — HTTP server: session map, timeout reaper, full request routing, handle.close()

Changes to src/index.ts

  • Export registerHandlers() so HTTP sessions can reuse all tool/resource handlers
  • Add startHttp() entry point — mirrors main() but without stdin/ppid guards
  • Fix resolveRootPath() to skip --http / --port flags when finding the bootstrap root
  • Entry-point branching: --httpstartHttp(), else → main() (stdio unchanged)

Test plan

  • pnpm run test — 312/312 pass, no regressions
  • node dist/index.js --http — server starts, logs listening on http://127.0.0.1:3100/mcp
  • MCP initialize via curl — HTTP 200, valid serverInfo, session ID header present
  • Two concurrent clients — distinct session UUIDs
  • stdio regression — echo initialize | node dist/index.js . responds correctly
  • Clean exit on SIGINT (Windows: process exits; graceful session-close logging verified via code inspection)

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-apps
Copy link

greptile-apps bot commented Mar 25, 2026

Greptile Summary

This PR adds a --http flag that runs the MCP server in a long-lived StreamableHTTP transport mode alongside the existing stdio mode. The core design is sound: a clean factory.ts decouples Server instantiation from handler registration, registerHandlers() is correctly extracted and exported from index.ts, and session lifecycle (UUID-keyed map, 30-minute idle reaper, SIGINT/SIGTERM shutdown) is well thought-through.

Key changes:

  • src/server/factory.ts — new thin factory for Server creation
  • src/server/http.ts — full HTTP session management and request routing
  • src/index.tsregisterHandlers() extracted and exported; resolveRootPath() fixed to skip --http/--port args; startHttp() entry point added
  • .gitignore — adds .agents/

Issues found:

  • The async callback passed to createHttpServer() has no top-level try/catch. Any await inside that throws (malformed init message, SDK I/O error, etc.) produces an unhandled promise rejection that can crash the Node process — this affects all request paths (POST, GET, DELETE).
  • In the DELETE handler, sessions.delete() is called after transport.close() with no error guard; if close() rejects the session entry leaks.
  • If transport.sessionId is null after the init handleRequest (malformed request), the connected Server+Transport pair is silently abandoned without cleanup.

Confidence Score: 3/5

  • The async request handler missing a top-level try/catch is a straightforward fix but is a real crash path in the HTTP server's primary request loop — it should be resolved before merge.
  • The overall architecture and index.ts refactor are clean. The two P1 issues (unhandled async errors + DELETE session leak) are in http.ts and are straightforward to fix, but the unhandled-rejection path can crash a long-lived server process under normal error conditions, which directly impacts the primary use-case of this PR.
  • src/server/http.ts — async request handler error handling and DELETE session cleanup

Important Files Changed

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()
Loading

Reviews (1): Last reviewed commit: "style: apply prettier formatting" | Re-trigger Greptile

Comment on lines +97 to +204
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' }));
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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' }));
    }
  }
});

Comment on lines +192 to +194
const session = sessions.get(sessionId)!;
await session.transport.close();
sessions.delete(sessionId);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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:

Suggested change
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`);

Comment on lines +134 to +166
// 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;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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 */ });
}

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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.
@PatrickSys PatrickSys merged commit c9bf17f into master Mar 25, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant