From 275efc17beabf379a16c68ff6a196300a8db28a6 Mon Sep 17 00:00:00 2001 From: xarmian Date: Tue, 22 Sep 2026 16:56:44 -0400 Subject: [PATCH] fix(web): a starting copy preview cancels the debounce it already answers (BUG-3151) (#1440) * fix(web): a starting copy preview cancels the debounce it already answers (BUG-3151) copy-dialog.spec.ts' coalescing test failed in 2 of 3 full desktop runs (4 preflights instead of 3) and passed in isolation. The CI traces show the 4th request's body is byte-identical to the 3rd's. The runner reads the current state when a run starts, so that run answers every edit made before it, including one whose debounce timer is still pending. When edits are spaced wider than the 250ms debounce while a preview is in flight, the earlier edit's timer queues the trailing run. The last edit's own timer is then still pending when that trailing run starts, and it fires afterwards to send the same body again. Suite load is what spaced the test's picks that wide; a 300ms gap reproduces it with no load, every time. runPreflight now cancels any pending debounce timer when it starts a run or queues the trailing one. The spec runs the collapse test at two pick spacings (0ms and 350ms). It waits out the debounce before counting, because the duplicate arrives after the point where the count used to be read. * test(e2e): derive the copy-dialog collapse spec's timings from the shared debounce constant (BUG-3151, lead condition) --- web/e2e/copy-dialog.spec.ts | 14 +++++++++++++- web/src/lib/components/items/CopyItemDialog.svelte | 11 ++++++++++- web/src/lib/items/copyPreflightTiming.ts | 4 ++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 web/src/lib/items/copyPreflightTiming.ts diff --git a/web/e2e/copy-dialog.spec.ts b/web/e2e/copy-dialog.spec.ts index 829719ae2..bdde381b3 100644 --- a/web/e2e/copy-dialog.spec.ts +++ b/web/e2e/copy-dialog.spec.ts @@ -2,6 +2,7 @@ import { test, expect } from './fixtures'; import { browserLogin, seedDoc } from './lib/collab-helpers'; import type { APIRequestContext, Locator, Page } from '@playwright/test'; import type { SuiteFixture } from './fixtures'; +import { COPY_PREFLIGHT_DEBOUNCE_MS } from '../src/lib/items/copyPreflightTiming'; /** * Cross-workspace copy / move dialog (PLAN-2373 / TASK-2355). @@ -601,7 +602,13 @@ test.describe('cross-workspace copy dialog (PLAN-2373 / TASK-2355)', () => { }); - test('override edits collapse to one in-flight preflight plus one trailing run', async ({ + // Two spacings (BUG-3151). Picks faster than the debounce + // (COPY_PREFLIGHT_DEBOUNCE_MS, CopyItemDialog's runner) collapse in + // the timer; picks SLOWER than it each fire a run, which queues the trailing + // run while #2 is held — and the last pick's own timer is then still pending + // when that trailing run starts. It used to fire afterwards with an identical + // body (4 requests), which only full-suite load made slow enough to happen. + for (const gap of [0, COPY_PREFLIGHT_DEBOUNCE_MS + 100]) test(`override edits collapse to one in-flight preflight plus one trailing run (picks ${gap}ms apart)`, async ({ page, fixture, request, @@ -657,11 +664,16 @@ test.describe('cross-workspace copy dialog (PLAN-2373 / TASK-2355)', () => { await expect.poll(() => started).toBe(2); await pick(1); + if (gap) await page.waitForTimeout(gap); await pick(2); // Single-flight: neither pick issued a request of its own. expect(started).toBe(2); releaseSecond(); await expect(dialog.getByRole('button', { name: 'Copy', exact: true })).toBeEnabled(); + // Outlast any debounce still pending, so a late duplicate is counted + // here rather than after the assertion (BUG-3151: this count used to be + // read before the stray timer fired, except under load). + await page.waitForTimeout(2 * COPY_PREFLIGHT_DEBOUNCE_MS + 100); // …and both were served by ONE trailing run, not one request each. expect(started).toBe(3); }); diff --git a/web/src/lib/components/items/CopyItemDialog.svelte b/web/src/lib/components/items/CopyItemDialog.svelte index a27e6aeb1..4f636565c 100644 --- a/web/src/lib/components/items/CopyItemDialog.svelte +++ b/web/src/lib/components/items/CopyItemDialog.svelte @@ -43,6 +43,7 @@ their own recovery paths; swallowing those into outcome-unknown would send the user hunting for an item that provably does not exist. -->