fix: e2e CI flake — goal-0017 emit fanout raced canvas-live-sync (Standing #1) (#33)

Root cause, confirmed via real CI history (6/6 shard-1 failures, all
after goal 0017 merged, zero before) and local reproduction (9/20
clean-canvas repeats failed with zero artificial load): goal 0017 gave
every direct-mutation service its own dataevent.Emit call, so a single
MCP update_workflow write now fires mill-data-changed TWICE
(SnapshotDraft + UpdateWorkflow) plus a third echo from the test's own
prior UI-driven CreateWorkflow. None carry payload content, so each
handler independently refetches — three fetches racing meant whichever
resolved last won unconditionally, letting a stale response beat an
already-applied newer one and wrongly show the external-change banner
on a clean canvas.

Fixed in useCanvasLiveSync.ts with a monotonic request-sequence guard
(the standard out-of-order-async-response fix): drop a fetch response
once a newer mill-data-changed event has arrived since it was
dispatched. Verified: 88 consecutive clean local repeats post-fix vs.
9/20 before it, same build.

Also: canvas-live-sync.spec.ts's cleanup hardened into an outer
try/finally (both tests) so a future assertion failure can't leave an
undeleted workflow / unattended-MCP-writes settings for later tests in
the same worker — defense-in-depth for the observed cascade,
independent of the root-cause fix. resizable-table.spec.ts's one
occurrence (PR #24, drag-handle bounding box) hardened with a
condition-based expect.poll wait at the point of use, additive to the
suite's existing retries: 1 (goal 0024 precedent, untouched).

BACKLOG.md Standing #1 checked off with the full root-cause writeup;
SPEC.md's realtime-lock section gets an Update note generalizing the
lesson for future mill-data-changed consumers.


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

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: alicoding

frontend/e2e/canvas-live-sync.spec.ts

@@ -125,6 +125,26 @@ async function restoreMCPWriteDefaults(page: Page): Promise<void> {
   }
 }
 
+// Closes the open editor tab, deletes the named workflow row, and
+// restores the shared MCP-write settings -- shared by both tests below,
+// each wrapping this in an outer try/finally so it ALWAYS runs, even
+// when an assertion above throws. Before this, both tests only ran
+// their cleanup on the happy path: a failed assertion inside the MCP
+// round-trip (this file's own live race, fixed in useCanvasLiveSync.ts)
+// left "E2E live sync clean"/"dirty" undeleted AND unattended MCP
+// writes still enabled for the rest of this worker's shard -- traced
+// live as the mechanism behind a real cascade (this spec's own flake
+// tripping configure-lists.spec.ts's "list-search node" test in the
+// same shard-1 CI run, docs/goals/BACKLOG.md Standing #1). Kept as
+// cleanup hardening regardless of the race fix -- any OTHER future
+// assertion failure in either test would hit the exact same cascade
+// without it.
+async function cleanupWorkflow(page: Page, label: string): Promise<void> {
+  await page.getByRole('button', { name: 'Close tab' }).last().click()
+  await clickRowAction(page, workflowRow(page, label), 'Delete')
+  await restoreMCPWriteDefaults(page)
+}
+
 test('clean canvas: an external MCP update_workflow redraws the open editor live, no reload', async ({ page }, testInfo) => {
   await enableUnattendedMCPWrites(page)
 
@@ -138,24 +158,24 @@ test('clean canvas: an external MCP update_workflow redraws the open editor live
   await row.click()
   await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(1)
 
-  const client = await connectMCPClient(testInfo.parallelIndex)
   try {
-    const workflowId = await findWorkflowIdByLabel(client, 'E2E live sync clean')
-    await updateWorkflowViaMCP(client, workflowId, twoNodeDefinition('E2E live sync clean', 'clean-path marker'))
-
-    // No page.reload(), no re-navigation -- the redraw has to happen
-    // purely from the `mill-data-changed` event this canvas subscribed
-    // to (useCanvasLiveSync.ts).
-    await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(2, { timeout: 10_000 })
-    await expect(activePanel(page).locator('.react-flow__node').filter({ hasText: 'Process: Inject text' })).toBeVisible()
-    await expect(page.getByTestId('external-change-banner')).toHaveCount(0)
+    const client = await connectMCPClient(testInfo.parallelIndex)
+    try {
+      const workflowId = await findWorkflowIdByLabel(client, 'E2E live sync clean')
+      await updateWorkflowViaMCP(client, workflowId, twoNodeDefinition('E2E live sync clean', 'clean-path marker'))
+
+      // No page.reload(), no re-navigation -- the redraw has to happen
+      // purely from the `mill-data-changed` event this canvas subscribed
+      // to (useCanvasLiveSync.ts).
+      await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(2, { timeout: 10_000 })
+      await expect(activePanel(page).locator('.react-flow__node').filter({ hasText: 'Process: Inject text' })).toBeVisible()
+      await expect(page.getByTestId('external-change-banner')).toHaveCount(0)
+    } finally {
+      await client.close()
+    }
   } finally {
-    await client.close()
+    await cleanupWorkflow(page, 'E2E live sync clean')
   }
-
-  await page.getByRole('button', { name: 'Close tab' }).last().click()
-  await clickRowAction(page, workflowRow(page, 'E2E live sync clean'), 'Delete')
-  await restoreMCPWriteDefaults(page)
 })
 
 test('dirty canvas: external MCP edit shows a banner, keeps the local edit, and Reload applies the fresh definition', async ({ page }, testInfo) => {
@@ -180,32 +200,32 @@ test('dirty canvas: external MCP edit shows a banner, keeps the local edit, and
   await activePanel(page).getByTestId('toggle-description').click()
   await activePanel(page).getByLabel('Description').fill('local unsaved edit')
 
-  const client = await connectMCPClient(testInfo.parallelIndex)
   try {
-    const workflowId = await findWorkflowIdByLabel(client, 'E2E live sync dirty')
-    await updateWorkflowViaMCP(client, workflowId, twoNodeDefinition('E2E live sync dirty', 'dirty-path marker'))
-
-    const banner = page.getByTestId('external-change-banner')
-    await expect(banner).toBeVisible({ timeout: 10_000 })
-
-    // The local edit is untouched -- the external change was NOT
-    // applied automatically while dirty.
-    await expect(activePanel(page).getByLabel('Description')).toHaveValue('local unsaved edit')
-    await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(1)
-
-    await banner.getByRole('button', { name: 'Reload' }).click()
-
-    // Reload discards the local draft and loads the fresh (external)
-    // definition.
-    await expect(banner).toHaveCount(0)
-    await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(2)
-    await expect(activePanel(page).locator('.react-flow__node').filter({ hasText: 'Process: Inject text' })).toBeVisible()
-    await expect(activePanel(page).getByLabel('Description')).toHaveValue('')
+    const client = await connectMCPClient(testInfo.parallelIndex)
+    try {
+      const workflowId = await findWorkflowIdByLabel(client, 'E2E live sync dirty')
+      await updateWorkflowViaMCP(client, workflowId, twoNodeDefinition('E2E live sync dirty', 'dirty-path marker'))
+
+      const banner = page.getByTestId('external-change-banner')
+      await expect(banner).toBeVisible({ timeout: 10_000 })
+
+      // The local edit is untouched -- the external change was NOT
+      // applied automatically while dirty.
+      await expect(activePanel(page).getByLabel('Description')).toHaveValue('local unsaved edit')
+      await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(1)
+
+      await banner.getByRole('button', { name: 'Reload' }).click()
+
+      // Reload discards the local draft and loads the fresh (external)
+      // definition.
+      await expect(banner).toHaveCount(0)
+      await expect(activePanel(page).locator('.react-flow__node')).toHaveCount(2)
+      await expect(activePanel(page).locator('.react-flow__node').filter({ hasText: 'Process: Inject text' })).toBeVisible()
+      await expect(activePanel(page).getByLabel('Description')).toHaveValue('')
+    } finally {
+      await client.close()
+    }
   } finally {
-    await client.close()
+    await cleanupWorkflow(page, 'E2E live sync dirty')
   }
-
-  await page.getByRole('button', { name: 'Close tab' }).last().click()
-  await clickRowAction(page, workflowRow(page, 'E2E live sync dirty'), 'Delete')
-  await restoreMCPWriteDefaults(page)
 })

frontend/e2e/resizable-table.spec.ts

@@ -26,6 +26,17 @@ test('Table columns are drag-resizable and long cells truncate with a hover titl
   const firstTrack = () =>
     table.evaluate((t) => parseFloat(getComputedStyle(t).gridTemplateColumns.split(' ')[0]))
   const before = await firstTrack()
+  // Condition-based wait, not a fixed-window one (docs/goals/BACKLOG.md
+  // Standing #1's CI flake investigation, 2026-08-12): a bare
+  // `boundingBox()` read right after the table becomes visible can still
+  // race the grid's own column-width layout pass under a loaded runner,
+  // occasionally returning null (PR #24's real flake -- the whole
+  // test's global `retries: 1`, playwright.config.ts, already masked
+  // it once; this polls for a stable box directly at the point of use
+  // instead of leaning on a full-test rerun to paper over layout timing).
+  await expect
+    .poll(() => handles.first().boundingBox().then((b) => b !== null), { timeout: 5_000 })
+    .toBe(true)
   const box = await handles.first().boundingBox()
   if (!box) throw new Error('resize handle has no bounding box')
   const x = box.x + box.width / 2

frontend/src/composition/useCanvasLiveSync.ts

@@ -100,14 +100,56 @@ export function useCanvasLiveSync(args: UseCanvasLiveSyncArgs): UseCanvasLiveSyn
     draftDescriptionRef.current = draftDescription
   }, [draftDescription])
 
+  // Real bug found via CI flake investigation (docs/goals/BACKLOG.md
+  // Standing #1, 2026-08-12): goal 0017 gave every direct-mutation
+  // service its own `dataevent.Emit("workflow", id)` call -- but for a
+  // single MCP `update_workflow` write, that now fires the SAME
+  // `mill-data-changed` event TWICE: once from `SnapshotDraft`
+  // (compositionservice_versioning.go's `mutateWorkflow`, archiving the
+  // draft before the edit lands) and once from `UpdateWorkflow` itself
+  // (compositionservice.go) -- plus a THIRD, from this canvas's own
+  // `CreateWorkflow` moments earlier (canvas-live-sync.spec.ts's "clean
+  // canvas" test creates the workflow via the UI first), which can
+  // still be in flight when the canvas mounts and subscribes. All three
+  // carry no content of their own -- each handler independently calls
+  // CompositionService.Workflows() to refetch -- so three near-
+  // simultaneous events dispatch three independent fetches whose
+  // RESPONSES can resolve in a different order than they were
+  // dispatched. Before this fix, whichever resolved LAST won
+  // unconditionally, so a stale response could win the
+  // decideExternalSyncAction comparison against a baseline a different,
+  // already-applied response had advanced past, wrongly deciding
+  // "prompt" and showing the external-change banner on a genuinely
+  // clean canvas -- confirmed locally (traced via a temporary
+  // arrival/resolution/decision log): 9/20 repeats of the "clean
+  // canvas" test failed on exactly this assertion with zero artificial
+  // load, matching 6/6 real CI failures found in the last ~30 CI runs
+  // (canvas-live-sync.spec.ts:151, all after goal 0017 merged, zero
+  // occurrences before). requestSeqRef is the standard fix for
+  // out-of-order async responses: each event bumps the counter at
+  // ARRIVAL time (not resolution time), and a response is dropped
+  // unless it's still the most recently dispatched one when it resolves
+  // -- correct regardless of how many of these events fire in a burst
+  // or which of their fetches happens to resolve first. Verified fixed:
+  // 88 consecutive clean local runs (0 failures) after this change, vs.
+  // 9/20 before it, same build.
+  const requestSeqRef = useRef(0)
+
   useEffect(() => {
     if (!workflowId) return
     return Events.On('mill-data-changed', (evt) => {
       const data = evt.data as { entity?: string; id?: string }
       if (data?.entity !== 'workflow' || data?.id !== workflowId) return
+      const seq = ++requestSeqRef.current
       CompositionService.Workflows()
         .then((all) => {
-          const fresh = (all ?? []).find((w) => w.ID === workflowId)
+          // A newer mill-data-changed event for this same workflow has
+          // already arrived and dispatched its own fetch since this one
+          // started -- this response is now stale (see the header
+          // comment above); applying or even just deciding on it would
+          // race the newer one. Drop it.
+          if (seq !== requestSeqRef.current) return
+          const fresh = (all ?? []).find((wf) => wf.ID === workflowId)
           // A concurrent external delete is out of this feature's scope
           // -- WorkTabShell's own once-lists-load tab-pruning handles a
           // since-deleted entity's open tab separately.