Skip to content

fix(cloud): refund MCP proxy upfront debit on every post-debit failure, not just non-ok HTTP (#11637) - #11652

Merged
lalalune merged 2 commits into
developfrom
fix/11637-mcp-proxy-refund-all-failures
Jul 2, 2026
Merged

fix(cloud): refund MCP proxy upfront debit on every post-debit failure, not just non-ok HTTP (#11637)#11652
lalalune merged 2 commits into
developfrom
fix/11637-mcp-proxy-refund-all-failures

Conversation

@NubsCarson

Copy link
Copy Markdown
Member

Closes #11637. [cloud-money]

Bug (MED, money — user over-charged)

mcp/proxy/[mcpId]/route.ts debits the caller upfront (reserveAndDeductCredits) but only refunded on a non-ok HTTP status. Every other post-debit failure kept the money → a live external MCP that's momentarily down silently over-charges the org:

  • unsafe/blocked external endpoint → 400
  • container has no load balancer → 503
  • endpoint misconfigured → 500
  • upstream unreachable / DNS / connection-refused → 502

Fix

A refundPrecharge(reason, status?) helper right after the debit, called on all four early-return failure branches; the non-ok HTTP branch now routes through the same helper. Refund = the exact upfront debit amount.

Proof (red-before / green-after)

New __tests__/mcp-proxy-refund.test.ts drives the real route with mocked deps:

  • bun test __tests__/mcp-proxy-refund.test.ts5/5 green (refund fires on 502/400/503/non-ok; NOT on success).
  • Red without the fix: stashing the route → the 3 new failure-path tests fail (refund never called). Confirmed.
  • typecheck + biome clean.

Money path — do not self-merge.

…ilure, not just non-ok HTTP (#11637)

mcp/proxy/[mcpId] debits the caller upfront (reserveAndDeductCredits) but
only refunded when the upstream returned a non-ok HTTP status. Every other
post-debit failure kept the money = a silent over-charge:
  - unsafe/blocked external endpoint → 400
  - container has no load balancer   → 503
  - endpoint misconfigured           → 500
  - upstream unreachable/DNS/refused  → 502

Add a `refundPrecharge(reason, status?)` helper right after the debit and
call it on all four early-return failure branches; the non-ok HTTP branch now
routes through the same helper. Refund amount = the exact upfront debit.

Test (__tests__/mcp-proxy-refund.test.ts): drives the real route with mocked
deps and asserts refundCredits fires on unreachable(502)/unsafe(400)/
container(503)/non-ok, and NOT on success. Red-before/green-after proven by
stashing the route: the three new failure-path cases fail (refund never
called) without the fix. typecheck + biome clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de445ba9-52ec-46aa-9398-beeb75ba0eeb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/11637-mcp-proxy-refund-all-failures

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@lalalune

lalalune commented Jul 2, 2026

Copy link
Copy Markdown
Member

Reviewed #11652 against #11637 and pushed one follow-up in f8f3c63fa9.

Finding fixed:

  • The PR refunded unsafe endpoint, container unavailable, misconfigured endpoint, fetch failure, and non-ok upstream status, but parseJsonBody(c.req.raw) still ran after the upfront debit and could throw on invalid JSON before any refund. That was another post-debit/non-delivered failure. I added an invalid_json refund branch that returns 400 and a regression test for it.
  • Added .github/issue-evidence/11637-mcp-proxy-refund.md so the money-path evidence is durable in the PR.

Fresh verification in /home/shaw/eliza-worktrees/pr-11652-mcp-refund:

bun test packages/cloud/api/__tests__/mcp-proxy-refund.test.ts
=> 6 pass, 0 fail, 14 expect() calls

bunx @biomejs/biome check .github/issue-evidence/11637-mcp-proxy-refund.md packages/cloud/api/mcp/proxy/[mcpId]/route.ts packages/cloud/api/__tests__/mcp-proxy-refund.test.ts --files-ignore-unknown=true
=> clean

git diff --check origin/develop...HEAD
=> clean

bun run --cwd packages/cloud/api typecheck
=> passed after generating ignored i18n prerequisites in the isolated worktree

Manual route review after the follow-up: every post-debit branch before upstream delivery now calls refundPrecharge (unsafe_endpoint, container_unavailable, endpoint_misconfigured, invalid_json, upstream_unreachable), and non-ok upstream responses still refund with status metadata. Successful responses still record usage without a second deduction and do not refund.

@lalalune lalalune left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after review and follow-up f8f3c63fa9. The remaining post-debit invalid-JSON branch now refunds, and the route-level suite covers all failure branches plus success/no-refund.

@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Claude encountered an error —— View job


I'll analyze this and get back to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

money: MCP metered proxy over-charges — refunds only on non-ok HTTP, keeps the debit on unreachable/unsafe/500/503 upstream failures

3 participants