[eric] browser: the caret is handed back once when the agent stops, not once per command (ENG-252)

frontend/src/shared/browserCommandHandler.ts

@@ -1,4 +1,5 @@
 import { getWebview, findWebviewByDomain, hasDomReady, markDomReady, isPendingLoad, wakePendingLoad, clearPendingLoad, type BrowserWebview } from './browserRegistry';
+import { scheduleCaretHandback } from './caretHandback';
 import { shouldSelfHealClick } from './selfHealClick';
 import { focusGuestForKeys } from './focusGuestForKeys';
 import { FP_EXPR, clickEffect } from './clickEffect';
@@ -2248,7 +2249,10 @@ async function handleBrowserCommand(data: Record<string, any>) {
     await runBrowserCommand(request_id, action, browser_id, tab_id, params);
   } finally {
     inflightCommands.delete(request_id);
-    restoreFocus();
+    // Coalesced, not immediate: a run is many commands, and restoring after each one handed the
+    // caret back and took it again dozens of times, which is ENG-252's actual complaint. The user
+    // still gets it back, once, when the agent stops.
+    scheduleCaretHandback(restoreFocus);
   }
 }
 

frontend/src/shared/caretHandback.test.ts

@@ -0,0 +1,86 @@
+// Run: npm test
+//
+// ENG-252. Measured from control flow: an agent run of N browser commands hands the caret back N
+// times, because capture/restore brackets each command. Each one is correct; the user's caret is
+// unusable. These tests pin the coalescing that fixes the frequency.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import {
+  scheduleCaretHandback,
+  cancelCaretHandback,
+  caretHandbackCount,
+  resetCaretHandbackCount,
+  caretHandbackPending,
+  HANDBACK_IDLE_MS,
+} from './caretHandback.ts';
+
+// A controllable clock: real timers would make this a sleep test.
+function fakeTimers() {
+  let now = 0;
+  let seq = 0;
+  const jobs = new Map<number, { at: number; fn: () => void }>();
+  const set = ((fn: () => void, ms: number) => { seq += 1; jobs.set(seq, { at: now + ms, fn }); return seq; }) as unknown as typeof setTimeout;
+  const clear = ((id: number) => { jobs.delete(id); }) as unknown as typeof clearTimeout;
+  const advance = (ms: number) => {
+    now += ms;
+    for (const [id, j] of [...jobs]) if (j.at <= now) { jobs.delete(id); j.fn(); }
+  };
+  return { set, clear, advance, pendingJobs: () => jobs.size };
+}
+
+test('a run of 20 commands hands the caret back ONCE, not 20 times', () => {
+  resetCaretHandbackCount();
+  const t = fakeTimers();
+  let restored = 0;
+  for (let i = 0; i < 20; i += 1) {
+    scheduleCaretHandback(() => { restored += 1; }, HANDBACK_IDLE_MS, t.set, t.clear);
+    t.advance(50);                       // commands arrive faster than the idle window
+  }
+  assert.equal(restored, 0, 'handed the caret back mid-run, which is the bug');
+  t.advance(HANDBACK_IDLE_MS + 10);
+  assert.equal(restored, 1, `handed back ${restored} times for one run`);
+  assert.equal(caretHandbackCount(), 1);
+});
+
+test('a single command still hands the caret back', () => {
+  resetCaretHandbackCount();
+  const t = fakeTimers();
+  let restored = 0;
+  scheduleCaretHandback(() => { restored += 1; }, HANDBACK_IDLE_MS, t.set, t.clear);
+  t.advance(HANDBACK_IDLE_MS + 10);
+  assert.equal(restored, 1, 'the user must still get their caret back after one command');
+});
+
+test('the LATEST restore wins, so a stale captured element is never used', () => {
+  resetCaretHandbackCount();
+  const t = fakeTimers();
+  const order: string[] = [];
+  scheduleCaretHandback(() => order.push('first'), HANDBACK_IDLE_MS, t.set, t.clear);
+  t.advance(50);
+  scheduleCaretHandback(() => order.push('second'), HANDBACK_IDLE_MS, t.set, t.clear);
+  t.advance(HANDBACK_IDLE_MS + 10);
+  assert.deepEqual(order, ['second'], `ran ${JSON.stringify(order)}`);
+});
+
+test('two runs separated by an idle gap hand back twice', () => {
+  resetCaretHandbackCount();
+  const t = fakeTimers();
+  let restored = 0;
+  const cmd = () => scheduleCaretHandback(() => { restored += 1; }, HANDBACK_IDLE_MS, t.set, t.clear);
+  cmd(); cmd(); t.advance(HANDBACK_IDLE_MS + 10);
+  cmd(); cmd(); t.advance(HANDBACK_IDLE_MS + 10);
+  assert.equal(restored, 2, 'each distinct run gets its own handback');
+});
+
+test('cancel drops the pending handback and leaves no timer', () => {
+  resetCaretHandbackCount();
+  const t = fakeTimers();
+  let restored = 0;
+  scheduleCaretHandback(() => { restored += 1; }, HANDBACK_IDLE_MS, t.set, t.clear);
+  assert.equal(caretHandbackPending(), true);
+  cancelCaretHandback(t.clear);
+  assert.equal(caretHandbackPending(), false);
+  t.advance(HANDBACK_IDLE_MS + 10);
+  assert.equal(restored, 0, 'a cancelled handback must not fire');
+  assert.equal(t.pendingJobs(), 0, 'a cancelled handback must not leak a timer');
+});

