Repository navigation
feat(web): first-run onboarding — intro, privacy and connect-and-verify wizard, recommendations from detected accounts, and Start a new project - #808
Conversation
0301449 to
a69c6e9
Compare
🌐 Website previewPreview retired when this pull request closed. |
a69c6e9 to
2f8533d
Compare
2f8533d to
2a80aaf
Compare
danyaberezun
left a comment
There was a problem hiding this comment.
Requesting changes. Reviewed 828a2ae.
Blocking
-
The privacy step can override an opt-out that was already confirmed (
useAnalyticsConsent.ts:57-73,OnboardingWizard.tsx:196-223). If sharing is turned off in Settings → Privacy or in another window while the step is open, Continue saves the staledraft = trueover it. → OnceanalyticsConsentConfirmedis true, adopt the savedanalyticsEnabled(or advance) and persist only an explicit wizard choice. Add a test: opt out in Settings during the step, press Continue, sharing stays off. -
Provider status is held in several independent caches with incomplete invalidation (
useHasProvider.ts:10-24,useProviderAuth.tsx:52-62,94-95,WelcomePanel.tsx:37,ProviderWarningBanner.tsx:15):- logout sends no
provider.changed, so the wizard later verifies a signed-out provider and drops it from the list after Back; - a login that finishes while disconnected is never picked up after reconnect;
- on hosts < 92, "No model provider connected" stays under "You're set";
- the warning banner can flash before the wizard.
→ Keep one store-owned, single-flight status snapshot, invalidated by
provider.changed, local login/logout and every welcome. PasshasProviderto the banner and don't render it while the gate ispending. - logout sends no
-
The onboarding e2e is nondeterministic, and its failures can be masked (
onboarding.spec.ts:41-67):- the non-waiting
count()falls back to a real provider (Anthropic in Settings, wheree2e-apikeysits behind "Show more"; Bedrock in the wizard/dialog); - the 8×200 ms loop can exit without answering the prompt;
- a timeout can skip the in-body
finallycleanup; the retry then hitstest.skip(anyProviderConfigured), and Playwright reports failed+skipped as flaky (green). Later onboarding tests on that lane then skip silently.
→ Wait for the exact
e2e-apikeyrow (click show-more in Settings), drive its known prompt and assert success. Move cleanup toafterEach(log out everycanLogoutprovider). Skip only when all configured providers areenv; otherwise fail. - the non-waiting
-
The post-create setup continuation isn't fenced (
setupPrompt.ts:13-15,defaultWorkspace.ts:6-23,ProjectTree.tsx:231-235). Navigating elsewhere whileworkspace.listis pending lets the stale continuation activate the new project's Default workspace and open the setup chat. From the rail, a list failure becomes an unhandled rejection (void adopt(...)). → Carry a navigation generation through the continuation, re-check it before activating Default and beforestartChatInTab, and catch errors. -
The API key is hidden for OAuth-capable providers although the copy promises it (
ConnectProviderOptions.tsx:217,panels.connectProvider.description,panels.onboarding.connect.subtitle). Anthropic, OpenAI, xAI, Kimi and Copilot offer only Sign in, with no route to Settings. Given the PR's own note on Anthropic's terms, first run steers Claude users to subscription sign-in and hides the API-key path. → Show the API key button as Settings does, or fix the copy and link to Settings → Providers.
Non-blocking
-
Login ownership (
useProviderAuth.tsx:64-78,ConnectProviderDialog.tsx:54-57,71-75,OnboardingWizard.tsx:150,providerLogin.ts:49-50):- closing the dialog or pressing Skip while
loginStartis pending leaves a hidden active login; - the handoff's unconditional
clearLogin()can drop a newer login without cancelling it; - supersede silently kills another client's live login (its dialog stays on "Working…") and leaks the
loginAnalyticsentry.
→ Bind the login to its owning operation (cancel a
loginIdthat arrives after the owner closed) and clear only the matching terminal login. Publish a terminal frame for the supersededloginIdand calldropLogin. - closing the dialog or pressing Skip while
-
smoke:loginsisn't hermetic and can pass falsely (scripts/smoke-provider-logins.ts):- the spawned host auto-loads the repo-root
.envdespite the restricted env; USERPROFILEis not set;- an immediate
successand a0/0run count as passes; - wire requests have no timeout, so the script hangs if the host dies;
- there is a free-port race, and
kill()isn't awaited beforermSync.
→ Use
cwd: sandboxwith--no-env-fileand setUSERPROFILE. Accept onlyauthUrl/deviceCode/prompt, and fail on zero outcomes. Bound requests and reject them on close. Let the host bind port 0 and await its exit before cleanup. - the spawned host auto-loads the repo-root
-
detectAccountstimeouts don't cancel I/O (detectAccounts.ts:79-86,262). With a probed path on a hung mount, each detect leaves up to 9 FS ops pending. On Bun 1.4.3 (16 CPUs), 16 blocked ops stall unrelated async FS on the host. → Make detection single-flight and reuse a pending read per path until it settles. -
createProjectrollback removes more than it created (projects.ts:402).rm -rfof the new folder also deletes anything written into it during bootstrap (hook output, a concurrent writer). → Remove.git, then use a non-recursivermdir. -
planin Balanced breaks "Save as preset" against v91 hosts (presets.ts:83). Preset saves aren't protocol-gated, and the old validator rejectsplan. → Gate preset saves that containplanon protocol 92, with an explanation.
Nits
- The cold-start predicate is derived twice (
selectors.ts:120,WelcomePanel.tsx:45-60) → use one store selector. panels.connectProvider.*andpanels.onboarding.connect.*carry identical copy, there are two "Continue" keys, andofferSetupis alwaystrue→ dedupe the keys and drop the option.- The test at
settings.test.ts:381duplicates:392→ delete one. loginPromptMessagehardcodes English UI fragments → use locale keys viat.- zh-CN
panels.welcome.openProjectSubtitleis stale after the rewording → delete it. - Assertions on the non-existent
connect-show-all(welcome.spec.ts:68) andonboarding-steps(onboarding.spec.ts:146) are vacuous, and theconnect-detectedcount is checked before the list renders (:148) → remove them, or move them after render. - The
chmod 0o400failure injection doesn't work as root (onboarding.spec.ts:317) → reuse the directory trick fromanalytics-consent.spec.ts. - No tests cover: zero models → Back,
createProjectrollback, the 1.5 s cap,onConnectedfiring once. - The live, workflow and artifact host spawners don't pin the new detection env vars → use one shared env helper.
- Stale docs:
apps/web/SPEC.md:143(compiler bailout) and:163(LoginDialog mount);- "Settings while it is open" → "Settings → Providers";
- the PR body's "JetBrains AI pinned first" / "Recommended" → the code actually places it last and labels the block "Detected".
- Other small issues:
- after Back from zero models, the provider can't be re-verified;
- the display name becomes the slug;
- BranchPicker shows "local only" on a load error.
|
Thanks for the thorough review, danyaberezun. Everything except 6–8 is fixed in 2c5de64. Items 6–8 are tracked in #949. Blocking
Non-blocking
Nits
Verification on 2c5de64: all fast gates pass, |
abdd700 to
73df092
Compare
b6f212e to
9230023
Compare
…etween the dialog and the wizard's privacy step, let custom presets place the TODO List, and align the layout e2e specs with Balanced opening on it
… lands, verify a provider connected before the wizard reaches Connect, and record the new-project Default exception in the top-level docs
…ed by every change, adopt a consent choice confirmed elsewhere, fence the new-project setup continuation, offer API keys on every provider, and make the onboarding e2e deterministic
…ending login on skip, split the wizard into step components, drop the viewport-cap table, and emit a created project once
…model.list is empty, instead of a fixed-delay re-read
danyaberezun
left a comment
There was a problem hiding this comment.
Requesting changes. Reviewed 3b7dc1e. Items 1–5 from my previous review are fixed. Before merge, please fix the three provider-status gaps below in one commit, and rebase.
- While this window has its own login open, another login's success no longer invalidates provider status (
apps/web/src/store/appStore.ts:4294-4303). The active-login guard returns{}before theproviderStatusVersionbump, so on a host without the new login-successprovider.changedthe snapshot stays stale. → Compute the bump before the guard and return it on the guard path too; keep the guard for dialog ownership. Test: a success frame for anotherloginIdwhile a login is active leavesactiveLoginunchanged and bumpsproviderStatusVersion. - Welcome no longer re-reads provider status on entry (
apps/web/src/panels/useProviderStatus.ts:33-35,WelcomePanel.tsx:37). main re-read it on every Welcome mount. Now a credential change the host doesn't push (apiCLI login/logout) leaves the banner wrong until a provider surface opens or the socket reconnects. → Invalidate the shared snapshot when Welcome mounts (reloadProviderStatus()in a mount effect). Test: remounting Welcome issues a freshprovider.statusread. - The connect handoff runs on a stale snapshot (
apps/web/src/panels/ConnectProviderOptions.tsx:91-103). The effect ignoresrefreshing, so pressing Get started while the post-logout re-read is in flight verifies the signed-out provider and adds it to the baseline. Reconnecting it after Back then never triggers verification; only Skip setup gets out. → Skip the effect whileauth.refreshing, and addrefreshingto its deps. Test (e2e): hold the post-logoutprovider.statusreply (ashideModelsholdsmodel.list), press Get started, and assert no verification starts. - The branch conflicts with main after #817 (
LoginDialog.tsx,BranchPicker.tsx,ChangesPanel.tsx,ProviderWarningBanner.tsx,ProvidersSettings.tsx, and the panels and shellSPEC.md). → Rebase onto main. - The PR body is stale, and it becomes the squash commit message: it says
PROTOCOL_VERSION/ONBOARDING_PROTOCOL_VERSION= 92 (now 94, plusLAYOUT_PLAN_PRESET_PROTOCOL_VERSION= 94) and "Submit sits next to Cancel" (now Continue). → Update it.
|
Heads-up: we're taking over the items from my last review (R1–R3, the rebase onto main, the PR body) and will push them to this branch shortly. Please hold pushes to |
Resolve the conflicts with #817's button system: BranchPicker keeps the local-only marker on both trigger forms, LoginDialog keeps Continue beside Cancel at size l, ProviderWarningBanner keeps the connect dialog with main's banner styling, and ProvidersSettings drops main's inline ProviderRow for the shared one. Move the onboarding buttons (wizard steps, connect options, create-project, ProviderRow) onto the five variants and three sizes.
…her login's success, and hand off only from a current snapshot
|
Pushed
Local checks: |
…eardown step, and drop wall-clock and inherited git-config dependence
…port, so a sign-in that lands before the opening re-read still hands off
…e projects folder with retries, and count the toast before it auto-dismisses
… a fixed 5 ms sleep
|
Flake audit of this PR's tests, started at Product race (found by the stress test)
Test determinism
Evidence (retries 0)
Not from this PR
|
Problem
A brand-new user opened ThinkRail to an empty Projects rail, a "No model provider connected" warning, and one Open project card. Connecting a model meant digging through Settings → Providers (JetBrains AI, then Subscriptions, then a long API-key list), and someone without a repo had nowhere to start. The analytics question popped up in the first seconds, a sign-in abandoned by a reload could park the next attempt on "Working…", and pi's terminal phrasing ("(default)", "(headless)", "press Enter", "(blank for github.com)") leaked into the login dialog. This implements the first-run redesign discussed in #742.
Approach
model.listandmodel.defaultread, no tokens spent. When it answers, the connect screen fades out and the You're set screen fades in, with no Done button: one line naming the provider and the default model ("Anthropic connected · Claude Fable 5 is the default model") over Welcome's own two cards, Start a new project and Open project. The wizard has no project step and no progress strip, so there is one project entry point instead of two copies. Nothing animates height, and reduced motion swaps instantly. Zero models warns and offers Check again and Back to the choices. Skip setup is on every screen before done except privacy. The wizard shows only on a true cold start (no projects, no provider, not dismissed). The gate lives in the store, so a remount mid-wizard keeps it. Reaching done or skipping persistsonboardingDismissed, so it never comes back. While it is active, the Projects rail and pane are not rendered, so the wizard has the whole window. It stays until a project opens.provider.detect):anthropic, Codex's ChatGPT sign-in →openai(falling back to legacyopenai-codex), Copilot editor plugins and the Copilot CLI →github-copilot, Kimi CLI →kimi-coding, Grok Build →xai.apikeymode, Grok's API-key scope and logged-out or revoked states are not sign-ins. An expired access token with a live refresh token still counts. Each tool's relocation env vars are honoured (CLAUDE_CONFIG_DIR,CODEX_HOME,XDG_CONFIG_HOME,COPILOT_HOME,KIMI_SHARE_DIR,GROK_HOME, …).plus,pro,max,team,business,enterprise,edu). Anything else,freeincluded, drops the badge but keeps the card.LOCALAPPDATAis honoured for Copilot.ConnectProviderOptionsis shared by the wizard and the Welcome banner's new ConnectProviderDialog:centralis installed, otherwise it is a plain row at the end of the list.ProviderRow, with the same buttons as Settings: Sign in where the provider supports it, API key where it accepts one. Login orchestration moved intouseProviderAuthso Settings and both new surfaces share it.provider.statussnapshot. It is invalidated byprovider.changed, by every welcome, by a successful login frame, by a local sign-out, and whenever a provider surface opens, and it is read single-flight per version.provider.logoutnow broadcastsprovider.changedtoo. The Welcome banner, the wizard gate, both connect surfaces and Settings read the same snapshot, and the banner never renders while the gate is still undecided.project.createmakes an empty git repo under~/thinkrail/<name>with an initial commit. The project keeps the name the user typed (the folder is its slug). The Welcome card can optionally open thesetting-up-a-projectchat right away. That continuation is dropped if the user navigates away first. A failed bootstrap removes.git, and the folder only while it is still empty. A repo with no remote now reads "main · local" in the branch picker, so it is not mistaken for a GitHub branch.selectOnboardingActive. Skipping setup brings the consent window back. Once a project exists, the wizard stops holding them.ConnectProviderOptionsreports a newly configured provider once throughonConnected. The banner's dialog then closes itself and toasts the provider and its model count before Welcome switches to "You're set". The wizard does neither: it verifies in place.Changes
OnboardingWizard,ConnectProviderOptions,ConnectProviderDialog,ConnectedLine,CreateProjectDialog,useProviderAuthanddetectAccounts;useConsentDraftis shared byAnalyticsConsentDialogand the wizard's privacy step;startChatInTabtakes an options object (preferredGroupId,prompt,draft);WelcomePanelgates the wizard through the store'sonboardingWizard;ProvidersSettingsuses the shared hook;BranchPickermarks local-only repos.onboardingWizardstate andselectOnboardingActive;Shellholds consent and interview prompts while onboarding is active;ProjectHomedrops the rail and pane in place without remounting Welcome.loginOptionstranslates pi's terminal phrasing into dialog copy.project.create(optionaldisplayName),provider.detect+DetectedAccount,AppConfig.onboardingDismissed;PROTOCOL_VERSION/ONBOARDING_PROTOCOL_VERSION/LAYOUT_PLAN_PRESET_PROTOCOL_VERSION= 94.project.createhandler;auth/detectAccountsbehindprovider.detect(six sources, each bounded by a 1.5 s timeout; resolves each to the first provider the current pi runtime can OAuth into; plan badges limited to known plan names);providerLoginsupersedes a same-provider pending login silently;provider.logoutbroadcastsprovider.changed;project.createtakes an optionaldisplayName, and its rollback never removes files it didn't create;plantool (clients refuse to save one to a pre-94 host, with an explanation). The contract'sLayoutToolIdalready listed it, but the settings validator did not, so a preset saved from the new Balanced default was rejected.bun run smoke:logins. It boots a throwaway host, starts every provider's sign-in methods over the wire, walks every select branch, and checks each reaches its first user step. It cancels everything and stores nothing.baseURL, so binary and desktop runs reset between tests;XDG_CONFIG_HOME,COPILOT_HOME,KIMI_SHARE_DIRandGROK_HOMEare pinned inside the e2e home, andGROK_AUTH_PATH,CLAUDE_SECURESTORAGE_CONFIG_DIRandCLAUDE_CODE_OAUTH_TOKENare cleared, so detection never reads the developer's real config.mainflake from fix(shared): re-check the Central artifact once a moved watch is live #830):jbcentral.test.ts"re-arms deeper…" now waits for the watcher's second re-arm instead of sleeping a fixed 5 ms.packages/locales/en.json, underpanels.onboarding.*,panels.connectProvider.*andpanels.createProject.*.check:i18npasses.Not in this PR:
anthropicOAuth sign-in, and detection recommending it when Claude Code is found, fall under that. This PR leaves pi's provider list as it is. Whether to hide that sign-in, drop the Claude Code match, or point Claude users at an API key or JetBrains AI needs a product and legal decision.central loginneeds a TTY and exits withbubbletea: could not open TTYwhen ThinkRail spawns it, so ThinkRail shows "couldn't launch". This likely affects the desktop app too and needs its own fix.Screenshots
1280×800 viewport, captured with the hermetic e2e host (no provider configured; its stub "E2E Test Provider" and Central fixture appear in the list). Before =
origin/mainat9d88588e(the latermaincommits don't touch these screens). The wizard's after shots are from this branch rebased onto2c97cb1a.The wizard, step by step
Intro, before = first launch on
mainPrivacy. Only while analytics consent is unconfirmed. Before, the consent window popped up over the first screen.
Connect. Nothing detected, so the Detected block holds only the Central fixture. Before = the banner's "Connect a provider" (Settings → Providers).
Zero models. The connection worked but no models came back: Check again re-runs the read, and Back returns to the choices.
Connect, with detected accounts. Claude Code (Max), Codex (ChatGPT Plus) and the Copilot CLI are signed in on this machine (1280×920, so the whole block fits).
You're set. Reached right after the in-place check, with no Done button: the provider and the default model over Welcome's two cards. No tokens are spent.
Start a new project from that card.
Around the wizard
Anthropic sign-in method
GitHub Copilot first step
Skills dialog
Branch picker on a repo with no remote
Related issues
Implements the first-run proposals from #742. Closes nothing. Login ownership, a hermetic
smoke:loginsand cancellable detection I/O are tracked in #949.Checklist
bun run lint,bun run typecheck,bun run testc0283dcf(merged withmainat6cd6f905), these pass locally and in CI:check:deps,check:boundaries,check:seams,check:spec-surface,check:i18n,check:compiler,lint,typecheckand the fullbun run test. That includes thedetectAccountsscenario tests: logout, Console vs subscription, keyring, expired tokens, Windows paths andLOCALAPPDATA, BOM, oversized files, known plan names.c0283dcf: every source, binary and desktop shard is green with no flaky retries. Locally,bun run e2eon2f6c6170gave 656 passed and 1 failed:extension-dialogs.spec.ts:178, which flakes at the same rate onmain. The onboarding specs settle the host before each cold start, fail rather than skip on a leaked provider, and sign every provider out inafterEach. The flake audit, with retries off, is in the PR comments.onboarding.spec.tsagainst the compiled binary (bun run e2e:binary), andbun run smoke:logins56/56 sign-in flows reaching their first step.SPEC.md/ top-level specs updated to reflect any boundary, contract, or behavior change