fix(cloud): refund stranded hold on non-streaming app-chat post-provider failure (#11169 part 1) - #11278
Merged
Conversation
…-provider processing fails (#11169 part 1) The non-streaming branch of apps/[id]/chat read the provider body, computed cost, and settled — all outside any refund scope. A throw before the settle (truncated body failing providerResponse.json(), transient calculateCost error) fell through to the outer catch, which returned 500 WITHOUT refunding the upfront hold. The streaming branch was covered by reconcileStreamProcessingError (#10837); this path was not. Extracts a pure reconcileNonStreamProcessingError helper mirroring the stream one: wraps the post-provider block, tracks whether reconcileCredits settled, and full-refunds iff the settle had not yet run — so a body/cost failure can't keep the advertiser's credits, while a post-settle throw correctly keeps the real charge. 3 unit tests drive the real helper. Residual 2 (/v1/chat reserve sizing) already landed on develop; residuals 3 (stranded synchronous reservation on dropped waitUntil) and 4 (abort-refund evasion) need the reservation-lifecycle sweep and are tracked separately on the issue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eservation-residuals
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Contributor
|
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 |
This was referenced Jul 2, 2026
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
This was referenced Jul 2, 2026
NubsCarson
added a commit
that referenced
this pull request
Jul 2, 2026
…d-settle throw can't double-refund (mint credits) (#11473) The #11271 mass-revert reintroduced a double-credit on POST /api/v1/apps/:id/chat non-streaming: #11278 re-fixed the stranded-hold refund but set the settle flag AFTER reconcileCredits returns. reconcileCredits is not transactional — it commits its org-balance refund before its earnings/counter writes — so a mid-settle throw (e.g. reverseCreatorEarnings deadlock) reached the catch as 'never settled' and refunded the FULL hold a second time (total credited = 2x reserved − actual). Restore the #11218 fence: flip nonStreamingSettleStarted IMMEDIATELY BEFORE invoking reconcileCredits and route the catch through reconcileNonStreamingSettleError (settleStarted gating + non_streaming_settle_error ledger tag), wrapped in .catch so a refund failure can't mask the original error. Collapse the divergent dead reconcileNonStreamProcessingError helper (+ its test) into the single settle helper. The route-level guard test (apps-chat-nonstreaming-settle-guard) was RED at develop tip (3 fail/1 pass, reconcileCredits called 2x, masked by concurrency-cancelled CI); now 4/4 green. apps-chat-stream-refund 7/7 green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part 1 of #11169. The non-streaming branch of
apps/[id]/chatread the provider body, computed cost, and settled — all outside any refund scope; a throw before the settle (truncated body failingproviderResponse.json(), transientcalculateCosterror) fell through to the outer catch, which returned 500 WITHOUT refunding the upfront hold. The streaming branch was covered byreconcileStreamProcessingError(#10837); this path had nothing.Extracts a pure
reconcileNonStreamProcessingErrorhelper (mirroring the stream one): wraps the post-provider block, tracks whetherreconcileCreditssettled, full-refunds iff the settle had not yet run — a body/cost failure can't keep the advertiser's credits, while a post-settle throw correctly keeps the real charge. 3 unit tests drive the real helper; existing stream-refund test still green; tsgo + biome clean.Scope note: #11169 residual 2 (
/v1/chatreserve sizing) already landed on develop; residuals 3 (stranded synchronous reservation on droppedwaitUntil/eviction — needs a stale-reservation sweep mirroringsweepStalePendingInferenceChargesDb) and 4 (abort-mid-stream refund evasion) are the reservation-lifecycle sweep work, tracked on the issue for a follow-up.🤖 Generated with Claude Code