diff --git a/web/src/lib/attachments/display.test.ts b/web/src/lib/attachments/display.test.ts index 995fdc3d..4fb427d4 100644 --- a/web/src/lib/attachments/display.test.ts +++ b/web/src/lib/attachments/display.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { formatBytes, iconForAttachment, isImage } from './display'; +import { + canBrowserPreview, + canOpenInViewer, + formatBytes, + iconForAttachment, + isImage +} from './display'; import { ATTACHMENT_ICON_IDS, ATTACHMENT_ICON_PATHS, iconSvg } from './icons/index'; import familyFixture from './mime-families.json'; @@ -182,4 +188,92 @@ describe('isImage', () => { expect(isImage('image/png')).toBe(true); expect(isImage('application/pdf')).toBe(false); }); + + // The DR-16 point in one assertion: isImage is deliberately looser than + // the viewer gate, which is why the gate has to be its own helper. + it('is looser than the viewer gate — it accepts what canOpenInViewer refuses', () => { + expect(isImage('image/svg+xml')).toBe(true); + expect(canOpenInViewer('image/svg+xml')).toBe(false); + }); +}); + +const VIEWER_TYPES = ['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/avif']; + +describe('canOpenInViewer — PLAN-2392 DR-16', () => { + it('accepts exactly the five safe raster types', () => { + for (const mime of VIEWER_TYPES) expect(canOpenInViewer(mime)).toBe(true); + }); + + // The whole reason this isn't `startsWith('image/')`: SVG carries active + // content, and TIFF/HEIC are types a browser may simply not decode. + it('refuses image/* types outside the allowlist', () => { + expect(canOpenInViewer('image/svg+xml')).toBe(false); + expect(canOpenInViewer('image/tiff')).toBe(false); + expect(canOpenInViewer('image/heic')).toBe(false); + expect(canOpenInViewer('image/bmp')).toBe(false); + expect(canOpenInViewer('image/jxl')).toBe(false); + }); + + it('refuses non-image types and a missing MIME', () => { + expect(canOpenInViewer('application/pdf')).toBe(false); + expect(canOpenInViewer('text/xml')).toBe(false); + expect(canOpenInViewer('')).toBe(false); + expect(canOpenInViewer(null)).toBe(false); + expect(canOpenInViewer(undefined)).toBe(false); + }); + + it('normalizes case and parameters before matching', () => { + expect(canOpenInViewer('IMAGE/PNG')).toBe(true); + expect(canOpenInViewer('image/jpeg; charset=binary')).toBe(true); + expect(canOpenInViewer(' image/webp ')).toBe(true); + }); + + // A prefix test would let `image/svg+xml; charset=utf-8` through some + // naive normalizations; pin that it doesn't. + it('does not admit a disallowed type by dressing it in parameters', () => { + expect(canOpenInViewer('image/svg+xml; charset=utf-8')).toBe(false); + }); +}); + +describe('canBrowserPreview — PLAN-2392 DR-5', () => { + it('accepts PDF, plain text and the whole viewer raster set', () => { + expect(canBrowserPreview('application/pdf')).toBe(true); + expect(canBrowserPreview('text/plain')).toBe(true); + for (const mime of VIEWER_TYPES) expect(canBrowserPreview(mime)).toBe(true); + }); + + it('refuses the other text/* subtypes browsers handle inconsistently', () => { + expect(canBrowserPreview('text/markdown')).toBe(false); + expect(canBrowserPreview('text/csv')).toBe(false); + expect(canBrowserPreview('text/xml')).toBe(false); + expect(canBrowserPreview('application/xml')).toBe(false); + }); + + it('refuses office documents, archives and force-downloaded types', () => { + expect(canBrowserPreview('application/msword')).toBe(false); + expect( + canBrowserPreview( + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + ) + ).toBe(false); + expect(canBrowserPreview('application/zip')).toBe(false); + expect(canBrowserPreview('text/html')).toBe(false); + expect(canBrowserPreview('application/javascript')).toBe(false); + expect(canBrowserPreview('image/svg+xml')).toBe(false); + }); + + it('refuses a missing MIME and normalizes like the viewer gate', () => { + expect(canBrowserPreview(null)).toBe(false); + expect(canBrowserPreview(undefined)).toBe(false); + expect(canBrowserPreview('')).toBe(false); + expect(canBrowserPreview('TEXT/PLAIN; charset=utf-8')).toBe(true); + }); + + // Superset relationship, stated once so a future edit to either set + // can't silently break it. + it('is a superset of the viewer gate', () => { + for (const mime of VIEWER_TYPES) { + expect(canOpenInViewer(mime) && canBrowserPreview(mime)).toBe(true); + } + }); }); diff --git a/web/src/lib/attachments/display.ts b/web/src/lib/attachments/display.ts index 86e769e2..3bfa9572 100644 --- a/web/src/lib/attachments/display.ts +++ b/web/src/lib/attachments/display.ts @@ -221,3 +221,52 @@ export function iconForAttachment( export function isImage(mime: string): boolean { return mime.startsWith('image/'); } + +/** + * The exact raster types the in-app image viewer may open (PLAN-2392 + * DR-16). Deliberately an allowlist and NOT an `image/` prefix test: + * `image/svg+xml` carries active content, and a legacy row, a + * mislabelled upload or an extensionless SVG sniffed as XML can all + * arrive wearing an `image/*` label. Formats a browser may not decode + * at all (`image/tiff`, `image/heic`) are excluded for the separate + * reason that a viewer that silently shows nothing is worse than the + * file panel. + * + * `isImage` survives unchanged as the general "is this a picture" + * predicate (icon choice, grouping); this is the narrower question of + * what may be handed to the viewer. + */ +const VIEWER_MIMES: ReadonlySet = new Set([ + 'image/png', + 'image/jpeg', + 'image/gif', + 'image/webp', + 'image/avif' +]); + +/** + * Additional types a browser renders honestly in a new tab (PLAN-2392 + * DR-5). PDF and plain text only — every other `text/*` subtype + * (markdown, CSV, XML) is downloaded or rendered inconsistently across + * browsers, and office documents, archives and the types the server + * force-downloads (HTML, JS) never preview. Those surfaces offer + * Download alone. + */ +const BROWSER_PREVIEW_MIMES: ReadonlySet = new Set([ + 'application/pdf', + 'text/plain' +]); + +/** May this MIME be opened in the in-app image viewer? (DR-16) */ +export function canOpenInViewer(mime: string | null | undefined): boolean { + return VIEWER_MIMES.has(normalizeMime(mime)); +} + +/** + * May this MIME be handed to the browser to display — the viewer's + * raster set plus PDF and plain text? (DR-5) + */ +export function canBrowserPreview(mime: string | null | undefined): boolean { + const m = normalizeMime(mime); + return VIEWER_MIMES.has(m) || BROWSER_PREVIEW_MIMES.has(m); +} diff --git a/web/src/lib/components/editor/attachment-chip.ts b/web/src/lib/components/editor/attachment-chip.ts index 89e40caf..cab0e2d7 100644 --- a/web/src/lib/components/editor/attachment-chip.ts +++ b/web/src/lib/components/editor/attachment-chip.ts @@ -306,19 +306,30 @@ export const AttachmentChip = Node.create({ this.options.workspaceSlug, forUuid, this.options.getDownloadUrl, - ).then((meta) => { - if (!meta) return; + ).then((result) => { if (destroyed) return; // NodeView torn down while HEAD was in flight if (deleted) return; // the target is gone; don't un-mark the chip if (currentUuid !== forUuid) return; // superseded - currentMime = meta.mime; + // A transient failure says nothing about whether the row + // exists — keep the filename-guess icon and stay + // retryable (PLAN-2392 DR-17). + if (result.status === 'transient') return; + // A 404 IS authoritative. This is the path editor undo + // takes: undo restores the chip node, but the delete was a + // REST row mutation Tiptap's history can't roll back, so + // the chip must render dead rather than link to a 404. + if (result.status === 'missing') { + markDeleted(); + return; + } + currentMime = result.mime; refreshIcon(); // The shared formatter renders "0 B" and doesn't guard // non-finite input; a chip with no known size should show // nothing at all, so the conditional lives here rather // than in the helper (PLAN-2392 DR-3b). const size = - Number.isFinite(meta.size) && meta.size > 0 ? formatBytes(meta.size) : ''; + Number.isFinite(result.size) && result.size > 0 ? formatBytes(result.size) : ''; sizeEl.textContent = size ? `· ${size}` : ''; }); }; diff --git a/web/src/lib/components/editor/attachment-image.ts b/web/src/lib/components/editor/attachment-image.ts index 175bc509..171ee3c7 100644 --- a/web/src/lib/components/editor/attachment-image.ts +++ b/web/src/lib/components/editor/attachment-image.ts @@ -310,9 +310,50 @@ export const AttachmentImage = Node.create({ // queued, so a superseded load simply has no callback left to run. let detachLoadListeners = () => {}; + /** + * Latch the permanent placeholder for a row the server says is + * gone. Same end state as the deletion broadcast, reached by a + * different route: the broadcast only fires for a delete that + * happened in THIS tab's session, while this covers a node whose + * row was already gone when it rendered — which is exactly what + * editor undo produces (PLAN-2392 DR-17). Tiptap/Yjs history + * owns the document; the delete was a REST row mutation it can't + * roll back, so undo restores a node pointing at nothing. + */ + function latchMissing(forUuid: string) { + if (deleted) return; + if (!forUuid || currentUuid !== forUuid) return; + deleted = true; + // Same reason the deletion listener does this: an in-flight + // load's `load` event would otherwise paint the image back. + detachLoadListeners(); + showMissing(); + } + + /** + * An `error` event carries no status code, so a deleted row + * and a network blip are indistinguishable at that layer — which + * is why every load failure has to stay retryable by default. A + * HEAD probe is what tells them apart: only a 404 latches, and a + * `transient` result leaves the retryable placeholder exactly as + * it was (and is not cached, so Retry re-issues the HEAD). + */ + function probeForMissing(forUuid: string) { + if (!forUuid || !opts.workspaceSlug || deleted) return; + void fetchAttachmentMetadata(opts.workspaceSlug, forUuid, opts.getDownloadUrl).then( + (result) => { + if (result.status === 'missing') latchMissing(forUuid); + } + ); + } + function loadImage(url: string) { detachLoadListeners(); - const onError = () => showMissing(); + const forUuid = currentUuid; + const onError = () => { + showMissing(); + probeForMissing(forUuid); + }; const onLoad = () => resetMissing(); img.addEventListener('error', onError, { once: true }); img.addEventListener('load', onLoad, { once: true }); @@ -404,12 +445,19 @@ export const AttachmentImage = Node.create({ if (currentUuid && opts.workspaceSlug) { const probeUuid = currentUuid; fetchAttachmentMetadata(opts.workspaceSlug, probeUuid, opts.getDownloadUrl).then( - (meta) => { + (result) => { // Bail if the NodeView's uuid changed (rotate/peer // op) while the probe was in flight — otherwise // we'd cache stale MIME state for the new image. - if (!toolbar || currentUuid !== probeUuid) return; - toolbarMime = meta?.mime ?? null; + if (currentUuid !== probeUuid) return; + // A 404 here is the same authoritative signal the + // load path acts on (DR-17), and this probe may + // well beat the to it. + if (result.status === 'missing') latchMissing(probeUuid); + if (!toolbar) return; + // `transient` leaves the MIME unknown rather than + // wrong: gating falls back to supportedFormats. + toolbarMime = result.status === 'ok' ? result.mime : null; refresh(); } ); @@ -591,9 +639,11 @@ export const AttachmentImage = Node.create({ opts.workspaceSlug, probeUuid, opts.getDownloadUrl - ).then((meta) => { - if (!toolbar || currentUuid !== probeUuid) return; - toolbarMime = meta?.mime ?? null; + ).then((result) => { + if (currentUuid !== probeUuid) return; + if (result.status === 'missing') latchMissing(probeUuid); + if (!toolbar) return; + toolbarMime = result.status === 'ok' ? result.mime : null; refresh(); }); } diff --git a/web/src/lib/components/editor/attachment-metadata.test.ts b/web/src/lib/components/editor/attachment-metadata.test.ts new file mode 100644 index 00000000..1e8e279a --- /dev/null +++ b/web/src/lib/components/editor/attachment-metadata.test.ts @@ -0,0 +1,226 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { + fetchAttachmentMetadata, + invalidateAttachmentMetadata, + mimeToFormat +} from './attachment-metadata'; + +// PLAN-2392 DR-17. The three arms exist so a caller can tell "the row is +// gone" (latch the placeholder — editor undo must not resurrect a deleted +// attachment) apart from "the request didn't make it" (stay retryable). +// The old helper collapsed both into `null` AND cached it, which made a +// one-off blip permanently sticky for the page's lifetime. + +const url = (id: string) => `/api/v1/workspaces/ws/attachments/${id}`; + +/** A HEAD response with the headers the helper reads. */ +function head(status: number, headers: Record = {}): Response { + return new Response(null, { status, headers }); +} + +let fetchMock: ReturnType; + +// Each test uses a fresh uuid AND invalidates it, because the module-level +// promise cache is process-wide and deliberately outlives a single probe. +let counter = 0; +function freshUuid(): string { + counter += 1; + return `uuid-${counter}`; +} + +beforeEach(() => { + fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('fetchAttachmentMetadata — result arms', () => { + it('returns ok with the parsed MIME and size on 200', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue( + head(200, { 'content-type': 'image/png; charset=binary', 'content-length': '4096' }) + ); + + const result = await fetchAttachmentMetadata('ws', uuid, url); + + expect(result).toEqual({ status: 'ok', mime: 'image/png', size: 4096 }); + // HEAD, not GET — a GET would pull the whole blob across the wire. + expect(fetchMock).toHaveBeenCalledWith(url(uuid), { + method: 'HEAD', + credentials: 'same-origin' + }); + }); + + it('falls back to a zero size when content-length is absent or junk', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(200, { 'content-type': 'application/pdf' })); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ + status: 'ok', + mime: 'application/pdf', + size: 0 + }); + }); + + it('reports 404 as missing — the authoritative "row is gone" answer', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(404)); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' }); + }); + + it('reports a 500 as transient, not missing', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(500)); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' }); + }); + + it('reports a mid-session 403 as transient, not missing', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(403)); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' }); + }); + + it('reports a network throw as transient rather than rejecting', async () => { + const uuid = freshUuid(); + fetchMock.mockRejectedValue(new TypeError('Failed to fetch')); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' }); + }); +}); + +describe('fetchAttachmentMetadata — caching is per-arm', () => { + it('caches an ok result for the page lifetime', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/webp', 'content-length': '10' })); + + await fetchAttachmentMetadata('ws', uuid, url); + await fetchAttachmentMetadata('ws', uuid, url); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('caches a missing result — deletion is durable, so stop asking', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(404)); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' }); + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('does NOT cache a transient failure — a retry re-issues the HEAD', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValueOnce(head(503)); + + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'transient' }); + + // ...and the row was fine all along: the second probe must reach the + // network and see that, rather than replaying the cached failure. + fetchMock.mockResolvedValueOnce( + head(200, { 'content-type': 'image/jpeg', 'content-length': '7' }) + ); + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ + status: 'ok', + mime: 'image/jpeg', + size: 7 + }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('does NOT cache a network throw either', async () => { + const uuid = freshUuid(); + fetchMock.mockRejectedValueOnce(new TypeError('offline')); + + await fetchAttachmentMetadata('ws', uuid, url); + + fetchMock.mockResolvedValueOnce(head(404)); + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ status: 'missing' }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('still shares one in-flight HEAD between concurrent callers that fail', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(500)); + + const [a, b] = await Promise.all([ + fetchAttachmentMetadata('ws', uuid, url), + fetchAttachmentMetadata('ws', uuid, url) + ]); + + expect(a).toEqual({ status: 'transient' }); + expect(b).toEqual({ status: 'transient' }); + // Eviction happens when the promise SETTLES, so dedupe survives. + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('keys the cache by workspace as well as uuid', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/gif', 'content-length': '1' })); + + await fetchAttachmentMetadata('ws-a', uuid, url); + await fetchAttachmentMetadata('ws-b', uuid, url); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('invalidate drops a cached entry so the next call refetches', async () => { + const uuid = freshUuid(); + fetchMock.mockResolvedValue(head(200, { 'content-type': 'image/png', 'content-length': '2' })); + + await fetchAttachmentMetadata('ws', uuid, url); + invalidateAttachmentMetadata('ws', uuid); + await fetchAttachmentMetadata('ws', uuid, url); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('transient eviction cannot delete a newer entry installed by an invalidate race', async () => { + const uuid = freshUuid(); + let releaseFirst!: (r: Response) => void; + fetchMock.mockImplementationOnce( + () => new Promise((resolve) => (releaseFirst = resolve)) + ); + + const slow = fetchAttachmentMetadata('ws', uuid, url); + + // A delete/transform lands, the entry is invalidated, and a fresh + // probe caches a good result — all before the first HEAD settles. + invalidateAttachmentMetadata('ws', uuid); + fetchMock.mockResolvedValueOnce( + head(200, { 'content-type': 'image/avif', 'content-length': '3' }) + ); + await fetchAttachmentMetadata('ws', uuid, url); + + releaseFirst(head(500)); + expect(await slow).toEqual({ status: 'transient' }); + + // The newer, good entry survived the older promise's eviction. + expect(await fetchAttachmentMetadata('ws', uuid, url)).toEqual({ + status: 'ok', + mime: 'image/avif', + size: 3 + }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); +}); + +describe('mimeToFormat', () => { + it('maps the recognized image MIMEs to the server-side format names', () => { + expect(mimeToFormat('image/jpeg')).toBe('jpeg'); + expect(mimeToFormat('image/jpg')).toBe('jpeg'); + expect(mimeToFormat('image/heif')).toBe('heic'); + expect(mimeToFormat('IMAGE/PNG')).toBe('png'); + }); + + it('returns null for non-images and unknown image subtypes', () => { + expect(mimeToFormat('application/pdf')).toBeNull(); + expect(mimeToFormat('image/jxl')).toBeNull(); + }); +}); diff --git a/web/src/lib/components/editor/attachment-metadata.ts b/web/src/lib/components/editor/attachment-metadata.ts index 0507db0d..643e2024 100644 --- a/web/src/lib/components/editor/attachment-metadata.ts +++ b/web/src/lib/components/editor/attachment-metadata.ts @@ -14,9 +14,9 @@ * immutable (the row is content-addressed; transforms produce * NEW rows), so there's no staleness concern. * - * Skipped silently when no workspace context is available (e.g. - * headless rendering / SSR) — callers see `null` and degrade UI - * accordingly without raising errors. + * Callers never see an exception: every failure is reported through the + * discriminated result below, so a surface with no workspace context + * (headless rendering / SSR) simply doesn't call this at all. */ /** Variants the download URL builder must support. Mirrors AttachmentImage. */ @@ -30,38 +30,83 @@ export interface AttachmentMetadata { size: number; } -const cache = new Map>(); +/** + * The outcome of a metadata probe (PLAN-2392 DR-17). + * + * The three arms exist because callers need to tell "the row is gone" + * apart from "the request didn't make it", and the old `null` return + * collapsed both: + * + * - `ok` — the HEAD succeeded; `mime` / `size` are usable. + * - `missing` — the server answered 404. AUTHORITATIVE: the row is + * gone, and a caller may latch a permanent + * missing-attachment placeholder on it. This is what + * keeps editor undo from resurrecting a deleted + * attachment as a live-looking node. + * - `transient` — any other non-2xx (5xx, 401/403 mid-session, a + * proxy hiccup) or a network throw. Says NOTHING + * about whether the row exists; callers keep whatever + * they were showing and stay retryable. + */ +export type AttachmentMetadataResult = + | ({ status: 'ok' } & AttachmentMetadata) + | { status: 'missing' } + | { status: 'transient' }; + +const cache = new Map>(); /** * Fetch (or read from cache) the MIME + size for an attachment. The * server registers HEAD alongside GET (TASK-877); chi doesn't auto- * route HEAD on GET handlers, so this must use HEAD — a GET would * pull the entire blob across the wire. + * + * Caching is per-arm (PLAN-2392 DR-17). `ok` and `missing` are both + * durable facts about a content-addressed row, so they're kept for the + * page lifetime. A `transient` result is NOT — it's evicted the moment + * it settles, so a blip can't make a live attachment look permanently + * unreadable for the rest of the session. The entry is still installed + * BEFORE the request settles, so concurrent callers for the same key + * share one in-flight HEAD either way; only the settled failure is + * dropped. */ export function fetchAttachmentMetadata( workspaceSlug: string, uuid: string, getDownloadUrl: AttachmentUrlBuilder -): Promise { +): Promise { const key = `${workspaceSlug}:${uuid}`; const existing = cache.get(key); if (existing) return existing; - const promise: Promise = (async () => { + const promise: Promise = (async () => { try { const resp = await fetch(getDownloadUrl(uuid), { method: 'HEAD', credentials: 'same-origin' }); - if (!resp.ok) return null; + if (resp.status === 404) return { status: 'missing' as const }; + if (!resp.ok) return { status: 'transient' as const }; const ctype = resp.headers.get('content-type') ?? ''; const mime = ctype.split(';')[0].trim(); const len = parseInt(resp.headers.get('content-length') ?? '0', 10); - return { mime, size: Number.isFinite(len) && len >= 0 ? len : 0 }; + return { + status: 'ok' as const, + mime, + size: Number.isFinite(len) && len >= 0 ? len : 0 + }; } catch { - return null; + return { status: 'transient' as const }; } })(); cache.set(key, promise); + // Evict a transient failure once it settles. The identity check keeps + // this from deleting a NEWER entry installed by an invalidate-then- + // refetch that raced this promise's resolution. + void promise.then((result) => { + if (result.status === 'transient' && cache.get(key) === promise) { + cache.delete(key); + } + }); return promise; } diff --git a/web/src/lib/components/timeline/ItemTimeline.svelte b/web/src/lib/components/timeline/ItemTimeline.svelte index 23a9a357..1ea411c5 100644 --- a/web/src/lib/components/timeline/ItemTimeline.svelte +++ b/web/src/lib/components/timeline/ItemTimeline.svelte @@ -120,7 +120,18 @@ fetchAttachmentMetadata(wsSlug, uuid, (id, variant) => attachmentDownloadUrl(wsSlug, id, variant) ).then((m) => { - if (!m) return; + // A transient failure (5xx / network) is not evidence about the + // row, and the helper deliberately doesn't cache it — so drop the + // probed mark too, or this panel would never ask again for the + // rest of the session (PLAN-2392 DR-17). A `missing` result IS + // authoritative: leave the mark set and leave `attMeta` without an + // entry, which is what the renderer already degrades to a missing + // placeholder on. + if (m.status === 'transient') { + probed.delete(uuid); + return; + } + if (m.status !== 'ok') return; if (reqWs !== wsSlug) return; const next = new Map(attMeta); // filename is left empty — the markdown alt text is the chip/img