fix: stop merger/extension tools from wedging on hung fn_task_show
AI merge review could park forever when the host fusion extension loaded fn_task_show and booted a second TaskStore without a tool timeout (FN-7956). - Skip host @runfusion/fusion extensions for sessionPurpose "merger" - Forward sessionPurpose into createFnAgent for that policy - Coalesce + 30s-bound extension TaskStore boots; ALS-propagate AbortSignal - Wrap every extension registerTool execute with 60s timeout/abort fail-closed - Unit tests for merger host-extension skip and tool timeout helpers
This commit is contained in:
97
packages/cli/src/__tests__/extension-tool-timeout.test.ts
Normal file
97
packages/cli/src/__tests__/extension-tool-timeout.test.ts
Normal file
@@ -0,0 +1,97 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
raceWithTimeoutAndAbort,
|
||||
wrapExtensionToolExecute,
|
||||
} from "../extension.js";
|
||||
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:15:
|
||||
FN-7956 hung AI merge review on unbounded extension fn_task_show. These unit tests lock the fail-closed timeout/abort budgets that unblock agent turns when store work wedges.
|
||||
*/
|
||||
|
||||
describe("raceWithTimeoutAndAbort", () => {
|
||||
it("resolves when the promise wins", async () => {
|
||||
await expect(
|
||||
raceWithTimeoutAndAbort(Promise.resolve("ok"), 1_000, undefined, "t"),
|
||||
).resolves.toBe("ok");
|
||||
});
|
||||
|
||||
it("rejects on timeout", async () => {
|
||||
await expect(
|
||||
raceWithTimeoutAndAbort(
|
||||
new Promise(() => {
|
||||
/* never settles */
|
||||
}),
|
||||
20,
|
||||
undefined,
|
||||
"slow-tool",
|
||||
),
|
||||
).rejects.toThrow(/slow-tool timed out after 20ms/);
|
||||
});
|
||||
|
||||
it("rejects when the signal aborts", async () => {
|
||||
const controller = new AbortController();
|
||||
const pending = raceWithTimeoutAndAbort(
|
||||
new Promise(() => {
|
||||
/* never settles */
|
||||
}),
|
||||
5_000,
|
||||
controller.signal,
|
||||
"aborted-tool",
|
||||
);
|
||||
controller.abort();
|
||||
await expect(pending).rejects.toMatchObject({ name: "AbortError" });
|
||||
});
|
||||
|
||||
it("rejects immediately when signal is already aborted", async () => {
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
await expect(
|
||||
raceWithTimeoutAndAbort(Promise.resolve("late"), 1_000, controller.signal, "pre-aborted"),
|
||||
).rejects.toMatchObject({ name: "AbortError" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("wrapExtensionToolExecute", () => {
|
||||
it("returns the tool result on success", async () => {
|
||||
const execute = vi.fn(async () => ({ content: [{ type: "text" as const, text: "hi" }] }));
|
||||
const wrapped = wrapExtensionToolExecute("fn_demo", execute, 1_000);
|
||||
await expect(wrapped("id", {}, undefined)).resolves.toEqual({
|
||||
content: [{ type: "text", text: "hi" }],
|
||||
});
|
||||
expect(execute).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("converts timeouts into isError tool results instead of hanging", async () => {
|
||||
const execute = vi.fn(
|
||||
() =>
|
||||
new Promise(() => {
|
||||
/* never settles */
|
||||
}),
|
||||
);
|
||||
const wrapped = wrapExtensionToolExecute("fn_hang", execute, 25);
|
||||
const result = await wrapped("id", {}, undefined);
|
||||
expect(result).toMatchObject({
|
||||
isError: true,
|
||||
details: { error: expect.stringMatching(/timed out after 25ms/) },
|
||||
});
|
||||
expect((result as { content: Array<{ text: string }> }).content[0].text).toContain("fn_hang failed");
|
||||
});
|
||||
|
||||
it("converts abort into isError tool results", async () => {
|
||||
const controller = new AbortController();
|
||||
const execute = vi.fn(
|
||||
() =>
|
||||
new Promise(() => {
|
||||
/* never settles */
|
||||
}),
|
||||
);
|
||||
const wrapped = wrapExtensionToolExecute("fn_abort", execute, 5_000);
|
||||
const pending = wrapped("id", {}, controller.signal);
|
||||
controller.abort();
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
isError: true,
|
||||
details: { error: "aborted" },
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -76,6 +76,7 @@ import { resolve, relative, isAbsolute, sep, basename, extname, join } from "nod
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { existsSync } from "node:fs";
|
||||
import { spawn, type ChildProcess } from "node:child_process";
|
||||
import { AsyncLocalStorage } from "node:async_hooks";
|
||||
|
||||
// ── Helpers ────────────────────────────────────────────────────────
|
||||
|
||||
@@ -234,17 +235,164 @@ interface CachedStoreEntry {
|
||||
|
||||
/** Cache stores per project root to avoid re-booting the backend on every tool call. */
|
||||
const storeCache = new Map<string, CachedStoreEntry>();
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Concurrent first-call fn_* tools must share one boot promise. Without this, two parallel cache misses each call createTaskStoreForBackend and contend on fusion:schema-applier advisory locks / pool setup — the pattern behind wedged fn_task_show during AI merge.
|
||||
*/
|
||||
const storeBootInflight = new Map<string, Promise<TaskStore>>();
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Propagate the active tool AbortSignal into getStore without rewriting every execute body. registerTool installs the signal here before invoking the real execute.
|
||||
*/
|
||||
const extensionToolSignal = new AsyncLocalStorage<AbortSignal | undefined>();
|
||||
/** Hard ceiling for extension TaskStore boot (second backend open inside a live dashboard/engine process). */
|
||||
const EXTENSION_STORE_BOOT_TIMEOUT_MS = 30_000;
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:15:
|
||||
Default wall-clock budget for every host-extension fn_* tool. Store/CRUD tools must not park an agent turn forever; long shell work belongs in coding builtins (bash), not extension tools.
|
||||
*/
|
||||
const EXTENSION_TOOL_TIMEOUT_MS = 60_000;
|
||||
|
||||
async function getStore(cwd: string): Promise<TaskStore> {
|
||||
function isAbortError(error: unknown): boolean {
|
||||
return (
|
||||
(error instanceof Error && error.name === "AbortError") ||
|
||||
(typeof DOMException !== "undefined" && error instanceof DOMException && error.name === "AbortError")
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Race a promise against a wall-clock timeout and optional AbortSignal.
|
||||
* Does not cancel the underlying work (Node has no structured cancel for store boot),
|
||||
* but unblocks the tool caller so the agent session can fail closed instead of wedging forever.
|
||||
*
|
||||
* @internal Exported for unit tests of the hang-prevention budget.
|
||||
*/
|
||||
export async function raceWithTimeoutAndAbort<T>(
|
||||
promise: Promise<T>,
|
||||
timeoutMs: number,
|
||||
signal: AbortSignal | undefined,
|
||||
label: string,
|
||||
): Promise<T> {
|
||||
if (signal?.aborted) {
|
||||
throw new DOMException(`${label} aborted`, "AbortError");
|
||||
}
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
let onAbort: (() => void) | undefined;
|
||||
let settled = false;
|
||||
try {
|
||||
return await new Promise<T>((resolve, reject) => {
|
||||
const settle = (fn: () => void) => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
if (timer !== undefined) clearTimeout(timer);
|
||||
if (onAbort && signal) signal.removeEventListener("abort", onAbort);
|
||||
fn();
|
||||
};
|
||||
// Swallow late rejections after timeout/abort so the orphaned work cannot surface as unhandledRejection.
|
||||
promise.then(
|
||||
(value) => settle(() => resolve(value)),
|
||||
(error) => settle(() => reject(error)),
|
||||
);
|
||||
if (timeoutMs > 0 && Number.isFinite(timeoutMs)) {
|
||||
timer = setTimeout(() => {
|
||||
settle(() =>
|
||||
reject(new Error(`${label} timed out after ${timeoutMs}ms`)),
|
||||
);
|
||||
}, timeoutMs);
|
||||
if (typeof timer === "object" && timer && "unref" in timer && typeof timer.unref === "function") {
|
||||
timer.unref();
|
||||
}
|
||||
}
|
||||
if (signal) {
|
||||
onAbort = () => settle(() => reject(new DOMException(`${label} aborted`, "AbortError")));
|
||||
signal.addEventListener("abort", onAbort, { once: true });
|
||||
}
|
||||
});
|
||||
} finally {
|
||||
if (timer !== undefined) clearTimeout(timer);
|
||||
if (onAbort && signal) signal.removeEventListener("abort", onAbort);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:MergeQueue 2026-07-15-11:15:
|
||||
* Wrap every extension tool execute with timeout + AbortSignal so a wedged store call cannot park the agent forever (FN-7956 fn_task_show hang).
|
||||
* Errors become isError tool results so the model can continue rather than leaving the turn blocked on an open tool call.
|
||||
*/
|
||||
export function wrapExtensionToolExecute<TArgs extends unknown[], TResult>(
|
||||
toolName: string,
|
||||
execute: (...args: TArgs) => TResult | Promise<TResult>,
|
||||
timeoutMs: number = EXTENSION_TOOL_TIMEOUT_MS,
|
||||
): (...args: TArgs) => Promise<TResult | {
|
||||
content: Array<{ type: "text"; text: string }>;
|
||||
details: { error: string };
|
||||
isError: true;
|
||||
}> {
|
||||
return async (...args: TArgs) => {
|
||||
// ExtensionAPI execute signature: (toolCallId, params, signal?, onUpdate?, ctx?)
|
||||
const signal = (args[2] instanceof AbortSignal ? args[2] : undefined) as AbortSignal | undefined;
|
||||
try {
|
||||
return await extensionToolSignal.run(signal, () =>
|
||||
raceWithTimeoutAndAbort(
|
||||
Promise.resolve(execute(...args)),
|
||||
timeoutMs,
|
||||
signal,
|
||||
toolName,
|
||||
),
|
||||
);
|
||||
} catch (error) {
|
||||
if (isAbortError(error)) {
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `${toolName} aborted.` }],
|
||||
details: { error: "aborted" },
|
||||
isError: true as const,
|
||||
};
|
||||
}
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
return {
|
||||
content: [{ type: "text" as const, text: `${toolName} failed: ${message}` }],
|
||||
details: { error: message },
|
||||
isError: true as const,
|
||||
};
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
async function getStore(cwd: string, signal?: AbortSignal): Promise<TaskStore> {
|
||||
const projectRoot = resolveProjectRoot(cwd);
|
||||
const existing = storeCache.get(projectRoot);
|
||||
if (existing) return existing.store;
|
||||
|
||||
const boot = await createTaskStoreForBackend({ rootDir: projectRoot });
|
||||
// FNXC:PostgresFinalCutover 2026-07-14-17:20: Agent tools cache only the
|
||||
// PostgreSQL factory result; the removed SQLite opt-out is an explicit error.
|
||||
storeCache.set(projectRoot, { store: boot.taskStore, shutdown: boot.shutdown });
|
||||
return boot.taskStore;
|
||||
const effectiveSignal = signal ?? extensionToolSignal.getStore();
|
||||
|
||||
let inflight = storeBootInflight.get(projectRoot);
|
||||
if (!inflight) {
|
||||
/*
|
||||
FNXC:PostgresFinalCutover 2026-07-14-17:20: Agent tools cache only the
|
||||
PostgreSQL factory result; the removed SQLite opt-out is an explicit error.
|
||||
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
First extension tool call in a dashboard/engine process boots a second TaskStore.
|
||||
Bound that boot and coalesce concurrent callers so a wedged boot cannot park every fn_* tool forever.
|
||||
*/
|
||||
inflight = (async () => {
|
||||
try {
|
||||
const boot = await createTaskStoreForBackend({ rootDir: projectRoot });
|
||||
storeCache.set(projectRoot, { store: boot.taskStore, shutdown: boot.shutdown });
|
||||
return boot.taskStore;
|
||||
} finally {
|
||||
storeBootInflight.delete(projectRoot);
|
||||
}
|
||||
})();
|
||||
storeBootInflight.set(projectRoot, inflight);
|
||||
}
|
||||
|
||||
return raceWithTimeoutAndAbort(
|
||||
inflight,
|
||||
EXTENSION_STORE_BOOT_TIMEOUT_MS,
|
||||
effectiveSignal,
|
||||
"fn extension TaskStore boot",
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -792,6 +940,23 @@ const workflowExtensionToolSpecs: Array<{
|
||||
// ── Extension entry point ──────────────────────────────────────────
|
||||
|
||||
export default function kbExtension(pi: ExtensionAPI) {
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:15:
|
||||
Intercept every registerTool so all fn_* executes share one timeout/abort budget.
|
||||
Without this, only hand-wrapped tools fail closed; FN-7956 hung on an unwrapped secondary-store path during merge review.
|
||||
*/
|
||||
const rawRegisterTool = pi.registerTool.bind(pi) as ExtensionAPI["registerTool"];
|
||||
pi.registerTool = ((tool: Parameters<ExtensionAPI["registerTool"]>[0]) => {
|
||||
if (!tool || typeof tool !== "object" || typeof (tool as { execute?: unknown }).execute !== "function") {
|
||||
return rawRegisterTool(tool);
|
||||
}
|
||||
const original = tool as { name?: string; execute: (...args: unknown[]) => unknown };
|
||||
return rawRegisterTool({
|
||||
...tool,
|
||||
execute: wrapExtensionToolExecute(original.name ?? "fn_tool", original.execute as (...args: unknown[]) => unknown),
|
||||
} as Parameters<ExtensionAPI["registerTool"]>[0]);
|
||||
}) as ExtensionAPI["registerTool"];
|
||||
|
||||
// Register GitHub tracking hook once per extension lifecycle so that
|
||||
// fn_task_create, fn_task_import_github*, fn_delegate_task, etc.
|
||||
// trigger tracking issue creation when settings enable it.
|
||||
@@ -1262,6 +1427,11 @@ export default function kbExtension(pi: ExtensionAPI) {
|
||||
id: Type.String({ description: "Task ID (e.g. FN-001)" }),
|
||||
}),
|
||||
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:15:
|
||||
Timeout/abort come from the extension-wide registerTool wrapper + getStore ALS signal.
|
||||
Keep this body lean; do not re-wrap (double budgets hide the real failure).
|
||||
*/
|
||||
async execute(_toolCallId, params, _signal, _onUpdate, ctx) {
|
||||
const store = await getStore(ctx.cwd);
|
||||
const task = await store.getTask(params.id);
|
||||
|
||||
@@ -42,6 +42,7 @@ const realpathSyncNativeMock = vi.fn((path: PathLike) => String(path));
|
||||
const readCustomProvidersMock = vi.fn(() => []);
|
||||
const packageManagerCwdCapture = vi.fn();
|
||||
const packageManagerSettingsCapture = vi.fn();
|
||||
const resourceLoaderOptionsCapture = vi.fn();
|
||||
|
||||
// Route async `exec` through the `execSync` mock so the promisify bridge works.
|
||||
// Use Symbol.for("nodejs.util.promisify.custom") directly to avoid async imports
|
||||
@@ -124,6 +125,9 @@ vi.mock("@earendil-works/pi-coding-agent", () => ({
|
||||
createReadTool: () => ({ name: "read" }),
|
||||
createWriteTool: () => ({ name: "write" }),
|
||||
DefaultResourceLoader: class {
|
||||
constructor(options: any) {
|
||||
resourceLoaderOptionsCapture(options);
|
||||
}
|
||||
async reload() {
|
||||
await reloadMock();
|
||||
}
|
||||
@@ -1235,6 +1239,50 @@ describe("createFnAgent", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("skips host extensions for merger sessions so dual-store fn_* tools cannot wedge merge", async () => {
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
FN-7956 hung AI merge review on extension fn_task_show (second TaskStore boot, no tool timeout).
|
||||
Merger sessions must not receive host @runfusion/fusion extension paths even with tools:coding.
|
||||
*/
|
||||
const { createFnAgent, setHostExtensionPaths } = await import("../pi.js");
|
||||
setHostExtensionPaths(["/mock/fusion-extension"]);
|
||||
|
||||
await createFnAgent({
|
||||
cwd: "/project",
|
||||
systemPrompt: "merge",
|
||||
tools: "coding",
|
||||
sessionPurpose: "merger",
|
||||
});
|
||||
|
||||
expect(resourceLoaderOptionsCapture).toHaveBeenCalled();
|
||||
const loaderOpts = resourceLoaderOptionsCapture.mock.calls.at(-1)?.[0] as {
|
||||
additionalExtensionPaths?: string[];
|
||||
};
|
||||
expect(loaderOpts.additionalExtensionPaths).toBeUndefined();
|
||||
|
||||
setHostExtensionPaths([]);
|
||||
});
|
||||
|
||||
it("still injects host extensions for coding non-merger sessions", async () => {
|
||||
const { createFnAgent, setHostExtensionPaths } = await import("../pi.js");
|
||||
setHostExtensionPaths(["/mock/fusion-extension"]);
|
||||
|
||||
await createFnAgent({
|
||||
cwd: "/project",
|
||||
systemPrompt: "execute",
|
||||
tools: "coding",
|
||||
sessionPurpose: "executor",
|
||||
});
|
||||
|
||||
const loaderOpts = resourceLoaderOptionsCapture.mock.calls.at(-1)?.[0] as {
|
||||
additionalExtensionPaths?: string[];
|
||||
};
|
||||
expect(loaderOpts.additionalExtensionPaths).toEqual(["/mock/fusion-extension"]);
|
||||
|
||||
setHostExtensionPaths([]);
|
||||
});
|
||||
|
||||
it("passes task-scoped env into bash spawn hook when provided", async () => {
|
||||
const { createFnAgent } = await import("../pi.js");
|
||||
|
||||
|
||||
@@ -67,6 +67,13 @@ export interface AgentRuntimeOptions {
|
||||
cwd: string;
|
||||
/** System prompt for the agent */
|
||||
systemPrompt: string;
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Session purpose must reach createFnAgent so merger lanes can skip host-extension fn_* tools.
|
||||
Those tools boot a second TaskStore via createTaskStoreForBackend and have been observed wedging merges on hung fn_task_show (no per-tool timeout, AbortSignal ignored).
|
||||
*/
|
||||
/** Lane purpose (executor/merger/triage/…). Used for host-extension policy and diagnostics. */
|
||||
sessionPurpose?: string;
|
||||
/**
|
||||
* Optional structured prompt layers for cross-session caching.
|
||||
* When present, runtimes that support prompt caching use the `stable`
|
||||
|
||||
@@ -680,17 +680,25 @@ export async function createResolvedAgentSession(
|
||||
// FNXC:GrokAcp 2026-07-12-06:30:
|
||||
// Gate customTools for non-pi runtimes before createSession so ACP/CLI
|
||||
// bridges (e.g. Grok loopback MCP) execute already-gated closures.
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Always forward sessionPurpose into runtime.createSession so pi host-extension policy can suppress dual-store fn_* tools on merger sessions (FN-7956 hang: wedged fn_task_show).
|
||||
*/
|
||||
const sessionCreateOptions: AgentRuntimeOptions =
|
||||
shouldWrapCustomToolsForRuntime(resolved.runtimeId)
|
||||
? {
|
||||
...effectiveRuntimeOptionsWithModel,
|
||||
sessionPurpose,
|
||||
customTools: wrapCustomToolsForPluginRuntime(
|
||||
effectiveRuntimeOptionsWithModel.customTools,
|
||||
effectiveRuntimeOptionsWithModel,
|
||||
{ runtimeId: resolved.runtimeId, sessionPurpose },
|
||||
),
|
||||
}
|
||||
: effectiveRuntimeOptionsWithModel;
|
||||
: {
|
||||
...effectiveRuntimeOptionsWithModel,
|
||||
sessionPurpose,
|
||||
};
|
||||
const result = await resolved.runtime.createSession(sessionCreateOptions);
|
||||
|
||||
const testModeActive = settings ? isTestModeActive(settings) : false;
|
||||
|
||||
@@ -1012,6 +1012,12 @@ export interface AgentOptions {
|
||||
systemPromptLayers?: SystemPromptLayers;
|
||||
tools?: "coding" | "readonly";
|
||||
customTools?: ToolDefinition[];
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Merger sessions must not load host @runfusion/fusion extension tools. Extension fn_task_show boots a second PostgreSQL TaskStore (createTaskStoreForBackend) inside the engine process and can hang indefinitely without a tool timeout, wedging the single-flight merge pump (observed FN-7956).
|
||||
*/
|
||||
/** Lane purpose for host-extension / tooling policy (e.g. "merger", "executor"). */
|
||||
sessionPurpose?: string;
|
||||
/**
|
||||
* Optional resolved tool-name allowlist. Undefined preserves the selected tool mode; an empty array deliberately exposes no matched tools.
|
||||
*
|
||||
@@ -2268,9 +2274,16 @@ export async function createFnAgent(options: AgentOptions): Promise<AgentResult>
|
||||
// since heartbeat/reviewer flows explicitly provide engine-owned tools.
|
||||
// This keeps summarizer/compaction sessions safe while retaining intended
|
||||
// delegation/memory tools for readonly engine sessions.
|
||||
const effectiveExtensionPaths = isReadonly ? [] : hostExtensionPaths;
|
||||
if (isReadonly && hostExtensionPaths.length > 0) {
|
||||
piLog.log(`readonly session — host extensions (${hostExtensionPaths.length}) skipped`);
|
||||
/*
|
||||
FNXC:MergeQueue 2026-07-15-11:08:
|
||||
Also skip host extensions for sessionPurpose "merger". Merge agents only need coding builtins (git/bash); host fn_* tools open a second store and can wedge the merge on hung fn_task_show. Engine-owned customTools (if ever supplied) still pass through.
|
||||
*/
|
||||
const skipHostExtensions = isReadonly || options.sessionPurpose === "merger";
|
||||
const effectiveExtensionPaths = skipHostExtensions ? [] : hostExtensionPaths;
|
||||
if (skipHostExtensions && hostExtensionPaths.length > 0) {
|
||||
piLog.log(
|
||||
`${isReadonly ? "readonly" : "merger"} session — host extensions (${hostExtensionPaths.length}) skipped`,
|
||||
);
|
||||
}
|
||||
|
||||
const resourceLoader = new DefaultResourceLoader({
|
||||
|
||||
Reference in New Issue
Block a user