feat: host-key daily budget and free-debate guards#67
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds per-grant free-debate call budgets, UTC host-key daily spend tracking, provider credential detection, and reservation-aware key resolution. Consensus and OCR flows pass host-call units, while tests, schema migrations, environment generation, and documentation reflect the new behavior. ChangesHost-key budgeted access
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Request as Request route
participant Resolver as resolveUserProviderApiKey
participant Spend as tryReserveHostSpend
participant Free as tryConsumeFreeServerAccess
Request->>Resolver: provider request with units
Resolver->>Spend: reserve estimated host spend
Spend-->>Resolver: reservation result
Resolver->>Free: consume free call after reservation
Free-->>Resolver: access result
Resolver-->>Request: provider key or blocked response
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR introduces an app-enforced daily spend cap for host (server) API keys and tightens “free debate” grants so they can’t be abused across multiple debates/requests, while preserving BYOK precedence.
Changes:
- Added host-key daily budget tracking/reservation (UTC-day rollup) and enforcement for server-key usage.
- Reworked free-debate grants to include a per-grant host-call budget plus peek-vs-consume semantics.
- Updated server-key resolution and consensus candidate scanning to avoid claiming grants during provider scanning.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/server-provider-keys.ts | Adds host-cred gating, free-debate peek/consume option, and host spend reservation on server-key resolution. |
| src/lib/host-spend.ts | New host-key daily budget logic backed by HostSpendDay DB row. |
| src/lib/host-credentials.ts | New helper to detect whether host credentials exist for a provider. |
| src/lib/free-debates.ts | Adds per-grant call budget and new peek/consume APIs for free server access. |
| src/lib/consensus-resolve.ts | Uses peek-only server-key availability while scanning consensus providers. |
| src/components/SettingsModal.tsx | Adjusts free-debate status typing to optionally include call budget. |
| src/app/api/consensus/route.ts | Reserves host spend before host-key consensus provider calls. |
| src/tests/host-spend.test.ts | Adds unit tests for host spend budgeting/reservation behavior. |
| src/tests/free-debates.test.ts | Updates tests for new peek/consume behavior and call budget semantics. |
| src/tests/byok-route-guards.test.ts | Updates guard tests for new free-debate and host-budget mocks/paths. |
| scripts/write-env.js | Adds HOST_KEY_DAILY_BUDGET_USD to generated env set. |
| README.md | Documents free-debate changes and host-key daily budget env var. |
| prisma/schema.prisma | Adds freeDebateCallsRemaining and introduces HostSpendDay model. |
| prisma/migrations/20260715120000_free_debate_call_budget/migration.sql | Adds DB column/table for call budget and spend tracking. |
| .env.example | Documents HOST_KEY_DAILY_BUDGET_USD configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3ab750b06
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 177-181: Update the provider-key usage statement around the
earlier authentication setup instructions to also mention host-key usage for
access codes and free grants, while retaining the existing
REQUIRE_USER_API_KEYS=false condition for other provider-key behavior. Keep the
wording consistent with the documented host-key requirements and priorities.
In `@src/__tests__/byok-route-guards.test.ts`:
- Around line 683-690: Update the test “reserves budget before consuming an open
free window” to assert call ordering, not merely that both methods were invoked.
Track invocation events or use each mock’s call-order metadata to verify
tryReserveHostSpend executes before tryConsumeFreeServerAccess, while preserving
the existing argument and response assertions.
In `@src/__tests__/host-spend.test.ts`:
- Around line 1-99: Extend the host spend test suite to cover releaseHostSpend
and getHostSpendStatus. Add a release test that verifies the rollback update
targets the same utcSpendDay() value as reservation, including the day-boundary
behavior, and add status assertions covering the relevant budget and usage data.
Reuse the existing Prisma mocks and environment setup rather than introducing
new test infrastructure.
In `@src/app/api/consensus/route.ts`:
- Around line 271-294: The candidate-building flow must recognize users with
unopened free grants, not only already-active windows. Update the candidate
eligibility logic that currently uses claimFree: false to perform a read-only
active-or-claimable check, allowing such candidates to reach the host-key path;
keep tryConsumeFreeServerAccess in that path as the atomic consumption step.
- Around line 458-464: Update the consensus error-handling flow around lastError
and the host_budget_exceeded response to track the provider candidate that
produced lastError. Return that tracked provider in the JSON response instead of
order[0], while preserving the existing fallback provider and status code when
no candidate is available.
In `@src/app/api/ocr/route.ts`:
- Around line 79-80: Validate images as a bounded array containing only valid
image strings before the resolver call that passes { units: images.length }.
Reject malformed or oversized input before reserving host spend or consuming a
grant, while preserving the existing flatMap processing for valid arrays.
In `@src/lib/host-spend.ts`:
- Around line 91-103: Update releaseHostSpend to accept the reserved spend day
as an argument and use it in the hostSpendDay.updateMany query instead of
recomputing utcSpendDay(). Capture and pass that day from the reserve flow so
compensation always targets the same row, while preserving the existing
validation and error handling.
In `@src/lib/server-provider-keys.ts`:
- Around line 123-140: Make post-reservation free-access enforcement fail closed
and exception-safe in src/lib/server-provider-keys.ts lines 123-140 and
src/app/api/consensus/route.ts lines 278-290: when re-authentication yields no
user identity, release the reserved host spend and return the existing no_key
response; wrap authentication and free-access consumption so any exception
releases the reservation before propagating it, while preserving successful
provider dispatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 600928c1-febc-4aab-90b0-0fc66d357c6d
📒 Files selected for processing (17)
.env.exampleREADME.mdprisma/migrations/20260715120000_free_debate_call_budget/migration.sqlprisma/migrations/20260715130000_backfill_free_debate_calls/migration.sqlprisma/schema.prismascripts/write-env.jssrc/__tests__/byok-route-guards.test.tssrc/__tests__/free-debates.test.tssrc/__tests__/host-spend.test.tssrc/app/api/consensus/route.tssrc/app/api/ocr/route.tssrc/components/SettingsModal.tsxsrc/lib/consensus-resolve.tssrc/lib/free-debates.tssrc/lib/host-credentials.tssrc/lib/host-spend.tssrc/lib/server-provider-keys.ts
📜 Review details
🧰 Additional context used
🪛 Squawk (2.59.0)
prisma/migrations/20260715120000_free_debate_call_budget/migration.sql
[warning] 2-2: Using 32-bit integer fields can result in hitting the max int limit. Use 64-bit integer values instead to prevent hitting this limit.
(prefer-bigint-over-int)
[warning] 7-7: Using 32-bit integer fields can result in hitting the max int limit. Use 64-bit integer values instead to prevent hitting this limit.
(prefer-bigint-over-int)
[warning] 8-8: When Postgres stores a datetime in a timestamp field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time. Use timestamptz instead of timestamp for your column type.
(prefer-timestamp-tz)
🔇 Additional comments (16)
src/lib/server-provider-keys.ts (1)
1-6: LGTM!Also applies to: 26-99
src/__tests__/byok-route-guards.test.ts (2)
6-9: LGTM!Also applies to: 31-40, 133-137, 595-596, 653-681, 692-735
26-28: 📐 Maintainability & Code QualityNo change needed for
tryClaimFreeServerAccesssrc/lib/free-debates.tsstill exports it as a deprecated alias oftryConsumeFreeServerAccess, so the test mock matches the production API.> Likely an incorrect or invalid review comment.prisma/schema.prisma (1)
17-20: LGTM!Also applies to: 29-35
src/lib/host-credentials.ts (1)
1-31: LGTM!src/lib/consensus-resolve.ts (1)
4-4: LGTM!Also applies to: 62-66
prisma/migrations/20260715130000_backfill_free_debate_calls/migration.sql (1)
1-6: 🗄️ Data Integrity & IntegrationUse a UTC-safe expiration check
freeDebateExpiresAtis a plaintimestamp, sofreeDebateExpiresAt > CURRENT_TIMESTAMPdepends on the session time zone. If the DB session isn’t pinned to UTC, this backfill can select the wrong rows.src/lib/host-spend.ts (1)
60-89: 🗄️ Data Integrity & IntegrationCheck whether this
upsertpath is atomic.HostSpendDay.dayis a single unique key, so Prisma may already useON CONFLICT; if it falls back to a read-then-write upsert, concurrent first-day reservations can still surfaceP2002and get reported as over budget.prisma/migrations/20260715120000_free_debate_call_budget/migration.sql (2)
1-2: 🎯 Functional CorrectnessConfirm the backfill migration restores
freeDebateCallsRemainingfor users with an in-progress window.This migration defaults
freeDebateCallsRemainingto0for every existing row, including users who currently hold an activefreeDebateExpiresAtwindow. Per the newisActiveWindowcheck (freeDebateCallsRemaining > 0), those users would read as inactive until the paired20260715130000_backfill_free_debate_callsmigration (not included in this file set) repopulates the value consistently with their prior window state.
5-11: 🗄️ Data Integrity & Integration
HostSpendDay.updatedAtis already Prisma-managed
TheHostSpendDaymodel declaresupdatedAt DateTime@updatedAt``, so theupsertcan omit it.> Likely an incorrect or invalid review comment..env.example (1)
36-39: LGTM!scripts/write-env.js (1)
13-13: LGTM!src/lib/free-debates.ts (2)
75-93: 🎯 Functional CorrectnessConfirm intent: exhausting a grant's call budget mid-window silently claims a fresh grant.
The claim
ORclause treatsfreeDebateCallsRemaining: { lte: 0 }the same as "no active window," so a user who burns through allFREE_DEBATE_MAX_CALLScalls beforefreeDebateExpiresAtelapses will immediately consume another unit offreeDebatesRemainingand get a fresh 30-minute window on their very next request — with no explicit signal to the caller that a second grant was just spent. If a single legitimately long debate can exceed 36 host calls, this quietly doubles the grant cost for one session. Worth confirming this is the intended tradeoff (rather than, say, surfacing an explicit "budget exhausted, please wait for a new grant" state).
4-55: LGTM!src/__tests__/free-debates.test.ts (1)
16-133: LGTM!src/components/SettingsModal.tsx (1)
418-422: LGTM!
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75256e8fc3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea8c32d607
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function hasServerCreds(provider: Provider): boolean { | ||
| switch (provider) { | ||
| case "gemini": | ||
| return Boolean(getConfiguredGeminiApiKey() || hasRealVertexProject()) |
There was a problem hiding this comment.
When Gemini is configured through Vertex with VERTEX_PROJECT_ID set but VERTEX_LOCATION missing or still your_vertex_location, this returns true even though getVertexConfig() rejects that setup before any provider call. Fresh evidence: the project placeholder check was added here, but getVertexConfig() still validates the location separately; in the free/access-code host path we reserve host spend and may consume a free call before constructing Vertex, so these misconfigured requests burn quota and then fail.
Useful? React with 👍 / 👎.
| export function estimateHostCallCents(provider: Provider, units = 1): number { | ||
| const n = Number.isFinite(units) ? Math.max(1, Math.floor(units)) : 1 | ||
| return ESTIMATE_CENTS[provider] * n |
There was a problem hiding this comment.
For host-key chat requests with very large message bodies, this still reserves only one fixed unit; /api/chat accepts the posted messages and the provider builders concatenate all message content into the prompt. A direct client can therefore send near-context-limit prompts that cost far more than the 3/10/8-cent reservation, so repeated large host-key chat calls can exceed HOST_KEY_DAILY_BUDGET_USD even though OCR now scales its reservation by page count.
Useful? React with 👍 / 👎.
| const enforced = await consumeFreeAfterReserve( | ||
| provider, | ||
| logLabel, | ||
| reserved.cents, | ||
| reserved.day |
There was a problem hiding this comment.
For a signed-in free-grant request to /api/ocr with multiple images, the spend reservation is scaled by images.length, but the free grant still burns only one call here. Fresh evidence: the OCR fix passes { units: images.length } only into tryReserveHostSpend, while consumeFreeAfterReserve has no unit count, so a direct client can process up to 20 pages per request and stretch the 36-call per-user free budget into hundreds of OCR pages.
Useful? React with 👍 / 👎.
App-side host-key daily budget plus tighter free-debate guards. Review fixes: 75256e8 then ea8c32d (eligibility for unopened free grants, reserve-day refunds, OCR input validation, fail-closed free enforce, consensus 402 mapping, tests).