From 809f0d69db27faafebb7e1e2cf5f2eeefa9ab209 Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Fri, 13 Nov 2026 19:27:21 +0000 Subject: [PATCH] fix(mcp): report HTTP listener recovery from its parsed failure rea mcp --http printed "Choose another --port or stop the process using it" for every listener failure except a missing host allowlist, so a host lookup failure, an unassigned address or a permission denial got port advice. StreamableHttpListenError already classifies the native failure and owns a recovery for each kind. Startup now always prints that owned recovery behind one shared lead-in, and the fixed message is removed. The native cause text stays out of stderr as before. Fixes #7346 Co-Authored-By: gensokyo-dev Co-Authored-By: Claude Opus 5.5 --- src/main/messages.ts | 5 ++-- src/main/transport.ts | 7 ++--- tests/boundary/mcp/mainErrors.test.ts | 40 +++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/main/messages.ts b/src/main/messages.ts index 7a37a10d8b..cce3f16ef8 100644 --- a/src/main/messages.ts +++ b/src/main/messages.ts @@ -10,5 +10,6 @@ export const MCP_SHUTDOWN_FAILED = "REA could not close cleanly. End the REA process before starting it again."; export const SERVER_START_FAILED = "Do not repeat this startup. Tell the user REA could not start and ask them to restart the MCP client, then run `rea doctor`."; -export const MCP_HTTP_LISTEN_FAILED = - "REA could not listen for MCP HTTP connections on the selected host and port. Choose another --port or stop the process using it."; +/** The listener error owns recovery for its parsed failure; never its native cause text. */ +export const mcpHttpListenFailed = (remediation: string): string => + `REA could not listen for MCP HTTP connections. ${remediation}`; diff --git a/src/main/transport.ts b/src/main/transport.ts index 8090ae613a..536faeffa2 100644 --- a/src/main/transport.ts +++ b/src/main/transport.ts @@ -12,7 +12,7 @@ import type { OptionalProviderLoadResult } from "../application/OptionalObservat import { OwnedMcpServers } from "./OwnedMcpServers.js"; import { MCP_CONNECTION_START_FAILED, - MCP_HTTP_LISTEN_FAILED, + mcpHttpListenFailed, } from "./messages.js"; import { reportMcpTransportError } from "./reportMcpTransportError.js"; @@ -181,10 +181,7 @@ export const startMcpTransport = async ( if (!cancelled) { const startFailure = cause instanceof StreamableHttpListenError - ? cause.classification.ok && - cause.classification.value.kind === "host_allowlist" - ? cause.remediationAction - : MCP_HTTP_LISTEN_FAILED + ? mcpHttpListenFailed(cause.remediationAction) : MCP_CONNECTION_START_FAILED; serverLogger.error( { diff --git a/tests/boundary/mcp/mainErrors.test.ts b/tests/boundary/mcp/mainErrors.test.ts index f7ce005646..52f1ac2d6c 100644 --- a/tests/boundary/mcp/mainErrors.test.ts +++ b/tests/boundary/mcp/mainErrors.test.ts @@ -8,7 +8,7 @@ import { MCP_CONNECTION_LOST, MCP_TRANSPORT_ERROR, MCP_CONNECTION_START_FAILED, - MCP_HTTP_LISTEN_FAILED, + mcpHttpListenFailed, MCP_SHUTDOWN_FAILED, SERVER_START_FAILED, } from "../../../src/main/messages.js"; @@ -251,7 +251,9 @@ describe("MCP runtime HTTP transport errors", () => { }), ), ).toBe(1); - expect(output).toEqual([`${refused.remediationAction}\n`]); + expect(output).toEqual([ + `${mcpHttpListenFailed(refused.remediationAction)}\n`, + ]); expect(output[0]).toContain("Set MCP_ALLOWED_HOSTS"); expect(output[0]).not.toContain("Choose another --port"); }); @@ -273,6 +275,38 @@ describe("MCP runtime HTTP transport errors", () => { }), ), ).toBe(1); - expect(output).toEqual([`${MCP_HTTP_LISTEN_FAILED}\n`]); + expect(output).toHaveLength(1); + expect(output[0]).toMatch( + /^REA could not listen for MCP HTTP connections\./u, + ); + expect(output[0]).not.toContain("SECRET"); }); + + it.each([ + ["ENOTFOUND", /host name resolution/u], + ["EADDRNOTAVAIL", /address assigned to this machine/u], + ["EACCES", /permitted to bind/u], + ["EADDRINUSE", /another --port or MCP_PORT/u], + ])( + "reports %s listener recovery from its parsed failure", + async (code, recovery) => { + const output: string[] = []; + const cause = Object.assign(new Error(`SECRET ${code}`), { code }); + expect( + await run( + dependencies({ + serve: () => Promise.reject(new StreamableHttpListenError(cause)), + output, + shutdown: [], + exitCodes: [], + }), + ), + ).toBe(1); + expect(output).toHaveLength(1); + expect(output[0]).toMatch(recovery); + expect(output[0]).not.toContain("SECRET"); + if (code !== "EADDRINUSE") + expect(output[0]).not.toContain("Choose another --port"); + }, + ); });