fix(mcp-server): return tool errors as isError results per MCP spec

[Scra3]

Summary

• Catch tool handler errors in tool-with-logging.ts and return them as { isError: true } results instead of throwing
• Per the MCP specification, tool execution errors should be reported within the result object, not as protocol-level errors

Problem

When a tool handler throws an error, the MCP SDK catches it and may replace the original message with a generic text. This means the LLM client receives an unhelpful error message and cannot adapt its behavior.

Solution

By returning { content: [{ type: 'text', text: message }], isError: true } instead of throwing, the error message is preserved and forwarded to the LLM as-is.

Test plan

• yarn workspace @forestadmin/mcp-server test — 483 tests pass
• yarn workspace @forestadmin/mcp-server lint — 0 errors
• Updated 24 test assertions across 9 test files

🤖 Generated with Claude Code

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

Rating	File	% Diff	Uncovered Line #s
	packages/mcp-server/src/utils/tool-with-logging.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

• Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

• Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

• File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

  • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

[EnkiP]

I think JSON.stringify(error) is better than String(error) to avoid returning things like "[object]"

[Scra3]

Code review

1 issue found — weak assertions in 4 test files.

Weak expect.any(String) assertions

In the "should return error result when no parsable error found" tests across 4 files, expect.any(String) is used when the output is deterministic ("[object Object]" — the error { unknownProperty: 'some value' } goes through withActivityLog → String(error) → new Error("[object Object]")).

Files:

• create.test.ts#L291
• delete.test.ts#L281
• execute-action.test.ts#L449
• update.test.ts#L306

Fix: Replace expect.any(String) with '[object Object]' — this is the exact, deterministic output.

Otherwise, no bugs found. Code is correct and CLAUDE.md compliant.