fix(collab): the editor is read-only through a force_refresh recovery, so no typing is lost (BUG-3697) (#2128)

On force_refresh the provider is destroyed, the tab's text is captured for
the recovery notice, and the editor is rebuilt only after the item refetch
resolves. The editor stayed editable through that refetch, so typing in the
window reached no Y.Doc, missed the recovery copy, and vanished on the
rebuild; if the refetch failed it stayed editable indefinitely, against a
comment calling it read-only.

recoveryFrozen is set in the same task as the capture, before it, and cleared
only when the rebuilt provider is wired; the failure arm leaves it set. The
editor's editable reads it. e2e bug-3697-typing-during-recovery parks the
refetch and asserts the editor refuses input inside the window and is
editable again after.

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

web/e2e/bug-3697-typing-during-recovery.spec.ts

@@ -0,0 +1,65 @@
+// BUG-3697: on force_refresh the provider is destroyed, the tab's text is
+// captured for the recovery notice, and the editor is rebuilt only after a
+// refetch of the item. The editor stayed editable through that refetch, so a
+// keystroke in the window reached no Y.Doc, missed the recovery copy, and
+// vanished on the rebuild. The editor is now read-only for the whole recovery.
+//
+// The window is held open deterministically: the refetch (a GET of this item
+// by id) is parked until the test releases it. The force_refresh itself is
+// BUG-3526's recipe: the tab goes offline holding a cursor, a direct write
+// prunes the op-log, and the reconnect is refreshed.
+import { test, expect } from './fixtures';
+import { browserLogin, EDITOR_SELECTOR, SYNCED_BADGE_SELECTOR, SYNC_TIMEOUT, seedDoc } from './lib/collab-helpers';
+
+test('the editor accepts no typing while a force_refresh recovery is in flight', async ({ browser, fixture, request }, testInfo) => {
+	test.skip(testInfo.project.name !== 'desktop-chromium', 'one project is enough');
+	test.setTimeout(120_000);
+	const { id, slug } = await seedDoc(fixture, request, 'BUG-3697');
+	const ctx = await browser.newContext();
+	const page = await ctx.newPage();
+
+	let hold = false;
+	let held = 0;
+	let release!: () => void;
+	const released = new Promise<void>((r) => { release = r; });
+	await page.route(new RegExp(`/api/v1/workspaces/[^/]+/items/${id}(\\?|$)`), async (route) => {
+		if (!hold || route.request().method() !== 'GET') return route.continue();
+		held++;
+		await released;
+		await route.continue();
+	});
+
+	await browserLogin(page);
+	await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${slug}`);
+	await expect(page.locator(SYNCED_BADGE_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT });
+	await page.locator(EDITOR_SELECTOR).click();
+	await page.keyboard.type('Stored first. ');
+	await page.waitForTimeout(3_000);
+
+	await ctx.setOffline(true);
+	await expect(page.locator(SYNCED_BADGE_SELECTOR)).toBeHidden({ timeout: SYNC_TIMEOUT });
+	const res = await request.patch(`/api/v1/workspaces/${fixture.workspaceSlug}/items/${slug}`, {
+		headers: { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' },
+		data: { content: 'Server body written while the tab was away.' },
+	});
+	expect(res.ok(), `the direct write failed: ${res.status()}`).toBeTruthy();
+
+	hold = true;
+	await ctx.setOffline(false);
+	await expect.poll(() => held, { timeout: SYNC_TIMEOUT, message: 'premise: the force_refresh refetch started and is parked' }).toBeGreaterThan(0);
+
+	// The recovery window: the editor must refuse input, and typing into it
+	// must change nothing.
+	const editor = page.locator(EDITOR_SELECTOR);
+	await expect(editor).toHaveAttribute('contenteditable', 'false');
+	await editor.click({ force: true });
+	await page.keyboard.type('Typed during recovery.');
+	await expect(editor).not.toContainText('Typed during recovery.');
+
+	hold = false;
+	release();
+	await expect(editor).toContainText('Server body written while the tab was away.', { timeout: SYNC_TIMEOUT });
+	// Control: the rebuilt editor is editable again.
+	await expect(editor).toHaveAttribute('contenteditable', 'true', { timeout: SYNC_TIMEOUT });
+	await ctx.close();
+});

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

@@ -2497,6 +2497,14 @@
 	// the new collab $effect run (after rebuild completes). Per
 	// Codex round 7 [P1] of TASK-1319.
 	let forceRefreshInFlight = false;
+	// The editor is read-only from a force_refresh until the rebuilt provider
+	// is wired, and stays so if the refetch fails (BUG-3697). The provider is
+	// destroyed by then, so a keystroke in that window reaches no Y.Doc, is not
+	// in the recovery copy captured at the start, and vanishes on the rebuild.
+	// $state, unlike forceRefreshInFlight, because the editor's `editable`
+	// reads it; Svelte applies it in the microtask after the handler, before
+	// any later keystroke event can run.
+	let recoveryFrozen = $state(false);
 	// Gated on `itemMatchesRef` (PLAN-2105 / TASK-2112 switch-safety): the
 	// instant `ref` changes, `item` still holds the PREVIOUS item until the
 	// new fetch resolves, so without this gate collabKey would stay the old
@@ -2867,6 +2875,9 @@
 			// either flushes a real difference or dedupes and stamps.
 			onOpLogCursor: () => settleCollabIfCurrent(ctx),
 			onForceRefresh: () => {
+				// BUG-3697: no keystroke is accepted that this recovery would
+				// then discard. Frozen in the same task as the capture below.
+				recoveryFrozen = true;
 				// TASK-2199: local edits this rebuild discards are captured first
 				// and handed back, never written over the server's text.
 				const recovered = offlineRecoveryOnForceRefresh({
@@ -2951,8 +2962,10 @@
 						// corruption force_refresh was meant to
 						// prevent. Surface a hard error and ask
 						// the user to reload manually instead.
-						// The provider is already destroyed so
-						// the editor is read-only effectively;
+						// The provider is already destroyed and
+						// recoveryFrozen stays true, so the editor
+						// stays read-only (BUG-3697: it used to stay
+						// editable, and typing here was lost);
 						// forceRefreshInFlight stays true so any
 						// in-flight or scheduled flush is blocked.
 						// Per Codex round 17 [P1] of TASK-1319.
@@ -2975,6 +2988,7 @@
 		// this point can safely schedule flushes again. Per Codex
 		// round 7 [P1] of TASK-1319.
 		forceRefreshInFlight = false;
+		recoveryFrozen = false;
 
 		return () => {
 			// Best-effort flush of items.content BEFORE we tear the
@@ -7680,7 +7694,7 @@
 								<Editor
 									content={editorContent}
 									onUpdate={handleContentUpdate}
-									editable={!peeking}
+									editable={!peeking && !recoveryFrozen}
 									itemId={item.id}
 									hostToken={attachmentHostToken}
 									ydoc={ydoc}

web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts

@@ -282,8 +282,8 @@ const CONTINUATIONS: SignedRow[] = [
 		why: 'connection state of this instance\'s own provider', reviewed: 'b0f9f4cc5f23',
 		may: ['staleConnecting'],
 	},
-	{ call: /\.get\(refreshCtx\.wsSlug, refreshCtx\.itemId\) \.then\($/, body: /./, why: 'force-refresh fetch: refreshGen against loadGeneration. Re-reviewed for TASK-2199: the enclosing onForceRefresh first captures the edits it discards (offlineRecovery), synchronously, before any await. Re-reviewed for BUG-3526: the capture reads provider.editsMayBeMissing instead of provider.unsentLocalEdits, still synchronous and before the fetch; the continuation and its fences are unchanged. Re-reviewed for BUG-3582: only the enclosing collab effect moved (onApplierRequest converts wiki-links before its setContent, synchronously, no await, no new write); this continuation is unchanged. Re-reviewed for PLAN-3578 step 2: only the enclosing collab effect moved again (onApplierPatch added to the provider options: a synchronous plan, the bracket callback and one dispatch; no await, no generation or identity write); this continuation is unchanged Re-reviewed for the PLAN-3578 retro fix (day 91): only the enclosing collab effect\'s onApplierPatch changed, building its one dispatch through patchTransaction (hunks) instead of one replaceWith; synchronous, no await, no generation or identity write; this continuation is unchanged.', reviewed: 'd22af24b6e57' },
-	{ call: /forceRefreshNonce \+= 1; \}\) \.catch\($/, body: /./, why: 'force-refresh failure: refreshGen against loadGeneration. Re-reviewed for TASK-2199 (the synchronous capture above it), and for BUG-3526 (only the input of that capture). Re-reviewed for BUG-3582: only the enclosing collab effect moved (the synchronous wiki-link conversion in the applier); this continuation is unchanged. Re-reviewed for PLAN-3578 step 2: the enclosing collab effect gained onApplierPatch (synchronous, no await, no generation write); this continuation is unchanged Re-reviewed for the PLAN-3578 retro fix (day 91): only the enclosing collab effect\'s onApplierPatch changed, building its one dispatch through patchTransaction (hunks) instead of one replaceWith; synchronous, no await, no generation or identity write; this continuation is unchanged.', reviewed: '756d8e1edf53' },
+	{ call: /\.get\(refreshCtx\.wsSlug, refreshCtx\.itemId\) \.then\($/, body: /./, why: 'force-refresh fetch: refreshGen against loadGeneration. Re-reviewed for TASK-2199: the enclosing onForceRefresh first captures the edits it discards (offlineRecovery), synchronously, before any await. Re-reviewed for BUG-3526: the capture reads provider.editsMayBeMissing instead of provider.unsentLocalEdits, still synchronous and before the fetch; the continuation and its fences are unchanged. Re-reviewed for BUG-3582: only the enclosing collab effect moved (onApplierRequest converts wiki-links before its setContent, synchronously, no await, no new write); this continuation is unchanged. Re-reviewed for PLAN-3578 step 2: only the enclosing collab effect moved again (onApplierPatch added to the provider options: a synchronous plan, the bracket callback and one dispatch; no await, no generation or identity write); this continuation is unchanged Re-reviewed for the PLAN-3578 retro fix (day 91): only the enclosing collab effect\'s onApplierPatch changed, building its one dispatch through patchTransaction (hunks) instead of one replaceWith; synchronous, no await, no generation or identity write; this continuation is unchanged. Re-reviewed for BUG-3697: the enclosing onForceRefresh now sets recoveryFrozen before the capture; this continuation writes nothing new.', reviewed: '118b4d7ad71c' },
+	{ call: /forceRefreshNonce \+= 1; \}\) \.catch\($/, body: /./, why: 'force-refresh failure: refreshGen against loadGeneration. Re-reviewed for TASK-2199 (the synchronous capture above it), and for BUG-3526 (only the input of that capture). Re-reviewed for BUG-3582: only the enclosing collab effect moved (the synchronous wiki-link conversion in the applier); this continuation is unchanged. Re-reviewed for PLAN-3578 step 2: the enclosing collab effect gained onApplierPatch (synchronous, no await, no generation write); this continuation is unchanged Re-reviewed for the PLAN-3578 retro fix (day 91): only the enclosing collab effect\'s onApplierPatch changed, building its one dispatch through patchTransaction (hunks) instead of one replaceWith; synchronous, no await, no generation or identity write; this continuation is unchanged. Re-reviewed for BUG-3697: only its comment changed (recoveryFrozen stays true on this arm, so the editor stays read-only); it writes nothing new.', reviewed: '299420d3bdfa' },
 	{
 		call: /^setTimeout\($/,
 		body: /teardownFlushed/,

web/src/lib/components/items/itemDetailOfflineEdits.test.ts

@@ -37,6 +37,18 @@ describe('TASK-2199: ItemDetail protects edits that have not reached the server'
 		expect(handler).toMatch(/if \(recovered\) offlineRecovery = recovered/);
 	});
 
+	it('BUG-3697: force_refresh freezes the editor before it captures, and only the rebuilt provider unfreezes it', () => {
+		const handler = between('onForceRefresh: () => {', 'void api.items');
+		const freeze = handler.indexOf('recoveryFrozen = true');
+		expect(freeze, 'onForceRefresh does not freeze the editor').toBeGreaterThan(-1);
+		expect(freeze, 'the freeze must come before the recovery capture').toBeLessThan(handler.indexOf('offlineRecoveryOnForceRefresh('));
+		// The refetch's failure arm must not unfreeze: the provider is gone.
+		const failure = between('.catch((err) => {', 'ydoc = doc;');
+		expect(failure).not.toMatch(/recoveryFrozen\s*=\s*false/);
+		expect(between('ydoc = doc;', 'return () => {')).toMatch(/recoveryFrozen = false/);
+		expect(CODE).toMatch(/editable=\{!peeking && !recoveryFrozen\}/);
+	});
+
 	it('in-app navigation asks leaveQuestion and cancels on a No, leaving tab close to beforeunload', () => {
 		const guard = between('beforeNavigate((nav) => {', '\n\t});');
 		expect(guard).toMatch(/if \(nav\.type === 'leave'\) return;/);