From 0eca47aa5a4015621aec631861f785aa578dcfac Mon Sep 17 00:00:00 2001 From: Ary Rabelo Date: Mon, 24 Aug 2026 17:13:30 -0300 Subject: [PATCH 1/2] fix(routes): return 405 for GET/DELETE on the execution MCP route The execution capability MCP server at /agent-executions/:id/mcp only declared a POST handler. A GET or DELETE request never reached handleExecutionCapabilities and fell through to the SPA route render instead: a 200 with the app shell HTML for an unknown execution id, or a 500 once TanStack's fallback tried to resolve a real one. The route is a stateless MCP Streamable HTTP server that only supports POST; the spec expects 405 for a method it does not support. Returning 500 for a real execution id is worse than a clean 405: MCP clients that probe the transport with a GET before falling back to POST-only mode read a 500 as "server is broken" and give up instead of retrying with POST. Add explicit GET/DELETE handlers that return 405 with an Allow: POST header, matching the api/$ catch-all's not-found handler pattern already used elsewhere in routes/. --- src/mcp-route-method-guard.test.ts | 38 +++++++++++++++++++ .../agent-executions/$executionId/mcp.ts | 17 +++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/mcp-route-method-guard.test.ts diff --git a/src/mcp-route-method-guard.test.ts b/src/mcp-route-method-guard.test.ts new file mode 100644 index 00000000..f73b51dd --- /dev/null +++ b/src/mcp-route-method-guard.test.ts @@ -0,0 +1,38 @@ +import assert from "node:assert/strict"; +import { it } from "vitest"; +import { Route } from "./routes/agent-executions/$executionId/mcp.js"; + +/** + * TanStack's `Constrain` handlers type resolves member + * access against the function-form branch of the union even though this + * route declares the plain object-literal form, so the generated route + * type cannot express calling an individual method handler directly. + */ +interface McpRouteMethodHandlers { + GET(): Response | Promise; + DELETE(): Response | Promise; +} + +// The execution capability MCP server is stateless and POST-only. Before this +// guard, an unhandled GET (SSE stream open) or DELETE (session terminate) +// fell through to the SPA route render: 200 HTML for an unknown execution id, +// or a 500 for a real one once it hit `handleExecutionCapabilities`. A 500 +// makes MCP Streamable HTTP clients treat the server as dead instead of +// retrying with the one method it actually supports. +it("rejects GET on the MCP route with 405 and an Allow: POST header", async () => { + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- the generated route type cannot express calling one handler directly + const handlers = Route.options.server?.handlers as unknown as McpRouteMethodHandlers; + const response = await handlers.GET(); + + assert.equal(response.status, 405); + assert.equal(response.headers.get("allow"), "POST"); +}); + +it("rejects DELETE on the MCP route with 405 and an Allow: POST header", async () => { + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- the generated route type cannot express calling one handler directly + const handlers = Route.options.server?.handlers as unknown as McpRouteMethodHandlers; + const response = await handlers.DELETE(); + + assert.equal(response.status, 405); + assert.equal(response.headers.get("allow"), "POST"); +}); diff --git a/src/routes/agent-executions/$executionId/mcp.ts b/src/routes/agent-executions/$executionId/mcp.ts index 428421c5..66f48310 100644 --- a/src/routes/agent-executions/$executionId/mcp.ts +++ b/src/routes/agent-executions/$executionId/mcp.ts @@ -9,6 +9,23 @@ export const Route = createFileRoute("/agent-executions/$executionId/mcp")({ request, new URL(request.url).pathname.split("/")[2] ?? "", ), + // This MCP server is stateless and POST-only: every call opens a + // fresh transport and closes it once the response finishes, so there + // is no SSE stream to resume (GET) or session to terminate (DELETE). + // Without an explicit handler these methods fell through to the SPA + // route render, which returned a misleading 200 (unknown execution + // id) or crashed to 500 (real execution id) instead of the 405 the + // MCP Streamable HTTP spec requires for an unsupported method - and a + // 500 makes MCP clients mark the server as dead instead of retrying. + GET: methodNotAllowed, + DELETE: methodNotAllowed, }, }, }); + +function methodNotAllowed(): Response { + return Response.json( + { error: "method_not_allowed" }, + { status: 405, headers: { Allow: "POST" } }, + ); +} From d496647ec5a70050a872f0fc1c9328555c90938a Mon Sep 17 00:00:00 2001 From: Ary Rabelo Date: Mon, 24 Aug 2026 17:13:43 -0300 Subject: [PATCH 2/2] fix(daemons): stop an invalid WebSocket close frame from killing the Hub ActiveDaemonRegistry.accept() listened for "message" and "close" on a daemon's WebSocket but never for "error". A daemon peer that sends a close frame with a status code the protocol forbids on the wire (ws's Receiver rejects codes like 1004-1006, which are reserved and must never appear on the wire) makes ws's Receiver emit WS_ERR_INVALID_CLOSE_CODE as an "error" event on that socket. With no listener, Node's EventEmitter rethrows it as an uncaught exception, which crashes the entire Hub process over a single bad connection instead of just that daemon's socket. Add an "error" listener that reports the failure through the same reportFailure/report pipeline other socket faults already use. ws still closes the underlying connection itself once the error fires, so the existing "close" handler continues to drive the normal offline-presence cleanup. Reproduced with a raw close frame containing status 1006 written directly to the client's underlying TCP socket (bypassing ws's own close() validation, which already rejects invalid codes client-side). Removing the fix crashes the vitest worker with the exact WS_ERR_INVALID_CLOSE_CODE RangeError seen in production. --- src/daemons/registry.test.ts | 19 ++++++++++++ src/daemons/registry.ts | 9 ++++++ .../test-utils/daemon-registry-harness.ts | 30 +++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/src/daemons/registry.test.ts b/src/daemons/registry.test.ts index eae6ff2d..316c6523 100644 --- a/src/daemons/registry.test.ts +++ b/src/daemons/registry.test.ts @@ -328,4 +328,23 @@ describe("daemon socket generations", () => { await assert.rejects(pending.promise, /daemon disconnected/u); }); + + it("survives an invalid WebSocket close code instead of crashing the process", async () => { + // Real production crash: a peer sending a close frame with a code the + // protocol forbids on the wire (1006 is reserved and must never be + // sent) makes `ws`'s Receiver emit `error` on the server socket. With + // no `error` listener, Node rethrows it as an uncaught exception and + // kills the whole Hub process. If `accept()` regresses, this test + // crashes the worker instead of merely failing an assertion. + daemon.sendInvalidClose(1006); + await daemon.waitUntilCurrentClosed(); + await new Promise((resolve) => setImmediate(resolve)); + + assertOneFailure(stream, { + operation: "daemon.socket.error", + component: "daemons", + failureKind: "network", + canary: "invalid-close-code-1006-never-logged", + }); + }); }); diff --git a/src/daemons/registry.ts b/src/daemons/registry.ts index f733fab0..dec91601 100644 --- a/src/daemons/registry.ts +++ b/src/daemons/registry.ts @@ -132,6 +132,15 @@ export class ActiveDaemonRegistry { previous?.agents.close(); previous?.socket.close(4001, "replaced"); socket.on("message", (data) => this.receive(active, readText(data))); + socket.on("error", (error) => { + // A peer that sends a malformed control frame (e.g. an invalid close + // status code) makes the `ws` receiver emit `error` on this socket. + // Node's EventEmitter rethrows unlistened `error` events as an + // uncaught exception, which without this listener kills the whole + // Hub process over one bad daemon connection. Report and let the + // subsequent `close` event drive the normal offline-presence cleanup. + this.report(error, "daemon.socket.error", daemon.id, "network"); + }); socket.on("close", () => { active.agents.close(); if (this.active.get(daemon.id)?.generation === active.generation) { diff --git a/src/daemons/test-utils/daemon-registry-harness.ts b/src/daemons/test-utils/daemon-registry-harness.ts index ecb920c5..d7001177 100644 --- a/src/daemons/test-utils/daemon-registry-harness.ts +++ b/src/daemons/test-utils/daemon-registry-harness.ts @@ -29,6 +29,11 @@ interface PendingRequest { request: z.infer["message"]; } +/** Shape of the internal fields `ws` leaves untyped but always populates. */ +interface WebSocketInternals { + _socket: { write(data: Buffer): void }; +} + export class DaemonRegistryHarness { private readonly presence = new DaemonPresence(); private readonly registry: ActiveDaemonRegistry; @@ -271,6 +276,10 @@ export class DaemonRegistryHarness { this.currentSocket().sendRaw(value); } + sendInvalidClose(code: number): void { + this.currentSocket().sendInvalidClose(code); + } + waitUntilCurrentClosed(): Promise { return this.currentSocket().waitUntilClosed(); } @@ -487,6 +496,27 @@ class RegistrySocket { this.socket.send(value); } + /** + * Writes a raw WebSocket close control frame directly onto the + * underlying TCP socket, bypassing `ws`'s own `close()` validation so a + * status code the protocol forbids on the wire (e.g. 1006, reserved for + * abnormal closure and never legally sent) reaches the server's + * `Receiver`, reproducing WS_ERR_INVALID_CLOSE_CODE. + */ + sendInvalidClose(code: number): void { + // `ws` does not type its internal `_socket`, but every `ws` WebSocket + // instance exposes the underlying net.Socket at runtime once open. + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion -- `ws` leaves `_socket` untyped + const internals = this.socket as unknown as WebSocketInternals; + const tcpSocket = internals._socket; + const mask = Buffer.alloc(4); + const payload = Buffer.alloc(2); + payload.writeUInt16BE(code, 0); + const maskedPayload = Buffer.alloc(2); + for (let i = 0; i < 2; i += 1) maskedPayload[i] = payload[i]! ^ mask[i]!; + tcpSocket.write(Buffer.concat([Buffer.from([0x88, 0x82]), mask, maskedPayload])); + } + close(): void { this.socket.close(); }