From 4de22eb0db3719d47ea4778d8a04ab60043267f8 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Tue, 21 Jul 2026 14:58:06 -0700 Subject: [PATCH 1/4] fix(onboarding): skip community profile setup for existing relay members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a user imports an existing keypair and connects to a relay where that key already has a kind:0 profile event, the community onboarding flow now detects this and completes immediately — landing the user directly in the app without prompting them to set a display name or avatar. The regression was introduced by PR #1936, which rewired App.tsx to unconditionally advance the transaction from 'connecting' to 'profile' once the relay was ready. The old hasProfileEvent gate in useFirstRunOnboardingGate was orphaned and never consulted. Changes: - communityOnboarding.tsx: add shouldSkipCommunityOnboarding(profile) pure helper; export for unit testing and reuse - App.tsx: replace the unconditional stage transition with an async getProfile() check; on hasProfileEvent markCommunityOnboardingComplete + clear the transaction (full skip); on no-event or error, proceed to 'profile' as before. A profileCheckTransactionRef guard prevents the async callback from firing twice if the effect re-runs - CommunityOnboardingFlow.tsx: seed displayName/avatarUrl from the relay profile when the profile step is shown (best-effort, errors silently ignored) - communityOnboarding.test.mjs: three new unit tests covering all three decision branches of shouldSkipCommunityOnboarding Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src/app/App.tsx | 39 +++++++++++++++-- .../onboarding/communityOnboarding.test.mjs | 42 +++++++++++++++++++ .../onboarding/communityOnboarding.tsx | 15 +++++++ .../onboarding/ui/CommunityOnboardingFlow.tsx | 26 +++++++++++- 4 files changed, 117 insertions(+), 5 deletions(-) diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index 56b84118ad..b0403c325d 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -20,6 +20,8 @@ import { useMachineOnboardingState } from "@/features/onboarding/machineOnboardi import { type FirstCommunityPage, useCommunityOnboarding, + markCommunityOnboardingComplete, + shouldSkipCommunityOnboarding, } from "@/features/onboarding/communityOnboarding"; import { CommunityOnboardingFlow } from "@/features/onboarding/ui/CommunityOnboardingFlow"; import { @@ -43,6 +45,7 @@ import { CommunityApplyErrorScreen } from "@/features/communities/ui/CommunityAp import { CommunityChangeOverlay } from "@/features/communities/ui/CommunityChangeOverlay"; import { createBuzzQueryClient } from "@/shared/api/queryClient"; import { isSharedIdentity as isSharedIdentityCmd } from "@/shared/api/tauri"; +import { getProfile } from "@/shared/api/tauriProfiles"; import { type AddCommunityDeepLinkPayload, listenForDeepLinks, @@ -279,6 +282,7 @@ function CommunityApp({ } = useCommunities(); const communityOnboarding = useCommunityOnboarding(); const connectingTransactionRef = useRef(null); + const profileCheckTransactionRef = useRef(null); const [isCommunityChangeOpen, setIsCommunityChangeOpen] = useState(false); const [resumeFirstCommunityPage, setResumeFirstCommunityPage] = useState(null); @@ -384,6 +388,7 @@ function CommunityApp({ useEffect(() => { if (transaction?.stage !== "connecting") { connectingTransactionRef.current = null; + profileCheckTransactionRef.current = null; } }, [transaction?.stage]); const targetIsReady = @@ -391,10 +396,36 @@ function CommunityApp({ community.isReady && community.appliedKey === communityKey; useEffect(() => { - if (transaction?.stage === "connecting" && targetIsReady) { - communityOnboarding.update({ stage: "profile", error: undefined }); - } - }, [communityOnboarding.update, targetIsReady, transaction?.stage]); + if (transaction?.stage !== "connecting" || !targetIsReady) return; + const transactionId = transaction.id; + const relayUrl = transaction.relayUrl; + if (profileCheckTransactionRef.current === transactionId) return; + profileCheckTransactionRef.current = transactionId; + void getProfile() + .then((profile) => { + if (shouldSkipCommunityOnboarding(profile)) { + markCommunityOnboardingComplete(profile.pubkey, relayUrl); + communityOnboarding.clear(); + } else { + communityOnboarding.update( + { stage: "profile", error: undefined }, + transactionId, + ); + } + }) + .catch(() => { + communityOnboarding.update( + { stage: "profile", error: undefined }, + transactionId, + ); + }); + }, [ + communityOnboarding, + targetIsReady, + transaction?.stage, + transaction?.id, + transaction?.relayUrl, + ]); // During "entering" the transaction stays alive as a curtain: the app mounts // underneath (already pointed at the Welcome channel route) while the // onboarding screen covers it, then fades once Welcome reports ready. diff --git a/desktop/src/features/onboarding/communityOnboarding.test.mjs b/desktop/src/features/onboarding/communityOnboarding.test.mjs index 8ffa59ec8e..983c3d1dad 100644 --- a/desktop/src/features/onboarding/communityOnboarding.test.mjs +++ b/desktop/src/features/onboarding/communityOnboarding.test.mjs @@ -5,6 +5,7 @@ import { clearCommunityOnboardingTransaction, loadCommunityOnboardingTransaction, markCommunityOnboardingComplete, + shouldSkipCommunityOnboarding, startCommunityOnboarding, updateCommunityOnboardingTransaction, updateCurrentCommunityOnboardingTransaction, @@ -147,3 +148,44 @@ test("completion is scoped by relay and pubkey and preserves legacy gate", () => ); assert.equal(storage.getItem("buzz-onboarding-complete.v1:pubkey"), "true"); }); + +// ── shouldSkipCommunityOnboarding ──────────────────────────────────────────── + +/** Minimal Profile stub — only the fields the helper inspects. */ +function makeProfile(hasProfileEvent, overrides = {}) { + return { + pubkey: "aabbcc", + displayName: null, + avatarUrl: null, + about: null, + nip05Handle: null, + ownerPubkey: null, + hasProfileEvent, + ...overrides, + }; +} + +test("shouldSkipCommunityOnboarding_hasProfileEvent_returnsTrue", () => { + assert.equal( + shouldSkipCommunityOnboarding(makeProfile(true)), + true, + "existing kind:0 ⇒ skip onboarding", + ); +}); + +test("shouldSkipCommunityOnboarding_noProfileEvent_returnsFalse", () => { + assert.equal( + shouldSkipCommunityOnboarding(makeProfile(false)), + false, + "no kind:0 ⇒ show profile step", + ); +}); + +test("shouldSkipCommunityOnboarding_fetchError_returnsFalse", () => { + // fetch error is represented as null — must never block onboarding + assert.equal( + shouldSkipCommunityOnboarding(null), + false, + "fetch error ⇒ show profile step (safe fallback)", + ); +}); diff --git a/desktop/src/features/onboarding/communityOnboarding.tsx b/desktop/src/features/onboarding/communityOnboarding.tsx index 76520b83be..20ee15fd31 100644 --- a/desktop/src/features/onboarding/communityOnboarding.tsx +++ b/desktop/src/features/onboarding/communityOnboarding.tsx @@ -3,6 +3,7 @@ import { normalizeRelayUrl, } from "@/features/communities/communityStorage"; import { setLocalStorageItemWithRecovery } from "@/shared/lib/localStorageQuota"; +import type { Profile } from "@/shared/api/types"; const STORAGE_KEY = "buzz-community-onboarding-transaction.v1"; @@ -227,6 +228,20 @@ export function markCommunityOnboardingComplete( storage.setItem(`buzz-onboarding-complete.v1:${pubkey}`, "true"); } +/** + * Returns true when a relay-profile check result means the user should skip + * community onboarding entirely and land directly in the app. + * + * A profile fetch error is represented as `null` and always returns false so + * that the fallback (show the profile step) applies — the skip must never + * block or strand onboarding. + */ +export function shouldSkipCommunityOnboarding( + profile: Profile | null, +): boolean { + return profile !== null && profile.hasProfileEvent === true; +} + import * as React from "react"; type CommunityOnboardingContextValue = { diff --git a/desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx b/desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx index 050aad96d0..10c194ce68 100644 --- a/desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx +++ b/desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx @@ -20,7 +20,7 @@ import { parseEmojiAvatarDataUrl, ProfileAvatarEditor, } from "@/features/profile/ui/ProfileAvatarEditor"; -import { updateProfile } from "@/shared/api/tauriProfiles"; +import { getProfile, updateProfile } from "@/shared/api/tauriProfiles"; import { getIdentity, importIdentity } from "@/shared/api/tauriIdentity"; import { listPersonas } from "@/shared/api/tauriPersonas"; import { relayClient } from "@/shared/api/relayClient"; @@ -286,6 +286,30 @@ export function CommunityOnboardingFlow({ transaction?.stage === "finalizing" || transaction?.stage === "entering"; + // Seed display name and avatar from the relay profile when the profile step + // is shown. This covers the case where the skip raced or was bypassed (e.g., + // the user navigated Back). Only seeds fields that are still empty so that + // any user edits are preserved. + React.useEffect(() => { + if (!isProfileStage) return; + void getProfile() + .then((profile) => { + if (profile.displayName) { + setDisplayName((prev) => + prev === "" ? (profile.displayName ?? "") : prev, + ); + } + if (profile.avatarUrl) { + setAvatarUrl((prev) => + prev === "" ? (profile.avatarUrl ?? "") : prev, + ); + } + }) + .catch(() => { + // Seeding is best-effort; silently ignore failures. + }); + }, [isProfileStage]); + React.useLayoutEffect(() => { if (isProfileStage && !isAvatarEditorOpen) { nameInputRef.current?.focus(); From 2366a7517e146ae2a0eff3088be06554470b6409 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Tue, 21 Jul 2026 15:12:32 -0700 Subject: [PATCH 2/4] fix(onboarding): guard profile-check skip against stale callbacks and timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two correctness issues flagged in review: 1. Stale-success guard: the connecting→profile transition now holds a live transactionRef (updated every render). Before completing onboarding the callback checks transactionRef.current?.id === transactionId && stage === 'connecting'. A cancelled or replaced transaction is a safe no-op. A 'settled' flag also ensures exactly one of {skip, show-profile} fires. 2. Timeout: getProfile() is now wrapped in a Promise.race via resolveProfileCheckAction (new export), which accepts a configurable scheduleTimeout for testability. Any hang produces show-profile after 10 s rather than stranding the user on the connecting screen. 3. Async orchestration tests for resolveProfileCheckAction cover all branches with controlled promises and a fake scheduler: has-event → skip with profile, no-event → show-profile, fetch rejects → show-profile, timeout → show-profile, late success after timeout → show-profile. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src/app/App.tsx | 45 ++++++--- .../onboarding/communityOnboarding.test.mjs | 97 +++++++++++++++++++ .../onboarding/communityOnboarding.tsx | 49 ++++++++++ mobile/pubspec.lock | 16 +-- 4 files changed, 184 insertions(+), 23 deletions(-) diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index b0403c325d..c1c224fd09 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -21,7 +21,7 @@ import { type FirstCommunityPage, useCommunityOnboarding, markCommunityOnboardingComplete, - shouldSkipCommunityOnboarding, + resolveProfileCheckAction, } from "@/features/onboarding/communityOnboarding"; import { CommunityOnboardingFlow } from "@/features/onboarding/ui/CommunityOnboardingFlow"; import { @@ -282,7 +282,14 @@ function CommunityApp({ } = useCommunities(); const communityOnboarding = useCommunityOnboarding(); const connectingTransactionRef = useRef(null); + // Tracks the ID of the profile-check request that has been launched for the + // current connecting transaction. Prevents the effect from launching a + // second request if it re-runs while a fetch is in flight. const profileCheckTransactionRef = useRef(null); + // Always reflects the live transaction object so async callbacks can perform + // an atomic check of both ID and stage before mutating state. + const transactionRef = useRef(communityOnboarding.transaction); + transactionRef.current = communityOnboarding.transaction; const [isCommunityChangeOpen, setIsCommunityChangeOpen] = useState(false); const [resumeFirstCommunityPage, setResumeFirstCommunityPage] = useState(null); @@ -401,24 +408,32 @@ function CommunityApp({ const relayUrl = transaction.relayUrl; if (profileCheckTransactionRef.current === transactionId) return; profileCheckTransactionRef.current = transactionId; - void getProfile() - .then((profile) => { - if (shouldSkipCommunityOnboarding(profile)) { - markCommunityOnboardingComplete(profile.pubkey, relayUrl); - communityOnboarding.clear(); - } else { - communityOnboarding.update( - { stage: "profile", error: undefined }, - transactionId, - ); - } - }) - .catch(() => { + + // Settled flag: ensures exactly one of {skip, show-profile} fires even if + // a timeout and a late success both resolve for the same request. + let settled = false; + + void resolveProfileCheckAction(getProfile, 10_000).then((result) => { + if (settled) return; + settled = true; + + // Atomic staleness guard: the transaction must still be the same one + // that launched this request AND must still be in the connecting stage. + // This covers: (a) user cancelled A and started B — B must not be + // cleared; (b) cancel without replacement — ref is null, guard fires. + const live = transactionRef.current; + if (live?.id !== transactionId || live.stage !== "connecting") return; + + if (result.action === "skip") { + markCommunityOnboardingComplete(result.profile.pubkey, relayUrl); + communityOnboarding.clear(); + } else { communityOnboarding.update( { stage: "profile", error: undefined }, transactionId, ); - }); + } + }); }, [ communityOnboarding, targetIsReady, diff --git a/desktop/src/features/onboarding/communityOnboarding.test.mjs b/desktop/src/features/onboarding/communityOnboarding.test.mjs index 983c3d1dad..10ef3c71d7 100644 --- a/desktop/src/features/onboarding/communityOnboarding.test.mjs +++ b/desktop/src/features/onboarding/communityOnboarding.test.mjs @@ -5,6 +5,7 @@ import { clearCommunityOnboardingTransaction, loadCommunityOnboardingTransaction, markCommunityOnboardingComplete, + resolveProfileCheckAction, shouldSkipCommunityOnboarding, startCommunityOnboarding, updateCommunityOnboardingTransaction, @@ -189,3 +190,99 @@ test("shouldSkipCommunityOnboarding_fetchError_returnsFalse", () => { "fetch error ⇒ show profile step (safe fallback)", ); }); + +// ── resolveProfileCheckAction — async orchestration ────────────────────────── + +/** + * Returns a fake scheduleTimeout that captures registered callbacks so tests + * can fire or skip them manually without real timers. + */ +function makeScheduler() { + const callbacks = []; + return { + schedule: (fn, _ms) => callbacks.push(fn), + fireTimeout: () => { + const fn = callbacks.shift(); + if (fn) fn(); + }, + pendingCount: () => callbacks.length, + }; +} + +test("resolveProfileCheckAction_hasProfileEvent_returnsSkipWithProfile", async () => { + const profile = makeProfile(true, { pubkey: "aabbcc" }); + const result = await resolveProfileCheckAction( + () => Promise.resolve(profile), + 10_000, + makeScheduler().schedule, + ); + assert.equal(result.action, "skip"); + assert.equal(result.profile.pubkey, "aabbcc"); +}); + +test("resolveProfileCheckAction_noProfileEvent_returnsShowProfile", async () => { + const result = await resolveProfileCheckAction( + () => Promise.resolve(makeProfile(false)), + 10_000, + makeScheduler().schedule, + ); + assert.equal(result.action, "show-profile"); +}); + +test("resolveProfileCheckAction_fetchRejects_returnsShowProfile", async () => { + const result = await resolveProfileCheckAction( + () => Promise.reject(new Error("network error")), + 10_000, + makeScheduler().schedule, + ); + assert.equal(result.action, "show-profile"); +}); + +test("resolveProfileCheckAction_timeout_returnsShowProfile", async () => { + // Fetch never settles; scheduler fires the timeout immediately. + const scheduler = makeScheduler(); + const result = await resolveProfileCheckAction( + () => new Promise(() => {}), // hangs forever + 10_000, + (fn, ms) => { + scheduler.schedule(fn, ms); + // Fire synchronously so the test does not wait for a real timer. + scheduler.fireTimeout(); + }, + ); + assert.equal( + result.action, + "show-profile", + "timeout ⇒ show-profile (never strands onboarding)", + ); +}); + +test("resolveProfileCheckAction_lateSuccessAfterTimeout_doesNotSkip", async () => { + // Fetch resolves AFTER the timeout has already fired. + // resolveProfileCheckAction must return show-profile from the timeout path, + // and the late fetch result must have no effect. + let resolveFetch; + const fetchPromise = new Promise((resolve) => { + resolveFetch = resolve; + }); + + const scheduler = makeScheduler(); + const resultPromise = resolveProfileCheckAction( + () => fetchPromise, + 10_000, + scheduler.schedule, + ); + + // Fire the timeout — race settles with the timeout rejection. + scheduler.fireTimeout(); + + // Now resolve the fetch with a kind:0 profile. + resolveFetch(makeProfile(true)); + + const result = await resultPromise; + assert.equal( + result.action, + "show-profile", + "late success after timeout must not complete onboarding", + ); +}); diff --git a/desktop/src/features/onboarding/communityOnboarding.tsx b/desktop/src/features/onboarding/communityOnboarding.tsx index 20ee15fd31..3f83c9d2d3 100644 --- a/desktop/src/features/onboarding/communityOnboarding.tsx +++ b/desktop/src/features/onboarding/communityOnboarding.tsx @@ -242,6 +242,55 @@ export function shouldSkipCommunityOnboarding( return profile !== null && profile.hasProfileEvent === true; } +/** + * Outcome of a profile-check attempt during the connecting → profile + * transition. Produced by `resolveProfileCheckAction`. + * + * - `{ action: "skip", profile }` — kind:0 exists; mark complete and enter + * the app. The resolved `Profile` is included so callers have the pubkey + * for `markCommunityOnboardingComplete` without a second fetch. + * - `{ action: "show-profile" }` — no kind:0, or the fetch failed / timed + * out; show the profile setup step. + */ +export type ProfileCheckAction = + | { action: "skip"; profile: Profile } + | { action: "show-profile" }; + +/** + * Runs a bounded profile fetch and returns the action to take at the + * `connecting → profile` transition. + * + * Accepts `fetchProfile` and `timeoutMs` as parameters so callers (and tests) + * can supply controlled implementations. The timeout promise uses the caller- + * supplied `setTimeout` so fake timers work in tests without touching globals. + * + * Any fetch error or timeout → `{ action: "show-profile" }` (never strands + * onboarding). + */ +export async function resolveProfileCheckAction( + fetchProfile: () => Promise, + timeoutMs: number, + scheduleTimeout: (fn: () => void, ms: number) => void = (fn, ms) => + void window.setTimeout(fn, ms), +): Promise { + try { + const profile = await Promise.race([ + fetchProfile(), + new Promise((_, reject) => + scheduleTimeout( + () => reject(new Error("profile-check-timeout")), + timeoutMs, + ), + ), + ]); + return shouldSkipCommunityOnboarding(profile) + ? { action: "skip", profile } + : { action: "show-profile" }; + } catch { + return { action: "show-profile" }; + } +} + import * as React from "react"; type CommunityOnboardingContextValue = { diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index dc062b8628..ef28d7e799 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -748,10 +748,10 @@ packages: dependency: transitive description: name: meta - sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" + sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349" url: "https://pub.dev" source: hosted - version: "1.17.0" + version: "1.18.0" mime: dependency: transitive description: @@ -1185,26 +1185,26 @@ packages: dependency: transitive description: name: test - sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" + sha256: "8d9ceddbab833f180fbefed08afa76d7c03513dfdba87ffcec2718b02bbcbf20" url: "https://pub.dev" source: hosted - version: "1.30.0" + version: "1.31.0" test_api: dependency: transitive description: name: test_api - sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" + sha256: "949a932224383300f01be9221c39180316445ecb8e7547f70a41a35bf421fb9e" url: "https://pub.dev" source: hosted - version: "0.7.10" + version: "0.7.11" test_core: dependency: transitive description: name: test_core - sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" + sha256: "1991d4cfe85d5043241acac92962c3977c8d2f2add1ee73130c7b286417d1d34" url: "https://pub.dev" source: hosted - version: "0.6.16" + version: "0.6.17" tuple: dependency: transitive description: From f836464a32670ddcae510f931667ffb1d075ecd9 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Tue, 21 Jul 2026 15:23:07 -0700 Subject: [PATCH 3/4] fix(onboarding): extract guard predicate, clear timeout, remove stale lockfile MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract isTransactionStillConnecting(live, transactionId) pure predicate from the inline guard in App.tsx. App.tsx now calls this named function, making the stale-result guard explicitly testable without mounting a component. - resolveProfileCheckAction: clear the timeout timer in finally so it does not linger after an immediate fetch success or failure. scheduleTimeout now returns ReturnType so clearTimeout has the handle. - Drop the redundant outer 'settled' flag from App.tsx: resolveProfileCheckAction resolves exactly once (Promise.race + timer cleared on settle) and the isTransactionStillConnecting guard is the correct single defense. - Add four unit tests for isTransactionStillConnecting: matching id+stage, replaced transaction, cancel without replacement, stage past connecting. - Restore mobile/pubspec.lock to origin/main — it was bumped by the mobile-fix pre-commit hook and is unrelated to this desktop-only change. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src/app/App.tsx | 22 +++----- .../onboarding/communityOnboarding.test.mjs | 56 +++++++++++++++++++ .../onboarding/communityOnboarding.tsx | 41 ++++++++++---- mobile/pubspec.lock | 16 +++--- 4 files changed, 104 insertions(+), 31 deletions(-) diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index c1c224fd09..e89bf61fbe 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -22,6 +22,7 @@ import { useCommunityOnboarding, markCommunityOnboardingComplete, resolveProfileCheckAction, + isTransactionStillConnecting, } from "@/features/onboarding/communityOnboarding"; import { CommunityOnboardingFlow } from "@/features/onboarding/ui/CommunityOnboardingFlow"; import { @@ -409,20 +410,15 @@ function CommunityApp({ if (profileCheckTransactionRef.current === transactionId) return; profileCheckTransactionRef.current = transactionId; - // Settled flag: ensures exactly one of {skip, show-profile} fires even if - // a timeout and a late success both resolve for the same request. - let settled = false; - + // resolveProfileCheckAction resolves exactly once (Promise.race + timer + // cleared on settle), so no settled flag is needed here. void resolveProfileCheckAction(getProfile, 10_000).then((result) => { - if (settled) return; - settled = true; - - // Atomic staleness guard: the transaction must still be the same one - // that launched this request AND must still be in the connecting stage. - // This covers: (a) user cancelled A and started B — B must not be - // cleared; (b) cancel without replacement — ref is null, guard fires. - const live = transactionRef.current; - if (live?.id !== transactionId || live.stage !== "connecting") return; + // Atomic staleness guard via isTransactionStillConnecting: the + // transaction must still be the same one that launched this request + // AND still be in connecting. Covers cancel+replacement (B's ID !== A's) + // and cancel-without-replacement (transactionRef.current is null). + if (!isTransactionStillConnecting(transactionRef.current, transactionId)) + return; if (result.action === "skip") { markCommunityOnboardingComplete(result.profile.pubkey, relayUrl); diff --git a/desktop/src/features/onboarding/communityOnboarding.test.mjs b/desktop/src/features/onboarding/communityOnboarding.test.mjs index 10ef3c71d7..e9645b191e 100644 --- a/desktop/src/features/onboarding/communityOnboarding.test.mjs +++ b/desktop/src/features/onboarding/communityOnboarding.test.mjs @@ -3,6 +3,7 @@ import test from "node:test"; import { clearCommunityOnboardingTransaction, + isTransactionStillConnecting, loadCommunityOnboardingTransaction, markCommunityOnboardingComplete, resolveProfileCheckAction, @@ -286,3 +287,58 @@ test("resolveProfileCheckAction_lateSuccessAfterTimeout_doesNotSkip", async () = "late success after timeout must not complete onboarding", ); }); + +// ── isTransactionStillConnecting — stale-transaction guard ─────────────────── + +/** + * Builds a minimal transaction stub for testing the guard predicate. + * Only `id` and `stage` are inspected by isTransactionStillConnecting. + */ +function makeTransaction(id, stage) { + return { + id, + stage, + source: "add-community", + relayUrl: "wss://relay.example", + communityName: "test", + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; +} + +test("isTransactionStillConnecting_matchingIdAndStage_returnsTrue", () => { + assert.equal( + isTransactionStillConnecting(makeTransaction("tx-a", "connecting"), "tx-a"), + true, + "same id + connecting stage → guard passes", + ); +}); + +test("isTransactionStillConnecting_replacedTransaction_returnsFalse", () => { + // Transaction B replaced A while the fetch was in flight. + // The callback for A must not clear B. + assert.equal( + isTransactionStillConnecting(makeTransaction("tx-b", "connecting"), "tx-a"), + false, + "different id (replacement) → guard rejects stale success", + ); +}); + +test("isTransactionStillConnecting_cancelWithNoReplacement_returnsFalse", () => { + // User cancelled without starting a new transaction; ref is null. + assert.equal( + isTransactionStillConnecting(null, "tx-a"), + false, + "null ref (cancel without replacement) → guard rejects stale success", + ); +}); + +test("isTransactionStillConnecting_stagePastConnecting_returnsFalse", () => { + // Fallback already advanced the transaction to 'profile' before the skip + // result arrived — double-completion must not occur. + assert.equal( + isTransactionStillConnecting(makeTransaction("tx-a", "profile"), "tx-a"), + false, + "stage advanced past connecting → guard rejects late action", + ); +}); diff --git a/desktop/src/features/onboarding/communityOnboarding.tsx b/desktop/src/features/onboarding/communityOnboarding.tsx index 3f83c9d2d3..bbb636b3a1 100644 --- a/desktop/src/features/onboarding/communityOnboarding.tsx +++ b/desktop/src/features/onboarding/communityOnboarding.tsx @@ -256,13 +256,28 @@ export type ProfileCheckAction = | { action: "skip"; profile: Profile } | { action: "show-profile" }; +/** + * Returns true when a live transaction snapshot still represents the + * same connecting request that launched the profile check. + * + * Extracted as a pure predicate so the stale-result guard in App.tsx can + * be unit-tested without mounting a component. + */ +export function isTransactionStillConnecting( + live: CommunityOnboardingTransaction | null | undefined, + transactionId: string, +): boolean { + return live?.id === transactionId && live.stage === "connecting"; +} + /** * Runs a bounded profile fetch and returns the action to take at the * `connecting → profile` transition. * - * Accepts `fetchProfile` and `timeoutMs` as parameters so callers (and tests) - * can supply controlled implementations. The timeout promise uses the caller- - * supplied `setTimeout` so fake timers work in tests without touching globals. + * Accepts `fetchProfile`, `timeoutMs`, and `scheduleTimeout` as parameters so + * callers (and tests) can supply controlled implementations. `scheduleTimeout` + * must return a cancellation handle (like `window.setTimeout`) so the timer + * can be cleared when the fetch settles before the deadline. * * Any fetch error or timeout → `{ action: "show-profile" }` (never strands * onboarding). @@ -270,17 +285,21 @@ export type ProfileCheckAction = export async function resolveProfileCheckAction( fetchProfile: () => Promise, timeoutMs: number, - scheduleTimeout: (fn: () => void, ms: number) => void = (fn, ms) => - void window.setTimeout(fn, ms), + scheduleTimeout: ( + fn: () => void, + ms: number, + ) => ReturnType = (fn, ms) => window.setTimeout(fn, ms), ): Promise { + let timerId: ReturnType | undefined; try { const profile = await Promise.race([ fetchProfile(), - new Promise((_, reject) => - scheduleTimeout( - () => reject(new Error("profile-check-timeout")), - timeoutMs, - ), + new Promise( + (_, reject) => + (timerId = scheduleTimeout( + () => reject(new Error("profile-check-timeout")), + timeoutMs, + )), ), ]); return shouldSkipCommunityOnboarding(profile) @@ -288,6 +307,8 @@ export async function resolveProfileCheckAction( : { action: "show-profile" }; } catch { return { action: "show-profile" }; + } finally { + if (timerId !== undefined) clearTimeout(timerId); } } diff --git a/mobile/pubspec.lock b/mobile/pubspec.lock index ef28d7e799..dc062b8628 100644 --- a/mobile/pubspec.lock +++ b/mobile/pubspec.lock @@ -748,10 +748,10 @@ packages: dependency: transitive description: name: meta - sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349" + sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" url: "https://pub.dev" source: hosted - version: "1.18.0" + version: "1.17.0" mime: dependency: transitive description: @@ -1185,26 +1185,26 @@ packages: dependency: transitive description: name: test - sha256: "8d9ceddbab833f180fbefed08afa76d7c03513dfdba87ffcec2718b02bbcbf20" + sha256: "280d6d890011ca966ad08df7e8a4ddfab0fb3aa49f96ed6de56e3521347a9ae7" url: "https://pub.dev" source: hosted - version: "1.31.0" + version: "1.30.0" test_api: dependency: transitive description: name: test_api - sha256: "949a932224383300f01be9221c39180316445ecb8e7547f70a41a35bf421fb9e" + sha256: "8161c84903fd860b26bfdefb7963b3f0b68fee7adea0f59ef805ecca346f0c7a" url: "https://pub.dev" source: hosted - version: "0.7.11" + version: "0.7.10" test_core: dependency: transitive description: name: test_core - sha256: "1991d4cfe85d5043241acac92962c3977c8d2f2add1ee73130c7b286417d1d34" + sha256: "0381bd1585d1a924763c308100f2138205252fb90c9d4eeaf28489ee65ccde51" url: "https://pub.dev" source: hosted - version: "0.6.17" + version: "0.6.16" tuple: dependency: transitive description: From 223f6ec3400e78804c1fe1f2346b1b0d918efa96 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Tue, 21 Jul 2026 15:52:46 -0700 Subject: [PATCH 4/4] test(e2e): fix stale deep-link-invite spec and add kind:0 skip coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The add-community spec was asserting pre-existing-profile behavior but the default mock identity (deadbeef...) is pre-seeded with has_profile_event:true. After the profile-skip feature landed, my new skip code fires correctly — but the spec expected the profile step to show, so it failed. Fix the existing test by passing profileReadError to force the error fallback path (no kind:0 => profile step shows). This preserves the test's original intent. Add a complementary test: identity WITH kind:0 + add-community deep link => transaction clears, app enters directly. This is the E2E regression test for the PR's actual skip behavior — previously uncovered. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/tests/e2e/deep-link-invite.spec.ts | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/desktop/tests/e2e/deep-link-invite.spec.ts b/desktop/tests/e2e/deep-link-invite.spec.ts index c3078aaca2..34f310d9ee 100644 --- a/desktop/tests/e2e/deep-link-invite.spec.ts +++ b/desktop/tests/e2e/deep-link-invite.spec.ts @@ -113,9 +113,15 @@ test("connect deep link shows a static acknowledgment during setup", async ({ test("add-community deep link starts onboarding when no community is configured", async ({ page, }) => { + // profileReadError forces the fallback path (error → profile step), so the + // test asserts pre-existing-profile behavior without the default mock + // identity's has_profile_event:true triggering the skip. await installMockBridge( page, - { pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK] }, + { + pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK], + profileReadError: "no-kind-0", + }, { skipCommunitySeed: true }, ); await page.goto("/"); @@ -142,6 +148,26 @@ test("add-community deep link starts onboarding when no community is configured" .toContain('"communityName":"Acme Team"'); }); +test("add-community deep link skips profile step when identity has an existing kind:0 profile", async ({ + page, +}) => { + // The default mock identity (deadbeef...) is pre-seeded with + // has_profile_event:true. The skip should fire on connecting → clear the + // transaction entirely, never showing the profile step. + await installMockBridge( + page, + { pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK] }, + { skipCommunitySeed: true }, + ); + await page.goto("/"); + + // Onboarding flow must disappear — the skip cleared the transaction. + await expect(page.getByTestId("community-onboarding-flow")).toHaveCount(0); + // handleCommunityOnboardingConnect already added the community when the + // transaction reached "connecting", so the app lands in the full UI. + await expect(page.getByTestId("sidebar-profile-avatar-button")).toBeVisible(); +}); + test("add-community deep link opens one editable prefill and acknowledges the queue", async ({ page, }) => {