diff --git a/frontend/src/shared/browserCommandHandler.ts b/frontend/src/shared/browserCommandHandler.ts index 3fdaed97..d874ee41 100644 --- a/frontend/src/shared/browserCommandHandler.ts +++ b/frontend/src/shared/browserCommandHandler.ts @@ -1,5 +1,6 @@ import { getWebview, findWebviewByDomain, hasDomReady, markDomReady, isPendingLoad, wakePendingLoad, clearPendingLoad, type BrowserWebview } from './browserRegistry'; import { shouldSelfHealClick } from './selfHealClick'; +import { focusGuestForKeys } from './focusGuestForKeys'; import { FP_EXPR, clickEffect } from './clickEffect'; import { store } from './state/store'; import { resumeBrowserCard } from './state/dashboardLayoutSlice'; @@ -812,11 +813,16 @@ function cdpKeyDescriptor(rawKey: string): CdpKeyDescriptor | null { async function handlePressKey(wv: BrowserWebview, params: Record): Promise> { const rawKey = (params.key as string) || ''; if (!rawKey) return { error: 'key parameter is required' }; - await evalInPage(wv, 'document.body && document.body.focus && document.body.focus(); true'); + // A key event follows the HOST window's focus, not the CDP target it was addressed to. With the + // user's caret in a composer, the agent's Enter lands in THEIR half-written message. Taking the + // webview first is the only thing that makes that unrepresentable; the in-guest body.focus() that + // used to sit here does not do it (measured: keys still went to the host). handleBrowserCommand + // hands the caret back when the command ends. + focusGuestForKeys(wv); const desc = cdpKeyDescriptor(rawKey); if (desc) { try { - // CDP key events are trusted AND scoped to THIS webview no matter where the user's cursor sits; the sendInputEvent path delivered to whatever had focus, which is the "agent typed into my note" bug. keyDown-with-text inserts the char; bare named keys use rawKeyDown so no stray char lands. + // keyDown-with-text inserts the char; bare named keys use rawKeyDown so no stray char lands. const down: Record = { type: desc.text ? 'keyDown' : 'rawKeyDown', key: desc.key, windowsVirtualKeyCode: desc.vk, nativeVirtualKeyCode: desc.vk }; if (desc.code) down.code = desc.code; if (desc.text) down.text = desc.text; @@ -827,9 +833,8 @@ async function handlePressKey(wv: BrowserWebview, params: Record): return { text: `Pressed ${rawKey}` }; } catch { /* fall through to the legacy path so a CDP hiccup never makes a key dead */ } } - // Legacy focus-dependent fallback (exotic keys or CDP unavailable): keeps every key that worked before working. + // Legacy fallback (exotic keys or CDP unavailable): keeps every key that worked before working. const keyCode = KEY_NAME_MAP[rawKey] || rawKey; - await evalInPage(wv, 'document.body && document.body.focus && document.body.focus(); true'); // Native OS-level key events have isTrusted=true, so hostile sites' keyboard handlers respect them. wv.sendInputEvent({ type: 'keyDown', keyCode }); wv.sendInputEvent({ type: 'char', keyCode }); @@ -1246,6 +1251,9 @@ async function clickBackendNode( return { text: `Focused ${label} and typed the text in${via}. Verified: the box now contains "${got}". Do NOT type it again.` }; }; try { + // DOM.focus lands inside the guest but leaves the HOST caret alone, and insertText follows + // the host: measured, a whole sentence went into the user's composer instead of the page. + focusGuestForKeys(wv); await sendCdp(wv, 'Input.insertText', { text: opts.text }, sessionId); } catch (err: any) { return { error: `Focused ${label} but could not type into it: ${err?.message || String(err)}` }; diff --git a/frontend/src/shared/focusGuestForKeys.test.ts b/frontend/src/shared/focusGuestForKeys.test.ts new file mode 100644 index 00000000..dfad90fc --- /dev/null +++ b/frontend/src/shared/focusGuestForKeys.test.ts @@ -0,0 +1,67 @@ +// Run: node --test (via frontend/scripts/run-tests.mjs) +// +// The invariant this file defends: an agent keystroke never reaches the user's own text box. +// It is enforced by giving the guest host focus BEFORE dispatching, because a synthetic key follows +// the host window's focus rather than the CDP target (measured in an Electron probe; both +// Input.dispatchKeyEvent and Input.insertText leaked a real string into a host ). +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { focusGuestForKeys, guestKeyTakeoverCount, resetGuestKeyTakeoverCount } from './focusGuestForKeys.ts'; + +function fakeWebview(): { focus: () => void; focused: number } { + const wv = { focused: 0, focus(): void { wv.focused += 1; } }; + return wv as { focus: () => void; focused: number }; +} + +function withActive(el: unknown, fn: () => void): void { + const doc = globalThis.document as unknown as { activeElement: unknown }; + const prev = doc.activeElement; + doc.activeElement = el; + try { fn(); } finally { doc.activeElement = prev; } +} + +const userInput = { tagName: 'INPUT', isContentEditable: false }; + +test('the guest is focused before any keystroke, so the key cannot land on the host', () => { + const wv = fakeWebview(); + withActive(userInput, () => focusGuestForKeys(wv as never)); + assert.equal(wv.focused, 1, 'the guest never took focus, so the keystroke would follow the host'); +}); + +test('taking the caret off a user text box is counted', () => { + resetGuestKeyTakeoverCount(); + const wv = fakeWebview(); + withActive(userInput, () => focusGuestForKeys(wv as never)); + assert.equal(guestKeyTakeoverCount(), 1); +}); + +test('a contenteditable counts too, since that is where a half-written message lives', () => { + resetGuestKeyTakeoverCount(); + const wv = fakeWebview(); + withActive({ tagName: 'DIV', isContentEditable: true }, () => focusGuestForKeys(wv as never)); + assert.equal(guestKeyTakeoverCount(), 1); +}); + +// The negative half: without it, "it counts" would pass on a version that counts unconditionally. +test('routine driving with no user caret involved counts nothing', () => { + resetGuestKeyTakeoverCount(); + const wv = fakeWebview(); + withActive({ tagName: 'BODY', isContentEditable: false }, () => focusGuestForKeys(wv as never)); + withActive(null, () => focusGuestForKeys(wv as never)); + assert.equal(guestKeyTakeoverCount(), 0, 'a takeover was counted when no user surface held the caret'); + assert.equal(wv.focused, 2, 'the guest must still be focused every time'); +}); + +test('a webview that already holds focus is not counted as a takeover', () => { + resetGuestKeyTakeoverCount(); + const wv = fakeWebview(); + withActive(wv, () => focusGuestForKeys(wv as never)); + assert.equal(guestKeyTakeoverCount(), 0); +}); + +test('a card that unmounted mid-command fails quiet instead of killing the run', () => { + const dead = { focus(): void { throw new Error('detached'); } }; + withActive(userInput, () => { + assert.doesNotThrow(() => focusGuestForKeys(dead as never)); + }); +}); diff --git a/frontend/src/shared/focusGuestForKeys.ts b/frontend/src/shared/focusGuestForKeys.ts new file mode 100644 index 00000000..3890e0c7 --- /dev/null +++ b/frontend/src/shared/focusGuestForKeys.ts @@ -0,0 +1,40 @@ +import type { BrowserWebview } from './browserRegistry'; + +// A synthetic key or text insert follows the HOST window's focus, not the CDP target it was +// addressed to. Measured in an isolated Electron probe, with the user's caret in a plain input: +// +// Input.dispatchKeyEvent -> the characters landed in the USER's input, not the page +// Input.insertText -> a whole string landed in the USER's input, not the page +// after focusing the element first -> both landed in the page, user's input untouched +// +// So the guest MUST hold host focus while the agent types, and the previously trusted in-guest +// document.body.focus() does not provide it (same probe, still leaked). This is also why the +// caret-restore idea in ENG-252 is not merely useless but harmful: hand the caret back mid-run and +// the agent's next Enter submits whatever the user was halfway through writing. + +let p_takeovers = 0; + +/** How many times a keystroke had to take the caret off a real user text surface. */ +export function guestKeyTakeoverCount(): number { + return p_takeovers; +} + +export function resetGuestKeyTakeoverCount(): void { + p_takeovers = 0; +} + +function p_isUserTextSurface(el: Element | null): boolean { + if (!el) return false; + return el.tagName === 'INPUT' || el.tagName === 'TEXTAREA' || (el as HTMLElement).isContentEditable === true; +} + +/** Give the guest the host focus its keystrokes need, counting when we took it from a user. */ +export function focusGuestForKeys(wv: BrowserWebview): void { + const before = typeof document !== 'undefined' ? document.activeElement : null; + if (before !== (wv as unknown as Element) && p_isUserTextSurface(before)) p_takeovers += 1; + try { + wv.focus(); + } catch { + // The card unmounted mid-command. The keystroke will miss, which beats it hitting the user. + } +}