feat(api): add root entitlement v2 APIs #3372
Conversation
📝 WalkthroughWalkthroughAdds Entitlements V2: new list and get-by-id endpoints across OpenAPI/spec, generated server and client code, JS validators/schemas, router wiring, connector/adapter filters, and v2 driver handlers and interfaces. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (30)
api/openapi.yaml (3)
11410-11418: Remove redundant explode on boolean and clarify description.explode has no effect for primitives and may confuse generators. The description is unclear.
Apply:
- description: Exclude inactive entitlements in the response (those scheduled for later or earlier) + description: Exclude entitlements that are not currently active (scheduled in the future or already ended). schema: type: boolean default: false - explode: false style: form
11485-11489: Tighten path parameter constraints for entitlementId.Add length constraints to match the 26-char ULID pattern and aid client validation.
Apply:
schema: type: string pattern: ^[0-7][0-9A-HJKMNP-TV-Za-hjkmnp-tv-z]{25}$ + minLength: 26 + maxLength: 26 example: 01G65Z755AFWAKHE12NY0CQ9FH
11404-11409: Be explicit about array serialization for entitlementType.Minor: some generators handle arrays better with explicit explode.
Apply:
schema: type: array items: $ref: '#/components/schemas/EntitlementType' style: form + explode: trueopenmeter/entitlement/connector.go (1)
73-74: Fix typo in comment"featueres" → "features".
-// For consistency, it is forbidden for entitlements to be created for featueres the keys of which could be mistaken for entitlement IDs. +// For consistency, entitlements must not be created for features whose keys could be mistaken for entitlement IDs.openmeter/server/router/router.go (1)
280-283: Refresh comment: handler is not just customer-scoped anymoreReflects root endpoints too.
-// V2 entitlement handler for customer-scoped operations +// V2 entitlement handler (customer-scoped and root operations)openmeter/entitlement/driver/v2/handler.go (1)
11-12: Doc comment: broaden scope beyond customer endpointsHandler now also exposes root list/get.
-// EntitlementHandler exposes V2 customer entitlement endpoints +// EntitlementHandler exposes V2 entitlement endpoints (customer-scoped and root)openmeter/entitlement/driver/v2/customer.go (1)
302-305: Return nil response on errorMinor cleanup: return nil instead of
requestwhen emitting an error to avoid confusion.- if cus != nil && cus.IsDeleted() { - return request, models.NewGenericPreConditionFailedError( + if cus != nil && cus.IsDeleted() { + return nil, models.NewGenericPreConditionFailedError( fmt.Errorf("customer is deleted [namespace=%s customer.id=%s]", cus.Namespace, cus.ID), ) }openmeter/entitlement/driver/v2/entitlement.go (2)
63-65: Remove duplicated OrderBy mappingOrderBy is set twice (first via camel→snake, then a switch). Keep one path to avoid drift.
- OrderBy: entitlement.ListEntitlementsOrderBy( - strcase.CamelToSnake(defaultx.WithDefault((*string)(params.OrderBy), string(entitlement.ListEntitlementsOrderByCreatedAt))), - ), + OrderBy: entitlement.ListEntitlementsOrderByCreatedAt, @@ - switch defaultx.WithDefault(params.OrderBy, "") { - case "createdAt": - p.OrderBy = entitlement.ListEntitlementsOrderByCreatedAt - case "updatedAt": - p.OrderBy = entitlement.ListEntitlementsOrderByUpdatedAt - default: - p.OrderBy = entitlement.ListEntitlementsOrderByCreatedAt - } + if params.OrderBy != nil { + switch *params.OrderBy { + case "createdAt": + p.OrderBy = entitlement.ListEntitlementsOrderByCreatedAt + case "updatedAt": + p.OrderBy = entitlement.ListEntitlementsOrderByUpdatedAt + } + }Also applies to: 78-85
44-51: Include allowed values in validation errorsHelps clients debug bad inputs quickly.
- return entitlement.ListEntitlementsParams{}, commonhttp.NewHTTPError(http.StatusBadRequest, errors.New("invalid entitlement type")) + return entitlement.ListEntitlementsParams{}, commonhttp.NewHTTPError( + http.StatusBadRequest, + fmt.Errorf("invalid entitlement type: %q (allowed: %v)", et, entitlement.EntitlementType("").StrValues()), + )api/openapi.cloud.yaml (4)
11361-11364: Clarify/validate mixed pagination (page/pageSize vs offset/limit).Spec allows both styles simultaneously but only mentions that “page takes precedence.” This is ambiguous and client-hostile.
- Either enforce mutual exclusivity (400 if both styles are provided) or remove one style.
- At minimum, codify behavior in the description:
- If page is provided that takes precedence and the paginated response is returned. + If both pagination styles are provided, the request is invalid and MUST return 400. + Use either page/pageSize or offset/limit, not both. If page is provided, the server will ignore offset/limit.If server already enforces this, please add a brief note under 400 explaining the validation error. Do you want me to propose an OpenAPI
oneOfparam grouping pattern to document exclusivity?Also applies to: 11422-11427
11413-11421: Remove redundant explode for boolean query param.
explode: falseis irrelevant for a non-array, non-object param and can confuse generators.- name: excludeInactive in: query required: false description: Exclude inactive entitlements in the response (those scheduled for later or earlier) schema: type: boolean default: false - explode: false style: formAlso consider rephrasing to: “Exclude entitlements not currently active (start in the future or ended).”
11358-11364: Document required auth scope for admin-only endpoint.Description says “administrative purposes only” but no per-operation
securityblock is present here. If global security allows user tokens, tighten scopes for this operation.I can add a
security:section referencing your admin scope (e.g.,OAuth2: [entitlements:read:all]) if you share the canonical scope name.
11401-11412: Minor: add explicit explode:true for arrays or inline examples.Default is explode=true for form, but some generators are sensitive. Optionally set
explode: trueand/or provideexamples:to lock behavior.api/client/javascript/src/client/schemas.ts (3)
2363-2367: Consistent capitalization: “ID” instead of “id”.Use “ID” consistently in title/description.
Apply this diff:
- /** - * Get entitlement by id - * @description Get entitlement by id. - */ + /** + * Get entitlement by ID + * @description Get entitlement by ID. + */
26079-26080: Tighten wording for excludeInactive.Minor clarity improvement.
Apply this diff:
- /** @description Exclude inactive entitlements in the response (those scheduled for later or earlier) */ + /** @description Exclude inactive entitlements (scheduled for the future or already expired). */
26081-26096: Clarify precedence between page-based and offset/limit pagination.You accept both styles but precedence isn’t documented here. Recommend stating that page-based pagination overrides offset/limit when both are supplied.
Apply this diff:
- /** @description Page index. - * - * Default is 1. */ + /** @description Page index. + * + * Default is 1. If provided together with offset/limit, page takes precedence and offset/limit are ignored. */ @@ - /** @description Number of items to skip. - * - * Default is 0. */ + /** @description Number of items to skip. + * + * Default is 0. Ignored when 'page' is provided. */ @@ - /** @description Number of items to return. - * - * Default is 100. */ + /** @description Number of items to return. + * + * Default is 100. Ignored when 'page' is provided. */openmeter/server/router/entitlement.go (1)
142-154: Add route-doc comments for consistency with V1 and other V2 sections.
Keeps router readable and aligned with generated specs.Apply:
// ------------------------------------------------------------ // Entitlement APIs (V2) // ------------------------------------------------------------ +// List entitlements (V2) +// (GET /api/v2/entitlements) func (a *Router) ListEntitlementsV2(w http.ResponseWriter, r *http.Request, params api.ListEntitlementsV2Params) { a.entitlementV2Handler.ListEntitlements().With(params).ServeHTTP(w, r) } +// Get an entitlement (V2) +// (GET /api/v2/entitlements/{entitlementId}) func (a *Router) GetEntitlementByIdV2(w http.ResponseWriter, r *http.Request, entitlementId string) { a.entitlementV2Handler.GetEntitlement().With(entitlementdriverv2.GetEntitlementHandlerParams{ EntitlementId: entitlementId, }).ServeHTTP(w, r) }api/spec/src/entitlements/v2/entitlements.tsp (5)
32-39: customerKeys example and wording are incorrectClarify it’s “customer keys” and fix the example/query name.
/** - * Filtering by multiple customers. - * - * Usage: `?customer=customer-1&customer-3` + * Filtering by multiple customer keys. + * + * Usage: `?customerKeys=customer-1&customerKeys=customer-3` */ @query(#{ explode: true }) customerKeys?: string[],
56-61: Clarify excludeInactive semanticsSpell out what “inactive” means and note the default.
/** - * Exclude inactive entitlements in the response (those scheduled for later or earlier) + * Exclude entitlements not active at the current time (future-scheduled or expired). Default: false. */ @query excludeInactive?: boolean = false,
15-19: Pagination docs don’t match behavior; both styles are always paginatedSpec always returns a paginated envelope; doc should state cursor vs offset precedence.
/** - * List all entitlements for all the subjects and features. This endpoint is intended for administrative purposes only. - * To fetch the entitlements of a specific subject please use the /api/v1/subjects/{subjectKeyOrID}/entitlements endpoint. - * If page is provided that takes precedence and the paginated response is returned. + * List all entitlements for all subjects and features (admin use). + * For a specific subject use /api/v1/subjects/{subjectKeyOrID}/entitlements. + * Supports cursor (page) and offset/limit; if both provided, cursor takes precedence. The response is always paginated. */Also applies to: 62-65
48-55: Include all enum values in exampleAdd “static” to the usage example for completeness.
/** * Filtering by multiple entitlement types. * - * Usage: `?entitlementType=metered&entitlementType=boolean` + * Usage: `?entitlementType=metered&entitlementType=boolean&entitlementType=static` */
74-76: Optional: add example for entitlementIdImproves SDK docs and try-it-out UX.
get( @path - entitlementId: ULID, + @example("01ARZ3NDEKTSV4RRFFQ69G5FAV") entitlementId: ULID, ): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors;api/client/javascript/src/zod/index.ts (5)
17984-17985: Fix JSDoc formatting and grammar.Missing leading asterisks and a minor grammar issue in the doc block.
- To fetch the entitlements of a specific subject please use the /api/v1/subjects/{subjectKeyOrID}/entitlements endpoint. - If page is provided that takes precedence and the paginated response is returned. + * To fetch the entitlements of a specific subject please use the /api/v1/subjects/{subjectKeyOrID}/entitlements endpoint. + * If page is provided, it takes precedence and a paginated response is returned.
17988-17996: Avoid drift: constants defined but not used in schema defaults.
listEntitlementsV2QueryExcludeInactiveDefaultandlistEntitlementsV2QueryOffsetDefaultaren’t applied. Either wire them into the schema or drop the constants to prevent divergence.Option A — apply the defaults (see related pagination comment for page/pageSize handling):
excludeInactive: zod.coerce .boolean() - .optional() + .default(listEntitlementsV2QueryExcludeInactiveDefault)offset: zod.coerce .number() .min(listEntitlementsV2QueryOffsetMin) - .optional() + .default(listEntitlementsV2QueryOffsetDefault)Option B — remove unused constants:
-export const listEntitlementsV2QueryExcludeInactiveDefault = false ... -export const listEntitlementsV2QueryOffsetDefault = 0
18043-18047: Validate sort pairing.Consider requiring
orderBywhenorderis provided (already included in the refine suggestion), or set a sane default fororderByiforderis present.
18065-18073: ULID: add description (and optional normalization).The regex correctly matches ULIDs. Add a brief description to aid errors, and optionally normalize to uppercase to avoid case-related surprises.
export const getEntitlementByIdV2Params = zod.object({ entitlementId: zod.coerce .string() - .regex(getEntitlementByIdV2PathEntitlementIdRegExp), + .regex(getEntitlementByIdV2PathEntitlementIdRegExp) + .describe('ULID (26 chars, Crockford Base32)'), })If desired:
// ... .string().transform(s => s.toUpperCase()).regex(ULID_REGEX)
17997-18059: Optionally export TS types for consumers.Handy for client callers and keeps types aligned with the schema.
}) +export type ListEntitlementsV2Query = zod.infer<typeof listEntitlementsV2QueryParams>api/client/go/client.gen.go (3)
8071-8096: Clarify mutual exclusivity/precedence for pagination params.Both page/pageSize and offset/limit are exposed. Define precedence or enforce mutual exclusivity in the server/spec to avoid ambiguous requests. Consider OpenAPI oneOf for the two pagination styles and 400 on mixed usage.
Would you like me to propose an OpenAPI snippet using oneOf for these param groups?
28988-29015: LGTM: typed response wrappers align with problem+json conventions.Optional: if the API returns 429, you may want an explicit typed field for it; otherwise it falls into the default bucket.
Also applies to: 29017-29045
42492-42565: Parsing is fine; consider more robust Content-Type checks.Using strings.Contains("json") works but is loose. Using mime.ParseMediaType and matching application/json or application/problem+json is safer and more explicit. Low priority given this is generated code.
Also applies to: 42567-42647
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
api/api.gen.go(6 hunks)api/client/go/client.gen.go(9 hunks)api/client/javascript/src/client/schemas.ts(2 hunks)api/client/javascript/src/zod/index.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(1 hunks)api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)openmeter/entitlement/adapter/entitlement.go(1 hunks)openmeter/entitlement/connector.go(1 hunks)openmeter/entitlement/driver/v2/customer.go(6 hunks)openmeter/entitlement/driver/v2/customer_metered.go(4 hunks)openmeter/entitlement/driver/v2/entitlement.go(1 hunks)openmeter/entitlement/driver/v2/handler.go(2 hunks)openmeter/server/router/entitlement.go(1 hunks)openmeter/server/router/router.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
openmeter/entitlement/driver/v2/entitlement.go (7)
api/api.gen.go (8)
ListEntitlementsParams(7978-8022)EntitlementV2(3216-3218)ListEntitlementsV2Params(8689-8738)EntitlementType(3212-3212)SortOrderASC(758-758)Feature(3273-3302)Entitlement(2571-2573)Customer(2188-2238)openmeter/entitlement/connector.go (4)
ListEntitlementsParams(32-52)ListEntitlementsOrderBy(12-12)ListEntitlementsOrderByCreatedAt(15-15)ListEntitlementsOrderByUpdatedAt(16-16)pkg/framework/transport/httptransport/argshandler.go (2)
HandlerWithArgs(11-14)NewHandlerWithArgs(18-29)pkg/framework/commonhttp/errors.go (1)
NewHTTPError(35-41)pkg/convert/ptr.go (1)
DerefHeaderPtr(59-64)openmeter/entitlement/driver/v2/mapping.go (1)
ParserV2(21-21)pkg/framework/commonhttp/encoder.go (1)
JSONResponseEncoder(39-41)
openmeter/server/router/router.go (1)
openmeter/entitlement/driver/v2/handler.go (2)
EntitlementHandler(12-24)NewEntitlementHandler(34-48)
api/client/go/client.gen.go (1)
api/api.gen.go (19)
Feature(3273-3302)EntitlementType(3212-3212)PaginationPage(7635-7635)PaginationPageSize(7638-7638)LimitOffsetOffset(7557-7557)LimitOffsetLimit(7554-7554)EntitlementOrderByOrderingOrder(7497-7497)EntitlementOrderByOrderingOrderBy(7500-7500)ListEntitlementsV2Params(8689-8738)EntitlementV2PaginatedResponse(3221-3233)BadRequestProblemResponse(1134-1134)UnauthorizedProblemResponse(7222-7222)ForbiddenProblemResponse(3512-3512)PreconditionFailedProblemResponse(5975-5975)InternalServerErrorProblemResponse(3621-3621)ServiceUnavailableProblemResponse(6317-6317)UnexpectedProblemResponse(7226-7226)EntitlementV2(3216-3218)NotFoundProblemResponse(4964-4964)
openmeter/entitlement/adapter/entitlement.go (2)
openmeter/ent/db/entitlement/where.go (2)
HasCustomerWith(1401-1410)IDIn(30-32)openmeter/ent/db/customer/where.go (2)
KeyIn(1062-1064)IDIn(31-33)
openmeter/entitlement/driver/v2/handler.go (2)
openmeter/entitlement/driver/v2/entitlement.go (2)
ListEntitlementsHandler(26-26)GetEntitlementHandler(120-120)openmeter/entitlement/connector.go (1)
Connector(54-79)
api/api.gen.go (1)
api/client/go/client.gen.go (9)
Feature(2987-3016)EntitlementType(2926-2926)PaginationPage(6993-6993)PaginationPageSize(6996-6996)LimitOffsetOffset(6915-6915)LimitOffsetLimit(6912-6912)EntitlementOrderByOrderingOrder(6855-6855)EntitlementOrderByOrderingOrderBy(6858-6858)ListEntitlementsV2Params(8047-8096)
openmeter/server/router/entitlement.go (3)
openmeter/server/router/router.go (1)
Router(218-244)api/api.gen.go (1)
ListEntitlementsV2Params(8689-8738)openmeter/entitlement/driver/v2/entitlement.go (1)
GetEntitlementHandlerParams(116-118)
🪛 Gitleaks (8.27.2)
api/client/go/client.gen.go
[high] 43758-43759: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Artifacts / Container image
- GitHub Check: Code Generators
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (28)
openmeter/entitlement/connector.go (1)
36-37: LGTM: new customer filters added to ListEntitlementsParamsThe additions are consistent with the V2 surface and downstream adapter changes.
api/spec/src/cloud/main.tsp (1)
216-218: Expose V2 root entitlements in Cloud surface — verifiedapi/api.gen.go exposes ListEntitlementsV2 and GetEntitlementByIdV2 and they forward to entitlementV2Handler.ListEntitlements() and entitlementV2Handler.GetEntitlement(), so the interface wiring matches the V2 handlers.
openmeter/server/router/router.go (2)
235-236: Update handler type to v2 EntitlementHandler — OKField type switch aligns with handler rename.
281-287: Constructor switch to NewEntitlementHandler — verifiedParams preserved and balance connector included; V2 routes are bound to entitlementV2Handler (constructed in openmeter/server/router/router.go and used in openmeter/server/router/entitlement.go for ListEntitlementsV2, GetEntitlementByIdV2 and the customer-scoped V2 endpoints).
openmeter/entitlement/driver/v2/customer_metered.go (1)
43-106: Receiver rename only — no functional changesLooks consistent with handler rename.
Also applies to: 126-209, 229-328, 348-409
openmeter/entitlement/driver/v2/customer.go (1)
30-91: Receiver rename only — OKInterfaces and router wiring align with the new type.
Also applies to: 104-195, 212-259, 276-322, 340-405, 409-416
openmeter/entitlement/driver/v2/entitlement.go (1)
36-42: No action required — repo targets Go 1.24.1 so stdlibslicesis supported.go.mod declares "go 1.24.1", so using slices.Contains is safe; no change needed.
api/openapi.cloud.yaml (2)
11485-11492: ULID pattern looks OK; ensure consistency across clients and server.Pattern permits lower/upper Crockford Base32 and enforces 26 chars starting with 0–7. Verify all generators/validators (Go/JS) use the same regex and error messaging.
Do you want me to add a shared
#/components/parameters/EntitlementIdso all paths reuse the same schema?
11429-11435: Response schemas reference EntitlementV2 types—confirm components exist and align.Ensure
#/components/schemas/EntitlementV2PaginatedResponseandEntitlementV2are present in this cloud spec and match the server payload shape.Also applies to: 11493-11499
api/client/javascript/src/client/schemas.ts (3)
2341-2347: LGTM on path and method mapping for listEntitlementsV2.Path registration and method gating look correct. Operation correctly points to
operations['listEntitlementsV2'].
26197-26201: Action: EntitlementV2 schemas present in OpenAPI; confirm server handlers use themEntitlementV2 and EntitlementV2PaginatedResponse are defined and referenced in api/openapi.yaml and api/openapi.cloud.yaml; client types and zod validators in api/client/javascript/src/client/schemas.ts and api/client/javascript/src/zod/index.ts match the expected query/response shapes. Server handler implementations were not inspected — confirm the handlers are wired to return/use these response schemas.
2334-2355: Confirm consumers don't rely on path-level typingNo in-repo references to paths['/api/v2/entitlements'].parameters.query were found; the generated schema sets path-level query?: never (api/client/javascript/src/client/schemas.ts:2334–2355). Verify downstream/external consumers or utilities do not read path-level parameters instead of using the operation-level typing.
openmeter/server/router/entitlement.go (2)
146-154: LGTM: V2 root endpoints are correctly wired to entitlementV2Handler.
The delegation and param passing look consistent with the v2 driver/types.
146-154: Router signatures match generated ServerInterface — no action required.Generated ServerInterface declares ListEntitlementsV2(w http.ResponseWriter, r *http.Request, params ListEntitlementsV2Params) and GetEntitlementByIdV2(w http.ResponseWriter, r *http.Request, entitlementId string), which match the router methods.
api/api.gen.go (6)
13512-13522: Unimplemented stubs are fine.501 placeholder responses are appropriate for the generated Unimplemented handler.
18960-18983: Path binding for entitlementId looks correct.chi URL param name matches the router path; error path uses InvalidParamFormatError as expected.
19709-19714: Routes wired correctly.Static and parameterized GET routes for /api/v2/entitlements don’t conflict; baseURL prefixing follows existing pattern.
20606-20911: Swagger embed updated.Assuming the spec changes are source of truth, this looks fine.
12612-12617: Approve — ServerInterface methods implemented and signatures match client.
Signatures align with the client and the concrete Router implements both methods (openmeter/server/router/entitlement.go:146–152).
18853-18958: Query binding LGTM — validations already implementedOrderBy is validated in openmeter/entitlement/driver/v2/entitlement.go (and openmeter/entitlement/driver/entitlement.go); pagination normalization/support for page/pageSize vs limit/offset is implemented in openmeter/entitlement/driver/v2/customer.go and handled (Page.IsZero -> use limit/offset) in openmeter/entitlement/adapter/entitlement.go.
api/client/javascript/src/zod/index.ts (2)
17997-18059: ```shell
#!/bin/bash
set -euo pipefailfile=$(fd -a 'openapi*.ya?ml' | head -n1 || true)
if [ -z "$file" ]; then
echo "openapi file not found"
exit 0
fiecho "OPENAPI FILE: $file"
echo "---- lines 11370-11396 ----"
sed -n '11370,11396p' "$file" || trueecho
echo "---- occurrences of 'customerIds' with context ----"
rg -n -C3 'customerIds' "$file" || trueecho
echo "---- search for entitlementId / ULID / pattern / format strings ----"
rg -n -C3 'entitlementId|ULID|ulid|pattern:|format:' "$file" || trueecho
echo "---- wider context for entitlementId / customerId occurrences ----"
rg -n -C20 'entitlementId|customerId' "$file" || trueecho
echo "---- search repo for regex/constant used in code (zod regex var) ----"
rg -n --hidden 'getEntitlementByIdV2PathEntitlementIdRegExp|EntitlementIdRegExp|entitlementIdRegExp|ulid' || true--- `17997-18059`: **Prevent accidental precedence: page/pageSize defaults make paging always “present”.** Defaults for page and pageSize will be injected even when callers use limit/offset, causing page-based pagination to take precedence and produce ambiguous server behavior. Remove .default() for page and pageSize, keep/explicitly set defaults for limit/offset, and enforce mutual exclusivity between {page,pageSize} and {limit,offset} (e.g., .superRefine(...) rejecting mixed use) or model pagination as a union. Also require orderBy when order is provided. File: api/client/javascript/src/zod/index.ts (17997–18059). Verify whether parsed results are serialized into query strings (search for parse/safeParse and URLSearchParams|qs.stringify|searchParams.set) before merging this change. </blockquote></details> <details> <summary>api/client/go/client.gen.go (6)</summary><blockquote> `11857-11862`: **LGTM: new ClientInterface methods added consistently.** --- `14318-14340`: **LGTM: client methods build requests and apply editors correctly.** --- `23523-23731`: **LGTM: ListEntitlementsV2 request builder encodes arrays with form+explode as expected.** --- `23732-23764`: **LGTM: GetEntitlementByIdV2 request builder path-param handling looks correct.** --- `24632-24636`: **LGTM: WithResponse interface mirrors the non-typed interface.** --- `43678-43949`: **Add a gitleaks allowlist for the embedded swaggerSpec (decoded & scanned — no obvious secrets)** I decoded api/client/go/client.gen.go's swaggerSpec and scanned for common secret/token patterns; none were found. Add a minimal gitleaks allowlist rule that targets the swaggerSpec block (match filename api/client/go/client.gen.go and the `var swaggerSpec = []string{...}` block) to suppress this recurring false positive. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| description: |- | ||
| Filtering by multiple customers. | ||
|
|
||
| Usage: `?customer=customer-1&customer-3` | ||
| schema: | ||
| type: array | ||
| items: | ||
| type: string | ||
| style: form |
There was a problem hiding this comment.
Fix parameter name and example for customerKeys; improve wording.
The example uses an undefined query key (“customer”) and is malformed.
Apply:
- description: |-
- Filtering by multiple customers.
-
- Usage: `?customer=customer-1&customer-3`
+ description: |-
+ Filter by multiple customer keys.
+
+ Usage: `?customerKeys=customer-1&customerKeys=customer-3`
+ explode: true📝 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.
| description: |- | |
| Filtering by multiple customers. | |
| Usage: `?customer=customer-1&customer-3` | |
| schema: | |
| type: array | |
| items: | |
| type: string | |
| style: form | |
| description: |- | |
| Filter by multiple customer keys. | |
| Usage: `?customerKeys=customer-1&customerKeys=customer-3` | |
| explode: true | |
| schema: | |
| type: array | |
| items: | |
| type: string | |
| style: form |
🤖 Prompt for AI Agents
In api/openapi.yaml around lines 11377 to 11385, the query example and wording
refer to a non-existent key "customer" and the example is malformed; update the
description to reference the correct parameter name "customerKeys" and show the
proper multi-value form encoding (e.g.
?customerKeys=customer-1&customerKeys=customer-3), adjust the sentence to
"Filter by one or more customer keys." and ensure the schema and style remain as
type: array / items: type: string / style: form so the example matches the
parameter name and format.
| description: |- | ||
| Filtering by multiple customers. | ||
|
|
||
| Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` | ||
| schema: | ||
| type: array | ||
| items: | ||
| type: string | ||
| style: form |
There was a problem hiding this comment.
Fix parameter name and example for customerIds; consider referencing a ULID schema.
The example again uses “customer” instead of “customerIds”.
Apply:
- description: |-
- Filtering by multiple customers.
-
- Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9`
+ description: |-
+ Filter by multiple customer IDs.
+
+ Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9`
schema:
type: array
items:
- type: string
+ type: string
+ # If available in components, prefer: $ref: '#/components/schemas/ULID'
style: form
+ explode: true📝 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.
| description: |- | |
| Filtering by multiple customers. | |
| Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` | |
| schema: | |
| type: array | |
| items: | |
| type: string | |
| style: form | |
| description: |- | |
| Filter by multiple customer IDs. | |
| Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9` | |
| schema: | |
| type: array | |
| items: | |
| type: string | |
| # If available in components, prefer: $ref: '#/components/schemas/ULID' | |
| style: form | |
| explode: true |
🤖 Prompt for AI Agents
In api/openapi.yaml around lines 11389 to 11397, the parameter/example uses
"customer" instead of the intended "customerIds" and the item type should
reference the existing ULID schema; change the parameter name and example to use
customerIds (e.g.
?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9),
and update the items/schema to $ref: '#/components/schemas/ULID' (or the correct
ULID schema path) so the parameter is named customerIds and validates as ULIDs.
| @get | ||
| @operationId("getEntitlementByIdV2") | ||
| @summary("Get entitlement by id") | ||
| get( | ||
| @path | ||
| entitlementId: ULID, | ||
| ): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors; |
There was a problem hiding this comment.
Missing @route for path parameter; GET by ID won’t bind entitlementId
Without a route segment, @path entitlementId won’t materialize in OpenAPI. Add @route("/{entitlementId}").
@get
@operationId("getEntitlementByIdV2")
@summary("Get entitlement by id")
+ @route("/{entitlementId}")
get(
@path
entitlementId: ULID,
): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors;📝 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.
| @get | |
| @operationId("getEntitlementByIdV2") | |
| @summary("Get entitlement by id") | |
| get( | |
| @path | |
| entitlementId: ULID, | |
| ): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors; | |
| @get | |
| @operationId("getEntitlementByIdV2") | |
| @summary("Get entitlement by id") | |
| @route("/{entitlementId}") | |
| get( | |
| @path | |
| entitlementId: ULID, | |
| ): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors; |
🤖 Prompt for AI Agents
In api/spec/src/entitlements/v2/entitlements.tsp around lines 70 to 76, the GET
operation is missing a route segment so the @path entitlementId parameter will
not be bound; add a route decorator such as @route("/{entitlementId}") directly
above the get() declaration so the entitlementId path parameter is included in
the OpenAPI path and will bind correctly.
| if len(params.CustomerKeys) > 0 { | ||
| query = query.Where(db_entitlement.HasCustomerWith( | ||
| customerdb.KeyIn(params.CustomerKeys...), | ||
| )) | ||
| } | ||
|
|
||
| if len(params.CustomerIDs) > 0 { | ||
| query = query.Where(db_entitlement.HasCustomerWith( | ||
| customerdb.IDIn(params.CustomerIDs...), | ||
| )) | ||
| } |
There was a problem hiding this comment.
Combine customerId/customerKey filters with OR and exclude deleted customers
Current logic stacks two HasCustomerWith() clauses, effectively AND-ing key and ID. If both filters are provided, no rows match unless a single customer satisfies both, which is likely unintended. It also omits the customer deletion predicate used elsewhere, leading to inconsistent results.
Apply this refactor to:
- OR keys and IDs
- AND with not-deleted
- Optionally scope customer namespace when provided (consistent with other filters)
- if len(params.CustomerKeys) > 0 {
- query = query.Where(db_entitlement.HasCustomerWith(
- customerdb.KeyIn(params.CustomerKeys...),
- ))
- }
-
- if len(params.CustomerIDs) > 0 {
- query = query.Where(db_entitlement.HasCustomerWith(
- customerdb.IDIn(params.CustomerIDs...),
- ))
- }
+ if len(params.CustomerKeys) > 0 || len(params.CustomerIDs) > 0 {
+ // OR across IDs and Keys, AND with not-deleted (and namespace when provided)
+ orPreds := []predicate.Customer{}
+ if len(params.CustomerKeys) > 0 {
+ orPreds = append(orPreds, customerdb.KeyIn(params.CustomerKeys...))
+ }
+ if len(params.CustomerIDs) > 0 {
+ orPreds = append(orPreds, customerdb.IDIn(params.CustomerIDs...))
+ }
+ andPreds := []predicate.Customer{
+ customerdb.Or(orPreds...),
+ customerNotDeletedAt(now),
+ }
+ if len(params.Namespaces) > 0 {
+ andPreds = append(andPreds, customerdb.NamespaceIn(params.Namespaces...))
+ }
+ query = query.Where(db_entitlement.HasCustomerWith(andPreds...))
+ }📝 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.
| if len(params.CustomerKeys) > 0 { | |
| query = query.Where(db_entitlement.HasCustomerWith( | |
| customerdb.KeyIn(params.CustomerKeys...), | |
| )) | |
| } | |
| if len(params.CustomerIDs) > 0 { | |
| query = query.Where(db_entitlement.HasCustomerWith( | |
| customerdb.IDIn(params.CustomerIDs...), | |
| )) | |
| } | |
| if len(params.CustomerKeys) > 0 || len(params.CustomerIDs) > 0 { | |
| // OR across IDs and Keys, AND with not-deleted (and namespace when provided) | |
| orPreds := []predicate.Customer{} | |
| if len(params.CustomerKeys) > 0 { | |
| orPreds = append(orPreds, customerdb.KeyIn(params.CustomerKeys...)) | |
| } | |
| if len(params.CustomerIDs) > 0 { | |
| orPreds = append(orPreds, customerdb.IDIn(params.CustomerIDs...)) | |
| } | |
| andPreds := []predicate.Customer{ | |
| customerdb.Or(orPreds...), | |
| customerNotDeletedAt(now), | |
| } | |
| if len(params.Namespaces) > 0 { | |
| andPreds = append(andPreds, customerdb.NamespaceIn(params.Namespaces...)) | |
| } | |
| query = query.Where(db_entitlement.HasCustomerWith(andPreds...)) | |
| } |
🤖 Prompt for AI Agents
In openmeter/entitlement/adapter/entitlement.go around lines 421 to 431, the
current code adds two separate HasCustomerWith() clauses which ANDs customer key
and ID filters and also omits the "not deleted" predicate and optional namespace
scoping; instead, build a single HasCustomerWith(...) call that ORs the
KeyIn(...) and IDIn(...) predicates when either is present, ANDs a predicate
that excludes deleted customers (use the same deleted check used elsewhere), and
when a customer namespace is supplied include that namespace predicate as well
so the combined customer filter reads: (KeyIn OR IDIn) AND NotDeleted AND
optional NamespaceEquals. Ensure you replace the two separate Where(...) calls
with this single consolidated predicate construction.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (9)
api/openapi.cloud.yaml (2)
11376-11387: Fix query example and label for customerKeys (wrong param name, malformed example).Use the correct parameter name and repeat the key per RFC3986; also clarify the label as “customer keys.”
- description: |- - Filtering by multiple customers. + description: |- + Filtering by multiple customer keys. @@ - Usage: `?customer=customer-1&customer-3` + Usage: `?customerKeys=customer-1&customerKeys=customer-3`
11388-11399: Fix query example and label for customerIds (wrong param name).Use the correct parameter name and repeat the key.
- description: |- - Filtering by multiple customers. + description: |- + Filtering by multiple customer IDs. @@ - Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` + Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9`api/api.gen.go (3)
8713-8731: Clarify and enforce pagination mode (page/pageSize vs offset/limit).Define exclusivity or precedence in the OpenAPI (source of truth) and enforce in the handler; current middleware accepts both without guidance.
Verify enforcement exists server-side:
#!/bin/bash rg -n -C3 -S 'ListEntitlementsV2\(' openmeter | rg -n -C3 -e '\bpage\b|\boffset\b|\blimit\b|\bpageSize\b'
20605-20615: Embedded swagger updated—ensure spec fixes propagate.Once the examples/pagination rules are corrected in the spec, regenerate this blob and all clients to keep docs consistent.
Quick check after regeneration:
#!/bin/bash rg -n -S 'Usage: `\?customer=' api/client api | sed -n '1,200p'
8695-8704: Fix incorrect query-param examples for customer filters.Comments show
?customer=but params arecustomerKeysandcustomerIds. Update the spec sources and regenerate; don’t hand-edit generated files. For reference only, this is the intended wording.Apply in spec (then regenerate):
-// Usage: `?customer=customer-1&customer-3` +// Usage: `?customerKeys=customer-1&customerKeys=customer-3` -// Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` +// Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9`Run to locate lingering wrong examples:
#!/bin/bash fd -a -t f 'openapi.*\.ya?ml$' | xargs -I{} rg -n -C2 -S 'Usage: `\?customer=' {} rg -n -C2 -S 'Usage: `\?customer=' apiapi/client/javascript/src/zod/index.ts (1)
18000-18008: Fix parameter names in examples.The examples reference
customerwhile the actual parameters arecustomerIdsandcustomerKeys. Also, the second example is missing the parameter name beforecustomer-3.Apply this diff to correct the parameter names in the examples:
- 'Filtering by multiple customers.\n\nUsage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9`' + 'Filtering by multiple customers.\n\nUsage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9`'- 'Filtering by multiple customers.\n\nUsage: `?customer=customer-1&customer-3`' + 'Filtering by multiple customers.\n\nUsage: `?customerKeys=customer-1&customerKeys=customer-3`'api/client/go/client.gen.go (1)
8053-8061: Fix customer filter usage examples (wrong query keys) — same issue as earlier comment.Examples use
customer=...but params arecustomerKeysandcustomerIds. Also the second value in the first example is missing the key. Please fix in the OpenAPI descriptions so regen preserves it.Apply this diff in the generated file (and mirror it in the OpenAPI spec descriptions):
- // Usage: `?customer=customer-1&customer-3` + // Usage: `?customerKeys=customer-1&customerKeys=customer-3` ... - // Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` + // Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9`Run to find and fix all occurrences across the repo (source-of-truth should be the spec):
#!/bin/bash rg -n -C2 -e 'Usage:\s*`\\\?customer=' -e 'Usage:\s*`\?customer=' rg -n -C2 'customer=customer-1&customer-3'api/client/javascript/src/client/schemas.ts (2)
26066-26070: Fix customerKeys usage example (wrong param name and missing repeated key).Replace
?customer=customer-1&customer-3with the correct repeatedcustomerKeysexamples.Apply this diff:
- /** @description Filtering by multiple customers. - * - * Usage: `?customer=customer-1&customer-3` */ + /** @description Filter by multiple customer keys. + * + * Usage: `?customerKeys=customer-1&customerKeys=customer-3` */
26070-26073: Fix customerIds usage example (wrong param name).Use
customerIdsin the example and keep repeated form consistent with other params.Apply this diff:
- /** @description Filtering by multiple customers. - * - * Usage: `?customer=01K4WAQ0J99ZZ0MD75HXR112H8&customer=01K4WAQ0J99ZZ0MD75HXR112H9` */ + /** @description Filter by multiple customer IDs. + * + * Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9` */
🧹 Nitpick comments (10)
api/openapi.cloud.yaml (2)
11412-11420: Simplify boolean query param and clarify meaning.explode is unnecessary for a boolean; refine wording to define “inactive.”
- description: Exclude inactive entitlements in the response (those scheduled for later or earlier) + description: Exclude entitlements not currently active (startAt in the future or already ended). schema: type: boolean default: false - explode: false style: form
11421-11426: Document/validate pagination strategy (page vs. offset).Both page/pageSize and offset/limit are exposed. Define precedence or enforce mutual exclusivity (return 400 on mix) to avoid ambiguous pagination.
api/api.gen.go (1)
8710-8711: Wording nit: make “ExcludeInactive” clearer.Suggest: “Exclude inactive entitlements (scheduled to start in the future or already ended).”
api/client/go/client.gen.go (3)
8068-8069: Nit: clarify wording for ExcludeInactive.Current: “those scheduled for later or earlier.” Consider clearer phrasing.
- // ExcludeInactive Exclude inactive entitlements in the response (those scheduled for later or earlier) + // ExcludeInactive Exclude inactive entitlements (scheduled to activate in the future or already expired).
8046-8096: Document pagination parameter expectations (avoid mixing page/pageSize with limit/offset).To reduce 400s and ambiguity, note mutual exclusivity in the spec description so the client doc reflects it.
If intended, add this note to the OpenAPI parameter descriptions for page/pageSize and limit/offset.
23523-23731: Set explicit Accept header for JSON/problem+json.Helps content negotiation and avoids proxies returning text/html.
@@ - req, err := http.NewRequest("GET", queryURL.String(), nil) + req, err := http.NewRequest("GET", queryURL.String(), nil) if err != nil { return nil, err } + req.Header.Set("Accept", "application/json, application/problem+json") @@ - req, err := http.NewRequest("GET", queryURL.String(), nil) + req, err := http.NewRequest("GET", queryURL.String(), nil) if err != nil { return nil, err } + req.Header.Set("Accept", "application/json, application/problem+json")Note: Prefer fixing this via the OpenAPI spec’s produces/content to keep generated code in sync.
Also applies to: 23732-23765
api/client/javascript/src/client/schemas.ts (4)
2342-2345: Clarify admin-only note and cross-reference correctness.Doc mentions “administrative purposes only” and points to /api/v2/customers/{customerIdOrKey}/entitlements. Please confirm that endpoint exists in v2 and consider stating required role/scope explicitly to avoid ambiguity.
26078-26079: Tighten wording for excludeInactive (ambiguous phrasing).“scheduled for later or earlier” is unclear; propose explicit wording and default.
Apply this diff:
- /** @description Exclude inactive entitlements in the response (those scheduled for later or earlier) */ + /** @description Exclude entitlements that are not currently active (future-scheduled or expired). Default: false. */
26080-26095: Document mutual exclusivity between page/pageSize and offset/limit.Support for both pagination styles is great; please document they are mutually exclusive to avoid ambiguous requests.
Apply this diff:
- /** @description Page index. - * - * Default is 1. */ + /** @description Page index (mutually exclusive with offset/limit). + * + * Default is 1. */ page?: components['parameters']['Pagination.page'] - /** @description The maximum number of items per page. - * - * Default is 100. */ + /** @description The maximum number of items per page (mutually exclusive with offset/limit). + * + * Default is 100. */ pageSize?: components['parameters']['Pagination.pageSize'] - /** @description Number of items to skip. - * - * Default is 0. */ + /** @description Number of items to skip (mutually exclusive with page/pageSize). + * + * Default is 0. */ offset?: components['parameters']['LimitOffset.offset'] - /** @description Number of items to return. - * - * Default is 100. */ + /** @description Number of items to return (mutually exclusive with page/pageSize). + * + * Default is 100. */ limit?: components['parameters']['LimitOffset.limit']Also applies to: 26088-26095
26185-26188: Add param doc for entitlementId (format/example).If IDs are ULIDs, add a short doc and example to guide clients.
Apply this diff:
- path: { - entitlementId: string - } + path: { + /** @description Entitlement ID (e.g., ULID). Example: `01K4WAQ0J99ZZ0MD75HXR112H8` */ + entitlementId: string + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
api/api.gen.go(6 hunks)api/client/go/client.gen.go(9 hunks)api/client/javascript/src/client/schemas.ts(2 hunks)api/client/javascript/src/zod/index.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/openapi.yaml
🧰 Additional context used
🧬 Code graph analysis (2)
api/api.gen.go (1)
api/client/go/client.gen.go (9)
Feature(2987-3016)EntitlementType(2926-2926)PaginationPage(6993-6993)PaginationPageSize(6996-6996)LimitOffsetOffset(6915-6915)LimitOffsetLimit(6912-6912)EntitlementOrderByOrderingOrder(6855-6855)EntitlementOrderByOrderingOrderBy(6858-6858)ListEntitlementsV2Params(8047-8096)
api/client/go/client.gen.go (1)
api/api.gen.go (19)
Feature(3273-3302)EntitlementType(3212-3212)PaginationPage(7635-7635)PaginationPageSize(7638-7638)LimitOffsetOffset(7557-7557)LimitOffsetLimit(7554-7554)EntitlementOrderByOrderingOrder(7497-7497)EntitlementOrderByOrderingOrderBy(7500-7500)ListEntitlementsV2Params(8689-8738)EntitlementV2PaginatedResponse(3221-3233)BadRequestProblemResponse(1134-1134)UnauthorizedProblemResponse(7222-7222)ForbiddenProblemResponse(3512-3512)PreconditionFailedProblemResponse(5975-5975)InternalServerErrorProblemResponse(3621-3621)ServiceUnavailableProblemResponse(6317-6317)UnexpectedProblemResponse(7226-7226)EntitlementV2(3216-3218)NotFoundProblemResponse(4964-4964)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Quickstart
- GitHub Check: E2E
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Code Generators
- GitHub Check: Build
- GitHub Check: Analyze (go)
🔇 Additional comments (20)
api/openapi.cloud.yaml (1)
11484-11491: ULID path param pattern looks correct.Pattern and example align with ULID (Crockford base32, 26 chars, time prefix 0–7).
api/api.gen.go (5)
12612-12617: ServerInterface additions LGTM.New methods and paths align with the described V2 surface.
13512-13522: Unimplemented stubs LGTM.Consistent 501 behavior for unimplemented operations.
18853-18958: Query binding for ListEntitlementsV2 looks correct.Multi-valued params use explode; error mapping is consistent.
18960-18983: Path binding for GetEntitlementByIdV2 LGTM.
19709-19714: Route wiring LGTM.GET /api/v2/entitlements and GET /api/v2/entitlements/{entitlementId} registered as expected.
api/client/javascript/src/zod/index.ts (4)
17982-17986: LGTM!The JSDoc comment clearly explains the purpose of the admin endpoint and provides helpful guidance to users about using the customer-specific endpoint for individual subjects.
17987-17994: LGTM!The default values and limits are well-defined and follow consistent naming conventions. The pagination parameters provide both offset-based and page-based options with reasonable defaults.
17996-18058: LGTM!The query parameters schema is comprehensive and well-structured. The validation rules are appropriate with sensible defaults and limits. The field descriptions provide clear usage examples for developers.
18064-18072: LGTM!The entitlement ID validation uses a proper regex pattern that appears to validate ULID format (base32 encoding with specific character set), and the Zod schema correctly applies this validation.
api/client/go/client.gen.go (7)
11857-11861: Interfaces extension looks good.Methods and signatures align with server shapes.
14318-14341: Client methods: LGTM.Context propagation and editor application are consistent with existing patterns.
24632-24636: WithResponse interfaces: LGTM.Typed wrappers are correctly added.
28988-29045: Response types: LGTM.Status helpers and payload fields match the API.
30884-30900: WithResponse wrappers: LGTM.Happy path and error propagation are consistent.
42492-42647: Parsers: LGTM.Content-type handling covers json and problem+json; default branch maps to unexpected problem.
43681-43949: Embedded swagger blob.Generated asset; no review feedback.
api/client/javascript/src/client/schemas.ts (3)
2334-2354: V2 route shape LGTM.Path registered with GET-only and other methods explicitly disallowed looks correct.
2355-2361: Confirm op-level path param is sufficient.Path-level
parameters.path?: neverwith op-levelpath.entitlementIdis fine, but verify your client/server codegen doesn’t rely on path-level param declarations for templating.
26192-26274: Response surface LGTM.Comprehensive problem responses with 404 for not found are in place.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
api/openapi.cloud.yaml (1)
11376-11394: Past issue resolved: query examples now match parameter names.Examples for customerKeys and customerIds correctly use repeated keys and the right param names. Thanks for fixing.
api/spec/src/entitlements/v2/entitlements.tsp (3)
16-18: Use “customer” instead of “subject” in docsAligns with the rest of the API surface.
- * To fetch the entitlements of a specific subject please use the /api/v2/customers/{customerIdOrKey}/entitlements endpoint. + * To fetch the entitlements of a specific customer please use the /api/v2/customers/{customerIdOrKey}/entitlements endpoint.
39-46: Type customerIds as ULID[] and fix doc to say “customer IDs”Ensures schema/clients carry ULID intent and improves generated validators.
- /** - * Filtering by multiple customers. - * - * Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9` - */ + /** + * Filtering by multiple customer IDs. + * + * Usage: `?customerIds=01K4WAQ0J99ZZ0MD75HXR112H8&customerIds=01K4WAQ0J99ZZ0MD75HXR112H9` + */ @query(#{ explode: true }) - customerIds?: string[], + customerIds?: ULID[],
69-75: Add @route for path parameter; entitlementId won’t bind otherwiseWithout a route segment,
@path entitlementIdwon’t materialize in OpenAPI.@get @operationId("getEntitlementByIdV2") @summary("Get entitlement by id") + @route("/{entitlementId}") get( @path entitlementId: ULID, ): EntitlementV2 | OpenMeter.NotFoundError | OpenMeter.CommonErrors;api/client/go/client.gen.go (1)
8053-8061: Customer filter example keys fixed — looks good.Examples now use customerKeys and customerIds correctly, matching the actual query params.
🧹 Nitpick comments (16)
api/openapi.cloud.yaml (5)
11421-11426: Avoid ambiguity between page/pageSize and offset/limit.Support for both pagination modes can confuse clients. Document precedence or reject mixed usage.
Apply this doc tweak to the operation description:
description: |- List all entitlements for all the customers and features. This endpoint is intended for administrative purposes only. To fetch the entitlements of a specific subject please use the /api/v2/customers/{customerIdOrKey}/entitlements endpoint. + Note: Use either page/pageSize or offset/limit, not both. Requests providing both will be rejected with 400.
11413-11420: Clarify “inactive” wording.Current phrasing is vague; specify future-start or past-end.
- description: Exclude inactive entitlements in the response (those scheduled for later or earlier) + description: Exclude entitlements that are not active at the time of the request (scheduled to start in the future or already ended).
11428-11476: Add 429 Too Many Requests response.Rate-limiting is common on list endpoints; include an explicit 429 mapping.
'403': description: The server understood the request but refuses to authorize it. content: application/problem+json: schema: $ref: '#/components/schemas/ForbiddenProblemResponse' + '429': + description: Too many requests. + content: + application/problem+json: + schema: + $ref: '#/components/schemas/TooManyRequestsProblemResponse' '412':Please verify that
TooManyRequestsProblemResponseexists in components or adjust to the correct schema name used in this spec.
11481-11482: Nit: capitalize ID in summary/description.Consistency with other operations.
- summary: Get entitlement by id - description: Get entitlement by id. + summary: Get entitlement by ID + description: Get entitlement by ID.
11505-11539: Also add 429 to the get-by-ID responses.Single-resource GETs are often rate-limited too.
'403': description: The server understood the request but refuses to authorize it. content: application/problem+json: schema: $ref: '#/components/schemas/ForbiddenProblemResponse' + '429': + description: Too many requests. + content: + application/problem+json: + schema: + $ref: '#/components/schemas/TooManyRequestsProblemResponse' '404':Please verify the referenced 429 problem schema name.
api/spec/src/entitlements/v2/entitlements.tsp (4)
31-38: Clarify that these are customer keysThe description currently says “customers” but the param is customerKeys.
- /** - * Filtering by multiple customers. - * - * Usage: `?customerKeys=customer-1&customerKeys=customer-3` - */ + /** + * Filtering by multiple customer keys. + * + * Usage: `?customerKeys=customer-1&customerKeys=customer-3` + */
47-54: Optional: pluralize entitlementType param for consistency with other array paramsKeeps naming consistent with customerIds/customerKeys. If you keep the current name for backward compatibility, feel free to ignore.
- @query(#{ explode: true }) - entitlementType?: EntitlementType[], + @query(#{ explode: true }) + entitlementTypes?: EntitlementType[],
56-60: Tighten wording of excludeInactive help textClarify what “later or earlier” means.
- /** - * Exclude inactive entitlements in the response (those scheduled for later or earlier) - */ + /** + * Exclude entitlements that are not currently active (scheduled to start in the future or already expired). + */
28-29: Optional: consider naming this featureKeys for clarityMatches the “keys/ids” pattern used elsewhere. If existing clients already rely on
feature, skip.- @query(#{ explode: true }) - feature?: string[], + @query(#{ explode: true }) + featureKeys?: string[],api/client/javascript/src/client/schemas.ts (4)
26062-26065: Nit: clarify feature filter phrasing.Prefer explicit “feature keys” to match terminology used elsewhere.
Apply this diff:
- /** @description Filtering by multiple features. + /** @description Filter by multiple feature keys.
26066-26073: Docs: be explicit about keys vs IDs in filter descriptions.The examples are correct; align the description lines to reduce ambiguity.
Apply this diff:
- /** @description Filtering by multiple customers. + /** @description Filter by multiple customer keys. @@ - /** @description Filtering by multiple customers. + /** @description Filter by multiple customer IDs.
26078-26079: Clarify excludeInactive semantics.Current text is vague (“scheduled for later or earlier”).
Apply this diff:
- /** @description Exclude inactive entitlements in the response (those scheduled for later or earlier) */ + /** @description Exclude entitlements not active at request time (now not within their active window, e.g., startAt..endAt). */
26080-26095: Document pagination parameter exclusivity (page/pageSize vs offset/limit).Having both styles without guidance is confusing.
Apply this diff:
- /** @description Page index. - * - * Default is 1. */ + /** @description Page index (1-based). Do not use together with offset/limit. + * + * Default is 1. */ @@ - /** @description The maximum number of items per page. - * - * Default is 100. */ + /** @description The maximum number of items per page. Do not use together with offset/limit. + * + * Default is 100. */ @@ - /** @description Number of items to skip. - * - * Default is 0. */ + /** @description Number of items to skip. Do not use together with page/pageSize. + * + * Default is 0. */ @@ - /** @description Number of items to return. - * - * Default is 100. */ + /** @description Number of items to return. Do not use together with page/pageSize. + * + * Default is 100. */Also confirm the zod validators enforce mutual exclusivity (or precedence) for these params.
api/client/go/client.gen.go (3)
8068-8069: Clarify docs for ExcludeInactive and ordering examples (update OpenAPI, then regen).The ExcludeInactive description is ambiguous; add explicit “relative to now” wording and provide order/orderBy examples to reduce client misuse.
Apply in the OpenAPI descriptions so regeneration preserves it:
- // ExcludeInactive Exclude inactive entitlements in the response (those scheduled for later or earlier) + // ExcludeInactive Exclude entitlements not active at request time + // (scheduled to start in the future or already expired).- // Order The order direction. + // Order The order direction. + // Usage: `?order=asc` or `?order=desc` Order *EntitlementOrderByOrderingOrder `form:"order,omitempty" json:"order,omitempty"` - // OrderBy The order by field. + // OrderBy The order by field. + // Example: `?orderBy=feature` OrderBy *EntitlementOrderByOrderingOrderBy `form:"orderBy,omitempty" json:"orderBy,omitempty"`Also applies to: 8091-8095
23523-23731: Prevent mixing page/pageSize with offset/limit (optional guard or spec constraint).Right now both pagination styles can be sent together. Consider a defensive guard here, or encode mutual exclusivity in the OpenAPI (oneOf) so generators enforce it.
Example guard (OK to keep in a thin wrapper instead of editing generated code):
func NewListEntitlementsV2Request(server string, params *ListEntitlementsV2Params) (*http.Request, error) { @@ - if params != nil { + if params != nil { + // Defensive: don't allow mixing page/pageSize with offset/limit + if (params.Page != nil || params.PageSize != nil) && (params.Offset != nil || params.Limit != nil) { + return nil, fmt.Errorf("do not mix page/pageSize with offset/limit") + } queryValues := queryURL.Query()
28988-29000: Response wrapper for ListEntitlementsV2 is fine; consider small convenience helpers in non-generated code.Optional: add helpers like Ok() to simplify client checks.
Example (place in a hand-written helper file):
func (r *ListEntitlementsV2Response) Ok() bool { return r.StatusCode() >= 200 && r.StatusCode() < 300 }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/api.gen.go(6 hunks)api/client/go/client.gen.go(9 hunks)api/client/javascript/src/client/schemas.ts(2 hunks)api/client/javascript/src/zod/index.ts(1 hunks)api/openapi.cloud.yaml(1 hunks)api/openapi.yaml(1 hunks)api/spec/src/entitlements/v2/entitlements.tsp(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- api/openapi.yaml
- api/client/javascript/src/zod/index.ts
- api/api.gen.go
🧰 Additional context used
🧬 Code graph analysis (1)
api/client/go/client.gen.go (1)
api/api.gen.go (19)
Feature(3273-3302)EntitlementType(3212-3212)PaginationPage(7635-7635)PaginationPageSize(7638-7638)LimitOffsetOffset(7557-7557)LimitOffsetLimit(7554-7554)EntitlementOrderByOrderingOrder(7497-7497)EntitlementOrderByOrderingOrderBy(7500-7500)ListEntitlementsV2Params(8689-8738)EntitlementV2PaginatedResponse(3221-3233)BadRequestProblemResponse(1134-1134)UnauthorizedProblemResponse(7222-7222)ForbiddenProblemResponse(3512-3512)PreconditionFailedProblemResponse(5975-5975)InternalServerErrorProblemResponse(3621-3621)ServiceUnavailableProblemResponse(6317-6317)UnexpectedProblemResponse(7226-7226)EntitlementV2(3216-3218)NotFoundProblemResponse(4964-4964)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: E2E
- GitHub Check: Quickstart
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Analyze (go)
🔇 Additional comments (18)
api/openapi.cloud.yaml (4)
11356-11547: Overall: V2 endpoints look consistent and well-formed.Good use of array query params with form style, proper ULID pattern on path, and standard problem responses.
11484-11491: ULID path param looks correct.Pattern and example match a 26-char Crockford Base32 ULID (case-insensitive with leading [0–7]).
11499-11516: Confirm security is applied.401/403 responses imply auth, but no per-op security block is shown. If these rely on global security, all good; otherwise add the appropriate
security:stanza here.Also applies to: 11441-11447
11452-11457: Validate 412 usage.412 is listed, but the operations don’t advertise precondition headers. Confirm server supports ETag/If-* on these GETs; otherwise remove 412 from responses.
Also applies to: 11522-11527
api/spec/src/entitlements/v2/entitlements.tsp (3)
7-9: Imports look correct for decorators in use
TypeSpec.HttpandTypeSpec.OpenAPIare appropriate here.
12-14: Surface and tagging LGTMRoute base and tag are consistent with v2 naming.
61-64: Confirm dual pagination styles are intentional and define precedenceHaving both page/pageSize (QueryPagination) and limit/offset (QueryLimitOffset) can cause ambiguity if callers send both. Define precedence in docs or drop one.
api/client/javascript/src/client/schemas.ts (3)
2334-2354: LGTM: GET-only V2 entitlements paths are wired correctly.GET allowed, others explicitly forbidden; path-level params set to never; op-level params defined below. Looks good.
Also applies to: 2355-2374
2342-2345: Verify admin-only enforcement for list endpoint.Docs mark it “administrative purposes only.” Please confirm server enforces appropriate authz/roles and returns 403 for non-admins.
26106-26179: LGTM: response shapes and error coverage look complete.200 payloads and problem+json mappings (400/401/403/404/412/500/503/default) align with expectations.
Also applies to: 26191-26273
api/client/go/client.gen.go (8)
11857-11861: New V2 client interface methods are consistent and minimal.Signatures match the params/types added in the server and keep parity with existing patterns.
14318-14328: Client methods follow established pattern (builder → editors → do).Correct use of WithContext and applyEditors; no issues.
Also applies to: 14330-14340
23732-23764: Path builder for GetEntitlementByIdV2 looks correct.Uses simple style for path param and preserves server base path semantics.
29017-29029: GetEntitlementByIdV2Response maps 404 explicitly — good.Covers the expected error set including NotFound.
30884-30900: WithResponse wrappers correctly delegate and parse.Pattern matches existing endpoints; no issues.
43681-43949: Embedded swagger updated — aligns with new V2 endpoints.No further action from client side.
23523-23731: Confirm entitlementType is an exploded enum array in the OpenAPI spec.Search shows entitlementType defined as a query array (style: form) at api/openapi.yaml:4001 and :11397 (and the matching api/openapi.cloud.yaml locations) but no explicit explode: true or schema.items.enum were found. Add explode: true on the parameter and specify schema.items.enum with the allowed values (e.g., metered, boolean) so clients serialize as repeated params (?entitlementType=metered&entitlementType=boolean).
42492-42565: Add 429 Too Many Requests to OpenAPI responses for rate-limited entitlement endpointsAdd explicit 429 responses in api/openapi*.yaml for entitlement routes so the generated Go client maps them to ApplicationProblemJSON429 instead of the generic default.
Verify generated client mapping at api/client/go/client.gen.go (approx. lines 42492–42565 and 42567–42648).
Repo search failed ("server: No such file or directory"); OpenAPI files weren't found in the sandbox — confirm api/openapi*.yaml paths, add 429 responses, and regenerate the client for verification.
2 participants
Overview
V2 for list and get
Fixes #(issue)
Notes for reviewer
Summary by CodeRabbit