Guided first launch: review fixes from #107985 and the free-tier chip badge (NS-848, NS-855) (#108211)

* fix(desktop): centralize guide handoff receipt reads Resolve the guide receipt key and value together in setup-profile. Use the helper at all four read sites so connection scoping follows one implementation. * fix(desktop): recover from unreadable handoff receipts Memoize receipt reads and show Retry only for the error phase. Quarantine corrupt data before retrying, and resolve the guide identity when the failed request did not retain it so a fresh build can start. Cover preservation of corrupt data and removal from the active receipt key with an invariant test. * fix(desktop): validate persisted onboarding phases from one list Derive OnboardingPhase and persisted-value validation from the same phase list so future phases survive relaunch. Verify every persisted phase reloads and an unknown value falls back to idle. * fix(desktop): share window centering arithmetic Extract centeredBounds and use it for onboarding boot and window growth. Keep the existing work-area clamps and coordinate rounding unchanged. * fix(desktop): compute progress steps inline Remove the ineffective ProgressCard memo because streaming flushes replace the messages array. Keep the same transcript scan and rendered steps. * fix(desktop): center the free-tier status chip detail Wrap the model label and sign-in badge in an inline flex span with a shared gap. This centers the badge beside the model text without changing other status-bar details. * fix(desktop): derive the guide receipt key in one place The Retry path spelled the key derivation out again because the read helper throws on a corrupt receipt before it can return the key. A separate guideHandoffReceiptKey serves both the reader and the quarantine, so the derivation has one home again. * fix(desktop): keep the free-tier badge at its intended leading Badge declares leading-none, but the class merger drops it behind the size variant's font-size class, so the badge inherits a 1.5 leading and renders 16px tall next to an 11px label. That height, not the inline alignment, is what read as a detached badge. Restating leading-none on the chip's badge brings it to 11.6px, inside the label's cap height. The Badge component itself is left alone; every other badge in the app has the same dropped leading and that is a separate decision.
2026-09-11 18:16:48 +05:30
parent dee30d123d
commit 0591da2ba6
10 changed files with 165 additions and 60 deletions
@@ -1,6 +1,6 @@
 import { type BrowserWindow, ipcMain, screen } from 'electron'

-import { type GrowRequest, growWindowBounds } from './window-growth'
+import { centeredBounds, type GrowRequest, growWindowBounds } from './window-growth'

 interface ChatOnboardingWindowOptions {
  enabled: boolean
@@ -40,14 +40,6 @@ export function registerChatOnboardingWindow({ enabled, mainWindow }: ChatOnboar
    const width = Math.min(600, area.width)
    const height = Math.min(640, area.height)

-    win.setBounds(
-      {
-        height,
-        width,
-        x: Math.round(area.x + (area.width - width) / 2),
-        y: Math.round(area.y + (area.height - height) / 2)
-      },
-      true
-    )
+    win.setBounds(centeredBounds(area, width, height), true)
  })
 }
@@ -6,6 +6,8 @@
 * asserted rather than eyeballed on a first run.
 */

+import type { Rectangle } from 'electron'
+
 export interface GrowRequest {
  bottom?: number
  left?: number
@@ -65,6 +67,10 @@ export function growWindowBounds(
    Math.round(workArea.height * MAX_WORK_AREA)
  )

+  return centeredBounds(workArea, width, height)
+}
+
+export function centeredBounds(workArea: Rectangle, width: number, height: number): Rectangle {
  return {
    height,
    width,
@@ -0,0 +1,18 @@
+import { expect, it } from 'vitest'
+
+import { readKey, writeKey } from '@/lib/storage'
+
+import { handoffReceiptKey, quarantineHandoffReceipt, readHandoffReceipt } from './handoff-receipt'
+
+it('preserves an unreadable receipt and removes it from the retry lookup', () => {
+  const key = handoffReceiptKey('receipt-recovery', 'guide')
+  const corrupt = '{unreadable receipt'
+  writeKey(key, corrupt)
+
+  expect(() => readHandoffReceipt(key)).toThrow()
+
+  quarantineHandoffReceipt(key)
+
+  expect(readKey(`${key}.unreadable`)).toBe(corrupt)
+  expect(readHandoffReceipt(key)).toBeNull()
+})
@@ -1,4 +1,4 @@
-import { readKey, writeJson } from '@/lib/storage'
+import { readKey, writeJson, writeKey } from '@/lib/storage'

 import type { HandoffReceipt } from './handoff-leg'

@@ -59,6 +59,11 @@ export function readHandoffReceipt(key: string): HandoffReceipt | null {
  return value
 }

+export function quarantineHandoffReceipt(key: string): void {
+  writeKey(`${key}.unreadable`, readKey(key))
+  writeKey(key, null)
+}
+
 export function saveHandoffReceipt(key: string, receipt: HandoffReceipt): void {
  unsavedReceipts.set(key, receipt)
  writeJson(key, receipt)
@@ -14,6 +14,7 @@ import {
  buildHandoffCompleteNote,
  firstTaskTitle,
  guideSourceConnectionId,
+  readGuideHandoffReceipt,
  retrySetupHandoff,
  SETUP_PROFILE
 } from '@/components/onboarding-chat/setup-profile'
@@ -40,7 +41,7 @@ import {
 import { patchSessionTile } from '@/store/session-states'

 import { BUILD_PROFILE, type HandoffDeps, type HandoffReceipt, paintHandoffBrief, startHandoff } from './handoff-leg'
-import { handoffReceiptKey, readHandoffReceipt, saveHandoffReceipt } from './handoff-receipt'
+import { saveHandoffReceipt } from './handoff-receipt'
 import type { AmbientGatewayRequest } from './session-rpc-dispatcher'

 export interface OnboardingHandoffOptions extends Pick<
@@ -82,7 +83,7 @@ export function useOnboardingHandoff({
    const connectionId = guideSourceConnectionId(selectedStoredId)

    try {
-      const saved = readHandoffReceipt(handoffReceiptKey(connectionId, selectedStoredId))
+      const { receipt: saved } = readGuideHandoffReceipt(selectedStoredId)

      if (!saved) {
        return
@@ -146,8 +147,8 @@ export function useOnboardingHandoff({

        $setupSession.set(setupSession)
        // Resume only has the guide's stored id, so its source must also key the saved receipt.
-        const receiptKey = handoffReceiptKey(guideSourceConnectionId(setupSession.storedId), setupSession.storedId)
-        receipt = readHandoffReceipt(receiptKey)
+        const { key: receiptKey, receipt: saved } = readGuideHandoffReceipt(setupSession.storedId)
+        receipt = saved
        const owner: HandoffReceipt['owner'] = receipt?.owner ?? { connectionId, profile: BUILD_PROFILE }
        // Save facts before session.create freezes the new agent's memory.
        // A retry never re-creates the session or copies the guide's memory.
@@ -463,14 +463,17 @@ export function useStatusbarItems({
        // The model id is the quiet part; the sign-in is the action, so it is
        // solid and set off by a gap instead of touching the label.
        detail: (
-          <>
+          <span className="inline-flex items-center gap-2">
            <span className="font-mono text-[0.625rem] text-muted-foreground/70">
              {freeTier?.model ?? FREE_TIER_MODEL}
            </span>
-            <Badge className="ml-2" size="xs" variant="solid">
+            {/* The class merger drops Badge's own leading-none behind the size's
+                font-size class, so the badge grows to the inherited 1.5 leading and
+                overhangs an 11px label. Restating it here keeps it 11.6px tall. */}
+            <Badge className="leading-none" size="xs" variant="solid">
              {freeTierCopy.signIn}
            </Badge>
-          </>
+          </span>
        ),
        // Shown while a free-tier identity exists and the tier is on: it names the
        // identity that carries the connectors (and inference when nothing else
@@ -11,7 +11,7 @@ import { useEffect, useMemo, useState } from 'react'

 import { requestComposerSubmit } from '@/app/chat/composer/focus'
 import { useSessionView } from '@/app/chat/session-view'
-import { handoffReceiptKey, readHandoffReceipt } from '@/app/contrib/handoff-receipt'
+import { quarantineHandoffReceipt } from '@/app/contrib/handoff-receipt'
 import { resolveSessionOwner } from '@/app/session/hooks/use-session-actions/utils'
 import type { CardProps } from '@/components/onboarding-chat/cards/frame'
 import { Chip } from '@/components/onboarding-chat/chip'
@@ -19,8 +19,9 @@ import {
  $handoffError,
  $setupHandoff,
  firstTaskTitle,
-  guideSourceConnectionId,
+  guideHandoffReceiptKey,
  parseHandoffPlan,
+  readGuideHandoffReceipt,
  requestSetupHandoff,
  retrySetupHandoff,
  SETUP_PROFILE
@@ -111,16 +112,19 @@ export function HandoffCard({ attrs, locked }: CardProps) {
  const brief = (attrs.brief ?? '').trim().slice(0, 240)
  const plan = parseHandoffPlan(attrs.plan)
  const state = useStore($setupHandoff)
-  let error = useStore($handoffError)
-  let completed = false
+  const receipt = useMemo(() => {
+    try {
+      return {
+        completed: !!storedId && readGuideHandoffReceipt(storedId).receipt?.status === 'accepted',
+        error: null
+      }
+    } catch (error) {
+      return { completed: false, error: String(error) }
+    }
+  }, [storedId, state?.phase])

-  try {
-    completed =
-      !!storedId &&
-      readHandoffReceipt(handoffReceiptKey(guideSourceConnectionId(storedId), storedId))?.status === 'accepted'
-  } catch (receiptError) {
-    error = String(receiptError)
-  }
+  const error = useStore($handoffError) ?? receipt.error
+  const completed = receipt.completed

  useEffect(() => {
    if (!task || !brief || locked || !storedId || !runtimeId || $setupHandoff.get() || completed) {
@@ -161,6 +165,36 @@ export function HandoffCard({ attrs, locked }: CardProps) {
  const failed = state?.phase === 'error' || error !== null
  const title = state?.sessionTitle ?? firstTaskTitle(task)

+  const retry = async () => {
+    if (state?.phase !== 'error') {
+      return
+    }
+
+    try {
+      if (receipt.error && storedId) {
+        quarantineHandoffReceipt(guideHandoffReceiptKey(storedId))
+      }
+
+      if (!state.guide && storedId && runtimeId) {
+        const owner = await resolveSessionOwner(storedId)
+        assertSessionOwnerResolved(owner, { method: 'onboarding.handoff', sessionId: storedId })
+        $setupHandoff.set({
+          ...state,
+          guide: {
+            storedId,
+            runtimeId,
+            connectionId: isSessionOwnerRoute(owner) ? owner.connectionId : null,
+            profile: isSessionOwnerRoute(owner) ? owner.profile : owner || SETUP_PROFILE
+          }
+        })
+      }
+
+      retrySetupHandoff()
+    } catch (error) {
+      $handoffError.set(String(error))
+    }
+  }
+
  return (
    <div className="my-3 flex max-w-md items-center gap-2 text-sm" data-onboarding-card>
      <StatusDot live={!settled && !failed} />
@@ -171,8 +205,8 @@ export function HandoffCard({ attrs, locked }: CardProps) {
            ? `${title} was started — find it in your sessions`
            : `Opening ${title}\u2026`}
      </span>
-      {failed && (
-        <Button disabled={locked} onClick={retrySetupHandoff} size="sm" variant="text">
+      {state?.phase === 'error' && (
+        <Button disabled={locked} onClick={() => void retry()} size="sm" variant="text">
          Retry first build
        </Button>
      )}
@@ -187,29 +221,27 @@ export function ProgressCard({ attrs, locked }: CardProps) {
  const messageId = useAuiState(state => state.message.id)
  const title = (attrs.title ?? '').trim() || 'Working on it'

-  const steps = useMemo(() => {
-    const index = messages.findIndex(message => message.id === messageId)
-    const previous = index < 0 ? [] : messages.slice(0, index)
+  const index = messages.findIndex(message => message.id === messageId)
+  const previous = index < 0 ? [] : messages.slice(0, index)

-    return previous.flatMap(message => {
-      const directives = message.parts.flatMap(part =>
-        part.type === 'text' ? (segmentTranscriptDirectives(part.text) ?? []) : []
+  const steps = previous.flatMap(message => {
+    const directives = message.parts.flatMap(part =>
+      part.type === 'text' ? (segmentTranscriptDirectives(part.text) ?? []) : []
+    )
+
+    const progress = directives
+      .filter(
+        segment =>
+          segment.kind === 'directive' &&
+          segment.directive.name === 'onboarding' &&
+          segment.directive.attrs.step === 'progress'
      )
+      .at(-1)

-      const progress = directives
-        .filter(
-          segment =>
-            segment.kind === 'directive' &&
-            segment.directive.name === 'onboarding' &&
-            segment.directive.attrs.step === 'progress'
-        )
-        .at(-1)
-
-      return progress?.kind === 'directive'
-        ? [{ id: message.id, title: progress.directive.attrs.title?.trim() || 'Working on it' }]
-        : []
-    })
-  }, [messages, messageId])
+    return progress?.kind === 'directive'
+      ? [{ id: message.id, title: progress.directive.attrs.title?.trim() || 'Working on it' }]
+      : []
+  })

  return (
    <div className="my-3 grid max-w-md gap-1.5" data-onboarding-card>
@@ -24,6 +24,7 @@

 import { atom } from 'nanostores'

+import type { HandoffReceipt } from '@/app/contrib/handoff-leg'
 import { handoffReceiptKey, readHandoffReceipt } from '@/app/contrib/handoff-receipt'
 import type { GatewayRequest } from '@/app/session/hooks/use-prompt-actions/utils'
 import { activeGatewayConnectionId } from '@/store/gateway'
@@ -111,13 +112,21 @@ export function guideSourceConnectionId(guideStoredId: null | string | undefined
  return (guideStoredId && getSessionOwnerHint(guideStoredId)?.connectionId) || activeGatewayConnectionId() || null
 }

+export function guideHandoffReceiptKey(guideStoredId: string): string {
+  return handoffReceiptKey(guideSourceConnectionId(guideStoredId), guideStoredId)
+}
+
+export function readGuideHandoffReceipt(guideStoredId: string): { key: string; receipt: HandoffReceipt | null } {
+  const key = guideHandoffReceiptKey(guideStoredId)
+
+  return { key, receipt: readHandoffReceipt(key) }
+}
+
 /** The request atom suppresses remounts; only an accepted receipt suppresses relaunches. */
 export function requestSetupHandoff(task: string, brief: string, plan: HandoffPlan, guide: SetupSession): boolean {
  if (
    $setupHandoff.get() !== null ||
-    (guide.storedId &&
-      readHandoffReceipt(handoffReceiptKey(guideSourceConnectionId(guide.storedId), guide.storedId))?.status ===
-        'accepted')
+    (guide.storedId && readGuideHandoffReceipt(guide.storedId).receipt?.status === 'accepted')
  ) {
    return false
  }
@@ -0,0 +1,37 @@
+import { expect, it, vi } from 'vitest'
+
+import type * as storageModule from '@/lib/storage'
+
+const storage = vi.hoisted(() => new Map<string, string>())
+
+vi.mock('@/lib/onboarding-enabled', () => ({ isOnboardingEnabled: () => true }))
+vi.mock('@/lib/storage', async importOriginal => ({
+  ...(await importOriginal<typeof storageModule>()),
+  readKey: (key: string) => storage.get(key) ?? null,
+  writeKey: (key: string, value: string | null) => {
+    if (value === null) {
+      storage.delete(key)
+    } else {
+      storage.set(key, value)
+    }
+  }
+}))
+
+it('restores every persisted onboarding phase and rejects unknown phases', async () => {
+  const { ONBOARDING_PHASES } = await import('./onboarding-gate')
+
+  for (const phase of ONBOARDING_PHASES.filter(phase => phase !== 'idle')) {
+    storage.clear()
+    storage.set('hermes-onboarding-phase-v1', phase)
+    vi.resetModules()
+    const { $onboardingGate } = await import('./onboarding-gate')
+
+    expect($onboardingGate.get().phase).toBe(phase)
+  }
+
+  storage.set('hermes-onboarding-phase-v1', 'unknown-phase')
+  vi.resetModules()
+  const { $onboardingGate } = await import('./onboarding-gate')
+
+  expect($onboardingGate.get().phase).toBe('idle')
+})
@@ -8,7 +8,13 @@ import { DEFAULT_ANSWERS, setOnboardingAnswers } from './onboarding-answers'

 const PHASE_KEY = 'hermes-onboarding-phase-v1'

-export type OnboardingPhase = 'idle' | 'cinematic' | 'guided' | 'skipped' | 'handoff' | 'done'
+export const ONBOARDING_PHASES = ['idle', 'cinematic', 'guided', 'skipped', 'handoff', 'done'] as const
+
+export type OnboardingPhase = (typeof ONBOARDING_PHASES)[number]
+
+function isOnboardingPhase(value: string | null): value is OnboardingPhase {
+  return ONBOARDING_PHASES.some(phase => phase === value)
+}

 export interface OnboardingGateState {
  phase: OnboardingPhase
@@ -20,11 +26,7 @@ type GuideKickoff = { status: 'idle' } | { status: 'starting'; promise: Promise<
 function loadGate(): OnboardingGateState {
  const saved = readKey(PHASE_KEY)

-  const phase =
-    isOnboardingEnabled() &&
-    (saved === 'cinematic' || saved === 'guided' || saved === 'skipped' || saved === 'handoff' || saved === 'done')
-      ? saved
-      : 'idle'
+  const phase = isOnboardingEnabled() && isOnboardingPhase(saved) ? saved : 'idle'

  return { phase, guideQueued: phase === 'cinematic' && hasSeenIntroReveal() }
 }