From 691faf41d0391d1a6230eb646f6a101c0eca45c1 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Thu, 13 Aug 2026 13:55:58 +0300 Subject: [PATCH 1/6] feat(billing): AIT-398 change plan in the terminal for existing subscribers `billing upgrade` used to punt any org with a paid plan to the browser Billing page, which meant a CLI-only user hit a sign-in wall for a change that needs no browser at all. It now uses the same two endpoints the dashboard's confirm dialog uses: usage-tier/preview for the billing effect, usage-tier to apply it. The terminal states the prorated charge and next-bill date for an upgrade, or that a downgrade starts at period end and charges nothing now, then asks for confirmation. Every claim comes from the preview; nothing is guessed. The interval is inherited from the subscription (monthly <-> annual stays on the Billing page), and the plan the org is already on is dropped from the picker. Custom plans, a pending cancellation, and an already-scheduled plan change still open the Billing page, which surfaces those states. --- CHANGELOG.md | 6 + package-lock.json | 4 +- package.json | 2 +- src/__tests__/billing.test.ts | 44 ++++-- src/commands/__tests__/billing.test.ts | 158 +++++++++++++++++--- src/commands/billing.ts | 198 ++++++++++++++++++++----- 6 files changed, 343 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83dfd84..125c282 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ All notable changes to `@gethookmyapp/cli` are documented here. +## 0.14.15 — 2026-08-13 + +### Changed + +- `hookmyapp billing upgrade` now changes the plan of an existing subscription in the terminal: it states what you'll be charged today and when your next bill lands, asks for confirmation, and applies the change. No browser, and no sign-in for a CLI-only user. Custom plans, a pending cancellation, and an already-scheduled plan change still open the Billing page (AIT-398). + ## 0.14.14 — 2026-08-12 ### Added diff --git a/package-lock.json b/package-lock.json index 7ddb61f..6b042b6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@gethookmyapp/cli", - "version": "0.14.12", + "version": "0.14.15", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@gethookmyapp/cli", - "version": "0.14.12", + "version": "0.14.15", "license": "MIT", "dependencies": { "@inquirer/prompts": "^7.0.0", diff --git a/package.json b/package.json index 2770b1f..1f2ce4c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@gethookmyapp/cli", - "version": "0.14.14", + "version": "0.14.15", "description": "HookMyApp CLI, No BS. Just go live.", "type": "module", "bin": { diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 8d91297..88ad98e 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -15,7 +15,7 @@ vi.mock('open', () => ({ })); // Mock @inquirer/prompts -vi.mock('@inquirer/prompts', () => ({ select: vi.fn() })); +vi.mock('@inquirer/prompts', () => ({ select: vi.fn(), confirm: vi.fn() })); // Mock workspace config vi.mock('../commands/workspace.js', () => ({ @@ -42,6 +42,7 @@ const WORKSPACE_ID = 'ws_TEST0070'; const ORG_PUBLIC_ID = 'org_abc12345'; const SUBSCRIPTION_PATH = `/organizations/${ORG_PUBLIC_ID}/billing/subscription`; const CHECKOUT_PATH = `/organizations/${ORG_PUBLIC_ID}/billing/checkout`; +const BILLING_BASE = `/organizations/${ORG_PUBLIC_ID}/billing`; const WORKSPACES = [{ id: WORKSPACE_ID, name: 'Acme', organizationPublicId: ORG_PUBLIC_ID }]; // Phase A backend DTO cleanup: planSlug + stripeSubscriptionId removed. @@ -267,6 +268,9 @@ describe('billing commands', () => { const PLANS_CATALOG = [ { slug: 'free', name: 'Launch', messages: 2000, priceInCents: 0, annualPriceInCents: 0 }, + // A second paid plan so the paid-tier path (which drops the plan the org + // is already on) still has something to offer. + { slug: 'starter', name: 'Build', messages: 600, priceInCents: 1200, annualPriceInCents: 12000 }, { slug: 'growth', name: 'Scale', messages: 1200, priceInCents: 2400, annualPriceInCents: 24000 }, ]; @@ -280,22 +284,44 @@ describe('billing commands', () => { }); } - it('opens the app Billing page (not /stripe/portal) when user has active subscription', async () => { - mockSubAndWorkspaces({ status: 'active', plan: { slug: 'growth', name: 'Scale' } }); + /** Paid tiers change plan in the terminal (AIT-398): preview → confirm → + * apply, no browser. Declines the confirmation so the assertion is about + * which path ran, not about applying a change. */ + async function runPaidPathDeclining(status: string): Promise { + const origTTY = process.stdout.isTTY; + process.stdout.isTTY = true; + const inq = await import('@inquirer/prompts'); + vi.mocked(inq.select).mockResolvedValueOnce('starter' as never); + vi.mocked(inq.confirm).mockResolvedValueOnce(false as never); + mockedApiClient.mockImplementation(async (path: string) => { + if (path === SUBSCRIPTION_PATH) return { status, plan: { slug: 'growth', name: 'Scale' } }; + if (path === '/workspaces') return WORKSPACES; + if (path === '/plans') return PLANS_CATALOG; + if (path === `${BILLING_BASE}/usage-tier/preview`) return { scheduled: true }; + throw new Error(`unexpected path: ${path}`); + }); - await billingUpgrade(); + try { + await billingUpgrade(); + } finally { + process.stdout.isTTY = origTTY; + } + } - expect(mockedOpen).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); + it('changes plan in the terminal when the user has an active subscription (no browser)', async () => { + await runPaidPathDeclining('active'); + + expect(mockedOpen).not.toHaveBeenCalled(); const paths = mockedApiClient.mock.calls.map((c) => c[0]); + expect(paths).toContain(`${BILLING_BASE}/usage-tier/preview`); expect(paths).not.toContain('/stripe/portal'); }); it('treats past_due on a paid plan as a subscriber', async () => { - mockSubAndWorkspaces({ status: 'past_due', plan: { slug: 'growth', name: 'Scale' } }); - - await billingUpgrade(); + await runPaidPathDeclining('past_due'); - expect(mockedOpen).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); + const paths = mockedApiClient.mock.calls.map((c) => c[0]); + expect(paths).toContain(`${BILLING_BASE}/usage-tier/preview`); }); it('rejects --json with UPGRADE_NO_JSON (no machine-readable form)', async () => { diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index ba807aa..81a7541 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -31,15 +31,22 @@ vi.mock('open', () => ({ default: vi.fn(async () => undefined) })); // a hardcoded plan list. let selectCalls: Array<{ choices: unknown }> = []; let selectAnswers: unknown[] = []; +// The paid-tier path (AIT-398) confirms the billing effect before applying it; +// tests queue the y/N answer the same way they queue select answers. +let confirmAnswers: boolean[] = []; vi.mock('@inquirer/prompts', () => ({ select: vi.fn(async (args: { choices: unknown }) => { selectCalls.push(args); return selectAnswers.shift(); }), + confirm: vi.fn(async () => confirmAnswers.shift() ?? false), })); function queueSelectAnswers(...answers: unknown[]): void { selectAnswers = answers; } +function queueConfirmAnswers(...answers: boolean[]): void { + confirmAnswers = answers; +} function capturedSelectCalls(): Array<{ choices: unknown }> { return selectCalls; } @@ -134,46 +141,155 @@ describe('billingManage — opens the app Billing page (portal retired)', () => }); }); -describe('billingUpgrade — active subscription path (portal retired)', () => { +const CATALOG = [ + { slug: 'free', name: 'Launch', messages: 2000, priceInCents: 0, annualPriceInCents: 0 }, + { slug: 'starter', name: 'Build', messages: 30000, priceInCents: 1200, annualPriceInCents: 12000 }, + { slug: 'growth', name: 'Scale', messages: 100000, priceInCents: 2400, annualPriceInCents: 24000, popular: true }, + // Non-whole monthly price (AIT-391): $39.99, not the rounded-to-$40 that + // Math.round(priceInCents / 100) used to render. + { slug: 'pro', name: 'Business', messages: 250000, priceInCents: 3999, annualPriceInCents: 39000 }, +]; + +describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', () => { + // An org on Build, billed monthly, period ending Sep 13. + const SUB = { + status: 'active', + plan: { slug: 'starter', name: 'Build', priceInCents: 1200, annualPriceInCents: 12000 }, + billingInterval: 'monthly', + currentPeriodEnd: '2026-09-13T10:45:22.000Z', + }; + const PREVIEW = '/organizations/org_abc12345/billing/usage-tier/preview'; + const APPLY = '/organizations/org_abc12345/billing/usage-tier'; + + /** Routes by path so a test only states the responses it cares about; + * `overrides` supplies the preview/apply results under test. */ + function mockApi( + overrides: Record, + sub: Record = SUB, + ): void { + vi.mocked(apiClient).mockImplementation(async (path: string) => { + if (path in overrides) return overrides[path]; + if (path === '/workspaces') return workspaces; + if (path === '/organizations/org_abc12345/billing/subscription') return sub; + if (path === '/plans') return CATALOG; + throw new Error(`unexpected path: ${path}`); + }); + } + + let origTTY: typeof process.stdout.isTTY; + let log: ReturnType; beforeEach(() => { vi.mocked(apiClient).mockReset(); vi.mocked(open).mockClear(); + selectCalls = []; + selectAnswers = []; + confirmAnswers = []; process.env.HOOKMYAPP_APP_URL = 'https://app.test'; + origTTY = process.stdout.isTTY; + process.stdout.isTTY = true; + log = vi.spyOn(console, 'log').mockImplementation(() => {}); }); afterEach(() => { delete process.env.HOOKMYAPP_APP_URL; + process.stdout.isTTY = origTTY; + log.mockRestore(); }); - test('When paid tier with active status, then billingUpgrade reads the org subscription route, opens the app Billing page, and never calls a /stripe/* route', async () => { - vi.mocked(apiClient).mockImplementation(async (path: string) => { - if (path === '/organizations/org_abc12345/billing/subscription') { - return { - status: 'active', - plan: { slug: 'launch', name: 'Launch+', priceInCents: 1900, annualPriceInCents: 19000 }, - }; - } - if (path === '/workspaces') return workspaces; - throw new Error(`unexpected path: ${path}`); + function printed(): string { + return log.mock.calls.flat().join('\n'); + } + + test('When upgrading, then the prorated charge is stated and confirming applies the change without a browser', async () => { + mockApi({ + [PREVIEW]: { scheduled: false, amountDueCents: 1200, currency: 'usd' }, + [APPLY]: { scheduled: false }, + }); + queueSelectAnswers('growth'); + queueConfirmAnswers(true); + + await expect(billingUpgrade()).resolves.toBeUndefined(); + + expect(printed()).toContain( + "You'll switch to Scale right away. We'll charge $12.00 today for the rest of this billing period (through Sep 13, 2026). On Sep 13, 2026 your next bill is the full Scale price.", + ); + expect(printed()).toContain('✓ Switched to Scale (100,000 messages/mo)'); + expect(vi.mocked(open)).not.toHaveBeenCalled(); + }); + + test('When the plan list is offered, then the current plan is excluded and the interval is not asked', async () => { + mockApi({ + [PREVIEW]: { scheduled: false, amountDueCents: 1200 }, + [APPLY]: { scheduled: false }, }); + queueSelectAnswers('growth'); + queueConfirmAnswers(true); + + await billingUpgrade(); + + const choices = capturedSelectCalls()[0].choices as Array<{ value: string }>; + expect(choices.map((c) => c.value)).toEqual(['growth', 'pro']); + expect(capturedSelectCalls()).toHaveLength(1); + }); + + test('When the change is a downgrade, then it is described as scheduled and charges nothing now', async () => { + mockApi({ + [PREVIEW]: { scheduled: true }, + [APPLY]: { scheduled: true, effectiveAt: '2026-09-13T10:45:22.000Z' }, + }); + queueSelectAnswers('pro'); + queueConfirmAnswers(true); + + await billingUpgrade(); + + expect(printed()).toContain( + "You'll keep Build and its usage until Sep 13, 2026. Business starts then, and nothing is charged now.", + ); + expect(printed()).toContain('✓ Business starts on Sep 13, 2026.'); + }); + + test('When the confirmation is declined, then nothing is applied', async () => { + mockApi({ [PREVIEW]: { scheduled: false, amountDueCents: 1200 } }); + queueSelectAnswers('growth'); + queueConfirmAnswers(false); await expect(billingUpgrade()).resolves.toBeUndefined(); + const paths = vi.mocked(apiClient).mock.calls.map((c) => String(c[0])); + expect(paths).not.toContain(APPLY); + expect(printed()).toContain('No changes made.'); + }); + + test('When a cancel is already pending, then the Billing page opens instead of promising an immediate switch', async () => { + mockApi({}, { ...SUB, cancelAtPeriodEnd: true }); + + await billingUpgrade(); + expect(vi.mocked(open)).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); const paths = vi.mocked(apiClient).mock.calls.map((c) => String(c[0])); - expect(paths.some((p) => p.startsWith('/stripe/'))).toBe(false); + expect(paths).not.toContain(PREVIEW); + }); + + test('When the org is on a Custom plan, then the Billing page opens (not in the tier catalog)', async () => { + mockApi({}, { ...SUB, plan: { slug: 'custom', name: 'Custom' } }); + + await billingUpgrade(); + + expect(vi.mocked(open)).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); + }); + + test('When no TTY, then it fails with UPGRADE_REQUIRES_TTY and applies nothing', async () => { + process.stdout.isTTY = false; + mockApi({}); + + await expect(billingUpgrade()).rejects.toThrow(/interactive terminal/i); + + const paths = vi.mocked(apiClient).mock.calls.map((c) => String(c[0])); + expect(paths).not.toContain(APPLY); + expect(vi.mocked(open)).not.toHaveBeenCalled(); }); }); describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { - const CATALOG = [ - { slug: 'free', name: 'Launch', messages: 2000, priceInCents: 0, annualPriceInCents: 0 }, - { slug: 'starter', name: 'Build', messages: 30000, priceInCents: 1200, annualPriceInCents: 12000 }, - { slug: 'growth', name: 'Scale', messages: 100000, priceInCents: 2400, annualPriceInCents: 24000, popular: true }, - // Non-whole monthly price (AIT-391): $39.99, not the rounded-to-$40 that - // Math.round(priceInCents / 100) used to render. - { slug: 'pro', name: 'Business', messages: 250000, priceInCents: 3999, annualPriceInCents: 39000 }, - ]; - let origTTY: typeof process.stdout.isTTY; beforeEach(() => { vi.mocked(apiClient).mockReset(); diff --git a/src/commands/billing.ts b/src/commands/billing.ts index a76a6b3..c238534 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -30,6 +30,53 @@ function orgBillingUrl(orgPublicId: string): string { return `${getEffectiveAppUrl()}/org/${orgPublicId}/billing`; } +/** `Sep 13, 2026`, matching the dashboard's confirm dialog. Fixed to en-US so + * the sentence reads the same everywhere the CLI runs. Null for a missing or + * unparseable date — callers drop the clause rather than print "Invalid Date". */ +function formatDate(iso?: string): string | null { + if (!iso) return null; + const d = new Date(iso); + if (Number.isNaN(d.getTime())) return null; + return d.toLocaleDateString('en-US', { month: 'short', day: 'numeric', year: 'numeric' }); +} + +type TierPreview = { scheduled: boolean; amountDueCents?: number; currency?: string }; + +/** The terminal wording of the dashboard's tier-switch dialog (AIT-398). Every + * claim comes from the server's preview: an upgrade states the prorated charge + * it returned, a downgrade (`scheduled`) states that nothing is charged now. + * Clauses that need the period end are dropped when the subscription doesn't + * carry one — never guessed. */ +function describeTierChange(args: { + targetName: string; + currentName: string; + preview: TierPreview; + currentPeriodEnd?: string; +}): string { + const { targetName, currentName, preview } = args; + const on = formatDate(args.currentPeriodEnd); + + if (preview.scheduled) { + return ( + `You'll keep ${currentName} and its usage until ${on ?? 'the end of your billing period'}. ` + + `${targetName} starts then, and nothing is charged now.` + ); + } + + const nextBill = on + ? `On ${on} your next bill is the full ${targetName} price.` + : `Your next bill is the full ${targetName} price.`; + const due = preview.amountDueCents ?? 0; + if (due > 0) { + const through = on ? ` (through ${on})` : ''; + return ( + `You'll switch to ${targetName} right away. We'll charge $${(due / 100).toFixed(2)} today ` + + `for the rest of this billing period${through}. ${nextBill}` + ); + } + return `You'll switch to ${targetName} right away at no extra charge today. ${nextBill}`; +} + const UPGRADE_POLL_INTERVAL_MS = 5_000; const UPGRADE_POLL_HINT_EVERY_MS = 60_000; @@ -139,16 +186,108 @@ export async function billingStatus(opts: { json?: boolean; human?: boolean } = } } +type CatalogPlan = { + slug: string; + name: string; + messages: number; + priceInCents: number; + annualPriceInCents: number; + popular?: true; +}; + +/** Live catalog — the CLI keeps no copy of plan names, limits, or prices; + * hardcoded copies drift and have shipped stale numbers before. A fetch + * failure fails the command; there is deliberately no fallback list. + * `exceptSlug` drops the plan the org is already on. */ +async function fetchPaidPlans(exceptSlug?: string): Promise { + const catalog = (await apiClient('/plans')) as CatalogPlan[]; + const paidPlans = catalog.filter((p) => p.priceInCents > 0 && p.slug !== exceptSlug); + if (paidPlans.length === 0) { + throw new ValidationError('No paid plans available. Try again later.', 'PLANS_EMPTY'); + } + return paidPlans; +} + +function planChoice(p: CatalogPlan): { name: string; value: string; description?: string } { + return { + name: `${p.name}: ${p.messages.toLocaleString('en-US')} messages — ${formatPrice(p.priceInCents)}/mo (or ${formatPrice(p.annualPriceInCents)}/yr)`, + value: p.slug, + ...(p.popular ? { description: 'Most popular' } : {}), + }; +} + +/** Plan change for an org that already pays (AIT-398). Same two endpoints the + * dashboard's confirm dialog uses, so the terminal states the billing effect + * and applies it — no browser, and no login wall for a CLI-only user. */ +async function changePlanInTerminal( + orgPublicId: string, + sub: { + plan: { slug: string; name: string }; + billingInterval?: 'monthly' | 'annual'; + currentPeriodEnd?: string; + }, +): Promise { + const plans = await fetchPaidPlans(sub.plan.slug); + + const { select, confirm } = await import('@inquirer/prompts'); + const planSlug = await select({ + message: 'Choose a plan', + choices: plans.map(planChoice), + }); + const target = plans.find((p) => p.slug === planSlug)!; + // Interval is whatever the subscription already bills on — switching monthly + // ↔ annual is a separate decision and stays on the Billing page. + const billingInterval = sub.billingInterval ?? 'monthly'; + const body = JSON.stringify({ planSlug, billingInterval }); + + const preview = (await apiClient(`/organizations/${orgPublicId}/billing/usage-tier/preview`, { + method: 'POST', + body, + })) as TierPreview; + console.log( + describeTierChange({ + targetName: target.name, + currentName: sub.plan.name, + preview, + currentPeriodEnd: sub.currentPeriodEnd, + }), + ); + + const ok = await confirm({ message: `Switch to ${target.name}?`, default: false }); + if (!ok) { + console.log('No changes made.'); + return; + } + + const result = (await apiClient(`/organizations/${orgPublicId}/billing/usage-tier`, { + method: 'POST', + body, + })) as { scheduled: boolean; effectiveAt?: string }; + + if (result.scheduled) { + const on = formatDate(result.effectiveAt ?? sub.currentPeriodEnd); + console.log( + on + ? `✓ ${target.name} starts on ${on}.` + : `✓ ${target.name} starts at the end of your billing period.`, + ); + return; + } + console.log( + `✓ Switched to ${target.name} (${target.messages.toLocaleString('en-US')} messages/mo).`, + ); +} + export async function billingUpgrade(opts: { json?: boolean } = {}): Promise { - // `billing upgrade` is interactive end-to-end: the free path prompts for a - // plan + interval, and both paths open a browser. There is no machine- - // readable form, so reject --json up front with a clear pointer instead of - // rendering an inquirer prompt that aborts into a generic error in non-TTY - // / --json contexts. + // `billing upgrade` is interactive end-to-end: both paths prompt for a plan + // and confirm before anything is charged. There is no machine-readable form, + // so reject --json up front with a clear pointer instead of rendering an + // inquirer prompt that aborts into a generic error in non-TTY / --json + // contexts. if (opts.json) { throw new ValidationError( - `billing upgrade is interactive (plan selection + browser checkout) and has no --json form. ` + - `Run it without --json from a terminal, or use \`${cliCommandPrefix()} billing manage\` for an existing subscription.`, + `billing upgrade is interactive (plan selection + confirmation) and has no --json form. ` + + `Run it without --json from a terminal, or use \`${cliCommandPrefix()} billing manage\` to open the Billing page.`, 'UPGRADE_NO_JSON', ); } @@ -161,14 +300,7 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise p.priceInCents > 0); - if (paidPlans.length === 0) { - throw new ValidationError('No paid plans available. Try again later.', 'PLANS_EMPTY'); + if (hasActiveSub) { + // States the terminal can't describe honestly stay on the Billing page, + // which surfaces them: a Custom plan isn't in the tier catalog at all, and + // a pending cancel or scheduled plan change would make "you'll switch right + // away" a lie about what the subscription is already doing. + if (sub.plan.slug === 'custom' || sub.cancelAtPeriodEnd === true || sub.pendingPlanChange) { + console.log('Opening your Billing page to update your plan...'); + await open(orgBillingUrl(orgPublicId)); + return; + } + await changePlanInTerminal(orgPublicId, sub); + return; } + const paidPlans = await fetchPaidPlans(); const { select } = await import('@inquirer/prompts'); const planSlug = await select({ message: 'Choose a plan', - choices: paidPlans.map((p) => ({ - name: `${p.name}: ${p.messages.toLocaleString('en-US')} messages — ${formatPrice(p.priceInCents)}/mo (or ${formatPrice(p.annualPriceInCents)}/yr)`, - value: p.slug, - ...(p.popular ? { description: 'Most popular' } : {}), - })), + choices: paidPlans.map(planChoice), }); const billingInterval = await select({ message: 'Billing interval', @@ -252,7 +378,7 @@ export function registerBillingCommand(_program: Command): void { const billingUpgradeCmd = billing .command('upgrade') - .description('Upgrade plan (interactive for free users, opens your Billing page for subscribers)') + .description('Change your plan (interactive)') .action(async () => { const { program: rootProgram } = await import('../index.js'); await billingUpgrade({ json: !!rootProgram.opts().json }); From 8991d93c6a9076fc327d3ca2e90f6ffc989e1554 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Thu, 13 Aug 2026 14:32:46 +0300 Subject: [PATCH 2/6] fix(billing): never assume an interval for a plan change Codex P1 on #58: the paid-tier path defaulted a missing `billingInterval` to monthly, so an annually billed customer could confirm what read as a plan change and have their billing cadence moved to monthly with it. The read model leaves the interval undefined when Stripe can't be reached, so that state now joins the other Billing-page fallbacks instead of being guessed at. Adds tests for the annual pass-through and for the missing interval opening the Billing page. --- src/__tests__/billing.test.ts | 4 +++- src/commands/__tests__/billing.test.ts | 25 +++++++++++++++++++++++++ src/commands/billing.ts | 21 +++++++++++++++------ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 88ad98e..9a50874 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -294,7 +294,9 @@ describe('billing commands', () => { vi.mocked(inq.select).mockResolvedValueOnce('starter' as never); vi.mocked(inq.confirm).mockResolvedValueOnce(false as never); mockedApiClient.mockImplementation(async (path: string) => { - if (path === SUBSCRIPTION_PATH) return { status, plan: { slug: 'growth', name: 'Scale' } }; + if (path === SUBSCRIPTION_PATH) { + return { status, plan: { slug: 'growth', name: 'Scale' }, billingInterval: 'monthly' }; + } if (path === '/workspaces') return WORKSPACES; if (path === '/plans') return PLANS_CATALOG; if (path === `${BILLING_BASE}/usage-tier/preview`) return { scheduled: true }; diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 81a7541..9a8bddc 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -269,6 +269,31 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', expect(paths).not.toContain(PREVIEW); }); + test('When the subscription bills annually, then the change is sent as annual', async () => { + mockApi( + { [PREVIEW]: { scheduled: false, amountDueCents: 0 }, [APPLY]: { scheduled: false } }, + { ...SUB, billingInterval: 'annual' }, + ); + queueSelectAnswers('growth'); + queueConfirmAnswers(true); + + await billingUpgrade(); + + const applyCall = vi.mocked(apiClient).mock.calls.find((c) => c[0] === APPLY)!; + expect(JSON.parse(String((applyCall[1] as { body: string }).body)).billingInterval).toBe('annual'); + }); + + test('When the subscription carries no billing interval, then the Billing page opens rather than assuming monthly', async () => { + const { billingInterval: _dropped, ...noInterval } = SUB; + mockApi({}, noInterval); + + await billingUpgrade(); + + expect(vi.mocked(open)).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); + const bodies = vi.mocked(apiClient).mock.calls.map((c) => JSON.stringify(c[1] ?? '')); + expect(bodies.some((b) => b.includes('monthly'))).toBe(false); + }); + test('When the org is on a Custom plan, then the Billing page opens (not in the tier catalog)', async () => { mockApi({}, { ...SUB, plan: { slug: 'custom', name: 'Custom' } }); diff --git a/src/commands/billing.ts b/src/commands/billing.ts index c238534..f18a0a7 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -223,7 +223,7 @@ async function changePlanInTerminal( orgPublicId: string, sub: { plan: { slug: string; name: string }; - billingInterval?: 'monthly' | 'annual'; + billingInterval: 'monthly' | 'annual'; currentPeriodEnd?: string; }, ): Promise { @@ -236,9 +236,10 @@ async function changePlanInTerminal( }); const target = plans.find((p) => p.slug === planSlug)!; // Interval is whatever the subscription already bills on — switching monthly - // ↔ annual is a separate decision and stays on the Billing page. - const billingInterval = sub.billingInterval ?? 'monthly'; - const body = JSON.stringify({ planSlug, billingInterval }); + // ↔ annual is a separate decision and stays on the Billing page. Never + // defaulted: guessing `monthly` for an annual customer would move their + // billing cadence behind a prompt that only mentioned the plan. + const body = JSON.stringify({ planSlug, billingInterval: sub.billingInterval }); const preview = (await apiClient(`/organizations/${orgPublicId}/billing/usage-tier/preview`, { method: 'POST', @@ -315,8 +316,16 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise Date: Thu, 13 Aug 2026 17:10:34 +0300 Subject: [PATCH 3/6] fix(billing): require a TTY on stdin too, assert the change payloads CodeRabbit on #58: - Major: the guard only checked stdout, so `billing upgrade < /dev/null` rendered a prompt with nothing to read the answer from. Both streams are checked now, matching how login.ts decides it can prompt. - Nitpick: the upgrade test asserted output but never that the apply call happened, or with what. Both requests are now asserted as POSTs carrying the chosen plan and the inherited interval, so the interval-preservation contract can't regress silently. --- src/__tests__/billing.test.ts | 10 +++++-- src/commands/__tests__/billing.test.ts | 40 ++++++++++++++++++++++++++ src/commands/billing.ts | 6 ++-- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 9a50874..52ea4ce 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -289,7 +289,9 @@ describe('billing commands', () => { * which path ran, not about applying a change. */ async function runPaidPathDeclining(status: string): Promise { const origTTY = process.stdout.isTTY; + const origStdinTTY = process.stdin.isTTY; process.stdout.isTTY = true; + process.stdin.isTTY = true; const inq = await import('@inquirer/prompts'); vi.mocked(inq.select).mockResolvedValueOnce('starter' as never); vi.mocked(inq.confirm).mockResolvedValueOnce(false as never); @@ -307,6 +309,7 @@ describe('billing commands', () => { await billingUpgrade(); } finally { process.stdout.isTTY = origTTY; + process.stdin.isTTY = origStdinTTY; } } @@ -349,10 +352,12 @@ describe('billing commands', () => { }); it('prompts free user for plan + interval and opens checkout', async () => { - // Free-tier path is interactive: guarded by a TTY check. Stub isTTY so - // the prompt branch runs instead of the non-TTY rejection. + // Free-tier path is interactive: guarded by a TTY check on both streams. + // Stub them so the prompt branch runs instead of the non-TTY rejection. const origTTY = process.stdout.isTTY; + const origStdinTTY = process.stdin.isTTY; process.stdout.isTTY = true; + process.stdin.isTTY = true; const inq = await import('@inquirer/prompts'); vi.mocked(inq.select) .mockResolvedValueOnce('growth' as never) @@ -397,6 +402,7 @@ describe('billing commands', () => { await run; } finally { process.stdout.isTTY = origTTY; + process.stdin.isTTY = origStdinTTY; vi.useRealTimers(); } diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 9a8bddc..0d3a5fb 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -177,6 +177,7 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', } let origTTY: typeof process.stdout.isTTY; + let origStdinTTY: typeof process.stdin.isTTY; let log: ReturnType; beforeEach(() => { vi.mocked(apiClient).mockReset(); @@ -186,12 +187,15 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', confirmAnswers = []; process.env.HOOKMYAPP_APP_URL = 'https://app.test'; origTTY = process.stdout.isTTY; + origStdinTTY = process.stdin.isTTY; process.stdout.isTTY = true; + process.stdin.isTTY = true; log = vi.spyOn(console, 'log').mockImplementation(() => {}); }); afterEach(() => { delete process.env.HOOKMYAPP_APP_URL; process.stdout.isTTY = origTTY; + process.stdin.isTTY = origStdinTTY; log.mockRestore(); }); @@ -216,6 +220,27 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', expect(vi.mocked(open)).not.toHaveBeenCalled(); }); + test('When applying, then preview and apply POST the same plan and the inherited interval', async () => { + mockApi({ + [PREVIEW]: { scheduled: false, amountDueCents: 1200 }, + [APPLY]: { scheduled: false }, + }); + queueSelectAnswers('growth'); + queueConfirmAnswers(true); + + await billingUpgrade(); + + const sent = vi + .mocked(apiClient) + .mock.calls.filter((c) => c[0] === PREVIEW || c[0] === APPLY) + .map((c) => ({ path: c[0], ...(c[1] as { method: string; body: string }) })); + expect(sent.map((s) => s.path)).toEqual([PREVIEW, APPLY]); + for (const call of sent) { + expect(call.method).toBe('POST'); + expect(JSON.parse(call.body)).toEqual({ planSlug: 'growth', billingInterval: 'monthly' }); + } + }); + test('When the plan list is offered, then the current plan is excluded and the interval is not asked', async () => { mockApi({ [PREVIEW]: { scheduled: false, amountDueCents: 1200 }, @@ -312,10 +337,22 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', expect(paths).not.toContain(APPLY); expect(vi.mocked(open)).not.toHaveBeenCalled(); }); + + test('When stdin is redirected, then it refuses rather than prompting into a pipe', async () => { + // stdout can be a terminal while stdin is a pipe (`billing upgrade < /dev/null`). + // The prompt would render and then have nothing to read the answer from. + process.stdin.isTTY = false; + mockApi({}); + + await expect(billingUpgrade()).rejects.toMatchObject({ code: 'UPGRADE_REQUIRES_TTY' }); + + expect(capturedSelectCalls()).toHaveLength(0); + }); }); describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { let origTTY: typeof process.stdout.isTTY; + let origStdinTTY: typeof process.stdin.isTTY; beforeEach(() => { vi.mocked(apiClient).mockReset(); vi.mocked(open).mockClear(); @@ -323,11 +360,14 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { selectAnswers = []; process.env.HOOKMYAPP_APP_URL = 'https://app.test'; origTTY = process.stdout.isTTY; + origStdinTTY = process.stdin.isTTY; process.stdout.isTTY = true; + process.stdin.isTTY = true; }); afterEach(() => { delete process.env.HOOKMYAPP_APP_URL; process.stdout.isTTY = origTTY; + process.stdin.isTTY = origStdinTTY; // Fake timers are per-test opt-in (three tests below poll under // vi.useFakeTimers()) — restore real timers here rather than at the end // of each test body, so a thrown assertion between `await run` and diff --git a/src/commands/billing.ts b/src/commands/billing.ts index f18a0a7..0f71678 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -303,8 +303,10 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise Date: Thu, 13 Aug 2026 17:13:39 +0300 Subject: [PATCH 4/6] test(billing): give the fake-timer poll tests a real-time budget CI timed out on "a poll tick hits a network blip" at the 5s default while the same test passes locally. advanceUntilSettled drives up to 500 real event-loop turns, so on a loaded runner the wall-clock cost of stepping fake timers can exceed the default budget with nothing actually wrong. Same family as 611f0f9 (cold-import warmup for these tests). Raises the free-tier block's timeout to 30s rather than retrying the job. --- src/commands/__tests__/billing.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 0d3a5fb..da5b785 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -474,4 +474,9 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { await advanceUntilSettled(run); await assertion; }); -}); + // advanceUntilSettled drives up to 500 real event-loop turns per poll test. + // On a loaded CI runner that can exceed the 5s default and time out even + // though nothing is wrong — it already timed out once on this branch. The + // block-level budget keeps these tests honest without making each one + // declare its own. +}, 30_000); From 3417cc974046ffe4a10f951df0efcebcebbb6eae Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Thu, 13 Aug 2026 17:17:14 +0300 Subject: [PATCH 5/6] fix(billing): check the TTY before any network call CodeRabbit on #58: the guard sat after workspace resolution and the subscription fetch, so a non-TTY run could fail on whatever those calls hit instead of on the reason it can't proceed. Moved it directly after the --json rejection; the test now asserts apiClient is never called. --- src/commands/__tests__/billing.test.ts | 7 ++++--- src/commands/billing.ts | 20 +++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index da5b785..d95e6e8 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -327,14 +327,15 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', expect(vi.mocked(open)).toHaveBeenCalledWith('https://app.test/org/org_abc12345/billing'); }); - test('When no TTY, then it fails with UPGRADE_REQUIRES_TTY and applies nothing', async () => { + test('When no TTY, then it fails with UPGRADE_REQUIRES_TTY before touching the API', async () => { process.stdout.isTTY = false; mockApi({}); await expect(billingUpgrade()).rejects.toThrow(/interactive terminal/i); - const paths = vi.mocked(apiClient).mock.calls.map((c) => String(c[0])); - expect(paths).not.toContain(APPLY); + // Guarding before the workspace/subscription lookups means a non-TTY run + // reports why it can't proceed instead of whatever those calls hit first. + expect(vi.mocked(apiClient)).not.toHaveBeenCalled(); expect(vi.mocked(open)).not.toHaveBeenCalled(); }); diff --git a/src/commands/billing.ts b/src/commands/billing.ts index 0f71678..cb8b20a 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -293,19 +293,13 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise Date: Thu, 13 Aug 2026 17:20:26 +0300 Subject: [PATCH 6/6] test(billing): assert the error code, not the message, on the no-TTY path CodeRabbit on #58: matching only /interactive terminal/i would pass for any error carrying that wording. Asserts UPGRADE_REQUIRES_TTY instead, matching the sibling stdin test and the rest of the file. --- src/commands/__tests__/billing.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index d95e6e8..94af3a8 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -331,7 +331,7 @@ describe('billingUpgrade — paid tier changes plan in the terminal (AIT-398)', process.stdout.isTTY = false; mockApi({}); - await expect(billingUpgrade()).rejects.toThrow(/interactive terminal/i); + await expect(billingUpgrade()).rejects.toMatchObject({ code: 'UPGRADE_REQUIRES_TTY' }); // Guarding before the workspace/subscription lookups means a non-TTY run // reports why it can't proceed instead of whatever those calls hit first.