diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6c5d526e2..ff2122782 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed the ask tool timeout so it auto-selects the recommended option even when the UI selector does not settle on its own. ([#4995](https://github.com/can1357/oh-my-pi/issues/4995)) + ## [16.3.15] - 2026-07-09 ### Changed diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index 42de7f930..c58ef975a 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -432,10 +432,16 @@ async function askSingleQuestion( const helpText = navigation ? "up/down navigate enter select ←/→ question esc cancel" : "up/down navigate enter select esc cancel"; + const timeoutController = typeof timeout === "number" && timeout > 0 ? new AbortController() : undefined; + const dialogSignal = + signal && timeoutController + ? AbortSignal.any([signal, timeoutController.signal]) + : (timeoutController?.signal ?? signal); + let timeoutId: NodeJS.Timeout | undefined; const dialogOptions = { initialIndex, timeout, - signal, + signal: dialogSignal, outline: true, onTimeout, helpText, @@ -454,18 +460,33 @@ async function askSingleQuestion( : undefined, }; const startMs = Date.now(); - const choice = signal - ? await untilAborted(signal, () => ui.select(prompt, optionsToShow, dialogOptions)) - : await ui.select(prompt, optionsToShow, dialogOptions); - if (!timeoutTriggered && choice === undefined && typeof timeout === "number") { - // Fallback for UI surfaces that enforce `timeout` without invoking - // `onTimeout`: their auto-cancel resolves right at the deadline. A - // cancel arriving well past the deadline is a deliberate user Esc on - // a surface that kept the dialog open — keep treating it as a cancel. - const elapsed = Date.now() - startMs; - timeoutTriggered = elapsed >= timeout && elapsed <= timeout + TIMEOUT_DETECTION_TOLERANCE_MS; + if (timeoutController && typeof timeout === "number") { + timeoutId = setTimeout(() => { + timeoutTriggered = true; + timeoutController.abort(); + }, timeout); + } + try { + const choice = dialogSignal + ? await untilAborted(dialogSignal, () => ui.select(prompt, optionsToShow, dialogOptions)) + : await ui.select(prompt, optionsToShow, dialogOptions); + if (!timeoutTriggered && choice === undefined && typeof timeout === "number") { + // Fallback for UI surfaces that enforce `timeout` without invoking + // `onTimeout`: their auto-cancel resolves right at the deadline. A + // cancel arriving well past the deadline is a deliberate user Esc on + // a surface that kept the dialog open — keep treating it as a cancel. + const elapsed = Date.now() - startMs; + timeoutTriggered = elapsed >= timeout && elapsed <= timeout + TIMEOUT_DETECTION_TOLERANCE_MS; + } + return { choice, timedOut: timeoutTriggered, navigation: navigationAction }; + } catch (error) { + if (timeoutTriggered && error instanceof Error && error.name === "AbortError") { + return { choice: undefined, timedOut: true, navigation: navigationAction }; + } + throw error; + } finally { + clearTimeout(timeoutId); } - return { choice, timedOut: timeoutTriggered, navigation: navigationAction }; }; const promptForCustomInput = async ( diff --git a/packages/coding-agent/test/ask-timeout.test.ts b/packages/coding-agent/test/ask-timeout.test.ts new file mode 100644 index 000000000..ebbf14f67 --- /dev/null +++ b/packages/coding-agent/test/ask-timeout.test.ts @@ -0,0 +1,91 @@ +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import type { AgentToolContext, AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import { getThemeByName, setThemeInstance } from "../src/modes/theme/theme"; +import type { ToolSession } from "../src/tools"; +import { AskTool, type AskToolDetails } from "../src/tools/ask"; + +type AskExecutionResult = AgentToolResult; + +async function drainMicrotasks(): Promise { + await Promise.resolve(); + await Promise.resolve(); +} + +function createAskTool(): AskTool { + return new AskTool({ + hasUI: true, + settings: { + get(key: string): unknown { + if (key === "ask.timeout") return 0.01; + if (key === "ask.notify") return "off"; + if (key === "speech.enabled") return false; + return undefined; + }, + }, + getPlanModeState: () => ({ enabled: false }), + } as unknown as ToolSession); +} + +describe("AskTool timeout", () => { + beforeAll(async () => { + const loaded = await getThemeByName("dark"); + if (!loaded) throw new Error("theme unavailable"); + setThemeInstance(loaded); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("auto-selects the recommended option when the selector does not settle", async () => { + vi.useFakeTimers(); + const select = vi.fn(() => new Promise(() => {})); + const abort = vi.fn(); + const context = { + hasUI: true, + ui: { + select, + editor: vi.fn(), + }, + abort, + } as unknown as AgentToolContext; + let result: AskExecutionResult | undefined; + let rejection: unknown; + + void createAskTool() + .execute( + "ask-timeout", + { + questions: [ + { + id: "db", + question: "Which database?", + options: [{ label: "SQLite" }, { label: "Postgres" }], + recommended: 1, + }, + ], + }, + undefined, + undefined, + context, + ) + .then( + value => { + result = value; + }, + error => { + rejection = error; + }, + ); + + await drainMicrotasks(); + vi.advanceTimersByTime(10); + await drainMicrotasks(); + + expect(rejection).toBeUndefined(); + expect(result?.details?.selectedOptions).toEqual(["Postgres"]); + expect(result?.details?.timedOut).toBe(true); + expect(abort).not.toHaveBeenCalled(); + }); +});