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

**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.

<details>
<summary>File changes</summary>

**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.

</details>

## 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 <taylorkmho@gmail.com>
This commit is contained in:
Taylor Ho
2026-08-07 15:03:43 -07:00
committed by GitHub
parent 2b873cf208
commit a5a9240241
2 changed files with 31 additions and 0 deletions

View File

@@ -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],

View File

@@ -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,
}) => {