Skip to content

fix(mcp): bound tool result size, stream-cap upstream reads, and report truncation explicitly - #489

Open
Jonathan-ai9 wants to merge 1 commit into
Divinity-Labs:mainfrom
Jonathan-ai9:fix/mcp-bounded-tool-results
Open

Jonathan-ai9 wants to merge 1 commit into
Divinity-Labs:mainfrom
Jonathan-ai9:fix/mcp-bounded-tool-results

Conversation

@Jonathan-ai9

Copy link
Copy Markdown
Contributor

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

  • Add a configurable max result size (env var / config, with a sensible default) applied to all tool results
  • Read upstream responses as a stream with a byte cap; abort the read once the cap is reached instead of buffering the full body
  • Check Content-Length up front and fail fast when it already exceeds the cap, without trusting it as the only guard
  • Replace JSON.stringify(data, null, 2) with compact JSON.stringify(data)
  • For collection results, return a bounded page plus total and a documented offset/limit (or cursor) parameter to request more; coordinated with L214 so both tools use the same paging contract
  • When a result is truncated, say so explicitly in the response, including how much was returned, the limit that applied, and how to get more (for example {"truncated": true, "returned": 50, "total": 1280, "limit_bytes": 65536})
  • Truncate on a structural boundary (whole items, not mid-JSON) so the output stays valid and parseable
  • Document the limit and paging parameters in the tool descriptions and README

Tests

  • Upstream body larger than the cap is not read in full (stream is cancelled; assert bytes consumed stays near the cap)
  • Response exceeding the output limit is truncated and flagged truncated: true
  • Truncated output is still valid JSON
  • Large collection returns one page, with total and a working way to request the next page
  • Response under the limit is returned unchanged and not flagged
  • Output contains no pretty-print indentation
  • Limit is configurable and the override is respected
  • Existing tool tests still pass

Notes

  • Behavior change: results larger than the default limit are now truncated, where previously they were returned in full. The limit is configurable for operators who need more.
  • Paging parameter names are shared with L214; if that PR lands first, this one should rebase onto its contract.

@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@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! 🚀

Learn more about application limits

@Miracle656

Copy link
Copy Markdown
Collaborator

Coordination note — ten PRs are editing one 179-line file

If 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 src/mcp/server.ts, which is 179 lines long. Every merge therefore conflicts everything else. The batch should have been sequenced, or scoped so the work genuinely separated — that was a mistake in how I wrote the issues, not in how you answered them.

Already merged:

Order I intend to take the rest in, so you can rebase once rather than repeatedly:

  1. feat: add configurable host allow-list to paid_call, refuse before any request #487 — paid_call host allow-list (paid_call will pay whatever URL the agent names #404). The gate that decides which hosts may be paid. This one goes next because everything else about spending is narrower than it.
  2. feat(mcp): bound cumulative spend, audit payments, serve over HTTP #491 / fix(mcp): bound cumulative daily paid-call spend, not just per call #492 — cumulative spend ceiling and payment audit (The spend cap is per call, so it is not a cap #405, Nothing records what the agent spent #406). These bound and record what happens to hosts the allow-list permits.
  3. fix: classify MCP paid-call errors by structured signals #480 — structural error classification (Errors are classified by searching the message text #410).
  4. fix(mcp): bound tool result size, stream-cap upstream reads, and report truncation explicitly #489 — bounded tool results (Tool results are unbounded #416).
  5. feat: add lens_get_price tool returning aggregated pair price with issuer-strict assets #493 / feat: add lens_list_pairs, lens_get_pools and lens_screen_pairs tools #482 / feat: declare the resources capability and expose pairs, venues, OpenAPI spec, and status #494 — the new Lens tools and the resources capability (An agent cannot ask Lens for a price #408, No tool exposes pairs, pools or the screener #409, The server advertises only tools, not resources #418).
  6. test(mcp): add end-to-end 402 payment loop test against a stub facilitator #490 — the end-to-end 402 loop test (No test drives a real 402 payment loop #423).

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: src/mcp/codes.ts is being extended by at least four of these, and #488 has already added a separate set of refusal reasons in src/net/ssrf.ts. We should end up with one vocabulary for "I refused to do that", not four. Whoever lands next, please look at what is already there and extend it rather than adding a parallel set — and say in your PR description what you added, so the next person can see it without reading the diff.

Nothing here reflects on the quality of the work. Several of these are among the best PRs this repo has had.

@Miracle656

Copy link
Copy Markdown
Collaborator

#487 has landed — the paid_call host allow-list. You are next in the MCP queue; please rebase on current main.

I resolved #487's conflicts myself because nine PRs were stuck behind it. Three things on main now that will shape your rebase:

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 config_, not config — renamed to stop it shadowing the imported config. Both gates initialise there: paidCallRules and bazaarFetchOptions.

3. src/mcp/ is exempt from the console.* guard, and must stay exempt. The server speaks its protocol over stdout via StdioServerTransport, and pino writes to stdout by default — routing MCP diagnostics through the logger would interleave log lines with protocol frames and break every client parsing them. Use console.error there. Everywhere else in the repo, use the logger; the guard will tell you.

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.
@Jonathan-ai9
Jonathan-ai9 force-pushed the fix/mcp-bounded-tool-results branch from adf58e3 to 25a7dbe Compare October 21, 2026 16:39
@Miracle656

Copy link
Copy Markdown
Collaborator

This has already landed. src/mcp/limits.ts is on main and its header names the issue:

/**
 * Size bounds for MCP tool results (#416).
 * …
 * Two independent ceilings:
 *   - the body read from upstream is capped while streaming, so an oversized
 *     response is never buffered in full (`readBodyCapped`);
 *   - the text handed to the agent is capped, and collections are additionally
 *     capped by item count and reported as a page with a total
 *     (`boundSerialized`).
 */

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 McpLimits resolved from the MCP_* env and overridable per server. I have closed #416. Leaving this PR open rather than closing it for you; close it whenever you have read this.

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:

silent truncation would make a partial answer look like a complete one

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.

@Miracle656 Miracle656 mentioned this pull request Oct 23, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tool results are unbounded

2 participants