fix(relay-reconnect): address Wes's CHANGES_REQUESTED findings

Finding 1 (relayReconnectController.ts): subscribeToConnectionState can invoke
its listener synchronously with the current state before returning. When that
sync emission signals connected, onConnected() → finish() → cancelTimers() runs
while unsubscribeConnectionState is still null — then the caller assigns all
three handles after finish, leaking them for the app lifetime.

Fix: subscribe first, then check resolved immediately after the return value is
assigned. If resolved, unsubscribe the handle (which cancelTimers couldn't reach
because the assignment hadn't happened yet) and return before installing the
poll interval and backstop timer.

Finding 2 (ProfileStep.tsx): reconnect() always returns false in phase 3, so
runConnectivityAction never calls markSuccess(), and the onboarding component
had no connection-state subscription. The error card stayed showing failure
even after the relay healed.

Fix: observe the shared relayConnectivitySuccess store (already used by the
sidebar) via useSyncExternalStore + useEffect in OnboardingRelayConnectionErrorCard.
Export subscribeRelayConnectivitySuccess and getRelayConnectivitySuccessSnapshot
from useSidebarRelayConnectionCard.ts so both surfaces share exactly one
success-signalling mechanism.

Test: add a sync-emission unit test with a subscribeToConnectionState fake that
calls the listener before returning its cleanup handle — asserts onSuccess fires
once, no interval/backstop installed, cleanup called.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

desktop/src/features/onboarding/ui/ProfileStep.tsx

@@ -2,6 +2,10 @@ import * as React from "react";
 import { toast } from "sonner";
 
 import { SidebarRelayConnectionCompactCard } from "@/features/sidebar/ui/SidebarRelayConnectionCard";
+import {
+  getRelayConnectivitySuccessSnapshot,
+  subscribeRelayConnectivitySuccess,
+} from "@/features/sidebar/ui/useSidebarRelayConnectionCard";
 import { useReconnectRelay } from "@/shared/api/useReconnectRelay";
 import { cn } from "@/shared/lib/cn";
 import { isRelayUnreachableError } from "@/shared/lib/relayError";
@@ -46,6 +50,16 @@ function OnboardingRelayConnectionErrorCard({
   const wasSavingRef = React.useRef(isSaving);
   const isActionPending = isReconnectActionPending || isReconnectPending;
 
+  // Observe the shared relay-connectivity-success store. When the relay
+  // recovers during phase 3 (controller reconnects without a click return
+  // value), the sidebar and this card share the same success signal so the
+  // onboarding card also flips to success state.
+  const relayConnectivitySuccess = React.useSyncExternalStore(
+    subscribeRelayConnectivitySuccess,
+    () => getRelayConnectivitySuccessSnapshot(undefined),
+    () => false,
+  );
+
   React.useEffect(() => {
     return () => {
       if (successTimeoutRef.current !== null) {
@@ -77,6 +91,16 @@ function OnboardingRelayConnectionErrorCard({
     }, ONBOARDING_CONNECTIVITY_SUCCESS_AUTO_DISMISS_MS);
   }, [message]);
 
+  // When the shared relay-connectivity-success store signals success (set by
+  // either the sidebar's handleReconnectRelay or the auto-heal path), mark
+  // this card as successful too. This covers the phase-3 case where
+  // reconnect() returns false but the relay later becomes reachable.
+  React.useEffect(() => {
+    if (relayConnectivitySuccess) {
+      markSuccess();
+    }
+  }, [relayConnectivitySuccess, markSuccess]);
+
   const runConnectivityAction = React.useCallback(
     (runAction: () => Promise<boolean | undefined>) => {
       if (reconnectActionPendingRef.current) {

desktop/src/features/sidebar/ui/useSidebarRelayConnectionCard.ts

@@ -14,12 +14,12 @@ function relaySuccessKey(relayUrl: string | null | undefined) {
   return relayUrl ?? DEFAULT_RELAY_SUCCESS_KEY;
 }
 
-function subscribeRelayConnectivitySuccess(listener: () => void) {
+export function subscribeRelayConnectivitySuccess(listener: () => void) {
   relayConnectivitySuccessListeners.add(listener);
   return () => relayConnectivitySuccessListeners.delete(listener);
 }
 
-function getRelayConnectivitySuccessSnapshot(
+export function getRelayConnectivitySuccessSnapshot(
   relayUrl: string | null | undefined,
 ) {
   return relayConnectivitySuccessKey === relaySuccessKey(relayUrl);

desktop/src/shared/api/relayReconnectController.test.mjs

@@ -469,3 +469,48 @@ test("OSS build (hookConfigured returns false) — runHook never called", async
     "runHook not called in OSS build",
   );
 });
+
+// ── Synchronous connected emission ───────────────────────────────────────────
+
+test("sync connected emission — onSuccess fires once, no interval/backstop installed, subscription cleaned up", async () => {
+  const ctrl = new RelayReconnectController();
+
+  // subscribeToConnectionState fake that invokes the listener synchronously
+  // with "connected" BEFORE returning the cleanup handle. This models the
+  // production subscribeToConnectionState documented behaviour: it fires the
+  // listener with the current state before returning.
+  let cleanupCalled = false;
+  const deps = makeDeps({
+    preconnectResult: async () => {
+      throw new Error("relay unreachable");
+    },
+    hookConfiguredResult: async () => false,
+  });
+
+  // Override subscribeToConnectionState with the synchronous-emission fake.
+  deps.subscribeToConnectionState = mock.fn((listener) => {
+    // Invoke immediately — simulates "already connected" at subscribe time.
+    listener("connected");
+    // Return cleanup handle (production unsubscribe fn).
+    const cleanup = () => {
+      cleanupCalled = true;
+    };
+    return cleanup;
+  });
+
+  await ctrl.start(deps);
+
+  assert.equal(
+    deps.onSuccess.mock.calls.length,
+    1,
+    "onSuccess fires exactly once",
+  );
+  assert.equal(deps._intervals.length, 0, "no poll interval installed");
+  assert.equal(deps._timers.length, 0, "no backstop timer installed");
+  assert.equal(cleanupCalled, true, "subscription cleanup handle was called");
+  assert.deepEqual(
+    ctrl.getState(),
+    { isPending: false, isWaitingOnReconnectHook: false },
+    "state is idle after sync success",
+  );
+});

desktop/src/shared/api/relayReconnectController.ts

@@ -194,12 +194,30 @@ export class RelayReconnectController {
       this.finish(onSuccess, true);
     };
 
+    // Subscribe FIRST. subscribeToConnectionState may invoke the listener
+    // synchronously with the current state (documented on the production
+    // implementation). If that sync emission resolves us, finish() runs and
+    // cancelTimers() cleans up the subscription handle before we reach the
+    // timer-install lines below.
     this.unsubscribeConnectionState = deps.subscribeToConnectionState(
       (state) => {
         if (state === "connected") onConnected();
       },
     );
 
+    // If the sync emission already finished the attempt, don't install timers
+    // that would live for the app lifetime. Also, cancelTimers() ran inside
+    // finish() before the return value of subscribeToConnectionState was
+    // assigned, so the subscription handle may not have been cleaned up yet —
+    // call unsubscribe now if it is still set.
+    if (resolved) {
+      if (this.unsubscribeConnectionState !== null) {
+        this.unsubscribeConnectionState();
+        this.unsubscribeConnectionState = null;
+      }
+      return false;
+    }
+
     this.pollIntervalId = deps.setInterval(() => {
       if (resolved || cancelled()) return;
       void deps