From 12aedd738884b31ee9729b6191d9cacdb49a6b92 Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 22 Jun 2026 17:10:49 +0000 Subject: [PATCH] fix(coding-agent): returned to ask options after custom escape Keep the ask tool on the current question when the custom answer editor is dismissed, so Escape from Other returns to the option selector instead of aborting or recording an empty answer. Fixes #3269 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/tools/ask.ts | 82 +++++++++++--------- packages/coding-agent/test/tools/ask.test.ts | 42 ++++++---- 3 files changed, 77 insertions(+), 51 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1c47a3a823..828ee11299 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `ask` returning `(cancelled)` or aborting the tool when Escape dismissed `Other (type your own)` custom input; it now returns to the option selector so the user can pick a listed answer instead. ([#3269](https://github.com/can1357/oh-my-pi/issues/3269)) + ## [16.1.15] - 2026-06-22 ### Added diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index 404ab7d6a9..86dfa6dd02 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -308,7 +308,7 @@ async function askSingleQuestion( } const customResult = await promptForCustomInput(); if (customResult.input === undefined) { - break; + continue; } customInput = customResult.input; break; @@ -332,51 +332,57 @@ async function askSingleQuestion( } selectedOptions = Array.from(selected); } else { - const displayOptions = addRecommendedSuffix(questionOptions, recommended); - const optionsWithNavigation: ExtensionUISelectItem[] = [...displayOptions, OTHER_OPTION]; - - let initialIndex = recommended; - const previouslySelected = selectedOptions[0]; - if (previouslySelected) { - const selectedIndex = questionOptions.findIndex(option => option.label === previouslySelected); - if (selectedIndex >= 0) initialIndex = selectedIndex; - } else if (customInput !== undefined) { - initialIndex = displayOptions.length; - } - if (initialIndex !== undefined) { - const maxIndex = Math.max(optionsWithNavigation.length - 1, 0); - initialIndex = Math.max(0, Math.min(initialIndex, maxIndex)); - } + while (true) { + const displayOptions = addRecommendedSuffix(questionOptions, recommended); + const optionsWithNavigation: ExtensionUISelectItem[] = [...displayOptions, OTHER_OPTION]; + + let initialIndex = recommended; + const previouslySelected = selectedOptions[0]; + if (previouslySelected) { + const selectedIndex = questionOptions.findIndex(option => option.label === previouslySelected); + if (selectedIndex >= 0) initialIndex = selectedIndex; + } else if (customInput !== undefined) { + initialIndex = displayOptions.length; + } + if (initialIndex !== undefined) { + const maxIndex = Math.max(optionsWithNavigation.length - 1, 0); + initialIndex = Math.max(0, Math.min(initialIndex, maxIndex)); + } - const { - choice, - timedOut: selectTimedOut, - navigation: arrowNavigation, - } = await selectOption(promptWithProgress, optionsWithNavigation, initialIndex, { - selectionMarker: "radio", - markableCount: displayOptions.length, - }); - timedOut = selectTimedOut; + const { + choice, + timedOut: selectTimedOut, + navigation: arrowNavigation, + } = await selectOption(promptWithProgress, optionsWithNavigation, initialIndex, { + selectionMarker: "radio", + markableCount: displayOptions.length, + }); + timedOut = selectTimedOut; - if (arrowNavigation) { - return { selectedOptions, customInput, timedOut, navigation: arrowNavigation }; - } - if (choice === undefined) { - if (!timedOut) { - return { selectedOptions, customInput, timedOut, cancelled: true }; + if (arrowNavigation) { + return { selectedOptions, customInput, timedOut, navigation: arrowNavigation }; + } + if (choice === undefined) { + if (!timedOut) { + return { selectedOptions, customInput, timedOut, cancelled: true }; + } + break; } - } else if (choice === OTHER_OPTION) { - if (!selectTimedOut) { + if (choice === OTHER_OPTION) { + if (selectTimedOut) { + break; + } const customResult = await promptForCustomInput(); - if (customResult.input !== undefined) { - customInput = customResult.input; - selectedOptions = []; + if (customResult.input === undefined) { + continue; } - // If editor was dismissed (undefined), keep prior selectedOptions/customInput intact + customInput = customResult.input; + selectedOptions = []; + break; } - } else { selectedOptions = [stripRecommendedSuffix(choice)]; customInput = undefined; + break; } if (navigation?.allowForward) { return { selectedOptions, customInput, timedOut, navigation: "forward" }; diff --git a/packages/coding-agent/test/tools/ask.test.ts b/packages/coding-agent/test/tools/ask.test.ts index b09f79ce37..50348e3322 100644 --- a/packages/coding-agent/test/tools/ask.test.ts +++ b/packages/coding-agent/test/tools/ask.test.ts @@ -496,7 +496,7 @@ describe("AskTool option descriptions", () => { step += 1; return selectItemLabel(options.find(o => selectItemLabel(o)?.endsWith("beta"))); } - return "Other (type your own)"; + return selectItemLabel(options.find(o => selectItemLabel(o)?.includes("Done selecting"))); }, editor, }); @@ -564,10 +564,11 @@ describe("AskTool custom input", () => { expect(abort).not.toHaveBeenCalled(); }); - it("aborts when editor is cancelled in single-question flow", async () => { + it("returns to the option selector when custom input is dismissed in single-question flow", async () => { const tool = new AskTool(createSession()); const abort = vi.fn(); const editor = vi.fn(async () => undefined); + let selectCalls = 0; const questions = [ { id: "details", @@ -576,22 +577,27 @@ describe("AskTool custom input", () => { }, ]; const context = createContext({ - select: async () => "Other (type your own)", + select: async () => { + selectCalls += 1; + return selectCalls === 1 ? "Other (type your own)" : "yes"; + }, editor, abort, }); - await expect( - tool.execute("call-editor-cancel", { questions }, undefined, undefined, context), - ).rejects.toBeInstanceOf(ToolAbortError); + const result = await tool.execute("call-editor-cancel", { questions }, undefined, undefined, context); + expect(result.details?.selectedOptions).toEqual(["yes"]); + expect(result.details?.customInput).toBeUndefined(); + expect(selectCalls).toBe(2); expect(editor).toHaveBeenCalledTimes(1); - expect(abort).toHaveBeenCalledTimes(1); + expect(abort).not.toHaveBeenCalled(); }); - it("continues multi-question flow when editor is dismissed on a fresh question", async () => { + it("returns to the option selector when custom input is dismissed in multi-question flow", async () => { const tool = new AskTool(createSession()); const abort = vi.fn(); const editor = vi.fn(async () => undefined); + let detailsVisits = 0; const questions = [ { id: "first", @@ -607,7 +613,10 @@ describe("AskTool custom input", () => { const context = createContext({ select: async prompt => { if (prompt.includes("First?")) return "one"; - if (prompt.includes("Details?")) return "Other (type your own)"; + if (prompt.includes("Details?")) { + detailsVisits += 1; + return detailsVisits === 1 ? "Other (type your own)" : "short"; + } return undefined; }, editor, @@ -616,10 +625,10 @@ describe("AskTool custom input", () => { const result = await tool.execute("call-editor-multi-dismiss", { questions }, undefined, undefined, context); - // Editor dismissed on "Details?" — flow continues with empty answer, not abort expect(result.details?.results?.[0]?.selectedOptions).toEqual(["one"]); - expect(result.details?.results?.[1]?.selectedOptions).toEqual([]); + expect(result.details?.results?.[1]?.selectedOptions).toEqual(["short"]); expect(result.details?.results?.[1]?.customInput).toBeUndefined(); + expect(detailsVisits).toBe(2); expect(editor).toHaveBeenCalledTimes(1); expect(abort).not.toHaveBeenCalled(); }); @@ -745,7 +754,7 @@ describe("AskTool custom input", () => { expect(renderedText).toContain("custom detail"); }); - it("preserves prior multi-select answers when custom editor is dismissed", async () => { + it("returns to the option selector when multi-select custom input is dismissed", async () => { const tool = new AskTool(createSession()); let step = 0; const editor = vi.fn(async () => undefined); @@ -757,7 +766,13 @@ describe("AskTool custom input", () => { if (!alphaOption) throw new Error("Missing alpha option"); return selectItemLabel(alphaOption); } - return "Other (type your own)"; + if (step === 1) { + step += 1; + return "Other (type your own)"; + } + const doneOption = options.find(option => selectItemLabel(option)?.includes("Done selecting")); + if (!doneOption) throw new Error("Missing done option"); + return selectItemLabel(doneOption); }, editor, }); @@ -786,6 +801,7 @@ describe("AskTool custom input", () => { throw new Error("Expected text result"); } expect(result.content[0].text).toContain("User selected: alpha"); + expect(step).toBe(2); expect(editor).toHaveBeenCalledTimes(1); }); });