Skip to content

fix(payments): stop double-settling on paid failures, disclose gateway max_tokens clamping (1.8.2)#32

Merged
VickyXAI merged 7 commits into
mainfrom
fix/max-tokens-clamp-disclosure
Jul 21, 2026
Merged

fix(payments): stop double-settling on paid failures, disclose gateway max_tokens clamping (1.8.2)#32
VickyXAI merged 7 commits into
mainfrom
fix/max-tokens-clamp-disclosure

Conversation

@VickyXAI

Copy link
Copy Markdown
Contributor

Closes #30.

Why 1.8.2 exists

1.8.1 is on PyPI reporting __version__ == "1.8.0". That artifact cannot be corrected in place, so the only repair is a version that supersedes it. This PR is that release, and it carries real fixes rather than only a number.

The money bug

_should_fallback treated a post-settlement failure as a transient one, so the model fallback chain advanced and signed a fresh payment per model. smart_chat on the premium complex tier is primary plus five fallbacks: six settlements, zero tokens. This is the CHARGED BUT REQUEST FAILED outcome already documented under 1.7.1.

The first attempt at this fix, in 7e2ac0b, tagged only httpx.TimeoutException and NetworkError. Three independent reviewers caught that the dominant post-settlement failure is a paid 5xx, which arrives as APIError(503) — exactly a status the fallback logic retries. Reproduced: a 402-then-503 chain across three models sent six signatures. ee651c8 tags every exception escaping the paid leg.

Over-tagging is the safe direction. Those handlers begin at an already-read 402 response and create_payment_payload is local EIP-712 signing with no network I/O, so the only exceptions taggable without a settlement are ones _should_fallback already refuses.

7afe20c extends the same guard to Solana, where _should_fallback_solana never consulted the tag and no paid leg set it.

The clamp disclosure

The gateway does not reject an over-ceiling max_tokens. It clamps to the model ceiling and quotes payment for the clamped value. Probed on the unpaid 402 leg, 2026-07-21:

model sent quoted gateway says
claude-opus-4.8 128000 $0.338090 128000
claude-opus-4.8 262144 $0.338090 128000
claude-opus-4.8 1000000 $0.338090 128000
zai/glm-5.2 1000000 $0.123139 262144
gpt-5.2 10^12 $0.190192 128000

The 402 disclosed the clamp in resource.description and the SDK read that string only to feed the signature, then discarded it. Callers now hear what they asked for and what they are paying for, before signing.

Hardened after review: the parse ran on server-controlled text immediately before signing, where a non-string value raised TypeError and aborted the call, and (\d[\d,]*) backtracked super-linearly (measured 2.78s at 16k digits; the bounded pattern is 0.0003s). It now stays silent on an ambiguous description rather than reporting a per-unit rate as the ceiling, and swallows its own failures.

Publish gate

publish.yml built and published with no test step. That is precisely how 1.8.1 shipped: the version guard test caught the drift on the push-triggered run, after the release was already cut. The build job now runs the suite first.

Also here

  • Removed a Raises: PaymentError: If budget is set and would be exceeded docstring. There is no budget parameter in the SDK and no client-side spend cap.
  • .gitattributes with * text=auto, normalizing the 17 mixed-ending files. fix(validation): stop capping max_tokens below what models actually serve #27 converted 3 of them as a side effect and turned a 51-line change into a 1045-line diff.

Verification

401 tests pass; black and ruff clean. 28 new tests, mutation-verified rather than assumed:

mutation result
revert the settled guard 5 fail
narrow it back to timeouts only (the incomplete fix) 1 fail
restore the ReDoS regex 1 fail
remove the Solana guard 4 fail

Solana tests use importorskip so the 3.9 job stays green.

Still open, not in this PR: there is no client-side spend cap anywhere. client.py computes cost_usd and signs it in the next statement, with no ceiling and no confirmation.

1bcMax added 7 commits July 21, 2026 00:26
…fter settlement

The comment justifying MAX_TOKENS_SANITY_LIMIT claimed the gateway "rejects
with that model's own number". It does not. Probed against the live 402 leg
2026-07-21 (unpaid quote leg only): opus-4.8 sent 262144 and 1000000 both
quote the 128000 price, and gpt-5.2 sent 1e12 returns a quote rather than a
400. The gateway silently clamps to the model ceiling and charges for the
clamped value, so there is no server-side rejection to fall back on.

The only disclosure is the 402's resource.description, which was read solely
to feed resource_description into the signature and then discarded — the one
string that would tell a caller "you asked for 500000, you are paying for
128000" was thrown away at the moment it was in hand. _warn_if_clamped now
surfaces it on every body-bearing payment path before the caller pays.

Separately: _should_fallback returned True for any timeout, including one
raised after the PAYMENT-SIGNATURE had gone out. The fallback chain then
signed a fresh payment per model, so smart_chat PREMIUM COMPLEX could settle
six times and return nothing — the "CHARGED BUT REQUEST FAILED" outcome the
CHANGELOG already documents. Errors raised past the settlement boundary are
now tagged and refused for fallback. Tagging via attribute rather than a new
exception type keeps callers catching httpx.TimeoutException working.

Also drops the Raises: docstring promising "PaymentError: If budget is set
and would be exceeded" — no budget parameter exists anywhere in the SDK.
The repository had no .gitattributes and mixed endings: 17 tracked files were
CRLF while the rest were LF. PR #27 converted 3 of them as a side effect of an
unrelated fix, which inflated that diff from 51 real lines to 1045 and buried
the behavioural change under formatting noise — `git diff` was ~95% churn and
`--ignore-cr-at-eol` was needed to review it at all. Converting only 3 left the
other 14 to regenerate the same problem on the next contributor's machine.

`* text=auto` plus `git add --renormalize .` converts the remaining 14 in one
formatting-only commit, so blame stays readable and endings stop depending on
who checked out the repo. Verified content-free: `git diff --ignore-cr-at-eol`
against the staged tree is empty.
1.8.1 shipped to PyPI with VERSION and __init__.py still reading 1.8.0, so the
installed package reports __version__ == '1.8.0' while PyPI says 1.8.1.
pyproject.toml was the only one of three locations I bumped.

tests/unit/test_version_consistency.py exists precisely to catch this and did
catch it — on CI, after the release had already been cut. I bumped, committed,
tagged and released without re-running the suite in between, so a known-failing
test never had a chance to stop the publish.

Fixing the artifact needs a new version; main alone doesn't repair a wheel
already on PyPI.
…n the clamp parse

The settlement guard shipped incomplete. _mark_settled was applied only to
httpx.TimeoutException and NetworkError, but the dominant post-settlement
failure is a paid 5xx, which surfaces as APIError(503) — precisely a status
_should_fallback treats as retriable. So the six-settlements-for-zero-tokens
path the previous commit claimed to close was still fully open, and the
CHANGELOG entry asserting otherwise was wrong.

Reproduced before the fix: a 402-then-503 chain across three models sent six
signatures. Every exception escaping the paid leg is now tagged. Over-tagging
is the safe direction: those handlers begin at an already-read 402 response and
create_payment_payload is local EIP-712 signing with no network I/O, so the only
exceptions taggable without a settlement are ones _should_fallback already
refuses.

_warn_if_clamped also had two ways to break the request it was diagnosing. It
runs on resource.description, a server-controlled string, immediately before
signing: a non-string value raised TypeError, and `(\d[\d,]*)` backtracked
super-linearly on a digit run (measured 2.78s at 16k digits). Now a bounded
pattern over a 512-char slice, silent when the description is ambiguous rather
than reporting a per-unit rate as the ceiling, and wrapped so nothing escapes.

Tests: 20 new, covering the tag classification, one-settlement-per-call for both
timeout and paid-5xx, that unpaid failures still walk the chain, and the parse.
Mutation-verified — reverting the guard fails 5, narrowing it back to timeouts
fails 1, restoring the old regex fails 1. Counts distinct signatures rather than
signed requests, since the paid leg replays one signature on 502/503.
… tests

The guard was Base-only. _should_fallback_solana never consulted the settled
tag and no Solana paid leg set it, so the double-payment path stayed fully open
on one of the two chains while the CHANGELOG read as though both were covered.
SPL USDC leaves the wallet on signing exactly like USDC on Base does.

Both Solana paid stream phases now tag anything that escapes, and
_should_fallback_solana refuses tagged exceptions ahead of its existing checks,
so the issue #6 permanent-reason guard is untouched. Tests mirror the Base ones
and assert both chains agree on what the tag means; mutation-verified by
removing the guard (4 fail). Guarded with importorskip so the 3.9 CI job, which
installs without the solana extra, stays green (see #19/#20).

publish.yml built and published on release with no test step. That is how 1.8.1
reached PyPI with VERSION and __init__.py still at 1.8.0: the guard test caught
it on the push-triggered run, after the release was cut, and PyPI does not allow
overwriting a published file. The build job now installs the dev+solana extras
and runs the suite before it builds.

Promotes the changelog's Unreleased section to 1.8.2 so the four version
declarations agree, and says plainly that 1.8.2 exists to supersede a wheel that
misreports its own version.
…doned paid streams

Review of #32 found three defects in the previous two commits.

`except Exception` was too wide. It tagged a paid-leg 402 — which means the
facilitator REJECTED the payment and the funds did not move — as
blockrun_payment_settled, the one case where the name is exactly backwards. It
also relabeled SDK bugs (AttributeError, KeyError, a PEP-479 RuntimeError) as
payment outcomes, and `from None` erased the __context__ that explained them:
x402.parse_payment_required deliberately raises an opaque "invalid format"
whose only diagnostic value is its cause.

Narrowed to `(httpx.HTTPError, APIError)`, which is provably exactly the
fallback-eligible set — TimeoutException and NetworkError are both HTTPError
subclasses, APIError is caught directly, and PaymentError is deliberately not
an APIError subclass — so nothing that could trigger a second settlement
escapes untagged, while rejections and bugs propagate as themselves. Re-raises
bare instead of `from None`, preserving traceback and context.

`async for chunk in self._astream_paid_phase(...)` did not close the inner
generator when the outer was closed, so an abandoned paid stream stranded the
`async with self._client.stream(...)` and its connection until GC finalization.
Extracting the paid phase introduced it; the sync path never had it because
`yield from` propagates close(). The same defect was already present at the
`chat_completion_stream` fallback boundary, so a caller that breaks mid-stream
stranded two generators. Both now aclose explicitly.

The ReDoS timings in the code comment were wrong — 8k/16k were cited as
1.07s/11.88s, never measured by the author. Re-measured on CPython 3.13:
4k 0.13s, 8k 0.49s, 16k 1.95s. Corrected in the comment, the CHANGELOG and the
test docstring so all three agree.

Tests: 9 new, and the first drafts of two were rewritten after mutation testing
showed they passed for the wrong reason — they asserted isolated shapes rather
than driving the client, and the async one could not distinguish a deterministic
close from CPython's asyncgen finalizer running at loop teardown. All 7
mutations now fail: removing either aclose, widening or narrowing the handlers,
removing either chain's guard, restoring the old regex.
… the publish gate

The CHANGELOG said the settlement guard covers "both Base and Solana", but on
Solana only the two streaming legs were tagged. The three non-stream paid legs
were unwrapped. Not exploitable today, because _should_fallback_solana is only
consulted from the streaming fallback loops and Solana non-stream chat exposes
no fallback_models — but the claim was wrong, and the hole would reopen
silently the day someone adds a fallback chain there.

publish.yml claimed to gate on "the same suite" as CI while running only pytest
on 3.11. Now runs black and ruff too, and the comment says plainly what it does
and does not cover: the 3.9 and 3.12 legs still only run on push, so
version-incompatible syntax is caught there, not here.

Also asserts the release tag matches VERSION before anything is built. Cutting
v1.8.3 from a 1.8.2 tree passed all 407 tests; PyPI's duplicate-version check
was the only thing standing between that and a wrong publish, and it only fires
after the release is cut. Same family as the 1.8.1 incident, closed at the same
place.
@VickyXAI
VickyXAI merged commit 6f45d88 into main Jul 21, 2026
3 checks passed
@VickyXAI
VickyXAI deleted the fix/max-tokens-clamp-disclosure branch July 21, 2026 14:56
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.

Release integrity: 1.8.1 on PyPI reports __version__ 1.8.0, and the v1.8.2 tag points at a commit on no branch

1 participant