Skip to content

fix(desktop): let imported and recovered identities finish onboarding - #5228

Merged
wesbillman merged 1 commit into
mainfrom
tho/onboarding-skip-spike
Aug 7, 2026
Merged

fix(desktop): let imported and recovered identities finish onboarding#5228
wesbillman merged 1 commit into
mainfrom
tho/onboarding-skip-spike

Conversation

@tellaho

@tellaho tellaho commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Category: fix
User Impact: People who onboard by importing an existing key or recovering from a phone can now use "Skip for now" (and Next) on the harness setup and model config steps, instead of getting stuck.

Problem: On the "Set up your agent harnesses" and "Configure your default model settings" onboarding steps, clicking Skip for now — or Next — did nothing for anyone who reached those steps by importing an existing key or recovering an identity from a phone. The app stayed frozen on the step.

Solution: The onboarding state machine sets continuingPubkeyRef to the current pubkey on import/recovery to keep the flow on onboarding until setup finishes (added in #4845). But complete() never cleared that ref, so once it matched the current pubkey the stage stayed pinned to onboarding forever — completion could never win. complete() now clears the ref so finishing/skipping actually settles the flow. Fresh-generated keys never set the ref, which is why first-run fresh-key skip already worked and the gap went unnoticed.

File changes

desktop/src/features/onboarding/machineOnboarding.ts
Clear continuingPubkeyRef inside complete() so an imported/recovered identity's "continuing" marker no longer outlives completion and pin the stage to onboarding.

desktop/tests/e2e/onboarding.spec.ts
Add a regression test that imports an existing key, reaches harness setup, clicks Skip for now, and asserts onboarding exits (reaches community onboarding). This fails without the fix. The existing skip tests only exercised the fresh-key path, which never set the ref — hence the gap.

Reproduction steps

  1. Start onboarding and choose Use an existing key (or recover from a phone); import a key and continue to Set up your agent harnesses.
  2. Click Skip for now (or Next). Before this change, nothing happens — the step is stuck. The same trap hits Configure your default model settings.
  3. With this change, Skip/Next advances out of onboarding as intended.
  4. Automated: pnpm build:e2e && pnpm exec playwright test onboarding.spec.ts --project=integration -g "imported-key users can skip out of harness setup" — passes with the fix, fails without it.

Root cause

Introduced by #4845 (feat(identity): recover desktop identity from a signed-in phone), which added continuingPubkeyRef.current === currentPubkey as an independent condition selecting the onboarding stage. That guard has no off switch: complete() set the completion flag but never cleared the ref, so the OR'd condition kept the stage pinned. Not a revert candidate — the guard's intent (keep a just-published identity in onboarding until setup finishes) is correct; it just needed to release on completion.

Importing an existing key or recovering from a phone sets continuingPubkeyRef to pin the onboarding stage until setup finishes, but complete() never cleared it — so the state machine stayed pinned to "onboarding" and Skip/Next on the harness setup and model config steps did nothing. Clear the ref in complete() so completion actually settles the flow. Add an e2e regression that imports a key, skips harness setup, and asserts onboarding exits (fails without the fix).

Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
@tellaho
tellaho marked this pull request as ready for review August 7, 2026 21:07
@tellaho
tellaho requested a review from a team as a code owner August 7, 2026 21:07
@wesbillman

Copy link
Copy Markdown
Collaborator

Release review by Carl on Wes’s behalf at exact head 67f9232c7b35ecc9d96de38c235b509dd3e04fe5: include this in the bugfix release. It fixes a separate serious onboarding trap with a correctly placed one-line state-machine change; no blocker found.

I traced every writer/reader of continuingPubkeyRef and every completion exit. Existing-key import and phone recovery set the marker before replacing the identity query so the new identity remains in machine onboarding while setup is unfinished. Every successful exit from harness setup and default-model config converges on complete(selectedPubkey). Before this patch, complete() persisted completion but left the independent continuingPubkeyRef.current === currentPubkey onboarding condition true forever, so Skip and Next genuinely could not win. Clearing the transient marker at the common completion boundary preserves its intended lifetime and releases every affected exit path.

Higher-priority safety states—reset failure, locked keyring, relaunch required, and identity lost—still win before completion/ready logic. Completion remains stored per pubkey before the marker clears and React state updates; there is no asynchronous gap inside that callback. Fresh-key onboarding never sets the marker and remains behaviorally unchanged.

Independent exact-head validation: typecheck passed; Biome passed on both changed files; E2E build passed; the new imported-key → harness Skip regression passed and reaches community onboarding; git diff --check passed; worktree clean; remote head unchanged. GitHub exact-head CI is fully green, including Desktop Core/build/Release Candidate and all smoke/integration shards.

Test behavior changed intentionally: the new integration test covers the manual existing-key import path that shipped broken. Phone recovery and config Next/Skip are not separately enumerated in E2E, but they use the exact same marker and complete() choke point; duplicating that expensive matrix is unnecessary for this release. A focused hook-level state-machine test would be a good follow-up.

This is independent of #5242 but belongs in the same corrective release: #5242 restores packaged runtime styling; #5228 prevents imported/recovered users from being trapped in onboarding. Merge #5228, then ensure the release candidate is generated from main containing both fixes. #5242 currently branches from main before #5228, but the paths do not overlap and the synthetic merge is clean.

@wesbillman
wesbillman merged commit a5a9240 into main Aug 7, 2026
26 checks passed
@wesbillman
wesbillman deleted the tho/onboarding-skip-spike branch August 7, 2026 22:03
wpfleger96 pushed a commit that referenced this pull request Aug 7, 2026
…format

* origin/main: (60 commits)
  feat(desktop): unify add agent flows (#5015)
  fix(buzz-agent): budget summarizer reasoning separately so it cannot starve the handoff summary (#5248)
  infra: bind development services to loopback (#4871)
  chore(release): release Buzz Desktop version 0.5.7 (#5252)
  fix(desktop): isolate relay admission tests (#5221)
  fix(desktop): externalize boot <style> to prevent Tauri CSP nonce override (#5242)
  fix(desktop): let imported and recovered identities finish onboarding (#5228)
  Recover from max-token response truncation (#5223)
  chore(release): release Buzz Desktop version 0.5.6 (#5214)
  fix(mobile): keep latest messages above composer (#4981)
  fix(sdk): preserve self-mention p tags in message and forum event builders (#4975)
  bump @tauri-apps/cli to ~2.11.4 to fix linux app icon issue (#4858)
  feat(desktop): adding rich link previews to messages (#3818)
  fix(buzz-agent): Responses reasoning summary, Anthropic display:summarized, ACP v2 messageId (#5195)
  fix(desktop): retain distinct agent instances in autocomplete (#5202)
  fix(desktop): defer channel visibility change to Save (#5203)
  feat(desktop): Projects follow-ups — access restrictions, fast loading, activity feed polish (#5073)
  refactor(cli): replace probe/decider/detail split with single typed extractor (#5191)
  fix(desktop): drop unhandled rejection from throwing window.Notification (#5143)
  fix(desktop): fence localStorage SecurityError from killing the React tree (#5142)
  ...

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
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