Repository navigation
fix(mcp): bound tool result size, stream-cap upstream reads, and report truncation explicitly - #489
Jonathan-ai9 wants to merge 1 commit into
Conversation
|
@Jonathan-ai9 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Coordination note — ten PRs are editing one 179-line fileIf your MCP PR has just gone red with a conflict, this is why, and it is my fault rather than yours. I published 22 MCP issues (#404–#425) as if they were independent. They are not: almost all of them land in Already merged:
Order I intend to take the rest in, so you can rebase once rather than repeatedly:
Please do not all rebase at once. If you are below the one currently in flight, wait until the one above you lands — otherwise you rebase, I merge someone else, and you rebase again. I will comment on your PR when it is your turn. One thing to settle between you: Nothing here reflects on the quality of the work. Several of these are among the best PRs this repo has had. |
|
#487 has landed — the I resolved #487's conflicts myself because nine PRs were stuck behind it. Three things on 1. Two refusal codes, deliberately separate. ERR_URL_BLOCKED // the target resolves somewhere we will never fetch (#488)
ERR_HOST_NOT_ALLOWED // reachable, but not on the paid_call allow-list (#487)If your PR adds a refusal, extend this vocabulary rather than starting a third. "I cannot reach there" and "I will not pay there" are different answers; whatever you add should be a third distinct answer, not a synonym for one of these. 2. The constructor takes 3. Push the rebase and I will review it. If it conflicts in a way that is not obvious, say so and I will resolve it rather than leave you stuck. |
A tool result is written straight into the agent's context window, so an unbounded one is self-inflicted denial of service whose failure gets blamed on the model. All four tools serialise compactly and are capped at MCP_MAX_RESULT_BYTES; a truncated result carries truncated, the total, the page returned, and a notice saying how to ask for more. bazaar_search gains limit/offset arguments, forwards them to the node and pages whatever comes back. lens_get_price keeps structuredContent only while the payload fits, since shipping it regardless would move the blow-up to another field. Upstream bodies are read through a capped stream reader that cancels at the cap, so an oversized body is never buffered; a discovery body too large to parse returns ERR_RESPONSE_TOO_LARGE, which is deliberately not one of the two pre-request refusals: we did fetch it, we will not dump it. Rebased onto main after Divinity-Labs#480 (structured paid-call errors), Divinity-Labs#487 (paid_call host allow-list) and Divinity-Labs#492 (spend ceiling) landed: the bounds now sit on top of those gates rather than beside them, and limit/offset go through the Zod tool schemas.
adf58e3 to
25a7dbe
Compare
|
This has already landed. It came in alongside the MCP resources work rather than through this PR, which is why this one is still open. Both halves you describe in the title are there — the upstream stream cap and the result-size bound — plus Worth repeating the one line from that module's doc comment, because it is the part that makes the fix correct rather than merely present:
A bounded collection is reported as a page with a total, so the agent knows it is holding part of a list. Capping without saying so would turn an incomplete answer into a confident wrong one, which is worse than the unbounded result. If you want another issue, comment on an unclaimed one and I will assign it before you start — there are open ones in the MCP area if that is where you would rather work. |
Closes #416
Summary
Tool results were serialised in full with
JSON.stringify(data, null, 2), so a large or hostile Bazaar response could fill the agent's context in a single call. This PR caps both what is read from the upstream and what is returned to the agent, removes pretty-printing, pages collections with a count, and states clearly in the result whenever anything was truncated.Why
An agent that spends its context on one oversized tool result cannot do the task it was called for, and the failure looks like the model being bad rather than the tool being unbounded. Pretty-printing inflates the output further with no benefit to a machine reader. Reading the whole upstream body before checking its size also lets a hostile node force the server to buffer megabytes.
Changes
Content-Lengthup front and fail fast when it already exceeds the cap, without trusting it as the only guardJSON.stringify(data, null, 2)with compactJSON.stringify(data)totaland a documentedoffset/limit(or cursor) parameter to request more; coordinated with L214 so both tools use the same paging contract{"truncated": true, "returned": 50, "total": 1280, "limit_bytes": 65536})Tests
truncated: truetotaland a working way to request the next pageNotes