diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f0c6dbb7b..af93c3ce5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,7 @@ - Fixed full-context forks cold-missing OpenAI prompt caches by persisting an inherited provider prompt-cache key separately from the new OMP session id, adding `--prompt-cache-key` for explicit cache affinity, and dropping automatic inheritance when startup changes the model, thinking level, system prompt, or tool schema. ([#5035](https://github.com/can1357/oh-my-pi/issues/5035)) - Fixed Codex advisor requests using local `-advisor` session labels as provider session IDs; advisors now use stable UUIDv7 provider identities while keeping labeled transcript names. ([#5040](https://github.com/can1357/oh-my-pi/issues/5040)) - Fixed macOS stdio MCP servers launching in a detached session, so `xcrun mcpbridge` can trigger the TCC Apple Events permission prompt and complete startup. ([#4987](https://github.com/can1357/oh-my-pi/issues/4987)) +- 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 diff --git a/packages/coding-agent/src/extensibility/extensions/types.ts b/packages/coding-agent/src/extensibility/extensions/types.ts index cfff0ed01..e3bb89739 100644 --- a/packages/coding-agent/src/extensibility/extensions/types.ts +++ b/packages/coding-agent/src/extensibility/extensions/types.ts @@ -125,6 +125,10 @@ export interface ExtensionUIDialogOptions { timeout?: number; /** Invoked when the UI times out while waiting for a selection/input */ onTimeout?: () => void; + /** Invoked when the UI-managed timeout countdown starts */ + onTimeoutStart?: () => void; + /** Invoked when user input resets a UI-managed timeout countdown */ + onTimeoutReset?: () => void; /** Initial cursor position for select dialogs (0-indexed) */ initialIndex?: number; /** Render an outlined list for select dialogs */ @@ -176,6 +180,8 @@ export type AutocompleteProviderFactory = (current: AutocompleteProvider) => Aut // and may be invoked from event handlers that have already taken the agent // loop's lock — hooks intentionally cannot. export interface ExtensionUIContext { + /** True when selector timeouts start only after the dialog is presented. */ + timeoutStartsOnPresentation?: boolean; /** Show a selector and return the selected label, even when an option also includes a description. */ select( title: string, diff --git a/packages/coding-agent/src/modes/components/hook-selector.ts b/packages/coding-agent/src/modes/components/hook-selector.ts index 99ea1ea1e..3165166ae 100644 --- a/packages/coding-agent/src/modes/components/hook-selector.ts +++ b/packages/coding-agent/src/modes/components/hook-selector.ts @@ -61,6 +61,8 @@ export interface HookSelectorOptions { tui?: TUI; timeout?: number; onTimeout?: () => void; + onTimeoutStart?: () => void; + onTimeoutReset?: () => void; initialIndex?: number; outline?: boolean; maxVisible?: number; @@ -178,6 +180,7 @@ export class HookSelectorComponent extends Container { #onLeftCallback: (() => void) | undefined; #onRightCallback: (() => void) | undefined; #onExternalEditorCallback: (() => void) | undefined; + #onTimeoutResetCallback: (() => void) | undefined; #slider: HookSelectorSlider | undefined; #sliderIndex: number = 0; #sliderComponent: Text | undefined; @@ -213,6 +216,7 @@ export class HookSelectorComponent extends Container { this.#onLeftCallback = opts?.onLeft; this.#onRightCallback = opts?.onRight; this.#onExternalEditorCallback = opts?.onExternalEditor; + this.#onTimeoutResetCallback = opts?.onTimeoutReset; if (opts?.slider && opts.slider.segments.length > 0) { this.#slider = opts.slider; this.#sliderIndex = Math.max(0, Math.min(opts.slider.index, opts.slider.segments.length - 1)); @@ -232,6 +236,7 @@ export class HookSelectorComponent extends Container { } if (opts?.timeout && opts.timeout > 0 && opts.tui) { + opts.onTimeoutStart?.(); this.#countdown = new CountdownTimer( opts.timeout, opts.tui, @@ -633,8 +638,10 @@ export class HookSelectorComponent extends Container { } handleInput(keyData: string): void { - // Reset countdown on any interaction - this.#countdown?.reset(); + if (this.#countdown) { + this.#countdown.reset(); + this.#onTimeoutResetCallback?.(); + } if (matchesSelectCancel(keyData)) { this.#onCancelCallback(); diff --git a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts index 684642b12..99a840721 100644 --- a/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts +++ b/packages/coding-agent/src/modes/controllers/extension-ui-controller.ts @@ -61,6 +61,7 @@ export class ExtensionUiController { async initHooksAndCustomTools(): Promise { // Create and set hook & tool UI context const uiContext: ExtensionUIContext = { + timeoutStartsOnPresentation: true, select: (title, options, dialogOptions) => this.showCollabAwareSelector(title, options, dialogOptions), confirm: (title, message, _dialogOptions) => this.showHookConfirm(title, message), input: (title, placeholder, dialogOptions) => this.showHookInput(title, placeholder, dialogOptions), @@ -624,6 +625,8 @@ export class ExtensionUiController { initialIndex: dialogOptions?.initialIndex, timeout: dialogOptions?.timeout, onTimeout: dialogOptions?.onTimeout, + onTimeoutStart: dialogOptions?.onTimeoutStart, + onTimeoutReset: dialogOptions?.onTimeoutReset, tui: this.ctx.ui, outline: dialogOptions?.outline, disabledIndices: dialogOptions?.disabledIndices, diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index 42de7f930..efaf65db5 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -380,6 +380,7 @@ interface AskSingleQuestionOptions { } interface UIContext { + timeoutStartsOnPresentation?: boolean; select( prompt: string, options: ExtensionUISelectItem[], @@ -389,6 +390,8 @@ interface UIContext { signal?: AbortSignal; outline?: boolean; onTimeout?: () => void; + onTimeoutStart?: () => void; + onTimeoutReset?: () => void; onLeft?: () => void; onRight?: () => void; helpText?: string; @@ -432,12 +435,30 @@ async function askSingleQuestion( const helpText = navigation ? "up/down navigate enter select ←/→ question esc cancel" : "up/down navigate enter select esc cancel"; + const timeoutMs = typeof timeout === "number" && timeout > 0 ? timeout : undefined; + const timeoutController = timeoutMs === undefined ? undefined : new AbortController(); + const dialogSignal = + signal && timeoutController + ? AbortSignal.any([signal, timeoutController.signal]) + : (timeoutController?.signal ?? signal); + let timeoutId: NodeJS.Timeout | undefined; + let timeoutStartedMs = Date.now(); + const armFallbackTimeout = (durationMs: number) => { + clearTimeout(timeoutId); + timeoutStartedMs = Date.now(); + timeoutId = setTimeout(() => { + timeoutTriggered = true; + timeoutController?.abort(); + }, durationMs); + }; const dialogOptions = { initialIndex, timeout, - signal, + signal: dialogSignal, outline: true, onTimeout, + onTimeoutStart: timeoutMs === undefined ? undefined : () => armFallbackTimeout(timeoutMs), + onTimeoutReset: timeoutMs === undefined ? undefined : () => armFallbackTimeout(timeoutMs), helpText, selectionMarker: marker?.selectionMarker, checkedIndices: marker?.checkedIndices, @@ -453,19 +474,32 @@ 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; + try { + const runSelect = () => { + const selection = ui.select(prompt, optionsToShow, dialogOptions); + if (timeoutMs !== undefined && !ui.timeoutStartsOnPresentation) { + armFallbackTimeout(timeoutMs); + } + return selection; + }; + const choice = dialogSignal ? await untilAborted(dialogSignal, runSelect) : await runSelect(); + 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() - timeoutStartedMs; + 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 ( @@ -614,6 +648,9 @@ async function askSingleQuestion( customInput = undefined; break; } + if (timedOut && selectedOptions.length === 0 && customInput === undefined) { + selectedOptions = getAutoSelectionOnTimeout(questionOptions, recommended); + } if (navigation?.allowForward) { return { selectedOptions, customInput, timedOut, navigation: "forward" }; } @@ -743,6 +780,7 @@ export class AskTool implements AgentTool { const extensionUi = context.ui; const ui: UIContext = { + timeoutStartsOnPresentation: extensionUi.timeoutStartsOnPresentation, select: (prompt, options, dialogOptions) => extensionUi.select(prompt, options, dialogOptions), editor: (title, prefill, dialogOptions, editorOptions) => extensionUi.editor(title, prefill, dialogOptions, editorOptions), 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..0caaf8bd6 --- /dev/null +++ b/packages/coding-agent/test/ask-timeout.test.ts @@ -0,0 +1,315 @@ +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import type { AgentToolContext, AgentToolResult } from "@oh-my-pi/pi-agent-core"; +import type { TUI } from "@oh-my-pi/pi-tui"; +import type { ExtensionUIDialogOptions, ExtensionUISelectItem } from "../src/extensibility/extensions"; +import { HookSelectorComponent } from "../src/modes/components/hook-selector"; +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; +type AskSelect = ( + title: string, + options: ExtensionUISelectItem[], + dialogOptions?: ExtensionUIDialogOptions, +) => Promise; + +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(() => Promise.withResolvers().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(); + }); + + it("honors selector timeout resets before using the fallback timeout", async () => { + vi.useFakeTimers(); + let resetTimeout: (() => void) | undefined; + const select = vi.fn((_title, _options, dialogOptions) => { + dialogOptions?.onTimeoutStart?.(); + resetTimeout = dialogOptions?.onTimeoutReset; + return Promise.withResolvers().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(); + expect(resetTimeout).toBeDefined(); + + vi.advanceTimersByTime(9); + resetTimeout?.(); + vi.advanceTimersByTime(9); + await drainMicrotasks(); + + expect(result).toBeUndefined(); + + vi.advanceTimersByTime(1); + await drainMicrotasks(); + + expect(rejection).toBeUndefined(); + expect(result?.details?.selectedOptions).toEqual(["Postgres"]); + expect(result?.details?.timedOut).toBe(true); + expect(abort).not.toHaveBeenCalled(); + }); + + it("does not run the fallback timeout while the selector is queued", async () => { + vi.useFakeTimers(); + let startTimeout: (() => void) | undefined; + const select = vi.fn((_title, _options, dialogOptions) => { + startTimeout = dialogOptions?.onTimeoutStart; + return Promise.withResolvers().promise; + }); + const abort = vi.fn(); + const context = { + hasUI: true, + ui: { + timeoutStartsOnPresentation: true, + 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(); + expect(startTimeout).toBeDefined(); + + vi.advanceTimersByTime(10); + await drainMicrotasks(); + + expect(result).toBeUndefined(); + + startTimeout?.(); + vi.advanceTimersByTime(10); + await drainMicrotasks(); + + expect(rejection).toBeUndefined(); + expect(result?.details?.selectedOptions).toEqual(["Postgres"]); + expect(result?.details?.timedOut).toBe(true); + expect(abort).not.toHaveBeenCalled(); + }); + + it("auto-selects timed-out single-choice questions before advancing multi-question asks", async () => { + vi.useFakeTimers(); + let callCount = 0; + const select = vi.fn((_title, _options, dialogOptions) => { + callCount += 1; + if (callCount === 1) { + dialogOptions?.onTimeoutStart?.(); + return Promise.withResolvers().promise; + } + return Promise.resolve("OAuth"); + }); + 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, + }, + { + id: "auth", + question: "Which auth?", + options: [{ label: "JWT" }, { label: "OAuth" }], + recommended: 0, + }, + ], + }, + undefined, + undefined, + context, + ) + .then( + value => { + result = value; + }, + error => { + rejection = error; + }, + ); + + await drainMicrotasks(); + vi.advanceTimersByTime(10); + await drainMicrotasks(); + await drainMicrotasks(); + + expect(rejection).toBeUndefined(); + expect(select).toHaveBeenCalledTimes(2); + expect(result?.details?.results?.[0]?.selectedOptions).toEqual(["Postgres"]); + expect(result?.details?.results?.[0]?.timedOut).toBe(true); + expect(result?.details?.results?.[1]?.selectedOptions).toEqual(["OAuth"]); + expect(result?.details?.results?.[1]?.timedOut).toBeUndefined(); + expect(abort).not.toHaveBeenCalled(); + }); + + it("notifies callers when the selector countdown starts and resets", () => { + vi.useFakeTimers(); + const onTimeoutStart = vi.fn(); + const onTimeoutReset = vi.fn(); + const selector = new HookSelectorComponent("Pick one", ["SQLite", "Postgres"], vi.fn(), vi.fn(), { + timeout: 10, + tui: { requestRender: vi.fn() } as unknown as TUI, + onTimeoutStart, + onTimeoutReset, + }); + + selector.handleInput("j"); + + expect(onTimeoutStart).toHaveBeenCalledTimes(1); + expect(onTimeoutReset).toHaveBeenCalledTimes(1); + selector.dispose(); + }); +});