Skip to content

fix(mcp): complete command captures exit code instead of erroring#17174

Closed
andrewgazelka wants to merge 2 commits intonushell:mainfrom
andrewgazelka:mcp-work
Closed

fix(mcp): complete command captures exit code instead of erroring#17174
andrewgazelka wants to merge 2 commits intonushell:mainfrom
andrewgazelka:mcp-work

Conversation

@andrewgazelka
Copy link
Contributor

@andrewgazelka andrewgazelka commented Dec 14, 2025

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.

  • Worker runs with --no-config-file (and apparently needs this or some reason)
  • Worker has isolated stdio (Stdio::null()) so external commands don't mix with JSON-RPC

Test plan

  • let a = (^ls nonexistent | complete); $a now returns {stdout:"",stderr:"...",exit_code:2} instead of erroring

andrewgazelka and others added 2 commits December 13, 2025 18:09
…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};
Copy link
Contributor

Choose a reason for hiding this comment

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

The solution will need to work in windows too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@fdncred
Copy link
Contributor

fdncred commented Jan 30, 2026

How do other MCP tools spawn externals and communicate? Is there something in the MCP docs about this usecase?

Answer:
Ah, when spawning externals, the externals need to only communicate on stderr as to not confuse the MCP. This is exactly what I do when debugging nushell.

@fdncred
Copy link
Contributor

fdncred commented Jan 30, 2026

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 Stdio::piped() and then process it.

I think something like this would work. I mean it's worth a try at least.

@andrewgazelka
Copy link
Contributor Author

andrewgazelka commented Jan 30, 2026

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 Stdio::piped() and then process it.

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 | complete in nushell and I coudln't get it working although I was kinda relying on claude code and not my brain and claude code is horribly wrong sometimes

@fdncred
Copy link
Contributor

fdncred commented Jan 30, 2026

the issue is it seemed to interfere with | complete in nushell and I coudln't get it working although I was kinda relying on claude code and not my brain and claude code is horribly wrong sometimes

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 dbg!() since they go to stderr and won't mess with nushell.

I wonder if it's possible for MCP to communicate differently?

@andrewgazelka
Copy link
Contributor Author

andrewgazelka commented Jan 30, 2026

the issue is it seemed to interfere with | complete in nushell and I coudln't get it working although I was kinda relying on claude code and not my brain and claude code is horribly wrong sometimes

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 dbg!() since they go to stderr and won't mess with nushell.

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)

@fdncred
Copy link
Contributor

fdncred commented Jan 31, 2026

I can see 2 alternatives right now, there's probably more.

  1. Do what you're already doing in this PR but make it cross platform - maybe use https://github.com/kouhe3/win_uds since nushell already uses it. (which we just recently used to replace this https://github.com/haraldh/rust_uds_windows) it's a type of unix socket that works on windows too but i'm not sure it's compatible with what we want to do.
  2. Maybe capture stdout ourselves and only pass on the json-rpc bits, thus filtering the bad things out. maybe print the "bad" things to stderr?

@andrewgazelka
Copy link
Contributor Author

andrewgazelka commented Jan 31, 2026

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)

@fdncred
Copy link
Contributor

fdncred commented Jan 31, 2026

I'm not opposed to nu --mcp --http with stdio as the default.

@andrewgazelka
Copy link
Contributor Author

I'm not opposed to nu --mcp --http with stdio as the default.

Would you be opposed to the Nushell using standard I/O by default and starting the HTTP and proxying to it?

@fdncred
Copy link
Contributor

fdncred commented Jan 31, 2026

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.

@andrewgazelka
Copy link
Contributor Author

andrewgazelka commented Jan 31, 2026

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]
Loading

@fdncred
Copy link
Contributor

fdncred commented Jan 31, 2026

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.

@andrewgazelka
Copy link
Contributor Author

Closing — too many conflicts with current main and the approach was experimental.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP: complete command doesn't capture external command errors

3 participants