From 5c5f4b7eacd71d3a04baaaa2f7c33c1a8a7362fa Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 6 Apr 2026 16:23:14 +0100 Subject: [PATCH] fix: flush pending changes when isPendingRevision clears after reconnect The existing fix in setChannelState('CONNECTED') calls handleUserChanges(), but at that point isPendingRevision is still true so changes are blocked. The real trigger must be in setIsPendingRevision(): when it transitions from true to false (after all CLIENT_RECONNECT messages are processed), call handleUserChanges() to flush any locally-queued edits. Also adds a targeted regression test that simulates the exact reconnect state transitions and verifies pending edits reach the server. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/static/js/collab_client.ts | 8 +++ .../specs/reconnect_flush.spec.ts | 53 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/static/js/collab_client.ts b/src/static/js/collab_client.ts index 50271db38..bf9576b45 100644 --- a/src/static/js/collab_client.ts +++ b/src/static/js/collab_client.ts @@ -442,7 +442,15 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad) }; const setIsPendingRevision = (value) => { + const wasPending = isPendingRevision; isPendingRevision = value; + // After reconnect, once all pending revisions from the server have been applied + // (isPendingRevision transitions from true to false), flush any unsent local changes + // that were queued while disconnected. The handleUserChanges() call in setChannelState + // (CONNECTED) is not sufficient because isPendingRevision is still true at that point. + if (wasPending && !value) { + handleUserChanges(); + } }; const idleFuncs = []; diff --git a/src/tests/frontend-new/specs/reconnect_flush.spec.ts b/src/tests/frontend-new/specs/reconnect_flush.spec.ts index 39f835bd0..3516f812f 100644 --- a/src/tests/frontend-new/specs/reconnect_flush.spec.ts +++ b/src/tests/frontend-new/specs/reconnect_flush.spec.ts @@ -33,12 +33,61 @@ test.describe('Pending changeset flush after reconnect', function () { // Reconnect User 1 by going back online. await page1.context().setOffline(false); - // The fix ensures handleUserChanges() is called in setUpSocket() after - // reconnection, so pending changes should be flushed automatically. + // The fix ensures handleUserChanges() is called when setIsPendingRevision + // transitions from true to false after reconnection, so pending changes + // should be flushed automatically. // Verify User 2 receives the offline edit. await expect(body2.locator('div').first()).toHaveText('initial text and offline edit'); await context1.close(); await context2.close(); }); + + test('bug #5108 regression: handleUserChanges is called when isPendingRevision clears', async function ({page}) { + // This test verifies the specific codepath: after reconnect, when the server + // finishes sending CLIENT_RECONNECT messages and setIsPendingRevision(false) is + // called, handleUserChanges() must be triggered to flush locally-queued edits. + // + // The bug was that setChannelState('CONNECTED') fires handleUserChanges() but at + // that point isPendingRevision is still true, so the changes are not sent. Only + // after all CLIENT_RECONNECT messages arrive does isPendingRevision become false, + // and nothing was calling handleUserChanges() at that point. + + const padId = await goToNewPad(page); + await clearPadContent(page); + await writeToPad(page, 'initial content'); + + const body = await getPadBody(page); + await expect(body.locator('div').first()).toHaveText('initial content'); + + // Step 1: Simulate the reconnecting state (as pad.ts socketReconnecting does). + await page.evaluate(() => { + const pad = (window as any).pad; + pad.collabClient.setStateIdle(); + pad.collabClient.setIsPendingRevision(true); + pad.collabClient.setChannelState('RECONNECTING'); + }); + + // Step 2: Type text while in "reconnecting" state (locally queued, not sent). + await page.keyboard.down('Control'); + await page.keyboard.press('End'); + await page.keyboard.up('Control'); + await page.keyboard.press('Enter'); + await page.keyboard.type('typed during reconnect'); + + // Step 3: Simulate reconnect completion. + // First set CONNECTED (handleUserChanges fires but bails because isPendingRevision is true). + // Then clear isPendingRevision, which should now trigger handleUserChanges() via the fix. + await page.evaluate(() => { + const pad = (window as any).pad; + pad.collabClient.setChannelState('CONNECTED'); + pad.collabClient.setIsPendingRevision(false); + }); + + // Step 4: Open a second view and verify the locally-typed text was flushed to the server. + const page2 = await page.context().newPage(); + await goToPad(page2, padId); + const body2 = await getPadBody(page2); + await expect(body2.locator('div').nth(1)).toHaveText('typed during reconnect', {timeout: 10000}); + }); });