Let Escape close plugin previews before stopping agents (#7110)

* fix(app): let Escape close an open popover before it interrupts the agent

The agent.interrupt shortcut listens on the window in the capture phase, so
it sees Escape before anything else. Registered overlays get first refusal
through dispatchTopWebOverlayKeyDown, but plugins cannot register, and draw
their floating panels with the Popover API instead. With such a panel open
and the composer focused while an agent runs, Escape closed the panel and
interrupted the agent in the same press.

While a popover is open, Escape is left to it and no app shortcut runs. A
second Escape, with nothing open, interrupts as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: RoundRedDot <roundreddot@gmail.com>

* test(app): cover Escape with a popover open through the shortcut path

The first test only checked hasOpenDomPopover and would still pass with the
guard removed from useKeyboardShortcuts. The check is now keyBelongsToDomPopover,
called by the hook, and a browser test drives it the way the app does: a
window capture listener for shortcuts, a popover closed by its own document
Escape listener, focus in a textarea. The first Escape closes the popover
with no interrupt; the second interrupts. Both new tests fail with the guard
removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: RoundRedDot <roundreddot@gmail.com>

* test(app): exercise plugin Escape through the real composer

Keep the contributor fix and replace the substitute shortcut-listener test with an installed-plugin browser regression. The first Escape must close the preview without cancelling the turn; the second must cancel it.

Co-authored-by: luke <luopeihuan@gmail.com>
Co-authored-by: RoundRedDot <roundreddot@gmail.com>

* test(app): check dismissed model pickers leave the viewport

---------

Co-authored-by: luke <luopeihuan@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: RoundRedDot <roundreddot@gmail.com>
Co-authored-by: paseo-bot[bot] <266920839+paseo-bot[bot]@users.noreply.github.com>
This commit is contained in:
paseo-bot[bot]
2026-10-16 09:14:10 +02:00
committed by GitHub
co-authored by Claude Opus 5.5 RoundRedDot luke
parent 4117dce2d3
commit 5acfbb3777
6 changed files with 162 additions and 5 deletions
@@ -89,9 +89,11 @@ async function dismissStackedModelPickerWithBackdrop(page: Page) {
.getByTestId("agent-controls-settings-list")
.getByRole("button", { name: /Select model/ })
.click();
await expect(page.getByTestId("agent-controls-model-browser-sheet")).toBeVisible();
await expect(page.getByTestId("agent-controls-model-browser-sheet")).toBeInViewport();
await page.mouse.click(MOBILE_VIEWPORT.width / 2, 24);
await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeVisible();
// In CI, Gorhom retains the dismissed sheet below the viewport.
// Dismissal is complete when it leaves the displayed surface.
await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeInViewport();
await expect(page.getByTestId("agent-controls-model-sheet")).toBeVisible();
await expect(page.getByTestId("agent-controls-settings-list")).toBeVisible();
}
@@ -139,7 +141,7 @@ test.describe("mobile bottom sheet reopen", () => {
await expect(sheet).toBeVisible();
await expect(page.getByTestId("agent-controls-settings-list")).toBeVisible();
await expect(page.getByTestId("agent-controls-model")).toContainText("Ten second stream");
await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeVisible();
await expect(page.getByTestId("agent-controls-model-browser-sheet")).not.toBeInViewport();
});
} finally {
await session.cleanup();
@@ -0,0 +1,47 @@
import { expect, test, type Page } from "../support/fixtures";
import { composerLocator, submitMessageWithButton } from "../support/helpers/composer";
import { seedMockAgentWorkspace, openAgentRoute } from "../support/helpers/mock-agent";
import { withPopoverTimelinePlugin } from "../support/helpers/plugin-timeline";
async function openPreviewWhileTyping(page: Page): Promise<void> {
await composerLocator(page).focus();
await page.getByRole("button", { name: "Hover plugin preview", exact: true }).hover();
await expect(page.getByRole("dialog", { name: "Plugin preview" })).toBeVisible();
await expect(composerLocator(page)).toBeFocused();
}
async function dismissPreview(page: Page): Promise<void> {
await page.keyboard.press("Escape");
await expect(page.getByRole("dialog", { name: "Plugin preview" })).not.toBeVisible();
}
test("Escape dismisses a plugin preview before interrupting the agent", async ({ page }) => {
test.setTimeout(120_000);
const agent = await seedMockAgentWorkspace({
repoPrefix: "plugin-popover-",
title: "Plugin preview keyboard regression",
model: "e2e-fast-stream",
});
try {
await withPopoverTimelinePlugin(async () => {
await openAgentRoute(page, agent);
await submitMessageWithButton(page, "Show the plugin preview.");
await agent.client.waitForFinish(agent.agentId, 30_000);
await expect(page.getByRole("button", { name: "Hover plugin preview" })).toBeVisible();
await submitMessageWithButton(page, "withhold synthetic user message until interrupted");
const stop = page.getByRole("button", { name: "Stop agent", exact: true });
await expect(stop).toBeVisible();
await openPreviewWhileTyping(page);
await dismissPreview(page);
await expect(stop).toBeVisible();
await expect(composerLocator(page)).toBeFocused();
await page.keyboard.press("Escape");
await expect(stop).not.toBeVisible();
await agent.client.waitForFinish(agent.agentId, 15_000);
});
} finally {
await agent.cleanup();
}
});
@@ -231,3 +231,46 @@ export async function attachPromptTimelineEvidence(page: Page, info: TestInfo):
contentType: "image/png",
});
}
const POPOVER_CLIENT_SOURCE = `import React, { useEffect, useRef } from "react";
import { z } from "zod";
function Preview() {
const panel = useRef(null);
useEffect(() => {
const close = (event) => {
if (event.key === "Escape" && panel.current.matches(":popover-open")) {
panel.current.hidePopover();
}
};
document.addEventListener("keydown", close, true);
return () => document.removeEventListener("keydown", close, true);
}, []);
return <div>
<button onMouseEnter={() => panel.current.showPopover()}>Hover plugin preview</button>
<div ref={panel} popover="manual" role="dialog" aria-label="Plugin preview">Plugin hover preview</div>
</div>;
}
export default function contribute(client) {
client.addTimelineTransformer({
id: "preview", query: { itemType: "user_message" },
transform({ item }) {
if (item.text !== "Show the plugin preview.") return;
return { items: [{ type: "plugin", kind: "preview", version: 1, data: {} }] };
},
});
client.addTimelineRenderer({
kind: "preview", version: 1, schema: z.object({}), Component: Preview,
});
return () => {};
}`;
/** Installs a timeline preview that owns Escape through the DOM Popover API. */
export async function withPopoverTimelinePlugin(run: () => Promise<void>): Promise<void> {
await withDirectoryPlugin({
pluginId: "timeline-popover-e2e",
source: POPOVER_CLIENT_SOURCE,
run,
});
}
@@ -40,7 +40,7 @@ import {
navigateToLastWorkspace,
useActiveWorkspaceSelection,
} from "@/stores/navigation-active-workspace-store";
import { dispatchTopWebOverlayKeyDown } from "@/lib/overlay-root";
import { dispatchTopWebOverlayKeyDown, keyBelongsToDomPopover } from "@/lib/overlay-root";
export function useKeyboardShortcuts({
enabled,
@@ -321,6 +321,10 @@ export function useKeyboardShortcuts({
return;
}
if (keyBelongsToDomPopover(event)) {
return;
}
// During IME composition, Enter confirms the candidate selection and must
// not route through global shortcuts like message send.
if (isImeComposingKeyboardEvent(event)) {
@@ -2,7 +2,11 @@ import React, { type RefCallback } from "react";
import { flushSync } from "react-dom";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { useWebOverlayRegistration } from "./overlay-root";
import {
hasOpenDomPopover,
keyBelongsToDomPopover,
useWebOverlayRegistration,
} from "./overlay-root";
function nextFrame(): Promise<void> {
return new Promise((resolve) => requestAnimationFrame(() => resolve()));
@@ -68,3 +72,35 @@ describe("useWebOverlayRegistration in the browser", () => {
expect(openerFocusCalls).toBe(1);
});
});
describe("hasOpenDomPopover", () => {
afterEach(() => {
document.body.replaceChildren();
});
it("is true only while a popover is shown", () => {
const panel = document.createElement("div");
panel.setAttribute("popover", "manual");
document.body.append(panel);
expect(hasOpenDomPopover()).toBe(false);
panel.showPopover();
expect(hasOpenDomPopover()).toBe(true);
panel.hidePopover();
expect(hasOpenDomPopover()).toBe(false);
});
});
describe("Escape with a plugin popover open", () => {
afterEach(() => {
document.body.replaceChildren();
});
it("leaves other keys to the shortcuts while a popover is open", () => {
const panel = document.createElement("div");
panel.setAttribute("popover", "manual");
document.body.append(panel);
panel.showPopover();
expect(keyBelongsToDomPopover({ key: "Enter" })).toBe(false);
expect(keyBelongsToDomPopover({ key: "Escape" })).toBe(true);
});
});
+25
View File
@@ -112,6 +112,31 @@ export function hasActiveWebOverlay(): boolean {
return getTopWebOverlay() !== undefined;
}
/**
* True while an element shown with the Popover API is open. Such a layer
* floats above the app like a registered overlay but lives outside the
* registry: plugins draw their panels this way, since they cannot register.
*/
export function hasOpenDomPopover(): boolean {
if (typeof document === "undefined") return false;
try {
return document.querySelector(":popover-open") !== null;
} catch {
// An engine without the Popover API rejects the selector, and has none open.
return false;
}
}
/**
* Whether a key belongs to an open popover rather than to the app's shortcuts.
* An open popover (a plugin's panel) is the top layer and closes on its own
* Escape listener, which runs after the shortcut engine's capture listener:
* Escape must not also interrupt the running agent.
*/
export function keyBelongsToDomPopover(event: Pick<KeyboardEvent, "key">): boolean {
return event.key === "Escape" && hasOpenDomPopover();
}
function getFocusableElements(scope: HTMLElement): HTMLElement[] {
const selector = [
"a[href]",