fix(mcp): complete command captures exit code instead of erroring#17174
fix(mcp): complete command captures exit code instead of erroring#17174andrewgazelka wants to merge 2 commits intonushell:mainfrom
Conversation
…nushell#17173) The MCP server now spawns a separate worker process for nushell evaluation, communicating via unix socket instead of stdio. This isolates external commands from the MCP JSON-RPC transport. Key changes: - Added worker process architecture: parent handles MCP transport, child evaluates code - Worker communicates via unix socket at a temp path based on parent PID - Worker runs with --no-config-file to avoid config interference with error handling - Added integration test that spawns actual worker binary The root cause was that config file loading interfered with how external command errors are handled. Skipping config files ensures clean, predictable behavior. Fixes nushell#17173 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The nushell project disallows unwrap() via clippy::unwrap_used. Use expect() with a clear message explaining the invariant.
| //! - Worker process: Evaluates nushell code with its own stdin/stdout | ||
|
|
||
| use std::io::{BufRead, BufReader, Write}; | ||
| use std::os::unix::net::{UnixListener, UnixStream}; |
There was a problem hiding this comment.
The solution will need to work in windows too.
There was a problem hiding this comment.
The solution will need to work in windows too.
more of just a POC for fixing it; curious what you think about it verus if there is some other better way
|
How do other MCP tools spawn externals and communicate? Is there something in the MCP docs about this usecase? Answer: |
|
Here's a rust example. use std::process::{Command, Stdio};
fn spawn_external_cleanly() -> std::io::Result<std::process::Child> {
let mut child = Command::new("some-tool")
.arg("--some")
.arg("option")
// Critical: discard child's stdout
.stdout(Stdio::null())
// Keep stderr going to our stderr (allowed in MCP stdio mode)
.stderr(Stdio::inherit())
// Usually also safe/wise to null stdin unless you need to write to it
.stdin(Stdio::null())
.spawn()?;
// You can still .wait() on the child later if you care about exit code
// let status = child.wait()?;
Ok(child)
}If we want to capture the output we'd use I think something like this would work. I mean it's worth a try at least. |
so I was trying to play around with this... the issue is it seemed to interfere with |
nushell's complete command is supposed to capture stderr and stdout. However, nushell communicates some things on stdout as well so it may not be complete that is confusing things but nushell in general. Like i said earlier, when I'm debugging nushell all my debug statements are eprintln!() or I wonder if it's possible for MCP to communicate differently? |
yes it can. I changed this a while ago to do MCP over streamable http(s) and it works perfectly. I might want to explore this again. The sad part is this requires the mcp server to be started as a daemon somewhere which is awesome (can use over SSH) but also not (bad DX locally) |
|
I can see 2 alternatives right now, there's probably more.
|
|
another thought that might be jank have streamable http option AND stdio option the stdio just spawns the streamable http server and is a proxy to it I'm kinda leaning towards this as I actually want streamable http regardless (for interacting with remote servers) |
|
I'm not opposed to |
Would you be opposed to the Nushell using standard I/O by default and starting the HTTP and proxying to it? |
I'm not sure what that means. Please explain. |
flowchart LR
Client[MCP Client] ---|stdio| Proxy[Stdio MCP sever<br>as Thin Proxy]
Proxy ---|HTTP| Server[HTTP MCP Server]
Server --> Nu[Nu Engine]
Nu --> Ext[External Commands]
|
|
I guess I'd have to see what the updates look like. If it makes it easier for everything then I'd probably be for it unless it's super brittle or complex. Note: Your graph didn't render because you didn't spell mermaid correctly. |
|
Closing — too many conflicts with current main and the approach was experimental. |
Summary
This is really jank and I feel there is almost certainly a better way, but it does fix the issue I was having.
Fixes #17173
The MCP server now spawns a separate worker process for nushell evaluation, communicating via unix socket instead of stdio. This isolates external commands from the MCP JSON-RPC transport.
--no-config-file(and apparently needs this or some reason)Stdio::null()) so external commands don't mix with JSON-RPCTest plan
let a = (^ls nonexistent | complete); $anow returns{stdout:"",stderr:"...",exit_code:2}instead of erroring