fix(billing): don't show an error when a checkout is superseded - #14705
Conversation
Picking a different plan mid-checkout makes the API replace the pending subscription operation. The replaced operation terminates as failed even though nothing went wrong, so the user saw "We couldn't update your subscription" while the replacement checkout was still succeeding. Suppress the error toast for that case, keyed on the API's `checkout_superseded` reason, and report it as `stale_operation` rather than `unknown` so it stops counting as an unexplained billing failure. Genuine failures are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
🎨 Storybook: ✅ Built — View Storybook🎭 Playwright: ✅ 1782 passed, 0 failed · 4 flaky📊 Browser Reports
📦 Bundle: 8.25 MB gzip 🔴 +149 BDetailsSummary
Category Glance App Entry Points — 3.64 kB (baseline 3.64 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.35 MB (baseline 1.35 MB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed / 1 unchanged Views & Navigation — 111 kB (baseline 111 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 12 added / 12 removed / 4 unchanged Panels & Settings — 551 kB (baseline 551 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed / 15 unchanged User & Accounts — 28.7 kB (baseline 28.7 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed / 3 unchanged Editors & Dialogs — 124 kB (baseline 124 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 6 added / 6 removed / 1 unchanged UI Components — 70.2 kB (baseline 70.2 kB) • ⚪ 0 BReusable component library chunks
Status: 6 added / 6 removed / 9 unchanged Data & Services — 3.47 MB (baseline 3.47 MB) • 🔴 +219 BStores, services, APIs, and repositories
Status: 14 added / 14 removed / 3 unchanged Utilities & Hooks — 386 kB (baseline 386 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 16 added / 16 removed / 20 unchanged Vendor & Third-Party — 15.7 MB (baseline 15.7 MB) • ⚪ 0 BExternal libraries and shared vendor chunks Status: 16 unchanged Other — 12.8 MB (baseline 12.8 MB) • ⚪ 0 BBundles that do not match a named category
Status: 70 added / 70 removed / 211 unchanged ⚡ Performance
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe billing operation store recognizes superseded subscription checkouts. It records these failures as ChangesBilling failure handling
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/platform/workspace/stores/billingOperationStore.test.ts`:
- Around line 441-465: Replace createPinia() with createTestingPinia({
stubActions: false }) in the billing operation store tests, while preserving
setActivePinia(...) so real store actions execute against the active testing
Pinia.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9cebec95-3f37-4241-a395-bd190744046f
📒 Files selected for processing (2)
src/platform/workspace/stores/billingOperationStore.test.tssrc/platform/workspace/stores/billingOperationStore.ts
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #14705 +/- ##
========================================
Coverage 78.28% 78.28%
========================================
Files 1790 1790
Lines 108911 108916 +5
Branches 29888 30771 +883
========================================
+ Hits 85258 85265 +7
+ Misses 23163 23161 -2
Partials 490 490
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
…test Matches the documented convention in docs/testing/vitest-patterns.md. All 48 tests in the file pass unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dante01yoon
left a comment
There was a problem hiding this comment.
Found one telemetry inconsistency in the superseded downgrade path. The toast behavior itself is narrow and the regression test covers the primary subscription-operation event.
A superseded operation emits two failure events when it carries downgrade-to-personal metadata. Only the generic one was reclassified, so a replaced downgrade checkout kept reporting `unknown` and inflating the unexplained-failure count this change exists to reduce. Hoist the category and use it for both emissions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/platform/workspace/stores/billingOperationStore.test.ts`:
- Around line 489-498: Update the billing failure telemetry assertions in the
relevant test to explicitly verify that the primary generic operation event uses
the stale_operation failure_category, while retaining the downgrade_to_personal
assertion and superseded-checkout failure coverage.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5851a72a-7590-4038-aca3-1b838368712d
📒 Files selected for processing (2)
src/platform/workspace/stores/billingOperationStore.test.tssrc/platform/workspace/stores/billingOperationStore.ts
The test asserted only the downgrade emission plus a negative on "unknown", so a primary event carrying some other wrong category still passed. Assert it explicitly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves the MERGE_CONFLICT after a push to main. Two things needed manual resolution beyond git's automatic merge: - billingOperationStore.test.ts had a marked conflict: both branches added independent new tests at the same insertion point (this PR's topup/cancel poll-failure-category tests, and main's checkout-superseded tests from #14705). Kept all four. - billingOperationStore.ts's handleFailure() auto-merged without conflict markers but was left with a duplicate `const failureCategory` declaration (main added superseded-checkout handling in the same spot this PR added categorizePollFailure()). Combined both: `stale_operation` for superseded checkouts, else categorizePollFailure() for provider_decline/api_rejected, with main's toast-suppression for superseded checkouts preserved. - PricingTable.test.ts's own new test referenced the pre-rename mock variable name `mockIsActiveSubscription`; main renamed it to `mockCanAccessSubscriptionFeatures` (#11464). Mechanical rename fix. Verified: pnpm typecheck and pnpm lint pass clean; targeted pnpm vitest run across all 12 files touched by the merge (312 tests) pass. Committed with --no-verify: husky's pre-commit hook (lint-staged) sweeps the full merge diff and fails on a pre-existing, unrelated oxlint error already on main (apps/website .../modelLaunchPages.test.ts, untouched by this merge) — approved by task owner to bypass for this merge commit.
What and why
Picking a different plan part-way through checkout makes the API replace the pending subscription operation with a new one. The replaced operation terminates as
failed— it never completed — even though nothing actually went wrong. The store treated that like any other failure and showed "We couldn't update your subscription. Please try again." while the replacement checkout was still running and about to succeed.Users hit this by starting a subscription, changing their mind, and picking a different plan: an error toast, immediately followed by a working subscription.
The change
handleFailurenow recognises the API'scheckout_supersededfailure reason and skips the error toast for it. The operation is still recorded as failed and still resolves normally — only the user-facing alarm goes away.The failure is also reported as
stale_operationinstead ofunknown, so a routine plan switch stops inflating the count of unexplained billing failures.stale_operationis an existing category; no telemetry types changed.Takes effect once the API returns
checkout_supersededfor a replaced checkout.Scope
One
ifand one telemetry value. Deliberately narrow:Verification
vitest run src/platform/workspace/stores/billingOperationStore.test.ts src/platform/workspace/composables/useSubscriptionCheckout.test.ts --coverage— 144 passed, run with coverage to match CI.stale_operation.pnpm typecheck,oxfmt --check,eslint— all clean.🤖 Generated with Claude Code