Skip to content

feat: scope credit grant idempotency key to customer#4716

Merged
tothandras merged 1 commit into
mainfrom
feat/credit-grant-key-customer-scope
Jul 16, 2026
Merged

feat: scope credit grant idempotency key to customer#4716
tothandras merged 1 commit into
mainfrom
feat/credit-grant-key-customer-scope

Conversation

@tothandras

@tothandras tothandras commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Documentation
    • Clarified credit grant idempotency-key semantics: the key is unique per customer; reusing it for the same customer returns HTTP 409 Conflict instead of creating a duplicate, and the same key may be reused across different customers.
  • Bug Fixes
    • Updated idempotency enforcement so credit-purchase key uniqueness is scoped to customer (preventing duplicates within a customer while allowing cross-customer reuse).
  • Tests
    • Adjusted end-to-end coverage to verify the new cross-customer and customer-scoped behaviors.

Greptile Summary

This PR scopes credit-grant idempotency keys to each customer. The main changes are:

  • Replaces namespace-level uniqueness with namespace-and-customer uniqueness.
  • Updates the API documentation and generated client models.
  • Adds migration files and updates generated schema metadata.
  • Adds end-to-end coverage for reusing a key across customers.

Confidence Score: 5/5

This looks safe to merge.

  • The latest changes do not introduce a new blocking issue.
  • The schema, migration metadata, API contract, and tests consistently use customer-scoped uniqueness.
  • No distinct follow-up failure remains after accounting for the existing rollback report.

Important Files Changed

Filename Overview
tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.up.sql Replaces the namespace-scoped partial unique index with a customer-scoped index.
tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql Restores the former namespace-scoped index during rollback.
openmeter/ent/schema/chargescreditpurchase.go Updates the source Ent schema to enforce uniqueness by namespace, customer, and key.
openmeter/ent/db/migrate/schema.go Updates generated migration metadata to match the new unique index.
e2e/customer_credits_v3_test.go Verifies that different customers can create distinct grants with the same key.
api/spec/packages/aip/src/customers/credits/grant.tsp Documents customer-scoped idempotency behavior for credit grants.

Reviews (3): Last reviewed commit: "feat: scope credit grant idempotency key..." | Re-trigger Greptile

Context used (3)

  • Context used - CLAUDE.md (source)
  • Context used - AGENTS.md (source)
  • Context used - api/spec/AGENTS.md (source)

@tothandras
tothandras requested a review from a team as a code owner July 15, 2026 14:06
@tothandras tothandras added the release-note/feature Release note: Exciting New Features label Jul 15, 2026
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Credit 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.

Changes

Credit grant idempotency

Layer / File(s) Summary
Customer-scoped uniqueness enforcement
openmeter/ent/schema/chargescreditpurchase.go, tools/migrate/migrations/*rescope-credit-grant-key-to-customer*
The partial unique index now covers namespace, customer_id, and key, with a reverse migration restoring the previous scope.
Cross-customer idempotency validation
e2e/customer_credits_v3_test.go
End-to-end coverage allows the same key for different customers and verifies separate grant IDs.
Idempotency contract documentation
api/spec/packages/aip/..., api/spec/packages/aip-client-javascript/..., api/v3/api.gen.go, api/v3/client/models_customers.go
API specifications, generated models, and Swagger documentation describe same-customer conflicts and cross-customer key reuse.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: area/billing

Suggested reviewers: turip

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: scoping the credit grant idempotency key to the customer.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/credit-grant-key-customer-scope

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

-- 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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

Fix in Claude Code Fix in Codex

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql (1)

1-7: 🩺 Stability & Availability | 🔵 Trivial

Both migration directions lock the table for the full index rebuild. Neither file uses CONCURRENTLY, so both the forward and reverse migrations take an ACCESS EXCLUSIVE lock on charge_credit_purchases for the whole drop+create. Per Atlas docs, going concurrent requires both a concurrent_index option in the diff config and an -- atlas:txmode none directive in the file (Postgres can't run CREATE/DROP INDEX CONCURRENTLY inside a transaction) - neither is present here.

  • tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql#L1-L7: if charge_credit_purchases is expected to be large/hot in production, consider adding -- atlas:txmode none and switching both statements to CONCURRENTLY, 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

📥 Commits

Reviewing files that changed from the base of the PR and between 0d94d28 and 122fe4d.

⛔ Files ignored due to path filters (3)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
  • openmeter/ent/db/migrate/schema.go is excluded by !**/ent/db/**
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (8)
  • api/spec/packages/aip-client-javascript/src/models/types.ts
  • api/spec/packages/aip/src/customers/credits/grant.tsp
  • api/v3/api.gen.go
  • api/v3/client/models_customers.go
  • e2e/customer_credits_v3_test.go
  • openmeter/ent/schema/chargescreditpurchase.go
  • tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.down.sql
  • tools/migrate/migrations/20260715085352_rescope-credit-grant-key-to-customer.up.sql

turip
turip previously approved these changes Jul 16, 2026
@tothandras
tothandras force-pushed the feat/credit-grant-key-customer-scope branch from 122fe4d to fb635f2 Compare July 16, 2026 15:31

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 122fe4d and fb635f2.

⛔ Files ignored due to path filters (3)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
  • openmeter/ent/db/migrate/schema.go is excluded by !**/ent/db/**
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (8)
  • api/spec/packages/aip-client-javascript/src/models/types.ts
  • api/spec/packages/aip/src/customers/credits/grant.tsp
  • api/v3/api.gen.go
  • api/v3/client/models_customers.go
  • e2e/customer_credits_v3_test.go
  • openmeter/ent/schema/chargescreditpurchase.go
  • tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql
  • tools/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";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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 || true

Repository: 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 || true

Repository: 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 || true

Repository: 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.
@tothandras
tothandras force-pushed the feat/credit-grant-key-customer-scope branch from fb635f2 to 75c0f14 Compare July 16, 2026 15:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql (1)

2-4: 🩺 Stability & Availability | 🔵 Trivial

Verify index locking for live migrations.

Normal DROP INDEX takes an ACCESS EXCLUSIVE lock, and normal CREATE INDEX can 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

📥 Commits

Reviewing files that changed from the base of the PR and between fb635f2 and 75c0f14.

⛔ Files ignored due to path filters (3)
  • api/v3/openapi.yaml is excluded by !**/openapi.yaml
  • openmeter/ent/db/migrate/schema.go is excluded by !**/ent/db/**
  • tools/migrate/migrations/atlas.sum is excluded by !**/*.sum, !**/*.sum
📒 Files selected for processing (8)
  • api/spec/packages/aip-client-javascript/src/models/types.ts
  • api/spec/packages/aip/src/customers/credits/grant.tsp
  • api/v3/api.gen.go
  • api/v3/client/models_customers.go
  • e2e/customer_credits_v3_test.go
  • openmeter/ent/schema/chargescreditpurchase.go
  • tools/migrate/migrations/20260716152332_rescope-credit-grant-key-to-customer.down.sql
  • tools/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

@tothandras
tothandras enabled auto-merge (squash) July 16, 2026 15:57
@tothandras
tothandras merged commit f1eac14 into main Jul 16, 2026
30 checks passed
@tothandras
tothandras deleted the feat/credit-grant-key-customer-scope branch July 16, 2026 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/feature Release note: Exciting New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants