refactor(attachments): extract the view-identity fence into one module

Four review rounds running found the same bug class: a continuation resuming after an await and writing state that belongs to a view the user has already left. Each round fixed instances; the next found more. The tripwire an earlier round set has been hit, so the invariant is hoisted into one implementation instead of N call sites agreeing by convention. web/src/lib/attachments/viewFence.ts owns it now: - viewIdentity(read) — the ONE place a component states what names its view. Returns tokens carrying a SNAPSHOT of the parts, so a continuation reads the workspace it was issued for off the token rather than off the live prop. A missing part voids the whole key, and a null key never matches — a half-identified view cannot pass a fence. - createFence(identity) — generation + identity. begin() coexists with its siblings; restart() supersedes them; invalidate() ends them. Used twice per surface, for fences 1 and 2. - createPaintFence(identity) — fence 3, the paint-time entry check. All three stay distinct: a prior round established that collapsing any two loses either A→B suppression or same-item-Retry reconciliation. Both consumers now build all three from a single identity declaration, so no call site can restate a shorter one — which was the recurring mistake (the workspace half kept going missing). Two outstanding findings fixed alongside: - StorageTab delete was not workspace-fenced: it used the live wsSlug after its await, so an A→B switch mid-request let the success/404 handling toast and reload against B. It now takes the workspace off the PAINTED identity (so the DELETE targets the row the user actually clicked), refuses a click whose paint is already stale, and fences the toast + reload. The broadcast stays ahead of the fence: a global (workspace, id) side effect, not a write into this view. - The strip's pendingUploads could resurrect externally deleted rows: the buffer was retained indefinitely and no successful response ever consumed it, so a deletion from another tab, followed by a load that legitimately returned no row, merged the stale upload back in — and kept doing so. A response is now treated as authoritative about the entries the buffer already held when that request went OUT; entries announced while it was in flight (the buffer's actual purpose) are untouched. No behaviour change from the refactor: all 592 existing tests pass unmodified. 18 added — 14 unit tests on the module, 2 per fix. Every new test mutation-tested. Claude-Session: https://claude.ai/code/session_01LmbFxQFDjcYKBLcTnor6DC
2026-09-24 19:32:10 +00:00 · 2026-08-03 16:30:18 +00:00
parent 3b4331dbe6
commit 783e9ef0f8
6 changed files with 955 additions and 175 deletions
@@ -0,0 +1,226 @@
+import { describe, it, expect } from 'vitest';
+import { viewIdentity, createFence, createPaintFence } from './viewFence';
+
+// The invariant these fences encode failed four review rounds running, always
+// by comparing only PART of the identity. These tests pin the module directly:
+// the identity rules, then each of the three fences, then the two collapses
+// that a prior round proved are NOT safe.
+
+describe('viewIdentity', () => {
+	it('is the whole record, not any one part', () => {
+		let ws = 'ws-a';
+		let item = 'item-1';
+		const view = viewIdentity(() => ({ ws, item }));
+
+		const token = view.capture();
+		expect(token.changed()).toBe(false);
+
+		// The item half.
+		item = 'item-2';
+		expect(token.changed()).toBe(true);
+		item = 'item-1';
+		expect(token.changed()).toBe(false);
+
+		// The workspace half — the one that kept getting forgotten.
+		ws = 'ws-b';
+		expect(token.changed()).toBe(true);
+	});
+
+	it('is not addressable while any part is missing', () => {
+		let item: string | null = null;
+		const view = viewIdentity(() => ({ ws: 'ws-a', item }));
+
+		expect(view.key()).toBeNull();
+		// A half-identified view must not pass a fence just because the half it
+		// does know still matches.
+		const blank = view.capture();
+		expect(blank.changed()).toBe(true);
+
+		item = '';
+		expect(view.key()).toBeNull();
+
+		item = 'item-1';
+		expect(view.key()).not.toBeNull();
+		expect(view.capture().changed()).toBe(false);
+	});
+
+	it('never matches a null key', () => {
+		const view = viewIdentity(() => ({ ws: 'ws-a' }));
+		expect(view.matches(null)).toBe(false);
+		expect(view.matches(view.key())).toBe(true);
+	});
+
+	it('cannot confuse two different splits of the same characters', () => {
+		const a = viewIdentity(() => ({ ws: 'x', item: 'yz' })).key();
+		const b = viewIdentity(() => ({ ws: 'xy', item: 'z' })).key();
+		expect(a).not.toBe(b);
+	});
+
+	it('cannot be collided by parts that embed the serialiser\'s own syntax', () => {
+		// The specific way a delimiter join fails: a part containing the entry
+		// separator FOLLOWED BY the next part's name and the name/value
+		// separator re-creates the exact byte sequence the join itself emits, so
+		// two DIFFERENT identities serialise the same — and a stale fence passes,
+		// which is the failure this module exists to prevent (Codex round 1).
+		// Reproduced for the NUL/SOH join this module used to use, and for
+		// separators built from the characters JSON has to escape.
+		const shapes: [string, string][] = [
+			[String.fromCharCode(0), String.fromCharCode(1)],
+			['|', ':'],
+			['"', ','],
+			['\\', '"'],
+			[']', '['],
+		];
+		for (const [entrySep, pairSep] of shapes) {
+			const a = viewIdentity(() => ({ item: `x${entrySep}ws${pairSep}y`, ws: 'z' })).key();
+			const b = viewIdentity(() => ({ item: 'x', ws: `y${entrySep}ws${pairSep}z` })).key();
+			expect(a).not.toBeNull();
+			expect(a).not.toBe(b);
+		}
+
+		// A join that drops the part NAMES collides without any crafted run at
+		// all: `x<sep>y` + `z` and `x` + `y<sep>z` are the same list of values.
+		for (const sep of [',', '|', String.fromCharCode(0)]) {
+			expect(viewIdentity(() => ({ item: `x${sep}y`, ws: 'z' })).key()).not.toBe(
+				viewIdentity(() => ({ item: 'x', ws: `y${sep}z` })).key()
+			);
+		}
+	});
+
+	it('does not depend on the order the parts are written in', () => {
+		const a = viewIdentity(() => ({ ws: 'w', item: 'i' })).key();
+		const b = viewIdentity(() => ({ item: 'i', ws: 'w' })).key();
+		expect(a).toBe(b);
+	});
+
+	it('snapshots the parts, so a request reads back what it was ISSUED for', () => {
+		let ws = 'ws-a';
+		const view = viewIdentity(() => ({ ws }));
+		const token = view.capture();
+		ws = 'ws-b';
+		// This is what stops a continuation from calling the API — or keying a
+		// cache invalidation — with whichever workspace happens to be live.
+		expect(token.value.ws).toBe('ws-a');
+	});
+});
+
+describe('createFence (fences 1 and 2)', () => {
+	it('stales a token once the fence restarts — same identity', () => {
+		const view = viewIdentity(() => ({ ws: 'ws-a', item: 'item-1' }));
+		const fence = createFence(view);
+
+		const first = fence.begin();
+		expect(first.stale()).toBe(false);
+		const second = fence.restart();
+		// A→A: the identity matches, so only the generation can tell a
+		// superseded request from the current one.
+		expect(first.stale()).toBe(true);
+		expect(second.stale()).toBe(false);
+	});
+
+	it('stales a token once the identity moves — same generation', () => {
+		let ws = 'ws-a';
+		const fence = createFence(viewIdentity(() => ({ ws })));
+
+		const token = fence.begin();
+		expect(token.stale()).toBe(false);
+		ws = 'ws-b';
+		expect(token.stale()).toBe(true);
+	});
+
+	it('stales across an A→B→A round trip, where the identity matches again', () => {
+		let ws = 'ws-a';
+		const fence = createFence(viewIdentity(() => ({ ws })));
+
+		const first = fence.restart();
+		ws = 'ws-b';
+		fence.restart();
+		ws = 'ws-a';
+		const current = fence.restart();
+
+		// The identity is back to A, so the identity compare alone would let
+		// A's first response write over A's current one.
+		expect(first.stale()).toBe(true);
+		expect(current.stale()).toBe(false);
+	});
+
+	it('begin() does not supersede its siblings; restart() does', () => {
+		const fence = createFence(viewIdentity(() => ({ ws: 'ws-a' })));
+
+		// Two concurrent mutations must both still be allowed to reconcile.
+		const one = fence.begin();
+		const two = fence.begin();
+		expect(one.stale()).toBe(false);
+		expect(two.stale()).toBe(false);
+
+		fence.invalidate();
+		expect(one.stale()).toBe(true);
+		expect(two.stale()).toBe(true);
+	});
+
+	it('keeps the request and view fences independent', () => {
+		// The whole reason there are two: a Retry restarts the REQUEST fence
+		// (superseding its own in-flight listing) without touching the VIEW
+		// fence, so a delete of a row still on screen keeps reconciling.
+		const view = viewIdentity(() => ({ ws: 'ws-a', item: 'item-1' }));
+		const requests = createFence(view);
+		const views = createFence(view);
+
+		const del = views.begin();
+		requests.restart(); // the Retry
+		expect(del.stale()).toBe(false);
+
+		views.invalidate(); // a real switch, or unmount
+		expect(del.stale()).toBe(true);
+	});
+});
+
+describe('createPaintFence (fence 3)', () => {
+	it('answers about the DOM, lagging the live props on purpose', () => {
+		let ws = 'ws-a';
+		const view = viewIdentity(() => ({ ws }));
+		const paint = createPaintFence(view);
+
+		expect(paint.isCurrent()).toBe(false); // nothing painted yet
+		paint.record(view.capture());
+		expect(paint.isCurrent()).toBe(true);
+
+		// Props update synchronously; the effect that repaints flushes later.
+		// In that window the controls on screen still belong to ws-a, and a
+		// click on one must be refused.
+		ws = 'ws-b';
+		expect(paint.isCurrent()).toBe(false);
+		expect(paint.painted()?.value.ws).toBe('ws-a');
+
+		paint.record(view.capture());
+		expect(paint.isCurrent()).toBe(true);
+	});
+
+	it('claims nothing for an un-addressable view', () => {
+		let item: string | null = null;
+		const view = viewIdentity(() => ({ ws: 'ws-a', item }));
+		const paint = createPaintFence(view);
+
+		paint.record(view.capture());
+		expect(paint.painted()).toBeNull();
+		expect(paint.isCurrent()).toBe(false);
+
+		// And a run that bails on a missing id stops claiming the previous view.
+		item = 'item-1';
+		paint.record(view.capture());
+		expect(paint.isCurrent()).toBe(true);
+		item = null;
+		paint.record(view.capture());
+		expect(paint.painted()).toBeNull();
+	});
+
+	it('is not a generation fence: repainting the same view stays current', () => {
+		const view = viewIdentity(() => ({ ws: 'ws-a' }));
+		const paint = createPaintFence(view);
+		const first = view.capture();
+		paint.record(first);
+		paint.record(view.capture());
+		expect(paint.isCurrent()).toBe(true);
+		expect(first.changed()).toBe(false);
+	});
+});
@@ -0,0 +1,207 @@
+/**
+ * The view-identity fence — ONE implementation of the invariant that a
+ * continuation resuming after an `await` may only write state belonging to the
+ * view the user is still looking at.
+ *
+ * WHY THIS EXISTS. Attachment surfaces (the item attachment strip, the Storage
+ * tab) stay MOUNTED across the navigations that change what they show: the
+ * strip lives outside ItemDetail's `{#key itemSlug}` block, and
+ * `/{user}/{ws}/settings` is one SvelteKit route, so a workspace switch changes
+ * `wsSlug` under a mounted tab. Every fetch, mutation and timer therefore has to
+ * say which view it belongs to. Written out by hand at each call site, that
+ * invariant failed the same way four review rounds running — always by
+ * comparing only PART of the identity, almost always by forgetting the
+ * workspace half. This module makes the identity a single value that is read in
+ * exactly one place per component and carried on a token, so no call site can
+ * assemble a partial one.
+ *
+ * THREE FENCES, THREE QUESTIONS. They are deliberately NOT collapsible into one
+ * — a prior review round established that merging any two loses either A→B
+ * suppression or same-item-Retry reconciliation:
+ *
+ *   1. REQUEST fence (`createFence`, restarted per request) — "is this RESPONSE
+ *      still the current one?" A Retry supersedes its own predecessor here.
+ *   2. VIEW fence (`createFence`, invalidated only when the view really
+ *      changes) — "may this async CONTINUATION still reconcile local state?" A
+ *      Retry is the same view reloading and must NOT invalidate it, or an
+ *      in-flight delete of a row still on screen stops rolling back.
+ *   3. PAINT-TIME fence (`createPaintFence`) — "does the CONTROL the user
+ *      clicked belong to what is on screen?" Props update synchronously and
+ *      effects flush later, so in between the DOM still shows the previous
+ *      view. This one has to be at ENTRY: the other two run after an await, and
+ *      no fence can unsend a request.
+ *
+ * Both (1) and (2) compare a captured GENERATION as well as the identity,
+ * because a counter alone misses A→B→A (the identity matches again) and an
+ * identity alone misses A→A (a superseded request for the same view).
+ */
+
+/**
+ * The parts that name a view. A record rather than a tuple so the captured
+ * snapshot can be read back by name — `token.value.ws` is the workspace the
+ * request was ISSUED for, which is what callers should pass to the API instead
+ * of re-reading the live prop.
+ */
+export type IdentityParts = Record<string, string | null | undefined>;
+
+/**
+ * Serialise the parts into a comparable key, or null when the view is not
+ * addressable yet.
+ *
+ * A MISSING PART MAKES THE WHOLE KEY NULL. That is the load-bearing rule: a
+ * view named by (workspace, item) is not identifiable while either half is
+ * unknown, and a null key never matches anything — so a half-identified view
+ * can't accidentally pass a fence.
+ */
+function serialize(parts: IdentityParts): string | null {
+	// Sorted so the key does not depend on property declaration order.
+	const names = Object.keys(parts).sort();
+	if (names.length === 0) return null;
+	const pairs: [string, string][] = [];
+	for (const name of names) {
+		const value = parts[name];
+		if (value === null || value === undefined || value === '') return null;
+		pairs.push([name, value]);
+	}
+	// JSON rather than a delimiter join: a hand-rolled separator has to be a
+	// character the parts cannot contain, and "identity parts are always slugs
+	// and uuids" is an assumption this module has no way to enforce. JSON quotes
+	// and escapes both halves, so no two distinct part sets can serialise the
+	// same — a collision here would silently let a stale fence pass, which is
+	// the exact failure this module exists to prevent (Codex round 1).
+	return JSON.stringify(pairs);
+}
+
+/** A captured identity: what the view was named when this token was taken. */
+export interface IdentityToken<T extends IdentityParts> {
+	/** Snapshot of the parts, for reading back the workspace/item that was captured. */
+	readonly value: T;
+	/** The comparable key, or null when the view wasn't addressable. */
+	readonly key: string | null;
+	/**
+	 * True when the live view no longer matches this capture — including when
+	 * the capture itself was never addressable.
+	 */
+	changed(): boolean;
+}
+
+export interface ViewIdentity<T extends IdentityParts> {
+	/** The live key, or null when a part is missing. */
+	key(): string | null;
+	/** True when `key` names the live view. A null key never matches. */
+	matches(key: string | null): boolean;
+	/** Take a token for the live view. */
+	capture(): IdentityToken<T>;
+}
+
+/**
+ * Declare what names a view. `read` is called on every capture/compare and must
+ * read the LIVE reactive values:
+ *
+ *     const view = viewIdentity(() => ({ ws: wsSlug, item: itemId }));
+ *
+ * This is the ONE place a component states its identity; every fence below is
+ * built from it, so there is no second call site that could state a shorter one.
+ */
+export function viewIdentity<T extends IdentityParts>(read: () => T): ViewIdentity<T> {
+	const identity: ViewIdentity<T> = {
+		key: () => serialize(read()),
+		matches: (key) => key !== null && key === serialize(read()),
+		capture: () => {
+			const value = { ...read() } as T;
+			const key = serialize(value);
+			return {
+				value,
+				key,
+				changed: () => key === null || !identity.matches(key),
+			};
+		},
+	};
+	return identity;
+}
+
+/** An identity token that also carries a generation. */
+export interface FenceToken<T extends IdentityParts> extends IdentityToken<T> {
+	/**
+	 * True when this token may no longer write: the fence was invalidated (or
+	 * restarted) since it was taken, or the view moved on.
+	 */
+	stale(): boolean;
+}
+
+export interface Fence<T extends IdentityParts> {
+	/**
+	 * Take a token WITHOUT superseding outstanding ones. For the view fence, and
+	 * for anything that must coexist with its siblings — two concurrent deletes
+	 * must not invalidate each other.
+	 */
+	begin(): FenceToken<T>;
+	/**
+	 * Supersede every outstanding token, then take a new one. For a request
+	 * fence: issuing request N+1 means request N's response may no longer write.
+	 */
+	restart(): FenceToken<T>;
+	/** Supersede every outstanding token without taking a new one. */
+	invalidate(): void;
+}
+
+/**
+ * A generation + identity fence. Two of these per surface: one restarted per
+ * request (fence 1) and one invalidated only on a real view change (fence 2).
+ */
+export function createFence<T extends IdentityParts>(identity: ViewIdentity<T>): Fence<T> {
+	let generation = 0;
+
+	function take(): FenceToken<T> {
+		const captured = identity.capture();
+		const gen = generation;
+		return {
+			value: captured.value,
+			key: captured.key,
+			changed: captured.changed,
+			stale: () => gen !== generation || captured.changed(),
+		};
+	}
+
+	return {
+		begin: take,
+		restart: () => {
+			generation++;
+			return take();
+		},
+		invalidate: () => {
+			generation++;
+		},
+	};
+}
+
+export interface PaintFence<T extends IdentityParts> {
+	/**
+	 * Claim what is now on screen. Recording a token whose key is null (an
+	 * un-addressable view) correctly claims nothing.
+	 */
+	record(token: IdentityToken<T> | null): void;
+	/** The identity the last paint claimed, for reading the parts back. */
+	painted(): IdentityToken<T> | null;
+	/** True when what is painted is what the live props name. */
+	isCurrent(): boolean;
+}
+
+/**
+ * Fence 3. Deliberately LAGS the live props by the prop-update → effect-flush
+ * window: that lag is the only thing that can answer "was this control rendered
+ * for the view that is on screen NOW?", which the live props cannot — they
+ * already read the NEW view while the OLD controls are still mounted.
+ */
+export function createPaintFence<T extends IdentityParts>(
+	identity: ViewIdentity<T>
+): PaintFence<T> {
+	let painted: IdentityToken<T> | null = null;
+	return {
+		record: (token) => {
+			painted = token && token.key !== null ? token : null;
+		},
+		painted: () => painted,
+		isCurrent: () => painted !== null && identity.matches(painted.key),
+	};
+}
@@ -19,31 +19,22 @@
 	 * block, so this component PERSISTS across an A→B item switch, per the
 	 * no-{#key} bug class from PLAN-2105 / TASK-2112.
 	 *
-	 * THE FENCE MODEL. Identity is the PAIR (workspace, item) everywhere — see
-	 * `viewKey`. There are exactly three fences, because there are exactly
-	 * three distinct questions, each asked at a different moment:
+	 * THE FENCE MODEL lives in `$lib/attachments/viewFence`, which owns the
+	 * whole invariant — read its header for why there are exactly three fences
+	 * and why they cannot be collapsed. This component supplies the identity
+	 * once (`view`, the PAIR (workspace, item)) and builds all three from it:
 	 *
-	 *   1. `switchedAway(gen, item, ws)` — "is this RESPONSE still current?"
-	 *      Guards every await-then-write inside the load effect, against the
-	 *      REQUEST generation. A Retry supersedes its own predecessor here.
-	 *   2. `viewChanged(gen, item, ws)` — "may this async CONTINUATION still
-	 *      reconcile local state?" Guards mutation continuations, against the
-	 *      VIEW generation, which a Retry deliberately does not bump: a delete
-	 *      of a row still on screen must roll back and toast even if the user
-	 *      hit Retry while it was in flight.
-	 *   3. PAINT-TIME identity — "does the CONTROL the user clicked belong to
-	 *      what is on screen?" Props update synchronously and effects flush
-	 *      later, so in between, the DOM still shows the previous view. Every
-	 *      entry point reached from a rendered control validates against an
-	 *      identity RECORDED WHEN THE EFFECT PAINTED (`paintedView`), never
-	 *      against the live props. Both control entry points use it —
-	 *      `handleDelete` for a tile, `retryLoad` for the error row. This fence
-	 *      has to be at ENTRY: the other two run after an await, and no fence
-	 *      can unsend a request.
-	 *
-	 * (1) and (2) compare a captured generation AND the pair, because a
-	 * generation counter alone misses A→B→A and the pair alone misses A→B→A.
-	 * (3) is a pure pair compare: it asks about the DOM, not about ordering.
+	 *   1. `loadFence` — "is this RESPONSE still current?" Restarted per
+	 *      request; guards every await-then-write inside the load effect.
+	 *   2. `viewFence` — "may this async CONTINUATION still reconcile local
+	 *      state?" Invalidated only when the view really changes, so a Retry
+	 *      (the same view reloading) leaves it alone: a delete of a row still
+	 *      on screen must roll back and toast even if the user hit Retry while
+	 *      it was in flight.
+	 *   3. `paint` — "does the CONTROL the user clicked belong to what is on
+	 *      screen?" Both control entry points fence on it — `handleDelete` for
+	 *      a tile, `retryLoad` for the error row — at ENTRY, because the other
+	 *      two run after an await and no fence can unsend a request.
 	 */
 	import { onDestroy, untrack } from 'svelte';
 	import { api, PadApiError } from '$lib/api/client';
@@ -59,6 +50,7 @@
 		registerAttachmentDeletionListener,
 		registerAttachmentUploadListener,
 	} from '$lib/attachments/events';
+	import { viewIdentity, createFence, createPaintFence } from '$lib/attachments/viewFence';

 	interface Props {
 		wsSlug: string;
@@ -175,43 +167,36 @@
 	 */
 	let retryRequestedFor: string | null = null;
 	/**
-	 * View identity key. `itemId` alone isn't it: `wsSlug` is reactive and the
-	 * strip stays mounted across a workspace change, so keying on the item
-	 * would classify that as a same-view Retry — leaving the previous
-	 * workspace's rows painted and its in-flight mutations still reconciling
-	 * (Codex fresh-angle round 2).
+	 * The ONE statement of what names this view. `itemId` alone isn't it:
+	 * `wsSlug` is reactive and the strip stays mounted across a workspace
+	 * change, so keying on the item would classify that as a same-view Retry —
+	 * leaving the previous workspace's rows painted and its in-flight mutations
+	 * still reconciling (Codex fresh-angle round 2). Declared once here so no
+	 * individual fence call site can restate a shorter identity; every fence
+	 * below derives from it, and every captured workspace is read back off a
+	 * token rather than off the live prop.
 	 */
-	function viewKey(ws: string, id: string): string {
-		return `${ws}\u0000${id}`;
-	}
-
+	const view = viewIdentity(() => ({ ws: wsSlug, item: itemId }));
+	/** Fence 1 — request generation. Restarted on every (re)run of the load effect. */
+	const loadFence = createFence(view);
 	/**
-	 * VIEW the currently painted tiles belong to — written by the load effect,
-	 * so it lags the live props by exactly the prop-update → effect-flush
-	 * window. That lag is the point: it is the only thing that can answer "was
-	 * this control rendered for the view that is on screen NOW?", which the
-	 * live props cannot (they already read the NEW view while the OLD tiles are
-	 * still mounted). Entry fence for `handleDelete` — the `viewChanged` check
-	 * in its catch runs after the await and cannot unsend a DELETE aimed at the
-	 * wrong item or workspace (final review round 2). Non-reactive on purpose:
-	 * read at click time, it must not re-trigger the render it describes.
+	 * Fence 2 — view generation. Invalidated only when the view actually
+	 * changes: a different (item, workspace), or unmount. A Retry is the SAME
+	 * item reloading, so it leaves this alone: a delete of a row still on screen
+	 * must reconcile (roll back, toast) even if the user hit Retry while it was
+	 * in flight. Fencing mutations on the REQUEST generation instead made a
+	 * Retry masquerade as an item switch and swallowed the failure.
 	 */
-	let paintedView: string | null = null;
+	const viewFence = createFence(view);
+	/**
+	 * Fence 3 — the view the currently painted tiles belong to. Recorded by the
+	 * load effect, so it lags the live props by exactly the prop-update →
+	 * effect-flush window; that lag is what makes it the only thing that can
+	 * answer "was this control rendered for the view that is on screen NOW?"
+	 */
+	const paint = createPaintFence(view);
+

-	// Two generations, because "which request is this?" and "which item is on
-	// screen?" are different questions and a Retry answers them differently
-	// (final-review P2).
-	//
-	// REQUEST generation — bumped on every (re)run of the fetch effect,
-	// including a Retry, so a superseded list() response can never write.
-	let loadGeneration = 0;
-	// VIEW generation — bumped only when the view actually changes: a different
-	// (item, workspace), or unmount. A Retry is the SAME item reloading, so it
-	// leaves this alone. Mutations fence on this: a delete of a row still on
-	// screen must reconcile (roll back, toast, broadcast) even if the user hit
-	// Retry while it was in flight. Fencing them on the request generation made
-	// a Retry masquerade as an item switch and swallowed the failure.
-	let viewGeneration = 0;

 	// Ids confirmed deleted while this item's list was loading. A deletion
 	// broadcast only filters the CURRENT array, so a list() response that was
@@ -240,18 +225,19 @@
 	let pendingUploads: StripAttachment[] = [];

 	$effect(() => {
-		const reqItemId = itemId;
-		const reqWsSlug = wsSlug;
+		// Restarting the request fence supersedes any response still in flight
+		// AND captures the identity this run belongs to. Reading the identity
+		// here is what makes (workspace, item) the effect's dependencies.
+		const req = loadFence.restart();
+		const { ws: reqWsSlug, item: reqItemId } = req.value;
 		// Read (and discard) so Retry re-runs this effect. The value never
 		// matters — only the dependency does.
 		void retryNonce;
-		const gen = ++loadGeneration;

 		// Claim the retry marker whether or not this run goes on to fetch, so a
 		// stale claim can't leak into a later, unrelated load. Read BEFORE the
 		// reset below, which branches on it.
-		const isRetry =
-			retryRequestedFor !== null && retryRequestedFor === viewKey(reqWsSlug, reqItemId ?? '');
+		const isRetry = retryRequestedFor !== null && retryRequestedFor === req.key;
 		retryRequestedFor = null;

 		// Clear synchronously on switch. Without this, A's tiles stay painted
@@ -267,12 +253,13 @@
 			// Whatever this run paints — rows, error row, or nothing — belongs to
 			// this (workspace, item). Recorded unconditionally: a Retry repaints
 			// the SAME view, so the value is unchanged, and a run that bails
-			// below on a missing id must still stop claiming the previous view.
-			paintedView = reqItemId && reqWsSlug ? viewKey(reqWsSlug, reqItemId) : null;
+			// below on a missing id must still stop claiming the previous view
+			// (an un-addressable token records nothing).
+			paint.record(req);
 			if (!isRetry) {
 				// The view itself changed (new item / workspace), so in-flight
 				// mutations captured against the old one must stop reconciling.
-				viewGeneration++;
+				viewFence.invalidate();
 				attachments = [];
 				expanded = false;
 				lightbox = null;
@@ -290,7 +277,7 @@
 		// deferred write in this component.
 		let loadingTimer: ReturnType<typeof setTimeout> | null = setTimeout(() => {
 			loadingTimer = null;
-			if (switchedAway(gen, reqItemId, reqWsSlug)) return;
+			if (req.stale()) return;
 			showLoading = true;
 		}, LOADING_DELAY_MS);
 		function stopLoadingMarker() {
@@ -301,18 +288,56 @@
 		}

 		void (async () => {
+			// Ids the pending buffer ALREADY held when this request went out. A
+			// response can be authoritative about exactly these — the row existed
+			// before the GET, so its absence is proof the row is gone (deleted by
+			// another tab or another user, which the in-process event bus cannot
+			// see). Merging them back in resurrected such rows, and because
+			// nothing ever consumed the buffer it did so on every subsequent
+			// load, forever (final review round 4).
+			//
+			// Entries announced AFTER this point are the buffer's actual purpose
+			// and are never touched: the GET may predate them, so their absence
+			// from the response says nothing.
+			const pendingAtRequest = new Set(pendingUploads.map((a) => a.id));
 			try {
 				const res = await api.attachments.list(reqWsSlug, {
 					item_id: reqItemId,
 					limit: MAX_FETCH,
 				});
-				if (switchedAway(gen, reqItemId, reqWsSlug)) return;
+				if (req.stale()) return;
 				const raw = res.attachments ?? [];
 				const rows = raw.filter((a) => !deletedIds.has(a.id)).map(toStripAttachment);
 				const seen = new Set(rows.map((a) => a.id));
+				// ...but only a COMPLETE page is authoritative about absence. The
+				// request is bounded at MAX_FETCH, so a truncated page may simply
+				// not reach a row that is still perfectly alive — and retiring a
+				// live upload would delete a good tile permanently, which is a
+				// worse failure than the resurrection this is fixing (Codex
+				// round 2). `total` is the server's own statement of how many
+				// live rows there are and `offset` is always 0 here, so a page
+				// that holds all of them IS complete even when it is exactly
+				// MAX_FETCH long (Codex round 3). Only when the response omits
+				// `total` does the page length have to stand in for it.
+				const pageIsComplete =
+					typeof res.total === 'number' ? res.total <= raw.length : raw.length < MAX_FETCH;
+				const covered = pageIsComplete ? pendingAtRequest : new Set<string>();
 				const missed = pendingUploads.filter(
-					(a) => !seen.has(a.id) && !deletedIds.has(a.id)
+					(a) => !seen.has(a.id) && !deletedIds.has(a.id) && !covered.has(a.id)
 				);
+				// Consume what this response covered. The exclusion above already
+				// makes every LATER load ignore these ids (a later request's own
+				// snapshot would contain them too), so this is the retention half
+				// of the fix rather than a second correctness gate: without it the
+				// buffer only ever grows, holding rows that nothing will ever read
+				// again. Deliberately not separately observable — a mutation that
+				// deletes this line survives the suite, and that is expected.
+				//
+				// Only on SUCCESS, and only for what the page COVERED: a failure
+				// is not authoritative, the catch arm below still needs the buffer
+				// to repaint the rows a failed listing would otherwise hide, and a
+				// truncated page proves nothing about what it didn't reach.
+				pendingUploads = pendingUploads.filter((a) => !covered.has(a.id));
 				// Cap the MERGE, not just the response: `missed` rides on top of
 				// up to MAX_FETCH server rows (DR-11).
 				const merged = [...missed, ...rows];
@@ -364,7 +389,7 @@
 				// roleLevel("guest") is below viewer, so they 403. That gap is
 				// pre-existing (inline images are already broken for them) and
 				// is tracked as BUG-2386, not absorbed here — PLAN-2382 DR-4b.
-				if (switchedAway(gen, reqItemId, reqWsSlug)) return;
+				if (req.stale()) return;
 				// Keep anything uploaded while this request was in flight: the
 				// upload SUCCEEDED, so dropping it would hide a row the editor
 				// and server both have, until a remount (Codex review round 2).
@@ -372,7 +397,7 @@
 				loadFailed = true;
 			} finally {
 				stopLoadingMarker();
-				if (!switchedAway(gen, reqItemId, reqWsSlug)) showLoading = false;
+				if (!req.stale()) showLoading = false;
 			}
 		})();

@@ -387,7 +412,7 @@
 		// view generation here would put the Retry bug straight back. Unmount
 		// is handled by onDestroy below, which runs only on destroy.
 		return () => {
-			loadGeneration++;
+			loadFence.invalidate();
 			stopLoadingMarker();
 		};
 	});
@@ -395,7 +420,7 @@
 	// Destroy invalidates the VIEW too — an in-flight delete that resolves
 	// after the component is gone must not toast or write.
 	onDestroy(() => {
-		viewGeneration++;
+		viewFence.invalidate();
 	});

 	/**
@@ -410,7 +435,6 @@
 	 * uploaded while the request was in flight.
 	 */
 	function retryLoad() {
-		if (!itemId || !wsSlug) return;
 		// The same ENTRY fence the delete control uses (fence 3): the painted
 		// error must belong to the view that is on screen NOW. If the parent has
 		// already swapped `itemId` or `wsSlug` and this effect hasn't flushed
@@ -418,13 +442,19 @@
 		// its way, and honouring the retry would preserve the previous view's
 		// rows across the switch (Codex round 11).
 		//
-		// `loadFailed` is only ever set by a response that passed `switchedAway`
-		// under the run that also wrote `paintedView`, so the two together say
-		// exactly what a separate `loadFailedFor` field used to: WHICH view's
-		// failure is painted. One paint-time identity, not two kept in step.
-		if (!loadFailed || paintedView !== viewKey(wsSlug, itemId)) return;
-		for (const a of attachments) invalidateAttachmentMetadata(wsSlug, a.id);
-		retryRequestedFor = viewKey(wsSlug, itemId);
+		// `loadFailed` is only ever set by a response that passed the request
+		// fence under the run that also recorded the paint, so the two together
+		// say exactly what a separate `loadFailedFor` field used to: WHICH
+		// view's failure is painted. One paint-time identity, not two kept in
+		// step.
+		if (!loadFailed || !paint.isCurrent()) return;
+		const painted = paint.painted();
+		if (!painted) return;
+		// Workspace read back off the PAINTED identity, not the live prop:
+		// `isCurrent()` has just established they agree, and taking it from the
+		// token is what stops the two from ever drifting apart again.
+		for (const a of attachments) invalidateAttachmentMetadata(painted.value.ws, a.id);
+		retryRequestedFor = painted.key;
 		retryNonce++;
 	}

@@ -504,38 +534,6 @@
 		});
 	});

