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

Merge PR #8265: fix(tui): multi-select ask dialog submits on Enter instead of dead-ending (@zhang17-24)

Browse Source
This commit is contained in:
can1357
2026-08-16 02:13:40 +02:00
parent 7b22a2309c b245be11c5
commit f83111c8d4
5 changed files with 161 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -186,6 +186,10 @@
- Added `externalThinking` setting for private scratchpad reasoning via the new `think` tool
### 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
packages/coding-agent/src/modes/components/ask-dialog.ts
+11 -3
View File
@@ -614,7 +614,9 @@ 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";
// 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}`;
@@ -692,8 +694,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. 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)) {
state.selectedOptions.delete(option.label);
clearNoteIfRow(state, rowItem.key);
packages/coding-agent/src/tools/ask.ts
+9 -2
View File
@@ -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<typeof askSchema, AskToolDetails> {
}
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");
packages/coding-agent/test/modes/components/ask-dialog.test.ts
+100 -43
View File
@@ -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", () => {
@@ -596,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<string | undefined>();
@@ -933,7 +998,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 +1015,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);
packages/coding-agent/test/tools/ask.test.ts
+37
View File
@@ -1607,6 +1607,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();