PR #51 - replohq/replopencode

When a tool server fails to start inside an agent sandbox, our logs currently say only that it failed — not why. This adds the actual error to that log line, so the next time a sandbox comes up missing its tools, whoever investigates can see the reason instead of guessing.

Context

Found while root-causing a production alert in Replo (REPL-32353, customer report USE-2645). Companion fix in the Replo repo: replohq/andytown#28101, which makes the sandbox retry a server whose bring-up failed. This PR is the observability half — it does not change behavior.

The gap

create() has two failure exits, and only one of them logs its reason:

tools/list fails after connect → failed to get tools with error
connect itself fails → server unavailable with status=failed and nothing else

Both connectRemote and connectLocal already capture the reason into status.error; the log site just dropped it on the floor.

The asymmetry matters because the silent path is the common one. Over 15 days of Replo production logs, server unavailable accounted for 109 of 111 occurrences against 2 for the path that does log its error. So in practice this class of failure was undiagnosable — we could see that a sandbox came up with zero tools from a server, and never why.

Implementation note

Status is a discriminated union: failed and needs_client_registration carry error: string, while needs_auth does not. The "error" in status check narrows to the two variants that have it and types as string | undefined, so there is no cast and no optional access on a field that may not exist.

Testing Done

New test in test/mcp/lifecycle.test.ts, alongside the existing unavailable remote server is marked failed without tools (same 503 Bun.serve fixture) and mirroring failed tool discovery surfaces the underlying error in the status, which is the analogue for the other path. It captures logs with the Logger.make + Logger.layer([capture], { mergeWithExisting: true }) pattern already used in test/session/prompt.test.ts.

Verified it fails without the source change (error comes back undefined) and passes with it. Full test/mcp/ suite: 64 pass, 0 fail. tsgo --noEmit on opencode: clean. oxlint: 5 warnings before and after — all pre-existing in untouched code, none added.

prettier --check flags src/mcp/index.ts, but it does so on a clean dev too; prettier's own output for the changed block is byte-identical to what is committed, so no unrelated lines were reformatted.

Risk

Log annotation only — no control flow, no data, no API contract. One thing worth a second look: this puts an error message into a log, and Replo's sandbox-perimeter work explicitly aims to keep credentials out of logs and error bodies. The reason travels via client.connect(transport); auth material rides in Authorization headers rather than URLs, so these errors surface status codes and transport failures. I believe the risk is low, but it is exactly the invariant that work established, so it deserves review rather than my word.

Rollback is a revert — nothing persists.

🤖 Generated with Claude Code

Summary by cubic

Adds the failure reason to the MCP "server unavailable" log line so connect failures are diagnosable rather than a bare status=failed.

Found while root-causing REPL-32353: a sandbox came up with zero tools, and the silent connect-failure path had logged 109 of 111 production occurrences over 15 days, versus 2 for the path that already included the error. This is logging-only — it does not change control flow. The "error" in status check narrows the discriminated union since needs_auth carries no error field. One thing to verify in review: the error travels via client.connect(transport); auth material rides in Authorization headers, so these errors surface status codes and transport failures, not credentials.