mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(onboarding): resolve overlay race condition between RPC and Redux state (#284)
OnboardingOverlay could reappear for already-onboarded users when the core config RPC call failed (sidecar not ready, timeout). The catch block hardcoded `false`, ignoring the persisted Redux `isOnboardedByUser` flag. Now reads `selectIsOnboarded` as a fallback in the catch block and combines both flags in shouldShow — either being true prevents the overlay. Also fixes DEV_FORCE_ONBOARDING which was a no-op (identical ternary branches). Closes #197
This commit is contained in:
@@ -50,6 +50,9 @@ Quick reference for anyone starting with Claude on this project. Updated by the
|
||||
- **Recovery Phrase moved to Settings** — MnemonicStep was removed from onboarding (was step 5). The same BIP39 generate/import functionality now lives in `app/src/components/settings/panels/RecoveryPhrasePanel.tsx`, accessible via Settings > Recovery Phrase. Onboarding completion logic moved into `handleSkillsNext` in `Onboarding.tsx`.
|
||||
- **E2E tests find onboarding buttons by label text** — `shared-flows.ts`, `login-flow.spec.ts`, `auth-access-control.spec.ts`, and `voice-mode.spec.ts` locate buttons by their visible label. Changing button labels requires updating all four files. Note: `voice-mode.spec.ts` still references legacy labels that don't match current steps (pre-existing tech debt).
|
||||
- **`ScreenPermissionsStep` always shows Continue** — The Continue button is always visible regardless of permission grant status, allowing users to skip the permissions step (#274).
|
||||
- **OnboardingOverlay RPC/Redux race condition** — `getOnboardingCompleted()` RPC can fail (sidecar not ready, timeout); the old catch block hardcoded `setOnboardingCompleted(false)`, ignoring the persisted `isOnboardedByUser` Redux flag. Fix: read `selectIsOnboarded` from `authSelectors.ts` in the catch block as fallback, and combine both flags in `shouldShow`: `!onboardingCompleted && !isOnboardedRedux`. Either flag being `true` is sufficient to skip onboarding (#197).
|
||||
- **`DEV_FORCE_ONBOARDING` was a no-op** — The old ternary had identical branches; fixed to actually force-show when the flag is set.
|
||||
- **`isOnboardedRedux` must be in useEffect deps** — When reading a selector value inside a useEffect, add it to the dependency array or the effect won't re-run when Redux state changes.
|
||||
|
||||
## Build Blockers: macOS Tahoe + whisper-rs
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import { useCallback, useEffect, useState } from 'react';
|
||||
import { createPortal } from 'react-dom';
|
||||
|
||||
import Onboarding from '../pages/onboarding/Onboarding';
|
||||
import { selectIsOnboarded } from '../store/authSelectors';
|
||||
import { useAppSelector } from '../store/hooks';
|
||||
import { DEV_FORCE_ONBOARDING } from '../utils/config';
|
||||
import {
|
||||
@@ -19,6 +20,7 @@ const OnboardingOverlay = () => {
|
||||
const token = useAppSelector(state => state.auth.token);
|
||||
const isAuthBootstrapComplete = useAppSelector(state => state.auth.isAuthBootstrapComplete);
|
||||
const user = useAppSelector(state => state.user.user);
|
||||
const isOnboardedRedux = useAppSelector(selectIsOnboarded);
|
||||
|
||||
/** null = still loading, true/false = resolved from core config */
|
||||
const [onboardingCompleted, setOnboardingCompleted] = useState<boolean | null>(null);
|
||||
@@ -54,14 +56,14 @@ const OnboardingOverlay = () => {
|
||||
const completed = await getOnboardingCompleted();
|
||||
if (mounted) setOnboardingCompleted(completed);
|
||||
} catch {
|
||||
if (mounted) setOnboardingCompleted(false);
|
||||
if (mounted) setOnboardingCompleted(isOnboardedRedux);
|
||||
}
|
||||
};
|
||||
void check();
|
||||
return () => {
|
||||
mounted = false;
|
||||
};
|
||||
}, [token, isAuthBootstrapComplete, userReady]);
|
||||
}, [token, isAuthBootstrapComplete, userReady, isOnboardedRedux]);
|
||||
|
||||
const handleDone = useCallback(async () => {
|
||||
setOnboardingCompleted(true);
|
||||
@@ -78,7 +80,7 @@ const OnboardingOverlay = () => {
|
||||
// Still loading the flag from core
|
||||
if (onboardingCompleted === null) return null;
|
||||
|
||||
const shouldShow = DEV_FORCE_ONBOARDING ? !onboardingCompleted : !onboardingCompleted;
|
||||
const shouldShow = DEV_FORCE_ONBOARDING || (!onboardingCompleted && !isOnboardedRedux);
|
||||
|
||||
if (!shouldShow) return null;
|
||||
|
||||
|
||||
@@ -89,4 +89,49 @@ describe('OnboardingOverlay', () => {
|
||||
|
||||
expect(screen.queryByText('Skip')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not render when RPC fails but Redux says onboarded', async () => {
|
||||
const { getOnboardingCompleted } = await import('../../utils/tauriCommands');
|
||||
vi.mocked(getOnboardingCompleted).mockRejectedValue(new Error('RPC error'));
|
||||
|
||||
renderWithProviders(<OnboardingOverlay />, {
|
||||
preloadedState: {
|
||||
auth: { ...baseAuth, isOnboardedByUser: { 'user-1': true } },
|
||||
user: baseUser,
|
||||
},
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(screen.queryByText('Skip')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('does not render when RPC returns false but Redux says onboarded', async () => {
|
||||
const { getOnboardingCompleted } = await import('../../utils/tauriCommands');
|
||||
vi.mocked(getOnboardingCompleted).mockResolvedValue(false);
|
||||
|
||||
renderWithProviders(<OnboardingOverlay />, {
|
||||
preloadedState: {
|
||||
auth: { ...baseAuth, isOnboardedByUser: { 'user-1': true } },
|
||||
user: baseUser,
|
||||
},
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(screen.queryByText('Skip')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
it('renders when both RPC fails and Redux says not onboarded', async () => {
|
||||
const { getOnboardingCompleted } = await import('../../utils/tauriCommands');
|
||||
vi.mocked(getOnboardingCompleted).mockRejectedValue(new Error('RPC error'));
|
||||
|
||||
renderWithProviders(<OnboardingOverlay />, {
|
||||
preloadedState: { auth: { ...baseAuth, isOnboardedByUser: {} }, user: baseUser },
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(screen.queryByText('Skip')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user