feat(attachments): add viewer open channel and per-host viewer host (TASK-2428)

Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC

web/src/lib/attachments/events.test.ts

@@ -2,9 +2,14 @@ import { describe, expect, it } from 'vitest';
 import {
 	createAttachmentHostToken,
 	isAttachmentPanelEventForHost,
+	isAttachmentViewerEventForHost,
 	notifyAttachmentPanelOpen,
+	notifyViewerOpen,
 	registerAttachmentPanelListener,
+	registerAttachmentViewerListener,
 	type AttachmentPanelOpenEvent,
+	type AttachmentViewerOpenEvent,
+	type LightboxImage,
 } from './events';
 
 /**
@@ -166,3 +171,232 @@ describe('the panel channel with two live hosts', () => {
 		expect(seen).toHaveLength(1);
 	});
 });
+
+/**
+ * The addressing layer for the image viewer (PLAN-2392 phase 3a / TASK-2428).
+ *
+ * Same shape of test as the panel channel above, and for the same reason: the
+ * bus is module-global while `ItemDetail` is mounted twice (master + peeked
+ * pane), so every case below has a concrete two-viewer bug behind it. The two
+ * channels are exercised separately because they are separate predicates —
+ * a shared rule stated twice is exactly what regresses in one place only.
+ */
+function image(over: Partial<LightboxImage> = {}): LightboxImage {
+	return {
+		id: 'att-1',
+		alt: 'a diagram',
+		filename: 'diagram.png',
+		mime_type: 'image/png',
+		size_bytes: 4096,
+		width: 800,
+		height: 600,
+		...over,
+	};
+}
+
+function viewerEvent(over: Partial<AttachmentViewerOpenEvent> = {}): AttachmentViewerOpenEvent {
+	return {
+		attachmentId: 'att-1',
+		workspaceSlug: 'ws-1',
+		itemId: 'item-1',
+		hostToken: 'host-a',
+		images: [image()],
+		index: 0,
+		invoker: null,
+		...over,
+	};
+}
+
+describe('isAttachmentViewerEventForHost', () => {
+	const host = { itemId: 'item-1', hostToken: 'host-a' };
+
+	it('matches when BOTH the item and the token are the host’s', () => {
+		expect(isAttachmentViewerEventForHost(viewerEvent(), host)).toBe(true);
+	});
+
+	it('ignores an event that matches only the item (the two-panes-one-item case)', () => {
+		expect(isAttachmentViewerEventForHost(viewerEvent({ hostToken: 'host-b' }), host)).toBe(
+			false
+		);
+	});
+
+	it('ignores an event that matches only the token', () => {
+		expect(isAttachmentViewerEventForHost(viewerEvent({ itemId: 'item-2' }), host)).toBe(false);
+	});
+
+	it('never matches on a missing half, on either side', () => {
+		// Two absences are not a match: a surface given no token must not be
+		// able to address every host at once, and a host without one must not
+		// consume unaddressed events.
+		expect(isAttachmentViewerEventForHost(viewerEvent({ hostToken: '' }), host)).toBe(false);
+		expect(isAttachmentViewerEventForHost(viewerEvent({ itemId: '' }), host)).toBe(false);
+		expect(
+			isAttachmentViewerEventForHost(viewerEvent(), { itemId: 'item-1', hostToken: '' })
+		).toBe(false);
+		expect(isAttachmentViewerEventForHost(viewerEvent(), { itemId: null, hostToken: null })).toBe(
+			false
+		);
+		expect(
+			isAttachmentViewerEventForHost(viewerEvent({ hostToken: '', itemId: '' }), {
+				itemId: '',
+				hostToken: '',
+			})
+		).toBe(false);
+	});
+
+	it('tolerates a null/undefined event', () => {
+		expect(isAttachmentViewerEventForHost(null, host)).toBe(false);
+		expect(isAttachmentViewerEventForHost(undefined, host)).toBe(false);
+	});
+});
+
+describe('viewer open channel', () => {
+	it('delivers to the addressed host only, with two hosts subscribed', () => {
+		const master = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		const peeked = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		const masterSeen: AttachmentViewerOpenEvent[] = [];
+		const peekedSeen: AttachmentViewerOpenEvent[] = [];
+		const offMaster = registerAttachmentViewerListener((e) => {
+			if (isAttachmentViewerEventForHost(e, master)) masterSeen.push(e);
+		});
+		const offPeeked = registerAttachmentViewerListener((e) => {
+			if (isAttachmentViewerEventForHost(e, peeked)) peekedSeen.push(e);
+		});
+		try {
+			notifyViewerOpen(viewerEvent({ hostToken: peeked.hostToken }));
+			expect(masterSeen).toHaveLength(0);
+			expect(peekedSeen).toHaveLength(1);
+
+			notifyViewerOpen(viewerEvent({ attachmentId: 'att-2', hostToken: master.hostToken }));
+			expect(masterSeen).toHaveLength(1);
+			expect(masterSeen[0].attachmentId).toBe('att-2');
+			expect(peekedSeen).toHaveLength(1);
+		} finally {
+			offMaster();
+			offPeeked();
+		}
+	});
+
+	it('carries the emit-time workspace, the set, the index and the invoker through unchanged', () => {
+		// The workspace is CAPTURED, not resolved by the host: the pane can
+		// switch workspace without remounting, so a host that read it live
+		// could serve a viewer opened in ws1 from ws2's endpoint.
+		const host = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		// A stand-in element: this suite runs in the node environment, and the
+		// bus only ever carries the reference — the host is where it is used.
+		const invoker = { tagName: 'IMG' } as unknown as HTMLElement;
+		const images = [image(), image({ id: 'att-2', alt: 'second' })];
+		let received: AttachmentViewerOpenEvent | null = null;
+		const off = registerAttachmentViewerListener((e) => {
+			if (isAttachmentViewerEventForHost(e, host)) received = e;
+		});
+		try {
+			notifyViewerOpen(
+				viewerEvent({
+					attachmentId: 'att-2',
+					workspaceSlug: 'other-ws',
+					hostToken: host.hostToken,
+					images,
+					index: 1,
+					invoker,
+				})
+			);
+		} finally {
+			off();
+		}
+		const got = received as AttachmentViewerOpenEvent | null;
+		expect(got).not.toBeNull();
+		expect(got!.workspaceSlug).toBe('other-ws');
+		expect(got!.images).toEqual(images);
+		expect(got!.index).toBe(1);
+		// The event's own invariant: the index names the attachment it opened on.
+		expect(got!.images[got!.index]?.id).toBe(got!.attachmentId);
+		expect(got!.invoker).toBe(invoker);
+	});
+
+	it('passes incomplete image metadata through as nulls', () => {
+		// An inline image knows only what its NodeView options give it, and its
+		// HEAD probe may not have completed. The viewer opens anyway.
+		const host = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		let received: AttachmentViewerOpenEvent | null = null;
+		const off = registerAttachmentViewerListener((e) => {
+			if (isAttachmentViewerEventForHost(e, host)) received = e;
+		});
+		try {
+			notifyViewerOpen(
+				viewerEvent({
+					hostToken: host.hostToken,
+					images: [
+						image({
+							filename: null,
+							mime_type: null,
+							size_bytes: null,
+							width: null,
+							height: null,
+						}),
+					],
+				})
+			);
+		} finally {
+			off();
+		}
+		const got = received as AttachmentViewerOpenEvent | null;
+		expect(got).not.toBeNull();
+		expect(got!.images[0].filename).toBeNull();
+		expect(got!.images[0].width).toBeNull();
+	});
+
+	it('drops an unaddressable or empty emission rather than broadcasting it', () => {
+		const seen: AttachmentViewerOpenEvent[] = [];
+		const off = registerAttachmentViewerListener((e) => seen.push(e));
+		try {
+			notifyViewerOpen(viewerEvent({ hostToken: '' }));
+			notifyViewerOpen(viewerEvent({ itemId: '' }));
+			notifyViewerOpen(viewerEvent({ attachmentId: '' }));
+			// A full-screen viewer with nothing to show is worse than no viewer.
+			notifyViewerOpen(viewerEvent({ images: [] }));
+			// The slug is a path segment of every image URL, and the host does
+			// not substitute its own: without it the viewer opens on 404s.
+			notifyViewerOpen(viewerEvent({ workspaceSlug: '' }));
+		} finally {
+			off();
+		}
+		expect(seen).toHaveLength(0);
+	});
+
+	it('returns a disposer that stops delivery', () => {
+		const host = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		const seen: AttachmentViewerOpenEvent[] = [];
+		const off = registerAttachmentViewerListener((e) => {
+			if (isAttachmentViewerEventForHost(e, host)) seen.push(e);
+		});
+		notifyViewerOpen(viewerEvent({ hostToken: host.hostToken }));
+		expect(typeof off).toBe('function');
+		off();
+		notifyViewerOpen(viewerEvent({ hostToken: host.hostToken }));
+		expect(seen).toHaveLength(1);
+		// Disposing twice is a no-op, not a throw — teardown paths can double-fire.
+		expect(() => off()).not.toThrow();
+	});
+
+	it('keeps the two channels separate', () => {
+		// One host, both channels: a panel emission must not open a viewer and
+		// a viewer emission must not open a panel, even though the identity
+		// fields (and the TOKEN — one per host, not one per channel) are shared.
+		const host = { itemId: 'item-1', hostToken: createAttachmentHostToken() };
+		const viewerSeen: AttachmentViewerOpenEvent[] = [];
+		const panelSeen: AttachmentPanelOpenEvent[] = [];
+		const offViewer = registerAttachmentViewerListener((e) => viewerSeen.push(e));
+		const offPanel = registerAttachmentPanelListener((e) => panelSeen.push(e));
+		try {
+			notifyAttachmentPanelOpen(event({ hostToken: host.hostToken }));
+			notifyViewerOpen(viewerEvent({ hostToken: host.hostToken }));
+		} finally {
+			offViewer();
+			offPanel();
+		}
+		expect(viewerSeen).toHaveLength(1);
+		expect(panelSeen).toHaveLength(1);
+		expect(viewerSeen[0].attachmentId).toBe('att-1');
+	});
+});

