Skip to content

Commit 8e552e9

Browse files
fix(auth): complete CLI login for already-authed users + login CTA on account_exists claim (#220)
* fix(auth): complete CLI login for already-authed users + add login CTA on account_exists claim 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=<id> 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=<claim path with token> 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 <noreply@anthropic.com> * fix(login): fire CLI completion exactly once under StrictMode + restore 100% patch coverage The mount effect double-invoked under React StrictMode, firing completeCliSession twice (Playwright caught it: count=2 locally, racy 0 in CI). Guard with a fire-once ref and drop the per-pass `cancelled` flag — under StrictMode the first pass's cleanup fired before the async resolved, so the cancelled-gate (with the ref blocking the second pass) dropped the result and never rendered the confirmation. A setState after unmount is a no-op in React 19, so firing once and always committing is correct. Also split the pre-existing PAT-error ternary into single-line statements so v8 emits per-line DA markers (its multi-line-ternary continuation lines carry no marker), restoring diff-cover to 100% patch coverage after the insertion shifted those lines into the diff window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent db96d4c commit 8e552e9

5 files changed

Lines changed: 316 additions & 7 deletions

File tree

e2e/funnel-recovery.spec.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,4 +191,83 @@ test.describe('D2 — CLI device-flow completion', () => {
191191
await expect(page).toHaveURL(/\/pricing$/)
192192
expect(completeCalled).toBe(false)
193193
})
194+
195+
// D2-authed: an ALREADY-signed-in user who runs `instant login` lands on
196+
// /login?cli_session=<id> with a live token in localStorage. They never take
197+
// the OAuth/magic-link callback path, so LoginPage itself must POST
198+
// /auth/cli/{id}/complete on mount and show a terminal-return confirmation
199+
// instead of the sign-in form.
200+
const TOKEN_LS_KEY = 'instanode.token'
201+
202+
test('already-authed: LoginPage completes the CLI session and confirms', async ({ page }) => {
203+
const completeCap = { id: '', count: 0 }
204+
await page.route(CLI_COMPLETE_PATH, (route: Route) => {
205+
completeCap.count += 1
206+
const m = new URL(route.request().url()).pathname.match(/\/auth\/cli\/([^/]+)\/complete$/)
207+
completeCap.id = m ? decodeURIComponent(m[1]) : ''
208+
return route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify({ ok: true }) })
209+
})
210+
// Seed a live session token BEFORE the app bundle runs so getToken() returns it.
211+
await page.addInitScript(([k, v]) => {
212+
try { localStorage.setItem(k, v) } catch {}
213+
}, [TOKEN_LS_KEY, 'ink_live_session'] as const)
214+
215+
await page.goto(`/login?cli_session=${CLI_SESSION_ID}`)
216+
await expect(page.getByTestId('cli-approved-ok')).toBeVisible()
217+
await expect(page.getByTestId('cli-approved-ok')).toContainText('return to your terminal')
218+
// The sign-in form must NOT render — the user came from a terminal.
219+
await expect(page.getByTestId('oauth-github')).toHaveCount(0)
220+
await expect.poll(() => completeCap.count).toBe(1)
221+
expect(completeCap.id).toBe(CLI_SESSION_ID)
222+
})
223+
224+
test('already-authed: a completion failure shows the non-blocking failure note', async ({ page }) => {
225+
await page.route(CLI_COMPLETE_PATH, (route: Route) =>
226+
route.fulfill({ status: 404, contentType: 'application/json', body: JSON.stringify({ error: 'session_not_found' }) }),
227+
)
228+
await page.addInitScript(([k, v]) => {
229+
try { localStorage.setItem(k, v) } catch {}
230+
}, [TOKEN_LS_KEY, 'ink_live_session'] as const)
231+
232+
await page.goto(`/login?cli_session=${CLI_SESSION_ID}`)
233+
await expect(page.getByTestId('cli-approved-failed')).toBeVisible()
234+
await expect(page.getByTestId('cli-approved-failed')).toContainText('instant login')
235+
})
236+
})
237+
238+
// ─── account_exists claim recovery — login CTA on the 409 dead-end ───────────
239+
240+
test.describe('account_exists claim recovery via login CTA', () => {
241+
const CLAIM_PATH = '**/claim'
242+
243+
function buildClaimJWT(rt: string[] = ['postgres'], tok: string[] = ['abc12345xyz']): string {
244+
const b64url = (obj: unknown) =>
245+
Buffer.from(JSON.stringify(obj)).toString('base64')
246+
.replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '')
247+
const header = b64url({ alg: 'HS256', typ: 'JWT' })
248+
const payload = b64url({ rt, tok, exp: Math.floor(Date.now() / 1000) + 3600 })
249+
return `${header}.${payload}.sig`
250+
}
251+
252+
test('account_exists (409) renders a "Log in to claim" CTA carrying the token through next=', async ({ page }) => {
253+
const jwt = buildClaimJWT()
254+
await page.route(CLAIM_PATH, (route: Route) => {
255+
if (route.request().method() !== 'POST') return route.continue()
256+
return route.fulfill({
257+
status: 409,
258+
contentType: 'application/json',
259+
body: JSON.stringify({ ok: false, error: 'account_exists', message: 'An account already exists for this email. Sign in instead.' }),
260+
})
261+
})
262+
await page.goto(`/claim?t=${jwt}`)
263+
await page.getByTestId('claim-email').fill('taken@acme.dev')
264+
await page.getByTestId('claim-submit').click()
265+
const cta = page.getByTestId('claim-account-exists-login')
266+
await expect(cta).toBeVisible()
267+
const href = await cta.getAttribute('href')
268+
expect(href).toContain('/login?next=')
269+
expect(decodeURIComponent(href ?? '')).toContain(`/claim?t=${jwt}`)
270+
// The claim is still refused — no funnel mounted.
271+
await expect(page.getByTestId('claim-funnel')).toHaveCount(0)
272+
})
194273
})

