fix(maestro): mobile drawer access, focus preservation, stale rAF focus (#140)

Three more fixes from acceptance review:

- structured.ts's header omitted the .sessions-btn attach.ts's terminal
  header carries. The sidebar is off-canvas on mobile, so a structured
  session had no way to reach it — add the same button and
  requestToggleDrawer wiring.

- ConnectionPool.reconcileViews always remounted a repaired connection
  with focus:false. If the mistyped pane was the one actually focused,
  disposing it dropped DOM focus while `focusedId` still named it. Pass
  `focus: this.focusedId === id` instead — `focusedId` only ever names a
  real focus (multiview.ts skips setFocus while previewing), so a
  preview or a merely-visible split neighbour still comes back
  unfocused.

- onMount's deferred requestAnimationFrame focus had no way to tell it
  had been superseded by an unmount (or a remount elsewhere) before it
  fired, so a parked pane could steal focus a frame later. A token
  bumped on every mount/unmount lets the callback recognize it's stale
  and no-op.

Regression tests: 7 new (structured.test.ts, connections.test.ts). 495
tests total.
This commit is contained in:
Miguel Palhas
2026-09-05 06:41:40 +01:00
parent 42ffc1a352
commit b11784e8e3
4 changed files with 167 additions and 5 deletions
+53 -1
View File
@@ -9,6 +9,7 @@ import type { Session } from "./types.js";
// the pool reaches the focused connection with them.
const sent: { id: string; data: string }[] = [];
const disposed: string[] = [];
const mounts: { id: string; focus: boolean | undefined }[] = [];
function fakeConn(kind: string, id: string) {
const el = document.createElement("div");
el.dataset.kind = kind;
@@ -17,7 +18,9 @@ function fakeConn(kind: string, id: string) {
sessionId: id,
kind,
el,
onMount() {},
onMount(opts?: { focus?: boolean }) {
mounts.push({ id, focus: opts?.focus });
},
onUnmount() {},
focusTerminal() {},
sendInput(data: string) {
@@ -39,6 +42,7 @@ afterEach(() => {
vi.unstubAllGlobals();
sent.length = 0;
disposed.length = 0;
mounts.length = 0;
});
/** Minimal fixture for the `CatalogState` a `CatalogStore` broadcasts. */
@@ -256,4 +260,52 @@ describe("ConnectionPool: view resolution races", () => {
expect(disposed).toEqual([]);
});
it("re-focuses the fresh connection when the replaced pane was the focused session", () => {
mockFetch();
const { store, socket } = fakeStore();
const pool = new ConnectionPool({ store });
const pane = document.createElement("div");
pool.mount(A, pane);
pool.setFocus(A);
mounts.length = 0;
socket.push([session(A, "Structured")]);
expect(disposed).toEqual(["attach:" + A]);
expect(mounts).toEqual([{ id: A, focus: true }]);
});
it("does not steal focus for a merely-visible pane that was never actually focused", () => {
mockFetch();
const { store, socket } = fakeStore();
const pool = new ConnectionPool({ store });
const paneA = document.createElement("div");
const paneB = document.createElement("div");
pool.mount(A, paneA);
pool.mount(B, paneB);
pool.setFocus(B); // B is the real keyboard focus; A is only visible
mounts.length = 0;
socket.push([session(A, "Structured")]);
expect(disposed).toEqual(["attach:" + A]);
expect(mounts).toEqual([{ id: A, focus: false }]);
});
it("does not focus a parked (unmounted) connection it repairs", () => {
mockFetch();
const { store, socket } = fakeStore();
const pool = new ConnectionPool({ store });
const pane = document.createElement("div");
pool.mount(A, pane);
pool.setFocus(A);
pool.unmount(A); // parked — no longer visible, `focusedId` still names it
mounts.length = 0;
socket.push([session(A, "Structured")]);
expect(disposed).toEqual(["attach:" + A]);
expect(mounts).toEqual([]); // parked repair calls onUnmount, never onMount
});
});
+9 -2
View File
@@ -171,8 +171,15 @@ export class ConnectionPool {
// siblings — append, never replaceChildren, or they'd all vanish.
this.hidden.appendChild(fresh.el);
}
if (wasMounted) fresh.onMount({ focus: false });
else fresh.onUnmount();
if (wasMounted) {
// `focusedId` only ever names a session the operator actually
// focused — never a merely-visible split neighbour or a preview
// (multiview.ts skips `setFocus` while previewing) — so this is
// exactly "was the keyboard actually in the pane being replaced."
fresh.onMount({ focus: this.focusedId === id });
} else {
fresh.onUnmount();
}
}
}
+87 -1
View File
@@ -1,5 +1,5 @@
// @vitest-environment happy-dom
import { beforeEach, describe, expect, it, vi } from "vitest";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
vi.mock("./api.js", async () => {
const actual = await vi.importActual<typeof import("./api.js")>("./api.js");
@@ -12,6 +12,7 @@ vi.mock("./api.js", async () => {
});
import { cancelStructuredTurn, resolveStructuredApproval, sendStructuredPrompt } from "./api.js";
import { TOGGLE_DRAWER_EVENT } from "./drawer.js";
import { CatalogStore } from "./events.js";
import { createStructuredConnection, type StructuredSocketFactory } from "./structured.js";
import type { ReducedState, Session, StructuredEvent, TranscriptRow } from "./types.js";
@@ -349,6 +350,22 @@ describe("structured connection: pane lifecycle", () => {
conn.dispose();
});
it("carries the same off-canvas sessions button as the terminal attach header", () => {
const conn = mount({ sockets: [new FakeSocket()] });
const btn = conn.el.querySelector(".sessions-btn") as HTMLButtonElement;
expect(btn).not.toBeNull();
expect(btn.getAttribute("aria-label")).toBe("Show sessions");
expect(btn.type).toBe("button");
let toggled = 0;
const onToggle = () => toggled++;
document.addEventListener(TOGGLE_DRAWER_EVENT, onToggle);
btn.click();
document.removeEventListener(TOGGLE_DRAWER_EVENT, onToggle);
expect(toggled).toBe(1);
conn.dispose();
});
it("focusTerminal focuses the composer only when it is enabled", () => {
const ws = new FakeSocket();
const conn = mount({ sockets: [ws] });
@@ -400,3 +417,72 @@ describe("structured connection: pane lifecycle", () => {
conn.dispose();
});
});
describe("structured connection: deferred mount focus", () => {
let frames: FrameRequestCallback[];
beforeEach(() => {
vi.clearAllMocks();
frames = [];
vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => {
frames.push(cb);
return frames.length;
});
});
afterEach(() => vi.unstubAllGlobals());
function runFrames(): void {
const pending = frames;
frames = [];
for (const cb of pending) cb(0);
}
it("does not focus the composer if the pane was parked before its deferred frame ran", () => {
const ws = new FakeSocket();
const conn = mount({ sockets: [ws] });
ws.open();
ws.emit({ type: "transcript_snapshot", snapshot: { cursor: 0, rows: [] } });
ws.emit({ type: "reduced_state", state: idleState });
conn.onMount({ focus: true });
conn.onUnmount(); // parked again before the frame below runs
runFrames();
const input = conn.el.querySelector(".structured-input") as HTMLTextAreaElement;
expect(document.activeElement).not.toBe(input);
conn.dispose();
});
it("does not focus the composer if it was re-mounted into a different pane before the stale frame ran", () => {
const ws = new FakeSocket();
const conn = mount({ sockets: [ws] });
ws.open();
ws.emit({ type: "transcript_snapshot", snapshot: { cursor: 0, rows: [] } });
ws.emit({ type: "reduced_state", state: idleState });
conn.onMount({ focus: true }); // stale frame #1
conn.onUnmount();
conn.onMount({ focus: false }); // the pane that actually won focus, without it
runFrames(); // both frames fire; only the latest should have any say
const input = conn.el.querySelector(".structured-input") as HTMLTextAreaElement;
expect(document.activeElement).not.toBe(input);
conn.dispose();
});
it("still focuses normally when nothing raced the frame", () => {
const ws = new FakeSocket();
const conn = mount({ sockets: [ws] });
ws.open();
ws.emit({ type: "transcript_snapshot", snapshot: { cursor: 0, rows: [] } });
ws.emit({ type: "reduced_state", state: idleState });
conn.onMount({ focus: true });
runFrames();
const input = conn.el.querySelector(".structured-input") as HTMLTextAreaElement;
expect(document.activeElement).toBe(input);
conn.dispose();
});
});
+18 -1
View File
@@ -12,6 +12,7 @@ import {
sendStructuredPrompt,
structuredWsUrl,
} from "./api.js";
import { requestToggleDrawer } from "./drawer.js";
import type { CatalogState, CatalogStore } from "./events.js";
import { paneTitleAttr, paneTitleHtml, type AttachInfo } from "./paneTitle.js";
import type { PaneConnection } from "./pane.js";
@@ -57,6 +58,7 @@ export function createStructuredConnection(
el.className = "attach structured";
el.innerHTML = `
<header class="attach-header">
<button type="button" class="btn btn-ghost btn-icon sessions-btn" aria-label="Show sessions" title="Show sessions">☰</button>
<span class="attach-title" title="${paneTitleAttr(sessionId, info)}">${paneTitleHtml(sessionId, info)}</span>
<span class="st-pills"></span>
<span class="attach-state" data-state="connecting">connecting…</span>
@@ -87,6 +89,12 @@ export function createStructuredConnection(
const cancelBtn = el.querySelector(".structured-cancel") as HTMLButtonElement;
disconnectBtn.addEventListener("click", () => opts.onRequestClose?.(sessionId));
// Mobile-only (CSS hides it on desktop, where the sidebar is always in
// view): the session list is an off-canvas drawer and this opens it —
// same control attach.ts's terminal header carries.
(el.querySelector(".sessions-btn") as HTMLElement).addEventListener("click", () =>
requestToggleDrawer(),
);
const setConnState = (text: string, kind: "connecting" | "open" | "closed" | "error") => {
stateEl.textContent = text;
@@ -346,17 +354,26 @@ export function createStructuredConnection(
}
connect();
// A pane can be parked again before its deferred focus frame fires (a
// preview stepped past it, a split closed it) — the frame must not focus a
// hidden textarea out from under whatever the operator moved to since.
// `mountToken` is bumped on every mount/unmount so a stale frame can tell
// it is no longer the most recent one.
let mountToken = 0;
return {
sessionId,
el,
onMount(mountOpts?: { focus?: boolean }) {
const focusOnMount = mountOpts?.focus !== false;
const token = ++mountToken;
requestAnimationFrame(() => {
if (token !== mountToken) return; // parked/remounted since this was scheduled
if (focusOnMount && !input.disabled) input.focus();
});
},
onUnmount() {
// Nothing to release: the composer needs no GPU context or size fit.
mountToken++;
},
focusTerminal() {
if (!input.disabled) input.focus();