morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #5371] [MERGED] fix(mcp): retain protocol framing for oversized tool errors #6904

New issue

Closed

opened 2026-11-01 21:43:57 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:57 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5371
Author: @gensokyo-dev
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/20/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/5273-mcp-resulttype-complete

📝 Commits (1)

f304689 fix(mcp): retain protocol framing for oversized tool errors

📊 Changes

3 files changed (+151 additions, -98 deletions)

View changed files

📝 docs/mcp-contracts.md (+9 -2)
📝 src/server/EvidenceMcpServer.test.ts (+133 -95)
📝 src/server/EvidenceMcpServer.ts (+9 -1)

📄 Description

Summary

REA's oversized-error replacement dropped the SDK's resultType on protocol 2026-07-28. The client rejected the compact resource_constraint reply instead of exposing its retained-Evidence continuation. Preserve that negotiated field at the transport owner; legacy replies keep their existing framing.

Refs #5273. This fixes the independently reproduced oversized-error path; the report's proprietary client/background behavior and original target remain unverified. Adapted from akushonkamen's patch, with contributor credit retained. The pinned SDK already negotiates modern revisions through the production serveStdio entry, so no constructor/version override is needed.

Contract and scope

Owner: ErrorEvidenceTransport.recoverError, after SDK framing and before stdio delivery. Existing read-only capability; additional maintainer scope decision is not applicable.
CLI, tool schemas/catalog, provider behavior and authority are unchanged. MCP error delivery retains SDK framing, target provenance, bounded output and the full original error in session Evidence.
resultType: "complete" means the exchange completed; it does not change Evidence partial/unknown/coverage semantics. No installation or generated-metadata changes.

Evidence and validation

Executed on Node 24.18.0 / npm 11.16.0, SDK 2.3.1, Linux:

Regression against the source base: 1 failed / 2 passed, with the exact missing required resultType decoder error. Fixed: all 3 pass. Both revisions use the public serveStdio entry and real SDK clients; no private version mutation. Cases cover success, fitting validation, oversized replacement, JSON escaping, full Evidence retention/provenance and reuse of the connection.
Real stdio, near-10-MiB malformed request: baseline client rejected the 1,807-byte replacement; candidate decoded the 1,831-byte tool error. Both exported the complete original error; normal replies remained 401 bytes.
npm run test:focused -- the changed test plus encoding, tool delivery, retained Evidence, session lifecycle/recovery, partial observations and inline composition: 52 tests passed.
npm run check: passed typecheck, formatting, lint, module/architecture guards, knip and metadata checks. Existing lint warnings remain.
npm run docs:check: passed, including 5 README consistency tests.
Real Ghidra 12.1.4 / JDK 21, gsudo 2.5.1 EXE (5,915,592 bytes; SHA-256 47d02ca82bc31ae9f4b6cc1e76dceffe610b9b664d3c18a17e0403463c48d749): modern-stdio cold analysis 313.725 s, warm 21.2 ms, 18,679 procedures / 4,006 strings. Both decoded with resultType: "complete"; validation errors, Evidence export and close succeeded. Whole completed workflow: 322.061 s elapsed / 319.747 s CPU / 1,383.9 MiB peak, no OOM, owned processes and temporary roots cleared. No target execution. An earlier diagnostic decoded a 405.053-s cold response, then exited 1 because the harness tried parsing a native SDK text error as JSON; the corrected complete rerun passed. That earlier attempt used 413.203 s / 394.523 s CPU / 1,406.0 MiB and also cleaned up.

Resource accounting uses an owned cgroup and payload-exit observer, one CPU, 8-GiB hard/7-GiB early guard, no swap. Near-frame baseline/candidate: 16.847/18.247 s elapsed, 8.839/9.105 s process-family CPU, 652.3/639.8 MiB peak; both cleaned up, no OOM. Single runs overlapped other bounded checks: no speedup claim. The mandatory field adds 24 bytes (~6 cl100k_base tokens); actual agent usage and whole malformed-input token costs are unmeasured. The completed real-Ghidra wire workflow estimates 7,980 cl100k_base tokens; including the initial harness attempt, 15,571. This includes discovery/progress/calls/replies; this named-call client did not request a catalog. Actual agent prompts/rendering and malformed-input token costs remain unmeasured. This is a framing correction, with no new analysis/cache work.

CI attempt 1 had one JavaScript resource-recovery assertion fail after its timeout case; remaining shards were cancelled by fail-fast. The identical focused file passed 2/2 on both baseline and candidate under the same resource limits. This does not establish the original cause; failed/cancelled CI jobs passed on attempt 2 without code changes. Final-head CI is green; Codex reported no findings and Greptile scored 5/5 with no actionable findings.

Author readiness

Human owner: @gensokyo-dev

The human owner explicitly delegated filling readiness, opening ready for review, and merging in the task request. Diff review and validation were performed by the agent under that delegation; no separate human diff review is claimed.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

One focused existing-boundary correction; linked report and meaningful coverage/verification gaps recorded.
Owning documentation updated; diff checked for unrelated changes, credentials and unsupported claims.
Final-head CI/review and long-analysis verification complete.

