first-run onboarding redesign - #5348
Conversation
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Here's a visual recap of what changed: Open the full interactive recap |
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🔴
Review Details
Incremental Code Review Summary
This update substantially removes obsolete onboarding and MCP integration code, trims the first-run flow, and updates tests, preview handling, docs, and the visual proof surface. The core onboarding refactor remains understandable, and focused reviewers found no new OAuth regression. The previously reported localization and translated-capability deduplication issues are still present and remain open; they are intentionally not reposted here.
New Findings
- 🔴 HIGH — Manual setup remains trapped in preview onboarding: the manual-settings handler preserves
onboarding=previewwhile navigating. A remounted Settings page reinitializes preview onboarding as active, so the preview flow can immediately show the onboarding overlay again instead of reaching Settings. - 🟡 MEDIUM — The
referencespreview step now dismisses itself: it still maps toextension, but the updated code no longer registers an extension. The extension branch callsfinishOnboarding(null)and renders nothing, so the supported preview URL does not render a usable screen. - 🟡 LOW — The Design visual proof still requests removed
introandmanualsteps: these URLs now fall back to the role screen, producing mislabeled duplicate coverage rather than validating the current onboarding flow.
🧪 Browser testing: Will run after this review (PR touches UI code); the full planner run was attempted but Chrome/browser automation tools were unavailable, despite the dev server being healthy.
| const [screen, setScreen] = useState<FirstRunScreen>(() => | ||
| previewStep === "references" ? "extension" : (previewStep ?? "intro"), | ||
| previewStep === "references" ? "extension" : (previewStep ?? "role"), |
There was a problem hiding this comment.
🟡 Keep the references preview step renderable
references is still accepted by ONBOARDING_PREVIEW_STEPS and mapped to the extension screen, but the updated code no longer registers an extension for this flow. The extension branch therefore calls finishOnboarding(null) and returns null, so ?onboarding=preview&step=references immediately dismisses instead of rendering a preview screen. Remove the stale preview step or provide a preview-safe extension before keeping it in the allowlist.
Additional Info
New issue reported by 1/4 reviewers and confirmed by source inspection; no app extension registrations remain in the current code search.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Incremental Code Review Summary
The latest update fixes the connecting-state shell identity issue, which has been verified and its prior comment resolved. The references preview-step issue remains present and its existing comment is preserved without reposting. The visual-proof route cleanup and prior manual-settings preview fix also remain intact.
New Findings
- 🟡 MEDIUM —
connect-and-integrationsno longer shows integrations: this mode remains part of the public configuration contract and is still resolved by core config, but the redesigned component removed the generic integrations/tools screen entirely. Apps using that mode now silently skip the MCP integrations setup. - 🟡 MEDIUM — Manual setup bypasses registered first-run extensions: the manual Settings action completes onboarding directly instead of entering the registered app-owned extension sequence, so apps with custom first-run screens lose them on the Configure manually path.
Focused reviewers otherwise found no additional regressions. Browser verification was attempted, but Chrome automation tools were unavailable; the local server was available with some unrelated Vite dependency warnings.
🧪 Browser testing: Will run after this review (PR touches UI code); this run was blocked by unavailable Chrome/browser automation tools.
| type FirstRunScreen = "choice" | "role" | "connecting" | "extension"; | ||
|
|
||
| const FIRST_RUN_SCREEN_ORDER: readonly Exclude<FirstRunScreen, "extension">[] = | ||
| ["intro", "choice", "manual", "tools", "role", "connecting", "ready"]; | ||
| ["role", "choice", "connecting"]; |
There was a problem hiding this comment.
🟡 Preserve the connect-and-integrations onboarding mode
connect-and-integrations remains a supported public mode in config.ts and is still returned by the mode resolver as including the generic MCP integrations catalog. This component no longer has a tools/integrations screen or mode-specific branch, so apps configured with that mode silently skip the integrations setup. Preserve the catalog for that mode or remove/deprecate the mode and update its config/docs contract together.
Additional Info
Reported by 1/3 reviewers and confirmed by searching the current config and resolver; existing references-preview and localization comments were not reposted.
This branch was successfully deployed
2 participants
The onboarding flow now starts with a short role question, then goes straight to a single "Choose your setup" screen. The old separate "intro" screen and "manual keys" screen are gone, so setup takes fewer steps.
First Screen:
Second Screen: