From cce88d207b6cd02e342482fcb4da58ac5199c585 Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Sat, 26 Sep 2026 04:11:49 +0530 Subject: [PATCH 1/2] fix(server): drain retained HTTP connections on shutdown --- src/server-shutdown.test.ts | 24 ++++++++++++++++++++++++ src/server-shutdown.ts | 5 +++++ 2 files changed, 29 insertions(+) diff --git a/src/server-shutdown.test.ts b/src/server-shutdown.test.ts index 7dca2eeaa..aa4bf4c01 100644 --- a/src/server-shutdown.test.ts +++ b/src/server-shutdown.test.ts @@ -27,6 +27,30 @@ assert.equal( ); await drainingShutdown; +let idleCloseCalls = 0; +let allCloseCalls = 0; +let finishIdleAwareHttpClose: (() => void) | undefined; +const idleAwareShutdown = shutdownHttpServer( + { + close(callback: (error?: Error) => void) { + finishIdleAwareHttpClose = () => callback(); + }, + closeIdleConnections() { + idleCloseCalls += 1; + }, + closeAllConnections() { + allCloseCalls += 1; + }, + }, + async () => { + assert.equal(idleCloseCalls, 1, "idle connections close when draining starts"); + finishIdleAwareHttpClose?.(); + }, +); +await idleAwareShutdown; +assert.equal(idleCloseCalls, 2, "idle connections close again after application cleanup"); +assert.equal(allCloseCalls, 1, "remaining connections close only after application cleanup"); + let finishApplicationClose: (() => void) | undefined; let shutdownResolved = false; diff --git a/src/server-shutdown.ts b/src/server-shutdown.ts index b9e6f4f0c..2caf4fc30 100644 --- a/src/server-shutdown.ts +++ b/src/server-shutdown.ts @@ -1,5 +1,7 @@ export interface ClosableHttpServer { close(callback: (error?: Error) => void): void; + closeIdleConnections?(): void; + closeAllConnections?(): void; } export async function shutdownHttpServer( @@ -12,7 +14,10 @@ export async function shutdownHttpServer( else resolve(); }); }); + httpServer.closeIdleConnections?.(); await closeApplication(); + httpServer.closeIdleConnections?.(); + httpServer.closeAllConnections?.(); await httpClosed; } From abc3dae35ab5a42a71d68eccd3f42e711960ba9b Mon Sep 17 00:00:00 2001 From: Waishnav <86405648+Waishnav@users.noreply.github.com> Date: Sat, 26 Sep 2026 04:11:49 +0530 Subject: [PATCH 2/2] fix(mcp): harden HTTP connection lifecycle --- src/cli.ts | 3 +- src/server.test.ts | 74 +++++++++++++++++++++++++++++++++++++++--- src/server.ts | 80 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 147 insertions(+), 10 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index 3037f65e8..f36f4127c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -324,7 +324,7 @@ async function serve(): Promise { const config = loadConfig(); await runStartupWorktreeCleanup(config); - const { createServer } = await import("./server.js"); + const { configureHttpServer, createServer } = await import("./server.js"); const { app, close, localAgentProviders } = createServer(config); const httpServer = app.listen(config.port, config.host, () => { console.log(`devspace listening on http://${config.host}:${config.port}/mcp`); @@ -338,6 +338,7 @@ async function serve(): Promise { console.log(`logging: ${config.logging.level} ${config.logging.format}`); console.log(`subagent providers: ${formatLocalAgentProviderStatusSummary(localAgentProviders)}`); }); + configureHttpServer(httpServer); let shuttingDown = false; const shutdown = async () => { diff --git a/src/server.test.ts b/src/server.test.ts index 59bcc5d37..6e69c6eee 100644 --- a/src/server.test.ts +++ b/src/server.test.ts @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { execFile } from "node:child_process"; import { createHash } from "node:crypto"; import { access, mkdtemp, mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { createServer as createHttpServer } from "node:http"; import { platform, tmpdir } from "node:os"; import { join } from "node:path"; import test, { type TestContext } from "node:test"; @@ -14,7 +15,14 @@ import { buildLocalAgentProviderStatuses } from "./local-agent-catalog.js"; import type { SubagentsConfig } from "./local-agent-config.js"; import { createReviewCheckpointManager } from "./review-checkpoints.js"; import { ProcessSessionManager } from "./process-sessions.js"; -import { createMcpServer, createServer } from "./server.js"; +import { shutdownHttpServer } from "./server-shutdown.js"; +import { + configureHttpServer, + createMcpServer, + createServer, + DEVSPACE_HTTP_HEADERS_TIMEOUT_MS, + DEVSPACE_HTTP_KEEP_ALIVE_TIMEOUT_MS, +} from "./server.js"; import { SqliteWorkspaceStore } from "./workspace-store.js"; import { WorkspaceRegistry } from "./workspaces.js"; import { writeTestDevspaceConfig } from "./test-support/config.test.js"; @@ -470,6 +478,14 @@ test("open_workspace scopes checkout reuse to OpenAI session metadata", async (t assert.ok(Array.isArray(structuredContent(unscoped).agents_files)); }); +test("HTTP listener advertises a tunnel-safe keep-alive lifetime", () => { + const httpServer = createHttpServer(); + configureHttpServer(httpServer); + + assert.equal(httpServer.keepAliveTimeout, DEVSPACE_HTTP_KEEP_ALIVE_TIMEOUT_MS); + assert.equal(httpServer.headersTimeout, DEVSPACE_HTTP_HEADERS_TIMEOUT_MS); +}); + test("HTTP endpoint serves modern MCP and stateless legacy clients", async (t) => { const { root, localBaseUrl, accessToken } = await httpServerFixture( t, @@ -503,6 +519,8 @@ test("HTTP endpoint serves modern MCP and stateless legacy clients", async (t) = {}, ); assert.equal(listed.status, 200, await listed.clone().text()); + assert.equal(listed.headers.get("connection"), "keep-alive"); + assert.match(listed.headers.get("keep-alive") ?? "", /timeout=300/); const listBody = await listed.json() as { result?: { tools?: Array<{ name?: string }> }; }; @@ -583,6 +601,52 @@ test("HTTP endpoint serves modern MCP and stateless legacy clients", async (t) = assert.match(await legacyTools.text(), /"open_workspace"/); }); +test("aborting one MCP request does not poison the next stateless request", async (t) => { + const { root, localBaseUrl, accessToken } = await httpServerFixture( + t, + "devspace-aborted-http-test-", + ); + const opened = await postModernMcp( + localBaseUrl, + accessToken, + "tools/call", + { + name: "open_workspace", + arguments: { path: root }, + _meta: { "openai/session": "aborted-http-test" }, + }, + ); + const openBody = await opened.json() as { + result?: { structuredContent?: { workspace_id?: string } }; + }; + const workspaceId = openBody.result?.structuredContent?.workspace_id; + assert.equal(typeof workspaceId, "string"); + + const controller = new AbortController(); + const toolCall = postModernMcp( + localBaseUrl, + accessToken, + "tools/call", + { + name: "exec_command", + arguments: { + workspace_id: workspaceId, + cmd: "node -e \"const fs=require('node:fs');fs.writeFileSync('started','');setTimeout(()=>fs.writeFileSync('finished',''),500)\"", + yield_time_ms: 1_000, + }, + }, + { signal: controller.signal }, + ); + await waitForFile(join(root, "started")); + controller.abort(); + await assert.rejects(toolCall, /abort/i); + + const listed = await postModernMcp(localBaseUrl, accessToken, "tools/list", {}); + assert.equal(listed.status, 200, await listed.clone().text()); + await listed.text(); + await waitForFile(join(root, "finished")); +}); + test("server shutdown waits for an active MCP tool call", async (t) => { const { root, localBaseUrl, accessToken, running } = await httpServerFixture( t, @@ -694,12 +758,10 @@ async function httpServerFixture( const running = createServer(config, { incomingArtifactAdapters: [] }); const httpServer = running.app.listen(0, "127.0.0.1"); await new Promise((resolve) => httpServer.once("listening", resolve)); + configureHttpServer(httpServer); t.after(async () => { - await new Promise((resolve, reject) => { - httpServer.close((error) => error ? reject(error) : resolve()); - }); - await running.close(); + await shutdownHttpServer(httpServer, running.close); await rm(root, { recursive: true, force: true }); }); @@ -909,6 +971,7 @@ function postModernMcp( accessToken: string | undefined, method: string, params: Record, + options: { signal?: AbortSignal } = {}, ): Promise { const mcpName = typeof params.name === "string" ? params.name @@ -924,6 +987,7 @@ function postModernMcp( "mcp-protocol-version": "2026-07-28", ...(mcpName ? { "mcp-name": mcpName } : {}), }, + signal: options.signal, body: JSON.stringify({ jsonrpc: "2.0", id: `modern-${method}`, diff --git a/src/server.ts b/src/server.ts index 0a2792f3f..27780d07d 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import { readFileSync } from "node:fs"; import { access, realpath } from "node:fs/promises"; +import type { Server as HttpServer } from "node:http"; import { fileURLToPath } from "node:url"; import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { createMcpExpressApp } from "@modelcontextprotocol/sdk/server/express.js"; @@ -92,6 +93,16 @@ interface RunningServer { close(): Promise; } +// Keep the local origin alive longer than the reverse proxy's pooled connection. +// Node must also advertise this timeout itself, so MCP responses drop hop-by-hop headers below. +export const DEVSPACE_HTTP_KEEP_ALIVE_TIMEOUT_MS = 5 * 60 * 1_000; +export const DEVSPACE_HTTP_HEADERS_TIMEOUT_MS = DEVSPACE_HTTP_KEEP_ALIVE_TIMEOUT_MS + 5_000; + +export function configureHttpServer(httpServer: HttpServer): void { + httpServer.keepAliveTimeout = DEVSPACE_HTTP_KEEP_ALIVE_TIMEOUT_MS; + httpServer.headersTimeout = DEVSPACE_HTTP_HEADERS_TIMEOUT_MS; +} + type TrackToolActivity = (operation: () => Promise) => Promise; class ToolActivityTracker { @@ -218,6 +229,32 @@ function requestLogFields(req: Request, config: ServerConfig): Record { + if (!body || typeof body !== "object" || Array.isArray(body)) return {}; + const request = body as { id?: unknown; method?: unknown }; + return { + rpcId: request.id, + rpcMethod: request.method, + }; +} + +function nodeMcpResponse(response: globalThis.Response): globalThis.Response { + const headers = new Headers(response.headers); + + // Connection is hop-by-hop state owned by Node's HTTP server. Preserving an + // application-supplied value prevents Node from advertising its real socket + // lifetime and can make a proxy reuse a connection while the server closes it. + headers.delete("connection"); + headers.delete("keep-alive"); + headers.delete("transfer-encoding"); + + return new globalThis.Response(response.body, { + status: response.status, + statusText: response.statusText, + headers, + }); +} + function assetBaseUrl(config: ServerConfig): string { return `${config.publicBaseUrl.replace(/\/+$/, "")}/mcp-app-assets`; } @@ -857,7 +894,9 @@ export function createServer( legacy: "stateless", onerror: logMcpHandlerError, }); - const mcpNodeHandler = toNodeHandler(mcpHandler, { + const mcpNodeHandler = toNodeHandler({ + fetch: async (request, options) => nodeMcpResponse(await mcpHandler.fetch(request, options)), + }, { onerror: logMcpHandlerError, }); @@ -868,12 +907,25 @@ export function createServer( app.use((req, res, next) => { const requestId = randomUUID(); const startedAt = performance.now(); + const path = requestPath(req); + const shouldLogRequest = config.logging.requests + && (config.logging.assets || !path.startsWith("/mcp-app-assets")); + let finished = false; res.locals.requestId = requestId; + if (shouldLogRequest) { + logEvent(config.logging, "debug", "http_request_start", { + requestId, + method: req.method, + path, + ...requestLogFields(req, config), + ...(path === "/mcp" ? rpcRequestLogFields(req.body) : {}), + }); + } + res.on("finish", () => { - const path = requestPath(req); - if (!config.logging.requests) return; - if (!config.logging.assets && path.startsWith("/mcp-app-assets")) return; + finished = true; + if (!shouldLogRequest) return; logEvent(config.logging, "info", "http_request", { requestId, @@ -882,6 +934,21 @@ export function createServer( status: res.statusCode, durationMs: Math.round(performance.now() - startedAt), ...requestLogFields(req, config), + ...(path === "/mcp" ? rpcRequestLogFields(req.body) : {}), + }); + }); + + res.on("close", () => { + if (finished || !shouldLogRequest) return; + logEvent(config.logging, "warn", "http_request_aborted", { + requestId, + method: req.method, + path, + headersSent: res.headersSent, + ...(res.headersSent ? { status: res.statusCode } : {}), + durationMs: Math.round(performance.now() - startedAt), + ...requestLogFields(req, config), + ...(path === "/mcp" ? rpcRequestLogFields(req.body) : {}), }); }); @@ -944,6 +1011,10 @@ export function createServer( logEvent(config.logging, "debug", "mcp_request", { requestId, method: req.method, + protocolVersion: req.header("mcp-protocol-version"), + mcpMethod: req.header("mcp-method"), + mcpName: req.header("mcp-name"), + ...rpcRequestLogFields(req.body), }); try { @@ -1011,6 +1082,7 @@ if (await isMainModule()) { console.log(`native artifact download: ${artifactDownloadStatus}`); console.log(`subagent providers: ${formatLocalAgentProviderStatusSummary(localAgentProviders)}`); }); + configureHttpServer(httpServer); let shuttingDown = false; const shutdown = async () => {