diff --git a/desktop/src/app/App.tsx b/desktop/src/app/App.tsx index 56b84118ad..e89bf61fbe 100644 --- a/desktop/src/app/App.tsx +++ b/desktop/src/app/App.tsx @@ -20,6 +20,9 @@ import { useMachineOnboardingState } from "@/features/onboarding/machineOnboardi import { type FirstCommunityPage, useCommunityOnboarding, + markCommunityOnboardingComplete, + resolveProfileCheckAction, + isTransactionStillConnecting, } from "@/features/onboarding/communityOnboarding"; import { CommunityOnboardingFlow } from "@/features/onboarding/ui/CommunityOnboardingFlow"; import { @@ -43,6 +46,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 +283,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); @@ -384,6 +396,7 @@ function CommunityApp({ useEffect(() => { if (transaction?.stage !== "connecting") { connectingTransactionRef.current = null; + profileCheckTransactionRef.current = null; } }, [transaction?.stage]); const targetIsReady = @@ -391,10 +404,39 @@ 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; + + // resolveProfileCheckAction resolves exactly once (Promise.race + timer + // cleared on settle), so no settled flag is needed here. + void resolveProfileCheckAction(getProfile, 10_000).then((result) => { + // 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); + communityOnboarding.clear(); + } else { + 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..e9645b191e 100644 --- a/desktop/src/features/onboarding/communityOnboarding.test.mjs +++ b/desktop/src/features/onboarding/communityOnboarding.test.mjs @@ -3,8 +3,11 @@ import test from "node:test"; import { clearCommunityOnboardingTransaction, + isTransactionStillConnecting, loadCommunityOnboardingTransaction, markCommunityOnboardingComplete, + resolveProfileCheckAction, + shouldSkipCommunityOnboarding, startCommunityOnboarding, updateCommunityOnboardingTransaction, updateCurrentCommunityOnboardingTransaction, @@ -147,3 +150,195 @@ 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)", + ); +}); + +// ── 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", + ); +}); + +// ── 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 76520b83be..bbb636b3a1 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,90 @@ 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; +} + +/** + * 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" }; + +/** + * 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`, `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). + */ +export async function resolveProfileCheckAction( + fetchProfile: () => Promise, + timeoutMs: number, + 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) => + (timerId = scheduleTimeout( + () => reject(new Error("profile-check-timeout")), + timeoutMs, + )), + ), + ]); + return shouldSkipCommunityOnboarding(profile) + ? { action: "skip", profile } + : { action: "show-profile" }; + } catch { + return { action: "show-profile" }; + } finally { + if (timerId !== undefined) clearTimeout(timerId); + } +} + 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(); 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, }) => {