Skip to content

fix(mcp): repair stdio startup diagnostics - #3563

Open
joestump wants to merge 2 commits into
charmbracelet:mainfrom
joestump:fix/mcp-stdio-diagnostics
Open

fix(mcp): repair stdio startup diagnostics#3563
joestump wants to merge 2 commits into
charmbracelet:mainfrom
joestump:fix/mcp-stdio-diagnostics

Conversation

@joestump

Copy link
Copy Markdown
Contributor

Two independent bugs that leave maybeStdioErr unable to do its job, so a stdio MCP server that dies during startup reports EOF and nothing else. Split out of #3522 at a reviewer's convenience — neither has anything to do with that PR's feature.

The symptom

Misconfigure a stdio server — a package name that doesn't exist, or node not on PATH:

{ "mcp": { "demo": { "type": "stdio", "command": "npx", "args": ["-y", "not-a-real-package"] } } }

The server shows as errored with EOF. The child's actual complaint is discarded, which is the exact case stdioCheck was written for.

Bug 1 — the diagnostic re-run duplicates argv0

exec.Cmd.Args already carries argv0 as its first element, and exec.CommandContext prepends Path as argv0 itself. Passing Args through whole therefore re-runs npx npx -y pkg:

cmd := exec.CommandContext(ctx, old.Path, old.Args...) // "npx" + ["npx", "-y", "pkg"]

So on the occasions the diagnostic did run, it reported that malformed command's failure — typically cannot execute binary file, the shell trying to exec itself as a script — instead of the child's real output.

Bug 2 — the transport is never unwrapped

createSession wraps every transport in a channelTransport before Connect, and the error path then hands that wrapper to maybeStdioErr:

transport = &channelTransport{inner: transport, name: name, gate: channelGate}
// ...
session, err := client.Connect(mcpCtx, transport, nil)
if err != nil {
    err = maybeStdioErr(err, transport)   // always the wrapper, never the CommandTransport

The transport.(*mcp.CommandTransport) assertion inside can therefore never match, and the function returns the bare EOF unchanged. Every stdio startup diagnostic has been silently dead since transports started being decorated.

The unwrap goes through a small transportWrapper interface rather than a type switch on channelTransport, so a future decorator cannot quietly reintroduce this — failing to implement the interface is the only way back to the bug.

Notes

  • Commits are ordered so each is green on its own: the argv0 fix lands first, because the unwrap's tests assert on the child's real output and would fail without it.
  • Three tests, each verified to fail with only its own fix reverted: TestStdioCheck_DoesNotDuplicateArgv0, TestMaybeStdioErr_UnwrapsChannelTransport, and TestMaybeStdioErr_UnwrapsEveryWrapper (which stacks a second, test-local decorator to pin that the unwrap peels the whole chain rather than a fixed number of layers).
  • go test -race -failfast ./... and golangci-lint run are green on this branch.
  • Both fixes have been running in a downstream fork; this is the upstream-adapted version, cut fresh from main.

🤖 Posted on behalf of @joestump by claude-opus-5 using Claude Code.

joestump and others added 2 commits August 16, 2026 07:44
exec.Cmd.Args already carries argv0 as its first element, and
exec.CommandContext prepends Path as argv0 itself — so stdioCheck's
diagnostic re-run built "npx npx -y pkg" from "npx -y pkg". Rather than
the child's real startup error, the joined error carried whatever that
malformed command produced ("cannot execute binary file", as the shell
tries to exec itself as a script), which is precisely the output
stdioCheck exists to replace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
maybeStdioErr exists so that a stdio server which dies during startup
reports why — it re-runs the command and joins the child's output onto
the bare io.EOF the SDK surfaces. It has not been able to do that since
transports started being decorated: createSession wraps every transport
in a channelTransport before Connect, so the *mcp.CommandTransport type
assertion never matches and the function returns the bare EOF
unchanged. A missing `npx`, or node not on PATH, reports "EOF" and
throws the child's stderr away.

Peel the decorators before asserting. The unwrap goes through a
transportWrapper interface rather than a type switch on channelTransport
so that adding another decorator cannot silently reintroduce this — a
wrapper that does not implement it is the only way back to the bug.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@meowgorithm meowgorithm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants