Skip to content

fix: default pm#883

Merged
johnyeocx merged 1 commit intomainfrom
fix/default-pm
Mar 5, 2026
Merged

fix: default pm#883
johnyeocx merged 1 commit intomainfrom
fix/default-pm

Conversation

@johnyeocx
Copy link
Collaborator

@johnyeocx johnyeocx commented Mar 5, 2026

Summary by cubic

Fixes billing attach and subscription updates when a customer has a card attached but no default_payment_method in Stripe by passing a fallback payment method. Also uses the Stripe customer's currency for in‑arrear line items.

  • Bug Fixes

    • Stripe subscriptions: add a fallback default_payment_method on create/update when the customer has an attached PM but no default; respect existing subscription defaults.
    • In‑arrear line items now use stripeCustomer.currency (fallback to org currency).
    • Minor cleanup: remove unused assignment in invoice item creation.
  • Tests

    • Added integration test to ensure attach succeeds when invoice_settings.default_payment_method is null.
    • Minor test cleanup and a temp scenario for a GBP Stripe customer.

Written for commit 6cd83f7. Summary will update on new commits.

Greptile Summary

This PR fixes a billing edge case where a Stripe customer has a payment method attached but invoice_settings.default_payment_method is null (e.g. after being manually cleared), causing subscription create/update operations to fail because Stripe had no PM to charge. It also corrects currency resolution for arrear line items to honour the customer's locked Stripe currency rather than always defaulting to the org-level currency.

Key changes:

  • Bug fixesexecuteStripeSubscriptionOperation.ts: when invoice_settings.default_payment_method is null on the customer but a resolved paymentMethod exists in BillingContext, it is now passed explicitly as default_payment_method on the subscription create call, and on the update call when the subscription itself also lacks a default PM.
  • Bug fixescustomerProductToArrearLineItems.ts: currency for arrear line items now prefers billingContext.stripeCustomer.currency over the org default, fixing incorrect currency usage for customers billed in a non-org currency (e.g. GBP).
  • ImprovementsprocessConsumablePricesForInvoiceCreated.ts: removes an unused variable assignment from createStripeInvoiceItems.
  • Improvements — New integration test (attach-pm-edge-case.test.ts) covering the fixed scenario end-to-end.
  • Improvements.opencode/opencode.json: adds Axiom MCP server for log querying tooling; .opencode/plans/alb-lambda-logging-fix.md documents the ALB → Lambda → Axiom logging pipeline fix.

Confidence Score: 4/5

  • Safe to merge — the fix is targeted, well-reasoned, and backed by a new integration test; the only concern is a WIP temp test file.
  • The core logic change is small and sound: it adds a narrow fallback only when both the customer and (for updates) the subscription lack a default PM. The new edge-case integration test directly validates the fixed path. The currency fix is straightforward. The only detractor is temp.test.ts being committed in a half-finished state with commented-out setup and a hardcoded Stripe product ID, which could cause confusion or flaky test runs in other environments.
  • server/tests/_temp/temp.test.ts — WIP test with commented-out setup and hardcoded Stripe product ID should be cleaned up before merging.

Important Files Changed

Filename Overview
server/src/internal/billing/v2/providers/stripe/utils/subscriptions/executeStripeSubscriptionOperation.ts Core fix: adds fallback default_payment_method to subscription create/update when the Stripe customer's invoice_settings.default_payment_method is null but a resolved payment method exists. Logic is sound — create always gets the fallback; update only gets it when neither the subscription nor the customer has a default PM already set.
server/src/internal/billing/v2/utils/lineItems/customerProductToArrearLineItems.ts Currency resolution for arrear line items now prefers the Stripe customer's locked currency over the org default, which is the correct behaviour for customers billed in a non-default currency (e.g. GBP).
server/src/external/stripe/webhookHandlers/handleStripeInvoiceCreated/tasks/processConsumablePricesForInvoiceCreated.ts Minor cleanup: removed unused invoiceItems variable assignment from createStripeInvoiceItems call. No behavioural change.
server/tests/integration/billing/attach/edge-cases/attach-pm-edge-case.test.ts New integration test covering the fixed edge case: a customer with an attached card but cleared invoice_settings.default_payment_method can still successfully attach a product. Clears the field using "" as string (Stripe SDK convention for unsetting string fields).
server/tests/_temp/temp.test.ts WIP exploratory test for GBP customer behaviour. Core setup is commented out and uses a hardcoded Stripe product ID, making it fragile outside the author's environment. Should not be considered production-ready test coverage.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant executeStripeSubscriptionOperation
    participant BillingContext
    participant StripeAPI

    Caller->>executeStripeSubscriptionOperation: call(ctx, billingContext, subscriptionAction)
    executeStripeSubscriptionOperation->>BillingContext: read paymentMethod
    executeStripeSubscriptionOperation->>BillingContext: read stripeCustomer.invoice_settings.default_payment_method
    Note over executeStripeSubscriptionOperation: customerHasDefaultPm = invoice_settings.default_payment_method

    alt paymentMethod exists AND customerHasDefaultPm is falsy
        executeStripeSubscriptionOperation->>executeStripeSubscriptionOperation: fallbackPaymentMethodParams = { default_payment_method: paymentMethod.id }
    else
        executeStripeSubscriptionOperation->>executeStripeSubscriptionOperation: fallbackPaymentMethodParams = {}
    end

    alt subscriptionAction.type == "create"
        executeStripeSubscriptionOperation->>StripeAPI: subscriptions.create({ ...params, ...fallbackPaymentMethodParams })
    else subscriptionAction.type == "update"
        executeStripeSubscriptionOperation->>BillingContext: read stripeSubscription.default_payment_method
        Note over executeStripeSubscriptionOperation: subscriptionHasDefaultPm = subscription.default_payment_method
        alt subscriptionHasDefaultPm is falsy
            executeStripeSubscriptionOperation->>StripeAPI: subscriptions.update({ ...params, ...fallbackPaymentMethodParams })
        else
            executeStripeSubscriptionOperation->>StripeAPI: subscriptions.update({ ...params })
        end
    end
    StripeAPI-->>Caller: Stripe.Subscription
