Repository navigation
Conversation
… step Re-gates fresh v2 users into /setup on first authenticated render (isV2 + no v2 workspaces + required steps incomplete) but treats the gate as advisory: - "Skip for now" / nav-away from /setup sets an in-memory flag and the gate won't re-fire this launch - "Don't show again" persists dismissedForever; reset via Settings > Experimental > Restart onboarding - V1 users never instantiate the gate (outer wrapper short-circuits) and don't see the Skip pill in /setup chrome Also drops the adopt-worktrees onboarding step entirely — project is now the terminal step and routes to /v2-workspaces on completion/skip, carrying the onboarding_finished telemetry the prior step owned.
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
📝 WalkthroughWalkthroughThis PR restructures the V2 onboarding flow by removing the "adopt-worktrees" step, adding skip and dismissal state flags to the onboarding store, implementing a gating guard to prevent users from bypassing setup until onboarding is complete, and integrating analytics tracking at onboarding completion. ChangesV2 Onboarding Flow Restructuring
Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingProgress
participant V2OnboardingGate
participant OnboardingStore
participant Navigator
User->>OnboardingProgress: click "Skip for now"
OnboardingProgress->>OnboardingStore: skipUntilNextLaunch()
OnboardingStore->>OnboardingStore: set skippedThisLaunch=true, track event
OnboardingProgress->>Navigator: navigate to /v2-workspaces
Note over OnboardingStore: skippedThisLaunch resets on next app launch
User->>OnboardingProgress: click "Don't show again"
OnboardingProgress->>OnboardingStore: dismissForever()
OnboardingStore->>OnboardingStore: set dismissedForever=true, track event, persist
OnboardingProgress->>Navigator: navigate to /v2-workspaces
Note over OnboardingStore: dismissedForever persists across launches
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Ready to review this PR? Stage has broken it down into 5 individual chapters for you: Chapters generated by Stage for commit 43431a8 on May 19, 2026 8:42pm UTC. |
Greptile SummaryThis PR re-enables the v2 onboarding gate as a non-blocking advisory redirect and drops the
Confidence Score: 3/5Functional correctness of the gate is sound, but the nav-away detection fires an unintended analytics event on every normal onboarding completion, which will corrupt the onboarding_skipped_until_next_launch metric from day one. The nav-away useLayoutEffect does not exclude the case where the user exits /setup after completing all required steps via the project page's own buttons. Because finishOnboarding simply calls navigate, the component sees a regular location change away from /setup, justLeftSetup becomes true, and skipUntilNextLaunch fires — emitting onboarding_skipped_until_next_launch for every user who finishes onboarding normally. Every onboarding completion in production will produce a spurious skip event alongside the intended onboarding_finished event, making the skip metric immediately unreliable. layout.tsx — the useLayoutEffect condition in V2OnboardingGateInner needs a !requiredComplete guard; all other files look clean.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/layout.tsx | Adds V2OnboardingGate / V2OnboardingGateInner — the nav-away detection fires skipUntilNextLaunch on normal completions, producing a spurious analytics event for every user who finishes the project step. |
| apps/desktop/src/renderer/stores/onboarding/onboardingStore.ts | Removes adopt-worktrees step, adds dismissedForever/skippedThisLaunch fields, implements skipUntilNextLaunch/dismissForever actions, partializes persistence correctly (skippedThisLaunch omitted). |
| apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | Project step now owns the terminal onboarding_finished telemetry event and navigates to /v2-workspaces on all outcomes; adopt-worktrees navigation removed cleanly. |
| apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx | Adds Skip dropdown (Skip for now / Don't show again) guarded by isV2; removes Worktrees pill from STEP_LABELS; clean implementation. |
| apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | Deleted — entire adopt-worktrees onboarding step removed as intended. |
| apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.test.ts | Deleted along with the selection utility it tested — correct cleanup. |
| apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.ts | Deleted — utility for the removed adopt-worktrees step, no longer needed. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Authenticated render] --> B{isV2CloudEnabled?}
B -- No --> C[null: no subscriptions]
B -- Yes --> D{dismissedForever?}
D -- Yes --> E[Gate suppressed: null]
D -- No --> F{skippedThisLaunch?}
F -- Yes --> E
F -- No --> G{no v2Workspaces AND requiredComplete=false AND not on setup?}
G -- No --> E
G -- Yes --> H[Navigate to firstIncompleteStep]
H --> I[setup/providers to setup/project]
I --> J{User action on project step}
J -- Continue or Select Repo --> K[markComplete + onboarding_finished navigate to v2-workspaces]
J -- Skip step button --> L[markSkipped + onboarding_finished navigate to v2-workspaces]
J -- Skip dropdown Skip for now --> M[skipUntilNextLaunch navigate to workspace]
J -- Skip dropdown Dont show again --> N[dismissForever navigate to workspace]
J -- Sidebar or cmd-K nav-away --> O[useLayoutEffect detects justLeftSetup=true: skipUntilNextLaunch]
K --> E
L --> E
M --> E
N --> E
O --> E
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
apps/desktop/src/renderer/routes/_authenticated/layout.tsx:83-88
**Nav-away fires spurious `onboarding_skipped_until_next_launch` on normal completions**
The `justLeftSetup` path does not distinguish between a user navigating away mid-flow and a user who completes the project step via "Continue with current" / "Skip for now" (the step button). In both cases the user leaves `/setup`, so `justLeftSetup = true` and `skipUntilNextLaunch()` is called — even though `requiredComplete` is already `true`. Every successful onboarding therefore emits an extra `onboarding_skipped_until_next_launch` analytics event alongside `onboarding_finished`, which will inflate that counter for every user who completes the flow.
```suggestion
useLayoutEffect(() => {
if (
justLeftSetup &&
!skippedThisLaunch &&
!dismissedForever &&
!requiredComplete
) {
skipUntilNextLaunch();
}
wasOnSetupRef.current = isOnSetup;
}, [
isOnSetup,
justLeftSetup,
skippedThisLaunch,
dismissedForever,
skipUntilNextLaunch,
requiredComplete,
]);
```
Reviews (1): Last reviewed commit: "feat(desktop): re-enable v2 onboarding a..." | Re-trigger Greptile
| useLayoutEffect(() => { | ||
| if (justLeftSetup && !skippedThisLaunch && !dismissedForever) { | ||
| skipUntilNextLaunch(); | ||
| } | ||
| wasOnSetupRef.current = isOnSetup; | ||
| }, [ |
There was a problem hiding this comment.
Nav-away fires spurious
onboarding_skipped_until_next_launch on normal completions
The justLeftSetup path does not distinguish between a user navigating away mid-flow and a user who completes the project step via "Continue with current" / "Skip for now" (the step button). In both cases the user leaves /setup, so justLeftSetup = true and skipUntilNextLaunch() is called — even though requiredComplete is already true. Every successful onboarding therefore emits an extra onboarding_skipped_until_next_launch analytics event alongside onboarding_finished, which will inflate that counter for every user who completes the flow.
| useLayoutEffect(() => { | |
| if (justLeftSetup && !skippedThisLaunch && !dismissedForever) { | |
| skipUntilNextLaunch(); | |
| } | |
| wasOnSetupRef.current = isOnSetup; | |
| }, [ | |
| useLayoutEffect(() => { | |
| if ( | |
| justLeftSetup && | |
| !skippedThisLaunch && | |
| !dismissedForever && | |
| !requiredComplete | |
| ) { | |
| skipUntilNextLaunch(); | |
| } | |
| wasOnSetupRef.current = isOnSetup; | |
| }, [ | |
| isOnSetup, | |
| justLeftSetup, | |
| skippedThisLaunch, | |
| dismissedForever, | |
| skipUntilNextLaunch, | |
| requiredComplete, | |
| ]); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/layout.tsx
Line: 83-88
Comment:
**Nav-away fires spurious `onboarding_skipped_until_next_launch` on normal completions**
The `justLeftSetup` path does not distinguish between a user navigating away mid-flow and a user who completes the project step via "Continue with current" / "Skip for now" (the step button). In both cases the user leaves `/setup`, so `justLeftSetup = true` and `skipUntilNextLaunch()` is called — even though `requiredComplete` is already `true`. Every successful onboarding therefore emits an extra `onboarding_skipped_until_next_launch` analytics event alongside `onboarding_finished`, which will inflate that counter for every user who completes the flow.
```suggestion
useLayoutEffect(() => {
if (
justLeftSetup &&
!skippedThisLaunch &&
!dismissedForever &&
!requiredComplete
) {
skipUntilNextLaunch();
}
wasOnSetupRef.current = isOnSetup;
}, [
isOnSetup,
justLeftSetup,
skippedThisLaunch,
dismissedForever,
skipUntilNextLaunch,
requiredComplete,
]);
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/OnboardingProgress/OnboardingProgress.tsx`:
- Line 20: The SKIP_TARGET constant currently points to "/workspace" but this
component's skip handlers are shown only for V2 users, so change SKIP_TARGET
from "/workspace" to "/v2-workspaces" (keep the "as const" typing) and update
any skip/navigation code in this component (references around the handlers using
SKIP_TARGET, e.g., the functions that perform router.push or navigate in the
skip buttons) to use the new value so post-onboarding routing matches the V2
flow.
🪄 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: 0e589533-1cac-4f73-a3b2-83a1b7c29087
📒 Files selected for processing (7)
apps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.test.tsapps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.tsapps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxapps/desktop/src/renderer/stores/onboarding/onboardingStore.ts
💤 Files with no reviewable changes (3)
- apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.ts
- apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx
- apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/utils/selection.test.ts
| } from "renderer/stores/onboarding"; | ||
| import { useSetupChromeStore } from "../StepShell"; | ||
|
|
||
| const SKIP_TARGET = "/workspace" as const; |
There was a problem hiding this comment.
Use the V2 workspace route for V2-only skip actions.
Line 20 sets SKIP_TARGET to /workspace, but these handlers are only shown for V2 users. This can route users to the wrong post-onboarding surface compared with the rest of this flow (/v2-workspaces).
Suggested fix
-const SKIP_TARGET = "/workspace" as const;
+const SKIP_TARGET = "/v2-workspaces" as const;Also applies to: 40-48
🤖 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/OnboardingProgress/OnboardingProgress.tsx`
at line 20, The SKIP_TARGET constant currently points to "/workspace" but this
component's skip handlers are shown only for V2 users, so change SKIP_TARGET
from "/workspace" to "/v2-workspaces" (keep the "as const" typing) and update
any skip/navigation code in this component (references around the handlers using
SKIP_TARGET, e.g., the functions that perform router.push or navigate in the
skip buttons) to use the new value so post-onboarding routing matches the V2
flow.
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx:20">
P1: The new v2 onboarding skip actions navigate to the v1 `/workspace` route. This can send fresh v2 users into the wrong flow; route skips to `/v2-workspaces` to stay consistent with the rest of v2 onboarding.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| } from "renderer/stores/onboarding"; | ||
| import { useSetupChromeStore } from "../StepShell"; | ||
|
|
||
| const SKIP_TARGET = "/workspace" as const; |
There was a problem hiding this comment.
P1: The new v2 onboarding skip actions navigate to the v1 /workspace route. This can send fresh v2 users into the wrong flow; route skips to /v2-workspaces to stay consistent with the rest of v2 onboarding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/setup/components/OnboardingProgress/OnboardingProgress.tsx, line 20:
<comment>The new v2 onboarding skip actions navigate to the v1 `/workspace` route. This can send fresh v2 users into the wrong flow; route skips to `/v2-workspaces` to stay consistent with the rest of v2 onboarding.</comment>
<file context>
@@ -10,22 +17,36 @@ import {
} from "renderer/stores/onboarding";
import { useSetupChromeStore } from "../StepShell";
+const SKIP_TARGET = "/workspace" as const;
+
const STEP_LABELS: Record<OnboardingStep, string> = {
</file context>
| const SKIP_TARGET = "/workspace" as const; | |
| const SKIP_TARGET = "/v2-workspaces" as const; |
Summary
Re-enables the v2 onboarding flow for fresh v2 users (PR #4177 had removed the hard-gate redirect because it was trapping users). New behavior treats the redirect as advisory:
isV2CloudEnabled && v2Workspaces.length === 0 && !requiredCompleteusers land on/setup/<firstIncompleteStep>.skippedThisLaunch— in-memory only (omitted frompartialize), set by Skip for now in the chrome dropdown or by navigating out of/setupvia sidebar / cmd-K. Gate doesn't re-trap them this launch; they see onboarding again next launch.dismissedForever— persisted, set by Don't show again. Cleared only by Settings → Experimental → Restart onboarding (reset()).V2OnboardingGateouter wrapper short-circuits when!isV2, so v1 users never instantiate the inner component (no live-queries onv2Workspaces, no store writes). Skip pill inOnboardingProgressis also wrapped inisV2 &&as defense-in-depth for v1 users who URL-navigate to/setup/*.Also drops the
adopt-worktreesstep from the onboarding flow entirely.projectis now the terminal step and routes to/v2-workspaceson complete/skip, carrying theonboarding_finishedtelemetry the worktree step used to own. The whole/setup/adopt-worktreesroute folder is deleted.Test plan
/setup/providerson first authenticated render/workspace, gate doesn't re-fire this session; restart app → gate fires again/workspace, gate never re-fires; Settings → Experimental → Restart onboarding clears it/setupvia sidebar / cmd-K → implicit Skip for now (same as button), no ping-pong back to/setup/setupif URL-accessed/v2-workspaces,onboarding_finishedevent fires with correct outcomeSummary by cubic
Re-enabled the v2 onboarding as a non-blocking gate and removed the Worktrees step. Fresh v2 users get a one-time redirect per launch to the next incomplete step, with options to skip or dismiss onboarding.
New Features
/setup/<firstIncompleteStep>. Leaving/setupcounts as “Skip for now” for this launch.Refactors
/setup/adopt-worktrees; Project is now the final step. On complete/skip, navigate to/v2-workspacesand emitonboarding_finished.skippedThisLaunch(in-memory) anddismissedForever(persisted).Written for commit 43431a8. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Refactor