fix(coding-agent): restore focus to the live editor-slot owner when a fullscreen overlay closes
When /settings (or the Extensions/Agents dashboard) is open and a tool approval prompt fires, ExtensionUiController.showHookSelector swaps the editor out of editorContainer for the HookSelectorComponent. On exit, the overlay's done() called overlayHandle.hide() + setFocus(editor), both pointing at the editor captured as preFocus when the overlay opened — now no longer mounted. The visible approval prompt then sat unreachable: Up/Down/Enter/Esc routed to the unmounted editor and only Ctrl+C escaped (issue #3349). SelectorController now exposes focusActiveEditorArea(), which restores focus to editorContainer.children[0] (the live slot owner) or falls back to the editor. Wired into showSettingsSelector, showExtensionsDashboard, and showAgentsDashboard close paths after overlay.hide(). Tests: unit test verifying focusActiveEditorArea picks the live slot owner; TUI overlay-focus regression pinning the post-fix contract plus a 'pre-fix snapshot' test pinning the broken pre-fix behavior so the restore-from-preFocus assumption can't silently change. Fixes #3349
This commit is contained in:
roboomp
@@ -4,6 +4,7 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the TUI freezing when a tool approval prompt fires while `/settings` (or the Extensions/Agents dashboard) is open. The fullscreen overlay's close handler restored focus to the editor it had captured at open time, but `ExtensionUiController` had since swapped the editor out of the editor slot for the approval prompt — so on exit the visible prompt sat unreachable while keystrokes routed to the now-unmounted editor (no Enter/Up/Down/Esc response, only Ctrl+C escaped). `SelectorController` now restores focus to whatever currently owns the editor slot via a `focusActiveEditorArea()` helper, applied to settings, extensions dashboard, and agents dashboard close paths. ([#3349](https://github.com/can1357/oh-my-pi/issues/3349))
|
||||
- Fixed all extension loading silently failing on the cross-compiled `omp-darwin-arm64` release binary (downloaded directly or via a Homebrew tap wrapper) because `__computeBunfsPackageRoot` mis-handled `import.meta.dir = "//root/omp-darwin-arm64"`. Bun 1.3.14 reports `<bunfs-root>/<binary-name>` for the compiled entry's `import.meta.dir`, but the pre-fix function joined `metaDir + "packages"` and produced `/root/omp-darwin-arm64/packages` — the binary basename was baked into every bunfs path, so the TypeBox/legacy-pi shims and every `@oh-my-pi/pi-*` package-root override failed `existsSync` validation and `resolveCanonicalPiSpecifier` fell through to a bunfs `Bun.resolveSync` that also could not find the module. The function now detects the bunfs-root + binary-basename shape (`path.basename(path.dirname(metaDir)) === "root"`) and strips the trailing binary segment by slicing the original `metaDir`; the production bunfs shim join path also preserves Bun's bunfs-native `//root` / `B:\~BUN\root` prefix that `path.join` would otherwise collapse. ([#3329](https://github.com/can1357/oh-my-pi/issues/3329))
|
||||
- Fixed llama.cpp discovery to prefer per-model `/v1/models` `meta.n_ctx`/`meta.n_ctx_train` values, refresh selected models after lazy load, and bypass fresh-cache reuse so server restarts update context windows. ([#3310](https://github.com/can1357/oh-my-pi/issues/3310))
|
||||
- Fixed `task.maxConcurrency: 0` serializing subagent spawns instead of running them unbounded. The settings UI labels `0` as "Unlimited", but the session-scoped spawn `Semaphore` clamped `max` via `Math.max(1, max)`, so the second subagent body in a batch always waited for the first to release the seat. The constructor now treats `max <= 0` (and any non-finite input) as unbounded via `Number.POSITIVE_INFINITY`, matching the eval `parallel()`/`pipeline()` worker-pool semantics ([#3305](https://github.com/can1357/oh-my-pi/issues/3305)).
|
||||
|
||||
@@ -84,6 +84,22 @@ export class SelectorController {
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Restore keyboard focus to whatever currently owns the editor slot. The
|
||||
* slot can hold the editor itself or a hook selector/input/editor pushed
|
||||
* in by `ExtensionUiController` — e.g. an approval prompt that fired while
|
||||
* a fullscreen overlay was up. `overlayHandle.hide()` restores focus to
|
||||
* the component focused when the overlay opened, which is stale in that
|
||||
* case (the editor was swapped out): keys land on a hidden editor and the
|
||||
* visible prompt receives nothing (issue #3349). Call this after the
|
||||
* overlay hides to re-target focus at the visible slot owner.
|
||||
*/
|
||||
focusActiveEditorArea(): void {
|
||||
const visible = this.ctx.editorContainer.children[0] ?? this.ctx.editor;
|
||||
this.ctx.ui.setFocus(visible);
|
||||
}
|
||||
|
||||
/**
|
||||
* Shows a selector component in place of the editor.
|
||||
* @param create Factory that receives a `done` callback and returns the component and focus target
|
||||
@@ -109,7 +125,7 @@ export class SelectorController {
|
||||
let overlayHandle: OverlayHandle | undefined;
|
||||
const done = () => {
|
||||
overlayHandle?.hide();
|
||||
this.ctx.ui.setFocus(this.ctx.editor);
|
||||
this.focusActiveEditorArea();
|
||||
this.ctx.ui.requestRender();
|
||||
};
|
||||
const selector = new SettingsSelectorComponent(
|
||||
@@ -224,6 +240,7 @@ export class SelectorController {
|
||||
});
|
||||
dashboard.onClose = () => {
|
||||
overlay.hide();
|
||||
this.focusActiveEditorArea();
|
||||
this.ctx.ui.requestRender();
|
||||
};
|
||||
dashboard.onRequestRender = () => {
|
||||
@@ -251,6 +268,7 @@ export class SelectorController {
|
||||
});
|
||||
dashboard.onClose = () => {
|
||||
overlay.hide();
|
||||
this.focusActiveEditorArea();
|
||||
this.ctx.ui.requestRender();
|
||||
};
|
||||
dashboard.onRequestRender = () => {
|
||||
|
||||
+85
@@ -0,0 +1,85 @@
|
||||
import { beforeAll, describe, expect, it, vi } from "bun:test";
|
||||
import { SelectorController } from "@oh-my-pi/pi-coding-agent/modes/controllers/selector-controller";
|
||||
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types";
|
||||
|
||||
beforeAll(async () => {
|
||||
await initTheme();
|
||||
});
|
||||
|
||||
interface EditorSlot {
|
||||
children: unknown[];
|
||||
clear: () => void;
|
||||
addChild: (child: unknown) => void;
|
||||
}
|
||||
|
||||
function createEditorSlot(...initial: unknown[]): EditorSlot {
|
||||
return {
|
||||
children: [...initial],
|
||||
clear() {
|
||||
this.children = [];
|
||||
},
|
||||
addChild(child: unknown) {
|
||||
this.children.push(child);
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createCtx(slot: EditorSlot, editor: unknown) {
|
||||
const setFocus = vi.fn();
|
||||
const ctx = {
|
||||
editor,
|
||||
editorContainer: slot,
|
||||
ui: {
|
||||
setFocus,
|
||||
requestRender: vi.fn(),
|
||||
},
|
||||
} as unknown as InteractiveModeContext;
|
||||
return { ctx, setFocus };
|
||||
}
|
||||
|
||||
describe("SelectorController.focusActiveEditorArea", () => {
|
||||
// Regression for issue #3349: closing a fullscreen overlay (settings,
|
||||
// extensions dashboard, agents dashboard) while a hook selector / approval
|
||||
// prompt occupies the editor slot must restore focus to that prompt — not
|
||||
// to the editor that the prompt replaced. Pre-fix, the close handlers
|
||||
// hardcoded `setFocus(this.ctx.editor)`, leaving keystrokes routed to a
|
||||
// no-longer-mounted editor while the visible prompt sat unreachable.
|
||||
|
||||
it("focuses the editor when the slot has only the editor in it", () => {
|
||||
const editor = { id: "editor" };
|
||||
const slot = createEditorSlot(editor);
|
||||
const { ctx, setFocus } = createCtx(slot, editor);
|
||||
|
||||
new SelectorController(ctx).focusActiveEditorArea();
|
||||
|
||||
expect(setFocus).toHaveBeenCalledTimes(1);
|
||||
expect(setFocus).toHaveBeenCalledWith(editor);
|
||||
});
|
||||
|
||||
it("focuses the active hook-selector-style prompt when the slot holds it instead of the editor", () => {
|
||||
const editor = { id: "editor" };
|
||||
const approvalPrompt = { id: "approval-prompt" };
|
||||
// Mirrors `ExtensionUiController.showHookSelector`: the hook surface
|
||||
// clears the slot and replaces the editor with its prompt component.
|
||||
const slot = createEditorSlot(approvalPrompt);
|
||||
const { ctx, setFocus } = createCtx(slot, editor);
|
||||
|
||||
new SelectorController(ctx).focusActiveEditorArea();
|
||||
|
||||
expect(setFocus).toHaveBeenCalledTimes(1);
|
||||
expect(setFocus).toHaveBeenCalledWith(approvalPrompt);
|
||||
expect(setFocus).not.toHaveBeenCalledWith(editor);
|
||||
});
|
||||
|
||||
it("falls back to the editor when the slot is empty (defensive)", () => {
|
||||
const editor = { id: "editor" };
|
||||
const slot = createEditorSlot();
|
||||
const { ctx, setFocus } = createCtx(slot, editor);
|
||||
|
||||
new SelectorController(ctx).focusActiveEditorArea();
|
||||
|
||||
expect(setFocus).toHaveBeenCalledTimes(1);
|
||||
expect(setFocus).toHaveBeenCalledWith(editor);
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { type Component, type Focusable, type OverlayFocusOwner, TUI } from "@oh-my-pi/pi-tui";
|
||||
import { type Component, Container, type Focusable, type OverlayFocusOwner, TUI } from "@oh-my-pi/pi-tui";
|
||||
import type { Terminal, TerminalAppearance } from "@oh-my-pi/pi-tui/terminal";
|
||||
|
||||
class MinimalTerminal implements Terminal {
|
||||
@@ -152,4 +152,98 @@ describe("TUI overlay focus", () => {
|
||||
tui.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("hands focus to the live editor-slot owner after a fullscreen overlay closes (issue #3349)", () => {
|
||||
// Repro for issue #3349: opening /settings (a fullscreen overlay)
|
||||
// while a tool approval prompt fires lands the prompt component in
|
||||
// the editor slot. When the overlay closes, `overlayHandle.hide()`
|
||||
// restores focus to the preFocus captured at open time — the
|
||||
// (now-unmounted) editor. Pre-fix, the visible prompt received no
|
||||
// keystrokes and the TUI looked frozen. The SelectorController fix
|
||||
// follows `hide()` with `setFocus(editorContainer.children[0] ?? editor)`;
|
||||
// this test pins that pattern at the TUI level.
|
||||
const terminal = new MinimalTerminal();
|
||||
const tui = new TUI(terminal);
|
||||
|
||||
const editor = new FocusRecorder("editor");
|
||||
const editorContainer = new Container();
|
||||
editorContainer.addChild(editor);
|
||||
tui.addChild(editorContainer);
|
||||
tui.setFocus(editor);
|
||||
|
||||
try {
|
||||
tui.start();
|
||||
|
||||
// /settings opens a fullscreen overlay. preFocus captured = editor.
|
||||
const settingsOverlay = new FocusRecorder("settings");
|
||||
const handle = tui.showOverlay(settingsOverlay, { fullscreen: true });
|
||||
expect(tui.getFocused()).toBe(settingsOverlay);
|
||||
|
||||
// While settings is open, a tool approval prompt swaps the editor
|
||||
// slot to a hook-selector component. Focus snaps back to the
|
||||
// settings overlay because it owns the top of the overlay stack.
|
||||
const approvalPrompt = new FocusRecorder("approval");
|
||||
editorContainer.clear();
|
||||
editorContainer.addChild(approvalPrompt);
|
||||
tui.setFocus(approvalPrompt);
|
||||
expect(tui.getFocused()).toBe(settingsOverlay);
|
||||
|
||||
// User Esc's out of settings. Replicate the post-fix close path:
|
||||
// hide(), then setFocus on whatever owns the slot right now.
|
||||
handle.hide();
|
||||
const slotOwner = editorContainer.children[0] ?? editor;
|
||||
tui.setFocus(slotOwner);
|
||||
|
||||
// The visible approval prompt now receives input. Pre-fix the
|
||||
// hide()-only restore left focus on the stale editor.
|
||||
terminal.sendInput("\x1b[B");
|
||||
terminal.sendInput("\r");
|
||||
expect(tui.getFocused()).toBe(approvalPrompt);
|
||||
expect(approvalPrompt.inputs).toEqual(["\x1b[B", "\r"]);
|
||||
expect(editor.inputs).toEqual([]);
|
||||
} finally {
|
||||
tui.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("pre-fix snapshot: hide() alone restores focus to the stale editor, missing the live slot owner (issue #3349)", () => {
|
||||
// Companion to the test above: pin the exact pre-fix behavior so a
|
||||
// future refactor of `overlayHandle.hide()` cannot silently change the
|
||||
// contract that the SelectorController fix compensates for. `hide()`
|
||||
// restores focus to `preFocus` captured at open time — the editor —
|
||||
// regardless of what currently occupies the editor slot. The
|
||||
// SelectorController close handlers MUST call `focusActiveEditorArea()`
|
||||
// after `hide()`; this test demonstrates why.
|
||||
const terminal = new MinimalTerminal();
|
||||
const tui = new TUI(terminal);
|
||||
|
||||
const editor = new FocusRecorder("editor");
|
||||
const editorContainer = new Container();
|
||||
editorContainer.addChild(editor);
|
||||
tui.addChild(editorContainer);
|
||||
tui.setFocus(editor);
|
||||
|
||||
try {
|
||||
tui.start();
|
||||
|
||||
const settingsOverlay = new FocusRecorder("settings");
|
||||
const handle = tui.showOverlay(settingsOverlay, { fullscreen: true });
|
||||
|
||||
const approvalPrompt = new FocusRecorder("approval");
|
||||
editorContainer.clear();
|
||||
editorContainer.addChild(approvalPrompt);
|
||||
tui.setFocus(approvalPrompt);
|
||||
|
||||
// Close the overlay WITHOUT the SelectorController's follow-up
|
||||
// `focusActiveEditorArea()`. hide() restores focus to preFocus.
|
||||
handle.hide();
|
||||
|
||||
terminal.sendInput("\x1b[B");
|
||||
expect(tui.getFocused()).toBe(editor);
|
||||
expect(editor.inputs).toEqual(["\x1b[B"]);
|
||||
expect(approvalPrompt.inputs).toEqual([]);
|
||||
} finally {
|
||||
tui.stop();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user