From 432635735b73021d8f8d14e4564947b5d614e888 Mon Sep 17 00:00:00 2001 From: zhangmo8 Date: Fri, 22 May 2026 16:16:11 +0800 Subject: [PATCH] fix(onboarding): refresh state on guide handoff - 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. --- .../onboarding-provider-mcp-handoff/plan.md | 42 +++++++++++++++++++ .../onboarding-provider-mcp-handoff/spec.md | 39 +++++++++++++++++ .../onboarding-provider-mcp-handoff/tasks.md | 7 ++++ .../settings/lib/guidedOnboardingSettings.ts | 20 ++++++++- .../onboarding/OnBoardingSpotlight.vue | 1 + .../composables/useGuidedOnboardingStep.ts | 19 +++++++-- 6 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 docs/issues/onboarding-provider-mcp-handoff/plan.md create mode 100644 docs/issues/onboarding-provider-mcp-handoff/spec.md create mode 100644 docs/issues/onboarding-provider-mcp-handoff/tasks.md diff --git a/docs/issues/onboarding-provider-mcp-handoff/plan.md b/docs/issues/onboarding-provider-mcp-handoff/plan.md new file mode 100644 index 000000000..d13105124 --- /dev/null +++ b/docs/issues/onboarding-provider-mcp-handoff/plan.md @@ -0,0 +1,42 @@ +# Implementation Plan + +## Cause + +There are two independent fragility points on the renderer side. Both surface +in packaged builds because timing in production is less forgiving than in dev. + +1. `useGuidedOnboardingStep.setStepStatus` returns the previous (possibly + `null`) value of its internal `onboardingState` ref when the backend IPC + throws. `continueGuidedOnboardingFromSettings` then resolves a `null` step + id, hits the fallback branch, and calls `windowPresenter.focusMainWindow()` + instead of `router.push({ name: 'settings-mcp' })`. The backend state is + already correct — the renderer just doesn't see it on the relevant tick. + +2. `GuidedOnboardingOverlay` always renders the dim `` from + `OnBoardingSpotlight`, even when `useOnBoarding` has not produced a + spotlight rect yet (target element not yet sized). With no cutout the path + covers the entire viewport with `pointer-events: auto`, producing the + "full-window dim, no popover, can't click" symptom while the layout + stabilizes. + +## Change + +- **Renderer composable resilience.** In `useGuidedOnboardingStep`, when an + onboarding IPC call fails, fall back to fetching fresh state via + `onboardingClient.getState()` before returning to the caller. Apply to + `setStepStatus`, `activateStep`, and `forceComplete` paths. +- **Navigation helper resilience.** `continueGuidedOnboardingFromSettings` + refreshes its `state` from `onboardingClient.getState()` when the caller + passes a `null`/stale value, so that a transient renderer hiccup cannot + force the helper into the "focus main window" branch. +- **Overlay defensive rendering.** `OnBoardingSpotlight` only renders its + dim `` when a cutout is present. With no cutout the parent overlay + still allows the panel to render at its fallback coordinates, but the + blocking dim no longer covers the window. + +## Validation + +- `pnpm run format` +- `pnpm run i18n` +- `pnpm run lint` +- `pnpm run typecheck` diff --git a/docs/issues/onboarding-provider-mcp-handoff/spec.md b/docs/issues/onboarding-provider-mcp-handoff/spec.md new file mode 100644 index 000000000..4142dbb94 --- /dev/null +++ b/docs/issues/onboarding-provider-mcp-handoff/spec.md @@ -0,0 +1,39 @@ +# Onboarding Provider → MCP Handoff + +## Problem + +In packaged builds, after the user finishes the `provider-model` guided step the +settings window does not automatically advance to the `settings-mcp` route, so +the MCP coachmark never appears in the expected sequence. Reopening the +settings window and clicking the MCP tab afterwards does surface the MCP +overlay, but in this fallback path the overlay renders as a full-window dim +without a visible popover, blocking subsequent interaction. + +Locally (dev) the same flow is continuous. The divergence is timing- and +device-sensitive — the user could not reproduce on their own machine but +observed it on another machine. + +## User Story + +As a first-time user completing the provider step in the packaged app, I want +the guide to continue into the MCP step without me having to navigate manually, +and when the MCP overlay does appear I want to be able to read it and click +through it. + +## Acceptance Criteria + +- After `provider-model` completes, the settings window advances to + `settings-mcp` even when the per-step state returned from the backend is + stale or missing, as long as the backend has actually progressed. +- When the guided onboarding overlay is asked to render but the spotlight + target element is not yet sized, the dim layer does not cover the window — + no interaction is blocked while the target is still being laid out. +- Existing behavior is preserved: when the target element is sized the dim and + cutout render as before and the user-facing copy/keys do not change. + +## Non-goals + +- No change to the backend step ordering or migration logic in + `onboardingRouteSupport.ts`. +- No redesign of the onboarding panel layout or copy. +- No change to the welcome page / main-window flow. diff --git a/docs/issues/onboarding-provider-mcp-handoff/tasks.md b/docs/issues/onboarding-provider-mcp-handoff/tasks.md new file mode 100644 index 000000000..5a511048c --- /dev/null +++ b/docs/issues/onboarding-provider-mcp-handoff/tasks.md @@ -0,0 +1,7 @@ +# Tasks + +- [x] Add SDD artifacts. +- [x] Harden `useGuidedOnboardingStep` IPC failure paths with a `getState` fallback. +- [x] Refresh `state` inside `continueGuidedOnboardingFromSettings` when caller passes a null/stale value. +- [x] Stop rendering the dim path in `OnBoardingSpotlight` when there is no cutout. +- [x] Run `pnpm run format`, `pnpm run i18n`, `pnpm run lint`, and `pnpm run typecheck`. diff --git a/src/renderer/settings/lib/guidedOnboardingSettings.ts b/src/renderer/settings/lib/guidedOnboardingSettings.ts index 312874946..a2d13129f 100644 --- a/src/renderer/settings/lib/guidedOnboardingSettings.ts +++ b/src/renderer/settings/lib/guidedOnboardingSettings.ts @@ -1,5 +1,6 @@ import type { GuidedOnboardingState, GuidedOnboardingStepId } from '@shared/contracts/routes' import { resolveGuidedOnboardingStepTarget } from '@shared/guidedOnboarding' +import { createOnboardingClient } from '@api/OnboardingClient' import { persistGuidedOnboardingResumeIntent } from '@/lib/onboardingResume' import type { Router } from 'vue-router' @@ -28,8 +29,23 @@ export async function continueGuidedOnboardingFromSettings(options: { focusMainWindow?: () => Promise | boolean } }) { - const { state, router, currentRoute, windowPresenter } = options - const stepId = resolveGuidedOnboardingResumeStepId(state) + const { router, currentRoute, windowPresenter } = options + let { state } = options + let stepId = resolveGuidedOnboardingResumeStepId(state) + + // If the caller passed a stale/null state, the local handler likely failed + // its IPC call (or never received a response). Re-read from the backend so a + // transient renderer hiccup cannot force the helper into the fallback branch + // that focuses the main window instead of advancing within settings. + if (!stepId) { + try { + state = await createOnboardingClient().getState() + stepId = resolveGuidedOnboardingResumeStepId(state) + } catch (error) { + console.warn('[GuidedOnboarding] Failed to refresh state from backend:', error) + } + } + const target = resolveGuidedOnboardingStepTarget(stepId) if (target?.surface === 'settings' && target.routeName) { diff --git a/src/renderer/src/components/onboarding/OnBoardingSpotlight.vue b/src/renderer/src/components/onboarding/OnBoardingSpotlight.vue index 274b8d9ca..c05c59276 100644 --- a/src/renderer/src/components/onboarding/OnBoardingSpotlight.vue +++ b/src/renderer/src/components/onboarding/OnBoardingSpotlight.vue @@ -7,6 +7,7 @@ focusable="false" > => { + try { + const refreshed = await onboardingClient.getState() + onboardingState.value = refreshed + return refreshed + } catch (error) { + console.warn(`[GuidedOnboarding] Failed to recover state after ${context}:`, error) + return onboardingState.value + } + } + const dismissGuide = () => { dismissed.value = true } @@ -91,7 +104,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) { return finalizeIfNeeded(onboardingState.value) } catch (error) { console.warn(`[GuidedOnboarding] Failed to set step ${stepId} status to ${status}:`, error) - return onboardingState.value + return recoverStateFromBackend(`setStepStatus(${stepId}, ${status})`) } } @@ -103,7 +116,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) { return onboardingState.value } catch (error) { console.warn(`[GuidedOnboarding] Failed to activate step ${targetStepId}:`, error) - return onboardingState.value + return recoverStateFromBackend(`activateStep(${targetStepId})`) } } @@ -131,7 +144,7 @@ export function useGuidedOnboardingStep(stepId: GuidedOnboardingStepId) { return onboardingState.value } catch (error) { console.warn(`[GuidedOnboarding] Failed to force complete onboarding from ${stepId}:`, error) - return onboardingState.value + return recoverStateFromBackend(`forceComplete(${stepId})`) } }