fix(settings): prevent paying users hitting 409 on stale Upgrade CTA (#3349)

* fix(settings): stop paying users hitting 409 on stale Upgrade CTA

UnifiedSettings.renderUpgradeSection branched on `isEntitled()`, which
returns false while the Convex entitlement snapshot is still null during
a cold WebSocket load. The modal's `onEntitlementChange` listener only
re-rendered the `api-keys` tab, so the stale "Upgrade to Pro" button in
the Settings tab was never replaced once the snapshot arrived.

Paying users who opened settings in that window saw "Upgrade to Pro",
clicked it, hit `/api/create-checkout`, got 409 `duplicate_subscription`,
and cascaded into the billing-portal fallback path (which itself can
dead-end on a generic Dodo login). Same class of bug as the 2026-04-17/18
panel-overlay duplicate-subscription incident called out in
panel-gating.ts:20-22 -- different surface, same race.

Three-part fix in src/components/UnifiedSettings.ts:

- renderUpgradeSection() returns a hidden wrapper for non-Dodo premium
  (API key / tester key / Clerk pro role) so those users don't get
  stuck on the loading placeholder indefinitely.
- A signed-in user whose Convex entitlement snapshot is still null gets
  a neutral loading placeholder, not "Upgrade to Pro". An early click
  can no longer submit before Convex hydrates.
- open()'s onEntitlementChange handler now swaps the .upgrade-pro-section
  element in place when the snapshot arrives. Click handlers are
  delegated at overlay level, so replacing the node needs no rebind.

Observed signal behind this fix:
- WORLDMONITOR-NY (2026-04-23): Checkout error: duplicate_subscription
- WORLDMONITOR-NZ (2026-04-23): getCustomerPortalUrl Server Error, same
  session, 4s later, triggered by the duplicate-subscription dialog.

* fix(settings): hide loading placeholder to avoid empty bordered card

The base .upgrade-pro-section CSS (main.css:22833) applies margin, padding,
border, and surface background. Without 'hidden', the empty loading
placeholder paints a visibly blank bordered box during the Convex
cold-load window — swapping one bad state (stale 'Upgrade to Pro') for
another (confusing empty card).

Adding 'hidden' lets the browser's default [hidden] { display: none }
suppress the card entirely. Element stays queryable for the replaceWith
swap in open(), so the onEntitlementChange listener still finds it.

* fix(settings): bounded readiness window + click-time isEntitled guard

Addresses P1 review on PR #3349: the initial fix treated
getEntitlementState() === null as "still loading", but null is ALSO a
terminal state when Convex is disabled (no VITE_CONVEX_URL), auth times
out at waitForConvexAuth (10s), or initEntitlementSubscription throws
(entitlements.ts:41,47,58,78). In those cases a signed-in free user
would have seen a permanently empty placeholder instead of the Upgrade
to Pro CTA — a real regression on the main conversion surface.

Changes in src/components/UnifiedSettings.ts:

- Add `entitlementReady` class flag + `entitlementReadyTimer`. The flag
  flips true on first snapshot OR after a 12s fallback timer kicks in.
  12s > the 10s waitForConvexAuth timeout so healthy-but-slow paths land
  on the real state before the fallback fires.
- Seed `entitlementReady = getEntitlementState() !== null` BEFORE the
  first render() so the initial paint branches on the current snapshot,
  not the stale value from a prior open/close cycle.
- renderUpgradeSection() now gates the loading placeholder on
  `!this.entitlementReady` so the signed-in-free branch eventually
  renders the Upgrade CTA even when Convex never hydrates.
- handleUpgradeClick() defensively re-checks isEntitled() at click time:
  if the snapshot arrives AFTER the 12s timer but BEFORE the user's
  click, route to the billing portal instead of triggering
  /api/create-checkout against an active subscription (which would 409
  and re-enter the exact duplicate_subscription → getCustomerPortalUrl
  cascade this PR is trying to eliminate).
- Extract replaceUpgradeSection() helper so both the listener and the
  fallback timer share the same in-place swap path.
- close() clears the timer.

* fix(settings): clear entitlementReadyTimer in destroy()

Mirror the close() cleanup. Without this, if destroy() is called during
the 12s fallback window the timer fires after teardown and invokes
replaceUpgradeSection() against a detached overlay. The early-return
inside replaceUpgradeSection (querySelector returns null) makes the
callback a no-op, but the stray async callback + DOM reference stay
alive until fire — tidy them up at destroy time.
This commit is contained in:
Elie Habib
2026-04-23 21:00:55 +04:00
committed by GitHub
parent 38218db7cd
commit fc94829ce7

View File

@@ -11,7 +11,8 @@ import { renderPreferences } from '@/services/preferences-content';
import { renderNotificationsSettings, type NotificationsSettingsResult } from '@/services/notifications-settings'; import { renderNotificationsSettings, type NotificationsSettingsResult } from '@/services/notifications-settings';
import { getAuthState } from '@/services/auth-state'; import { getAuthState } from '@/services/auth-state';
import { track } from '@/services/analytics'; import { track } from '@/services/analytics';
import { isEntitled, hasFeature, onEntitlementChange } from '@/services/entitlements'; import { isEntitled, hasFeature, onEntitlementChange, getEntitlementState } from '@/services/entitlements';
import { hasPremiumAccess } from '@/services/panel-gating';
import { getSubscription, openBillingPortal, prereserveBillingPortalTab } from '@/services/billing'; import { getSubscription, openBillingPortal, prereserveBillingPortalTab } from '@/services/billing';
import { createApiKey, listApiKeys, revokeApiKey, type ApiKeyInfo } from '@/services/api-keys'; import { createApiKey, listApiKeys, revokeApiKey, type ApiKeyInfo } from '@/services/api-keys';
@@ -62,6 +63,14 @@ export class UnifiedSettings {
private apiKeysError = ''; private apiKeysError = '';
private newlyCreatedKey: string | null = null; private newlyCreatedKey: string | null = null;
private unsubscribeEntitlement: (() => void) | null = null; private unsubscribeEntitlement: (() => void) | null = null;
// Bounded "entitlement snapshot might still arrive" window. Starts false
// on open() when currentState is null, flips true on first snapshot OR
// after a fallback timeout so signed-in free users aren't stranded on an
// empty placeholder when Convex is disabled / auth times out / init
// silently fails (all of which leave currentState === null forever — see
// src/services/entitlements.ts:41,47,58,78).
private entitlementReady = false;
private entitlementReadyTimer: ReturnType<typeof setTimeout> | null = null;
constructor(config: UnifiedSettingsConfig) { constructor(config: UnifiedSettingsConfig) {
this.config = config; this.config = config;
@@ -221,6 +230,10 @@ export class UnifiedSettings {
public open(tab?: TabId): void { public open(tab?: TabId): void {
if (tab) this.activeTab = tab; if (tab) this.activeTab = tab;
this.resetPanelDraft(); this.resetPanelDraft();
// Seed entitlementReady BEFORE render() so the first paint of
// renderUpgradeSection branches on the current snapshot state, not the
// stale value left over from a previous open/close cycle.
this.entitlementReady = getEntitlementState() !== null;
this.render(); this.render();
this.overlay.classList.add('active'); this.overlay.classList.add('active');
localStorage.setItem('wm-settings-open', '1'); localStorage.setItem('wm-settings-open', '1');
@@ -232,6 +245,7 @@ export class UnifiedSettings {
// delivers data, so a paid API Starter user sees the upgrade CTA briefly). // delivers data, so a paid API Starter user sees the upgrade CTA briefly).
this.unsubscribeEntitlement?.(); this.unsubscribeEntitlement?.();
this.unsubscribeEntitlement = onEntitlementChange(() => { this.unsubscribeEntitlement = onEntitlementChange(() => {
this.entitlementReady = true;
const panel = this.overlay.querySelector<HTMLElement>('[data-panel-id="api-keys"]'); const panel = this.overlay.querySelector<HTMLElement>('[data-panel-id="api-keys"]');
if (panel) { if (panel) {
panel.innerHTML = this.renderApiKeysContent(); panel.innerHTML = this.renderApiKeysContent();
@@ -241,7 +255,38 @@ export class UnifiedSettings {
void this.loadApiKeys(); void this.loadApiKeys();
} }
} }
this.replaceUpgradeSection();
}); });
// Bounded fallback: the entitlement listener can legitimately never
// fire (no VITE_CONVEX_URL, Convex API fails to load, waitForConvexAuth
// times out at 10s, or init throws — see entitlements.ts:41,47,58,78).
// Without this timer, the signed-in-free branch of renderUpgradeSection
// would show a blank placeholder for the entire session. 12s > the 10s
// auth timeout so the healthy-but-slow path lands on the real state;
// any later path falls back to "Upgrade to Pro" with handleUpgradeClick
// defensively re-checking isEntitled() at click time.
if (this.entitlementReadyTimer) clearTimeout(this.entitlementReadyTimer);
if (!this.entitlementReady) {
this.entitlementReadyTimer = setTimeout(() => {
this.entitlementReadyTimer = null;
if (this.entitlementReady) return;
this.entitlementReady = true;
this.replaceUpgradeSection();
}, 12_000);
}
}
/**
* Swap the .upgrade-pro-section wrapper in place. Click handlers are
* delegated at overlay level, so replacing the node needs no rebind.
*/
private replaceUpgradeSection(): void {
const upgradeSection = this.overlay.querySelector('.upgrade-pro-section');
if (!upgradeSection) return;
const fresh = document.createElement('template');
fresh.innerHTML = this.renderUpgradeSection().trim();
const next = fresh.content.firstElementChild;
if (next) upgradeSection.replaceWith(next);
} }
public close(): void { public close(): void {
@@ -254,6 +299,10 @@ export class UnifiedSettings {
this.pendingNotifs = null; this.pendingNotifs = null;
this.unsubscribeEntitlement?.(); this.unsubscribeEntitlement?.();
this.unsubscribeEntitlement = null; this.unsubscribeEntitlement = null;
if (this.entitlementReadyTimer) {
clearTimeout(this.entitlementReadyTimer);
this.entitlementReadyTimer = null;
}
this.resetPanelDraft(); this.resetPanelDraft();
localStorage.removeItem('wm-settings-open'); localStorage.removeItem('wm-settings-open');
document.removeEventListener('keydown', this.escapeHandler); document.removeEventListener('keydown', this.escapeHandler);
@@ -283,6 +332,15 @@ export class UnifiedSettings {
this.pendingNotifs = null; this.pendingNotifs = null;
this.unsubscribeEntitlement?.(); this.unsubscribeEntitlement?.();
this.unsubscribeEntitlement = null; this.unsubscribeEntitlement = null;
// Mirror close() — without this, a destroy() during the 12s fallback
// window leaves the timer live; it fires after teardown and calls
// replaceUpgradeSection() against a detached overlay (no-op via the
// querySelector early return, but a stray async callback + DOM
// reference alive longer than intended).
if (this.entitlementReadyTimer) {
clearTimeout(this.entitlementReadyTimer);
this.entitlementReadyTimer = null;
}
document.removeEventListener('keydown', this.escapeHandler); document.removeEventListener('keydown', this.escapeHandler);
this.overlay.remove(); this.overlay.remove();
} }
@@ -424,6 +482,34 @@ export class UnifiedSettings {
} }
private renderUpgradeSection(): string { private renderUpgradeSection(): string {
// Non-Dodo premium (API key / tester key / Clerk pro role without a
// Convex subscription): neither "Upgrade" nor "Manage Billing" is
// actionable. Checked FIRST so these users don't get stuck on the
// loading placeholder below — their Convex entitlement snapshot may
// never arrive at all.
if (!isEntitled() && hasPremiumAccess()) {
return '<div class="upgrade-pro-section upgrade-pro-hidden" hidden></div>';
}
// Signed-in user whose Convex entitlement snapshot has not arrived yet
// AND whose bounded-wait window has not expired. Rendering "Upgrade to
// Pro" in this window is how paying users click through to
// /api/create-checkout and hit 409 duplicate_subscription — same race
// as the 2026-04-17/18 panel-overlay incident fixed in panel-gating.ts,
// different surface. The entitlementReady flag is flipped either by
// the onEntitlementChange listener (healthy path) or by a 12s fallback
// timer in open() (Convex-disabled / auth-timeout / init-fail paths
// where currentState would otherwise stay null forever and strand a
// signed-in free user on an empty placeholder).
if (!this.entitlementReady && getAuthState().user && getEntitlementState() === null) {
// `hidden` so the browser's default `[hidden] { display: none }`
// suppresses the empty card — without it, the base `.upgrade-pro-
// section` styles (margin + padding + border + surface background
// in main.css:22833) paint a visibly empty bordered box during the
// Convex cold-load window, which is exactly the state we're trying
// to clean up. Element stays queryable for the replaceWith swap in
// open().
return '<div class="upgrade-pro-section upgrade-pro-loading" hidden aria-hidden="true"></div>';
}
if (isEntitled()) { if (isEntitled()) {
const sub = getSubscription(); const sub = getSubscription();
const planName = sub?.displayName ?? 'Pro'; const planName = sub?.displayName ?? 'Pro';
@@ -457,6 +543,27 @@ export class UnifiedSettings {
`; `;
} }
// Fallback branch: 12s timer fired but Convex never delivered a
// snapshot. entitlementReady===true does NOT prove the user is free —
// it just means we've given up waiting. A paying user whose auth/query
// is simply very slow (beyond the 10s waitForConvexAuth timeout) would
// otherwise race into in-modal startCheckout here and reproduce the
// 409 duplicate_subscription cascade this PR exists to eliminate.
// Render the card with a plain anchor to /pro instead: /pro has its
// own entitlement gating on fresh page load, and navigating away is a
// no-op for backend subscription state. The `upgrade-pro-cta-link`
// class does NOT match the `.upgrade-pro-cta` delegated click handler
// (line ~95), so the browser handles the navigation natively.
if (getAuthState().user && getEntitlementState() === null) {
return `
<div class="upgrade-pro-section upgrade-pro-fallback">
<div class="upgrade-pro-title">Upgrade to Pro</div>
<div class="upgrade-pro-desc">Unlock all panels, AI analysis, and priority data refresh.</div>
<a class="upgrade-pro-cta-link" href="/pro" target="_blank" rel="noopener">View plans →</a>
</div>
`;
}
return ` return `
<div class="upgrade-pro-section"> <div class="upgrade-pro-section">
<div class="upgrade-pro-title">Upgrade to Pro</div> <div class="upgrade-pro-title">Upgrade to Pro</div>
@@ -467,6 +574,20 @@ export class UnifiedSettings {
} }
private handleUpgradeClick(): void { private handleUpgradeClick(): void {
// Defense in depth: the upgrade CTA can only be clicked when either (a)
// the user is genuinely free-tier, or (b) the 12s fallback timer fired
// before the Convex snapshot arrived. In (b), the snapshot might land
// AFTER the timer but BEFORE the click — re-check isEntitled() here so
// a late-arriving "you're a paying user" state routes to the billing
// portal instead of triggering /api/create-checkout against an active
// subscription (which would 409 and re-enter the duplicate_subscription
// → getCustomerPortalUrl cascade this PR is trying to eliminate).
if (isEntitled()) {
this.close();
const reservedWin = prereserveBillingPortalTab();
void openBillingPortal(reservedWin);
return;
}
this.close(); this.close();
if (this.config.isDesktopApp) { if (this.config.isDesktopApp) {
window.open('https://worldmonitor.app/pro', '_blank'); window.open('https://worldmonitor.app/pro', '_blank');