From b3b95e769f9c2d74276da74e736e6de97862ca55 Mon Sep 17 00:00:00 2001 From: left-to-right <2201218482@qq.com> Date: Wed, 12 Aug 2026 01:08:25 +0800 Subject: [PATCH 1/2] fix(tui): multi-select ask dialog submits on Enter instead of dead-ending Enter in a multi-select ask question toggled the focused option exactly like Space, with the only submit path hidden behind the Submit tab that appears only for multi/multi-question dialogs. Enter now submits the current selection (matching single-select), Space still toggles, and the footer hint reflects it. Fixes #8252 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/modes/components/ask-dialog.ts | 12 ++- .../test/modes/components/ask-dialog.test.ts | 81 +++++++++---------- 3 files changed, 48 insertions(+), 46 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a53f31baf..631973d60 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -31,6 +31,7 @@ ### Fixed +- Fixed the ask dialog's multi-select mode dead-ending on Enter: Space toggles options and Enter submits the current selection (matching single-select), instead of Enter silently toggling the focused option with the only submit path hidden behind the Submit tab ([#8252](https://github.com/can1357/oh-my-pi/issues/8252)). - Fixed `/usage`, `/advisor status`, and every other panel command answering only after the agent stopped working. Since `17.0.1` their output was queued until the turn settled (to stop mid-turn transcript mounts duplicating rows in native scrollback, issues #4806/#6767), and the deferral was silent, so on a long turn the command was indistinguishable from a dead one. The panel now renders immediately above the editor in an anchored container that is cleared and rebuilt in place, never entering the transcript, and the full output still lands in the transcript at the next settle. The preview is capped to 40% of the viewport (minimum 6 rows) so a tall report cannot push the prompt off screen. - Fixed the todo panel showing no progress while the agent worked through a plan: every sub-todo read as unchecked no matter how far along the run was. Three causes, all in the collapsed (default) view — the walking viewport dropped *every* closed row, so a completion only ever removed a line and the card's strike-reveal animation ran against a row nobody rendered; the phase the agent was actually in was the one phase header rendered without a `done/total` count; and the 60s todo auto-clear deleted closed tasks from an unfinished plan, resetting the phase counter to `0/n` and renumbering the stages until the next `todo` call restored the real snapshot. The viewport now keeps the newest closed task as a checked lead row (additive to the open-task cap), every phase header carries its progress, counts include abandoned tasks, and auto-clear only fires once the whole list is settled. - Status-line `usage` now renders monthly Cursor quotas (`mo N%`) in addition to the existing `5h` / `7d` windows ([#7998](https://github.com/can1357/oh-my-pi/pull/7998) by [@dnth](https://github.com/dnth)). diff --git a/packages/coding-agent/src/modes/components/ask-dialog.ts b/packages/coding-agent/src/modes/components/ask-dialog.ts index 80ed90e3c..77addd389 100644 --- a/packages/coding-agent/src/modes/components/ask-dialog.ts +++ b/packages/coding-agent/src/modes/components/ask-dialog.ts @@ -602,7 +602,7 @@ export class AskDialogComponent implements Component { return `Enter submit · ↑/↓ scroll ·${scroll} ${cancel}`; } const question = this.#questions[this.#currentQuestionIndex()]; - const action = question?.multi ? "Space/Enter toggle · n note" : "Enter select · n note"; + const action = question?.multi ? "Space toggle · Enter submit" : "Enter select · n note"; const tabs = this.#hasSubmitTab() ? " · Tab/←/→" : ""; if (this.#questionCanPage && indicator) { return `${action} · ↑/↓${tabs} · ${cancel} · ${pageKeysLabel()} ${indicator}`; @@ -680,8 +680,14 @@ export class AskDialogComponent implements Component { const option = question.options[rowItem.optionIndex ?? -1]; if (!option) return; if (question.multi) { - // Multi is toggle-only: Enter and Space both toggle, and the - // answer is confirmed from the Submit tab. + if (isEnter) { + // Enter confirms the current selection without toggling the + // focused option; Space toggles. Matches single-select + // Enter-to-submit so the multi dialog never dead-ends on an + // undiscoverable Submit tab (#8252). + this.#finishSubmit(); + return; + } if (state.selectedOptions.has(option.label)) { state.selectedOptions.delete(option.label); clearNoteIfRow(state, rowItem.key); diff --git a/packages/coding-agent/test/modes/components/ask-dialog.test.ts b/packages/coding-agent/test/modes/components/ask-dialog.test.ts index aa5251d59..8ac52200a 100644 --- a/packages/coding-agent/test/modes/components/ask-dialog.test.ts +++ b/packages/coding-agent/test/modes/components/ask-dialog.test.ts @@ -187,7 +187,7 @@ describe("AskDialogComponent", () => { ]); }); - it("multi-select: Space and Enter both toggle without advancing; Submit tab confirms", () => { + it("multi-select: Space toggles options; Enter submits the current selection", () => { const onSubmit = vi.fn(); const onCancel = vi.fn(); const onPrompt = vi.fn(); @@ -207,22 +207,24 @@ describe("AskDialogComponent", () => { onPrompt, }); - // Space on Option A - toggles without advancing + // Space toggles without submitting. component.handleInput(SPACE); expect(onSubmit).not.toHaveBeenCalled(); - // Down to Option B, Enter - toggles B, still no submit and no movement - component.handleInput(DOWN); - component.handleInput(ENTER); + // Space again toggles the same option back off. + component.handleInput(SPACE); expect(onSubmit).not.toHaveBeenCalled(); - // Tab to the Submit tab (present even for a single multi question), - // Enter confirms the selection. - component.handleInput(TAB); + // Space once more re-selects it. + component.handleInput(SPACE); + expect(onSubmit).not.toHaveBeenCalled(); + + // Enter submits the current selection without toggling the focused + // option (issue #8252). component.handleInput(ENTER); expect(onSubmit).toHaveBeenCalledTimes(1); - expect(onSubmit.mock.calls[0][0].results[0].selectedOptions).toEqual(["Option A", "Option B"]); + expect(onSubmit.mock.calls[0][0].results[0].selectedOptions).toEqual(["Option A"]); }); it("tab-state persistence: answer question 0, Tab forward, Tab back, answer still present", () => { @@ -933,7 +935,7 @@ describe("AskDialogComponent", () => { } }); - it("single-question multi-select: Enter toggles instead of submitting", () => { + it("single-question multi-select: Enter submits the current selection immediately", () => { const onSubmit = vi.fn(); const questions: ExtensionAskDialogQuestion[] = [ { @@ -950,42 +952,34 @@ describe("AskDialogComponent", () => { onPrompt: vi.fn(), }); - // Enter on Option B toggles it — no submit, no tab movement. - component.handleInput(DOWN); - component.handleInput(ENTER); - expect(onSubmit).not.toHaveBeenCalled(); - - // The toggle registered: Submit tab confirms only Option B. - component.handleInput(TAB); - component.handleInput(ENTER); - expect(onSubmit).toHaveBeenCalledTimes(1); - expect(onSubmit.mock.calls[0][0].results[0].selectedOptions).toEqual(["Option B"]); - }); - - it("multi-select: Enter on a checked option toggles it off; empty answer submits from Submit tab", () => { - const onSubmit = vi.fn(); - const questions: ExtensionAskDialogQuestion[] = [ - { - id: "q1", - question: "Choose multiple?", - options: [{ label: "Option A" }, { label: "Option B" }], - multi: true, - }, - ]; - - const component = new AskDialogComponent(questions, { - onSubmit, - onCancel: vi.fn(), - onPrompt: vi.fn(), - }); - - // Space checks Option A, Enter on the same row unchecks it. + // Space selects Option A; Enter submits right away — no need to + // discover the Submit tab (issue #8252). component.handleInput(SPACE); component.handleInput(ENTER); - // Submit tab warns about the unanswered question but still submits. - component.handleInput(TAB); - expect(render(component).toLowerCase()).toContain("unanswered"); + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0].results[0].selectedOptions).toEqual(["Option A"]); + }); + + it("multi-select: Enter submits an empty selection instead of dead-ending", () => { + const onSubmit = vi.fn(); + const questions: ExtensionAskDialogQuestion[] = [ + { + id: "q1", + question: "Choose multiple?", + options: [{ label: "Option A" }, { label: "Option B" }], + multi: true, + }, + ]; + + const component = new AskDialogComponent(questions, { + onSubmit, + onCancel: vi.fn(), + onPrompt: vi.fn(), + }); + + // Enter with nothing selected submits the empty selection rather than + // toggling or blocking on the Submit tab. component.handleInput(ENTER); expect(onSubmit).toHaveBeenCalledTimes(1); @@ -1385,3 +1379,4 @@ describe("AskDialogComponent", () => { expect(result.selectedOptions).toEqual(["Option A"]); }); }); + From b245be11c51cb3c7c5e5e491d61407d368aa7351 Mon Sep 17 00:00:00 2001 From: left-to-right <2201218482@qq.com> Date: Wed, 12 Aug 2026 22:58:02 +0800 Subject: [PATCH 2/2] fix(ask): address review comments on multi-select Enter submit - Multi-question dialogs now advance on Enter in multi-select mode instead of submitting every question at once (roboomp blocking) - Ask tool accepts an empty multi-select submission as a "select none" answer instead of aborting as a cancellation (codex P2) - Footer hint reflects advance vs submit for multi-question dialogs - Move the #8252 changelog entry to Unreleased (codex P2) Co-Authored-By: Claude Opus 4.7 --- packages/coding-agent/CHANGELOG.md | 5 +- .../src/modes/components/ask-dialog.ts | 12 ++-- packages/coding-agent/src/tools/ask.ts | 11 +++- .../test/modes/components/ask-dialog.test.ts | 64 ++++++++++++++++++- packages/coding-agent/test/tools/ask.test.ts | 37 +++++++++++ 5 files changed, 120 insertions(+), 9 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 631973d60..3acfccf5c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the ask dialog's multi-select mode dead-ending on Enter: Space toggles options and Enter submits the current selection (matching single-select), instead of Enter silently toggling the focused option with the only submit path hidden behind the Submit tab ([#8252](https://github.com/can1357/oh-my-pi/issues/8252)). + ## [17.2.13] - 2026-08-11 ### Added @@ -31,7 +35,6 @@ ### Fixed -- Fixed the ask dialog's multi-select mode dead-ending on Enter: Space toggles options and Enter submits the current selection (matching single-select), instead of Enter silently toggling the focused option with the only submit path hidden behind the Submit tab ([#8252](https://github.com/can1357/oh-my-pi/issues/8252)). - Fixed `/usage`, `/advisor status`, and every other panel command answering only after the agent stopped working. Since `17.0.1` their output was queued until the turn settled (to stop mid-turn transcript mounts duplicating rows in native scrollback, issues #4806/#6767), and the deferral was silent, so on a long turn the command was indistinguishable from a dead one. The panel now renders immediately above the editor in an anchored container that is cleared and rebuilt in place, never entering the transcript, and the full output still lands in the transcript at the next settle. The preview is capped to 40% of the viewport (minimum 6 rows) so a tall report cannot push the prompt off screen. - Fixed the todo panel showing no progress while the agent worked through a plan: every sub-todo read as unchecked no matter how far along the run was. Three causes, all in the collapsed (default) view — the walking viewport dropped *every* closed row, so a completion only ever removed a line and the card's strike-reveal animation ran against a row nobody rendered; the phase the agent was actually in was the one phase header rendered without a `done/total` count; and the 60s todo auto-clear deleted closed tasks from an unfinished plan, resetting the phase counter to `0/n` and renumbering the stages until the next `todo` call restored the real snapshot. The viewport now keeps the newest closed task as a checked lead row (additive to the open-task cap), every phase header carries its progress, counts include abandoned tasks, and auto-clear only fires once the whole list is settled. - Status-line `usage` now renders monthly Cursor quotas (`mo N%`) in addition to the existing `5h` / `7d` windows ([#7998](https://github.com/can1357/oh-my-pi/pull/7998) by [@dnth](https://github.com/dnth)). diff --git a/packages/coding-agent/src/modes/components/ask-dialog.ts b/packages/coding-agent/src/modes/components/ask-dialog.ts index 77addd389..179a0a0f2 100644 --- a/packages/coding-agent/src/modes/components/ask-dialog.ts +++ b/packages/coding-agent/src/modes/components/ask-dialog.ts @@ -602,7 +602,9 @@ export class AskDialogComponent implements Component { return `Enter submit · ↑/↓ scroll ·${scroll} ${cancel}`; } const question = this.#questions[this.#currentQuestionIndex()]; - const action = question?.multi ? "Space toggle · Enter submit" : "Enter select · n note"; + // Enter advances in multi-question dialogs and submits single-question ones. + const enterAction = this.#hasSubmitTab() ? "next" : "submit"; + const action = question?.multi ? `Space toggle · Enter ${enterAction}` : "Enter select · n note"; const tabs = this.#hasSubmitTab() ? " · Tab/←/→" : ""; if (this.#questionCanPage && indicator) { return `${action} · ↑/↓${tabs} · ${cancel} · ${pageKeysLabel()} ${indicator}`; @@ -682,10 +684,10 @@ export class AskDialogComponent implements Component { if (question.multi) { if (isEnter) { // Enter confirms the current selection without toggling the - // focused option; Space toggles. Matches single-select - // Enter-to-submit so the multi dialog never dead-ends on an - // undiscoverable Submit tab (#8252). - this.#finishSubmit(); + // focused option; Space toggles. Advances to the next question + // (submitting only for a single-question dialog), matching + // single-select Enter (#8252). + this.#advanceAfterQuestion(); return; } if (state.selectedOptions.has(option.label)) { diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index 0730a207c..2781a24c3 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -753,7 +753,8 @@ function formatSingleQuestionResponse(result: { : `User added note: ${result.note}`, ); } - return responseParts.length > 0 ? responseParts.join("\n") : "User cancelled the selection"; + if (responseParts.length > 0) return responseParts.join("\n"); + return result.multi ? "User did not select any options" : "User cancelled the selection"; } // ============================================================================= @@ -949,9 +950,15 @@ export class AskTool implements AgentTool { } if (params.questions.length === 1) { const result = results[0]; + // An empty multi-select submission is a valid "select none" + // answer (#8265 review); only a truly empty single-select + // result counts as cancellation. if ( !result || - (!result.timedOut && result.selectedOptions.length === 0 && result.customInput === undefined) + (!result.timedOut && + !result.multi && + result.selectedOptions.length === 0 && + result.customInput === undefined) ) { context.abort(); throw new ToolAbortError("Ask tool was cancelled by the user"); diff --git a/packages/coding-agent/test/modes/components/ask-dialog.test.ts b/packages/coding-agent/test/modes/components/ask-dialog.test.ts index 8ac52200a..c464403d6 100644 --- a/packages/coding-agent/test/modes/components/ask-dialog.test.ts +++ b/packages/coding-agent/test/modes/components/ask-dialog.test.ts @@ -598,6 +598,69 @@ describe("AskDialogComponent", () => { expect(onSubmit.mock.calls[0][0].results[0].customInput).toBe("custom detail"); }); + it("multi-question, multi-select: Enter on a plain option advances, does not submit", () => { + const onSubmit = vi.fn(); + const onCancel = vi.fn(); + const onPrompt = vi.fn(); + + const questions: ExtensionAskDialogQuestion[] = [ + { + id: "q1", + question: "Choose multiple?", + options: [{ label: "Option A" }, { label: "Option B" }], + multi: true, + }, + { + id: "q2", + question: "Second question?", + options: [{ label: "Option C" }, { label: "Option D" }], + }, + ]; + + const component = new AskDialogComponent(questions, { + onSubmit, + onCancel, + onPrompt, + }); + + // Space toggles Option A; Enter on the plain option row confirms and + // advances to Q2 instead of submitting the whole dialog (#8265 review). + component.handleInput(SPACE); + component.handleInput(ENTER); + expect(onSubmit).not.toHaveBeenCalled(); + + // On Q2: Down to Option D and Enter advances to the Submit tab. + component.handleInput(DOWN); + component.handleInput(ENTER); + expect(onSubmit).not.toHaveBeenCalled(); + + // On the Submit tab Enter submits once with both answers. + component.handleInput(ENTER); + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit.mock.calls[0][0].results).toEqual([ + { + id: "q1", + question: "Choose multiple?", + options: ["Option A", "Option B"], + multi: true, + selectedOptions: ["Option A"], + customInput: undefined, + note: undefined, + timedOut: undefined, + }, + { + id: "q2", + question: "Second question?", + options: ["Option C", "Option D"], + multi: false, + selectedOptions: ["Option D"], + customInput: undefined, + note: undefined, + timedOut: undefined, + }, + ]); + }); + it("defers a timeout that fires during a pending prompt and honors the resolved custom input", async () => { vi.useFakeTimers(); const deferred = Promise.withResolvers(); @@ -1379,4 +1442,3 @@ describe("AskDialogComponent", () => { expect(result.selectedOptions).toEqual(["Option A"]); }); }); - diff --git a/packages/coding-agent/test/tools/ask.test.ts b/packages/coding-agent/test/tools/ask.test.ts index 6a5688d26..5f9c76633 100644 --- a/packages/coding-agent/test/tools/ask.test.ts +++ b/packages/coding-agent/test/tools/ask.test.ts @@ -1619,6 +1619,43 @@ describe("AskTool rich ask dialog", () => { expect(abort).toHaveBeenCalledTimes(1); }); + it("accepts an empty multi-select submission instead of aborting", async () => { + const tool = new AskTool(createSession()); + const abort = vi.fn(); + const askDialog = vi.fn().mockResolvedValue({ + kind: "submit", + results: [ + { + id: "q1", + question: "Choose?", + options: ["A", "B"], + multi: true, + selectedOptions: [], + customInput: undefined, + timedOut: undefined, + }, + ], + }); + const context = createContext({ askDialog, abort }); + + const result = await tool.execute( + "call-empty-multi", + { + questions: [{ id: "q1", question: "Choose?", options: [{ label: "A" }, { label: "B" }], multi: true }], + }, + undefined, + undefined, + context, + ); + + expect(abort).not.toHaveBeenCalled(); + expect(result.details?.selectedOptions).toEqual([]); + expect(result.content[0]?.type).toBe("text"); + if (result.content[0]?.type === "text") { + expect(stripAnsi(result.content[0].text)).toContain("User did not select any options"); + } + }); + it("returns chat redirect result when askDialog returns kind chat", async () => { const tool = new AskTool(createSession()); const abort = vi.fn();