Skip to content

Feat: update complete onboarding helper#299

Merged
graycyrus merged 3 commits into
tinyhumansai:mainfrom
YellowSnnowmann:feat/201-update-complete-onboarding-helper
Apr 3, 2026
Merged

Feat: update complete onboarding helper#299
graycyrus merged 3 commits into
tinyhumansai:mainfrom
YellowSnnowmann:feat/201-update-complete-onboarding-helper

Conversation

@YellowSnnowmann
Copy link
Copy Markdown
Contributor

@YellowSnnowmann YellowSnnowmann commented Apr 3, 2026

Summary

  • completeOnboardingIfVisible now delegates unconditionally to walkOnboarding instead of gating on a pre-check that could fire before the overlay rendered.
  • walkOnboarding already polls up to 8 × 400 ms for the overlay, so the extra guard was redundant and introduced a race window.
  • conversations-web-channel-flow spec updated to use completeOnboardingIfVisible (matching the rest of the suite and removing the direct walkOnboarding call).
  • e2e/Dockerfile gains clang libclang-dev cmake, required by whisper-rs-sys (bindgen + CMake build of whisper.cpp) on the Linux E2E runner.

Problem

  • completeOnboardingIfVisible called onboardingOverlayLikelyVisible() synchronously before the overlay had time to render after auth. If the check ran too early it returned false and the helper silently skipped onboarding, leaving the app stuck behind the overlay for the rest of the spec.
  • The conversations spec called walkOnboarding directly rather than the shared helper, diverging from the pattern the rest of the suite uses.
  • The Linux E2E Docker image was missing clang, libclang-dev, and cmake, causing whisper-rs-sys to fail at the bindgen and CMake stages during test:e2e:build.

Solution

  • Removed the if (await onboardingOverlayLikelyVisible()) guard from completeOnboardingIfVisible; the function now calls walkOnboarding directly.
  • walkOnboarding's internal polling loop handles the timing safely — it waits for the overlay and exits cleanly when it never appears (already on Home), so no extra guard is needed.
  • Updated the conversations spec import and call site to use completeOnboardingIfVisible.
  • Added clang libclang-dev cmake to the apt-get install block in e2e/Dockerfile so the image can build whisper-rs-sys cleanly on both x86-64 and aarch64 Linux.

Submission Checklist

  • Unit tests — N/A; change is E2E harness only, no unit-testable logic added.
  • E2E / integration — Directly modifies E2E helpers and specs; the fix is verified by the specs that were previously flaking due to the race.
  • Doc comments — Updated JSDoc on completeOnboardingIfVisible to describe delegation and polling behaviour.
  • Inline comments — Onboarding section comment updated to document the current 5-step flow (Welcome → Local AI → Screen & Accessibility → Tools → Skills).

Impact

  • Desktop E2E only. No runtime, sidecar, or product code changed.
  • Removes a flake class from all specs that call completeOnboardingIfVisible after auth.
  • Docker image rebuild required to pick up the new system dependencies (clang libclang-dev cmake).

Related

Summary by CodeRabbit

Release Notes

This release contains internal testing and infrastructure improvements with no user-facing changes.

  • Tests

    • Updated end-to-end test helpers and specifications for onboarding flow execution.
  • Chores

    • Enhanced E2E Docker build environment with additional system dependencies.

YellowSnnowmann and others added 3 commits April 3, 2026 13:28
… walkOnboarding

The previous implementation did a single onboardingOverlayLikelyVisible()
check before calling walkOnboarding. This created a timing race: if onboarding
had not yet rendered at call time the helper returned early, leaving the
overlay blocking subsequent steps.

walkOnboarding already handles both cases correctly — it polls up to
8 × 400 ms for the overlay before giving up, then no-ops gracefully if not
visible. completeOnboardingIfVisible now delegates to it unconditionally,
eliminating the race without changing observable behaviour.

Also updates the section comment to accurately describe the current 5-step
onboarding sequence (Welcome → Local AI → Screen & Accessibility → Tools →
Skills) following the removal of MnemonicStep in tinyhumansai#279.

Closes part of tinyhumansai#201

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
conversations-web-channel-flow.spec.ts was the only spec that called
walkOnboarding directly instead of the shared completeOnboardingIfVisible
helper. Align it with all other specs so the single resilient code path
is used consistently.

Closes tinyhumansai#201

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updated the e2e Dockerfile to include clang and cmake as additional dependencies, enhancing the build environment for end-to-end testing.
@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: 2b496447-9ee7-4d07-9693-0568ce3d61db

📥 Commits

Reviewing files that changed from the base of the PR and between b7b3d62 and 13637d9.

📒 Files selected for processing (3)
  • app/test/e2e/helpers/shared-flows.ts
  • app/test/e2e/specs/conversations-web-channel-flow.spec.ts
  • e2e/Dockerfile

📝 Walkthrough

Walkthrough

Modified the E2E onboarding helper to unconditionally delegate to walkOnboarding instead of pre-checking visibility, updated test specs to use the revised helper, and added system build dependencies (clang, libclang-dev, cmake) to the E2E Docker image.

Changes

Cohort / File(s) Summary
E2E Onboarding Flow Helpers and Tests
app/test/e2e/helpers/shared-flows.ts, app/test/e2e/specs/conversations-web-channel-flow.spec.ts
Simplified completeOnboardingIfVisible to always delegate to walkOnboarding (which internally polls for overlay visibility); removed pre-check conditional. Updated test spec to call completeOnboardingIfVisible instead of walkOnboarding directly.
Docker Build Configuration
e2e/Dockerfile
Added system packages clang, libclang-dev, and cmake to apt-get install layer for E2E environment.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • senamakel

Poem

🐰 A helper now trusts its delegate,
No pre-checks, just graceful elate!
One function calls another with care,
While Docker prepares the build's layer,
Simpler flows—oh, what a state! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Feat: update complete onboarding helper' directly corresponds to the primary change: refactoring the completeOnboardingIfVisible helper function to remove synchronous pre-checks and always delegate to walkOnboarding.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@graycyrus graycyrus merged commit 898fe13 into tinyhumansai:main Apr 3, 2026
9 checks passed
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
* fix(e2e): make completeOnboardingIfVisible resilient by delegating to walkOnboarding

The previous implementation did a single onboardingOverlayLikelyVisible()
check before calling walkOnboarding. This created a timing race: if onboarding
had not yet rendered at call time the helper returned early, leaving the
overlay blocking subsequent steps.

walkOnboarding already handles both cases correctly — it polls up to
8 × 400 ms for the overlay before giving up, then no-ops gracefully if not
visible. completeOnboardingIfVisible now delegates to it unconditionally,
eliminating the race without changing observable behaviour.

Also updates the section comment to accurately describe the current 5-step
onboarding sequence (Welcome → Local AI → Screen & Accessibility → Tools →
Skills) following the removal of MnemonicStep in tinyhumansai#279.

Closes part of tinyhumansai#201

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(e2e): use completeOnboardingIfVisible in conversations spec

conversations-web-channel-flow.spec.ts was the only spec that called
walkOnboarding directly instead of the shared completeOnboardingIfVisible
helper. Align it with all other specs so the single resilient code path
is used consistently.

Closes tinyhumansai#201

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(e2e): add clang and cmake to Dockerfile dependencies

Updated the e2e Dockerfile to include clang and cmake as additional dependencies, enhancing the build environment for end-to-end testing.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

2 participants