web/src/lib/attachments/events.ts

@@ -247,3 +247,153 @@ export function notifyAttachmentPanelOpen(event: AttachmentPanelOpenEvent): void
 	if (!event?.attachmentId || !event.itemId || !event.hostToken) return;
 	for (const fn of panelListeners) fn(event);
 }
+
+/**
+ * Image viewer (PLAN-2392 phase 3a, TASK-2428).
+ *
+ * Tapping an image — an inline editor image, and in phase 3c the surfaces that
+ * still mount their own viewer — opens the full-screen `Lightbox`. Same problem
+ * as the panel channel above, same answer: the emitters include Tiptap
+ * NodeViews, which are imperative DOM and cannot mount a Svelte component, so
+ * they signal through this bus and an `ItemDetail`-owned host does the mounting.
+ *
+ * ADDRESSING is DR-8's, unchanged and shared: a host consumes an event only
+ * when BOTH `itemId` and `hostToken` are its own, because the bus is global
+ * while `ItemDetail` is mounted more than once (master pane + peeked pane).
+ * The token is the SAME one the panel channel uses — one token per HOST, not
+ * one per channel — so `createAttachmentHostToken` is not duplicated here.
+ *
+ * WHY THE STRIP AND THE TIMELINE ARE NOT ON THIS CHANNEL: they mount `Lightbox`
+ * directly and keep doing so. The a11y contract lives in the component, and the
+ * lease stack makes coexisting mounts safe, so consolidating producers buys
+ * nothing until 3c gives them a single surface to consolidate ONTO. A decision,
+ * not an omission.
+ *
+ * `mutationsEnabled` is deliberately absent from this channel and from the
+ * host: 3a's viewer has no mutating action, so it would be a dead prop. It is a
+ * hard prerequisite of 3c's Delete, and when it arrives its source must be the
+ * host's own gate.
+ */
+export interface LightboxImage {
+	id: string;
+	alt: string;
+	/**
+	 * Metadata the viewer may caption with, all NULLABLE for the same reason
+	 * the panel's three are: an emitter knows only what its own surface gives
+	 * it, and an inline image's HEAD probe may not have completed or may have
+	 * failed. Structurally a superset of the `Lightbox` component's own
+	 * `LightboxImage` ({id, alt}), so a set built for this channel is passed
+	 * straight through to it.
+	 */
+	filename: string | null;
+	mime_type: string | null;
+	size_bytes: number | null;
+	width: number | null;
+	height: number | null;
+}
+
+export interface AttachmentViewerOpenEvent {
+	/** UUID of the attachment the viewer opens ON. */
+	attachmentId: string;
+	/**
+	 * Workspace the images are read from, CAPTURED AT EMIT — never read live
+	 * from the host. The pane switches workspace without remounting, so a host
+	 * that resolved the slug itself at render time could serve a viewer opened
+	 * in ws1 from ws2's endpoint. The emitter knows which workspace the click
+	 * happened in; that is the answer the viewer must keep.
+	 */
+	workspaceSlug: string;
+	/**
+	 * UUID of the item whose `ItemDetail` mount should SHOW the viewer.
+	 *
+	 * ROUTING, not ownership — see `AttachmentPanelOpenEvent.itemId` for the
+	 * full argument. It names the host in front of the user; it asserts nothing
+	 * about which item the attachment belongs to, and nothing about permission.
+	 */
+	itemId: string;
+	/** Identity of the `ItemDetail` mount that owns the emitting surface. */
+	hostToken: string;
+	/**
+	 * The set the viewer's ←/→ page through, in the emitting surface's own
+	 * order. Readonly because the viewer must not reorder or mutate a set the
+	 * emitter still owns.
+	 */
+	images: readonly LightboxImage[];
+	/** Index to open at. `images[index]?.id === attachmentId` at emit. */
+	index: number;
+	/**
+	 * The element the viewer returns focus to on close. Null when the emitter
+	 * has no stable element to offer.
+	 */
+	invoker: HTMLElement | null;
+}
+
+const viewerListeners = new Set<(event: AttachmentViewerOpenEvent) => void>();
+
+/**
+ * "Is this event mine?" — the single predicate every viewer host must use.
+ *
+ * Deliberately a separate function from the panel's rather than one generic
+ * over both: the two events are different shapes, and a shared predicate would
+ * have to be typed loosely enough to accept anything with two string fields.
+ * The RULE is identical and stated once, in `isAddressable`: both sides must be
+ * fully addressable before a comparison means anything — two empty tokens are
+ * not a match, they are two absences.
+ *
+ * The event parameter accepts `null | undefined` where the panel's does not.
+ * That is the signature TASK-2428 specifies, and it matches what both
+ * functions have always DONE at runtime (`if (!event) return false`) — the
+ * panel's type is simply narrower than its behaviour. Widening the panel's to
+ * match is a change to a shipped surface and belongs to whoever next touches
+ * it, not to this task.
+ */
+export function isAttachmentViewerEventForHost(
+	event: AttachmentViewerOpenEvent | null | undefined,
+	host: { itemId: string | null | undefined; hostToken: string | null | undefined }
+): boolean {
+	if (!event) return false;
+	const from = { itemId: event.itemId, hostToken: event.hostToken };
+	const to = { itemId: host?.itemId ?? '', hostToken: host?.hostToken ?? '' };
+	if (!isAddressable(from) || !isAddressable(to)) return false;
+	return from.itemId === to.itemId && from.hostToken === to.hostToken;
+}
+
+/**
+ * Subscribe to open-viewer requests. Returns a dispose function — call it from
+ * the host's teardown, or the listener leaks and fires into a dead component.
+ * Listeners receive EVERY emission; filter with
+ * `isAttachmentViewerEventForHost`.
+ */
+export function registerAttachmentViewerListener(
+	fn: (event: AttachmentViewerOpenEvent) => void
+): () => void {
+	viewerListeners.add(fn);
+	return () => viewerListeners.delete(fn);
+}
+
+/**
+ * Request that the owning host open the image viewer.
+ *
+ * No-op when the event can't address a host, can't be fetched, or carries no
+ * images: an emission missing any identity field would either reach nobody or
+ * invite a "matches anything" reading of the predicate; one without a workspace
+ * would open a viewer whose every image URL 404s (the slug is a path segment,
+ * and the host deliberately does not substitute its own); and an empty set
+ * would open a full-screen viewer showing nothing.
+ *
+ * What it deliberately does NOT police: the `index` (the viewer clamps, and
+ * dropping the whole emission over an off-by-one would be a silent no-op where
+ * showing the neighbouring image is harmless), and the MIME types in the set —
+ * what is viewable is the EMITTER's judgement, made against the surface it is
+ * emitting from, not a rule this channel can state for every future producer.
+ *
+ * (Named per TASK-2428's normative signature — `notifyViewerOpen`, without the
+ * `Attachment` infix the sibling emitters carry. The task spells the exported
+ * surface out explicitly, so it wins over the local naming rhyme.)
+ */
+export function notifyViewerOpen(event: AttachmentViewerOpenEvent): void {
+	if (!event?.attachmentId || !event.itemId || !event.hostToken) return;
+	if (!event.workspaceSlug) return;
+	if (!event.images?.length) return;
+	for (const fn of viewerListeners) fn(event);
+}

