-
Notifications
You must be signed in to change notification settings - Fork 152
feat(api): v3 apps, billing profiles and customer billing #3779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a comprehensive Apps and Billing surface: new Apps (Stripe, sandbox, external invoicing) specs and operations, BillingProfile/workflow/tax types and CRUD operations, customer billing models/endpoints (including Stripe checkout/portal session flows), new shared/invoice types, generated Go server stubs, and two new linter rules for AIP casing and friendly-name enforcement. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant CustomerAPI as Customer Billing Service
participant AppsAPI as Apps Service
participant Stripe as Stripe API
Client->>CustomerAPI: POST /customers/{id}/billing/stripe/checkout (stripe_options)
CustomerAPI->>AppsAPI: Resolve BillingProfile & AppCustomerData (resolve app refs)
AppsAPI-->>CustomerAPI: Return app configs (Stripe settings)
CustomerAPI->>AppsAPI: Build session payload (consent, tax, customer resolution)
AppsAPI->>Stripe: POST /checkout/sessions (session payload)
Stripe-->>AppsAPI: session_id, url, client_secret
AppsAPI-->>CustomerAPI: CreateStripeCheckoutSessionResult
CustomerAPI-->>Client: 201 CreateResponse (session info)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
d0258a2 to
9f2d17c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/spec/lib/rules/friendly-name.js`:
- Around line 84-112: The interface handler accesses node.name without checking
for null like the model/enum/union handlers; modify the interface callback (the
function assigned to "interface") to first guard that node.name exists (e.g.,
return early or skip the endsWith checks when node.name is falsy) before
computing isEndpointsOrOperations and calling node.name.endsWith; preserve
existing diagnostics logic (reportDiagnostic with messageId 'avoid' or
'default') and only perform those checks when node.name is non-null.
In `@api/spec/src/v3/shared/properties.tsp`:
- Around line 143-148: Update the block comment for the interval field to
accurately describe that it holds a full ISO 8601 duration string (e.g., "P1M")
rather than a unit of time: change the JSDoc above the interval:
ISO8601Duration; declaration to something like "The interval duration in ISO
8601 format (e.g., 'P1M')." and remove or replace the incorrect "The unit of
time for the interval" line; leave the existing `@summary` and `@example` as-is.
🧹 Nitpick comments (8)
api/spec/src/v3/apps/customer.tsp (2)
5-23: Looks good overall!The model is well-documented with clear summaries and appropriate visibility settings. One tiny nit: since
AppCustomerDataStripeandAppCustomerDataCustomInvoicingare in the sameAppsnamespace, you could drop theApps.prefix on lines 15 and 22. It's not wrong as-is, just a bit more verbose than needed.✨ Optional: remove redundant namespace prefix
- stripe?: Apps.AppCustomerDataStripe; + stripe?: AppCustomerDataStripe;- custom_invoicing?: Apps.AppCustomerDataCustomInvoicing; + custom_invoicing?: AppCustomerDataCustomInvoicing;
29-43: Consider adding@visibilitydecorators for consistency.The fields in
AppCustomerDatahave explicit@visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update)decorators, but these fields inAppCustomerDataStripedon't. Depending on how the codegen toolchains handle nested models, this inconsistency might lead to unexpected behavior (e.g., fields being excluded from certain operations).If these fields should follow the same read/create/update pattern, adding the visibility decorators would keep things uniform and predictable.
♻️ Suggested change
`@summary`("Stripe customer ID") `@example`("cus_1234567890") + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) customer_id?: string; /** * The Stripe default payment method ID. */ `@summary`("Stripe default payment method ID") `@example`("pm_1234567890") + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) default_payment_method_id?: string;api/spec/lib/rules/utils.js (1)
1-4: Consider compiling the regex once for better performance.The regex is reconstructed on every call to
isPascalCaseNoAcronyms. SincepascalCaseExceptionsis a constant, you could compile the regex once at module load time.♻️ Suggested optimization
/** * Exceptions for PascalCase naming convention. */ const pascalCaseExceptions = ['OAuth2', 'URL', 'API', 'UI', 'ID'] +const pascalCaseRegex = new RegExp( + `^(?:[A-Z][a-z0-9]+|${pascalCaseExceptions.join('|')})+[A-Z]?$|^[A-Z]+$`, +) /** * Checks whether a given value is in PascalCase * `@param` value the value to check * `@returns` true if the value is in PascalCase */ export function isPascalCaseNoAcronyms(value) { if (value === undefined || value === null || value === '') { return true } - return new RegExp( - `^(?:[A-Z][a-z0-9]+|${pascalCaseExceptions.join('|')})+[A-Z]?$|^[A-Z]+$`, - ).test(value) + return pascalCaseRegex.test(value) }Also applies to: 16-18
api/spec/lib/rules/friendly-name.js (1)
114-158: Consider extracting shared handlers to reduce duplication.The
model,enum, andunionhandlers are identical betweenfriendlyNameRuleandfriendlyNameAIPRule. You could extract these into shared helper functions to keep things DRY. Not a blocker, but would make maintenance easier!api/spec/src/v3/customers/operations.tsp (1)
105-113: Minor: Summary says "Update" but operation isupsert.The
@summarysays "Update customer billing data" but the operation is namedupsert, which semantically means create-or-update (not just update). Consider aligning the summary with the operation semantics.Suggested fix
`@put` `@operationId`("upsert-customer-billing") - `@summary`("Update customer billing data") + `@summary`("Upsert customer billing data") `@extension`(Shared.InternalExtension, true)api/spec/src/v3/billing/profile.tsp (1)
77-91: Missing@visibilitydecorator onalignmentfield.The
intervalfield on line 90-91 has@visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update), but thealignmentfield on line 80 doesn't have any visibility decorator. For consistency (and to ensure the field appears in the expected lifecycle phases), consider adding the same visibility:Suggested fix
/** * The alignment for collecting the pending line items into an invoice. */ + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) alignment?: BillingWorkflowCollectionAlignment = DefaultBillingWorkflowCollectionAlignment;api/spec/src/v3/apps/stripe.tsp (2)
66-68: Consider usingint64for thecreatedtimestamp.Unix timestamps will overflow
int32after January 19, 2038. While that's a ways off, usingint64(likeexpires_aton line 312) would future-proof this field:Suggested fix
/** * Unix timestamp when the event was created. */ - created: int32; + created: int64;
196-201: Consider adding an explicit discriminator to thecustomerunion.The
StripeCustomerId | StripeCustomerKey | StripeCustomerCreateunion is structurally distinguishable (each model has different fields), and your examples show clear usage patterns. However, the codebase already uses@discriminated()elsewhere for unions, which helps OpenAPI generators handle polymorphism more reliably. Adding an explicit discriminator here (like the pattern inprofile.tspandapp.tsp) would make this more consistent and future-proof.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (32)
api/spec/lib/index.jsapi/spec/lib/rules/casing.jsapi/spec/lib/rules/friendly-name.jsapi/spec/lib/rules/utils.jsapi/spec/src/v3/apps/app.tspapi/spec/src/v3/apps/capability.tspapi/spec/src/v3/apps/catalog.tspapi/spec/src/v3/apps/customer.tspapi/spec/src/v3/apps/custominvoicing.tspapi/spec/src/v3/apps/index.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/apps/sandbox.tspapi/spec/src/v3/apps/stripe.tspapi/spec/src/v3/billing/index.tspapi/spec/src/v3/billing/profile.tspapi/spec/src/v3/billing/tax.tspapi/spec/src/v3/customers/billing.tspapi/spec/src/v3/customers/operations.tspapi/spec/src/v3/entitlements/access.tspapi/spec/src/v3/entitlements/entitlements.tspapi/spec/src/v3/invoices/index.tspapi/spec/src/v3/invoices/invoice.tspapi/spec/src/v3/invoices/party.tspapi/spec/src/v3/konnect.tspapi/spec/src/v3/openmeter.tspapi/spec/src/v3/shared/address.tspapi/spec/src/v3/shared/index.tspapi/spec/src/v3/shared/properties.tspapi/spec/src/v3/subscriptions/operations.tspapi/spec/src/v3/subscriptions/subscription.tspapi/v3/api.gen.goapi/v3/server/routes.go
🚧 Files skipped from review as they are similar to previous changes (13)
- api/spec/src/v3/entitlements/access.tsp
- api/spec/src/v3/apps/index.tsp
- api/spec/src/v3/subscriptions/operations.tsp
- api/spec/src/v3/customers/billing.tsp
- api/spec/src/v3/apps/capability.tsp
- api/spec/src/v3/invoices/invoice.tsp
- api/spec/src/v3/apps/app.tsp
- api/spec/src/v3/entitlements/entitlements.tsp
- api/spec/src/v3/invoices/index.tsp
- api/spec/lib/index.js
- api/spec/src/v3/konnect.tsp
- api/spec/src/v3/shared/index.tsp
- api/spec/src/v3/billing/index.tsp
🧰 Additional context used
📓 Path-based instructions (2)
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/v3/openmeter.tspapi/spec/src/v3/shared/address.tspapi/spec/src/v3/apps/catalog.tspapi/spec/src/v3/customers/operations.tspapi/spec/src/v3/apps/customer.tspapi/spec/src/v3/billing/profile.tspapi/spec/src/v3/invoices/party.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/subscriptions/subscription.tspapi/spec/src/v3/shared/properties.tspapi/spec/src/v3/apps/custominvoicing.tspapi/spec/src/v3/apps/sandbox.tspapi/spec/src/v3/apps/stripe.tspapi/spec/src/v3/billing/tax.tsp
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
api/v3/server/routes.goapi/v3/api.gen.go
🧠 Learnings (1)
📚 Learning: 2025-11-25T15:10:24.409Z
Learnt from: CR
Repo: openmeterio/openmeter PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T15:10:24.409Z
Learning: Generate API clients and OpenAPI spec via `make gen-api` from TypeSpec definitions
Applied to files:
api/spec/src/v3/apps/operations.tsp
🧬 Code graph analysis (1)
api/spec/lib/rules/casing.js (1)
api/spec/lib/rules/utils.js (2)
isPascalCaseNoAcronyms(11-19)isSnakeCase(39-45)
⏰ 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). (8)
- GitHub Check: Artifacts / Benthos Collector Container image
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Test
- GitHub Check: Code Generators
- GitHub Check: Migration Checks
- GitHub Check: Repository Scan
- GitHub Check: Analyze (go)
🔇 Additional comments (29)
api/spec/src/v3/apps/customer.tsp (1)
45-56: LGTM!This model is well-documented and consistent with the visibility pattern used in
AppCustomerData. The reference toCommon.Labelsis a nice reuse of shared types.api/spec/src/v3/apps/sandbox.tsp (1)
1-11: Clean and well-structured TypeSpec model.The
AppSandboxmodel is properly documented, uses the correct namespace, and follows the@friendlyNamedecorator convention. The spread ofAppBase<AppType.Sandbox>is the right pattern for extending the base app type.api/spec/src/v3/shared/properties.tsp (1)
122-141: Nice additions to the shared properties.The
TaxIdentificationCodescalar andRecurringPeriodmodel are well-defined with appropriate constraints and documentation. Good use of@friendlyNamedecorators and helpful@exampleannotations.api/spec/lib/rules/casing.js (2)
86-106: Good separation of enum name and member name validation.The updated enum handler now properly validates both the enum name and individual member names use PascalCase. Clean implementation!
110-133: Nice addition of a dedicated rule for enum values.Breaking out the enum value validation into its own rule with
severity: 'error'makes sense - it keeps naming convention warnings separate from stricter value format requirements. The defensive check for string values on line 121 is a good touch.api/spec/src/v3/customers/operations.tsp (2)
10-10: Looks good!Import for the new billing types is properly placed with the other local imports.
95-103: Nice clean interface definition!The
getoperation follows the established patterns in this file nicely. The internal/private extensions are appropriately applied, and the response types are consistent with other operations.api/spec/src/v3/apps/catalog.tsp (2)
7-26: Solid enum definition!The
AppInstallMethodsenum is well-documented. Keeping the OAuth2 option commented out for future use is a reasonable approach. The naming convention follows the pattern in the codebase.
28-89: Great model with a thorough example!The
AppCatalogItemmodel is well-structured:
- Comprehensive example using Stripe as a real-world scenario
- All fields are read-only (makes sense for catalog items)
- Good documentation for each field
The example at lines 33-58 really helps API consumers understand the expected shape.
api/spec/src/v3/shared/address.tsp (1)
5-58: Well-structured Address model!The model is clean and follows TypeSpec conventions nicely:
- Proper visibility annotations on all fields
- Good use of
CountryCodetype for country validation- Consistent snake_case naming
One observation: including
phone_numberin Address is a bit unconventional (typically contact info is separate from postal address), but it's a reasonable design choice if that's how the billing domain models addresses.api/spec/src/v3/billing/tax.tsp (3)
3-28: TaxConfig model looks good!The structure is clean and the documentation explains the fallback behavior well.
One thing to consider: other models in the spec use
@visibilityannotations on their fields, but these fields don't have them. If this model is used in create/read/update contexts, you might want to add visibility annotations for consistency with the rest of the API.
48-60: Nice validation on Stripe tax codes!The
@pattern("^txcd_\\d{8}$")regex correctly validates Stripe's tax code format, and the exampletxcd_10000000demonstrates a valid value. This will help catch invalid codes early.
62-73: Solid custom invoicing config!The
@maxLength(64)constraint is a reasonable upper bound. The documentation clearly states the code is interpreted by the custom invoicing provider.api/spec/src/v3/invoices/party.tsp (3)
5-21: Excellent documentation of design decisions!Really appreciate this comment block explaining what was intentionally omitted and why. This kind of context is super helpful for future maintainers wondering "why didn't they include X?"
23-59: BillingParty model is well-structured!The visibility annotations are thoughtfully applied:
idis read-only (system-generated)keyis create+read (immutable after creation)- Other fields support full CRUD
One small question:
addressesis an array with@maxItems(1). If there's only ever one address allowed, is there a reason to use an array rather than a singleaddress?: Shared.Address? This might be intentional for API evolution or alignment with external standards - just curious!
61-71: Clean tax identity wrapper!This thin wrapper around the tax identification code provides good extensibility if you need to add more tax identity fields later (like country or type).
api/spec/src/v3/subscriptions/subscription.tsp (1)
12-17: Enum naming convention looks good!The switch to PascalCase identifiers (e.g.,
Active: "active") while preserving the original snake_case string values is the right approach. This aligns with TypeSpec conventions and keeps the wire format stable. The@exampleupdate on line 36 correctly references the renamedSubscriptionEditTimingEnum.Immediatemember.Also applies to: 25-28, 36-36
api/spec/src/v3/billing/profile.tsp (2)
226-298: App models and payment settings look solid.The
BillingProfileAppsandBillingProfileAppReferencesmodels provide a clean pattern for expandable responses. TheCollectionMethodenum andBillingWorkflowPaymentSettingsare well-documented with sensible defaults.
31-32: Consider using@oneOfand@discriminatedfor theappsfield if disambiguation is needed.TypeSpec unions emit as
anyOfby default in OpenAPI, which can be ambiguous for consumers. If the expand settings truly govern which variant is returned (as your doc comment suggests), and the variants aren't structurally obvious, adding@oneOfwith@discriminatedwill help generate a proper OpenAPIoneOfwith a discriminator. This lets consumers reliably distinguish betweenBillingProfileAppsandBillingProfileAppReferencesat runtime. Check if the models have natural discriminator properties before applying this—you might not need it if they're already structurally distinct.api/spec/src/v3/openmeter.tsp (1)
53-56: Nice addition for the customer billing endpoint.The new
CustomerBillingEndpointsinterface follows the existing patterns nicely - nested under/openmeter/customers/{customerId}/billing, tagged appropriately withCustomersTag, and extending the operations interface. Clean and consistent with the rest of the file.api/spec/src/v3/apps/custominvoicing.tsp (3)
7-51: Excellent documentation on the integration workflow!The doc comments on
AppCustomInvoicingclearly explain the bi-directional sync pattern, state hooks, and integration flow. This is really helpful for developers implementing the integration. The suppress directive for repeated prefix grouping makes sense here since the hooks are conceptually distinct settings.
99-145: Mapping models look clean.The
CustomInvoicingLineExternalIdMappingandCustomInvoicingLineDiscountExternalIdMappingmodels provide a straightforward way to link internal IDs to external system IDs. Documentation is clear about the purpose of each field.
238-300: Payment triggers cover the key scenarios well.The
CustomInvoicingPaymentTriggerenum has a good set of states for payment lifecycle management - from success (Paid) through various failure modes (PaymentFailed,PaymentUncollectible) to administrative actions (Void). The detailed doc comments for each state are a nice touch.api/spec/src/v3/apps/stripe.tsp (3)
32-37: Good security practice with the secret API key handling.Using
@secretdecorator and limiting visibility toCreateandUpdate(excludingRead) ensures the secret API key is never exposed in API responses. Themasked_api_keyfield provides a safe way to show users which key is configured. Nice pattern!
745-758: Single-value enum for extensibility - makes sense.Having
StripeCheckoutSessionModewith justSetupfor now leaves room to addPaymentandSubscriptionmodes later. The doc comment on line 725-726 clearly states the current limitation. Good forward-thinking design.
591-639: Custom text params are well-structured.The nested objects for each text block (
after_submit,shipping_address,submit,terms_of_service_acceptance) allow room for Stripe to add more properties to each block in the future. The@maxLength(1200)constraint correctly reflects Stripe's limits.api/spec/src/v3/apps/operations.tsp (1)
1-14: LGTM! Clean operations aggregation file.The file properly sets up the imports and using directives for the v3 Apps module. One small thing to consider: you're importing
@typespec/reston line 2 but there's no correspondingusing TypeSpec.Rest;statement. If REST decorators aren't used directly in this file, that import might be unnecessary—though it could be needed transitively by the imported local modules. Worth a quick check if you want to keep the imports minimal!api/v3/api.gen.go (1)
1-4: Generated code looks good!This is auto-generated code from oapi-codegen, so any changes would need to go through the TypeSpec source files. The generated patterns for discriminated unions (the
As*,From*,Merge*helpers andMarshalJSON/UnmarshalJSONimplementations) follow the expected oapi-codegen conventions. The new types and server interface methods align well with the v3 billing/apps API surface described in the PR.api/v3/server/routes.go (1)
86-161: Nice stub pattern for the new endpoints!This cleanly adds the new API surface (App Catalog, Billing Profiles, Customer Billing, Stripe integration) while delegating to the
Unimplementedstruct that returns 501 Not Implemented. This matches the PR objective of "API spec only, unimplemented endpoints."A few thoughts for when these get implemented later:
- The existing handlers follow a pattern like
s.metersHandler.CreateMeter().ServeHTTP(w, r)with dedicated handler structs- You'll likely want to follow the same pattern when implementing these (e.g.,
s.billingProfilesHandler,s.appCatalogHandler, etc.)- Consider grouping related endpoints into handler structs similar to the existing
metersHandler,customersHandlerpatternsFor now though, this is exactly what's needed to expose the API contract!
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
9f2d17c to
5d3cb75
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 65-68: The schema field "created" is currently typed as int32
which will overflow in 2038; change its type to int64 in the Stripe event
interface definition (the "created" field declaration) and update any associated
serializers/parsers, validation logic, and tests that expect int32 so they
accept/produce 64-bit timestamps (ensure type annotations, generated code, and
any JSON/Protobuf mapping are updated to use 64-bit integer handling).
♻️ Duplicate comments (1)
api/spec/lib/rules/friendly-name.js (1)
84-113: Missing null check onnode.namefor interfaces infriendlyNameAIPRule.Same issue as previously flagged - line 89 accesses
node.name.endsWith()without checking ifnode.nameexists. While interfaces typically always have names, staying consistent with the other handlers would be nice.
🧹 Nitpick comments (12)
api/spec/lib/rules/friendly-name.js (2)
11-26: Missing null check onnode.namefor interfaces infriendlyNameRule.The model, enum, and union handlers all guard
node.namebefore accessing it, but the interface handler here usesnode.namedirectly on line 20 without that check. For consistency with the other handlers (and defensive coding), you might want to add the same guard.📝 Suggested fix for consistency
interface: (node) => { + if (!node.name) { + return + } + const hasFriendlyName = node.decorators.some( (d) => d.decorator.name === '$friendlyName', )
75-160: Consider extracting shared handler logic to reduce duplication.The model, enum, and union handlers are identical between
friendlyNameRuleandfriendlyNameAIPRule. You could extract a helper function to DRY this up a bit. Totally optional though - the current approach is clear and works fine.♻️ Example helper extraction
// Helper function to create common handlers const createCommonHandlers = (context) => ({ model: (node) => { if ( node.name && !node.decorators.some((d) => d.decorator.name === '$friendlyName') ) { context.reportDiagnostic({ format: { type: node.kind, name: node.name }, target: node, messageId: 'default', }) } }, enum: (node) => { // ... same pattern }, union: (node) => { // ... same pattern }, }) // Then in each rule's create function: create: (context) => ({ interface: (node) => { /* rule-specific logic */ }, ...createCommonHandlers(context), }),api/spec/src/v3/shared/consts.tsp (1)
20-24: Nice additions for the new Apps and Billing domains!The constants follow the established pattern well. One small thought: the
BillingDescriptionis noticeably shorter than the others (compare toAppsDescriptionright above it). Consider expanding it to give users a better sense of what the Billing domain covers - maybe mention invoice lifecycle, payment tracking, or how it integrates with apps?api/spec/src/v3/apps/capability.tsp (1)
48-68: Clean capability model design!The structure is straightforward and the example is helpful. One thing to consider: the
nameanddescriptionfields don't have any length constraints. Other parts of the API use@minLength/@maxLengthfor string fields (likeInvoiceNumber). Depending on how this data gets stored or displayed, you might want to add some reasonable bounds here too.💡 Optional: Add length constraints
/** * Name of the capability. */ + `@minLength`(1) + `@maxLength`(256) name: string; /** * Description of the capability. */ + `@maxLength`(1024) description: string;api/spec/src/v3/billing/operations.tsp (1)
68-69: Missing blank line before delete methodTiny formatting nit - there's a blank line between other methods but not before
delete. Keeps things consistent!✨ Add blank line
): Shared.UpdateResponse<BillingProfile> | Common.NotFound | Common.ErrorResponses; + /** * Delete a billing profile.api/spec/src/v3/apps/custominvoicing.tsp (1)
105-106: Minor casing inconsistency in friendlyName.The
friendlyNameuses "ID" (uppercase) while the model name uses "Id" (mixed case). Consider aligning them for consistency:-@friendlyName("BillingAppCustomInvoicingLineExternalIDMapping") +@friendlyName("BillingAppCustomInvoicingLineExternalIdMapping")api/spec/src/v3/billing/tax.tsp (1)
62-73: Consider adding a pattern or format hint for custom invoicing tax code.The
maxLength(64)constraint is good, but unlike the Stripe config, there's no guidance on expected format. If there are any common formats or examples for custom invoicing tax codes, adding an@exampledecorator would help API consumers.api/spec/src/v3/apps/app.tsp (1)
90-99: Consider adding atypefield to AppReference for consistency.The
AppReferencemodel only has anidfield. While this is functional, you might want to consider whether consumers would benefit from knowing the app type when they receive a reference. This would help with client-side routing or validation without requiring an additional lookup.api/spec/src/v3/billing/profile.tsp (1)
81-102: Collection settings interval default might need documentation clarification.The
intervalfield has@example("P1D")(1 day) but defaults to"PT1H"(1 hour). While this isn't technically wrong, it might confuse API consumers since the example differs from the default. Consider aligning them or adding a note.📝 Suggested alignment
Either change the example to match the default:
- `@example`("P1D") + `@example`("PT1H")Or update the default to match the example if P1D is more realistic:
- interval?: string = "PT1H"; + interval?: string = "P1D";api/spec/src/v3/apps/operations.tsp (3)
105-180: Consider addingCommon.NotFoundto return types.The operations in
AppCustomInvoicingOperationsall take aninvoiceIdpath parameter, but the return types only includevoid | Common.ErrorResponses. If the invoice doesn't exist, a 404 response would be expected. Compare withAppCatalogOperations.getwhich correctly includesCommon.NotFound.🔧 Proposed fix
draftSynchronized( `@path` invoiceId: Shared.ULID, `@body` body: CustomInvoicingDraftSynchronizedRequest, - ): void | Common.ErrorResponses; + ): void | Common.NotFound | Common.ErrorResponses; issuingSynchronized( `@path` invoiceId: Shared.ULID, `@body` body: CustomInvoicingFinalizedRequest, - ): void | Common.ErrorResponses; + ): void | Common.NotFound | Common.ErrorResponses; updatePaymentStatus( `@path` invoiceId: Shared.ULID, `@body` body: CustomInvoicingUpdatePaymentStatusRequest, - ): void | Common.ErrorResponses; + ): void | Common.NotFound | Common.ErrorResponses;
185-212: Consider addingCommon.NotFoundfor the webhook handler.The
handleWebhookoperation takes anappIdpath parameter, but if that app doesn't exist, callers would expect a 404. The return type currently only hasStripeWebhookResponse | Common.ErrorResponses.🔧 Proposed fix
handleWebhook( `@path` appId: Shared.ULID, `@body` body: StripeWebhookEvent, - ): StripeWebhookResponse | Common.ErrorResponses; + ): StripeWebhookResponse | Common.NotFound | Common.ErrorResponses;
1-4: Remove unused imports.The imports for
@typespec/restand@typespec/openapi3aren't actually used anywhere in this file—there's no correspondingusingstatement for either, and no decorators from those namespaces appear in the code. Removing them keeps things clean and reduces unnecessary dependencies.🧹 Proposed cleanup
import "@typespec/http"; -import "@typespec/rest"; import "@typespec/openapi"; -import "@typespec/openapi3"; import "../common/error.tsp";
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
api/client/go/client.gen.gois excluded by!api/client/**api/client/javascript/src/client/schemas.tsis excluded by!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yamlapi/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (37)
api/api.gen.goapi/spec/lib/index.jsapi/spec/lib/rules/casing.jsapi/spec/lib/rules/friendly-name.jsapi/spec/lib/rules/utils.jsapi/spec/src/app/marketplace.tspapi/spec/src/app/stripe.tspapi/spec/src/v3/apps/app.tspapi/spec/src/v3/apps/capability.tspapi/spec/src/v3/apps/catalog.tspapi/spec/src/v3/apps/customer.tspapi/spec/src/v3/apps/custominvoicing.tspapi/spec/src/v3/apps/index.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/apps/sandbox.tspapi/spec/src/v3/apps/stripe.tspapi/spec/src/v3/billing/index.tspapi/spec/src/v3/billing/operations.tspapi/spec/src/v3/billing/profile.tspapi/spec/src/v3/billing/tax.tspapi/spec/src/v3/customers/billing.tspapi/spec/src/v3/customers/operations.tspapi/spec/src/v3/entitlements/access.tspapi/spec/src/v3/entitlements/entitlements.tspapi/spec/src/v3/invoices/index.tspapi/spec/src/v3/invoices/invoice.tspapi/spec/src/v3/invoices/party.tspapi/spec/src/v3/konnect.tspapi/spec/src/v3/openmeter.tspapi/spec/src/v3/shared/address.tspapi/spec/src/v3/shared/consts.tspapi/spec/src/v3/shared/index.tspapi/spec/src/v3/shared/properties.tspapi/spec/src/v3/subscriptions/operations.tspapi/spec/src/v3/subscriptions/subscription.tspapi/v3/api.gen.goapi/v3/server/routes.go
🚧 Files skipped from review as they are similar to previous changes (13)
- api/spec/lib/rules/utils.js
- api/spec/src/v3/apps/sandbox.tsp
- api/spec/src/v3/apps/catalog.tsp
- api/spec/src/v3/shared/address.tsp
- api/spec/src/v3/shared/properties.tsp
- api/spec/lib/rules/casing.js
- api/spec/src/v3/customers/operations.tsp
- api/spec/src/v3/subscriptions/subscription.tsp
- api/spec/src/v3/invoices/index.tsp
- api/spec/src/v3/apps/index.tsp
- api/spec/src/v3/shared/index.tsp
- api/spec/src/v3/apps/customer.tsp
- api/spec/src/v3/entitlements/entitlements.tsp
🧰 Additional context used
📓 Path-based instructions (2)
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/v3/invoices/invoice.tspapi/spec/src/app/stripe.tspapi/spec/src/v3/apps/capability.tspapi/spec/src/v3/subscriptions/operations.tspapi/spec/src/v3/billing/operations.tspapi/spec/src/v3/billing/tax.tspapi/spec/src/v3/apps/app.tspapi/spec/src/v3/entitlements/access.tspapi/spec/src/v3/shared/consts.tspapi/spec/src/v3/openmeter.tspapi/spec/src/v3/invoices/party.tspapi/spec/src/v3/billing/index.tspapi/spec/src/app/marketplace.tspapi/spec/src/v3/customers/billing.tspapi/spec/src/v3/apps/stripe.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/konnect.tspapi/spec/src/v3/billing/profile.tspapi/spec/src/v3/apps/custominvoicing.tsp
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
api/v3/server/routes.go
🧠 Learnings (1)
📚 Learning: 2025-11-25T15:10:24.409Z
Learnt from: CR
Repo: openmeterio/openmeter PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T15:10:24.409Z
Learning: Generate API clients and OpenAPI spec via `make gen-api` from TypeSpec definitions
Applied to files:
api/spec/src/v3/openmeter.tspapi/spec/src/v3/konnect.tsp
🧬 Code graph analysis (2)
api/spec/lib/index.js (2)
api/spec/lib/rules/casing.js (2)
casingAIPErrorsRule(110-133)casingAIPErrorsRule(110-133)api/spec/lib/rules/friendly-name.js (2)
friendlyNameAIPRule(75-160)friendlyNameAIPRule(75-160)
api/v3/server/routes.go (2)
api/v3/api.gen.go (5)
Unimplemented(2939-2939)ListAppCatalogItemsParams(2212-2215)BillingAppType(1170-1170)ULID(2100-2100)ListBillingProfilesParams(2256-2259)api/v3/server/server.go (1)
Server(97-108)
⏰ 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). (8)
- GitHub Check: Quickstart
- GitHub Check: Code Generators
- GitHub Check: Build
- GitHub Check: Migration Checks
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Repository Scan
- GitHub Check: Analyze (go)
🔇 Additional comments (47)
api/spec/src/v3/entitlements/access.tsp (1)
14-14: LGTM!Nice catch updating the example to use
EntitlementType.Staticto match the PascalCase enum member naming. This keeps things consistent with the updated enum definition inentitlements.tsp. 👍api/spec/src/app/marketplace.tsp (1)
181-206: Nice catch on the typo fix! 👍The correction from "interation" → "integration" is good. The example looks well-structured and clearly demonstrates the
MarketplaceListingmodel with all its capabilities.api/spec/src/app/stripe.tsp (1)
669-669: Nice catch on the typo fix!"Stripe integration" is indeed the correct spelling. The example looks good overall - well-structured with realistic values and proper TypeSpec decorator usage.
api/spec/src/v3/subscriptions/operations.tsp (1)
18-24: Looks good! 👍The casing change from
immediatetoImmediatealigns nicely with PascalCase conventions for enum members, which is standard practice in TypeSpec/TypeScript. This keeps things consistent with the related changes insubscription.tsp.api/spec/lib/rules/friendly-name.js (1)
27-56: LGTM!The model and enum handlers are implemented consistently with proper null checks on
node.namebefore accessing it. Clean and straightforward logic.api/spec/lib/index.js (3)
2-8: LGTM!The imports are cleanly organized - bringing in the new AIP-specific rules (
casingAIPErrorsRule,friendlyNameAIPRule) alongside their base counterparts.
18-30: LGTM!New rules properly added to the rules array. The ordering is logical with base rules followed by their AIP variants.
45-56: LGTM!The
aipruleset configuration makes sense - it correctly enables the AIP-specific variants (casingAIPErrorsRule,friendlyNameAIPRule) while thelegacyruleset continues to use the basefriendlyNameRule. Clean separation of concerns!api/spec/src/v3/invoices/invoice.tsp (1)
5-17: Looks good!The
InvoiceNumberscalar is well-designed with sensible constraints. The documentation does a nice job explaining the uniqueness considerations - that's helpful context for consumers. The example format is clear and the length bounds are reasonable.api/spec/src/v3/apps/capability.tsp (1)
5-36: Well-documented enum!The capability types are clearly defined with good doc comments. The snake_case values are consistent with REST conventions. Nice work!
api/spec/src/v3/billing/operations.tsp (2)
65-68: Small naming considerationThe operation is called
updatebut usesShared.UpsertRequest<BillingProfile>. If this is truly an upsert (create-or-update), the naming might confuse consumers. If it's strictly an update (resource must exist), consider using a different request type name. Either way, just something to double-check for API clarity!
16-55: Solid CRUD interface!The operations follow REST conventions nicely, and I like that you've marked everything as internal and unstable since these are unimplemented. The create operation docs explain the billing profile concept well, and the error responses are appropriately typed.
api/spec/src/v3/invoices/party.tsp (3)
5-21: Great documentation!Love that you documented the omitted fields and why they were omitted. This is super helpful for future maintainers who might wonder "why don't we have emails here?"
53-58: Quick design question on addressesSince
@maxItems(1)restricts this to a single address, is there a reason to use an array rather than justaddress?: Shared.Address? Arrays with max 1 item can be a bit awkward for consumers. Totally fine if there's a reason (like future expansion plans or alignment with another spec), just wanted to flag it!
27-59: Clean party model!The visibility decorators are set up nicely -
idbeing read-only,keybeing read/create only, and the rest allowing updates. The field typing with shared types keeps things consistent across the API.api/spec/src/v3/apps/custominvoicing.tsp (2)
7-51: Nice work on the documentation!The integration workflow documentation in the model comment is really helpful — it clearly explains the bidirectional sync pattern and the hook-based state control. This will make the API much easier for integrators to understand.
238-300: Well-designed payment trigger enum.The
CustomInvoicingPaymentTriggerenum covers the essential payment lifecycle states nicely. The documentation for each variant clearly explains when to use it and what state transition it causes.api/spec/src/v3/apps/stripe.tsp (3)
114-159: Great examples for the checkout session request!The multiple examples covering different customer identification scenarios (by ID, by key, with creation, with address/tax collection) are really helpful for API consumers. This is a nice touch for discoverability.
32-37: Good use of@secretdecorator for API key.Properly marking
secret_api_keywith@secretand limiting visibility to Create/Update ensures sensitive data handling is correct. The masked key on Read visibility is a nice pattern.
745-758: Checkout session mode limited to "setup" — intentional?The enum only includes
Setupmode. The documentation mentions "Currently only 'setup' mode is supported" which suggests this is intentional. Just double-checking this is the desired scope for now, as Stripe also supports "payment" and "subscription" modes.api/spec/src/v3/billing/index.tsp (1)
1-3: Clean module organization.Standard barrel file pattern — imports are logically ordered. Looks good!
api/spec/src/v3/customers/billing.tsp (1)
6-34: Well-structured customer billing data model.The model cleanly separates workflow overrides, billing profile references, and app-specific data. Good documentation on the default billing profile fallback behavior. The visibility settings look correct for a CRUD resource.
api/spec/src/v3/billing/tax.tsp (2)
30-46: Clear tax behavior enum.Simple and effective —
inclusiveandexclusivecover the standard tax treatment options. Documentation clearly explains what each value means.
52-60: Stripe tax code pattern is spot on.The regex
^txcd_\d{8}$and the exampletxcd_10000000match Stripe's actual tax code format perfectly. They use the "txcd_" prefix followed by exactly 8 digits, so this validation is exactly right. Adding both the pattern constraint and a concrete example makes it super clear for anyone using this API—nice work!api/spec/src/v3/apps/app.tsp (4)
1-28: Imports and AppType enum look good!Clean import structure and the enum values use snake_case for the wire format which aligns with typical API conventions. The doc comments are helpful for consumers.
30-46: AppStatus enum is well-defined.Good documentation explaining what "unauthorized" means and how to resolve it. This will help API consumers understand the expected behavior.
48-72: AppBase generic model structure is solid.The generic constraint
<T extends AppType>provides good type safety. All fields are read-only which makes sense for these computed/system-managed properties.
74-88: Discriminated union looks correct.Using
envelope: "none"withdiscriminatorPropertyName: "type"is the right pattern for a flat discriminator. The@summarydecorators on each variant help with documentation.api/spec/src/v3/konnect.tsp (4)
10-12: Imports added for apps and billing modules.These imports are necessary to support the new endpoint interfaces. Good placement alongside existing imports.
44-45: Tag metadata additions for Apps and Billing.Consistent with the existing pattern for other tags. This ensures proper OpenAPI documentation grouping.
61-64: CustomerBillingEndpoints nested under customers path.Using
CustomersTagfor customer billing operations makes sense since it's a sub-resource of customers. The route structure/openmeter/customers/{customerId}/billingfollows RESTful conventions nicely.
76-91: New app and billing profile endpoints look well-structured.The route hierarchy makes sense:
/openmeter/app-catalogfor catalog operations/openmeter/apps/custom-invoicingand/openmeter/apps/stripefor app-specific operations/openmeter/profilesfor billing profilesOne small thing to verify: the billing profiles route uses
/openmeter/profilesrather than/openmeter/billing-profiles. Is this intentional for brevity, or should it be more explicit?api/spec/src/v3/openmeter.tsp (3)
10-11: Imports match konnect.tsp.Keeps both API surfaces in sync which is important for feature parity.
41-42: Tag metadata additions consistent with konnect.tsp.
57-87: All new endpoints mirror konnect.tsp correctly.The endpoint definitions and routes are identical to konnect.tsp, ensuring API parity between OpenMeter and Konnect. This is the expected pattern based on the existing file structure.
api/v3/server/routes.go (4)
87-89: Package-levelunimplementedvariable works but consider future refactoring.This approach is fine for the current "API spec only" purpose. When these endpoints get implemented, you'll want to follow the existing pattern with dedicated handlers (like
s.metersHandler,s.customersHandler, etc.) rather than updating each method individually.
91-125: App Catalog and related handlers are consistently structured.All handlers follow the same delegation pattern. A few notes:
pTypeparameter naming (Lines 95, 99, 103) is a bit unusual - typically you'd seeappTypeor justtype_. Thepprefix isn't immediately obvious (perhaps "path"?).The handlers correctly pass through all parameters to the unimplemented stubs.
127-147: Billing Profiles handlers look good.Standard CRUD operations following the existing patterns in the file.
149-161: Customer Billing handlers complete the surface.These round out the customer billing operations. The
CreateCustomerStripeCheckoutSessionnicely captures the Stripe-specific customer flow.api/spec/src/v3/billing/profile.tsp (5)
8-38: BillingProfile model is well-structured.Good use of
Shared.Resourcefor common fields. The visibility modifiers are appropriate - supplier and workflow can be updated, apps are set at creation, and default is mutable.
51-79: BillingWorkflow with optional nested settings is flexible.Making all settings optional with sensible defaults allows for incremental configuration. This is a good API design choice.
122-137: Discriminated union for collection alignment is clean.The const default and union structure work well together. Using a const for the default ensures type safety.
236-282: Visibility mismatch between BillingProfileApps and BillingProfileAppReferences.
BillingProfileApps(Lines 240-258) has all fields withLifecycle.Readonly, whileBillingProfileAppReferences(Lines 264-282) also has onlyLifecycle.Read. However, looking at Line 30-31 inBillingProfile, theappsfield usesBillingProfileAppReferenceswithLifecycle.Read, Lifecycle.Create.This is actually correct - the parent field controls create visibility, and the nested fields are read-only after creation. Just wanted to call out that this is intentional and makes sense!
284-308: CollectionMethod and PaymentSettings are straightforward.Good default of
ChargeAutomaticallyfor payment collection. The@summarydecorators will help with API documentation.api/spec/src/v3/apps/operations.tsp (3)
18-51: LGTM!The models are well-documented with clear explanations for each property. The spread pattern (
...InstallAppRequest) for extending the base request is idiomatic TypeSpec. The default value forcreate_billing_profileis a nice touch for developer ergonomics.
53-100: LGTM!The
AppCatalogOperationsinterface is nicely structured with consistent use of decorators and appropriate return types. IncludingCommon.NotFoundfor operations that take a path parameter is exactly right.
214-239: LGTM!The
createCheckoutSessionoperation is well-documented with helpful context about Checkout Session modes and links to Stripe docs. Since there's no path parameter that could result in a 404, the return type is appropriate here.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
gergely-kurucz-konghq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM besides comments from @turip!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/billing/profile.tsp`:
- Around line 27-31: The spec declares BillingProfile.apps as Create-visible but
BillingProfileAppReferences' fields are Read-only, preventing clients from
sending app IDs on create; update the visibility so request/response parity is
accurate by either making the fields inside BillingProfileAppReferences
Create-visible (add Lifecycle.Create to each field's `@visibility` where
appropriate) so clients can supply app IDs on create, or change
BillingProfile.apps to Read-only if these references are server-assigned; modify
references to the symbols BillingProfile.apps and BillingProfileAppReferences
and adjust the `@visibility`(Lifecycle.Read, Lifecycle.Create) annotations
accordingly.
- Around line 203-225: The spec is inconsistent: either make the workflow tax
settings required or make the per-app tax reference optional; update the schema
so BillingWorkflowTaxSettings behavior matches app optionality—if tax features
can be disabled then change the `tax` field in BillingProfileApps and
BillingProfileAppReferences to be optional (nullable/optional reference) so apps
are not required to supply a tax app, otherwise make BillingWorkflowTaxSettings
non-optional (remove the optional marker) so tax settings must always exist;
locate and update the definitions for BillingWorkflowTaxSettings,
BillingProfileApps, and BillingProfileAppReferences to apply the chosen
consistent rule.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (3)
api/spec/src/v3/billing/profile.tspapi/spec/src/v3/customers/billing.tspapi/v3/api.gen.go
🚧 Files skipped from review as they are similar to previous changes (1)
- api/spec/src/v3/customers/billing.tsp
🧰 Additional context used
📓 Path-based instructions (1)
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/v3/billing/profile.tsp
⏰ 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). (9)
- GitHub Check: Quickstart
- GitHub Check: E2E
- GitHub Check: Test
- GitHub Check: Lint
- GitHub Check: Migration Checks
- GitHub Check: Build
- GitHub Check: Code Generators
- GitHub Check: Repository Scan
- GitHub Check: Analyze (go)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
79a3276 to
f47c252
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 256-369: Update the
CreateStripeCheckoutSessionRequestOptions.currency doc comment to state that
currency codes are uppercase to match Shared.CurrencyCode (which uses regex
^[A-Z]{3}$ and example "USD"); locate the currency field in the
CreateStripeCheckoutSessionRequestOptions model and change the wording from
"lowercase" to "uppercase" (or remove casing note) so the documentation aligns
with the Shared.CurrencyCode definition.
In `@api/spec/src/v3/billing/operations.tsp`:
- Around line 30-35: The doc comment for the "Create a new billing profile"
operation uses the phrase "linked to a specific app" which is ambiguous because
BillingProfile can reference multiple apps; update that sentence in the comment
(the block describing BillingProfile creation) to pluralize and clarify—e.g.,
change "linked to a specific app" to "linked to specific apps" or "linked to one
or more apps (tax/invoicing/payment)"—so the description of BillingProfile in
the operations.tsp comment accurately reflects that multiple apps may be
referenced.
In `@api/spec/src/v3/invoices/party.tsp`:
- Around line 65-70: The BillingPartyTaxIdentity model currently defines code as
optional (code?), which allows empty tax_id objects; change the model so code is
required (remove the optional marker from the code property in
BillingPartyTaxIdentity) and keep the existing `@visibility` annotations, ensuring
consumers simply omit tax_id on BillingParty when no tax info exists rather than
sending an empty object; update any related validators/tests that assume code is
optional to expect a required Shared.TaxIdentificationCode.
♻️ Duplicate comments (1)
api/spec/lib/rules/friendly-name.js (1)
11-24: Guardnode.namebefore use in interface handlers.If an interface is anonymous,
node.namecan be undefined andendsWithwill throw; a quick guard keeps this consistent with other handlers and avoids a hard crash.🔧 Suggested fix
interface: (node) => { + if (!node.name) { + return + } const hasFriendlyName = node.decorators.some( (d) => d.decorator.name === '$friendlyName', )interface: (node) => { + if (!node.name) { + return + } const hasFriendlyName = node.decorators.some( (d) => d.decorator.name === '$friendlyName', ) const isEndpointsOrOperations = node.name.endsWith('Endpoints') || node.name.endsWith('Operations')Also applies to: 84-112
🧹 Nitpick comments (7)
api/spec/lib/rules/casing.js (1)
110-116: Consider a more specific description.The description on line 113 is identical to
casingAIPRule. Since this rule specifically validates enum values are snake_case (and with error severity!), a more descriptive message could help folks understand what tripped the linter.✨ Optional improvement
export const casingAIPErrorsRule = createRule({ name: 'casing-aip-errors', severity: 'error', - description: 'Ensure proper casing style for AIP naming conventions.', + description: 'Ensure enum member values use snake_case for AIP error codes.', messages: { value: paramMessage`The values of ${'type'} types must use ${'casing'}`, },api/spec/src/v3/invoices/party.tsp (1)
53-58: Consider a singularaddressif only one is allowed.
addresses+@maxItems(1)still exposes a list in generated SDKs, and some toolchains ignoremaxItems. If the product truly allows only one address, modeling it as a singular field is clearer and avoids client-side confusion. Please verify with downstream codegen expectations before changing.♻️ Suggested change (if single address only)
- /** - * Regular post addresses for where information should be sent if needed. - */ - `@maxItems`(1) - `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) - addresses?: Shared.Address[]; + /** + * Regular post address for where information should be sent if needed. + */ + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) + address?: Shared.Address;api/spec/src/v3/apps/custominvoicing.tsp (1)
99-121: Consider consistentExternalIdcasing in the friendlyName.
This one usesExternalIDwhile the discount mapping usesExternalId, which could lead to inconsistent schema names in codegen/docs.♻️ Suggested tweak
-@friendlyName("BillingAppCustomInvoicingLineExternalIDMapping") +@friendlyName("BillingAppCustomInvoicingLineExternalIdMapping")api/spec/src/v3/apps/customer.tsp (1)
29-43: Consider adding@visibilitydecorators for consistency.
AppCustomerDataStripefields (customer_id,default_payment_method_id) don't have explicit@visibilitydecorators, whileAppCustomerDataCustomInvoicing.labelsdoes have@visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update).If these Stripe fields should be readable and settable by API consumers, adding explicit visibility would keep things consistent across the models. If they're intentionally different (e.g., system-managed), that's fine too!
♻️ Suggested change for consistency
model AppCustomerDataStripe { /** * The Stripe customer ID used. */ `@summary`("Stripe customer ID") `@example`("cus_1234567890") + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) customer_id?: string; /** * The Stripe default payment method ID. */ `@summary`("Stripe default payment method ID") `@example`("pm_1234567890") + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) default_payment_method_id?: string; }api/spec/src/v3/openmeter.tsp (1)
85-87: Quick question on route path.The billing profiles endpoint uses
/openmeter/profiles, but it's tagged withShared.BillingTag. For discoverability and consistency with other billing-related routes, would/openmeter/billing/profilesmake more sense?That said, if "profiles" is unambiguous in this API and you want to keep routes short, the current path works fine too!
api/spec/src/v3/apps/app.tsp (1)
1-6: Circular imports confirmed—keep an eye on codegen stability.This file imports
./sandbox.tsp,./stripe.tsp,./custominvoicing.tsp, and./catalog.tsp, which all import back to./app.tspforAppBaseandAppType. While TypeSpec typically handles these circular references gracefully, some codegen toolchains can choke on them, especially as the spec grows.If you hit codegen issues down the line, consider extracting the base types (
AppType,AppBase) into a separatebase.tspfile that the concrete models import from, breaking the cycle.api/spec/src/v3/customers/operations.tsp (1)
149-179: Add internal/unstable extensions to the request model for consistency.The operation
createCheckoutSessionis marked with@extension(Shared.InternalExtension, true)and@extension(Shared.UnstableExtension, true). Since the request model is only used by this operation, tagging it the same way ensures consistency across generated docs and client surfaces.Suggested tweak
+@extension(Shared.InternalExtension, true) +@extension(Shared.UnstableExtension, true) `@friendlyName`("BillingCustomerStripeCreateCheckoutSessionRequest") model CustomerBillingStripeCreateCheckoutSessionRequest {
f47c252 to
6408dd6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 140-144: The description string inside the example object with
title "With existing OpenMeter and Stripe customer" is garbled and missing
"Stripe"; update the description field (the object that currently reads
description: "Create a checkout session with existing openmeter and customer.")
to a clear sentence like "Create a checkout session with existing OpenMeter and
Stripe customer." so it matches the title and fixes the typo.
♻️ Duplicate comments (2)
api/spec/src/v3/billing/profile.tsp (1)
198-225: Align tax app optionality with tax settings.This was flagged in a previous review and still seems worth addressing:
BillingWorkflowTaxSettingsis optional with defaults forenabled/enforced, but the tax app inBillingProfileAppReferencesis required. If tax can truly be disabled, making the tax app optional would better reflect that. If a tax app is always required, consider documenting that distinction more clearly.api/spec/src/v3/apps/stripe.tsp (1)
295-300: Fix the currency code documentation casing mismatch.The doc says "lowercase" but
Shared.CurrencyCodeis defined with uppercase pattern^[A-Z]{3}$and example "USD". Should update the doc to match:/** - * Three-letter ISO 4217 currency code in lowercase. + * Three-letter ISO 4217 currency code (e.g., "USD"). * * Required for payment mode sessions. Optional for setup mode sessions. */ currency?: Shared.CurrencyCode;
🧹 Nitpick comments (4)
api/spec/src/v3/billing/profile.tsp (4)
44-48: Consider adding visibility to theidfield.Other reference-style models in the codebase typically specify visibility. Without it, the default behavior may not match your intent for how this reference is used across different lifecycle operations.
💡 Possible fix
model BillingProfileReference { /** * The ID of the billing profile. */ + `@visibility`(Lifecycle.Read, Lifecycle.Create) id: Shared.ULID; }
86-102: Inconsistent visibility and type usage in collection settings.A couple of things to consider here:
The
alignmentfield (line 90) is missing@visibility, whileinterval(line 100) has it. This inconsistency might cause unexpected behavior in generated code.The
intervalfield usesstringwith ISO8601 encoding, but there's already aShared.ISO8601Durationscalar with proper pattern validation. Using it would give you consistent validation.♻️ Suggested fix
model BillingWorkflowCollectionSettings { /** * The alignment for collecting the pending line items into an invoice. */ + `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) alignment?: BillingWorkflowCollectionAlignment = DefaultBillingWorkflowCollectionAlignment; /** * This grace period can be used to delay the collection of the pending line items specified in * alignment. * * This is useful, in case of multiple subscriptions having slightly different billing periods. */ - `@encode`(DurationKnownEncoding.ISO8601) `@example`("P1D") `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) - interval?: string = "PT1H"; + interval?: Shared.ISO8601Duration = "PT1H"; }
171-196: Consider usingShared.ISO8601Durationfordraft_period.Same as the
intervalfield above — using the shared scalar gives you pattern validation for free and keeps the API consistent.♻️ Suggested fix
/** * The period for the invoice to be kept in draft status for manual reviews. */ - `@encode`(DurationKnownEncoding.ISO8601) `@example`("P1D") `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) - draft_period?: string = "P0D"; + draft_period?: Shared.ISO8601Duration = "P0D";
288-332: Consider usingShared.ISO8601Durationfordue_after.For consistency with other duration fields and to get automatic pattern validation.
♻️ Suggested fix
/** * The period after which the invoice is due. * With some payment solutions it's only applicable for manual collection method. */ - `@encode`(DurationKnownEncoding.ISO8601) `@example`("P30D") `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) - due_after?: string = "P30D"; + due_after?: Shared.ISO8601Duration = "P30D"; }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/billing/profile.tsp`:
- Around line 86-102: The alignment field in model
BillingWorkflowCollectionSettings lacks the visibility decorator present on
interval; update the alignment declaration (alignment?:
BillingWorkflowCollectionAlignment = DefaultBillingWorkflowCollectionAlignment)
to include `@visibility`(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) above
it so it matches the interval field's visibility annotations and ensures
consistent code generation.
- Around line 304-332: The doc comments for the models
BillingWorkflowPaymentChargeAutomaticallySettings and
BillingWorkflowPaymentSendInvoiceSettings incorrectly call the field
collection_method a "payment method"; update both comments to describe
collection_method as the collection mechanism (e.g., "The collection method for
the invoice" or "How the invoice is collected, e.g., automatic charge or send
invoice") so the documentation accurately reflects the meaning of the
collection_method field.
♻️ Duplicate comments (2)
api/spec/src/v3/billing/profile.tsp (1)
198-225: Tax settings optionality note from prior review still applies.As flagged previously, the tax workflow settings can be entirely optional (disabled), while the tax app reference in
BillingProfileAppReferencesis required. If this is intentional (apps = infrastructure, settings = behavior), it might be worth adding a doc comment to clarify. Otherwise, consider aligning the optionality.api/spec/src/v3/apps/stripe.tsp (1)
294-300: Fix the currency code documentation casing mismatch.The doc says "lowercase" but
Shared.CurrencyCodeuses the pattern^[A-Z]{3}$with an example of "USD" — that's uppercase. The examples in this very file also use"USD". Just needs a quick doc fix:Suggested fix
/** - * Three-letter ISO 4217 currency code in lowercase. + * Three-letter ISO 4217 currency code in uppercase (e.g., "USD"). * * Required for payment mode sessions. Optional for setup mode sessions. */
🧹 Nitpick comments (1)
api/spec/src/v3/apps/stripe.tsp (1)
134-144: Consider clarifying the example title and description.This example uses both an existing OpenMeter customer (
customer: #{ id: ... }) and an existing Stripe customer (stripe_customer_id). The title "With existing customer" is a bit ambiguous about which customer. Something like "With existing OpenMeter and Stripe customer" would make the distinction clearer for API consumers.Suggested improvement
#{ - title: "With existing customer", - description: "Create a checkout session with existing customer.", + title: "With existing OpenMeter and Stripe customer", + description: "Create a checkout session linking an existing OpenMeter customer to an existing Stripe customer.", }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 541-548: The documentation for model
CreateCheckoutSessionTaxIdCollection currently states "Defaults to false" for
the required field enabled, which conflicts with the field being required;
update the schema so it’s consistent: either make
CreateCheckoutSessionTaxIdCollection.enabled optional and document its default
(e.g., set enabled?: boolean with "Defaults to false"), or keep it required and
remove the "Defaults to false" line from the comment for
tax_id_collection.enabled so no default is implied; locate the
CreateCheckoutSessionTaxIdCollection model and adjust the enabled property and
its JSDoc accordingly.
🧹 Nitpick comments (2)
api/spec/src/v3/apps/stripe.tsp (2)
271-276: Consider annotating URL fields with a URI format.This helps OpenAPI consumers validate and generate nicer types for these URLs. As per coding guidelines.
💡 Suggested tweak
/** * URL to redirect customers who cancel the checkout session. * * Not allowed when ui_mode is "embedded". */ + `@format`("uri") cancel_url?: string; @@ /** * Return URL for embedded checkout sessions after payment authentication. * * Required if ui_mode is "embedded" and redirect-based payment methods are enabled. */ + `@format`("uri") return_url?: string; @@ /** * Success URL to redirect customers after completing payment or setup. * * Not allowed when ui_mode is "embedded". * See: https://docs.stripe.com/payments/checkout/custom-success-page */ + `@format`("uri") success_url?: string;Also applies to: 328-341
196-202: Thecustomerunion needs disambiguation for code generators.The
StripeCustomerCreatemodel includes bothidandkeyfields (inherited fromCustomers.Customer), which creates overlap withStripeCustomerIdandStripeCustomerKey. AoneOfdiscriminator can't reliably distinguish between them—an object with just{id}or just{key}could match multiple union members. Consider adding a discriminator field (likekind: "id" | "key" | "create") or restructuring the shapes to make them explicitly mutually exclusive.
de8b7c7 to
79db515
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@api/spec/lib/rules/casing.js`:
- Around line 110-133: The enum handler in casingAIPErrorsRule currently treats
empty strings as valid because isSnakeCase('') returns true; update the guard in
the enum callback (the loop over model.members in the enum handler) to
explicitly reject empty-string values before calling isSnakeCase — e.g., check
that member.value is a non-empty string (e.g., typeof member.value === 'string'
&& member.value.length > 0) and report a diagnostic (using the same
context.reportDiagnostic flow) when the value is an empty string so empty enum
values are flagged as invalid.
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 321-327: The metadata property currently uses a loose type
`Record<string>` which allows any value types; update the type declaration for
the `metadata` field to `Record<string, string>` (i.e., change `metadata?:
Record<string>;` to use string values) so the schema for Stripe Checkout
metadata enforces string-to-string pairs; also scan references to `metadata`
(e.g., in any builders/parsers or tests) and adjust or add conversions if
callers currently pass non-string values.
In `@api/spec/src/v3/shared/consts.tsp`:
- Around line 20-25: Update the BillingDescription string constant to remove the
awkward comma after "profiles" and make the sentence flow: edit the value of
BillingDescription (the constant next to BillingTag) to something like "Billing
manages billing profiles and invoices for customers." or an equivalent phrasing
without the comma.
♻️ Duplicate comments (2)
api/spec/lib/rules/friendly-name.js (1)
84-113: Guardnode.namebeforeendsWithto avoid a possible throw.
If TypeSpec ever allows unnamed interfaces,node.name.endsWith(...)would throw. A small guard keeps this safe and matches the other handlers.Can you confirm from the TypeSpec AST/docs whether `interface` nodes are always named? If they are, we can ignore this.🛠️ Suggested guard
interface: (node) => { + if (!node.name) { + return + } const hasFriendlyName = node.decorators.some( (d) => d.decorator.name === '$friendlyName', ) const isEndpointsOrOperations = node.name.endsWith('Endpoints') || node.name.endsWith('Operations')api/spec/src/v3/billing/operations.tsp (1)
29-43: Doc clarification needed for app association.The comment says "linked to a specific app" but a BillingProfile can reference multiple apps (tax, invoicing, payment). Consider pluralizing to avoid confusion.
🧹 Nitpick comments (6)
api/spec/lib/rules/utils.js (1)
1-4: Escape exception literals and cache the regex.If the exception list ever includes regex metacharacters (e.g.,
C++,C#,X.Y), the current pattern will misbehave. Also, rebuilding the RegExp on every call is avoidable.💡 Suggested tweak
/** * Exceptions for PascalCase naming convention. */ const pascalCaseExceptions = ['OAuth2', 'URL', 'API', 'UI', 'ID'] +const escapeRegex = (s) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') +const pascalCaseExceptionsPattern = pascalCaseExceptions + .map(escapeRegex) + .join('|') +const pascalCaseNoAcronymsRegex = new RegExp( + `^(?:[A-Z][a-z0-9]+|${pascalCaseExceptionsPattern})+[A-Z]?$|^[A-Z]+$`, +) export function isPascalCaseNoAcronyms(value) { if (value === undefined || value === null || value === '') { return true } - return new RegExp( - `^(?:[A-Z][a-z0-9]+|${pascalCaseExceptions.join('|')})+[A-Z]?$|^[A-Z]+$`, - ).test(value) + return pascalCaseNoAcronymsRegex.test(value) }Also applies to: 16-18
api/spec/src/v3/apps/customer.tsp (1)
29-42: Are Stripe IDs meant to be writable?
Ifcustomer_id/default_payment_method_idare provider-managed, consider marking them read-only to prevent clients from setting or changing external mappings.🔒 Optional visibility tightening
model AppCustomerDataStripe { /** * The Stripe customer ID used. */ `@summary`("Stripe customer ID") `@example`("cus_1234567890") + `@visibility`(Lifecycle.Read) customer_id?: string; /** * The Stripe default payment method ID. */ `@summary`("Stripe default payment method ID") `@example`("pm_1234567890") + `@visibility`(Lifecycle.Read) default_payment_method_id?: string; }api/spec/src/v3/apps/custominvoicing.tsp (1)
238-300: Consider adding a "Refunded" trigger.The
CustomInvoicingPaymentTriggerenum covers most payment scenarios nicely. One common case that might be missing is handling refunds. If an invoice is partially or fully refunded, how would the external system communicate that?If refunds are meant to be handled through a separate mechanism or aren't part of this app's scope, it might be worth adding a brief comment clarifying that.
api/spec/src/v3/apps/catalog.tsp (1)
11-26: Consider removing or documenting the commented-out OAuth2 code.The commented-out
WithOAuth2member (lines 12-15) is a bit of a loose end. If OAuth2 support is planned for the future, adding a TODO comment or linking to a tracking issue would help. Otherwise, it might be cleaner to remove it and add it when it's actually needed.Not a big deal, just a housekeeping thing!
api/spec/src/v3/apps/app.tsp (1)
90-99: Consider ifAppReferenceneeds atypefield.The
AppReferencemodel only contains anid. In some API patterns, references also include thetypeto help consumers quickly understand what kind of app they're dealing with without needing a separate fetch.If you always expect consumers to resolve the full app object, this is fine as-is. Just flagging it as something to think about for usability.
api/spec/src/v3/billing/operations.tsp (1)
68-69: Minor: Missing blank line before delete operation.There's no blank line between the
updateclosing brace and thedeletedoc comment, unlike the spacing between other operations. Super minor, but worth fixing for consistency.✏️ Suggested fix
): Shared.UpdateResponse<BillingProfile> | Common.NotFound | Common.ErrorResponses; + /** * Delete a billing profile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@api/spec/src/v3/apps/customer.tsp`:
- Around line 1-62: The file references Common.Labels (used by
AppCustomerDataStripe.labels and AppCustomerDataExternalInvoicing.labels) but
lacks the Common namespace import; add an import for the common properties
module (import "../common/properties.tsp";) at the top of the file so
Common.Labels is available for the models AppCustomerDataStripe and
AppCustomerDataExternalInvoicing.
In `@api/spec/src/v3/apps/stripe.tsp`:
- Around line 615-622: The file has an unclosed doc comment "Status." and is
missing the corresponding status field which breaks the following docblock for
locale; close the doc comment and add a properly documented status property
(matching the pattern used for other models) immediately before the existing
locale field — include a short doc comment describing the status (e.g., current
state of the portal session) and declare the property (status: string;) so the
locale JSDoc is not swallowed by the previous comment and the model matches
Stripe's spec.
In `@api/spec/src/v3/customers/operations.tsp`:
- Around line 95-165: The TypeSpec defines createPortalSession (operationId
"create-customer-stripe-portal-session") but no corresponding server
handler/route was added; add a handler and route registration in the same style
as the existing get, upsert, upsertAppData and createCheckoutSession handlers
(implement a createPortalSession handler function that accepts customerId and
Apps.CreateStripeCustomerPortalSessionRequest and returns
Shared.CreateResponse<Apps.CreateStripeCustomerPortalSessionResult> |
Common.Conflict | Common.NotFound | Common.ErrorResponses, and wire it into
api/v3/server/routes.go with the POST "/stripe/portal-sessions" subroute), or
alternatively remove createPortalSession from the TypeSpec to keep parity.
♻️ Duplicate comments (1)
api/spec/src/v3/apps/stripe.tsp (1)
104-110: Tighten Stripemetadatato string→string pairs.
Stripe metadata values are strings;Record<string>is too loose. Suggest tightening both request options and result payload toRecord<string, string>.🔧 Suggested diff
- metadata?: Record<string>; + metadata?: Record<string, string>;- metadata?: Record<string>; + metadata?: Record<string, string>;Also applies to: 488-492
api/spec/src/v3/apps/stripe.tsp
Outdated
| /** | ||
| * Status. | ||
| /** | ||
| * The IETF language tag of the locale customer portal is displayed in. | ||
| * | ||
| * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale | ||
| */ | ||
| locale: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file at the specified lines with context
cat -n api/spec/src/v3/apps/stripe.tsp | sed -n '605,635p'Repository: openmeterio/openmeter
Length of output: 791
🏁 Script executed:
# Get more context around this model to see its definition and other fields
cat -n api/spec/src/v3/apps/stripe.tsp | sed -n '590,640p'Repository: openmeterio/openmeter
Length of output: 1320
🏁 Script executed:
# Let's check the model name and full definition
cat -n api/spec/src/v3/apps/stripe.tsp | sed -n '570,630p'Repository: openmeterio/openmeter
Length of output: 1996
🏁 Script executed:
# Let's verify if the status field appears elsewhere in the file or in similar models
rg -n "status.*string" api/spec/src/v3/apps/stripe.tsp | head -20Repository: openmeterio/openmeter
Length of output: 87
🏁 Script executed:
# Let's see the context around line 498 to understand how status is defined
cat -n api/spec/src/v3/apps/stripe.tsp | sed -n '490,510p'Repository: openmeterio/openmeter
Length of output: 684
🏁 Script executed:
# Let's check if there are other models that have status field to understand the pattern
rg -B5 "status\?" api/spec/src/v3/apps/stripe.tsp | head -40Repository: openmeterio/openmeter
Length of output: 235
Add the missing status field to fix the broken doc comment.
The /** Status. comment on line 615 is unclosed and breaks the next doc block. The field itself is missing. Based on the Stripe API structure and the pattern used in other models in this file, add the status field with proper documentation:
/**
* Return URL.
*
* See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-return_url
*/
return_url: string;
/**
* Status.
+ *
+ * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-status
*/
+ status?: string;
+
+ /**
* The IETF language tag of the locale customer portal is displayed in.
*
* See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale
*/
locale: string;📝 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.
| /** | |
| * Status. | |
| /** | |
| * The IETF language tag of the locale customer portal is displayed in. | |
| * | |
| * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale | |
| */ | |
| locale: string; | |
| /** | |
| * Status. | |
| * | |
| * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-status | |
| */ | |
| status?: string; | |
| /** | |
| * The IETF language tag of the locale customer portal is displayed in. | |
| * | |
| * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale | |
| */ | |
| locale: string; |
🤖 Prompt for AI Agents
In `@api/spec/src/v3/apps/stripe.tsp` around lines 615 - 622, The file has an
unclosed doc comment "Status." and is missing the corresponding status field
which breaks the following docblock for locale; close the doc comment and add a
properly documented status property (matching the pattern used for other models)
immediately before the existing locale field — include a short doc comment
describing the status (e.g., current state of the portal session) and declare
the property (status: string;) so the locale JSDoc is not swallowed by the
previous comment and the model matches Stripe's spec.
6e024a0 to
d1e4237
Compare
API spec only, unimplemented endpoints
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.