From cf4703057cf626a15d3679df3dacfa7daba9f2fc Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Wed, 12 Aug 2026 16:17:55 +0300 Subject: [PATCH 1/8] fix(billing): upgrade prompt renders the live plan catalog from GET /plans The free-tier upgrade prompt showed a hardcoded, stale plan list. It now fetches GET /plans and renders paid tiers with live names, limits, and prices; free is excluded and a fetch failure fails the command with no fallback list. --- src/__tests__/billing.test.ts | 6 +++ src/commands/__tests__/billing.test.ts | 72 ++++++++++++++++++++++++++ src/commands/billing.ts | 27 ++++++++-- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 6632d07..124d2fa 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -237,10 +237,16 @@ describe('billing commands', () => { vi.unstubAllEnvs(); }); + const PLANS_CATALOG = [ + { slug: 'free', name: 'Launch', messages: 2000, priceInCents: 0, annualPriceInCents: 0 }, + { slug: 'growth', name: 'Scale', messages: 1200, priceInCents: 2400, annualPriceInCents: 24000 }, + ]; + function mockSubAndWorkspaces(sub: any, checkoutUrl?: string) { mockedApiClient.mockImplementation(async (path: string) => { if (path === SUBSCRIPTION_PATH) return sub; if (path === '/workspaces') return WORKSPACES; + if (path === '/plans') return PLANS_CATALOG; if (path === CHECKOUT_PATH && checkoutUrl) return { url: checkoutUrl }; throw new Error(`unexpected path: ${path}`); }); diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index b1702d9..9a362f8 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -21,6 +21,25 @@ vi.mock('../_helpers.js', async (importOriginal) => { // without this stub, it would try to launch the user's browser. vi.mock('open', () => ({ default: vi.fn(async () => undefined) })); +// billingUpgrade's free-tier path prompts via @inquirer/prompts. Record each +// call's `choices` argument (capturedSelectCalls) and let tests queue the +// answers a user would pick (queueSelectAnswers) instead of asserting against +// a hardcoded plan list. +let selectCalls: Array<{ choices: unknown }> = []; +let selectAnswers: unknown[] = []; +vi.mock('@inquirer/prompts', () => ({ + select: vi.fn(async (args: { choices: unknown }) => { + selectCalls.push(args); + return selectAnswers.shift(); + }), +})); +function queueSelectAnswers(...answers: unknown[]): void { + selectAnswers = answers; +} +function capturedSelectCalls(): Array<{ choices: unknown }> { + return selectCalls; +} + import open from 'open'; import { apiClient } from '../../api/client.js'; import { billingManage, billingUpgrade } from '../billing.js'; @@ -100,3 +119,56 @@ describe('billingUpgrade — active subscription path (portal retired)', () => { expect(paths.some((p) => p.startsWith('/stripe/'))).toBe(false); }); }); + +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 }, + { slug: 'pro', name: 'Business', messages: 250000, priceInCents: 3900, annualPriceInCents: 39000 }, + ]; + + let origTTY: typeof process.stdout.isTTY; + beforeEach(() => { + vi.mocked(apiClient).mockReset(); + vi.mocked(open).mockClear(); + selectCalls = []; + selectAnswers = []; + process.env.HOOKMYAPP_APP_URL = 'https://app.test'; + origTTY = process.stdout.isTTY; + process.stdout.isTTY = true; + }); + afterEach(() => { + delete process.env.HOOKMYAPP_APP_URL; + process.stdout.isTTY = origTTY; + }); + + test('When on free tier, then plan choices come from GET /plans with limits and prices, free excluded', async () => { + // apiClient queue: workspaces union → subscription (free) → /plans → checkout + vi.mocked(apiClient) + .mockResolvedValueOnce(workspaces) + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockResolvedValueOnce(CATALOG) + .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }); + queueSelectAnswers('growth', 'monthly'); // helper on the @inquirer/prompts mock + + await billingUpgrade(); + + expect(vi.mocked(apiClient)).toHaveBeenCalledWith('/plans'); + const planChoices = capturedSelectCalls()[0].choices as Array<{ value: string; name: string; description?: string }>; + expect(planChoices.map((c) => c.value)).toEqual(['starter', 'growth', 'pro']); + expect(planChoices[1].name).toBe('Scale: 100,000 messages — $24/mo (or $240/yr)'); + expect(planChoices[1].description).toBe('Most popular'); + }); + + test('When GET /plans fails, then upgrade fails with that error and no checkout is minted', async () => { + vi.mocked(apiClient) + .mockResolvedValueOnce(workspaces) + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockRejectedValueOnce(new Error('service unavailable')); + + await expect(billingUpgrade()).rejects.toThrow(); + const paths = vi.mocked(apiClient).mock.calls.map((c) => c[0]); + expect(paths).not.toContain('/organizations/org_abc12345/billing/checkout'); + }); +}); diff --git a/src/commands/billing.ts b/src/commands/billing.ts index 1d64ab4..4a39e28 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -124,14 +124,31 @@ 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'); + } + const { select } = await import('@inquirer/prompts'); const planSlug = await select({ message: 'Choose a plan', - choices: [ - { name: 'Build: 500 messages', value: 'starter' }, - { name: 'Scale: 1,200 messages', value: 'growth', description: 'Most popular' }, - { name: 'Business: 2,500 messages', value: 'pro' }, - ], + choices: paidPlans.map((p) => ({ + name: `${p.name}: ${p.messages.toLocaleString('en-US')} messages — $${Math.round(p.priceInCents / 100)}/mo (or $${Math.round(p.annualPriceInCents / 100)}/yr)`, + value: p.slug, + ...(p.popular ? { description: 'Most popular' } : {}), + })), }); const billingInterval = await select({ message: 'Billing interval', From 19eb2f7453a8b37f23e4a870fe5d784ad9f5afbb Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Wed, 12 Aug 2026 16:36:17 +0300 Subject: [PATCH 2/8] feat(billing): upgrade waits for payment and confirms in the terminal billingUpgrade now polls the org subscription every 5s after checkout opens until the plan leaves free, then prints a confirmation with the new plan name and message limit. Transient failures (network blips, 5xx) are swallowed; permanent errors (expired auth, etc.) abort the poll and surface normally. Also fixes the legacy src/__tests__/billing.test.ts free-tier test, which hung once billingUpgrade started polling after checkout. --- src/__tests__/billing.test.ts | 59 ++++++++++++++-- src/commands/__tests__/billing.test.ts | 97 ++++++++++++++++++++++++-- src/commands/billing.ts | 51 +++++++++++++- 3 files changed, 196 insertions(+), 11 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 124d2fa..385d849 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -4,6 +4,9 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../api/client.js', () => ({ apiClient: vi.fn(), setWorkspaceContext: vi.fn(), + // pollForUpgrade's transient-error check calls this on every poll failure; + // default to "not a network blip" so it never masks a real error. + isNetworkFailure: vi.fn(() => false), })); // Mock open @@ -57,6 +60,31 @@ const freeSub = { plan: { slug: 'free', name: 'Free', messages: 50, priceInCents: 0, annualPriceInCents: 0 }, }; +/** Advance fake timers in small steps until `settleOn` resolves/rejects (or a + * generous step cap is hit). A single large `advanceTimersByTimeAsync` jump + * can race ahead of the pending promise chain before pollForUpgrade's first + * `setTimeout` is even registered (this suite's beforeEach calls + * vi.resetModules() every test, forcing a cold re-import of + * '@inquirer/prompts' each time) — especially under full-suite load, where + * scheduling is less predictable than running this file alone. */ +async function advanceUntilSettled( + settleOn: Promise, + { stepMs = 50, maxSteps = 1000 } = {}, +): Promise { + let settled = false; + settleOn.then( + () => { + settled = true; + }, + () => { + settled = true; + }, + ); + for (let i = 0; i < maxSteps && !settled; i++) { + await vi.advanceTimersByTimeAsync(stepMs); + } +} + function mockSubAndUsage(sub: any, usage: { totalMessages: number; limit: number; percentage: number }) { mockedApiClient.mockImplementation(async (path: string) => { if (path === '/workspaces') return WORKSPACES; @@ -301,15 +329,36 @@ describe('billing commands', () => { vi.mocked(inq.select) .mockResolvedValueOnce('growth' as never) .mockResolvedValueOnce('annual' as never); - mockSubAndWorkspaces( - { status: 'active', plan: { slug: 'free', name: 'Free' } }, - 'https://checkout.stripe.com/x', - ); + // billingUpgrade polls the subscription after checkout opens until the + // plan leaves free — flip the mocked subscription response to upgraded + // once the checkout call has minted a URL, so the first poll tick + // resolves instead of looping forever on the persistent apiClient + // mockImplementation. + let checkoutMinted = false; + mockedApiClient.mockImplementation(async (path: string) => { + if (path === SUBSCRIPTION_PATH) { + return checkoutMinted + ? { status: 'active', plan: { slug: 'growth', name: 'Scale', messages: 1200 } } + : { status: 'active', plan: { slug: 'free', name: 'Free' } }; + } + if (path === '/workspaces') return WORKSPACES; + if (path === '/plans') return PLANS_CATALOG; + if (path === CHECKOUT_PATH) { + checkoutMinted = true; + return { url: 'https://checkout.stripe.com/x' }; + } + throw new Error(`unexpected path: ${path}`); + }); + + vi.useFakeTimers(); try { - await billingUpgrade(); + const run = billingUpgrade(); + await advanceUntilSettled(run); + await run; } finally { process.stdout.isTTY = origTTY; + vi.useRealTimers(); } expect(inq.select).toHaveBeenCalledTimes(2); diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 9a362f8..3f88537 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -1,8 +1,12 @@ -import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, test, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest'; vi.mock('../../api/client.js', () => ({ apiClient: vi.fn(), setWorkspaceContext: vi.fn(), + // pollForUpgrade's transient-error check calls this on every poll failure; + // default to "not a network blip" so permanent errors (AuthError, etc.) + // fall through to the `err instanceof ApiError` check and then rethrow. + isNetworkFailure: vi.fn(() => false), })); // getDefaultWorkspaceId is read off the user's local profile; stub to a fixed @@ -42,6 +46,7 @@ function capturedSelectCalls(): Array<{ choices: unknown }> { import open from 'open'; import { apiClient } from '../../api/client.js'; +import { AuthError } from '../../output/error.js'; import { billingManage, billingUpgrade } from '../billing.js'; const workspaces = [ @@ -49,6 +54,45 @@ const workspaces = [ { id: 'ws_test', name: 'Acme', organizationPublicId: 'org_abc12345' }, ]; +/** Advance fake timers in small steps until `settleOn` resolves/rejects (or a + * generous step cap is hit). A single large `advanceTimersByTimeAsync` jump + * can race ahead of the pending promise chain before pollForUpgrade's first + * `setTimeout` is even registered (dynamic `import('@inquirer/prompts')` + + * several mocked/real awaits) — especially under full-suite load, where + * scheduling is less predictable than running this file alone. Stepping + * small and checking settlement each time avoids guessing a fixed total. */ +async function advanceUntilSettled( + settleOn: Promise, + { stepMs = 100, maxSteps = 500 } = {}, +): Promise { + let settled = false; + settleOn.then( + () => { + settled = true; + }, + () => { + settled = true; + }, + ); + for (let i = 0; i < maxSteps && !settled; i++) { + await vi.advanceTimersByTimeAsync(stepMs); + } +} + +// billingUpgrade's free-tier path resolves `@inquirer/prompts` via a dynamic +// `import()` at call time. Warm that module resolution once here, outside any +// fake-timer test — the first cold `import()` in this file can take more +// event-loop turns than a fake-timer poll test can reliably interleave with. +// Also warm vi.useFakeTimers() itself: its first-ever call in the process +// lazily loads the underlying timer-faking library, and a poll test that +// enables fake timers for the first time can otherwise register its +// setTimeout against the not-yet-patched real timer. +beforeAll(async () => { + await import('@inquirer/prompts'); + vi.useFakeTimers(); + vi.useRealTimers(); +}); + describe('billingManage — opens the app Billing page (portal retired)', () => { beforeEach(() => { vi.mocked(apiClient).mockReset(); @@ -144,21 +188,28 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { }); test('When on free tier, then plan choices come from GET /plans with limits and prices, free excluded', async () => { - // apiClient queue: workspaces union → subscription (free) → /plans → checkout + // apiClient queue: workspaces union → subscription (free) → /plans → checkout → poll (upgraded) + // billingUpgrade polls after checkout, so fake timers + a poll response are + // needed to let the command resolve instead of waiting on a real 5s timer. + vi.useFakeTimers(); vi.mocked(apiClient) .mockResolvedValueOnce(workspaces) .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) .mockResolvedValueOnce(CATALOG) - .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }); + .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }) + .mockResolvedValueOnce({ plan: { slug: 'growth', name: 'Scale', messages: 100000 }, status: 'active' }); queueSelectAnswers('growth', 'monthly'); // helper on the @inquirer/prompts mock - await billingUpgrade(); + const run = billingUpgrade(); + await advanceUntilSettled(run); + await run; expect(vi.mocked(apiClient)).toHaveBeenCalledWith('/plans'); const planChoices = capturedSelectCalls()[0].choices as Array<{ value: string; name: string; description?: string }>; expect(planChoices.map((c) => c.value)).toEqual(['starter', 'growth', 'pro']); expect(planChoices[1].name).toBe('Scale: 100,000 messages — $24/mo (or $240/yr)'); expect(planChoices[1].description).toBe('Most popular'); + vi.useRealTimers(); }); test('When GET /plans fails, then upgrade fails with that error and no checkout is minted', async () => { @@ -171,4 +222,42 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { const paths = vi.mocked(apiClient).mock.calls.map((c) => c[0]); expect(paths).not.toContain('/organizations/org_abc12345/billing/checkout'); }); + + test('When checkout opens, then upgrade polls the subscription and confirms once the plan flips', async () => { + vi.useFakeTimers(); + vi.mocked(apiClient) + .mockResolvedValueOnce(workspaces) + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockResolvedValueOnce(CATALOG) + .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }) + // poll #1: still free; poll #2: upgraded + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockResolvedValueOnce({ plan: { slug: 'growth', name: 'Scale', messages: 100000 }, status: 'active' }); + queueSelectAnswers('growth', 'monthly'); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const run = billingUpgrade(); + await advanceUntilSettled(run); + await run; + + expect(log.mock.calls.flat().join('\n')).toContain('Upgraded to Scale'); + vi.useRealTimers(); + }); + + test('When polling hits a permanent error (expired auth), then upgrade aborts instead of waiting forever', async () => { + vi.useFakeTimers(); + vi.mocked(apiClient) + .mockResolvedValueOnce(workspaces) + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockResolvedValueOnce(CATALOG) + .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }) + .mockRejectedValueOnce(new AuthError()); // poll #1: token expired + queueSelectAnswers('growth', 'monthly'); + + const run = billingUpgrade(); + const assertion = expect(run).rejects.toBeInstanceOf(AuthError); + await advanceUntilSettled(run); + await assertion; + vi.useRealTimers(); + }); }); diff --git a/src/commands/billing.ts b/src/commands/billing.ts index 4a39e28..b6b88d4 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -1,9 +1,9 @@ import { Command } from 'commander'; import open from 'open'; -import { apiClient } from '../api/client.js'; +import { apiClient, isNetworkFailure } from '../api/client.js'; import { output } from '../output/format.js'; import { c } from '../output/color.js'; -import { ValidationError } from '../output/error.js'; +import { ApiError, ValidationError } from '../output/error.js'; import { addExamples } from '../output/help.js'; import { cliCommandPrefix } from '../output/cli-self.js'; import { getEffectiveAppUrl } from '../config/env-profiles.js'; @@ -23,6 +23,47 @@ function orgBillingUrl(orgPublicId: string): string { return `${getEffectiveAppUrl()}/org/${orgPublicId}/billing`; } +const UPGRADE_POLL_INTERVAL_MS = 5_000; +const UPGRADE_POLL_HINT_EVERY_MS = 60_000; + +/** Mirrors the `channels connect` wait pattern: poll the org subscription + * until the plan leaves `free` with an active status. No hard timeout — + * a periodic hint keeps the wait visibly alive; Ctrl+C cancels. ONLY + * transient failures (network blips, 5xx) are swallowed; permanent errors + * (expired auth, revoked permission, client-outdated) rethrow — polling + * forever on those would tell the user to "finish checkout" while the CLI + * itself is the thing that's broken. */ +async function pollForUpgrade( + orgPublicId: string, +): Promise<{ plan: { slug: string; name: string; messages: number }; status: string }> { + const startedAt = Date.now(); + let lastHintAt = startedAt; + for (;;) { + await new Promise((resolve) => setTimeout(resolve, UPGRADE_POLL_INTERVAL_MS)); + let sub: { plan: { slug: string; name: string; messages: number }; status: string } | null; + try { + sub = (await apiClient( + `/organizations/${orgPublicId}/billing/subscription`, + )) as { plan: { slug: string; name: string; messages: number }; status: string }; + } catch (err) { + const transient = + isNetworkFailure(err) || + (err instanceof ApiError && (err.statusCode ?? 0) >= 500); + if (!transient) throw err; + sub = null; + } + if (sub && sub.plan.slug !== 'free' && ['active', 'trialing'].includes(sub.status)) { + return sub; + } + if (Date.now() - lastHintAt >= UPGRADE_POLL_HINT_EVERY_MS) { + lastHintAt = Date.now(); + console.log( + `Still waiting (${Math.round((Date.now() - startedAt) / 60_000)} min) — finish checkout in your browser, or Ctrl+C to cancel.`, + ); + } + } +} + export async function billingManage(opts: { json?: boolean } = {}): Promise { const workspaceId = await getDefaultWorkspaceId(); const url = orgBillingUrl(await resolveOrgPublicIdForWorkspace(workspaceId)); @@ -163,6 +204,12 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise Date: Wed, 12 Aug 2026 16:40:51 +0300 Subject: [PATCH 3/8] test(billing): guarantee fake timer cleanup via afterEach The three billingUpgrade poll tests called vi.useRealTimers() at the end of each test body, after the assertions. A thrown assertion would skip that call and leak fake timers into every later test in the file. Move the reset into the describe block's existing afterEach so cleanup runs regardless of how the test exits. --- src/commands/__tests__/billing.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 3f88537..8ba12e9 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -185,6 +185,12 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { afterEach(() => { delete process.env.HOOKMYAPP_APP_URL; process.stdout.isTTY = origTTY; + // 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 + // cleanup can't leak fake timers into later tests (no other afterEach + // resets them). + vi.useRealTimers(); }); test('When on free tier, then plan choices come from GET /plans with limits and prices, free excluded', async () => { @@ -209,7 +215,6 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { expect(planChoices.map((c) => c.value)).toEqual(['starter', 'growth', 'pro']); expect(planChoices[1].name).toBe('Scale: 100,000 messages — $24/mo (or $240/yr)'); expect(planChoices[1].description).toBe('Most popular'); - vi.useRealTimers(); }); test('When GET /plans fails, then upgrade fails with that error and no checkout is minted', async () => { @@ -241,7 +246,6 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { await run; expect(log.mock.calls.flat().join('\n')).toContain('Upgraded to Scale'); - vi.useRealTimers(); }); test('When polling hits a permanent error (expired auth), then upgrade aborts instead of waiting forever', async () => { @@ -258,6 +262,5 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { const assertion = expect(run).rejects.toBeInstanceOf(AuthError); await advanceUntilSettled(run); await assertion; - vi.useRealTimers(); }); }); From 1ad7b5e7b3beca42a32739a2827ea7e479e41924 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Wed, 12 Aug 2026 16:44:02 +0300 Subject: [PATCH 4/8] fix(errors): append the stable error code to human-mode error lines --- src/__tests__/error.test.ts | 2 +- src/output/__tests__/error.test.ts | 4 ++-- .../__tests__/output-error-code.test.ts | 20 +++++++++++++++++++ src/output/error.ts | 6 +++++- 4 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 src/output/__tests__/output-error-code.test.ts diff --git a/src/__tests__/error.test.ts b/src/__tests__/error.test.ts index d0ec9b7..86b2949 100644 --- a/src/__tests__/error.test.ts +++ b/src/__tests__/error.test.ts @@ -83,7 +83,7 @@ describe('outputError', () => { it('in human mode writes Error: to stderr', () => { const err = new CliError('Bad request', 'API_ERROR'); outputError(err, { human: true }); - expect(mockWrite).toHaveBeenCalledWith('Error: Bad request\n'); + expect(mockWrite).toHaveBeenCalledWith('Error: Bad request (API_ERROR)\n'); }); it('in JSON mode writes nested envelope with code, message, and status', () => { diff --git a/src/output/__tests__/error.test.ts b/src/output/__tests__/error.test.ts index 6bb8d89..8d153ae 100644 --- a/src/output/__tests__/error.test.ts +++ b/src/output/__tests__/error.test.ts @@ -98,10 +98,10 @@ describe('outputError JSON envelope (D1)', () => { expect(parsed.error.hint).toBe('Run: hookmyapp channels list'); }); - test('When human mode, then plain text is emitted (unchanged behavior)', () => { + test('When human mode, then plain text with the code suffix is emitted', () => { const err = new ValidationError('Bad input', 'BAD_INPUT'); outputError(err, { human: true }); - expect(stderrSpy).toHaveBeenCalledWith('Error: Bad input\n'); + expect(stderrSpy).toHaveBeenCalledWith('Error: Bad input (BAD_INPUT)\n'); }); test('When userMessage is a non-string object (bad API body), then outputError does not throw and serializes it (HOOKMYAPP-CLI-J)', () => { diff --git a/src/output/__tests__/output-error-code.test.ts b/src/output/__tests__/output-error-code.test.ts new file mode 100644 index 0000000..210e5c8 --- /dev/null +++ b/src/output/__tests__/output-error-code.test.ts @@ -0,0 +1,20 @@ +import { describe, test, expect, vi, afterEach } from 'vitest'; +import { outputError, UnexpectedError, ApiError } from '../error.js'; + +describe('outputError human mode', () => { + afterEach(() => vi.restoreAllMocks()); + + test('When a code exists, then it is appended so screenshots are diagnosable', () => { + const write = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + outputError(new UnexpectedError('Something went wrong. Try again later.', 'UNKNOWN_ERROR'), { human: true }); + expect(write).toHaveBeenCalledWith('Error: Something went wrong. Try again later. (UNKNOWN_ERROR)\n'); + }); + + test('When the error has no code, then the line is unchanged', () => { + const write = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const err = new ApiError('Organization not found.', 404); + (err as { code?: string }).code = undefined as unknown as string; + outputError(err, { human: true }); + expect(write).toHaveBeenCalledWith('Error: Organization not found.\n'); + }); +}); diff --git a/src/output/error.ts b/src/output/error.ts index bc9c8b1..ce55957 100644 --- a/src/output/error.ts +++ b/src/output/error.ts @@ -375,7 +375,11 @@ export function wrapCommanderError(err: Error & { code?: string }): CliError { export function outputError(error: CliError, opts: { human?: boolean }): void { if (opts.human) { - process.stderr.write(`Error: ${error.userMessage}\n`); + // Diagnosability without internals: the stable machine code makes a + // screenshot of a generic error actionable (which failure class fired), + // while the message itself stays the safe userMessage. + const codeSuffix = error.code ? ` (${error.code})` : ''; + process.stderr.write(`Error: ${error.userMessage}${codeSuffix}\n`); return; } From 202512f2be178e81049b63c681abc7023135fda0 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Wed, 12 Aug 2026 16:47:16 +0300 Subject: [PATCH 5/8] =?UTF-8?q?release:=200.14.14=20=E2=80=94=20live=20pla?= =?UTF-8?q?n=20catalog,=20upgrade=20confirmation,=20error=20codes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 10 ++++++++++ package.json | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9612fc7..bab1a44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ All notable changes to `@gethookmyapp/cli` are documented here. +## 0.14.14 — 2026-08-12 + +### Added + +- `hookmyapp billing upgrade` now fetches live plan catalog from the backend and polls until the upgrade completes with a ✓ confirmation (AIT-391). + +### Changed + +- Human-mode error messages now include stable error-code suffixes (e.g. `ERROR (INVALID_CREDENTIALS)`) for debugging and consistency with machine-mode output (AIT-391). + ## 0.14.13 - Replace example phone numbers and email fixtures with reserved fictional values. diff --git a/package.json b/package.json index 6f0e655..2770b1f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@gethookmyapp/cli", - "version": "0.14.13", + "version": "0.14.14", "description": "HookMyApp CLI, No BS. Just go live.", "type": "module", "bin": { From cd1e1e5b0a6f5953aaad63f9b1e3a381dd72924b Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Wed, 12 Aug 2026 16:55:09 +0300 Subject: [PATCH 6/8] docs: neutralize catalog comment + fix changelog example --- CHANGELOG.md | 2 +- src/commands/billing.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bab1a44..83dfd84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ All notable changes to `@gethookmyapp/cli` are documented here. ### Changed -- Human-mode error messages now include stable error-code suffixes (e.g. `ERROR (INVALID_CREDENTIALS)`) for debugging and consistency with machine-mode output (AIT-391). +- Human-mode error messages now include stable error-code suffixes (e.g. `Error: (INVALID_CREDENTIALS)`) for debugging and consistency with machine-mode output (AIT-391). ## 0.14.13 diff --git a/src/commands/billing.ts b/src/commands/billing.ts index b6b88d4..13ad9e0 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -173,9 +173,9 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise p.priceInCents > 0); if (paidPlans.length === 0) { From e7f068d02a0d416fd49ae89a3db66e3b5f982261 Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Thu, 13 Aug 2026 07:36:43 +0300 Subject: [PATCH 7/8] fix(billing): poll treats NetworkError as transient, cent-accurate prices, deterministic legacy test --- src/__tests__/billing.test.ts | 15 +++++++++++- src/commands/__tests__/billing.test.ts | 34 ++++++++++++++++++++++++-- src/commands/billing.ts | 18 ++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index 385d849..b144e00 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest'; // Mock apiClient vi.mock('../api/client.js', () => ({ @@ -94,6 +94,19 @@ function mockSubAndUsage(sub: any, usage: { totalMessages: number; limit: number }); } +// vi.useFakeTimers()'s first-ever call in a worker process lazily loads the +// underlying timer-faking library; a poll test that enables fake timers for +// the first time can register pollForUpgrade's setTimeout against the +// not-yet-patched real timer, then hang on the real 5s test timeout instead +// of the faked one (only the "prompts free user..." test below uses fake +// timers in this file). Cycle it once here, outside any timed test, so the +// library is warm before it matters — same fix as the sibling +// commands/__tests__/billing.test.ts. +beforeAll(() => { + vi.useFakeTimers(); + vi.useRealTimers(); +}); + describe('billing commands', () => { let billingStatus: (opts: { human?: boolean }) => Promise; let billingUpgrade: (opts?: { json?: boolean }) => Promise; diff --git a/src/commands/__tests__/billing.test.ts b/src/commands/__tests__/billing.test.ts index 8ba12e9..ba807aa 100644 --- a/src/commands/__tests__/billing.test.ts +++ b/src/commands/__tests__/billing.test.ts @@ -46,7 +46,7 @@ function capturedSelectCalls(): Array<{ choices: unknown }> { import open from 'open'; import { apiClient } from '../../api/client.js'; -import { AuthError } from '../../output/error.js'; +import { AuthError, NetworkError } from '../../output/error.js'; import { billingManage, billingUpgrade } from '../billing.js'; const workspaces = [ @@ -169,7 +169,9 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { { 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 }, - { slug: 'pro', name: 'Business', messages: 250000, priceInCents: 3900, annualPriceInCents: 39000 }, + // 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; @@ -215,6 +217,9 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { expect(planChoices.map((c) => c.value)).toEqual(['starter', 'growth', 'pro']); expect(planChoices[1].name).toBe('Scale: 100,000 messages — $24/mo (or $240/yr)'); expect(planChoices[1].description).toBe('Most popular'); + // Non-whole price renders 2dp instead of Math.round dropping the cents + // (1999¢ used to render as "$20"). + expect(planChoices[2].name).toBe('Business: 250,000 messages — $39.99/mo (or $390/yr)'); }); test('When GET /plans fails, then upgrade fails with that error and no checkout is minted', async () => { @@ -248,6 +253,31 @@ describe('billingUpgrade — free tier plan prompt (GET /plans)', () => { expect(log.mock.calls.flat().join('\n')).toContain('Upgraded to Scale'); }); + test('When a poll tick hits a network blip (apiClient throws NetworkError), then it is swallowed and polling continues to success', async () => { + // apiClient wraps a raw fetch failure in its own NetworkError before it + // ever reaches pollForUpgrade — isNetworkFailure() (mocked false in this + // file) inspects the raw fetch-error shape and doesn't recognize the + // wrapper. pollForUpgrade must still treat NetworkError itself as + // transient, or a single blip aborts the wait instead of riding it out. + vi.useFakeTimers(); + vi.mocked(apiClient) + .mockResolvedValueOnce(workspaces) + .mockResolvedValueOnce({ plan: { slug: 'free' }, status: 'active' }) + .mockResolvedValueOnce(CATALOG) + .mockResolvedValueOnce({ url: 'https://checkout.stripe.com/c/pay_123' }) + // poll #1: transient network blip; poll #2: upgraded + .mockRejectedValueOnce(new NetworkError()) + .mockResolvedValueOnce({ plan: { slug: 'growth', name: 'Scale', messages: 100000 }, status: 'active' }); + queueSelectAnswers('growth', 'monthly'); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + const run = billingUpgrade(); + await advanceUntilSettled(run); + await run; + + expect(log.mock.calls.flat().join('\n')).toContain('Upgraded to Scale'); + }); + test('When polling hits a permanent error (expired auth), then upgrade aborts instead of waiting forever', async () => { vi.useFakeTimers(); vi.mocked(apiClient) diff --git a/src/commands/billing.ts b/src/commands/billing.ts index 13ad9e0..a76a6b3 100644 --- a/src/commands/billing.ts +++ b/src/commands/billing.ts @@ -3,7 +3,7 @@ import open from 'open'; import { apiClient, isNetworkFailure } from '../api/client.js'; import { output } from '../output/format.js'; import { c } from '../output/color.js'; -import { ApiError, ValidationError } from '../output/error.js'; +import { ApiError, NetworkError, ValidationError } from '../output/error.js'; import { addExamples } from '../output/help.js'; import { cliCommandPrefix } from '../output/cli-self.js'; import { getEffectiveAppUrl } from '../config/env-profiles.js'; @@ -19,6 +19,13 @@ import { getDefaultWorkspaceId, resolveOrgPublicIdForWorkspace } from './_helper // shared resolveOrgPublicIdForWorkspace helper (AIT-263 — one derivation for // customers + billing, never a bare row[0]). +/** Whole-dollar prices render as `$20`; anything with cents renders 2dp + * (`$19.99`) — `Math.round` was dropping cents entirely (1999¢ → "$20"). */ +function formatPrice(cents: number): string { + const dollars = cents / 100; + return Number.isInteger(dollars) ? `$${dollars}` : `$${dollars.toFixed(2)}`; +} + function orgBillingUrl(orgPublicId: string): string { return `${getEffectiveAppUrl()}/org/${orgPublicId}/billing`; } @@ -46,7 +53,14 @@ async function pollForUpgrade( `/organizations/${orgPublicId}/billing/subscription`, )) as { plan: { slug: string; name: string; messages: number }; status: string }; } catch (err) { + // apiClient wraps raw fetch failures in its own NetworkError before + // they ever reach here — isNetworkFailure() inspects the *raw* fetch + // error shape (TypeError / ECONNREFUSED / etc.) and doesn't recognize + // the wrapper, so a plain network blip was falling through to the + // rethrow below and aborting the poll. Treat the wrapped NetworkError + // as transient too. const transient = + err instanceof NetworkError || isNetworkFailure(err) || (err instanceof ApiError && (err.statusCode ?? 0) >= 500); if (!transient) throw err; @@ -186,7 +200,7 @@ export async function billingUpgrade(opts: { json?: boolean } = {}): Promise ({ - name: `${p.name}: ${p.messages.toLocaleString('en-US')} messages — $${Math.round(p.priceInCents / 100)}/mo (or $${Math.round(p.annualPriceInCents / 100)}/yr)`, + 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' } : {}), })), From 611f0f926186e52638e7d9f35075a4a63027015d Mon Sep 17 00:00:00 2001 From: Or Dvir Date: Thu, 13 Aug 2026 08:07:44 +0300 Subject: [PATCH 8/8] test(billing): warm the cold ../index.js import before fake timers to fix the CI hang --- src/__tests__/billing.test.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/__tests__/billing.test.ts b/src/__tests__/billing.test.ts index b144e00..8d91297 100644 --- a/src/__tests__/billing.test.ts +++ b/src/__tests__/billing.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; // Mock apiClient vi.mock('../api/client.js', () => ({ @@ -94,19 +94,6 @@ function mockSubAndUsage(sub: any, usage: { totalMessages: number; limit: number }); } -// vi.useFakeTimers()'s first-ever call in a worker process lazily loads the -// underlying timer-faking library; a poll test that enables fake timers for -// the first time can register pollForUpgrade's setTimeout against the -// not-yet-patched real timer, then hang on the real 5s test timeout instead -// of the faked one (only the "prompts free user..." test below uses fake -// timers in this file). Cycle it once here, outside any timed test, so the -// library is warm before it matters — same fix as the sibling -// commands/__tests__/billing.test.ts. -beforeAll(() => { - vi.useFakeTimers(); - vi.useRealTimers(); -}); - describe('billing commands', () => { let billingStatus: (opts: { human?: boolean }) => Promise; let billingUpgrade: (opts?: { json?: boolean }) => Promise; @@ -364,6 +351,17 @@ describe('billing commands', () => { throw new Error(`unexpected path: ${path}`); }); + // getDefaultWorkspaceId() lazy-imports '../index.js' (the full CLI + // entry module) to read --workspace off the parsed program options. + // This suite's beforeEach calls vi.resetModules() every test, so that + // import is cold here — a real, disk-bound module-graph load, not a + // microtask. Warm it under REAL timers before flipping to fake ones: + // triggering that cold import for the first time while fake timers are + // already active starves it of the real setImmediate/IO ticks it needs + // to resolve, and vi.advanceTimersByTimeAsync() never drives those, + // so the whole command hangs until vitest's real 5s test timeout. + await import('../index.js'); + vi.useFakeTimers(); try { const run = billingUpgrade();