web/src/lib/attachments/viewerResource.svelte.ts

@@ -0,0 +1,59 @@
+import { untrack } from 'svelte';
+import { nextViewerResourceGen, viewerResourceKey } from './viewerResource';
+
+/**
+ * The viewer-resource generation as a live counter (TASK-2428).
+ *
+ * The pure rule lives next door in `viewerResource.ts`; this is the reactive
+ * wrapper `ItemDetail` actually uses. It is a module rather than four lines
+ * inline in a 7,000-line component for one reason: the `$effect` below is the
+ * subtlest thing in this task, and inline it could only ever be exercised
+ * through `AttachmentViewerHost`'s props — which are handed the answer and so
+ * cannot tell a correct counter from one that never moves, nor a healthy flush
+ * from a self-invalidating one (Codex round 6).
+ *
+ * SELF-WRITE DISCIPLINE (CONVE-1688). The effect writes `gen`, which it must
+ * therefore never READ in its tracked scope: an `$effect` that depends on what
+ * it writes aborts the flush, and an aborted flush strands unrelated reactivity
+ * elsewhere in the same batch while reporting nothing in a production build.
+ * So the tracked scope reads ONLY the caller's inputs, and both the comparison
+ * and the write happen inside `untrack`. `lastKey` is a plain `let` for the
+ * same reason — as `$state` it would reintroduce exactly that dependency.
+ *
+ * Must be called during component initialisation, like any `$effect` owner.
+ */
+export interface ViewerResourceInput {
+	/** Workspace the CURRENTLY LOADED item belongs to, not the route's. */
+	workspaceSlug: string;
+	/** UUID of the loaded item. */
+	itemId: string | null | undefined;
+	/** Whether the loaded item matches the requested ref (the switch boundary). */
+	loaded: boolean;
+}
+
+export interface ViewerResourceGen {
+	/** Advances only on a real loaded-item resource change. */
+	readonly current: number;
+}
+
+export function createViewerResourceGen(read: () => ViewerResourceInput): ViewerResourceGen {
+	let gen = $state(0);
+	let lastKey = '';
+
+	$effect(() => {
+		const input = read();
+		const key = viewerResourceKey(input.workspaceSlug, input.itemId, input.loaded);
+		untrack(() => {
+			const next = nextViewerResourceGen(key, lastKey, gen);
+			if (next === gen) return;
+			lastKey = key;
+			gen = next;
+		});
+	});
+
+	return {
+		get current() {
+			return gen;
+		},
+	};
+}

web/src/lib/attachments/viewerResource.test.ts

@@ -0,0 +1,82 @@
+import { describe, expect, it } from 'vitest';
+import { nextViewerResourceGen, viewerResourceKey } from './viewerResource';
+
+/**
+ * The generation half of the viewer's lifecycle rule (TASK-2428).
+ *
+ * Extracted from `ItemDetail` precisely so these sequences can be stated: a
+ * host-level test cannot distinguish a correct generation from one that simply
+ * never moves, because all it ever sees is the number it was handed.
+ *
+ * The sequences below are the ones `ItemDetail` actually produces — the key is
+ * `itemMatchesRef ? {ws, item.id} : ''`, which reads empty mid-switch.
+ */
+describe('viewerResourceKey', () => {
+	it('is empty until the loaded item matches the requested ref', () => {
+		expect(viewerResourceKey('ws', 'item-a', false)).toBe('');
+		expect(viewerResourceKey('ws', undefined, true)).toBe('');
+		expect(viewerResourceKey('ws', 'item-a', true)).not.toBe('');
+	});
+
+	it('includes the workspace, so the same ref in two workspaces is two resources', () => {
+		// A reused pane can navigate ws1→ws2 carrying `?item=<ref>` where both
+		// workspaces own that ref (IDEA-2135).
+		expect(viewerResourceKey('ws1', 'item-a', true)).not.toBe(
+			viewerResourceKey('ws2', 'item-a', true)
+		);
+	});
+
+	it('contains no control characters', () => {
+		// A NUL separator makes the whole source file binary to grep — caught
+		// in review on the first draft of this rule.
+		expect(viewerResourceKey('ws', 'item-a', true)).toMatch(/^[\x20-\x7e]+$/);
+	});
+});
+
+describe('nextViewerResourceGen', () => {
+	/** Replays a key sequence, returning the generation after each step. */
+	function replay(keys: string[]): number[] {
+		let gen = 0;
+		let lastKey = '';
+		return keys.map((key) => {
+			const next = nextViewerResourceGen(key, lastKey, gen);
+			if (next !== gen) {
+				lastKey = key;
+				gen = next;
+			}
+			return gen;
+		});
+	}
+
+	it('advances once when the first item loads', () => {
+		expect(replay(['', 'ws A'])).toEqual([0, 1]);
+	});
+
+	it('does NOT advance on a same-item reload', () => {
+		// The whole point: a collection schema edit refetches the item already
+		// on screen. `loadGeneration` moves; this must not.
+		expect(replay(['ws A', 'ws A', 'ws A'])).toEqual([1, 1, 1]);
+	});
+
+	it('counts an A→B switch once, through the empty mid-switch key', () => {
+		expect(replay(['ws A', '', 'ws B'])).toEqual([1, 1, 2]);
+	});
+
+	it('does not count a boundary flap that lands back on the same item', () => {
+		expect(replay(['ws A', '', 'ws A'])).toEqual([1, 1, 1]);
+	});
+
+	it('counts A→B→A as three distinct resources, not two', () => {
+		// Rapid j/k paging. Coming BACK to A is a resource change too: whatever
+		// was open belonged to B.
+		expect(replay(['ws A', '', 'ws B', '', 'ws A'])).toEqual([1, 1, 2, 2, 3]);
+	});
+
+	it('counts a cross-workspace switch that keeps the ref', () => {
+		expect(replay(['ws1 A', 'ws2 A'])).toEqual([1, 2]);
+	});
+
+	it('never advances on the empty key alone', () => {
+		expect(replay(['', '', ''])).toEqual([0, 0, 0]);
+	});
+});

web/src/lib/attachments/viewerResource.ts

@@ -0,0 +1,48 @@
+/**
+ * The viewer's resource-identity rule (PLAN-2392 phase 3a, TASK-2428).
+ *
+ * `AttachmentViewerHost` closes an open viewer on a RESOURCE SWITCH and on
+ * nothing else. Half of that is the host's own `itemId`; the other half is a
+ * generation counter, because the id alone cannot tell a same-item resource
+ * change from a same-item RELOAD — and `ItemDetail` reloads constantly (a
+ * collection schema edit refetches the item it is already showing).
+ *
+ * The two counters `ItemDetail` already has are both wrong for this:
+ * `loadGeneration` is a non-reactive fence bumped by EVERY `loadData()`, and
+ * `itemGen` is bumped by every optimistic item write. Keying a viewer on either
+ * tears it down on a refresh that changed nothing the user can see.
+ *
+ * This module is the rule itself, extracted from the component for one reason:
+ * `ItemDetail` is 7,000 lines with no unit harness, so a rule left inline is
+ * only ever tested through the host's props — which cannot see whether the
+ * generation was computed correctly in the first place.
+ */
+
+/** Identity of a loaded item resource, or `''` for "nothing loaded". */
+export function viewerResourceKey(
+	workspaceSlug: string | null | undefined,
+	itemId: string | null | undefined,
+	loaded: boolean
+): string {
+	if (!loaded || !itemId) return '';
+	// The workspace is part of the identity, not decoration: a reused pane can
+	// navigate ws1→ws2 carrying the same `?item=<ref>` where both workspaces
+	// own that ref (IDEA-2135). A plain `::` separator, deliberately: the first
+	// draft used a NUL, which makes the whole source file binary to grep.
+	return `${workspaceSlug ?? ''}::${itemId}`;
+}
+
+/**
+ * The next generation for an observed key. Returns the CURRENT generation
+ * unchanged when nothing advanced, so the caller's write is a no-op.
+ *
+ * An EMPTY key never advances it. Empty is "no loaded resource right now" —
+ * what the switch boundary reads mid-load — so A→''→B is ONE transition and
+ * counts once, at B. (Going empty still closes an open viewer: that is the
+ * host's `itemId` arm, which fires immediately rather than waiting for a
+ * replacement that may never arrive.)
+ */
+export function nextViewerResourceGen(key: string, lastKey: string, gen: number): number {
+	if (!key || key === lastKey) return gen;
+	return gen + 1;
+}