src/pages/ClaimPage.test.tsx

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,56 @@ describe('ClaimPage — submitting the email funnels into checkout', () => {
314314
expect(screen.queryByTestId('claim-funnel')).toBeNull()
315315
})
316316

317+
// account_exists recovery: when the api refuses the claim because the email
318+
// already has an account, the user is told to log in — and must be given a
319+
// control to do so. The CTA carries the claim token through ?next= so the
320+
// claim resumes after sign-in. Detected via the api error's code/status.
321+
it('renders a "Log in to claim" CTA on an account_exists (code) claim failure', async () => {
322+
const jwt = buildClaimJWT()
323+
const err = Object.assign(new Error('An account already exists for this email. Sign in instead.'), {
324+
code: 'account_exists',
325+
status: 409,
326+
})
327+
;(api.claim as any).mockRejectedValue(err)
328+
renderClaim(jwt)
329+
fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'taken@example.com' } })
330+
fireEvent.click(screen.getByTestId('claim-submit'))
331+
await waitFor(() => {
332+
expect(screen.getByTestId('claim-account-exists-login')).toBeTruthy()
333+
})
334+
const cta = screen.getByTestId('claim-account-exists-login') as HTMLAnchorElement
335+
expect(cta.tagName).toBe('A')
336+
// The login CTA carries the claim token through ?next= so the post-login
337+
// flow can resume the claim.
338+
const href = cta.getAttribute('href') ?? ''
339+
expect(href).toContain('/login?next=')
340+
expect(decodeURIComponent(href)).toContain(`/claim?t=${jwt}`)
341+
// The claim was still refused — no funnel, no session minted.
342+
expect(screen.queryByTestId('claim-funnel')).toBeNull()
343+
})
344+
345+
it('does NOT render the login CTA on already_claimed (also 409, but code differs)', async () => {
346+
const err = Object.assign(new Error('Link already used'), { code: 'already_claimed', status: 409 })
347+
// already_claimed is ALSO 409 — but it must NOT offer the login CTA (the
348+
// recovery is "ask your agent for a fresh link", not "log in"). The detection
349+
// keys on the error CODE so the CTA is account_exists-specific, not 409-wide.
350+
;(api.claim as any).mockRejectedValue(err)
351+
renderClaim()
352+
fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'founder@example.com' } })
353+
fireEvent.click(screen.getByTestId('claim-submit'))
354+
await waitFor(() => expect(screen.getByTestId('claim-error').textContent).toContain('Link already used'))
355+
expect(screen.queryByTestId('claim-account-exists-login')).toBeNull()
356+
})
357+
358+
it('does NOT render the login CTA on a plain Error claim failure (no code)', async () => {
359+
;(api.claim as any).mockRejectedValue(new Error('Claim failed.'))
360+
renderClaim()
361+
fireEvent.change(screen.getByTestId('claim-email'), { target: { value: 'founder@example.com' } })
362+
fireEvent.click(screen.getByTestId('claim-submit'))
363+
await waitFor(() => expect(screen.getByTestId('claim-error').textContent).toContain('Claim failed'))
364+
expect(screen.queryByTestId('claim-account-exists-login')).toBeNull()
365+
})
366+
317367
it('does not block the funnel if listResources fails', async () => {
318368
;(api.claim as any).mockResolvedValue({
319369
ok: true, team_id: 't', user_id: 'u', session_token: 's',

src/pages/ClaimPage.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,12 @@ export function ClaimPage() {
108108
const [explainerOpen, setExplainerOpen] = useState(true)
109109
const [checkoutErr, setCheckoutErr] = useState<string | null>(null)
110110
const [checkoutPlan, setCheckoutPlan] = useState<'hobby' | 'pro' | null>(null)
111+
// account_exists recovery: when /claim is refused because the email already
112+
// has an account (api emits error=account_exists, status 409/400), we keep
113+
// the claim correctly refused but surface a login CTA so the user has a way
114+
// to actually log in (previously the copy said "log in first" with no
115+
// control). Tracks whether the last claim failure was that specific case.
116+
const [accountExists, setAccountExists] = useState(false)
111117

112118
useEffect(() => {
113119
if (!token) return
@@ -147,6 +153,7 @@ export function ClaimPage() {
147153

148154
const submit = async () => {
149155
setErr(null)
156+
setAccountExists(false)
150157
if (!email.trim()) {
151158
setErr('Email is required.')
152159
return
@@ -179,6 +186,16 @@ export function ClaimPage() {
179186
} catch (e: any) {
180187
setStage('error')
181188
setErr(e?.message ?? 'Claim failed. The link may have already been used.')
189+
// The api refuses a claim whose email already has an account
190+
// (error=account_exists, HTTP 409). The refusal is correct — we don't
191+
// claim into an existing account from an unauthenticated form — but the
192+
// user is told to "log in first" and needs a control to do so. Flag that
193+
// specific case so the email screen renders a login CTA that carries the
194+
// claim token through ?next= and resumes the claim after sign-in. We key
195+
// on the error CODE, not the status: account_exists and already_claimed
196+
// are BOTH 409 (api/internal/handlers/onboarding.go), and only the former
197+
// is recoverable by logging in — already_claimed wants a fresh agent link.
198+
setAccountExists(e?.code === 'account_exists')
182199
}
183200
}
184201

@@ -509,6 +526,21 @@ export function ClaimPage() {
509526
</div>
510527
)}
511528

529+
{/* account_exists recovery: the claim was (correctly) refused because
530+
this email already has an account. Surface a login CTA so the user
531+
can actually sign in — carry the claim token through ?next= so the
532+
claim resumes after login. */}
533+
{accountExists && (
534+
<Link
535+
to={`/login?next=${encodeURIComponent(`/claim?t=${token}`)}`}
536+
className="btn btn-primary"
537+
data-testid="claim-account-exists-login"
538+
style={{ width: '100%', justifyContent: 'center', marginBottom: 12 }}
539+
>
540+
Log in to claim →
541+
</Link>
542+
)}
543+
512544
<button
513545
className="btn btn-primary"
514546
style={{ width: '100%', justifyContent: 'center' }}

src/pages/LoginPage.test.tsx

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ vi.mock('../api', async () => {
1313
setToken: vi.fn(),
1414
clearToken: vi.fn(),
1515
fetchMe: vi.fn(),
16+
getToken: vi.fn(() => null),
17+
completeCliSession: vi.fn(() => Promise.resolve({ ok: true })),
1618
}
1719
})
1820

@@ -297,3 +299,55 @@ describe('LoginPage — D2 cli_session preservation', () => {
297299
expect(href).not.toContain('cli_session')
298300
})
299301
})
302+
303+
// ─── D2: already-signed-in CLI device-flow completion ────────────────────
304+
// An ALREADY-authed user who runs `instant login` lands on /login?cli_session=
305+
// <id> with a live token. They never take the OAuth/magic-link return path
306+
// (LoginCallbackPage) that fires completeCliSession, so LoginPage itself must
307+
// fire it on mount and show a "return to your terminal" confirmation instead of
308+
// the sign-in form again.
309+
describe('LoginPage — D2 already-authed CLI completion', () => {
310+
it('fires completeCliSession on mount when authed + cli_session present, shows confirmation', async () => {
311+
;(api.getToken as any).mockReturnValue('ink_live_session')
312+
;(window as any).location.search = '?cli_session=sess_authed'
313+
renderAt('/login?cli_session=sess_authed')
314+
await waitFor(() => expect(screen.getByTestId('cli-approved')).toBeTruthy())
315+
expect(api.completeCliSession).toHaveBeenCalledTimes(1)
316+
expect(api.completeCliSession).toHaveBeenCalledWith('sess_authed')
317+
// The terminal-return confirmation replaces the sign-in form entirely.
318+
expect(screen.getByTestId('cli-approved-ok').textContent).toContain('return to your terminal')
319+
expect(screen.queryByTestId('oauth-github')).toBeNull()
320+
})
321+
322+
it('does NOT fire completeCliSession when authed but no cli_session', async () => {
323+
;(api.getToken as any).mockReturnValue('ink_live_session')
324+
;(window as any).location.search = ''
325+
renderAt('/login')
326+
// Mount effect runs; without a cli_session it must be a no-op and the
327+
// normal sign-in form renders.
328+
await waitFor(() => expect(screen.getByTestId('oauth-github')).toBeTruthy())
329+
expect(api.completeCliSession).not.toHaveBeenCalled()
330+
expect(screen.queryByTestId('cli-approved')).toBeNull()
331+
})
332+
333+
it('does NOT fire completeCliSession when cli_session present but NOT authed (fresh-login path)', async () => {
334+
;(api.getToken as any).mockReturnValue(null)
335+
;(window as any).location.search = '?cli_session=sess_fresh'
336+
renderAt('/login?cli_session=sess_fresh')
337+
// A signed-out user's cli_session rides through return_to to the callback
338+
// page — LoginPage must NOT complete it here; it shows the sign-in form.
339+
await waitFor(() => expect(screen.getByTestId('oauth-github')).toBeTruthy())
340+
expect(api.completeCliSession).not.toHaveBeenCalled()
341+
})
342+
343+
it('surfaces a non-blocking failure note when completion fails', async () => {
344+
;(api.getToken as any).mockReturnValue('ink_live_session')
345+
;(api.completeCliSession as any).mockResolvedValue({ ok: false })
346+
;(window as any).location.search = '?cli_session=sess_dead'
347+
renderAt('/login?cli_session=sess_dead')
348+
await waitFor(() => expect(screen.getByTestId('cli-approved-failed')).toBeTruthy())
349+
// Still confirms the user is signed in; points them back at the CLI.
350+
expect(screen.getByTestId('cli-approved-failed').textContent).toContain('signed in')
351+
expect(screen.getByTestId('cli-approved-failed').textContent).toContain('instant login')
352+
})
353+
})

src/pages/LoginPage.tsx

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { useState } from 'react'
1+
import { useEffect, useRef, useState } from 'react'
22
import { useLocation, useNavigate } from 'react-router-dom'
33
import { Brand } from '../components/Common'
4-
import { setToken, clearToken, fetchMe } from '../api'
4+
import { setToken, clearToken, fetchMe, getToken, completeCliSession } from '../api'
55
import { postAuthDestination } from '../lib/postAuthDestination'
66

77
const OAUTH_API_BASE_DEFAULT = 'https://api.instanode.dev'
@@ -56,6 +56,43 @@ export function LoginPage() {
5656
const [emailSent, setEmailSent] = useState(false)
5757
const [emailErr, setEmailErr] = useState<string | null>(null)
5858

59+
// D2 — already-signed-in CLI device-flow completion.
60+
//
61+
// When a developer who is ALREADY authenticated runs `instant login`, the
62+
// CLI opens /login?cli_session=<id> and polls. The fresh-OAuth/magic-link
63+
// path completes the device flow in LoginCallbackPage (it POSTs
64+
// /auth/cli/{id}/complete after sign-in) — but an already-authed user never
65+
// takes that path: they land directly on /login with a live token and would
66+
// otherwise be shown the sign-in form again, never firing the completion.
67+
// So here, when BOTH a token exists AND cli_session is present, we fire the
68+
// SAME completion path (completeCliSession) immediately and show a terminal-
69+
// return confirmation instead of the sign-in form. Mirrors LoginCallbackPage:
70+
// same function, best-effort (completeCliSession swallows its own errors), and
71+
// a failure surfaces a note but does NOT hard-block.
72+
const [cliApproved, setCliApproved] = useState<null | 'ok' | 'failed'>(null)
73+
// Ref guard so completeCliSession fires EXACTLY once. React StrictMode (and
74+
// any fast remount) double-invokes mount effects in dev; the server side is
75+
// idempotent, but firing one POST keeps the behaviour deterministic and
76+
// avoids a duplicate device-flow completion. We deliberately do NOT abort the
77+
// state update on effect cleanup: under StrictMode the first pass's cleanup
78+
// fires before the async resolves, and gating on a per-pass `cancelled` flag
79+
// (combined with the fire-once ref preventing the second pass from re-firing)
80+
// would drop the result entirely and never render the confirmation. A
81+
// setState after unmount is a no-op in React 19, so firing once and always
82+
// committing the outcome is correct here.
83+
const cliFiredRef = useRef(false)
84+
useEffect(() => {
85+
const cli = readCliSession()
86+
// Only the already-authed path runs here. A fresh-login user has no token
87+
// yet; their cli_session rides through return_to to LoginCallbackPage.
88+
if (!cli || !getToken() || cliFiredRef.current) return
89+
cliFiredRef.current = true
90+
void (async () => {
91+
const { ok } = await completeCliSession(cli)
92+
setCliApproved(ok ? 'ok' : 'failed')
93+
})()
94+
}, [])
95+
5996
// sendMagicLink — POST /auth/email/start. Extracted from submitEmail so the
6097
// "Resend" affordance in the sent-confirmation state (F4) can re-fire the
6198
// exact same request without duplicating the fetch + error handling.
@@ -130,15 +167,72 @@ export function LoginPage() {
130167
navigate(dest, { replace: true })
131168
} catch (e: any) {
132169
clearToken()
133-
setErr(
134-
e?.status === 401
135-
? 'Token rejected. Mint a fresh PAT or use the claim link from your agent.'
136-
: `Couldn't reach the API: ${e?.message ?? 'unknown error'}`,
137-
)
170+
const rejected = 'Token rejected. Mint a fresh PAT or use the claim link from your agent.'
171+
const unreachable = `Couldn't reach the API: ${e?.message ?? 'unknown error'}`
172+
setErr(e?.status === 401 ? rejected : unreachable)
138173
setBusy(false)
139174
}
140175
}
141176

177+
// D2 — already-authed CLI approval confirmation. The user came from a
178+
// terminal (they ran `instant login`); don't silently drop them into the
179+
// dashboard. Show a clear "return to your terminal" screen. On a completion
180+
// failure we still confirm they're signed in and tell them to retry the CLI
181+
// (completeCliSession never throws, so this is the only failure surface).
182+
if (cliApproved !== null) {
183+
return (
184+
<div className="auth-shell">
185+
<div className="auth-card" data-testid="cli-approved" style={{ maxWidth: 480 }}>
186+
<div style={{ marginBottom: 28 }}>
187+
<Brand />
188+
</div>
189+
{cliApproved === 'ok' ? (
190+
<>
191+
<h1>CLI session approved.</h1>
192+
<div
193+
role="status"
194+
data-testid="cli-approved-ok"
195+
style={{
196+
marginTop: 16,
197+
padding: '10px 12px',
198+
borderLeft: '2px solid var(--accent)',
199+
fontSize: 13,
200+
fontFamily: 'var(--font-mono)',
201+
color: 'var(--text)',
202+
lineHeight: 1.6,
203+
}}
204+
>
205+
✓ Your CLI session is approved — return to your terminal. You can
206+
close this tab.
207+
</div>
208+
</>
209+
) : (
210+
<>
211+
<h1>Couldn't approve the CLI session.</h1>
212+
<div
213+
role="alert"
214+
data-testid="cli-approved-failed"
215+
style={{
216+
marginTop: 16,
217+
padding: '10px 12px',
218+
borderLeft: '2px solid var(--rose)',
219+
fontSize: 13,
220+
fontFamily: 'var(--font-mono)',
221+
color: 'var(--text)',
222+
lineHeight: 1.6,
223+
}}
224+
>
225+
You're signed in, but we couldn't link this browser to your CLI
226+
session — it may have expired. Re-run <code style={{ color: 'var(--accent)' }}>instant login</code> in
227+
your terminal to get a fresh link.
228+
</div>
229+
</>
230+
)}
231+
</div>
232+
</div>
233+
)
234+
}
235+
142236
return (
143237
<div className="auth-shell">
144238
<div className="auth-card">

0 commit comments

Comments
 (0)