apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts (1)

52-53: Add missing E2E coverage in validate-code tests

We currently only verify stackable: false in apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts. To fully exercise the new API behavior, please add tests covering:

• A positive stackable offer scenario (assert that { offer.stackable: true } is returned).
• A client‐side request against a server-only offer (should be rejected with the appropriate error code).
• A mismatched customer_type on the payload versus the offer (should be rejected with OFFER_CUSTOMER_TYPE_DOES_NOT_MATCH).

Example snippets to slot into the file:

it("should surface stackable: true for stackable offers", async ({ expect }) => {
  await Payments.setup({ offerOverrides: { stackable: true } });
  const { code } = await Payments.createPurchaseUrlAndGetCode();
  const res = await niceBackendFetch("/api/latest/payments/purchases/validate-code", {
    method: "POST",
    accessType: "client",
    body: { full_code: code },
  });
  const json = await res.json();
  expect(json.offer.stackable).toBe(true);
});

it("should reject client accessType against a server-only offer", async ({ expect }) => {
  await Payments.setup({ offerOverrides: { serverOnly: true } });
  const { code } = await Payments.createPurchaseUrlAndGetCode();
  const res = await niceBackendFetch("/api/latest/payments/purchases/validate-code", {
    method: "POST",
    accessType: "client",
    body: { full_code: code },
  });
  expect(res.status).toBe(403);
  const json = await res.json();
  expect(json.code).toBe("OFFER_ACCESS_TYPE_NOT_ALLOWED");
});

it("should reject when customer_type does not match the offer", async ({ expect }) => {
  await Payments.setup({ offerOverrides: { customer_type: "team" } });
  const { code } = await Payments.createPurchaseUrlAndGetCode();
  const res = await niceBackendFetch("/api/latest/payments/purchases/validate-code", {
    method: "POST",
    accessType: "client",
    body: { full_code: code, customer_type: "user" },
  });
  expect(res.status).toBe(400);
  const json = await res.json();
  expect(json.code).toBe("OFFER_CUSTOMER_TYPE_DOES_NOT_MATCH");
});

• File to update:

• apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts (around line 52)

apps/e2e/tests/snapshot-serializer.ts (2)

67-74: Also strip camelCase variant 'stripeAccountId'

Some codepaths/SDKs may surface camelCase. Strip both to avoid inconsistent redaction across snapshots.

Apply:

 const stripFieldsIfString = [
   "secret_api_key",
   "publishable_client_key",
   "secret_server_key",
   "super_secret_admin_key",
-  "stripe_account_id",
+  "stripe_account_id",
+  "stripeAccountId",
 ] as const;

---

72-73: Add regex to strip raw Stripe acct_ IDs from snapshot strings

A quick scan of the codebase showed hard-coded acct_ IDs in your E2E tests, which could slip into snapshots:

• apps/e2e/tests/backend/endpoints/api/v1/internal/payments/setup.test.ts
  • Lines 105, 124, 138: URLs containing acct_1PgafTB7WZ01zgkW

To prevent future leaks, consider extending your snapshot serializer to replace any bare acct_ prefixes with a placeholder. For example, in apps/e2e/tests/snapshot-serializer.ts (around lines 72–73), add:

// add to stringRegexReplacements
[/\bacct_[0-9A-Za-z]+\b/g, "<stripped stripe acct id>"],

This keeps the change narrow—only touching true Stripe account IDs—while avoiding unintended replacements of “account.”

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T18:36:37.703Z
Learning: Applies to **/snapshot-serializer.ts : Adjust snapshot-serializer.ts to control snapshot formatting and handle non-deterministic values as needed

apps/backend/src/lib/payments.test.tsx (3)

60-75: FIFO ledger absorption makes the expectation (11) correct; previous bot comment is no longer applicable

The narrative “Expired +10 absorbs earlier -3; active +11 remains” is consistent with FIFO consumption against the earliest non-expired positive bucket at the time of the debit. Given:

• +10 (expires 2025-01-31T23:59:59Z)
• +11 (no expiry)
• -3 (no expiry, 2025-01-30Z)

The -3 applies to the +10 bucket (which later expires), leaving the +11 unaffected at “now” (2025-02-01Z) → 11. This aligns with the ledger model exercised here. Good as-is.

Also applies to: 83-85

---

175-212: Weekly accumulation math checks out

From 2025-02-01 to 2025-02-15: elapsed full weeks = 2 ⇒ occurrences = 3 ⇒ 3 × 4 × quantity(1) = 12. Comment and assertion align with getIntervalsElapsed + 1 behavior.

---

253-291: expires: 'when-repeated' should be a single active grant per current repeat window, not an accumulation across started windows

The assertion of 7 is correct. The implementation adds windowed positive transactions (via addWhenRepeatedItemWindowTransactions) that expire at each window boundary; at any instant within the billing period only the current window’s grant is active. An earlier bot suggestion to expect 21 assumed accumulation of prior windows, which is not how the windowing works.

packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)

1204-1206: Client createCheckoutUrl narrowing verified – no object literal usage detected

The createCheckoutUrl(offerId: string) implementation in client-app-impl.ts correctly restricts the parameter to a string and delegates to the interface. A repository-wide search found no calls passing object literals (e.g. {…}) to this method, and the import of InlineOffer is no longer used.

• File & Location

• packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (lines 1204–1206): definition of async createCheckoutUrl(offerId: string).
  • Call sites
• No instances of createCheckoutUrl({...}) across the codebase.
  • Unused import
• InlineOffer is imported on line 41 but not referenced elsewhere—consider removing this import as an optional cleanup.

apps/backend/src/lib/payments.test.tsx (6)

2-2: Add afterEach import to ensure timers are always restored

You restore timers at the end of each test, but a failing assertion would skip that line. Import afterEach so we can centralize cleanup.

-import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

---

39-43: Centralize fake timer cleanup to avoid leaks on failures

Add an afterEach that always restores real timers; then you can drop per-test vi.useRealTimers() calls for cleaner tests.

 describe('getItemQuantityForCustomer - manual changes (no subscription)', () => {
   beforeEach(() => {
     vi.useFakeTimers();
   });
+  afterEach(() => {
+    vi.useRealTimers();
+  });

Follow-up: consider removing the vi.useRealTimers() at the end of each test in this describe block (and similarly in the subscriptions block) to avoid repetition.

---

6-23: Reduce reliance on as any in the Prisma mock

The helper works, but heavy as any weakens type checks. Consider typing only the fields you exercise to keep tests safer without extra noise.

Minimal approach:

• Define a narrow interface for the methods you use (subscription.findMany, itemQuantityChange.findMany/findFirst, projectUser.findUnique, team.findUnique).
• Cast only the nested mocks to that interface instead of the whole object.

---

25-37: Tighten Tenancy typing in createMockTenancy

Returning as any hides shape regressions. If feasible, return a Pick<Tenancy, 'id' | 'config' | 'branchId' | 'organization' | 'project'> and type config.payments as Partial<Tenancy['config']['payments']>.

---

409-411: Remove redundant vi.useFakeTimers() calls inside tests

You already enable fake timers in beforeEach; calling vi.useFakeTimers() again inside these tests is unnecessary.

-    vi.useFakeTimers();
+    // rely on suite-level beforeEach fake timers

Also applies to: 360-361

---

132-717: Consider a few additional edge-case tests for completeness

• Subscription with currentPeriodStart in the future (now < start) should yield 0.
• Manual positive with a future expiresAt boundary inside a period (to ensure it drops precisely at boundary).
• Repeat=weekly with expires: 'when-purchase-expires' evaluated exactly at a boundary instant (e.g., 2025-02-15T00:00:00Z) to confirm off-by-one behavior.

I can draft these test cases if helpful.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

packages/stack-shared/src/config/schema.ts (1)

652-667: Fix potential runtime error when isAddOnTo is not an object.

The transformation assumes offer.isAddOnTo has an Object.keys() method when not false, but this could throw if isAddOnTo is null, undefined, or a non-object value.

Apply this diff to add proper validation:

-    const isAddOnTo = offer.isAddOnTo === false ?
-      false as const :
-      typedFromEntries(Object.keys(offer.isAddOnTo).map((key) => [key, true as const]));
+    const isAddOnTo = offer.isAddOnTo === false ?
+      false as const :
+      typedFromEntries(Object.keys(offer.isAddOnTo || {}).map((key) => [key, true as const]));

apps/backend/src/app/api/latest/payments/purchases/purchase-session/route.tsx (3)

1-145: Consider adding validation for multiple non-stackable purchases.

The route currently handles group upgrades/downgrades but may benefit from explicit validation preventing duplicate non-stackable offer purchases.

#!/bin/bash
# Description: Search for existing validation logic preventing duplicate non-stackable purchases

# Check if validatePurchaseSession already handles this case
rg -n "Customer already has.*subscription.*stackable" apps/backend/src/lib/payments.tsx

---

81-82: Fix Stripe metadata null value issue.

Stripe metadata does not accept null values. Only include offerId when it's present to avoid Stripe API rejection.

Apply this diff to fix the metadata construction:

-      metadata: {
-        offerId: data.offerId ?? null,
-        offer: JSON.stringify(data.offer),
-      },
+      metadata: {
+        ...(data.offerId ? { offerId: String(data.offerId) } : {}),
+        offer: JSON.stringify(data.offer),
+      },

---

124-125: Fix duplicate Stripe metadata null value issue.

Same issue as above - Stripe metadata requires string values only.

Apply this diff:

-        metadata: {
-          offerId: data.offerId ?? null,
-          offer: JSON.stringify(data.offer),
-        },
+        metadata: {
+          ...(data.offerId ? { offerId: String(data.offerId) } : {}),
+          offer: JSON.stringify(data.offer),
+        },

apps/backend/src/app/api/latest/internal/payments/test-mode-purchase-session/route.tsx (2)

87-88: Date consistency issue in subscription period boundaries.

Creating Date objects at different times for currentPeriodStart and in the addInterval call could lead to inconsistent period boundaries in edge cases.

Apply this diff to ensure consistency:

-                currentPeriodStart: new Date(),
-                currentPeriodEnd: addInterval(new Date(), selectedPrice.interval!),
+                const now = new Date();
+                currentPeriodStart: now,
+                currentPeriodEnd: addInterval(now, selectedPrice.interval!),

---

47-114: Missing validation for duplicate non-stackable purchases.

The test-mode route should validate that customers can't purchase non-stackable offers they already own, similar to the production route.

The validation is actually handled in validatePurchaseSession at line 40-46, which throws an error for duplicate non-stackable purchases. No additional validation needed here.

apps/backend/src/lib/payments.tsx (2)

262-299: Add database constraint for subscription uniqueness.

The code assumes one subscription per offer per customer, but this isn't enforced at the database level. Multiple subscriptions could violate this assumption.

#!/bin/bash
# Description: Check if unique constraint exists in Prisma schema for subscription uniqueness

# Search for unique constraints on Subscription model
rg -A 10 "model Subscription" apps/backend/prisma/schema.prisma | rg "@@unique|@unique"

---

183-214: Add test coverage for repeat with "never" expiry.

The code path where inc.repeat is truthy and inc.expires is "never" or undefined (lines 208-213) lacks test coverage.

Would you like me to create a test case that covers the scenario where items have a repeat interval but never expire? This would ensure the ledger algorithm correctly handles accumulating quantities over time without expiration.

apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/page-client.tsx (1)

127-132: Verified team offer exists; consider dynamic next-tier and tighten window flags

• In apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/projects/page-client.tsx (lines 127–132), you’re calling

  await props.team.createCheckoutUrl({ offerId: "team" });

  This matches the team offer defined in apps/backend/prisma/seed.ts (lines 96–102), which has customerType: "team" and includes 3 dashboard_admins seats.
• If your intent is to upgrade from a free/default plan → team, hard-coding "team" is correct. However, once a customer is already on the team plan (3 seats) and exceeds that limit, you’ll want to direct them to the next tier (e.g. the growth offer, which includes 5 admins). To avoid this pitfall, consider:
  • Moving offer IDs into a shared config/constants file.
  • Dynamically selecting the “next” plan based on current quantity (e.g. pick the offer with the next-highest includedItems.dashboard_admins.quantity).
• Nit: strengthen the window.open call to prevent referrer leakage:

  - window.open(checkoutUrl, "_blank", "noopener");
  + window.open(checkoutUrl, "_blank", "noopener,noreferrer");

packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts (1)

41-41: Remove unused InlineOffer import

InlineOffer isn’t used on the client implementation anymore. Drop it to prevent drift and satisfy noUnusedLocals settings.

Apply:

-import { Customer, InlineOffer, Item } from "../../customers";
+import { Customer, Item } from "../../customers";

packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts (1)

580-583: Simplify and Deduplicate Offer Normalization in createCheckoutUrl

Both the user and team branches in server-app-impl.ts repeat the same logic to pull either offerId or an inline offer. Extracting that into a small helper will make future changes (e.g. altering the schema or adding validation) in one place.

Locations to update:

• packages/template/src/lib/stack-app/apps/implementations/server-app-impl.ts
  • Lines 580–583 (user platform)
  • Lines 719–722 (team platform)

Suggested refactor:

// Add near the top of server-app-impl.ts (or in a shared utils file)
function normalizeCheckoutOptions(
  options: { offerId: string } | { offer: InlineOffer }
): string | InlineOffer {
  return "offerId" in options ? options.offerId : options.offer;
}

// Usage in both methods
async createCheckoutUrl(options: { offerId: string } | { offer: InlineOffer }) {
  return await app._interface.createCheckoutUrl(
    "user",                          // or "team"
    crud.id,
    normalizeCheckoutOptions(options),
    null
  );
},

This change preserves the existing signature—StackServerInterface.createCheckoutUrl continues to accept string | InlineOffer—while centralizing the normalization logic.

apps/backend/src/app/api/latest/internal/payments/test-mode-purchase-session/route.tsx (1)

47-96: Consider extracting the group handling logic to a reusable function.

The group offer replacement logic (lines 47-96) contains complex business rules that could benefit from being extracted into a dedicated function for better testability and reusability.

Consider extracting this logic into a separate function in lib/payments.tsx:

export async function replaceGroupSubscriptionsInTestMode(options: {
  prisma: PrismaClientTransaction,
  tenancy: Tenancy,
  groupId: string,
  subscriptions: Subscriptions,
  data: { customerId: string, offerId?: string, offer: Offer },
  quantity: number,
  selectedPrice: SelectedPrice,
}) {
  // Extract lines 47-96 logic here
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: stack-auth/stack-auth#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T18:36:37.703Z
Learning: Applies to **/*.{ts,tsx} : Prefer ES6 Map over Record where feasible

apps/dashboard/src/app/(main)/purchase/[code]/page-client.tsx (1)

156-174: Well-structured user feedback for purchase restrictions.

The conditional alerts effectively communicate:

• When an offer has already been purchased (for non-stackable offers)
• When purchasing will result in a plan change due to conflicting group offers

This addresses the past review comments from N2D4 about showing appropriate messages for these scenarios.

apps/backend/src/lib/payments.tsx (2)

183-214: Coverage gap: repeat truthy + expires='never' branch lacks a test

The branch that emits FAR_FUTURE_DATE with repeat truthy and expires 'never' isn’t explicitly tested.

Add/confirm a test for this path (weekly repeat, never expires). Quick search script:

#!/bin/bash
rg -n --type=ts -C3 "repeat.*'week'|expires.*'never'|FAR_FUTURE_DATE" apps/backend/src | sed -n '1,200p'
rg -n --type=ts -C3 "repeat.*'week'|expires.*'never'|FAR_FUTURE_DATE" -g "*test*"

---

245-302: Enforce one subscription per (tenancy, customer, offer) at DB level or handle multiples

Current logic assumes uniqueness but schema doesn’t enforce it; duplicates could break find-based checks elsewhere.

If domain requires uniqueness, add a Prisma unique constraint:

@@unique([tenancyId, customerType, customerId, offerId])

If multiples are allowed, adjust aggregation (e.g., sum quantities or return multiple entries) accordingly.

packages/stack-shared/src/config/schema.ts (1)

650-665: Fix: isAddOnTo transformation throws when undefined/null

Object.keys(offer.isAddOnTo) will throw if isAddOnTo is undefined. Normalize safely.

-  const offers = typedFromEntries(typedEntries(prepared.payments.offers).map(([key, offer]) => {
-    const isAddOnTo = offer.isAddOnTo === false ?
-      false as const :
-      typedFromEntries(Object.keys(offer.isAddOnTo).map((key) => [key, true as const]));
+  const offers = typedFromEntries(typedEntries(prepared.payments.offers).map(([key, offer]) => {
+    const isAddOnTo =
+      offer.isAddOnTo && offer.isAddOnTo !== false
+        ? typedFromEntries(typedKeys(offer.isAddOnTo).map((k) => [k, true as const]))
+        : false as const;
     const prices = offer.prices === "include-by-default" ?
       "include-by-default" as const :
       typedFromEntries(typedEntries(offer.prices).map(([key, value]) => {
         const data = { serverOnly: false, ...(value ?? {}) };
         return [key, data];
       }));
     return [key, {
       ...offer,
       isAddOnTo,
       prices,
     }];
   }));

apps/dashboard/src/app/(main)/purchase/[code]/page-client.tsx (2)

46-52: Consider improving quantity parsing robustness.

While the current implementation handles NaN cases, consider adding bounds checking and integer validation.

 const quantityNumber = useMemo((): number => {
   const n = parseInt(quantityInput, 10);
   if (Number.isNaN(n)) {
     return 0;
   }
-  return n;
+  // Ensure minimum of 0 and maximum reasonable value
+  return Math.max(0, Math.min(n, 10000));
 }, [quantityInput]);

---

214-224: Consider adding debouncing for quantity input.

While the current implementation works, rapid typing could trigger many re-renders and calculations.

Consider debouncing the quantity input to improve performance:

+import { useDebounce } from "@/hooks/use-debounce"; // or your preferred debounce hook

 const [quantityInput, setQuantityInput] = useState<string>("1");
+const debouncedQuantityInput = useDebounce(quantityInput, 300);

 const quantityNumber = useMemo((): number => {
-  const n = parseInt(quantityInput, 10);
+  const n = parseInt(debouncedQuantityInput, 10);
   // ... rest of the logic
-}, [quantityInput]);
+}, [debouncedQuantityInput]);

apps/e2e/tests/backend/endpoints/api/v1/payments/validate-code.test.ts (1)

64-158: Solid new E2E coverage; add two more scenarios to lock behavior down

Great additions for non-stackable and same-group conflicts. Please also add:

• Stackable offer flow: prior purchase of the same offer with stackable: true should keep already_bought_non_stackable = false and conflicting_group_offers = [].
• Include-by-default group default: validating a priced offer in a group that has a default include-by-default offer should not list the default in conflicting_group_offers (after fixing the route, see my API comment).

This aligns with “apps/e2e/**: Always add new E2E tests when you change the API or SDK interface”.

Also applies to: 160-254

apps/backend/src/app/api/latest/payments/purchases/validate-code/route.ts (2)

70-74: Nit: prefer typedKeys over Object.keys for stronger typing

Minor consistency/readability improvement.

-    const groupId = Object.keys(groups).find((g) => offer.groupId === g);
+    const groupId = typedKeys(groups).find((g) => offer.groupId === g);

And import typedKeys:

-import { filterUndefined, getOrUndefined, typedEntries, typedFromEntries } from "@stackframe/stack-shared/dist/utils/objects";
+import { filterUndefined, getOrUndefined, typedEntries, typedFromEntries, typedKeys } from "@stackframe/stack-shared/dist/utils/objects";

---

59-67: Optional: reuse validatePurchaseSession() to keep conflict logic in one place

Consider delegating to validatePurchaseSession() for computing already_bought_non_stackable and conflicting_group_offers to avoid future drift between this route and the purchase-session flow.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.