web/src/lib/attachments/viewerResourceGen.svelte.test.ts

@@ -0,0 +1,185 @@
+import { describe, it, expect, afterEach } from 'vitest';
+import { flushSync } from 'svelte';
+import { createViewerResourceGen } from './viewerResource.svelte';
+
+/**
+ * The generation EFFECT (TASK-2428) — the production one `ItemDetail` calls,
+ * not a copy of it.
+ *
+ * Two things are under test, and the second is why this file exists at all:
+ *
+ *  1. Which transitions advance the counter. Driven through the same inputs
+ *     `ItemDetail` supplies (`loadedItemWsSlug`, `item?.id`, `itemMatchesRef`),
+ *     replaying the sequences a real session produces — a route alias, a
+ *     schema-edit reload, an A→B switch through the empty mid-switch boundary,
+ *     a cross-workspace nav that keeps the ref.
+ *
+ *  2. That the effect keeps the FLUSH alive. An `$effect` that reads a `$state`
+ *     it also writes in its tracked scope aborts the flush and strands
+ *     unrelated reactivity in the same batch, reporting nothing in a production
+ *     build (CONVE-1688). A test that only checks the counter's VALUE passes
+ *     through that, because the counter is what the aborted effect already
+ *     wrote. So every case below runs beside an independent probe effect that
+ *     must keep observing — the neighbour is the assertion.
+ */
+
+// One reactive input object, the shape ItemDetail's reader closes over.
+const input = $state({ workspaceSlug: '', itemId: null as string | null, loaded: false });
+
+interface Harness {
+	gen: () => number;
+	/** Values the neighbouring effect observed, in order. */
+	probeSeen: number[];
+	/** How many times the neighbour re-ran. */
+	probeRuns: () => number;
+	stop: () => void;
+}
+
+let running: Harness | null = null;
+
+function start(): Harness {
+	const probeSeen: number[] = [];
+	let probeRuns = 0;
+	let read!: () => number;
+	const stop = $effect.root(() => {
+		const resource = createViewerResourceGen(() => ({
+			workspaceSlug: input.workspaceSlug,
+			itemId: input.itemId,
+			loaded: input.loaded,
+		}));
+		read = () => resource.current;
+		// The neighbour. It depends on the counter, so a stranded flush shows up
+		// as a value it never sees — and it is a SEPARATE effect, so an aborted
+		// batch takes it down with the one that aborted.
+		$effect(() => {
+			probeSeen.push(resource.current);
+			probeRuns += 1;
+		});
+	});
+	flushSync();
+	running = { gen: () => read(), probeSeen, probeRuns: () => probeRuns, stop };
+	return running;
+}
+
+/** Applies one observable state of the loaded item, then settles. */
+function set(next: Partial<typeof input>) {
+	Object.assign(input, next);
+	flushSync();
+}
+
+/** The mid-switch boundary: `itemMatchesRef` false, item not yet adopted. */
+const MID_SWITCH = { loaded: false };
+
+afterEach(() => {
+	running?.stop();
+	running = null;
+	Object.assign(input, { workspaceSlug: '', itemId: null, loaded: false });
+});
+
+describe('createViewerResourceGen', () => {
+	it('advances once when the first item loads, and the neighbour sees it', () => {
+		const h = start();
+		expect(h.gen()).toBe(0);
+
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		expect(h.gen()).toBe(1);
+		// Not just the value: the downstream effect actually re-ran with it.
+		expect(h.probeSeen).toEqual([0, 1]);
+	});
+
+	it('does NOT advance on a collection-only or username-only route alias', () => {
+		// `/dave/ws/tasks/TASK-1` → `/dave/ws/bugs/TASK-1` (or a different
+		// username for the same workspace). The route change triggers a
+		// `loadData()`, which re-adopts the SAME item under the SAME workspace,
+		// so every input this effect reads is re-asserted unchanged. That is
+		// the whole reason the route is not one of its inputs.
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		const runsBefore = h.probeRuns();
+
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+
+		expect(h.gen()).toBe(1);
+		// The neighbour did not re-run either: nothing downstream was disturbed
+		// by a refresh that changed nothing.
+		expect(h.probeRuns()).toBe(runsBefore);
+	});
+
+	it('does NOT advance on the reload that follows a collection schema edit', () => {
+		// `loadData()` flips `loading` and re-adopts the same item. On this
+		// path `itemMatchesRef` never goes false (the item is not nulled), so
+		// the inputs never leave the loaded state.
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+
+		expect(h.gen()).toBe(1);
+		expect(h.probeSeen).toEqual([0, 1]);
+	});
+
+	it('counts an A→B switch ONCE, through the empty mid-switch boundary', () => {
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+
+		set(MID_SWITCH);
+		expect(h.gen()).toBe(1); // the gap is not a resource
+		set({ workspaceSlug: 'ws', itemId: 'item-b', loaded: true });
+
+		expect(h.gen()).toBe(2);
+		expect(h.probeSeen).toEqual([0, 1, 2]);
+	});
+
+	it('counts a boundary flap that lands back on the same item as NOTHING', () => {
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		const runsBefore = h.probeRuns();
+
+		set(MID_SWITCH);
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+
+		expect(h.gen()).toBe(1);
+		expect(h.probeRuns()).toBe(runsBefore);
+	});
+
+	it('advances on a same-ref DIFFERENT-WORKSPACE change', () => {
+		// A reused pane navigating ws1→ws2 while carrying `?item=<ref>`, where
+		// both workspaces own that ref (IDEA-2135). The plain item id would
+		// miss it if the two happened to resolve to the same id; the workspace
+		// is part of the identity precisely so it cannot.
+		const h = start();
+		set({ workspaceSlug: 'ws1', itemId: 'item-a', loaded: true });
+		set({ workspaceSlug: 'ws2', itemId: 'item-a', loaded: true });
+
+		expect(h.gen()).toBe(2);
+		expect(h.probeSeen).toEqual([0, 1, 2]);
+	});
+
+	it('counts A→B→A as three resources, keeping the neighbour live throughout', () => {
+		// Rapid j/k paging. Coming back to A is a change too: whatever was open
+		// belonged to B. This is also the sequence a self-invalidating effect
+		// survives longest, because its first write is a no-op.
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+		set(MID_SWITCH);
+		set({ workspaceSlug: 'ws', itemId: 'item-b', loaded: true });
+		set(MID_SWITCH);
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: true });
+
+		expect(h.gen()).toBe(3);
+		expect(h.probeSeen).toEqual([0, 1, 2, 3]);
+		// Still reacting after five transitions — the flush was never aborted.
+		set(MID_SWITCH);
+		set({ workspaceSlug: 'ws', itemId: 'item-c', loaded: true });
+		expect(h.probeSeen).toEqual([0, 1, 2, 3, 4]);
+	});
+
+	it('never advances while nothing is loaded', () => {
+		const h = start();
+		set({ workspaceSlug: 'ws', itemId: 'item-a', loaded: false });
+		set({ workspaceSlug: 'ws', itemId: null, loaded: false });
+
+		expect(h.gen()).toBe(0);
+		expect(h.probeSeen).toEqual([0]);
+	});
+});

web/src/lib/components/attachments/AttachmentViewerHost.svelte

