From a5a9240241ad584c839f79af546df9bf0216ca1f Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Fri, 7 Aug 2026 15:03:43 -0700 Subject: [PATCH] fix(desktop): let imported and recovered identities finish onboarding (#5228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **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. Signed-off-by: Taylor Ho --- .../features/onboarding/machineOnboarding.ts | 5 ++++ desktop/tests/e2e/onboarding.spec.ts | 26 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/desktop/src/features/onboarding/machineOnboarding.ts b/desktop/src/features/onboarding/machineOnboarding.ts index affdf72009..eacc2c3562 100644 --- a/desktop/src/features/onboarding/machineOnboarding.ts +++ b/desktop/src/features/onboarding/machineOnboarding.ts @@ -176,6 +176,11 @@ export function useMachineOnboardingState({ completionKey(MACHINE_ONBOARDING_COMPLETION_STORAGE_KEY, pubkey), "true", ); + // Clear the "continuing" marker so completion actually settles the flow. + // Imported/recovered identities set continuingPubkeyRef to pin the stage + // to "onboarding" until setup finishes; leaving it set after complete() + // keeps the stage pinned forever, so Skip/Next appear to do nothing. + continuingPubkeyRef.current = null; setCompletedPubkey(pubkey); }, [currentPubkey], diff --git a/desktop/tests/e2e/onboarding.spec.ts b/desktop/tests/e2e/onboarding.spec.ts index b1e96aaf27..e5217da3d3 100644 --- a/desktop/tests/e2e/onboarding.spec.ts +++ b/desktop/tests/e2e/onboarding.spec.ts @@ -803,6 +803,32 @@ test("first-launch key import continues to machine setup", async ({ page }) => { await expect(page.getByTestId("app-loading-gate")).toHaveCount(0); }); +test("imported-key users can skip out of harness setup", async ({ page }) => { + // Regression: importing an existing key sets the onboarding state machine's + // "continuing" marker, which pinned the stage to onboarding even after + // complete() ran — so Skip/Next silently did nothing. The fresh-key skip + // tests never exercised the import path, so this gap shipped. Prove an + // imported-key user actually leaves onboarding on Skip. + await installMockBridge(page, undefined, { + skipCommunitySeed: true, + skipOnboardingSeed: true, + }); + await page.goto("/"); + + await page.getByRole("button", { name: "Use an existing key" }).click(); + const importedNsec = nsecEncode(hexToBytes(TEST_IDENTITIES.alice.privateKey)); + await page.getByTestId("nostr-import-nsec-input").fill(importedNsec); + await page.getByTestId("nostr-import-submit").click(); + + await expect(page.getByTestId("onboarding-page-2")).toBeVisible(); + await page.getByTestId("onboarding-setup-skip").click(); + + // Reaching community onboarding proves machine onboarding completed rather + // than staying pinned on the setup step. + await expect(page.getByText("Join or create a community")).toBeVisible(); + await expect(page.getByTestId("onboarding-page-2")).toHaveCount(0); +}); + test("first-launch encrypted backup import asks for a passphrase and continues", async ({ page, }) => {