diff --git a/CHANGELOG.md b/CHANGELOG.md index 46d748d08..564d2a81b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ notes. Entries describe what users experience, not internal refactors. ### Fixed +- Local references attached while saved chat history is loading remain attached + when the conversation finishes restoring. - Reloading after a completed save restores the saved document instead of an older recovery snapshot or the original file. - Opening a copied editor tab no longer removes the original tab's reload diff --git a/docs/explorations/2026-10-20-nightly-interaction-stability.md b/docs/explorations/2026-10-20-nightly-interaction-stability.md new file mode 100644 index 000000000..8e92ec497 --- /dev/null +++ b/docs/explorations/2026-10-20-nightly-interaction-stability.md @@ -0,0 +1,94 @@ +# Nightly interaction stability + +The cross-browser run for PR #287 (`39249862820`) passed after retrying three +cases. This follow-up treats those retries as evidence to investigate, rather +than increasing their timeouts or retry budgets. PR #287 was merged before +starting this branch, against main at `6525b804`. + +## Findings + +### Sources added during history restoration disappeared + +Firefox's source-reference case found the attachment row, filled its search +field, and then lost the row while trying to open its disclosure. The trace +shows a permanent disappearance, not a temporarily moving click target. + +`ReferenceSources.forget()` cleared the conversation maps while leaving the +active entries reference detached from those maps. An attachment added during +the asynchronous startup restore went into that detached map. Selecting the +same conversation after restoration created an empty map and lost the new +attachment. The correction belongs to reference ownership, rather than a +longer click timeout or a different locator. + +### Historical transcript setup raced native scroll delivery + +WebKit's composer case set `scrollTop` to zero and immediately changed layout. +Reading back zero did not establish that the native scroll event had run and +updated the transcript's following state. Subsequent composer resizing could +therefore still follow the latest message. Another nightly run (`39338214745`) +showed the same failure for PDF, rather than DOCX. + +The test needs to establish a historical reader through an actual interaction +and its visible processed-scroll signal before testing layout preservation. +Following new content and preserving a detached reader remain separate cases. + +### Firefox canvas clicks preceded keyboard-capture focus + +The save-overlap case failed before installing its held-conversion fault: +the first document text read remained empty after inserting `Recovery version`. +The SDK defers its input-area focus through a zero-delay task on Firefox. +A completed canvas click therefore does not establish that trusted text input +will reach the document. Save setup must focus the real SDK input area and +verify it is focused and editable, while retaining the exact document-content, +disk-content and recovery-ownership assertions. + +## Boundaries and acceptance + +- Keep source ownership scoped to the selected conversation. Clearing existing + references must still invalidate captured requests; switching conversations + must never share attachments. +- Preserve the production chat following and layout behavior. Synchronize the + E2E reader setup with actual interaction completion. +- Preserve trusted browser input and the real editor/save/conversion path. + Do not patch the vendor SDK for a test preparation race. +- Do not increase timeouts, retries, error allowlists or skip affected engines. +- Reverse each regression against the old behavior, run focused cross-browser + checks without retries, then run the PR gates and complete nightly browser + matrix with the slow-network budgets. + +## Reverse validation + +- With the old reference ownership implementation, both parameterized unit + cases (`forget()` and forgetting the selected conversation) lose `New notes` + when the same conversation is selected again. The browser regression holds + initial history loading until after attach/search; releasing it loses the + attachment row before the fix. +- With only the visible processed-scroll gate removed, the retained delayed + native-scroll scenario fails twice in WebKit: the historical offset becomes + `1449` instead of `0` after resizing. The corrected setup uses native Home + navigation and waits for the visible jump-to-latest control before resizing. +- With only explicit SDK input focus removed from the save setup helper, + holding Firefox's deferred focus task leaves the exact document text empty + instead of `Recovery version`. The regression verifies that the task was + intercepted and the canvas still owned focus before attempting trusted input. + +These controls delay the relevant event or storage boundary, not the assertion +deadline. The ordinary retry policy and all existing timeouts remain unchanged. + +## Local verification + +- Lint and TypeScript checks passed. The full format check also verified all + 1511 immutable evaluation artifacts. +- Full coverage run: 252 test files, 6801 tests passed. Statements 87.79%, + branches 83.94%, functions 88.77%, lines 90.25%; module thresholds passed. +- Source unit suites: 20 passed. The gated source scenario passed twice each in + Firefox and WebKit, without retries. The complete sources/artifacts spec + passed all 8 cases in Chromium. +- Firefox's complete save spec passed all 12 cases. The final deferred-focus + regression and held-recovery overlap passed together; the original overlap + also passed in Chromium. +- Composer layout/navigation passed all 24 cases across four formats, three + repetitions and WebKit/Firefox. The delayed-scroll fault passed twice per + engine and once in Chromium, with retries disabled. + +Remote checks are recorded on the follow-up PR for its exact submitted head. diff --git a/lib/agent-plugin/references/reference-sources.ts b/lib/agent-plugin/references/reference-sources.ts index 5ce91391b..6ea27850e 100644 --- a/lib/agent-plugin/references/reference-sources.ts +++ b/lib/agent-plugin/references/reference-sources.ts @@ -40,12 +40,14 @@ export class ReferenceSources { if (id !== undefined) { const entries = this.conversations.get(id); entries?.clear(); - this.conversations.delete(id); - if (entries === this.entries) this.entries = new Map(); + if (entries !== this.entries) this.conversations.delete(id); } else { this.entries.clear(); - for (const entries of this.conversations.values()) entries.clear(); - this.conversations.clear(); + for (const [conversation, entries] of this.conversations) { + entries.clear(); + // A source attached while history restores still belongs to the selected conversation. + if (entries !== this.entries) this.conversations.delete(conversation); + } } } capture() { diff --git a/test/e2e/agent-composer-ux.spec.ts b/test/e2e/agent-composer-ux.spec.ts index 6cc770832..a637ac080 100644 --- a/test/e2e/agent-composer-ux.spec.ts +++ b/test/e2e/agent-composer-ux.spec.ts @@ -144,9 +144,31 @@ for (const ext of ['docx', 'xlsx', 'pptx', 'pdf'] as const) { await expect.poll(() => input.evaluate((el) => el.clientHeight)).toBeLessThan(narrowHeight); await expect(input).toHaveValue(draft); await expect.poll(() => log.evaluate((el) => el.scrollHeight - el.scrollTop - el.clientHeight)).toBeLessThan(3); - await log.evaluate((el) => { - el.scrollTop = 0; - }); + if (ext === 'docx') { + // Inject delayed native scroll delivery so the readiness gate is exercised + // deterministically. This schedules a browser fault, not a readiness wait. + await log.evaluate((el) => { + let scheduled = false; + const holdScroll = (event: Event) => { + event.stopImmediatePropagation(); + if (el.scrollTop !== 0 || scheduled) return; + scheduled = true; + setTimeout(() => { + el.removeEventListener('scroll', holdScroll, true); + el.dispatchEvent(new Event('scroll')); + }, 500); + }; + el.addEventListener('scroll', holdScroll, true); + }); + } + const scrollLatest = page.getByRole('button', { name: 'Scroll to latest' }); + await expect(scrollLatest).toBeHidden(); + await log.focus(); + await log.press('Home'); + // The offset can change before the native scroll event detaches the reader. + // Wait for its visible result before a resize can change transcript geometry. + await expect(scrollLatest).toBeVisible(); + await expect.poll(() => log.evaluate((el) => el.scrollTop)).toBe(0); await resize.press('Home'); await expect.poll(() => log.evaluate((el) => el.scrollTop)).toBe(0); await input.fill(draft + draft); diff --git a/test/e2e/agent-sources-artifacts.spec.ts b/test/e2e/agent-sources-artifacts.spec.ts index 05d380470..1d7f04494 100644 --- a/test/e2e/agent-sources-artifacts.spec.ts +++ b/test/e2e/agent-sources-artifacts.spec.ts @@ -92,6 +92,36 @@ test('attached reference excerpts reach only the explicitly selected AI service' await page.addInitScript(() => { localStorage.setItem('document-assistant-enabled', 'true'); localStorage.setItem('agent_conversation_saving_v2', 'true'); + // Hold startup hydration until a source has been attached and searched. + // A normal reload below must restore the captured citation without a gate. + if (!sessionStorage.getItem('source-test-history-released')) { + const open = IDBFactory.prototype.open; + const success = Object.getOwnPropertyDescriptor(IDBRequest.prototype, 'onsuccess')!; + const host = window as unknown as { releaseSourceHistory?: () => void }; + let delayed = false; + IDBFactory.prototype.open = function (...args: Parameters) { + const request = open.apply(this, args); + if (args[0] === 'document-ai-conversations' && !delayed) { + delayed = true; + Object.defineProperty(request, 'onsuccess', { + set(callback: ((this: IDBRequest, event: Event) => void) | null) { + success.set!.call( + request, + callback && + function (event: Event) { + host.releaseSourceHistory = () => { + sessionStorage.setItem('source-test-history-released', 'true'); + callback.call(request, event); + }; + }, + ); + }, + get: () => success.get!.call(request), + }); + } + return request; + }; + } localStorage.setItem( 'agent-writing-endpoint', JSON.stringify({ @@ -133,6 +163,12 @@ test('attached reference excerpts reach only the explicitly selected AI service' await expect(page.locator('.agent-source-row')).toContainText('notes.txt'); const sourceSearch = page.getByRole('searchbox', { name: 'Search sources', exact: true }); await sourceSearch.fill('only this'); + await page.waitForFunction( + () => typeof (window as unknown as { releaseSourceHistory?: unknown }).releaseSourceHistory === 'function', + ); + await page.evaluate(() => (window as unknown as { releaseSourceHistory: () => void }).releaseSourceHistory()); + await expect(page.locator('.agent-history-status')).toHaveText('Chat history saving is enabled on this device.'); + await expect(page.locator('.agent-source-row')).toContainText('notes.txt'); await page.locator('.agent-source-row summary').click(); await expect(page.locator('.agent-source-preview mark')).toHaveText('Only this'); await page.locator('.agent-source-row summary').click(); diff --git a/test/e2e/save-to-file.spec.ts b/test/e2e/save-to-file.spec.ts index 2d06e833b..b827d72f9 100644 --- a/test/e2e/save-to-file.spec.ts +++ b/test/e2e/save-to-file.spec.ts @@ -133,6 +133,32 @@ const waitForEditorReady = (page: Page) => { timeout: 90_000 }, ); +/** A canvas click can precede Firefox's deferred keyboard-capture focus task. */ +const focusDocumentInput = async (page: Page) => { + const input = page.frameLocator('iframe[name="frameEditor"]').locator('#area_id'); + await input.focus(); + await expect(input).toBeFocused(); + await expect(input).toBeEditable(); +}; + +/** Seed save scenarios through trusted input after the canvas has selected a caret. */ +const insertDocumentText = async (page: Page, text: string) => { + await focusDocumentInput(page); + await page.keyboard.insertText(text); +}; + +const documentText = (page: Page) => + page.evaluate(() => { + const win = window.__ooFrames.readyEditor() as any; + const logic = (win.Asc?.editor ?? win.editor).WordControl.m_oLogicDocument; + const paragraphs: any[] = []; + for (const element of logic.Content) element.GetAllParagraphs({ All: true }, paragraphs); + return paragraphs + .map((paragraph) => paragraph.GetText({ Numbering: false })) + .join('') + .trim(); + }); + test.describe('saving into the document own file (real editor)', () => { test.describe.configure({ timeout: 240_000 }); @@ -267,26 +293,56 @@ test.describe('saving into the document own file (real editor)', () => { expect(await savedText(page)).toMatch(/first version second version/i); }); + test('save scenario input reaches the document before deferred Firefox focus runs', async ({ page, browserName }) => { + test.skip(browserName !== 'firefox', 'the SDK defers keyboard-capture focus only in Firefox'); + await page.goto('/editor?new=docx'); + await waitForEditorReady(page); + const sdk = page.frameLocator('iframe[name="frameEditor"]').locator('#editor_sdk'); + await sdk.click({ position: { x: 300, y: 200 } }); + await expect(page.frameLocator('iframe[name="frameEditor"]').locator('#area_id')).toBeFocused(); + await page.evaluate(() => { + const win = window.__ooFrames.readyEditor() as any; + // Ensure the canvas focus below produces a real focus event every time. + win.document.getElementById('area_id').focus(); + const nativeTimeout = win.setTimeout.bind(win); + const deferred: (() => void)[] = []; + // Firefox's SDK focus handler redirects canvas focus in a zero-delay task. + // Hold that task so input sent before keyboard-capture focus is lost reliably. + win.setTimeout = (callback: TimerHandler, delay?: number, ...args: unknown[]) => { + if (delay === 0 && typeof callback === 'function') { + deferred.push(() => callback(...args)); + return 0; + } + return nativeTimeout(callback, delay, ...args); + }; + (window as any).__releaseInputFocus = () => { + for (const callback of deferred) callback(); + }; + // Intercept only tasks scheduled synchronously by this real focus event. + try { + win.document.getElementById('editor_sdk').focus(); + } finally { + win.setTimeout = nativeTimeout; + } + (window as any).__deferredInputFocus = deferred.length; + }); + try { + expect(await page.evaluate(() => (window as any).__deferredInputFocus)).toBeGreaterThan(0); + await expect(sdk).toBeFocused(); + await insertDocumentText(page, 'Recovery version'); + await expect.poll(() => documentText(page)).toBe('Recovery version'); + } finally { + await page.evaluate(() => (window as any).__releaseInputFocus()); + } + }); + test('Ctrl+S reaches disk while a recovery conversion is awaiting delivery', async ({ page }) => { await page.goto('/editor?new=docx'); await waitForEditorReady(page); const sdk = page.frameLocator('iframe[name="frameEditor"]').locator('#editor_sdk'); await sdk.click({ position: { x: 300, y: 200 } }); - // This case measures save ownership, so seed its baseline atomically through - // the real browser input path rather than racing the SDK's initial key hooks. - await page.keyboard.insertText('Recovery version'); - const documentText = () => - page.evaluate(() => { - const win = window.__ooFrames.readyEditor() as any; - const logic = (win.Asc?.editor ?? win.editor).WordControl.m_oLogicDocument; - const paragraphs: any[] = []; - for (const element of logic.Content) element.GetAllParagraphs({ All: true }, paragraphs); - return paragraphs - .map((paragraph) => paragraph.GetText({ Numbering: false })) - .join('') - .trim(); - }); - await expect.poll(documentText).toBe('Recovery version'); + await insertDocumentText(page, 'Recovery version'); + await expect.poll(() => documentText(page)).toBe('Recovery version'); await page.evaluate(() => { const frame = document.querySelector('iframe[name="frameEditor"]')!; const converter = (frame.contentWindow as any).AscCommon.x2t; @@ -311,9 +367,10 @@ test.describe('saving into the document own file (real editor)', () => { }); await page.waitForFunction(() => typeof (window as any).__releaseRecovery === 'function'); try { + await focusDocumentInput(page); await page.keyboard.press('End'); await page.keyboard.type(' manual version', { delay: 30 }); - await expect.poll(documentText).toBe('Recovery version manual version'); + await expect.poll(() => documentText(page)).toBe('Recovery version manual version'); await page.keyboard.press('Control+s'); await waitForWrites(page, 1); expect(await savedText(page)).toContain('Recovery version manual version'); diff --git a/test/unit/agent-reference-sources.test.ts b/test/unit/agent-reference-sources.test.ts index 2effc0dcf..a211dd7d4 100644 --- a/test/unit/agent-reference-sources.test.ts +++ b/test/unit/agent-reference-sources.test.ts @@ -42,6 +42,26 @@ it('forgets deleted conversation attachments and invalidates their snapshots', ( expect(current.isCurrent()).toBe(false); expect(sources.list()).toEqual([]); }); +it.each(['all', 'selected'] as const)('keeps new attachments owned after forgetting %s references', (scope) => { + const sources = new ReferenceSources(); + sources.select('current'); + sources.add({ id: 'old', connector: 'local', title: 'Old notes', text: 'private' }); + const obsolete = sources.capture(); + sources.forget(scope === 'all' ? undefined : 'current'); + expect(sources.list()).toEqual([]); + expect(obsolete.isCurrent()).toBe(false); + sources.add({ id: 'new', connector: 'local', title: 'New notes', text: 'attached during restoration' }); + const fresh = sources.capture(); + sources.select('current'); + expect(sources.list().map((source) => source.title)).toEqual(['New notes']); + expect(fresh.isCurrent()).toBe(true); + sources.select('other'); + expect(sources.list()).toEqual([]); + sources.forget('other'); + sources.select('current'); + expect(sources.list().map((source) => source.title)).toEqual(['New notes']); + expect(fresh.isCurrent()).toBe(true); +}); it('releases canceled file reads and ignores their late completion', async () => { let complete!: (value: string) => void; const file = {