@@ -0,0 +1,170 @@
+<!--
+	AttachmentViewerHost — the ONE consumer of the open-viewer channel for one
+	`ItemDetail` mount (PLAN-2392 phase 3a, TASK-2428).
+
+	Sibling of `AttachmentPanelHost`, same reasons: the emitters — inline editor
+	image NodeViews — are imperative DOM and cannot mount a Svelte component, so
+	they signal through the module-global bus in `$lib/attachments/events` and
+	something on the other side owns the viewer. That owner is `ItemDetail`,
+	which mounts exactly one of these with the SAME `hostToken` it gives the
+	panel host: one token per HOST, not one per channel.
+
+	Kept a separate component rather than a block inside `ItemDetail` for the
+	same reason as the panel host: the addressing rule is the load-bearing part
+	of DR-8 and has to be testable with TWO hosts mounted at once, which is what
+	the pane host does at runtime (a master pane plus a peeked pane). Folded into
+	a 6,000-line component it would be unreachable by any test.
+
+	THE STRIP AND THE TIMELINE ARE NOT ROUTED HERE. They mount `Lightbox`
+	directly and keep doing so; task 1's lease stack makes coexisting mounts
+	safe, and consolidating producers belongs with 3c's convergence, where there
+	is a single surface to host. A decision, not an omission.
+
+	WORKSPACE COMES OFF THE EVENT. `wsSlug` is captured at emit and carried on
+	the request, never read live from the host — the pane switches workspace
+	without remounting, so a live read could serve a viewer opened in ws1 from
+	ws2's endpoint.
+
+	NO `mutationsEnabled`. 3a's viewer has no mutating action, so threading a
+	permission here would be a dead prop. It is 3c's, together with Delete.
+
+	LIFECYCLE — ONE RULE. Clear on a RESOURCE SWITCH: the host's `itemId`
+	changing, or `resourceGen` advancing. Nothing else closes the viewer.
+
+	  - NOT the route. A collection-only or username-only change preserves the
+	    loaded item, and a viewer must not close because a URL segment moved.
+	  - NOT `ItemDetail`'s `loadGeneration`, which is a non-reactive counter
+	    bumped by EVERY `loadData()` — including a same-item schema reload after
+	    a collection edit. Keying on it would tear the viewer down on a refresh
+	    that changed nothing the viewer can see. `resourceGen` is the dedicated
+	    reactive counter that advances only when the LOADED item resource
+	    actually changes.
+
+	DR-5c's delete-tracks-by-id and DR-14's archive/restore are 3c's, and are
+	deliberately absent here even though the panel host has them.
+-->
+<script lang="ts">
+	import { untrack } from 'svelte';
+	import Lightbox from '$lib/components/common/Lightbox.svelte';
+	import {
+		isAttachmentViewerEventForHost,
+		registerAttachmentViewerListener,
+		type AttachmentViewerOpenEvent,
+	} from '$lib/attachments/events';
+
+	interface Props {
+		/** Parent item UUID. Null/undefined while the item is loading or mid-switch. */
+		itemId: string | null | undefined;
+		/** This `ItemDetail` mount's identity on the bus — the panel host's token. */
+		hostToken: string;
+		/**
+		 * Reactive generation of the LOADED item resource. Advances only on a
+		 * real resource change, never on a same-item refresh. See the lifecycle
+		 * note above for why this is not `loadGeneration`.
+		 */
+		resourceGen?: number;
+	}
+
+	let { itemId, hostToken, resourceGen = 0 }: Props = $props();
+
+	let request = $state<AttachmentViewerOpenEvent | null>(null);
+
+	/**
+	 * Builds a close handler BOUND to the request it was rendered for, so a
+	 * stale continuation cannot dismiss a newer viewer — the shape
+	 * `AttachmentPanelHost` uses, for the same reason: the child is destroyed by
+	 * nulling `request` (item switch), but a callback it already scheduled can
+	 * still land afterwards and would otherwise clear whatever is current BY
+	 * THEN.
+	 */
+	function closeRequest(target: AttachmentViewerOpenEvent | null): () => void {
+		return () => {
+			if (target && request !== target) return;
+			request = null;
+			// Focus returns to the element that opened the viewer — the only use
+			// this host makes of `invoker`. `Lightbox` manages no focus of its
+			// own, so without this a close leaves focus on <body> and the
+			// keyboard user restarts from the top of the document.
+			//
+			// `isConnected` because the opener can be gone by now: an editor
+			// NodeView is re-rendered on any document change, and focusing a
+			// detached node silently does nothing on some engines and moves
+			// focus to <body> on others.
+			const invoker = target?.invoker;
+			if (invoker?.isConnected) invoker.focus();
+			// Deliberately only on a USER close. The lifecycle teardown below
+			// does not return focus: it fires because the item under the viewer
+			// is being replaced, so the invoker is part of a subtree on its way
+			// out — putting focus back into it would land the user inside
+			// content that is about to disappear. Where focus goes after a
+			// switch is the pane's business, not the viewer's.
+		};
+	}
+
+	// Plain `let`, not $state: written and read only inside `untrack`ed effect
+	// bodies. As $state they would make each effect depend on what it writes,
+	// which aborts the flush and strands unrelated reactivity (CONVE-1688).
+	// Both are seeded from the initial props DELIBERATELY (hence `untrack`): a
+	// host that mounts on an already-loaded item has nothing to clear — only a
+	// TRANSITION is a resource switch.
+	let lastItemId = untrack(() => itemId ?? '');
+	let lastResourceGen = untrack(() => resourceGen);
+
+	// Subscribe once. `itemId` / `hostToken` are read inside the callback at
+	// EMIT time, so the comparison always uses the host's current address —
+	// deliberately not captured, since this component (like `ItemDetail`) can
+	// outlive an A→B item switch. The disposer returned by the registration is
+	// the effect's teardown, so an unmounted host stops receiving events.
+	$effect(() => {
+		return registerAttachmentViewerListener((event) => {
+			if (!isAttachmentViewerEventForHost(event, { itemId, hostToken })) return;
+			request = event;
+		});
+	});
+
+	// The one lifecycle rule. Both arms are the same event — "the resource under
+	// this host changed" — so they share one effect and one clear.
+	//
+	// ANY change of the id counts, INCLUDING one to null. The id is
+	// `ItemDetail`'s `itemMatchesRef ? item?.id : null`, so null means "the
+	// item this viewer belongs to is no longer the one on screen" — the start
+	// of a switch, a cross-workspace nav, an unload. Treating null as merely
+	// "not a resource yet" and waiting for the next non-empty id leaves the
+	// viewer stranded over a skeleton or an error page whenever the incoming
+	// item never arrives (Codex round 3).
+	//
+	// A same-resource REFRESH never reaches this branch: `loadData()` does not
+	// null `item`, and `itemMatchesRef` does not depend on the collection or
+	// username segments, so a refresh and a collection-only route change both
+	// keep the id exactly where it was. That is what makes clearing on every id
+	// change safe, and it is why the generation exists — it is the ONLY thing
+	// that can tell a same-id resource change from a same-id reload.
+	$effect(() => {
+		const id = itemId ?? '';
+		const gen = resourceGen;
+		untrack(() => {
+			if (id === lastItemId && gen === lastResourceGen) return;
+			lastItemId = id;
+			lastResourceGen = gen;
+			request = null;
+		});
+	});
+</script>
+
+<!--
+	Remounted per open via `{#key request}`: `Lightbox` seeds its index once
+	through `untrack`, so prop-sync does not work — opening a second image while
+	the first viewer is up must produce a NEW component instance.
+
+	`wsSlug` comes off the request, per the note above.
+-->
+{#key request}
+	{#if request}
+		<Lightbox
+			images={[...request.images]}
+			index={request.index}
+			wsSlug={request.workspaceSlug}
+			onClose={closeRequest(request)}
+		/>
+	{/if}
+{/key}

web/src/lib/components/attachments/AttachmentViewerHost.svelte.test.ts

@@ -0,0 +1,419 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { flushSync, mount, unmount } from 'svelte';
+
+// TASK-2428. The viewer is exercised THROUGH its host, because the host is
+// where the two rules that matter live: an event is consumed only when both
+// `itemId` and `hostToken` are this host's own (DR-8), and an open viewer is
+// closed by a RESOURCE SWITCH and by nothing else. Nothing routes into it yet —
+// the emitters land in the next task — so these tests emit on the bus directly,
+// which is also the only way to drive a NodeView-originated open.
+//
+// The bus stays REAL: addressing is the thing under test.
+//
+// What jsdom CANNOT prove here, and is therefore the browser suite's: focus
+// entry, background inertness, and the viewer's real stacking against a panel
+// or a menu opened at the same time.
+
+// The bus stays REAL, with ONE wrapper: the registration is counted and its
+// disposer is tracked, because "an unmounted host stops receiving events" is
+// otherwise untestable through the DOM — a destroyed component renders nothing
+// whether or not its listener leaked, so a leak would pass silently.
+const subs = vi.hoisted(() => ({ registered: 0, disposed: 0 }));
+vi.mock('$lib/attachments/events', async (importOriginal) => {
+	const actual = await importOriginal<typeof import('$lib/attachments/events')>();
+	return {
+		...actual,
+		registerAttachmentViewerListener: (fn: Parameters<
+			typeof actual.registerAttachmentViewerListener
+		>[0]) => {
+			subs.registered += 1;
+			const off = actual.registerAttachmentViewerListener(fn);
+			return () => {
+				subs.disposed += 1;
+				off();
+			};
+		},
+	};
+});
+
+const { notifyViewerOpen } = await import('$lib/attachments/events');
+type ViewerEvent = import('$lib/attachments/events').AttachmentViewerOpenEvent;
+type LightboxImage = import('$lib/attachments/events').LightboxImage;
+const { default: AttachmentViewerHost } = await import('./AttachmentViewerHost.svelte');
+
+const ATT_ID = '11111111-2222-4333-8444-555555555555';
+const ATT_ID_2 = '99999999-8888-4777-8666-555555555555';
+
+function image(over: Partial<LightboxImage> = {}): LightboxImage {
+	return {
+		id: ATT_ID,
+		alt: 'a diagram',
+		filename: 'diagram.png',
+		mime_type: 'image/png',
+		size_bytes: 4096,
+		width: 800,
+		height: 600,
+		...over,
+	};
+}
+
+function openEvent(over: Partial<ViewerEvent> = {}): ViewerEvent {
+	return {
+		attachmentId: ATT_ID,
+		workspaceSlug: 'ws',
+		itemId: 'item-a',
+		hostToken: 'host-1',
+		images: [image()],
+		index: 0,
+		invoker: null,
+		...over,
+	};
+}
+
+/**
+ * The viewer is a fixed-position overlay rendered into its host's own mount
+ * container, which is what lets a two-host test say WHICH host opened rather
+ * than only how many viewers exist.
+ */
+function viewers(scope: ParentNode = document): HTMLElement[] {
+	return Array.from(scope.querySelectorAll<HTMLElement>('.lightbox-backdrop'));
+}
+
+function viewer(): HTMLElement | null {
+	return viewers()[0] ?? null;
+}
+
+function viewerImage(): HTMLImageElement | null {
+	return document.querySelector<HTMLImageElement>('.lightbox-image');
+}
+
+interface HostProps {
+	itemId: string | null;
+	hostToken: string;
+	resourceGen: number;
+}
+
+// Two reactive props objects, declared at the top level because `$state(...)`
+// may only initialize a declaration. Two of them because the pane host runs a
+// master and a peeked ItemDetail at once, which is exactly what DR-8's
+// addressing exists for.
+const propsA = $state<HostProps>({ itemId: 'item-a', hostToken: 'host-1', resourceGen: 1 });
+const propsB = $state<HostProps>({ itemId: 'item-a', hostToken: 'host-2', resourceGen: 1 });
+
+describe('AttachmentViewerHost', () => {
+	let target: HTMLElement;
+	const mounted: ReturnType<typeof mount>[] = [];
+
+	beforeEach(() => {
+		subs.registered = 0;
+		subs.disposed = 0;
+		Object.assign(propsA, { itemId: 'item-a', hostToken: 'host-1', resourceGen: 1 });
+		Object.assign(propsB, { itemId: 'item-a', hostToken: 'host-2', resourceGen: 1 });
+		target = document.body.appendChild(document.createElement('div'));
+	});
+
+	afterEach(() => {
+		while (mounted.length) unmount(mounted.pop()!);
+		target.remove();
+		document.querySelectorAll('.viewer-host-target').forEach((el) => el.remove());
+	});
+
+	/** Mounts a host in its OWN container, and hands that container back. */
+	function mountHost(props: HostProps): HTMLElement {
+		const container = target.appendChild(document.createElement('div'));
+		container.className = 'viewer-host-target';
+		mounted.push(mount(AttachmentViewerHost, { target: container, props }));
+		flushSync();
+		return container;
+	}
+
+	it('opens for an event addressed to it, on the event’s workspace', () => {
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+
+		expect(viewer()).not.toBeNull();
+		// The workspace is the one CAPTURED at emit, not one the host resolved:
+		// the pane switches workspace without remounting.
+		expect(viewerImage()?.getAttribute('src')).toContain('/workspaces/ws/attachments/');
+		expect(viewerImage()?.getAttribute('alt')).toBe('a diagram');
+	});
+
+	it('serves the emit-time workspace even when it is not the host’s current one', () => {
+		mountHost(propsA);
+		notifyViewerOpen(openEvent({ workspaceSlug: 'other-ws' }));
+		flushSync();
+
+		expect(viewerImage()?.getAttribute('src')).toContain('/workspaces/other-ws/attachments/');
+	});
+
+	it('ignores an event addressed to the OTHER host, with both mounted', () => {
+		const containerA = mountHost(propsA);
+		const containerB = mountHost(propsB);
+
+		// Same item, other host token: exactly one viewer may open, and it must
+		// be the ADDRESSED one — counting viewers alone would pass if the wrong
+		// host opened. Matching on itemId alone would open two.
+		notifyViewerOpen(openEvent({ hostToken: 'host-2' }));
+		flushSync();
+
+		expect(viewers()).toHaveLength(1);
+		expect(viewers(containerB)).toHaveLength(1);
+		expect(viewers(containerA)).toHaveLength(0);
+
+		// ...and the reverse direction, so neither host is simply inert.
+		notifyViewerOpen(openEvent({ hostToken: 'host-1' }));
+		flushSync();
+		expect(viewers(containerA)).toHaveLength(1);
+		expect(viewers()).toHaveLength(2);
+	});
+
+	it('ignores an event for a different item on its own token', () => {
+		mountHost(propsA);
+		notifyViewerOpen(openEvent({ itemId: 'item-b' }));
+		flushSync();
+
+		expect(viewer()).toBeNull();
+	});
+
+	it('opens at the requested index and pages through the whole set', () => {
+		mountHost(propsA);
+		notifyViewerOpen(
+			openEvent({
+				attachmentId: ATT_ID_2,
+				images: [image(), image({ id: ATT_ID_2, alt: 'second' })],
+				index: 1,
+			})
+		);
+		flushSync();
+
+		expect(viewerImage()?.getAttribute('src')).toContain(ATT_ID_2);
+		document.querySelector<HTMLButtonElement>('.lightbox-nav.prev')!.click();
+		flushSync();
+		expect(viewerImage()?.getAttribute('src')).toContain(ATT_ID);
+	});
+
+	it('REMOUNTS on a second open, so the new index is honoured', () => {
+		// Lightbox seeds `current` once through `untrack`, so a prop update
+		// would leave the second open showing the first image.
+		mountHost(propsA);
+		const images = [image(), image({ id: ATT_ID_2, alt: 'second' })];
+		notifyViewerOpen(openEvent({ images, index: 0 }));
+		flushSync();
+		expect(viewerImage()?.getAttribute('src')).toContain(ATT_ID);
+
+		notifyViewerOpen(openEvent({ attachmentId: ATT_ID_2, images, index: 1 }));
+		flushSync();
+		expect(viewers()).toHaveLength(1);
+		expect(viewerImage()?.getAttribute('src')).toContain(ATT_ID_2);
+	});
+
+	it('closes on the viewer’s own close control', () => {
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+		document.querySelector<HTMLButtonElement>('.lightbox-close')!.click();
+		flushSync();
+
+		expect(viewer()).toBeNull();
+	});
+
+	it('returns focus to the invoker on close', () => {
+		const invoker = target.appendChild(document.createElement('button'));
+		mountHost(propsA);
+		notifyViewerOpen(openEvent({ invoker }));
+		flushSync();
+		document.querySelector<HTMLButtonElement>('.lightbox-close')!.click();
+		flushSync();
+
+		expect(document.activeElement).toBe(invoker);
+	});
+
+	it('does not throw when the invoker is gone by the time the viewer closes', () => {
+		// An editor NodeView is re-rendered on any document change, so the
+		// element that opened the viewer can be detached by now.
+		const invoker = target.appendChild(document.createElement('button'));
+		mountHost(propsA);
+		notifyViewerOpen(openEvent({ invoker }));
+		flushSync();
+		invoker.remove();
+		expect(() =>
+			document.querySelector<HTMLButtonElement>('.lightbox-close')!.click()
+		).not.toThrow();
+		flushSync();
+		expect(viewer()).toBeNull();
+	});
+
+	it('closes when the host switches item', () => {
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewer()).not.toBeNull();
+
+		propsA.itemId = 'item-b';
+		flushSync();
+		expect(viewer()).toBeNull();
+	});
+
+	it('closes when the loaded resource changes under a stable item id', () => {
+		// The generation is the arm that survives a mid-switch where the id
+		// prop never visibly settles on a different value.
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+
+		propsA.resourceGen = 2;
+		flushSync();
+		expect(viewer()).toBeNull();
+	});
+
+	it('does NOT close on a same-resource refresh', () => {
+		// The whole reason for a dedicated generation: ItemDetail's
+		// `loadGeneration` is bumped by every loadData(), including the
+		// same-item reload after a collection schema edit. Keying on that would
+		// tear the viewer down on a refresh that changed nothing it can see.
+		//
+		// This is the shape a refresh has at this boundary: `loadData()` does
+		// not null `item`, and `itemMatchesRef` ignores the collection and
+		// username segments, so both a reload and a collection-only route
+		// change leave the id AND the generation exactly where they were —
+		// re-asserted, never absent. (The `loading` flip that used to destroy
+		// this host outright is why it is mounted at ItemDetail's top level.)
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+
+		propsA.itemId = 'item-a';
+		propsA.resourceGen = 1;
+		flushSync();
+
+		expect(viewer()).not.toBeNull();
+		expect(viewerImage()?.getAttribute('src')).toContain(ATT_ID);
+	});
+
+	it('closes when the item goes away without another arriving', () => {
+		// The id is `itemMatchesRef ? item?.id : null`, so null means the item
+		// this viewer belongs to is no longer the one on screen. Waiting for the
+		// next non-empty id instead would strand the viewer over a skeleton or
+		// an error page whenever the incoming item never loads.
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewer()).not.toBeNull();
+
+		propsA.itemId = null;
+		flushSync();
+		expect(viewer()).toBeNull();
+	});
+
+	it('opens normally on a host that mounted with an already-loaded resource', () => {
+		// The lifecycle rule is about TRANSITIONS. A host seeded from its
+		// initial props has nothing to clear, and must not swallow the first
+		// event because its own mount looked like a change.
+		mountHost(propsA);
+		flushSync();
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewer()).not.toBeNull();
+	});
+
+	it('unsubscribes from the bus on unmount, so a dead host receives nothing', () => {
+		// Asserted through the disposer, not the DOM: a destroyed component
+		// renders nothing whether or not its listener leaked, so a DOM-only
+		// check here would pass with the subscription still live and firing
+		// into a dead view for the rest of the session.
+		const before = subs.registered;
+		mountHost(propsA);
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewer()).not.toBeNull();
+		expect(subs.registered).toBe(before + 1);
+		expect(subs.disposed).toBe(0);
+
+		unmount(mounted.pop()!);
+		flushSync();
+		expect(subs.disposed).toBe(1);
+
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewer()).toBeNull();
+	});
+
+	it('subscribes ONCE and still addresses on its CURRENT item after a switch', () => {
+		// The registration effect reads `itemId` / `hostToken` inside the
+		// callback at EMIT time rather than in its tracked scope. Reading them
+		// outside would both re-run the effect on every item switch (tearing the
+		// listener down and re-adding it) AND capture the address, so the host
+		// would keep answering for the item it mounted on. Counting
+		// registrations catches the first half; the emissions below catch the
+		// second, which is the one that silently breaks a tap after a switch.
+		const before = subs.registered;
+		mountHost(propsA);
+		propsA.itemId = 'item-b';
+		propsA.resourceGen = 2;
+		flushSync();
+
+		// The address it mounted with is now stale and must be ignored...
+		notifyViewerOpen(openEvent({ itemId: 'item-a' }));
+		flushSync();
+		expect(viewer()).toBeNull();
+
+		// ...while the item it is showing NOW is answered.
+		notifyViewerOpen(openEvent({ itemId: 'item-b' }));
+		flushSync();
+		expect(viewer()).not.toBeNull();
+
+		expect(subs.registered).toBe(before + 1);
+		expect(subs.disposed).toBe(0);
+	});
+
+	it('keeps the flush alive: a teardown does not strand neighbouring reactivity', () => {
+		// The self-write hazard (CONVE-1688): an $effect that writes a $state it
+		// also reads in its tracked scope ABORTS the flush, which strands
+		// unrelated reactivity elsewhere in the same flush and reports nothing
+		// in a production build. A test that only checks this host's own
+		// counter can pass while the component is quietly breaking its
+		// neighbours, so the assertion has to be about a NEIGHBOUR.
+		//
+		// Two hosts driven by ONE props object: they share a flush, so if the
+		// first one's lifecycle effect aborts it, the second's teardown never
+		// runs and its viewer stays on screen. Exercised open → mutate →
+		// teardown → RE-open → mutate, because a hazard that no-ops on its
+		// first write only shows once the state has actually moved.
+		const first = mountHost(propsA);
+		const neighbour = mountHost(propsA);
+
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewers(first)).toHaveLength(1);
+		expect(viewers(neighbour)).toHaveLength(1);
+
+		propsA.resourceGen = 2;
+		flushSync();
+		expect(viewers(first)).toHaveLength(0);
+		expect(viewers(neighbour)).toHaveLength(0);
+
+		// Re-open on the same (now current) resource and drive it again.
+		notifyViewerOpen(openEvent());
+		flushSync();
+		expect(viewers(neighbour)).toHaveLength(1);
+
+		propsA.itemId = 'item-b';
+		flushSync();
+		expect(viewers(first)).toHaveLength(0);
+		expect(viewers(neighbour)).toHaveLength(0);
+
+		// And the neighbour is still LIVE, not merely emptied: it answers the
+		// new address, which it could not do if its effects had been stranded.
+		notifyViewerOpen(openEvent({ itemId: 'item-b' }));
+		flushSync();
+		expect(viewers(neighbour)).toHaveLength(1);
+	});
+
+	// The bound-close invariant is NOT tested here: driving it needs the close
+	// callback of a viewer the host has already destroyed, and a click on the
+	// detached button never reaches Svelte's delegated root handler — the
+	// version of this test written that way passed with the guard deleted.
+	// It lives in `AttachmentViewerHostClose.svelte.test.ts`, against a stubbed
+	// Lightbox that hands the callback back.
+});

