diff --git a/scripts/add-defender-exclusion.ps1 b/backend/scripts/add-defender-exclusion.ps1 similarity index 100% rename from scripts/add-defender-exclusion.ps1 rename to backend/scripts/add-defender-exclusion.ps1 diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts index 0425b228..23a8b9c6 100644 --- a/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts @@ -5,6 +5,7 @@ import { setCanvasInteractionActive } from '@/shared/canvasInteractionState'; import { getLastInteractedBrowser } from '@/shared/browserFocus'; import { getScrollFocusedCard } from '@/shared/cardScrollFocus'; import { APP_WINDOW_SELECTOR, CANVAS_OWNER, heldBy, WheelGesture } from './wheelGestureOwner'; +import { markInteraction } from '@/shared/interactionPriority'; import { getWebview } from '@/shared/browserRegistry'; import { applyBrowserZoom } from '@/shared/browserZoom'; import { syncTiledGeometry } from '../../canvas/tiledGeometry'; @@ -435,6 +436,12 @@ export function useCanvasControls( gesture.at = Date.now(); } + // The canvas is handling this wheel: every "someone else owns it" branch above has already + // returned, so this is the one point where a pan or a zoom is committed to. Streaming yields + // to it (ENG-301); a wheel that scrolled a transcript never reaches here and never stalls the + // answer the user is reading. + markInteraction(); + e.preventDefault(); if (inertiaFrameRef.current) { cancelAnimationFrame(inertiaFrameRef.current); diff --git a/frontend/src/shared/interactionPriority.test.ts b/frontend/src/shared/interactionPriority.test.ts index 160ffa50..486b89a1 100644 --- a/frontend/src/shared/interactionPriority.test.ts +++ b/frontend/src/shared/interactionPriority.test.ts @@ -1,7 +1,8 @@ // ENG-301: mid-gesture stream deltas must yield to the hand. Pins the decay contract. import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { markInteraction, interactionActive } from './interactionPriority'; +import { readFileSync } from 'node:fs'; +import { markInteraction, interactionActive, installInteractionListeners } from './interactionPriority'; test('quiet by default', () => { assert.equal(interactionActive(), false); @@ -13,3 +14,39 @@ test('active immediately after a gesture, decays after 350ms', async () => { await new Promise((r) => setTimeout(r, 400)); assert.equal(interactionActive(), false); }); + +// The bug this file caused: a blanket capture-phase wheel listener marked EVERY wheel, so scrolling +// the transcript of the answer you were reading paused that answer and then dumped it in one burst +// ("streams halfway, stops, then re-streams everything super fast"). The canvas owns that decision +// now and calls markInteraction() itself; nothing here may listen for wheel again. +test('installing listeners does NOT claim wheel: a transcript scroll must never stall the stream', () => { + const seen: string[] = []; + const g = globalThis as { window?: unknown }; + const realWindow = g.window; + g.window = { addEventListener: (type: string) => { seen.push(type); } }; + try { + installInteractionListeners(); + } finally { + g.window = realWindow; + } + assert.ok(seen.length > 0, 'the install must actually have run (it latches after the first call)'); + assert.equal(seen.includes('wheel'), false, 'wheel must be claimed by the canvas, not globally'); + assert.equal(seen.includes('pointerdown'), true, 'a card drag must still suppress streaming'); + assert.equal(seen.includes('pointermove'), true); +}); + +test('the canvas is the one that marks a wheel, at the point it commits to handling it', () => { + // Resolved from cwd, not from import.meta.url: the runner bundles tests into .test-build/, so a + // URL relative to the bundle points at a directory that holds no sources. + const src = readFileSync( + 'src/app/pages/Dashboard/hooks/interaction/useCanvasControls.ts', + 'utf8', + ); + const mark = src.indexOf('markInteraction();'); + assert.ok(mark > 0, 'the canvas must mark its own gesture'); + // Ordering, not just presence: every "another surface owns this wheel" branch returns above the + // mark, which is exactly what leaves a transcript scroll unmarked. + assert.ok(src.indexOf('gesture.owner = windowEl;') < mark, 'app-window branch returns before the mark'); + assert.ok(src.indexOf('gesture.owner = CANVAS_OWNER;') < mark, 'the canvas claim precedes the mark'); + assert.ok(src.indexOf('e.preventDefault();', mark) - mark < 200, 'the mark sits with the canvas handling'); +}); diff --git a/frontend/src/shared/interactionPriority.ts b/frontend/src/shared/interactionPriority.ts index bff40b5c..e99ab287 100644 --- a/frontend/src/shared/interactionPriority.ts +++ b/frontend/src/shared/interactionPriority.ts @@ -1,8 +1,17 @@ -// While the user is mid-gesture (dragging a card, wheeling the canvas), streamed tokens must not -// steal frames: each delta lands as a React commit, and under several live agents those commits -// burst to 50-66ms right through the drag (measured on exp.13, the "still feels glitchy" report). -// Capture-phase listeners keep a decaying "interacting" stamp; stream dispatch consults it and -// falls back to the 1Hz buffer during the gesture. Smoothness beats token immediacy for ~a second. +// While the user is mid-gesture (dragging a card, panning or zooming the canvas), streamed tokens +// must not steal frames: each delta lands as a React commit, and under several live agents those +// commits burst to 50-66ms right through the drag (measured on exp.13, the "still feels glitchy" +// report). A decaying "interacting" stamp is kept here; stream dispatch consults it and falls back +// to the buffer during the gesture. Smoothness beats token immediacy for ~a second. +// +// WHEEL IS NOT MARKED HERE, and that is the point. A blanket capture-phase wheel listener marks +// EVERY wheel in the app, including the one thing a user does most while an answer streams: scroll +// the transcript they are reading. That paused the stream for as long as their hand kept moving and +// then dumped the backlog in one burst, which is the "streams halfway, stops, then re-streams +// everything super fast" report. The canvas already decides who owns a wheel (wheelGestureOwner + +// useCanvasControls) and calls markInteraction() at the single point where it commits to handling +// one, so a pan and a zoom still suppress streaming and a transcript scroll never does. One owner, +// one decision, no second copy of the rule. const DECAY_MS = 350; // -Infinity, not 0: performance.now() is near 0 at process start, so 0 would read as 'mid-gesture' for the app's first 350ms. let p_lastInteraction = Number.NEGATIVE_INFINITY; @@ -19,7 +28,6 @@ let p_installed = false; export function installInteractionListeners(): void { if (p_installed || typeof window === 'undefined') return; p_installed = true; - window.addEventListener('wheel', markInteraction, { capture: true, passive: true }); window.addEventListener('pointerdown', markInteraction, { capture: true, passive: true }); window.addEventListener('pointermove', (e: PointerEvent) => { if (e.buttons) markInteraction(); }, { capture: true, passive: true }); } diff --git a/frontend/src/shared/ws/BackgroundDeltaBuffer.test.ts b/frontend/src/shared/ws/BackgroundDeltaBuffer.test.ts index 3f3a5fed..ed1499af 100644 --- a/frontend/src/shared/ws/BackgroundDeltaBuffer.test.ts +++ b/frontend/src/shared/ws/BackgroundDeltaBuffer.test.ts @@ -3,6 +3,7 @@ // interleave them wrong on flush. import { test } from 'node:test'; import assert from 'node:assert'; +import { readFileSync } from 'node:fs'; import { BackgroundDeltaBuffer } from './BackgroundDeltaBuffer'; test('same-message deltas coalesce into one payload, byte-exact', () => { @@ -41,3 +42,18 @@ test('nothing is lost across an evict-then-take sequence (byte accounting)', () if (last) seen.push(last.text); assert.strictEqual(seen.join(''), '12345'); }); + +// The hold ceiling (WebSocketManager.armBgFlush). A gesture that keeps going keeps re-arming the +// flush, so without a ceiling a long canvas pan holds the answer for as long as the hand moves and +// then dumps the whole backlog at once. Asserted on the source because the timer lives inside the +// manager (which needs a live store and socket); the decision itself is what a regression breaks. +test('a live gesture cannot hold streamed text forever', () => { + const src = readFileSync('src/shared/ws/WebSocketManager.ts', 'utf8'); + assert.match(src, /BG_MAX_HOLD_MS\s*=\s*\d+/, 'a ceiling must exist'); + const arm = src.slice(src.indexOf('private armBgFlush()'), src.indexOf('private flushBgDelta()')); + assert.match(arm, /heldTooLong/, 'the re-arm branch must consult the ceiling'); + assert.match(arm, /interactionActive\(\)\s*&&\s*!heldTooLong/, + 'past the ceiling the stream wins even while the hand is still moving'); + const flush = src.slice(src.indexOf('private flushBgDelta()')); + assert.match(flush, /bgHoldSince = null/, 'the hold clock must reset on every flush'); +}); diff --git a/frontend/src/shared/ws/WebSocketManager.ts b/frontend/src/shared/ws/WebSocketManager.ts index 94f2ce9d..d46bb3aa 100644 --- a/frontend/src/shared/ws/WebSocketManager.ts +++ b/frontend/src/shared/ws/WebSocketManager.ts @@ -50,6 +50,9 @@ import { notifyAgentCompletion, notifyWorkflowRun } from '../notifications'; let firstAgentResponseMarked = false; installInteractionListeners(); +// Ceiling on how long a live gesture may hold streamed text before the stream gets a frame anyway. +const BG_MAX_HOLD_MS = 1000; + // Thin wrapper around getAuthToken so the connect() call site stays synchronous. If the token isn't cached yet, returns '' and the WS handshake will 4401, onclose catches that and refreshes the token before the next reconnect. const _getAuthTokenSafe = (): string => { try { return getAuthToken() || ''; } catch { return ''; } @@ -94,6 +97,8 @@ class WebSocketManager { private skipStreamEvents: boolean; private backgrounded = false; private bgBuffer = new BackgroundDeltaBuffer(); + // When the current buffered run started, so a gesture cannot hold a stream forever (see armBgFlush). + private bgHoldSince: number | null = null; private bgFlushTimer: ReturnType | null = null; private sessionId: string | null; @@ -206,11 +211,17 @@ class WebSocketManager { private armBgFlush() { if (this.bgFlushTimer !== null) return; + if (this.bgHoldSince === null) this.bgHoldSince = Date.now(); // Gesture-buffered deltas flush fast (250ms) once the hand stops; true background stays 1Hz. const delay = this.backgrounded ? 1000 : 250; this.bgFlushTimer = setTimeout(() => { this.bgFlushTimer = null; - if (!this.backgrounded && interactionActive()) { this.armBgFlush(); return; } + // A gesture that keeps going keeps re-arming, so without a ceiling a long pan can hold an + // answer for as long as the hand moves and then dump it in one burst. Past the ceiling the + // stream wins and gets its frame; 1Hz mid-gesture is the rate the backgrounded case already + // treats as acceptable, so this cannot be worse than what a hidden chat already pays. + const heldTooLong = this.bgHoldSince !== null && Date.now() - this.bgHoldSince >= BG_MAX_HOLD_MS; + if (!this.backgrounded && interactionActive() && !heldTooLong) { this.armBgFlush(); return; } this.flushBgDelta(); }, delay); } @@ -220,6 +231,7 @@ class WebSocketManager { clearTimeout(this.bgFlushTimer); this.bgFlushTimer = null; } + this.bgHoldSince = null; const p = this.bgBuffer.take(); if (p && this.sessionId) { store.dispatch(streamDelta({ sessionId: this.sessionId, messageId: p.messageId, delta: p.text }));