[addyCooks]

Description

An MCP server reports a failed call with isError: true and the reason in content. MCPClient turned the content into text but never read the flag, so the call looked like a success downstream. ACP clients (the VS Code extension, the desktop app, editors such as Zed) showed it as completed, and nanocoder run --json filed the text under toolCalls[].result instead of .error. The model itself always received the text.

executeToolCall now returns the existing {llmContent, isError} output when the server sets isError: true, which processToolUse already carries onto ToolResult.isError (the same mechanism execute_bash uses for a non-zero exit). The text the model receives is unchanged, and a clean result is still a plain string. ACP, the plain runtime and the interactive loop already read isError, so none of them needed a change.

Checked with a stdio MCP server (echo and an always-failing boom) behind a scripted OpenAI-compatible stub:

• ACP, yolo and auto-accept: boom was completed before and is failed now; echo stays completed.
• nanocoder --plain --json run: boom moved from result to error.
• The tool message sent to the model is identical before and after ("boom failure").

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully)
• Tests cover both success and error scenarios

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md)

[github-actions]

nc-review: comments — 1 nit

@addyCooks — a few things worth a look, none blocking.

The change makes MCPClient.callTool return the existing StructuredToolOutput shape ({llmContent, isError}) when a server answers with isError: true, so processToolUse propagates the failure flag to the ACP status, the --plain --json telemetry and the interactive display, while a clean result stays a plain string. I traced the flag through the registry handler, processToolUse, the ACP conversation, the plain runtime and the subagent path; all of them already read isError, and the two new tests would fail against the pre-change code, so the fix is correct and reasonably scoped. One edge case is worth a look: a failed MCP call with empty content still tells the model the tool "executed successfully".

⚪ nit · completeness · source/mcp/mcp-client.ts:1221

The failure flag travels to the client, but never to the model — the model only ever sees llmContent. For a server that answers isError: true with an empty content array, the text the model receives is still:

return 'Tool executed successfully (no output)';

so the model is told the call succeeded while the editor now shows it as failed. The new spec even pins this text (llmContent: 'Tool executed successfully (no output)' with isError: true). This is a rare shape (a server that reports a failure without giving a reason), so it is not worth blocking on, but the fallback string could acknowledge the failure — e.g. "Tool execution failed (no output)" when result.isError is true — so the model is not misled. The flag itself cannot be forwarded, since ToolResult.content is the only channel to the model.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}