web/src/lib/components/attachments/AttachmentViewerHostClose.svelte.test.ts

@@ -0,0 +1,155 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { flushSync, mount, unmount } from 'svelte';
+
+/**
+ * The bound-close invariant, on its own because it needs `Lightbox` stubbed
+ * (TASK-2428).
+ *
+ * `AttachmentViewerHost` hands each viewer a close handler BOUND to the request
+ * it was rendered for, so a continuation from a viewer the host has already
+ * destroyed cannot dismiss the one the user opened since. Driving that through
+ * the real component is impossible in jsdom: the old close button is detached
+ * by then, and a click on a detached node never reaches Svelte's delegated root
+ * handler — a test written that way passes with the guard deleted (Codex round
+ * 4). Holding the callback directly is the only honest way to fire it.
+ */
+vi.mock('$lib/components/common/Lightbox.svelte', async () => ({
+	default: (await import('./fixtures/LightboxStub.svelte')).default,
+}));
+
+const { notifyViewerOpen } = await import('$lib/attachments/events');
+const { lightboxStubCalls } = await import('./fixtures/lightboxStub');
+const { default: AttachmentViewerHost } = await import('./AttachmentViewerHost.svelte');
+type ViewerEvent = import('$lib/attachments/events').AttachmentViewerOpenEvent;
+
+const ATT_ID = '11111111-2222-4333-8444-555555555555';
+const ATT_ID_2 = '99999999-8888-4777-8666-555555555555';
+
+function openEvent(over: Partial<ViewerEvent> = {}): ViewerEvent {
+	return {
+		attachmentId: ATT_ID,
+		workspaceSlug: 'ws',
+		itemId: 'item-a',
+		hostToken: 'host-1',
+		images: [
+			{
+				id: ATT_ID,
+				alt: 'a diagram',
+				filename: 'diagram.png',
+				mime_type: 'image/png',
+				size_bytes: 4096,
+				width: 800,
+				height: 600,
+			},
+		],
+		index: 0,
+		invoker: null,
+		...over,
+	};
+}
+
+function stub(): HTMLElement | null {
+	return document.querySelector<HTMLElement>('.lightbox-stub');
+}
+
+const props = $state({ itemId: 'item-a' as string | null, hostToken: 'host-1', resourceGen: 1 });
+
+describe('AttachmentViewerHost — bound close', () => {
+	let target: HTMLElement;
+	const mounted: ReturnType<typeof mount>[] = [];
+
+	beforeEach(() => {
+		lightboxStubCalls.length = 0;
+		Object.assign(props, { itemId: 'item-a', hostToken: 'host-1', resourceGen: 1 });
+		target = document.body.appendChild(document.createElement('div'));
+		mounted.push(mount(AttachmentViewerHost, { target, props }));
+		flushSync();
+	});
+
+	afterEach(() => {
+		while (mounted.length) unmount(mounted.pop()!);
+		target.remove();
+	});
+
+	it('remounts the viewer per open, rather than re-using one instance', () => {
+		// Lightbox seeds its index once through `untrack`, so a re-used instance
+		// would silently keep showing the first image.
+		notifyViewerOpen(openEvent());
+		flushSync();
+		notifyViewerOpen(openEvent({ attachmentId: ATT_ID_2, images: [{ ...openEvent().images[0], id: ATT_ID_2 }] }));
+		flushSync();
+
+		expect(lightboxStubCalls).toHaveLength(2);
+		expect(stub()?.dataset.attachmentId).toBe(ATT_ID_2);
+	});
+
+	it('a close from a DESTROYED viewer cannot dismiss the newer one', () => {
+		notifyViewerOpen(openEvent());
+		flushSync();
+		const staleClose = lightboxStubCalls[0].onClose;
+
+		notifyViewerOpen(openEvent({ attachmentId: ATT_ID_2, images: [{ ...openEvent().images[0], id: ATT_ID_2 }] }));
+		flushSync();
+
+		staleClose();
+		flushSync();
+
+		expect(stub()).not.toBeNull();
+		expect(stub()?.dataset.attachmentId).toBe(ATT_ID_2);
+	});
+
+	it('the CURRENT viewer’s close still closes it', () => {
+		// The guard must not be so broad that it makes closing impossible.
+		notifyViewerOpen(openEvent());
+		flushSync();
+		lightboxStubCalls[0].onClose();
+		flushSync();
+
+		expect(stub()).toBeNull();
+	});
+
+	it('a close from a viewer torn down by a resource switch does not close the next one', () => {
+		notifyViewerOpen(openEvent());
+		flushSync();
+		const staleClose = lightboxStubCalls[0].onClose;
+
+		// The host switches item, destroying that viewer...
+		props.itemId = 'item-b';
+		props.resourceGen = 2;
+		flushSync();
+		expect(stub()).toBeNull();
+
+		// ...and one opens on the new item.
+		notifyViewerOpen(openEvent({ itemId: 'item-b', attachmentId: ATT_ID_2 }));
+		flushSync();
+		expect(stub()).not.toBeNull();
+
+		staleClose();
+		flushSync();
+		expect(stub()).not.toBeNull();
+	});
+
+	it('returns focus to the invoker only on the bound close', () => {
+		const invoker = target.appendChild(document.createElement('button'));
+		const other = target.appendChild(document.createElement('button'));
+		other.focus();
+
+		notifyViewerOpen(openEvent({ invoker }));
+		flushSync();
+		lightboxStubCalls[0].onClose();
+		flushSync();
+		expect(document.activeElement).toBe(invoker);
+
+		// A stale close returns nothing: it did not close anything, so moving
+		// the user's focus would be a jump out of whatever they are now in.
+		notifyViewerOpen(openEvent({ invoker }));
+		flushSync();
+		const staleClose = lightboxStubCalls[1].onClose;
+		notifyViewerOpen(openEvent({ attachmentId: ATT_ID_2, invoker: null }));
+		flushSync();
+		other.focus();
+		staleClose();
+		flushSync();
+		expect(document.activeElement).toBe(other);
+	});
+});

