Skip to content

fix: upstream provider funding refusal must not read as the customer owing money (#1411) - #1439

Merged
sakibsadmanshajib merged 1 commit into
mainfrom
fix/1411-upstream-funds-exhaustion
Aug 29, 2026
Merged

sakibsadmanshajib merged 1 commit into
mainfrom
fix/1411-upstream-funds-exhaustion

Conversation

@sakibsadmanshajib

Copy link
Copy Markdown
Owner

Summary

Issue #1411 measured the OpenRouter account funding every paid alias at
about $1.59 of a $10 purchase, with no fallback provider configured. That
means an upstream 402 Payment Required (OpenRouter's own account out of
funds) is an imminent, previously untested failure mode. This PR
establishes what actually happens on that path and fixes the one real
defect found.

Investigation (with evidence, not just reasoning from the code)

  1. Real upstream shape, confirmed live against OpenRouter's own docs
    (https://openrouter.ai/docs/api_reference/errors-and-debugging.md,
    fetched 2026-08-29): { error: { code: number, message: string, metadata?: {...} } }, HTTP status equal to error.code. 402 maps to
    error_type: payment_required.
  2. Hold release was already correct. Every dispatch site
    (inference/orchestrator.go, inference/stream.go,
    inference/stream_responses.go, chat/dispatch.go,
    images/handler.go, audio/handler.go) already releases the
    reservation on any non-2xx upstream response before writing the
    customer error, and accounting.Service.releaseLocked is idempotent
    and reachable on a background context. No charge is ever attempted on
    this path. Now pinned by new integration tests (see below).
  3. Message sanitization was already correct. The provider-blind
    allowlist (PR fix: stop upstream billing and quota text reaching the chat surface #1303) already default-denies money vocabulary
    ("insufficient", "credits", "balance", ...) for any status outside
    400/404/413/422, so a 402 always collapsed to a generic fallback
    message. Existing fixture in provider_blind_allowlist_test.go already
    covered this.
  4. The real defect: the HTTP status code itself. Nothing remapped
    httpStatus. WriteProviderBlindUpstreamError forwarded a 402 that is
    about the UPSTREAM's own account balance straight through to the Hive
    customer, whose own reservation had already succeeded (they have
    credit; the upstream vendor relationship does not). A literal 402
    tells the customer the opposite: that they must pay. This is the fix
    in this PR.
  5. RAG has no reservation at all, a pre-existing, already-tracked gap
    (issue RAG chat and agent tasks serve inference without reserving credits, so insufficient credits cannot fire #669), unrelated to and unchanged by this PR. For RAG, "is the
    hold released" is moot because there is no hold.
  6. No pre-zero balance warning exists, confirmed by OpenRouter account is at $1.59 of $10 purchased credit, and nothing warns before it hits zero #1411's own body.
    Scoped as a separate, future change (monitoring/ops, no shared code
    path with this fix).

Fix

apps/edge-api/internal/errors/provider_blind.go,
WriteProviderBlindUpstreamError: map upstream 402 Payment Required to
the same 503 Service Unavailable / upstream_unavailable verdict a
503/504 already gets, reusing the existing "temporarily unavailable"
message path instead of inventing a new one. The original upstream status
(402) stays in the operator log for diagnosis; only the customer-facing
status changes.

One call site, so every caller already routing through it inherits the
fix: chat session dispatch (Open WebUI), /v1/chat/completions and
/v1/responses (sync and streaming), /v1/messages (delegates
in-process to the chat-completions chain), images, audio, and RAG's error
path.

Not touched, deliberately: images/handler.go and audio/handler.go's
own direct 402 writes for HIVE's own reservation refusal (a different,
pre-existing, already-tracked inconsistency, issue #633). That is a
different condition, the caller's own Hive balance, and must keep
meaning "you must pay."

Money-path bound

No token class billed changes, and no charge amount changes in either
direction. This only changes an HTTP status code and message on a path
that already released the hold and never finalized a charge, both before
and after this change. math/big, append-only ledger, and the one-credit
floor are all untouched.

Tests

  • TestWriteProviderBlindUpstreamErrorRemaps402ToUpstreamUnavailable
    (unit, errors package): pins the status remap, the sanitized message,
    and that the operator log keeps the real 402 and raw body.
  • TestExecuteSync_UpstreamPaymentRequired_ReleasesHoldAndDoesNotBill and
    TestExecuteStreaming_UpstreamPaymentRequired_ReleasesHoldAndDoesNotBill
    (integration, inference package): drive the real
    Orchestrator.executeSync / executeStreaming lifecycle against an
    in-process httptest.Server serving OpenRouter's documented 402 shape,
    asserting the customer-visible status/body, that release was called
    with reason upstream_error, and that finalize was never called (never
    billed).

All three are simulated: an in-process httptest.Server stub, never
a call to the real OpenRouter API. Draining the real wallet to observe
this would cause the exact outage this PR exists to prevent. The shape of
the simulated body is real (confirmed against OpenRouter's own docs
above); only the transport is faked.

Mutation check: temporarily disabled the 402 case in
WriteProviderBlindUpstreamError (renamed the switch case to an unused
status so it fell through to the default branch). All three new tests
went red with the exact 402-not-503 failure. Restored the fix; all three
went green again. Full edge-api suite (go test ./apps/edge-api/...)
passes, 0 regressions across all 20+ packages including rag, chat,
images, audio.

Buglog entry

{"date":"2026-08-29","error_message":"OpenRouter 402 Payment Required (own account out of funds) forwarded verbatim as HTTP 402 to the Hive customer","root_cause":"WriteProviderBlindUpstreamError only sanitized the MESSAGE body for an upstream refusal; it never remapped the HTTP STATUS itself, so a provider funding refusal (about Hive's own OpenRouter balance) was indistinguishable, at the status-code level, from a caller quota refusal (about the customer's own Hive balance)","fix":"remap upstream 402 to 503/upstream_unavailable in WriteProviderBlindUpstreamError, reusing the existing 503/504 temporarily-unavailable message path; original status kept in the operator log","tags":["billing","provider-blind","accounting","reservations","issue-1411"]}

Related

Closes/addresses #1411 (the specific customer-visible refusal shape).
Touches the same file family as #744 (provider hostname leak, already
fixed, this PR does not regress it) and #1327 (label-list boundary,
unrelated code path, not touched). Does not touch #600 (orphan-hold
reaper, not needed here since release already fires synchronously on
this path) or #633 (Hive's own quota-refusal status divergence, a
separate condition).

