Merge PR #8441: fix(extensions): pause tool-call timeout during human dialogs (@Seljuke)
# Conflicts: # packages/coding-agent/test/extensions-runner.test.ts
This commit is contained in:
@@ -134,6 +134,9 @@
|
||||
- Fixed omp plugin install failing with cloning errors for legacy Pi extensions whose tool schemas use legacy-typebox builders.
|
||||
- Fixed omp update aborting with chmod ENOENT when concurrent update runs overlapped by using unique download temporary paths.
|
||||
- Fixed the browser tool executable probe launching the user's installed GUI Chromium on Windows: the `--version` version probe from ecb22957 was Linux-scoped but ran for every platform candidate, so on Windows it could hand off to a running `chrome.exe`, open a normal browser window, then reject the candidate and fall back to cached Chrome for Testing. The probe is now confined to Linux ([#8445](https://github.com/can1357/oh-my-pi/issues/8445)).
|
||||
### Fixed
|
||||
|
||||
- Made extension tool-call timeouts configurable and paused them during user dialogs.
|
||||
|
||||
## [17.3.0] - 2026-08-13
|
||||
|
||||
|
||||
@@ -5494,6 +5494,18 @@ export const SETTINGS_SCHEMA = {
|
||||
|
||||
"commit.changelogMaxDiffChars": { type: "number", default: 120000 },
|
||||
|
||||
"extensionHandlers.toolCallTimeoutMs": {
|
||||
type: "number",
|
||||
default: 30_000,
|
||||
ui: {
|
||||
tab: "tools",
|
||||
group: "Extensions",
|
||||
label: "Tool Call Handler Timeout (ms)",
|
||||
description:
|
||||
"Positive finite active-work timeout for extension tool_call handlers; invalid values use 30000ms, and time awaiting OMP-owned dialogs does not count",
|
||||
},
|
||||
},
|
||||
|
||||
"dev.autoqa": {
|
||||
type: "boolean",
|
||||
default: true,
|
||||
|
||||
@@ -92,6 +92,10 @@ export function testSetExtensionHandlerTimeoutMs(timeoutMs: number): void {
|
||||
extensionHandlerTimeoutMs = timeoutMs;
|
||||
}
|
||||
|
||||
function normalizeHandlerTimeout(timeoutMs: number): number {
|
||||
return Number.isFinite(timeoutMs) && timeoutMs > 0 ? timeoutMs : EXTENSION_HANDLER_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
/**
|
||||
* Dedicated cap for `session_shutdown` handlers. The generic 30s budget is
|
||||
* appropriate for events extensions can observe (e.g. `session_start`,
|
||||
@@ -117,6 +121,11 @@ function handlerTimeoutForEvent(eventType: string): number {
|
||||
const EXTENSION_HANDLER_TIMEOUT = Symbol("extensionHandlerTimeout");
|
||||
const EXTENSION_HANDLER_ABORTED = Symbol("extensionHandlerAborted");
|
||||
|
||||
interface HandlerTimeoutBudget {
|
||||
pause(): void;
|
||||
resume(): void;
|
||||
}
|
||||
|
||||
function attachHandlerSignal(
|
||||
dialogOptions: ExtensionUIDialogOptions | undefined,
|
||||
handlerSignal: AbortSignal,
|
||||
@@ -127,22 +136,41 @@ function attachHandlerSignal(
|
||||
return { ...dialogOptions, signal: AbortSignal.any([dialogOptions.signal, handlerSignal]) };
|
||||
}
|
||||
|
||||
function createHandlerUIContext(ui: ExtensionUIContext, handlerSignal: AbortSignal): ExtensionUIContext {
|
||||
function createHandlerUIContext(
|
||||
ui: ExtensionUIContext,
|
||||
handlerSignal: AbortSignal,
|
||||
timeoutBudget?: HandlerTimeoutBudget,
|
||||
): ExtensionUIContext {
|
||||
const askDialog = ui.askDialog;
|
||||
const runDialog = async <T>(dialog: () => Promise<T>): Promise<T> => {
|
||||
timeoutBudget?.pause();
|
||||
try {
|
||||
return await dialog();
|
||||
} finally {
|
||||
timeoutBudget?.resume();
|
||||
}
|
||||
};
|
||||
const dialogMethods = {
|
||||
select: (title, options, dialogOptions) =>
|
||||
ui.select(title, options, attachHandlerSignal(dialogOptions, handlerSignal)),
|
||||
runDialog(() => ui.select(title, options, attachHandlerSignal(dialogOptions, handlerSignal))),
|
||||
confirm: (title, message, dialogOptions) =>
|
||||
ui.confirm(title, message, attachHandlerSignal(dialogOptions, handlerSignal)),
|
||||
runDialog(() => ui.confirm(title, message, attachHandlerSignal(dialogOptions, handlerSignal))),
|
||||
input: (title, placeholder, dialogOptions) =>
|
||||
ui.input(title, placeholder, attachHandlerSignal(dialogOptions, handlerSignal)),
|
||||
runDialog(() => ui.input(title, placeholder, attachHandlerSignal(dialogOptions, handlerSignal))),
|
||||
askDialog: askDialog
|
||||
? (questions, dialogOptions) =>
|
||||
askDialog.call(ui, questions, attachHandlerSignal(dialogOptions, handlerSignal))
|
||||
runDialog(() => askDialog.call(ui, questions, attachHandlerSignal(dialogOptions, handlerSignal)))
|
||||
: undefined,
|
||||
custom: (factory, options) =>
|
||||
runDialog(() =>
|
||||
ui.custom(factory, {
|
||||
...options,
|
||||
signal: options?.signal ? AbortSignal.any([options.signal, handlerSignal]) : handlerSignal,
|
||||
}),
|
||||
),
|
||||
editor: (title, prefill, dialogOptions, editorOptions) =>
|
||||
ui.editor(title, prefill, attachHandlerSignal(dialogOptions, handlerSignal), editorOptions),
|
||||
} satisfies Pick<ExtensionUIContext, "select" | "confirm" | "input" | "askDialog" | "editor">;
|
||||
runDialog(() => ui.editor(title, prefill, attachHandlerSignal(dialogOptions, handlerSignal), editorOptions)),
|
||||
} satisfies Pick<ExtensionUIContext, "select" | "confirm" | "input" | "askDialog" | "custom" | "editor">;
|
||||
const delegatedMethods = new Map<PropertyKey, unknown>();
|
||||
|
||||
return new Proxy(ui, {
|
||||
@@ -167,10 +195,14 @@ function createHandlerUIContext(ui: ExtensionUIContext, handlerSignal: AbortSign
|
||||
* `pi.setModel()` and then reading `ctx.model` would see a stale model.
|
||||
* Prototype delegation keeps every getter live while overriding `ui`.
|
||||
*/
|
||||
function createHandlerContext(ctx: ExtensionContext, handlerSignal: AbortSignal): ExtensionContext {
|
||||
function createHandlerContext(
|
||||
ctx: ExtensionContext,
|
||||
handlerSignal: AbortSignal,
|
||||
timeoutBudget?: HandlerTimeoutBudget,
|
||||
): ExtensionContext {
|
||||
const scoped: ExtensionContext = Object.create(ctx);
|
||||
Object.defineProperty(scoped, "ui", {
|
||||
value: createHandlerUIContext(ctx.ui, handlerSignal),
|
||||
value: createHandlerUIContext(ctx.ui, handlerSignal, timeoutBudget),
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
});
|
||||
@@ -190,7 +222,7 @@ function createHandlerContext(ctx: ExtensionContext, handlerSignal: AbortSignal)
|
||||
* can `clearTimeout` on the winning branch.
|
||||
*/
|
||||
async function raceHandlerWithTimeout<T>(
|
||||
work: (handlerSignal: AbortSignal) => Promise<T> | T,
|
||||
work: (handlerSignal: AbortSignal, timeoutBudget: HandlerTimeoutBudget) => Promise<T> | T,
|
||||
timeoutMs: number,
|
||||
signal?: AbortSignal,
|
||||
): Promise<T | typeof EXTENSION_HANDLER_TIMEOUT | typeof EXTENSION_HANDLER_ABORTED> {
|
||||
@@ -203,13 +235,52 @@ async function raceHandlerWithTimeout<T>(
|
||||
>();
|
||||
const onAbort = () => resolveInterrupt(EXTENSION_HANDLER_ABORTED);
|
||||
signal?.addEventListener("abort", onAbort, { once: true });
|
||||
const timer = setTimeout(() => {
|
||||
let timer: Timer | undefined;
|
||||
let remainingMs = timeoutMs;
|
||||
let activeSince = performance.now();
|
||||
let pauseDepth = 0;
|
||||
let settled = false;
|
||||
const clearTimer = () => {
|
||||
if (timer === undefined) return;
|
||||
clearTimeout(timer);
|
||||
timer = undefined;
|
||||
};
|
||||
const expire = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimer();
|
||||
timeoutController.abort(new DOMException(`Handler timed out after ${timeoutMs}ms`, "TimeoutError"));
|
||||
resolveInterrupt(EXTENSION_HANDLER_TIMEOUT);
|
||||
}, timeoutMs);
|
||||
};
|
||||
const armTimer = () => {
|
||||
if (settled || pauseDepth > 0) return;
|
||||
activeSince = performance.now();
|
||||
timer = setTimeout(expire, Math.max(0, remainingMs));
|
||||
};
|
||||
const settle = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
clearTimer();
|
||||
};
|
||||
const timeoutBudget: HandlerTimeoutBudget = {
|
||||
pause: () => {
|
||||
if (settled) return;
|
||||
pauseDepth++;
|
||||
if (pauseDepth !== 1) return;
|
||||
remainingMs = Math.max(0, remainingMs - (performance.now() - activeSince));
|
||||
clearTimer();
|
||||
if (remainingMs <= 0) expire();
|
||||
},
|
||||
resume: () => {
|
||||
if (settled || pauseDepth === 0) return;
|
||||
pauseDepth--;
|
||||
if (pauseDepth === 0) armTimer();
|
||||
},
|
||||
};
|
||||
armTimer();
|
||||
try {
|
||||
if (signal?.aborted) return EXTENSION_HANDLER_ABORTED;
|
||||
const workPromise = Promise.resolve(work(handlerSignal));
|
||||
const workPromise = Promise.resolve(work(handlerSignal, timeoutBudget));
|
||||
const result = await Promise.race([workPromise, interruptPromise]);
|
||||
if (result === EXTENSION_HANDLER_TIMEOUT) {
|
||||
await Promise.race([
|
||||
@@ -222,7 +293,7 @@ async function raceHandlerWithTimeout<T>(
|
||||
}
|
||||
return result;
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
settle();
|
||||
signal?.removeEventListener("abort", onAbort);
|
||||
}
|
||||
}
|
||||
@@ -1043,23 +1114,33 @@ export class ExtensionRunner {
|
||||
ext: Extension,
|
||||
timeoutMs: number,
|
||||
onFailure?: (kind: "timeout" | "error", message: string) => TResult,
|
||||
outerSignal?: AbortSignal,
|
||||
): Promise<TResult | undefined> {
|
||||
const signal =
|
||||
// `session_stop` carries its own signal on the event; `tool_call` receives
|
||||
// the outer dispatch signal (loop request or wrapper execute) so an abort
|
||||
// while a handler awaits a human dialog cancels the dialog and settles the
|
||||
// gate without executing the underlying tool. Compose whichever apply.
|
||||
const sessionStopSignal =
|
||||
event.type === "session_stop" && "signal" in event && event.signal instanceof AbortSignal
|
||||
? event.signal
|
||||
: undefined;
|
||||
const signals = [outerSignal, sessionStopSignal].filter((s): s is AbortSignal => s !== undefined);
|
||||
const signal = signals.length === 0 ? undefined : signals.length === 1 ? signals[0] : AbortSignal.any(signals);
|
||||
if (signal?.aborted) return undefined;
|
||||
const registrationScope: ToolRegistrationScope = { pending: new Set(), closed: false };
|
||||
let handlerResult: TResult | typeof EXTENSION_HANDLER_TIMEOUT | typeof EXTENSION_HANDLER_ABORTED | undefined;
|
||||
let handlerFailure: { error: unknown } | undefined;
|
||||
try {
|
||||
handlerResult = await raceHandlerWithTimeout(
|
||||
async handlerSignal => {
|
||||
async (handlerSignal, budget) => {
|
||||
registrationScope.signal = handlerSignal;
|
||||
let result: TResult | undefined;
|
||||
try {
|
||||
result = await this.#toolRegistrationScope.run(registrationScope, () =>
|
||||
handler(event, createHandlerContext(ctx, handlerSignal)),
|
||||
handler(
|
||||
event,
|
||||
createHandlerContext(ctx, handlerSignal, event.type === "tool_call" ? budget : undefined),
|
||||
),
|
||||
);
|
||||
} catch (error) {
|
||||
handlerFailure = { error };
|
||||
@@ -1220,8 +1301,8 @@ export class ExtensionRunner {
|
||||
/**
|
||||
* Emit a `tool_call` event to every subscribed extension before the tool executes.
|
||||
*
|
||||
* Each handler is bounded by `extensionHandlerTimeoutMs` (default 30s). This
|
||||
* matches the timeout policy already applied to `emitToolResult` and every
|
||||
* Each handler is bounded by `extensionHandlers.toolCallTimeoutMs` (default
|
||||
* 30s). This matches the timeout policy already applied to `emitToolResult` and every
|
||||
* other handler routed through `#runHandlerWithTimeout`; without it a single
|
||||
* hung extension (unresolved `await`, network call with no timeout) would
|
||||
* park `ExtensionToolWrapper.execute` indefinitely and freeze tool
|
||||
@@ -1232,9 +1313,11 @@ export class ExtensionRunner {
|
||||
* pre-execution gate — an unresponsive extension MUST NOT be treated as
|
||||
* silent consent to run the tool.
|
||||
*/
|
||||
async emitToolCall(event: ToolCallEvent): Promise<ToolCallEventResult | undefined> {
|
||||
async emitToolCall(event: ToolCallEvent, signal?: AbortSignal): Promise<ToolCallEventResult | undefined> {
|
||||
const ctx = this.createContext();
|
||||
const timeoutMs = extensionHandlerTimeoutMs;
|
||||
const timeoutMs = normalizeHandlerTimeout(
|
||||
this.settings?.get("extensionHandlers.toolCallTimeoutMs") ?? extensionHandlerTimeoutMs,
|
||||
);
|
||||
let result: ToolCallEventResult | undefined;
|
||||
|
||||
for (const ext of this.extensions) {
|
||||
@@ -1255,6 +1338,7 @@ export class ExtensionRunner {
|
||||
? `Extension ${ext.path} timed out after ${timeoutMs}ms`
|
||||
: `Extension ${ext.path} failed: ${message}`,
|
||||
}),
|
||||
signal,
|
||||
);
|
||||
|
||||
if (handlerResult) {
|
||||
@@ -1266,6 +1350,9 @@ export class ExtensionRunner {
|
||||
}
|
||||
}
|
||||
|
||||
if (signal?.aborted) {
|
||||
return { block: true, reason: `Tool execution was cancelled while an extension handler was pending` };
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
@@ -234,6 +234,8 @@ export interface ExtensionCustomOptions {
|
||||
overlayOptions?: OverlayOptions | (() => OverlayOptions);
|
||||
/** Invoked with the overlay handle once the overlay is created (overlay mode only). */
|
||||
onHandle?: (handle: OverlayHandle) => void;
|
||||
/** Abort the custom UI and reject its promise. */
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
/** Wrap the current autocomplete provider with additional behavior (pi-compatible). */
|
||||
|
||||
@@ -205,15 +205,18 @@ export class ExtensionToolWrapper<TParameters extends TSchema = TSchema, TDetail
|
||||
let effectiveParams = params;
|
||||
if (!loopEmittedToolCall && this.runner.hasHandlers("tool_call")) {
|
||||
try {
|
||||
const callResult = (await this.runner.emitToolCall({
|
||||
type: "tool_call",
|
||||
toolName: this.tool.name,
|
||||
toolCallId,
|
||||
input: normalizeToolEventInput(
|
||||
this.tool.name,
|
||||
resolveToolEventInput(this.tool, toolEventArgs(params, context)),
|
||||
),
|
||||
})) as ToolCallEventResult | undefined;
|
||||
const callResult = (await this.runner.emitToolCall(
|
||||
{
|
||||
type: "tool_call",
|
||||
toolName: this.tool.name,
|
||||
toolCallId,
|
||||
input: normalizeToolEventInput(
|
||||
this.tool.name,
|
||||
resolveToolEventInput(this.tool, toolEventArgs(params, context)),
|
||||
),
|
||||
},
|
||||
signal,
|
||||
)) as ToolCallEventResult | undefined;
|
||||
|
||||
if (callResult?.block) {
|
||||
const reason = callResult.reason || "Tool execution was blocked by an extension";
|
||||
|
||||
@@ -1053,14 +1053,12 @@ export class ExtensionUiController {
|
||||
const savedText = this.ctx.editor.getText();
|
||||
const keybindings = KeybindingsManager.inMemory();
|
||||
|
||||
const { promise, resolve } = Promise.withResolvers<T>();
|
||||
const { promise, resolve, reject } = Promise.withResolvers<T>();
|
||||
let component: (Component & { dispose?(): void }) | undefined;
|
||||
let overlayHandle: OverlayHandle | undefined;
|
||||
let closed = false;
|
||||
|
||||
const close = (result: T) => {
|
||||
if (closed) return;
|
||||
closed = true;
|
||||
const cleanup = () => {
|
||||
component?.dispose?.();
|
||||
overlayHandle?.hide();
|
||||
overlayHandle = undefined;
|
||||
@@ -1071,35 +1069,55 @@ export class ExtensionUiController {
|
||||
}
|
||||
this.ctx.ui.setFocus(this.ctx.editor);
|
||||
this.ctx.ui.requestRender();
|
||||
resolve(result);
|
||||
};
|
||||
const finish = (settle: () => void) => {
|
||||
if (closed) return;
|
||||
closed = true;
|
||||
options?.signal?.removeEventListener("abort", onAbort);
|
||||
try {
|
||||
cleanup();
|
||||
} finally {
|
||||
settle();
|
||||
}
|
||||
};
|
||||
const fail = (error: unknown) => finish(() => reject(error));
|
||||
const onAbort = () => fail(options?.signal?.reason ?? new DOMException("Dialog aborted", "AbortError"));
|
||||
const close = (result: T) => finish(() => resolve(result));
|
||||
|
||||
Promise.try(() => factory(this.ctx.ui, theme, keybindings, close)).then(c => {
|
||||
if (closed) {
|
||||
c.dispose?.();
|
||||
return;
|
||||
}
|
||||
component = c;
|
||||
if (options?.overlay) {
|
||||
const overlayOptions =
|
||||
typeof options.overlayOptions === "function" ? options.overlayOptions() : options.overlayOptions;
|
||||
overlayHandle = this.ctx.ui.showOverlay(
|
||||
component,
|
||||
overlayOptions ?? {
|
||||
anchor: "bottom-center",
|
||||
width: "100%",
|
||||
maxHeight: "100%",
|
||||
margin: 0,
|
||||
},
|
||||
);
|
||||
options.onHandle?.(overlayHandle);
|
||||
return;
|
||||
}
|
||||
this.ctx.editorContainer.clear();
|
||||
this.ctx.editorContainer.addChild(component);
|
||||
this.ctx.ui.setFocus(component);
|
||||
this.ctx.ui.requestRender();
|
||||
});
|
||||
if (options?.signal?.aborted) {
|
||||
fail(options.signal.reason ?? new DOMException("Dialog aborted", "AbortError"));
|
||||
return promise;
|
||||
}
|
||||
options?.signal?.addEventListener("abort", onAbort, { once: true });
|
||||
|
||||
Promise.try(() => factory(this.ctx.ui, theme, keybindings, close))
|
||||
.then(c => {
|
||||
if (closed) {
|
||||
c.dispose?.();
|
||||
return;
|
||||
}
|
||||
component = c;
|
||||
if (options?.overlay) {
|
||||
const overlayOptions =
|
||||
typeof options.overlayOptions === "function" ? options.overlayOptions() : options.overlayOptions;
|
||||
overlayHandle = this.ctx.ui.showOverlay(
|
||||
component,
|
||||
overlayOptions ?? {
|
||||
anchor: "bottom-center",
|
||||
width: "100%",
|
||||
maxHeight: "100%",
|
||||
margin: 0,
|
||||
},
|
||||
);
|
||||
options.onHandle?.(overlayHandle);
|
||||
return;
|
||||
}
|
||||
this.ctx.editorContainer.clear();
|
||||
this.ctx.editorContainer.addChild(component);
|
||||
this.ctx.ui.setFocus(component);
|
||||
this.ctx.ui.requestRender();
|
||||
})
|
||||
.catch(fail);
|
||||
return promise;
|
||||
}
|
||||
|
||||
|
||||
@@ -1387,7 +1387,7 @@ export class AgentSession {
|
||||
// Pre-scheduling tool_call wiring: extension handlers run at arg-prep
|
||||
// time so a block/revision lands before concurrency resolution,
|
||||
// tool_execution_start, and the wrapper's approval gate.
|
||||
this.agent.beforeToolCall = ctx => this.#beforeToolCall(ctx);
|
||||
this.agent.beforeToolCall = (ctx, signal) => this.#beforeToolCall(ctx, signal);
|
||||
this.agent.providerSessionState = this.#providerSessionState;
|
||||
this.#syncAgentSessionId();
|
||||
this.#todo.syncFromBranch();
|
||||
@@ -3390,7 +3390,7 @@ export class AgentSession {
|
||||
* emit a second event (nested xd:// device dispatches and direct non-loop
|
||||
* execution still emit there).
|
||||
*/
|
||||
async #beforeToolCall(ctx: BeforeToolCallContext): Promise<BeforeToolCallResult | undefined> {
|
||||
async #beforeToolCall(ctx: BeforeToolCallContext, signal?: AbortSignal): Promise<BeforeToolCallResult | undefined> {
|
||||
const runner = this.#extensionRunner;
|
||||
if (!runner?.hasHandlers("tool_call")) return undefined;
|
||||
const metadata = ctx.toolCall.providerMetadata;
|
||||
@@ -3408,12 +3408,15 @@ export class AgentSession {
|
||||
? { actions: computer.actions, pendingSafetyChecks: computer.pendingSafetyChecks }
|
||||
: ctx.args;
|
||||
runner.markToolCallEmitted(ctx.toolCall.id, ctx.tool.name);
|
||||
const callResult = await runner.emitToolCall({
|
||||
type: "tool_call",
|
||||
toolName: ctx.tool.name,
|
||||
toolCallId: ctx.toolCall.id,
|
||||
input: normalizeToolEventInput(ctx.tool.name, resolveToolEventInput(ctx.tool, eventArgs)),
|
||||
});
|
||||
const callResult = await runner.emitToolCall(
|
||||
{
|
||||
type: "tool_call",
|
||||
toolName: ctx.tool.name,
|
||||
toolCallId: ctx.toolCall.id,
|
||||
input: normalizeToolEventInput(ctx.tool.name, resolveToolEventInput(ctx.tool, eventArgs)),
|
||||
},
|
||||
signal,
|
||||
);
|
||||
if (callResult?.block) {
|
||||
return { block: true, reason: callResult.reason || "Tool execution was blocked by an extension" };
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { AgentMessage, AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import type { ImageContent, TextContent } from "@oh-my-pi/pi-ai";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { ExtensionRuntime, loadExtensions } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/loader";
|
||||
import {
|
||||
EXTENSION_HANDLER_TIMEOUT_MS,
|
||||
@@ -1241,6 +1242,38 @@ describe("ExtensionRunner", () => {
|
||||
});
|
||||
|
||||
describe("handler timeouts", () => {
|
||||
const initializeRunner = (runner: ExtensionRunner, uiContext: ExtensionUIContext): void => {
|
||||
runner.initialize(
|
||||
{
|
||||
sendMessage: () => {},
|
||||
sendUserMessage: () => {},
|
||||
appendEntry: () => {},
|
||||
setLabel: () => {},
|
||||
getActiveTools: () => [],
|
||||
getAllTools: () => [],
|
||||
setActiveTools: async () => {},
|
||||
getCommands: () => [],
|
||||
setModel: async () => false,
|
||||
getThinkingLevel: () => undefined,
|
||||
setThinkingLevel: () => {},
|
||||
getSessionName: () => undefined,
|
||||
setSessionName: async () => {},
|
||||
},
|
||||
{
|
||||
getModel: () => undefined,
|
||||
isIdle: () => true,
|
||||
abort: () => {},
|
||||
hasPendingMessages: () => false,
|
||||
shutdown: () => {},
|
||||
getContextUsage: () => undefined,
|
||||
compact: async () => {},
|
||||
getSystemPrompt: () => [],
|
||||
},
|
||||
undefined,
|
||||
uiContext,
|
||||
);
|
||||
};
|
||||
|
||||
it("times out session_start handlers, emits an error, and continues to sibling extensions", async () => {
|
||||
const hangExtensionPath = path.join(tempDir.path(), "hang-session-start.ts");
|
||||
const fastExtensionPath = path.join(tempDir.path(), "fast-session-start.ts");
|
||||
@@ -1354,7 +1387,7 @@ describe("ExtensionRunner", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("times out tool_call handlers with fail-closed policy so a hung extension cannot indefinitely block tool execution (#3948)", async () => {
|
||||
it("uses the configured tool_call timeout and fails closed so a hung extension cannot block execution (#3948)", async () => {
|
||||
const hangExtensionPath = path.join(tempDir.path(), "hang-tool-call.ts");
|
||||
fs.writeFileSync(
|
||||
hangExtensionPath,
|
||||
@@ -1374,14 +1407,14 @@ describe("ExtensionRunner", () => {
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
undefined,
|
||||
Settings.isolated({ "extensionHandlers.toolCallTimeoutMs": 10 }),
|
||||
);
|
||||
const warnSpy = vi.spyOn(logger, "warn").mockImplementation(() => {});
|
||||
const errors: Array<{ extensionPath: string; event: string; error: string }> = [];
|
||||
runner.onError(err => {
|
||||
errors.push(err);
|
||||
});
|
||||
testSetExtensionHandlerTimeoutMs(10);
|
||||
|
||||
const executeCalls: unknown[] = [];
|
||||
const tool: AgentTool = {
|
||||
name: "sleepy",
|
||||
@@ -1422,6 +1455,62 @@ describe("ExtensionRunner", () => {
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("falls back to the default tool_call timeout for invalid configured values", async () => {
|
||||
const extensionPath = path.join(tempDir.path(), "invalid-timeout-tool-call.ts");
|
||||
fs.writeFileSync(
|
||||
extensionPath,
|
||||
`
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async () => {
|
||||
await Promise.withResolvers().promise;
|
||||
});
|
||||
}
|
||||
`,
|
||||
);
|
||||
const loaded = await loadTestExtensions([extensionPath]);
|
||||
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
for (const configuredTimeout of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) {
|
||||
const runner = new ExtensionRunner(
|
||||
loaded.extensions,
|
||||
loaded.runtime,
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
undefined,
|
||||
Settings.isolated({ "extensionHandlers.toolCallTimeoutMs": configuredTimeout }),
|
||||
);
|
||||
let settled = false;
|
||||
const decision = runner
|
||||
.emitToolCall({
|
||||
type: "tool_call",
|
||||
toolName: "guarded",
|
||||
toolCallId: "invalid-timeout-call",
|
||||
input: {},
|
||||
})
|
||||
.then(result => {
|
||||
settled = true;
|
||||
return result;
|
||||
});
|
||||
|
||||
vi.advanceTimersByTime(EXTENSION_HANDLER_TIMEOUT_MS - 1);
|
||||
expect(settled).toBe(false);
|
||||
|
||||
vi.advanceTimersByTime(1);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
vi.advanceTimersByTime(0);
|
||||
expect(await decision).toEqual({
|
||||
block: true,
|
||||
reason: `Extension ${extensionPath} timed out after ${EXTENSION_HANDLER_TIMEOUT_MS}ms`,
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("fails closed when a tool_call handler registration cannot activate", async () => {
|
||||
const extensionPath = path.join(tempDir.path(), "tool-call-registration.ts");
|
||||
fs.writeFileSync(
|
||||
@@ -1534,19 +1623,19 @@ describe("ExtensionRunner", () => {
|
||||
expect(errors).toEqual([]);
|
||||
});
|
||||
|
||||
it("aborts a tool_call handler's confirmation before returning its timeout block", async () => {
|
||||
it("pauses a tool_call handler timeout during standard and custom dialogs, then resumes its budget", async () => {
|
||||
const extensionPath = path.join(tempDir.path(), "confirm-tool-call.ts");
|
||||
const markerPath = path.join(tempDir.path(), "confirm-settled.txt");
|
||||
fs.writeFileSync(
|
||||
extensionPath,
|
||||
`
|
||||
import * as fs from "node:fs";
|
||||
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async (_event, ctx) => {
|
||||
ctx.ui.notify("Waiting for confirmation");
|
||||
await new Promise(resolve => setTimeout(resolve, 8));
|
||||
await ctx.ui.confirm("High-risk command", "Allow this command?");
|
||||
fs.writeFileSync(${JSON.stringify(markerPath)}, "settled");
|
||||
await ctx.ui.custom(() => ({}));
|
||||
ctx.ui.notify("Custom settled");
|
||||
await Promise.withResolvers().promise;
|
||||
});
|
||||
}
|
||||
`,
|
||||
@@ -1561,66 +1650,151 @@ describe("ExtensionRunner", () => {
|
||||
modelRegistry,
|
||||
);
|
||||
const dialog = Promise.withResolvers<boolean>();
|
||||
const handlerStarted = Promise.withResolvers<void>();
|
||||
const confirmationStarted = Promise.withResolvers<void>();
|
||||
const customStarted = Promise.withResolvers<void>();
|
||||
const customCompleted = Promise.withResolvers<void>();
|
||||
let dialogSignal: AbortSignal | undefined;
|
||||
const notify = vi.fn<ExtensionUIContext["notify"]>();
|
||||
const notify: ExtensionUIContext["notify"] = message => {
|
||||
if (message === "Waiting for confirmation") handlerStarted.resolve();
|
||||
if (message === "Custom settled") customCompleted.resolve();
|
||||
};
|
||||
const confirm: ExtensionUIContext["confirm"] = async (_title, _message, dialogOptions) => {
|
||||
dialogSignal = dialogOptions?.signal;
|
||||
confirmationStarted.resolve();
|
||||
dialogSignal?.addEventListener("abort", () => dialog.resolve(false), { once: true });
|
||||
return await dialog.promise;
|
||||
};
|
||||
const customDialog = Promise.withResolvers<void>();
|
||||
let customSignal: AbortSignal | undefined;
|
||||
const custom: ExtensionUIContext["custom"] = async <T>(...args: Parameters<ExtensionUIContext["custom"]>) => {
|
||||
customSignal = args[1]?.signal;
|
||||
customStarted.resolve();
|
||||
await customDialog.promise;
|
||||
return undefined as T;
|
||||
};
|
||||
const uiPrototype = Object.create(runner.getUIContext(), {
|
||||
confirm: { value: confirm },
|
||||
custom: { value: custom },
|
||||
notify: { value: notify },
|
||||
});
|
||||
const uiContext: ExtensionUIContext = Object.create(uiPrototype);
|
||||
initializeRunner(runner, uiContext);
|
||||
vi.useFakeTimers();
|
||||
let now = 0;
|
||||
const performanceNow = vi.spyOn(performance, "now").mockImplementation(() => now);
|
||||
try {
|
||||
testSetExtensionHandlerTimeoutMs(25);
|
||||
|
||||
const tool: AgentTool = {
|
||||
name: "guarded",
|
||||
label: "Guarded",
|
||||
description: "must not execute after the extension gate times out",
|
||||
parameters: Type.Object({}),
|
||||
strict: true,
|
||||
execute: async () => ({ content: [{ type: "text", text: "ran" }] }),
|
||||
};
|
||||
const wrapped = new ExtensionToolWrapper(tool, runner);
|
||||
|
||||
const execution = wrapped.execute("tool-call-id", {});
|
||||
await handlerStarted.promise;
|
||||
expect(dialogSignal).toBeUndefined();
|
||||
|
||||
now = 8;
|
||||
vi.advanceTimersByTime(8);
|
||||
await confirmationStarted.promise;
|
||||
expect(dialogSignal).toBeDefined();
|
||||
|
||||
now = 108;
|
||||
vi.advanceTimersByTime(100);
|
||||
expect(dialogSignal?.aborted).toBe(false);
|
||||
|
||||
dialog.resolve(true);
|
||||
await customStarted.promise;
|
||||
expect(customSignal).toBeDefined();
|
||||
expect(customSignal?.aborted).toBe(false);
|
||||
|
||||
now = 208;
|
||||
vi.advanceTimersByTime(100);
|
||||
expect(customSignal?.aborted).toBe(false);
|
||||
|
||||
customDialog.resolve();
|
||||
await customCompleted.promise;
|
||||
|
||||
now = 225;
|
||||
vi.advanceTimersByTime(17);
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
vi.advanceTimersByTime(0);
|
||||
await expect(execution).rejects.toThrow(`Extension ${extensionPath} timed out after 25ms`);
|
||||
} finally {
|
||||
performanceNow.mockRestore();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("cancels a pending confirmation and blocks tool execution when the outer dispatch aborts (#4223)", async () => {
|
||||
const extensionPath = path.join(tempDir.path(), "confirm-abort-tool-call.ts");
|
||||
fs.writeFileSync(
|
||||
extensionPath,
|
||||
`
|
||||
export default function(pi) {
|
||||
pi.on("tool_call", async (_event, ctx) => {
|
||||
await ctx.ui.confirm("High-risk command", "Allow this command?");
|
||||
});
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
const result = await loadTestExtensions([extensionPath]);
|
||||
const runner = new ExtensionRunner(
|
||||
result.extensions,
|
||||
result.runtime,
|
||||
tempDir.path(),
|
||||
sessionManager,
|
||||
modelRegistry,
|
||||
);
|
||||
let dialogSignal: AbortSignal | undefined;
|
||||
const dialog = Promise.withResolvers<boolean>();
|
||||
const confirmationStarted = Promise.withResolvers<void>();
|
||||
const confirm: ExtensionUIContext["confirm"] = async (_title, _message, dialogOptions) => {
|
||||
dialogSignal = dialogOptions?.signal;
|
||||
confirmationStarted.resolve();
|
||||
dialogSignal?.addEventListener("abort", () => dialog.resolve(false), { once: true });
|
||||
return await dialog.promise;
|
||||
};
|
||||
const uiPrototype = Object.create(runner.getUIContext(), {
|
||||
confirm: { value: confirm },
|
||||
notify: { value: notify },
|
||||
});
|
||||
const uiContext: ExtensionUIContext = Object.create(uiPrototype);
|
||||
runner.initialize(
|
||||
{
|
||||
sendMessage: () => {},
|
||||
sendUserMessage: () => {},
|
||||
appendEntry: () => {},
|
||||
setLabel: () => {},
|
||||
getActiveTools: () => [],
|
||||
getAllTools: () => [],
|
||||
setActiveTools: async () => {},
|
||||
getCommands: () => [],
|
||||
setModel: async () => false,
|
||||
getThinkingLevel: () => undefined,
|
||||
setThinkingLevel: () => {},
|
||||
getSessionName: () => undefined,
|
||||
setSessionName: async () => {},
|
||||
},
|
||||
{
|
||||
getModel: () => undefined,
|
||||
isIdle: () => true,
|
||||
abort: () => {},
|
||||
hasPendingMessages: () => false,
|
||||
shutdown: () => {},
|
||||
getContextUsage: () => undefined,
|
||||
compact: async () => {},
|
||||
getSystemPrompt: () => [],
|
||||
},
|
||||
undefined,
|
||||
uiContext,
|
||||
);
|
||||
testSetExtensionHandlerTimeoutMs(10);
|
||||
initializeRunner(runner, uiContext);
|
||||
let executed = false;
|
||||
|
||||
const tool: AgentTool = {
|
||||
name: "guarded",
|
||||
label: "Guarded",
|
||||
description: "must not execute after the extension gate times out",
|
||||
description: "must not execute after the dispatch aborts",
|
||||
parameters: Type.Object({}),
|
||||
strict: true,
|
||||
execute: async () => ({ content: [{ type: "text", text: "ran" }] }),
|
||||
execute: async () => {
|
||||
executed = true;
|
||||
return { content: [{ type: "text", text: "ran" }] };
|
||||
},
|
||||
};
|
||||
const wrapped = new ExtensionToolWrapper(tool, runner);
|
||||
|
||||
await expect(wrapped.execute("tool-call-id", {})).rejects.toThrow(
|
||||
`Extension ${extensionPath} timed out after 10ms`,
|
||||
);
|
||||
expect(notify).toHaveBeenCalledWith("Waiting for confirmation");
|
||||
const controller = new AbortController();
|
||||
const execution = wrapped.execute("tool-call-id", {} as never, controller.signal);
|
||||
await confirmationStarted.promise;
|
||||
|
||||
expect(dialogSignal).toBeDefined();
|
||||
expect(dialogSignal?.aborted).toBe(false);
|
||||
|
||||
controller.abort();
|
||||
await expect(execution).rejects.toThrow();
|
||||
|
||||
expect(dialogSignal?.aborted).toBe(true);
|
||||
expect(fs.readFileSync(markerPath, "utf8")).toBe("settled");
|
||||
expect(executed).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test";
|
||||
import { afterEach, beforeAll, describe, expect, it, type Mock, vi } from "bun:test";
|
||||
import { Container, type OverlayOptions, setKeybindings } from "@oh-my-pi/pi-tui";
|
||||
import { KeybindingsManager } from "../../../src/config/keybindings";
|
||||
import type { ExtensionAskDialogQuestion, ExtensionUIContext } from "../../../src/extensibility/extensions";
|
||||
@@ -316,4 +316,34 @@ describe("ExtensionUiController custom overlay", () => {
|
||||
margin: 0,
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects and restores the editor when a custom factory fails", async () => {
|
||||
const harness = makeHarness();
|
||||
const ui = await harness.init();
|
||||
const failure = new Error("custom factory failed");
|
||||
|
||||
await expect(ui.custom(() => Promise.reject(failure))).rejects.toBe(failure);
|
||||
|
||||
expect(harness.editorContainer.children).toEqual([harness.editor]);
|
||||
expect(harness.setFocus).toHaveBeenLastCalledWith(harness.editor);
|
||||
});
|
||||
|
||||
it("aborts a pending custom factory and disposes its late component", async () => {
|
||||
const harness = makeHarness();
|
||||
const ui = await harness.init();
|
||||
const controller = new AbortController();
|
||||
const factory = Promise.withResolvers<Container>();
|
||||
const component = new Container() as Container & { dispose: Mock<() => void> };
|
||||
component.dispose = vi.fn();
|
||||
|
||||
const pending = ui.custom(() => factory.promise, { signal: controller.signal });
|
||||
controller.abort();
|
||||
|
||||
await expect(pending).rejects.toBe(controller.signal.reason);
|
||||
factory.resolve(component);
|
||||
await flushMicrotasks();
|
||||
|
||||
expect(component.dispose).toHaveBeenCalledTimes(1);
|
||||
expect(harness.editorContainer.children).toEqual([harness.editor]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user