fix(onboarding): keep skip completion moving after task sync failure

[aqilaziz]

Summary

• Allows onboarding completion to continue when persisting onboarding task metadata fails.
• Adds a focused regression test for the openhuman.app_state_snapshot timed out after 30000ms failure path.
• Keeps the existing completion flag write as the required source of truth for finishing onboarding.

Problem

• In issue ONBOARDING - Core RPC openhuman.app_state_snapshot timed out after 30000ms - when google auth is rejected. #1728, rejecting Google auth could leave users stuck when skipping onboarding.
• setOnboardingTasks persists onboarding metadata and refreshes core local state; that refresh can call openhuman.app_state_snapshot and time out.
• When that best-effort metadata write rejected, completeAndExit stopped before setting the onboarding completed flag or navigating home.

Solution

• Wrap the onboarding task metadata persistence in a try/catch and log the failure.
• Continue through backend notification, setOnboardingCompletedFlag(true), walkthrough state, and navigation.
• Add a regression test that rejects setOnboardingTasks with the reported timeout and asserts onboarding still completes.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage >= 80% - focused regression covers the changed failure branch; CI coverage gate remains authoritative.
• Coverage matrix updated - N/A: behavior-only onboarding completion resilience; no feature row added/removed/renamed.
• All affected feature IDs from the matrix are listed in the PR description under ## Related - N/A: no matrix feature IDs changed.
• No new external network dependencies introduced (mock backend used per Testing Strategy)
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) - N/A: no release-cut smoke checklist surface changed.
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Frontend onboarding only.
• No API, schema, migration, dependency, Rust, or Tauri behavior changes.
• User-visible effect: Skip/Continue can reach Home even if the onboarding task metadata refresh times out.

Related

• Closes: ONBOARDING - Core RPC openhuman.app_state_snapshot timed out after 30000ms - when google auth is rejected. #1728
• Follow-up PR(s)/TODOs: N/A

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: codex/1728-onboarding-skip-timeout-fork
• Commit SHA: ca2f3cb

Validation Run

• pnpm --filter openhuman-app format:check - blocked locally on fork-main baseline drift; see Validation Blocked. Touched files pass targeted Prettier check.
• pnpm typecheck - passed.
• Focused tests: pnpm --dir app exec vitest run src/pages/onboarding/__tests__/OnboardingLayout.test.tsx --config test/vitest.config.ts - passed, 7 tests.
• Rust fmt/check (if changed): N/A: no Rust files changed.
• Tauri fmt/check (if changed): N/A: no Tauri files changed.

Validation Blocked

• command: pnpm format:check
• error: Local fork baseline (openhuman-app@0.53.43) reports pre-existing Prettier drift in 26 unrelated files, for example package.json, src/pages/Conversations.tsx, and test/wdio.conf.ts. The two touched onboarding files pass targeted Prettier check.
• impact: Full local format check cannot be used on this fork baseline, but the patch files are formatted and GitHub CI should evaluate the merge result against current upstream main.

Behavior Changes

• Intended behavior change: onboarding task metadata persistence is best-effort during completion.
• User-visible effect: users are no longer blocked from reaching Home by an openhuman.app_state_snapshot timeout while skipping/completing onboarding.

Parity Contract

• Legacy behavior preserved: the app still attempts to persist onboarding task metadata before completing.
• Guard/fallback/dispatch parity checks: setOnboardingCompletedFlag(true) remains required and still rejects on failure; only the metadata sync failure is swallowed.

Duplicate / Superseded PR Handling

• Duplicate PR(s): None found for #1728, app_state_snapshot, or onboarding skip timeout. Broad open-PR search returned unrelated Add agent task orchestration #1768, feat: add Chinese (简体中文) i18n support #1518, and Fix core-state poll logs and rewards timeout UX #1321.
• Canonical PR: This PR.
• Resolution (closed/superseded/updated): N/A.

Summary by CodeRabbit

• Bug Fixes

  • Enhanced onboarding flow to handle persistence failures gracefully. If task data fails to save, users can still complete onboarding and proceed instead of getting stuck.

• Tests

  • Added test coverage verifying onboarding completes successfully when task persistence fails.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds error resilience to onboarding completion by wrapping task persistence in a try/catch block within completeAndExit. When persisting tasks fails, the function logs a warning and continues the exit flow instead of propagating the error. A new test verifies this graceful degradation works correctly.

Changes

Onboarding Task Persistence Error Handling

Layer / File(s)	Summary
Error-resilient task persistence in onboarding completion app/src/pages/onboarding/OnboardingLayout.tsx, app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx	completeAndExit wraps await setOnboardingTasks(...) in try/catch, logging warnings on failure but continuing execution. A new test verifies onboarding still completes and navigates to /home when task persistence fails.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A graceful hop through error's way,
When tasks won't save, we'll exit anyway,
Try-catch whispers, "proceed with care,"
The test confirms completion's there! ✨

🚥 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 describes the main change: adding error handling to allow onboarding completion to proceed when task persistence fails.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx (1)

232-247: 💤 Low value

Consider asserting the warning was logged.

The test correctly verifies that onboarding completes despite task persistence failure, but it doesn't assert that console.warn was actually called. Adding this assertion would provide complete coverage of the catch block's behavior.

✅ Optional assertion to add

    expect(mockSetOnboardingCompletedFlag).toHaveBeenCalledWith(true);
    expect(screen.getByTestId('home-page')).toBeInTheDocument();
+   expect(warnSpy).toHaveBeenCalledWith(
+     '[onboarding] Failed to persist onboarding tasks; continuing completion',
+     expect.any(Error)
+   );

    warnSpy.mockRestore();

🤖 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 `@app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx` around lines
232 - 247, The test "still completes onboarding when persisting onboarding tasks
fails" sets up a console.warn spy (warnSpy) but never asserts it was called;
update the test to assert that the warnSpy was invoked after clicking the
'complete-btn' (e.g., expect(warnSpy).toHaveBeenCalled() or toHaveBeenCalledWith
a message containing the rejection/error), keeping the existing
mockSetOnboardingTasks rejection and other assertions intact so the catch path
in the onboarding completion flow is fully covered.

🤖 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.

Nitpick comments:
In `@app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx`:
- Around line 232-247: The test "still completes onboarding when persisting
onboarding tasks fails" sets up a console.warn spy (warnSpy) but never asserts
it was called; update the test to assert that the warnSpy was invoked after
clicking the 'complete-btn' (e.g., expect(warnSpy).toHaveBeenCalled() or
toHaveBeenCalledWith a message containing the rejection/error), keeping the
existing mockSetOnboardingTasks rejection and other assertions intact so the
catch path in the onboarding completion flow is fully covered.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: da704e4f-6c51-4d35-aa07-3a93b6e88ee3

📥 Commits

Reviewing files that changed from the base of the PR and between 3165306 and ca2f3cb.

📒 Files selected for processing (2)

• app/src/pages/onboarding/OnboardingLayout.tsx
• app/src/pages/onboarding/__tests__/OnboardingLayout.test.tsx