fix(cloud): pay affiliate earnings only from collected markup - #11976
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Independent verification: I traced the same defect and arrived at the same fix (collection-gated payout + per-request self-referral guard). This PR is a superset (it also includes the affiliate markup in the reservation via a cost multiplier — the more robust leg). I ran the cloud/shared affiliate suite locally on the current develop tip and |
|
Local follow-up verification after ab071d2:
The first plain targeted bun test run also had all 8 assertions pass, but exited nonzero because the repo-level bunfig text coverage report was too broad for this single-file invocation; rerunning with LCOV produced a clean exit. |
… (#12047) The embeddings route omitted affiliateCode from its reserveCredits context — a residual of #11972 that the #11976 reserve-inclusion fix covered on /v1/chat/completions and /v1/messages but not here. The upfront hold was base+platform only (buffered 1.5x), while the deferred billUsage still credited the affiliate the full attacker-set markup (up to 1000%) as cashable redeemable earnings. With the org balance exhausted, the settle recorded an uncollectable overage and the affiliate credit stood — minting money the platform never collected, repeatable per request via 2-account collusion. Thread affiliateCode into the reserve context exactly like the covered routes, so resolveBillableAffiliate folds the markup into the hold via estimatedCostMultiplier and the payout is always backed by collected money (fail-closed at reserve time). Regression suite drives the REAL route + REAL reserveCredits/billUsage (affiliate resolution, markup math, earnings credit all real; only auth/embedder/pricing/ledger-reserve/writers stubbed) and asserts the money invariant itself: reserved >= settled with the affiliate present, plus no-header / self-referral / inactive-code guards. The key test fails on the unfixed route (hold multiplier absent, /usr/bin/zsh.15 hold vs .10 settle). Fixes #12017
…ngs (#12017 residual) (#12060) #12047 closed #12017 leg 1: the embeddings reserve now threads affiliateCode, so the upfront hold folds in the attacker-set markup (up to 1000%) and the request fails closed when the org cannot cover it. Leg 2 was still open: billUsage ran WITHOUT the reservation, so the #11976 collectedAffiliateEarnings clamp was a no-op on this route. estimateTokens is chars/4; CJK/emoji-heavy input tokenizes at >1.5x that, so the provider-reported actual cost can blow past even the affiliate-inclusive buffered hold. The overage debit then fails (uncollected_overage, credits.ts, no throw) while the affiliate is still credited the FULL nominal preAffiliateTotalCost x markup% - a smaller but still repeatable cashable mint of money the platform never collected, violating #11976's own invariant (affiliate earnings never exceed collected revenue). Fix: - Hand billUsage a settler-backed VIEW of the reservation whose reconcile routes through the route's existing first-call-wins settler, preserving the #10557 single-settle-owner invariant (no double-settlement is possible; the route's explicit settle becomes an idempotent safety net). billUsage now reconciles BEFORE its affiliate-earnings write, so the clamp sees the reconciliation and pays the affiliate only from COLLECTED markup - 0 on an uncollected_overage. - Pass the server-generated requestId (#11588) into the billUsage context so the affiliate-earnings dedupe sourceId is deterministic (ai_billing:usage:<requestId>) instead of legacy-random; the route's own #11588 comment claimed this wiring but it was never hooked up. - Update embeddings-credit-leak.test.ts to the new (stronger) contract: billUsage receives the settler-backed view, reconciles through it, and the ledger still settles exactly once. Regression proof (real route + real ai-billing/credits reserve/reconcile CTEs + real seeded ai_pricing_entries catalog row + real affiliates repo + real redeemable-earnings ledger on PGlite, loud pgliteReady guard) in embeddings-affiliate-clamp.integration.test.ts: - org funded to only the base hold -> 402 before the provider call, nothing minted (real-ledger proof of #12047's leg 1); - fully funded org -> affiliate still earns the full collected markup and the dedupe sourceId is requestId-keyed; - uncollectable overage -> affiliate paid exactly the collected markup (0 when nothing above the base cost was collected), never the nominal 10x. These two clamp tests are red on develop tip (post-#12047) and green with this change. Refs #12017 #12047 #11972 #11976 #10557 #11588. [cloud-security]
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
… cached gates (#15409) * perf(cloud): tier-3 inference hot path — deferred billing admission + cached gates warm TTFB through the chat-completions gateway is 1.6-1.8s (3.2s cold) against a 0.15s cerebras-direct provider call, with shaw's tier-1/2 (a07084e) warm. the remaining floor is per-request cross-provider round-trips. this moves the biggest ones off the critical path, expected warm savings per step: - deferred billing admission (INFERENCE_DEFERRED_ADMISSION, default off): the durable admission write (db-ledger postgres transaction / kv pending charge, ~100-400ms every request) is started immediately but not awaited — it runs under executionCtx.waitUntil concurrently with the provider call. the critical path keeps a cached 402 gate (15s org-balance hint + 60s in-isolate refusal blocklist); the settler awaits the admission before settling, so the exactly-once ledger/kv reconciliation is unchanged and a refused admission is charged directly via the fail-closed debit. 402 fires at worst one request later than today. ~100-400ms/request. - enforceOrgRateLimit in-isolate 5s decision lease: removes the per-call redis client build + tcp/tls connect + 4-cmd pipeline (and the org-tier cache read) from warm repeats; approximate within the lease window by design, local budget = min(remaining, pro-rated window share). ~100-400ms/request when REDIS_RATE_LIMITING=true, no-op otherwise. - shouldBlockUser in-isolate 60s memo (+ invalidation on recorded violation / reset): removes an uncached cross-provider postgres read from every session/JWT-auth inference request (api-key fast path already skipped it). ~100-400ms/request on that path. - getCachedGatewayModelById in-isolate 60s per-model memo: removes the warm full-catalog shared-cache read for non-name-pattern models. ~10-60ms/request. - calculateCost: audited, already memoized in-isolate (60s persisted-pricing cache) — no change needed. not changed: request/response shapes, insufficient-credit semantics (402 path intact, window documented), monetized-app reserveInferenceCredits (#11976 — stays synchronous), affiliate-marked requests (#12749 — stay on the sync reserve), the tier-2 synchronous admission (untouched fallback underneath). flag-gated default-off everywhere (wrangler.toml), same soak-then-cutover discipline as tier-1/2. profiling table + safety analysis in packages/cloud/api/docs/inference-hot-path.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cloud): tier-3 review fixes — convergent rate-limit lease + flag-gate all decision caches addresses qa-agent adversarial review on #15409 (D1/D2/D3): - D1: the org rate-limit lease is now CONVERGENT, not lossy. every leased request is carried into the next authoritative check — checkRateLimitRedis gains carriedCount and appends the locally-served requests to the sliding window BEFORE counting, and the carry survives lease expiry (plain map; entries only replaced once their count is flushed, and a flag flip still drains pending carries). a hot isolate can now exceed the org limit by at most ONE in-flight lease budget before the window catches up and denies — never the sustained ~(1+B)x overshoot the review reproduced. proven by a new convergence test driving 5x the limit through a simulated real window: allowed <= limit + one budget. - D2: all three decision caches (rate-limit lease, 60s shouldBlockUser memo, 60s model-catalog memo) are now gated behind a new second flag INFERENCE_HOT_PATH_CACHES (default "false" in all wrangler blocks) — deliberately separate from INFERENCE_DEFERRED_ADMISSION because they are orthogonal to billing. both flags off = byte-identical to today, so "rollback = flip the flag" now covers every behavior change in the PR. (one output-identical micro-change stays unconditional: groq-native catalog lookups no longer fetch the merged catalog they never used.) - D3: corrected the 402-window phrasing in the doc + module header — serial traffic 402s one request later; the honest concurrent bound is one full 15s hint window per org fleet-wide plus in-flight streams (zero-delta vs tier-2 on the prod kv config; the weakening applies only to the db ledger). tests: +3 (flag-off parity for lease + memo, D1 convergence proof); full affected sweep re-run green (99 shared + 101 api). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Closes #11972.
Summary
Testing