From b774de6d76b06f507a6fbef760638f44ef144451 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Sun, 26 Jul 2026 17:59:50 -0400 Subject: [PATCH] Harden profile data loading --- js/subscriptions.js | 80 ++++++--------- js/subscriptionsHelpers.js | 4 + .../ProfileDirectMessageActions.js | 6 +- .../ProfilePlaylistsController.js | 27 +++-- tests/dm-profile-snapshot-render.test.mjs | 45 +++++++++ ...file-playlists-progressive-render.test.mjs | 35 +++++++ tests/subscriptions-manager.test.mjs | 99 +++++++++++++++++++ 7 files changed, 237 insertions(+), 59 deletions(-) create mode 100644 tests/dm-profile-snapshot-render.test.mjs create mode 100644 tests/profile-playlists-progressive-render.test.mjs diff --git a/js/subscriptions.js b/js/subscriptions.js index 96a271ab..f1f6243f 100644 --- a/js/subscriptions.js +++ b/js/subscriptions.js @@ -180,6 +180,7 @@ class SubscriptionsManager { this.subscribedPubkeys = new Set(); this.subsEventId = null; this.subsEventCreatedAt = null; + this.subsEventPubkey = null; // Last live-subscription event id we triggered a refresh for (loop guard). this._lastHandledListEventId = null; // Most recently fetched list ciphertext, reused by decrypt retries so they @@ -247,6 +248,7 @@ class SubscriptionsManager { this.subscribedPubkeys = new Set(cachedSnapshot.subscribedPubkeys); this.subsEventId = cachedSnapshot.eventId; this.subsEventCreatedAt = cachedSnapshot.createdAt; + this.subsEventPubkey = cachedSnapshot.eventPubkey; this.currentUserPubkey = normalizedUserPubkey; this.uiReady = true; this.dataReady = true; @@ -281,6 +283,7 @@ class SubscriptionsManager { subscribedPubkeys: Array.from(this.subscribedPubkeys), eventId: this.subsEventId, createdAt: this.subsEventCreatedAt, + eventPubkey: this.subsEventPubkey, }); } @@ -389,63 +392,39 @@ class SubscriptionsManager { profileCache.getProfileData(normalizedUserPubkey, "subscriptions"), ); hadCachedSnapshot = cachedSnapshot.hasSnapshot; - const shouldForceFullFetch = !cachedSnapshot.hasSnapshot; + // Legacy snapshots did not record the source author. Fetch their full + // authoritative state once so a cache created by an older buggy build + // cannot keep suppressing the real list with an invalid timestamp. + const shouldForceFullFetch = + !cachedSnapshot.hasSnapshot || + cachedSnapshot.eventPubkey !== normalizedUserPubkey; const incrementalSince = shouldForceFullFetch ? 0 : computeIncrementalSinceWithOverlap(cachedSnapshot.createdAt, 1); - // Fetch user and session actor subscription lists in parallel to reduce - // total relay wait time during login. - const fetchPromises = [ - nostrClient.fetchListIncrementally({ - kind: SUBSCRIPTION_SET_KIND, - pubkey: normalizedUserPubkey, - dTag: SUBSCRIPTION_LIST_IDENTIFIER, - relayUrls, - since: incrementalSince, - timeoutMs: 12000, - }), - ]; - - const normalizedSessionActorPubkey = normalizeNostrPubkey( - nostrClient?.sessionActor?.pubkey, - ); - const shouldFetchSessionActor = - normalizedSessionActorPubkey && - normalizedSessionActorPubkey !== normalizedUserPubkey; - if (shouldFetchSessionActor) { - const sessionCachedSnapshot = parseCachedSubscriptionSnapshot( - profileCache.getProfileData( - normalizedSessionActorPubkey, - "subscriptions", - ), - ); - const shouldForceSessionFetch = !sessionCachedSnapshot.hasSnapshot; - const sessionSince = shouldForceSessionFetch - ? 0 - : computeIncrementalSinceWithOverlap(sessionCachedSnapshot.createdAt, 1); - fetchPromises.push( - nostrClient.fetchListIncrementally({ - kind: SUBSCRIPTION_SET_KIND, - pubkey: normalizedSessionActorPubkey, - dTag: SUBSCRIPTION_LIST_IDENTIFIER, - relayUrls, - since: sessionSince, - timeoutMs: 12000, - }), - ); - } - - const fetchResults = await Promise.all(fetchPromises); - let events = fetchResults[0] || []; - if (shouldFetchSessionActor && fetchResults[1]?.length) { - events = events.concat(fetchResults[1]); - } + // Subscription lists are private, account-scoped state. In particular, + // a BitLogin session actor can differ from the active profile pubkey. + // Never mix its events into this read: choosing the newest event across + // both accounts could overwrite the active account's subscriptions. + const events = await nostrClient.fetchListIncrementally({ + kind: SUBSCRIPTION_SET_KIND, + pubkey: normalizedUserPubkey, + dTag: SUBSCRIPTION_LIST_IDENTIFIER, + relayUrls, + since: incrementalSince, + timeoutMs: 12000, + }); if (events.length) { const deduped = new Map(); for (const event of events) { - if (!event || typeof event !== "object" || !event.id) { + if ( + !event || + typeof event !== "object" || + !event.id || + (event.pubkey && + normalizeHexPubkey(event.pubkey) !== normalizedUserPubkey) + ) { continue; } const existing = deduped.get(event.id); @@ -471,6 +450,7 @@ class SubscriptionsManager { this.subscribedPubkeys.clear(); this.subsEventId = null; this.subsEventCreatedAt = null; + this.subsEventPubkey = null; this.uiReady = true; this.dataReady = true; this.loaded = true; @@ -513,6 +493,7 @@ class SubscriptionsManager { this.subsEventCreatedAt = Number.isFinite(newest.created_at) ? newest.created_at : null; + this.subsEventPubkey = normalizedUserPubkey; let decryptResult; try { @@ -687,6 +668,7 @@ class SubscriptionsManager { this.subscribedPubkeys.clear(); this.subsEventId = null; this.subsEventCreatedAt = null; + this.subsEventPubkey = null; this._lastHandledListEventId = null; this._pendingDecryptEvent = null; this.currentUserPubkey = null; diff --git a/js/subscriptionsHelpers.js b/js/subscriptionsHelpers.js index 8ae47e84..dfd243cb 100644 --- a/js/subscriptionsHelpers.js +++ b/js/subscriptionsHelpers.js @@ -196,6 +196,7 @@ export function parseCachedSubscriptionSnapshot(cached) { subscribedPubkeys: normalized, eventId: null, createdAt: null, + eventPubkey: null, hasSnapshot: true, }; } @@ -220,6 +221,7 @@ export function parseCachedSubscriptionSnapshot(cached) { const createdAt = Number.isFinite(createdAtCandidate) ? Math.floor(createdAtCandidate) : null; + const eventPubkey = normalizeHexPubkey(cached.eventPubkey); const hasSnapshot = Array.isArray(cached.subscribedPubkeys) || Array.isArray(cached.subPubkeys) || @@ -229,6 +231,7 @@ export function parseCachedSubscriptionSnapshot(cached) { subscribedPubkeys: normalized, eventId, createdAt, + eventPubkey, hasSnapshot, }; } @@ -237,6 +240,7 @@ export function parseCachedSubscriptionSnapshot(cached) { subscribedPubkeys: [], eventId: null, createdAt: null, + eventPubkey: null, hasSnapshot: false, }; } diff --git a/js/ui/profileModal/ProfileDirectMessageActions.js b/js/ui/profileModal/ProfileDirectMessageActions.js index a25e350c..13098c30 100644 --- a/js/ui/profileModal/ProfileDirectMessageActions.js +++ b/js/ui/profileModal/ProfileDirectMessageActions.js @@ -753,7 +753,11 @@ export class ProfileDirectMessageActions { handleDirectMessagesUpdated(detail = {}) { if ( this.controller.activeMessagesRequest && - detail?.reason !== "load-incremental" + detail?.reason !== "load-incremental" && + // A persisted, locally encrypted snapshot is safe to show while the + // slower relay history/decryption pass is still running. Suppressing it + // made the DM pane appear blank until that full pass completed. + detail?.reason !== "snapshot" ) { return; } diff --git a/js/ui/profileModal/ProfilePlaylistsController.js b/js/ui/profileModal/ProfilePlaylistsController.js index 8380f19b..5e79cb20 100644 --- a/js/ui/profileModal/ProfilePlaylistsController.js +++ b/js/ui/profileModal/ProfilePlaylistsController.js @@ -79,6 +79,8 @@ export class ProfilePlaylistsController { this.playlists = []; this.expanded = new Set(); this.loading = false; + this.refreshRequestId = 0; + this.fetchCreatorPlaylists = fetchCreatorPlaylists; this._drag = null; this._dropTarget = null; this._onChanged = () => { @@ -139,16 +141,22 @@ export class ProfilePlaylistsController { return; } + const requestId = ++this.refreshRequestId; this.setLoading(true); let playlists = []; try { - playlists = await fetchCreatorPlaylists(this.pubkey, { + playlists = await this.fetchCreatorPlaylists(this.pubkey, { includeEmpty: true, }); } catch (error) { devLogger.warn("[playlists] Failed to load playlists for pane:", error); } this.playlists = playlists; + this.setLoading(false); + // Show the playlist structure as soon as the list itself is available. + // Resolving every referenced video only improves labels/thumbnails and + // must not keep the management pane blank on a cold cache. + this.render(); // Fetch the referenced creators' videos so the expanded sublists can show // real titles/thumbnails (cache may be cold in the modal). @@ -165,15 +173,16 @@ export class ProfilePlaylistsController { } const nostrService = this.mainController.services?.nostrService; if (authors.size && typeof nostrService?.fetchVideosByAuthors === "function") { - try { - await nostrService.fetchVideosByAuthors([...authors]); - } catch (error) { - // best effort — fall back to coordinate labels - } + void nostrService.fetchVideosByAuthors([...authors]) + .catch(() => { + // Best effort — keep the coordinate labels if metadata is unavailable. + }) + .then(() => { + if (this.refreshRequestId === requestId) { + this.render(); + } + }); } - - this.setLoading(false); - this.render(); } // coord -> { title, thumbnail } from the app's active videos. diff --git a/tests/dm-profile-snapshot-render.test.mjs b/tests/dm-profile-snapshot-render.test.mjs new file mode 100644 index 00000000..4f036f29 --- /dev/null +++ b/tests/dm-profile-snapshot-render.test.mjs @@ -0,0 +1,45 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { ProfileDirectMessageActions } from "../js/ui/profileModal/ProfileDirectMessageActions.js"; + +test("DM pane renders a persisted snapshot while the relay load is still active", () => { + const scheduled = []; + const actor = "a".repeat(64); + const controller = { + activeMessagesRequest: Symbol("relay-load"), + directMessagesCache: [], + directMessagesLastActor: null, + helper: { resolveActiveDmActor: () => actor }, + renderer: { + scheduleDirectMessagesRender: (detail) => scheduled.push(detail), + setMessagesLoadingState: () => {}, + }, + }; + const actions = new ProfileDirectMessageActions({}, controller); + const messages = [{ event: { id: "cached-message" } }]; + + actions.handleDirectMessagesUpdated({ messages, reason: "snapshot" }); + + assert.deepEqual(controller.directMessagesCache, messages); + assert.equal(controller.directMessagesLastActor, actor); + assert.deepEqual(scheduled, [{ messages, actorPubkey: actor, reason: "snapshot" }]); +}); + +test("DM pane waits for relay increments but ignores unrelated updates during a load", () => { + const controller = { + activeMessagesRequest: Symbol("relay-load"), + directMessagesCache: [], + directMessagesLastActor: null, + helper: { resolveActiveDmActor: () => "a".repeat(64) }, + renderer: { + scheduleDirectMessagesRender: () => assert.fail("unrelated update must not render"), + setMessagesLoadingState: () => {}, + }, + }; + const actions = new ProfileDirectMessageActions({}, controller); + + actions.handleDirectMessagesUpdated({ messages: [{ event: { id: "other" } }], reason: "load" }); + + assert.deepEqual(controller.directMessagesCache, []); +}); diff --git a/tests/profile-playlists-progressive-render.test.mjs b/tests/profile-playlists-progressive-render.test.mjs new file mode 100644 index 00000000..c384ec0c --- /dev/null +++ b/tests/profile-playlists-progressive-render.test.mjs @@ -0,0 +1,35 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { ProfilePlaylistsController } from "../js/ui/profileModal/ProfilePlaylistsController.js"; + +test("playlist refresh renders the list before deferred video metadata completes", async () => { + const controller = new ProfilePlaylistsController({ + getActivePubkey: () => "a".repeat(64), + services: { + nostrService: { + fetchVideosByAuthors: () => new Promise(() => {}), + }, + }, + }); + const renders = []; + let resolveMetadata; + controller.listEl = {}; + controller.setLoading = () => {}; + controller.render = () => renders.push(controller.playlists.map((playlist) => playlist.id)); + controller.fetchCreatorPlaylists = async () => [{ + id: "existing", + items: [{ type: "a", value: `30078:${"b".repeat(64)}:video` }], + }]; + controller.mainController.services.nostrService.fetchVideosByAuthors = () => + new Promise((resolve) => { + resolveMetadata = resolve; + }); + + await controller.refresh(); + assert.deepEqual(renders, [["existing"]], "list renders before metadata request settles"); + + resolveMetadata(); + await new Promise((resolve) => setTimeout(resolve, 0)); + assert.deepEqual(renders, [["existing"], ["existing"]], "metadata completion refreshes labels later"); +}); diff --git a/tests/subscriptions-manager.test.mjs b/tests/subscriptions-manager.test.mjs index 5a7dffa4..91398a33 100644 --- a/tests/subscriptions-manager.test.mjs +++ b/tests/subscriptions-manager.test.mjs @@ -1324,6 +1324,7 @@ test("loadSubscriptions uses overlap window and deterministic latest-event tie-b subscribedPubkeys: [oldAuthor], createdAt: 1000, eventId: "cached-event", + eventPubkey: pubkey, }); nostrClient.fetchListIncrementally = async (params = {}) => { @@ -1378,6 +1379,104 @@ test("loadSubscriptions uses overlap window and deterministic latest-event tie-b } }); +test("loadSubscriptions never applies a different BitLogin session actor's list", async () => { + const SubscriptionsManager = subscriptions.constructor; + const manager = new SubscriptionsManager(); + const activePubkey = "a".repeat(64); + const sessionPubkey = "b".repeat(64); + const activeAuthor = "c".repeat(64); + const sessionAuthor = "d".repeat(64); + const originalFetchIncremental = nostrClient.fetchListIncrementally; + const originalSessionActor = nostrClient.sessionActor; + const originalPool = nostrClient.pool; + const originalDecryptSubscriptionEvent = manager.decryptSubscriptionEvent; + + nostrClient.sessionActor = { pubkey: sessionPubkey }; + nostrClient.pool = { list: async () => [] }; + nostrClient.fetchListIncrementally = async ({ pubkey }) => { + assert.equal(pubkey, activePubkey, "only the active account may be fetched"); + return [ + { + id: "active-list", + pubkey: activePubkey, + created_at: 100, + content: "active-ciphertext", + tags: [["encrypted", "nip04"]], + }, + { + id: "foreign-list", + pubkey: sessionPubkey, + created_at: 200, + content: "foreign-ciphertext", + tags: [["encrypted", "nip04"]], + }, + ]; + }; + manager.decryptSubscriptionEvent = async (event) => ({ + ok: true, + plaintext: JSON.stringify([["p", event.id === "active-list" ? activeAuthor : sessionAuthor]]), + }); + + try { + await manager.updateFromRelays(activePubkey, { allowPermissionPrompt: false }); + assert.equal(manager.subsEventId, "active-list"); + assert.deepEqual(manager.getSubscribedAuthors(), [activeAuthor]); + } finally { + nostrClient.fetchListIncrementally = originalFetchIncremental; + nostrClient.sessionActor = originalSessionActor; + nostrClient.pool = originalPool; + manager.decryptSubscriptionEvent = originalDecryptSubscriptionEvent; + localStorage.clear(); + profileCache.memoryCache.clear(); + profileCache.activePubkey = null; + } +}); + +test("loadSubscriptions repairs a legacy cached list with a full active-account fetch", async () => { + const SubscriptionsManager = subscriptions.constructor; + const manager = new SubscriptionsManager(); + const activePubkey = "e".repeat(64); + const staleAuthor = "f".repeat(64); + const correctAuthor = "1".repeat(64); + const originalFetchIncremental = nostrClient.fetchListIncrementally; + const originalPool = nostrClient.pool; + const originalDecryptSubscriptionEvent = manager.decryptSubscriptionEvent; + + profileCache.setProfileData(activePubkey, "subscriptions", { + subscribedPubkeys: [staleAuthor], + eventId: "legacy-foreign-event", + createdAt: 9999, + }); + nostrClient.pool = { list: async () => [] }; + nostrClient.fetchListIncrementally = async (params) => { + assert.equal(params.since, 0, "legacy source-less cache must not suppress a full fetch"); + return [{ + id: "correct-active-event", + pubkey: activePubkey, + created_at: 100, + content: "correct-ciphertext", + tags: [["encrypted", "nip04"]], + }]; + }; + manager.decryptSubscriptionEvent = async () => ({ + ok: true, + plaintext: JSON.stringify([["p", correctAuthor]]), + }); + + try { + await manager.updateFromRelays(activePubkey, { allowPermissionPrompt: false }); + assert.deepEqual(manager.getSubscribedAuthors(), [correctAuthor]); + assert.equal(manager.subsEventPubkey, activePubkey); + } finally { + nostrClient.fetchListIncrementally = originalFetchIncremental; + nostrClient.pool = originalPool; + manager.decryptSubscriptionEvent = originalDecryptSubscriptionEvent; + localStorage.clear(); + profileCache.memoryCache.clear(); + profileCache.activePubkey = null; + } +}); + test("handleSubscriptionListEvent does not re-trigger on the live-sub echo (loop guard)", () => { const SubscriptionsManager = subscriptions.constructor; const manager = new SubscriptionsManager();