From 96457a34d0812f9fb69ebcfe0e6f5eaab1d8aeef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20Andr=C3=A9?= Date: Sun, 4 Oct 2026 19:14:41 +0200 Subject: [PATCH] fix(chat): preserve answer focus across deferred session activation Recheck input, interruption and modal ownership when deferred activation runs rather than only before scheduling it. Cancel pending frames on deactivation, disposal or superseding session activation so a stale callback cannot redirect an answer into the composer. Preserve ordinary composer and conversation activation and phone keyboard behavior. Gate the final activation frame in the browser fixture to deterministically cover competing controls, modal ownership, stale callbacks and normal activation. Seven focus tests and UI TypeScript pass. Correct the transcript fixture's permission-receipts response to valid empty JSON; all 46 transcript tests pass without assertion relaxation. These address the three full-CI failures reported after PR845: the focus race and invalid receipt mock were independently reproduced on its baseline as well as its head. --- dev-docs/NATIVE_INTERRUPTION_UX.md | 3 + .../src/components/session/session-view.tsx | 51 ++++--- .../browser/fixtures/activation-frame-gate.ts | 33 ++++ .../browser/fixtures/interruption-dock.tsx | 14 +- .../browser/interruption-selection.test.ts | 144 +++++++++++++++++- .../tests/browser/session-rendering.test.ts | 7 +- 6 files changed, 226 insertions(+), 26 deletions(-) create mode 100644 packages/ui/tests/browser/fixtures/activation-frame-gate.ts diff --git a/dev-docs/NATIVE_INTERRUPTION_UX.md b/dev-docs/NATIVE_INTERRUPTION_UX.md index 3f6b9ddf6..7c945493d 100644 --- a/dev-docs/NATIVE_INTERRUPTION_UX.md +++ b/dev-docs/NATIVE_INTERRUPTION_UX.md @@ -35,6 +35,9 @@ instead of clipping controls; field scrolling can chain into that outer scroller by request kind/id, preserving partial answers and rejection reasons through native object replacement, queue navigation and session navigation. - Collapse only hides the panel body. It never cancels or refuses a request. +- Deferred session-activation focus must recheck the current input and modal owner + when it runs, and expire when the pane deactivates or unmounts. An answer field + focused during navigation retains keyboard input instead of yielding to the composer. - Pending questions appear only in the dock. There are no “View in discussion” or “Answer in dock” links and no interruption-specific transcript reveal state. - Only the panel submits replies. Transcript tools no longer register document-wide diff --git a/packages/ui/src/components/session/session-view.tsx b/packages/ui/src/components/session/session-view.tsx index 63106be2a..9ddc55795 100644 --- a/packages/ui/src/components/session/session-view.tsx +++ b/packages/ui/src/components/session/session-view.tsx @@ -294,8 +294,8 @@ export const SessionView: Component = (props) => { createEffect( on( - () => props.isActive, - (isActive) => { + () => [props.isActive, props.sessionId] as const, + ([isActive]) => { if (!isActive) { if (props.focusConversationOnActivate) props.onConversationFocusHandled?.() clearConversationPlaybackForSession(props.instanceId, props.sessionId) @@ -307,33 +307,42 @@ export const SessionView: Component = (props) => { // Don't steal focus from other inputs (command palette, dialogs, selectors, etc.) if (typeof document === "undefined") return - const activeEl = document.activeElement as HTMLElement | null - const activeIsInput = - activeEl?.tagName === "INPUT" || - activeEl?.tagName === "TEXTAREA" || - activeEl?.tagName === "SELECT" || - Boolean(activeEl?.isContentEditable) - if (activeIsInput) return - - const modalOpen = Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) - if (modalOpen) return + const activeEl = document.activeElement + const focusIsProtected = () => { + const current = document.activeElement as HTMLElement | null + return current?.matches("input, textarea, select") || current?.isContentEditable + || Boolean(current?.closest(".interruption-dock")) + || Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) + } + if (focusIsProtected()) return // Defer until the session pane is visible and the textarea is mounted. - requestAnimationFrame(() => { - requestAnimationFrame(() => { - if (!props.isActive) return + // Cleanup also fences already-dispatched frames across rapid reactivation. + let cancelled = false + let frame: number + onCleanup(() => { + cancelled = true + cancelAnimationFrame(frame) + }) + frame = requestAnimationFrame(function waitForActivatedSession() { + if (cancelled || !props.isActive) return + frame = requestAnimationFrame(function focusActivatedSession() { + if (cancelled || !props.isActive || !rootRef?.isConnected) return + const activeElement = document.activeElement + const focusIsUnclaimed = + !activeElement || activeElement === document.body || activeElement === document.documentElement + // Input/modal ownership can change while either frame is pending. + // Preserve a newly claimed control, including the dock's own chrome. + const focusIsBlocked = focusIsProtected() || (!focusIsUnclaimed && activeElement !== activeEl) if (props.focusConversationOnActivate) { - const activeElement = document.activeElement - const focusIsUnclaimed = - !activeElement || activeElement === document.body || activeElement === document.documentElement - const modalIsOpen = Boolean(document.querySelector('[role="dialog"][aria-modal="true"]')) - if (focusIsUnclaimed && !modalIsOpen && focusConversationStream(rootRef)) { + if (focusIsUnclaimed && !focusIsBlocked && focusConversationStream(rootRef)) { props.onConversationFocusHandled?.() return } props.onConversationFocusHandled?.() - if (!focusIsUnclaimed || modalIsOpen) return + if (!focusIsUnclaimed) return } + if (focusIsBlocked) return if (promptInputApi) { promptInputApi.focus() return diff --git a/packages/ui/tests/browser/fixtures/activation-frame-gate.ts b/packages/ui/tests/browser/fixtures/activation-frame-gate.ts new file mode 100644 index 000000000..9682ced76 --- /dev/null +++ b/packages/ui/tests/browser/fixtures/activation-frame-gate.ts @@ -0,0 +1,33 @@ +// Hold only SessionView's final activation-focus frame. Transcript/layout frames +// keep running, and native frame IDs/cancellation retain their normal meaning. +export function installActivationFrameGate() { + const request = window.requestAnimationFrame.bind(window) + const cancel = window.cancelAnimationFrame.bind(window) + const held = new Map() + let paused = false + window.requestAnimationFrame = callback => { + if (!paused || callback.name !== "focusActivatedSession") return request(callback) + const id = request(() => {}) + held.set(id, { callback, cancelled: false }) + return id + } + window.cancelAnimationFrame = id => { + const entry = held.get(id) + if (entry) entry.cancelled = true + cancel(id) + } + return { + pause: () => { paused = true }, + pending: () => [...held.values()].filter(entry => !entry.cancelled).length, + cancelled: () => [...held.values()].filter(entry => entry.cancelled).length, + flush: (cancelledOnly = false) => { + for (const [id, entry] of [...held]) { + if (cancelledOnly && !entry.cancelled) continue + held.delete(id) + cancel(id) + // Explicitly exercise callbacks already dispatched when cleanup occurs. + if (cancelledOnly || !entry.cancelled) entry.callback(performance.now()) + } + }, + } +} diff --git a/packages/ui/tests/browser/fixtures/interruption-dock.tsx b/packages/ui/tests/browser/fixtures/interruption-dock.tsx index 682c2b879..0b412f6e6 100644 --- a/packages/ui/tests/browser/fixtures/interruption-dock.tsx +++ b/packages/ui/tests/browser/fixtures/interruption-dock.tsx @@ -1,4 +1,4 @@ -import { For } from "solid-js" +import { For, createSignal } from "solid-js" import { render } from "solid-js/web" import SessionView from "../../../src/components/session/session-view" import { InterruptionDock } from "../../../src/components/interruption-dock" @@ -18,6 +18,7 @@ import { loadMessages, loadMessageAnchor } from "../../../src/stores/session-api import { applyUiSettings } from "./ui-settings" import { getQuestionToolSearchText } from "../../../src/components/tool-call/search-text" import "../../../src/index.css" +import { installActivationFrameGate } from "./activation-frame-gate" const instanceId = "interruptions", sessionId = "s", toolId = "question-tool" let messageId = "msg_0000" @@ -110,18 +111,27 @@ serverApi.patchStateOwner = async (_owner, patch) => { serverApi.fetchPermissionReceipts = async () => ({ receipts: [] }) await applyUiSettings({ locale: "en", showMessageTimeline: false, toolInputsVisibility: "hidden", toolOutputExpansion: "expanded", toolCallExpansionDefaults: { preset: "custom", thinking: "collapsed", tools: { other: "expanded" } } }) +const activationFrames = installActivationFrameGate() +const [active, setActive] = createSignal(true) +const [conversationFocus, setConversationFocus] = createSignal(false) +const [phone, setPhone] = createSignal(false) +let focusHandled = 0 function App() { const panel = return
focusInterruption(instanceId)} /> {id => {}} onModelChange={async () => {}} />} + escapeInDebounce={false} isActive={active()} isPhoneLayout={phone()} focusConversationOnActivate={conversationFocus()} + onConversationFocusHandled={() => { focusHandled++ }} + interruptionPanel={panel} onAgentChange={async () => {}} onModelChange={async () => {}} />}
} render(() => , document.getElementById("root")!) const store = messageStoreBus.getOrCreate(instanceId) ;(window as any).fixture = { + activationFrames, active: setActive, conversationFocus: setConversationFocus, phone: setPhone, + focusHandled: () => focusHandled, replies, windows, theme: setThemePreference, ask: () => emit("form.created", { form: form() }), diff --git a/packages/ui/tests/browser/interruption-selection.test.ts b/packages/ui/tests/browser/interruption-selection.test.ts index c217dffb4..9e4a69afd 100644 --- a/packages/ui/tests/browser/interruption-selection.test.ts +++ b/packages/ui/tests/browser/interruption-selection.test.ts @@ -1,7 +1,7 @@ import assert from "node:assert/strict" import { after, before, test } from "node:test" import { fileURLToPath } from "node:url" -import { chromium, type Browser } from "playwright" +import { chromium, type Browser, type Page } from "playwright" import { createServer, type ViteDevServer } from "vite" import solid from "vite-plugin-solid" @@ -47,9 +47,16 @@ test("a question stays selected and focused when the newly visited session recei await answer.fill("Draft before switching") // This session has no request: the same question remains visible after the pane remount. - await page.evaluate(() => (window as any).fixture.switch("other")) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.activationFrames.pause() + fixture.switch("other") + }) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) assert.equal(await answer.inputValue(), "Draft before switching") await answer.fill("Continue the original answer") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await answer.evaluate(element => element === document.activeElement), true) const pending = await page.evaluate(async modulePath => { const { addPermissionToQueue, getPermissionQueue } = await import(/* @vite-ignore */ modulePath) @@ -63,6 +70,7 @@ test("a question stays selected and focused when the newly visited session recei assert.deepEqual(pending, [{ id: "permission-other", sessionID: "other" }]) assert.equal(await answer.inputValue(), "Continue the original answer") + assert.equal(await page.locator(".prompt-input").inputValue(), "") assert.equal(await answer.evaluate(element => element === document.activeElement), true) assert.equal(await page.locator(".interruption-session").innerText(), "Main session") assert.equal(await page.locator(".interruption-position").innerText(), "2 / 2") @@ -73,3 +81,135 @@ test("a question stays selected and focused when the newly visited session recei await page.close() } }) + +async function withActivationPage(run: (page: Page) => Promise) { + const page = await browser.newPage({ viewport: { width: 1100, height: 800 } }) + const errors: string[] = [] + page.on("pageerror", error => errors.push(error.message)) + try { + await page.route("**/api/**", route => route.fulfill({ contentType: "application/json", body: "{}" })) + await page.goto(url, { waitUntil: "domcontentloaded", timeout: 60000 }) + await page.waitForFunction(() => Boolean((window as any).fixture?.snapshot().ids.length), undefined, { timeout: 60000 }) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.active(false) + fixture.activationFrames.pause() + ;(document.activeElement as HTMLElement)?.blur() + }) + await run(page) + assert.deepEqual(errors, []) + } finally { + await page.close() + } +} + +async function activate(page: Page, conversation: boolean) { + await page.evaluate(conversation => { + const fixture = (window as any).fixture + fixture.active(false) + ;(document.activeElement as HTMLElement)?.blur() + fixture.conversationFocus(conversation) + fixture.active(true) + }, conversation) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) +} + +for (const conversation of [false, true]) { + const mode = conversation ? "conversation" : "composer" + test(`deferred ${mode} activation respects inputs, dock focus and a newly opened modal`, async () => { + await withActivationPage(async page => { + await page.evaluate(() => (window as any).fixture.ask()) + for (const target of ["input", "textarea", "select", "editable", "dock", "dock-button", "modal"]) { + await activate(page, conversation) + const preserved = await page.evaluate(target => { + let control: HTMLElement + if (target === "dock" || target === "dock-button") { + control = document.querySelector(target === "dock" ? ".interruption-dock" : ".interruption-toggle")! + } else { + control = document.createElement(target === "editable" || target === "modal" ? "div" : target) + control.id = "competing-focus" + if (target === "editable") control.contentEditable = "true" + if (target === "modal") { + control.setAttribute("role", "dialog") + control.setAttribute("aria-modal", "true") + } + document.body.append(control) + } + if (target !== "modal") control.focus() + const focused = document.activeElement + ;(window as any).fixture.activationFrames.flush() + return focused === document.activeElement + }, target) + assert.equal(preserved, true, `${mode} must preserve ${target} ownership`) + await page.evaluate(() => document.getElementById("competing-focus")?.remove()) + } + }) + }) + + test(`ordinary ${mode} activation still focuses the requested surface`, async () => { + await withActivationPage(async page => { + await activate(page, conversation) + const handled = await page.evaluate(() => { + const fixture = (window as any).fixture + const before = fixture.focusHandled() + fixture.activationFrames.flush() + return fixture.focusHandled() - before + }) + assert.equal(handled, conversation ? 1 : 0) + assert.equal(await page.locator(conversation ? ".message-stream" : ".prompt-input") + .evaluate(element => element === document.activeElement), true) + }) + }) +} + +test("inactive, disposed and superseded activation frames cannot reclaim focus", async () => { + await withActivationPage(async page => { + await activate(page, false) + await page.evaluate(() => (window as any).fixture.active(false)) + assert.equal(await page.evaluate(() => (window as any).fixture.activationFrames.cancelled()), 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "an inactive callback must be inert even if already dispatched") + + await activate(page, false) + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.active(false) + fixture.active(true) + }) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "an old activation must not become valid again after reactivation") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".prompt-input").evaluate(element => element === document.activeElement), true) + + await activate(page, false) + await page.evaluate(() => (window as any).fixture.switch("other")) + await page.waitForFunction(() => (window as any).fixture.activationFrames.pending() === 1) + assert.equal(await page.evaluate(() => { + ;(window as any).fixture.activationFrames.flush(true) + return document.activeElement === document.body + }), true, "a disposed pane must not focus its detached composer") + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".prompt-input").evaluate(element => element === document.activeElement), true) + }) +}) + +test("phone activation does not summon the composer keyboard", async () => { + await withActivationPage(async page => { + await page.evaluate(() => { + const fixture = (window as any).fixture + fixture.phone(true) + fixture.active(true) + }) + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(() => resolve())))) + assert.equal(await page.evaluate(() => (window as any).fixture.activationFrames.pending()), 0) + assert.equal(await page.evaluate(() => document.activeElement === document.body), true) + await activate(page, true) + await page.evaluate(() => (window as any).fixture.activationFrames.flush()) + assert.equal(await page.locator(".message-stream").evaluate(element => element === document.activeElement), true) + }) +}) diff --git a/packages/ui/tests/browser/session-rendering.test.ts b/packages/ui/tests/browser/session-rendering.test.ts index 265ff33cb..ef87d9a47 100644 --- a/packages/ui/tests/browser/session-rendering.test.ts +++ b/packages/ui/tests/browser/session-rendering.test.ts @@ -54,7 +54,12 @@ async function open(name: string, run: (page: Page) => Promise) { if (window.fixtureScrollEvents.length > 80) window.fixtureScrollEvents.shift(); }, { capture: true, passive: true }); })()`) - await page.route("**/api/**", route => route.fulfill({ contentType: route.request().url().includes("events") ? "text/event-stream" : "application/json", body: "" })) + await page.route("**/api/**", route => { + if (new URL(route.request().url()).pathname.endsWith("/permission-receipts")) { + return route.fulfill({ contentType: "application/json", body: JSON.stringify({ receipts: [] }) }) + } + return route.fulfill({ contentType: route.request().url().includes("events") ? "text/event-stream" : "application/json", body: "" }) + }) await runWithDiagnosticCleanup({ run: async () => { await page.goto(`${baseUrl}/fixture?${name}`)