[codex] Add global onboarding flow with AMR#2269
Conversation
d6e740a to
ea3c668
Compare
|
Hey @qiongyu1999, quick coordination note: #2272 is also open against the same onboarding surface and shares the same core files, with the main difference being the no-AMR variant versus this AMR-enabled path. Worth keeping the two variants explicitly paired or consolidating once the team decides which route should land first, so review effort doesn't split across duplicate hunks. |
mrcfps
left a comment
There was a problem hiding this comment.
@qiongyu1999 Thanks for pushing this onboarding flow — I found two concrete issues in the new main path that should be fixed before this lands.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.| onCreated={(projectId, project) => { | ||
| if (project) { | ||
| setProjects((curr) => [ | ||
| project, | ||
| ...curr.filter((p) => p.id !== project.id), | ||
| ]); | ||
| } | ||
| navigate({ kind: 'project', projectId, conversationId: null, fileName: null }); |
There was a problem hiding this comment.
Problem: this success path never marks onboarding complete. handleCompleteOnboarding is only reached through onFinish() / skip, but the main generate path goes straight from onCreated to navigate(...) here.
Why it matters: after the user completes the most important onboarding action, onboardingCompleted still stays false, and the bootstrap redirect in App.tsx routes them back to /onboarding on the next reload/session instead of treating onboarding as finished.
Evidence: this callback updates the project list and navigates into the new project, but it never calls handleCompleteOnboarding or onCompleteOnboarding, while App.tsx:443-447 now redirects every user with onboardingCompleted === false into the onboarding route.
Suggested change: treat successful design-system creation as onboarding completion before navigating (or invoke the same onFinish() path from the embedded flow) and add a regression test that creates a system from onboarding and asserts the persisted config flips onboardingCompleted to true.
| <div className="onboarding-view__form-grid"> | ||
| <OnboardingDropdown | ||
| label={t('settings.onboardingRoleLabel')} | ||
| placeholder={t('settings.onboardingSelectPlaceholder')} | ||
| value={profile.role} | ||
| options={roleOptions} | ||
| onChange={(value) => setProfile((current) => ({ ...current, role: value }))} | ||
| /> | ||
| <OnboardingDropdown | ||
| label={t('settings.onboardingOrgSizeLabel')} | ||
| placeholder={t('settings.onboardingSelectPlaceholder')} | ||
| value={profile.orgSize} | ||
| options={orgSizeOptions} | ||
| onChange={(value) => setProfile((current) => ({ ...current, orgSize: value }))} | ||
| /> | ||
| <OnboardingDropdown | ||
| label={t('settings.onboardingUseCaseLabel')} | ||
| placeholder={t('settings.onboardingSelectMultiplePlaceholder')} | ||
| value={profile.useCase} | ||
| options={useCaseOptions} | ||
| multiple | ||
| onChange={(value) => { | ||
| if (!Array.isArray(value)) return; | ||
| setProfile((current) => ({ ...current, useCase: value })); | ||
| }} | ||
| /> | ||
| <OnboardingDropdown | ||
| label={t('settings.onboardingSourceLabel')} | ||
| placeholder={t('settings.onboardingSelectPlaceholder')} | ||
| value={profile.source} | ||
| options={sourceOptions} | ||
| onChange={(value) => setProfile((current) => ({ ...current, source: value }))} | ||
| /> |
There was a problem hiding this comment.
Problem: the new "About you" step is currently write-only. These controls only update the local profile state, and nothing in the continue/finish paths ever persists or forwards that state anywhere.
Why it matters: users are asked for organization size, use cases, and discovery source, but that input is silently discarded, so this step has no product effect and does not satisfy the PR goal of letting users "share lightweight profile context."
Evidence: the only references to profile in this file are the useState initializer and these field setters; handlePrimaryAction() and onFinish() never read profile, and no config/API call is made from this step.
Suggested change: either persist the profile payload before advancing (config/API/analytics, whichever is intended) or remove/hide this step until there is a consumer. Please add a regression test around the chosen persistence path so the form does not slip back to being a no-op.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
Why
First-run Open Design setup currently needs a clearer global flow instead of scattering runtime, profile, and design-system setup across later settings surfaces. This PR adds a focused onboarding path so new users can pick a runtime, share lightweight profile context, and decide whether to build a design system before entering the main workspace.
The main pain this addresses is activation friction: users need to understand AMR, CLI, and BYOK choices up front, while still having a short path to skip or continue into the product.
What users will see
New users land on
/onboardingwith a standalone Welcome flow and no app sidebar/top bar. The flow has three steps: Connect, About you, and Design system.Connect highlights AMR Cloud as the recommended route, with local CLI scanning and BYOK setup available inline. AMR currently uses a mocked login handoff: Continue opens the AMR entry in a separate browser page, shows a waiting state, then advances after a short delay.
About you collects organization size, use cases, and discovery source with custom dropdown UI. Design system embeds the existing creation flow in onboarding mode, with shorter copy, clear value messaging, upload/source options, and a standalone skip action.
Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-pack/tools-prflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates, orcraft/, or change to the skills protocolTRANSLATIONS.mdfor the locale workflow)package.json(dependenciesordevDependencies); workspace-packagepackage.jsonfiles are out of scope. Include a paragraph on what we get vs. what bytes we ship (seeCONTRIBUTING.md→ Code style)Screenshots
Validated locally at
http://127.0.0.1:19111/onboardingduring the UI iteration. Screenshots should be attached from the browser preview before marking ready for review.Bug fix verification
Validation
pnpm install(passes; emits expected Node engine warnings because local Node isv26.1.0while the repo targets~24)pnpm --filter @open-design/web typecheck(passes; same Node engine warnings)git diff --check origin/main...HEAD(passes)