fix(attachments): close four review findings across the wave-A surfaces

From the orchestrator's fresh-angle Codex pass. All four are the same
shape — something read after an await, or captured once and never
refreshed — on a component tree built around a no-{#key} item switch.

- The delete descriptor snapshotted identity AFTER its confirmation, so
  an async in-app confirm (which is what DR-18 asks for) left a window
  where the user could switch items and delete the attachment they were
  no longer looking at. Snapshot first, re-check the gate and the
  identity on the way out.
- MenuItem's anchor rows had no Space activation. Native anchors take
  Enter only, and role=menuitem does not add it, so Space would do
  nothing on Download and Open while working on every button row beside
  them.
- The image NodeView had no destroyed flag, so a HEAD probe in flight at
  teardown could latch a placeholder onto detached DOM. The chip NodeView
  has always had one.
- CommentEditor configures its extensions once in onMount, but the
  composer is deliberately reused across an item switch, so its chips
  kept emitting events addressed to the PREVIOUS item — which the host
  then correctly ignored, i.e. a tap that silently did nothing. Push the
  addressing onto the live options, the same way capabilities are pushed.

The first two fixes are mutation-tested: reverting either fails the new
test.

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

@@ -220,6 +220,50 @@ describe('attachment action descriptors', () => {
 		expect(announceMock).not.toHaveBeenCalled();
 	});
 
+	it('deletes the attachment the user confirmed, not whatever the context holds later', async () => {
+		// An in-app confirmation is a whole UI interaction, so the surface can
+		// switch items underneath it — the pane this renders in is built around
+		// a no-{#key} A→B switch. The descriptor must act on what was confirmed.
+		const live: Ctx = ctx();
+		const del = action('delete');
+		if (del.element !== 'button') throw new Error('unreachable');
+
+		await del.run({
+			...live,
+			confirmDelete: async () => {
+				live.attachment = { id: 'att-OTHER', filename: 'other.pdf', mime_type: 'application/pdf' };
+				return true;
+			},
+			get attachment() {
+				return live.attachment;
+			},
+		} as Ctx);
+
+		// Identity moved while the confirmation was open, so the delete is
+		// abandoned rather than aimed at the newly-shown attachment.
+		expect(deleteMock).not.toHaveBeenCalled();
+		expect(announceMock).not.toHaveBeenCalled();
+	});
+
+	it('abandons the delete if mutation rights are lost while the confirmation is open', async () => {
+		const live: Ctx = ctx();
+		const del = action('delete');
+		if (del.element !== 'button') throw new Error('unreachable');
+
+		await del.run({
+			...live,
+			confirmDelete: async () => {
+				live.mutationsEnabled = false;
+				return true;
+			},
+			get mutationsEnabled() {
+				return live.mutationsEnabled;
+			},
+		} as Ctx);
+
+		expect(deleteMock).not.toHaveBeenCalled();
+	});
+
 	it('treats a 404 as authoritative: broadcasts and does not throw', async () => {
 		deleteMock.mockRejectedValue(Object.assign(new Error('gone'), { code: 'not_found' }));
 		const onDeleted = vi.fn();

web/src/lib/attachments/actions.ts

@@ -208,13 +208,27 @@ export const ATTACHMENT_ACTIONS: readonly AttachmentAction[] = [
 			// Belt and braces: a renderer that draws a disabled row can still
 			// be asked to run it by a stray keyboard activation.
 			if (!ctx.mutationsEnabled || !addressable(ctx)) return;
-			if (ctx.confirmDelete && !(await ctx.confirmDelete(ctx.attachment))) return;
 
-			// Capture identity before the await: the surface may switch views
-			// mid-flight, and the broadcast + metadata-cache key must name the
-			// workspace the DELETE actually targeted.
+			// Snapshot identity BEFORE the confirmation, not after. `ctx` may be
+			// a live object owned by a surface that survives an item switch (the
+			// no-{#key} pane is built around exactly that), and `confirmDelete`
+			// is allowed to be async — an in-app confirmation is a whole UI
+			// interaction, so the user has all the time in the world to switch
+			// items or lose their mutation rights while it is up. Reading
+			// `ctx.attachment.id` after the await could name a DIFFERENT
+			// attachment than the one the user was shown.
 			const ws = ctx.workspaceSlug;
 			const id = ctx.attachment.id;
+			const subject = { ...ctx.attachment };
+
+			if (ctx.confirmDelete && !(await ctx.confirmDelete(subject))) return;
+
+			// Re-check the gate on the way out of the confirmation: permission
+			// can be revoked while it is open (a pane being peeked closes the
+			// mutation gate), and identity must still agree with what was
+			// confirmed.
+			if (!ctx.mutationsEnabled) return;
+			if (ctx.workspaceSlug !== ws || ctx.attachment.id !== id) return;
 			try {
 				await api.attachments.delete(ws, id);
 			} catch (err) {

web/src/lib/components/CommentEditor.svelte

@@ -206,6 +206,37 @@
 		empty = editor.isEmpty;
 	});
 
+	/**
+	 * Keep the attachment NodeViews' addressing current (PLAN-2392 DR-8).
+	 *
+	 * The extensions are configured once, inside `onMount`, which captures
+	 * whatever `itemId` / `hostToken` were at that moment. That is fine for the
+	 * body editor — it is remounted per item behind a `{#key}` — but this
+	 * composer is deliberately REUSED across a no-{#key} item switch (see
+	 * `doSubmit` above, which exists for the same reason). Left alone, a chip
+	 * in the composer would keep emitting events addressed to the PREVIOUS
+	 * item, and the host, which matches on both fields, would correctly ignore
+	 * them — a tap that silently does nothing.
+	 *
+	 * Mutating the live extension options is the established shape here;
+	 * `Editor.svelte` pushes `supportedFormats` the same way once server
+	 * capabilities resolve. The NodeViews read these at emit time, so a write
+	 * is enough — nothing needs to re-render.
+	 */
+	$effect(() => {
+		const nextItemId = itemId ?? '';
+		const nextToken = hostToken;
+		if (!editor || editor.isDestroyed) return;
+		for (const name of ['attachmentChip', 'attachmentImage']) {
+			const ext = editor.extensionManager.extensions.find(
+				(e: { name: string }) => e.name === name
+			);
+			if (!ext) continue;
+			ext.options.itemId = nextItemId;
+			ext.options.hostToken = nextToken;
+		}
+	});
+
 	onDestroy(() => {
 		editor?.destroy();
 	});

web/src/lib/components/common/MenuItem.svelte

@@ -60,6 +60,20 @@
 	// skips rows with `[role^="menuitem"]:not(:disabled)` (Menu.svelte:130),
 	// which no anchor can ever match.
 	const asAnchor = $derived(href !== undefined && !disabled);
+
+	/**
+	 * A native anchor activates on Enter but NOT on Space, while every other
+	 * row in this menu is a <button>, which activates on both. `role="menuitem"`
+	 * doesn't add the behavior — it only changes what is announced — so without
+	 * this, Space would silently do nothing on exactly the rows a keyboard user
+	 * is most likely to try it on (Download, Open).
+	 */
+	function anchorKeydown(e: KeyboardEvent) {
+		if (e.key !== ' ' && e.key !== 'Spacebar') return;
+		// Space scrolls the page by default; the menu is the active surface.
+		e.preventDefault();
+		(e.currentTarget as HTMLAnchorElement).click();
+	}
 </script>
 
 {#snippet body()}
@@ -85,6 +99,7 @@
 		aria-checked={checked}
 		aria-describedby={describedBy}
 		{onclick}
+		onkeydown={anchorKeydown}
 	>
 		{@render body()}
 	</a>

web/src/lib/components/common/MenuItem.svelte.test.ts

@@ -126,6 +126,27 @@ describe('MenuItem.svelte', () => {
 		expect(el.hasAttribute('download')).toBe(false);
 	});
 
+	it('activates an anchor row on Space, like every other row in the menu', async () => {
+		// A native anchor activates on Enter but not Space, and role="menuitem"
+		// does not add the behavior — so without an explicit handler, Space
+		// would silently do nothing on Download and Open while working on
+		// every button row beside them.
+		const onclick = vi.fn();
+		render(MenuItem, {
+			props: { href: '/api/v1/workspaces/ws/attachments/att-1', onclick, children: label },
+		});
+		await tick();
+
+		const el = row() as HTMLAnchorElement;
+		const evt = new KeyboardEvent('keydown', { key: ' ', bubbles: true, cancelable: true });
+		el.dispatchEvent(evt);
+		await tick();
+
+		expect(onclick).toHaveBeenCalledTimes(1);
+		// Space scrolls the page by default; the open menu is the active surface.
+		expect(evt.defaultPrevented).toBe(true);
+	});
+
 	it('renders a disabled anchor as a disabled button so keyboard nav skips it', async () => {
 		render(MenuItem, {
 			props: { href: '/api/v1/workspaces/ws/attachments/att-1', disabled: true, children: label },

web/src/lib/components/editor/attachment-image.ts

@@ -290,6 +290,9 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
 			// Deletion is authoritative: a load still in flight when it lands
 			// must not be allowed to paint the image back (Codex round 15).
 			let deleted = false;
+			// True once the NodeView is torn down. Async continuations (HEAD
+			// probes, transform results) must not touch DOM after that.
+			let destroyed = false;
 
 			function showMissing() {
 				missing.textContent = `📎 ${currentAlt || 'Attachment unavailable'}`;
@@ -337,7 +340,7 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
 			 * roll back, so undo restores a node pointing at nothing.
 			 */
 			function latchMissing(forUuid: string) {
-				if (deleted) return;
+				if (destroyed || deleted) return;
 				if (!forUuid || currentUuid !== forUuid) return;
 				deleted = true;
 				// Same reason the deletion listener does this: an in-flight
@@ -360,7 +363,7 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
 			 * placeholder permanently unlatchable.
 			 */
 			function probeForMissing(forUuid: string) {
-				if (!forUuid || !opts.workspaceSlug || deleted) return;
+				if (!forUuid || !opts.workspaceSlug || deleted || destroyed) return;
 				void revalidateAttachmentMetadata(opts.workspaceSlug, forUuid, opts.getDownloadUrl).then(
 					(result) => {
 						if (result.status === 'missing') latchMissing(forUuid);
@@ -695,6 +698,11 @@ export const AttachmentImage = Node.create<AttachmentImageOptions>({
 					if (toolbar) toolbar.classList.add('attachment-image-toolbar-hidden');
 				},
 				destroy() {
+					// Set FIRST: every async continuation below fences on it, and
+					// a HEAD probe in flight at teardown would otherwise resolve
+					// into detached DOM (the chip NodeView has carried this flag
+					// since it was written; the image one did not).
+					destroyed = true;
 					detachLoadListeners();
 					disposeDeletionListener();
 					// Tear down the refresher subscription so the