fix(brief): Latest Brief panel locks out Pro users — gate reads Clerk metadata, not entitlement (#3177)

* fix(brief): Latest Brief panel locks out Pro users — gate reads wrong field

Reported: PR #3166 shipped a WEB_CLERK_PRO_ONLY_PANELS gate that
downgrades the Latest Brief panel to FREE_TIER/ANONYMOUS when the
user isn't Clerk-Pro. The downgrade condition was:

  state.user?.role !== 'pro'

state.user.role is derived from Clerk's publicMetadata.plan via
getCurrentClerkUser(). That field is NOT kept in sync with the
real entitlement for many users — the source of truth is the
Convex entitlements table, not Clerk metadata. Result: a confirmed
Pro user (Convex entitlement.features.tier = 1+) sees every other
premium panel unlock (hasPremiumAccess consults isEntitled() per
PR #3167) but the Latest Brief panel shows 'Upgrade to Pro'.

Fix: swap the condition from 'state.user?.role !== \'pro\'' to
'!hasTier(1)' — the same Convex-backed entitlement check
hasPremiumAccess uses. The panel's own auth subscription keeps the
separate role-based guard inside refresh() as defence in depth
(belt-and-suspenders), but the top-level gating no longer
over-fires on the wrong field.

No new behaviour for users without an entitlement. Typecheck clean.

* fix(brief): panel-side role gate also reads Convex entitlement (not Clerk metadata)

Reviewer caught that the prior PR (#3177) fixed the layout-level gate
but left the panel's own refresh() guard reading authState.user.role
— same stale-publicMetadata bug. A user whose Convex entitlement
says tier=1 but whose Clerk publicMetadata.plan is unset would
unlock past the layout gate (now correct) and then still hit the
panel's local renderUpgradeRequired() path.

Fix: swap the local role check to hasTier(1) — the same Convex
snapshot the layout now consults. Now BOTH gates agree on the
source of truth.

* fix(brief): defer Pro gate when entitlement snapshot hasn't arrived yet

Review flagged a transient 'Upgrade to Pro' flash for Pro users on
initial load. The auth-state subscription can fire before the Convex
entitlement snapshot arrives; hasTier(1) returns false by default
when currentState is null, so a Pro user briefly sees the upgrade
overlay until onEntitlementChange re-runs the gate with the real
snapshot.

Fix: treat 'entitlement not yet loaded' as distinct from 'free user'.
Both panel-layout.ts gate AND LatestBriefPanel.refresh() now check
getEntitlementState() !== null before applying the Clerk-Pro-only
downgrade. During the unknown window the panel stays in its loading
state; the onEntitlementChange listener re-runs updatePanelGating
once the snapshot lands and either unlocks or gates correctly.

No behaviour change for free users (entitlement snapshot arrives
with tier=0, still correctly gates). No behaviour change for the
steady-state Pro case. Only the cold-start window differs: flash
of upgrade-overlay → clean loading state.

* fix(brief): drop client entitlement gate from panel refresh — let server decide

Reviewer's sharper read on PR #3177: the prior 'defer-if-unknown' fix
still blocks Pro users whenever the Convex entitlement subscription
is late, skipped, or failed to establish. getEntitlementState() can
stay null indefinitely if the Convex client auth never connects;
hasTier(1) would stay false; the panel would stay on renderLoading()
forever and the server-side /api/latest-brief check would never even
fire.

The correct architecture: the server is authoritative. /api/latest-
brief already does its own entitlement check against the Clerk JWT.
Client-side entitlement is a fast-path optimisation, never a gate.

Fix: switch both call sites to AFFIRMATIVE DENIAL ONLY.

  LatestBriefPanel.refresh()
    Before: if snapshot null -> renderLoading (fetch never fires);
            if snapshot + free -> renderUpgradeRequired.
    After:  if snapshot != null AND !hasTier(1) -> renderUpgradeRequired.
            Otherwise fall through and FIRE THE FETCH. The 403 path
            (BriefAccessError 'upgrade_required') already renders the
            upgrade CTA when the server says free.

  panel-layout.ts updatePanelGating
    Already shaped as affirmative-denial (snapshot != null AND !hasTier).
    Updated the comment to make the invariant explicit so a future
    refactor doesn't flip it back to positive-gating.

Consequence: an API-key-only user with a free Clerk account will
fire one doomed fetch per refresh and see renderUpgradeRequired a
beat later than before. Accepted — the alternative locked legitimate
Pro users out whenever Convex was anything other than perfectly
healthy, which is a materially worse failure mode.

Both tsconfigs typecheck clean. No test changes needed — the
BriefAccessError path was already covered by existing tests.
This commit is contained in:
Elie Habib
2026-04-18 20:49:39 +04:00
committed by GitHub
parent b5824d0512
commit 048e5486ac
2 changed files with 40 additions and 13 deletions

View File

@@ -100,7 +100,7 @@ import { CustomWidgetPanel } from '@/components/CustomWidgetPanel';
import { openWidgetChatModal } from '@/components/WidgetChatModal';
import { loadWidgets, saveWidget } from '@/services/widget-store';
import type { CustomWidgetSpec } from '@/services/widget-store';
import { initEntitlementSubscription, destroyEntitlementSubscription, isEntitled, onEntitlementChange, shouldReloadOnEntitlementChange } from '@/services/entitlements';
import { initEntitlementSubscription, destroyEntitlementSubscription, isEntitled, hasTier, getEntitlementState, onEntitlementChange, shouldReloadOnEntitlementChange } from '@/services/entitlements';
import { initSubscriptionWatch, destroySubscriptionWatch } from '@/services/billing';
import { getUserId } from '@/services/user-identity';
import { initPaymentFailureBanner } from '@/components/payment-failure-banner';
@@ -310,15 +310,29 @@ export class PanelLayoutManager implements AppModule {
const isPremium = WEB_PREMIUM_PANELS.has(key);
let reason = getPanelGateReason(state, isPremium);
// Clerk-pro-only panels: even when hasPremiumAccess() returns true
// via API/tester key, these panels cannot function without a Clerk
// userId bound to a PRO plan. Downgrade the gate reason so the user
// sees the correct CTA (sign-in or upgrade) instead of an unlocked
// panel that then fails to fetch.
// Clerk-pro-only panels: even when hasPremiumAccess() returns
// true via API/tester key, these panels need a Clerk userId
// bound to a PRO entitlement. We DO NOT trust client-side
// entitlement state as an authoritative gate — the server-side
// /api/latest-brief check is authoritative. We only downgrade
// the gate reason here as AFFIRMATIVE DENIAL: when we KNOW
// (snapshot loaded AND tier < 1) the user is free. In every
// other case — snapshot not yet loaded, Convex subscription
// skipped, transient failure — we leave the panel unlocked
// and let the server 403 path drive the upgrade CTA inside
// the panel's refresh() catch block.
//
// Prior iterations of this code tried the opposite — gating
// positively on hasTier(1) — and locked legitimate Pro users
// out whenever the Convex snapshot was late, skipped, or
// failed. Affirmative-denial-only is the right shape: never
// over-gate, accept the one-doomed-fetch-per-session cost
// for API-key-only + free-Clerk users as the lesser harm.
if (
reason === PanelGateReason.NONE &&
WEB_CLERK_PRO_ONLY_PANELS.has(key) &&
state.user?.role !== 'pro'
getEntitlementState() !== null &&
!hasTier(1)
) {
reason = state.user ? PanelGateReason.FREE_TIER : PanelGateReason.ANONYMOUS;
}

View File

@@ -23,6 +23,7 @@ import { Panel } from './Panel';
import { getClerkToken, clearClerkTokenCache } from '@/services/clerk';
import { PanelGateReason, hasPremiumAccess } from '@/services/panel-gating';
import { getAuthState, subscribeAuthState } from '@/services/auth-state';
import { hasTier, getEntitlementState } from '@/services/entitlements';
import { h, rawHtml, replaceChildren, clearChildren } from '@/utils/dom-utils';
interface LatestBriefReady {
@@ -182,12 +183,24 @@ export class LatestBriefPanel extends Panel {
this.renderSignInRequired();
return;
}
// Mixed-auth edge case: desktop/tester keys open the panel even
// when the signed-in Clerk account is FREE. /api/latest-brief
// verifies entitlement from the JWT's userId and returns 403
// for free accounts. Render the upgrade CTA locally instead of
// bouncing through a doomed fetch.
if (authState.user?.role !== 'pro') {
// Client-side entitlement is NOT authoritative. /api/latest-brief
// does its own server-side entitlement check against the Clerk
// JWT — that IS the source of truth. We only use the client
// snapshot for AFFIRMATIVE DENIAL: skip the doomed fetch when
// we KNOW the user is free. If the snapshot is missing, stale,
// or the Convex subscription failed to establish, we fall
// through and let the server decide. The server's 403 response
// is translated to renderUpgradeRequired() in the catch block
// below (via BriefAccessError).
//
// Consequence: an API-key-only user with a free Clerk account
// will fire one doomed fetch per refresh and see the upgrade
// CTA a beat later than they would with a client-side gate.
// Accepted — the alternative (trusting the client snapshot as
// a gate) locked legitimate Pro users out whenever the Convex
// entitlement subscription was skipped or failed, which is a
// worse failure mode.
if (getEntitlementState() !== null && !hasTier(1)) {
this.renderUpgradeRequired();
return;
}