Loading

Last reviewed commit: 6cd83f7

Greptile also left 1 inline comment on this PR.

@johnyeocx johnyeocx requested a review from ay-rod as a code owner March 5, 2026 09:25
@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
autumn-vite Ready Ready Preview, Comment Mar 5, 2026 9:26am

Request Review

@johnyeocx johnyeocx merged commit 1e67896 into main Mar 5, 2026
6 of 9 checks passed
Comment on lines 77 to 89
const customerId = "temp";

const pro = products.pro({
id: "pro",
items: [items.monthlyMessages({ includedUsage: 500 })],
});

const premium = products.premium({
id: "premium",
items: [items.monthlyMessages({ includedUsage: 1000 })],
});

const { autumnV1, testClockId, ctx } = await initScenario({
customerId,
// customerId,
setup: [
s.customer({ paymentMethod: "success" }),
s.products({ list: [pro, premium] }),
// s.customer({ paymentMethod: "success" }),
// s.products({ list: [pro, premium] }),
],
actions: [],
});

const stripeCli = createStripeCli({ org: ctx.org, env: ctx.env });

const coupon = await createPercentCoupon({
stripeCli,
percentOff: 20,
duration: "repeating",
durationInMonths: 1,
});

await autumnV1.billing.attach({
customer_id: customerId,
product_id: pro.id,
redirect_mode: "if_required",
discounts: [
{
reward_id: coupon.id,
},
],
});

await autumnV1.billing.attach({
customer_id: customerId,
product_id: premium.id,
redirect_mode: "if_required",
});

await autumnV1.billing.attach({
customer_id: customerId,
product_id: pro.id,
redirect_mode: "if_required",
});
await testStripeCustomerWithGBP({ ctx, autumn: autumnV1 });
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Work-in-progress test committed with commented-out setup

The initScenario call has customerId, s.customer, and s.products all commented out. This means the scenario initialises with no customer and no products, so the ctx object passed to testStripeCustomerWithGBP may lack a properly configured org/environment. Since the hardcoded Stripe product ID ("prod_U5XwDFBQB4TQJ7") also looks environment-specific, this test is likely to silently no-op or fail in other environments. Consider either fully enabling it (with proper fixtures) or keeping it out of the committed codebase until it's ready.

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/tests/_temp/temp.test.ts
Line: 77-89

Comment:
**Work-in-progress test committed with commented-out setup**

The `initScenario` call has `customerId`, `s.customer`, and `s.products` all commented out. This means the scenario initialises with no customer and no products, so the `ctx` object passed to `testStripeCustomerWithGBP` may lack a properly configured org/environment. Since the hardcoded Stripe `product` ID (`"prod_U5XwDFBQB4TQJ7"`) also looks environment-specific, this test is likely to silently no-op or fail in other environments. Consider either fully enabling it (with proper fixtures) or keeping it out of the committed codebase until it's ready.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 8 files

Confidence score: 4/5

  • This PR is likely safe to merge, with only a small process risk rather than a runtime regression risk.
  • The main issue is in .opencode/plans/alb-lambda-logging-fix.md: the verification command uses macOS-only date -v, which can cause the documented CloudWatch check to fail on GNU/Linux.
  • Given the issue severity (4/10) and that it affects verification steps in documentation/planning, impact appears limited and non-blocking for production behavior.
  • Pay close attention to .opencode/plans/alb-lambda-logging-fix.md - update the date -v command to a cross-platform/GNU-compatible form so validation works across environments.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".opencode/plans/alb-lambda-logging-fix.md">

<violation number="1" location=".opencode/plans/alb-lambda-logging-fix.md:41">
P2: The CloudWatch command uses a macOS-specific `date -v` flag, so the verification steps fail on GNU/Linux environments.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

--namespace AWS/Lambda \
--metric-name Errors \
--dimensions Name=FunctionName,Value=alb-listener-us-east-2 \
--start-time $(date -u -v-24H +%Y-%m-%dT%H:%M:%SZ) \
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The CloudWatch command uses a macOS-specific date -v flag, so the verification steps fail on GNU/Linux environments.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .opencode/plans/alb-lambda-logging-fix.md, line 41:

<comment>The CloudWatch command uses a macOS-specific `date -v` flag, so the verification steps fail on GNU/Linux environments.</comment>

<file context>
@@ -0,0 +1,89 @@
+  --namespace AWS/Lambda \
+  --metric-name Errors \
+  --dimensions Name=FunctionName,Value=alb-listener-us-east-2 \
+  --start-time $(date -u -v-24H +%Y-%m-%dT%H:%M:%SZ) \
+  --end-time $(date -u +%Y-%m-%dT%H:%M:%SZ) \
+  --period 3600 \
</file context>
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant