fix(onboarding): clear all onboarding state on logout so overlay reappears#167
Merged
fix(onboarding): clear all onboarding state on logout so overlay reappears#167
Conversation
…download snackbar - Add persisted `onboardingDeferredByUser` state so "Set up later" survives across sessions (no more overlay nagging on every launch) - Add SetupBanner for non-intrusive "Finish setting up" reminder with resume path - Convert LocalAIStep to fire-and-forget download, advancing immediately - Add LocalAIDownloadSnackbar (bottom-left, collapsible) for persistent download progress that doesn't block the main chrome - Extract shared helpers (formatBytes, formatEta, progressFromStatus) to localAiHelpers.ts - Add Vitest tests for overlay gating, banner, and snackbar Closes #101
…orts The OnboardingOverlay test mocked utils/config with only DEV_FORCE_ONBOARDING, which shadowed the global mock from setup.ts and dropped IS_DEV — causing store/index.ts to fail on CI.
Two tests called .history() and .turn() on the (Agent, TempDir) tuple instead of destructuring it first, causing compilation errors on CI.
…pears On logout, `isOnboardedByUser` and `isAnalyticsEnabledByUser` were not reset in the `_clearToken` reducer, and the workspace `.skip_onboarding` flag file was never deleted. This caused the onboarding overlay to be permanently skipped after the first completion — even for new users on the same machine. - Clear `isOnboardedByUser` and `isAnalyticsEnabledByUser` in `_clearToken` - Call `openhumanWorkspaceOnboardingFlagSet(false)` on logout, clear-all, and auth recovery paths - Add 3s timeout fallback in OnboardingOverlay for slow user profile loads - Add unit test asserting `clearToken` resets all per-user fields Closes #117
…y-fresh-install # Conflicts: # .claude/memory.md # app/src/components/LocalAIDownloadSnackbar.tsx # app/src/components/OnboardingOverlay.tsx # app/src/pages/onboarding/steps/LocalAIStep.tsx # app/src/utils/localAiHelpers.ts # src/openhuman/agent/tests.rs
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughClears per-user onboarding and analytics state on token clear, removes the workspace onboarding flag in three logout/auth-recovery flows, and adds a 3-second user-load timeout so the onboarding overlay can proceed even if the user profile is slow or absent. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsHome
participant AuthStore
participant TauriFS as WorkspaceFlagAPI
User->>SettingsHome: Click "Logout" / "Clear All App Data"
SettingsHome->>AuthStore: dispatch(clearToken())
SettingsHome->>TauriFS: openhumanWorkspaceOnboardingFlagSet(false)
TauriFS-->>SettingsHome: ack / error
SettingsHome-->>User: redirect to "/"
sequenceDiagram
participant AppStartup
participant UserProvider
participant AuthStore
participant OnboardingOverlay
participant TauriFS as WorkspaceFlagAPI
AppStartup->>UserProvider: bootstrap auth
UserProvider->>AuthStore: provide token/state
OnboardingOverlay->>AuthStore: read token, isAuthBootstrapComplete
OnboardingOverlay->>OnboardingOverlay: start 3s timer (userLoadTimedOut)
OnboardingOverlay->>AuthStore: check user?._id or userLoadTimedOut
OnboardingOverlay->>TauriFS: read workspace flag
TauriFS-->>OnboardingOverlay: hasFlag / noFlag
OnboardingOverlay-->>AppStartup: render onboarding or skip
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)
Comment |
![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: 3
🧹 Nitpick comments (1)
app/src/components/settings/SettingsHome.tsx (1)
22-43: Extract shared logout cleanup to avoid path driftBoth logout paths now duplicate the same workspace-flag clear block. Given this flow must stay synchronized, extracting a shared helper will reduce future divergence risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/settings/SettingsHome.tsx` around lines 22 - 43, Summary: Duplicate cleanup logic in handleLogout and clearAllAppData should be extracted into a shared helper to keep both paths synchronized. Create a new helper function (e.g., performLogoutCleanup or clearTokenAndWorkspaceFlag) that does the dispatch(clearToken()) and the try/catch around openhumanWorkspaceOnboardingFlagSet(false) (and return or surface errors as needed); then replace the duplicated blocks in handleLogout and clearAllAppData to call that helper. Keep handleLogout's additional try/catch around tauriLogout and the final window.location.hash assignment unchanged so only the shared cleanup is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/OnboardingOverlay.tsx`:
- Around line 30-39: The timeout flag userLoadTimedOut in OnboardingOverlay is
set true but never reset, causing future sessions to skip gating; update the
useEffect that creates the timer (and/or add a separate effect) to reset
userLoadTimedOut to false whenever token becomes falsy or when user._id appears
(i.e., on changes to token, isAuthBootstrapComplete, or user?._id) and keep the
existing timer logic that sets it true after 3s; refer to the userLoadTimedOut
state and the existing useEffect that uses setTimeout/clearTimeout to implement
this reset so the flag does not persist across sessions.
In `@app/src/providers/UserProvider.tsx`:
- Around line 58-62: The catch block in the UserProvider recovery path currently
swallows errors from openhumanWorkspaceOnboardingFlagSet(false); update it to
catch the error (e) and emit a namespaced debug/warn log so failures remain
observable—for example call your app's namespaced logger or debug utility (e.g.,
debug("UserProvider:openhumanWorkspaceOnboardingFlagSet", e) or
logger.warn("UserProvider:openhumanWorkspaceOnboardingFlagSet failed", e))
inside the catch; keep it best-effort (do not rethrow) so auth recovery still
proceeds.
In `@app/src/store/authSlice.ts`:
- Around line 59-62: The analytics map reset leaves missing keys undefined which
current readers treat as enabled; update all reads of
state.auth.isAnalyticsEnabledByUser to treat missing entries as false (e.g.,
replace expressions like state.auth.isAnalyticsEnabledByUser[userId] with
state.auth.isAnalyticsEnabledByUser[userId] ?? false or use a selector/getter
that returns false by default), and ensure any selector or helper (e.g.,
getIsAnalyticsEnabled, isAnalyticsEnabledByUser access sites) is updated
accordingly so post-logout missing keys do not imply opt-in.
---
Nitpick comments:
In `@app/src/components/settings/SettingsHome.tsx`:
- Around line 22-43: Summary: Duplicate cleanup logic in handleLogout and
clearAllAppData should be extracted into a shared helper to keep both paths
synchronized. Create a new helper function (e.g., performLogoutCleanup or
clearTokenAndWorkspaceFlag) that does the dispatch(clearToken()) and the
try/catch around openhumanWorkspaceOnboardingFlagSet(false) (and return or
surface errors as needed); then replace the duplicated blocks in handleLogout
and clearAllAppData to call that helper. Keep handleLogout's additional
try/catch around tauriLogout and the final window.location.hash assignment
unchanged so only the shared cleanup is centralized.
🪄 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: 3f811a15-1e46-4c5c-bd34-258d290690e6
📒 Files selected for processing (6)
.claude/memory.mdapp/src/components/OnboardingOverlay.tsxapp/src/components/settings/SettingsHome.tsxapp/src/providers/UserProvider.tsxapp/src/store/__tests__/authSlice.test.tsapp/src/store/authSlice.ts
- Add console.warn in UserProvider catch block for workspace flag clear - Add inline comment explaining userLoadTimedOut stickiness is harmless Closes #117
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
isOnboardedByUserandisAnalyticsEnabledByUserin the_clearTokenreducer so persisted onboarding state doesn't survive logoutopenhumanWorkspaceOnboardingFlagSet(false)on all three logout paths (Settings logout, Clear All Data, UserProvider auth recovery) to delete the workspace.skip_onboardingflag fileOnboardingOverlayso the overlay still renders if user profile fetch is slowclearTokenresets all per-user fields to{}Test plan
yarn typecheck,yarn lint,yarn format:check,yarn buildall passyarn tauri devlaunches and core connectsCloses #117
Summary by CodeRabbit
Bug Fixes
Tests
Documentation