Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions api/spec/packages/aip-client-javascript/src/models/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3429,8 +3429,9 @@ export interface CreateCreditGrantRequest {
/**
* Idempotency key for the credit grant creation request.
*
* When provided, reusing the same key returns an HTTP 409 Conflict instead of
* creating a duplicate grant, which makes create requests safe to retry.
* Unique per customer: reusing the same key for the same customer returns an HTTP
* 409 Conflict instead of creating a duplicate grant, which makes create requests
* safe to retry. The same key may be reused across different customers.
*/
key?: string
}
Expand Down Expand Up @@ -3493,8 +3494,9 @@ export interface CreditGrant {
/**
* Idempotency key for the credit grant creation request.
*
* When provided, reusing the same key returns an HTTP 409 Conflict instead of
* creating a duplicate grant, which makes create requests safe to retry.
* Unique per customer: reusing the same key for the same customer returns an HTTP
* 409 Conflict instead of creating a duplicate grant, which makes create requests
* safe to retry. The same key may be reused across different customers.
*/
key?: string
/**
Expand Down Expand Up @@ -5944,8 +5946,9 @@ export interface CreateCreditGrantRequestInput {
/**
* Idempotency key for the credit grant creation request.
*
* When provided, reusing the same key returns an HTTP 409 Conflict instead of
* creating a duplicate grant, which makes create requests safe to retry.
* Unique per customer: reusing the same key for the same customer returns an HTTP
* 409 Conflict instead of creating a duplicate grant, which makes create requests
* safe to retry. The same key may be reused across different customers.
*/
key?: string
}
Expand Down Expand Up @@ -6008,8 +6011,9 @@ export interface CreditGrantInput {
/**
* Idempotency key for the credit grant creation request.
*
* When provided, reusing the same key returns an HTTP 409 Conflict instead of
* creating a duplicate grant, which makes create requests safe to retry.
* Unique per customer: reusing the same key for the same customer returns an HTTP
* 409 Conflict instead of creating a duplicate grant, which makes create requests
* safe to retry. The same key may be reused across different customers.
*/
key?: string
/**
Expand Down
5 changes: 3 additions & 2 deletions api/spec/packages/aip/src/customers/credits/grant.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,9 @@ model CreditGrant {
/**
* Idempotency key for the credit grant creation request.
*
* When provided, reusing the same key returns an HTTP 409 Conflict instead of
* creating a duplicate grant, which makes create requests safe to retry.
* Unique per customer: reusing the same key for the same customer returns an HTTP
* 409 Conflict instead of creating a duplicate grant, which makes create requests
* safe to retry. The same key may be reused across different customers.
*/
@visibility(Lifecycle.Read, Lifecycle.Create)
key?: Shared.ExternalResourceKey;
Expand Down
1,471 changes: 737 additions & 734 deletions api/v3/api.gen.go

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions api/v3/client/models_customers.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions api/v3/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5440,8 +5440,9 @@ components:
description: |-
Idempotency key for the credit grant creation request.

When provided, reusing the same key returns an HTTP 409 Conflict instead of
creating a duplicate grant, which makes create requests safe to retry.
Unique per customer: reusing the same key for the same customer returns an HTTP
409 Conflict instead of creating a duplicate grant, which makes create requests
safe to retry. The same key may be reused across different customers.
expires_at:
allOf:
- $ref: '#/components/schemas/DateTime'
Expand Down Expand Up @@ -9432,8 +9433,9 @@ components:
description: |-
Idempotency key for the credit grant creation request.

When provided, reusing the same key returns an HTTP 409 Conflict instead of
creating a duplicate grant, which makes create requests safe to retry.
Unique per customer: reusing the same key for the same customer returns an HTTP
409 Conflict instead of creating a duplicate grant, which makes create requests
safe to retry. The same key may be reused across different customers.
additionalProperties: false
description: CreditGrant create request.
CreateCreditGrantTaxConfig:
Expand Down
16 changes: 10 additions & 6 deletions e2e/customer_credits_v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,18 @@ func TestV3CreateCreditGrantIdempotencyKey(t *testing.T) {
c.requireStatus(http.StatusCreated, err)
})

