From 298d3d16d8a5fee096ea132a2647399d366d9247 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 13 Feb 2026 20:54:27 +0000 Subject: [PATCH] fix: prevent auth prompts when signer is unavailable Three bugs fixed: 1. Race condition in emitState(): states$ was emitted before pendingChallenges$, so relay-state-manager's states$ subscriber would read stale pendingChallenges$.value. Now pendingChallenges$ is emitted first for consistent reads. 2. relay-state-manager only subscribed to states$, missing pendingChallenges$ changes. Now subscribes to both. 3. canAccountSign used constructor.name which is fragile under minification. Now uses account.type !== "readonly" (stable property from applesauce-accounts). Added 3 regression tests verifying pendingChallenges$.value consistency when observed from states$ subscribers. https://claude.ai/code/session_01XqrjeQVtJKw9uC1XAw6rqd --- .../src/__tests__/relay-auth-manager.test.ts | 93 +++++++++++++++++++ .../src/relay-auth-manager.ts | 10 +- src/hooks/useAccount.ts | 6 +- src/services/relay-state-manager.ts | 12 ++- 4 files changed, 112 insertions(+), 9 deletions(-) diff --git a/packages/relay-auth-manager/src/__tests__/relay-auth-manager.test.ts b/packages/relay-auth-manager/src/__tests__/relay-auth-manager.test.ts index cd93a72..5164b7a 100644 --- a/packages/relay-auth-manager/src/__tests__/relay-auth-manager.test.ts +++ b/packages/relay-auth-manager/src/__tests__/relay-auth-manager.test.ts @@ -677,6 +677,99 @@ describe("RelayAuthManager", () => { signer$.next(null); expect(manager.hasSignerAvailable()).toBe(false); }); + + it("pendingChallenges$.value should be consistent when states$ fires (no stale reads)", () => { + // This test simulates what relay-state-manager does: subscribes to states$ + // and reads pendingChallenges$.value inside the callback. The value must + // always reflect the current signer state (not stale data from a previous emission). + signer$.next(createMockSigner()); + + const relay = createMockRelay("wss://relay.example.com"); + relay.connected$.next(true); + manager.monitorRelay(relay); + relay.challenge$.next("test-challenge"); + + // Collect pendingChallenges$.value as seen from within states$ subscriber + const valuesSeenFromStates: number[] = []; + manager.states$.subscribe(() => { + valuesSeenFromStates.push(manager.pendingChallenges$.value.length); + }); + + // Clear tracking (ignore emissions from setup) + valuesSeenFromStates.length = 0; + + // Signer removed (logout) — when states$ fires, pendingChallenges$.value must be 0 + signer$.next(null); + + // Every states$ emission after signer removal should see empty pendingChallenges + expect(valuesSeenFromStates.length).toBeGreaterThan(0); + for (const count of valuesSeenFromStates) { + expect(count).toBe(0); + } + }); + + it("pendingChallenges$.value should be consistent when signer goes null→non-null→null", () => { + const relay = createMockRelay("wss://relay.example.com"); + relay.connected$.next(true); + manager.monitorRelay(relay); + relay.challenge$.next("test-challenge"); + + const valuesSeenFromStates: number[] = []; + manager.states$.subscribe(() => { + valuesSeenFromStates.push(manager.pendingChallenges$.value.length); + }); + + // Signer available then immediately removed (e.g., brief account switch) + valuesSeenFromStates.length = 0; + signer$.next(createMockSigner()); + signer$.next(null); + + // The last states$ emission should see 0 pending challenges + expect(valuesSeenFromStates[valuesSeenFromStates.length - 1]).toBe(0); + }); + + it("should never transiently expose stale challenges during signer removal", () => { + // Regression test: pendingChallenges$ must emit [] BEFORE states$ when signer goes null + signer$.next(createMockSigner()); + + const relay1 = createMockRelay("wss://relay1.example.com"); + const relay2 = createMockRelay("wss://relay2.example.com"); + relay1.connected$.next(true); + relay2.connected$.next(true); + manager.monitorRelay(relay1); + manager.monitorRelay(relay2); + relay1.challenge$.next("challenge-1"); + relay2.challenge$.next("challenge-2"); + + // Verify challenges are pending + expect(manager.pendingChallenges$.value).toHaveLength(2); + + // Track ALL emissions from both observables during signer removal + const emissions: Array<{ source: string; pendingCount: number }> = []; + manager.pendingChallenges$.subscribe((c) => { + emissions.push({ + source: "pendingChallenges$", + pendingCount: c.length, + }); + }); + manager.states$.subscribe(() => { + emissions.push({ + source: "states$", + pendingCount: manager.pendingChallenges$.value.length, + }); + }); + emissions.length = 0; + + // Remove signer + signer$.next(null); + + // pendingChallenges$ must emit before states$, so by the time states$ fires, + // pendingChallenges$.value is already 0 + const statesEmissions = emissions.filter((e) => e.source === "states$"); + for (const emission of statesEmissions) { + expect(emission.pendingCount).toBe(0); + } + }); }); describe("states$ observable", () => { diff --git a/packages/relay-auth-manager/src/relay-auth-manager.ts b/packages/relay-auth-manager/src/relay-auth-manager.ts index 0ae8e53..7083fdd 100644 --- a/packages/relay-auth-manager/src/relay-auth-manager.ts +++ b/packages/relay-auth-manager/src/relay-auth-manager.ts @@ -431,10 +431,9 @@ export class RelayAuthManager { } private emitState(): void { - // Emit a snapshot of current states - this.states$.next(new Map(this._relayStates)); - - // Derive and emit pending challenges + // Derive and emit pending challenges FIRST. + // This ensures that subscribers to states$ who read pendingChallenges$.value + // see consistent, up-to-date data (not stale values from a previous emission). const now = Date.now(); const challenges: PendingAuthChallenge[] = []; @@ -456,6 +455,9 @@ export class RelayAuthManager { } this.pendingChallenges$.next(challenges); + + // Emit states snapshot after pendingChallenges is updated + this.states$.next(new Map(this._relayStates)); } } diff --git a/src/hooks/useAccount.ts b/src/hooks/useAccount.ts index 4e25797..de3d84c 100644 --- a/src/hooks/useAccount.ts +++ b/src/hooks/useAccount.ts @@ -6,13 +6,15 @@ import accounts from "@/services/accounts"; * Check if an account can sign events * Read-only accounts cannot sign and should not be prompted for auth * + * Uses account.type (from applesauce-accounts) instead of constructor.name + * to be robust against minification. + * * @param account - The account to check (can be undefined) * @returns true if the account can sign, false otherwise */ export function canAccountSign(account: typeof accounts.active): boolean { if (!account) return false; - const accountType = account.constructor.name; - return accountType !== "ReadonlyAccount"; + return account.type !== "readonly"; } /** diff --git a/src/services/relay-state-manager.ts b/src/services/relay-state-manager.ts index 3269859..6254a80 100644 --- a/src/services/relay-state-manager.ts +++ b/src/services/relay-state-manager.ts @@ -50,11 +50,17 @@ class RelayStateManager { this.initialized = true; - // Subscribe to auth manager state changes to re-notify listeners - const authSub = relayAuthManager.states$.subscribe(() => { + // Subscribe to auth manager state and pending challenge changes + const stateSub = relayAuthManager.states$.subscribe(() => { this.notifyListeners(); }); - this.authUnsubscribe = () => authSub.unsubscribe(); + const challengeSub = relayAuthManager.pendingChallenges$.subscribe(() => { + this.notifyListeners(); + }); + this.authUnsubscribe = () => { + stateSub.unsubscribe(); + challengeSub.unsubscribe(); + }; // Subscribe to existing relays pool.relays.forEach((relay) => {