-	// Mirrors ItemDetail's `switchedAway`: the generation catches a newer load,
-	// the identity compare closes the A→B→A gap where generations could
-	// otherwise line up.
-	//
-	// Identity is the (workspace, item) PAIR, exactly as `viewChanged` and
-	// `viewKey` treat it. Requests already capture the workspace they were
-	// issued against; checking only the item let a same-item workspace switch
-	// pass this fence, so a superseded response, failure or loading timer could
-	// write into the new view during the prop-update → effect-flush window
-	// (final review round 2).
-	//
-	// Deliberately untested, unlike the other two fences: a workspace change is
-	// a dependency of the load effect, and Svelte's scheduler always flushes
-	// that re-run — which clears every one of these fields — before a pending
-	// response continuation gets to write. So the omission has no observable
-	// symptom to pin a test to. It is fixed because "the next effect run happens
-	// to overwrite it" is a scheduling accident, not a fence, and because
-	// identity being the (workspace, item) pair should not have exceptions.
-	function switchedAway(gen: number, reqItemId: string, reqWsSlug: string): boolean {
-		return gen !== loadGeneration || itemId !== reqItemId || wsSlug !== reqWsSlug;
-	}
-
-	/**
-	 * The mutation fence. Same shape as `switchedAway`, but against the VIEW
-	 * generation — a Retry re-runs the load without changing what is on screen,
-	 * so an in-flight delete for this item must still reconcile (final-review
-	 * P2). The id compare closes the A→B→A gap the counter alone can miss.
-	 */
-	function viewChanged(gen: number, reqItemId: string, reqWsSlug: string): boolean {
-		return gen !== viewGeneration || itemId !== reqItemId || wsSlug !== reqWsSlug;
-	}
-
 	let visible = $derived(expanded ? attachments : attachments.slice(0, COLLAPSED_TILES));
 	// Overflow is derived from the FETCHED ROWS, never the response's `total`
 	// (DR-9) — otherwise an item with >50 attachments advertises a count that
