diff --git a/desktop/src/features/messages/ui/MessageComposer.tsx b/desktop/src/features/messages/ui/MessageComposer.tsx index d13cec95eba..022d60b4d1e 100644 --- a/desktop/src/features/messages/ui/MessageComposer.tsx +++ b/desktop/src/features/messages/ui/MessageComposer.tsx @@ -57,7 +57,7 @@ import { usePersistentAgentMentionHydration } from "./usePersistentAgentMentionH import { useComposerContentState } from "./useComposerContentState"; import { useDraftPersistLifecycle } from "./useDraftPersistSnapshot"; import { submitMessageEdit } from "./submitMessageEdit"; -import { useComposerLinkPreviews } from "./useComposerLinkPreviews"; +import { useManagedComposerLinkPreviews } from "./useComposerLinkPreviews"; import { scheduleSettleGatedAutoSubmit } from "./messageComposerAutoSubmit"; import type { MessageComposerProps } from "./MessageComposer.types"; function MessageComposerImpl({ @@ -100,14 +100,14 @@ function MessageComposerImpl({ syncComposerContentFromEditor, syncContentRefFromEditorRef, } = useComposerContentState(); - const [previewContent, setPreviewContent] = React.useState(""); const { previewList: composerLinkPreviews, getReadyTags: getReadyLinkPreviewTags, hasPendingSnapshots: hasPendingLinkPreviewSnapshots, // Ref lets the submit guard block Enter/form/auto-submit until snapshots settle. hasPendingSnapshotsRef: hasPendingLinkPreviewSnapshotsRef, - } = useComposerLinkPreviews(previewContent, editTarget == null); + updateContent: updateLinkPreviewContent, + } = useManagedComposerLinkPreviews(editTarget == null); const [isEmojiPickerOpen, setIsEmojiPickerOpen] = React.useState(false); const [isFormattingOpen, setIsFormattingOpen] = React.useState(false); const [spoileredAttachmentUrls, setSpoileredAttachmentUrls] = React.useState< @@ -269,7 +269,7 @@ function MessageComposerImpl({ onLinkShortcut: () => onLinkShortcutRef.current?.() ?? false, onUpdate: ({ cursor, linkPreviewContent, text }) => { setComposerContentFromText(text); - setPreviewContent(linkPreviewContent); + updateLinkPreviewContent(linkPreviewContent); mentions.updateMentionQuery(text, cursor); channelLinks.updateChannelQuery(text, cursor); emojiAutocomplete.updateEmojiQuery(text, cursor); diff --git a/desktop/src/features/messages/ui/useComposerLinkPreviews.test.mjs b/desktop/src/features/messages/ui/useComposerLinkPreviews.test.mjs new file mode 100644 index 00000000000..049c57f70c4 --- /dev/null +++ b/desktop/src/features/messages/ui/useComposerLinkPreviews.test.mjs @@ -0,0 +1,752 @@ +import assert from "node:assert/strict"; +import { after, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; + +// ── Composer-hook regression: fast clear + re-paste of the same URL ─────────── +// +// useComposerLinkPreviews feeds useResolvedLinkPreviews from DEBOUNCED content +// (350ms) but tracks URL-presence newness from the LIVE content. Without that +// live-href signal a fast clear-then-repaste of the same URL inside the debounce +// window never commits an empty debounced set, so the resolver never sees the +// URL leave and never refetches — and the stale snapshot tag built from the +// pre-clear metadata stays sendable. This drives the real composer hook through +// that gesture and asserts the stale tag is not sendable and a fresh fetch is +// forced. (PR #5510, second follow-up.) + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +/** @type {Map Promise>} */ +const ipcHandlers = new Map(); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); + dom.window.__TAURI_INTERNALS__ = { + invoke: (cmd, args) => { + const handler = ipcHandlers.get(cmd); + return handler + ? handler(args) + : Promise.reject(new Error(`unmocked Tauri command: ${cmd}`)); + }, + transformCallback: () => Math.random(), + }; +}); + +after(() => dom.window.close()); + +const HREF = "https://example.com/composer-re-entry"; +const DEBOUNCE_WAIT_MS = 400; // > LINK_PREVIEW_DEBOUNCE_MS (350) + +function metadata(overrides = {}) { + return { + title: "A story", + siteName: "Example", + description: "Story description", + imageDataUrl: null, + imageDomain: null, + imageFetchState: "none", + imageRetryAfterMs: null, + faviconDataUrl: null, + ...overrides, + }; +} + +test("composer input versions retain only active hrefs while re-entry advances", async () => { + const { updateComposerLinkPreviewInput } = await import( + "./useComposerLinkPreviews.tsx" + ); + const secondHref = "https://example.com/second"; + let input = { + content: "", + hrefs: new Set(), + hrefVersions: new Map(), + nextHrefVersion: 0, + }; + + input = updateComposerLinkPreviewInput(input, `see ${HREF}`); + const firstVersion = input.hrefVersions.get(HREF); + assert.equal(input.hrefVersions.size, 1); + + input = updateComposerLinkPreviewInput(input, `see ${secondHref}`); + assert.deepEqual( + [...input.hrefVersions.keys()], + [secondHref], + "departed href history is pruned instead of retained for the composer lifetime", + ); + + input = updateComposerLinkPreviewInput(input, `see ${HREF}`); + assert.deepEqual([...input.hrefVersions.keys()], [HREF]); + assert.ok( + input.hrefVersions.get(HREF) > firstVersion, + "a re-entered href receives a new monotonic version after its old entry was pruned", + ); +}); + +test("composer forces a refetch and drops the stale tag on a fast clear+re-paste of the same URL", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { resetLinkPreviewMetadataCache } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const { updateComposerLinkPreviewInput, useComposerLinkPreviews } = + await import("./useComposerLinkPreviews.tsx"); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + + // Relay origin for media URLs (composer fetches it once on mount). + ipcHandlers.set("get_relay_http_url", () => + Promise.resolve("https://relay.example.com"), + ); + // Media upload always succeeds instantly so a snapshot tag can be built. + ipcHandlers.set("upload_media_bytes", () => + Promise.resolve({ + url: "https://relay.example.com/media/x", + sha256: "deadbeef", + size: 1, + type: "image/png", + uploaded: 0, + }), + ); + // Call 1 (initial paste) resolves to a transient failure -> sendable fallback, + // instantly. Call 2 (the forced re-entry refetch) resolves to a success but + // only when we release `resolveRefetch`, so the test can observe the + // intermediate window where the stale tag is gone and Send is held pending + // BEFORE the fresh result lands (in the app the refetch is a real network + // round-trip; instant resolution would collapse the window under test). + let fetchCalls = 0; + let resolveRefetch; + ipcHandlers.set("fetch_link_preview_metadata", () => { + fetchCalls += 1; + if (fetchCalls === 1) { + return Promise.resolve( + metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }), + ); + } + return new Promise((resolve) => { + resolveRefetch = () => + resolve( + metadata({ + imageDataUrl: "data:image/png;base64,QQ==", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ); + }); + }); + + const flushDebounceAndSettle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, DEBOUNCE_WAIT_MS)); + }); + }; + + try { + let previewInput = updateComposerLinkPreviewInput( + { + content: "", + hrefs: new Set(), + hrefVersions: new Map(), + nextHrefVersion: 0, + }, + `see ${HREF}`, + ); + const { result, rerender, unmount } = renderHook( + ({ content, hrefVersions }) => + useComposerLinkPreviews(content, true, hrefVersions), + { initialProps: previewInput }, + ); + + // 1. Paste settles to a transient-failure fallback with a ready snapshot tag. + await flushDebounceAndSettle(); + assert.equal(fetchCalls, 1); + assert.equal( + result.current.getReadyTags().length, + 1, + "the transient fallback produced a sendable snapshot tag", + ); + assert.equal(result.current.hasPendingSnapshots, false); + + // 2. Fast gesture: clear the URL, then re-paste the SAME URL, with both + // editor updates folded into one React batch. The debounced candidates + // and the committed live href set therefore never observe empty. + // Model two editor onUpdate calls folded into one React batch. The final + // href set equals the previous commit, but the update-boundary version has + // advanced because the URL left and re-entered between those updates. + await act(async () => { + previewInput = updateComposerLinkPreviewInput(previewInput, "see "); + previewInput = updateComposerLinkPreviewInput( + previewInput, + `see ${HREF}`, + ); + rerender(previewInput); + }); + + // 3a. The stale tag must be gone AS SOON AS the same URL re-enters — this is + // the core invariant and it holds synchronously (render-time detection + // drops the tag and excludes the href from sendable output), so no timer + // needs to fire first. Send is held pending until a fresh tag lands. + await act(async () => {}); + assert.equal( + result.current.getReadyTags().length, + 0, + "stale snapshot tag must not be sendable the moment the URL re-enters", + ); + assert.equal( + result.current.hasPendingSnapshots, + true, + "Send must be held pending after a clear+re-paste", + ); + + // 3b. Once the debounce settles, the re-entry has forced a fresh fetch. It is + // still in flight (the deferred refetch has NOT resolved), so the stale + // tag stays gone and Send stays pending. Pre-fix the re-entry is + // invisible, so no refetch starts and this fails fast. + await flushDebounceAndSettle(); + assert.equal( + fetchCalls, + 2, + "the re-entry forced a fresh fetch (still in flight)", + ); + assert.equal( + result.current.getReadyTags().length, + 0, + "stale snapshot tag must not be sendable while the re-entry refetches", + ); + assert.equal( + result.current.hasPendingSnapshots, + true, + "Send must be held pending while the re-entry refetches", + ); + + // 4. The forced refetch resolves (success); a fresh tag becomes sendable and + // its media is the freshly-fetched image, not the pre-clear empty + // fallback (proving the tag was rebuilt from new metadata). + await act(async () => { + resolveRefetch(); + }); + await flushDebounceAndSettle(); + const [freshTag] = result.current.getReadyTags(); + assert.equal( + result.current.getReadyTags().length, + 1, + "a fresh sendable tag lands after the refetch", + ); + assert.ok( + freshTag?.includes("https://relay.example.com/media/x"), + "the sendable tag carries the freshly-fetched snapshot media", + ); + assert.equal(result.current.hasPendingSnapshots, false); + + unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); + +// A blocked re-entry can leave again before its forced refetch settles. The +// abandoned phase must not poison a later paste of the now-healthy cached result. +test("a removed blocked re-entry can later use metadata that resolved while absent", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { resetLinkPreviewMetadataCache } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const { updateComposerLinkPreviewInput, useComposerLinkPreviews } = + await import("./useComposerLinkPreviews.tsx"); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + ipcHandlers.set("get_relay_http_url", () => + Promise.resolve("https://relay.example.com"), + ); + ipcHandlers.set("upload_media_bytes", () => + Promise.resolve({ + url: "https://relay.example.com/media/fresh-after-absence", + sha256: "f8e5", + size: 1, + type: "image/png", + uploaded: 0, + }), + ); + + let fetchCalls = 0; + let resolveRefetch; + ipcHandlers.set("fetch_link_preview_metadata", () => { + fetchCalls += 1; + if (fetchCalls === 1) { + return Promise.resolve( + metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }), + ); + } + return new Promise((resolve) => { + resolveRefetch = () => + resolve( + metadata({ + imageDataUrl: "data:image/png;base64,QQ==", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ); + }); + }); + + const settle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, DEBOUNCE_WAIT_MS)); + }); + }; + + try { + let previewInput = updateComposerLinkPreviewInput( + { + content: "", + hrefs: new Set(), + hrefVersions: new Map(), + nextHrefVersion: 0, + }, + `see ${HREF}`, + ); + const { result, rerender, unmount } = renderHook( + ({ content, hrefVersions }) => + useComposerLinkPreviews(content, true, hrefVersions), + { initialProps: previewInput }, + ); + + await settle(); + assert.equal(result.current.getReadyTags().length, 1); + + // Re-enter the cached negative and wait until its forced refetch is in flight. + await act(async () => { + previewInput = updateComposerLinkPreviewInput(previewInput, "see "); + previewInput = updateComposerLinkPreviewInput( + previewInput, + `see ${HREF}`, + ); + rerender(previewInput); + }); + await settle(); + assert.equal(fetchCalls, 2); + assert.equal(result.current.getReadyTags().length, 0); + assert.equal(result.current.hasPendingSnapshots, true); + + // Remove the blocked href, then let its refetch populate healthy metadata + // while no candidate is active. + await act(async () => { + previewInput = updateComposerLinkPreviewInput(previewInput, "see "); + rerender(previewInput); + }); + await settle(); + await act(async () => resolveRefetch()); + await settle(); + assert.equal(result.current.getReadyTags().length, 0); + + // A later paste should use the healthy result immediately after debounce; + // the abandoned "blocked" phase must not survive and suppress its tag. + await act(async () => { + previewInput = updateComposerLinkPreviewInput( + previewInput, + `see ${HREF}`, + ); + rerender(previewInput); + }); + await settle(); + const [freshTag] = result.current.getReadyTags(); + assert.equal( + fetchCalls, + 2, + "healthy metadata is preserved without a third fetch", + ); + assert.equal(result.current.getReadyTags().length, 1); + assert.ok( + freshTag?.includes("https://relay.example.com/media/fresh-after-absence"), + "the later paste becomes sendable with metadata resolved while absent", + ); + assert.equal(result.current.hasPendingSnapshots, false); + + unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); + +// ── Composer-hook regression: stale in-flight upload after re-entry ─────────── +// +// A pre-clear transient-fallback snapshot upload (U1) can still be in flight +// when the URL is cleared and re-pasted. The re-entry forces a refetch to fresh +// metadata, and the upload effect starts a fresh upload (U2). When the stale U1 +// finally settles it must NOT publish a snapshot tag built from the pre-clear +// metadata — even though its re-entry phase marker was already cleared once the +// refetch reached fresh ready. The per-href upload generation fence makes the +// stale completion a no-op, and the generation-aware dedup guard lets U2 start +// even while U1's slot is still occupied. Reverting either the generation fence +// in `.then` or the generation-aware dedup guard makes this fail (U1 publishes +// its stale favicon, or U2 never starts). (PR #5510, third follow-up.) +test("a stale in-flight upload cannot publish after the URL re-enters and a fresh upload wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { resetLinkPreviewMetadataCache } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const { updateComposerLinkPreviewInput, useComposerLinkPreviews } = + await import("./useComposerLinkPreviews.tsx"); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + + ipcHandlers.set("get_relay_http_url", () => + Promise.resolve("https://relay.example.com"), + ); + + // Gate the FIRST media upload (U1's favicon — its image is null in the + // transient fallback, so the favicon is U1's only upload) so it stays in + // flight across the clear + re-paste and the fresh-metadata resolution. Every + // later upload resolves instantly with a "fresh" URL. If a stale tag ever + // reaches the sendable set it will carry the STALE favicon URL, which the + // assertions forbid. + let uploadCalls = 0; + let releaseStaleUpload; + ipcHandlers.set("upload_media_bytes", () => { + uploadCalls += 1; + if (uploadCalls === 1) { + return new Promise((resolve) => { + releaseStaleUpload = () => + resolve({ + url: "https://relay.example.com/media/STALE", + sha256: "5741313", + size: 1, + type: "image/png", + uploaded: 0, + }); + }); + } + return Promise.resolve({ + url: "https://relay.example.com/media/FRESH", + sha256: "f8e5", + size: 1, + type: "image/png", + uploaded: 0, + }); + }); + + // Call 1 (initial paste): transient failure WITH a favicon, so it produces a + // sendable fallback whose upload (the favicon) is the gated U1. Call 2 (the + // forced re-entry refetch): a full success that only resolves when released, + // so the intermediate window (U1 in flight, refetch pending) is observable. + let fetchCalls = 0; + let resolveRefetch; + ipcHandlers.set("fetch_link_preview_metadata", () => { + fetchCalls += 1; + if (fetchCalls === 1) { + return Promise.resolve( + metadata({ + faviconDataUrl: "data:image/png;base64,QQ==", + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }), + ); + } + return new Promise((resolve) => { + resolveRefetch = () => + resolve( + metadata({ + faviconDataUrl: "data:image/png;base64,Qg==", + imageDataUrl: "data:image/png;base64,Qw==", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ); + }); + }); + + const flushDebounceAndSettle = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, DEBOUNCE_WAIT_MS)); + }); + }; + + try { + let previewInput = updateComposerLinkPreviewInput( + { + content: "", + hrefs: new Set(), + hrefVersions: new Map(), + nextHrefVersion: 0, + }, + `see ${HREF}`, + ); + const { result, rerender, unmount } = renderHook( + ({ content, hrefVersions }) => + useComposerLinkPreviews(content, true, hrefVersions), + { initialProps: previewInput }, + ); + + // 1. Paste settles to a transient-failure fallback. Its favicon upload (U1) + // is gated and stays in flight, so no sendable tag exists yet. + await flushDebounceAndSettle(); + assert.equal(fetchCalls, 1); + assert.equal( + uploadCalls, + 1, + "U1 (the stale fallback favicon) is in flight", + ); + assert.equal( + result.current.getReadyTags().length, + 0, + "U1 has not settled, so no snapshot tag is sendable yet", + ); + + // 2. Fast gesture: clear then re-paste the SAME URL inside the debounce. + await act(async () => { + previewInput = updateComposerLinkPreviewInput(previewInput, "see "); + previewInput = updateComposerLinkPreviewInput( + previewInput, + `see ${HREF}`, + ); + rerender(previewInput); + }); + + // 3. The re-entry forces a fresh fetch; resolve it to fresh success. The + // upload effect must start a FRESH upload (U2) even though U1 still holds + // the slot, then produce a fresh sendable tag. + await flushDebounceAndSettle(); + assert.equal(fetchCalls, 2, "the re-entry forced a fresh fetch"); + await act(async () => { + resolveRefetch(); + }); + await flushDebounceAndSettle(); + assert.ok( + uploadCalls >= 2, + "a fresh upload (U2) started despite U1 still holding the slot", + ); + const [freshTag] = result.current.getReadyTags(); + assert.equal( + result.current.getReadyTags().length, + 1, + "the fresh upload produced a sendable tag", + ); + assert.ok( + freshTag?.includes("https://relay.example.com/media/FRESH"), + "the sendable tag carries the freshly-uploaded media", + ); + assert.ok( + !freshTag?.includes("https://relay.example.com/media/STALE"), + "the sendable tag must not carry the stale pre-clear media", + ); + + // 4. Release the stale U1. Its completion must be a no-op: it cannot + // overwrite the fresh tag with one built from pre-clear metadata. + await act(async () => { + releaseStaleUpload(); + }); + await flushDebounceAndSettle(); + const [tagAfterStale] = result.current.getReadyTags(); + assert.equal( + result.current.getReadyTags().length, + 1, + "still exactly one sendable tag after the stale upload settles", + ); + assert.ok( + tagAfterStale?.includes("https://relay.example.com/media/FRESH") && + !tagAfterStale?.includes("https://relay.example.com/media/STALE"), + "the stale upload cannot publish its pre-clear tag after settling", + ); + + unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); + +// The ordinary production gesture starts from a mounted empty composer. Live +// href tracking must not mark the pasted href handled before the debounced +// candidate exists, or the eventual resolver pass will reuse a cached negative. +test("composer refetches a cached negative when a link is pasted into an empty draft", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { resetLinkPreviewMetadataCache } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const { useComposerLinkPreviews } = await import( + "./useComposerLinkPreviews.tsx" + ); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + ipcHandlers.set("get_relay_http_url", () => + Promise.resolve("https://relay.example.com"), + ); + ipcHandlers.set("upload_media_bytes", () => + Promise.resolve({ + url: "https://relay.example.com/media/fresh", + sha256: "f8e5", + size: 1, + type: "image/png", + uploaded: 0, + }), + ); + + let fetchCalls = 0; + ipcHandlers.set("fetch_link_preview_metadata", () => { + fetchCalls += 1; + return Promise.resolve( + fetchCalls === 1 + ? metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }) + : metadata({ + imageDataUrl: "data:image/png;base64,QQ==", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ); + }); + + try { + // Seed the shared cache with the negative result before the composer sees A. + const { useResolvedLinkPreviews } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const cached = renderHook(() => + useResolvedLinkPreviews([ + { + kind: "generic-link", + href: HREF, + title: HREF, + provider: "example.com", + imageUrl: null, + }, + ]), + ); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + assert.equal(fetchCalls, 1, "the negative was cached before paste"); + cached.unmount(); + + const { result, rerender, unmount } = renderHook( + ({ content }) => useComposerLinkPreviews(content), + { initialProps: { content: "" } }, + ); + await act(async () => rerender({ content: `see ${HREF}` })); + assert.equal( + fetchCalls, + 1, + "debounce has not resolved the pasted href yet", + ); + assert.equal(result.current.hasPendingSnapshots, true); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, DEBOUNCE_WAIT_MS)); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + assert.equal(fetchCalls, 2, "paste invalidated and refetched the negative"); + assert.equal(result.current.getReadyTags().length, 1); + unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); +// A concurrent render may execute the hook and then suspend before commit. No +// href presence, block, generation, or tag mutation from that abandoned render +// may affect the previously committed composer. +test("an abandoned concurrent render cannot invalidate the committed snapshot tag", async () => { + const React = await import("react"); + const { act, cleanup, render } = await import("@testing-library/react"); + const { resetLinkPreviewMetadataCache } = await import( + "@/shared/lib/useResolvedLinkPreviews.ts" + ); + const { useComposerLinkPreviews } = await import( + "./useComposerLinkPreviews.tsx" + ); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + ipcHandlers.set("get_relay_http_url", () => + Promise.resolve("https://relay.example.com"), + ); + ipcHandlers.set("upload_media_bytes", () => + Promise.resolve({ + url: "https://relay.example.com/media/stable", + sha256: "57ab1e", + size: 1, + type: "image/png", + uploaded: 0, + }), + ); + ipcHandlers.set("fetch_link_preview_metadata", () => + Promise.resolve( + metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }), + ), + ); + + let latest; + const never = new Promise(() => {}); + function Suspender() { + throw never; + } + function Harness({ content, suspend }) { + latest = useComposerLinkPreviews(content); + return suspend ? React.createElement(Suspender) : null; + } + + try { + const view = render( + React.createElement(Harness, { content: `see ${HREF}`, suspend: false }), + ); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, DEBOUNCE_WAIT_MS)); + }); + assert.equal(latest.getReadyTags().length, 1); + + // Render a clear that executes the hook but suspends before commit, then + // supersede it with the unchanged committed content. + await act(async () => { + React.startTransition(() => + view.rerender( + React.createElement(Harness, { content: "", suspend: true }), + ), + ); + await Promise.resolve(); + }); + await act(async () => { + view.rerender( + React.createElement(Harness, { + content: `see ${HREF}`, + suspend: false, + }), + ); + await Promise.resolve(); + }); + + assert.equal( + latest.getReadyTags().length, + 1, + "the committed tag remains sendable after the abandoned render", + ); + assert.equal(latest.hasPendingSnapshots, false); + view.unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); diff --git a/desktop/src/features/messages/ui/useComposerLinkPreviews.tsx b/desktop/src/features/messages/ui/useComposerLinkPreviews.tsx index 3f251a719d1..1ea94821cf8 100644 --- a/desktop/src/features/messages/ui/useComposerLinkPreviews.tsx +++ b/desktop/src/features/messages/ui/useComposerLinkPreviews.tsx @@ -192,7 +192,75 @@ async function uploadSnapshotMedia( } } -export function useComposerLinkPreviews(content: string, enabled = true) { +export function extractComposerLinkPreviewHrefs(content: string): string[] { + return extractSupportedLinkPreviews(content) + .filter((preview) => + preview.href.startsWith("buzz://") + ? true + : isValidLinkPreviewSnapshotCanonicalUrl(preview.href), + ) + .map((preview) => preview.href); +} + +export interface ComposerLinkPreviewInput { + content: string; + hrefs: Set; + hrefVersions: Map; + nextHrefVersion: number; +} + +export function updateComposerLinkPreviewInput( + current: ComposerLinkPreviewInput, + content: string, +): ComposerLinkPreviewInput { + const nextHrefs = new Set(extractComposerLinkPreviewHrefs(content)); + const nextVersions = new Map(); + let nextHrefVersion = current.nextHrefVersion; + for (const href of nextHrefs) { + if (current.hrefs.has(href)) { + const version = current.hrefVersions.get(href); + if (version !== undefined) nextVersions.set(href, version); + continue; + } + nextHrefVersion += 1; + nextVersions.set(href, nextHrefVersion); + } + return { + content, + hrefs: nextHrefs, + hrefVersions: nextVersions, + nextHrefVersion, + }; +} + +export function useComposerLinkPreviewInput() { + const [input, setInput] = React.useState(() => ({ + content: "", + hrefs: new Set(), + hrefVersions: new Map(), + nextHrefVersion: 0, + })); + const update = React.useCallback( + (content: string) => + setInput((current) => updateComposerLinkPreviewInput(current, content)), + [], + ); + return [input, update] as const; +} + +export function useManagedComposerLinkPreviews(enabled = true) { + const [input, updateContent] = useComposerLinkPreviewInput(); + return { + ...useComposerLinkPreviews(input.content, enabled, input.hrefVersions), + updateContent, + }; +} + +export function useComposerLinkPreviews( + content: string, + enabled = true, + liveHrefVersions?: ReadonlyMap, +) { const [suppressed, setSuppressed] = React.useState(false); // Debounce the content that drives resolution so typing a URL character by // character does not churn a new candidate href (and a flickering card) per @@ -201,16 +269,14 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // `hasUnresolvedLiveCandidates` below, which keeps Send disabled until the // live candidates resolve — so no synchronous flush is needed at submit. const [debounced, setDebounced] = React.useState(content); - const debouncedRef = React.useRef(debounced); - debouncedRef.current = debounced; React.useEffect(() => { - if (content === debouncedRef.current) return; + if (content === debounced) return; const timer = window.setTimeout( () => setDebounced(content), LINK_PREVIEW_DEBOUNCE_MS, ); return () => window.clearTimeout(timer); - }, [content]); + }, [content, debounced]); const extractCandidates = React.useCallback( (source: string) => enabled @@ -230,12 +296,37 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // resolved (debounce not yet fired after a paste/keystroke), Send must still // treat the preview as pending so a fast Enter cannot ship a bare link ahead // of resolution. - const liveCandidatesRef = React.useRef([]); - liveCandidatesRef.current = extractCandidates(content).map( - (preview) => preview.href, + const liveCandidates = React.useMemo( + () => extractCandidates(content).map((preview) => preview.href), + [extractCandidates, content], ); + const liveCandidatesKey = liveCandidates.join("\n"); + const liveHrefVersionsKey = liveCandidates + .map((href) => `${href}\0${liveHrefVersions?.get(href) ?? ""}`) + .join("\n"); + // Submit and async-completion paths read only the last COMMITTED live set. + // Updating this during render would let an abandoned concurrent render leak + // uncommitted editor content into a later submit or upload completion. + const liveCandidatesRef = React.useRef([]); + React.useLayoutEffect(() => { + liveCandidatesRef.current = liveCandidates; + }, [liveCandidates]); + // A URL freshly entering the composer (paste, or finishing typing one) should + // get a fresh fetch rather than a stale negative cache hit — the user is + // actively asking for this link's card now. useResolvedLinkPreviews handles + // the timing (invalidate a newly-present href's NEGATIVE cache entry before + // it reads the cache); healthy hits and passive message-list scroll are + // untouched, so the shared cache still does its job everywhere else. Pass the + // LIVE hrefs for newness tracking so a fast clear-then-repaste of the same URL + // within the debounce window (which never commits an empty `candidates`) is + // still seen as a re-entry and refetched — not served the stale negative. const resolvedPreviews = useResolvedLinkPreviews( suppressed ? [] : candidates, + { + refetchNewNegatives: true, + liveHrefs: liveCandidates, + liveHrefVersions, + }, ); // Entity links resolve to null metadata when the relay lookup has nothing // for them; keep their safe fallback cards rather than dropping them. @@ -246,7 +337,7 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // Clear a "hide previews" suppression as soon as the LIVE draft has no // supported candidates — not the debounced set, whose lag would otherwise let // a clear-then-retype race keep suppression stuck on after the draft changed. - const liveCandidatesEmpty = liveCandidatesRef.current.length === 0; + const liveCandidatesEmpty = liveCandidates.length === 0; React.useEffect(() => { if (liveCandidatesEmpty) setSuppressed(false); }, [liveCandidatesEmpty]); @@ -258,9 +349,17 @@ export function useComposerLinkPreviews(content: string, enabled = true) { readyTagsByHrefRef.current = readyTags; const suppressedRef = React.useRef(suppressed); suppressedRef.current = suppressed; - const uploadsRef = React.useRef(new Set()); + const uploadsRef = React.useRef(new Map()); + // Per-href upload generation. Bumped whenever a stale-negative href re-enters + // (below), so an upload started before a re-entry can be recognized as stale + // when it settles and dropped without publishing its pre-re-entry tag — the + // `reenteringHrefsRef` phase marker alone is not enough, since it is cleared + // the moment fresh metadata arrives, which can be before the OLD upload + // resolves. Keyed uploads also let a fresh upload start while a superseded one + // is still in flight (its generation no longer matches), so the composer is + // never left tagless waiting on a doomed upload. + const uploadGenerationRef = React.useRef(new Map()); const activeHrefsRef = React.useRef(new Set()); - activeHrefsRef.current = new Set(candidates.map((preview) => preview.href)); React.useEffect(() => { if (getCachedRelayOrigin()) return; @@ -279,15 +378,117 @@ export function useComposerLinkPreviews(content: string, enabled = true) { ); }, [candidates]); + // Compare live content with the LAST COMMITTED href set during render, but do + // not mutate anything here. This gives synchronous submit/pending selectors a + // pure fence for a stale fallback on the candidate render. The layout effect + // below commits the block, generation bump, and tag removal only if React + // actually commits this render; an abandoned concurrent render leaks nothing. + const committedLiveHrefsRef = React.useRef>(new Set()); + const committedLiveHrefVersionsRef = React.useRef>( + new Map(), + ); + // Hrefs that re-entered with a STALE negative fallback stay blocked until the + // resolver visibly cycles pending -> ready. Healthy image re-entries are not + // blocked because their cached metadata remains valid and does not refetch. + const reenteringHrefsRef = React.useRef< + Map + >(new Map()); + const reenteredLiveHrefs = liveCandidates.filter((href) => { + const version = liveHrefVersions?.get(href); + return version === undefined + ? !committedLiveHrefsRef.current.has(href) + : committedLiveHrefVersionsRef.current.get(href) !== version; + }); + const staleReenteredHrefs = reenteredLiveHrefs.filter( + (href) => + !reenteringHrefsRef.current.has(href) && + previews.some( + (preview) => + preview.href === href && + preview.snapshotReady && + preview.imageState === "fallback", + ), + ); + const staleReenteredKey = staleReenteredHrefs.join("\n"); + + // biome-ignore lint/correctness/useExhaustiveDependencies: stable href keys intentionally represent the live/stale sets; the arrays are rebuilt each render. + React.useLayoutEffect(() => { + const previousLiveHrefs = committedLiveHrefsRef.current; + const activeLiveHrefs = new Set(liveCandidates); + activeHrefsRef.current = activeLiveHrefs; + committedLiveHrefsRef.current = activeLiveHrefs; + committedLiveHrefVersionsRef.current = new Map(liveHrefVersions); + + // Leaving the live draft ends the current re-entry cycle. Prune its phase so + // a later paste can consume healthy metadata that resolved while absent. + // Also advance the upload generation: an upload started before removal must + // never publish into a later incarnation of the same href. + for (const href of previousLiveHrefs) { + if (activeLiveHrefs.has(href)) continue; + reenteringHrefsRef.current.delete(href); + uploadGenerationRef.current.set( + href, + (uploadGenerationRef.current.get(href) ?? 0) + 1, + ); + } + + if (staleReenteredHrefs.length === 0) return; + + for (const href of staleReenteredHrefs) { + reenteringHrefsRef.current.set(href, "blocked"); + // Fence any upload built from pre-re-entry metadata. A fresh generation + // can start while the superseded upload is still in flight. + uploadGenerationRef.current.set( + href, + (uploadGenerationRef.current.get(href) ?? 0) + 1, + ); + } + const drop = new Set(staleReenteredHrefs); + setReadyTags((current) => { + let changed = false; + const next = { ...current }; + for (const href of drop) + if (href in next) { + delete next[href]; + changed = true; + } + return changed ? next : current; + }); + }, [liveCandidatesKey, liveHrefVersionsKey, staleReenteredKey]); + + const isHrefReentering = (href: string) => + reenteringHrefsRef.current.has(href) || staleReenteredHrefs.includes(href); + React.useEffect(() => { for (const preview of previews) { + // A re-entering href stays blocked until the resolver's forced refetch has + // visibly cycled through pending: seeing `!snapshotReady` (pending) marks + // "refetching"; only once it is ready AGAIN after that is the block lifted + // and a tag built from the fresh metadata. The stale pre-clear fallback + // (still `snapshotReady` and never pending) can never rebuild the tag. + const phase = reenteringHrefsRef.current.get(preview.href); + if (phase !== undefined) { + if (!preview.snapshotReady) { + reenteringHrefsRef.current.set(preview.href, "refetching"); + continue; + } + if (phase === "blocked") continue; + reenteringHrefsRef.current.delete(preview.href); + } + // The generation captured here fences this upload's completion: a live + // re-entry bumps `uploadGenerationRef` (above), so an in-flight upload + // started from stale pre-clear metadata carries an older generation and + // its `.then` (below) becomes a no-op. The dedup guard is generation-aware + // too, so a superseded in-flight upload does not block starting the fresh + // one at the new generation. + const generation = uploadGenerationRef.current.get(preview.href) ?? 0; if ( !preview.snapshotReady || readyTags[preview.href] || - uploadsRef.current.has(preview.href) + uploadsRef.current.get(preview.href) === generation ) continue; - uploadsRef.current.add(preview.href); + uploadsRef.current.set(preview.href, generation); // Upload image and favicon independently so one failure degrades to the // surviving media instead of dropping the whole preview. A snapshot tag // with empty media fields is valid (renders as text + favicon, or @@ -307,6 +508,20 @@ export function useComposerLinkPreviews(content: string, enabled = true) { ]) .then(([image, favicon]) => { if (!activeHrefsRef.current.has(preview.href)) return; + // If this href re-entered while the upload was in flight, its metadata + // is stale (the resolver is refetching). Drop the result rather than + // writing back a snapshot tag built from the pre-re-entry metadata; + // the forced refetch's own upload will produce the fresh tag. + if (reenteringHrefsRef.current.has(preview.href)) return; + // Durable generation fence, independent of the phase marker: if this + // href re-entered while the upload was in flight, its generation was + // bumped, so this stale completion is dropped even if the marker has + // already been cleared (e.g. the forced refetch reached fresh ready + // and the effect deleted the marker before U1 settled). + if ( + (uploadGenerationRef.current.get(preview.href) ?? 0) !== generation + ) + return; const failedMedia = [image.failed, favicon.failed].filter( (label): label is "thumbnail" | "favicon" => label !== null, ); @@ -335,7 +550,12 @@ export function useComposerLinkPreviews(content: string, enabled = true) { setReadyTags((current) => ({ ...current, [preview.href]: tag })); }) .finally(() => { - uploadsRef.current.delete(preview.href); + // Only clear the slot if this upload is still the current one for the + // href. A superseded upload (older generation) must not delete the + // entry belonging to the fresh upload (U2) that replaced it, or the + // dedup guard would let a third upload start and race again. + if (uploadsRef.current.get(preview.href) === generation) + uploadsRef.current.delete(preview.href); }); void uploadPromise; } @@ -344,7 +564,9 @@ export function useComposerLinkPreviews(content: string, enabled = true) { readyTagsRef.current = suppressed ? [["link-preview", "none"]] : candidates.flatMap((candidate) => - readyTags[candidate.href] ? [readyTags[candidate.href]] : [], + readyTags[candidate.href] && !isHrefReentering(candidate.href) + ? [readyTags[candidate.href]] + : [], ); // A preview is "settling" from paste until its sendable tag exists: metadata // is still resolving, or it resolved and the snapshot media is uploading. @@ -358,6 +580,7 @@ export function useComposerLinkPreviews(content: string, enabled = true) { (preview) => !preview.href.startsWith("buzz://") && (preview.imageState === "pending" || + isHrefReentering(preview.href) || (preview.snapshotReady && !readyTags[preview.href])), ); // A supported link in the LIVE content that resolution has not caught up to @@ -366,7 +589,7 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // before resolution even starts. buzz:// links never snapshot, so ignore them. const hasUnresolvedLiveCandidates = !suppressed && - liveCandidatesRef.current.some( + liveCandidates.some( (href) => !href.startsWith("buzz://") && !readyTags[href] && @@ -378,7 +601,6 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // settling, so a link whose metadata or upload stalls never traps the // composer. Resets whenever settling ends or the live candidate set changes. const [settleDisableExpired, setSettleDisableExpired] = React.useState(false); - const liveCandidatesKey = liveCandidatesRef.current.join("\n"); // biome-ignore lint/correctness/useExhaustiveDependencies: liveCandidatesKey intentionally restarts the anti-trap cap when the link set changes while still settling, so a replaced/added link gets a fresh disable window rather than inheriting the prior link's near-expired timer. React.useEffect(() => { if (!hasSettlingSnapshots) { @@ -433,18 +655,20 @@ export function useComposerLinkPreviews(content: string, enabled = true) { // Snapshot tags for a submit, read synchronously at submit start from the // LIVE candidate set (liveCandidatesRef) via `selectSubmitTags` — so the tags // always correspond to the content actually being sent, never a debounced set - // that still holds a just-removed URL. No await: Send is disabled until every - // settling preview has its tag (or the anti-trap cap fires), so at submit time - // the tags that will ever exist already exist. - const getReadyTags = React.useCallback( - () => - selectSubmitTags( - liveCandidatesRef.current, - readyTagsByHrefRef.current, - suppressedRef.current, + // that still holds a just-removed URL. Re-entering hrefs are excluded: their + // retained tag was built from stale metadata the resolver is refetching, and + // it must not ship until a fresh tag replaces it. No await: Send is disabled + // until every settling preview has its tag (or the anti-trap cap fires), so at + // submit time the tags that will ever exist already exist. + const getReadyTags = React.useCallback(() => { + return selectSubmitTags( + liveCandidatesRef.current.filter( + (href) => !reenteringHrefsRef.current.has(href), ), - [], - ); + readyTagsByHrefRef.current, + suppressedRef.current, + ); + }, []); return { previewList, getReadyTags, diff --git a/desktop/src/shared/lib/useResolvedLinkPreviews.test.mjs b/desktop/src/shared/lib/useResolvedLinkPreviews.test.mjs index c0c91d9f162..a34806ffa0d 100644 --- a/desktop/src/shared/lib/useResolvedLinkPreviews.test.mjs +++ b/desktop/src/shared/lib/useResolvedLinkPreviews.test.mjs @@ -1,10 +1,13 @@ import assert from "node:assert/strict"; -import test from "node:test"; +import { after, before, test } from "node:test"; + +import { JSDOM } from "jsdom"; import { __linkPreviewMetadataTest, fetchBuzzEntityMetadata, isBuzzEntityPreview, + resetLinkPreviewMetadataCache, resolveLinkPreview, withEntityFallbacks, } from "./useResolvedLinkPreviews.ts"; @@ -432,3 +435,413 @@ test("Buzz repository metadata stays image-less and exposes default branch", asy assert.equal(result?.imageDataUrl, null); assert.equal(result?.imageDomain, null); }); + +test("invalidateNegative drops a cached null miss so the next load refetches", async () => { + // A URL freshly entering the composer clears a stale hard miss (null) so it + // refetches, instead of riding the cached blank. + const now = 1_000; + let calls = 0; + let nextResult = null; + const loader = __linkPreviewMetadataTest.createMetadataLoader({ + fetcher: async () => { + calls += 1; + return nextResult; + }, + now: () => now, + }); + + assert.equal((await loader.load(preview.href)).metadata, null); + assert.equal(calls, 1); + + loader.invalidateNegative(preview.href); + nextResult = metadata(); + assert.deepEqual((await loader.load(preview.href)).metadata, metadata()); + assert.equal(calls, 2); +}); + +test("invalidateNegative drops a cached transient failure so the next load refetches", async () => { + // A transient image failure is a NEGATIVE by contract (option docs + the + // loader's own retry boundary), so re-entering the composer must refetch it + // — not reuse the cached transient entry. Regression for the leak where + // invalidateNegative only cleared hard `null` misses. (PR #5510) + const now = 1_000; + let calls = 0; + const loader = __linkPreviewMetadataTest.createMetadataLoader({ + fetcher: async () => { + calls += 1; + return calls === 1 + ? metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 10_000, + }) + : metadata({ + imageDataUrl: "data:image/jpeg;base64,abc", + imageDomain: "images.example.com", + imageFetchState: "image", + }); + }, + now: () => now, + }); + + assert.equal( + (await loader.load(preview.href)).metadata?.imageFetchState, + "transient_failure", + ); + assert.equal(calls, 1); + + // Bust well before the retry boundary (now is frozen); the cache-bust — not + // the cooldown — is what forces the refetch. + loader.invalidateNegative(preview.href); + assert.equal( + (await loader.load(preview.href)).metadata?.imageFetchState, + "image", + ); + assert.equal(calls, 2); +}); + +test("invalidateNegative leaves a healthy cached hit untouched", async () => { + // A settled positive (instant card, no redundant fetch) must survive a bust + // so passive scroll re-renders keep riding the cache. + const now = 1_000; + let calls = 0; + const loader = __linkPreviewMetadataTest.createMetadataLoader({ + fetcher: async () => { + calls += 1; + return metadata(); + }, + now: () => now, + }); + + assert.deepEqual((await loader.load(preview.href)).metadata, metadata()); + assert.equal(calls, 1); + + loader.invalidateNegative(preview.href); + assert.deepEqual((await loader.load(preview.href)).metadata, metadata()); + assert.equal(calls, 1); +}); + +test("invalidateNegative leaves an in-flight fetch untouched", async () => { + // A fetch still in flight is cached as a Promise, not a resolved entry. + // Busting mid-flight must not cancel or duplicate it: the pending load + // resolves normally and no second fetch is started. + const now = 1_000; + let calls = 0; + let releaseFetch; + const loader = __linkPreviewMetadataTest.createMetadataLoader({ + fetcher: () => { + calls += 1; + return new Promise((resolve) => { + releaseFetch = () => resolve(metadata()); + }); + }, + now: () => now, + }); + + const pending = loader.load(preview.href); + assert.equal(calls, 1); + + // Bust while the fetch is still in flight — the Promise entry is left alone. + loader.invalidateNegative(preview.href); + assert.equal(calls, 1, "no redundant fetch started by the bust"); + + releaseFetch(); + assert.deepEqual((await pending).metadata, metadata()); + assert.equal(calls, 1, "the original in-flight fetch resolved, not a retry"); +}); + +// ── Hook-level regression: retained resolved-state invalidation on re-entry ─── +// +// The loader tests above prove `invalidateNegative` drops the SHARED loader +// cache entry. But `useResolvedLinkPreviews` also retains its OWN +// `resolvedMetadata` React state, and the render that scheduled the +// invalidating effect has already read the stale negative from it. Dropping the +// loader key alone leaves that local key in place, so a re-entered +// `transient_failure` still resolves to a `snapshotReady` fallback the composer +// can turn into a sendable snapshot tag from STALE metadata before the retry +// lands. This drives the REAL hook to prove the local key is cleared too, so +// the re-entry renders as pending (no `snapshotReady`) until the retry wins. +// (PR #5510) Regression: without the local-state clear the re-entry assertion +// below sees `snapshotReady: true` instead of pending. + +const dom = new JSDOM("", { + url: "http://localhost", +}); + +before(() => { + Object.assign(globalThis, { + document: dom.window.document, + HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, + window: dom.window, + }); + // @tauri-apps/api/core reads window.__TAURI_INTERNALS__.invoke at call time. + // A per-test handler map lets each test control the fetch resolution timing. + dom.window.__TAURI_INTERNALS__ = { + invoke: (cmd, args) => { + const handler = ipcHandlers.get(cmd); + return handler + ? handler(args) + : Promise.reject(new Error(`unmocked Tauri command: ${cmd}`)); + }, + transformCallback: () => Math.random(), + }; +}); + +after(() => dom.window.close()); + +/** @type {Map Promise>} */ +const ipcHandlers = new Map(); + +const hookPreview = { + kind: "generic-link", + href: "https://example.com/hook-re-entry", + provider: "example.com", + title: "example.com/hook-re-entry", + typeLabel: "link", +}; + +test("hook clears retained resolved state so a re-entered transient failure renders pending until the retry wins", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { useResolvedLinkPreviews } = await import( + "./useResolvedLinkPreviews.ts" + ); + + // Isolate from any loader state leaked by earlier tests in this process. + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + + // Gate every fetch so we can observe the state between renders. Call 1 caches + // a transient failure; call 2 (the re-entry retry) succeeds. + let calls = 0; + const releases = []; + ipcHandlers.set("fetch_link_preview_metadata", () => { + calls += 1; + const attempt = calls; + return new Promise((resolve) => { + releases.push(() => + resolve( + attempt === 1 + ? metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }) + : metadata({ + imageDataUrl: "data:image/jpeg;base64,abc", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ), + ); + }); + }); + + // scheduleAfterPaint uses requestAnimationFrame -> setTimeout(0); flush both + // plus a microtask turn so the queued load() actually fires. + const flushScheduledLoads = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + const settle = async () => { + await act(async () => { + // Release any pending fetch and let its .then() commit setResolvedMetadata. + while (releases.length > 0) releases.shift()(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + try { + const { result, rerender, unmount } = renderHook( + ({ previews }) => + useResolvedLinkPreviews(previews, { refetchNewNegatives: true }), + { initialProps: { previews: [hookPreview] } }, + ); + + // 1. Initial paste: pending until the fetch resolves to a transient failure. + assert.equal(result.current[0].imageState, "pending"); + assert.equal(result.current[0].snapshotReady, undefined); + + await flushScheduledLoads(); + await settle(); + + // Transient failure cached: a sendable fallback (this is the state that must + // NOT survive re-entry as ready). + assert.equal(result.current[0].imageState, "fallback"); + assert.equal(result.current[0].snapshotReady, true); + assert.equal(calls, 1); + + // 2. URL removed from the composer. + rerender({ previews: [] }); + assert.deepEqual(result.current, []); + + // 3. URL re-entered. The invalidation effect must drop BOTH the loader entry + // and the retained local key, so this settles to pending with no + // snapshotReady — not the stale fallback. Without the local-state clear + // the hook returns snapshotReady: true here. + rerender({ previews: [hookPreview] }); + await act(async () => {}); + assert.equal( + result.current[0].imageState, + "pending", + "re-entered transient failure must render pending, not a stale fallback", + ); + assert.equal( + result.current[0].snapshotReady, + undefined, + "no snapshotReady before the retry resolves — nothing sendable from stale metadata", + ); + + // 4. Successful retry wins. + await flushScheduledLoads(); + await settle(); + assert.equal(result.current[0].imageState, "image"); + assert.equal(result.current[0].snapshotReady, true); + assert.equal(calls, 2, "the re-entry triggered a fresh fetch"); + + unmount(); + } finally { + cleanup(); + ipcHandlers.clear(); + } +}); + +// ── Hook-level regression: in-flight shared entry defeats the local clear ───── +// +// The shared metadataLoader is intentionally shared across every hook instance +// (composer + message list). `invalidateNegative` deliberately leaves an +// in-flight Promise entry alone — but that means when a URL re-enters the +// composer WHILE another instance has a fetch in flight for the same canonical +// URL, the shared drop is a no-op. Gating the local-state clear on that drop +// (the earlier fix) leaves this hook's retained `transient_failure` in place, +// so it keeps returning `snapshotReady: true` — a sendable tag built from stale +// metadata — until that shared fetch resolves. This drives the real hook +// through that exact interleaving to prove the local negative is cleared on +// re-entry regardless of the shared entry's shape. (PR #5510, follow-up.) + +const otherHookPreview = { + kind: "generic-link", + href: "https://example.com/hook-inflight-re-entry", + provider: "example.com", + title: "example.com/hook-inflight-re-entry", + typeLabel: "link", +}; + +test("hook clears a re-entered negative even when the shared cache holds an in-flight fetch, not a settled entry", async () => { + const { act, cleanup, renderHook } = await import("@testing-library/react"); + const { useResolvedLinkPreviews } = await import( + "./useResolvedLinkPreviews.ts" + ); + + resetLinkPreviewMetadataCache(); + ipcHandlers.clear(); + + // Gate every fetch. Call 1 (composer's initial paste) resolves to a transient + // failure. Call 2 is the SHARED in-flight fetch a message-list instance starts + // after the shared cache is dropped; it is left unreleased so it is still a + // Promise in the shared cache when the composer re-enters. + let calls = 0; + const releases = []; + ipcHandlers.set("fetch_link_preview_metadata", () => { + calls += 1; + const attempt = calls; + return new Promise((resolve) => { + releases.push(() => + resolve( + attempt === 1 + ? metadata({ + imageFetchState: "transient_failure", + imageRetryAfterMs: 900_000, + }) + : metadata({ + imageDataUrl: "data:image/jpeg;base64,abc", + imageDomain: "images.example.com", + imageFetchState: "image", + }), + ), + ); + }); + }); + + const flushScheduledLoads = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + const releaseAll = async () => { + await act(async () => { + while (releases.length > 0) releases.shift()(); + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + }; + + const composer = renderHook( + ({ previews }) => + useResolvedLinkPreviews(previews, { refetchNewNegatives: true }), + { initialProps: { previews: [otherHookPreview] } }, + ); + // A passive message-list instance sharing the same loader (no refetch). + const messageList = renderHook( + ({ previews }) => useResolvedLinkPreviews(previews, {}), + { initialProps: { previews: [] } }, + ); + + try { + // 1. Composer paste resolves to a transient failure -> sendable fallback. + await flushScheduledLoads(); + await act(async () => { + releases.shift()(); // release call 1 only + await new Promise((resolve) => setTimeout(resolve, 0)); + }); + assert.equal(composer.result.current[0].imageState, "fallback"); + assert.equal(composer.result.current[0].snapshotReady, true); + assert.equal(calls, 1); + + // 2. Composer URL leaves. + composer.rerender({ previews: [] }); + assert.deepEqual(composer.result.current, []); + + // 3. The shared negative is dropped (simulating expiry/eviction), then a + // message-list instance starts a fresh fetch for the SAME canonical URL + // and it is left IN FLIGHT — a Promise, not a settled entry, in the + // shared cache. + act(() => { + resetLinkPreviewMetadataCache(); + }); + messageList.rerender({ previews: [otherHookPreview] }); + await flushScheduledLoads(); + assert.equal(calls, 2, "message-list started the shared in-flight fetch"); + + // 4. Composer re-enters WHILE that fetch is in flight. invalidateNegative is + // a no-op (the shared entry is a Promise), so the earlier drop-gated clear + // would leave the retained transient failure — a sendable tag from stale + // metadata. The local negative must be cleared regardless: pending, no + // snapshotReady. + composer.rerender({ previews: [otherHookPreview] }); + await act(async () => {}); + assert.equal( + composer.result.current[0].imageState, + "pending", + "re-entry during a shared in-flight fetch must render pending, not a stale fallback", + ); + assert.equal( + composer.result.current[0].snapshotReady, + undefined, + "no snapshotReady while the shared fetch is in flight — nothing sendable from stale metadata", + ); + // The composer coalesced onto the in-flight fetch — no third request. + assert.equal(calls, 2, "re-entry coalesced onto the in-flight fetch"); + + // 5. The shared fetch resolves successfully; both instances settle. + await releaseAll(); + await flushScheduledLoads(); + assert.equal(composer.result.current[0].imageState, "image"); + assert.equal(composer.result.current[0].snapshotReady, true); + } finally { + composer.unmount(); + messageList.unmount(); + cleanup(); + ipcHandlers.clear(); + } +}); diff --git a/desktop/src/shared/lib/useResolvedLinkPreviews.ts b/desktop/src/shared/lib/useResolvedLinkPreviews.ts index 5f2a5535b54..8ef93e7d13e 100644 --- a/desktop/src/shared/lib/useResolvedLinkPreviews.ts +++ b/desktop/src/shared/lib/useResolvedLinkPreviews.ts @@ -86,6 +86,13 @@ function metadataCacheKey(href: string): string { } } +function isNegativeMetadata(metadata: LinkPreviewMetadata | null): boolean { + // A cached NEGATIVE is a result that should be retried when the URL freshly + // re-enters the composer: a hard miss (null) or a transient image failure. + // A healthy hit (`image`/`rejected`/no state) is a settled positive. + return metadata === null || metadata.imageFetchState === "transient_failure"; +} + function metadataExpiry( metadata: LinkPreviewMetadata | null, now: number, @@ -186,6 +193,23 @@ function createMetadataLoader({ deleteKey(key: string) { cache.delete(key); }, + /** + * Drop a cached NEGATIVE result (a resolved null or a transient failure) so + * the next load refetches. Used when a URL freshly enters the composer: a + * user pasting a link that previously blanked should get a new attempt now, + * not the stale miss. A healthy cached hit and an in-flight fetch are left + * untouched, so passive scroll re-renders still ride the cache as before. + * Returns whether a negative entry was actually dropped, so callers can + * invalidate their own derived state (e.g. retained React metadata) in step. + */ + invalidateNegative(href: string): boolean { + const key = metadataCacheKey(href); + const cached = cache.get(key); + if (!cached || cached instanceof Promise) return false; + if (!isNegativeMetadata(cached.metadata)) return false; + cache.delete(key); + return true; + }, load, peek, reset() { @@ -417,16 +441,129 @@ export function withEntityFallbacks( export function useResolvedLinkPreviews( previews: SupportedLinkPreview[], + { + refetchNewNegatives = false, + liveHrefs, + liveHrefVersions, + }: { + /** + * When a preview href is newly present since the last run, drop any cached + * NEGATIVE (null/transient-fail) metadata for it so it refetches instead of + * resolving to a stale miss. Used by the composer: a freshly pasted link + * should get a new attempt. Off by default so passive renders (the message + * list) keep riding the cache. Healthy cached hits are never invalidated. + */ + refetchNewNegatives?: boolean; + /** + * The hrefs present in the caller's LIVE (undebounced) content. When given, + * newness is judged against this set instead of the resolved `previews`, so + * a URL that leaves and re-enters the live content is treated as re-entered + * even when a debounce swallowed the intermediate empty state (the composer + * debounces resolution, so `previews` may never observe the URL leaving). + * Resolution timing still follows `previews`; only the invalidation decision + * uses this. Omit to track newness against `previews` (the default). + */ + liveHrefs?: readonly string[]; + /** + * Per-href entry versions captured at the live editor-update boundary. + * Unlike committed href-set equality, a bumped version preserves an + * intermediate leave/re-entry even when React batches both updates into one + * commit with the same final href set. + */ + liveHrefVersions?: ReadonlyMap; + } = {}, ): ResolvedLinkPreview[] { const [resolvedMetadata, setResolvedMetadata] = React.useState({}); const [retryGeneration, setRetryGeneration] = React.useState(0); - + const seenHrefsRef = React.useRef>(new Set()); + const handledHrefVersionsRef = React.useRef>(new Map()); + // Drive newness tracking from a stable string key so an equivalent href list + // does not restart the effect. The effect closes over the committed render's + // hrefs; do not mirror them into a ref during render, because an abandoned + // concurrent render could otherwise leak uncommitted presence into the live + // effect from the previous commit. + const currentHrefs = liveHrefs ?? previews.map((preview) => preview.href); + const newnessKey = currentHrefs + .map((href) => `${href}\0${liveHrefVersions?.get(href) ?? ""}`) + .join("\n"); + // biome-ignore lint/correctness/useExhaustiveDependencies: newnessKey is the stable identity for currentHrefs; depending on the freshly allocated array would rerun this effect every render. React.useEffect(() => { let cancelled = false; let retryAt = Number.POSITIVE_INFINITY; let retryTimer: ReturnType | null = null; + if (refetchNewNegatives) { + // Invalidate first, before the peek/load loop below reads the cache, so a + // newly-present href loads fresh instead of resolving to its stale miss. + // buzz:// entity links resolve off the relay, not this cache — skip them. + // Newness is judged against the live href set when supplied (so a + // debounce-swallowed leave/re-entry still counts), else against previews. + const seen = seenHrefsRef.current; + const handledVersions = handledHrefVersionsRef.current; + const liveNow = currentHrefs; + // Preserve handled hrefs only while they remain live. A live href is not + // marked handled until its debounced preview exists and invalidation has + // actually been attempted; otherwise blank -> paste would consume + // newness during the 350ms debounce and later reuse the stale negative. + const next = new Set( + [...seen].filter((href) => liveNow.includes(href)), + ); + const nextVersions = new Map( + [...handledVersions].filter(([href]) => liveNow.includes(href)), + ); + const reenteredKeys: string[] = []; + for (const preview of previews) { + const version = liveHrefVersions?.get(preview.href); + const alreadyHandled = + version === undefined + ? seen.has(preview.href) + : handledVersions.get(preview.href) === version; + if ( + alreadyHandled || + !liveNow.includes(preview.href) || + preview.href.startsWith("buzz://") + ) { + continue; + } + // Drop any settled NEGATIVE from the SHARED loader cache so the load + // below refetches instead of resolving to the stale miss. (A no-op when + // the shared entry is healthy, in-flight, or absent.) + metadataLoader.invalidateNegative(preview.href); + reenteredKeys.push(metadataCacheKey(preview.href)); + next.add(preview.href); + if (version !== undefined) nextVersions.set(preview.href, version); + } + seenHrefsRef.current = next; + handledHrefVersionsRef.current = nextVersions; + // Dropping the loader entry alone is not enough: this hook retains its own + // resolved metadata, and the render that scheduled this effect already + // read the stale negative from it. Clear this hook's OWN negative key for + // every re-entered href — gating on whether the shared loader dropped a + // settled entry misses the case where another hook left an in-flight + // Promise in the shared cache (invalidateNegative leaves Promises alone + // and the loop below merely coalesces onto it), which would otherwise + // keep this hook's retained `transient_failure` as a `snapshotReady` + // fallback the composer could turn into a sendable snapshot tag from stale + // metadata until that fetch resolves. Clearing the local negative renders + // the re-entered link as pending until the fresh load wins. Healthy local + // hits are kept, so passive re-renders still show their card instantly. + if (reenteredKeys.length > 0) { + setResolvedMetadata((current) => { + let changed = false; + const nextMetadata = { ...current }; + for (const key of reenteredKeys) { + const value = nextMetadata[key]; + if (value !== undefined && isNegativeMetadata(value)) { + delete nextMetadata[key]; + changed = true; + } + } + return changed ? nextMetadata : current; + }); + } + } + const scheduleRetry = ( { expiresAt, key }: Pick, loader: typeof metadataLoader, @@ -485,7 +622,7 @@ export function useResolvedLinkPreviews( for (const cancel of cancelScheduledLoads) cancel(); if (retryTimer !== null) clearTimeout(retryTimer); }; - }, [previews, retryGeneration]); + }, [previews, refetchNewNegatives, retryGeneration, newnessKey]); return React.useMemo( () =>