codex: address PR review feedback (#199)

Author: maxktz

packages/paykit/src/core/__tests__/validate-options.test.ts

@@ -7,7 +7,7 @@ function createOptions(currency?: string): PayKitOptions {
   return {
     database: "postgresql://localhost:5432/paykit",
     stripe: {
-      ...(currency ? { currency: currency as never } : {}),
+      ...(currency !== undefined ? { currency: currency as never } : {}),
       secretKey: "sk_test_123",
       webhookSecret: "whsec_123",
     },
@@ -31,4 +31,10 @@ describe("core/validate-options", () => {
       "must be a lowercase three-letter currency code",
     );
   });
+
+  it("rejects empty Stripe currency", () => {
+    expect(() => assertValidPayKitOptions(createOptions(""))).toThrow(
+      "must be a lowercase three-letter currency code",
+    );
+  });
 });

packages/paykit/src/core/validate-options.ts

@@ -30,15 +30,21 @@ export function assertValidPayKitOptions(
     assertValidTrustedOrigin(origin);
   }
 
-  if (options.stripe?.currency) {
-    assertValidStripeCurrency(options.stripe.currency);
+  const currency = options.stripe?.currency;
+  if (currency !== undefined) {
+    assertValidStripeCurrency(currency);
   }
 }
 