@@ -640,32 +638,30 @@
 	async function handleDelete(att: StripAttachment) {
 		if (!canDelete) return;

-		const reqItemId = itemId;
-		const reqWsSlug = wsSlug;
-		if (!reqItemId || !reqWsSlug) return;
 		// ENTRY fence (fence 3 — see the header). The clicked tile was painted
-		// for `paintedView`; the live props may already name a different view,
-		// because they update synchronously and the load effect that repaints
-		// these tiles flushes later. In that window a click on a stale tile
-		// would send a DELETE while the user is looking at another item or
-		// workspace — and the `viewChanged` check in the catch below runs after
-		// the request, so it can suppress the rollback but cannot unsend the
-		// request (final review round 2). The only fix is to refuse here,
-		// before the confirm and before the call.
-		if (paintedView !== viewKey(reqWsSlug, reqItemId)) return;
+		// for `paint`'s identity; the live props may already name a different
+		// view, because they update synchronously and the load effect that
+		// repaints these tiles flushes later. In that window a click on a stale
+		// tile would send a DELETE while the user is looking at another item or
+		// workspace — and the view-fence check in the catch below runs after the
+		// request, so it can suppress the rollback but cannot unsend the request
+		// (final review round 2). The only fix is to refuse here, before the
+		// confirm and before the call.
+		if (!paint.isCurrent()) return;

 		if (typeof window !== 'undefined' && !window.confirm(confirmMessage(att))) return;
 		// window.confirm blocks the thread, so nothing can have moved between
 		// the fence above and here.

-		// Capture identity BEFORE the await: a switch mid-delete must not roll
-		// the tile back into a DIFFERENT item's strip, and must not toast over
-		// it. The DELETE itself still lands — it targets an id, not a view.
-		// `wsSlug` is captured for the same reason: it is reactive, and the
-		// broadcast + metadata-cache key must name the workspace the DELETE
-		// actually targeted, not whichever one is current when it resolves
-		// (Codex round 6).
-		const gen = viewGeneration;
+		// Capture identity BEFORE the await (fence 2): a switch mid-delete must
+		// not roll the tile back into a DIFFERENT item's strip, and must not
+		// toast over it. The DELETE itself still lands — it targets an id, not a
+		// view. The workspace comes off the token for the same reason: it is
+		// reactive, and the request + broadcast + metadata-cache key must name
+		// the workspace the DELETE actually targeted, not whichever one is
+		// current when it resolves (Codex round 6).
+		const req = viewFence.begin();
+		const reqWsSlug = req.value.ws;
 		const index = attachments.findIndex((a) => a.id === att.id);

 		// Optimistic removal.
@@ -703,7 +699,7 @@
 				return;
 			}

-			if (viewChanged(gen, reqItemId, reqWsSlug)) return;
+			if (req.stale()) return;

 			// Someone else announced this deletion while our own call was in
 			// flight — the row is gone regardless of why ours failed, so don't
@@ -1264,6 +1264,115 @@ describe('ItemAttachmentStrip', () => {
 		expect(names.some((n) => n.includes('old1.png'))).toBe(true);
 	});

+	it('lets a later load retire a pending upload deleted outside this tab', async () => {
+		// `pendingUploads` was retained forever and no successful response ever
+		// consumed it, so it re-merged onto EVERY later load. The event bus is
+		// process-local, so a deletion from another tab or another user, followed
+		// by a load that legitimately returns no row, resurrected the tile — and
+		// kept resurrecting it (final review round 4).
+		//
+		// The rule: a response is authoritative about the entries the buffer
+		// already held when that request went OUT. Entries announced while it was
+		// in flight are the buffer's actual purpose and stay.
+		listMock.mockRejectedValueOnce(new Error('offline'));
+		mountStrip('item-a');
+		await settle();
+		expect(errorRow()).not.toBeNull();
+
+		// Uploaded during the outage: buffered, and rightly shown.
+		broadcastUpload('item-a', uploaded('elsewhere-deleted'));
+		flushSync();
+		expect(tiles()).toHaveLength(1);
+
+		// Retry. The GET goes out AFTER the upload, so the server's answer is
+		// authoritative — and it says the row is gone, because another tab
+		// deleted it.
+		listMock.mockResolvedValueOnce(response([att({ id: 'still-here' })]));
+		target.querySelector<HTMLButtonElement>('.att-retry')?.click();
+		flushSync();
+		await settle();
+
+		const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
+		expect(names.some((n) => n.includes('elsewhere-deleted'))).toBe(false);
+		expect(names.some((n) => n.includes('still-here.png'))).toBe(true);
+	});
+
+	it('does not retire a pending upload on a TRUNCATED page', async () => {
+		// The request is bounded at 50 rows, so a FULL page is not proof of
+		// absence — a still-live row can simply be past it. Retiring it there
+		// would delete a good tile permanently, which is worse than the
+		// resurrection the retirement rule exists to fix (Codex round 2).
+		listMock.mockRejectedValueOnce(new Error('offline'));
+		mountStrip('item-a');
+		await settle();
+
+		broadcastUpload('item-a', uploaded('past-the-bound'));
+		flushSync();
+
+		// The retry's page comes back FULL, and the server says there are more.
+		const full = Array.from({ length: 50 }, (_, i) => att({ id: `s${i}` }));
+		listMock.mockResolvedValueOnce({ attachments: full, total: 60, limit: 50, offset: 0 });
+		target.querySelector<HTMLButtonElement>('.att-retry')?.click();
+		flushSync();
+		await settle();
+
+		target.querySelector<HTMLElement>('.att-more-expand')?.click();
+		flushSync();
+		const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
+		expect(names.some((n) => n.includes('past-the-bound.png'))).toBe(true);
+	});
+
+	it('treats a page that holds every live row as complete, even at the bound', async () => {
+		// `total` is the server's own count and `offset` is always 0 here, so a
+		// page of exactly MAX_FETCH rows with `total: 50` HAS reached everything.
+		// Reading "full page" as "truncated" would leave an externally deleted
+		// upload buffered — resurrectable indefinitely (Codex round 3).
+		listMock.mockRejectedValueOnce(new Error('offline'));
+		mountStrip('item-a');
+		await settle();
+
+		broadcastUpload('item-a', uploaded('elsewhere-deleted'));
+		flushSync();
+
+		const full = Array.from({ length: 50 }, (_, i) => att({ id: `s${i}` }));
+		listMock.mockResolvedValueOnce({ attachments: full, total: 50, limit: 50, offset: 0 });
+		target.querySelector<HTMLButtonElement>('.att-retry')?.click();
+		flushSync();
+		await settle();
+
+		target.querySelector<HTMLElement>('.att-more-expand')?.click();
+		flushSync();
+		const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
+		expect(names.some((n) => n.includes('elsewhere-deleted'))).toBe(false);
+		expect(names).toHaveLength(50);
+	});
+
+	it('still keeps an upload announced while THIS request was in flight', async () => {
+		// The other side of the same rule, and the reason the buffer exists at
+		// all: this GET was issued BEFORE the upload, so its silence about the
+		// row proves nothing and the tile must survive the response. Pins the
+		// consumption above to entries the request could actually have covered.
+		listMock.mockRejectedValueOnce(new Error('offline'));
+		mountStrip('item-a');
+		await settle();
+
+		const pending = deferred<AttachmentListResponse>();
+		listMock.mockReturnValueOnce(pending.promise);
+		target.querySelector<HTMLButtonElement>('.att-retry')?.click();
+		flushSync();
+
+		// Announced AFTER the retry's request went out.
+		broadcastUpload('item-a', uploaded('mid-flight'));
+		flushSync();
+
+		pending.resolve(response([att({ id: 'server-row' })]));
+		await settle();
+
+		const names = tiles().map((el) => el.getAttribute('aria-label') ?? '');
+		expect(names.some((n) => n.includes('mid-flight.png'))).toBe(true);
+		expect(names.some((n) => n.includes('server-row.png'))).toBe(true);
+	});
+
 	it('does not double-count an upload the refetch also returns', async () => {
 		const pending = deferred<AttachmentListResponse>();
 		listMock.mockReturnValue(pending.promise);
@@ -1,5 +1,5 @@
 <script lang="ts">
-	import { untrack } from 'svelte';
+	import { onDestroy, untrack } from 'svelte';
 	import { page } from '$app/state';
 	import { api, PadApiError } from '$lib/api/client';
 	import { announceAttachmentDeleted } from '$lib/attachments/events';
@@ -19,6 +19,7 @@
 		type StorageFilterSelections
 	} from '$lib/attachments/storageFilters';
 	import AttachmentIcon from '$lib/attachments/icons/AttachmentIcon.svelte';
+	import { viewIdentity, createFence, createPaintFence } from '$lib/attachments/viewFence';

 	// ── Props ────────────────────────────────────────────────────────────────
 	interface Props {
@@ -159,9 +160,41 @@
 		attachments.find((a) => a.item_id === filterItemId)?.item_title ?? ''
 	);

-	// The workspace whose view is currently loaded. A plain `let` (not `$state`)
-	// so writing it from the effect below can't re-trigger it.
-	let loadedWsSlug: string | null = null;
+	// ── View identity + fences (PLAN-2392, shared with the item attachment
+	// strip) ─────────────────────────────────────────────────────────────────
+	//
+	// This tab lives at `/{user}/{ws}/settings` — ONE SvelteKit route — so a
+	// workspace switch changes `wsSlug` under a MOUNTED component and every
+	// request, continuation and rendered control has to say which workspace it
+	// belongs to. `$lib/attachments/viewFence` owns that invariant; see its
+	// header. The identity is stated ONCE here, so no individual fence can
+	// restate a shorter one, and each request reads its workspace back off its
+	// own token rather than off the live prop.
+	const view = viewIdentity(() => ({ ws: wsSlug }));
+	/**
+	 * Fence 2 — the view generation. Mutations fence on this rather than on the
+	 * paint alone: a paint token has no generation, so an A→B→A round trip (or
+	 * an unmount) leaves an identity that still matches and a stale continuation
+	 * would sail through (Codex round 3).
+	 */
+	const viewFence = createFence(view);
+	/** Fence 3 — the workspace whose view is currently painted (and loaded). */
+	const paint = createPaintFence(view);
+
+	// Destroy invalidates EVERY fence and un-paints. The strip gets its request
+	// fence invalidated for free (its load lives in an $effect, whose teardown
+	// runs on destroy); this tab's loaders are plain async functions, so nothing
+	// else retires their tokens — and a list or usage request that fails after
+	// unmount would otherwise raise an error toast for a tab that is gone
+	// (Codex round 5). "Nothing is painted any more" is exactly what the paint
+	// fence is for, so no separate destroyed flag is needed.
+	onDestroy(() => {
+		viewFence.invalidate();
+		listFence.invalidate();
+		usageFence.invalidate();
+		workspaceViewFence.invalidate();
+		paint.record(null);
+	});

 	/**
 	 * The tab's only load trigger. It owns BOTH transitions, in one effect and
@@ -187,11 +220,20 @@
 	 * deep-link branch can't fire a second one behind it.
 	 */
 	$effect(() => {
-		const ws = wsSlug;
+		// Captured outside `untrack` so `wsSlug` (via the identity) and
+		// `initialItemId` are this effect's dependencies.
+		const here = view.capture();
 		const incoming = initialItemId;
+		if (!here.key) return;
 		untrack(() => {
-			if (ws !== loadedWsSlug) {
-				loadedWsSlug = ws;
+			if (!paint.isCurrent()) {
+				paint.record(here);
+				// The view itself changed, so in-flight mutations captured
+				// against the old one must stop reconciling. Bumped here, not
+				// derived from the paint: an A→B→A round trip lands back on an
+				// identity that MATCHES, and only a generation can tell that
+				// apart from never having left (Codex round 3).
+				viewFence.invalidate();
 				// Everything workspace-scoped is meaningless in the new
 				// workspace and must not survive the switch: the rows and
 				// usage figure obviously, but also the two workspace-scoped
@@ -218,7 +260,7 @@

 	/**
 	 * Refetch after the item scope changed. Clears the rendered rows first:
-	 * the `listGen` fence stops a stale RESPONSE from landing, but on its own
+	 * the list fence stops a stale RESPONSE from landing, but on its own
 	 * it would leave the previous item's rows on screen while the new request
 	 * runs — indefinitely if that request fails (Codex round 5).
 	 */
@@ -239,34 +281,35 @@
 		retargetScope();
 	}

-	let usageGen = 0;
+	/**
+	 * Fence 1, usage channel. Both halves matter, for the same reasons as the
+	 * list's. The workspace check: this tab stays mounted across a workspace
+	 * change, so a figure fetched for the workspace the user just left must not
+	 * paint over the one they switched to (nor raise its error toast). The
+	 * generation check: on an A→B→A round trip the workspace matches again, so
+	 * only the generation can tell the first A request from the current one
+	 * (Codex round 1).
+	 */
+	const usageFence = createFence(view);

 	async function loadUsage() {
-		// Both halves of the list load's fence, for the same reasons. The
-		// workspace check: this tab stays mounted across a workspace change, so a
-		// figure fetched for the workspace the user just left must not paint over
-		// the one they switched to (nor raise its error toast). The generation
-		// check: on an A→B→A round trip the workspace matches again, so only the
-		// generation can tell the first A request from the current one
-		// (Codex round 1).
-		const gen = ++usageGen;
-		const reqWsSlug = wsSlug;
+		const req = usageFence.restart();
 		try {
-			const next = await api.attachments.storageUsage(reqWsSlug);
-			if (gen !== usageGen || reqWsSlug !== wsSlug) return;
+			const next = await api.attachments.storageUsage(req.value.ws);
+			if (req.stale()) return;
 			usage = next;
 		} catch (err) {
-			if (gen !== usageGen || reqWsSlug !== wsSlug) return;
+			if (req.stale()) return;
 			const msg = err instanceof Error ? err.message : 'Failed to load storage usage';
 			toastStore.show(msg, 'error');
 		}
 	}

-	// Every list load shares one generation, so a slower earlier request can
-	// never overwrite a newer one's rows/total/offset — the deep-link re-seed
-	// can now start a load while onMount's or a filter change's is still in
-	// flight (Codex round 4).
-	let listGen = 0;
+	// Every list load shares one fence, so a slower earlier request can never
+	// overwrite a newer one's rows/total/offset — the deep-link re-seed can
+	// start a load while onMount's or a filter change's is still in flight
+	// (Codex round 4).
+	const listFence = createFence(view);
 	// A list request is in flight. Only meaningful once the tab's initial
 	// `loading` gate is down — it keeps a scope retarget (which clears the
 	// rows) from flashing "no attachments match" before the new page lands.
@@ -286,21 +329,21 @@
 	 * and nothing refetches eagerly.
 	 */
 	async function loadList(opts: { retry?: boolean } = {}) {
-		const gen = ++listGen;
-		listLoading = true;
-		listError = false;
 		// `wsSlug` is reactive and this tab stays mounted across a workspace
 		// change, so the request's workspace is captured and re-checked: the
 		// generation alone can't tell a superseded workspace from the current
 		// one, and both the rows and the metadata-cache keys are workspace
 		// scoped (Codex fresh-angle round 2).
-		const reqWsSlug = wsSlug;
+		const req = listFence.restart();
+		const reqWsSlug = req.value.ws;
+		listLoading = true;
+		listError = false;
 		if (opts.retry) {
 			for (const a of attachments) invalidateAttachmentMetadata(reqWsSlug, a.id);
 		}
 		try {
 			const resp: AttachmentListResponse = await api.attachments.list(reqWsSlug, buildFilters());
-			if (gen !== listGen || reqWsSlug !== wsSlug) return;
+			if (req.stale()) return;
 			attachments = resp.attachments ?? [];
 			if (opts.retry) {
 				// The rows the refetch returned get the same treatment — at
@@ -311,12 +354,12 @@
 			limit = resp.limit ?? limit;
 			offset = resp.offset ?? offset;
 		} catch (err) {
-			if (gen !== listGen || reqWsSlug !== wsSlug) return;
+			if (req.stale()) return;
 			listError = true;
 			const msg = err instanceof Error ? err.message : 'Failed to load attachments';
 			toastStore.show(msg, 'error');
 		} finally {
-			if (gen === listGen && reqWsSlug === wsSlug) listLoading = false;
+			if (!req.stale()) listLoading = false;
 		}
 	}

@@ -328,11 +371,10 @@
 	 * Load (or reload) everything the tab shows for the current workspace, behind
 	 * the whole-tab `loading` gate. Used on mount and on every workspace change.
 	 */
-	let viewGen = 0;
+	const workspaceViewFence = createFence(view);

 	async function loadWorkspaceView() {
-		const gen = ++viewGen;
-		const reqWsSlug = wsSlug;
+		const req = workspaceViewFence.restart();
 		loading = true;
 		try {
 			await Promise.all([loadList(), loadUsage()]);
@@ -343,7 +385,7 @@
 			// finishes with `wsSlug` back at A while A's current load is still in
 			// flight (Codex round 1). It can't strand: whatever supersedes a load
 			// is itself a load, and the newest one's `finally` always runs.
-			if (gen === viewGen && reqWsSlug === wsSlug) loading = false;
+			if (!req.stale()) loading = false;
 		}
 	}

@@ -359,18 +401,53 @@
 	// ── Actions ──────────────────────────────────────────────────────────────

 	async function handleDelete(att: AttachmentListItem) {
+		// ENTRY fence (fence 3). The clicked row was painted for the workspace
+		// this tab has LOADED, which during the prop-update → effect-flush window
+		// is not necessarily the one `wsSlug` already names. Taking the workspace
+		// off the paint means the DELETE targets the row the user actually
+		// clicked; refusing when the paint is stale is the only lever that works
+		// at all, because every check below runs after the await and no fence can
+		// unsend a request.
+		const painted = paint.painted();
+		if (!painted || !paint.isCurrent()) return;
+		const reqWsSlug = painted.value.ws;
+		// Fence 2 for everything after the await. Deliberately NOT the paint
+		// token: that carries an identity but no generation, so an A→B→A round
+		// trip (or an unmount) would leave it matching again and let a stale
+		// continuation toast and refetch (Codex round 3).
+		const req = viewFence.begin();
+
 		const ok = confirm(
 			`Delete ${att.filename}? The blob is reclaimed by garbage collection after a grace period.`
 		);
 		if (!ok) return;
 		try {
-			await api.attachments.delete(wsSlug, att.id);
+			await api.attachments.delete(reqWsSlug, att.id);
 			// Same broadcast the item attachment strip does (PLAN-2382 /
 			// TASK-2384): an editor open in another tab-pane still holds live
 			// <img>/chip NodeViews for this attachment, and an already-loaded
 			// image never re-requests, so without this they keep presenting a
 			// row the server no longer has (Codex round 14).
-			announceAttachmentDeleted(wsSlug, att.id);
+			//
+			// Deliberately AHEAD of the fence, like the strip's: this is a GLOBAL
+			// side effect keyed by (workspace, id), not a write into this view's
+			// local state, and a workspace switch says nothing about whether the
+			// row is gone. Skipping it would leave every other mounted surface
+			// stale on proof we already have.
+			announceAttachmentDeleted(reqWsSlug, att.id);
+			// The toast DOES belong to a view: after an A→B switch mid-request it
+			// would announce A's deletion over B (final review round 4).
+			if (req.stale()) {
+				// The refetch does NOT. This tab holds no tombstones and does not
+				// subscribe to the deletion bus, so a list request that raced the
+				// DELETE can have repainted the row — and on an A→B→A round trip
+				// the fence above suppresses the only thing that heals it. A
+				// refetch is a request for the truth about what is ON SCREEN, not
+				// a write into a view the user left, so it is gated on the paint
+				// rather than on the generation (Codex round 4).
+				if (paint.isCurrent() && !painted.changed()) await reload();
+				return;
+			}
 			toastStore.show(`Deleted ${att.filename}`, 'success');
 			await reload();
 		} catch (err) {
@@ -380,11 +457,19 @@
 			// an error for something that is in fact already done
 			// (Codex round 20; matches the attachment strip's handling).
 			if (err instanceof PadApiError && err.code === 'not_found') {
-				announceAttachmentDeleted(wsSlug, att.id);
+				announceAttachmentDeleted(reqWsSlug, att.id);
+				if (req.stale()) {
+					// Same reasoning as the success arm: a 404 is just as
+					// authoritative that the row is gone, so the on-screen list
+					// still deserves the correction.
+					if (paint.isCurrent() && !painted.changed()) await reload();
+					return;
+				}
 				toastStore.show(`${att.filename} was already deleted`, 'info');
 				await reload();
 				return;
 			}
+			if (req.stale()) return;
 			const msg = err instanceof Error ? err.message : 'Failed to delete attachment';
 			toastStore.show(msg, 'error');
 		}
@@ -15,6 +15,8 @@ import type {
 const listMock =
 	vi.fn<(ws: string, filters: Record<string, unknown>) => Promise<AttachmentListResponse>>();
 const usageMock = vi.fn<(ws: string) => Promise<WorkspaceStorageInfo>>();
+const deleteMock = vi.fn<(ws: string, id: string) => Promise<void>>();
+const announceMock = vi.fn<(ws: string, id: string) => void>();
 const toastMock = vi.fn<(message: string, kind?: string) => void>();

 class FakeApiError extends Error {
@@ -33,7 +35,7 @@ vi.mock('$lib/api/client', () => ({
 			storageUsage: (ws: string) => usageMock(ws),
 			downloadUrl: (ws: string, id: string, variant?: string) =>
 				`/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`,
-			delete: vi.fn(),
+			delete: (ws: string, id: string) => deleteMock(ws, id),
 		},
 	},
 }));
@@ -43,7 +45,7 @@ vi.mock('$app/state', () => ({
 }));

 vi.mock('$lib/attachments/events', () => ({
-	announceAttachmentDeleted: vi.fn(),
+	announceAttachmentDeleted: (ws: string, id: string) => announceMock(ws, id),
 }));

 vi.mock('$lib/components/editor/attachment-metadata', () => ({
@@ -106,6 +108,9 @@ describe('StorageTab workspace switching', () => {
 		listMock.mockResolvedValue(response([]));
 		usageMock.mockReset();
 		usageMock.mockResolvedValue(usage(1));
+		deleteMock.mockReset();
+		deleteMock.mockResolvedValue(undefined);
+		announceMock.mockReset();
 		toastMock.mockReset();
 		props.wsSlug = 'ws-a';
 		props.initialItemId = '';
@@ -256,6 +261,158 @@ describe('StorageTab workspace switching', () => {
 		expect(text()).not.toContain('stale-a.pdf');
 	});

+	it('fences a delete on the workspace it was issued for', async () => {
+		// The delete used the LIVE `wsSlug` after its own await, so an A→B switch
+		// mid-request let A's success handling announce the deletion and reload
+		// against B — a success toast for a row the user is no longer looking at,
+		// and a refetch of B's list on A's completion (final review round 4).
+		//
+		// The broadcast is deliberately NOT fenced: it is a global side effect
+		// keyed by (workspace, id), and a switch says nothing about whether the
+		// row is gone.
+		listMock.mockResolvedValueOnce(response([att({ id: 'a1', filename: 'from-a.pdf' })]));
+		mountTab();
+		await settle();
+		expect(rows()).toHaveLength(1);
+
+		const slowDelete = deferred<void>();
+		deleteMock.mockReturnValueOnce(slowDelete.promise);
+		const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
+		target.querySelector<HTMLButtonElement>('.btn-remove')?.click();
+		flushSync();
+		expect(deleteMock).toHaveBeenCalledWith('ws-a', 'a1');
+
+		listMock.mockResolvedValueOnce(response([att({ id: 'b1', filename: 'from-b.pdf' })]));
+		props.wsSlug = 'ws-b';
+		flushSync();
+		await settle();
+		expect(text()).toContain('from-b.pdf');
+		toastMock.mockClear();
+		const listCalls = listMock.mock.calls.length;
+
+		slowDelete.resolve();
+		await settle();
+
+		// Announced against the workspace the DELETE targeted...
+		expect(announceMock).toHaveBeenCalledWith('ws-a', 'a1');
+		// ...but no toast over B, and no reload of B on A's completion.
+		expect(toastMock).not.toHaveBeenCalled();
+		expect(listMock.mock.calls.length).toBe(listCalls);
+		expect(text()).toContain('from-b.pdf');
+		confirmSpy.mockRestore();
+	});
+
+	it('still suppresses a delete continuation after an A→B→A round trip', async () => {
+		// The identity is back to ws-a by the time the delete resolves, so an
+		// identity compare alone lets it through. Only a view GENERATION can tell
+		// "never left" from "left and came back" — which is why the mutation
+		// fence is not the paint token (Codex round 3).
+		listMock.mockResolvedValueOnce(response([att({ id: 'a1', filename: 'from-a.pdf' })]));
+		mountTab();
+		await settle();
+
+		const slowDelete = deferred<void>();
+		deleteMock.mockReturnValueOnce(slowDelete.promise);
+		const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
+		target.querySelector<HTMLButtonElement>('.btn-remove')?.click();
+		flushSync();
+		expect(deleteMock).toHaveBeenCalledWith('ws-a', 'a1');
+
+		props.wsSlug = 'ws-b';
+		flushSync();
+		await settle();
+		props.wsSlug = 'ws-a';
+		flushSync();
+		await settle();
+
+		toastMock.mockClear();
+		const listCalls = listMock.mock.calls.length;
+		slowDelete.resolve();
+		await settle();
+
+		expect(announceMock).toHaveBeenCalledWith('ws-a', 'a1');
+		// No toast: it belongs to the view that was on screen when the delete
+		// started, and the user has been elsewhere since.
+		expect(toastMock).not.toHaveBeenCalled();
+		// But the tab IS showing ws-a again, and the list it repainted on the way
+		// back can have raced the DELETE — so the corrective refetch still runs,
+		// against ws-a (Codex round 4).
+		expect(listMock.mock.calls.length).toBe(listCalls + 1);
+		expect(listMock).toHaveBeenLastCalledWith('ws-a', expect.anything());
+		confirmSpy.mockRestore();
+	});
+
+	it('does not toast or refetch for a delete that resolves after unmount', async () => {
+		listMock.mockResolvedValueOnce(response([att({ id: 'a1', filename: 'from-a.pdf' })]));
+		mountTab();
+		await settle();
+
+		const slowDelete = deferred<void>();
+		deleteMock.mockReturnValueOnce(slowDelete.promise);
+		const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
+		target.querySelector<HTMLButtonElement>('.btn-remove')?.click();
+		flushSync();
+
+		unmount(instance!);
+		instance = undefined;
+		toastMock.mockClear();
+		const listCalls = listMock.mock.calls.length;
+
+		slowDelete.resolve();
+		await settle();
+
+		// The broadcast still fires — other surfaces need the proof — but
+		// nothing writes into a dead instance.
+		expect(announceMock).toHaveBeenCalledWith('ws-a', 'a1');
+		expect(toastMock).not.toHaveBeenCalled();
+		expect(listMock.mock.calls.length).toBe(listCalls);
+		confirmSpy.mockRestore();
+	});
+
+	it('does not toast for a list or usage request that fails after unmount', async () => {
+		// The loaders are plain async functions, not effects, so nothing retires
+		// their tokens on destroy unless onDestroy does it — and an error toast
+		// for a tab that is gone is a global side effect the user still sees
+		// (Codex round 5).
+		const slowList = deferred<AttachmentListResponse>();
+		const slowUsage = deferred<WorkspaceStorageInfo>();
+		listMock.mockReturnValueOnce(slowList.promise);
+		usageMock.mockReturnValueOnce(slowUsage.promise);
+		mountTab();
+		await settle();
+
+		unmount(instance!);
+		instance = undefined;
+		toastMock.mockReset();
+
+		slowList.reject(new Error('list blew up'));
+		slowUsage.reject(new Error('usage blew up'));
+		await settle();
+
+		expect(toastMock).not.toHaveBeenCalled();
+	});
+
+	it('refuses a delete click that lands after the workspace already switched', async () => {
+		// Props update synchronously and the reload effect flushes later, so a
+		// click can land on the previous workspace's still-painted row while
+		// `wsSlug` already reads the next one. No fence after the await can
+		// unsend that request, so the entry fence has to refuse it.
+		listMock.mockResolvedValueOnce(response([att({ id: 'a1', filename: 'from-a.pdf' })]));
+		mountTab();
+		await settle();
+
+		const confirmSpy = vi.spyOn(window, 'confirm').mockReturnValue(true);
+		props.wsSlug = 'ws-b';
+		// No flushSync between the switch and the click: that IS the window.
+		target.querySelector<HTMLButtonElement>('.btn-remove')?.click();
+		flushSync();
+		await settle();
+
+		expect(deleteMock).not.toHaveBeenCalled();
+		expect(confirmSpy).not.toHaveBeenCalled();
+		confirmSpy.mockRestore();
+	});
+
 	it('ignores a superseded usage response and its error toast', async () => {
 		const slowA = deferred<WorkspaceStorageInfo>();
 		usageMock.mockReturnValueOnce(slowA.promise);