Skip to content

fix(onboarding): resolve overlay race condition between RPC and Redux state#284

Merged
graycyrus merged 1 commit into
mainfrom
fix/onboarding-overlay-race-condition
Apr 3, 2026
Merged

fix(onboarding): resolve overlay race condition between RPC and Redux state#284
graycyrus merged 1 commit into
mainfrom
fix/onboarding-overlay-race-condition

Conversation

@graycyrus
Copy link
Copy Markdown
Contributor

@graycyrus graycyrus commented Apr 3, 2026

Summary

Fixes the OnboardingOverlay race condition where the core config RPC call (getOnboardingCompleted()) could fail (sidecar not ready, timeout, permission error), causing the overlay to reappear for already-onboarded users.

Root cause

The catch block in the RPC effect hardcoded setOnboardingCompleted(false), ignoring the Redux-persisted isOnboardedByUser flag that may already be true.

Changes

  • OnboardingOverlay.tsx: Import and read selectIsOnboarded from Redux. Use it as fallback in the RPC error catch block. Combine both flags in shouldShow so either being true prevents the overlay. Also fixes DEV_FORCE_ONBOARDING which was a no-op (identical ternary branches).
  • OnboardingOverlay.test.tsx: Add 3 new test cases covering RPC failure + Redux onboarded, RPC false + Redux onboarded, and RPC failure + Redux not onboarded (regression guard).

Behavioral impact

Scenario Before After
RPC succeeds, returns true Hidden Hidden
RPC succeeds, returns false, Redux false Shows Shows
RPC succeeds, returns false, Redux true Shows (BUG) Hidden (FIXED)
RPC fails, Redux true Shows (BUG) Hidden (FIXED)
RPC fails, Redux false Shows (correct) Shows (correct)
DEV_FORCE_ONBOARDING set No-op branch Actually forces show

Test plan

  • All 7 OnboardingOverlay unit tests pass (4 existing + 3 new)
  • yarn typecheck — pass
  • yarn lint — pass (0 errors)
  • yarn format:check — pass
  • yarn build — pass

Closes #197

Summary by CodeRabbit

  • Bug Fixes

    • Fixed onboarding state recovery: when fetching onboarding status fails, the system now preserves persisted user state instead of resetting it.
    • Fixed development-mode onboarding display behavior to work as intended.
  • Tests

    • Added comprehensive test coverage for onboarding fallback scenarios.

… state

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
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 676d2d01-f3ed-499a-b283-3b2ef7ce8a9d

📥 Commits

Reviewing files that changed from the base of the PR and between 04146bf and fc3f259.

📒 Files selected for processing (3)
  • .claude/memory.md
  • app/src/components/OnboardingOverlay.tsx
  • app/src/components/__tests__/OnboardingOverlay.test.tsx

📝 Walkthrough

Walkthrough

This PR fixes a race condition in OnboardingOverlay where file-system and Redux onboarding state disagreed, causing the overlay to incorrectly reappear. The fix treats either flag as sufficient to skip onboarding, with RPC-error fallback logic and updated effect dependencies.

Changes

Cohort / File(s) Summary
Documentation
.claude/memory.md
Documented the race condition fix, Redux fallback logic, and React dependency array correction for isOnboardedRedux.
Component Logic
app/src/components/OnboardingOverlay.tsx
Added selectIsOnboarded Redux selector, changed RPC error handling to fall back to Redux state, updated effect dependencies, and fixed overlay visibility to require both onboarding flags as false (or force via dev flag).
Test Coverage
app/src/components/__tests__/OnboardingOverlay.test.tsx
Added three new test cases covering RPC failure paths with Redux state coordination: RPC error with Redux onboarded, RPC false with Redux onboarded, and RPC error with Redux not onboarded.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A race betwixt two flags once ran,
Redux and file, at odds they fanned.
Now either true shall seal the fate,
No phantom onboarding—hip-hip-great!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing a race condition between RPC and Redux state in the OnboardingOverlay component.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #197: evaluates both RPC and Redux flags together, treats either as sufficient to skip onboarding, handles RPC error paths by falling back to Redux state instead of forcing false, and adds comprehensive regression tests.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the onboarding overlay race condition: component logic updates, memory documentation, and test coverage for the specific issue with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/onboarding-overlay-race-condition

Comment @coderabbitai help to get the list of available commands and usage tips.

@graycyrus graycyrus merged commit de41bf8 into main Apr 3, 2026
14 checks passed
@senamakel senamakel deleted the fix/onboarding-overlay-race-condition branch April 4, 2026 21:55
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
… state (tinyhumansai#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 tinyhumansai#197
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] OnboardingOverlay workspace flag race condition with Redux state

1 participant