web/src/lib/components/attachments/fixtures/LightboxStub.svelte

@@ -0,0 +1,27 @@
+<!--
+	Test double for `Lightbox` (TASK-2428). Records the props of every mount so
+	a test can invoke a DESTROYED viewer's `onClose`, and renders a marker with
+	the image it was opened on. Deliberately dumb: the real component's
+	behaviour is covered against the real component elsewhere.
+-->
+<script lang="ts">
+	import { untrack } from 'svelte';
+	import { lightboxStubCalls, type LightboxStubCall } from './lightboxStub';
+
+	interface Props {
+		images: { id: string }[];
+		index?: number;
+		wsSlug: string;
+		onClose: () => void;
+	}
+
+	let { images, index = 0, wsSlug, onClose }: Props = $props();
+
+	// Captured ONCE at mount, `untrack`ed like the real component's index seed:
+	// the host remounts per open, so a recorded call belongs to exactly one
+	// viewer instance — which is the whole point of the recording.
+	const call: LightboxStubCall = untrack(() => ({ images, index, wsSlug, onClose }));
+	lightboxStubCalls.push(call);
+</script>
+
+<div class="lightbox-stub" data-attachment-id={images[index]?.id ?? ''} data-ws={wsSlug}></div>

web/src/lib/components/attachments/fixtures/lightboxStub.ts

