feat(admins): POST /api/admins/credits sets a balance and records why (chat#1948) - #824
Conversation
… (chat#1948) Every route under /api/admins/credits was a GET, so topping up an account meant a direct database write with no record of who did it or why. 155 accounts sit at or below zero with no supported way to unblock any of them. - POST /api/admins/credits sets an absolute balance, gated on Recoup org membership via validateAdminAuth. reason is required and non-empty; granted_by comes from credentials and a granted_by in the body is ignored. - The balance move and the grant record go through the new grant_credits_with_audit function so they cannot drift apart. A balance that moved without the grant explaining it is the exact failure this endpoint exists to remove. - The response carries expires_at, so the one-month reset window is a value the caller can act on rather than a footnote. - GET /api/admins/credits/events returns a grants array beside events, so a hand-set balance is attributable through the endpoint that already answers "where did this account's credits go". - insertCreditsUsage takes an optional timestamp so a grant that creates the first credits row starts the refill clock at a known moment. Omitted by every existing caller, whose behaviour is unchanged. Implements items 1-3 of recoupable/chat#1948. Contract: recoupable/docs#295. Schema: recoupable/database#55.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR adds an authenticated admin credit-grant endpoint. It validates requests, records audited balance updates, calculates grant expiration, and returns credit grants alongside usage events. ChangesAdmin credit management
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AdminClient
participant AdminCreditsRoute
participant postAdminCreditsHandler
participant validateGrantCreditsRequest
participant grantCreditsWithAudit
participant Supabase
AdminClient->>AdminCreditsRoute: POST credit grant
AdminCreditsRoute->>postAdminCreditsHandler: Forward request
postAdminCreditsHandler->>validateGrantCreditsRequest: Authenticate and validate
validateGrantCreditsRequest-->>postAdminCreditsHandler: Normalized request
postAdminCreditsHandler->>grantCreditsWithAudit: Grant credits with audit data
grantCreditsWithAudit->>Supabase: Call grant_credits_with_audit RPC
Supabase-->>grantCreditsWithAudit: Recorded grant row
grantCreditsWithAudit-->>postAdminCreditsHandler: Grant details
postAdminCreditsHandler-->>AdminClient: Grant response with expiration
Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
lib/supabase/credit_grants/grantCreditsWithAudit.ts (1)
32-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename this Supabase operation to use an allowed operation prefix.
grantCreditsWithAudit.tsuses thegrantprefix. Rename the file and exported function to anupdate*operation name, such asupdateCreditGrantsWithAudit, and update its imports.As per coding guidelines and path instructions, “Name Supabase operation files using
select[TableName].ts,insert[TableName].ts,update[TableName].ts,delete[TableName].ts, orget[Descriptive].tsas appropriate.”🤖 Prompt for 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. In `@lib/supabase/credit_grants/grantCreditsWithAudit.ts` around lines 32 - 52, Rename the Supabase operation file and exported function grantCreditsWithAudit to updateCreditGrantsWithAudit, preserving its existing RPC behavior and return handling. Update every import and call site to use the new symbol and align the filename with the allowed update[TableName] operation convention.Sources: Coding guidelines, Path instructions
lib/admins/credits/validateGrantCreditsRequest.ts (1)
37-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the oversized request-flow functions into focused helpers.
Both functions exceed the 20-line limit and combine orchestration with parsing, lookup, or response construction.
lib/admins/credits/validateGrantCreditsRequest.ts#L37-L73: extract JSON parsing and validation-error response creation into private helpers.lib/admins/credits/postAdminCreditsHandler.ts#L17-L62: extract account lookup and grant-response serialization into private helpers.As per coding guidelines, “Flag functions longer than 20 lines” and “Keep functions small and focused.”
🤖 Prompt for 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. In `@lib/admins/credits/validateGrantCreditsRequest.ts` around lines 37 - 73, Split the oversized request flows into focused private helpers: in lib/admins/credits/validateGrantCreditsRequest.ts lines 37-73, extract JSON parsing and validation-error response construction while preserving existing responses and orchestration; in lib/admins/credits/postAdminCreditsHandler.ts lines 17-62, extract account lookup and grant-response serialization into private helpers, keeping the handler’s behavior unchanged.Source: Coding guidelines
lib/admins/credits/getAdminCreditsEventsHandler.ts (2)
30-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the handler before adding more retrieval logic.
getAdminCreditsEventsHandlerspans 47 lines and combines validation, period calculation, pagination, three database reads, response construction, and error handling. Extract the parallel data retrieval or response construction into a focused helper.As per coding guidelines: “Flag functions longer than 20 lines” and “Keep functions small and focused.”
🤖 Prompt for 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. In `@lib/admins/credits/getAdminCreditsEventsHandler.ts` around lines 30 - 42, Split getAdminCreditsEventsHandler into focused helpers, extracting either the parallel database retrieval or the response construction while keeping validation, period calculation, pagination, and error handling behavior unchanged. Ensure the handler delegates to the new helper and remains within the small-function guideline.Source: Coding guidelines
30-42: 🩺 Stability & Availability | 🔵 TrivialVerify database#55 is deployed before this endpoint.
selectCreditGrantsnow runs for every request. If thecredit_grantsmigration is absent, the selector rethrows the query error and this handler returns500, including for accounts with no grants. Complete migration and preview verification before deployment.As per PR objectives: preview verification remains pending database#55.
🤖 Prompt for 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. In `@lib/admins/credits/getAdminCreditsEventsHandler.ts` around lines 30 - 42, Before enabling the `selectCreditGrants` call in the `Promise.all` within the handler, verify that database migration `#55` is deployed and confirmed in preview; do not deploy this endpoint while the `credit_grants` migration remains unavailable.
🤖 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 `@lib/admins/credits/validateGrantCreditsRequest.ts`:
- Around line 7-18: Remove account_id from grantCreditsBodySchema and update the
grant-credits handler contract to obtain the target account exclusively from
validated authentication context or another trusted server-side workflow. Ensure
account selection no longer depends on user-supplied request-body data while
preserving validation for remaining_credits and reason.
In `@lib/supabase/credits_usage/insertCreditsUsage.ts`:
- Around line 27-30: Update the timestamp conditional in the insert payload of
insertCreditsUsage to check specifically for undefined rather than truthiness,
ensuring supplied empty-string values are preserved and validated by the
database instead of omitted.
---
Nitpick comments:
In `@lib/admins/credits/getAdminCreditsEventsHandler.ts`:
- Around line 30-42: Split getAdminCreditsEventsHandler into focused helpers,
extracting either the parallel database retrieval or the response construction
while keeping validation, period calculation, pagination, and error handling
behavior unchanged. Ensure the handler delegates to the new helper and remains
within the small-function guideline.
- Around line 30-42: Before enabling the `selectCreditGrants` call in the
`Promise.all` within the handler, verify that database migration `#55` is deployed
and confirmed in preview; do not deploy this endpoint while the `credit_grants`
migration remains unavailable.
In `@lib/admins/credits/validateGrantCreditsRequest.ts`:
- Around line 37-73: Split the oversized request flows into focused private
helpers: in lib/admins/credits/validateGrantCreditsRequest.ts lines 37-73,
extract JSON parsing and validation-error response construction while preserving
existing responses and orchestration; in
lib/admins/credits/postAdminCreditsHandler.ts lines 17-62, extract account
lookup and grant-response serialization into private helpers, keeping the
handler’s behavior unchanged.
In `@lib/supabase/credit_grants/grantCreditsWithAudit.ts`:
- Around line 32-52: Rename the Supabase operation file and exported function
grantCreditsWithAudit to updateCreditGrantsWithAudit, preserving its existing
RPC behavior and return handling. Update every import and call site to use the
new symbol and align the filename with the allowed update[TableName] operation
convention.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: df639b84-be78-48b5-b4be-fab346cd3353
⛔ Files ignored due to path filters (8)
lib/admins/credits/__tests__/getAdminCreditsEventsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/admins/credits/__tests__/postAdminCreditsHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/admins/credits/__tests__/validateGrantCreditsRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/credits/__tests__/getGrantExpiresAt.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/credit_grants/__tests__/grantCreditsWithAudit.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/credit_grants/__tests__/selectCreditGrants.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/supabase/credits_usage/__tests__/insertCreditsUsage.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**types/database.types.tsis excluded by none and included by none
📒 Files selected for processing (8)
app/api/admins/credits/route.tslib/admins/credits/getAdminCreditsEventsHandler.tslib/admins/credits/postAdminCreditsHandler.tslib/admins/credits/validateGrantCreditsRequest.tslib/credits/getGrantExpiresAt.tslib/supabase/credit_grants/grantCreditsWithAudit.tslib/supabase/credit_grants/selectCreditGrants.tslib/supabase/credits_usage/insertCreditsUsage.ts
There was a problem hiding this comment.
All reported issues were addressed across 16 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Preview verification and bot review turned up three real defects. 1. expires_at overstated the guarantee for month-end grants. setMonth(+1) is not the inverse of the reset's setMonth(-1): a Jan 31 grant reported Mar 3 while the reset actually fires around Mar 2, so an admin was told the balance was safe a day longer than it was. Now clamped to the last day of the target month (Jan 31 -> Feb 28), which is at or before the real reset in every case. Erring late is the only direction that misleads, so the tests pin that as a property across six dates, checked against a replica of the refill predicate. UTC accessors so the result does not move with the host timezone. (cubic P2) 2. insertCreditsUsage's optional timestamp was dead code. It was written before the INSERT branch moved into grant_credits_with_audit; nothing calls it with a timestamp now. Reverted the file and dropped its test. This also moots the truthiness-vs-undefined comment on the same line. (cubic P3, CodeRabbit) 3. selectCreditGrants ordered only by created_at, so grants written in the same instant came back in a non-deterministic order, which near the 500-row cap changes which rows return at all. Added the id DESC tiebreaker the sibling selectUsageEvents already has. (cubic P3) Also pins the 400 envelope in the validator tests, including missing_fields being absent on unparseable JSON, so a field moving to message cannot pass a status-only assertion. (cubic P3) Verified live on the preview before these changes; re-verifying after.
Preview verification — 2026-08-06Run against the preview built from this PR's head, with Two builds, because the first pass found real defects:
Credentials: a Privy bearer for Auth
Happy path
Deliberate failure probesAll 400s, each with Source of truthQueried Postgres directly rather than trusting the responses:
Read-back
Three defects found, all fixed in
|
The staff-facing credit grant. Third of three PRs for chat#1948.
Merge order: docs#295 → database#55 → this PR. database#55 is a hard dependency: this code calls
grant_credits_with_auditand readscredit_grants.Why
Every route under
/api/admins/creditsis aGET, and the internal writers tocredits_usageare only reachable from the Stripe webhook and the credits read path. So a top-up today is a direct database write that records nothing about who made it or why — indistinguishable from a Stripe top-up or a monthly reset minutes later. Measured on prod 2026-08-06: 155 accounts at or below zero, with no supported way for staff to unblock any of them.What ships
POST /api/admins/creditsexpires_at.GET /api/admins/credits/eventsgrantsarray besideevents.grant_credits_with_auditDecisions worth reviewing
The admin gate is
validateAdminAuth, notcanAccessAccount. The issue proposed reusingcanAccessAccountbecause it grantsRECOUP_ORG_IDmembers universal access. It does — but it also returns true whenever the caller merely shares any organization with the target (canAccessAccount.ts L40-44), which on a credits write would let two members of the same customer org grant each other credits.validateAdminAuth→checkIsAdmingates on Recoup org membership alone, and is what every other admin route already uses. Flagged on the issue as a decision callout.The two writes are atomic. Moving the balance and recording the grant are separate writes, and through PostgREST they can drift on partial failure — which is why
deduct_credits_with_auditexists on the debit side. Here the drift is the bug: a balance that moved with no grant row is the untraceable write this endpoint exists to remove.previous_creditsis captured inside the function so it cannot be stale by the time the balance moves.granted_bynever comes from the body. It is resolved from credentials; agranted_byin the request body is ignored, and there is a test pinning that.404 is checked, not left to the FK, so an unknown
account_idis a clean 404 rather than a 500 from a constraint violation — and nothing is written on that path.The one-month expiry is surfaced, not fixed. A grant bumps
credits_usage.timestamp, so the reset clock restarts and the balance gets a full month;expires_atcomes back in the response and the caveat is spelled out in the docs. Making a grant survive resets outright would mean teachingcheckAndResetCreditsabout grant provenance — a change to the read path every account hits, for a case re-granting already covers. Deliberate, and recorded as such on the issue.Verification
TDD throughout — every unit RED before GREEN, including the three new
grantsassertions on the existing events handler test, which failed against the unmodified handler before the change.tsc --noEmitclean for every file in this diff (the remaining errors are the pre-existinglib/triggerbaseline, untouched here).eslintandprettierclean.Not yet verified against the preview —
grant_credits_with_auditandcredit_grantsdo not exist in the database until database#55 is applied, so the live run happens after that lands. Results will be posted here as a documented-vs-actual matrix covering the happy path, 400 (non-UUID, negative, blank reason), 401, 403, 404, and the grant reading back through the events endpoint.Implements items 1-3 of recoupable/chat#1948.
Summary by cubic
Adds an admin endpoint to set an account’s credit balance and record who and why, and surfaces those grants in the events API. Requires
credit_grantsandgrant_credits_with_auditin the database; implements items 1–3 of recoupable/chat#1948.New Features
POST /api/admins/credits: setsremaining_credits, recordsgranted_byfrom credentials andreason, returnsexpires_at.validateAdminAuth; balance move and audit run together viagrant_credits_with_audit; returns 404 for unknownaccount_id.GET /api/admins/credits/events: adds agrantsarray for the same account/period; not counted intotal_count.Bug Fixes
expires_atto the last day of the target month and use UTC so it never overstates the reset window.selectCreditGrantsordering with anidDESC tiebreaker.timestampparam frominsertCreditsUsage.Written for commit 758725c. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes