fix(gateway): re-check credits on env-var retry - #2149
Merged
Merged
Conversation
The initial credit gate in chat.ts only fires when the first attempt is about to use LLMGateway env-var tokens. Retry/fallback paths went through resolveProviderContext, which switched to env-var tokens without re-validating credits — letting a $0-credits org be billed via used_mode="credits" once a primary attempt failed over. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
WalkthroughThe PR modifies credit validation logic in the gateway's chat module. It redefines which models are considered "free" for credit-gate purposes to use only the catalog's ChangesCredit Validation and Provider Fallback Gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 6/8 reviews remaining, refill in 13 minutes and 53 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/gateway/src/chat/tools/resolve-provider-context.ts`:
- Around line 143-145: Replace the user-facing error that exposes
organization.id in the HTTPException thrown in resolve-provider-context.ts:
remove the interpolated organization.id and use a generic, non-identifying
message (e.g. "Not enough credits. Please add more credits or contact support.")
when constructing the HTTPException instance so the thrown HTTPException no
longer leaks internal organization identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d35b77d2-c910-41fe-8c48-cf762be36092
📒 Files selected for processing (2)
apps/gateway/src/chat/chat.tsapps/gateway/src/chat/tools/resolve-provider-context.ts
Comment on lines
+143
to
+145
| throw new HTTPException(402, { | ||
| message: `Organization ${organization.id} has insufficient credits`, | ||
| }); |
Contributor
There was a problem hiding this comment.
Avoid exposing internal organization ID in user-facing error message.
The error message includes organization.id, which leaks an internal identifier to the end user. The equivalent check in chat.ts uses a more generic message without exposing IDs:
"Not enough credits. Please add more credits or contact support."
Consider aligning with the existing pattern for consistency and to avoid exposing internal data.
Suggested fix
throw new HTTPException(402, {
- message: `Organization ${organization.id} has insufficient credits`,
+ message: `Not enough credits. Please add more credits or contact support.`,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| throw new HTTPException(402, { | |
| message: `Organization ${organization.id} has insufficient credits`, | |
| }); | |
| throw new HTTPException(402, { | |
| message: `Not enough credits. Please add more credits or contact support.`, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/gateway/src/chat/tools/resolve-provider-context.ts` around lines 143 -
145, Replace the user-facing error that exposes organization.id in the
HTTPException thrown in resolve-provider-context.ts: remove the interpolated
organization.id and use a generic, non-identifying message (e.g. "Not enough
credits. Please add more credits or contact support.") when constructing the
HTTPException instance so the thrown HTTPException no longer leaks internal
organization identifiers.
There was a problem hiding this comment.
Pull request overview
This PR closes a billing loophole in the gateway retry/fallback flow where a request that initially used a BYOK provider key could later retry using LLMGateway environment tokens without re-checking that the organization has positive credits.
Changes:
- Added a credit gate inside
resolveProviderContextto prevent env-var token fallback when org credits are non-positive (except for catalog-flagged free models). - Wired the new gate into both
creditsmode andhybridmode (no-provider-key) branches inresolveProviderContext. - Added an explanatory comment in
chat.tsabout intentionally using the baremodelInfo.freeflag in the initial credit gate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| apps/gateway/src/chat/tools/resolve-provider-context.ts | Adds a mirrored credit check to protect env-var fallback paths during retries/fallback. |
| apps/gateway/src/chat/chat.ts | Clarifies why the initial credit gate uses modelInfo.free rather than isModelTrulyFree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+116
to
+122
| // with non-positive credits. Free models (explicitly flagged in the catalog) | ||
| // are exempt. | ||
| function assertOrganizationHasCreditsForEnvFallback( | ||
| organization: OrgInfo, | ||
| modelInfo: ModelDefinition, | ||
| ): void { | ||
| if (modelInfo.free) { |
Comment on lines
+125
to
+145
| const regularCredits = parseFloat(organization.credits ?? "0"); | ||
| const devPlanCreditsRemaining = | ||
| organization.devPlan !== "none" | ||
| ? parseFloat(organization.devPlanCreditsLimit ?? "0") - | ||
| parseFloat(organization.devPlanCreditsUsed ?? "0") | ||
| : 0; | ||
| const totalAvailableCredits = regularCredits + devPlanCreditsRemaining; | ||
| if (totalAvailableCredits > 0) { | ||
| return; | ||
| } | ||
| if (organization.devPlan !== "none" && devPlanCreditsRemaining <= 0) { | ||
| const renewalDate = organization.devPlanExpiresAt | ||
| ? new Date(organization.devPlanExpiresAt).toLocaleDateString() | ||
| : "your next billing date"; | ||
| throw new HTTPException(402, { | ||
| message: `Dev Plan credit limit reached. Upgrade your plan or wait for renewal on ${renewalDate}.`, | ||
| }); | ||
| } | ||
| throw new HTTPException(402, { | ||
| message: `Organization ${organization.id} has insufficient credits`, | ||
| }); |
Comment on lines
219
to
252
| @@ -211,6 +246,7 @@ export async function resolveProviderContext( | |||
| if (providerKey) { | |||
| usedToken = providerKey.token; | |||
| } else { | |||
| assertOrganizationHasCreditsForEnvFallback(organization, modelInfo); | |||
| const envResult = getProviderEnv(usedProvider as Provider, { | |||
| excludedIndices: options.excludedEnvKeyIndices, | |||
| }); | |||
Comment on lines
+2855
to
+2857
| // We trust the bare `modelInfo.free` flag here: free models are always | ||
| // marked explicitly in the catalog, so a `free: true` model is intended | ||
| // to be usable without credits. Do not switch this to isModelTrulyFree. |
12 tasks
This was referenced May 28, 2026
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
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.
Summary
chat.tsonly fires when the first attempt is about to use LLMGateway env-var tokens; BYOK paths intentionally skip it so a $0-credits org can still use its own provider key.resolveProviderContext, which switched to env-var tokens without re-validating credits, allowing a primary BYOK attempt to fail over and bill the org viaused_mode="credits"with $0 balance.resolveProviderContextso the env-var fallback paths (credits and hybrid-no-key branches) refuse fallback when total available credits are non-positive, preserving the BYOK-with-$0 use case.chat.tsexplaining that the baremodelInfo.freeflag is intentional andisModelTrulyFreeshould not be substituted here.Test plan
pnpm test:unitpasses🤖 Generated with Claude Code
Summary by CodeRabbit