t.Run("the same key conflicts across different customers in a namespace", func(t *testing.T) {
t.Run("the same key is allowed across different customers", func(t *testing.T) {
key := ulid.Make().String()

_, err := c.Customers.Credits.Grants.Create(t.Context(), createCustomer("credit_grant_idem_cust_a"), grant(&key))
grantA, err := c.Customers.Credits.Grants.Create(t.Context(), createCustomer("credit_grant_idem_cust_a"), grant(&key))
c.requireStatus(http.StatusCreated, err)
require.NotNil(t, grantA)

_, err = c.Customers.Credits.Grants.Create(t.Context(), createCustomer("credit_grant_idem_cust_b"), grant(&key))
requireProblem(t, err, http.StatusConflict)
grantB, err := c.Customers.Credits.Grants.Create(t.Context(), createCustomer("credit_grant_idem_cust_b"), grant(&key))
c.requireStatus(http.StatusCreated, err)
require.NotNil(t, grantB)

require.NotEqual(t, grantA.ID, grantB.ID)
})

t.Run("an over-length key is rejected with 400", func(t *testing.T) {
Expand Down Expand Up @@ -331,8 +335,8 @@ func TestV3VoidCreditGrantPaymentAdjustmentNone(t *testing.T) {

// TestV3CreditGrantKeyReadAndFilter verifies that the idempotency key is exposed
// on the get/list read responses and that list credit grants can be filtered by
// key. The key column carries a unique partial index per namespace, so an
// equality filter targets at most one grant.
// key. The key column carries a unique partial index per namespace and customer,
// so a customer-scoped equality filter targets at most one grant.
func TestV3CreditGrantKeyReadAndFilter(t *testing.T) {
c := newV3Client(t)
currency := v3sdk.BillingCurrencyCode("USD")
Expand Down
4 changes: 2 additions & 2 deletions openmeter/ent/db/migrate/schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions openmeter/ent/schema/chargescreditpurchase.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ func (ChargeCreditPurchase) Indexes() []ent.Index {
return []ent.Index{
index.Fields("tax_code_id").
StorageKey("chargecreditpurchases_tax_code_id"),
// Idempotency key, unique per namespace. Partial so it is enforced only while live:
// NULL means no idempotency requested, and a soft-deleted grant must not permanently
// reserve a key the caller may reuse.
index.Fields("namespace", "key").
// Idempotency key, unique per customer within a namespace. Partial so it is enforced
// only while live: NULL means no idempotency requested, and a soft-deleted grant must
// not permanently reserve a key the caller may reuse.
index.Fields("namespace", "customer_id", "key").
Annotations(
entsql.IndexWhere("key IS NOT NULL AND deleted_at IS NULL"),
).
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-- 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

Comment thread
tothandras marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -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

-- create index "chargecreditpurchase_namespace_customer_id_key" to table: "charge_credit_purchases"
-- the old (namespace, key) unique index guarantees no (namespace, customer_id, key) duplicates exist,
-- so widening the key scope cannot fail on existing data
-- atlas:nolint MF101
CREATE UNIQUE INDEX "chargecreditpurchase_namespace_customer_id_key" ON "charge_credit_purchases" ("namespace", "customer_id", "key") WHERE ((key IS NOT NULL) AND (deleted_at IS NULL));
3 changes: 2 additions & 1 deletion tools/migrate/migrations/atlas.sum
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
h1:pD8amlaJqQ4l47KWTH/dmX/J0xSjGUAoxkQ0ZxOjFTk=
h1:sHp6EpaiKRdL9B9trZ/xwvONr0fdPUZGrOmNafRNcYQ=
20240826120919_init.up.sql h1:tc1V91/smlmaeJGQ8h+MzTEeFjjnrrFDbDAjOYJK91o=
20240903155435_entitlement-expired-index.up.sql h1:Hp8u5uckmLXc1cRvWU0AtVnnK8ShlpzZNp8pbiJLhac=
20240917172257_billing-entities.up.sql h1:Q1dAMo0Vjiit76OybClNfYPGC5nmvov2/M2W1ioi4Kw=
Expand Down Expand Up @@ -236,3 +236,4 @@ h1:pD8amlaJqQ4l47KWTH/dmX/J0xSjGUAoxkQ0ZxOjFTk=
20260716150454_create_gathering_detail_cleanup_indexes.up.sql h1:5d1aWsv+0mzERisQ5Ht29pbjsiOrK2Qpou6x9BgCli4=
20260716150455_cleanup_gathering_invoice_details.up.sql h1:W4u33tHXJy+GfryC8j46bHi2SiQwDk6brBOex/fQ4rw=
20260716150456_drop_gathering_detail_cleanup_indexes.up.sql h1:IJ6P54dqEy/LFG9hID2T9yBoN29FRTH/iLTSSTkdpDQ=
20260716152332_rescope-credit-grant-key-to-customer.up.sql h1:gbeATYlchOi0pwaTUFX+nrbLoS0jZbVDMHn8uMTck0c=
Loading