refactor(core): pluggable payment provider adapter layer#119
Merged
Conversation
Provider customer creation was gated behind testing.enabled, causing customerPortal to fail for free-only users in production. Now upsertCustomer always provisions the Stripe customer (idempotently). Also removes dead webhook code for customer.upsert/customer.delete actions that no provider ever emitted.
Replace hardcoded Stripe integration with a pluggable provider interface. Core is now provider-agnostic, enabling future adapters (Polar, Paddle, etc.). - Rename StripeRuntime → PaymentProvider interface with id/name fields - Replace StripeProviderConfig with PayKitProviderConfig (id, name, createAdapter) - Move Stripe SDK implementation (~900 lines) from core to @paykitjs/stripe - Update all services: ctx.stripe.* → ctx.provider.* - Remove stripe SDK dependency from core package - Decouple CLI commands from Stripe-specific diagnostics
Merges fix(core): always create provider customer on upsert. Adapts new test to use PaymentProvider interface instead of old StripeRuntime/StripeProviderConfig pattern.
normalizeSchema expects an array of plan() values, not a keyed object.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughPayKit refactors Stripe-specific code into a provider-agnostic architecture: introduces Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API as PayKit API
participant Service as Customer Service
participant Provider as PaymentProvider Adapter
participant DB as Database
Client->>API: call upsertCustomer(input)
API->>Service: upsertCustomer(ctx, input)
Service->>DB: read/lock customer row
Service->>Provider: if needs sync -> createCustomer/updateCustomer(...)
Provider-->>Service: providerCustomer (id, metadata, etc.)
Service->>DB: persist providerCustomer and synced fields
Service-->>API: return synced customer
API-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/paykit/src/api/define-route.ts (1)
440-444:⚠️ Potential issue | 🔴 CriticalDon't call provider-backed upsert from the generic customer-resolution path.
resolveCustomer()now goes throughupsertCustomer(), which in turn callsupsertProviderCustomer(). With the current Stripe adapter that still does a rawcustomers.create(...), concurrent first requests for the same user can create duplicate provider customers before the local mapping is written. This also makes every read-onlyrequireCustomerroute depend on provider availability. Please keep this path DB-only, or make the provider upsert atomic/idempotent before routing request auth through it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/api/define-route.ts` around lines 440 - 444, The generic customer-resolution path currently calls upsertCustomer which invokes upsertProviderCustomer and can create duplicate provider customers; change the resolution to perform a DB-only mapping upsert instead of calling the provider-backed path. Concretely, replace the call to upsertCustomer(...) in resolveCustomer()/requireCustomer flow with a new or existing DB-only helper (e.g., upsertLocalCustomerMapping or upsertCustomer(..., { skipProvider: true })) that only writes the local customer mapping and does not call upsertProviderCustomer(); alternatively make upsertProviderCustomer atomic/idempotent before reusing it here, but do not call provider-backed upsert from the generic resolution path as-is.packages/paykit/src/cli/commands/status.ts (1)
34-39:⚠️ Potential issue | 🟠 MajorDon’t report the provider as healthy without an actual probe.
This now prints a green provider check based only on config presence, so invalid keys or an unusable adapter can still end with “Everything looks good”. That’s a regression for a diagnostic command. Consider exposing an optional provider
diagnostics()/checkConnection()hook and render this section as “configured” vs “verified”.Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/cli/commands/status.ts` around lines 34 - 39, The status output currently uses hasProvider to mark the provider as healthy without an actual probe; update the logic in the status command (where p.log.info prints the provider line and the similar block at the other occurrence) to call an optional provider diagnostics/checkConnection hook (e.g., provider.diagnostics() or provider.checkConnection()) when available, await its result, and render “Provider verified” (green) only if the probe succeeds, otherwise render “Provider configured” (dim/yellow) or “Provider failed” (red) with the diagnostic error; if the hook is not implemented fall back to “Provider configured” (not verified) so the UI differentiates configured vs verified.packages/paykit/src/subscription/subscription.service.ts (1)
721-752:⚠️ Potential issue | 🟠 MajorMake these provider mutations require
subscription, or persist a fallback state unconditionally.
resumeSubscription,cancelSubscription, andscheduleSubscriptionChangeall returnsubscription?, but these branches only refreshproviderData/billing dates when it exists. If an adapter returnsundefinedhere, local state keeps stale or missingsubscriptionScheduleIdvalues after the provider mutation, so later resume/cancel flows can target the wrong schedule or lose the new one entirely. Either tighten those provider contracts to always return asubscription, or update local billing/provider state even in the fallback path.Also applies to: 903-915, 957-969
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/subscription/subscription.service.ts` around lines 721 - 752, The provider adapter may return providerResult.subscription as undefined, but the code only updates provider/billing state when it exists; change the logic in the resume/cancel/schedule flows (calls to ctx.provider.resumeSubscription, cancelSubscription, scheduleSubscriptionChange) so that syncSubscriptionFromProvider and syncSubscriptionBillingState are invoked with a deterministic fallback object when providerResult.subscription is missing (use activeSubscriptionRef.subscriptionId, activeSubscriptionRef.subscriptionScheduleId, activeSubscription.status and any available period dates or nulls) so providerData.subscriptionId and subscriptionScheduleId and billing dates are always persisted; update the same pattern at the other locations mentioned (the blocks around syncSubscriptionFromProvider and syncSubscriptionBillingState at the ranges corresponding to lines ~903-915 and ~957-969) and ensure deleteScheduledSubscriptionsInGroupIfNeeded and replaceSubscriptionSchedule still run as before.packages/paykit/src/customer/customer.service.ts (1)
353-377:⚠️ Potential issue | 🟠 MajorDon't short-circuit provider upserts once a local mapping exists.
After the first sync, this function returns the cached provider customer and never calls
ctx.provider.upsertCustomer(...)again. That means laterupsertCustomer()calls won't propagate changedname/metadata, and they also can't heal a provider-side customer that was deleted out of band.Suggested direction
export async function upsertProviderCustomer( ctx: PayKitContext, input: { customerId: string }, ): Promise<{ customerId: string; providerCustomer: ProviderCustomer; providerCustomerId: string }> { const providerId = ctx.provider.id; const existingCustomer = await getCustomerByIdOrThrow(ctx.database, input.customerId); - const existingProviderCustomer = getProviderCustomer(existingCustomer, providerId); - const existingProviderCustomerId = existingProviderCustomer?.id ?? null; - - if (existingProviderCustomerId) { - return { - customerId: input.customerId, - providerCustomer: existingProviderCustomer as ProviderCustomer, - providerCustomerId: existingProviderCustomerId, - }; - } const { providerCustomer } = await ctx.provider.upsertCustomer({ createTestClock: ctx.options.testing?.enabled === true, id: existingCustomer.id, email: existingCustomer.email ?? undefined, name: existingCustomer.name ?? undefined, metadata: existingCustomer.metadata ?? undefined, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/customer/customer.service.ts` around lines 353 - 377, upsertProviderCustomer currently returns the cached mapping when getProviderCustomer(...) finds an existingProviderCustomerId, which prevents calling ctx.provider.upsertCustomer(...) to propagate changes or recreate deleted provider-side customers; change the flow in upsertProviderCustomer so it always calls ctx.provider.upsertCustomer(...) (passing id/email/name/metadata/createTestClock), then use the returned providerCustomer to update or create the local mapping (replace or persist the providerCustomerId/ProviderCustomer), and finally return the updated providerCustomer and providerCustomerId; use existing symbols getCustomerByIdOrThrow, getProviderCustomer, ctx.provider.upsertCustomer, and existingProviderCustomerId to locate and implement this fix.
🧹 Nitpick comments (4)
packages/paykit/src/core/errors.ts (1)
28-28: Make this error message auth-scheme agnostic.
"provider test secret key"still bakes in Stripe-style auth. For a generic provider interface, wording like"test credentials"will age better for adapters that use tokens or OAuth instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/core/errors.ts` at line 28, Update the PROVIDER_TEST_KEY_REQUIRED error message to be auth-scheme agnostic by replacing the Stripe-specific phrase "provider test secret key" with a more generic term such as "provider test credentials" (i.e., edit the value of the PROVIDER_TEST_KEY_REQUIRED constant in packages/paykit/src/core/errors.ts to read something like "Testing mode requires provider test credentials").packages/paykit/src/core/__tests__/context.test.ts (1)
34-39: Avoid double-casting the adapter mock.
as unknown as PaymentProviderremoves compile-time coverage exactly where this test is supposed to guard the new adapter contract. Prefer a small typed stub/helper withvi.fn()methods andsatisfies PaymentProviderso interface drift is caught by the test instead of being masked.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/core/__tests__/context.test.ts` around lines 34 - 39, Replace the double-cast mock "adapter" (currently declared as const adapter = { id: "test", name: "Test" } as unknown as PaymentProvider) with a small strongly-typed stub that implements the PaymentProvider contract using vi.fn() for methods and use the TypeScript "satisfies PaymentProvider" clause; update the provider.createAdapter to return that stub so the test uses a real typed adapter and will fail at compile time if the PaymentProvider interface drifts (refer to symbols: adapter, PaymentProvider, provider, PayKitProviderConfig, createAdapter).packages/paykit/src/index.ts (1)
27-33: Call out this barrel change as a breaking API change.Removing the Stripe-specific top-level type exports means existing
import type { StripeProviderConfig, StripeProviderOptions } from "paykitjs"consumers will fail at compile time after upgrade. Please pair this with an explicit migration note and semver/release-plan check before shipping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/index.ts` around lines 27 - 33, You removed Stripe-specific top-level type exports causing a breaking API change for consumers expecting StripeProviderConfig and StripeProviderOptions; restore compatibility by re-exporting those types from the barrel (i.e., add exports for StripeProviderConfig and StripeProviderOptions alongside PayKitProviderConfig/PaymentProvider etc.) or add a clear migration path: add a deprecation alias that re-exports StripeProviderConfig/StripeProviderOptions from their original module, update the changelog/release notes with a migration example, and ensure the release is versioned as a major bump per semver before shipping.packages/paykit/src/core/context.ts (1)
36-41: Add an invariant betweenoptions.providerand the created adapter.Core now keeps two identities for the same provider:
options.provider.id/nameandprovider.id/name. If a provider package accidentally returns different values fromcreateAdapter(), you'll persist customer/provider data under one key while CLI/status output shows another. A small assertion here would make the new extension point much safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/core/context.ts` around lines 36 - 41, Add a runtime invariant that ensures the adapter returned by options.provider.createAdapter() matches the original provider identity: after calling const provider = options.provider.createAdapter(), assert that provider.id === options.provider.id and provider.name === options.provider.name (or throw a clear Error mentioning which field mismatched and both values). This check should live in the same scope as the provider assignment in the function that returns { options, database, provider } so a mismatch fails fast and surfaces the offending provider implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/superpowers/plans/2026-04-12-payment-provider-adapter.md`:
- Around line 167-168: Replace the hardcoded absolute verification command (the
Run line containing "cd
/Users/maxktz/.superset/worktrees/paykit/maxktz/provider-adapter-paykit-research
&& pnpm typecheck...") with a repo-relative invocation: either change to a
relative path from the repository root (e.g., using ./ or the repository
directory variable) or derive the repo root dynamically (e.g., use git rev-parse
--show-toplevel or a MAKEFILE/package script) and run pnpm typecheck from there;
apply the same change to the other occurrences mentioned (lines referenced as
324-325 and 564-588) so all verification commands are portable for CI and other
contributors.
In `@docs/superpowers/specs/2026-04-12-payment-provider-adapter-design.md`:
- Around line 192-199: The docs claim "CLI — untouched" but the PR modifies CLI
behavior (Stripe-specific diagnostics removed and provider reporting changed in
packages/paykit/src/cli/commands/check.ts); update the "What Doesn't Change"
section to remove or amend the "CLI — untouched" bullet and replace it with a
short, accurate note such as "CLI — modified (Stripe diagnostics removed;
provider reporting updated in packages/paykit/src/cli/commands/check.ts)" so
readers see the scope of the refactor.
In `@packages/paykit/src/cli/commands/check.ts`:
- Around line 70-73: The Provider section currently only prints the configured
name/id using p.log.info and config.options.provider.name/id which gives a false
"pass"; update the code in check.ts so it either performs a real verification
(call the provider adapter's health/auth method, e.g., provider.authenticate()
or provider.healthCheck() and log success/failure based on its result) or, if
diagnostics are out of scope, change the message text from a check mark to a
neutral label like "configured" (e.g., replace the green check icon and wording
with "Configured" while keeping name/id) and ensure any error from a real check
is caught and logged via p.log.error.
In `@packages/paykit/src/cli/commands/push.ts`:
- Around line 29-33: The log in push.ts dereferences
config.options.provider.name/id without checking provider; update the push
command to guard config.options.provider before using it (same pattern as the
status command): if provider is missing, avoid accessing name/id, log a clear
message or run the existing validation/abort path, and only create the context
or call p.log.info with provider.name/provider.id after confirming
config.options.provider exists; locate the p.log.info call in push.ts and the
surrounding context-creation logic to apply the guard around those accesses.
In `@packages/stripe/src/stripe-provider.ts`:
- Around line 514-542: The upsertCustomer implementation always calls
client.customers.create (and testHelpers.testClocks.create) which is not
idempotent; change upsertCustomer to first look up an existing provider customer
by the stable PayKit id (use metadata.customerId or a Stripe search on customers
with metadata.customerId === data.id) and if found call client.customers.update
instead of create, and reuse its test_clock (or only create a test clock when
none exists), and also apply a stable idempotency key when creating resources
(pass idempotency_key in the API options) to protect against retries; update
references in the function (client.customers.create, client.customers.update,
client.testHelpers.testClocks.create, normalizeStripeTestClock) accordingly so
duplicate customers/test clocks are not minted.
- Around line 869-875: In handleWebhook, treat the Stripe signature header
case-insensitively by locating the header key in data.headers using a
case-insensitive lookup (e.g., search for any key equal to "stripe-signature"
ignoring case) before assigning signature; keep the existing error throw
(PayKitError.from(...)) if no matching header is found, and then pass the found
signature into client.webhooks.constructEvent(data.body, signature,
options.webhookSecret) as before.
- Around line 894-900: The Stripe client is created without an explicit API
version in the stripe() factory; update the StripeSdk instantiation in stripe()
/ createAdapter() so it passes a pinned apiVersion (e.g., options.apiVersion or
a hardcoded stable version) to the constructor used in createStripeProvider;
modify the call where new StripeSdk(options.secretKey) is created to supply the
apiVersion option so all normalized helpers like normalizeStripeTestClock and
invoice/subscription handling see a consistent API shape.
---
Outside diff comments:
In `@packages/paykit/src/api/define-route.ts`:
- Around line 440-444: The generic customer-resolution path currently calls
upsertCustomer which invokes upsertProviderCustomer and can create duplicate
provider customers; change the resolution to perform a DB-only mapping upsert
instead of calling the provider-backed path. Concretely, replace the call to
upsertCustomer(...) in resolveCustomer()/requireCustomer flow with a new or
existing DB-only helper (e.g., upsertLocalCustomerMapping or upsertCustomer(...,
{ skipProvider: true })) that only writes the local customer mapping and does
not call upsertProviderCustomer(); alternatively make upsertProviderCustomer
atomic/idempotent before reusing it here, but do not call provider-backed upsert
from the generic resolution path as-is.
In `@packages/paykit/src/cli/commands/status.ts`:
- Around line 34-39: The status output currently uses hasProvider to mark the
provider as healthy without an actual probe; update the logic in the status
command (where p.log.info prints the provider line and the similar block at the
other occurrence) to call an optional provider diagnostics/checkConnection hook
(e.g., provider.diagnostics() or provider.checkConnection()) when available,
await its result, and render “Provider verified” (green) only if the probe
succeeds, otherwise render “Provider configured” (dim/yellow) or “Provider
failed” (red) with the diagnostic error; if the hook is not implemented fall
back to “Provider configured” (not verified) so the UI differentiates configured
vs verified.
In `@packages/paykit/src/customer/customer.service.ts`:
- Around line 353-377: upsertProviderCustomer currently returns the cached
mapping when getProviderCustomer(...) finds an existingProviderCustomerId, which
prevents calling ctx.provider.upsertCustomer(...) to propagate changes or
recreate deleted provider-side customers; change the flow in
upsertProviderCustomer so it always calls ctx.provider.upsertCustomer(...)
(passing id/email/name/metadata/createTestClock), then use the returned
providerCustomer to update or create the local mapping (replace or persist the
providerCustomerId/ProviderCustomer), and finally return the updated
providerCustomer and providerCustomerId; use existing symbols
getCustomerByIdOrThrow, getProviderCustomer, ctx.provider.upsertCustomer, and
existingProviderCustomerId to locate and implement this fix.
In `@packages/paykit/src/subscription/subscription.service.ts`:
- Around line 721-752: The provider adapter may return
providerResult.subscription as undefined, but the code only updates
provider/billing state when it exists; change the logic in the
resume/cancel/schedule flows (calls to ctx.provider.resumeSubscription,
cancelSubscription, scheduleSubscriptionChange) so that
syncSubscriptionFromProvider and syncSubscriptionBillingState are invoked with a
deterministic fallback object when providerResult.subscription is missing (use
activeSubscriptionRef.subscriptionId,
activeSubscriptionRef.subscriptionScheduleId, activeSubscription.status and any
available period dates or nulls) so providerData.subscriptionId and
subscriptionScheduleId and billing dates are always persisted; update the same
pattern at the other locations mentioned (the blocks around
syncSubscriptionFromProvider and syncSubscriptionBillingState at the ranges
corresponding to lines ~903-915 and ~957-969) and ensure
deleteScheduledSubscriptionsInGroupIfNeeded and replaceSubscriptionSchedule
still run as before.
---
Nitpick comments:
In `@packages/paykit/src/core/__tests__/context.test.ts`:
- Around line 34-39: Replace the double-cast mock "adapter" (currently declared
as const adapter = { id: "test", name: "Test" } as unknown as PaymentProvider)
with a small strongly-typed stub that implements the PaymentProvider contract
using vi.fn() for methods and use the TypeScript "satisfies PaymentProvider"
clause; update the provider.createAdapter to return that stub so the test uses a
real typed adapter and will fail at compile time if the PaymentProvider
interface drifts (refer to symbols: adapter, PaymentProvider, provider,
PayKitProviderConfig, createAdapter).
In `@packages/paykit/src/core/context.ts`:
- Around line 36-41: Add a runtime invariant that ensures the adapter returned
by options.provider.createAdapter() matches the original provider identity:
after calling const provider = options.provider.createAdapter(), assert that
provider.id === options.provider.id and provider.name === options.provider.name
(or throw a clear Error mentioning which field mismatched and both values). This
check should live in the same scope as the provider assignment in the function
that returns { options, database, provider } so a mismatch fails fast and
surfaces the offending provider implementation.
In `@packages/paykit/src/core/errors.ts`:
- Line 28: Update the PROVIDER_TEST_KEY_REQUIRED error message to be auth-scheme
agnostic by replacing the Stripe-specific phrase "provider test secret key" with
a more generic term such as "provider test credentials" (i.e., edit the value of
the PROVIDER_TEST_KEY_REQUIRED constant in packages/paykit/src/core/errors.ts to
read something like "Testing mode requires provider test credentials").
In `@packages/paykit/src/index.ts`:
- Around line 27-33: You removed Stripe-specific top-level type exports causing
a breaking API change for consumers expecting StripeProviderConfig and
StripeProviderOptions; restore compatibility by re-exporting those types from
the barrel (i.e., add exports for StripeProviderConfig and StripeProviderOptions
alongside PayKitProviderConfig/PaymentProvider etc.) or add a clear migration
path: add a deprecation alias that re-exports
StripeProviderConfig/StripeProviderOptions from their original module, update
the changelog/release notes with a migration example, and ensure the release is
versioned as a major bump per semver before shipping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cc17d3e4-0ae5-4212-ae49-e77102e805e1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
docs/superpowers/plans/2026-04-12-payment-provider-adapter.mddocs/superpowers/specs/2026-04-12-payment-provider-adapter-design.mde2e/cli/check.test.tse2e/smoke/setup.tspackages/paykit/package.jsonpackages/paykit/src/api/define-route.tspackages/paykit/src/cli/commands/check.tspackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/commands/status.tspackages/paykit/src/cli/utils/format.tspackages/paykit/src/core/__tests__/context.test.tspackages/paykit/src/core/context.tspackages/paykit/src/core/errors.tspackages/paykit/src/customer/__tests__/customer.service.test.tspackages/paykit/src/customer/customer.api.tspackages/paykit/src/customer/customer.service.tspackages/paykit/src/index.tspackages/paykit/src/product/product-sync.service.tspackages/paykit/src/providers/provider.tspackages/paykit/src/providers/stripe.tspackages/paykit/src/subscription/subscription.service.tspackages/paykit/src/testing/testing.service.tspackages/paykit/src/types/events.tspackages/paykit/src/types/options.tspackages/paykit/src/webhook/webhook.service.tspackages/stripe/package.jsonpackages/stripe/src/__tests__/stripe-provider.test.tspackages/stripe/src/__tests__/stripe.test.tspackages/stripe/src/index.tspackages/stripe/src/stripe-provider.ts
💤 Files with no reviewable changes (5)
- packages/paykit/package.json
- e2e/cli/check.test.ts
- packages/paykit/src/cli/utils/format.ts
- packages/paykit/src/types/events.ts
- packages/paykit/src/providers/stripe.ts
docs/superpowers/specs/2026-04-12-payment-provider-adapter-design.md
Outdated
Show resolved
Hide resolved
Comment on lines
+514
to
+542
| async upsertCustomer(data) { | ||
| let testClock: ProviderTestClock | undefined; | ||
| if (data.createTestClock) { | ||
| assertStripeTestKey(options); | ||
| const clock = await client.testHelpers.testClocks.create({ | ||
| frozen_time: Math.floor(Date.now() / 1000), | ||
| name: data.id, | ||
| }); | ||
| testClock = normalizeStripeTestClock(clock); | ||
| } | ||
|
|
||
| const customer = await client.customers.create({ | ||
| email: data.email, | ||
| metadata: { | ||
| customerId: data.id, | ||
| ...data.metadata, | ||
| }, | ||
| name: data.name, | ||
| test_clock: testClock?.id, | ||
| }); | ||
|
|
||
| return { | ||
| providerCustomer: { | ||
| id: customer.id, | ||
| frozenTime: testClock?.frozenTime.toISOString(), | ||
| testClockId: testClock?.id, | ||
| }, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
upsertCustomer is not idempotent yet.
Line 525 always creates a new Stripe customer, so a retried request or a second upsert for the same data.id will mint duplicates. When createTestClock is set, it can also leak extra test clocks. Please make this path idempotent before release—at minimum by using a stable idempotency key, and ideally by reusing/updating an existing provider customer for the same PayKit customer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe/src/stripe-provider.ts` around lines 514 - 542, The
upsertCustomer implementation always calls client.customers.create (and
testHelpers.testClocks.create) which is not idempotent; change upsertCustomer to
first look up an existing provider customer by the stable PayKit id (use
metadata.customerId or a Stripe search on customers with metadata.customerId ===
data.id) and if found call client.customers.update instead of create, and reuse
its test_clock (or only create a test clock when none exists), and also apply a
stable idempotency key when creating resources (pass idempotency_key in the API
options) to protect against retries; update references in the function
(client.customers.create, client.customers.update,
client.testHelpers.testClocks.create, normalizeStripeTestClock) accordingly so
duplicate customers/test clocks are not minted.
Comment on lines
+894
to
+900
| export function stripe(options: StripeOptions): PayKitProviderConfig { | ||
| return { | ||
| ...options, | ||
| id: "stripe", | ||
| kind: "stripe", | ||
| name: "Stripe", | ||
| createAdapter(): PaymentProvider { | ||
| return createStripeProvider(new StripeSdk(options.secretKey), options); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the StripeSdk class definition
fd -t f "stripe" packages/stripe/src/ | head -20Repository: getpaykit/paykit
Length of output: 197
🏁 Script executed:
# Locate and read the stripe-provider.ts file to understand the context
cat -n packages/stripe/src/stripe-provider.ts | sed -n '890,910p'Repository: getpaykit/paykit
Length of output: 416
🏁 Script executed:
# Search for StripeSdk definition and constructor
rg -A 10 "class StripeSdk|constructor.*secretKey" packages/stripe/Repository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
# Check if there's a defined API version anywhere in the project
rg -i "apiversion|api.version|stripe.*version" packages/stripe/src/ --type ts --type tsxRepository: getpaykit/paykit
Length of output: 87
🏁 Script executed:
# Check imports at the top of stripe-provider.ts
cat -n packages/stripe/src/stripe-provider.ts | head -50Repository: getpaykit/paykit
Length of output: 2084
🏁 Script executed:
# Search for StripeSdk definition with correct syntax
rg "class StripeSdk|StripeSdk.*=" packages/stripe/ -t tsRepository: getpaykit/paykit
Length of output: 368
🏁 Script executed:
# Search for apiVersion references
rg "apiVersion" packages/stripe/ -t tsRepository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
# Check the test files for StripeSdk usage
cat -n packages/stripe/src/__tests__/stripe.test.ts | head -100Repository: getpaykit/paykit
Length of output: 3466
🏁 Script executed:
# Look for createStripeProvider function to see if it handles versions
rg -A 20 "function createStripeProvider" packages/stripe/src/stripe-provider.tsRepository: getpaykit/paykit
Length of output: 714
🏁 Script executed:
# Check if there are any comments or documentation about Stripe API versions
rg -B 5 -A 5 "StripeSdk" packages/stripe/src/stripe-provider.ts | head -50Repository: getpaykit/paykit
Length of output: 1312
🏁 Script executed:
# Check package.json for Stripe SDK version
cat packages/stripe/package.jsonRepository: getpaykit/paykit
Length of output: 878
🏁 Script executed:
# Search for any Stripe documentation or version references
rg "stripe.*version|apiVersion|api.version" packages/stripe/ -iRepository: getpaykit/paykit
Length of output: 42
🏁 Script executed:
# Look for any environment variables or configuration
rg "STRIPE|stripe" packages/stripe/package.jsonRepository: getpaykit/paykit
Length of output: 113
🏁 Script executed:
# Check Stripe SDK documentation via web search about constructor options
web_search
Stripe JavaScript SDK constructor apiVersion parameterRepository: getpaykit/paykit
Length of output: 155
🌐 Web query:
Stripe JavaScript SDK v19 constructor apiVersion parameter options
💡 Result:
The Stripe JavaScript SDK (Stripe.js and @stripe/stripe-js) does not have a "v19" version. The @stripe/stripe-js npm package is currently at v9.x (as of March 2026), and Stripe.js uses named versions like 'dahlia', 'acacia', or 'clover' loaded via script tags (e.g., https://js.stripe.com/dahlia/stripe.js), not numeric versions like v19. The Stripe Node.js SDK has a v19.x series (released starting September 2025), but that is server-side only. For the Stripe(publishableKey, options) constructor in Stripe.js: - The apiVersion option in the second parameter allows overriding the account's default API version for requests made by Stripe.js. - However, it is explicitly documented as available only on Stripe.js v3. For later versions (post-v3, such as acacia, clover, dahlia), the API version is pinned to match the Stripe.js version and cannot be overridden. Each versioned Stripe.js uses a fixed compatible API version (e.g., acacia uses 2024-12-18.acacia or similar). Example usage (v3 only): const stripe = Stripe('pk_test_...', { apiVersion: '2020-08-27' }); In modern versions, align your server-side API version with the client-side Stripe.js version for compatibility. Note: Confusion may arise from the Node.js SDK v19, but the query specifies "JavaScript SDK," referring to the browser/client-side Stripe.js.
Citations:
- 1: https://docs.stripe.com/js/initializing.md
- 2: https://docs.stripe.com/sdks/stripejs-versioning
- 3: https://docs.stripe.com/js
- 4: https://docs.stripe.com/js/initializing
- 5: https://docs.stripe.com/sdks/stripejs-versioning.md
- 6: https://github.com/stripe/stripe-js/tags
🌐 Web query:
Stripe Node.js SDK v19 constructor apiVersion parameter
💡 Result:
In Stripe Node.js SDK v19, the Stripe client is constructed using new Stripe(apiKey, options?), where options is an object that includes the apiVersion parameter. The apiVersion option allows overriding the pinned API version for the SDK (v19.0.0 pins 2025-09-30.clover). Default is null, using the SDK's pinned version. Example: const stripe = new Stripe('sk_test_...', { apiVersion: '2025-09-30.clover', // or another version like '2022-11-15' // other options: maxNetworkRetries, timeout, etc. }); This matches the standard constructor pattern across SDK versions, confirmed in the repository README and release notes. v19 introduces no breaking changes to the constructor; major constructor refactor (true ES6 class) occurs in v22.
Citations:
- 1: https://github.com/stripe/stripe-node/releases/tag/v19.0.0
- 2: https://github.com/stripe/stripe-node/blob/v19.0.0/CHANGELOG.md
- 3: https://github.com/stripe/stripe-node?tab=readme-ov-file
- 4: https://github.com/stripe/stripe-node/releases/tag/v22.0.0
Pin the Stripe API version in the client constructor.
The adapter currently instantiates the Stripe SDK without an explicit API version: new StripeSdk(options.secretKey). Since the SDK supports an apiVersion option and this adapter normalizes Stripe responses assuming specific shapes (e.g., normalizeStripeTestClock, invoice/subscription structures), behavior will drift across tenants if their Stripe accounts have different default API versions. Pass a consistent version explicitly when creating the client.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe/src/stripe-provider.ts` around lines 894 - 900, The Stripe
client is created without an explicit API version in the stripe() factory;
update the StripeSdk instantiation in stripe() / createAdapter() so it passes a
pinned apiVersion (e.g., options.apiVersion or a hardcoded stable version) to
the constructor used in createStripeProvider; modify the call where new
StripeSdk(options.secretKey) is created to supply the apiVersion option so all
normalized helpers like normalizeStripeTestClock and invoice/subscription
handling see a consistent API shape.
- Split upsertCustomer into createCustomer + updateCustomer on PaymentProvider - Store synced email/name/metadata snapshot to avoid redundant provider calls - Only call provider when customer data actually changed - Add optional check() to PaymentProvider for CLI diagnostics - Restore provider verification in CLI check/status/push commands - Case-insensitive webhook signature header lookup - Make error messages provider-agnostic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/paykit/src/customer/__tests__/customer.service.test.ts (1)
447-498: Test name may be misleading - this tests "existing provider customer without snapshot", not "first sync".The test "calls provider when no snapshot exists (first sync)" actually sets up a customer that already has a provider customer id (
stripe: { id: "cus_existing" }) but is missing thesyncedEmail/syncedName/syncedMetadatafields. This is a valid scenario (migrated data), but "first sync" typically implies no provider customer exists yet. Consider renaming to clarify:- it("calls provider when no snapshot exists (first sync)", async () => { + it("calls provider updateCustomer when snapshot fields are missing", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/customer/__tests__/customer.service.test.ts` around lines 447 - 498, The test name is misleading: "calls provider when no snapshot exists (first sync)" actually covers an existing provider customer with missing snapshot fields; rename the test to reflect that scenario (e.g., "calls provider for existing provider customer missing snapshot fields" or "calls provider when provider id exists but synced snapshot missing") and update any related test description/comments; locate the test in customer.service.test.ts where the it(...) block defines providerMock and existingCustomer (stripe: { id: "cus_existing" }) and change the string passed to it(...) to the clearer name.packages/paykit/src/customer/customer.service.ts (1)
353-362: Metadata comparison withJSON.stringifymay have ordering issues.
JSON.stringifycomparison works for simple cases but can produce false positives when object key ordering differs (e.g.,{a:1, b:2}vs{b:2, a:1}). While JavaScript objects maintain insertion order in modern engines, data retrieved from databases or APIs may have different ordering.Consider a stable comparison:
♻️ Suggested improvement
function providerCustomerNeedsSync( existing: ProviderCustomer, customer: { email: string | null; name: string | null; metadata: Record<string, string> | null }, ): boolean { if ((existing.syncedEmail ?? null) !== (customer.email ?? null)) return true; if ((existing.syncedName ?? null) !== (customer.name ?? null)) return true; - const existingMeta = JSON.stringify(existing.syncedMetadata ?? null); - const currentMeta = JSON.stringify(customer.metadata ?? null); - return existingMeta !== currentMeta; + const existingMeta = existing.syncedMetadata ?? null; + const currentMeta = customer.metadata ?? null; + if (existingMeta === null && currentMeta === null) return false; + if (existingMeta === null || currentMeta === null) return true; + const existingKeys = Object.keys(existingMeta).sort(); + const currentKeys = Object.keys(currentMeta).sort(); + if (existingKeys.length !== currentKeys.length) return true; + return existingKeys.some((k, i) => k !== currentKeys[i] || existingMeta[k] !== currentMeta[k]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/paykit/src/customer/customer.service.ts` around lines 353 - 362, The providerCustomerNeedsSync function currently compares metadata by JSON.stringify which can yield false positives due to key order differences; replace that comparison with a stable deep-equality check: normalize both existing.syncedMetadata and customer.metadata (e.g., sort keys recursively or use a deepEqual utility) and compare the normalized objects instead of stringifying, keeping the early checks for syncedEmail and syncedName intact; update references to existing.syncedMetadata and customer.metadata in providerCustomerNeedsSync to use the normalization/deep-equal helper.packages/stripe/src/stripe-provider.ts (2)
514-542:createCustomerrelies on service-layer idempotency.The provider's
createCustomeralways creates a new Stripe customer. Idempotency is now handled at the service layer (customer.service.ts) which only callscreateCustomerwhen no provider customer id exists. This is acceptable but worth noting that retriedcreateCustomercalls (e.g., due to network issues after Stripe creates the customer but before the response is received) could still create duplicates.Consider adding an idempotency key for additional safety:
🛡️ Optional: Add idempotency key
const customer = await client.customers.create({ email: data.email, metadata: { customerId: data.id, ...data.metadata, }, name: data.name, test_clock: testClock?.id, + }, { + idempotencyKey: `create-customer-${data.id}`, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe/src/stripe-provider.ts` around lines 514 - 542, createCustomer currently always issues new Stripe calls and can produce duplicates on retries; add an idempotency key to the Stripe requests by passing a stable idempotency key (e.g. use data.id or generate a request-scoped UUID if absent) to client.customers.create (and to client.testHelpers.testClocks.create when creating testClock) via the Stripe request options so retries are deduplicated; ensure the returned providerCustomer shape remains the same and use the same idempotency key for both calls when testClock is created.
568-577: Polling loop could benefit from exponential backoff.The test clock advancement polls every 2 seconds for up to 2 minutes. Consider exponential backoff to reduce API calls while still responding quickly when the clock is ready.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe/src/stripe-provider.ts` around lines 568 - 577, Replace the fixed 2s sleep in the polling loop that calls client.testHelpers.testClocks.retrieve (looking for data.testClockId to become "ready") with an exponential backoff: use a baseDelay (e.g., 500ms) and double it each iteration up to a maxDelay (e.g., 2000ms), and await that computed delay before retrying; keep the same retry cap/timeout behavior and still call normalizeStripeTestClock when ready and throw the same Error if not ready. Update the loop surrounding client.testHelpers.testClocks.retrieve / normalizeStripeTestClock to compute delay = Math.min(baseDelay * 2**attempt, maxDelay) and await it instead of the fixed 2000ms sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stripe/src/stripe-provider.ts`:
- Around line 932-939: The Stripe client instantiated in stripe(options)
currently calls new StripeSdk(options.secretKey) without an explicit apiVersion;
update the constructor call where StripeSdk is created (in the stripe function
that returns createStripeProvider) to include a pinned apiVersion option
matching the target Stripe Node SDK stable release (e.g., the 2025 stable API
version) so responses remain consistent with the adapter expectations in
createStripeProvider and downstream code that relies on specific
invoice/subscription shapes.
---
Nitpick comments:
In `@packages/paykit/src/customer/__tests__/customer.service.test.ts`:
- Around line 447-498: The test name is misleading: "calls provider when no
snapshot exists (first sync)" actually covers an existing provider customer with
missing snapshot fields; rename the test to reflect that scenario (e.g., "calls
provider for existing provider customer missing snapshot fields" or "calls
provider when provider id exists but synced snapshot missing") and update any
related test description/comments; locate the test in customer.service.test.ts
where the it(...) block defines providerMock and existingCustomer (stripe: { id:
"cus_existing" }) and change the string passed to it(...) to the clearer name.
In `@packages/paykit/src/customer/customer.service.ts`:
- Around line 353-362: The providerCustomerNeedsSync function currently compares
metadata by JSON.stringify which can yield false positives due to key order
differences; replace that comparison with a stable deep-equality check:
normalize both existing.syncedMetadata and customer.metadata (e.g., sort keys
recursively or use a deepEqual utility) and compare the normalized objects
instead of stringifying, keeping the early checks for syncedEmail and syncedName
intact; update references to existing.syncedMetadata and customer.metadata in
providerCustomerNeedsSync to use the normalization/deep-equal helper.
In `@packages/stripe/src/stripe-provider.ts`:
- Around line 514-542: createCustomer currently always issues new Stripe calls
and can produce duplicates on retries; add an idempotency key to the Stripe
requests by passing a stable idempotency key (e.g. use data.id or generate a
request-scoped UUID if absent) to client.customers.create (and to
client.testHelpers.testClocks.create when creating testClock) via the Stripe
request options so retries are deduplicated; ensure the returned
providerCustomer shape remains the same and use the same idempotency key for
both calls when testClock is created.
- Around line 568-577: Replace the fixed 2s sleep in the polling loop that calls
client.testHelpers.testClocks.retrieve (looking for data.testClockId to become
"ready") with an exponential backoff: use a baseDelay (e.g., 500ms) and double
it each iteration up to a maxDelay (e.g., 2000ms), and await that computed delay
before retrying; keep the same retry cap/timeout behavior and still call
normalizeStripeTestClock when ready and throw the same Error if not ready.
Update the loop surrounding client.testHelpers.testClocks.retrieve /
normalizeStripeTestClock to compute delay = Math.min(baseDelay * 2**attempt,
maxDelay) and await it instead of the fixed 2000ms sleep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 634b1cca-634f-43ad-871a-b96606847f23
📒 Files selected for processing (12)
.gitignoredocs/superpowers/plans/2026-04-09-docs-content.mdpackages/paykit/src/cli/commands/check.tspackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/commands/status.tspackages/paykit/src/core/errors.tspackages/paykit/src/customer/__tests__/customer.service.test.tspackages/paykit/src/customer/customer.service.tspackages/paykit/src/providers/provider.tspackages/stripe/src/__tests__/stripe-provider.test.tspackages/stripe/src/__tests__/stripe.test.tspackages/stripe/src/stripe-provider.ts
💤 Files with no reviewable changes (1)
- docs/superpowers/plans/2026-04-09-docs-content.md
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- packages/paykit/src/core/errors.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/stripe/src/tests/stripe-provider.test.ts
- packages/stripe/src/tests/stripe.test.ts
- packages/paykit/src/cli/commands/status.ts
- packages/paykit/src/cli/commands/push.ts
Comment on lines
+932
to
939
| export function stripe(options: StripeOptions): PayKitProviderConfig { | ||
| return { | ||
| ...options, | ||
| id: "stripe", | ||
| kind: "stripe", | ||
| name: "Stripe", | ||
| createAdapter(): PaymentProvider { | ||
| return createStripeProvider(new StripeSdk(options.secretKey), options); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Pin the Stripe API version to ensure consistent behavior.
The Stripe client is instantiated without an explicit apiVersion. Since the adapter normalizes Stripe responses assuming specific shapes (e.g., subscription items, invoice structures), behavior may differ across environments with different Stripe account API versions.
🛡️ Suggested fix
export function stripe(options: StripeOptions): PayKitProviderConfig {
return {
id: "stripe",
name: "Stripe",
createAdapter(): PaymentProvider {
- return createStripeProvider(new StripeSdk(options.secretKey), options);
+ return createStripeProvider(
+ new StripeSdk(options.secretKey, { apiVersion: "2025-03-31.basil" }),
+ options,
+ );
},
};
},
Stripe Node.js SDK latest stable API version 2025
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe/src/stripe-provider.ts` around lines 932 - 939, The Stripe
client instantiated in stripe(options) currently calls new
StripeSdk(options.secretKey) without an explicit apiVersion; update the
constructor call where StripeSdk is created (in the stripe function that returns
createStripeProvider) to include a pinned apiVersion option matching the target
Stripe Node SDK stable release (e.g., the 2025 stable API version) so responses
remain consistent with the adapter expectations in createStripeProvider and
downstream code that relies on specific invoice/subscription shapes.
Resolve conflicts from main's CLI refactor (shared utils, spinners, update notifications). Replace checkStripe with generic checkProvider that uses adapter.check().
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/paykit/src/cli/commands/status.ts`:
- Around line 91-94: The webhook endpoint rendering currently assumes every
providerResult.webhookEndpoints item is healthy; update the mapping over
providerResult.webhookEndpoints in the status command to inspect ep.status and
render an appropriate message (e.g., "registered" for healthy/active states, and
"unhealthy" or "status: <value>" for non-healthy states), including the actual
ep.status value in the output; apply the same conditional rendering fix to the
other similar block that handles webhook endpoints (the second mapping at the
later location) so non-healthy endpoints are reported correctly instead of
always saying "registered".
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 109-133: checkProvider currently lets exceptions from
providerConfig.createAdapter() or adapter.check?.() bubble up; wrap the adapter
creation and check call in a try/catch inside checkProvider so any thrown error
is converted into a ProviderCheckResult with account.ok = false, account.message
set to the error message (or a generic "Provider check failed" if none), and
webhookEndpoints = null; ensure you reference providerConfig.createAdapter() and
adapter.check?.() in the try block and return the normalized failure object from
the catch handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3bb9b80c-896f-4e9c-8016-4e020f1d5495
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
e2e/cli/status.test.tspackages/paykit/package.jsonpackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/commands/status.tspackages/paykit/src/cli/utils/shared.tspackages/stripe/package.json
💤 Files with no reviewable changes (2)
- packages/paykit/package.json
- e2e/cli/status.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/stripe/package.json
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 130-133: The returned object currently discards provider-supplied
webhookEndpoints when PaymentProvider.check() fails; update the failure branch
in the function that returns { account, webhookEndpoints } so it preserves and
forwards result.webhookEndpoints (if present) instead of always setting
webhookEndpoints: null; locate the call site in
packages/paykit/src/cli/utils/shared.ts where you construct the failure return
(the block that returns account: { ok: false, message: result.error ?? "Provider
check failed" }), and change it to include webhookEndpoints:
result.webhookEndpoints (or result.webhookEndpoints ?? null) so endpoint-level
diagnostics from PaymentProvider.check() are retained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 19fbcbac-a528-4cab-8182-ef8b23d605d3
📒 Files selected for processing (2)
packages/paykit/src/cli/commands/status.tspackages/paykit/src/cli/utils/shared.ts
✅ Files skipped from review due to trivial changes (1)
- packages/paykit/src/cli/commands/status.ts
Comment on lines
+130
to
+133
| return { | ||
| account: { ok: false, message: result.error ?? "Provider check failed" }, | ||
| webhookEndpoints: null, | ||
| }; |
There was a problem hiding this comment.
Preserve webhookEndpoints on failed provider checks.
The PaymentProvider.check() contract in packages/paykit/src/providers/provider.ts allows webhookEndpoints independently of ok, but Lines 130-133 always drop them. That means status/push lose the endpoint-level diagnostics exactly when the provider reports a failing check.
🔧 Proposed fix
return {
account: { ok: false, message: result.error ?? "Provider check failed" },
- webhookEndpoints: null,
+ webhookEndpoints: result.webhookEndpoints ?? null,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/paykit/src/cli/utils/shared.ts` around lines 130 - 133, The returned
object currently discards provider-supplied webhookEndpoints when
PaymentProvider.check() fails; update the failure branch in the function that
returns { account, webhookEndpoints } so it preserves and forwards
result.webhookEndpoints (if present) instead of always setting webhookEndpoints:
null; locate the call site in packages/paykit/src/cli/utils/shared.ts where you
construct the failure return (the block that returns account: { ok: false,
message: result.error ?? "Provider check failed" }), and change it to include
webhookEndpoints: result.webhookEndpoints (or result.webhookEndpoints ?? null)
so endpoint-level diagnostics from PaymentProvider.check() are retained.
This was referenced Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PaymentProviderinterface replacing hardcodedStripeRuntime@paykitjs/stripepackagemaxktz/review-customer-portal-tests(always create provider customer on upsert)Changes
StripeRuntime→PaymentProviderinterface withid/namefieldsStripeProviderConfig→PayKitProviderConfigwithcreateAdapter()factoryctx.stripe.*→ctx.provider.*across all service files@paykitjs/stripeprovider: stripe({ secretKey, webhookSecret })Test plan
Summary by CodeRabbit