_{🔄 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/5371 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/20/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/5273-mcp-resulttype-complete` --- ### 📝 Commits (1) - [`f304689`](https://github.com/morluto/rea/commit/f3046898d060ed2772ceae01ab678b29f53d0d59) fix(mcp): retain protocol framing for oversized tool errors ### 📊 Changes **3 files changed** (+151 additions, -98 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+9 -2) 📝 `src/server/EvidenceMcpServer.test.ts` (+133 -95) 📝 `src/server/EvidenceMcpServer.ts` (+9 -1) </details> ### 📄 Description ## Summary REA's oversized-error replacement dropped the SDK's `resultType` on protocol `2026-07-28`. The client rejected the compact `resource_constraint` reply instead of exposing its retained-Evidence continuation. Preserve that negotiated field at the transport owner; legacy replies keep their existing framing. Refs #5273. This fixes the independently reproduced oversized-error path; the report's proprietary client/background behavior and original target remain unverified. Adapted from [akushonkamen's patch](https://github.com/akushonkamen/rea/commit/efd645e30b5b205e9be3ac20b718f1dc6af586ff), with contributor credit retained. The pinned SDK already negotiates modern revisions through the production `serveStdio` entry, so no constructor/version override is needed. ## Contract and scope - Owner: `ErrorEvidenceTransport.recoverError`, after SDK framing and before stdio delivery. Existing read-only capability; additional maintainer scope decision is not applicable. - CLI, tool schemas/catalog, provider behavior and authority are unchanged. MCP error delivery retains SDK framing, target provenance, bounded output and the full original error in session Evidence. - `resultType: "complete"` means the exchange completed; it does not change Evidence partial/unknown/coverage semantics. No installation or generated-metadata changes. ## Evidence and validation Executed on Node 24.18.0 / npm 11.16.0, SDK 2.3.1, Linux: - Regression against the source base: 1 failed / 2 passed, with the exact `missing required resultType` decoder error. Fixed: all 3 pass. Both revisions use the public `serveStdio` entry and real SDK clients; no private version mutation. Cases cover success, fitting validation, oversized replacement, JSON escaping, full Evidence retention/provenance and reuse of the connection. - Real stdio, near-10-MiB malformed request: baseline client rejected the 1,807-byte replacement; candidate decoded the 1,831-byte tool error. Both exported the complete original error; normal replies remained 401 bytes. - `npm run test:focused --` the changed test plus encoding, tool delivery, retained Evidence, session lifecycle/recovery, partial observations and inline composition: **52 tests passed**. - `npm run check`: passed typecheck, formatting, lint, module/architecture guards, knip and metadata checks. Existing lint warnings remain. - `npm run docs:check`: passed, including **5** README consistency tests. - Real Ghidra 12.1.4 / JDK 21, gsudo 2.5.1 EXE (5,915,592 bytes; SHA-256 `47d02ca82bc31ae9f4b6cc1e76dceffe610b9b664d3c18a17e0403463c48d749`): modern-stdio cold analysis **313.725 s**, warm **21.2 ms**, **18,679** procedures / **4,006** strings. Both decoded with `resultType: "complete"`; validation errors, Evidence export and close succeeded. Whole completed workflow: **322.061 s elapsed / 319.747 s CPU / 1,383.9 MiB peak**, no OOM, owned processes and temporary roots cleared. No target execution. An earlier diagnostic decoded a 405.053-s cold response, then exited 1 because the harness tried parsing a native SDK text error as JSON; the corrected complete rerun passed. That earlier attempt used 413.203 s / 394.523 s CPU / 1,406.0 MiB and also cleaned up. Resource accounting uses an owned cgroup and payload-exit observer, one CPU, 8-GiB hard/7-GiB early guard, no swap. Near-frame baseline/candidate: 16.847/18.247 s elapsed, 8.839/9.105 s process-family CPU, 652.3/639.8 MiB peak; both cleaned up, no OOM. Single runs overlapped other bounded checks: no speedup claim. The mandatory field adds 24 bytes (~6 `cl100k_base` tokens); actual agent usage and whole malformed-input token costs are unmeasured. The completed real-Ghidra wire workflow estimates 7,980 `cl100k_base` tokens; including the initial harness attempt, 15,571. This includes discovery/progress/calls/replies; this named-call client did not request a catalog. Actual agent prompts/rendering and malformed-input token costs remain unmeasured. This is a framing correction, with no new analysis/cache work. CI attempt 1 had one JavaScript resource-recovery assertion fail after its timeout case; remaining shards were cancelled by fail-fast. The identical focused file passed **2/2** on both baseline and candidate under the same resource limits. This does not establish the original cause; failed/cancelled CI jobs passed on attempt 2 without code changes. Final-head CI is green; Codex reported no findings and Greptile scored 5/5 with no actionable findings. ## Author readiness Human owner: @gensokyo-dev The human owner explicitly delegated filling readiness, opening ready for review, and merging in the task request. Diff review and validation were performed by the agent under that delegation; no separate human diff review is claimed. - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] One focused existing-boundary correction; linked report and meaningful coverage/verification gaps recorded. - [x] Owning documentation updated; diff checked for unrelated changes, credentials and unsupported claims. - [x] Final-head CI/review and long-analysis verification complete. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:43:57 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:51 +00:00

[PR #6904] [CLOSED] fix(captures): enforce object types and numeric bounds on HAR schemas (#6890) #7436

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6904

No description provided.

Rows
Columns