[PR #1288] [MERGED] fix(mcp): measure tool errors against their text-only delivery #2768

New issue

Closed

opened 2026-10-17 21:10:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:10:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1288
Author: @kaoru0822-kitauji
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/mcp-error-text-budget

📝 Commits (1)

7b51121 fix(mcp): measure tool errors against their text-only delivery

📊 Changes

8 files changed (+103 additions, -14 deletions)

View changed files

📝 docs/mcp-contracts.md (+2 -1)
📝 scripts/lib/real-ghidra-large-results.mjs (+5 -3)
📝 src/server/EvidenceMcpServer.ts (+2 -1)
📝 src/server/toolResult.ts (+7 -1)
📝 src/server/toolResultEncoding.test.ts (+19 -0)
📝 src/server/toolResultEncoding.ts (+20 -5)
📝 tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts (+46 -2)
📝 tests/process-global/mcpDeliveryEnvironment.test.ts (+2 -1)

📄 Description

Summary

Since #1277, an MCP tool error reaches the client as text only: EvidenceMcpServer.send removes the private structuredContent carrier. But both ToolResultDelivery.toErrorToolResult and EvidenceMcpServer.recoverError still measured the error with encodeToolResult, which counts the structured JSON and the escaped text. An error whose delivered text fits the budget, but whose doubled size does not (roughly ½ to 1× the budget), was replaced by resource_constraint. When Evidence retention is unavailable or fails, the original diagnostic is then lost even though it would have fit.

This was flagged by the Codex review on #1277 (src/server/EvidenceMcpServer.ts:125, "Measure errors against the text-only envelope") and was still present on main.

Change

encodeToolResult takes the delivered representations: "structured-and-text" (default, successful results, unchanged accounting) or "text" (errors). Text-only accounting uses the envelope without structuredContent and counts only escaped text bytes and code units.
toErrorToolResult and recoverError measure errors as "text".
docs/mcp-contracts.md says that only an error's text counts against the budget.

Tests

src/server/toolResultEncoding.test.ts: text-only byte accounting equals the actual {content:[{type:"text",…}]} serialization, including multi-byte, quote/newline and lone-surrogate text, and fails one byte below.
tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts: a new case delivers a ½-budget + 64 KiB diagnostic intact (fails on main, where it becomes resource_constraint). The existing oversized cases now use a diagnostic larger than the whole budget.
tests/process-global/mcpDeliveryEnvironment.test.ts: the per-server budget diagnostic now sits between the small and large budgets, so it still distinguishes the two servers.
scripts/lib/real-ghidra-large-results.mjs: the oversized-error probe relied on double counting. It now uses a selector of " characters (2 request bytes each, 4 response bytes each once the echoed diagnostic is escaped as text). The request is about 5.4 MB, under the 10 MiB receive limit, and the error response is about 10.7 MB, over the budget.

Verification run:

npm run check:changed, npx tsc --noEmit, npm run docs:check
npm run build:cached and npm run test:focused -- on all of tests/boundary/mcp/*.test.ts, tests/process-global/mcpDeliveryEnvironment.test.ts and tests/acceptance/tools/historicalCaptureResources.test.ts (all passing)

Real Ghidra verification was not performed (no Ghidra on this host). The large-result verifier change was checked only by computing the request and response sizes. Please run verify:ghidra large results before relying on it.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1288 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/mcp-error-text-budget` --- ### 📝 Commits (1) - [`7b51121`](https://github.com/morluto/rea/commit/7b51121f7cee21a5938f10ac3b5762e0bb41a58d) fix(mcp): measure tool errors against their text-only delivery ### 📊 Changes **8 files changed** (+103 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+2 -1) 📝 `scripts/lib/real-ghidra-large-results.mjs` (+5 -3) 📝 `src/server/EvidenceMcpServer.ts` (+2 -1) 📝 `src/server/toolResult.ts` (+7 -1) 📝 `src/server/toolResultEncoding.test.ts` (+19 -0) 📝 `src/server/toolResultEncoding.ts` (+20 -5) 📝 `tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts` (+46 -2) 📝 `tests/process-global/mcpDeliveryEnvironment.test.ts` (+2 -1) </details> ### 📄 Description ## Summary Since #1277, an MCP tool error reaches the client as text only: `EvidenceMcpServer.send` removes the private `structuredContent` carrier. But both `ToolResultDelivery.toErrorToolResult` and `EvidenceMcpServer.recoverError` still measured the error with `encodeToolResult`, which counts the structured JSON **and** the escaped text. An error whose delivered text fits the budget, but whose doubled size does not (roughly ½ to 1× the budget), was replaced by `resource_constraint`. When Evidence retention is unavailable or fails, the original diagnostic is then lost even though it would have fit. This was flagged by the Codex review on #1277 (`src/server/EvidenceMcpServer.ts:125`, "Measure errors against the text-only envelope") and was still present on `main`. ## Change - `encodeToolResult` takes the delivered representations: `"structured-and-text"` (default, successful results, unchanged accounting) or `"text"` (errors). Text-only accounting uses the envelope without `structuredContent` and counts only escaped text bytes and code units. - `toErrorToolResult` and `recoverError` measure errors as `"text"`. - `docs/mcp-contracts.md` says that only an error's text counts against the budget. ## Tests - `src/server/toolResultEncoding.test.ts`: text-only byte accounting equals the actual `{content:[{type:"text",…}]}` serialization, including multi-byte, quote/newline and lone-surrogate text, and fails one byte below. - `tests/boundary/mcp/oversizedErrorDeliveryMcp.test.ts`: a new case delivers a ½-budget + 64 KiB diagnostic intact (fails on `main`, where it becomes `resource_constraint`). The existing oversized cases now use a diagnostic larger than the whole budget. - `tests/process-global/mcpDeliveryEnvironment.test.ts`: the per-server budget diagnostic now sits between the small and large budgets, so it still distinguishes the two servers. - `scripts/lib/real-ghidra-large-results.mjs`: the oversized-error probe relied on double counting. It now uses a selector of `"` characters (2 request bytes each, 4 response bytes each once the echoed diagnostic is escaped as text). The request is about 5.4 MB, under the 10 MiB receive limit, and the error response is about 10.7 MB, over the budget. Verification run: - `npm run check:changed`, `npx tsc --noEmit`, `npm run docs:check` - `npm run build:cached` and `npm run test:focused --` on all of `tests/boundary/mcp/*.test.ts`, `tests/process-global/mcpDeliveryEnvironment.test.ts` and `tests/acceptance/tools/historicalCaptureResources.test.ts` (all passing) **Real Ghidra verification was not performed** (no Ghidra on this host). The large-result verifier change was checked only by computing the request and response sizes. Please run `verify:ghidra` large results before relying on it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-17 21:10:01 +00:00