fix(coding-agent): stop ESC from aborting agent run while @ autocomplete is open

When the autocomplete popup is visible, ESC unconditionally falls through
to the editor base class so it can dismiss the popup. Only an ESC with no
popup visible reaches onEscape and routes to the global interrupt handler.

Removed the shouldBypassAutocompleteOnEscape callback that paved over the
popup-dismissal path whenever the agent was busy (streaming, bash, /btw,
auto-compaction, etc.) — that is precisely the moment the user is most
likely to hit ESC while the popup is open, and dismissing-then-aborting in
one keystroke is a footgun, not a feature. Two-press semantics now match
the standard TUI/IDE pattern: first ESC closes the popup, second ESC
aborts.

Tests cover both branches in custom-editor-keybindings.test.ts and the
input-controller escape suite no longer asserts the dead callback.

Fixes #1655
This commit is contained in:
roboomp
2026-06-01 17:26:10 +00:00
parent fbdc064186
commit 27f4965e00
6 changed files with 51 additions and 28 deletions
+1
View File
@@ -9,6 +9,7 @@
### Fixed
- Fixed a single ESC press both dismissing the @/slash autocomplete popup and aborting the running agent operation. ESC now drains the popup first; only when no popup is visible does it route to the global interrupt handler (matching the standard TUI/IDE pattern). The `shouldBypassAutocompleteOnEscape` editor hook is removed — it had become the trigger for this bug ([#1655](https://github.com/can1357/oh-my-pi/issues/1655)).
- Fixed Claude Code slash command discovery to load subdirectory commands recursively while preserving basename commands (e.g. `/apply`) and adding namespace aliases (e.g. `/opsx:apply`) for tools that install colon-namespaced workflows ([#1523](https://github.com/can1357/oh-my-pi/issues/1523)).
- Fixed the `eval` tool's per-cell `timeout` killing cells that were not stalled. The timeout is now a plain wall-clock budget on the cell's **own** work that is **paused only while a host-side `agent()`/`parallel()`/`llm()` bridge call is in flight** — those calls pump a heartbeat that re-arms the watchdog, so a long fanout or a slow (e.g. reasoning-tier) completion runs to completion instead of being aborted mid-flight (a subagent's time-to-first-token, a long quiet nested tool, or an entire oneshot `llm()` request no longer trip it). Nothing else re-arms the budget: ordinary compute, `print`/stdout, `log()`/`phase()`, and non-agent tool calls all count against it, so a cell that is not delegating to an agent/llm is bounded by the regular wall-clock timeout (and the timeout message no longer says "of inactivity"). The heartbeat is a pure keepalive — never persisted or rendered.
@@ -49,7 +49,6 @@ export class CustomEditor extends Editor {
* them, skipping any occurrence inside code spans, fenced blocks, or XML sections. */
decorateText = (text: string): string => highlightMagicKeywords(text);
onEscape?: () => void;
shouldBypassAutocompleteOnEscape?: () => boolean;
onClear?: () => void;
onExit?: () => void;
onCycleThinkingLevel?: () => void;
@@ -186,12 +185,15 @@ export class CustomEditor extends Editor {
}
// Intercept configured interrupt shortcut.
// Default behavior keeps autocomplete dismissal, but parent can prioritize global interrupt handling.
if (this.#matchesAction(data, "app.interrupt") && this.onEscape) {
if (!this.isShowingAutocomplete() || this.shouldBypassAutocompleteOnEscape?.()) {
this.onEscape();
return;
}
// When the autocomplete popup is visible, ESC's first job is to dismiss
// the popup — let super.handleInput() route it to #cancelAutocomplete().
// The user can press ESC again afterward to fire the global interrupt
// handler. This matches the standard TUI/IDE pattern and prevents a
// single ESC from both closing an @ completion and aborting an active
// agent run (#1655).
if (this.#matchesAction(data, "app.interrupt") && this.onEscape && !this.isShowingAutocomplete()) {
this.onEscape();
return;
}
// Intercept configured clear shortcut
@@ -86,21 +86,6 @@ export class InputController {
setupKeyHandlers(): void {
this.ctx.editor.setActionKeys("app.interrupt", this.ctx.keybindings.getKeys("app.interrupt"));
this.ctx.editor.shouldBypassAutocompleteOnEscape = () =>
Boolean(
this.ctx.loadingAnimation ||
this.ctx.hasActiveBtw() ||
this.ctx.hasActiveOmfg() ||
this.ctx.session.isStreaming ||
this.ctx.session.isCompacting ||
this.ctx.session.isGeneratingHandoff ||
this.ctx.session.isBashRunning ||
this.ctx.session.isEvalRunning ||
this.ctx.autoCompactionLoader ||
this.ctx.retryLoader ||
this.ctx.autoCompactionEscapeHandler ||
this.ctx.retryEscapeHandler,
);
this.ctx.editor.onEscape = () => {
if (this.ctx.loopModeEnabled) {
this.ctx.pauseLoop();
@@ -47,3 +47,44 @@ describe("CustomEditor temporary model selector keybinding", () => {
expect(onSelectModelTemporary).toHaveBeenCalledTimes(1);
});
});
describe("CustomEditor escape key dispatch", () => {
function installAutocompleteProvider(editor: CustomEditor) {
editor.setAutocompleteProvider({
async getSuggestions() {
return { items: [{ label: "src/", value: "src/" }], prefix: "@" };
},
applyCompletion(lines, cursorLine, cursorCol) {
return { lines, cursorLine, cursorCol };
},
});
}
it("dismisses the autocomplete popup on the first ESC and only fires onEscape on the second", async () => {
const editor = createEditor();
const onEscape = vi.fn();
editor.onEscape = onEscape;
installAutocompleteProvider(editor);
editor.handleInput("@");
// Yield so the async provider populates and the popup opens.
await Bun.sleep(0);
expect(editor.isShowingAutocomplete()).toBe(true);
editor.handleInput("\x1b");
expect(editor.isShowingAutocomplete()).toBe(false);
expect(onEscape).not.toHaveBeenCalled();
editor.handleInput("\x1b");
expect(onEscape).toHaveBeenCalledTimes(1);
});
it("fires onEscape immediately when no autocomplete popup is visible", () => {
const editor = createEditor();
const onEscape = vi.fn();
editor.onEscape = onEscape;
editor.handleInput("\x1b");
expect(onEscape).toHaveBeenCalledTimes(1);
});
});
@@ -7,7 +7,6 @@ type StartPendingSubmissionSpy = Mock<InteractiveModeContext["startPendingSubmis
type FakeEditor = {
onEscape?: () => void;
onSubmit?: (text: string) => Promise<void>;
shouldBypassAutocompleteOnEscape?: () => boolean;
onClear?: () => void;
onExit?: () => void;
onSuspend?: () => void;
@@ -200,7 +199,6 @@ describe("InputController escape behavior", () => {
expect(spies.startPendingSubmission).toHaveBeenCalledWith({ text: "hello", images: undefined });
expect(spies.onInputCallback).toHaveBeenCalledWith(submission);
expect(editor.shouldBypassAutocompleteOnEscape?.()).toBe(true);
editor.onEscape?.();
expect(spies.cancelPendingSubmission).toHaveBeenCalledTimes(1);
@@ -269,7 +267,6 @@ describe("InputController escape behavior", () => {
const controller = new InputController(ctx);
controller.setupKeyHandlers();
expect(editor.shouldBypassAutocompleteOnEscape?.()).toBe(true);
editor.onEscape?.();
expect(spies.handleBtwEscape).toHaveBeenCalledTimes(1);
@@ -283,7 +280,6 @@ describe("InputController escape behavior", () => {
const controller = new InputController(ctx);
controller.setupKeyHandlers();
expect(editor.shouldBypassAutocompleteOnEscape?.()).toBe(true);
editor.onEscape?.();
expect(spies.handleBtwEscape).toHaveBeenCalledTimes(1);
@@ -299,7 +295,6 @@ describe("InputController escape behavior", () => {
const controller = new InputController(ctx);
controller.setupKeyHandlers();
expect(editor.shouldBypassAutocompleteOnEscape?.()).toBe(true);
editor.onEscape?.();
expect(spies.handleBtwEscape).toHaveBeenCalledTimes(1);
@@ -4,7 +4,6 @@ import type { InteractiveModeContext } from "../src/modes/types";
type FakeEditor = {
onEscape?: () => void;
shouldBypassAutocompleteOnEscape?: () => boolean;
onClear?: () => void;
onExit?: () => void;
onSuspend?: () => void;