feat(desktop): v2 onboarding setup flow#4080
Merged
Conversation
Adds /setup/* with 5 steps (providers, gh-cli, permissions, project, adopt-worktrees), a hard redirect gate in _authenticated/layout.tsx, and a Restart entry point in Settings → Experimental. Reuses existing chat-service auth, permissions, and workspace-import APIs; adds system.detectGhCli for gh detection.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a default-to-v2 onboarding flow: persisted Zustand onboarding store, five ordered setup steps and routes (providers, gh-cli, permissions, project, adopt-worktrees), system tRPC detectGhCli, macOS permission deep-link updates, onboarding UI primitives, provider connection pages, adopt-worktrees import, and a restart-onboarding experimental setting. ChangesV2 Onboarding System
sequenceDiagram
participant User
participant Renderer as Renderer (Routes/UI)
participant Onboard as Onboarding Store
participant tRPC as electronTrpc
participant Main as Main Process
User->>Renderer: Navigate to workspace
Renderer->>Onboard: are required steps complete?
Onboard-->>Renderer: no
Renderer->>User: redirect to first incomplete /setup route
User->>Renderer: visit /setup/gh-cli
Renderer->>tRPC: system.detectGhCli.query()
tRPC->>Main: probe known gh paths and run "gh --version"
Main-->>tRPC: installed/version/path or failure
tRPC-->>Renderer: gh status
alt installed
Renderer->>Onboard: markComplete("gh-cli")
Renderer->>User: navigate to /setup/permissions
else not installed
Renderer->>User: show install UI (Recheck / Skip)
User->>Renderer: Click Recheck
Renderer->>tRPC: refetch detectGhCli
end
User->>Renderer: click "Open Settings" on permissions row
Renderer->>tRPC: permissions.requestX mutation
tRPC->>Main: openExternal macOS preference deep-link
Main->>User: OS settings shown / permission grant
Renderer->>tRPC: permission status polling detects change
tRPC-->>Renderer: updated status
Renderer->>Onboard: markComplete("permissions")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ding-flow-implementation-plan
Greptile Summary
Confidence Score: 3/5Safe to merge for most paths, but the Restart onboarding dialog actively misleads users about auto-advance behavior. One P1 (misleading dialog copy vs. manualWalkthrough: true implementation) plus two P2s (non-reactive .getState() in layout render, unstable AutoAdvance callback) push the score to 3. ExperimentalSettings.tsx (P1 dialog copy), layout.tsx (non-reactive store read), adopt-worktrees/page.tsx (unstable callback).
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx | Adds Restart onboarding entry point; dialog description promises auto-advance but reset() sets manualWalkthrough: true which disables auto-advance — misleading UX. |
| apps/desktop/src/renderer/routes/_authenticated/layout.tsx | Adds onboarding redirect gate using non-reactive useOnboardingStore.getState() snapshot instead of the hook — works in practice due to navigation-driven re-renders but is fragile. |
| apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | Worktree adoption step; AutoAdvance receives an unstable finishFlow callback which causes its useEffect to re-fire on every parent render — should use useCallback or a ref. |
| apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | Project selection step; handleSelectFolder and handleAddAnother are byte-for-byte identical — one is dead code. |
| apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts | Well-structured Zustand store with persistence and devtools; reset() correctly sets manualWalkthrough: true but this is inconsistent with the dialog copy in ExperimentalSettings. |
| apps/desktop/src/lib/trpc/routers/system.ts | New detectGhCli tRPC procedure — probes known paths sequentially then falls back to PATH; execFile with 3s timeout is safe. |
| apps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsx | Provider selection hub with tab UI for Claude Code and Codex; auto-advance logic, reconfigure panels, and per-method routing look correct. |
| apps/desktop/src/renderer/routes/_authenticated/setup/gh-cli/page.tsx | GitHub CLI detection step; auto-advance when already installed and not in manual walkthrough; Recheck button triggers refetch — logic is sound. |
| apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx | macOS permissions step; polls every 2s, gates Continue on FDA + Accessibility, correct use of mutation for each permission type. |
| apps/desktop/src/lib/trpc/routers/permissions.ts | Fixes two macOS System Settings deep link URLs for Apple Events and Local Network — straightforward URL corrections. |
| apps/desktop/src/renderer/routes/_authenticated/setup/layout.tsx | Setup flow shell layout with drag region, progress bar, and ChatServiceProvider wrapper — looks correct. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[AuthenticatedLayout renders] --> B{isV2CloudEnabled?}
B -- No --> Z[Render app normally]
B -- Yes --> C{requiredComplete?\nproviders + project done}
C -- Yes --> Z
C -- No --> D[Navigate to first incomplete step]
D --> E[Step: Providers]
E --> F[Step: GitHub CLI]
F --> G[Step: Permissions]
G --> H[Step: Project]
H --> I[Step: Adopt Worktrees]
I --> J{isV2CloudEnabled?}
J -- Yes --> K[v2-workspace editor]
J -- No --> L[v1 workspace editor]
S([Settings: Restart onboarding\nreset + manualWalkthrough=true]) --> E
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx:173-179
**Dialog copy contradicts `manualWalkthrough: true` behavior**
The description promises "Steps you've already satisfied — such as sign-in, providers, and projects — will auto-advance," but `reset()` sets `manualWalkthrough: true`, which explicitly disables auto-advance in every step's `shouldAutoAdvance` guard. After clicking "Restart," users will have to manually click through every step even when providers are already connected and projects already exist — the opposite of what the dialog says.
### Issue 2 of 4
apps/desktop/src/renderer/routes/_authenticated/layout.tsx:202-209
**Non-reactive store snapshot in component render**
`useOnboardingStore.getState()` is a synchronous snapshot; calling it inside the component render body does not subscribe to store changes, so `AuthenticatedLayout` will not re-render when onboarding state updates in the background. In the happy path this is fine (navigation always triggers a re-render), but any future background update to the store while the layout stays mounted would leave the gate reading stale state. Use the reactive hook instead:
```ts
const requiredComplete = useOnboardingStore(selectRequiredStepsComplete);
const firstIncompleteStep = useOnboardingStore(selectFirstIncompleteStep);
```
### Issue 3 of 4
apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx:113-128
**`handleSelectFolder` and `handleAddAnother` are identical**
Both functions do exactly the same thing: call `openNew()`, mark the project complete, clear `manualWalkthrough`, and open the new project in the workspace. One of them is dead code and can be removed; the other can be shared between the empty-state and the "Select new repo" button in the has-projects branch.
### Issue 4 of 4
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx:102-110
**`AutoAdvance` fires on every render due to unstable `onAdvance` prop**
`onAdvance` is `finishFlow`, a new function reference on every render of `OnboardingAdoptWorktreesPage`. The `useEffect` in `AutoAdvance` lists it as a dependency, so the effect (and therefore `finishFlow` → navigation) will re-fire each time the parent re-renders. Wrap `finishFlow` in `useCallback`, or apply the same ref pattern already used in `ProjectWorktrees`:
```ts
const finishFlow = useCallback(() => {
markComplete("adopt-worktrees");
setManualWalkthrough(false);
void navigateAfterFlow(true);
}, [markComplete, setManualWalkthrough, navigateAfterFlow]);
```
Reviews (1): Last reviewed commit: "feat(desktop): v2 onboarding setup flow" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+173
to
+179
| <AlertDialog> | ||
| <AlertDialogTrigger asChild> | ||
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| size="sm" | ||
| disabled={!isV2CloudEnabled} |
There was a problem hiding this comment.
manualWalkthrough: true behavior
The description promises "Steps you've already satisfied — such as sign-in, providers, and projects — will auto-advance," but reset() sets manualWalkthrough: true, which explicitly disables auto-advance in every step's shouldAutoAdvance guard. After clicking "Restart," users will have to manually click through every step even when providers are already connected and projects already exist — the opposite of what the dialog says.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx
Line: 173-179
Comment:
**Dialog copy contradicts `manualWalkthrough: true` behavior**
The description promises "Steps you've already satisfied — such as sign-in, providers, and projects — will auto-advance," but `reset()` sets `manualWalkthrough: true`, which explicitly disables auto-advance in every step's `shouldAutoAdvance` guard. After clicking "Restart," users will have to manually click through every step even when providers are already connected and projects already exist — the opposite of what the dialog says.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+202
to
+209
| return <Navigate to="/create-organization" replace />; | ||
| } | ||
|
|
||
| const onboardingState = useOnboardingStore.getState(); | ||
| const requiredComplete = selectRequiredStepsComplete(onboardingState); | ||
| const isOnSetupRoute = location.pathname.startsWith("/setup"); | ||
| if (isV2CloudEnabled && !requiredComplete && !isOnSetupRoute) { | ||
| const target = STEP_ROUTES[selectFirstIncompleteStep(onboardingState)]; |
There was a problem hiding this comment.
useOnboardingStore.getState() is a synchronous snapshot; calling it inside the component render body does not subscribe to store changes, so AuthenticatedLayout will not re-render when onboarding state updates in the background. In the happy path this is fine (navigation always triggers a re-render), but any future background update to the store while the layout stays mounted would leave the gate reading stale state. Use the reactive hook instead:
const requiredComplete = useOnboardingStore(selectRequiredStepsComplete);
const firstIncompleteStep = useOnboardingStore(selectFirstIncompleteStep);Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/layout.tsx
Line: 202-209
Comment:
**Non-reactive store snapshot in component render**
`useOnboardingStore.getState()` is a synchronous snapshot; calling it inside the component render body does not subscribe to store changes, so `AuthenticatedLayout` will not re-render when onboarding state updates in the background. In the happy path this is fine (navigation always triggers a re-render), but any future background update to the store while the layout stays mounted would leave the gate reading stale state. Use the reactive hook instead:
```ts
const requiredComplete = useOnboardingStore(selectRequiredStepsComplete);
const firstIncompleteStep = useOnboardingStore(selectFirstIncompleteStep);
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+113
to
+128
| setManualWalkthrough(false); | ||
| await openProjectInWorkspace(project.id); | ||
| } | ||
| }; | ||
|
|
||
| const handleAddAnother = async () => { | ||
| const created = await openNew(); | ||
| const project = created[0]; | ||
| if (project) { | ||
| markComplete("project"); | ||
| setManualWalkthrough(false); | ||
| await openProjectInWorkspace(project.id); | ||
| } | ||
| }; | ||
|
|
||
| const handleContinueWithCurrent = () => { |
There was a problem hiding this comment.
handleSelectFolder and handleAddAnother are identical
Both functions do exactly the same thing: call openNew(), mark the project complete, clear manualWalkthrough, and open the new project in the workspace. One of them is dead code and can be removed; the other can be shared between the empty-state and the "Select new repo" button in the has-projects branch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
Line: 113-128
Comment:
**`handleSelectFolder` and `handleAddAnother` are identical**
Both functions do exactly the same thing: call `openNew()`, mark the project complete, clear `manualWalkthrough`, and open the new project in the workspace. One of them is dead code and can be removed; the other can be shared between the empty-state and the "Select new repo" button in the has-projects branch.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+102
to
+110
|
|
||
| if (!projects || projects.length === 0) { | ||
| // No projects → nothing to adopt. Skip this step entirely (even in walkthrough) | ||
| // since the page has no actionable content. | ||
| return <AutoAdvance onAdvance={finishFlow} />; | ||
| } | ||
|
|
||
| return ( | ||
| <AdoptWorktreesContent |
There was a problem hiding this comment.
AutoAdvance fires on every render due to unstable onAdvance prop
onAdvance is finishFlow, a new function reference on every render of OnboardingAdoptWorktreesPage. The useEffect in AutoAdvance lists it as a dependency, so the effect (and therefore finishFlow → navigation) will re-fire each time the parent re-renders. Wrap finishFlow in useCallback, or apply the same ref pattern already used in ProjectWorktrees:
const finishFlow = useCallback(() => {
markComplete("adopt-worktrees");
setManualWalkthrough(false);
void navigateAfterFlow(true);
}, [markComplete, setManualWalkthrough, navigateAfterFlow]);Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx
Line: 102-110
Comment:
**`AutoAdvance` fires on every render due to unstable `onAdvance` prop**
`onAdvance` is `finishFlow`, a new function reference on every render of `OnboardingAdoptWorktreesPage`. The `useEffect` in `AutoAdvance` lists it as a dependency, so the effect (and therefore `finishFlow` → navigation) will re-fire each time the parent re-renders. Wrap `finishFlow` in `useCallback`, or apply the same ref pattern already used in `ProjectWorktrees`:
```ts
const finishFlow = useCallback(() => {
markComplete("adopt-worktrees");
setManualWalkthrough(false);
void navigateAfterFlow(true);
}, [markComplete, setManualWalkthrough, navigateAfterFlow]);
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
apps/desktop/src/lib/trpc/routers/permissions.ts (1)
74-84: ⚡ Quick winURL scheme updates look correct; consider unifying the remaining mutations.
The
Privacy_AutomationandPrivacy_LocalNetworkanchors are valid undercom.apple.settings.PrivacySecurity.extension, so both updated deep-links are correct for macOS Ventura+.
requestFullDiskAccess,requestAccessibility, andrequestMicrophone(lines 44–72) still use the legacycom.apple.preference.securityscheme. macOS Ventura and newer support some older prefpane calls, though it's unclear how long those will continue to work. Migrating the remaining three to the new.extensionscheme would make the file consistent and more future-proof.♻️ Proposed consistency fix for remaining mutations
requestFullDiskAccess: publicProcedure.mutation(async () => { await shell.openExternal( - "x-apple.systempreferences:com.apple.preference.security?Privacy_AllFiles", + "x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?Privacy_AllFiles", ); }), requestAccessibility: publicProcedure.mutation(async () => { await shell.openExternal( - "x-apple.systempreferences:com.apple.preference.security?Privacy_Accessibility", + "x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?Privacy_Accessibility", ); }), // requestMicrophone fallback path: - "x-apple.systempreferences:com.apple.preference.security?Privacy_Microphone", + "x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?Privacy_Microphone",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/lib/trpc/routers/permissions.ts` around lines 74 - 84, Update the remaining mutations to use the new PrivacySecurity.extension deep-link format: in requestFullDiskAccess, requestAccessibility, and requestMicrophone replace the legacy "x-apple.systempreferences:com.apple.preference.security?..." URLs with "x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?{Anchor}" (use Privacy_AllFiles for full disk access, Privacy_Accessibility for accessibility, and Privacy_Microphone for microphone) while keeping the shell.openExternal call and async mutation behavior unchanged.apps/desktop/src/renderer/routes/_authenticated/setup/components/StepShell/StepShell.tsx (1)
15-21: ⚡ Quick winUse explicit
ReactNodeimport instead ofReact.ReactNodenamespace reference.
Reactis not imported in this file, yetReact.ReactNodeis used as a type in three interfaces. Other files in this PR (SetupButton.tsx,ProviderOptionCard.tsx) consistently useimport type { ReactNode } from "react". Depending on the project'stsconfig.jsonsettings, the bare namespace reference may fail typecheck in stricter setups.♻️ Proposed fix
import { cn } from "@superset/ui/utils"; import { useEffect } from "react"; +import type { ReactNode } from "react"; import { create } from "zustand";interface StepShellProps { - children: React.ReactNode; + children: ReactNode; /** Route path to navigate to when the chrome's Back button is clicked. */ backTo?: string; maxWidth?: "sm" | "md" | "lg" | "xl"; className?: string; }interface StepHeaderProps { title: string; subtitle?: string; - icon?: React.ReactNode; + icon?: ReactNode; }interface SupersetPillProps { - children: React.ReactNode; + children: ReactNode; }Also applies to: 57-61, 75-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/components/StepShell/StepShell.tsx` around lines 15 - 21, The interfaces (e.g., StepShellProps) use the React.ReactNode namespace but the file doesn't import React; update the file to import the type explicitly (import type { ReactNode } from "react") and replace all occurrences of React.ReactNode with the imported ReactNode in the interfaces and any other type positions (lines referenced around StepShellProps and the two other interfaces at 57-61 and 75-77) to match the project's pattern used in SetupButton.tsx and ProviderOptionCard.tsx.apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx (1)
108-126: ⚡ Quick win
handleSelectFolderandhandleAddAnotherare identical — consolidate and add error handling.Both handlers are character-for-character the same. Additionally, neither wraps
openNew()in a try/catch, so if the file-picker throws (e.g., OS-level dialog error), the rejection is unhandled.♻️ Proposed fix
-const handleSelectFolder = async () => { - const created = await openNew(); - const project = created[0]; - if (project) { - markComplete("project"); - setManualWalkthrough(false); - await openProjectInWorkspace(project.id); - } -}; - -const handleAddAnother = async () => { - const created = await openNew(); - const project = created[0]; - if (project) { - markComplete("project"); - setManualWalkthrough(false); - await openProjectInWorkspace(project.id); - } -}; +const handleOpenNew = async () => { + try { + const created = await openNew(); + const project = created[0]; + if (project) { + markComplete("project"); + setManualWalkthrough(false); + await openProjectInWorkspace(project.id); + } + } catch (err) { + toast.error(err instanceof Error ? err.message : "Failed to open project"); + } +};Then replace both
handleSelectFolderandhandleAddAnothercall sites withhandleOpenNew.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx` around lines 108 - 126, handleSelectFolder and handleAddAnother are duplicates and lack error handling; consolidate them into a single async handler (e.g., handleOpenNew) that calls openNew() inside a try/catch, logs or surfaces any error, and only proceeds to call markComplete("project"), setManualWalkthrough(false), and await openProjectInWorkspace(project.id) when a project exists; then replace all call sites of handleSelectFolder and handleAddAnother with the new handleOpenNew.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx`:
- Around line 205-210: The onboarding gate currently reads a non-reactive
snapshot via useOnboardingStore.getState() and only exempts "/setup" routes,
which blocks the custom-provider flow; change to subscribe to the onboarding
store (e.g., const onboardingState = useOnboardingStore(select => select) or use
selectors like useOnboardingStore(selectRequiredStepsComplete) so the component
re-renders on changes, and widen the route exemption check to include the
settings models path (e.g., const isOnSetupRoute =
location.pathname.startsWith("/setup") ||
location.pathname.startsWith("/settings/models") or detect target ===
"/settings/models" before redirecting), keeping the existing logic with
isV2CloudEnabled, selectRequiredStepsComplete, selectFirstIncompleteStep,
STEP_ROUTES and Navigate semantics.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Around line 119-128: AutoAdvance's effect depends on the onAdvance prop (which
is finishFlow) and thus re-runs on every parent render causing repeated
navigateAfterFlow calls; fix by making the effect run only once: inside
AutoAdvance create a ref (e.g., calledRef) to track whether onAdvance has been
invoked, assign onAdvance to a ref if needed, and use an effect with an empty
dependency array that checks calledRef and calls onAdvance (or
onAdvanceRef.current()) only once, preventing repeated
markComplete/setManualWalkthrough and duplicate navigateAfterFlow network
requests.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/page.tsx`:
- Around line 76-81: The SetupButton on the Codex page must also honor
oauthDialog.isPreparing to prevent duplicate OAuth starts and show the correct
label; update the disabled prop and the conditional label logic in the JSX that
renders SetupButton (the block using SetupButton, handleConnect,
isStartingOAuth, isAuthenticated) to use (isStartingOAuth ||
oauthDialog.isPreparing) for both disabled and the ternary that currently
decides between "Preparing…", "Reconnect", and "Connect".
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeBrandIcon/ClaudeBrandIcon.tsx`:
- Around line 15-17: The ClaudeBrandIcon component is missing default size
classes so when callers omit className or iconClassName the container and SVG
can render oversized; update the JSX that builds className for the root and for
<ClaudeLogo> (the places using cn("flex items-center justify-center
bg-[`#D97757`]", className) and cn("text-white", iconClassName)) to include
sensible defaults (e.g., add container size classes like "w-8 h-8" to the root
and icon size classes like "w-6 h-6" to the <ClaudeLogo> call) while still
merging any provided className/iconClassName via the cn utility.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ProviderOptionCard/ProviderOptionCard.tsx`:
- Around line 23-52: The button in ProviderOptionCard uses the selected prop for
visuals but lacks an accessibility state; update the button element (in
ProviderOptionCard) to include aria-pressed={selected} so assistive tech
receives the selection state (keep existing type="button" and onSelect handler
unchanged).
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/SupersetIcon/SupersetIcon.tsx`:
- Around line 9-15: The SVG in the SupersetIcon component lacks an explicit
role, so add role="img" to the <svg> element (the one with viewBox="0 0 64 60",
className={cn("text-foreground", className)} and aria-label="Superset") to
ensure assistive tech consistently treats it as an image; keep the existing
aria-label and inner <title> untouched so screen readers receive the same
accessible name.
In `@apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts`:
- Around line 141-143: selectRequiredStepsComplete currently treats a skipped
required step as satisfied (REQUIRED_STEPS.every((s) => state.completed[s] ||
state.skipped[s])), which lets callers of markSkipped bypass required gates;
change the predicate to only consider completions so REQUIRED_STEPS are
satisfied only when state.completed[s] is true (leave skipped useful for
optional steps but not for REQUIRED_STEPS). Update selectRequiredStepsComplete
to check only state.completed for each s in REQUIRED_STEPS and audit callers of
markSkipped (and markComplete) to ensure required steps are auto-advanced via
markComplete.
- Around line 104-108: markSkipped currently only sets skipped and startedAt and
doesn't mark onboarding finished; update markSkipped to mirror markComplete's
final-step check by computing whether every step in ONBOARDING_STEP_ORDER is
either in state.completed or in the new skipped map and, if so, set completedAt
= Date.now(); ensure this uses the same state keys as markComplete (completed,
skipped, completedAt, startedAt) so skipping the last step (e.g.,
"adopt-worktrees") correctly sets completedAt.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/permissions.ts`:
- Around line 74-84: Update the remaining mutations to use the new
PrivacySecurity.extension deep-link format: in requestFullDiskAccess,
requestAccessibility, and requestMicrophone replace the legacy
"x-apple.systempreferences:com.apple.preference.security?..." URLs with
"x-apple.systempreferences:com.apple.settings.PrivacySecurity.extension?{Anchor}"
(use Privacy_AllFiles for full disk access, Privacy_Accessibility for
accessibility, and Privacy_Microphone for microphone) while keeping the
shell.openExternal call and async mutation behavior unchanged.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/components/StepShell/StepShell.tsx`:
- Around line 15-21: The interfaces (e.g., StepShellProps) use the
React.ReactNode namespace but the file doesn't import React; update the file to
import the type explicitly (import type { ReactNode } from "react") and replace
all occurrences of React.ReactNode with the imported ReactNode in the interfaces
and any other type positions (lines referenced around StepShellProps and the two
other interfaces at 57-61 and 75-77) to match the project's pattern used in
SetupButton.tsx and ProviderOptionCard.tsx.
In `@apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx`:
- Around line 108-126: handleSelectFolder and handleAddAnother are duplicates
and lack error handling; consolidate them into a single async handler (e.g.,
handleOpenNew) that calls openNew() inside a try/catch, logs or surfaces any
error, and only proceeds to call markComplete("project"),
setManualWalkthrough(false), and await openProjectInWorkspace(project.id) when a
project exists; then replace all call sites of handleSelectFolder and
handleAddAnother with the new handleOpenNew.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a5365c1-f530-4ba9-a88a-893640158c43
📒 Files selected for processing (39)
apps/desktop/plans/done/20260504-1200-v2-onboarding-flow.mdapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/permissions.tsapps/desktop/src/lib/trpc/routers/system.tsapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsxapps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/components/SetupButton/SetupButton.tsxapps/desktop/src/renderer/routes/_authenticated/setup/components/SetupButton/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/components/StepShell/StepShell.tsxapps/desktop/src/renderer/routes/_authenticated/setup/components/StepShell/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/gh-cli/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/layout.tsxapps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/api-key/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/custom/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/api-key/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/custom/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ApiKeyForm/ApiKeyForm.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ApiKeyForm/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeBrandIcon/ClaudeBrandIcon.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeBrandIcon/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeLogo/ClaudeLogo.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeLogo/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/CodexBrandIcon/CodexBrandIcon.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/CodexBrandIcon/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ProviderOptionCard/ProviderOptionCard.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ProviderOptionCard/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/SupersetIcon/SupersetIcon.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/components/SupersetIcon/index.tsapps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsxapps/desktop/src/renderer/stores/onboarding/index.tsapps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
Comment on lines
+205
to
+210
| const onboardingState = useOnboardingStore.getState(); | ||
| const requiredComplete = selectRequiredStepsComplete(onboardingState); | ||
| const isOnSetupRoute = location.pathname.startsWith("/setup"); | ||
| if (isV2CloudEnabled && !requiredComplete && !isOnSetupRoute) { | ||
| const target = STEP_ROUTES[selectFirstIncompleteStep(onboardingState)]; | ||
| return <Navigate to={target} replace />; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the gate condition and custom-provider settings navigation coexist.
rg -n --type=tsx -C2 'useOnboardingStore\.getState\(\)|startsWith\("/setup"\)|/settings/models' \
apps/desktop/src/renderer/routes/_authenticated/layout.tsx \
apps/desktop/src/renderer/routes/_authenticated/setup/providersRepository: superset-sh/superset
Length of output: 92
🌐 Web query:
In Zustand, does calling store.getState() inside a React component subscribe the component to state updates, or is it just a snapshot?
💡 Result:
Calling store.getState() inside a React component does not subscribe the component to state updates; it provides a snapshot of the current state at the time of the call.[1][2][3][4] The official Zustand documentation describes getState() as providing "non-reactive fresh state," intended for accessing state outside of React components or in non-reactive contexts.[2][4][5] In contrast, using the useStore hook (e.g., useStore((state) => state.value)) subscribes the component to re-render when the selected state changes.[6] Multiple sources from Zustand GitHub discussions confirm this: store.getState() is for vanilla (non-React) state reading and does not trigger React re-renders or subscriptions.[1][3] Components only subscribe via the hook; getState() merely returns the current state snapshot during the current render.[7][8] If the component appears to update after using getState(), it's because another part of the component (or a parent) uses useStore, causing re-renders where getState() fetches the fresh value each time.[7]
Citations:
- 1: Difference between `useStore` and `store.getState()` pmndrs/zustand#2194
- 2: https://www.npmjs.com/package/zustand
- 3: Clarification on using getState() vs useStore and potential side effects pmndrs/zustand#2712
- 4: https://cdn.jsdelivr.net/npm/zustand@4.3.8/readme.md
- 5: https://www.github.com/pmndrs/zustand
- 6: https://zustand.docs.pmnd.rs/getting-started/introduction
- 7: https://www.reddit.com/r/reactjs/comments/1ogx7pm/using_getstate_in_zustand_why_am_i_getting_the/
- 8: https://dev.to/muratcanyuksel/something-to-consider-when-sharing-state-with-zustand-47pe
🏁 Script executed:
# Search for useOnboardingStore.getState() usage and gate conditions
rg -n 'useOnboardingStore\.getState\(\)' apps/desktop/src/renderer/routes/_authenticated/layout.tsx -A 10Repository: superset-sh/superset
Length of output: 563
🏁 Script executed:
# Search for /settings/models and custom provider routes
rg -n '/settings/models' apps/desktop/src/renderer/routes/ -B 2 -A 2Repository: superset-sh/superset
Length of output: 6067
🏁 Script executed:
# Find custom provider setup flow
fd -e ts -e tsx apps/desktop/src/renderer/routes/_authenticated/setup | head -20Repository: superset-sh/superset
Length of output: 582
🏁 Script executed:
# Check the actual code at lines 205-210 in the layout file
sed -n '200,215p' apps/desktop/src/renderer/routes/_authenticated/layout.tsxRepository: superset-sh/superset
Length of output: 611
🏁 Script executed:
# Check for comments or TODOs around the gate logic
sed -n '200,215p' apps/desktop/src/renderer/routes/_authenticated/layout.tsx | head -30Repository: superset-sh/superset
Length of output: 611
🏁 Script executed:
# Search for onboarding flow documentation or issue comments
rg -n 'isV2CloudEnabled|onboarding|setup' apps/desktop/src/renderer/routes/_authenticated/layout.tsx -B 3 -A 3 | head -50Repository: superset-sh/superset
Length of output: 1491
🏁 Script executed:
# Check if there are tests or docs about the redirect behavior
fd -e md -e test -e spec apps/desktop | xargs rg -l 'onboarding.*redirect|settings.*models.*setup' 2>/dev/null | head -5Repository: superset-sh/superset
Length of output: 496
🏁 Script executed:
# Read the onboarding flow documentation
cat apps/desktop/plans/done/20260504-1200-v2-onboarding-flow.mdRepository: superset-sh/superset
Length of output: 50376
Onboarding gate blocks the custom-provider settings flow and uses a non-reactive store snapshot.
When users select "Custom Model" during provider setup, the page routes to /settings/models. However, the gate at line 205-208 exempts only /setup* routes—not /settings/models—so it redirects users back to setup, preventing the custom provider flow. Additionally, useOnboardingStore.getState() at line 205 is a snapshot and does not subscribe the component to state updates, so it won't re-render if onboarding state changes.
💡 Suggested fix
- const onboardingState = useOnboardingStore.getState();
- const requiredComplete = selectRequiredStepsComplete(onboardingState);
+ const requiredComplete = useOnboardingStore(selectRequiredStepsComplete);
+ const firstIncompleteStep = useOnboardingStore(selectFirstIncompleteStep);
const isOnSetupRoute = location.pathname.startsWith("/setup");
- if (isV2CloudEnabled && !requiredComplete && !isOnSetupRoute) {
- const target = STEP_ROUTES[selectFirstIncompleteStep(onboardingState)];
+ const isOnboardingBypassRoute = location.pathname.startsWith("/settings/models");
+ if (
+ isV2CloudEnabled &&
+ !requiredComplete &&
+ !isOnSetupRoute &&
+ !isOnboardingBypassRoute
+ ) {
+ const target = STEP_ROUTES[firstIncompleteStep];
return <Navigate to={target} replace />;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const onboardingState = useOnboardingStore.getState(); | |
| const requiredComplete = selectRequiredStepsComplete(onboardingState); | |
| const isOnSetupRoute = location.pathname.startsWith("/setup"); | |
| if (isV2CloudEnabled && !requiredComplete && !isOnSetupRoute) { | |
| const target = STEP_ROUTES[selectFirstIncompleteStep(onboardingState)]; | |
| return <Navigate to={target} replace />; | |
| const requiredComplete = useOnboardingStore(selectRequiredStepsComplete); | |
| const firstIncompleteStep = useOnboardingStore(selectFirstIncompleteStep); | |
| const isOnSetupRoute = location.pathname.startsWith("/setup"); | |
| const isOnboardingBypassRoute = location.pathname.startsWith("/settings/models"); | |
| if ( | |
| isV2CloudEnabled && | |
| !requiredComplete && | |
| !isOnSetupRoute && | |
| !isOnboardingBypassRoute | |
| ) { | |
| const target = STEP_ROUTES[firstIncompleteStep]; | |
| return <Navigate to={target} replace />; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/routes/_authenticated/layout.tsx` around lines 205
- 210, The onboarding gate currently reads a non-reactive snapshot via
useOnboardingStore.getState() and only exempts "/setup" routes, which blocks the
custom-provider flow; change to subscribe to the onboarding store (e.g., const
onboardingState = useOnboardingStore(select => select) or use selectors like
useOnboardingStore(selectRequiredStepsComplete) so the component re-renders on
changes, and widen the route exemption check to include the settings models path
(e.g., const isOnSetupRoute = location.pathname.startsWith("/setup") ||
location.pathname.startsWith("/settings/models") or detect target ===
"/settings/models" before redirecting), keeping the existing logic with
isV2CloudEnabled, selectRequiredStepsComplete, selectFirstIncompleteStep,
STEP_ROUTES and Navigate semantics.
Comment on lines
+76
to
+81
| <SetupButton onClick={handleConnect} disabled={isStartingOAuth}> | ||
| {isStartingOAuth | ||
| ? "Preparing…" | ||
| : isAuthenticated | ||
| ? "Reconnect" | ||
| : "Connect"} |
Contributor
There was a problem hiding this comment.
oauthDialog.isPreparing not checked — button is enabled and label is wrong during dialog preparation.
The Claude Code page guards with isStartingOAuth || oauthDialog.isPreparing in both the disabled prop and the button label. The Codex page only checks isStartingOAuth, leaving the button clickable while the dialog is still preparing, which can trigger duplicate OAuth initiations.
🐛 Proposed fix
-<SetupButton onClick={handleConnect} disabled={isStartingOAuth}>
- {isStartingOAuth
+<SetupButton onClick={handleConnect} disabled={isStartingOAuth || oauthDialog.isPreparing}>
+ {isStartingOAuth || oauthDialog.isPreparing
? "Preparing…"
: isAuthenticated
? "Reconnect"
: "Connect"}
</SetupButton>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <SetupButton onClick={handleConnect} disabled={isStartingOAuth}> | |
| {isStartingOAuth | |
| ? "Preparing…" | |
| : isAuthenticated | |
| ? "Reconnect" | |
| : "Connect"} | |
| <SetupButton onClick={handleConnect} disabled={isStartingOAuth || oauthDialog.isPreparing}> | |
| {isStartingOAuth || oauthDialog.isPreparing | |
| ? "Preparing…" | |
| : isAuthenticated | |
| ? "Reconnect" | |
| : "Connect"} | |
| </SetupButton> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/page.tsx`
around lines 76 - 81, The SetupButton on the Codex page must also honor
oauthDialog.isPreparing to prevent duplicate OAuth starts and show the correct
label; update the disabled prop and the conditional label logic in the JSX that
renders SetupButton (the block using SetupButton, handleConnect,
isStartingOAuth, isAuthenticated) to use (isStartingOAuth ||
oauthDialog.isPreparing) for both disabled and the ternary that currently
decides between "Preparing…", "Reconnect", and "Connect".
Comment on lines
+15
to
+17
| className={cn("flex items-center justify-center bg-[#D97757]", className)} | ||
| > | ||
| <ClaudeLogo className={cn("text-white", iconClassName)} /> |
Contributor
There was a problem hiding this comment.
Add default sizing to avoid oversized fallback rendering.
On Line 15 and Line 17, there are no default size classes. If callers omit className/iconClassName, the SVG can render at its intrinsic default size and distort layout.
Suggested fix
export function ClaudeBrandIcon({
className,
iconClassName,
}: ClaudeBrandIconProps) {
return (
<div
- className={cn("flex items-center justify-center bg-[`#D97757`]", className)}
+ className={cn(
+ "flex size-8 items-center justify-center rounded-md bg-[`#D97757`]",
+ className,
+ )}
>
- <ClaudeLogo className={cn("text-white", iconClassName)} />
+ <ClaudeLogo className={cn("size-4 text-white", iconClassName)} />
</div>
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| className={cn("flex items-center justify-center bg-[#D97757]", className)} | |
| > | |
| <ClaudeLogo className={cn("text-white", iconClassName)} /> | |
| export function ClaudeBrandIcon({ | |
| className, | |
| iconClassName, | |
| }: ClaudeBrandIconProps) { | |
| return ( | |
| <div | |
| className={cn( | |
| "flex size-8 items-center justify-center rounded-md bg-[`#D97757`]", | |
| className, | |
| )} | |
| > | |
| <ClaudeLogo className={cn("size-4 text-white", iconClassName)} /> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ClaudeBrandIcon/ClaudeBrandIcon.tsx`
around lines 15 - 17, The ClaudeBrandIcon component is missing default size
classes so when callers omit className or iconClassName the container and SVG
can render oversized; update the JSX that builds className for the root and for
<ClaudeLogo> (the places using cn("flex items-center justify-center
bg-[`#D97757`]", className) and cn("text-white", iconClassName)) to include
sensible defaults (e.g., add container size classes like "w-8 h-8" to the root
and icon size classes like "w-6 h-6" to the <ClaudeLogo> call) while still
merging any provided className/iconClassName via the cn utility.
Comment on lines
+23
to
+52
| <button | ||
| type="button" | ||
| onClick={onSelect} | ||
| className={cn( | ||
| "relative flex w-full items-center gap-4 rounded-xl border p-4 text-left transition-colors", | ||
| selected | ||
| ? "border-[rgba(255,136,70,0.6)] bg-[rgba(255,91,0,0.08)]" | ||
| : "border-[#2a2827] bg-[#201e1c] hover:bg-[#2a2827]", | ||
| )} | ||
| > | ||
| <div className="size-11 shrink-0 overflow-hidden rounded-lg">{icon}</div> | ||
| <div className="min-w-0 flex-1 space-y-0.5"> | ||
| <div className="flex items-center gap-2"> | ||
| <span className="text-[13px] font-semibold text-[#eae8e6]"> | ||
| {title} | ||
| </span> | ||
| {recommended && ( | ||
| <span className="rounded-md bg-[#151110] px-1.5 py-0.5 text-[10px] font-medium text-[#a8a5a3]"> | ||
| Recommended | ||
| </span> | ||
| )} | ||
| </div> | ||
| <p className="text-[11px] text-[#a8a5a3]">{description}</p> | ||
| </div> | ||
| {selected && ( | ||
| <div className="flex size-6 shrink-0 items-center justify-center rounded-full bg-[rgba(255,91,0,0.8)] text-white"> | ||
| <LuCheck className="size-3.5" strokeWidth={3} /> | ||
| </div> | ||
| )} | ||
| </button> |
Contributor
There was a problem hiding this comment.
Add aria-pressed to communicate selection state to assistive technologies.
The selected prop drives a visual checkmark and border change, but screen readers have no programmatic signal that the button is in a pressed/selected state.
♿ Proposed fix
<button
type="button"
+ aria-pressed={selected}
onClick={onSelect}
className={cn(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/ProviderOptionCard/ProviderOptionCard.tsx`
around lines 23 - 52, The button in ProviderOptionCard uses the selected prop
for visuals but lacks an accessibility state; update the button element (in
ProviderOptionCard) to include aria-pressed={selected} so assistive tech
receives the selection state (keep existing type="button" and onSelect handler
unchanged).
Comment on lines
+9
to
+15
| <svg | ||
| viewBox="0 0 64 60" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className={cn("text-foreground", className)} | ||
| aria-label="Superset" | ||
| > |
Contributor
There was a problem hiding this comment.
Add role="img" for consistent screen-reader behavior.
The SVG has both aria-label and an inner <title>, which is correct — but without role="img", some assistive technologies may ignore aria-label on an SVG element and fall back to treating it as a generic element. Adding role="img" makes the intent unambiguous.
♿ Proposed fix
<svg
viewBox="0 0 64 60"
fill="none"
xmlns="http://www.w3.org/2000/svg"
className={cn("text-foreground", className)}
+ role="img"
aria-label="Superset"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <svg | |
| viewBox="0 0 64 60" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className={cn("text-foreground", className)} | |
| aria-label="Superset" | |
| > | |
| <svg | |
| viewBox="0 0 64 60" | |
| fill="none" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className={cn("text-foreground", className)} | |
| role="img" | |
| aria-label="Superset" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/components/SupersetIcon/SupersetIcon.tsx`
around lines 9 - 15, The SVG in the SupersetIcon component lacks an explicit
role, so add role="img" to the <svg> element (the one with viewBox="0 0 64 60",
className={cn("text-foreground", className)} and aria-label="Superset") to
ensure assistive tech consistently treats it as an image; keep the existing
aria-label and inner <title> untouched so screen readers receive the same
accessible name.
Tracks onboarding_started, onboarding_step_started, onboarding_step_completed, onboarding_step_skipped, onboarding_provider_connected (provider + method), onboarding_restarted, and onboarding_finished (with outcome + duration_ms).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx (1)
130-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent repeated auto-finish calls in
AutoAdvance
AutoAdvancere-runs on Line 132 wheneveronAdvanceidentity changes; hereonAdvance={finishFlow}(Line 117) is recreated on each render (Line 92). That can firefinishFlowmultiple times (duplicate onboarding completion + repeated navigation/fetch).Proposed minimal fix
function AutoAdvance({ onAdvance }: { onAdvance: () => void }) { + const onAdvanceRef = useRef(onAdvance); + useEffect(() => { + onAdvanceRef.current = onAdvance; + }, [onAdvance]); + useEffect(() => { - onAdvance(); - }, [onAdvance]); + onAdvanceRef.current(); + // run once on mount + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); return ( <div className="flex h-full w-full items-center justify-center bg-[`#151110`]"> <Spinner className="size-6 text-[`#a8a5a3`]" /> </div> ); }#!/bin/bash # Verify the unstable-callback pattern and effect dependency in this file. rg -n -C3 'function AutoAdvance|useEffect\(|onAdvance=\{finishFlow\}|const finishFlow\s*=' apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxAlso applies to: 117-117, 92-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx` around lines 130 - 133, AutoAdvance calls onAdvance inside a useEffect that reruns whenever onAdvance identity changes (causing finishFlow to fire multiple times); fix by stabilizing the callback identity or isolating the effect: wrap finishFlow in a stable useCallback where it's defined (so the prop onAdvance passed to AutoAdvance doesn't change each render), or change AutoAdvance to only run on mount by removing onAdvance from its dependency array and invoking it once (ensuring you intentionally accept the stale-closure risk). Refer to AutoAdvance, onAdvance, and finishFlow when making the change.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsx (1)
8-15: ⚡ Quick winPrefer tsconfig alias imports for local setup components
These relative imports should be switched to project aliases for consistency and move-safety (same file already uses
renderer/...aliases).As per coding guidelines,
apps/desktop/**/*.{ts,tsx}: Use alias as defined intsconfig.jsonwhen possible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsx` around lines 8 - 15, Replace the relative imports for local setup components with the project's tsconfig alias paths: update the imports that reference SetupButton, StepHeader, StepShell, SupersetPill, ClaudeBrandIcon, and SupersetIcon to use the renderer/... aliases (e.g., import { SetupButton } from "renderer/routes/_authenticated/setup/components/SetupButton" or the equivalent alias path used in the repo) instead of the current "../../" relative paths so they match the rest of the file and remain safe to move/rename.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsx`:
- Around line 41-46: The ref wasAuthedOnMount is only set when status changes
which can miss the case where status is unresolved during OAuth; set the ref
immediately on mount to the current isAuthenticated value instead of waiting for
status (e.g., replace the useEffect that checks status with a mount-only effect
that does if (wasAuthedOnMount.current === null) wasAuthedOnMount.current =
Boolean(isAuthenticated)); make the same change for the other similar useEffect
blocks that reference wasAuthedOnMount (the blocks around the ranges you noted)
so the auth-success detection, auto-navigation and analytics fire reliably.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Around line 130-133: AutoAdvance calls onAdvance inside a useEffect that
reruns whenever onAdvance identity changes (causing finishFlow to fire multiple
times); fix by stabilizing the callback identity or isolating the effect: wrap
finishFlow in a stable useCallback where it's defined (so the prop onAdvance
passed to AutoAdvance doesn't change each render), or change AutoAdvance to only
run on mount by removing onAdvance from its dependency array and invoking it
once (ensuring you intentionally accept the stale-closure risk). Refer to
AutoAdvance, onAdvance, and finishFlow when making the change.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsx`:
- Around line 8-15: Replace the relative imports for local setup components with
the project's tsconfig alias paths: update the imports that reference
SetupButton, StepHeader, StepShell, SupersetPill, ClaudeBrandIcon, and
SupersetIcon to use the renderer/... aliases (e.g., import { SetupButton } from
"renderer/routes/_authenticated/setup/components/SetupButton" or the equivalent
alias path used in the repo) instead of the current "../../" relative paths so
they match the rest of the file and remain safe to move/rename.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fbbb3f2-371e-4d10-9aee-ab3b3745e148
📒 Files selected for processing (6)
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/api-key/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/api-key/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/page.tsxapps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/api-key/page.tsx
- apps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/page.tsx
- apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
- apps/desktop/src/renderer/routes/_authenticated/setup/providers/codex/api-key/page.tsx
Comment on lines
+41
to
+46
| const wasAuthedOnMount = useRef<boolean | null>(null); | ||
| useEffect(() => { | ||
| if (status !== undefined && wasAuthedOnMount.current === null) { | ||
| wasAuthedOnMount.current = isAuthenticated; | ||
| } | ||
| }, [status, isAuthenticated]); |
Contributor
There was a problem hiding this comment.
Auth success can be missed when initial status is unresolved
If status is still undefined when the user starts OAuth, and the first resolved status is authenticated, wasAuthedOnMount.current gets set to true and the success block won’t run. That skips both auto-navigation and the connected analytics event in this path.
Suggested patch
const wasAuthedOnMount = useRef<boolean | null>(null);
+const didStartOAuth = useRef(false);
useEffect(() => {
if (status !== undefined && wasAuthedOnMount.current === null) {
wasAuthedOnMount.current = isAuthenticated;
}
}, [status, isAuthenticated]);
useEffect(() => {
- if (wasAuthedOnMount.current === false && isAuthenticated) {
+ if (
+ isAuthenticated &&
+ (wasAuthedOnMount.current === false || didStartOAuth.current)
+ ) {
track("onboarding_provider_connected", {
provider: "anthropic",
method: "oauth",
});
+ didStartOAuth.current = false;
navigate({ to: "/setup/providers", replace: true });
}
}, [isAuthenticated, navigate]);
const handleConnect = () => {
+ didStartOAuth.current = true;
void startAnthropicOAuth();
};Also applies to: 52-60, 62-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/providers/claude-code/page.tsx`
around lines 41 - 46, The ref wasAuthedOnMount is only set when status changes
which can miss the case where status is unresolved during OAuth; set the ref
immediately on mount to the current isAuthenticated value instead of waiting for
status (e.g., replace the useEffect that checks status with a mount-only effect
that does if (wasAuthedOnMount.current === null) wasAuthedOnMount.current =
Boolean(isAuthenticated)); make the same change for the other similar useEffect
blocks that reference wasAuthedOnMount (the blocks around the ranges you noted)
so the auth-success detection, auto-navigation and analytics fire reliably.
- Reactive store selectors in _authenticated/layout.tsx so the gate re-renders when onboarding state updates (greptile P2, coderabbit major) - selectRequiredStepsComplete only counts completed steps so a stray markSkipped call can't bypass the gate (coderabbit major) - markSkipped also sets completedAt when all steps done (coderabbit minor) - Stabilize finishFlow / skipFlow / navigateAfterFlow with useCallback so AutoAdvance doesn't re-fire each render (greptile P2, coderabbit minor) - Dedupe handleSelectFolder + handleAddAnother into handleSelectNewRepo (greptile P2) - Restart-onboarding dialog copy now matches manualWalkthrough behavior (greptile P1)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx (2)
46-85: 💤 Low valueSilent
catchinnavigateAfterFlowhides workspace fetch failures.Falling through to project/welcome routing on error is a reasonable resilience choice, but swallowing the error makes diagnostics harder when users report a wrong post-onboarding landing. Consider at least logging via your existing logger so the failure is observable.
♻️ Suggested logging
- } catch { - // fall through to project / welcome routing - } + } catch (err) { + // Fall through to project / welcome routing, but record the failure + // so an unexpected post-onboarding destination can be diagnosed. + console.warn("navigateAfterFlow: workspaces.getAllGrouped failed", err); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx` around lines 46 - 85, The catch in navigateAfterFlow silently swallows errors from utils.workspaces.getAllGrouped.fetch; change the catch to capture the error (e.g., catch (err)) and log it before falling through so failures are observable — use the app logger if available or console.error and include context like "navigateAfterFlow: failed to fetch workspaces" and the error and any relevant ids (e.g., lastViewedWorkspaceId) so debugging via navigateAfterFlow, utils.workspaces.getAllGrouped.fetch, isV2CloudEnabled, and navigate is possible.
178-202: 💤 Low value
handleImportAllalways proceeds toonFinisheven when every import fails.Per-project failures emit individual error toasts, but if
totalImportedends at0after one or more errors, the user sees no aggregate confirmation before being navigated to the workspace/welcome route. This is acceptable for a skip-friendly onboarding flow, but consider either surfacing a single summary toast on partial/full failure, or staying on the page so the user can retry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx` around lines 178 - 202, handleImportAll currently always calls onFinish even when no worktrees were successfully imported; change the flow so you either (A) only call onFinish when totalImported > 0, and when totalImported === 0 show an aggregate error/notice via toast.error (or toast.info) so the user can retry, or (B) keep onFinish but also show a summary toast when any imports failed (compare totalImported vs sum of expected imports) before navigating. Update the logic around handleImportAll to check totalImported and error state after the loop (referencing handleImportAll, importAllWorktrees.mutateAsync, totalImported, toast.error/toast.success, and onFinish) and implement one of the two behaviors above so the user isn’t navigated away silently on complete failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Around line 172-176: The current auto-finish and subtitle logic treat
per-project scan errors as empty results because ProjectWorktrees calls
onResultRef.current([]) on error; update the results shape to include an errored
flag (e.g., { worktrees, loaded, errored }), have ProjectWorktrees pass errored:
true via onResultRef/onResult when isError, and then change the parent logic
that computes total/allLoaded and the useEffect that calls onFinish() to exclude
errored projects (i.e., only count projects with loaded && !errored for the
“nothing to adopt” auto-finish path); also update StepHeader/subtitle rendering
to check errored vs truly-empty so scan failures show error rows instead of the
“already tracked” message.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Around line 46-85: The catch in navigateAfterFlow silently swallows errors
from utils.workspaces.getAllGrouped.fetch; change the catch to capture the error
(e.g., catch (err)) and log it before falling through so failures are observable
— use the app logger if available or console.error and include context like
"navigateAfterFlow: failed to fetch workspaces" and the error and any relevant
ids (e.g., lastViewedWorkspaceId) so debugging via navigateAfterFlow,
utils.workspaces.getAllGrouped.fetch, isV2CloudEnabled, and navigate is
possible.
- Around line 178-202: handleImportAll currently always calls onFinish even when
no worktrees were successfully imported; change the flow so you either (A) only
call onFinish when totalImported > 0, and when totalImported === 0 show an
aggregate error/notice via toast.error (or toast.info) so the user can retry, or
(B) keep onFinish but also show a summary toast when any imports failed (compare
totalImported vs sum of expected imports) before navigating. Update the logic
around handleImportAll to check totalImported and error state after the loop
(referencing handleImportAll, importAllWorktrees.mutateAsync, totalImported,
toast.error/toast.success, and onFinish) and implement one of the two behaviors
above so the user isn’t navigated away silently on complete failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fab33805-41fd-47df-a77b-44f7df7f2c84
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsxapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxapps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/routes/_authenticated/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src/renderer/routes/_authenticated/settings/experimental/components/ExperimentalSettings/ExperimentalSettings.tsx
- apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
- apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
…orkspace The default workspace auto-created by openNew → ensureMainWorkspace is type "branch" and has no v2 pane layout, so navigating to /v2-workspace/$id renders an empty surface (no terminal, no chat). Project step (Select new repo): in v2 navigate to /project/$projectId so the user creates their first worktree via the v2 workspace creation flow. v1 keeps the previous behavior (branch workspaces work fine in v1). Adopt-worktrees finishFlow/skipFlow: in v2 only consider worktree-type workspaces when picking a navigation target. If none exist, fall back to the project page.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`:
- Around line 219-222: The subtitle string incorrectly uses "aren't" for the
singular case; update the conditional that builds the subtitle (the JSX/ternary
using nothingToAdopt and total in page.tsx) to choose "isn't" when total === 1
and "aren't" otherwise, keeping the existing pluralization logic for
"worktree(s)" intact so the message reads "Found 1 worktree on disk that isn't
yet tracked." for the singular case.
- Around line 178-182: The effect calling onFinish can run twice when
manualWalkthrough flips; add a one-time guard (e.g., finishedRef via
useRef<boolean>(false)) and check finishedRef.current before invoking onFinish
inside the useEffect (set finishedRef.current = true immediately before calling
onFinish) to ensure onFinish runs at most once; also update the manual
Continue/Import all/Skip handlers (the handlers that call
setManualWalkthrough(false) / onFinish) to consult the same finishedRef and
return early if it is already true so button clicks cannot trigger a second
concurrent onFinish/navigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85b5121b-aee6-41c8-b5c4-54811e506580
📒 Files selected for processing (2)
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
Comment on lines
+219
to
+222
| : nothingToAdopt | ||
| ? "All worktrees on disk are already tracked." | ||
| : `Found ${total} worktree${total === 1 ? "" : "s"} on disk that aren't yet tracked.` | ||
| } |
Contributor
There was a problem hiding this comment.
Subject-verb agreement bug for total === 1 case.
When exactly one worktree is found, the subtitle renders “Found 1 worktree on disk that aren't yet tracked.” — should be “isn't”.
📝 Proposed fix
subtitle={
!allLoaded
? "Scanning your projects for unadopted worktrees…"
: nothingToAdopt
? "All worktrees on disk are already tracked."
- : `Found ${total} worktree${total === 1 ? "" : "s"} on disk that aren't yet tracked.`
+ : total === 1
+ ? "Found 1 worktree on disk that isn't yet tracked."
+ : `Found ${total} worktrees on disk that aren't yet tracked.`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| : nothingToAdopt | |
| ? "All worktrees on disk are already tracked." | |
| : `Found ${total} worktree${total === 1 ? "" : "s"} on disk that aren't yet tracked.` | |
| } | |
| subtitle={ | |
| !allLoaded | |
| ? "Scanning your projects for unadopted worktrees…" | |
| : nothingToAdopt | |
| ? "All worktrees on disk are already tracked." | |
| : total === 1 | |
| ? "Found 1 worktree on disk that isn't yet tracked." | |
| : `Found ${total} worktrees on disk that aren't yet tracked.` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx`
around lines 219 - 222, The subtitle string incorrectly uses "aren't" for the
singular case; update the conditional that builds the subtitle (the JSX/ternary
using nothingToAdopt and total in page.tsx) to choose "isn't" when total === 1
and "aren't" otherwise, keeping the existing pluralization logic for
"worktree(s)" intact so the message reads "Found 1 worktree on disk that isn't
yet tracked." for the singular case.
Clicking "Restart onboarding" now clears completion flags without forcing the user through every step. Auto-advance still applies, so a user with already-configured prerequisites (signed in, provider connected, projects exist) breezes through the same way a fresh user does — just from a clean state.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
pushed a commit
that referenced
this pull request
May 6, 2026
* feat(desktop): v2 onboarding setup flow Adds /setup/* with 5 steps (providers, gh-cli, permissions, project, adopt-worktrees), a hard redirect gate in _authenticated/layout.tsx, and a Restart entry point in Settings → Experimental. Reuses existing chat-service auth, permissions, and workspace-import APIs; adds system.detectGhCli for gh detection. * feat(desktop): add posthog events for v2 onboarding flow Tracks onboarding_started, onboarding_step_started, onboarding_step_completed, onboarding_step_skipped, onboarding_provider_connected (provider + method), onboarding_restarted, and onboarding_finished (with outcome + duration_ms). * fix(desktop): address bot review on v2 onboarding setup flow - Reactive store selectors in _authenticated/layout.tsx so the gate re-renders when onboarding state updates (greptile P2, coderabbit major) - selectRequiredStepsComplete only counts completed steps so a stray markSkipped call can't bypass the gate (coderabbit major) - markSkipped also sets completedAt when all steps done (coderabbit minor) - Stabilize finishFlow / skipFlow / navigateAfterFlow with useCallback so AutoAdvance doesn't re-fire each render (greptile P2, coderabbit minor) - Dedupe handleSelectFolder + handleAddAnother into handleSelectNewRepo (greptile P2) - Restart-onboarding dialog copy now matches manualWalkthrough behavior (greptile P1) * fix(desktop): route v2 setup completion to project page, not branch workspace The default workspace auto-created by openNew → ensureMainWorkspace is type "branch" and has no v2 pane layout, so navigating to /v2-workspace/$id renders an empty surface (no terminal, no chat). Project step (Select new repo): in v2 navigate to /project/$projectId so the user creates their first worktree via the v2 workspace creation flow. v1 keeps the previous behavior (branch workspaces work fine in v1). Adopt-worktrees finishFlow/skipFlow: in v2 only consider worktree-type workspaces when picking a navigation target. If none exist, fall back to the project page. * fix(desktop): keep manualWalkthrough false on onboarding reset Clicking "Restart onboarding" now clears completion flags without forcing the user through every step. Auto-advance still applies, so a user with already-configured prerequisites (signed in, provider connected, projects exist) breezes through the same way a fresh user does — just from a clean state.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/setup/*with five steps: AI providers, GitHub CLI, macOS permissions, project, and adopt worktrees._authenticated/layout.tsx: when v2 is enabled and required steps (providers + project) are incomplete, users are sent into setup until they finish./v2-workspace/$workspaceId(v2) or/workspace/$workspaceId(v1) based onuseIsV2CloudEnabledso users land in the correct version's workspace editor.system.detectGhClitRPC procedure probes/opt/homebrew/bin,/usr/local/bin,/usr/bin,/bin, then PATH forgh.chatService.auth.getAnthropic/OpenAIStatus, OAuth dialogs,setAnthropic/OpenAIApiKey,permissions.getStatus,projects.getRecents/openNew/close, andworkspaces.getExternalWorktrees/importAllWorktrees— no new functionality on the server side beyond gh detection.The plan that drove this work moved to
apps/desktop/plans/done/20260504-1200-v2-onboarding-flow.md.Test plan
bun dev→ Settings → Experimental → "Restart onboarding"/v2-workspace/$workspaceIdwhen v2 is enabledbun run typecheck && bun run lintcleanSummary by cubic
Adds a v2 onboarding flow at
/setup/*with five steps and a redirect gate that keeps users in setup until required steps are done. Completion now routes v2 users to/project/$projectId(or an existing worktree if available) and v1 users to the v1 workspace, with full onboarding telemetry.New Features
/setup/*pages with a progress bar, clickable completed steps, and a shared Back via a step shell._authenticated/layout.tsxwhen v2 is enabled until providers and project are complete.system.detectGhClitRPCprocedure returns install status, version, and path by probing common locations and$PATH.Bug Fixes
/project/$projectIdinstead of a branch workspace; adopt-worktrees finish/skip prefers worktree-type workspaces and falls back to the project page.Written for commit 1825686. Summary will update on new commits.
Summary by CodeRabbit