@@ -0,0 +1,16 @@
+/**
+ * Recording surface for `LightboxStub.svelte` (TASK-2428).
+ *
+ * Exists so a test can hold a viewer's `onClose` AFTER that viewer has been
+ * destroyed — the only way to drive the stale-continuation case, since a click
+ * on a detached button never reaches Svelte's delegated root handler and so
+ * proves nothing (Codex round 4 found the click-based version vacuous).
+ */
+export interface LightboxStubCall {
+	images: { id: string }[];
+	index: number;
+	wsSlug: string;
+	onClose: () => void;
+}
+
+export const lightboxStubCalls: LightboxStubCall[] = [];

web/src/lib/components/items/ItemDetail.svelte

@@ -43,7 +43,9 @@
 	import CopyItemDialog from '$lib/components/items/CopyItemDialog.svelte';
 	import ItemAttachmentStrip from '$lib/components/items/ItemAttachmentStrip.svelte';
 	import AttachmentPanelHost from '$lib/components/attachments/AttachmentPanelHost.svelte';
+	import AttachmentViewerHost from '$lib/components/attachments/AttachmentViewerHost.svelte';
 	import { createAttachmentHostToken } from '$lib/attachments/events';
+	import { createViewerResourceGen } from '$lib/attachments/viewerResource.svelte';
 	import { copyToClipboard } from '$lib/utils/clipboard';
 	import { repairDeadItemLastRoute } from '$lib/collections/paneUrlParams';
 	import { isSamePaneTarget, breadcrumbParentTarget } from '$lib/collections/paneTarget';
@@ -410,6 +412,36 @@
 		onReady?.(scrollReady);
 	});
 
+	// Viewer-resource generation (PLAN-2392 / TASK-2428). A DEDICATED reactive
+	// counter that advances only when the LOADED item resource actually changes
+	// — the item this instance is showing, workspace included (IDEA-2135's
+	// same-ref-different-workspace case), and only once it matches the requested
+	// ref rather than mid-switch.
+	//
+	// It exists because neither of the two counters already here can be used:
+	// `loadGeneration` is a plain non-reactive fence bumped by EVERY loadData()
+	// — including the same-item reload after a collection schema edit — so a
+	// consumer keyed on it would tear down on a refresh that changed nothing it
+	// can see, and `itemGen` is bumped by every optimistic item write. The route
+	// is not usable either: a collection-only or username-only URL change
+	// preserves the item.
+	//
+	// Derived-then-observed rather than incremented at each `item = ...` site:
+	// there are ~30 of those and most are same-item writes. One key comparison
+	// states the rule once and cannot be forgotten by a future assignment.
+	//
+	// Both the rule and the effect that applies it live in
+	// `$lib/attachments/viewerResource*` so they are unit-testable: inline here
+	// they could only be exercised through the host's props, which are handed the
+	// answer and so cannot tell a correct counter from one that never moves
+	// (Codex round 6). The self-write discipline this effect needs — never read
+	// `gen` in the tracked scope — is documented and pinned there.
+	const viewerResource = createViewerResourceGen(() => ({
+		workspaceSlug: loadedItemWsSlug,
+		itemId: item?.id,
+		loaded: itemMatchesRef && item !== null,
+	}));
+
 	// Resolved identity for the host's `?item == master` guard (TASK-2173). Non-
 	// reactive-write $derived (CONVE-1688: the emit effect below only READS it):
 	// the loaded item's {id, ref, slug} once it matches the requested ref/slug,
@@ -5816,6 +5848,29 @@
 	{/if}
 {/if}
 
+<!-- The image viewer's host (PLAN-2392 phase 3a / TASK-2428). One per
+     `ItemDetail` mount, sharing the token the panel host uses — one token per
+     HOST, not one per channel — and carrying no `mutationsEnabled`, because
+     3a's viewer has no mutating action and a dead prop is worse than none.
+
+     TOP LEVEL, not beside `AttachmentPanelHost` inside the `{:else if item &&
+     collection}` branch, and that placement is the whole lifecycle rule: every
+     `loadData()` sets `loading = true`, which swaps that entire subtree for the
+     skeleton — including a same-item schema reload after a collection edit.
+     Mounted in there, an open viewer would be destroyed by a background refresh
+     that changed nothing the user can see. Out here it survives, and closes on
+     exactly one thing: a resource switch, via the props below.
+
+     `itemId` is gated on `itemMatchesRef` (the TASK-2112 switch boundary) so a
+     mid-switch host addresses nothing; `resourceGen` is the dedicated counter
+     that advances only on a real loaded-item resource change — never on
+     `loadGeneration`, which every refresh bumps. -->
+<AttachmentViewerHost
+	itemId={itemMatchesRef ? item?.id : null}
+	hostToken={attachmentHostToken}
+	resourceGen={viewerResource.current}
+/>
+
 <style>
 	.center-message {
 		display: flex;