frontend/src/shared/caretHandback.ts

@@ -0,0 +1,67 @@
+// When should the user's caret be handed back? (ENG-252, 5th filing)
+//
+// The restore itself has existed since ENG-226 and works. Traced from control flow: the socket
+// delivers ONE command per message, and `handleBrowserCommand` brackets each one with
+// capture/restore, while every keyboard primitive inside takes host focus. So an agent run of N
+// commands is literally steal, restore, steal, restore, N times. Every command behaves correctly
+// and the caret is unusable, which is why four fixes "passed" and the user kept reopening it.
+//
+// The defect is the FREQUENCY, so the fix is fewer handbacks, not better ones: coalesce them, and
+// give the caret back once the agent has actually stopped touching the browser.
+//
+// Deliberately NOT the more aggressive option (agent owns the caret for a whole run, user cannot
+// type at all until it finishes). That is a product judgement about a resource measured to be
+// singular, and it belongs to a human. This keeps the existing end state and only removes the
+// thrash, so it cannot make the current behaviour worse.
+
+export const HANDBACK_IDLE_MS = 400;
+
+interface Pending {
+  restore: () => void;
+  timer: ReturnType<typeof setTimeout>;
+}
+
+let p_pending: Pending | null = null;
+let p_handbacks = 0;
+
+/** How many times the caret was actually handed back. The number ENG-252 is about. */
+export function caretHandbackCount(): number {
+  return p_handbacks;
+}
+
+export function resetCaretHandbackCount(): void {
+  p_handbacks = 0;
+}
+
+/**
+ * Schedule the caret handback, superseding any already-pending one.
+ *
+ * Called at the end of every browser command. A run of back-to-back commands therefore schedules N
+ * times and hands back ONCE, after the agent goes quiet for HANDBACK_IDLE_MS. The restore closure
+ * is the newest one, so it reflects the most recent capture rather than a stale element.
+ */
+export function scheduleCaretHandback(
+  restore: () => void,
+  idleMs: number = HANDBACK_IDLE_MS,
+  setTimer: typeof setTimeout = setTimeout,
+  clearTimer: typeof clearTimeout = clearTimeout,
+): void {
+  if (p_pending) clearTimer(p_pending.timer);
+  const timer = setTimer(() => {
+    p_pending = null;
+    p_handbacks += 1;
+    restore();
+  }, idleMs);
+  p_pending = { restore, timer };
+}
+
+/** Drop any pending handback without running it, for teardown. */
+export function cancelCaretHandback(clearTimer: typeof clearTimeout = clearTimeout): void {
+  if (!p_pending) return;
+  clearTimer(p_pending.timer);
+  p_pending = null;
+}
+
+export function caretHandbackPending(): boolean {
+  return p_pending !== null;
+}