Plan and full investigation write-up: Obsidian vault,
hive/plan-2026-08-29-upstream-funds-exhaustion.md.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WyEwUxZCArdn1ZUDkTvuQ1

…tomer owes money (#1411)

OpenRouter's own account balance is draining and every paid alias has no
fallback provider, so a 402 Payment Required from the upstream is an
imminent, untested failure mode. Investigation found the reservation
release and the message sanitization for this shape were already correct
(release fires on every non-2xx upstream response before the customer
error is written, and the provider-blind allowlist already collapses
money vocabulary), but nothing remapped the HTTP status itself:
WriteProviderBlindUpstreamError forwarded a 402 about the UPSTREAM's own
funding straight through to the Hive customer, who already has credit and
whose reservation already succeeded. A literal 402 tells them the
opposite: that they must pay.

Map upstream 402 to the same 503/upstream_unavailable verdict a 503 or
504 already gets, reusing the existing "temporarily unavailable" message
path rather than inventing a new one. The original upstream status stays
in the operator log for diagnosis. One call site, so every caller that
already routes through it inherits the fix: chat session dispatch,
/v1/chat/completions and /v1/responses (sync and streaming),
/v1/messages, images, audio, and RAG's error path.

Two new tests drive the real refusal path: a unit test on the sanitizer
directly, and two integration tests that run the real
Orchestrator.executeSync / executeStreaming lifecycle against OpenRouter's
documented error envelope (confirmed live against
openrouter.ai/docs/api_reference/errors-and-debugging.md) served by an
in-process httptest stub, asserting the customer-visible status and body,
that the reservation is released with reason upstream_error, and that it
is never finalized (never billed). A mutation check (temporarily
disabling the 402 case) turned all three tests red, then the fix was
restored and they went green again.

Buglog entry:
{"date":"2026-08-29","error_message":"OpenRouter 402 Payment Required (own account out of funds) forwarded verbatim as HTTP 402 to the Hive customer","root_cause":"WriteProviderBlindUpstreamError only sanitized the MESSAGE body for an upstream refusal; it never remapped the HTTP STATUS itself, so a provider funding refusal (about Hive's own OpenRouter balance) was indistinguishable, at the status-code level, from a caller quota refusal (about the customer's own Hive balance)","fix":"remap upstream 402 to 503/upstream_unavailable in WriteProviderBlindUpstreamError, reusing the existing 503/504 temporarily-unavailable message path; original status kept in the operator log","tags":["billing","provider-blind","accounting","reservations","issue-1411"]}

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WyEwUxZCArdn1ZUDkTvuQ1
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 49 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 29f8c083-88c3-4908-afff-d988c35effb1

📥 Commits

Reviewing files that changed from the base of the PR and between 908e48a and 48d34cf.

📒 Files selected for processing (3)
  • apps/edge-api/internal/errors/provider_blind.go
  • apps/edge-api/internal/errors/provider_blind_payment_required_test.go
  • apps/edge-api/internal/inference/upstream_payment_required_test.go

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.

@sakibsadmanshajib

Copy link
Copy Markdown
Owner Author

Adversarial review pipeline (per .claude/rules/orchestrator.md)

CodeRabbit CLI: SKIPPED. Rate limited (Review limit reached... You've used all 3 included reviews, 27 minute reset window at the time this PR was opened). Not counted as a clean pass, flagged here explicitly per the standing fallback rule.

Plain adversarial pass: performed directly on the diff (no defects found). Checked specifically for:

  • Whether the responseStatus remap leaks into any OTHER status branch (429/503/504/400/422): confirmed no, every other case leaves responseStatus == httpStatus, verified both by re-reading the switch and by the full existing test suite staying green (no case previously pinned on those statuses changed behavior).
  • Whether the operator log loses the real upstream status after the remap: confirmed no, logProviderBlindUpstreamError is called with the original httpStatus (402), not responseStatus (503); pinned by TestWriteProviderBlindUpstreamErrorRemaps402ToUpstreamUnavailable's log assertions.
  • Whether /v1/messages (Anthropic-compatible surface) actually inherits the fix or has its own status handling: traced anthropic/translate_writer.go → reshapeToAnthropicError → writeAnthropicError(w, status, ...), confirmed status passes through unchanged from the recorded WriteProviderBlindUpstreamError call, so it does inherit.
  • Whether any frontend/SDK code could be relying on the old (402, api_error, upstream_error) triple from this specific call site: grepped for insufficient_quota and confirmed the pre-existing convention for HIVE's own quota refusals (a different condition, images/audio's direct 402 writes, reservation_guard.go's 429) is untouched and uses different code values than this path did before or after, so no collision.

Security review (mandatory, money + input-adjacent path): performed directly. No SQL, no command exec, no file paths, no new regex, no new shared mutable state, no secrets. The change touches only which HTTP status/message the gateway writes back for an already-classified upstream failure; it introduces no new trust boundary and does not change what is billed, reserved, or released, only pinned tighter by the new integration tests (release still fires with reason upstream_error, finalize is still never called).

Static analysis: go vet ./apps/edge-api/internal/errors/... ./apps/edge-api/internal/inference/... clean. staticcheck not present in the toolchain image (NOSTATICCHECK), noted rather than silently skipped.

Full suite: go test ./apps/edge-api/... green across all packages (errors, inference, chat, rag, images, audio, anthropic, auth, authz, ...), 0 regressions.

Mutation test: temporarily disabled the 402 case (renamed the switch case to an unreachable status), all three new tests went red with the exact "402, want 503" failure, restored the fix, all three green again. Full command output kept in the PR description.

No inline threads opened: no defect was found to attach one to. If a later stream (CodeRabbit, once the rate limit resets, or a human reviewer) finds something, address it as a normal follow-up commit on this branch.

@sakibsadmanshajib
sakibsadmanshajib merged commit 5f8298c into main Aug 29, 2026
26 checks passed
@sakibsadmanshajib
sakibsadmanshajib deleted the fix/1411-upstream-funds-exhaustion branch August 29, 2026 17:28
sakibsadmanshajib added a commit that referenced this pull request Aug 29, 2026
## Summary

This is the batched buglog follow-up for the pull requests merged to
`main` on 2026-08-29. Its diff is `.wolf/buglog.jsonl` and nothing else.

Per `.claude/rules/openwolf.md`, every fixed bug, error, failed test or
failed build must be logged, but the line may never be appended on a fix
branch. `merge=union` in `.gitattributes` resolves concurrent appends
locally and is ignored by GitHub's server side merge, so two branches
that both appended land in hard conflict there. An unmergeable pull
request gets no `refs/pull/N/merge`, no `pull_request` run and therefore
zero checks, and the required status gate then blocks the merge for a
reason the page never states (issue #873). Each fix accordingly carried
its entry in its own pull request body, and this pull request copies
them onto `main` in one batch, which the protocol explicitly prefers
over one pull request per entry.

## Scope examined

Fifty nine pull requests merged to `main` on 2026-08-29. Forty eight of
them carried at least one entry, for eighty two entries in total. Thirty
two of those were already on `main` and are skipped, leaving fifty
appended here from thirty four pull requests.

The largest block of skips comes from #1342, the equivalent batch for
the 2026-08-28 merges, which merged earlier the same day and already
landed thirty six entries covering #1257, #1268, #1276, #1277, #1287,
#1292, #1293, #1294, #1296, #1301, #1303, #1305, #1313, #1335 and #1337.

## What landed

Fifty entries appended, one JSON object per line, append only. The 232
pre-existing lines are byte identical to `origin/main` (verified by
hashing the first 232 lines of the result against the base file). Every
line in the resulting file parses as JSON and carries `error_message`,
`root_cause`, `fix` and `tags`.

| Source | Entries |
|---|---|
| #1083 | 2 |
| #1277 | 1 |
| #1278 | 1 |
| #1298 | 1 |
| #1334 | 1 |
| #1336 | 3 |
| #1343 | 1 |
| #1346 | 1 |
| #1351 | 1 |
| #1365 | 2 |
| #1368 | 1 |
| #1369 | 1 |
| #1371 | 3 |
| #1375 | 3 |
| #1376 | 1 |
| #1378 | 1 |
| #1379 | 2 |
| #1388 | 5 |
| #1389 | 3 |
| #1390 | 2 |
| #1393 | 1 |
| #1394 | 1 |
| #1410 | 1 |
| #1417 | 1 |
| #1421 | 1 |
| #1423 | 1 |
| #1424 | 1 |
| #1426 | 1 |
| #1429 | 1 |
| #1431 | 1 |
| #1433 | 1 |
| #1434 | 1 |
| #1436 | 1 |
| #1439 | 1 |

Entries are copied verbatim from their source pull request bodies.
Nothing was rewritten, no field was invented, and no field was added. No
JSON needed repair: all eighty two extracted entries parsed on the first
attempt and all four required fields were present on every one.

## Merged pull requests that carried no entry

Eleven of the fifty nine. Recorded here because the gap is itself the
useful signal.

| Pull request | Title | Assessment |
|---|---|---|
| #1013 | chore(deps): bump the go-minor-patch group across 1 directory
with 4 updates | Dependabot bump, no defect fixed, no entry expected |
| #1015 | chore(deps): bump the go-minor-patch group across 1 directory
with 6 updates | Dependabot bump, no entry expected |
| #1016 | chore(deps): bump golang from 1.26-alpine to 1.27-alpine in
/deploy/docker | Dependabot bump, no entry expected |
| #1218 | chore(deps): bump postcss from 8.5.19 to 8.5.26 in
/apps/desktop | Dependabot bump, no entry expected |
| #1219 | chore(deps): bump golang.org/x/crypto from 0.41.0 to 0.52.0 in
/apps/control-plane | Dependabot bump, no entry expected |
| #1342 | chore: batch buglog entries for the 2026-08-28 merges | The
previous batch pull request itself, correctly carries no entry of its
own |
| #1364 | chore: remove four dead skills and record the patterns that
cost time | Protocol gap. The body records patterns that cost time,
which is the shape of a buglog entry, but none was written as one |
| #1383 | test: retire stale expected-failure markers, restore the ones
that are true (#1381, #1382, #1324) | Protocol gap. Stale `it.fails`
markers reading as red is a real defect that was fixed here and should
have carried an entry |
| #1384 | docs: correct D-047, hive-auto reverted to variable pricing
(D-059) | Decision ledger correction, arguably a documentation defect,
no entry written |
| #1387 | chore(deps): bump next from 15.5.23 to 16.3.3 in
/apps/agent-console | Dependabot bump, no entry expected |
| #1398 | docs: rescue the 2026-08-25 parity captures and add the
2026-08-29 QA matrix evidence | Documentation and evidence rescue, no
entry written |

Six of the eleven are Dependabot bumps and one is the previous batch, so
the genuine protocol gaps are #1364, #1383, #1384 and #1398. Of those,
#1383 is the one worth a follow-up: it fixed a real defect class (a
stale expected-failure marker reads as a red "Expect test to fail" and
gets dismissed as pre-existing) and left no record.

## Entries skipped as already present

Thirty two. Thirty of them matched an entry already on `main` on
`error_message`, `id` or `fix`. Two more from #1278 are semantic
duplicates that an exact match would have missed, and were skipped after
reading the landed entries they duplicate:

- #1278's `streaming content_block_start omits text field` entry is
covered by the consolidated
`bug-2026-08-28-anthropic-sdk-wire-conformance` entry landed from #1296,
whose root cause names the same `omitempty` on
`StreamContentBlock.Text`.
- #1278's `GET /v1/models leaked an upstream provider name` entry is
covered by `BUG-1284`, landed from #1300, which names the same
`public.model_aliases.summary` publication path.

#1278's third entry, on `top_k` forwarding producing a 400, is not
covered anywhere on `main` and is appended here. #1342 recorded #1278 as
fully "merged into #1296", which was accurate for two of its three
entries.

## Note on entry quality

One appended entry is thin: #1277's parity re-score record carries
`error_message` of `n/a` and a root cause of "console had no
privacy/data-policy surface at all". It is a parity gap record rather
than a defect record. It is included exactly as written rather than
embellished, per the protocol's preference for the author's own words.

## Test plan

- [x] Branch cut fresh from `origin/main`, diff is `.wolf/buglog.jsonl`
and nothing else
- [x] First 232 lines byte identical to the base file (md5 match)
- [x] All 282 resulting lines parse as JSON and carry `error_message`,
`root_cause`, `fix` and `tags`
- [x] No `.wolf/` telemetry (`anatomy.md`, `memory.md`,
`token-ledger.json`, `hooks/_session.json`, `buglog.json`) in the commit
- [ ] The six required checks report green via the inert path allowlist
in `.github/workflows/ci.yml`

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant