Repository navigation
fix: enforce max_tokens as a billing bound and refuse n other than 1 - #1305
Conversation
Issue #1283. Two request parameters were accepted and then silently ignored on the live gateway, both confirmed against production with the OpenAI SDK v7.8.0. max_tokens was not enforced. A request carrying max_tokens: 8 on hive-free came back with 1650 completion tokens, all of them billed, roughly 200 times the ceiling the caller set to bound their own spend. Root cause: applyReasoningHeadroom, shipped by PR #1225 for issue #1171, inflated every completion limit field the caller had set by the pool's reasoning reserve (4096 on three of the four hive-free members) before dispatch. Its premise was that hidden reasoning would burn the reserve while visible content survived inside the caller's own budget. Nothing in an OpenAI-compatible request tells an upstream that part of a ceiling is earmarked for hidden reasoning, so the model spends the inflated ceiling on whichever it emits first. Because the rewrite deliberately skipped a ceiling the caller had NOT set, its entire effect was overriding ceilings callers HAD set. The fix enforces at both boundaries. Request boundary: the inflation is removed, so the caller's ceiling reaches the provider unchanged. This is also what OpenAI specifies for a reasoning model, where reasoning tokens count against the ceiling. The reasoning_reserve_tokens column and the zero-content guard, the other half of #1171, both stay. Settlement boundary: whatever the provider reports, the metered completion count is capped at the caller's own ceiling before anything reads it, so the ledger charge, the usage rollup and the usage block the caller receives are the same number by construction. The bound also covers the two branches that charge the reservation hold rather than a measured quantity, the sync zero-content capture and the streaming missing-usage capture, since a flat authorization floor charged against a request capped at 8 completion tokens breaches the same invariant far harder than the overrun that reported it. Those captures still charge and still never bill zero, so the fail-closed property of D-034 and D-048 is untouched. Both boundaries, not one: the money guarantee must not depend on every upstream honouring a parameter we merely forwarded, and a bound applied to the charge alone would leave the caller reading a completion count they were never billed for. A variable-price alias is a documented carve-out, since it settles on the cost the upstream reported rather than a token count times a catalog rate. n other than 1 is now refused with 400 unsupported_parameter and param n, on both the chat and legacy completion surfaces, rather than accepted and quietly answered with a single choice. No route in this catalog can generate more than one choice per request, and honouring it would multiply generated tokens against a single per-request ceiling. Arithmetic is unchanged and still runs through metering.ChargeCredits, so every figure stays math/big. No provider identity reaches the customer in anything touched here.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
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. Comment |
sakibsadmanshajib
left a comment
There was a problem hiding this comment.
Independent security and money-path review (stage 6, mandatory)
The prior reviewer on this PR was lost to a session limit before posting, so this is the first independent pass. Streams run: adversarial money-path review (this one), security-reviewer domain pass, CodeRabbit CLI. /codex:adversarial-review is SKIPPED, stated explicitly rather than counted as a pass: the Codex CLI reports You've hit your usage limit ... try again at Sep 10th, 2026, so it produced no opinion at all. Note also that the CodeRabbit GitHub App check on this PR reports pass with the annotation Review rate limited, which is likewise not a clean pass; the CodeRabbit CLI run below is the one that actually executed.
What I verified rather than took on trust
The deletion is justified. I read the removed applyReasoningHeadroom at the base commit. Its field loop is raw, present := decoded[field]; if !present { continue }, so a caller who set no ceiling was never inflated. One hundred percent of its effect was therefore raising ceilings callers had set. The author's central claim holds and deletion over repair is the right call.
Mutations re-run independently, not read off the table. Four of the five, each applied to a clean checkout of a2f45392a in an isolated worktree and run in the repo Docker toolchain:
| Mutation | Observed |
|---|---|
B, remove both clampUsageToCeiling call sites |
RED. Sync: finalize output_tokens = 1650, want 8, actual_credits = 6620, want 52. Streaming: same figures plus the forwarded usage frame still carrying "completion_tokens":1650 |
C, sync zero-content capture back to reservation.Held() |
RED. charged 10000 credits against a ceiling worth 108, the exact message the PR body quotes |
D, unsupportedChoiceCount returns false |
RED, all three subtests |
E, streaming missing-usage capture back to reservation.Held() |
RED. charged 10000 credits against a ceiling worth 32 |
The suite is green unmutated: go vet and go test -short clean across ./apps/edge-api/... and ./apps/control-plane/.... gofmt -l lists chat_completions.go and completions.go, but the diff is pre-existing struct-literal alignment on NeedStreaming and NeedReasoning lines this PR never touched, exactly as the PR body says.
Deleted and amended tests. The three removed tests all pinned the deleted behaviour and their removal is correct; TestExecuteSync_CallerCeilingReachesProviderUnchanged is the inverse of the strongest one. The amendment to TestExecuteSync_ZeroContentLength_Twice_CapturesHold is honest rather than a weakening: dropping max_tokens:200 from its body is required for the test to keep asserting a full-hold capture, because a ceiling of 200 now bounds that capture, and the with-ceiling half is pinned by a separate new test. Both halves are covered, one per test.
Nothing bills zero. capCaptureAtCeiling only calls CreditsForTokens with ceiling > 0, so the nonzero-quantity branch always applies and the one-credit floor at pricing.go:243 always fires. D-034 and D-048 fail-closed are intact, and the new tests assert credits < 1 is a failure.
Arithmetic. No float64 anywhere in the new production code. The only new arithmetic is int64 token addition; every credit figure still routes through metering.ChargeCredits.
No blanket under-billing. TestExecuteSync_NoCeilingLeavesUsageAlone is a real negative control and I confirmed the clamp is a no-op at ceiling <= 0. But see the two specific under-billing holes below, which that control does not cover.
Verdict
The invariant holds on the four branches the tests pin, and the two-boundary design is right. It does not hold on two further branches, both reproduced empirically, and one of them is a caller-triggerable revenue bypass. Findings are inline. My recommendation is do not merge as is: F1 and F2 are small, contained fixes in files this PR already owns, and shipping the contract sentence in support-matrix.json while those two branches exist publishes a guarantee the code does not keep.
Nothing here is a customer overcharge regression, so this is not an emergency; F1 costs Hive money, F2 overcharges only on a branch that needs a misbehaving upstream.
…stimate Answers the money-path review on this pull request. Five findings, each with its reproduction kept as a regression guard in completion_ceiling_review_test.go. Finding 1, caller-triggerable revenue bypass. The ceiling was read as the smaller of max_tokens and max_completion_tokens, but the outbound body carried both fields verbatim, so the request boundary and the settlement boundary enforced different numbers. OpenAI treats max_completion_tokens as authoritative and max_tokens as deprecated, and Groq documents the same preference, so pairing max_tokens 1 with max_completion_tokens 100000 bought a full-size generation for the price of one completion token, on the expensive output side and unbounded in size. pinCompletionCeiling now writes the settled minimum back over every ceiling field present before dispatch. It only ever narrows: an absent field stays absent and an unreadable or non-positive one is left as written, since raising it would widen what may be generated. Finding 2, overcharge past the ceiling. clampUsageToCeiling is a no-op when the upstream sent no usage block, which is exactly the branch that then prices the request from content length, so a synchronous 200 carrying content and no usage charged far past the ceiling. The streaming path survived only because its unconfirmed branch discards that figure for a hold capture that is already bounded. settlementCredits now bounds the estimated completion count by the ceiling itself, which covers both paths and the session chat surface through ChatSettlementCredits. Finding 3, variable-price aliases. On an upstream_actual alias the charge comes from the cost the upstream reported for the generation, which the clamp never touches, so clamping the usage block only advertised a completion count nobody was billed on, the exact divergence that function exists to prevent. The clamp is now gated on the pricing mode, the same test capCaptureAtCeiling already applies. hive-auto is live in this mode. Finding 4, the customer-facing support matrix. The never-billed-past-N sentence is now qualified for the variable-price case on both endpoints that serve hive-auto, rather than deleted. Finding 5, best_of. The same unfixed defect n had: nothing read or refused it, the outbound body is re-marshalled from a map so it reached the provider intact, and the generated OpenAPI contract advertised it. Refused on the same terms as n. Finding 6, documentation only. The reading of max_tokens 0, negatives and non-integer spellings as no ceiling is now stated where the invariant is stated. No behaviour change.
…rompt cost Answers the second adversarial stream on this pull request. Three defects, all in code this pull request introduced, each with its reproduction kept as a regression guard. Finding 1, HIGH. pinCompletionCeiling kept any ceiling field it could not parse as an integer, on the reasoning that raising it would widen what the provider may generate. That reasoning is wrong for a field this gateway cannot read: it cannot be called smaller either, and the provider may well parse it. So max_tokens 1 paired with max_completion_tokens 100000.5 reopened the entire bypass through a spelling the JSON number type happens not to cover. A field is now kept only when it reads as a positive integer at or below the ceiling, the same rule clampCompletionLimit already applied to what it cannot read. Finding 2, HIGH. clampCompletionLimit fills in every ceiling field the endpoint speaks, and filled them at VariablePriceMaxCompletionTokens regardless of a smaller ceiling already in the body. On chat that wrote max_completion_tokens 16384 in beside a max_tokens of 1, and since OpenAI treats the newer spelling as authoritative, a variable-price alias generated four orders of magnitude more than was asked for and billed the upstream cost of it, with no settlement clamp behind it. The filled value is now the lower of that constant and any smaller ceiling the caller set. Finding 3, HIGH. Both hold-capture branches bound the capture with capCaptureAtCeiling, whose bound is the catalog price of the whole request at the ceiling, prompt included. Both are reached precisely because no usable usage block arrived, so the metered input count is zero, and passing that through priced a large prompt at nothing: a 100,000-token prompt capped at a handful of output tokens collapsed to a couple of credits. captureInputTokens now falls back to the same content-length prompt estimate settlementCredits uses on the same no-usage path. Finding 4 was a hot-path allocation observation. Partly answered: a byte scan now settles the common single-ceiling chat request without a second full decode. The rest was declined on the thread. Each of the three fixes was mutation-checked: reverting it turns its own test red, and all three were red together before the fixes landed. The existing TestExecuteStreaming_HoldCaptureBoundedByCallerCeiling was amended rather than deleted, since its upper bound had encoded the zero-priced prompt of finding 3.
Review round two, stream 2 of 2: Antigravity adversarial passSecond independent adversarial stream on this money path, run against 8078f56 with Four findings. Three were real defects in code this pull request introduced and are fixed in 446151c, each mutation-checked. The fourth is declined with reasoning below. 1, HIGH, unparseable ceiling field left in the outbound body. 2, HIGH, an invented ceiling larger than the one the caller set. 3, HIGH, hold captures priced the prompt at zero. Both capture branches bound the charge with 4, LOW, three JSON decodes of the request body before dispatch. Partly taken, mostly declined. Taken: Mutation evidence for the three fixes, all reverted afterwards:
|
The first and second adversarial streams both reported findings numbered from one, and the source comments said only round two, which reads as the same numbering in both places. Comment text only, no behaviour change.
## Summary This is the batched buglog follow-up for the 21 pull requests merged during the 2026-08-28 session. 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. ## Source pull requests 1240, 1251, 1253, 1257, 1268, 1276, 1277, 1278, 1281, 1287, 1292, 1293, 1294, 1296, 1300, 1301, 1303, 1305, 1313, 1335, 1337. All merged. Every entry came from a "Buglog entry" heading in one of those bodies. Nothing was invented for a pull request that carried none. ## What landed 36 entries appended, one JSON object per line, append only. The 196 pre-existing lines are byte identical to `origin/main`. | Source | Entries | |---|---| | #1240 | 1 | | #1251 | 1 | | #1253 | 1 | | #1257 | 4 | | #1268 | 5 | | #1276 | 2 | | #1277 | 1 (of 2 in the body) | | #1278 | 0 (merged into #1296) | | #1281 | 2 | | #1287 | 2 | | #1292 | 3 | | #1293 | 1 | | #1294 | 3 | | #1296 | 1 | | #1300 | 1 | | #1301 | 1 | | #1303 | 1 | | #1305 | 3 | | #1313 | 1 | | #1335 | 1 | | #1337 | 1 | Note on #1268: its first "Buglog entry" heading says "None yet" in prose and carries no JSON. Its two later headings, from the CI live lane and from the intermittent tool call failure, carry the five entries taken here. ## Deduplication - **#1278 dropped, folded into #1296.** Both describe the same defect: `omitempty` on `StreamContentBlock.Text` dropped the required `"text":""` from every text `content_block_start`, crashing the real Anthropic SDK's stream accumulator (issue #1274). #1278 is the conformance suite that found it and shipped it marked xfail; #1296 is the fix, and its entry carries the fuller root cause and the actual remedy. One bug, one entry. #1296's entry gains a `discovered_by` field naming #1278 so the discovery is not lost. - **#1277's first entry dropped.** The same body carries a later "Buglog entry (revised)" heading written after the review round found the page's claims did not match what the code enforces. The revised entry is the one taken. - Checked and kept as distinct: #1313 and #1337 are two different hooks (`decision-citation-check.js` and `secrets-scanner.js`) blind to the same MultiEdit payload shape, fixed in two different pull requests, so two entries. #1240 and #1335 are two different `account_not_provisioned` defects, one an observability gap at the edge boundary and one a console mint that should have refused, so two entries. #1305's three entries are three separate rounds of defects in the same money path change, each with its own root cause. ## Corrections against what actually merged Each entry was checked against the merged tree at `origin/main`, not against its own claim. - **#1240.** The entry said the log line went in at `AuthSnapshot.TenantUUID`. On `main` the check is the exported `authz.ParseTenantID(TenantLookup)` that `TenantUUID` delegates to, which the images and audio routing adapters (two further silent call sites found in the same review) also call, and `key_id` is deliberately not logged because CodeQL's clear text logging check flags any field named `*Key*` (alert #31). The `fix` field now says so. - **#1276, first entry.** The entry named `app/console/analytics/page.tsx` as the home of the five fetch helpers and their `Promise.all`. On `main` they live in `apps/web-console/lib/analytics/overview-fetch.ts`, extracted during review. Path corrected. - **#1277.** Its `error_message` was the placeholder `n/a`. Reconstructed from the pull request's own correction narrative: the page as first written published a blanket no content stored claim false for `/v1/batches`, `/v1/files` and `/v1/rag`, a product wide provider blindness claim disproved by catalogue summaries that name vendors (#1284), a metering claim anchored to the console side `UsageEventRow` projection rather than the `usage_events` table, and a 1:1 alias to route claim that is a property of seed data rather than of `SelectRoute`. **#1303 needed no correction.** Its original root cause asserted a live mid stream provider leak on the session chat relay that measurement disproved, and the author had already corrected the body before merge. The corrected version is what was taken, including the sentence recording that the session chat relay did not leak an error frame but silently truncated instead. Every other entry's central claim was verified present in the merged tree, among them `metering.SupportsIncludeUsage`, `sanitize.VariablePriceFrame` in the batch dispatcher, the revoke and regrant in `20260828_01_service_role_public_schema_grant.sql` with the `anon` assertions in `ci-throwaway-db.sh`, `normalizeReasoningUsage` now called from `normalizeChatCompletion`, `signup.SyncTenantMembershipRole`, `TestKeyViewHidesALimitThatIsNotEnforced`, `TestListEventsLatencyCrossesTheWire`, `mask-api-keys.mjs` and `md-table.mjs`, `StreamContentBlock.Text` as `*string`, `redactSnapshot`, `httpx.ReadBody`, `sanitize.ReplaceErrorFrame` with the default deny tail in `provider_blind.go`, `pinCompletionCeiling` and `captureInputTokens` with `applyReasoningHeadroom` gone, `requireBillingTenant`, and `hooks.selfcheck.js` wired into the Repo policy lints check. ## Verification - `node .wolf/hooks/bugstore.selfcheck.js` reports `bugstore selfcheck OK`. - All 232 lines parse as a single JSON object each. - Every appended entry carries `error_message`, `root_cause`, `fix` and `tags`. - Scanned for credentials: no API key, bearer token, JWT, password, AWS key or Postgres DSN with a password appears in any entry. The `hk_` occurrences are prefix descriptions in prose, not keys. - `git diff origin/main...HEAD --name-only` prints `.wolf/buglog.jsonl` and nothing else. No `.wolf/` telemetry was staged. ## Review No adversarial review streams were run, deliberately. This change is records only: it adds no code, no test, no configuration and no behavior, and `.wolf/buglog.jsonl` is on the inert path allowlist in `.github/workflows/ci.yml`, so the six required checks report green without running their heavy steps. If a check does fail here, that is a real signal about the file rather than about the pipeline. https://claude.ai/code/session_01WyEwUxZCArdn1ZUDkTvuQ1 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
#1348) (#1371) Three defects an SDK caller meets in the first minutes, landed together because they are the same file family and the same failure philosophy: an honest error beats a confident wrong answer. Closes #1319. Closes #1318. Closes #1348. Also the family folded into #1318: #996, #1285 and #882 (all already closed, listed for the record). ## 1. images.generate returned a fake success (#1319) `POST /v1/images/generations` answered HTTP 200 with an empty `data` array. Every SDK reports that as success, so the caller code fails somewhere further away from the cause and a retry loop can never recover. Both image paths now refuse a 2xx that carries no usable image and return the sanitized upstream error (502, provider blind, raw text to the operator log only). Two details that are not cosmetic: - The guard runs BEFORE settlement and releases the hold. The flat 0.05 USD equivalent reservation was previously finalized for a response containing no image, so the caller paid for nothing. - It counts payloads rather than array entries. `data: []` and `data: [{}]` are the same answer to a caller, and a length check alone would have passed the second one through as a success. ## 2. audio.speech refused every OpenAI voice and answered 500 (#1318) Two separable defects, both fixed. **Status.** An unsupported voice is a bad request. It is now a 400 `invalid_request_error` with `param: "voice"` and code `invalid_value`, refused before route selection so no reservation is taken for a request that cannot succeed. Previously the name was forwarded, the upstream answered 400, and that reached the caller as a sanitized 500 that invited a retry which could never work. **The voice set. Decision: MAP, not refuse.** The brief allowed either. Mapping wins because the live conformance suite (`packages/sdk-tests/js/tests/audio/audio.test.ts`) sends `voice: "alloy"`, as does every OpenAI SDK example in existence: refusing with a list would be honest and would still leave every stock example broken, which is the opposite of an OpenAI-compatible surface. The eleven OpenAI stock names now translate onto the six upstream voices (arbitrary pairing by rough timbre, but stable, so the same request always gets the same voice), and a name in neither set gets the 400 above naming the whole roster. GET /v1/audio/voices keeps advertising the six distinct upstream voices and not the eleven aliases, because the aliases are other names for those same six and listing them would fill every client dropdown with duplicates. The agreement is pinned by a test: every id the endpoint advertises is accepted by the speech handler and reaches the upstream unchanged. ## 3. Error mapping blamed the model for the caller mistake (#1348) An invalid message role and an oversized `max_tokens` both returned `400 {"message":"hive-small is not available.","code":"upstream_error"}`. Fixed at both ends. **Refuse locally what needs no upstream.** `/v1/chat/completions` now validates the messages array itself: an unknown or missing role, an empty array, and a non-object entry are refused with `invalid_request_error`, the offending `param` (`messages[0].role`, `messages`) and a code an SDK can branch on (`invalid_value`, `empty_array`, `invalid_type`). Same shape as the `n` refusal from PR #1305. Note the empty-array case was reaching the provider too: the pre-existing check reads `len(req.Messages)`, which is the length of the raw JSON bytes, so `messages: []` passed it. **Stop calling a request refusal an availability verdict.** When an upstream answers 400 or 422, `WriteProviderBlindUpstreamError` now emits `invalid_request_error` with code `invalid_request` instead of `api_error` with `upstream_error`, and the LiteLLM fallback-bookkeeping collapse stops claiming the alias is unavailable on those statuses. A 404 and every 5xx keep the availability verdict, which is what they actually mean; that gate has its own test, so a blanket rewording turns it red. Not fixed here, and deliberately: the message still cannot name WHICH parameter on an upstream-originated refusal. The upstream sentence that knew was collapsed by the provider-blind allowlist, and inventing a field name would be worse than pointing at the payload. Naming the field for an oversized `max_tokens` needs a per-alias output ceiling in the catalog, which is a separate change. ## Provider blindness Nothing new is forwarded from an upstream. The images guard hands the upstream body to the existing sanitizer bounded at 4 KiB (it also writes to the operator log, and the empty-image path can otherwise carry a 10 MiB body), and at 502 the sanitizer collapses to a Hive-authored sentence regardless of the text. The two new refusals (voice, messages) are written by this gateway from its own strings and never touch upstream text at all. Every new test asserts on the serialized bytes that no provider name, URL or exception class appears. ## Tests that can actually fail Every assertion reads the serialized response and the status code, never an internal struct. `ImageData.URL` and `B64JSON` are `omitempty` pointers, which is exactly the shape that makes a struct-level assertion blind to what reached the wire. Each guard was mutated away and the tests confirmed RED: | Mutation | Result | | --- | --- | | `hasImagePayload` guard disabled | 6 subtests red, observed `status = 200, want 502 (body: {"created":1700000000,"data":[]})` | | voice refusal and voice rewrite disabled | 16 subtests red across translation and refusal | | `providerBlindRequestShaped` forced false | red, and it reproduced the issue text verbatim: `{"message":"hive-small is not available.","type":"api_error","code":"upstream_error"}` | | `providerBlindRequestShaped` forced true | the 404/502/503 gate test red | | `validateChatMessages` call disabled | all 5 subtests red | | `chatRoles` cut to user only | 5 role subtests red | One existing assertion changed, in `provider_blind_allowlist_test.go`: the collapsed fallback for a 400 is now "Invalid request for hive-auto. Check the request parameters." rather than "hive-auto request failed." The leak assertions that test exists for are untouched. `go test ./apps/edge-api/... -count=1 -short` is green in the toolchain image. ## Verified locally versus live - **Locally (this worktree, toolchain image):** every behaviour above, at the wire level, plus the mutation matrix. - **Live spot check only:** the three defects were measured live on 2026-08-29 by the issues themselves and re-confirmed by reading the code, not re-derived. The fixed behaviour cannot be proven live before merge, because the demo box runs the deployed image; it needs a post-deploy re-run of the SDK conformance suite. ## Conformance suite markers flipped The three `it.fails` markers pointing at #1318 and #1319 are now plain `it`. Left as `it.fails` they would report FAILURE the moment the fix works, since vitest inverts them. ## Found while verifying, not fixed here On the deployed box, `GET /v1/audio/voices` answers 401 `missing bearer`. The route is registered without an authorizer on purpose (#997), but `authSelectorMiddleware` intercepts every `/v1/*` path and sends a request with no Authorization header to the JWT path, which rejects it before the mux is reached. So Open WebUI voice dropdowns still fall back to their hardcoded OpenAI list, which is the exact #996 shape. It is on main, not a stale image. Filed separately rather than folded in here, because it is a change to the auth middleware and this PR is otherwise pure input validation. The voice translation in this PR does make that fallback list work, which is a real if accidental repair of the chat surface. ## Buglog entry To be appended to `.wolf/buglog.jsonl` on main in a buglog-only pull request after this merges (issue #873), not on this branch. ```json {"id":"bug-mtdw1319-a41c02","timestamp":"2026-08-29T07:05:00.000Z","related_bugs":[],"occurrences":1,"last_seen":"2026-08-29T07:05:00.000Z","date":"2026-08-29","title":"images.generate answered 200 with an empty data array and billed the hold","error_message":"POST /v1/images/generations returned HTTP 200 with data: [], no image and no error, and the flat image reservation was finalized for it","root_cause":"Neither handleGeneration nor handleEdit checked the decoded ImageResponse for a usable payload before settling and writing 200; the only route with supports_image_generation is a text model carrying a legacy capability flag, so an imageless 2xx is its normal answer","fix":"Added hasImagePayload, counting entries that actually carry a url or b64_json rather than array length, and refused before settlement on both paths with the sanitized upstream error at 502 while releasing the hold; the upstream body handed to the sanitizer is bounded at 4 KiB so its operator log line cannot carry a multi-megabyte 2xx body","verification":"Six wire-level subtests over the three imageless shapes on both endpoints, asserting status, envelope, absence of a data key, hold released and not finalized; mutation with the guard disabled turns all six red","tags":["images","billing","fake-success","edge-api","issue-1319"],"pr":1371} {"id":"bug-mtdw1318-7be519","timestamp":"2026-08-29T07:05:01.000Z","related_bugs":[],"occurrences":1,"last_seen":"2026-08-29T07:05:01.000Z","date":"2026-08-29","title":"audio.speech rejected every OpenAI voice name and answered 500","error_message":"POST /v1/audio/speech with the OpenAI default voice alloy failed with HTTP 500 and a bare internal error, making the endpoint uncallable by an unmodified OpenAI SDK","root_cause":"The handler rewrote only the model key and forwarded voice verbatim to groq/orpheus-v1-english, whose roster is six entirely different names; the upstream 400 then reached the caller through the provider-blind path as a sanitized 500, so a caller could not tell a bad parameter from a broken gateway and a retry could never succeed","fix":"Added resolveVoice, translating the eleven OpenAI stock names onto the six upstream voices and normalizing case and whitespace, with the resolved value written back into the dispatched body; a name in neither set is refused before route selection with a 400 invalid_request_error naming param voice, code invalid_value, and the supported roster, so no reservation is taken for a request that cannot succeed","verification":"Wire-level tests over all eleven stock names plus case and whitespace variants asserting the DISPATCHED body carries an accepted voice, an agreement test that every id advertised by GET /v1/audio/voices is accepted, and refusal tests for unknown, missing and blank voices; mutation disabling the guard and the rewrite turns sixteen subtests red","tags":["audio","tts","openai-compat","status-mapping","edge-api","issue-1318","issue-996","issue-1285"],"pr":1371} {"id":"bug-mtdw1348-c93d44","timestamp":"2026-08-29T07:05:02.000Z","related_bugs":[],"occurrences":1,"last_seen":"2026-08-29T07:05:02.000Z","date":"2026-08-29","title":"Malformed chat requests were answered as a model availability problem","error_message":"An invalid message role and an oversized max_tokens both returned 400 with message hive-small is not available. and code upstream_error, pointing a customer debugging their own payload at model status","root_cause":"Two causes. Nothing validated the messages array, so a bad role was forwarded, refused upstream, and the LiteLLM fallback-group bookkeeping that came back was collapsed by sanitizeProviderBlindMessage into the is not available sentence regardless of status. Separately WriteProviderBlindUpstreamError labelled every non-429 non-503 upstream failure api_error with code upstream_error, including a 400 that means the caller request was refused. The pre-existing empty-messages check read len on a json.RawMessage, which is a byte length, so messages: [] passed it too","fix":"Added validateChatMessages refusing an unknown or missing role, an empty array and a non-object entry with invalid_request_error, the offending param and codes invalid_value, empty_array and invalid_type, matching the n refusal shape from PR 1305. Added providerBlindRequestShaped so an upstream 400 or 422 is relabelled invalid_request_error with code invalid_request and gets a request-shaped sentence instead of an availability verdict, while 404 and every 5xx keep theirs","verification":"Five wire-level subtests for the malformed shapes, a positive test that all six valid roles still pass, and two error-package tests pinning both sides of the status gate; mutations disabling the validator, cutting the role list, and forcing the status gate to false and to true each turn the matching test red, with the false mutation reproducing the reported body verbatim","tags":["error-mapping","chat-completions","validation","provider-blind","edge-api","issue-1348"],"pr":1371} ```
## 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>
Closes #1283.
What was broken
Both defects were confirmed against production with the real OpenAI SDK v7.8.0 (workflow run 33209920195, 2026-08-28), so this is shipped behaviour, not a theoretical gap.
max_tokenswas not enforced.max_tokens: 8onhive-freereturned 1650 completion tokens, and all 1650 were billed. The caller asked for 8 to bound their own spend and was charged roughly 200 times that.n: 2returned a single choice with HTTP 200 and no indication the parameter had been dropped.The billing invariant, stated precisely
Scope of the guarantee, so nobody has to re-derive it from the diff:
/v1/chat/completions,/v1/completionsand/v1/responses: the measured charge, the content-estimate fallback, the sync zero-content hold capture, and the streaming missing-usage hold capture.EnforceVariablePriceBoundsforces a ceiling Hive chose rather than one the caller asked for, so reading it afterwards would bound the charge by our own number and guarantee the caller nothing.CreditsForTokensfloors any non-zero quantity at one credit, so the fail-closed rules in D-034 and D-048 (no served request bills zero) are untouched.Pricing.IsUpstreamActual) settles on the cost the upstream reported for the generation, not a token count times a catalog rate, so there is no per-token figure to cap. Its spend stays bounded byEnforceVariablePriceBoundsforcing the outbound ceiling toVariablePriceMaxCompletionTokens, and by the hold clamp in control-plane'sfinalizeLocked. No route with a nonzero reasoning reserve is variable-priced today.Arithmetic is unchanged and still runs through
metering.ChargeCredits, so every figure staysmath/big. No float64 was introduced.Root cause of defect 1
applyReasoningHeadroom, shipped by PR #1225 for issue #1171 and live since 2026-08-26, inflated every completion-limit field the caller had set byprovider_routes.reasoning_reserve_tokensbefore dispatch. That column is 4096 on three of the fourhive-freepool members, somax_tokens: 8went upstream as4104.Its premise was that hidden reasoning would burn the reserve while visible content survived inside the caller's own budget. The premise is not enforceable: nothing in an OpenAI-compatible request tells an upstream that part of a ceiling is earmarked for hidden reasoning, so the model spends the inflated ceiling on whichever it emits first. On the reported request (
Write a long story about the sea.) that was visible story text.The sharper finding is that the rewrite deliberately skipped a ceiling the caller had NOT set. Its entire effect was therefore overriding ceilings callers HAD set. It had no legitimate operating mode, so it is deleted rather than tuned.
This was not a symptom patched at one call site.
applyReasoningHeadroomhad exactly three callers (orchestrator.gosync,stream.go,stream_responses.go); all three are gone, and the settlement bound is applied at each path's single metering seam rather than per branch.Root cause of defect 2
N *intwas declared on bothChatCompletionRequestandCompletionRequestand read by nothing. The raw body was forwarded to LiteLLM, the parameter was ignored upstream, and one choice came back with HTTP 200.Where enforcement lives, and why
Both boundaries. Either alone is a hope rather than a proof.
Request boundary. The caller's ceiling now reaches the provider unchanged. This is the only boundary that can stop a caller receiving 1650 tokens they capped at 8, and it is what makes the reported usage honest rather than merely cheap. It also restores OpenAI semantics, where a reasoning model's hidden reasoning counts against the ceiling.
Settlement boundary. The metered completion count is capped at the caller's own ceiling before anything reads it. It clamps the ONE usage object both the customer response and the charge derive from, and rewrites the already-marshalled body to match, so the number the caller reads and the number they pay for cannot diverge. Request-boundary correctness cannot carry the money guarantee on its own: it would depend on every upstream honouring a parameter we merely forwarded, on no future rewrite of the outbound body, and on no provider counting hidden reasoning into
completion_tokens. PR #1225 is the proof that the outbound body only has to be wrong once.A settlement-only fix was also rejected. The caller would still receive and be shown a completion count they explicitly capped, and every OpenAI-compatible cost dashboard reads
usage.completion_tokensunconditionally.What issue #1171 keeps, and what it loses
Its zero-content guard, the fail-closed half, is untouched: an empty-content
finish_reason=lengthcompletion on a reserving pool is still retried once and still captures rather than settling full price.provider_routes.reasoning_reserve_tokenskeeps a reader as the flag that gates it, so no column is orphaned.What issue #1171 loses is the product half: a reasoning member given a very small ceiling can again answer with empty content. That outcome is now strictly cheaper than before this PR, because the capture that follows it is bounded by the caller's ceiling instead of charging the flat
DefaultHoldTexthold. Routing a small-ceiling request to a non-reasoning member, or sendingreasoning_effort: low, are the honest ways to recover the product half and are follow-up work, not a silent rewrite of the caller's request.Defect 2 decision: reject, not support
nother than 1 now returns 400unsupported_parameterwithparam: "n"on both/v1/chat/completionsand/v1/completions. Absent andn: 1pass through untouched;n: 0and negatives are refused too, which OpenAI also rejects.Rejecting rather than honouring, because no route in this catalog can serve
n > 1: the free pool's Groq members accept onlyn=1on their OpenAI-compatible surface, and OpenRouter does not implement the parameter across the pool. Honouring it would additionally multiply generated tokens against a single per-requestmax_tokensceiling, which is the invariant above. Silently returning one choice was the only unacceptable option, and a declared 400 is the OpenAI-contract-correct answer for a parameter the upstream genuinely cannot serve.The refusal message names the parameter and nothing about who serves the request, so it stays provider-blind.
support-matrix.jsonnotes for both endpoints now record the refusal and the settlement bound.Tests, written first and mutation-checked
Every behavioural test below was written before the fix and observed failing against the unfixed code. Each fix was then reverted individually to prove the test catches its absence rather than merely its deletion, and restored.
TestExecuteSync_CallerCeilingReachesProviderUnchangedreporteddispatched max_tokens = 4104, want the caller's own 8clampUsageToCeilingcall sites inexecuteSyncandexecuteStreaming, leaving the helper intact so this tests the wiring rather than the functionTestExecuteSync_BilledCompletionTokensCappedAtCallerCeilingandTestExecuteStreaming_BilledCompletionTokensCappedAtCallerCeilingreservation.Held()TestExecuteSync_ZeroContentCaptureBoundedByCallerCeilingreportedcharged 10000 credits against a ceiling worth 108unsupportedChoiceCountreturns falseTestNGreaterThanOneRejectedsubtestsreservation.Held()TestExecuteStreaming_HoldCaptureBoundedByCallerCeilingreportedcharged 10000 credits against a ceiling worth 32Every mutation was reverted and the suite returns green.
Negative controls are in the suite too, so the clamp cannot quietly become a discount on every request:
TestExecuteSync_NoCeilingLeavesUsageAlonepins that a caller who set no ceiling is metered exactly what the provider reported,TestNOneOrAbsentPassesThroughpins that only the unservable shape is refused, andTestSyncOverrunDispatchesOncepins that an over-ceiling response does not become a second provider call.The money tests price against the real
hive-freecatalog row (1,000,000 credits per million input, 4,000,000 per million output, D-048 and migration20260824_02_free_pool_router.sql) rather than a fixture, so the magnitude they assert is the magnitude the issue measured.TestExecuteSync_ZeroContentLength_Twice_CapturesHoldwas amended rather than deleted: its request now sets no ceiling, which is the condition under which a capture is still at the full hold. Both halves of that behaviour are now pinned, one per test. The three tests that pinned the headroom inflation are removed along with the behaviour they described.Verification
Run in the repo's Docker toolchain, on a throwaway toolchain container with a worktree-unique
COMPOSE_PROJECT_NAME. No shared database was touched; these are-shortunit tests.Zero failures across both modules.
gofmtis clean on every file this PR touches. Two pre-existinggofmtmisalignments inchat_completions.goandcompletions.gowere left alone as unrelated noise.Review round two, first stream: five defects found in this branch, all fixed
An independent adversarial money-path review of a2f4539 reproduced five defects in this branch itself. Every one is answered in 8078f56, and every reproduction is kept as a regression guard in
completion_ceiling_review_test.go.Finding 1, HIGH, caller-triggerable revenue bypass. The ceiling was read as the smaller of
max_tokensandmax_completion_tokens, but once the headroom rewrite was deleted the outbound body forwarded both fields verbatim, so the request boundary and the settlement boundary enforced different numbers. OpenAI treatsmax_completion_tokensas authoritative andmax_tokensas deprecated, and Groq documents the same preference, so a caller pairingmax_tokens: 1withmax_completion_tokens: 100000received a full-size generation and paid for one completion token, on the four-to-one expensive output side and unbounded in generation size.pinCompletionCeilingnow writes the settled minimum back over every ceiling field present, on all three dispatch paths, beforeEnforceVariablePriceBoundsso a variable-price alias still gets its own cap applied on top. It only ever narrows: an absent field stays absent, and an unreadable or non-positive one is left as written, because raising it would widen what the provider may generate.Finding 2, HIGH, overcharge past the ceiling.
clampUsageToCeilingis a no-op when the upstream sent no usage block, which is exactly the branch that then prices the request from content length, so a synchronous 200 carrying content and no usage charged far past the ceiling. Streaming survived only by accident, because its unconfirmed branch discards that estimate for a hold capture that is already bounded. The bound went intosettlementCredits, the single function both settlement paths route through, which also closes the same hole on the session chat surface throughChatSettlementCredits.Finding 3, MEDIUM, variable-price aliases. On an
upstream_actualalias the charge is derived from the cost the upstream reported for the generation, which the clamp never touches, so clamping the usage block only advertised a completion count nobody was billed on: the exact divergence that function exists to prevent. The clamp is now gated on the pricing mode, the same testcapCaptureAtCeilingalready applies.hive-autois live in that mode.Finding 4, MEDIUM, published contract. The never-billed-past-N sentence is now qualified for the variable-price case on both endpoints that serve
hive-auto, rather than deleted.Finding 5, MEDIUM,
best_of. The identical unfixed defectnhad, on the same endpoint: nothing read or refused it, the outbound body is re-marshalled from a map so it reached the provider intact, and the generated OpenAPI contract advertised it. Refused on the same terms asn, with the same guard shape and the same provider-blindness assertions.Finding 6, LOW, documentation only. That
max_tokens: 0, negatives,8.0and"8"all read as no ceiling is now stated where the invariant is stated, with the reasoning. No behaviour change.Finding 7 requested no change and was rebutted on the thread.
All five code findings were red before the fix and green after, and the whole of
./apps/edge-api/...and./apps/control-plane/...passes withgo vetclean andgofmtclean on every touched file.Review round two, second stream: three more defects, all fixed
A second independent adversarial stream ran against 8078f56, the commit that answered the first stream. Codex is exhausted until 2026-09-10 and was replaced by Antigravity rather than skipped, on owner instruction. It found three real defects in code this pull request introduced, all fixed in 446151c, plus one performance observation. Full stream output and the mutation table are in the pull request comment.
Finding 1, HIGH, an unparseable ceiling field survived into the outbound body.
pinCompletionCeilingkept any ceiling field it could not parse as an integer, on the reasoning that overwriting it would widen what the provider may generate. That reasoning does not hold for a field this gateway cannot read: it cannot be called smaller either, and the provider may well parse it. Somax_tokens: 1paired withmax_completion_tokens: 100000.5reopened the entire first-stream bypass through a spelling the JSON number type happens not to cover. A field is now kept only when it reads as a positive integer at or below the ceiling, which is the ruleclampCompletionLimitalready applied to what it cannot read.Finding 2, HIGH, the gateway invented a ceiling larger than the one the caller set.
clampCompletionLimitfills in every ceiling field the endpoint speaks, and filled them atVariablePriceMaxCompletionTokensregardless of a smaller ceiling already present. On chat that wrotemax_completion_tokens: 16384in beside amax_tokens: 1, and since the newer spelling is the authoritative one, a variable-price alias generated four orders of magnitude more than was asked for and billed the upstream cost of it, with no settlement clamp behind it because that mode has none. The filled value is now the lower of that constant and any smaller ceiling the caller set, and the same limit narrows present fields too, so the function is correct independently of call order.Finding 3, HIGH, hold captures priced the prompt at zero. Both capture branches bound the charge with
capCaptureAtCeiling, whose bound is the catalog price of the whole request at the ceiling, prompt included. Both are reached precisely because no usable usage block arrived, so the metered input count is zero, and passing that through priced a large prompt at nothing: a 100,000-token prompt capped at ten output tokens collapsed the capture from the hold to roughly forty credits, a free serve of the expensive half of the request.captureInputTokensnow falls back to the same content-length prompt estimatesettlementCreditsuses on the same no-usage path.TestExecuteStreaming_HoldCaptureBoundedByCallerCeilingwas amended rather than deleted, because its upper bound had encoded exactly this zero-priced prompt.Finding 4, LOW, three JSON decodes of the request body before dispatch. Partly taken:
pinCompletionCeilingnow settles the common single-ceiling chat request with a byte scan and returns before decoding. The rest was declined on the thread, since threading one shared decoded map through three functions with different contracts, one of which refuses the request outright on a decode error, is a larger and riskier diff on a money path than a call count justifies, on a request already about to make a network call to a language model.Each of the three fixes was mutation-checked: reverting it turns its own test red, and all three were red together before the fixes landed.
Not in scope
sampling-params.test.ts. Adding the same file here would collide with it.WHYprose in migration20260826_01_route_reasoning_reserve.sqlstill describes the headroom mechanism. Applied migrations are never edited in this repo; the column's surviving purpose is documented at its remaining reader inzero_content_guard.go.Buglog entry
{"id":"bug-2026-08-28-max-tokens-not-enforced","date":"2026-08-28","title":"max_tokens silently inflated by the reasoning reserve, billing 206x the caller's ceiling; n>1 silently truncated to one choice","error_message":"max_tokens: 8 on hive-free returned usage.completion_tokens 1650, all billed; n: 2 returned HTTP 200 with choices.length 1 and no error","root_cause":"applyReasoningHeadroom (PR #1225, issue #1171) inflated every completion-limit field the caller had set by provider_routes.reasoning_reserve_tokens (4096 on three of four hive-free members) before dispatch, so max_tokens 8 went upstream as 4104. Its premise that hidden reasoning would burn the reserve while visible content stayed inside the caller's budget is unenforceable: nothing upstream separates the two, so the model spent the inflated ceiling on visible output. Because the rewrite skipped a ceiling the caller had NOT set, its entire effect was overriding ceilings callers HAD set. Separately, ChatCompletionRequest.N and CompletionRequest.N were declared and read by nothing, so n was forwarded, ignored upstream, and one choice returned with no error. Two hold-capture branches (sync zero-content, streaming missing-usage) also charged the flat DefaultHoldText authorization floor regardless of the caller's ceiling, a far larger breach of the same invariant.","fix":"Deleted applyReasoningHeadroom and its three call sites so the caller's ceiling reaches the provider unchanged. Added completion_ceiling.go: requestedCompletionCeiling reads the ceiling from the caller's original body before any outbound rewrite, clampUsageToCeiling caps usage.CompletionTokens at it on the single usage object both the response and the charge derive from, rewriteNormalizedUsage keeps the response body in step, and capCaptureAtCeiling bounds both hold-capture branches without ever reaching zero. Refused n other than 1 with 400 unsupported_parameter and param n on both chat and legacy completions.","tags":["billing","money-path","overcharge","max_tokens","openai-conformance","issue-1283","issue-1171","edge-api","inference","free-pool"]}A second entry for the three defects the review found in this branch. Both lines go to main together in the buglog-only pull request.
{"id":"bug-2026-08-28-completion-ceiling-review-defects","date":"2026-08-28","title":"Completion-ceiling fix shipped a caller-triggerable revenue bypass, an unbounded content estimate, and a usage clamp on aliases it could not bill","error_message":"Request {max_tokens: 1, max_completion_tokens: 100000} was dispatched unchanged and settled at output_tokens 1, actual_credits 24 where the unclamped generation was worth 6620; a synchronous 200 carrying content and no usage block against max_tokens 8 charged 1323 credits versus 32 ceiling-priced; on an upstream_actual alias the caller read completion_tokens 8 while paying the upstream cost of 1650","root_cause":"Three separate causes in one change. First, requestedCompletionCeiling takes the smaller of the two chat spellings but nothing rewrote the outbound body, so after applyReasoningHeadroom was deleted both fields were forwarded verbatim and the request boundary enforced a different number than settlement; OpenAI and Groq both prefer max_completion_tokens, so the larger field governed generation while the smaller governed billing. Second, clampUsageToCeiling returns early when usage is nil, which is the same condition that routes settlement into the content-length estimate in settlementCredits, leaving the one path that guesses the completion count as the one path with no ceiling on it; the streaming path was masked because settleStream overwrites the estimate with a bounded hold capture. Third, clampUsageToCeiling was called unconditionally, but an upstream_actual alias settles from respBody or acc.RawUsageChunk, which the clamp never reads, so it lowered the number the caller reads without lowering the charge.","fix":"Added pinCompletionCeiling, which writes the settled minimum back over every ceiling field present in the outbound body before dispatch and only ever narrows, wired on executeSync, executeStreaming and executeResponsesStreaming ahead of EnforceVariablePriceBounds. Added a ceiling parameter to settlementCredits that bounds the content-length completion estimate, with ChatSettlementCredits reading the ceiling from the request bytes it already parses so the session chat surface is covered too. Gated clampUsageToCeiling on route.Pricing.IsUpstreamActual, the same test capCaptureAtCeiling applies. Refused best_of other than 1 on legacy completions. Qualified the support-matrix sentence for the variable-price case and documented that non-positive and non-integer ceiling spellings read as no ceiling.","tags":["billing","money-path","revenue-bypass","overcharge","max_tokens","max_completion_tokens","best_of","upstream-actual","issue-1283","pr-1305","edge-api","inference","code-review"]}A third entry for the three defects the second adversarial stream found. All three lines go to main together in the buglog-only pull request.
{"id":"bug-2026-08-28-completion-ceiling-second-stream-defects","date":"2026-08-28","title":"Ceiling pinning kept unreadable fields, the variable-price bound invented a larger ceiling than the caller set, and hold captures priced the prompt at zero","error_message":"Request {max_tokens: 1, max_completion_tokens: 100000.5} was dispatched with the float intact; a variable-price request sending only {max_tokens: 1} was dispatched with max_completion_tokens 16384 written in beside it; a streaming request with a prompt of roughly 11334 tokens and max_tokens 8 whose upstream sent no usage block settled at 32 credits rather than the price of the prompt","root_cause":"Three causes in the fix for the first adversarial stream. First, pinCompletionCeiling skipped any present ceiling field that failed to unmarshal into int64, on a never-widen rule that does not hold for a value the gateway cannot read at all, since the provider may parse what encoding/json refuses and OpenAI treats max_completion_tokens as authoritative. Second, clampCompletionLimit sets every field the endpoint speaks to VariablePriceMaxCompletionTokens when absent, with no reference to a smaller ceiling already present, and an upstream_actual alias has no settlement clamp behind that. Third, capCaptureAtCeiling computes its bound as the catalog price of the whole request at the ceiling, but both callers pass the metered input count, which is zero exactly because those branches are reached when no usable usage block arrived, so the prompt was priced at nothing and the capture collapsed to the output side alone.","fix":"pinCompletionCeiling now keeps a field only when it reads as a positive integer at or below the ceiling and overwrites everything else present, logging the raw literal truncated. clampCompletionLimit derives an effective limit as the lower of VariablePriceMaxCompletionTokens and any smaller positive ceiling already in the body, and uses it both to fill absent fields and to narrow present ones. Added captureInputTokens, which falls back to the content-length prompt estimate settlementCredits already uses when the upstream reported no usage, and wired it into both capCaptureAtCeiling call sites. Added a byte-scan early-out to pinCompletionCeiling so an ordinary single-ceiling chat request pays no extra decode.","tags":["billing","money-path","revenue-bypass","free-serve","max_tokens","max_completion_tokens","upstream-actual","hold-capture","issue-1283","pr-1305","edge-api","inference","code-review"]}