feat: scope credit grant idempotency key to customer#4716
Conversation
📝 WalkthroughWalkthroughCredit grant idempotency keys are now unique per customer within a namespace. Same-customer reuse conflicts, while different customers may reuse the key. Database indexes, migrations, API documentation, generated models, Swagger content, and end-to-end tests reflect this behavior. ChangesCredit grant idempotency
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
| -- reverse: create index "chargecreditpurchase_namespace_customer_id_key" to table: "charge_credit_purchases" | ||
| DROP INDEX "chargecreditpurchase_namespace_customer_id_key"; | ||
| -- reverse: drop index "chargecreditpurchase_namespace_key" from table: "charge_credit_purchases" | ||
| CREATE UNIQUE INDEX "chargecreditpurchase_namespace_key" ON "charge_credit_purchases" ("namespace", "key") WHERE ((key IS NOT NULL) AND (deleted_at IS NULL)); |
There was a problem hiding this comment.
Rollback Can Violate Old Uniqueness
After the new schema allows two customers in one namespace to use the same key, rolling back tries to recreate the old unique (namespace, key) index. Any such records make this statement fail, leaving the rollback incomplete.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sql
Line: 4
Comment:
**Rollback Can Violate Old Uniqueness**
After the new schema allows two customers in one namespace to use the same key, rolling back tries to recreate the old unique `(namespace, key)` index. Any such records make this statement fail, leaving the rollback incomplete.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql (1)
1-7: 🩺 Stability & Availability | 🔵 TrivialBoth migration directions lock the table for the full index rebuild. Neither file uses
CONCURRENTLY, so both the forward and reverse migrations take anACCESS EXCLUSIVElock oncharge_credit_purchasesfor the whole drop+create. Per Atlas docs, going concurrent requires both aconcurrent_indexoption in the diff config and an-- atlas:txmode nonedirective in the file (Postgres can't runCREATE/DROP INDEX CONCURRENTLYinside a transaction) - neither is present here.
tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql#L1-L7: ifcharge_credit_purchasesis expected to be large/hot in production, consider adding-- atlas:txmode noneand switching both statements toCONCURRENTLY, or confirm the team has already accepted this lock window for a table this size.tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sql#L1-L4: same fix applies here if you go concurrent on the up side, so the down migration stays consistent.🤖 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 `@tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql` around lines 1 - 7, Both migration directions rebuild indexes under an ACCESS EXCLUSIVE lock. In tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql:1-7 and tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sql:1-4, either add the Atlas concurrent-index diff configuration and txmode none directives, then use CONCURRENTLY for both DROP INDEX and CREATE UNIQUE INDEX statements, or explicitly confirm the existing lock window is acceptable for production.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In
`@tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql`:
- Around line 1-7: Both migration directions rebuild indexes under an ACCESS
EXCLUSIVE lock. In
tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql:1-7
and
tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sql:1-4,
either add the Atlas concurrent-index diff configuration and txmode none
directives, then use CONCURRENTLY for both DROP INDEX and CREATE UNIQUE INDEX
statements, or explicitly confirm the existing lock window is acceptable for
production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c44c2f8-fb93-4212-9a0a-1a9f898e77a6
⛔ Files ignored due to path filters (3)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (8)
api/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/v3/api.gen.goapi/v3/client/models_customers.goe2e/customer_credits_v3_test.goopenmeter/ent/schema/chargescreditpurchase.gotools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sqltools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql
122fe4d to
fb635f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql`:
- Line 2: Update the migration to create the replacement unique index
concurrently before dropping "chargecreditpurchase_namespace_key"; then remove
the old index afterward. Ensure the Atlas migration is non-transactional so the
concurrent index creation is valid, preserving uniqueness while avoiding write
blocking.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7dd6b1a8-d009-4d54-b5da-18c829430be6
⛔ Files ignored due to path filters (3)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (8)
api/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/v3/api.gen.goapi/v3/client/models_customers.goe2e/customer_credits_v3_test.goopenmeter/ent/schema/chargescreditpurchase.gotools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sqltools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql
🚧 Files skipped from review as they are similar to previous changes (5)
- api/spec/packages/aip-client-javascript/src/models/types.ts
- e2e/customer_credits_v3_test.go
- openmeter/ent/schema/chargescreditpurchase.go
- api/v3/client/models_customers.go
- api/spec/packages/aip/src/customers/credits/grant.tsp
| @@ -0,0 +1,7 @@ | |||
| -- drop index "chargecreditpurchase_namespace_key" from table: "charge_credit_purchases" | |||
| DROP INDEX "chargecreditpurchase_namespace_key"; | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how this repository handles concurrent Atlas index migrations.
rg -n -C2 --glob '*.sql' \
'atlas:txmode|CREATE (UNIQUE )?INDEX CONCURRENTLY|DROP INDEX CONCURRENTLY' \
tools/migrate/migrations || trueRepository: openmeterio/openmeter
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the migration file and nearby migration conventions.
sed -n '1,120p' tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql
printf '\n--- DOWN ---\n'
sed -n '1,120p' tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql
printf '\n--- MIGRATION FILES WITH TX MODE / CONCURRENT INDEX REFERENCES ---\n'
rg -n -C2 'atlas:txmode|CONCURRENTLY|CREATE UNIQUE INDEX|DROP INDEX' tools/migrate/migrations || true
printf '\n--- FILE LIST AROUND THIS MIGRATION ---\n'
git ls-files 'tools/migrate/migrations/*rescope-credit-grant-key-to-customer*'Repository: openmeterio/openmeter
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- TARGET UP ---'
cat -n tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql
echo
echo '--- TARGET DOWN ---'
cat -n tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql
echo
echo '--- NARROW TXMODE / CONCURRENT SEARCH ---'
rg -n -C1 'atlas:txmode|CONCURRENTLY|CREATE UNIQUE INDEX|DROP INDEX' \
tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.* \
tools/migrate/migrations/*.sql || trueRepository: openmeterio/openmeter
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql
echo '--- FILE SIZE / LINE COUNT ---'
wc -l "$file"
echo
echo '--- FILE CONTENT ---'
cat -n "$file"
echo
echo '--- MATCHES IN THIS FILE ---'
rg -n 'atlas:txmode|CONCURRENTLY|CREATE UNIQUE INDEX|DROP INDEX' "$file" || true
echo
echo '--- NEARBY MIGRATIONS FOR THIS TABLE/INDEX NAME ---'
rg -n -C2 'chargecreditpurchase|credit-purchase|credit-grant|namespace_key' tools/migrate/migrations/202607*.sql tools/migrate/migrations/202606*.sql || trueRepository: openmeterio/openmeter
Length of output: 4659
Build the replacement index concurrently
This CREATE UNIQUE INDEX will block credit-purchase writes while it runs. The old (namespace, key) index already covers the wider uniqueness here, so build the new one first and drop the old index afterward in a non-transactional Atlas migration.
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 2-2: A normal DROP INDEX acquires an ACCESS EXCLUSIVE lock on the table, blocking other accesses until the index drop can complete. Drop the index CONCURRENTLY.
(require-concurrent-index-deletion)
🤖 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
`@tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql`
at line 2, Update the migration to create the replacement unique index
concurrently before dropping "chargecreditpurchase_namespace_key"; then remove
the old index afterward. Ensure the Atlas migration is non-transactional so the
concurrent index creation is valid, preserving uniqueness while avoiding write
blocking.
Sources: Coding guidelines, Path instructions, Linters/SAST tools
The credit grant idempotency key was unique per (namespace, key), so two different customers in the same namespace could not reuse the same key even though grant creation and key lookup are both customer-scoped. Re-scope the partial unique index to (namespace, customer_id, key) and document the scope on the API field. Loosening the constraint cannot fail on existing data: any rows violating the new index would have violated the old one.
fb635f2 to
75c0f14
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql (1)
2-4: 🩺 Stability & Availability | 🔵 TrivialVerify index locking for live migrations.
Normal
DROP INDEXtakes anACCESS EXCLUSIVElock, and normalCREATE INDEXcan block writes. If this runs against live traffic, confirm the migration runner’s transaction mode and table size; use Atlas-generated concurrent operations where supported, or schedule an appropriate maintenance window.🤖 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 `@tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql` around lines 2 - 4, Review the migration runner’s transaction mode and the size/live-traffic characteristics of charge_credit_purchases, then adjust the DROP INDEX and CREATE UNIQUE INDEX operations in this migration to use Atlas-supported concurrent operations where available. If concurrent execution is incompatible with the runner or transaction, document or enforce an appropriate maintenance window instead.Source: Linters/SAST tools
🤖 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
`@tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql`:
- Around line 3-4: Update the rollback migration before recreating
chargecreditpurchase_namespace_key to detect duplicate active, non-null keys
across customers, then either apply explicit cleanup/merge semantics or abort
with an actionable error identifying the conflicting records. Only drop and
recreate the namespace-wide unique index after the preflight check succeeds.
---
Nitpick comments:
In
`@tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql`:
- Around line 2-4: Review the migration runner’s transaction mode and the
size/live-traffic characteristics of charge_credit_purchases, then adjust the
DROP INDEX and CREATE UNIQUE INDEX operations in this migration to use
Atlas-supported concurrent operations where available. If concurrent execution
is incompatible with the runner or transaction, document or enforce an
appropriate maintenance window instead.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a437d35-775f-4c45-9a0f-41ef1e1eea8a
⛔ Files ignored due to path filters (3)
api/v3/openapi.yamlis excluded by!**/openapi.yamlopenmeter/ent/db/migrate/schema.gois excluded by!**/ent/db/**tools/migrate/migrations/atlas.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (8)
api/spec/packages/aip-client-javascript/src/models/types.tsapi/spec/packages/aip/src/customers/credits/grant.tspapi/v3/api.gen.goapi/v3/client/models_customers.goe2e/customer_credits_v3_test.goopenmeter/ent/schema/chargescreditpurchase.gotools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sqltools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql
🚧 Files skipped from review as they are similar to previous changes (6)
- api/spec/packages/aip/src/customers/credits/grant.tsp
- api/v3/client/models_customers.go
- e2e/customer_credits_v3_test.go
- openmeter/ent/schema/chargescreditpurchase.go
- api/spec/packages/aip-client-javascript/src/models/types.ts
- api/v3/api.gen.go
Summary by CodeRabbit
Greptile Summary
This PR scopes credit-grant idempotency keys to each customer. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "feat: scope credit grant idempotency key..." | Re-trigger Greptile
Context used (3)