fix(desktop): stop auto-opting users into v2 + onboarding#4177
Conversation
Two recurring complaints on top of each other: fresh installs were being flipped to v2 without explicit opt-in, and v2 users were then force-walked through the experimental onboarding flow on first launch. v2 + onboarding aren't ready to be defaults — only users who explicitly opt in via Settings → Experimental should land there. - Delete V2DefaultResolver. Its only job was to auto-set optInV2=true on detected-fresh installs. With it gone, optInV2 stays null until the user toggles it, and useIsV2CloudEnabled returns false. - Remove the /setup/* redirect in _authenticated/layout.tsx so fresh v2 users land on the dashboard like v1 users. - Drop isFreshInstall from the store + the useIsFreshInstall hook (only consumer was the redirect gate). - Drop the projects.hasAny / workspaces.hasAny tRPC procedures (only consumer was V2DefaultResolver). Preserved: the optInV2 toggle and "Restart onboarding" action in Settings → Experimental, the full /setup/* route tree + onboarding store, and any persisted optInV2=true from explicit toggles.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThis PR removes fresh-install detection: it strips persisted ChangesFresh Install Detection Removal
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Greptile SummaryThis PR removes the automatic opt-in to the v2 UI and forced
Confidence Score: 4/5Safe to merge — the auto-opt-in and redirect are cleanly removed across all call-sites, v2 stays on for users who explicitly enabled it, and the onboarding flow itself is untouched. The change is a well-scoped removal: V2DefaultResolver, its two hasAny tRPC procedures, the isFreshInstall store field, and the authenticated-layout redirect are all deleted consistently. The only minor gap is that the Zustand persist store drops isFreshInstall without bumping its version, so the orphaned key lingers in existing users localStorage — harmless at runtime but worth a one-line cleanup. apps/desktop/src/renderer/stores/v2-local-override.ts — the persist config could use a version bump to evict the stale isFreshInstall key from localStorage.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/stores/v2-local-override.ts | Removes isFreshInstall field and its setter from the Zustand persist store; no version bump or migration added for existing persisted state. |
| apps/desktop/src/renderer/routes/_authenticated/layout.tsx | Removes the isFreshInstall/onboarding redirect block and associated imports cleanly; authenticated layout logic is otherwise unchanged. |
| apps/desktop/src/renderer/routes/-layout.tsx | Removes V2DefaultResolver from the root layout; straightforward import and JSX cleanup. |
| apps/desktop/src/renderer/components/V2DefaultResolver/V2DefaultResolver.tsx | File deleted — the auto-detect-and-flip component is entirely removed. |
| apps/desktop/src/lib/trpc/routers/projects/projects.ts | Removes the hasAny tRPC procedure that was only consumed by the now-deleted V2DefaultResolver. |
| apps/desktop/src/lib/trpc/routers/workspaces/procedures/query.ts | Removes the hasAny tRPC procedure that was only consumed by the now-deleted V2DefaultResolver. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[App launch] --> B{optInV2 in localStorage?}
B -- "null - fresh install" --> C[v1 dashboard]
B -- "true - user enabled" --> D[v2 dashboard]
D --> E{Restart onboarding in Settings}
E --> F[onboardingStore.reset]
F --> G[Navigate to /setup/providers]
C --> H{User toggles v2 on in Settings}
H --> I[setOptInV2 true to localStorage]
I --> D
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/stores/v2-local-override.ts:11-17
The `isFreshInstall` field is being removed from the persisted store, but there is no version bump or `migrate` function to clean the orphaned key out of existing users' localStorage. Zustand's `persist` middleware will simply ignore the unknown `isFreshInstall` key on rehydration, so behaviour is correct — but the stale value will accumulate in localStorage indefinitely for anyone who ran the old build. A version bump with a trivial migration would clean it up.
```suggestion
persist(
(set) => ({
optInV2: null,
setOptInV2: (optInV2) => set({ optInV2 }),
}),
{
name: "v2-local-override-v2",
version: 1,
migrate: (persisted) => {
const state = persisted as Record<string, unknown>;
const { isFreshInstall: _dropped, ...rest } = state;
return rest;
},
},
),
```
Reviews (1): Last reviewed commit: "fix(desktop): stop auto-opting users int..." | Re-trigger Greptile
| persist( | ||
| (set) => ({ | ||
| optInV2: null, | ||
| isFreshInstall: null, | ||
| setOptInV2: (optInV2) => set({ optInV2 }), | ||
| setIsFreshInstall: (isFreshInstall) => set({ isFreshInstall }), | ||
| }), | ||
| { name: "v2-local-override-v2" }, | ||
| ), |
There was a problem hiding this comment.
The
isFreshInstall field is being removed from the persisted store, but there is no version bump or migrate function to clean the orphaned key out of existing users' localStorage. Zustand's persist middleware will simply ignore the unknown isFreshInstall key on rehydration, so behaviour is correct — but the stale value will accumulate in localStorage indefinitely for anyone who ran the old build. A version bump with a trivial migration would clean it up.
| persist( | |
| (set) => ({ | |
| optInV2: null, | |
| isFreshInstall: null, | |
| setOptInV2: (optInV2) => set({ optInV2 }), | |
| setIsFreshInstall: (isFreshInstall) => set({ isFreshInstall }), | |
| }), | |
| { name: "v2-local-override-v2" }, | |
| ), | |
| persist( | |
| (set) => ({ | |
| optInV2: null, | |
| setOptInV2: (optInV2) => set({ optInV2 }), | |
| }), | |
| { | |
| name: "v2-local-override-v2", | |
| version: 1, | |
| migrate: (persisted) => { | |
| const state = persisted as Record<string, unknown>; | |
| const { isFreshInstall: _dropped, ...rest } = state; | |
| return rest; | |
| }, | |
| }, | |
| ), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/stores/v2-local-override.ts
Line: 11-17
Comment:
The `isFreshInstall` field is being removed from the persisted store, but there is no version bump or `migrate` function to clean the orphaned key out of existing users' localStorage. Zustand's `persist` middleware will simply ignore the unknown `isFreshInstall` key on rehydration, so behaviour is correct — but the stale value will accumulate in localStorage indefinitely for anyone who ran the old build. A version bump with a trivial migration would clean it up.
```suggestion
persist(
(set) => ({
optInV2: null,
setOptInV2: (optInV2) => set({ optInV2 }),
}),
{
name: "v2-local-override-v2",
version: 1,
migrate: (persisted) => {
const state = persisted as Record<string, unknown>;
const { isFreshInstall: _dropped, ...rest } = state;
return rest;
},
},
),
```
How can I resolve this? If you propose a fix, please make it concise.…opt-ins V2DefaultResolver wrote optInV2=true to v2-local-override-v2 for every fresh-detected install in the ~24h window since #4176 shipped. Those writes are indistinguishable from explicit user toggles in the store, so without bumping the key those users would silently stay on v2 even though they never made a real choice. Bumping to v2-local-override-v3 clears the field for everyone: - existing v1 users: no change (their key was already null/absent) - recently auto-flipped users: land back on v1, can opt in if they want - explicit opt-ins: have to toggle once more Acceptable trade-off vs leaving a small group force-stuck on v2.
…le auto-opt-ins" This reverts 3ca8703. The bump also clears optInV2 for users who explicitly toggled v2 on and went through onboarding — they'd silently land back on v1 with no projects visible (v2 workspaces live in cloud, not v1 local), looking broken. Worse trade-off than leaving the small ~24h auto-flip cohort stuck on v2.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
…h#4177) * fix(desktop): stop auto-opting users into v2 + onboarding Two recurring complaints on top of each other: fresh installs were being flipped to v2 without explicit opt-in, and v2 users were then force-walked through the experimental onboarding flow on first launch. v2 + onboarding aren't ready to be defaults — only users who explicitly opt in via Settings → Experimental should land there. - Delete V2DefaultResolver. Its only job was to auto-set optInV2=true on detected-fresh installs. With it gone, optInV2 stays null until the user toggles it, and useIsV2CloudEnabled returns false. - Remove the /setup/* redirect in _authenticated/layout.tsx so fresh v2 users land on the dashboard like v1 users. - Drop isFreshInstall from the store + the useIsFreshInstall hook (only consumer was the redirect gate). - Drop the projects.hasAny / workspaces.hasAny tRPC procedures (only consumer was V2DefaultResolver). Preserved: the optInV2 toggle and "Restart onboarding" action in Settings → Experimental, the full /setup/* route tree + onboarding store, and any persisted optInV2=true from explicit toggles. * fix(desktop): bump v2-local-override persist key to clear stale auto-opt-ins V2DefaultResolver wrote optInV2=true to v2-local-override-v2 for every fresh-detected install in the ~24h window since superset-sh#4176 shipped. Those writes are indistinguishable from explicit user toggles in the store, so without bumping the key those users would silently stay on v2 even though they never made a real choice. Bumping to v2-local-override-v3 clears the field for everyone: - existing v1 users: no change (their key was already null/absent) - recently auto-flipped users: land back on v1, can opt in if they want - explicit opt-ins: have to toggle once more Acceptable trade-off vs leaving a small group force-stuck on v2. * Revert "fix(desktop): bump v2-local-override persist key to clear stale auto-opt-ins" This reverts 3ca8703. The bump also clears optInV2 for users who explicitly toggled v2 on and went through onboarding — they'd silently land back on v1 with no projects visible (v2 workspaces live in cloud, not v1 local), looking broken. Worse trade-off than leaving the small ~24h auto-flip cohort stuck on v2.
Summary
optInV2: trueand then force-walked through the experimental/setup/*onboarding flow on first launch. Both behaviors are removed — v2 and onboarding are explicit-opt-in only via Settings → Experimental.V2DefaultResolver,useIsFreshInstall, theisFreshInstallstore field, the redirect block in_authenticated/layout.tsx, and theprojects.hasAny/workspaces.hasAnytRPC procedures (all unused once the auto-flip is gone)./setup/*route tree, and any persistedoptInV2: truefrom users who explicitly toggled.Test plan
v2-local-override-v2localStorage): launches into v1 dashboard, no/setup/*redirect./setup/providers→ ... when v2 is on.Summary by cubic
Stops auto-opting fresh installs into v2 and removes the forced
/setup/*onboarding. v2 and onboarding are now explicit opt-in via Settings → Experimental; all users land on the dashboard.Bug Fixes
/setup/*and the fresh-install auto-set of v2.optInV2values inv2-local-override-v2to avoid resetting explicit opt-ins; recently auto-flipped installs will remain on v2 unless toggled off.Refactors
V2DefaultResolver,useIsFreshInstall, and theisFreshInstallfield from thev2-local-overridestore.projects.hasAnyandworkspaces.hasAny.Written for commit fb55ffc. Summary will update on new commits.
Summary by CodeRabbit
Refactor
Bug Fix