smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #6240: fix(tui): lock plan review overlay after a choice commits (@roboomp)

Browse Source
This commit is contained in:
can1357
2026-07-22 21:13:18 +02:00
parent 2393f3fd1e 27b0d1722b
commit 1fecdb84fd
3 changed files with 58 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -219,6 +219,7 @@
- Fixed the transcript keeping finalized assistant blocks in the live compose walk after their rows entered native terminal scrollback, making each stream tick's `TranscriptContainer.render` depth-linear in session length. Fully committed finalized blocks are now compacted out of the local frame regardless of post-finalize version tracking; a later mutation no longer recommits on ordinary frames (no duplication) and rehydrates on the next destructive full replay (no loss). Compose cost for a live tail tick is now flat as depth grows (`bench/transcript-compose.bench.ts`: ratio(N5000/N500) 2.30 → 0.90) ([#5930](https://github.com/can1357/oh-my-pi/issues/5930)).
- Fixed `/quit` and `/exit` hanging during interactive shutdown by making the mnemopi dispose path retain the current session and flush in-flight extractions without sleeping the bank; the `/memory enqueue` path and end-of-session backend enqueue still perform full cross-session consolidation. ([#3641](https://github.com/can1357/oh-my-pi/issues/3641))
- Fixed interactive bash shortcut `cd` commands leaving the OMP session and status-line working directory unchanged.
- Fixed the fullscreen Plan Review overlay staying interactive with no feedback after a choice was picked: while the approval ran slow async work (e.g. context compaction) the overlay remained mounted, arrow keys still moved the cursor, and repeat Enter/Esc were silently swallowed, so users on Ghostty and macOS Terminal thought `/plan` was frozen. The overlay now locks input and shows a "submitting…" indicator the moment a choice commits ([#5926](https://github.com/can1357/oh-my-pi/issues/5926)).
## [17.0.3] - 2026-07-17
packages/coding-agent/src/modes/components/plan-review-overlay.ts
+20 -4
View File
@@ -152,6 +152,14 @@ export class PlanReviewOverlay implements Component {
* motion mouse reports and cleared when the pointer leaves the option rows. */
#hoveredOption: number | undefined;
// Once a choice fires, the promise-based caller resolves but keeps this
// overlay mounted while it runs slow async approval work (e.g. context
// compaction). Lock input and switch to a "submitting" indicator so the
// overlay stops looking interactive and repeat Enter/Esc are not silently
// swallowed with zero feedback (#5926).
#committed = false;
/** Label of the committed choice, shown while the async approval settles. */
#committedLabel: string | undefined;
#annotating = false;
#input: Input;
@@ -283,11 +291,14 @@ export class PlanReviewOverlay implements Component {
#confirmSelection(): void {
const index = this.#selectedIndex;
if (index >= 0 && index < this.#options.length && !this.#disabled.has(index)) {
this.#committed = true;
this.#committedLabel = this.#options[index]!;
this.callbacks.onPick(this.#options[index]!);
}
}
handleInput(keyData: string): void {
if (this.#committed) return;
if (keyData.startsWith("\x1b[<") && this.#handleMouse(keyData)) return;
if (this.#annotating) {
if (this.callbacks.onAnnotationExternalEditor && matchesAppExternalEditor(keyData)) {
@@ -300,6 +311,7 @@ export class PlanReviewOverlay implements Component {
return;
}
if (matchesSelectCancel(keyData)) {
this.#committed = true;
this.callbacks.onCancel();
return;
}
@@ -838,10 +850,14 @@ export class PlanReviewOverlay implements Component {
const innerWidth = Math.max(1, width - 4);
const bodyContentWidth = sidebarShown ? splitBodyWidth(width, sidebarWidth) : innerWidth;
const sliderLines = this.#renderSliderLines();
const optionLines = this.#renderOptionLines();
const committed = this.#committed;
const sliderLines = committed ? [] : this.#renderSliderLines();
const submittingLabel = this.#committedLabel ? `${this.#committedLabel} — submitting…` : "Submitting…";
const optionLines = committed ? [theme.bold(theme.fg("accent", submittingLabel))] : this.#renderOptionLines();
const promptLines = this.#promptTitle ? [theme.bold(theme.fg("accent", this.#promptTitle))] : [];
const footerLines = this.#renderFooterLines(innerWidth);
const footerLines = committed
? [theme.fg("dim", "Applying your selection — this can take a moment while context is prepared.")]
: this.#renderFooterLines(innerWidth);
// Chrome rows: top border, two dividers, bottom border, plus the
// prompt/slider/option/footer rows between them.
@@ -884,7 +900,7 @@ export class PlanReviewOverlay implements Component {
for (const line of promptLines) out.push(row(line, width));
for (const line of sliderLines) out.push(row(line, width));
for (let i = 0; i < optionLines.length; i++) {
this.#optionClickRows.set(out.length, i);
if (!committed) this.#optionClickRows.set(out.length, i);
out.push(row(optionLines[i]!, width));
}
out.push(divider(width));
packages/coding-agent/test/modes/components/plan-review-overlay.test.ts
+37 -6
View File
@@ -75,7 +75,7 @@ describe("PlanReviewOverlay", () => {
expect(onPick).toHaveBeenCalledWith("Approve and execute");
});
it("moves the option cursor with up/down and confirms the new target", () => {
it("moves the option cursor with down and confirms the new target", () => {
const onPick = vi.fn();
const overlay = new PlanReviewOverlay(
"plan",
@@ -85,13 +85,44 @@ describe("PlanReviewOverlay", () => {
overlay.handleInput(DOWN);
overlay.handleInput(ENTER);
expect(onPick).toHaveBeenCalledWith("Approve and compact context");
});
onPick.mockClear();
it("confirms the up-moved target from a lower start", () => {
const onPick = vi.fn();
const overlay = new PlanReviewOverlay(
"plan",
{ promptTitle: "next", options: APPROVAL_OPTIONS, initialIndex: 1 },
{ onPick, onCancel: vi.fn() },
);
overlay.handleInput(UP);
overlay.handleInput(ENTER);
expect(onPick).toHaveBeenCalledWith("Approve and execute");
});
it("locks input and shows a submitting indicator after a pick, ignoring repeat keys", () => {
const onPick = vi.fn();
const onCancel = vi.fn();
const overlay = new PlanReviewOverlay(
"plan",
{ promptTitle: "next", options: APPROVAL_OPTIONS },
{ onPick, onCancel },
);
overlay.handleInput(ENTER);
expect(onPick).toHaveBeenCalledTimes(1);
expect(onPick).toHaveBeenCalledWith("Approve and execute");
const committed = stripVTControlCharacters(overlay.render(80).join("\n"));
expect(committed).toContain("Approve and execute — submitting…");
// The async approval window keeps the overlay mounted; further keys must
// not fire a second callback or move the cursor (#5926).
overlay.handleInput(DOWN);
overlay.handleInput(ENTER);
overlay.handleInput("\x1b");
expect(onPick).toHaveBeenCalledTimes(1);
expect(onCancel).not.toHaveBeenCalled();
});
it("skips disabled options and never confirms them", () => {
const onPick = vi.fn();
// Disable index 2 ("Approve and keep context").
@@ -654,14 +685,14 @@ describe("PlanReviewOverlay", () => {
expect(hoverRow(overlay, "Approve and keep context")).toBe(true);
expect(optionLineRaw(overlay, "Approve and keep context")).toContain(selectedBg);
// Pointer onto the top border (a non-option row) drops the highlight.
overlay.handleInput("\x1b[<35;6;1M");
expect(optionLineRaw(overlay, "Approve and keep context")).not.toContain(selectedBg);
// Hover is visual only: the keyboard cursor stays on index 0, so Enter still
// confirms the first option rather than the hovered one.
overlay.handleInput(ENTER);
expect(onPick).toHaveBeenCalledWith("Approve and execute");
// Pointer onto the top border (a non-option row) drops the highlight.
overlay.handleInput("\x1b[<35;6;1M");
expect(optionLineRaw(overlay, "Approve and keep context")).not.toContain(selectedBg);
});
it("never hovers a disabled option", () => {