feat(api): add app endpoints to v3#3767
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR adds a comprehensive Apps/Billing integration layer to the API, introducing app catalog management, Stripe checkout integration, custom invoicing synchronization, and OAuth2 flows. It includes updated linting rules for enum naming conventions, extensive TypeSpec definitions, generated Go API types, HTTP handlers, and server routing integration. All handler implementations are currently scaffolded with NotImplemented errors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/spec/src/v3/subscriptions/operations.tsp (1)
30-31: SubscriptionChange is omitting properties that don't exist on SubscriptionCreate.The
OmitProperties<SubscriptionCreate, "customer_id" | "customer_key">won't work as intended because SubscriptionCreate doesn't have top-levelcustomer_idorcustomer_keyproperties—it has acustomerobject with nestedidandkeyfields instead. If the goal is to prevent changes to the customer assignment in a subscription update, you'll probably want to omit"customer"directly.
🤖 Fix all issues with AI agents
In @api/spec/src/v3/apps/capability.tsp:
- Around line 42-47: The description string for the AppCapability example (entry
with AppCapabilityType.CollectPayments, key "stripe_collect_payment", name
"Collect Payments") has a grammar nit; update the description to "Stripe
Payments collects outstanding revenue using the customer’s default payment
method." (or "Stripe Payments collects outstanding revenue with the customer's
default payment method.") so the product name is capitalized and the phrase
about the default payment method reads clearly.
In @api/spec/src/v3/apps/catalog.tsp:
- Around line 1-4: catalog.tsp currently imports app.tsp creating a circular
import with app.tsp; extract the shared types (e.g., AppType and AppStatus) into
a new module (for example shared/app-types.tsp) and update both catalog.tsp and
app.tsp to import those types instead of importing each other, ensuring no
runtime circular dependency remains.
- Around line 33-61: In the example block for AppType.Stripe update the
description string on the AppType.Stripe example from "Stripe interation allows
you to collect payments with Stripe." to "Stripe integration allows you to
collect payments with Stripe." and make capability keys consistent (use a single
form such as "stripe_collect_payments" for the CollectPayments capability) by
updating the key value in the corresponding capability object; locate these in
the example for AppType.Stripe where capabilities are defined and adjust the
description and the CollectPayments capability.key to the chosen consistent
name.
In @api/spec/src/v3/apps/operations.tsp:
- Around line 35-42: The getOAuth2InstallURL operation currently declares its
return as Shared.GetResponse<AppCatalogItem> but should return the OAuth2
install URL shape; update the return type to
Shared.GetResponse<AppOAuth2InstallResponse> (preserving the existing
Common.NotFound | Common.ErrorResponses union) so the endpoint returns the
AppOAuth2InstallResponse model defined in oauth.tsp; locate and modify the
method declaration for getOAuth2InstallURL to use AppOAuth2InstallResponse in
the Shared.GetResponse wrapper.
In @api/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.go:
- Around line 14-16: The response alias is wrong: replace
GetAppCatalogItemOauth2InstallUrlResponse = api.BillingAppCatalogItem with a new
simple response type that returns only the install URL (e.g., define
api.BillingAppOauth2InstallUrlResponse struct with a single string field like
InstallUrl or Url) and update the handler typedef
GetAppCatalogItemOauth2InstallUrlHandler to use that new response type; add the
new type into the api package and ensure any code that constructs or returns the
response uses the InstallUrl field.
In @api/v3/handlers/apps/handle_stripe_webhook.go:
- Around line 14-46: Update HandleStripeWebhookRequest to include RawPayload
[]byte and StripeSignature string and populate them in the request factory
before parsing the JSON body: read the raw request body bytes (io.ReadAll) into
RawPayload, capture the Stripe-Signature header via
r.Header.Get("Stripe-Signature"), then parse the JSON into Body from those raw
bytes (e.g., json.Unmarshal or io.NopCloser + bytes.NewReader) so signature
verification can later use RawPayload and StripeSignature while preserving the
existing Body field (api.BillingAppStripeWebhookEvent) and existing handler
names (HandleStripeWebhookRequest, HandleStripeWebhook).
🧹 Nitpick comments (12)
api/spec/lib/rules/casing.js (1)
110-133: Good separation of concerns!Splitting the value check into its own rule with
errorseverity (vswarningfor names) is a nice touch - it clearly signals that incorrect enum values are a harder failure than naming style issues.One small thing: the description on line 113 is identical to
casingAIPRule. Consider making it more specific, e.g., "Ensure enum values use snake_case for AIP naming conventions." to help differentiate them in lint output.📝 Suggested description tweak
export const casingAIPErrorsRule = createRule({ name: 'casing-aip-errors', severity: 'error', - description: 'Ensure proper casing style for AIP naming conventions.', + description: 'Ensure enum values use snake_case for AIP naming conventions.', messages: {api/spec/lib/rules/friendly-name.js (2)
84-112: Nice AIP-specific interface handling!The logic for avoiding
friendlyNameon*Endpointsand*Operationsinterfaces while requiring it on others is clear and well-structured.One tiny edge case to consider: unlike the
model,enum, andunionhandlers which checknode.name &&, the interface handler accessesnode.namedirectly on line 89. Ifnode.namecould ever beundefined, this would throw. TypeSpec interfaces should always have names in practice, but adding a guard would make it bulletproof.🛡️ Optional defensive check
interface: (node) => { + if (!node.name) return + const hasFriendlyName = node.decorators.some( (d) => d.decorator.name === '$friendlyName', )
114-158: Heads up: duplicated handlers between rulesThe
model,enum, andunionhandlers infriendlyNameAIPRuleare identical to those infriendlyNameRule. Not urgent, but if you ever need to update the logic, you'd have to change it in two places.Could extract these into shared helper functions down the road if this file grows.
api/v3/handlers/apps/handler.go (1)
25-33: Consider validatingresolveNamespaceisn't nil.If a caller accidentally passes
nilforresolveNamespace, you'll get a nil pointer panic later when it's invoked. A quick nil check in the constructor could save some debugging headaches:💡 Optional defensive check
func New( resolveNamespace func(ctx context.Context) (string, error), options ...httptransport.HandlerOption, ) Handler { + if resolveNamespace == nil { + panic("resolveNamespace cannot be nil") + } return &handler{ resolveNamespace: resolveNamespace, options: options, } }api/v3/handlers/apps/submit_custom_invoicing_issuing_synchronized.go (1)
24-46: Tiny readability win: avoid naming the op argrequest.
In Line 37 the parameter namerequestis a bit confusing next to the importedrequestpackage used above; I’d rename it toreq.Proposed tweak
- func(ctx context.Context, request SubmitCustomInvoicingIssuingSynchronizedRequest) (SubmitCustomInvoicingIssuingSynchronizedResponse, error) { + func(ctx context.Context, req SubmitCustomInvoicingIssuingSynchronizedRequest) (SubmitCustomInvoicingIssuingSynchronizedResponse, error) { return nil, apierrors.NewNotImplementedError(ctx, nil) },api/spec/src/v3/apps/app.tsp (2)
45-69: Consider whetherAppBase.definitionshould embed the wholeAppCatalogItem.
If installed-app reads are frequent, embedding the full catalog item may be heavier than needed; an ID/key reference (or a slim “catalog item ref”) can reduce coupling. If you want the embed, this is totally fine—just calling out the tradeoff early.
1-3: Consider consolidating these tightly coupled modules to avoid circular import.Both
app.tspandcatalog.tspimport each other—TypeSpec can handle this (it supports circular references via its emitter), but it's worth restructuring for cleaner code organization. You've got a few options:
- Merge them into a single file if they're tightly coupled
- Extract shared types (e.g.,
AppType,AppStatus) into a separate shared module that both files import- Keep them separate but eliminate the back-reference by restructuring how
AppBaseandAppCatalogItemrelateThis will make the dependency graph clearer and easier to maintain long-term.
api/spec/src/v3/apps/capability.tsp (1)
48-68: MakeAppCapabilityexplicitly read-only if it’s catalog-only.
Given it’s used underAppCatalogItem.capabilities(which is read-only), I’d also mark the fields ofAppCapabilitywith@visibility(Lifecycle.Read)for consistency and to avoid accidental use in write models later.Proposed tweak
model AppCapability { /** * Type of the capability. */ + @visibility(Lifecycle.Read) type: AppCapabilityType; /** * Key of the capability. */ + @visibility(Lifecycle.Read) key: Shared.ResourceKey; /** * Name of the capability. */ + @visibility(Lifecycle.Read) name: string; /** * Description of the capability. */ + @visibility(Lifecycle.Read) description: string; }api/v3/handlers/apps/update_custom_invoicing_payment_status.go (1)
37-39: Optional: rename op parameterrequest→reqfor clarity.
Keeps things easier to scan alongside the importedrequestpackage used above.api/spec/src/v3/konnect.tsp (1)
69-81: Route consistency nit: consider aligning app-catalog under/openmeter/apps/*.
/openmeter/app-catalogis the odd one out vs/openmeter/apps/custom-invoicingand/openmeter/apps/stripe. If that’s intentional (e.g., “catalog is shared, not an app instance”), all good—otherwise I’d rename to something like/openmeter/apps/catalogfor consistency.api/v3/handlers/apps/create_stripe_checkout_session.go (1)
14-39: Handler scaffold is consistent; small readability tweak + please verify body-size limits.
- Readability: you can decode into
CreateStripeCheckoutSessionRequestdirectly (no need for the concreteapi.*name).- Also worth double-checking
request.ParseBodyhas a sane max body size (to avoid easy memory pressure attacks on public endpoints).Proposed tweak
- func(ctx context.Context, r *http.Request) (CreateStripeCheckoutSessionRequest, error) { - body := api.BillingAppStripeCreateCheckoutSessionRequest{} + func(ctx context.Context, r *http.Request) (CreateStripeCheckoutSessionRequest, error) { + var body CreateStripeCheckoutSessionRequest if err := request.ParseBody(r, &body); err != nil { return CreateStripeCheckoutSessionRequest{}, err } return body, nil },api/spec/src/v3/apps/stripe.tsp (1)
65-68: Consider int64 for Unix timestamp to avoid Y2038 issue.The
createdfield usesint32for the Unix timestamp. While this matches how Stripe currently sends timestamps, signed 32-bit integers will overflow after January 2038. This is a minor future-proofing concern - not urgent but worth considering.Suggested change
/** * Unix timestamp when the event was created. */ - created: int32; + created: int64;
📜 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 (33)
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/custominvoicing.tspapi/spec/src/v3/apps/index.tspapi/spec/src/v3/apps/oauth.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/apps/sandbox.tspapi/spec/src/v3/apps/stripe.tspapi/spec/src/v3/entitlements/access.tspapi/spec/src/v3/entitlements/entitlements.tspapi/spec/src/v3/invoices/invoice.tspapi/spec/src/v3/konnect.tspapi/spec/src/v3/openmeter.tspapi/spec/src/v3/shared/consts.tspapi/spec/src/v3/subscriptions/operations.tspapi/spec/src/v3/subscriptions/subscription.tspapi/v3/api.gen.goapi/v3/handlers/apps/create_stripe_checkout_session.goapi/v3/handlers/apps/get_app_catalog_item.goapi/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.goapi/v3/handlers/apps/handle_stripe_webhook.goapi/v3/handlers/apps/handler.goapi/v3/handlers/apps/list_app_catalog.goapi/v3/handlers/apps/submit_custom_invoicing_draft_synchronized.goapi/v3/handlers/apps/submit_custom_invoicing_issuing_synchronized.goapi/v3/handlers/apps/update_custom_invoicing_payment_status.goapi/v3/server/routes.goapi/v3/server/server.go
🧰 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/apps/sandbox.tspapi/spec/src/v3/invoices/invoice.tspapi/spec/src/v3/subscriptions/operations.tspapi/spec/src/v3/entitlements/access.tspapi/spec/src/v3/shared/consts.tspapi/spec/src/v3/openmeter.tspapi/spec/src/v3/subscriptions/subscription.tspapi/spec/src/v3/apps/app.tspapi/spec/src/v3/apps/index.tspapi/spec/src/v3/apps/catalog.tspapi/spec/src/v3/entitlements/entitlements.tspapi/spec/src/v3/konnect.tspapi/spec/src/v3/apps/oauth.tspapi/spec/src/v3/apps/operations.tspapi/spec/src/v3/apps/capability.tspapi/spec/src/v3/apps/custominvoicing.tspapi/spec/src/v3/apps/stripe.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/handlers/apps/submit_custom_invoicing_issuing_synchronized.goapi/v3/handlers/apps/handler.goapi/v3/handlers/apps/handle_stripe_webhook.goapi/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.goapi/v3/handlers/apps/create_stripe_checkout_session.goapi/v3/handlers/apps/list_app_catalog.goapi/v3/server/server.goapi/v3/handlers/apps/submit_custom_invoicing_draft_synchronized.goapi/v3/handlers/apps/get_app_catalog_item.goapi/v3/handlers/apps/update_custom_invoicing_payment_status.goapi/v3/server/routes.goapi/v3/api.gen.go
🧠 Learnings (3)
📚 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
📚 Learning: 2025-11-07T12:29:09.087Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 3581
File: openmeter/notification/eventhandler/webhook.go:391-396
Timestamp: 2025-11-07T12:29:09.087Z
Learning: In the OpenMeter notification system (specifically in openmeter/notification/eventhandler/webhook.go), webhook channels are equivalent to notification rules. When sending webhook messages, the rule ID (event.Rule.ID) should be used as the channel identifier in the Channels field of SendMessageInput.
Applied to files:
api/v3/handlers/apps/handle_stripe_webhook.go
📚 Learning: 2025-11-07T11:28:40.430Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 3581
File: openmeter/notification/webhook/svix/webhook.go:253-257
Timestamp: 2025-11-07T11:28:40.430Z
Learning: In the openmeter notification system (specifically in openmeter/notification/webhook/svix/webhook.go), event type filtering for webhooks is not allowed or supported. Only channel-based filtering is used.
Applied to files:
api/v3/handlers/apps/handle_stripe_webhook.go
🧬 Code graph analysis (7)
api/v3/handlers/apps/handler.go (10)
api/v3/handlers/apps/list_app_catalog.go (1)
ListAppCatalogItemsHandler(16-16)api/v3/handlers/apps/get_app_catalog_item.go (1)
GetAppCatalogItemHandler(16-16)api/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.go (1)
GetAppCatalogItemOauth2InstallUrlHandler(16-16)api/v3/handlers/apps/submit_custom_invoicing_draft_synchronized.go (1)
SubmitCustomInvoicingDraftSynchronizedHandler(21-21)api/v3/handlers/apps/submit_custom_invoicing_issuing_synchronized.go (1)
SubmitCustomInvoicingIssuingSynchronizedHandler(21-21)api/v3/handlers/apps/update_custom_invoicing_payment_status.go (1)
UpdateCustomInvoicingPaymentStatusHandler(21-21)api/v3/handlers/apps/create_stripe_checkout_session.go (1)
CreateStripeCheckoutSessionHandler(17-17)api/v3/handlers/apps/handle_stripe_webhook.go (1)
HandleStripeWebhookHandler(21-21)pkg/framework/transport/httptransport/options.go (1)
HandlerOption(19-21)api/v3/api.gen.go (1)
Handler(2879-2881)
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/handlers/apps/list_app_catalog.go (5)
api/v3/api.gen.go (2)
ListAppCatalogItemsParams(1578-1581)AppCatalogItemPagePaginatedResponse(203-208)pkg/framework/transport/httptransport/argshandler.go (2)
HandlerWithArgs(11-14)NewHandlerWithArgs(18-29)api/v3/apierrors/errors_ctors.go (1)
NewNotImplementedError(280-290)pkg/framework/commonhttp/encoder.go (1)
JSONResponseEncoder(40-42)pkg/framework/transport/httptransport/options.go (1)
AppendOptions(81-83)
api/v3/server/server.go (2)
api/v3/handlers/apps/handler.go (2)
Handler(9-18)New(25-33)pkg/framework/transport/httptransport/options.go (1)
WithErrorHandler(29-33)
api/v3/handlers/apps/get_app_catalog_item.go (5)
api/v3/api.gen.go (2)
BillingAppType(882-882)BillingAppCatalogItem(311-326)pkg/framework/transport/httptransport/argshandler.go (2)
HandlerWithArgs(11-14)NewHandlerWithArgs(18-29)api/v3/apierrors/errors_ctors.go (1)
NewNotImplementedError(280-290)pkg/framework/commonhttp/encoder.go (1)
JSONResponseEncoder(40-42)pkg/framework/transport/httptransport/options.go (3)
AppendOptions(81-83)WithOperationName(41-47)WithErrorEncoder(35-39)
api/v3/server/routes.go (3)
api/v3/server/server.go (1)
Server(98-110)openmeter/server/server.go (1)
Server(27-29)api/v3/api.gen.go (3)
ListAppCatalogItemsParams(1578-1581)BillingAppType(882-882)ULID(1507-1507)
api/v3/api.gen.go (1)
api/v3/handlers/apps/handler.go (1)
Handler(9-18)
⏰ 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: Lint
- GitHub Check: Artifacts / Container image
- GitHub Check: Build
- GitHub Check: Migration Checks
- GitHub Check: Code Generators
- GitHub Check: Test
- GitHub Check: Repository Scan
- GitHub Check: Analyze (go)
🔇 Additional comments (34)
api/spec/lib/rules/utils.js (2)
1-4: Nice approach with the exceptions list!Defining
pascalCaseExceptionsas a separate constant makes it easy to extend later if more acronym exceptions are needed (e.g.,HTTP,JSON, etc.). Clean and maintainable.
16-18: LGTM!The dynamic regex construction is elegant. It correctly handles combinations like
OAuth2URLorAPIClientwhile still enforcing PascalCase for non-exception words.api/spec/lib/index.js (2)
2-8: LGTM!Imports are well-organized and align with the new rule exports.
45-56: Nice ruleset organization!Good call keeping
friendlyNameRulein the legacy ruleset while usingfriendlyNameAIPRulein the aip ruleset. This preserves backward compatibility for existing specs while enabling the new AIP conventions for new work.api/spec/lib/rules/casing.js (1)
87-105: LGTM!Clean implementation - checking both enum names and member names for PascalCase makes sense for AIP conventions.
api/spec/src/v3/subscriptions/subscription.tsp (3)
12-17: Nice enum pattern here!Using PascalCase member names (
Active,Inactive, etc.) with explicit lowercase string values ("active","inactive") is a solid approach. It keeps the wire format stable while following TypeScript naming conventions for the generated code. This also means existing API consumers won't see any breaking changes in the serialized values. 👍
24-28: Consistent with the pattern above – looks good!Same approach applied here with
SubscriptionEditTimingEnum. The snake_case string values ("immediate","next_billing_cycle") match what API consumers will see on the wire.
36-36: Example annotation updated correctly.The example now references
SubscriptionEditTimingEnum.Immediateto match the new member name.api/spec/src/v3/subscriptions/operations.tsp (1)
19-24: Default value update looks correct.The default now uses
SubscriptionEditTimingEnum.Immediateto match the updated enum member name. Behavior stays the same – cancel immediately when no timing is specified.api/spec/src/v3/entitlements/entitlements.tsp (1)
7-11: Nice enum refactor!The switch to PascalCase members with explicit lowercase string mappings is a solid pattern. It keeps the serialized API output stable (
"metered","static","boolean") while making the TypeSpec code more idiomatic. Good stuff.api/spec/src/v3/entitlements/access.tsp (1)
14-16: Example updated correctly.The
@example(EntitlementType.Static)change properly aligns with the renamed enum member. Everything looks consistent.api/v3/handlers/apps/handler.go (1)
9-18: Clean interface design.The Handler interface is well-organized, grouping all app-related operations logically. The method names are self-documenting, and the return types align with the handler definitions in the other files.
api/spec/src/v3/apps/index.tsp (1)
1-1: Simple index file, looks good.This pulls in the operations module to expose the Apps API surface. Clean and straightforward.
api/spec/src/v3/apps/sandbox.tsp (1)
1-11: Clean sandbox app definition.Nice use of the spread operator with the generic
AppBase<AppType.Sandbox>pattern. The documentation clearly explains the sandbox's purpose for testing billing features. Consistent with the other app type definitions.api/v3/server/server.go (1)
16-16: LGTM! Apps handler integration looks good.The wiring follows the existing patterns nicely. One thing to keep in mind: unlike the other handlers (
eventsHandler,customersHandler, etc.) which receive their respective services,appsHandlercurrently only getsresolveNamespace. When the actual implementation lands, you'll likely need to add apps-related services to theConfigstruct and wire them through here.Also applies to: 104-104, 143-143, 153-153
api/spec/src/v3/shared/consts.tsp (1)
20-22: Nice addition!The new
AppsTagandAppsDescriptionconstants follow the established naming conventions and the description clearly explains what Apps are for. Good consistency with the other tag constants in this file.api/spec/src/v3/invoices/invoice.tsp (1)
1-17: Looks solid!The
InvoiceNumberscalar is well-documented with clear explanations of the uniqueness considerations. The constraints (@minLength(1),@maxLength(256)) are reasonable.I noticed there's no
@patternconstraint for format validation — I'm guessing that's intentional since the format can vary depending on the app settings (as the docs mention). Just wanted to confirm that's the expected design here.api/spec/src/v3/openmeter.tsp (2)
65-76: Routes look good, just noting the structure difference.The new app-related endpoints are well-organized. I noticed the routing structure has:
/openmeter/app-catalog— at the top level/openmeter/apps/custom-invoicingand/openmeter/apps/stripe— nested under/apps/This makes sense semantically (catalog for discovery vs. specific app integrations), just wanted to confirm that's the intended design.
10-10: LGTM!The apps import and tag metadata are wired up correctly, following the same pattern as the other tags in this file.
Also applies to: 40-40
api/v3/handlers/apps/submit_custom_invoicing_issuing_synchronized.go (1)
14-22: Watch out for*struct{}response + returningnil(encoder expectations).
You alias the response to*struct{}and later returnnil; that’s fine ifcommonhttp.EmptyResponseEncodernever touchesresponse. Worth double-checking to avoid a future nil deref if the encoder changes.api/v3/handlers/apps/list_app_catalog.go (1)
13-34: Nice, consistent handler scaffolding.
Types/aliases + wiring match the establishedhttptransport.NewHandlerWithArgspattern and should be easy to fill in later.api/v3/handlers/apps/update_custom_invoicing_payment_status.go (1)
14-22: Same note as the other 204 handlers: confirm nil is safe withEmptyResponseEncoder.
ReturningnilforUpdateCustomInvoicingPaymentStatusResponse = *struct{}is fine as long as the encoder never inspects it.Also applies to: 24-41
api/v3/server/routes.go (2)
9-41: Routes wiring looks clean and consistent with the rest of the server.
The.With(...).ServeHTTP(...)passthrough pattern matches the other domains and should be straightforward to maintain.
11-41: Good catch on the defensive thinking, butappsHandleris solid. ✓The sanity check confirms that
appsHandleris always initialized—it's created viaappshandler.New()in theNewServer()constructor (server.go:143) before the Server struct is returned. Since there's only one constructor and no test-only alternatives that bypass it, there's no path to a nil panic here.api/spec/src/v3/apps/catalog.tsp (1)
87-92: Naming looks good! Theinstall_methodsfield is in line with v3's established snake_case convention. All the other fields across v3 TypeSpec follow the same pattern (likeevent_type,value_property,enable_draft_sync_hook, etc.), so this fits right in and won't stick out in generated SDKs or OpenAPI.api/spec/src/v3/konnect.tsp (2)
11-11: New Apps import looks good — please confirm codegen stays clean.Since this expands the public surface, it’d be great to run
make gen-apiand ensure TypeSpec/OpenAPI/Go generation stays stable. Based on learnings,make gen-apiis the expected workflow here.
35-35: Nice: Apps tag metadata is now first-class in Konnect.This should make the new endpoints discoverable and documented consistently alongside the other tags.
api/v3/handlers/apps/get_app_catalog_item.go (1)
13-33: Looks good; please confirmBillingAppTypevalidation happens somewhere.Right now the arg decoder is a passthrough, which is fine if routing/binding guarantees only allowed
BillingAppTypevalues reach this handler—just worth confirming so we don’t end up with “unknown app type” silently flowing into business logic later.api/spec/src/v3/apps/custominvoicing.tsp (1)
7-321: Verify the spec's invoice number generation and immutability claims against implementation.The spec makes explicit behavioral guarantees: auto-generated invoice numbers with
"INV-"prefix and immutability once invoices reach the issued state. The test suite validates against"INV-TECU-1"format when hooks are disabled, so the behavior is being tested. However, I couldn't locate the complete code path for the"INV-"prefix generation in the billing service's sequence definitions—onlyGATHERandDRAFTprefixes are defined.Before marking this complete, confirm:
- Where the
"INV-"prefix is actually generated (it may be state-specific or in a code path not yet located)- Whether the immutability guarantee at the issued state is enforced in the invoice state machine
- That all three behavioral claims (prefix generation, number replacement, immutability timing) are consistently implemented
If these are real guarantees, all good; if they're aspirational, consider softening the language in the spec docs.
api/v3/handlers/apps/submit_custom_invoicing_draft_synchronized.go (1)
1-47: Clean scaffolding for the draft synchronization endpoint.The handler follows the established pattern nicely - request/response type definitions, proper body parsing, and the NotImplemented placeholder. The 204 No Content response encoder is appropriate for this void-returning endpoint. Looks good to go!
api/spec/src/v3/apps/operations.tsp (1)
44-174: Well-documented custom invoicing and Stripe operations.The operation interfaces are nicely structured with clear documentation explaining the purpose of each endpoint. The inline comments describing when each callback should be invoked (draft sync, issuing sync, payment status updates) will be super helpful for integration developers. The Stripe operations with links to Stripe's documentation are a nice touch too.
api/spec/src/v3/apps/oauth.tsp (1)
1-162: Solid OAuth2 flow modeling per RFC 6749.This file does a great job capturing the OAuth2 authorization code grant flow. The suppress directives for casing on
error_descriptionanderror_uriare the right call since the OAuth2 spec mandates those exact names. The RFC references in the comments make it easy to cross-reference. The composite model approach for combining success and error params makes sense given how OAuth2 callbacks work.api/spec/src/v3/apps/stripe.tsp (1)
1-64: Comprehensive Stripe Checkout integration surface.Really thorough coverage of the Stripe Checkout Session API here. The customer union type (
StripeCustomerId | StripeCustomerKey | StripeCustomerCreate) is a nice ergonomic touch for API consumers. Good use of@secretfor the API key field, and the inline links to Stripe documentation will help developers understand the options.Also applies to: 69-684
api/v3/api.gen.go (1)
1-4: Generated file - no manual review needed.This file is auto-generated by oapi-codegen from the OpenAPI spec. Any issues here should be addressed in the source TypeSpec files rather than this generated output.
| @example(#{ | ||
| type: AppCapabilityType.CollectPayments, | ||
| key: "stripe_collect_payment", | ||
| name: "Collect Payments", | ||
| description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", | ||
| }) |
There was a problem hiding this comment.
Example text nit: “Stripe payments collects …”
Minor grammar tweak: “Stripe Payments collects …” or “Stripe Payments collects … using the customer’s default payment method.”
🤖 Prompt for AI Agents
In @api/spec/src/v3/apps/capability.tsp around lines 42 - 47, The description
string for the AppCapability example (entry with
AppCapabilityType.CollectPayments, key "stripe_collect_payment", name "Collect
Payments") has a grammar nit; update the description to "Stripe Payments
collects outstanding revenue using the customer’s default payment method." (or
"Stripe Payments collects outstanding revenue with the customer's default
payment method.") so the product name is capitalized and the phrase about the
default payment method reads clearly.
| import "../shared/index.tsp"; | ||
| import "./capability.tsp"; | ||
| import "./app.tsp"; | ||
|
|
There was a problem hiding this comment.
Same circular import risk here (catalog.tsp ↔ app.tsp).
If you address it on one side, it’ll resolve here too (e.g., extract AppType/AppStatus into a third file imported by both).
🤖 Prompt for AI Agents
In @api/spec/src/v3/apps/catalog.tsp around lines 1 - 4, catalog.tsp currently
imports app.tsp creating a circular import with app.tsp; extract the shared
types (e.g., AppType and AppStatus) into a new module (for example
shared/app-types.tsp) and update both catalog.tsp and app.tsp to import those
types instead of importing each other, ensuring no runtime circular dependency
remains.
| @example(#{ | ||
| type: AppType.Stripe, | ||
| name: "Stripe", | ||
| description: "Stripe interation allows you to collect payments with Stripe.", | ||
| capabilities: #[ | ||
| #{ | ||
| type: AppCapabilityType.CalculateTax, | ||
| key: "stripe_calculate_tax", | ||
| name: "Calculate Tax", | ||
| description: "Stripe Tax calculates tax portion of the invoices.", | ||
| }, | ||
| #{ | ||
| type: AppCapabilityType.InvoiceCustomers, | ||
| key: "stripe_invoice_customers", | ||
| name: "Invoice Customers", | ||
| description: "Stripe invoices customers with due amount.", | ||
| }, | ||
| #{ | ||
| type: AppCapabilityType.CollectPayments, | ||
| key: "stripe_collect_payments", | ||
| name: "Collect Payments", | ||
| description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", | ||
| } | ||
| ], | ||
| install_methods: #[ | ||
| AppInstallMethods.WithOAuth2, | ||
| AppInstallMethods.WithAPIKey | ||
| ], | ||
| }) |
There was a problem hiding this comment.
Fix example typo: “Stripe interation” → “Stripe integration”.
Also: your capability key examples alternate between stripe_collect_payment / stripe_collect_payments; not a blocker, but consistency helps copy/paste users.
🤖 Prompt for AI Agents
In @api/spec/src/v3/apps/catalog.tsp around lines 33 - 61, In the example block
for AppType.Stripe update the description string on the AppType.Stripe example
from "Stripe interation allows you to collect payments with Stripe." to "Stripe
integration allows you to collect payments with Stripe." and make capability
keys consistent (use a single form such as "stripe_collect_payments" for the
CollectPayments capability) by updating the key value in the corresponding
capability object; locate these in the example for AppType.Stripe where
capabilities are defined and adjust the description and the CollectPayments
capability.key to the chosen consistent name.
| @get | ||
| @route("/{type}/install/oauth2") | ||
| @operationId("get-app-catalog-item-oauth2-install-url") | ||
| @summary("Get OAuth2 install URL for an app catalog item") | ||
| getOAuth2InstallURL( | ||
| @path type: AppType, | ||
| ): Shared.GetResponse<AppCatalogItem> | Common.NotFound | Common.ErrorResponses; | ||
| } |
There was a problem hiding this comment.
Return type mismatch for OAuth2 install URL endpoint.
The getOAuth2InstallURL operation returns Shared.GetResponse<AppCatalogItem>, but based on the AppOAuth2InstallResponse model defined in oauth.tsp, this should return the OAuth2 install URL response instead. This looks like it might've been copied from the get operation above.
Suggested fix
@get
@route("/{type}/install/oauth2")
@operationId("get-app-catalog-item-oauth2-install-url")
@summary("Get OAuth2 install URL for an app catalog item")
getOAuth2InstallURL(
@path type: AppType,
- ): Shared.GetResponse<AppCatalogItem> | Common.NotFound | Common.ErrorResponses;
+ ): AppOAuth2InstallResponse | Common.NotFound | Common.ErrorResponses;🤖 Prompt for AI Agents
In @api/spec/src/v3/apps/operations.tsp around lines 35 - 42, The
getOAuth2InstallURL operation currently declares its return as
Shared.GetResponse<AppCatalogItem> but should return the OAuth2 install URL
shape; update the return type to Shared.GetResponse<AppOAuth2InstallResponse>
(preserving the existing Common.NotFound | Common.ErrorResponses union) so the
endpoint returns the AppOAuth2InstallResponse model defined in oauth.tsp; locate
and modify the method declaration for getOAuth2InstallURL to use
AppOAuth2InstallResponse in the Shared.GetResponse wrapper.
| GetAppCatalogItemOauth2InstallUrlRequest = api.BillingAppType | ||
| GetAppCatalogItemOauth2InstallUrlResponse = api.BillingAppCatalogItem | ||
| GetAppCatalogItemOauth2InstallUrlHandler httptransport.HandlerWithArgs[GetAppCatalogItemOauth2InstallUrlRequest, GetAppCatalogItemOauth2InstallUrlResponse, GetAppCatalogItemOauth2InstallUrlRequest] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the actual file and examine it
find . -name "get_app_catalog_item_oauth2_install_url.go" -type fRepository: openmeterio/openmeter
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Search for OAuth2 URL related types across the codebase
rg "OAuth2.*Url|InstallUrl|oauth2.*url" -i --type go | head -50Repository: openmeterio/openmeter
Length of output: 6265
🏁 Script executed:
#!/bin/bash
# Look for api.BillingAppCatalogItem definition
rg "type BillingAppCatalogItem" -A 10 --type goRepository: openmeterio/openmeter
Length of output: 634
🏁 Script executed:
#!/bin/bash
# Search for similar OAuth2 endpoints and how they define responses
rg "Oauth2|OAuth2" --type go | grep -i "response\|request" | head -20Repository: openmeterio/openmeter
Length of output: 3024
🏁 Script executed:
#!/bin/bash
# Find the GetOauth2InstallURLOutput type definition
rg "type GetOauth2InstallURLOutput" -A 5 --type goRepository: openmeterio/openmeter
Length of output: 357
🏁 Script executed:
#!/bin/bash
# Also check if there's a v3 equivalent or what the API spec says
rg "GetAppCatalogItemOauth2InstallUrl" -B 5 -A 15 --type go | head -100Repository: openmeterio/openmeter
Length of output: 6075
🏁 Script executed:
#!/bin/bash
# Look at the actual handler file content to see the implementation
cat -n api/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.goRepository: openmeterio/openmeter
Length of output: 1762
Response type should return a URL, not a full catalog item.
The response type api.BillingAppCatalogItem doesn't match what an OAuth2 install URL endpoint should return. Looking at the v1 endpoint (in openmeter/app/marketplace.go), there's a GetOauth2InstallURLOutput that has just a URL field—that's the right pattern here.
Since this is currently a stub, you'll want to define a proper response type (e.g., something like BillingAppOauth2InstallUrlResponse with an installUrl field) that aligns with what the endpoint should actually return.
🤖 Prompt for AI Agents
In @api/v3/handlers/apps/get_app_catalog_item_oauth2_install_url.go around lines
14 - 16, The response alias is wrong: replace
GetAppCatalogItemOauth2InstallUrlResponse = api.BillingAppCatalogItem with a new
simple response type that returns only the install URL (e.g., define
api.BillingAppOauth2InstallUrlResponse struct with a single string field like
InstallUrl or Url) and update the handler typedef
GetAppCatalogItemOauth2InstallUrlHandler to use that new response type; add the
new type into the api package and ensure any code that constructs or returns the
response uses the InstallUrl field.
| type ( | ||
| HandleStripeWebhookRequest struct { | ||
| AppId api.ULID | ||
| Body api.BillingAppStripeWebhookEvent | ||
| } | ||
| HandleStripeWebhookResponse = *struct{} | ||
| HandleStripeWebhookParams = api.ULID | ||
| HandleStripeWebhookHandler httptransport.HandlerWithArgs[HandleStripeWebhookRequest, HandleStripeWebhookResponse, HandleStripeWebhookParams] | ||
| ) | ||
|
|
||
| func (h *handler) HandleStripeWebhook() HandleStripeWebhookHandler { | ||
| return httptransport.NewHandlerWithArgs( | ||
| func(ctx context.Context, r *http.Request, appId HandleStripeWebhookParams) (HandleStripeWebhookRequest, error) { | ||
| body := api.BillingAppStripeWebhookEvent{} | ||
| if err := request.ParseBody(r, &body); err != nil { | ||
| return HandleStripeWebhookRequest{}, err | ||
| } | ||
|
|
||
| return HandleStripeWebhookRequest{ | ||
| AppId: appId, | ||
| Body: body, | ||
| }, nil | ||
| }, | ||
| func(ctx context.Context, request HandleStripeWebhookRequest) (HandleStripeWebhookResponse, error) { | ||
| return nil, apierrors.NewNotImplementedError(ctx, nil) | ||
| }, | ||
| commonhttp.EmptyResponseEncoder[HandleStripeWebhookResponse](http.StatusNoContent), | ||
| httptransport.AppendOptions( | ||
| h.options, | ||
| httptransport.WithOperationName("handle-stripe-webhook"), | ||
| httptransport.WithErrorEncoder(apierrors.GenericErrorEncoder()), | ||
| )..., | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the request.ParseBody function
fd -e go | head -20Repository: openmeterio/openmeter
Length of output: 768
🏁 Script executed:
# Search for ParseBody implementation
rg "func.*ParseBody" --type go -A 5Repository: openmeterio/openmeter
Length of output: 479
🏁 Script executed:
# Look for existing Stripe-related code and patterns
rg "Stripe" --type go -iRepository: openmeterio/openmeter
Length of output: 50378
🏁 Script executed:
# Check the api types for BillingAppStripeWebhookEvent
rg "BillingAppStripeWebhookEvent" --type go -B 2 -A 5Repository: openmeterio/openmeter
Length of output: 2636
🏁 Script executed:
# Look for any existing webhook signature verification patterns
rg "signature|Signature" --type go -i -B 2 -A 2 | head -50Repository: openmeterio/openmeter
Length of output: 3859
🏁 Script executed:
# Check the Stripe webhook driver implementation
cat api/v3/request/body.goRepository: openmeterio/openmeter
Length of output: 572
🏁 Script executed:
# Look at existing Stripe webhook handler
cat openmeter/app/stripe/httpdriver/webhook.goRepository: openmeterio/openmeter
Length of output: 17135
🏁 Script executed:
# Search for any body preservation patterns in the codebase
rg "io.ReadAll\|TeeReader\|DumpBody" --type go -B 2 -A 2Repository: openmeterio/openmeter
Length of output: 47
🏁 Script executed:
# Look for any middleware that reads the body
rg "ioutil.ReadAll\|io.NopCloser" --type go -B 2 -A 2 | head -50Repository: openmeterio/openmeter
Length of output: 47
🏁 Script executed:
# Check if there are any other webhook handlers using similar patterns
rg "HandleStripeWebhook" --type go -B 5 -A 10Repository: openmeterio/openmeter
Length of output: 10907
🏁 Script executed:
# Look at the route/registration for the new handler
rg "handle_stripe_webhook|HandleStripeWebhook" --type go -l | xargs grep -l "Route\|register" 2>/dev/null | head -5Repository: openmeterio/openmeter
Length of output: 82
Thread the raw payload and Stripe-Signature header now to dodge refactoring this later.
The current shape won't let you verify webhook signatures once you implement the logic. Even though it's a scaffold, I'd suggest capturing:
- The raw request payload bytes (for
webhook.ConstructEventWithTolerance) - The
Stripe-Signatureheader value
Your existing Stripe webhook handler in openmeter/app/stripe/httpdriver/webhook.go shows the right pattern—grab these before parsing, so you have what you need when it's time to verify.
🤖 Prompt for AI Agents
In @api/v3/handlers/apps/handle_stripe_webhook.go around lines 14 - 46, Update
HandleStripeWebhookRequest to include RawPayload []byte and StripeSignature
string and populate them in the request factory before parsing the JSON body:
read the raw request body bytes (io.ReadAll) into RawPayload, capture the
Stripe-Signature header via r.Header.Get("Stripe-Signature"), then parse the
JSON into Body from those raw bytes (e.g., json.Unmarshal or io.NopCloser +
bytes.NewReader) so signature verification can later use RawPayload and
StripeSignature while preserving the existing Body field
(api.BillingAppStripeWebhookEvent) and existing handler names
(HandleStripeWebhookRequest, HandleStripeWebhook).
2 participants
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.