From 041f63f43bb1e753461399322d9576e4dc236033 Mon Sep 17 00:00:00 2001 From: alt-glitch Date: Thu, 23 Jul 2026 08:55:37 +0530 Subject: [PATCH] fix(desktop): capability-gate buy row, consistent refill precedence, opt-in fixture switcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review fixes: - buyCreditsDisabledReason gates on can_change_plan (fallback is_admin) — is_admin is the deprecated OWNER/ADMIN display check and wrongly blocked FINANCE_ADMINs (the gateway sends can_change_plan on billing.state). - The no-card auto-refill row applies the same policy-over-card precedence as the buy row, so a policy-blocked org never sees an Add card CTA there. - The no-card banner stops claiming auto-refill is disabled while an enabled config is still running its saved payment method. - Exact-count assertion on the Add card buttons in the component test. And the fixture preview dropdown is now opt-in even in dev (localStorage 'hermes:billing-preview' = '1' + reload) — it cluttered the billing header of every dev session. --- .../src/app/settings/billing/index.test.tsx | 4 +- .../src/app/settings/billing/index.tsx | 14 +++++- .../billing/use-billing-state.test.ts | 45 +++++++++++++++++++ .../app/settings/billing/use-billing-state.ts | 18 ++++++-- 4 files changed, 74 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/app/settings/billing/index.test.tsx b/apps/desktop/src/app/settings/billing/index.test.tsx index 91171e018ba..f5539596897 100644 --- a/apps/desktop/src/app/settings/billing/index.test.tsx +++ b/apps/desktop/src/app/settings/billing/index.test.tsx @@ -675,8 +675,8 @@ describe('BillingSettings', () => { expect( screen.getByText('Buying top-up credits and auto-refill stay disabled until a card is on file. Add one on the portal.') ).toBeTruthy() - // Banner + buy row + auto-refill row each carry the fix. - expect(screen.getAllByRole('button', { name: /Add card/ }).length).toBeGreaterThan(0) + // Banner + buy row + auto-refill row each carry the fix — exactly three. + expect(screen.getAllByRole('button', { name: /Add card/ })).toHaveLength(3) }) it('does not show the no-card notice when a card is on file', async () => { diff --git a/apps/desktop/src/app/settings/billing/index.tsx b/apps/desktop/src/app/settings/billing/index.tsx index 040d37b3b43..2b6e2d09ca2 100644 --- a/apps/desktop/src/app/settings/billing/index.tsx +++ b/apps/desktop/src/app/settings/billing/index.tsx @@ -575,8 +575,20 @@ function BillingSettingsWithDevFixtures() { ) } +// The fixture switcher is opt-in even in dev — it cluttered the billing header of +// every dev session. Summon it for a visual-QA pass with +// `localStorage.setItem('hermes:billing-preview', '1')` + reload (see the billing +// revamp plan doc's runbook); production builds compile the whole path out. +function billingPreviewEnabled(): boolean { + try { + return localStorage.getItem('hermes:billing-preview') === '1' + } catch { + return false + } +} + export function BillingSettings() { - if (import.meta.env.DEV) { + if (import.meta.env.DEV && billingPreviewEnabled()) { return } diff --git a/apps/desktop/src/app/settings/billing/use-billing-state.test.ts b/apps/desktop/src/app/settings/billing/use-billing-state.test.ts index a2259d1c333..22c681cf44a 100644 --- a/apps/desktop/src/app/settings/billing/use-billing-state.test.ts +++ b/apps/desktop/src/app/settings/billing/use-billing-state.test.ts @@ -183,6 +183,19 @@ describe('deriveBillingView', () => { }) }) + it('drops the auto-refill claim from the banner while auto-refill is actually running', () => { + const view = deriveBillingView( + okBilling({ + ...postTrainBillingState, + auto_reload: { ...postTrainBillingState.auto_reload, enabled: true }, + card: null + }), + okSubscription(postTrainSubscriptionState) + ) + + expect(view.notice?.message).toBe('Buying top-up credits stays disabled until a card is on file. Add one on the portal.') + }) + it('keeps an enabled auto-refill config manageable even after the card disappears', () => { const view = deriveBillingView( okBilling({ @@ -210,6 +223,38 @@ describe('deriveBillingView', () => { expect(view.topupRow?.description).not.toContain('single charge') }) + it('applies the same policy precedence to the no-card auto-refill row', () => { + const view = deriveBillingView( + okBilling({ ...postTrainBillingState, can_charge: false, card: null }), + okSubscription(postTrainSubscriptionState) + ) + + // Policy-blocked → the plain off row (portal caption), never Add card. + expect(view.refillRow?.action).toMatchObject({ label: 'Turn on' }) + expect(view.refillRow?.caption).toBe('Turn on auto-refill from the portal.') + }) + + it('gates the buy row on can_change_plan, not the deprecated is_admin role check', () => { + // A FINANCE_ADMIN: legacy is_admin=false but server-granted capability true. + const view = deriveBillingView( + okBilling({ ...postTrainBillingState, can_change_plan: true, card: null, is_admin: false }), + okSubscription(postTrainSubscriptionState) + ) + + expect(view.topupRow).toMatchObject({ action: { label: 'Add card' }, value: 'No card on file' }) + }) + + it('falls back to is_admin when the server omits can_change_plan', () => { + const view = deriveBillingView( + okBilling({ ...postTrainBillingState, can_change_plan: undefined, card: null, is_admin: false }), + okSubscription(postTrainSubscriptionState) + ) + + // Legacy payloads keep the old member gating. + expect(view.topupRow?.action).toBeUndefined() + expect(view.topupRow?.description).toContain('admin') + }) + it('links the off-with-card auto-refill row to the portal it names', () => { const view = deriveBillingView(okBilling(postTrainBillingState), okSubscription(postTrainSubscriptionState)) diff --git a/apps/desktop/src/app/settings/billing/use-billing-state.ts b/apps/desktop/src/app/settings/billing/use-billing-state.ts index 2581c0fdaea..effb58c08a0 100644 --- a/apps/desktop/src/app/settings/billing/use-billing-state.ts +++ b/apps/desktop/src/app/settings/billing/use-billing-state.ts @@ -313,7 +313,11 @@ function noCardNotice(billing: BillingStateResponse): BillingNoticeView | undefi return { action: { label: 'Add card', url: billing.portal_url ?? FALLBACK_PORTAL_BILLING_URL }, - message: 'Buying top-up credits and auto-refill stay disabled until a card is on file. Add one on the portal.', + // An already-enabled auto-refill keeps running its saved payment method, so + // the banner must not claim it is disabled in that state. + message: billing.auto_reload?.enabled + ? 'Buying top-up credits stays disabled until a card is on file. Add one on the portal.' + : 'Buying top-up credits and auto-refill stay disabled until a card is on file. Add one on the portal.', title: 'No payment method on file', tone: 'warn' } @@ -603,8 +607,10 @@ function autoReloadRow(billing: BillingStateResponse): BillingAccountRowView { // choice the user can change — name the blocker on the row and offer the fix // instead of a dead Manage button or a link-less portal caption. An enabled // config (card removed later) falls through to its normal row so it can still - // be managed / turned off. - if (!billing.card && !autoReload?.enabled) { + // be managed / turned off. Policy blockers outrank the missing card here too + // (same precedence as the buy row): a member or a spending-disabled org falls + // through to the plain portal rows below, never an Add card call-to-action. + if (!billing.card && !autoReload?.enabled && !buyCreditsDisabledReason(billing)) { return { action: { label: 'Add card', url: portalUrl }, caption: 'Needs a card on file before it can be turned on.', @@ -775,7 +781,11 @@ function topupCreditsValue(billing: BillingStateResponse, usage?: UsageModelData } function buyCreditsDisabledReason(billing: BillingStateResponse): null | string { - if (!billing.is_admin) { + // Capability, not the legacy role check: `is_admin` is OWNER/ADMIN only and + // deprecated for gating (a FINANCE_ADMIN charges via can_change_plan). The + // gateway sends can_change_plan; fall back to is_admin when it's absent, + // mirroring agent/billing_view.py. + if (!(billing.can_change_plan ?? billing.is_admin)) { return resolveRefusal({ kind: 'role_required', message: '' }).message }