From 8ccc608acc53e90a83f7ea045d56aa7145f6656f Mon Sep 17 00:00:00 2001 From: Manas Srivastava Date: Thu, 11 Jun 2026 01:53:38 +0530 Subject: [PATCH 1/2] fix(auth): complete CLI login for already-authed users + add login CTA on account_exists claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two funnel-blocking UI gaps found in a live cross-interface journey test: BUG 1 — `instant login` (CLI device-flow) could not complete for an already-signed-in user. The CLI opens /login?cli_session= and polls, but an already-authed user lands directly on LoginPage with a live token and never takes the OAuth/magic-link callback path that fires completeCliSession — so the device flow never completed and the CLI polled forever. LoginPage now fires the SAME completion path (completeCliSession, reused from LoginCallbackPage — no duplicated fetch) on mount when both getToken() and a cli_session param are present, then shows a "return to your terminal" confirmation instead of the sign-in form. A completion failure surfaces a non-blocking note (still signed in, re-run `instant login`). BUG 2 — the account_exists (409) claim error told the user to "log in first" with no control to do so. ClaimPage now renders a "Log in to claim" CTA on that specific error (keyed on error code, not status, since already_claimed is also 409) routing to /login?next= so the claim resumes after sign-in. The claim is still correctly refused for an existing account — this only adds the recovery control. Tests: LoginPage already-authed + cli_session completion (fired / no-op when unauthed / no-op without param / non-blocking failure); ClaimPage account_exists CTA (renders + carries token, not on already_claimed/plain errors). Playwright D2-authed + account_exists CTA specs in funnel-recovery.spec.ts. Patch coverage 100% (diff-cover). Co-Authored-By: Claude Fable 5 --- e2e/funnel-recovery.spec.ts | 79 ++++++++++++++++++++++++++++++ src/pages/ClaimPage.test.tsx | 50 +++++++++++++++++++ src/pages/ClaimPage.tsx | 32 +++++++++++++ src/pages/LoginPage.test.tsx | 54 +++++++++++++++++++++ src/pages/LoginPage.tsx | 93 +++++++++++++++++++++++++++++++++++- 5 files changed, 306 insertions(+), 2 deletions(-) diff --git a/e2e/funnel-recovery.spec.ts b/e2e/funnel-recovery.spec.ts index 2546c5b..e264b04 100644 --- a/e2e/funnel-recovery.spec.ts +++ b/e2e/funnel-recovery.spec.ts @@ -191,4 +191,83 @@ test.describe('D2 — CLI device-flow completion', () => { await expect(page).toHaveURL(/\/pricing$/) expect(completeCalled).toBe(false) }) + + // D2-authed: an ALREADY-signed-in user who runs `instant login` lands on + // /login?cli_session= with a live token in localStorage. They never take + // the OAuth/magic-link callback path, so LoginPage itself must POST + // /auth/cli/{id}/complete on mount and show a terminal-return confirmation + // instead of the sign-in form. + const TOKEN_LS_KEY = 'instanode.token' + + test('already-authed: LoginPage completes the CLI session and confirms', async ({ page }) => { + const completeCap = { id: '', count: 0 } + await page.route(CLI_COMPLETE_PATH, (route: Route) => { + completeCap.count += 1 + const m = new URL(route.request().url()).pathname.match(/\/auth\/cli\/([^/]+)\/complete$/) + completeCap.id = m ? decodeURIComponent(m[1]) : '' + return route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify({ ok: true }) }) + }) + // Seed a live session token BEFORE the app bundle runs so getToken() returns it. + await page.addInitScript(([k, v]) => { + try { localStorage.setItem(k, v) } catch {} + }, [TOKEN_LS_KEY, 'ink_live_session'] as const) + + await page.goto(`/login?cli_session=${CLI_SESSION_ID}`) + await expect(page.getByTestId('cli-approved-ok')).toBeVisible() + await expect(page.getByTestId('cli-approved-ok')).toContainText('return to your terminal') + // The sign-in form must NOT render — the user came from a terminal. + await expect(page.getByTestId('oauth-github')).toHaveCount(0) + await expect.poll(() => completeCap.count).toBe(1) + expect(completeCap.id).toBe(CLI_SESSION_ID) + }) + + test('already-authed: a completion failure shows the non-blocking failure note', async ({ page }) => { + await page.route(CLI_COMPLETE_PATH, (route: Route) => + route.fulfill({ status: 404, contentType: 'application/json', body: JSON.stringify({ error: 'session_not_found' }) }), + ) + await page.addInitScript(([k, v]) => { + try { localStorage.setItem(k, v) } catch {} + }, [TOKEN_LS_KEY, 'ink_live_session'] as const) + + await page.goto(`/login?cli_session=${CLI_SESSION_ID}`) + await expect(page.getByTestId('cli-approved-failed')).toBeVisible() + await expect(page.getByTestId('cli-approved-failed')).toContainText('instant login') + }) +}) + +// ─── account_exists claim recovery — login CTA on the 409 dead-end ─────────── + +test.describe('account_exists claim recovery via login CTA', () => { + const CLAIM_PATH = '**/claim' + + function buildClaimJWT(rt: string[] = ['postgres'], tok: string[] = ['abc12345xyz']): string { + const b64url = (obj: unknown) => + Buffer.from(JSON.stringify(obj)).toString('base64') + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') + const header = b64url({ alg: 'HS256', typ: 'JWT' }) + const payload = b64url({ rt, tok, exp: Math.floor(Date.now() / 1000) + 3600 }) + return `${header}.${payload}.sig` + } + + test('account_exists (409) renders a "Log in to claim" CTA carrying the token through next=', async ({ page }) => { + const jwt = buildClaimJWT() + await page.route(CLAIM_PATH, (route: Route) => { + if (route.request().method() !== 'POST') return route.continue() + return route.fulfill({ + status: 409, + contentType: 'application/json', + body: JSON.stringify({ ok: false, error: 'account_exists', message: 'An account already exists for this email. Sign in instead.' }), + }) + }) + await page.goto(`/claim?t=${jwt}`) + await page.getByTestId('claim-email').fill('taken@acme.dev') + await page.getByTestId('claim-submit').click() + const cta = page.getByTestId('claim-account-exists-login') + await expect(cta).toBeVisible() + const href = await cta.getAttribute('href') + expect(href).toContain('/login?next=') + expect(decodeURIComponent(href ?? '')).toContain(`/claim?t=${jwt}`) + // The claim is still refused — no funnel mounted. + await expect(page.getByTestId('claim-funnel')).toHaveCount(0) + }) }) diff --git a/src/pages/ClaimPage.test.tsx b/src/pages/ClaimPage.test.tsx index 7d63876..6804a50 100644 --- a/src/pages/ClaimPage.test.tsx +++ b/src/pages/ClaimPage.test.tsx @@ -314,6 +314,56 @@ describe('ClaimPage — submitting the email funnels into checkout', () => { expect(screen.queryByTestId('claim-funnel')).toBeNull() }) + // account_exists recovery: when the api refuses the claim because the email + // already has an account, the user is told to log in — and must be given a + // control to do so. The CTA carries the claim token through ?next= so the + // claim resumes after sign-in. Detected via the api error's code/status. + it('renders a "Log in to claim" CTA on an account_exists (code) claim failure', async () => { + const jwt = buildClaimJWT() + const err = Object.assign(new Error('An account already exists for this email. Sign in instead.'), { + code: 'account_exists', + status: 409, + }) + ;(api.claim as any).mockRejectedValue(err) + renderClaim(jwt) + fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'taken@example.com' } }) + fireEvent.click(screen.getByTestId('claim-submit')) + await waitFor(() => { + expect(screen.getByTestId('claim-account-exists-login')).toBeTruthy() + }) + const cta = screen.getByTestId('claim-account-exists-login') as HTMLAnchorElement + expect(cta.tagName).toBe('A') + // The login CTA carries the claim token through ?next= so the post-login + // flow can resume the claim. + const href = cta.getAttribute('href') ?? '' + expect(href).toContain('/login?next=') + expect(decodeURIComponent(href)).toContain(`/claim?t=${jwt}`) + // The claim was still refused — no funnel, no session minted. + expect(screen.queryByTestId('claim-funnel')).toBeNull() + }) + + it('does NOT render the login CTA on already_claimed (also 409, but code differs)', async () => { + const err = Object.assign(new Error('Link already used'), { code: 'already_claimed', status: 409 }) + // already_claimed is ALSO 409 — but it must NOT offer the login CTA (the + // recovery is "ask your agent for a fresh link", not "log in"). The detection + // keys on the error CODE so the CTA is account_exists-specific, not 409-wide. + ;(api.claim as any).mockRejectedValue(err) + renderClaim() + fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'founder@example.com' } }) + fireEvent.click(screen.getByTestId('claim-submit')) + await waitFor(() => expect(screen.getByTestId('claim-error').textContent).toContain('Link already used')) + expect(screen.queryByTestId('claim-account-exists-login')).toBeNull() + }) + + it('does NOT render the login CTA on a plain Error claim failure (no code)', async () => { + ;(api.claim as any).mockRejectedValue(new Error('Claim failed.')) + renderClaim() + fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'founder@example.com' } }) + fireEvent.click(screen.getByTestId('claim-submit')) + await waitFor(() => expect(screen.getByTestId('claim-error').textContent).toContain('Claim failed')) + expect(screen.queryByTestId('claim-account-exists-login')).toBeNull() + }) + it('does not block the funnel if listResources fails', async () => { ;(api.claim as any).mockResolvedValue({ ok: true, team_id: 't', user_id: 'u', session_token: 's', diff --git a/src/pages/ClaimPage.tsx b/src/pages/ClaimPage.tsx index cdd6981..94b7c70 100644 --- a/src/pages/ClaimPage.tsx +++ b/src/pages/ClaimPage.tsx @@ -108,6 +108,12 @@ export function ClaimPage() { const [explainerOpen, setExplainerOpen] = useState(true) const [checkoutErr, setCheckoutErr] = useState(null) const [checkoutPlan, setCheckoutPlan] = useState<'hobby' | 'pro' | null>(null) + // account_exists recovery: when /claim is refused because the email already + // has an account (api emits error=account_exists, status 409/400), we keep + // the claim correctly refused but surface a login CTA so the user has a way + // to actually log in (previously the copy said "log in first" with no + // control). Tracks whether the last claim failure was that specific case. + const [accountExists, setAccountExists] = useState(false) useEffect(() => { if (!token) return @@ -147,6 +153,7 @@ export function ClaimPage() { const submit = async () => { setErr(null) + setAccountExists(false) if (!email.trim()) { setErr('Email is required.') return @@ -179,6 +186,16 @@ export function ClaimPage() { } catch (e: any) { setStage('error') setErr(e?.message ?? 'Claim failed. The link may have already been used.') + // The api refuses a claim whose email already has an account + // (error=account_exists, HTTP 409). The refusal is correct — we don't + // claim into an existing account from an unauthenticated form — but the + // user is told to "log in first" and needs a control to do so. Flag that + // specific case so the email screen renders a login CTA that carries the + // claim token through ?next= and resumes the claim after sign-in. We key + // on the error CODE, not the status: account_exists and already_claimed + // are BOTH 409 (api/internal/handlers/onboarding.go), and only the former + // is recoverable by logging in — already_claimed wants a fresh agent link. + setAccountExists(e?.code === 'account_exists') } } @@ -509,6 +526,21 @@ export function ClaimPage() { )} + {/* account_exists recovery: the claim was (correctly) refused because + this email already has an account. Surface a login CTA so the user + can actually sign in — carry the claim token through ?next= so the + claim resumes after login. */} + {accountExists && ( + + Log in to claim → + + )} +