Remove Polar and make PayKit Stripe-only#195
Open
maxktz wants to merge 1 commit into
Open
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughRefactors PayKit to a Stripe-only architecture: updates core options/provider, database schema/migrations, services, webhook/API handling, CLI, demo app wiring/UI/env, documentation, e2e harness/tests, and removes Polar and ChangesStripe-only refactor and ecosystem updates
Sequence Diagram(s)sequenceDiagram
participant Client
participant NextRoute(/paykit)
participant PayKit
participant Stripe
Client->>NextRoute(/paykit): HTTP GET/POST
NextRoute(/paykit)->>PayKit: paykit.handler(request)
PayKit->>Stripe: API/webhook interactions
Stripe-->>PayKit: events/responses
PayKit-->>NextRoute(/paykit): response
NextRoute(/paykit)-->>Client: HTTP response
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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 (6)
apps/web/src/components/sections/readme-code-content.ts (1)
28-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStripe wiring update LGTM, but the example callback uses
awaitwithoutasync.Lines 36-38 use
await sendEmail(...)inside a non-async arrow, which is a syntax error for anyone copying this hero snippet.📝 Proposed fix
on: { - "subscription.activated": ({ customer, plan }) => { + "subscription.activated": async ({ customer, plan }) => { await sendEmail(customer.email, "Welcome to Pro!") }, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/sections/readme-code-content.ts` around lines 28 - 40, The event handler for "subscription.activated" in the createPayKit call uses await inside a non-async arrow function causing a syntax error; update the handler for that event (the "subscription.activated" callback used in paykit/createPayKit) to be async (e.g., make the arrow async: async ({ customer, plan }) => { await sendEmail(...) }) or remove the await and return the promise from sendEmail, ensuring sendEmail is invoked correctly.packages/paykit/src/core/create-paykit.ts (1)
68-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAttach a rejection handler to the cached context promise to prevent unhandledRejection.
initContext()can reject (e.g., pending migrations). IncreatePayKit,contextPromise = initContext(options)is created eagerly and passed intogetApi, butwrapMethodsonlyawaits the ctx promise inside each method wrapper (andhandler()/$contextdo the same), so no immediate.catch/await is attached at context creation time. This can allow Node to emitunhandledRejectionbefore any caller invokes a method/handler.Add a rejection handler right after setting
contextPromise(e.g.,contextPromise.catch(() => {}), or equivalent) so the rejection is handled without swallowing the error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/core/create-paykit.ts` around lines 68 - 90, The cached context promise created in createPayKit (contextPromise assigned via initContext in getContext) can reject and cause unhandledRejection because no handler is attached immediately; after you set contextPromise (contextPromise ??= initContext(options)) attach a rejection handler (e.g., contextPromise.catch(() => {})) so the rejection is handled right away while preserving the existing behavior where wrappers (handler/$context/wrapped API methods produced by getApi/wrapMethods) still await the promise; ensure you add this line in the getContext scope right after the assignment to contextPromise.packages/paykit/src/cli/commands/status.ts (1)
80-86:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent section label: error path still says
Provider.Success output (Lines 150, 156) was renamed to
Stripe, but this failure block still printsProvider/Fix provider issues. Align for consistency.Proposed fix
- p.log.error(`Provider\n ${picocolors.red("✖")} ${providerResult.account.message}`); - p.outro("Fix provider issues before continuing"); + p.log.error(`Stripe\n ${picocolors.red("✖")} ${providerResult.account.message}`); + p.outro("Fix Stripe issues before continuing");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/cli/commands/status.ts` around lines 80 - 86, The failure block prints "Provider" and "Fix provider issues" but the success messages were renamed to "Stripe"; update the error section to match by replacing the "Provider" label and the outro text accordingly. Locate the block that checks providerResult.account.ok (uses s.stop(""), p.log.error, p.outro, await database.end(), process.exit) and change the displayed label in p.log.error from "Provider" to "Stripe" and update p.outro to "Fix Stripe issues before continuing" so the success and failure texts are consistent.packages/paykit/src/cli/commands/push.ts (1)
49-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent section label: error path still says
Provider.The success path (Line 96) and config errors now use the
Stripelabel, but this failure block still printsProvider. Align it for consistent CLI output.Proposed fix
- p.log.error(`Provider\n ${picocolors.red("✖")} ${providerResult.account.message}`); + p.log.error(`Stripe\n ${picocolors.red("✖")} ${providerResult.account.message}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/cli/commands/push.ts` around lines 49 - 54, The error block that checks providerResult.account.ok prints the wrong section label "Provider"; update the log to use the "Stripe" label for consistency with the success and config error paths by changing the string passed to p.log.error from `Provider\n ${picocolors.red("✖")} ...` to `Stripe\n ${picocolors.red("✖")} ${providerResult.account.message}`, leaving the surrounding calls (s.stop(""), p.cancel("Push failed"), process.exit(1)) and symbols (providerResult.account.ok, s.stop, p.log.error, picocolors.red, p.cancel, process.exit) unchanged.packages/paykit/src/webhook/webhook.api.ts (1)
17-22:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConfirm whether the legacy
PAYKIT_ALLOW_STALE_SIGNATURESenv var is still intended.The rename from
allowStaleSignatures→allowUnsignedPayloadcarried over the old env var on Line 19 alongside the newPAYKIT_ALLOW_UNSIGNED_PAYLOADS. If this is intentional backward compatibility it should be documented with a short comment; otherwise it's a dangling reference to a removed flag that silently keeps a signature-bypass path alive.🔧 If backward compat is not intended
return ( process.env.PAYKIT_ALLOW_UNSIGNED_PAYLOADS === "1" || - process.env.PAYKIT_ALLOW_STALE_SIGNATURES === "1" || process.env.NODE_ENV === "development" || process.env.NODE_ENV === "test" );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/webhook/webhook.api.ts` around lines 17 - 22, The conditional that checks environment variables in webhook.api.ts still references the legacy PAYKIT_ALLOW_STALE_SIGNATURES alongside PAYKIT_ALLOW_UNSIGNED_PAYLOADS; either remove the legacy reference or explicitly document it: if backward compatibility is intended, add a short inline comment next to the return expression explaining that PAYKIT_ALLOW_STALE_SIGNATURES is kept for legacy support and should be removed in a future major; if not intended, delete the PAYKIT_ALLOW_STALE_SIGNATURES check from the return expression so only PAYKIT_ALLOW_UNSIGNED_PAYLOADS (and NODE_ENV checks) control unsigned-stub behavior.packages/paykit/src/stripe/stripe-provider.ts (1)
943-955:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the signature guard behind
allowUnsignedPayload.
allowUnsignedPayloadis checked only afterPROVIDER_SIGNATURE_MISSINGcan already be thrown, so the new flag never works for unsigned deliveries.packages/paykit/src/cli/commands/listen.tsnow relies on this path for direct tunnel replays.Suggested fix
async handleWebhook(data) { - const headerKey = Object.keys(data.headers).find( - (k) => k.toLowerCase() === "stripe-signature", - ); - const signature = headerKey ? data.headers[headerKey] : undefined; - if (!signature) { - throw PayKitError.from("BAD_REQUEST", PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING); - } - - const event = data.allowUnsignedPayload - ? (JSON.parse(data.body) as StripeSdk.Event) - : await client.webhooks.constructEventAsync(data.body, signature, options.webhookSecret); + const event = data.allowUnsignedPayload + ? (JSON.parse(data.body) as StripeSdk.Event) + : await (async () => { + const headerKey = Object.keys(data.headers).find( + (k) => k.toLowerCase() === "stripe-signature", + ); + const signature = headerKey ? data.headers[headerKey] : undefined; + if (!signature) { + throw PayKitError.from( + "BAD_REQUEST", + PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING, + ); + } + return client.webhooks.constructEventAsync( + data.body, + signature, + options.webhookSecret, + ); + })(); return [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/stripe/stripe-provider.ts` around lines 943 - 955, In handleWebhook, the signature missing guard runs before allowUnsignedPayload is checked so unsigned deliveries are blocked; change the flow in the handleWebhook method to first branch on data.allowUnsignedPayload and, if true, parse JSON from data.body into a StripeSdk.Event, otherwise perform the existing signature lookup (headerKey/signature) and throw the PayKitError with PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING if no signature, then call client.webhooks.constructEventAsync(...) with data.body, signature and options.webhookSecret; keep all existing variable names (headerKey, signature, data.allowUnsignedPayload, PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING, client.webhooks.constructEventAsync) and return the event as before.
🧹 Nitpick comments (3)
e2e/cli/push.test.ts (1)
85-93: 💤 Low valueNit:
providerRowsname is stale.The rows no longer come from a
providerfield; considerproRowsfor clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/cli/push.test.ts` around lines 85 - 93, Rename the stale variable providerRows to proRows in the select block and update its use when extracting the first row into proProduct; specifically update the identifier providerRows in the lines using ctx.database.select(...).from(product)... to proRows and adjust any subsequent references (e.g., the assignment to proProduct) so the new name is used consistently.packages/paykit/src/cli/utils/shared.ts (1)
181-186: ⚖️ Poor tradeoff
checkActiveSubscriptionsOnOtherProvideris now a no-op.It ignores both params and always returns
[], yet callers inpush.ts/status.tsstill computeproviderId = "stripe"and await it as a preflight check. In a Stripe-only world this cross-provider check is dead. Consider removing the function and its call sites in a follow-up to avoid misleading preflight semantics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/cli/utils/shared.ts` around lines 181 - 186, The function checkActiveSubscriptionsOnOtherProvider is a no-op and causes misleading preflight behavior; remove the function and its call sites instead of leaving it returning [] — locate references to checkActiveSubscriptionsOnOtherProvider in push.ts and status.ts (where providerId is computed as "stripe"), delete the await calls and any related handling, and remove the import/export of checkActiveSubscriptionsOnOtherProvider from packages/paykit/src/cli/utils/shared.ts; ensure any leftover variables (e.g., providerId or returned values) are cleaned up or adjusted and run tests to confirm no unused-import/variable errors.packages/paykit/src/cli/commands/init.ts (1)
551-552: Open TODO forpaykitjs listen.The forwarded webhook command is hardcoded to the Stripe CLI with a TODO to swap in
paykitjs listen.Want me to open an issue to track replacing this with
paykitjs listen?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/paykit/src/cli/commands/init.ts` around lines 551 - 552, Replace the hardcoded webhookCommand string with a small helper that prefers the new paykit CLI: implement a getWebhookListenCommand() (and optional detectPaykitCli()) and assign webhookCommand = getWebhookListenCommand(port) so it returns "paykitjs listen --forward-to localhost:3000/paykit/webhook" when the paykit CLI is available and falls back to "stripe listen --forward-to ..." otherwise; update the TODO by removing the hardcoded string and ensuring the helper is referenced from init.ts (webhookCommand) and covered by a simple unit test for both code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/test-utils/harness/stripe.ts`:
- Around line 91-94: The current use of cardPaymentButton.evaluate(...click())
bypasses Playwright actionability checks; keep the existing count() conditional
but replace the evaluate call with an actionable locator click: use
cardPaymentButton.first().waitFor({ state: "visible" }) (or waitFor({ state:
"enabled" }) as appropriate) and then await cardPaymentButton.first().click() so
the click uses Playwright's built-in waits and retries; update the code in the
block referencing the cardPaymentButton locator accordingly.
In `@packages/paykit/src/cli/commands/listen.ts`:
- Around line 94-106: The function getEnvStripeOptions currently returns a fake
"whsec_placeholder" when no webhook secret env var exists; change it to
explicitly represent "unknown" by returning webhookSecret as undefined (and
update the function signature to allow webhookSecret?: string | undefined)
instead of synthesizing a value, and then ensure callers (e.g.,
ensureTunnelWebhook in stripe-provider.ts and any code that prints
options.webhookSecret) treat undefined as "secret unknown" and avoid
echoing/saving a placeholder.
- Around line 682-692: The current catch around getPayKitConfig() treats every
error as "no config" and falls back to envs; change it to only fall back when
the error is a genuine "config not found" case and rethrow all other errors
(syntax/validation/missing env) so they surface. Implement a check (e.g., add or
use an isConfigNotFound(error) helper or test error.name/message/code) inside
the catch for getPayKitConfig(...) — if isConfigNotFound(error) is true then
call loadDotEnv(params.cwd) and stripeOptions = getEnvStripeOptions(), otherwise
throw the error (respecting params.configPath || params.requireConfig logic).
Ensure references: getPayKitConfig, params.configPath, params.requireConfig,
loadDotEnv, getEnvStripeOptions, basePath, stripeOptions.
In `@packages/paykit/src/database/migrations/0001_stripe_only_schema.sql`:
- Around line 65-70: The migration adds a new column paykit_payment_method.brand
but never backfills it from the existing JSON provider_data, so dropping
provider_data will lose historical brand values; update the migration (around
ALTER TABLE "paykit_payment_method" ADD COLUMN "brand" text and the related
blocks at the other ranges) to run an UPDATE that sets brand =
provider_data->>'brand' for all rows (and ensure NULL-safe handling), verify the
backfill completes before any DROP COLUMN "provider_data" statements, and apply
the same backfill logic for the other affected migration sections (lines
referenced 89-104 and 147-148) so historical brand data is preserved.
- Around line 55-60: The migration currently adds stripe_frozen_time as type
timestamp which drops the Z/UTC offset; change the ALTER TABLE that adds
stripe_frozen_time on paykit_customer to use timestamptz instead of timestamp,
and update any related backfill/CAST logic (the backfill that converts
ISO-8601/Z values into stripe_frozen_time) to cast to timestamptz so the
original instant is preserved when reading/writing from node-postgres; ensure
the column name stripe_frozen_time and the ALTER TABLE statement are the only
places modified.
In `@packages/paykit/src/payment-method/payment-method.service.ts`:
- Around line 49-54: The current lookup in
database.query.paymentMethod.findFirst excludes soft-deleted rows
(isNull(paymentMethod.deletedAt)), so after deletePaymentMethodByProviderId a
later upsert creates a duplicate; change the upsert flow to include soft-deleted
rows by removing the isNull filter (or explicitly searching both deleted and
non-deleted) when querying by stripePaymentMethodId
(paymentMethod.stripePaymentMethodId) and if a row is found with deletedAt set,
perform an update to clear deletedAt and refresh the record fields instead of
inserting a new row; keep the existing update path that clears deletedAt intact
and ensure deletePaymentMethodByProviderId still sets deletedAt only.
---
Outside diff comments:
In `@apps/web/src/components/sections/readme-code-content.ts`:
- Around line 28-40: The event handler for "subscription.activated" in the
createPayKit call uses await inside a non-async arrow function causing a syntax
error; update the handler for that event (the "subscription.activated" callback
used in paykit/createPayKit) to be async (e.g., make the arrow async: async ({
customer, plan }) => { await sendEmail(...) }) or remove the await and return
the promise from sendEmail, ensuring sendEmail is invoked correctly.
In `@packages/paykit/src/cli/commands/push.ts`:
- Around line 49-54: The error block that checks providerResult.account.ok
prints the wrong section label "Provider"; update the log to use the "Stripe"
label for consistency with the success and config error paths by changing the
string passed to p.log.error from `Provider\n ${picocolors.red("✖")} ...` to
`Stripe\n ${picocolors.red("✖")} ${providerResult.account.message}`, leaving
the surrounding calls (s.stop(""), p.cancel("Push failed"), process.exit(1)) and
symbols (providerResult.account.ok, s.stop, p.log.error, picocolors.red,
p.cancel, process.exit) unchanged.
In `@packages/paykit/src/cli/commands/status.ts`:
- Around line 80-86: The failure block prints "Provider" and "Fix provider
issues" but the success messages were renamed to "Stripe"; update the error
section to match by replacing the "Provider" label and the outro text
accordingly. Locate the block that checks providerResult.account.ok (uses
s.stop(""), p.log.error, p.outro, await database.end(), process.exit) and change
the displayed label in p.log.error from "Provider" to "Stripe" and update
p.outro to "Fix Stripe issues before continuing" so the success and failure
texts are consistent.
In `@packages/paykit/src/core/create-paykit.ts`:
- Around line 68-90: The cached context promise created in createPayKit
(contextPromise assigned via initContext in getContext) can reject and cause
unhandledRejection because no handler is attached immediately; after you set
contextPromise (contextPromise ??= initContext(options)) attach a rejection
handler (e.g., contextPromise.catch(() => {})) so the rejection is handled right
away while preserving the existing behavior where wrappers
(handler/$context/wrapped API methods produced by getApi/wrapMethods) still
await the promise; ensure you add this line in the getContext scope right after
the assignment to contextPromise.
In `@packages/paykit/src/stripe/stripe-provider.ts`:
- Around line 943-955: In handleWebhook, the signature missing guard runs before
allowUnsignedPayload is checked so unsigned deliveries are blocked; change the
flow in the handleWebhook method to first branch on data.allowUnsignedPayload
and, if true, parse JSON from data.body into a StripeSdk.Event, otherwise
perform the existing signature lookup (headerKey/signature) and throw the
PayKitError with PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING if no signature,
then call client.webhooks.constructEventAsync(...) with data.body, signature and
options.webhookSecret; keep all existing variable names (headerKey, signature,
data.allowUnsignedPayload, PAYKIT_ERROR_CODES.PROVIDER_SIGNATURE_MISSING,
client.webhooks.constructEventAsync) and return the event as before.
In `@packages/paykit/src/webhook/webhook.api.ts`:
- Around line 17-22: The conditional that checks environment variables in
webhook.api.ts still references the legacy PAYKIT_ALLOW_STALE_SIGNATURES
alongside PAYKIT_ALLOW_UNSIGNED_PAYLOADS; either remove the legacy reference or
explicitly document it: if backward compatibility is intended, add a short
inline comment next to the return expression explaining that
PAYKIT_ALLOW_STALE_SIGNATURES is kept for legacy support and should be removed
in a future major; if not intended, delete the PAYKIT_ALLOW_STALE_SIGNATURES
check from the return expression so only PAYKIT_ALLOW_UNSIGNED_PAYLOADS (and
NODE_ENV checks) control unsigned-stub behavior.
---
Nitpick comments:
In `@e2e/cli/push.test.ts`:
- Around line 85-93: Rename the stale variable providerRows to proRows in the
select block and update its use when extracting the first row into proProduct;
specifically update the identifier providerRows in the lines using
ctx.database.select(...).from(product)... to proRows and adjust any subsequent
references (e.g., the assignment to proProduct) so the new name is used
consistently.
In `@packages/paykit/src/cli/commands/init.ts`:
- Around line 551-552: Replace the hardcoded webhookCommand string with a small
helper that prefers the new paykit CLI: implement a getWebhookListenCommand()
(and optional detectPaykitCli()) and assign webhookCommand =
getWebhookListenCommand(port) so it returns "paykitjs listen --forward-to
localhost:3000/paykit/webhook" when the paykit CLI is available and falls back
to "stripe listen --forward-to ..." otherwise; update the TODO by removing the
hardcoded string and ensuring the helper is referenced from init.ts
(webhookCommand) and covered by a simple unit test for both code paths.
In `@packages/paykit/src/cli/utils/shared.ts`:
- Around line 181-186: The function checkActiveSubscriptionsOnOtherProvider is a
no-op and causes misleading preflight behavior; remove the function and its call
sites instead of leaving it returning [] — locate references to
checkActiveSubscriptionsOnOtherProvider in push.ts and status.ts (where
providerId is computed as "stripe"), delete the await calls and any related
handling, and remove the import/export of
checkActiveSubscriptionsOnOtherProvider from
packages/paykit/src/cli/utils/shared.ts; ensure any leftover variables (e.g.,
providerId or returned values) are cleaned up or adjusted and run tests to
confirm no unused-import/variable errors.
🪄 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: 1059bfa6-d80b-4f37-9f95-137993970930
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (103)
README.mdapps/demo/drizzle.config.tsapps/demo/next.config.jsapps/demo/package.jsonapps/demo/paykit.config.tsapps/demo/paykit.polar.config.tsapps/demo/paykit.stripe.config.tsapps/demo/scripts/push-sandbox.tsapps/demo/scripts/sandbox.tsapps/demo/src/app/_components/checkout-page-content.tsxapps/demo/src/app/_components/features-panel.tsxapps/demo/src/app/_components/subscribe-panel.tsxapps/demo/src/app/paykit-polar/[[...slug]]/route.tsapps/demo/src/app/paykit-stripe/[[...slug]]/route.tsapps/demo/src/app/paykit/[[...slug]]/route.tsapps/demo/src/env.jsapps/demo/src/lib/paykit-client.tsapps/demo/src/lib/paykit-scenarios.tsapps/demo/src/lib/paykit.tsapps/demo/src/lib/paykit/polar.tsapps/demo/src/lib/paykit/stripe.tsapps/demo/src/lib/scenario-config.tsapps/demo/src/server/api/root.tsapps/demo/src/server/api/routers/paykit-route.tsapps/demo/src/server/db.tsapps/web/content/docs/concepts/cli.mdxapps/web/content/docs/concepts/payment-providers.mdxapps/web/content/docs/get-started/index.mdxapps/web/content/docs/get-started/installation.mdxapps/web/content/docs/providers/creem.mdxapps/web/content/docs/providers/lemonsqueezy.mdxapps/web/content/docs/providers/meta.jsonapps/web/content/docs/providers/paddle.mdxapps/web/content/docs/providers/paypal.mdxapps/web/content/docs/providers/polar.mdxapps/web/content/docs/providers/stripe.mdxapps/web/src/components/docs/docs-icons.tsxapps/web/src/components/docs/features.tsxapps/web/src/components/sections/demo/demo-types.tsxapps/web/src/components/sections/features-section.tsxapps/web/src/components/sections/readme-code-content.tse2e/cli/init.test.tse2e/cli/push.test.tse2e/cli/setup.tse2e/cli/status.test.tse2e/core/subscribe/cancel-end-of-cycle.test.tse2e/core/subscribe/renewal.test.tse2e/core/webhook/duplicate-webhook.test.tse2e/core/webhook/subscription-deleted.test.tse2e/package.jsone2e/test-utils/env.tse2e/test-utils/harness/index.tse2e/test-utils/harness/polar.tse2e/test-utils/harness/stripe.tse2e/test-utils/harness/types.tse2e/test-utils/setup.tse2e/vitest.config.tspackages/paykit/package.jsonpackages/paykit/src/api/__tests__/define-route.test.tspackages/paykit/src/api/__tests__/methods.test.tspackages/paykit/src/api/methods.tspackages/paykit/src/cli/commands/init.tspackages/paykit/src/cli/commands/listen.tspackages/paykit/src/cli/commands/push.tspackages/paykit/src/cli/commands/status.tspackages/paykit/src/cli/utils/get-config.tspackages/paykit/src/cli/utils/shared.tspackages/paykit/src/core/__tests__/context.test.tspackages/paykit/src/core/__tests__/create-paykit.test.tspackages/paykit/src/core/context.tspackages/paykit/src/core/create-paykit.tspackages/paykit/src/customer/__tests__/customer.service.test.tspackages/paykit/src/customer/customer.service.tspackages/paykit/src/database/migrations/0001_stripe_only_schema.sqlpackages/paykit/src/database/migrations/meta/0001_snapshot.jsonpackages/paykit/src/database/migrations/meta/_journal.jsonpackages/paykit/src/database/schema.tspackages/paykit/src/index.tspackages/paykit/src/invoice/invoice.service.tspackages/paykit/src/payment-method/payment-method.service.tspackages/paykit/src/payment/payment.service.tspackages/paykit/src/product/product.service.tspackages/paykit/src/providers/provider.tspackages/paykit/src/stripe/stripe-provider.tspackages/paykit/src/subscription/subscription.service.tspackages/paykit/src/testing/testing.service.tspackages/paykit/src/types/instance.tspackages/paykit/src/types/options.tspackages/paykit/src/utilities/dependencies/paykit-package-list.tspackages/paykit/src/webhook/webhook.api.tspackages/paykit/src/webhook/webhook.service.tspackages/polar/package.jsonpackages/polar/src/index.tspackages/polar/src/polar-provider.tspackages/polar/tsconfig.jsonpackages/polar/tsdown.config.tspackages/stripe/package.jsonpackages/stripe/src/__tests__/stripe-provider.test.tspackages/stripe/src/__tests__/stripe.test.tspackages/stripe/src/index.tspackages/stripe/tsconfig.jsonpackages/stripe/tsdown.config.tsscripts/publish-dist.mjs
💤 Files with no reviewable changes (27)
- apps/demo/src/lib/paykit/stripe.ts
- packages/stripe/tsconfig.json
- packages/stripe/src/tests/stripe.test.ts
- apps/web/content/docs/providers/lemonsqueezy.mdx
- apps/web/content/docs/providers/creem.mdx
- apps/web/content/docs/providers/paypal.mdx
- packages/stripe/src/index.ts
- apps/demo/src/lib/paykit/polar.ts
- apps/web/content/docs/providers/polar.mdx
- apps/demo/paykit.stripe.config.ts
- apps/demo/src/lib/paykit-scenarios.ts
- packages/stripe/package.json
- packages/polar/tsconfig.json
- apps/web/content/docs/providers/paddle.mdx
- e2e/test-utils/harness/polar.ts
- packages/polar/tsdown.config.ts
- packages/stripe/tsdown.config.ts
- packages/paykit/src/testing/testing.service.ts
- packages/stripe/src/tests/stripe-provider.test.ts
- packages/polar/src/polar-provider.ts
- e2e/package.json
- apps/demo/src/app/paykit-stripe/[[...slug]]/route.ts
- apps/demo/src/app/paykit-polar/[[...slug]]/route.ts
- packages/polar/package.json
- packages/polar/src/index.ts
- apps/demo/paykit.polar.config.ts
- apps/demo/src/lib/scenario-config.ts
Comment on lines
+91
to
+94
| const cardPaymentButton = page.locator('[data-testid="card-accordion-item-button"]'); | ||
| if ((await cardPaymentButton.count()) > 0) { | ||
| await cardPaymentButton.evaluate((el) => (el as HTMLElement).click()); | ||
| } |
There was a problem hiding this comment.
Use locator-based clicks (with visible/enabled waits) for the Stripe card accordion control
In e2e/test-utils/harness/stripe.ts (lines 91-94), cardPaymentButton.evaluate(el => (el as HTMLElement).click()) bypasses Playwright’s actionability checks/retry. Keep the optional count()/conditional logic, but switch to await cardPaymentButton.first().waitFor({ state: "visible" }) + await cardPaymentButton.first().click() (or an equivalent enabled/ready wait) to avoid no-op clicks and downstream flakiness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/test-utils/harness/stripe.ts` around lines 91 - 94, The current use of
cardPaymentButton.evaluate(...click()) bypasses Playwright actionability checks;
keep the existing count() conditional but replace the evaluate call with an
actionable locator click: use cardPaymentButton.first().waitFor({ state:
"visible" }) (or waitFor({ state: "enabled" }) as appropriate) and then await
cardPaymentButton.first().click() so the click uses Playwright's built-in waits
and retries; update the code in the block referencing the cardPaymentButton
locator accordingly.
Comment on lines
+94
to
+106
| function getEnvStripeOptions(): { secretKey: string; webhookSecret: string } { | ||
| const secretKey = process.env.E2E_STRIPE_SK ?? process.env.STRIPE_SECRET_KEY; | ||
| if (!secretKey) { | ||
| throw new Error( | ||
| "No PayKit config found and no Stripe secret key found in env. Set E2E_STRIPE_SK or STRIPE_SECRET_KEY, or pass --config.", | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| secretKey, | ||
| webhookSecret: | ||
| process.env.E2E_STRIPE_WHSEC ?? process.env.STRIPE_WEBHOOK_SECRET ?? "whsec_placeholder", | ||
| }; |
There was a problem hiding this comment.
Avoid inventing a webhook secret.
When STRIPE_WEBHOOK_SECRET is absent this returns "whsec_placeholder", and packages/paykit/src/stripe/stripe-provider.ts echoes options.webhookSecret back from ensureTunnelWebhook() when it updates an existing endpoint. In paykitjs listen --forward-to, that placeholder can then be printed as if it were the real signing secret, which nudges users to save an invalid value and breaks later direct delivery. Handle "secret unknown" explicitly instead of synthesizing one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/cli/commands/listen.ts` around lines 94 - 106, The
function getEnvStripeOptions currently returns a fake "whsec_placeholder" when
no webhook secret env var exists; change it to explicitly represent "unknown" by
returning webhookSecret as undefined (and update the function signature to allow
webhookSecret?: string | undefined) instead of synthesizing a value, and then
ensure callers (e.g., ensureTunnelWebhook in stripe-provider.ts and any code
that prints options.webhookSecret) treat undefined as "secret unknown" and avoid
echoing/saving a placeholder.
Comment on lines
+682
to
+692
| try { | ||
| config = await getPayKitConfig({ configPath: params.configPath, cwd: params.cwd }); | ||
| basePath = config.options.basePath ?? basePath; | ||
| stripeOptions = config.options.stripe; | ||
| } catch (error) { | ||
| if (params.configPath || params.requireConfig) { | ||
| throw error; | ||
| } | ||
| loadDotEnv(params.cwd); | ||
| stripeOptions = getEnvStripeOptions(); | ||
| } |
There was a problem hiding this comment.
Don't downgrade config load failures to env fallback.
This catches every getPayKitConfig() failure when --forward-to is used. A config with missing env vars, syntax errors, or validation errors now gets treated like "no config", which falls back to the default base path and env Stripe credentials and can wire the tunnel to the wrong local route or Stripe account. Only fall back on a genuine "config not found" case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/cli/commands/listen.ts` around lines 682 - 692, The
current catch around getPayKitConfig() treats every error as "no config" and
falls back to envs; change it to only fall back when the error is a genuine
"config not found" case and rethrow all other errors (syntax/validation/missing
env) so they surface. Implement a check (e.g., add or use an
isConfigNotFound(error) helper or test error.name/message/code) inside the catch
for getPayKitConfig(...) — if isConfigNotFound(error) is true then call
loadDotEnv(params.cwd) and stripeOptions = getEnvStripeOptions(), otherwise
throw the error (respecting params.configPath || params.requireConfig logic).
Ensure references: getPayKitConfig, params.configPath, params.requireConfig,
loadDotEnv, getEnvStripeOptions, basePath, stripeOptions.
Comment on lines
+55
to
+60
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_customer_id" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_test_clock_id" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_frozen_time" timestamp;--> statement-breakpoint | ||
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_synced_email" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_synced_name" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_customer" ADD COLUMN "stripe_synced_metadata" jsonb;--> statement-breakpoint |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PostgreSQL, what happens when an ISO 8601 string with a Zsuffix is cast totimestampversustimestamptz, and how are timestamp without time zonevalues typically deserialized into JavaScriptDate objects by Node/Postgres drivers?
💡 Result:
In PostgreSQL, casting an ISO 8601 string with a Z suffix (indicating UTC) to different timestamp types results in significant differences in how that value is processed and stored [1]: 1. Casting to timestamp (or timestamp without time zone): PostgreSQL ignores the Z suffix and any other time zone indicators [1]. It treats the input purely as calendar and clock fields, storing the values exactly as they appear in the string [1][2]. Consequently, no time zone adjustment occurs, and the UTC context is effectively discarded [1]. 2. Casting to timestamptz (or timestamp with time zone): PostgreSQL interprets the input as having a specific time zone [1]. Because the Z suffix explicitly denotes UTC, PostgreSQL converts the input time to UTC (if it is not already) and stores it as a UTC value [1]. Note that the original time zone is not retained in the stored data; the value is simply normalized to UTC for storage and, upon retrieval, adjusted to the session's current TimeZone setting for display [1][2]. Regarding deserialization in Node.js via drivers like node-postgres (pg): - By default, common drivers such as node-postgres automatically parse timestamp and timestamptz columns into JavaScript Date objects [3]. - For timestamp without time zone values, these drivers often interpret the string as being in the local time of the Node.js process (derived from process.env.TZ or the system clock) rather than UTC [3][4][5]. This can lead to discrepancies if your database stores UTC values in timestamp columns but your Node.js server runs in a different time zone [6][4][5]. - To avoid these issues, many developers override the default type parser for the timestamp type (OID 1114) to explicitly treat the incoming string as UTC [7][8]. A common approach is to append "+0000" to the raw string before passing it to the Date constructor, or by using Date.UTC to manually construct the date object, ensuring the absolute time value remains correct regardless of the local server environment [6][7]. Using timestamptz columns is generally recommended to mitigate these client-side deserialization ambiguities, as the driver handles them more predictably as UTC [2][3][5].
Citations:
- 1: https://www.postgresql.org/docs/current/datatype-datetime.html
- 2: https://boringsql.com/posts/know-the-time-in-postgresql/
- 3: https://github.com/brianc/node-postgres/blob/master/docs/pages/features/types.mdx
- 4: Wrong timestamp to JavaScript date conversion brianc/node-postgres#993
- 5: https://ellenaua.medium.com/difference-between-date-timestamp-with-timezone-and-timestamp-without-timezone-in-f138d9735db6
- 6: parseInputDatesAsUTC is not documented brianc/node-postgres#2141
- 7: https://stackoverflow.com/questions/20712291/use-node-postgres-to-get-postgres-timestamp-without-timezone-in-utc
- 8: Provide option to interpret dates as UTC brianc/node-postgres#783
Store stripe_frozen_time as timestamptz to preserve the Stripe instant.
Casting an ISO 8601 ...Z value to PostgreSQL timestamp (timestamp without time zone) ignores the Z, and node-postgres typically parses such values into JS Date as local/naive time—so round-tripping (e.g., via toISOString()) can shift the instant. Store as timestamptz and cast accordingly in the backfill.
💡 Suggested fix
-ALTER TABLE "paykit_customer" ADD COLUMN "stripe_frozen_time" timestamp;--> statement-breakpoint
+ALTER TABLE "paykit_customer" ADD COLUMN "stripe_frozen_time" timestamptz;--> statement-breakpoint
@@
- WHEN "provider"->'stripe'->>'frozenTime' ~ '^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}'
- THEN ("provider"->'stripe'->>'frozenTime')::timestamp
+ WHEN "provider"->'stripe'->>'frozenTime' ~ '^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}'
+ THEN ("provider"->'stripe'->>'frozenTime')::timestamptz🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/database/migrations/0001_stripe_only_schema.sql` around
lines 55 - 60, The migration currently adds stripe_frozen_time as type timestamp
which drops the Z/UTC offset; change the ALTER TABLE that adds
stripe_frozen_time on paykit_customer to use timestamptz instead of timestamp,
and update any related backfill/CAST logic (the backfill that converts
ISO-8601/Z values into stripe_frozen_time) to cast to timestamptz so the
original instant is preserved when reading/writing from node-postgres; ensure
the column name stripe_frozen_time and the ALTER TABLE statement are the only
places modified.
Comment on lines
+65
to
+70
| ALTER TABLE "paykit_payment_method" ADD COLUMN "stripe_payment_method_id" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_payment_method" ADD COLUMN "type" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_payment_method" ADD COLUMN "brand" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_payment_method" ADD COLUMN "last4" text;--> statement-breakpoint | ||
| ALTER TABLE "paykit_payment_method" ADD COLUMN "expiry_month" integer;--> statement-breakpoint | ||
| ALTER TABLE "paykit_payment_method" ADD COLUMN "expiry_year" integer;--> statement-breakpoint |
There was a problem hiding this comment.
Backfill brand before dropping provider_data.
brand is added here, but the backfill never copies provider_data->>'brand'. Once provider_data is dropped, every historical payment method loses its stored brand permanently.
💡 Suggested fix
UPDATE "paykit_payment_method"
SET
"stripe_payment_method_id" = "provider_data"->>'methodId',
"type" = "provider_data"->>'type',
+ "brand" = "provider_data"->>'brand',
"last4" = "provider_data"->>'last4',
"expiry_month" = CASEAlso applies to: 89-104, 147-148
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/database/migrations/0001_stripe_only_schema.sql` around
lines 65 - 70, The migration adds a new column paykit_payment_method.brand but
never backfills it from the existing JSON provider_data, so dropping
provider_data will lose historical brand values; update the migration (around
ALTER TABLE "paykit_payment_method" ADD COLUMN "brand" text and the related
blocks at the other ranges) to run an UPDATE that sets brand =
provider_data->>'brand' for all rows (and ensure NULL-safe handling), verify the
backfill completes before any DROP COLUMN "provider_data" statements, and apply
the same backfill logic for the other affected migration sections (lines
referenced 89-104 and 147-148) so historical brand data is preserved.
Comment on lines
49
to
54
| const existingRow = await database.query.paymentMethod.findFirst({ | ||
| where: and( | ||
| eq(paymentMethod.providerId, input.providerId), | ||
| sql`${paymentMethod.providerData}->>'methodId' = ${input.paymentMethod.providerMethodId}`, | ||
| eq(paymentMethod.stripePaymentMethodId, input.paymentMethod.providerMethodId), | ||
| isNull(paymentMethod.deletedAt), | ||
| ), | ||
| }); |
There was a problem hiding this comment.
Resurrect soft-deleted payment methods instead of inserting duplicates.
The update path clears deletedAt, but this lookup excludes deleted rows. After deletePaymentMethodByProviderId, a later upsert for the same Stripe payment method will insert a second record instead of reviving the original one, which breaks idempotency for detach/reattach or out-of-order webhook flows.
💡 Suggested fix
const existingRow = await database.query.paymentMethod.findFirst({
- where: and(
- eq(paymentMethod.stripePaymentMethodId, input.paymentMethod.providerMethodId),
- isNull(paymentMethod.deletedAt),
- ),
+ where: eq(paymentMethod.stripePaymentMethodId, input.paymentMethod.providerMethodId),
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/paykit/src/payment-method/payment-method.service.ts` around lines 49
- 54, The current lookup in database.query.paymentMethod.findFirst excludes
soft-deleted rows (isNull(paymentMethod.deletedAt)), so after
deletePaymentMethodByProviderId a later upsert creates a duplicate; change the
upsert flow to include soft-deleted rows by removing the isNull filter (or
explicitly searching both deleted and non-deleted) when querying by
stripePaymentMethodId (paymentMethod.stripePaymentMethodId) and if a row is
found with deletedAt set, perform an update to clear deletedAt and refresh the
record fields instead of inserting a new row; keep the existing update path that
clears deletedAt intact and ensure deletePaymentMethodByProviderId still sets
deletedAt only.
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
Verification
Summary by CodeRabbit
New Features
stripeoption increatePayKit({ stripe: { secretKey, webhookSecret } }), eliminating the need for separate provider packages.Refactor
Documentation
Chores
@paykitjs/stripeand@paykitjs/polarpackages; Stripe support is now integrated into core.