-function assertValidStripeCurrency(currency: string): void {
-  if (currency !== currency.toLowerCase() || !/^[a-z]{3}$/.test(currency)) {
+function assertValidStripeCurrency(currency: unknown): void {
+  if (
+    typeof currency !== "string" ||
+    currency !== currency.toLowerCase() ||
+    !/^[a-z]{3}$/.test(currency)
+  ) {
+    const received = typeof currency === "string" ? currency : String(currency);
     throw new Error(
-      `PayKit option \`stripe.currency\` must be a lowercase three-letter currency code. Received "${currency}".`,
+      `PayKit option \`stripe.currency\` must be a lowercase three-letter currency code. Received "${received}".`,
     );
   }
 

packages/paykit/src/customer/customer.service.ts

@@ -11,7 +11,7 @@ import {
   product,
   subscription,
 } from "../database/schema";
-import { getProductByHash } from "../product/product.service";
+import { getProductByPlan } from "../product/product.service";
 import type { ProviderCustomer } from "../providers/provider";
 import {
   getActiveSubscriptionInGroup,
@@ -160,7 +160,7 @@ export async function ensureDefaultPlansForCustomer(
       continue;
     }
 
-    const storedPlan = await getProductByHash(ctx.database, defaultPlan.id, defaultPlan.hash);
+    const storedPlan = await getProductByPlan(ctx.database, defaultPlan);
     if (!storedPlan) {
       continue;
     }

packages/paykit/src/product/__tests__/product.service.test.ts

@@ -0,0 +1,79 @@
+import { describe, expect, it, vi } from "vitest";
+
+import type { PayKitDatabase } from "../../database";
+import type { StoredProduct } from "../../types/models";
+import type { NormalizedPlan } from "../../types/schema";
+import { getProductByPlan } from "../product.service";
+
+const now = new Date("2026-01-01T00:00:00Z");
+
+function createStoredProduct(overrides: Partial<StoredProduct> = {}): StoredProduct {
+  return {
+    createdAt: now,
+    group: "base",
+    hash: "old_hash",
+    id: "pro",
+    internalId: "prod_123",
+    isDefault: false,
+    name: "Pro",
+    priceAmount: 2900,
+    priceCurrency: "usd",
+    priceInterval: "month",
+    stripePriceId: "price_123",
+    stripeProductId: "prod_stripe_123",
+    updatedAt: now,
+    version: 1,
+    ...overrides,
+  };
+}
+
+function createPlan(overrides: Partial<NormalizedPlan> = {}): NormalizedPlan {
+  return {
+    group: "base",
+    hash: "new_hash",
+    id: "pro",
+    includes: [],
+    isDefault: false,
+    name: "Pro",
+    priceAmount: 2900,
+    priceCurrency: "usd",
+    priceInterval: "month",
+    trialDays: null,
+    ...overrides,
+  };
+}
+
+function createDatabase(storedProduct: StoredProduct) {
+  const productFindFirst = vi.fn().mockResolvedValueOnce(null).mockResolvedValueOnce(storedProduct);
+  const productFeatureFindMany = vi.fn().mockResolvedValue([]);
+
+  return {
+    database: {
+      query: {
+        product: {
+          findFirst: productFindFirst,
+        },
+        productFeature: {
+          findMany: productFeatureFindMany,
+        },
+      },
+    } as unknown as PayKitDatabase,
+    productFeatureFindMany,
+    productFindFirst,
+  };
+}
+
+describe("product.service", () => {
+  it("falls back to a semantically matching product when only the hash changed", async () => {
+    const storedProduct = createStoredProduct();
+    const { database } = createDatabase(storedProduct);
+
+    await expect(getProductByPlan(database, createPlan())).resolves.toEqual(storedProduct);
+  });
+
+  it("does not fall back when the stored product differs from the normalized plan", async () => {
+    const { database } = createDatabase(createStoredProduct({ priceAmount: 3900 }));
+
+    await expect(getProductByPlan(database, createPlan())).resolves.toBeNull();
+  });
+});

packages/paykit/src/product/product.service.ts

@@ -5,7 +5,7 @@ import { generateId } from "../core/utils";
 import type { PayKitDatabase } from "../database";
 import { feature, product, productFeature } from "../database/schema";
 import type { StoredFeature, StoredProduct, StoredProductFeature } from "../types/models";
-import type { NormalizedFeature, NormalizedPlanFeature } from "../types/schema";
+import type { NormalizedFeature, NormalizedPlan, NormalizedPlanFeature } from "../types/schema";
 
 export interface StoredProductSnapshot {
   features: readonly StoredProductFeature[];
@@ -109,6 +109,62 @@ export async function getProductByHash(
   return result ?? null;
 }
 
+function serializeFeatureConfig(config: Record<string, unknown> | null): string {
+  return JSON.stringify(config ?? null);
+}
+
+function productFeaturesMatch(
+  existing: readonly StoredProductFeature[],
+  next: readonly NormalizedPlanFeature[],
+): boolean {
+  if (existing.length !== next.length) {
+    return false;
+  }
+
+  return existing.every((storedFeature, index) => {
+    const nextFeature = next[index];
+    return (
+      nextFeature != null &&
+      storedFeature.featureId === nextFeature.id &&
+      storedFeature.limit === nextFeature.limit &&
+      storedFeature.resetInterval === nextFeature.resetInterval &&
+      serializeFeatureConfig(storedFeature.config) === serializeFeatureConfig(nextFeature.config)
+    );
+  });
+}
+
+function productSnapshotMatchesPlan(
+  snapshot: StoredProductSnapshot,
+  plan: NormalizedPlan,
+): boolean {
+  return (
+    snapshot.product.group === plan.group &&
+    snapshot.product.isDefault === plan.isDefault &&
+    (snapshot.product.priceAmount ?? null) === plan.priceAmount &&
+    (snapshot.product.priceCurrency ?? null) === plan.priceCurrency &&
+    (snapshot.product.priceInterval ?? null) === plan.priceInterval &&
+    productFeaturesMatch(snapshot.features, plan.includes)
+  );
+}
+
+/** Finds the stored product for a normalized plan, including old hash-compatible rows. */
+export async function getProductByPlan(
+  database: PayKitDatabase,
+  plan: NormalizedPlan,
+): Promise<StoredProduct | null> {
+  const exact = await getProductByHash(database, plan.id, plan.hash);
+  if (exact) {
+    return exact;
+  }
+
+  const latest = await getLatestProductSnapshot(database, plan.id);
+  if (!latest || !productSnapshotMatchesPlan(latest, plan)) {
+    return null;
+  }
+
+  return latest.product;
+}
+
 export async function getProductByInternalId(
   database: PayKitDatabase,
   internalId: string,

packages/paykit/src/subscription/subscription.service.ts

@@ -13,7 +13,7 @@ import { upsertInvoiceRecord } from "../invoice/invoice.service";
 import { getDefaultPaymentMethod } from "../payment-method/payment-method.service";
 import {
   getDefaultProductInGroup,
-  getProductByHash,
+  getProductByPlan,
   getProductByInternalId,
   getProductByProviderData,
   getProductFeatures,
@@ -103,7 +103,7 @@ export async function loadSubscribeContext(ctx: PayKitContext, input: SubscribeI
   const matchingProduct = input.productInternalId
     ? await getProductByInternalId(ctx.database, input.productInternalId)
     : normalizedPlan
-      ? await getProductByHash(ctx.database, input.planId, normalizedPlan.hash)
+      ? await getProductByPlan(ctx.database, normalizedPlan)
       : null;
   const storedPlan = matchingProduct ? withProviderInfo(matchingProduct, providerId) : null;
 
@@ -245,11 +245,11 @@ async function cancelExistingProviderSubscriptionForCheckout(
     return;
   }
 
-  if (!completion.subCtx.isUpgrade) {
+  if (!completion.subCtx.isUpgrade && !completion.subCtx.isPaidCurrencyChange) {
     throw PayKitError.from(
       "BAD_REQUEST",
       PAYKIT_ERROR_CODES.PROVIDER_WEBHOOK_INVALID,
-      `Checkout completion is only valid for new paid subscriptions or upgrades to "${completion.subCtx.storedPlan.id}"`,
+      `Checkout completion is only valid for new paid subscriptions, upgrades, or cross-currency changes to "${completion.subCtx.storedPlan.id}"`,
     );
   }
 

packages/paykit/src/types/schema.ts

@@ -2,6 +2,8 @@ import { createHash } from "node:crypto";
 
 import * as z from "zod";
 
+import { DEFAULT_STRIPE_CURRENCY } from "../stripe/currency";
+
 const payKitFeatureSymbol = Symbol.for("paykit.feature");
 const payKitFeatureIncludeSymbol = Symbol.for("paykit.feature_include");
 const payKitPlanSymbol = Symbol.for("paykit.plan");
@@ -429,7 +431,7 @@ export function normalizeSchema(
       isDefault,
       name: exportedPlan.name ?? deriveNameFromId(exportedPlan.id),
       priceAmount: exportedPlan.price ? Math.round(exportedPlan.price.amount * 100) : null,
-      priceCurrency: exportedPlan.price ? (input?.priceCurrency ?? "usd") : null,
+      priceCurrency: exportedPlan.price ? (input?.priceCurrency ?? DEFAULT_STRIPE_CURRENCY) : null,
       priceInterval: exportedPlan.price?.interval ?? null,
       trialDays: null,
     };