From e6dd71d6e6dca8adda9f32220bb914cc4fae7f6d Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Sat, 6 Jun 2026 11:30:15 +0530 Subject: [PATCH] fix(api): detect live 'healthy' stacks + reject 200 ok:false promo codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two UI↔API contract-mismatch bugs in src/api/index.ts, both surfaced by the 2026-06-06 bug-hunt pass (docs/ci/BUGHUNT-BACKLOG-2026-06-06.md P1-2, P1-3). Bug 1 (P1-2) — StackCreatePage never detected a live stack. The api emits status="healthy" for a successfully-deployed stack (stack.go:382), but the dashboard's StackStatus union omits 'healthy' and the create-page poll only treats === 'running' as live. A successful dashboard stack deploy therefore spun in the 'building' stage until the ~5-min poll timeout instead of flipping to 'live'. Add normaliseStackStatus() mirroring normaliseDeploymentStatus ('healthy'/'deploying' → 'running', 'deleting' → 'stopped') and apply it in fetchStackStatus(), listStacks(), and createStack() so the type and runtime value agree in one place; the page's existing === 'running' checks now trip for a live stack. Bug 2 (P1-3) — invalid promo codes validated as "ok". POST /api/v1/billing/promotion/validate is LIVE and reports validation FAILURES as 200 ok:false (not 4xx). call() only throws on non-2xx, so validatePromotion built a phantom "valid" promo (empty discount) from an ok:false body; the 404→seeds fallback was dead code (handler never 404s). Now inspect `ok`: a 200 ok:false (or ok:true without a discount) throws an APIError carrying the api's error/message so the UI renders the real rejection. Drop the dead seed table + fix the stale "mocked until api ships" header comment (P3-2). Also correct the misleading "LIVE" comment on cancelSubscription (P3-1 — the /billing/cancel route was removed by policy). Tests (src/api/index.test.ts): proven fail-before/pass-after by stashing the src fix — the 'healthy'/'deploying' stack cases and the 200 ok:false promo cases fail against old code. 100% patch coverage on both changed regions. Co-Authored-By: Claude Opus 4.8 --- src/api/index.test.ts | 210 ++++++++++++++++++++++++++++++++++-------- src/api/index.ts | 178 ++++++++++++++++++++++------------- 2 files changed, 284 insertions(+), 104 deletions(-) diff --git a/src/api/index.test.ts b/src/api/index.test.ts index 252cbc7..98d3d6d 100644 --- a/src/api/index.test.ts +++ b/src/api/index.test.ts @@ -641,27 +641,28 @@ describe('changePlan()', () => { }) }) -// ─── validatePromotion() (P3) ──────────────────────────────────────────── -// Until api ships POST /api/v1/billing/promotion/validate, this helper -// falls back to a small set of seed codes on a 404. The mock + fallback -// path together must: -// - return a Promotion shape when the api responds 200 -// - return the seed Promotion for a known seed code when the api 404s -// - throw promotion_not_found for an unknown code when the api 404s -// - propagate non-404 errors (e.g. 410 expired) untouched -describe('validatePromotion() (P3)', () => { - it('returns the api Promotion on a 200 response', async () => { +// ─── validatePromotion() (P1-3 — LIVE endpoint, 200 ok:false contract) ───── +// POST /api/v1/billing/promotion/validate is LIVE. Its quirk: VALIDATION +// FAILURES come back as 200 with ok:false (NOT a 4xx). The helper must: +// - return a Promotion when the api responds 200 ok:true + discount +// - REJECT a 200 ok:false body (invalid / expired / wrong-plan) — the bug +// this fixes: those used to surface as a phantom "valid" promo +// - propagate true non-2xx (400/401/429/5xx) untouched +// - reject empty input before any network call +describe('validatePromotion() (P1-3)', () => { + it('returns the api Promotion on a 200 ok:true response', async () => { const m = installFetch() m.mockResolvedValueOnce(jsonResponse({ ok: true, code: 'PARTNER25', - discount: { kind: 'percent_off', value: 25, applies_to: 6, unit: 'months' }, - valid_until: '2026-12-31T00:00:00Z', + discount: { kind: 'percent_off', value: 25, applies_to: ['pro'], max_uses: 100 }, + valid_until: '2026-12-31T23:59:59Z', })) const r = await validatePromotion('PARTNER25', 'pro') expect(r.promotion.code).toBe('PARTNER25') - expect(r.promotion.discount).toEqual({ kind: 'percent_off', value: 25, applies_to: 6, unit: 'months' }) - expect(r.promotion.valid_until).toBe('2026-12-31T00:00:00Z') + expect(r.promotion.discount.kind).toBe('percent_off') + expect(r.promotion.discount.value).toBe(25) + expect(r.promotion.valid_until).toBe('2026-12-31T23:59:59Z') }) it('POSTs {code, plan} to /api/v1/billing/promotion/validate (uppercased + trimmed)', async () => { @@ -669,7 +670,7 @@ describe('validatePromotion() (P3)', () => { m.mockResolvedValueOnce(jsonResponse({ ok: true, code: 'TWITTER15', - discount: { kind: 'percent_off', value: 15, applies_to: 3, unit: 'months' }, + discount: { kind: 'percent_off', value: 15, applies_to: ['pro'] }, valid_until: '2026-09-01T00:00:00Z', })) await validatePromotion(' twitter15 ', 'pro') @@ -679,39 +680,77 @@ describe('validatePromotion() (P3)', () => { expect(JSON.parse(init.body as string)).toEqual({ code: 'TWITTER15', plan: 'pro' }) }) - it('falls back to the seed table when the api 404s on a known seed code', async () => { + // REGRESSION GUARD (P1-3): a 200 ok:false body is a REJECTION. Before the + // fix this resolved to a "valid" promo with an undefined discount. It must + // now throw, carrying the api's error code + message. + it('rejects a 200 ok:false body (invalid code) instead of reporting valid', async () => { const m = installFetch() - m.mockResolvedValueOnce(jsonResponse( - { error: 'not_found', message: 'no such route' }, - { status: 404, statusText: 'Not Found' }, - )) - const r = await validatePromotion('TWITTER15', 'pro') - expect(r.promotion.code).toBe('TWITTER15') - expect(r.promotion.discount.kind).toBe('percent_off') - expect(r.promotion.discount.value).toBe(15) + m.mockResolvedValueOnce(jsonResponse({ + ok: false, + error: 'promotion_not_found', + message: 'Code not found.', + agent_action: 'Tell the user this promo code is invalid.', + })) + await expect(validatePromotion('NONEXISTENT', 'pro')).rejects.toMatchObject({ + code: 'promotion_not_found', + message: 'Code not found.', + }) + // It DID hit the live endpoint (no dead seed fallback short-circuit). + expect(m).toHaveBeenCalledTimes(1) }) - it('throws promotion_not_found on 404 for an unknown code', async () => { + it('rejects a 200 ok:false body for a wrong-plan / expired code', async () => { const m = installFetch() - m.mockResolvedValueOnce(jsonResponse( - { error: 'not_found' }, - { status: 404, statusText: 'Not Found' }, - )) - await expect(validatePromotion('NONEXISTENT', 'pro')).rejects.toMatchObject({ - status: 404, - code: 'promotion_not_found', + m.mockResolvedValueOnce(jsonResponse({ + ok: false, + error: 'promotion_invalid', + message: "Promotion code \"OLDCODE\" is not valid for the pro plan.", + })) + await expect(validatePromotion('OLDCODE', 'pro')).rejects.toMatchObject({ + code: 'promotion_invalid', + }) + }) + + // Defensive: a malformed 200 ok:true WITHOUT a discount is still a rejection + // (we never surface a phantom valid promo with an empty discount). + it('rejects a 200 ok:true body that is missing the discount field', async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ ok: true, code: 'WEIRD' })) + await expect(validatePromotion('WEIRD', 'pro')).rejects.toMatchObject({ + code: 'promotion_invalid', }) }) - it('propagates 410 expired errors with the api message', async () => { + it('propagates a true 4xx (e.g. 429 rate-limited) untouched', async () => { const m = installFetch() m.mockResolvedValueOnce(jsonResponse( - { error: 'promotion_expired', message: 'This code has expired.' }, - { status: 410, statusText: 'Gone' }, + { error: 'rate_limit_exceeded', message: 'Too many validations.' }, + { status: 429, statusText: 'Too Many Requests' }, )) - await expect(validatePromotion('OLDCODE', 'pro')).rejects.toMatchObject({ - status: 410, - code: 'promotion_expired', + await expect(validatePromotion('ANYCODE', 'pro')).rejects.toMatchObject({ + status: 429, + code: 'rate_limit_exceeded', + }) + }) + + it('falls back to the normalised input code when the api omits code on success', async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, + discount: { kind: 'percent_off', value: 10, applies_to: ['pro'] }, + // no `code`, no `valid_until` (never-expiring code) + })) + const r = await validatePromotion('partner10', 'pro') + expect(r.promotion.code).toBe('PARTNER10') + expect(r.promotion.valid_until).toBeUndefined() + }) + + it('uses default reason copy when a 200 ok:false body omits error/message', async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ ok: false })) + await expect(validatePromotion('BARE', 'pro')).rejects.toMatchObject({ + code: 'promotion_invalid', + message: 'This promotion code is not valid for the selected plan.', }) }) @@ -1065,6 +1104,23 @@ describe('listStacks() env field', () => { const r = await listStacks() expect(r.items[0].env).toBe('production') }) + + // REGRESSION GUARD (P1-2): list rows from the api carry 'healthy' for live + // stacks. listStacks() must normalise it to 'running' so list/detail status + // comparisons (and the StatusPill) agree with the StackStatus type. + it("normalises 'healthy' list rows to 'running'", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, + items: [ + { stack_id: 'stk-live', name: 'demo', status: 'healthy', tier: 'pro', env: 'production', created_at: 'x' }, + { stack_id: 'stk-deploying', name: 'demo2', status: 'deploying', tier: 'pro', env: 'production', created_at: 'y' }, + ], + })) + const r = await listStacks() + expect(r.items[0].status).toBe('running') + expect(r.items[1].status).toBe('running') + }) }) // ─── listDeployments() — GET /api/v1/deployments adapter ───────────────── @@ -1796,6 +1852,19 @@ describe('createStack()', () => { expect(r.stack.status).toBe('building') expect(r.stack.url).toBeNull() }) + + // P1-2: a rare synchronous cached-build returns 'healthy'. createStack() must + // normalise it to 'running' so StackCreatePage.onSubmit skips to the live + // stage instead of polling. + it("normalises a synchronous 'healthy' response to 'running'", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, slug: 'cached-1', status: 'healthy', + url: 'https://cached-1.deployment.instanode.dev', + })) + const r = await createStack(fakeFile('a.tar.gz', 100), {}) + expect(r.stack.status).toBe('running') + }) }) // ─── fetchStackStatus() — GET /api/v1/stacks/:slug polling helper (D09/C06) ── @@ -1853,6 +1922,71 @@ describe('fetchStackStatus()', () => { m.mockResolvedValueOnce(jsonResponse({ error: 'internal' }, { status: 500 })) await expect(fetchStackStatus('s1')).rejects.toMatchObject({ status: 500 }) }) + + // REGRESSION GUARD (P1-2): the api emits 'healthy' for a LIVE stack, but the + // dashboard's StackStatus / StackCreatePage poll only treat 'running' as + // live. Without normalisation a successful deploy spins to the 5-min poll + // timeout. fetchStackStatus() must map 'healthy' → 'running' so the + // create-page poll's `status === 'running'` live-test trips. + it("normalises a live stack's 'healthy' status to 'running'", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, + stack_id: 'live-stack', + name: 'shipped', + status: 'healthy', // ← raw api live state + tier: 'pro', + services: [{ name: 'app', url: 'https://live-stack.deployment.instanode.dev', status: 'healthy' }], + })) + const r = await fetchStackStatus('live-stack') + expect(r.stack?.status).toBe('running') + expect(r.stack?.url).toBe('https://live-stack.deployment.instanode.dev') + }) + + it("normalises 'deploying' (rolling out) to 'running'", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, stack_id: 'rolling', status: 'deploying', tier: 'pro', + })) + const r = await fetchStackStatus('rolling') + expect(r.stack?.status).toBe('running') + }) + + it("passes 'failed' through unchanged so the poll can stop", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, stack_id: 'broke', status: 'failed', tier: 'pro', + })) + const r = await fetchStackStatus('broke') + expect(r.stack?.status).toBe('failed') + }) + + it("maps an unknown/empty status to 'building' (spinner, not crash)", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, stack_id: 'mystery', status: 'pending-something', tier: 'pro', + })) + const r = await fetchStackStatus('mystery') + expect(r.stack?.status).toBe('building') + }) + + it("maps 'deleting' to 'stopped' (terminal-ish, no compute)", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, stack_id: 'going-away', status: 'deleting', tier: 'pro', + })) + const r = await fetchStackStatus('going-away') + expect(r.stack?.status).toBe('stopped') + }) + + it("passes 'building' through unchanged (still in flight)", async () => { + const m = installFetch() + m.mockResolvedValueOnce(jsonResponse({ + ok: true, stack_id: 'wip', status: 'building', tier: 'pro', + })) + const r = await fetchStackStatus('wip') + expect(r.stack?.status).toBe('building') + }) }) // ─── APIError tier-wall envelope (P2-W2-16) ────────────────────────────── diff --git a/src/api/index.ts b/src/api/index.ts index 3d626f5..f1ae8af 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -901,7 +901,9 @@ export async function listStacks(): Promise<{ ok: true; items: DashboardStack[]; id: s.stack_id ?? '', slug: s.stack_id ?? '', name: s.name ?? '', - status: (s.status as DashboardStack['status']) ?? 'building', + // Normalise the api's 'healthy' (live) → 'running' so list/detail status + // comparisons agree with the type. See normaliseStackStatus. + status: normaliseStackStatus(s.status), url: null, created_at: s.created_at ?? '', team_id: '', @@ -980,6 +982,47 @@ function normaliseDeploymentStatus(s: string | undefined): DeploymentStatus { } } +// normaliseStackStatus — map the api's stack lifecycle status onto the +// dashboard's StackStatus vocabulary, mirroring normaliseDeploymentStatus. +// +// The api's stack statuses are: building | deploying | healthy | failed | +// stopped | deleting (api/internal/models/stack.go:564). The dashboard's +// StackStatus union is intentionally narrower: building | running | failed | +// stopped (types.ts). The two enums DISAGREE on the live state — the api calls +// it 'healthy', the dashboard calls it 'running' (matching the deployment +// path's StatusPill vocabulary). +// +// Before this normalizer, fetchStackStatus()/listStacks() force-cast the raw +// api string with `as DashboardStack['status']`, so a live stack surfaced as +// the runtime string 'healthy' — which the StackCreatePage poll's +// `status === 'running'` live-test never matched. A successful stack deploy +// therefore spun in the 'building' stage until the ~5-minute poll timeout +// instead of flipping to 'live'. Normalising here makes the type and the +// runtime value agree in one place: every consumer that compares against +// 'running' now works for a live stack. +// - healthy / deploying → running (the dashboard's single live/in-flight-to- +// live state; deploying is "build done, rolling out" which the create page +// should keep treating as in-progress-but-succeeding → render as running so +// the live test trips as soon as the pod is up). +// - deleting → stopped (no compute; the create flow never observes +// it, but a list view should show it as terminal rather than spinning). +function normaliseStackStatus(s: string | undefined): StackStatus { + switch (s) { + case 'healthy': + case 'deploying': + return 'running' // dashboard speaks 'running' for the live state + case 'building': + case 'failed': + case 'stopped': + case 'running': + return s + case 'deleting': + return 'stopped' // terminal-ish; consumes no compute + default: + return 'building' + } +} + function adaptDeployment(d: DeploymentRespItem): DashboardDeployment { // The server's `env` field is the env_vars map (legacy alias); the env // scope name lives under `environment`. New callers may also send a @@ -1620,7 +1663,10 @@ export async function createStack( // resolved). const stack: CreateStackResponse = { slug: body?.slug ?? body?.stack_id ?? body?.stack_slug ?? '', - status: (body?.status as StackStatus) ?? 'building', + // Normalise the lifecycle status: the api may return a cached-build + // 'healthy' synchronously (rare), which the StackCreatePage onSubmit path + // compares against 'running' to skip straight to the live stage. + status: normaliseStackStatus(body?.status), url: body?.url ?? null, name: body?.name, env: body?.env ?? body?.environment, @@ -1659,7 +1705,11 @@ export async function fetchStackStatus( id: r.stack_id, slug: r.stack_id, name: r.name ?? '', - status: (r.status as DashboardStack['status']) ?? 'building', + // CRITICAL (P1-2): the api emits 'healthy' for a live stack, but the + // StackCreatePage poll only treats 'running' as live. Normalise here so + // a successful deploy flips to the live stage instead of spinning to the + // 5-minute poll timeout. See normaliseStackStatus. + status: normaliseStackStatus(r.status), url: derivedURL, created_at: '', team_id: '', @@ -1760,7 +1810,11 @@ export async function deleteCustomDomain(stackSlug: string, id: string): Promise // propagate as APIError so the page's checkoutErr state // can surface them inline. // -// cancelSubscription — LIVE. POST /api/v1/billing/cancel. +// cancelSubscription — NOT live. POST /api/v1/billing/cancel was removed by +// policy (cancellation is support-only; memory project_no_self_serve_cancel_downgrade, +// api router.go:1133). This helper is unreferenced by any page/component and +// would 404 if called. Kept as an explicit "intentionally absent" marker; +// do NOT wire it into the UI. (P3-1 — filed for cleanup, harmless dead code.) // Billing wire shape — DERIVED from the OpenAPI snapshot (WireBillingState = // components['schemas']['BillingStateResponse']) so an api rename of e.g. @@ -1895,42 +1949,53 @@ export async function createCheckout( return { ok: true, short_url: r.short_url, subscription_id: r.subscription_id } } -// ─── Promotion validation (P3 — mocked until api ships endpoint) ──────── +// ─── Promotion validation (LIVE — POST /api/v1/billing/promotion/validate) ── +// +// The endpoint is LIVE (api/internal/handlers/billing_promotion.go, registered +// at router.go:1148). Its envelope is UNUSUAL: validation FAILURES are reported +// as **200 with `ok:false`**, NOT a 4xx. Only true request errors are non-2xx: +// 200 ok:true + code + discount + valid_until? — valid for this plan +// 200 ok:false + error + message + agent_action — invalid / wrong-plan / +// expired / exhausted +// 400 invalid_body — empty code / bad JSON +// 401 unauthorized — no session +// 429 rate_limit_exceeded — >30 validations / hour // -// Contract proposed for `POST /api/v1/billing/promotion/validate`: -// request: { code: string, plan: string } -// response: { ok: true, code, discount: { kind: "percent_off" | "amount_off" -// | "free_period", -// value: number, -// applies_to?: number, -// unit?: "months" | "days" }, -// valid_until: string /* ISO */ } -// errors: 404 { error: "promotion_not_found", message: "Code not found." } -// 410 { error: "promotion_expired", message: "This code has expired." } -// 409 { error: "promotion_not_applicable", -// message: "Code can't be applied to this plan." } +// CRITICAL (P1-3): call() only throws on a non-2xx response, so a 200 ok:false +// body returns normally. The previous implementation built a "valid" Promotion +// straight from that body — surfacing an invalid/expired/wrong-plan code as a +// VALID promo with an empty/undefined discount (a misleading-discount surface +// on the checkout path, and a money bug once Razorpay offers wire up). We now +// inspect `ok` and throw an APIError carrying the api's error/message so the UI +// renders the real rejection. There is no 404→seeds fallback any more: the +// handler never returns 404 (it shipped), so that branch was dead code. // -// api/internal/plans/promotion_test.go already has the engine -// (`plans.validatePromotion(code, plan) (Promotion, error)`) — the missing -// piece is the HTTP handler. Until that ships, this function transparently -// falls back to a small in-memory table of three seed codes so the upgrade -// flow is testable end-to-end. The mock activates on 404 (endpoint not -// registered) OR on a network error to /api/v1/billing/promotion/validate. +// `discount` is passed through verbatim from the api's promotionDiscount +// envelope ({ kind, value, applies_to, max_uses, description }); the type below +// stays permissive because it's display-only and the api may add fields. export type Promotion = { code: string discount: { kind: 'percent_off' | 'amount_off' | 'free_period' value: number - applies_to?: number + // api sends applies_to as a list of plan slugs; legacy callers expected a + // number. Tolerate either — this field is display-only. + applies_to?: number | string[] unit?: 'months' | 'days' + max_uses?: number + description?: string } - valid_until: string + /** ISO timestamp; the api omits it for never-expiring codes. */ + valid_until?: string } -const PROMOTION_SEEDS: Record = { - TWITTER15: { kind: 'percent_off', value: 15, applies_to: 3, unit: 'months' }, - LAUNCH50: { kind: 'percent_off', value: 50, applies_to: 1, unit: 'months' }, - COMEBACK10: { kind: 'percent_off', value: 10, applies_to: 1, unit: 'months' }, +type PromotionValidateBody = { + ok: boolean + code?: string + discount?: Promotion['discount'] + valid_until?: string + error?: string + message?: string } export async function validatePromotion( @@ -1941,43 +2006,24 @@ export async function validatePromotion( if (!normalized) { throw new APIError(400, 'promotion_invalid', 'Enter a code.') } - try { - const r = await call<{ - ok: boolean - code: string - discount: Promotion['discount'] - valid_until: string - }>('/api/v1/billing/promotion/validate', { - method: 'POST', - body: JSON.stringify({ code: normalized, plan }), - }) - return { - ok: true, - promotion: { code: r.code, discount: r.discount, valid_until: r.valid_until }, - } - } catch (e: any) { - // 404 = endpoint not yet shipped on api → fall back to local seeds so - // the upgrade flow is demo-able. Any other status (400/410/409/etc.) - // is a real validation error and propagates so the UI can show it. - const status = e?.status - if (status === 404 || status === undefined || status === 0) { - const seed = PROMOTION_SEEDS[normalized] - if (!seed) { - throw new APIError(404, 'promotion_not_found', 'Code not found.') - } - return { - ok: true, - promotion: { - code: normalized, - discount: seed, - // Mocked seeds are valid through 2026-09-01 — matches the spec - // example in the P3 brief. Replace with server response once the - // endpoint ships. - valid_until: '2026-09-01T00:00:00Z', - }, - } - } - throw e + // Non-2xx (400/401/429/5xx) throws from call() and propagates untouched. + const r = await call('/api/v1/billing/promotion/validate', { + method: 'POST', + body: JSON.stringify({ code: normalized, plan }), + }) + // 200 ok:false is a VALIDATION REJECTION, not a success. Surface it as an + // APIError so the page renders the api's reason ("Code not found", "expired", + // "not applicable to this plan", ...) instead of a phantom valid promo. + if (!r.ok || !r.discount) { + throw new APIError( + 400, + r.error || 'promotion_invalid', + r.message || 'This promotion code is not valid for the selected plan.', + ) + } + return { + ok: true, + promotion: { code: r.code ?? normalized, discount: r.discount, valid_until: r.valid_until }, } }