Skip to content

feat(mcp): bound cumulative spend, audit payments, serve over HTTP - #491

Closed
AGWAM001 wants to merge 2 commits into
Divinity-Labs:mainfrom
AGWAM001:mcp-spend-controls-and-tool-coverage
Closed

AGWAM001 wants to merge 2 commits into
Divinity-Labs:mainfrom
AGWAM001:mcp-spend-controls-and-tool-coverage

Conversation

@AGWAM001

@AGWAM001 AGWAM001 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #406, #419 and #421. Adds cumulative spend controls, a payment audit trail and an HTTP transport to the MCP server.

  • Test coverage (Only the error paths of one tool are tested #421) — bazaar_search (with and without a query, empty results in both shapes, non-2xx, malformed body, transport failure), ListTools (advertised names, result parsed with the SDK's own ListToolsResultSchema, every tool validated with ToolSchema, inputSchema.type === "object", each required entry declared in properties) and the unknown-tool path. The four pre-existing tests are untouched and still pass.
  • Spend ceiling — maxPrice bounds one mistake, not a day of retries, and paid_call signs with a real mainnet key. A cumulative USD ceiling now reserves each call's maximum before the payment runs and settles it afterwards.
  • Audit ledger (Nothing records what the agent spent #406) — every attempt is recorded, and a paid call is refused rather than spent when a durable ledger was configured but cannot be written.
  • Streamable HTTP — refuses to start without a bearer token, since this transport exposes paid_call to the network.

Related issue

Closes #406 — the payment audit ledger (nothing records what the agent spent).
Closes #419 — the HTTP transport.
Closes #421 — the tool test coverage.

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Checklist

  • I have read CONTRIBUTING.md
  • npx tsc --noEmit passes
  • npm run build passes — not verified here, so left unticked
  • I added / updated tests where relevant
  • I updated docs where relevant (README.md, .env.example, three changesets)

Test evidence

  • npx vitest run tests/mcp.test.ts — 40 passed
  • npx vitest run tests/mcp-http.test.ts — 22 passed
  • Full suite — 722 passed, 1 skipped
  • npx tsc --noEmit — clean

tests/aggregator.property.test.ts fails on this machine with "Failed to resolve entry for package fast-check", a devDependency resolution problem in this install. That file imports only src/aggregator/bestRoute and src/db, so this diff cannot account for it.

Notes for reviewers

Three new tests deliberately characterize handlePaidCallError's lowercased-substring matching rather than endorse it:

  1. No client registered for scheme: exact and network: stellar:pubnet — local x402 misconfiguration, yet reported as ERR_SERVICE_UNREACHABLE because the message contains "network".
  2. A Horizon-shaped error carrying extras.result_codes.operations: ["op_no_trust"] — reported as ERR_INTERNAL_FAILURE because only e.message is read.
  3. A 200 response with an unparseable body — reported as ERR_SERVICE_UNREACHABLE, conflating a transport failure with a bad payload.

Making the matcher read structured fields would fail those tests on purpose; that is what #421 means by plain strings hiding the fragility.

Upstream error fixtures reuse messages the published packages really throw (Failed to create payment payload: ..., Invalid Stellar asset address: ..., undici's TypeError: fetch failed with an ECONNREFUSED cause) instead of invented text. No key material appears anywhere; hosts are bazaar.example and the TEST-NET address 203.0.113.10.

The #421 tests and the spend-control feature are in one commit because tests/mcp.test.ts now imports src/mcp/budget.ts and src/mcp/ledger.ts. Say so if you would rather they land separately.

@drips-wave

drips-wave Bot commented Oct 17, 2026

Copy link
Copy Markdown

@AGWAM001 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

paid_call signs with a real mainnet key, and maxPrice only bounds one mistake —
a caller in a retry loop could spend without limit. Add a cumulative USD ceiling
that reserves each call's maximum before the payment runs and settles it after,
record every attempt in an audit ledger, and refuse to spend at all when a
durable ledger was asked for and cannot be written. The same tools are also
served over Streamable HTTP, which refuses to start without a bearer token since
it exposes paid_call to the network.

Cover Divinity-Labs#421: bazaar_search, ListTools and the unknown-tool path had no tests, and
the simulated upstream errors were plain strings that hid how handlePaidCallError
classifies on lowercased message substrings. Fixtures now use the shapes that
@x402/fetch, @x402/core and @x402/stellar really throw, including one Horizon
error whose op_no_trust signal is invisible to that matcher.
@Miracle656

Copy link
Copy Markdown
Collaborator

Needs a rebase — #488 (the SSRF guard for bazaar_search) merged an hour ago and touches src/mcp/server.ts and src/mcp/codes.ts, so this conflicts now.

Two things worth knowing before you rebase, because three PRs are converging on the same file:

Push the rebase and I will review properly. The spend ceiling is the half of #404 that actually bounds the loss — an allow-list stops the money going somewhere strange, a ceiling stops it going somewhere allowed ten thousand times.

@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.

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.

Only the error paths of one tool are tested There is no HTTP transport, only stdio Nothing records what the agent spent

2 participants