From 2bf07e935ffef89e30b8f53a9a617bf42ead14f5 Mon Sep 17 00:00:00 2001 From: amianthus <49116958+SirTenzin@users.noreply.github.com> Date: Fri, 22 May 2026 16:22:20 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20preflight=20tax=20addres?= =?UTF-8?q?s=20existence=20before=20taxing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../operations/createStripeCustomer.ts | 2 + .../operations/getExpandedStripeCustomer.ts | 7 + .../operations/getOrCreateStripeCustomer.ts | 5 + .../setup/fetchStripeCustomerForBilling.ts | 5 + .../utils/invoices/createInvoiceForBilling.ts | 6 +- .../buildStripeSubscriptionUpdateAction.ts | 8 +- .../executeStripeSubscriptionOperation.ts | 6 +- .../tax/shouldEnableStripeAutomaticTax.ts | 43 +++++ .../automatic-tax-no-address-error.test.ts | 165 +++++++++--------- 9 files changed, 156 insertions(+), 91 deletions(-) create mode 100644 server/src/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.ts diff --git a/server/src/external/stripe/customers/operations/createStripeCustomer.ts b/server/src/external/stripe/customers/operations/createStripeCustomer.ts index 1b61d813c..c977095b7 100644 --- a/server/src/external/stripe/customers/operations/createStripeCustomer.ts +++ b/server/src/external/stripe/customers/operations/createStripeCustomer.ts @@ -14,6 +14,7 @@ export const createStripeCustomer = async ({ customer: Customer; options?: { testClockId?: string; + expandTax?: boolean; }; }): Promise => { const { org, env } = ctx; @@ -43,6 +44,7 @@ export const createStripeCustomer = async ({ "test_clock", "invoice_settings.default_payment_method", "discount.source.coupon.applies_to", + ...(options.expandTax ? ["tax"] : []), ], }, idempotencyKey diff --git a/server/src/external/stripe/customers/operations/getExpandedStripeCustomer.ts b/server/src/external/stripe/customers/operations/getExpandedStripeCustomer.ts index d815f6898..03b6e57c3 100644 --- a/server/src/external/stripe/customers/operations/getExpandedStripeCustomer.ts +++ b/server/src/external/stripe/customers/operations/getExpandedStripeCustomer.ts @@ -28,28 +28,34 @@ export function getExpandedStripeCustomer({ ctx, stripeCustomerId, errorOnNotFound, + expandTax, }: { ctx: AutumnContext; stripeCustomerId: string; errorOnNotFound: true; + expandTax?: boolean; }): Promise; export function getExpandedStripeCustomer({ ctx, stripeCustomerId, errorOnNotFound, + expandTax, }: { ctx: AutumnContext; stripeCustomerId?: string; errorOnNotFound?: false; + expandTax?: boolean; }): Promise; export async function getExpandedStripeCustomer({ ctx, stripeCustomerId, errorOnNotFound = false, + expandTax = false, }: { ctx: AutumnContext; stripeCustomerId?: string; errorOnNotFound?: boolean; + expandTax?: boolean; }): Promise { const { org, env } = ctx; const stripeCli = createStripeCli({ org, env }); @@ -63,6 +69,7 @@ export async function getExpandedStripeCustomer({ "test_clock", "invoice_settings.default_payment_method", "discount.source.coupon.applies_to", + ...(expandTax ? ["tax"] : []), ], }), ); diff --git a/server/src/external/stripe/customers/operations/getOrCreateStripeCustomer.ts b/server/src/external/stripe/customers/operations/getOrCreateStripeCustomer.ts index 4307bcbf5..420d49690 100644 --- a/server/src/external/stripe/customers/operations/getOrCreateStripeCustomer.ts +++ b/server/src/external/stripe/customers/operations/getOrCreateStripeCustomer.ts @@ -22,6 +22,7 @@ export const getOrCreateStripeCustomer = async ({ customer: Customer; options?: { updateDb?: boolean; + expandTax?: boolean; }; }): Promise => { const { logger } = ctx; @@ -29,6 +30,7 @@ export const getOrCreateStripeCustomer = async ({ const currentStripeCustomer = await getExpandedStripeCustomer({ ctx, stripeCustomerId: customer.processor?.id, + expandTax: options.expandTax, }); if (currentStripeCustomer) return currentStripeCustomer; @@ -40,6 +42,9 @@ export const getOrCreateStripeCustomer = async ({ const stripeCustomer = await createStripeCustomer({ ctx, customer, + options: { + expandTax: options.expandTax, + }, }); if (options.updateDb) { diff --git a/server/src/internal/billing/v2/providers/stripe/setup/fetchStripeCustomerForBilling.ts b/server/src/internal/billing/v2/providers/stripe/setup/fetchStripeCustomerForBilling.ts index 91292d98c..ba1df71a6 100644 --- a/server/src/internal/billing/v2/providers/stripe/setup/fetchStripeCustomerForBilling.ts +++ b/server/src/internal/billing/v2/providers/stripe/setup/fetchStripeCustomerForBilling.ts @@ -19,15 +19,20 @@ export const fetchStripeCustomerForBilling = async ({ }) => { const { org, env } = ctx; const stripeCli = createStripeCli({ org, env }); + const expandTax = !!ctx.org.config.automatic_tax; const stripeCus = createIfMissing ? await getOrCreateStripeCustomer({ ctx, customer: fullCus, + options: { + expandTax, + }, }) : await getExpandedStripeCustomer({ ctx, stripeCustomerId: fullCus.processor?.id, + expandTax, }); if (!stripeCus) { diff --git a/server/src/internal/billing/v2/providers/stripe/utils/invoices/createInvoiceForBilling.ts b/server/src/internal/billing/v2/providers/stripe/utils/invoices/createInvoiceForBilling.ts index b2f00e4f9..a3ebd1550 100644 --- a/server/src/internal/billing/v2/providers/stripe/utils/invoices/createInvoiceForBilling.ts +++ b/server/src/internal/billing/v2/providers/stripe/utils/invoices/createInvoiceForBilling.ts @@ -16,6 +16,7 @@ import type { Stripe } from "stripe"; import { createStripeCli } from "@/external/connect/createStripeCli"; import type { AutumnContext } from "@/honoUtils/HonoEnv"; import { mergeStripeMetadata } from "@/internal/billing/v2/providers/stripe/utils/common/mergeStripeMetadata"; +import { shouldEnableStripeAutomaticTax } from "@/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax"; const stripeDiscountsToInvoiceParams = ({ stripeDiscounts, @@ -93,10 +94,7 @@ export const createInvoiceForBilling = async ({ stripeDiscounts: billingContext.stripeDiscounts ?? [], }); - // Skip auto_tax in invoice mode: send_invoice has no address-collection - // UI so Stripe Tax rejects. charge_automatically relies on Stripe's - // address waterfall. - const wantsAutoTax = !!ctx.org.config.automatic_tax && !isInvoiceMode; + const wantsAutoTax = shouldEnableStripeAutomaticTax({ ctx, billingContext }); const draftInvoice = await createStripeInvoice({ stripeCli, stripeCusId: billingContext.stripeCustomer?.id ?? "none", diff --git a/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/buildStripeSubscriptionUpdateAction.ts b/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/buildStripeSubscriptionUpdateAction.ts index 85460aa40..fc7ef74b4 100644 --- a/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/buildStripeSubscriptionUpdateAction.ts +++ b/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/buildStripeSubscriptionUpdateAction.ts @@ -9,6 +9,7 @@ import { notNullish } from "@shared/utils/utils"; import type Stripe from "stripe"; import type { AutumnContext } from "@/honoUtils/HonoEnv"; import { stripeDiscountsToParams } from "@/internal/billing/v2/providers/stripe/utils/discounts/stripeDiscountsToParams"; +import { shouldEnableStripeAutomaticTax } from "@/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax"; export const buildStripeSubscriptionUpdateAction = ({ ctx, @@ -96,10 +97,9 @@ export const buildStripeSubscriptionUpdateAction = ({ }), // Propagate auto_tax onto every sub.update so existing subs catch up - // when the org flag flips. Baked in here (not execute) for log - // self-description. Skipped in invoice mode: send_invoice invoices - // can't collect address, so Stripe Tax rejects. - ...(ctx.org.config.automatic_tax && !billingContext.invoiceMode + // when the org flag flips. Baked in here for log self-description; + // execute re-applies the same preflight before writing to Stripe. + ...(shouldEnableStripeAutomaticTax({ ctx, billingContext }) ? { automatic_tax: { enabled: true } } : {}), }; diff --git a/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/executeStripeSubscriptionOperation.ts b/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/executeStripeSubscriptionOperation.ts index 92bfe03c9..4f6c9a478 100644 --- a/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/executeStripeSubscriptionOperation.ts +++ b/server/src/internal/billing/v2/providers/stripe/utils/subscriptions/executeStripeSubscriptionOperation.ts @@ -4,6 +4,7 @@ import { createStripeCli } from "@/external/connect/createStripeCli"; import type { AutumnContext } from "@/honoUtils/HonoEnv"; import { buildAutumnSubscriptionMetadata } from "@/internal/billing/v2/providers/stripe/utils/common/autumnStripeMetadata"; import { mergeStripeMetadata } from "@/internal/billing/v2/providers/stripe/utils/common/mergeStripeMetadata"; +import { shouldEnableStripeAutomaticTax } from "@/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax"; import { willStripeSubscriptionUpdateCreateInvoice } from "./willStripeSubscriptionUpdateCreateInvoice"; export const executeStripeSubscriptionOperation = async ({ @@ -46,10 +47,7 @@ export const executeStripeSubscriptionOperation = async ({ actionSource: billingContext.actionSource, }), }); - // Skip auto_tax in invoice mode: send_invoice has no address-collection - // UI so Stripe Tax rejects. - const wantsAutoTax = - !!ctx.org.config.automatic_tax && !billingContext.invoiceMode; + const wantsAutoTax = shouldEnableStripeAutomaticTax({ ctx, billingContext }); const taxRateParams = billingContext.taxRateId ? { default_tax_rates: [billingContext.taxRateId] } diff --git a/server/src/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.ts b/server/src/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.ts new file mode 100644 index 000000000..30344b9c7 --- /dev/null +++ b/server/src/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.ts @@ -0,0 +1,43 @@ +import type { BillingContext } from "@autumn/shared"; +import type Stripe from "stripe"; +import type { AutumnContext } from "@/honoUtils/HonoEnv"; + +const hasUsableTaxAddress = (address?: Stripe.Address | null) => { + return Boolean(address?.country); +}; + +const customerHasUsableTaxLocation = (stripeCustomer?: Stripe.Customer) => { + if (!stripeCustomer) return true; + + if (stripeCustomer.tax?.automatic_tax) { + return ["supported", "not_collecting"].includes( + stripeCustomer.tax.automatic_tax, + ); + } + + return ( + hasUsableTaxAddress(stripeCustomer.address) || + hasUsableTaxAddress(stripeCustomer.shipping?.address) + ); +}; + +export const shouldEnableStripeAutomaticTax = ({ + ctx, + billingContext, +}: { + ctx: AutumnContext; + billingContext: BillingContext; +}) => { + if (!ctx.org.config.automatic_tax) return false; + + // Invoice mode uses send_invoice and has no address collection UI. + if (billingContext.invoiceMode) return false; + + // Use only the already-fetched Stripe customer. If setup did not fetch one, + // do not fetch again on the write path. + if (!customerHasUsableTaxLocation(billingContext.stripeCustomer)) { + return false; + } + + return true; +}; diff --git a/server/tests/integration/billing/tax/automatic-tax-no-address-error.test.ts b/server/tests/integration/billing/tax/automatic-tax-no-address-error.test.ts index 3d384e2e7..dd49c10aa 100644 --- a/server/tests/integration/billing/tax/automatic-tax-no-address-error.test.ts +++ b/server/tests/integration/billing/tax/automatic-tax-no-address-error.test.ts @@ -1,93 +1,100 @@ /** - * Regression guard: when `automatic_tax: true` but the customer has no - * address, attach must surface an actionable tax/address error (Stripe's - * `customer_tax_location_invalid` or a typed RecaseError) instead of a - * generic 500. Covers both v1 `/v1/attach` and v2 `/v1/billing.attach`. + * Regression guard for orgs that enable `automatic_tax` after customers + * already have paid subscriptions but no Stripe tax location on file. + * + * Red-failure mode (current behavior): + * - Pro -> Premium upgrade sends `automatic_tax.enabled=true` to Stripe. + * - Stripe rejects with `customer_tax_location_invalid`. + * + * Green-success criteria (after fix): + * - Upgrade succeeds by falling back to no automatic tax for this mutation. + * - Resulting subscription and upgrade invoice have automatic tax disabled. */ import { expect, test } from "bun:test"; +import type { AttachParamsV1Input } from "@autumn/shared"; import { products } from "@tests/utils/fixtures/products.js"; import { initScenario, s } from "@tests/utils/testInitUtils/initScenario.js"; import chalk from "chalk"; +import { OrgService } from "@/internal/orgs/OrgService.js"; -function hasActionableTaxSignal(err: unknown): boolean { - const errorString = JSON.stringify(err, [ - "message", - "code", - "name", - "type", - ]).toLowerCase(); - return ( - errorString.includes("tax") || - errorString.includes("address") || - errorString.includes("location") - ); -} +test.concurrent( + `${chalk.yellowBright("automatic-tax-no-address (v2 pre-flip upgrade): succeeds without tax when Stripe customer has no location")}`, + async () => { + const customerId = "tax-no-address-preflip-upgrade"; + const pro = products.pro({ id: "pro", items: [] }); + const premium = products.premium({ id: "premium", items: [] }); -test.concurrent(`${chalk.yellowBright("automatic-tax-no-address-error (v1 legacy /v1/attach): customer without address surfaces actionable error")}`, async () => { - const customerId = "tax-no-address-v1"; - const proProd = products.pro({ id: "pro", items: [] }); - - const { autumnV1 } = await initScenario({ - customerId, - setup: [ - s.platform.create({ - configOverrides: { automatic_tax: true }, - taxRegistrations: ["AU"], - }), - s.customer({ - testClock: false, - paymentMethod: "success", - }), - s.products({ list: [proProd] }), - ], - actions: [], - }); - - let caughtError: unknown; - try { - await autumnV1.attach({ - customer_id: customerId, - product_id: `pro_${customerId}`, + const { ctx, customer, autumnV2_2 } = await initScenario({ + customerId, + setup: [ + s.platform.create({ + taxRegistrations: ["AU"], + }), + s.customer({ + testClock: false, + paymentMethod: "success", + }), + s.products({ list: [pro, premium] }), + ], + actions: [], }); - } catch (err) { - caughtError = err; - } - expect(caughtError).toBeDefined(); - expect(hasActionableTaxSignal(caughtError)).toBe(true); -}, 240_000); + const stripeCustomerId = customer!.processor!.id!; + const stripeCustomerBefore = + await ctx.stripeCli.customers.retrieve(stripeCustomerId); + if ("deleted" in stripeCustomerBefore && stripeCustomerBefore.deleted) { + throw new Error("Stripe customer was unexpectedly deleted"); + } + expect(stripeCustomerBefore.address).toBeNull(); -test.concurrent(`${chalk.yellowBright("automatic-tax-no-address-error (v2 /v1/billing.attach): customer without address surfaces actionable error")}`, async () => { - const customerId = "tax-no-address-v2"; - const proProd = products.pro({ id: "pro", items: [] }); - - const { autumnV2_2 } = await initScenario({ - customerId, - setup: [ - s.platform.create({ - configOverrides: { automatic_tax: true }, - taxRegistrations: ["AU"], - }), - s.customer({ - testClock: false, - paymentMethod: "success", - }), - s.products({ list: [proProd] }), - ], - actions: [], - }); - - let caughtError: unknown; - try { - await autumnV2_2.billing.attach({ - customer_id: customerId, - plan_id: `pro_${customerId}`, + await OrgService.update({ + db: ctx.db, + orgId: ctx.org.id, + updates: { + config: { ...ctx.org.config, automatic_tax: false }, + }, }); - } catch (err) { - caughtError = err; - } - expect(caughtError).toBeDefined(); - expect(hasActionableTaxSignal(caughtError)).toBe(true); -}, 240_000); + await autumnV2_2.billing.attach({ + customer_id: customerId, + plan_id: pro.id, + }); + + const initialSubscriptions = await ctx.stripeCli.subscriptions.list({ + customer: stripeCustomerId, + limit: 1, + }); + expect(initialSubscriptions.data[0].automatic_tax.enabled).toBe(false); + + await OrgService.update({ + db: ctx.db, + orgId: ctx.org.id, + updates: { + config: { ...ctx.org.config, automatic_tax: true }, + }, + }); + + await autumnV2_2.billing.attach({ + customer_id: customerId, + plan_id: premium.id, + }); + + const upgradedSubscriptions = await ctx.stripeCli.subscriptions.list({ + customer: stripeCustomerId, + limit: 1, + }); + const upgradedSubscription = upgradedSubscriptions.data[0]; + expect(upgradedSubscription).toBeDefined(); + expect(upgradedSubscription.automatic_tax.enabled).toBe(false); + + const invoices = await ctx.stripeCli.invoices.list({ + customer: stripeCustomerId, + limit: 5, + }); + const upgradeInvoice = invoices.data[0]; + expect(upgradeInvoice).toBeDefined(); + expect(upgradeInvoice.automatic_tax.enabled).toBe(false); + }, + 300_000, +);