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
Original file line number Diff line number Diff line change
Expand Up @@ -3969,6 +3969,7 @@ export const createCreditGrantRequest = z
'Draw-down priority of the grant. Lower values have higher priority.',
),
expires_after: iso8601Duration.optional(),
key: externalResourceKey.optional(),
})
.describe('CreditGrant create request.')

Expand Down Expand Up @@ -4011,6 +4012,7 @@ export const creditGrant = z
'Draw-down priority of the grant. Lower values have higher priority.',
),
expires_after: iso8601Duration.optional(),
key: externalResourceKey.optional(),
expires_at: dateTime.optional(),
voided_at: dateTime.optional(),
status: creditGrantStatus,
Expand Down
28 changes: 28 additions & 0 deletions api/spec/packages/aip-client-javascript/src/models/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3569,6 +3569,13 @@ export interface CreateCreditGrantRequest {
* Defaults to never expiring.
*/
expires_after?: string
/**
* 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.
*/
key?: string
}

/**
Expand Down Expand Up @@ -3626,6 +3633,13 @@ export interface CreditGrant {
* Defaults to never expiring.
*/
expires_after?: string
/**
* 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.
*/
key?: string
Comment on lines +3636 to +3642

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 Response Type Leaks Key
The server treats key as create-only, but the generated TypeScript response model still exposes it on CreditGrant. The OpenAPI response schema and Go response conversion do not serialize this field, so TS callers of create/get/list can read CreditGrant.key from the SDK type and always receive undefined at runtime. Please keep the field on CreateCreditGrantRequest only, or update the response contract and serializer to return it consistently.

Context Used: api/spec/AGENTS.md (source)

Prompt To Fix With AI
This is a comment left during a code review.
Path: api/spec/packages/aip-client-javascript/src/models/types.ts
Line: 3649-3655

Comment:
**Response Type Leaks Key**
The server treats `key` as create-only, but the generated TypeScript response model still exposes it on `CreditGrant`. The OpenAPI response schema and Go response conversion do not serialize this field, so TS callers of create/get/list can read `CreditGrant.key` from the SDK type and always receive `undefined` at runtime. Please keep the field on `CreateCreditGrantRequest` only, or update the response contract and serializer to return it consistently.

