[kevoconnell]

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.

^{Written for commit 5917dc7. Summary will update on new commits.}

[github-actions]

Hey! Your PR title Log why an MCP server was unavailable, not just that it was doesn't follow conventional commit format.

Please update it to start with one of:

• feat: or feat(scope): new feature
• fix: or fix(scope): bug fix
• docs: or docs(scope): documentation changes
• chore: or chore(scope): maintenance tasks
• refactor: or refactor(scope): code refactoring
• test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

[cubic-dev-ai]

2 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode/src/mcp/index.ts">

<violation number="1" location="packages/opencode/src/mcp/index.ts:406">
P3: For the `needs_auth` status the log line now carries an `error: undefined` field, which is noise in exactly the log this PR is improving. Spread the field conditionally so it is absent rather than `undefined` when the status has no error.</violation>

<violation number="2" location="packages/opencode/src/mcp/index.ts:406">
P2: The newly logged `status.error` is raw `error.message` from the connect path. For local servers this can embed subprocess/stderr text, and for remote servers it can embed URL or server-returned content, so a server or misconfigured URL containing credentials would now dump them into logs. Mask/sanitize the error string (strip URLs' userinfo, cap length) before logging, or verify none of these transports place credentials in `error.message`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: The newly logged status.error is raw error.message from the connect path. For local servers this can embed subprocess/stderr text, and for remote servers it can embed URL or server-returned content, so a server or misconfigured URL containing credentials would now dump them into logs. Mask/sanitize the error string (strip URLs' userinfo, cap length) before logging, or verify none of these transports place credentials in error.message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/mcp/index.ts, line 406:

<comment>The newly logged `status.error` is raw `error.message` from the connect path. For local servers this can embed subprocess/stderr text, and for remote servers it can embed URL or server-returned content, so a server or misconfigured URL containing credentials would now dump them into logs. Mask/sanitize the error string (strip URLs' userinfo, cap length) before logging, or verify none of these transports place credentials in `error.message`.</comment>

<file context>
@@ -399,7 +399,12 @@ const layer = Layer.effect(
+              key,
+              type: mcp.type,
+              status: status.status,
+              error: "error" in status ? status.error : undefined,
+            })
           }
</file context>

[cubic-dev-ai]

P3: For the needs_auth status the log line now carries an error: undefined field, which is noise in exactly the log this PR is improving. Spread the field conditionally so it is absent rather than undefined when the status has no error.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode/src/mcp/index.ts, line 406:

<comment>For the `needs_auth` status the log line now carries an `error: undefined` field, which is noise in exactly the log this PR is improving. Spread the field conditionally so it is absent rather than `undefined` when the status has no error.</comment>

<file context>
@@ -399,7 +399,12 @@ const layer = Layer.effect(
+              key,
+              type: mcp.type,
+              status: status.status,
+              error: "error" in status ? status.error : undefined,
+            })
           }
</file context>

Suggested change

	error: "error" in status ? status.error : undefined,
	...("error" in status ? { error: status.error } : {}),