Replica-safe credits check for plugins#1592
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a replica-safe usage-credits flag: a boolean Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin Endpoint
participant Replica as Read Replica
participant Primary as Primary DB
participant Cron as Scheduler
Plugin->>Replica: SELECT orgs.has_usage_credits ...
alt has_usage_credits = true
Replica-->>Plugin: allow request / proceed
else has_usage_credits = false
Replica-->>Plugin: deny or require billing flow (e.g., 429)
end
note over Primary,Cron: Writes and syncs happen only on Primary
Cron->>Primary: CALL public.refresh_orgs_has_usage_credits()
Primary->>Primary: backfill/compute has_usage_credits from grants
Primary->>Replica: replication streams updated has_usage_credits
Primary-->>Primary: trigger on usage_credit_grants -> sync function updates orgs flag
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: 81d66a075a
ℹ️ 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".
| -- Run daily after credits expiry (03:00:30 UTC) so replicas get a stable replicated flag. | ||
| INSERT INTO "public"."cron_tasks" ( | ||
| "name", | ||
| "description", | ||
| "task_type", |
There was a problem hiding this comment.
The new replica-safe flag is only refreshed by a daily cron task, so any credit purchase or consumption after that run will leave orgs.has_usage_credits stale until the next day. Because buildPlanValidationExpression now relies on this flag for plugin read-path gating, orgs can be incorrectly blocked from updates after topping up (false negatives) or continue to receive updates after exhausting credits (false positives) for up to 24 hours. Consider updating the flag transactionally when credits change or running the refresh much more frequently to keep plan enforcement accurate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the plugin read-path plan/credits validation to be safe on read replicas (which don’t replicate views/functions) by introducing a replicated orgs.has_usage_credits flag and refreshing it via a scheduled job, plus documenting the replica data contract for plugin endpoints.
Changes:
- Add
orgs.has_usage_creditsreplicated boolean + a daily refresh function and cron task registration. - Update backend plan validation SQL to rely on the replicated flag (with a backward-compatible fallback for replicas missing the new column).
- Document replica-safe querying rules for
/updates,/stats,/channel_self.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260206213247_org_has_usage_credits_flag.sql | Adds replicated org credits flag + refresh function + cron task entry. |
| supabase/functions/_backend/utils/postgres_schema.ts | Exposes orgs.has_usage_credits in Drizzle schema. |
| supabase/functions/_backend/utils/pg.ts | Switches plan validation from usage_credit_balances to replica-safe org flag (with column-missing fallback). |
| bun.lock | Adds configVersion field to the Bun lockfile. |
| AGENTS.md | Documents replica data contract for plugin read-path endpoints. |
| // IMPORTANT: read replicas replicate table data but not views/functions. | ||
| // Keep this expression replica-safe by relying on the replicated org flag. | ||
| // | ||
| // Also keep it backward compatible with replicas that haven't replicated the | ||
| // new column yet: we read it via `to_jsonb(row)->>'has_usage_credits'` so the | ||
| // query still parses even if the column doesn't exist. | ||
| const orgCreditsAlias = alias(schema.orgs, 'org_credits') | ||
| const hasCreditsExpression = sql`EXISTS ( | ||
| SELECT 1 | ||
| FROM public.usage_credit_balances ucb | ||
| WHERE ucb.org_id = ${ownerColumn} | ||
| AND COALESCE(ucb.available_credits, 0) > 0 | ||
| FROM ${orgCreditsAlias} | ||
| WHERE ${orgCreditsAlias.id} = ${ownerColumn} | ||
| AND COALESCE((to_jsonb(org_credits) ->> 'has_usage_credits')::boolean, false) = true | ||
| )` |
There was a problem hiding this comment.
This changes the plan validation logic for plugin endpoints (/updates, /stats, /channel_self) from a live balance view to a replicated flag. There should be a regression test that covers “credits grant enables access” and “credits depleted disables access” (and ideally the “column missing on replica defaults false” fallback) to prevent future changes from accidentally reintroducing replica-only relations or breaking plan gating.
| -- Update orgs that have a row in the balances view. | ||
| UPDATE "public"."orgs" AS o | ||
| SET "has_usage_credits" = (COALESCE(b."available_credits", 0) > 0) | ||
| FROM "public"."usage_credit_balances" AS b | ||
| WHERE b."org_id" = o."id"; |
There was a problem hiding this comment.
The daily refresh function updates every org with a balance row even when the boolean value doesn’t change. This causes unnecessary writes/WAL and extra replication churn. Add a predicate to only update rows where has_usage_credits differs from the computed value.
| -- Update orgs that have a row in the balances view. | |
| UPDATE "public"."orgs" AS o | |
| SET "has_usage_credits" = (COALESCE(b."available_credits", 0) > 0) | |
| FROM "public"."usage_credit_balances" AS b | |
| WHERE b."org_id" = o."id"; | |
| -- Update orgs that have a row in the balances view, but only when the flag changes. | |
| UPDATE "public"."orgs" AS o | |
| SET "has_usage_credits" = (COALESCE(b."available_credits", 0) > 0) | |
| FROM "public"."usage_credit_balances" AS b | |
| WHERE b."org_id" = o."id" | |
| AND o."has_usage_credits" IS DISTINCT FROM (COALESCE(b."available_credits", 0) > 0); |
| CREATE OR REPLACE FUNCTION "public"."refresh_orgs_has_usage_credits"() | ||
| RETURNS void | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| SET search_path = '' | ||
| AS $$ | ||
| BEGIN |
There was a problem hiding this comment.
This SECURITY DEFINER function doesn’t set an explicit owner (many other internal cron functions are ALTERed to OWNER TO postgres). Setting the owner explicitly helps keep privileges consistent across environments and avoids surprises if migrations run under a different role.
| -- Run daily after credits expiry (03:00:30 UTC) so replicas get a stable replicated flag. | ||
| INSERT INTO "public"."cron_tasks" ( | ||
| "name", | ||
| "description", | ||
| "task_type", | ||
| "target", | ||
| "run_at_hour", | ||
| "run_at_minute", | ||
| "run_at_second" | ||
| ) | ||
| VALUES ( | ||
| 'refresh_org_usage_credits_flag', | ||
| 'Refresh orgs.has_usage_credits from usage credit balances (replicated flag for read replicas)', | ||
| 'function', | ||
| 'public.refresh_orgs_has_usage_credits()', | ||
| 3, | ||
| 0, | ||
| 30 | ||
| ) |
There was a problem hiding this comment.
Refreshing orgs.has_usage_credits only once per day can make plan enforcement stale (e.g., org tops up credits but remains blocked until the next run, or consumes credits to 0 but keeps access until the next run). Since this flag is used on the plugin read-path, it should be kept near-real-time via primary-side triggers or by updating the flag as part of credit grant/consumption flows (keeping the daily refresh as a fallback for expiries).
| 0, | ||
| 30 | ||
| ) | ||
| ON CONFLICT ("name") DO NOTHING; |
There was a problem hiding this comment.
The cron_tasks registration uses ON CONFLICT DO NOTHING. If the task already exists (e.g., in a long-lived/staging DB), schedule/target changes in this migration won’t apply. Prefer ON CONFLICT (name) DO UPDATE to keep the task definition in sync.
| ON CONFLICT ("name") DO NOTHING; | |
| ON CONFLICT ("name") DO UPDATE | |
| SET | |
| "description" = EXCLUDED."description", | |
| "task_type" = EXCLUDED."task_type", | |
| "target" = EXCLUDED."target", | |
| "run_at_hour" = EXCLUDED."run_at_hour", | |
| "run_at_minute" = EXCLUDED."run_at_minute", | |
| "run_at_second" = EXCLUDED."run_at_second"; |
| AND COALESCE(ucb.available_credits, 0) > 0 | ||
| FROM ${orgCreditsAlias} | ||
| WHERE ${orgCreditsAlias.id} = ${ownerColumn} | ||
| AND COALESCE((to_jsonb(org_credits) ->> 'has_usage_credits')::boolean, false) = true |
There was a problem hiding this comment.
The SQL uses a hard-coded table alias identifier in to_jsonb(org_credits). This is fragile (a refactor of the alias string would silently break the query) and is easy to mismatch with the actual alias emitted by Drizzle. Prefer building the row reference from the same alias variable used in the FROM clause so the identifier stays consistent.
| AND COALESCE((to_jsonb(org_credits) ->> 'has_usage_credits')::boolean, false) = true | |
| AND COALESCE((to_jsonb(${orgCreditsAlias}) ->> 'has_usage_credits')::boolean, false) = true |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/plugin-credits-flag.test.ts`:
- Line 28: The Supabase-generated TypeScript types are out of date so the new
orgs column has_usage_credits is not present; regenerate the DB types and commit
them. Run the Supabase type generator (e.g. `supabase gen types typescript
--project-id ...` or your project’s configured command), update the committed
types file(s) referenced by your codebase (the generated Database types imported
by tests such as in tests/plugin-credits-flag.test.ts), then re-run the tests to
ensure references to has_usage_credits (including other occurrences around lines
46-50) type-check correctly.
🧹 Nitpick comments (4)
supabase/migrations/20260206213247_org_has_usage_credits_flag.sql (2)
14-34: Redundant second backfill UPDATE.The first
UPDATE(lines 14–24) already setshas_usage_creditsto the result ofEXISTS(...), which covers both thetrueandfalsecases for every row where the current value differs. The secondUPDATE(lines 27–34) is therefore a no-op — it only targets rows that are notfalseand have no grants, but those rows were already set tofalseby the first statement.Remove the redundant second UPDATE
UPDATE "public"."orgs" AS o SET "has_usage_credits" = EXISTS ( SELECT 1 FROM "public"."usage_credit_grants" AS g WHERE g."org_id" = o."id" ) WHERE o."has_usage_credits" IS DISTINCT FROM EXISTS ( SELECT 1 FROM "public"."usage_credit_grants" AS g WHERE g."org_id" = o."id" ); --- Ensure orgs without any grants are false (and avoid needless writes). -UPDATE "public"."orgs" AS o -SET "has_usage_credits" = false -WHERE NOT EXISTS ( - SELECT 1 - FROM "public"."usage_credit_grants" AS g - WHERE g."org_id" = o."id" -) -AND o."has_usage_credits" IS DISTINCT FROM false;
72-104: Minor: trigger function lacks explicitREVOKE/GRANTlike the refresh function.The refresh function has explicit
REVOKE ALL ... FROM PUBLIC+GRANT ... TO service_role, but the trigger function (sync_org_has_usage_credits_from_grants) does not. Since it's invoked only via the trigger (owned by postgres), this isn't a security hole, but adding matchingREVOKE/GRANTwould be consistent.tests/plugin-credits-flag.test.ts (2)
38-38: Useit.concurrent()for parallel test execution.Per coding guidelines, tests should use
it.concurrent()instead ofit().Fix
- it('allows /updates when has_usage_credits is true (replica-safe)', async () => { + it.concurrent('allows /updates when has_usage_credits is true (replica-safe)', async () => {As per coding guidelines:
tests/**/*.{ts,test.ts}: "Design tests for parallel execution; useit.concurrent()instead ofit()to run tests in parallel within the same file."
31-36: Manual cleanup inafterAllmay leave orphaned data on test failure.If
resetAppDatathrows, the subsequent deletes won't run. Consider wrapping each cleanup step independently or using try/catch to ensure all cleanup is attempted. Also note thatresetAppDataalready handles errors internally (doesn't throw), but the subsequent manual deletes could fail and prevent later ones from executing.
| .eq('customer_id', stripeCustomerId) | ||
|
|
||
| // Ensure default state is "no credits flag". | ||
| await supabase.from('orgs').update({ has_usage_credits: false }).eq('id', orgId) |
There was a problem hiding this comment.
Pipeline failure: has_usage_credits not in generated Supabase types.
The CI is failing with TS2353: 'has_usage_credits' does not exist in type. The Supabase-generated Database types need to be regenerated after the new migration so the orgs table type includes the new column. Run supabase gen types typescript (or equivalent) and commit the updated type definitions.
Also applies to: 46-50
🧰 Tools
🪛 GitHub Actions: Run tests
[error] 28-28: vue-tsc --noEmit: TS2353: Object literal may only specify known properties, and 'has_usage_credits' does not exist in type '{ created_at?: string | null | undefined; created_by?: string | undefined; customer_id?: string | null | undefined; email_preferences?: Json | undefined; enforce_encrypted_bundles?: boolean | undefined; ... 13 more ...; use_new_rbac?: boolean | undefined; }'.
🤖 Prompt for AI Agents
In `@tests/plugin-credits-flag.test.ts` at line 28, The Supabase-generated
TypeScript types are out of date so the new orgs column has_usage_credits is not
present; regenerate the DB types and commit them. Run the Supabase type
generator (e.g. `supabase gen types typescript --project-id ...` or your
project’s configured command), update the committed types file(s) referenced by
your codebase (the generated Database types imported by tests such as in
tests/plugin-credits-flag.test.ts), then re-run the tests to ensure references
to has_usage_credits (including other occurrences around lines 46-50) type-check
correctly.
|
Summary (AI generated)
public.usage_credit_balances(not present on read replicas).orgs.has_usage_creditsflag + daily refresh job to keep replica-safe “has credits” signal./updates,/stats,/channel_self.Test plan (AI generated)
bun run lintbun run lint:backendScreenshots (AI generated)
Checklist (AI generated)
bun run lint:backend && bun run lint.accordingly.
my tests
Generated with AI
Summary by CodeRabbit
Documentation
Infrastructure