fix(mcp): reply instead of hanging when a command exits non-zero

[jahvon]

Part 1/2. Base: main. Independent of the Python stack (#439–#443).

Summary

Any MCP tool call whose flow subprocess exited non-zero left the client waiting on a request that never got a response, until it hit an idle timeout. A failing test, a script that raises, an executable that is not found — each one hung the agent instead of reporting the failure.

internal/mcp/command_executor.go passed a value to errors.As:

var exitErr exec.ExitError        // value
if !errors.As(err, &exitErr) {    // panics

ExitError.Error() has a pointer receiver, so exec.ExitError does not implement error, and errors.As panics on such a target — errors: *target must be interface or implement error. That panic killed the tool handler's goroutine before it could reply.

The fix is *exec.ExitError, matching tests/utils/runner.go:35, which already had it right. That was the only other occurrence in the repo.

Notable Changes

• Restores the behavior the comment always described: a non-zero exit is a normal outcome whose detail is already in the captured output, so it returns as output with a nil error. Real failures (binary missing, context cancelled) still surface as errors.
• Affects every tool that shells out: execute, run_command, run_executable, get_execution_logs, sync_executables, and write_flowfile's post-write sync.

Why it was invisible

Nothing looked wrong from outside the process:

• The subprocess had already run and finished — flow logs showed the run recorded with its terminal failed status and error message, about a second in.
• No child or grandchild process was left running while the call still hung, so nothing was blocked on I/O.
• The server kept answering other requests instantly (get_info responded while the call was stuck) — only the one request was lost, so it read as a slow command rather than a crash.

The existing MCP tests all use a mocked CommandExecutor, so none of them ever exercised the real exit path.

Testing

• New TestExecuteContext_NonZeroExitReturnsOutputNotPanic drives the real FlowCLIExecutor against a stand-in binary via FLOW_CLI_BINARY. Verified it fails with the original panic when the fix is reverted, and passes with it.
• Plus success-path and missing-binary cases, so the "swallow exit errors, surface real ones" split is pinned.
• flow validate passes (generate → lint → unit + e2e → schema validation).

🤖 Generated with Claude Code

https://claude.ai/code/session_01R328pa3FUUfga4gYah1iQi

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!