**Context Used:** api/spec/AGENTS.md ([source](https://app.greptile.com/openmeter/github/openmeterio/openmeter/-/custom-context?memory=28ba6068-00f9-4629-9b78-8e49cc802858))

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

/**
* The timestamp when the credit grant expires.
*
Expand Down Expand Up @@ -5432,6 +5446,13 @@ export interface CreateCreditGrantRequestInput {
* Defaults to never expiring.
*/
expires_after?: string
/**
* 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.
*/
key?: string
}

export interface CreditGrantInput {
Expand Down Expand Up @@ -5483,6 +5504,13 @@ export interface CreditGrantInput {
* Defaults to never expiring.
*/
expires_after?: string
/**
* 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.
*/
key?: string
/**
* The timestamp when the credit grant expires.
*
Expand Down
9 changes: 9 additions & 0 deletions api/spec/packages/aip/src/customers/credits/grant.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,15 @@ model CreditGrant {
@visibility(Lifecycle.Create)
expires_after?: Shared.ISO8601Duration;

/**
* 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.
Comment on lines +196 to +200

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify the idempotency scope in the docs.

The wording reads like any key reuse returns 409, but the supplied stack context says cross-customer reuse is allowed. Tightening this to the actual scope would keep the contract honest.

✏️ Suggested wording
- * When provided, reusing the same key returns an HTTP 409 Conflict instead of
- * creating a duplicate grant, which makes create requests safe to retry.
+ * When provided, 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.

As per path instructions, "The declared API should be accurate, in parity with the actual implementation, and easy to understand."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* 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.
/**
* Idempotency key for the credit grant creation request.
*
* When provided, 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.
🤖 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 `@api/spec/packages/aip/src/customers/credits/grant.tsp` around lines 196 -
200, The documentation for the credit grant idempotency key in grant.tsp
currently implies any key reuse always returns 409, which is broader than the
actual behavior. Update the comment near the credit grant request definition to
state the idempotency scope accurately: duplicate reuse should conflict only
within the same customer/context, while cross-customer reuse is allowed. Keep
the wording aligned with the request/response contract described by the grant
creation model so the docs match the implementation.

Source: Path instructions

*/
@visibility(Lifecycle.Create)
key?: Shared.ExternalResourceKey;

/**
* The timestamp when the credit grant expires.
*
Expand Down
683 changes: 346 additions & 337 deletions api/v3/api.gen.go

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions api/v3/handlers/customers/credits/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ func fromAPICreateCreditGrantRequest(ns string, customerID api.ULID, body api.Cr
FundingMethod: fromAPIBillingCreditFundingMethod(body.FundingMethod),
Priority: body.Priority,
Labels: lo.FromPtrOr(body.Labels, api.Labels{}),
Key: body.Key,
}

if body.ExpiresAfter != nil {
Expand Down
8 changes: 8 additions & 0 deletions api/v3/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8636,6 +8636,14 @@ components:
The duration after which the credit grant expires.

Defaults to never expiring.
key:
allOf:
- $ref: '#/components/schemas/ExternalResourceKey'
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.
additionalProperties: false
description: CreditGrant create request.
CreateCreditGrantTaxConfig:
Expand Down
65 changes: 65 additions & 0 deletions e2e/customer_credits_v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,68 @@ func TestV3CreateCreditGrantMissingTaxCode(t *testing.T) {
}
assert.True(t, mentionsTaxCode, "response should name the missing tax code, problem: %+v", problem)
}

func TestV3CreateCreditGrantIdempotencyKey(t *testing.T) {
c := newV3Client(t)
currency := apiv3.CurrencyCode("USD")

createCustomer := func(prefix string) string {
status, customer, problem := c.CreateCustomer(apiv3.CreateCustomerRequest{
Key: uniqueKey(prefix),
Name: "Credit Grant Idempotency Test Customer",
Currency: &currency,
})
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
require.NotNil(t, customer)
return customer.Id
}

grant := func(key *string) apiv3.CreateCreditGrantRequest {
return apiv3.CreateCreditGrantRequest{
Name: "idempotency grant",
Amount: apiv3.Numeric("10"),
Currency: currency,
FundingMethod: apiv3.BillingCreditFundingMethodNone,
Key: key,
}
}

t.Run("reusing a key for the same customer returns 409", func(t *testing.T) {
customerID := createCustomer("credit_grant_idem_conflict")
key := ulid.Make().String()

status, _, problem := c.CreateCreditGrant(customerID, grant(&key))
require.Equal(t, http.StatusCreated, status, "first create must succeed, problem: %+v", problem)

status, _, problem = c.CreateCreditGrant(customerID, grant(&key))
require.Equal(t, http.StatusConflict, status, "reusing an idempotency key must be a 409, problem: %+v", problem)
})

t.Run("omitting the key allows duplicates", func(t *testing.T) {
customerID := createCustomer("credit_grant_idem_nil")

status, _, problem := c.CreateCreditGrant(customerID, grant(nil))
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)

status, _, problem = c.CreateCreditGrant(customerID, grant(nil))
require.Equal(t, http.StatusCreated, status, "grants without a key must not collide, problem: %+v", problem)
})

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

status, _, problem := c.CreateCreditGrant(createCustomer("credit_grant_idem_cust_a"), grant(&key))
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)

status, _, problem = c.CreateCreditGrant(createCustomer("credit_grant_idem_cust_b"), grant(&key))
require.Equal(t, http.StatusConflict, status, "the key is unique per namespace, not per customer, problem: %+v", problem)
})

t.Run("an over-length key is rejected with 400", func(t *testing.T) {
customerID := createCustomer("credit_grant_idem_overlong")
key := strings.Repeat("k", 257)

status, _, problem := c.CreateCreditGrant(customerID, grant(&key))
require.Equal(t, http.StatusBadRequest, status, "an over-length key must be a 400, problem: %+v", problem)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ func (a *adapter) CreateCharge(ctx context.Context, in creditpurchase.CreateChar
SetNillablePriority(in.Intent.Priority).
SetFeatureFilters(pq.StringArray(in.Intent.FeatureFilters.Normalize())).
SetSettlement(in.Intent.Settlement).
SetNillableKey(in.Intent.Key).
SetStatusDetailed(initialStatus)

create, err = chargemeta.Create(create, chargemeta.CreateInput{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func MapChargeBaseFromDB(dbEntity *entdb.ChargeCreditPurchase) creditpurchase.Ch
FeatureFilters: creditpurchase.FeatureFilters(dbEntity.FeatureFilters).Normalize(),
Settlement: dbEntity.Settlement,
},
Key: dbEntity.Key,
},
}
}
Expand Down
3 changes: 3 additions & 0 deletions openmeter/billing/charges/creditpurchase/charge.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ func (c Charge) Validate() error {
type Intent struct {
meta.Intent
IntentMutableFields

// Key is the optional idempotency key: a retried create with the same key returns a conflict.
Key *string `json:"key,omitempty"`
}

type IntentMutableFields struct {
Expand Down
2 changes: 2 additions & 0 deletions openmeter/billing/creditgrant/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ type CreateInput struct {
TaxConfig *productcatalog.TaxConfig
Filters *GrantFilters
ExpiresAfter *datetime.ISODuration
// Key is the optional idempotency key: a retried create with the same key returns a conflict.
Key *string
}

type GrantFilters struct {
Expand Down
1 change: 1 addition & 0 deletions openmeter/billing/creditgrant/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ func toIntent(input creditgrant.CreateInput) creditpurchase.Intent {
ExpiresAt: calculateExpiresAt(effectiveAt, input.ExpiresAfter),
Settlement: toSettlement(input),
},
Key: input.Key,
}

if input.Filters != nil {
Expand Down
16 changes: 15 additions & 1 deletion openmeter/ent/db/chargecreditpurchase.go

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

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

80 changes: 80 additions & 0 deletions openmeter/ent/db/chargecreditpurchase/where.go

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

Loading
Loading