diff --git a/web/e2e/bug-3697-typing-during-recovery.spec.ts b/web/e2e/bug-3697-typing-during-recovery.spec.ts new file mode 100644 index 000000000..2ce844155 --- /dev/null +++ b/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((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(); +}); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index 8775dd719..fb6fe3b02 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/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 @@ { + 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;/);