Skip to content

Commit 6692c32

Browse files
authored
fix(onboarding): refresh state on guide handoff (#1660)
- useGuidedOnboardingStep falls back to getState() when an IPC call fails, so the next handoff sees the real backend state instead of a stale/null cached one. - continueGuidedOnboardingFromSettings re-reads the onboarding state when no resume step id can be resolved, preventing the helper from focusing the main window while settings should be routing to settings-mcp. - OnBoardingSpotlight no longer draws its dim path when there is no cutout, so a target that has not been measured yet does not produce a full-window block. Co-authored-by: zhangmo8 <zhangmo8@users.noreply.github.com>
1 parent 83c7b56 commit 6692c32

6 files changed

Lines changed: 123 additions & 5 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
# Implementation Plan
2+
3+
## Cause
4+
5+
There are two independent fragility points on the renderer side. Both surface
6+
in packaged builds because timing in production is less forgiving than in dev.
7+
8+
1. `useGuidedOnboardingStep.setStepStatus` returns the previous (possibly
9+
`null`) value of its internal `onboardingState` ref when the backend IPC
10+
throws. `continueGuidedOnboardingFromSettings` then resolves a `null` step
11+
id, hits the fallback branch, and calls `windowPresenter.focusMainWindow()`
12+
instead of `router.push({ name: 'settings-mcp' })`. The backend state is
13+
already correct — the renderer just doesn't see it on the relevant tick.
14+
15+
2. `GuidedOnboardingOverlay` always renders the dim `<path>` from
16+
`OnBoardingSpotlight`, even when `useOnBoarding` has not produced a
17+
spotlight rect yet (target element not yet sized). With no cutout the path
18+
covers the entire viewport with `pointer-events: auto`, producing the
19+
"full-window dim, no popover, can't click" symptom while the layout
20+
stabilizes.
21+
22+
## Change
23+
24+
- **Renderer composable resilience.** In `useGuidedOnboardingStep`, when an
25+
onboarding IPC call fails, fall back to fetching fresh state via
26+
`onboardingClient.getState()` before returning to the caller. Apply to
27+
`setStepStatus`, `activateStep`, and `forceComplete` paths.
28+
- **Navigation helper resilience.** `continueGuidedOnboardingFromSettings`
29+
refreshes its `state` from `onboardingClient.getState()` when the caller
30+
passes a `null`/stale value, so that a transient renderer hiccup cannot
31+
force the helper into the "focus main window" branch.
32+
- **Overlay defensive rendering.** `OnBoardingSpotlight` only renders its
33+
dim `<path>` when a cutout is present. With no cutout the parent overlay
34+
still allows the panel to render at its fallback coordinates, but the
35+
blocking dim no longer covers the window.
36+
37+
## Validation
38+
39+
- `pnpm run format`
40+
- `pnpm run i18n`
41+
- `pnpm run lint`
42+
- `pnpm run typecheck`
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Onboarding Provider → MCP Handoff
2+
3+
## Problem
4+
5+
In packaged builds, after the user finishes the `provider-model` guided step the
6+
settings window does not automatically advance to the `settings-mcp` route, so
7+
the MCP coachmark never appears in the expected sequence. Reopening the
8+
settings window and clicking the MCP tab afterwards does surface the MCP
9+
overlay, but in this fallback path the overlay renders as a full-window dim
10+
without a visible popover, blocking subsequent interaction.
11+
12+
Locally (dev) the same flow is continuous. The divergence is timing- and
13+
device-sensitive — the user could not reproduce on their own machine but
14+
observed it on another machine.
15+
16+
## User Story
17+
18+
As a first-time user completing the provider step in the packaged app, I want
19+
the guide to continue into the MCP step without me having to navigate manually,
20+
and when the MCP overlay does appear I want to be able to read it and click
21+
through it.
22+
23+
## Acceptance Criteria
24+
25+
- After `provider-model` completes, the settings window advances to
26+
`settings-mcp` even when the per-step state returned from the backend is
27+
stale or missing, as long as the backend has actually progressed.
28+
- When the guided onboarding overlay is asked to render but the spotlight
29+
target element is not yet sized, the dim layer does not cover the window —
30+
no interaction is blocked while the target is still being laid out.
31+
- Existing behavior is preserved: when the target element is sized the dim and
32+
cutout render as before and the user-facing copy/keys do not change.
33+
34+
## Non-goals
35+
36+
- No change to the backend step ordering or migration logic in
37+
`onboardingRouteSupport.ts`.
38+
- No redesign of the onboarding panel layout or copy.
39+
- No change to the welcome page / main-window flow.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Tasks
2+
3+
- [x] Add SDD artifacts.
4+
- [x] Harden `useGuidedOnboardingStep` IPC failure paths with a `getState` fallback.
5+
- [x] Refresh `state` inside `continueGuidedOnboardingFromSettings` when caller passes a null/stale value.
6+
- [x] Stop rendering the dim path in `OnBoardingSpotlight` when there is no cutout.
7+
- [x] Run `pnpm run format`, `pnpm run i18n`, `pnpm run lint`, and `pnpm run typecheck`.

src/renderer/settings/lib/guidedOnboardingSettings.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { GuidedOnboardingState, GuidedOnboardingStepId } from '@shared/contracts/routes'
22
import { resolveGuidedOnboardingStepTarget } from '@shared/guidedOnboarding'
3+
import { createOnboardingClient } from '@api/OnboardingClient'
34
import { persistGuidedOnboardingResumeIntent } from '@/lib/onboardingResume'
45
import type { Router } from 'vue-router'
56

@@ -28,8 +29,23 @@ export async function continueGuidedOnboardingFromSettings(options: {
2829
focusMainWindow?: () => Promise<boolean> | boolean
2930
}
3031
}) {
31-
const { state, router, currentRoute, windowPresenter } = options
32-
const stepId = resolveGuidedOnboardingResumeStepId(state)
32+
const { router, currentRoute, windowPresenter } = options
33+
let { state } = options
34+
let stepId = resolveGuidedOnboardingResumeStepId(state)
35+
36+
// If the caller passed a stale/null state, the local handler likely failed
37+
// its IPC call (or never received a response). Re-read from the backend so a
38+
// transient renderer hiccup cannot force the helper into the fallback branch
39+
// that focuses the main window instead of advancing within settings.
40+
if (!stepId) {
41+
try {
42+
state = await createOnboardingClient().getState()
43+
stepId = resolveGuidedOnboardingResumeStepId(state)
44+
} catch (error) {
45+
console.warn('[GuidedOnboarding] Failed to refresh state from backend:', error)
46+
}
47+
}
48+
3349
const target = resolveGuidedOnboardingStepTarget(stepId)
3450

3551
if (target?.surface === 'settings' && target.routeName) {

src/renderer/src/components/onboarding/OnBoardingSpotlight.vue

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
focusable="false"
88
>
99
<path
10+
v-if="cutoutPathD"
1011
data-testid="onboarding-spotlight-path"
1112
:d="pathD"
1213
:fill="fillColor"

src/renderer/src/composables/useGuidedOnboardingStep.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,19 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) {
7070
}
7171
}
7272

73+
const recoverStateFromBackend = async (
74+
context: string
75+
): Promise<GuidedOnboardingState | null> => {
76+
try {
77+
const refreshed = await onboardingClient.getState()
78+
onboardingState.value = refreshed
79+
return refreshed
80+
} catch (error) {
81+
console.warn(`[GuidedOnboarding] Failed to recover state after ${context}:`, error)
82+
return onboardingState.value
83+
}
84+
}
85+
7386
const dismissGuide = () => {
7487
dismissed.value = true
7588
}
@@ -91,7 +104,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) {
91104
return finalizeIfNeeded(onboardingState.value)
92105
} catch (error) {
93106
console.warn(`[GuidedOnboarding] Failed to set step ${stepId} status to ${status}:`, error)
94-
return onboardingState.value
107+
return recoverStateFromBackend(`setStepStatus(${stepId}, ${status})`)
95108
}
96109
}
97110

@@ -103,7 +116,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) {
103116
return onboardingState.value
104117
} catch (error) {
105118
console.warn(`[GuidedOnboarding] Failed to activate step ${targetStepId}:`, error)
106-
return onboardingState.value
119+
return recoverStateFromBackend(`activateStep(${targetStepId})`)
107120
}
108121
}
109122

@@ -131,7 +144,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) {
131144
return onboardingState.value
132145
} catch (error) {
133146
console.warn(`[GuidedOnboarding] Failed to force complete onboarding from ${stepId}:`, error)
134-
return onboardingState.value
147+
return recoverStateFromBackend(`forceComplete(${stepId})`)
135148
}
136149
}
137150

0 commit comments

Comments
 (0)