fix(web): the item pane never puts an older row over a newer one (BUG-3036) (#1430)

Every writer in the item pane answers with the whole row and assigns it to `item`. Per-field ordering cannot see across fields, so a slow response for one field put an older row on screen over a newer write to another field, while the server kept the newer one. An SSE re-read did the same. Events that arrived during a save were skipped, not queued, so the pane stayed behind the server until something unrelated refreshed it. Snapshots are now ordered by the ROW's seq (BUG-3037), not a client dispatch count. A dispatch count drops an older successful response because of a newer write that then fails, which is the BUG-2773 hole. withInflightTags, the funnel every server snapshot already goes through, refuses a snapshot of the shown item whose seq is strictly below the one on screen. Equal or missing seq is accepted, and a different item is never compared. Follow-ups that used the incoming row now read the row actually installed. A delta-sync pass for an older row returns before it bumps itemGen. The three non-destructive skip-while-saving sites now owe a re-read, which runs once the save (or title edit) ends. The seq order makes a late re-read harmless. Verification: a Playwright spec with real commits and delayed responses (a slow field PATCH, a slow SSE re-read, a change during a save). Removing the funnel guard turns the first two red. Restoring the skip turns the third red. Relaxing strict to non-strict fails the unit test. Claude-Session: https://claude.ai/code/session_01X4uYKs5ZPF28eCWfm8VPvE
2026-10-27 06:47:07 +00:00 · 2026-09-22 10:11:21 -04:00
parent 0393bc9180
commit 3c6a08013d
6 changed files with 519 additions and 29 deletions
@@ -0,0 +1,322 @@
+import { test, expect } from './fixtures';
+import { browserLogin } from './lib/collab-helpers';
+import type { APIRequestContext, APIResponse, Page, Route } from '@playwright/test';
+import type { SuiteFixture } from './fixtures';
+
+/**
+ * BUG-3036 — the item pane must not put an OLDER row on screen over a newer one.
+ *
+ * Every writer in the pane answers with the whole row and assigns it to `item`.
+ * Per-field ordering (fieldWriteOrder.ts) cannot see across fields, so a
+ * response that was merely SLOW used to revert a newer write to a different
+ * field on screen, while the server kept it. The pane now refuses a snapshot of
+ * the shown item whose `seq` is strictly below the one on screen.
+ *
+ * The second half: an SSE change that arrived while a save was in flight used
+ * to be SKIPPED, so the pane stayed behind the server until something unrelated
+ * refreshed it. It is now owed, and re-read once the save settles.
+ *
+ * Every hold below is a REAL request: it goes to the real server with
+ * `route.fetch()` and commits there. Only the delivery of the response to the
+ * page is delayed. So each response is a genuine older row, not one the test
+ * made up.
+ */
+
+const DESKTOP = { width: 1200, height: 900 };
+
+function authHeaders(fixture: SuiteFixture) {
+	return { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' };
+}
+
+async function seed(fixture: SuiteFixture, request: APIRequestContext, tag: string) {
+	const schema = JSON.stringify({
+		fields: [
+			{ key: 'alpha', label: 'Alpha', type: 'text' },
+			{ key: 'beta', label: 'Beta', type: 'text' },
+			{ key: 'gamma', label: 'Gamma', type: 'text' },
+		],
+	});
+	const coll = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections`, {
+		headers: authHeaders(fixture),
+		data: { name: `B3036 ${tag} ${Date.now()}`, prefix: 'BSO', schema },
+	});
+	expect(coll.ok(), await coll.text()).toBeTruthy();
+	const { slug: collSlug } = await coll.json();
+	const created = await request.post(
+		`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${collSlug}/items`,
+		{
+			headers: authHeaders(fixture),
+			data: {
+				title: `B3036 ${tag} ${Date.now()}`,
+				fields: JSON.stringify({ alpha: 'alpha-0', beta: 'beta-0', gamma: 'gamma-0' }),
+				content: '',
+			},
+		},
+	);
+	expect(created.ok(), await created.text()).toBeTruthy();
+	const item = (await created.json()) as { id: string; slug: string };
+	return { collSlug, item };
+}
+
+async function serverFields(fixture: SuiteFixture, request: APIRequestContext, id: string) {
+	const row = await (
+		await request.get(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${id}`, {
+			headers: authHeaders(fixture),
+		})
+	).json();
+	return { fields: JSON.parse(row.fields) as Record<string, string>, seq: row.seq as number };
+}
+
+function fieldInput(page: Page, label: string) {
+	return page
+		.locator('.item-page-host > .item-page')
+		.locator(`.field-row:has(.field-label:text-is("${label}"))`)
+		.locator('input.field-input');
+}
+
+/** A response held after its request committed on the real server. */
+type Held = { route: Route; response: APIResponse };
+
+function isItemPatch(route: Route, itemId: string) {
+	return route.request().method() === 'PATCH' && new URL(route.request().url()).pathname.endsWith(`/items/${itemId}`);
+}
+
+function patchedKeys(route: Route): string[] {
+	return Object.keys(JSON.parse(route.request().postData() ?? '{}').fields_patch ?? {});
+}
+
+test.describe('BUG-3036: whole-item snapshots are ordered by the row seq', () => {
+	test.beforeEach(({}, testInfo) => {
+		test.skip(testInfo.project.name !== 'desktop-chromium', 'one browser is enough for response ordering');
+	});
+
+	test('a slow field response does not revert a newer write to another field', async ({ page, fixture, request }) => {
+		await page.setViewportSize(DESKTOP);
+		await browserLogin(page);
+		// No SSE: its re-read would repair the screen and hide the defect. This
+		// test is about the PATCH responses alone. The SSE re-read has its own
+		// test below.
+		await page.route('**/api/v1/events*', (route) => route.abort());
+		const { collSlug, item } = await seed(fixture, request, 'patch');
+
+		let heldAlpha: Held | null = null;
+		const alphaCommitted = new Promise<void>((resolve) => {
+			void page.route(`**/api/v1/workspaces/*/items/${item.id}`, async (route) => {
+				if (!isItemPatch(route, item.id) || heldAlpha || !patchedKeys(route).includes('alpha')) {
+					return route.fallback();
+				}
+				// The write commits on the server now. Its answer is held.
+				heldAlpha = { route, response: await route.fetch() };
+				resolve();
+			});
+		});
+
+		await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}/${item.slug}`);
+		const alpha = fieldInput(page, 'Alpha');
+		const beta = fieldInput(page, 'Beta');
+		await expect(alpha).toHaveValue('alpha-0');
+
+		await alpha.fill('alpha-1');
+		await alpha.blur();
+		await alphaCommitted;
+		const afterAlpha = await serverFields(fixture, request, item.id);
+		expect(afterAlpha.fields.alpha, 'PREMISE: the held alpha write committed').toBe('alpha-1');
+
+		// The newer write, to a DIFFERENT field. Its response arrives first.
+		// Beta's FIRST attempt is refused 409: its token is the seq on screen,
+		// which is older than alpha's held commit. The pane refetches and retries,
+		// so wait for the attempt that SUCCEEDED.
+		const betaResponse = page.waitForResponse(
+			(r) =>
+				r.request().method() === 'PATCH' &&
+				r.url().includes(`/items/${item.id}`) &&
+				r.ok() &&
+				'beta' in (JSON.parse(r.request().postData() ?? '{}').fields_patch ?? {}),
+		);
+		await beta.fill('beta-1');
+		await beta.blur();
+		expect((await betaResponse).ok()).toBeTruthy();
+		await expect(beta, 'PREMISE: the newer write rendered').toHaveValue('beta-1');
+		const afterBeta = await serverFields(fixture, request, item.id);
+		expect(afterBeta.seq, 'PREMISE: the held response is a genuinely older row').toBeGreaterThan(afterAlpha.seq);
+
+		// Deliver the OLDER response now. Before the fix it assigned the whole
+		// item, with beta still at beta-0.
+		const held = heldAlpha as Held | null;
+		expect(held, 'PREMISE: alpha response was held').not.toBeNull();
+		const delivered = page.waitForResponse(
+			(r) => r.request().method() === 'PATCH' && r.url().includes(`/items/${item.id}`),
+		);
+		await held!.route.fulfill({ response: held!.response });
+		await delivered;
+		// The response is delivered; give the pane's continuation a moment to run
+		// before asserting the screen did NOT change. (An absence assertion, so
+		// the skip mutant below is what shows this leg can go red.)
+		await page.waitForTimeout(300);
+
+		await expect(beta, 'BUG-3036: a slow response reverted a newer write on screen').toHaveValue('beta-1');
+		await expect(alpha).toHaveValue('alpha-1');
+		const final = await serverFields(fixture, request, item.id);
+		expect(final.fields).toMatchObject({ alpha: 'alpha-1', beta: 'beta-1' });
+	});
+
+	test('a slow SSE re-read does not revert a field written while it was in flight', async ({
+		page,
+		fixture,
+		request,
+	}) => {
+		await page.setViewportSize(DESKTOP);
+		await browserLogin(page);
+
+		// THE EVENT STREAM IS THE TEST'S. The window this leg needs is narrow on a
+		// live stream: beta's own SSE event bumps the pane's itemGen, and that
+		// already drops the older re-read if it lands first (measured: with the
+		// guard removed, this leg passed on a live stream). The defect is the
+		// re-read resolving AFTER beta's response renders and BEFORE beta's event,
+		// so the stream is held, answered once with the one event this leg is
+		// about, and held again on every reconnect (`retry` keeps the browser from
+		// reconnecting inside the test anyway).
+		const streams: Route[] = [];
+		await page.route('**/api/v1/events*', (route) => {
+			streams.push(route);
+		});
+
+		const { collSlug, item } = await seed(fixture, request, 'sse');
+		await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}/${item.slug}`);
+		const beta = fieldInput(page, 'Beta');
+		const gamma = fieldInput(page, 'Gamma');
+		await expect(gamma).toHaveValue('gamma-0');
+		await expect.poll(() => streams.length, { message: 'PREMISE: the pane opened its event stream' }).toBeGreaterThan(0);
+
+		// Hold the pane's re-reads (it reads by SLUG), armed only now so the
+		// load's own GET is not caught.
+		const heldReads: Held[] = [];
+		await page.route(`**/api/v1/workspaces/*/items/${item.slug}`, async (route) => {
+			if (route.request().method() !== 'GET') return route.fallback();
+			heldReads.push({ route, response: await route.fetch() });
+		});
+		// The same event also makes the sync service pull a delta (`/changes`)
+		// and the local index pull its own (`/items-changes`). The sync pass bumps
+		// the pane's itemGen and installs the row, which drops the re-read above by
+		// the older fence before it can land (measured: bump, then install at the
+		// incremental path). Hold both, so the SSE re-read is the one being tested.
+		// The incremental path's own seq check is exercised when they are released
+		// at the end.
+		const heldDeltas: Held[] = [];
+		const DELTAS = ['**/api/v1/workspaces/*/items-changes*', '**/api/v1/workspaces/*/changes*'];
+		for (const pattern of DELTAS) {
+			await page.context().route(pattern, async (route) => {
+				heldDeltas.push({ route, response: await route.fetch() });
+			});
+		}
+
+		// Someone else writes gamma for real, and the pane hears about it.
+		const external = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${item.id}`, {
+			headers: authHeaders(fixture),
+			data: { fields_patch: { gamma: 'gamma-ext' } },
+		});
+		expect(external.ok(), await external.text()).toBeTruthy();
+		const ext = await external.json();
+		const event = { type: 'item_updated', item_id: item.id, actor: 'user', source: 'api', timestamp: Date.now(), seq: ext.seq };
+		await streams[streams.length - 1].fulfill({
+			status: 200,
+			headers: { 'Content-Type': 'text/event-stream', 'Cache-Control': 'no-cache' },
+			body: `retry: 600000\n\nevent: item_updated\ndata: ${JSON.stringify(event)}\n\n`,
+		});
+		// The re-read it causes is served from the row as it stands NOW — gamma-ext,
+		// beta-0 — and held.
+		await expect.poll(() => heldReads.length, { message: 'PREMISE: the event made the pane re-read' }).toBeGreaterThan(0);
+		const olderCount = heldReads.length;
+
+		// A newer write lands while that read is in flight. Beta's FIRST attempt
+		// may be refused 409 (its token can predate the external write); the pane
+		// refetches by id and retries, so wait for the attempt that SUCCEEDED.
+		const betaResponse = page.waitForResponse(
+			(r) =>
+				r.request().method() === 'PATCH' &&
+				r.url().includes(`/items/${item.id}`) &&
+				r.ok() &&
+				'beta' in (JSON.parse(r.request().postData() ?? '{}').fields_patch ?? {}),
+		);
+		await beta.fill('beta-1');
+		await beta.blur();
+		await betaResponse;
+		await expect(beta, 'PREMISE: the newer write rendered').toHaveValue('beta-1');
+		// beta's response carries the external write: the server merged beta onto it.
+		await expect(gamma, 'PREMISE: the external write is on screen').toHaveValue('gamma-ext');
+
+		// Deliver the older read(s) — the row from BEFORE beta.
+		for (const h of heldReads.splice(0, olderCount)) await h.route.fulfill({ response: h.response });
+		// An absence assertion: give the continuation a moment, then assert the
+		// screen did NOT change. The mutant run is what shows it can go red.
+		await page.waitForTimeout(500);
+
+		await expect(beta, 'BUG-3036: a slow SSE re-read reverted a newer write on screen').toHaveValue('beta-1');
+		await expect(gamma).toHaveValue('gamma-ext');
+
+		// Release everything else, the held deltas included: those are older rows
+		// too, and the incremental path must refuse them as well.
+		for (const h of heldReads.splice(0)) await h.route.fulfill({ response: h.response });
+		for (const h of heldDeltas.splice(0)) await h.route.fulfill({ response: h.response });
+		await page.unroute(`**/api/v1/workspaces/*/items/${item.slug}`);
+		for (const pattern of DELTAS) await page.context().unroute(pattern);
+		await page.waitForTimeout(500);
+		await expect(beta, 'BUG-3036: an older delta reverted a newer write on screen').toHaveValue('beta-1');
+	});
+
+	test('a change that arrives during a save is shown once the save settles', async ({
+		page,
+		fixture,
+		request,
+	}) => {
+		await page.setViewportSize(DESKTOP);
+		await browserLogin(page);
+		const { collSlug, item } = await seed(fixture, request, 'owed');
+
+		let heldAlpha: Held | null = null;
+		const alphaCommitted = new Promise<void>((resolve) => {
+			void page.route(`**/api/v1/workspaces/*/items/${item.id}`, async (route) => {
+				if (!isItemPatch(route, item.id) || heldAlpha || !patchedKeys(route).includes('alpha')) {
+					return route.fallback();
+				}
+				heldAlpha = { route, response: await route.fetch() };
+				resolve();
+			});
+		});
+
+		await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${collSlug}/${item.slug}`);
+		const alpha = fieldInput(page, 'Alpha');
+		const gamma = fieldInput(page, 'Gamma');
+		await expect(gamma).toHaveValue('gamma-0');
+
+		// A save is in flight: committed, answer held, so the pane is 'saving'.
+		await alpha.fill('alpha-1');
+		await alpha.blur();
+		await alphaCommitted;
+		await expect(page.locator('.item-page-host .save-status')).toContainText('Saving');
+
+		// Someone else writes gamma during the save, and its SSE event reaches the
+		// pane while it is still saving. That delivery cannot be observed from here
+		// (a deferred event issues no request), so the wait is a plain delay. What
+		// shows the event DID land mid-save is the counterfactual: had it arrived
+		// after the save settled, the ordinary re-read would show gamma-ext with
+		// skipping restored too, and the skip mutant would survive. It is run red
+		// for exactly that reason.
+		const external = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${item.id}`, {
+			headers: authHeaders(fixture),
+			data: { fields_patch: { gamma: 'gamma-ext' } },
+		});
+		expect(external.ok(), await external.text()).toBeTruthy();
+		await page.waitForTimeout(1500);
+		await expect(gamma, 'PREMISE: the change is not shown while the save is in flight').toHaveValue('gamma-0');
+
+		// The save settles with its (older) row, which lacks gamma-ext.
+		const held = heldAlpha as Held | null;
+		await held!.route.fulfill({ response: held!.response });
+
+		await expect(gamma, 'BUG-3036: a change skipped during a save was never shown').toHaveValue('gamma-ext', {
+			timeout: 5000,
+		});
+		await expect(alpha).toHaveValue('alpha-1');
+	});
+});
@@ -4,6 +4,7 @@
 	import { api, PadApiError, isUpdateConflictError, type ImportURLResponse } from '$lib/api/client';
 	import { confirmOpenChildrenOrThrow, isOpenChildrenError } from '$lib/items/openChildrenError';
 	import { WriteOrder, fieldWriteTarget, submitOrderedOCC } from '$lib/items/fieldWriteOrder';
+	import { isOlderSnapshot } from '$lib/items/itemSnapshotOrder';
 	import { occTokenFor, type OCCToken } from '$lib/items/occToken';
 	import { marked } from 'marked';
 	import { collectionStore } from '$lib/stores/collections.svelte';
@@ -1385,13 +1386,11 @@
 				// carry a stale field value back to the server, because no write
 				// names a key the user did not edit. What remains is a display
 				// concern, which is what this refetch now serves.
-				if (
-					event.items_changed &&
-					item &&
-					itemMatchesRef &&
-					saveStatus !== 'saving' &&
-					!editingTitle
-				) {
+				// Mid-edit, the refetch is OWED rather than skipped (BUG-3036): a
+				// skip dropped the change until some unrelated event came along.
+				if (event.items_changed && item && itemMatchesRef && (saveStatus === 'saving' || editingTitle)) {
+					refreshOwed = true;
+				} else if (event.items_changed && item && itemMatchesRef) {
 					const reqItemId = item.id;
 					const reqWsSlug = wsSlug;
 					const reqItemSlug = itemSlug;
@@ -1464,11 +1463,16 @@
 				return;
 			}

-			// Non-destructive updates: skip if the user is actively
-			// editing the title or has a pending content save in flight.
-			// They'll catch up on the next idle event (and the
-			// syncService onTabResume path also covers anything missed).
-			if (saveStatus === 'saving' || editingTitle) return;
+			// Non-destructive updates: DEFER while the user is actively editing
+			// the title or has a save in flight. The re-read runs once that ends
+			// (`refreshOwed`, BUG-3036). It used to be skipped outright and wait
+			// for "the next idle event", which may never come: the pane stayed
+			// behind the server, and the save's own response then installed a
+			// row older than the change it had skipped.
+			if (saveStatus === 'saving' || editingTitle) {
+				refreshOwed = true;
+				return;
+			}

 			// Capture the item this event was scoped to *before* awaiting.
 			// Otherwise a navigation that completes during the in-flight
@@ -1502,9 +1506,14 @@
 						// a debounced save is in flight, but a user
 						// mid-keystroke with no save yet pending would
 						// still lose chars without this branch.
+						//
+						// The follow-ups read `item`, the row actually INSTALLED,
+						// not `updated`: an older row than the one shown is
+						// refused by withInflightTags (BUG-3036), and its stale
+						// collection and slug must not drive them.
 						item = adoptServerItem(updated);
-						void refreshCollectionIfMoved(updated);
-						const links = await refreshLinksPreservingOnFailure(reqWsSlug, updated.slug);
+						void refreshCollectionIfMoved(item);
+						const links = await refreshLinksPreservingOnFailure(reqWsSlug, item.slug);
 						if (!item || item.id !== reqItemId || myItemGen !== itemGen) return;
 						itemLinks = links;
 					} catch {
@@ -1592,8 +1601,12 @@
 				return;
 			}

-			// Don't refresh non-destructive updates if the user is actively editing
-			if (saveStatus === 'saving' || editingTitle) return;
+			// Don't refresh non-destructive updates if the user is actively
+			// editing — DEFER it until they stop (BUG-3036, as the SSE handler).
+			if (saveStatus === 'saving' || editingTitle) {
+				refreshOwed = true;
+				return;
+			}

 			// Capture the item this sync was scoped to *before* awaiting.
 			// Same race guard as the SSE handler above and loadData() —
@@ -1611,6 +1624,10 @@
 					// Same collab-aware adoption rule as the SSE
 					// handler above (TASK-1262).
 					if (!item || item.id !== reqItemId) return;
+					// An OLDER row than the one shown is dropped, and BEFORE the
+					// bump: bumping for it would cancel a fresher in-flight refetch
+					// and then install nothing (BUG-3036).
+					if (isOlderSnapshot(item, updated)) return;
 					// Bump the item-snapshot gen so an in-flight refetch drops (round 9).
 					const myItemGen = ++itemGen;
 					item = adoptServerItem(updated);
@@ -1630,9 +1647,12 @@
 				item = adoptServerItem(updated);
 				// Same collection-moved handling as the SSE + incremental
 				// adoptions above — a long tab absence can span a move too
-				// (codex round 2 P1).
-				void refreshCollectionIfMoved(updated);
-				const links = await refreshLinksPreservingOnFailure(reqWsSlug, updated.slug);
+				// (codex round 2 P1). The follow-ups read `item`, the row actually
+				// INSTALLED: an older row than the one shown is refused by
+				// withInflightTags (BUG-3036), and the screen is then at least as
+				// new as this read — so the cursor below may still advance.
+				void refreshCollectionIfMoved(item);
+				const links = await refreshLinksPreservingOnFailure(reqWsSlug, item.slug);
 				if (!item || item.id !== reqItemId || myItemGen !== itemGen) return;
 				itemLinks = links;
 				// Advance the cursor now that the ITEM reload succeeded. The links
@@ -1869,6 +1889,10 @@
 		// Per Codex review round 12.
 		clearTimeout(saveStatusTimer);
 		saveStatusTimer = undefined;
+		// Cleared BEFORE the status goes idle: a refresh owed to the previous
+		// item must not fire against this one when the effect below sees the
+		// status drop (BUG-3036).
+		refreshOwed = false;
 		saveStatus = 'idle';
 		// Capture the URL parts this load was scoped to. Used in the catch
 		// path to detect whether the user has navigated away before this
@@ -3457,7 +3481,18 @@
 	// saver's latest desired set at assignment time is the only race-free
 	// guarantee that a concurrent snapshot can't drop the unsaved tags. Per
 	// Codex PR #659 rounds 8/9.
+	//
+	// It is also where a snapshot OLDER than the one on screen is refused
+	// (BUG-3036): every writer in this pane answers with the whole row, so a
+	// response that is merely slow would otherwise put an earlier row over a later
+	// one while the server keeps the later. Ordered by the row's `seq` — see
+	// `itemSnapshotOrder.ts` for why that and not a dispatch count. A refused
+	// snapshot returns the item already shown, so `item = withInflightTags(x)`
+	// leaves it untouched; a site that goes on to USE `x` after assigning it asks
+	// `isOlderSnapshot` itself first.
 	function withInflightTags(next: Item): Item {
+		const shown = untrack(() => item);
+		if (shown && isOlderSnapshot(shown, next)) return shown;
 		const saver = tagSavers.get(next.id);
 		// Only a burst typed under the CURRENT identity owns `item.tags` (BUG-3084
 		// codex round 1): the identity listener's load would otherwise render the
@@ -3476,6 +3511,50 @@
 		);
 	}

+	/**
+	 * A re-read an SSE event or sync pass asked for while a save was in flight or
+	 * the title was being edited (BUG-3036). Those used to be SKIPPED, so a change
+	 * made elsewhere during a save was dropped until something unrelated caused a
+	 * refresh, and the save's response then installed its own older row.
+	 *
+	 * Deferring is safe only because of the snapshot order in `withInflightTags`:
+	 * the re-read may resolve before or after the save it waited on, and an older
+	 * row cannot replace a newer one either way.
+	 *
+	 * A plain `let`, like `hasPending` in FieldEditor: the effect below reads it,
+	 * and making it reactive would have the effect's own write retrigger it.
+	 */
+	let refreshOwed = false;
+
+	async function runOwedRefresh() {
+		if (destroyed || !item) return;
+		const reqItemId = item.id;
+		const reqWsSlug = wsSlug;
+		const reqItemSlug = itemSlug;
+		const myItemGen = ++itemGen;
+		try {
+			const updated = await api.items.get(reqWsSlug, reqItemSlug);
+			if (!item || item.id !== reqItemId || myItemGen !== itemGen) return;
+			// Follow-ups read the row actually installed (see the SSE re-read).
+			item = adoptServerItem(updated);
+			void refreshCollectionIfMoved(item);
+			const links = await refreshLinksPreservingOnFailure(reqWsSlug, item.slug);
+			if (!item || item.id !== reqItemId || myItemGen !== itemGen) return;
+			itemLinks = links;
+		} catch {
+			// Best-effort, as the refreshes it stands in for.
+		}
+	}
+
+	$effect(() => {
+		// Tracks the two conditions the refreshes were deferred on, and nothing
+		// else: `refreshOwed` is plain, and the re-read runs untracked.
+		if (saveStatus === 'saving' || editingTitle) return;
+		if (!refreshOwed) return;
+		refreshOwed = false;
+		untrack(() => void runOwedRefresh());
+	});
+
 	// BUG-2178 (codex R1): an SSE / delta-sync adoption can change the item's
 	// COLLECTION — a cross-collection move by another user/tab, or this pane's
 	// own move when the full-page host's retarget same-ref-guards to a noop
@@ -92,7 +92,7 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
 	jumpToSection: { reviewed: '17407aeb431b', why: 'switches this instance\'s tab and scrolls to an anchor', may: ['document.getElementById', 'document.getElementById(anchorId).scrollIntoView'] },
 	ensureGraphComp: { reviewed: 'c1565cfb8a18', why: 'lazy-loads a component module into this instance', may: ['ItemGraphComp', 'graphLoadError'] },
 	handleCopyRef: { reviewed: 'fb3adcaf167a', why: 'switchedAway before the copied flag' },
-	loadData: { reviewed: '451e2cd3d6e8', why: 'IS the load: myGen against loadGeneration after every await' },
+	loadData: { reviewed: '280f389164ae', why: 'IS the load: myGen against loadGeneration after every await' },
 	startEditTitle: { reviewed: '113d9b9da03f', why: 'focuses and sizes the input it opened synchronously', may: ['el', 'titleInputEl.focus', 'titleInputEl.setSelectionRange'] },
 	saveTitle: { reviewed: '8a6bedbc107a', why: 'gen against loadGeneration on both arms' },
 	updateField: {
@@ -127,6 +127,9 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
 		bareAwaits: ['await new Promise((r) => setTimeout(r, 50));'],
 	},
 	refreshLinksPreservingOnFailure: { reviewed: '28ef88d9c682', why: 'returns a value; its callers fence' },
+	// BUG-3036: the re-read an SSE event or sync pass deferred while a save was in
+	// flight. The SSE item_updated shape: itemGen and the item id after each await.
+	runOwedRefresh: { reviewed: '9dadf261b60f', why: 'itemGen and the item id after each await, as the SSE item_updated re-read it stands in for' },
 	flushCollabBeforeRestore: { reviewed: '0d45557c8da9', why: 'identityHeld before its failure toast' },
 	closeCopyDialog: { reviewed: '1c925f081a26', why: 'restores focus after closing synchronously', may: ['paneMenuTrigger.focus'] },
 	closePushDialog: { reviewed: 'bda8c7529677', why: 'restores focus after closing synchronously', may: ['paneMenuTrigger.focus'] },
@@ -161,8 +164,8 @@ interface SignedRow extends Row {

 /** Async functions that are not top-level declarations, in the script. */
 const NESTED: SignedRow[] = [
-	{ body: /event\.type === 'collection_updated'/, why: 'SSE: callbackGen after the collection fetch, itemGen on item branches', reviewed: 'af4846d29b06' },
-	{ body: /result\.type === 'caught_up'/, why: 'sync: callbackGen after the reconciliation, itemGen on item branches', reviewed: '1f7b5cff922e' },
+	{ body: /event\.type === 'collection_updated'/, why: 'SSE: callbackGen after the collection fetch, itemGen on item branches', reviewed: '55875d030fa4' },
+	{ body: /result\.type === 'caught_up'/, why: 'sync: callbackGen after the reconciliation, itemGen on item branches', reviewed: 'f19ccd555a1c' },
 	{ body: /flushCollabContent\(/, why: 'collab save: isForegroundCurrent (genAtFlush) before UI feedback', reviewed: '05b4038a08b0' },
 ];

@@ -336,7 +339,7 @@ const CONTINUATIONS: SignedRow[] = [
 		may: ['renameOverride'],
 	},
 	{ call: /^setTimeout\($/, body: /copied = false/, why: 'copy-flag reset: switchedAway', reviewed: '199047839886' },
-	{ call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: '686654a9efa6' },
+	{ call: /api\.items\.get\(wsSlug, itemSlug\)\.catch\($/, body: /./, why: 'loadData item fetch: sets a flag local to that load and re-throws', reviewed: 'f04e453039e6' },
 	{
 		call: /^setTimeout\($/,
 		body: /staleConnecting = true/,
@@ -404,7 +407,8 @@ const HELPERS: Record<string, string> = {
 	settleCollabIfCurrent: '0d243d24a373',
 	showSaved: '3f1ef91fac07',
 	switchedAway: '8ba85844f406',
-	withInflightTags: '11523b4d849a',
+	// BUG-3036: refuses a snapshot of the shown item whose seq is strictly older.
+	withInflightTags: 'd9aeb96e6bd1',
 };

 /**
@@ -120,11 +120,16 @@ describe('ItemDetail: a failed same-item links refresh keeps the links it has',
 	});

 	it('routes every same-item refresh through the preserving helper', () => {
-		// Three refresh callers: the SSE adopt path, the incremental adopt path,
-		// and the long-absence reload. All three fetch by `updated.slug`, i.e.
-		// the item already loaded, which is what makes preserving correct.
-		const calls = count(code, 'await refreshLinksPreservingOnFailure(reqWsSlug, updated.slug)');
-		expect(calls).toBe(3);
+		// Four refresh callers: the SSE adopt path, the incremental adopt path,
+		// the long-absence reload, and the refresh a save deferred (BUG-3036).
+		// All four fetch the links of the item already loaded, which is what
+		// makes preserving correct. Three spell it `item.slug` — the row they
+		// actually INSTALLED, since withInflightTags may refuse an older
+		// `updated` (BUG-3036) — and the incremental path, which returns before
+		// installing an older row, still spells it `updated.slug`.
+		expect(count(code, 'await refreshLinksPreservingOnFailure(reqWsSlug, item.slug)')).toBe(3);
+		expect(count(code, 'await refreshLinksPreservingOnFailure(reqWsSlug, updated.slug)')).toBe(1);
+		expect(count(code, 'await refreshLinksPreservingOnFailure(')).toBe(4);
 	});

 	it('leaves exactly two direct api.links.list calls: the helper and the initial load', () => {
@@ -0,0 +1,33 @@
+import { describe, expect, it } from 'vitest';
+import { isOlderSnapshot } from './itemSnapshotOrder';
+
+describe('isOlderSnapshot (BUG-3036)', () => {
+	const shown = { id: 'a', seq: 10 };
+
+	it('refuses a strictly older snapshot of the item on screen', () => {
+		expect(isOlderSnapshot(shown, { id: 'a', seq: 9 })).toBe(true);
+	});
+
+	it('accepts an equal seq — a re-read of an unchanged row', () => {
+		expect(isOlderSnapshot(shown, { id: 'a', seq: 10 })).toBe(false);
+	});
+
+	it('accepts a newer seq', () => {
+		expect(isOlderSnapshot(shown, { id: 'a', seq: 11 })).toBe(false);
+	});
+
+	it('never refuses a different item, whatever its seq', () => {
+		expect(isOlderSnapshot(shown, { id: 'b', seq: 1 })).toBe(false);
+	});
+
+	it('accepts when either side has no seq', () => {
+		expect(isOlderSnapshot({ id: 'a' }, { id: 'a', seq: 1 })).toBe(false);
+		expect(isOlderSnapshot(shown, { id: 'a' })).toBe(false);
+		expect(isOlderSnapshot({ id: 'a', seq: null }, { id: 'a', seq: 1 })).toBe(false);
+	});
+
+	it('accepts when nothing is on screen', () => {
+		expect(isOlderSnapshot(null, { id: 'a', seq: 1 })).toBe(false);
+		expect(isOlderSnapshot(undefined, { id: 'a', seq: 1 })).toBe(false);
+	});
+});
@@ -0,0 +1,47 @@
+/**
+ * Ordering for WHOLE-ITEM snapshots installed by the item pane (BUG-3036).
+ *
+ * `fieldWriteOrder.ts` orders writes to ONE field against each other. This is
+ * the layer above it: every writer in the pane — a field PATCH, a title or tag
+ * save, a restore, an SSE-driven re-read — answers with the whole row and
+ * assigns it to `item`, so a response that is merely SLOW puts an older row on
+ * screen over a newer one while the server keeps the newer.
+ *
+ * The order is the ROW's `seq`, not a client dispatch count. `seq` is stamped
+ * from the workspace's mutation cursor on every create / update / soft-delete /
+ * restore of the row, under the write lock, so of two snapshots of one row the
+ * higher `seq` is the later mutation. A dispatch count would answer a different
+ * question — "was a newer write SENT?" — and drop an older successful response
+ * on the strength of a newer write that then FAILS, so a save the server kept
+ * would never be shown (the BUG-2773 hole `fieldWriteOrder.ts`'s two counters
+ * exist to avoid). `seq` also orders writers no client counter can see: other
+ * tabs, agents, the SSE re-read.
+ *
+ * Dropping an older snapshot loses nothing that was written: field writes are
+ * merged server-side (`fields_patch`), so the newer row already carries the
+ * older write's value.
+ */
+
+type Snapshot = { id: string; seq?: number | null };
+
+/**
+ * True when `incoming` is a snapshot of the item currently shown and records a
+ * STRICTLY earlier mutation of it than what is on screen — it must not replace
+ * it.
+ *
+ * Deliberately narrow, so it can only ever refuse what is provably older:
+ *   - a DIFFERENT item is never refused — the comparison is against the item on
+ *     screen, so after a switch nothing about the previous item can block the
+ *     next one's first snapshot;
+ *   - EQUAL seq is accepted: re-reading an unchanged row can still carry fresher
+ *     derived data (links, hydrated relation targets) that do not bump the row;
+ *   - a MISSING seq on either side is accepted, which is the behaviour before
+ *     this existed (a snapshot cached before the column shipped has none).
+ */
+export function isOlderSnapshot(current: Snapshot | null | undefined, incoming: Snapshot): boolean {
+	if (!current || current.id !== incoming.id) return false;
+	const have = current.seq;
+	const got = incoming.seq;
+	if (typeof have !== 'number' || typeof got !== 'number') return false;
+	return got < have;
+}