Show onboarding before public backend auth gate#1385
Open
neubig wants to merge 13 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
PR Artifacts Cleaned Up The |
Contributor
❌ Mock-LLM E2E Tests55/60 passed · 1 failed · 4 skipped Commit:
🔍 Failure details (1)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
❌ Mock-LLM Docker E2E Test Results49/60 passed · 3 failed · 8 skipped Commit:
🔍 Failure details (3)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured❌ settings/mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile❌ skills/mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keywordPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
❌ Mock-LLM E2E Tests55/60 passed · 1 failed · 4 skipped Commit:
🔍 Failure details (1)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
🛑 Mock-LLM Docker E2E Test Results32/42 passed · 2 failed · 8 skipped · Commit:
🔍 Failure details (2)❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
❌ Mock-LLM E2E Tests55/60 passed · 1 failed · 4 skipped Commit:
🔍 Failure details (1)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
🛑 Mock-LLM Docker E2E Test Results21/31 passed · 2 failed · 8 skipped · Commit:
🔍 Failure details (2)❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
❌ Mock-LLM E2E Tests55/60 passed · 1 failed · 4 skipped Commit:
🔍 Failure details (1)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configuredPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
1 task
Contributor
❌ Mock-LLM Docker E2E Test Results49/60 passed · 3 failed · 8 skipped Commit:
🔍 Failure details (3)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › shows the auth screen when no key is configured❌ settings/mock-llm-profile-management.spec.ts › active profile deletion + reconciliation › active profile is deletable and reconciliation activates another profile❌ skills/mock-llm-skills.spec.ts › skill loading: project, user, and deletion › project skill in workspace/.agents/skills/ triggers on matching keywordPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
❌ Mock-LLM E2E Tests56/60 passed · 1 failed · 3 skipped Commit:
🔍 Failure details (1)❌ backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline errorPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
🛑 Mock-LLM Docker E2E Test Results50/58 passed · 2 failed · 6 skipped · Commit:
🔍 Failure details (2)❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline error❌ chromium › backends/mock-llm-auth-modes.spec.ts › auth mode: public gate › rejects an incorrect key with an inline errorPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
neubig
force-pushed
the
fix-public-onboarding
branch
from
e81bcd6 to
012625d
Compare
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
🛑 Mock-LLM Docker E2E Test Results53/53 passed · Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
all-hands-bot
left a comment
all-hands-bot
left a comment
Contributor
There was a problem hiding this comment.
✅ QA Report: PASS
First-run public/no-key behavior now shows onboarding before the standalone backend auth gate, and skipping onboarding falls back to the existing auth gate.
Does this PR achieve its stated goal?
Yes. I ran the app in public mode with no baked session key and no configured backend key on both main and fix-public-onboarding; main went directly to the standalone Add a backend auth gate, while the PR showed the onboarding modal's first Add a backend step with the OpenHands Cloud option and Skip for now. After clicking Skip for now, the PR returned to the same standalone backend gate, preserving the fallback path for users who skip/complete onboarding.
| Phase | Result |
|---|---|
| Environment Setup | ✅ npm install completed and both branches served locally with npm run dev:frontend; no tests/linters/typecheck run. |
| CI Status | ✅ Visible checks are green/skipped except this qa-changes review check still in progress: 15 success, 2 skipped, 1 in progress. |
| Functional Verification | ✅ Browser QA confirmed the before/after flow and skip fallback. |
Functional Verification
Test 1: Public/no-key first-run route shows onboarding before auth gate
Step 1 — Reproduce / establish baseline without the fix:
Ran the main branch in public mode with no baked/frontend key:
git switch main
env -u VITE_SESSION_API_KEY VITE_AUTH_REQUIRED=true VITE_BACKEND_BASE_URL= \
npm run dev:frontend -- --port 3202 --host 127.0.0.1Server readiness output included:
➜ Local: http://127.0.0.1:3202/
Then opened http://localhost:3202/ in a real browser and clicked Confirm preferences. The visible page content was:
## Add a backend
Host Name*
Host*
API Key
Connect
This confirms the baseline behavior described by the PR: a first-run public/frontend-only user lands directly on the standalone backend/API-key gate, with no onboarding or Cloud login setup step first.
Step 2 — Apply the PR's changes:
Switched to the PR branch and ran the same public/no-key scenario on a fresh origin:
git switch fix-public-onboarding
env -u VITE_SESSION_API_KEY VITE_AUTH_REQUIRED=true VITE_BACKEND_BASE_URL= \
npm run dev:frontend -- --port 3201 --host 127.0.0.1Server readiness output included:
➜ Local: http://127.0.0.1:3201/
Step 3 — Re-run with the fix in place:
Opened http://localhost:3201/ in a real browser. The active first screen was the onboarding modal's backend setup step:
## Add a backend
Connect OpenHands to an agent server or OpenHands Cloud to get started.
Host Name
A friendly label for this backend
Host
URL of your agent server
API Key
Next
or
#### OpenHands Cloud
Skip the setup — connect instantly with your OpenHands Cloud account.
Login with OpenHands Cloud
Advanced
Skip for now
The screenshot/browser state showed the backend setup card as the active first onboarding step, including Login with OpenHands Cloud and Skip for now. I did not see a red unreachable-backend warning on this no-backend first-run screen. This confirms the PR delivers the intended onboarding-first public flow.
Test 2: Skipping onboarding preserves the existing auth gate fallback
Step 1 — Baseline behavior:
On main, the fallback/auth gate is the standalone Add a backend screen shown above.
Step 2 — PR behavior:
On the PR branch, clicked Skip for now in the onboarding modal. The browser then showed:
## Add a backend
Host Name*
Host*
API Key
Connect
This confirms the PR keeps the existing standalone Add Backend/API-key screen for users who skip onboarding, matching the PR summary.
Issues Found
None.
This PR review was created by an AI agent (OpenHands) on behalf of the user.
Contributor
🛑 Mock-LLM Docker E2E Test Results48/48 passed · Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
|
| Status | Test | Duration |
|---|
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses)
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
|
✅ Review complete. This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here. |
all-hands-bot
approved these changes
Jun 17, 2026
all-hands-bot
left a comment
all-hands-bot
left a comment
Contributor
There was a problem hiding this comment.
🟡 Acceptable — Clean, focused fix for a real first-run UX problem. The refactoring of BackendConnectionOptions as a shared component is the right call; it keeps the two supported backend connection paths (manual + Cloud OAuth) in sync between the onboarding and the standalone Add Backend modal. Step reordering (backend → agent instead of agent → backend) makes sense for the public/frontend-only deployment model. Two minor nits below.
[IMPROVEMENT OPPORTUNITIES]
- [
check-backend-step.tsx, Line 100] Stale comment: JSDoc still says “Step 1”;CheckBackendStepis now step 0. The description also only covers the edit-backend path; the new first-run no-backend path (rendersBackendConnectionOptionsinstead) is not reflected. - [
check-backend-step.tsx, Line 204] Redundant class: ThehiddenCSS class on the configuration-fields wrapper has no visual effect because the innerBackendConnectionOptionsis already conditionallynullwhenhideConfigurationFieldsis true. Thedivis still needed for itsdata-testid, but the className binding can be dropped.
[TESTING GAPS]
None — unit tests updated, new unit tests added, E2E tests updated. Before/after behavior GIFs are in the PR description. Well done.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
New conditional render path in the root bootstrap for first-run public deployments. The code is well-gated (authMissing && !onboardingCompleted), lazy-loads the onboarding bundle, and existing auth gate behaviour is preserved once onboarding is dismissed. Regression risk is low but the path is user-visible on every first-run of a public/frontend-only deployment.
VERDICT:
✅ Worth merging: Core logic is sound, step-order change is well-motivated, shared component extraction is clean.
KEY INSIGHT:
The key architectural win here is BackendConnectionOptions — extracting the two-column manual+cloud layout into a shared component ensures first-run onboarding and the settings Add Backend modal can never diverge in which connection methods they offer.
Improve this review? If any feedback above seems incorrect or irrelevant to this repository, you can teach the reviewer to do better:
- Add a
.agents/skills/custom-codereview-guide.mdfile to your branch (or edit it if one already exists) with the/codereviewtrigger and the context the reviewer is missing. See the customization docs for the required frontmatter format.- Re-request a review - the reviewer reads guidelines from the PR branch, so your changes take effect immediately.
- When your PR is merged, the guideline file goes through normal code review by repository maintainers.
Resolve with AI? Install the iterate skill in your agent and run
/iterateto automatically drive this PR through CI, review, and QA until it's merge-ready.Was this review helpful? React with 👍 or 👎 to give feedback.
This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation
| ) : null} | ||
| <div | ||
| data-testid="onboarding-backend-configuration-fields" | ||
| className={cn(hideConfigurationFields && "hidden")} |
Contributor
There was a problem hiding this comment.
🟡 Suggestion: The hidden CSS class on this wrapper has no visual effect — the inner BackendConnectionOptions is already conditionally null when hideConfigurationFields is true, so the element is empty and the class is never applied to anything visible. The outer div is needed for its data-testid; just drop the class binding:
Suggested change
| className={cn(hideConfigurationFields && "hidden")} | |
| <div | |
| data-testid="onboarding-backend-configuration-fields" | |
| > |
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
hieptl
reviewed
Jun 18, 2026
hieptl
left a comment
hieptl
left a comment
Contributor
There was a problem hiding this comment.
Hello @neubig,
I followed the steps below and noticed a potential issue:
Step 1: Delete the ~/.openhands directory.
Step 2: Check out the fix-public-onboarding branch.
Step 3: Run npm i.
Step 4: Run the following command:
npm run dev:frontend -- --host 0.0.0.0 --port 12000Then, navigate to http://localhost:12000/.
In my case, the Add Backend modal appears first. Is this the expected behavior?
Please see the attached image for additional context.
Thank you very much! 🙏
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
🛑 Mock-LLM Docker E2E Test Results48/58 passed · 2 failed · 8 skipped · Commit:
🔍 Failure details (2)❌ chromium › regressions/mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shell❌ chromium › regressions/mock-llm-ui-regressions.spec.ts › UI regressions › scopes standalone styles to the agent-server-ui shellPosted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM E2E Tests60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
Contributor
✅ Mock-LLM Docker E2E Test Results60/60 passed Commit:
Posted by the Mock-LLM E2E workflow · results are deterministic (scripted LLM responses) |
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
5 participants
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.
HUMAN:
I tested this manually and it works as expected.
AGENT:
Why
Public/frontend-only deployments with no configured backend key were showing the standalone Add Backend/API-key gate first, which skipped the intended first-run onboarding path. Users should see onboarding first so they can connect a backend before choosing the agent that will use it.
Fixes #1384.
Summary
How to Test
PATH=/usr/bin:$PATH npm run typecheckPATH=/usr/bin:$PATH npm run test -- __tests__/root.test.tsx __tests__/components/onboarding/onboarding-modal.test.tsx __tests__/components/backends/backend-form-modal.test.tsxPATH=/usr/bin:$PATH npm run buildBehavior
Previous behavior
Fixed behavior
Note: local verification used
/usr/bin/node(Node 20.19.2) where needed. The default/usr/local/bin/nodein this sandbox is Node 22 and can hit a JSON import-attribute issue in the pinned@openhands/typescript-clientpackage.This PR description was updated by an AI agent (OpenHands) on behalf of the user.
🐳 Docker images for this PR
• GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas
ghcr.io/openhands/agent-canvasghcr.io/openhands/agent-server:1.28.1-pythonopenhands-automation==1.0.0a100da388a049733cd54b2b40e9c916f18eb58b8fc5Pull (multi-arch manifest)
# Multi-arch manifest — Docker automatically pulls the correct architecture docker pull ghcr.io/openhands/agent-canvas:sha-0da388aRun
All tags pushed for this build
About Multi-Architecture Support
sha-0da388a) is a multi-arch manifest supporting both amd64 and arm64sha-0da388a-amd64) are also available if needed