From 27f4965e00b0fa5b09dbb798dcf2a05b29a4e58e Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 1 Jun 2026 17:26:10 +0000 Subject: [PATCH] fix(coding-agent): stop ESC from aborting agent run while @ autocomplete is open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/modes/components/custom-editor.ts | 16 ++++---- .../src/modes/controllers/input-controller.ts | 15 ------- .../test/custom-editor-keybindings.test.ts | 41 +++++++++++++++++++ .../test/input-controller-escape.test.ts | 5 --- .../test/input-controller-keybindings.test.ts | 1 - 6 files changed, 51 insertions(+), 28 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2ca63b66c..1dbe3e8d5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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. diff --git a/packages/coding-agent/src/modes/components/custom-editor.ts b/packages/coding-agent/src/modes/components/custom-editor.ts index bb97c1598..30445463f 100644 --- a/packages/coding-agent/src/modes/components/custom-editor.ts +++ b/packages/coding-agent/src/modes/components/custom-editor.ts @@ -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 diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index 5bca6d000..123bce98c 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -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(); diff --git a/packages/coding-agent/test/custom-editor-keybindings.test.ts b/packages/coding-agent/test/custom-editor-keybindings.test.ts index 23a246c2e..98fa4e4d5 100644 --- a/packages/coding-agent/test/custom-editor-keybindings.test.ts +++ b/packages/coding-agent/test/custom-editor-keybindings.test.ts @@ -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); + }); +}); diff --git a/packages/coding-agent/test/input-controller-escape.test.ts b/packages/coding-agent/test/input-controller-escape.test.ts index 5d2bd5dff..5231a1e3f 100644 --- a/packages/coding-agent/test/input-controller-escape.test.ts +++ b/packages/coding-agent/test/input-controller-escape.test.ts @@ -7,7 +7,6 @@ type StartPendingSubmissionSpy = Mock void; onSubmit?: (text: string) => Promise; - 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); diff --git a/packages/coding-agent/test/input-controller-keybindings.test.ts b/packages/coding-agent/test/input-controller-keybindings.test.ts index 1700b7ab5..4cc6aa421 100644 --- a/packages/coding-agent/test/input-controller-keybindings.test.ts +++ b/packages/coding-agent/test/input-controller-keybindings.test.ts @@ -4,7 +4,6 @@ import type { InteractiveModeContext } from "../src/modes/types"; type FakeEditor = { onEscape?: () => void; - shouldBypassAutocompleteOnEscape?: () => boolean; onClear?: () => void; onExit?: () => void; onSuspend?: () => void;