From 11d04d9d5efef4e835c6b36f089e1b535251c3ce Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 7 May 2026 23:58:12 -0400 Subject: [PATCH] refactor(desktop): tighten onboarding store + overlay Drop the dead isOnboardingBusy/BUSY set, factor the catch-fallback dance into safeReq, and share a single reloadAndConnect helper between PKCE submit, device-code success, external recheck, and api-key save. In the overlay, extract Step / CodeBlock / FlowFooter / CancelBtn / DocsLink atoms so the four sign-in panels share the same chrome instead of repeating it inline. Net effect: fewer literal divs, one place to touch the spacing, and the code-block + footer rows are reusable across future flows. --- .../components/desktop-onboarding-overlay.tsx | 271 ++++++++---------- apps/desktop/src/store/onboarding.ts | 199 ++++++------- 2 files changed, 214 insertions(+), 256 deletions(-) diff --git a/apps/desktop/src/components/desktop-onboarding-overlay.tsx b/apps/desktop/src/components/desktop-onboarding-overlay.tsx index 992756f498a..b8833356df2 100644 --- a/apps/desktop/src/components/desktop-onboarding-overlay.tsx +++ b/apps/desktop/src/components/desktop-onboarding-overlay.tsx @@ -38,6 +38,8 @@ interface ApiKeyOption { short?: string } +const MIN_KEY_LENGTH = 8 + const API_KEY_OPTIONS: ApiKeyOption[] = [ { id: 'openrouter', @@ -97,17 +99,11 @@ const FLOW_SUBTITLES: Record = { external: 'Sign in once in your terminal, then come back to chat.' } -function providerTitle(provider: OAuthProvider) { - return PROVIDER_DISPLAY[provider.id]?.title ?? provider.name -} +const providerTitle = (p: OAuthProvider) => PROVIDER_DISPLAY[p.id]?.title ?? p.name +const orderOf = (p: OAuthProvider) => PROVIDER_DISPLAY[p.id]?.order ?? 99 -function sortProviders(providers: OAuthProvider[]) { - return [...providers].sort((a, b) => { - const order = (PROVIDER_DISPLAY[a.id]?.order ?? 99) - (PROVIDER_DISPLAY[b.id]?.order ?? 99) - - return order !== 0 ? order : a.name.localeCompare(b.name) - }) -} +const sortProviders = (providers: OAuthProvider[]) => + [...providers].sort((a, b) => orderOf(a) - orderOf(b) || a.name.localeCompare(b.name)) export function DesktopOnboardingOverlay({ enabled, onCompleted, requestGateway }: DesktopOnboardingOverlayProps) { const onboarding = useStore($desktopOnboarding) @@ -124,28 +120,23 @@ export function DesktopOnboardingOverlay({ enabled, onCompleted, requestGateway ) useEffect(() => { - if (!enabled && !onboarding.requested) { - return + if (enabled || onboarding.requested) { + void refreshOnboarding(ctx) } - - void refreshOnboarding(ctx) }, [ctx, enabled, onboarding.requested]) if (!visible) { return null } + const { flow } = onboarding + const showPicker = flow.status === 'idle' || flow.status === 'success' + return (
-
- {onboarding.flow.status === 'idle' || onboarding.flow.status === 'success' ? ( - - ) : ( - - )} -
+
{showPicker ? : }
) @@ -181,18 +172,18 @@ function Picker({ ctx }: { ctx: OnboardingContext }) { return (
{providers === null ? ( - }>Looking up providers... + Looking up providers... ) : ( ordered.map(provider => ( void startProviderOAuth(p, ctx)} provider={provider} /> )) )} - setOnboardingMode('apikey')}>I have an API key + setOnboardingMode('apikey')}>I have an API key
) } -function ModeSwitchLink({ children, onClick }: { children: React.ReactNode; onClick: () => void }) { +function FooterLink({ children, onClick }: { children: React.ReactNode; onClick: () => void }) { return (
) : null} +
{API_KEY_OPTIONS.map(o => ( - ) : null} + {option.docsUrl ? Get a key : null}
setValue(event.target.value)} - onKeyDown={event => event.key === 'Enter' && void submit()} + onChange={e => setValue(e.target.value)} + onKeyDown={e => e.key === 'Enter' && void submit()} placeholder={option.placeholder || 'Paste API key'} type={isLocal ? 'text' : 'password'} value={value} @@ -345,11 +323,11 @@ function FlowPanel({ ctx, flow }: { ctx: OnboardingContext; flow: OnboardingFlow const title = 'provider' in flow && flow.provider ? providerTitle(flow.provider) : '' if (flow.status === 'starting') { - return }>Starting sign-in for {title}... + return Starting sign-in for {title}... } if (flow.status === 'submitting') { - return }>Verifying your code with {title}... + return Verifying your code with {title}... } if (flow.status === 'success') { @@ -378,80 +356,45 @@ function FlowPanel({ ctx, flow }: { ctx: OnboardingContext; flow: OnboardingFlow if (flow.status === 'awaiting_user') { return ( -
-
-

Sign in with {title}

-
    -
  1. We opened {title} in your browser.
    2. -
  2. Authorize Hermes there.
    3. -
  3. Copy the authorization code and paste it below.
    4. -
-
+ +
    +
  1. We opened {title} in your browser.
    2. +
  2. Authorize Hermes there.
    3. +
  3. Copy the authorization code and paste it below.
    4. +
setOnboardingCode(event.target.value)} - onKeyDown={event => event.key === 'Enter' && void submitOnboardingCode(ctx)} + onChange={e => setOnboardingCode(e.target.value)} + onKeyDown={e => e.key === 'Enter' && void submitOnboardingCode(ctx)} placeholder="Paste authorization code" value={flow.code} /> -
- -
- - -
-
-
+ + ) } if (flow.status === 'external_pending') { return ( -
-
-

Sign in with {title}

-

- {title} signs in through its own CLI. Run this command in a terminal, then come back and pick "I've signed - in": -

-
-
- {flow.provider.cli_command} - -
-
- {flow.provider.docs_url ? ( - - ) : ( - - )} -
- - -
-
-
+ + ) } @@ -460,44 +403,82 @@ function FlowPanel({ ctx, flow }: { ctx: OnboardingContext; flow: OnboardingFlow } return ( -
-
-

Sign in with {title}

-

- We opened {title} in your browser. Enter this code there: -

-
-
- {flow.start.user_code} - -
-
- -
-
- - Waiting for you to authorize... -
- -
-
-
+ +

We opened {title} in your browser. Enter this code there:

+ void copyDeviceCode()} text={flow.start.user_code} /> + Re-open verification page}> + + + Waiting for you to authorize... + + + + ) } -function Status({ children, icon }: { children: React.ReactNode; icon: React.ReactNode }) { +function Step({ children, title }: { children: React.ReactNode; title: string }) { return ( -
- {icon} +
+

{title}

+ {children} +
+ ) +} + +function CodeBlock({ + copied, + large, + onCopy, + text +}: { + copied: boolean + large?: boolean + onCopy: () => void + text: string +}) { + return ( +
+ {text} + +
+ ) +} + +function FlowFooter({ children, left }: { children: React.ReactNode; left?: React.ReactNode }) { + return ( +
+
{left}
+
{children}
+
+ ) +} + +function CancelBtn({ size = 'default' }: { size?: 'default' | 'sm' }) { + return ( + + ) +} + +function DocsLink({ children, href }: { children: React.ReactNode; href: string }) { + return ( + + ) +} + +function Status({ children }: { children: React.ReactNode }) { + return ( +
+ {children}
) diff --git a/apps/desktop/src/store/onboarding.ts b/apps/desktop/src/store/onboarding.ts index 032c4046011..c2299cbba41 100644 --- a/apps/desktop/src/store/onboarding.ts +++ b/apps/desktop/src/store/onboarding.ts @@ -11,13 +11,16 @@ import { import { notify, notifyError } from '@/store/notifications' import type { OAuthProvider, OAuthStartResponse } from '@/types/hermes' +type PkceStart = Extract +type DeviceStart = Extract + export type OnboardingMode = 'apikey' | 'oauth' export type OnboardingFlow = | { status: 'idle' } | { provider: OAuthProvider; status: 'starting' } - | { code: string; provider: OAuthProvider; start: Extract; status: 'awaiting_user' } - | { copied: boolean; provider: OAuthProvider; start: Extract; status: 'polling' } + | { code: string; provider: OAuthProvider; start: PkceStart; status: 'awaiting_user' } + | { copied: boolean; provider: OAuthProvider; start: DeviceStart; status: 'polling' } | { provider: OAuthProvider; start: OAuthStartResponse; status: 'submitting' } | { copied: boolean; provider: OAuthProvider; status: 'external_pending' } | { provider: OAuthProvider; status: 'success' } @@ -32,6 +35,11 @@ export interface DesktopOnboardingState { requested: boolean } +export interface OnboardingContext { + onCompleted?: () => void + requestGateway: (method: string, params?: Record) => Promise +} + const INITIAL: DesktopOnboardingState = { configured: true, flow: { status: 'idle' }, @@ -41,32 +49,21 @@ const INITIAL: DesktopOnboardingState = { requested: false } -export const $desktopOnboarding = atom(INITIAL) - -export interface OnboardingContext { - onCompleted?: () => void - requestGateway: (method: string, params?: Record) => Promise -} - const POLL_MS = 2000 +const COPY_FLASH_MS = 1500 -const BUSY: ReadonlySet = new Set([ - 'starting', - 'awaiting_user', - 'polling', - 'submitting', - 'external_pending' -]) +export const $desktopOnboarding = atom(INITIAL) let pollTimer: number | null = null -function patch(update: Partial) { - $desktopOnboarding.set({ ...$desktopOnboarding.get(), ...update }) -} +const errMessage = (e: unknown) => (e instanceof Error ? e.message : String(e)) -function setFlow(flow: OnboardingFlow) { - patch({ flow }) -} +const patch = (update: Partial) => + $desktopOnboarding.set({ ...$desktopOnboarding.get(), ...update }) + +const setFlow = (flow: OnboardingFlow) => patch({ flow }) + +const sessionIdFor = (flow: OnboardingFlow) => ('start' in flow && flow.start ? flow.start.session_id : undefined) function clearPoll() { if (pollTimer !== null) { @@ -75,23 +72,40 @@ function clearPoll() { } } -function errMessage(error: unknown) { - return error instanceof Error ? error.message : String(error) +async function safeReq(ctx: OnboardingContext, method: string, fallback: T): Promise { + try { + return await ctx.requestGateway(method) + } catch { + return fallback + } } async function checkRuntime(ctx: OnboardingContext) { const [status, runtime] = await Promise.all([ - ctx.requestGateway<{ provider_configured?: boolean }>('setup.status').catch( - () => ({}) as { provider_configured?: boolean } - ), - ctx - .requestGateway<{ error?: string; ok?: boolean }>('setup.runtime_check') - .catch(() => ({ ok: false }) as { error?: string; ok?: boolean }) + safeReq<{ provider_configured?: boolean }>(ctx, 'setup.status', {}), + safeReq<{ error?: string; ok?: boolean }>(ctx, 'setup.runtime_check', { ok: false }) ]) return runtime.ok !== undefined ? Boolean(runtime.ok) : Boolean(status.provider_configured) } +function notifyReady(provider: string) { + notify({ kind: 'success', title: 'Hermes is ready', message: `${provider} connected.` }) +} + +async function reloadAndConnect(ctx: OnboardingContext, providerName: string, onFail: () => void) { + await ctx.requestGateway('reload.env').catch(() => undefined) + const ok = await checkRuntime(ctx) + + if (ok) { + notifyReady(providerName) + completeDesktopOnboarding() + ctx.onCompleted?.() + } else { + onFail() + } +} + export function requestDesktopOnboarding(reason = 'No inference provider is configured.') { patch({ reason, requested: true }) } @@ -106,9 +120,7 @@ export function setOnboardingMode(mode: OnboardingMode) { } export async function refreshOnboarding(ctx: OnboardingContext) { - const ok = await checkRuntime(ctx) - - if (ok) { + if (await checkRuntime(ctx)) { completeDesktopOnboarding() ctx.onCompleted?.() @@ -123,10 +135,7 @@ export async function refreshOnboarding(ctx: OnboardingContext) { try { const { providers } = await listOAuthProviders() - patch({ - providers, - mode: providers.length > 0 ? 'oauth' : 'apikey' - }) + patch({ providers, mode: providers.length > 0 ? 'oauth' : 'apikey' }) } catch { patch({ providers: [], mode: 'apikey' }) } @@ -134,34 +143,15 @@ export async function refreshOnboarding(ctx: OnboardingContext) { return false } -async function finalize(provider: OAuthProvider, ctx: OnboardingContext) { - clearPoll() - setFlow({ status: 'success', provider }) - notify({ kind: 'success', title: 'Hermes is ready', message: `${provider.name} connected.` }) - await ctx.requestGateway('reload.env').catch(() => undefined) - const ok = await checkRuntime(ctx) - - if (ok) { - completeDesktopOnboarding() - ctx.onCompleted?.() - } else { - setFlow({ - status: 'error', - provider, - message: 'Connected, but Hermes still cannot resolve a usable provider.' - }) - } -} - export async function startProviderOAuth(provider: OAuthProvider, ctx: OnboardingContext) { + clearPoll() + if (provider.flow === 'external') { - clearPoll() setFlow({ status: 'external_pending', provider, copied: false }) return } - clearPoll() setFlow({ status: 'starting', provider }) try { @@ -181,24 +171,23 @@ export async function startProviderOAuth(provider: OAuthProvider, ctx: Onboardin } } -async function pollDevice( - provider: OAuthProvider, - start: Extract, - ctx: OnboardingContext -) { +async function pollDevice(provider: OAuthProvider, start: DeviceStart, ctx: OnboardingContext) { try { - const resp = await pollOAuthSession(provider.id, start.session_id) + const { error_message, status } = await pollOAuthSession(provider.id, start.session_id) - if (resp.status === 'approved') { - await finalize(provider, ctx) - } else if (resp.status !== 'pending') { + if (status === 'approved') { clearPoll() - setFlow({ - status: 'error', - provider, - start, - message: resp.error_message || `Sign-in ${resp.status}.` - }) + setFlow({ status: 'success', provider }) + await reloadAndConnect(ctx, provider.name, () => + setFlow({ + status: 'error', + provider, + message: 'Connected, but Hermes still cannot resolve a usable provider.' + }) + ) + } else if (status !== 'pending') { + clearPoll() + setFlow({ status: 'error', provider, start, message: error_message || `Sign-in ${status}.` }) } } catch (error) { clearPoll() @@ -228,7 +217,14 @@ export async function submitOnboardingCode(ctx: OnboardingContext) { const resp = await submitOAuthCode(provider.id, start.session_id, code.trim()) if (resp.ok && resp.status === 'approved') { - await finalize(provider, ctx) + setFlow({ status: 'success', provider }) + await reloadAndConnect(ctx, provider.name, () => + setFlow({ + status: 'error', + provider, + message: 'Connected, but Hermes still cannot resolve a usable provider.' + }) + ) } else { setFlow({ status: 'error', provider, start, message: resp.message || 'Token exchange failed.' }) } @@ -238,15 +234,8 @@ export async function submitOnboardingCode(ctx: OnboardingContext) { } export function cancelOnboardingFlow() { - const { flow } = $desktopOnboarding.get() clearPoll() - - const sessionId = - flow.status === 'awaiting_user' || flow.status === 'polling' || flow.status === 'submitting' - ? flow.start?.session_id - : flow.status === 'error' - ? flow.start?.session_id - : undefined + const sessionId = sessionIdFor($desktopOnboarding.get().flow) if (sessionId) { cancelOAuthSession(sessionId).catch(() => undefined) @@ -275,7 +264,7 @@ async function copyAndFlash(text: string, predicate: (flow: OnboardingFlow) => b if (predicate(current) && 'copied' in current) { setFlow({ ...current, copied: false }) } - }, 1500) + }, COPY_FLASH_MS) } export async function copyDeviceCode() { @@ -285,10 +274,8 @@ export async function copyDeviceCode() { return } - await copyAndFlash( - flow.start.user_code, - f => f.status === 'polling' && f.start.session_id === flow.start.session_id - ) + const sid = flow.start.session_id + await copyAndFlash(flow.start.user_code, f => f.status === 'polling' && f.start.session_id === sid) } export async function copyExternalCommand() { @@ -298,7 +285,8 @@ export async function copyExternalCommand() { return } - await copyAndFlash(flow.provider.cli_command, f => f.status === 'external_pending' && f.provider.id === flow.provider.id) + const id = flow.provider.id + await copyAndFlash(flow.provider.cli_command, f => f.status === 'external_pending' && f.provider.id === id) } export async function recheckExternalSignin(ctx: OnboardingContext) { @@ -308,19 +296,14 @@ export async function recheckExternalSignin(ctx: OnboardingContext) { return } - const ok = await checkRuntime(ctx) - - if (ok) { - notify({ kind: 'success', title: 'Hermes is ready', message: `${flow.provider.name} connected.` }) - completeDesktopOnboarding() - ctx.onCompleted?.() - } else { + const { provider } = flow + await reloadAndConnect(ctx, provider.name, () => setFlow({ status: 'error', - provider: flow.provider, - message: `Hermes still cannot reach ${flow.provider.name}. Run \`${flow.provider.cli_command}\` in a terminal first.` + provider, + message: `Hermes still cannot reach ${provider.name}. Run \`${provider.cli_command}\` in a terminal first.` }) - } + ) } export async function saveOnboardingApiKey(envKey: string, value: string, label: string, ctx: OnboardingContext) { @@ -332,25 +315,19 @@ export async function saveOnboardingApiKey(envKey: string, value: string, label: try { await setEnvVar(envKey, trimmed) - await ctx.requestGateway('reload.env').catch(() => undefined) - const ok = await checkRuntime(ctx) + let stillFailing = false + await reloadAndConnect(ctx, label, () => { + stillFailing = true + }) - if (ok) { - notify({ kind: 'success', title: 'Hermes is ready', message: `${label} connected.` }) - completeDesktopOnboarding() - ctx.onCompleted?.() - - return { ok: true } + if (stillFailing) { + return { ok: false, message: `Saved, but Hermes still cannot reach ${label}. Double-check the value.` } } - return { ok: false, message: `Saved, but Hermes still cannot reach ${label}. Double-check the value.` } + return { ok: true } } catch (error) { notifyError(error, `Could not save ${label}`) return { ok: false, message: errMessage(error) } } } - -export function isOnboardingBusy(flow: OnboardingFlow) { - return BUSY.has(flow.status) -}