fix(plugins): agent runs hang when tool-result middleware never settles (#110731)

* fix(plugins): prevent tool-result middleware from freezing runs

* test(plugins): cover middleware timeout policy

* test(plugins): provide hook context in timeout test

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
xingzhou
2026-07-18 17:11:59 -07:00
committed by GitHub
co-authored by Peter Steinberger
parent b550c2cf9a
commit 9b42782f76
3 changed files with 162 additions and 3 deletions
@@ -1,7 +1,7 @@
// Imported by loader.test.ts to keep its mocked suite in one Vitest module graph.
import fs from "node:fs";
import path from "node:path";
import { afterAll, afterEach, describe, expect, it } from "vitest";
import { afterAll, afterEach, describe, expect, it, vi } from "vitest";
import { withEnv } from "../test-utils/env.js";
import { createHookRunner } from "./hooks.js";
import { loadOpenClawPlugins } from "./loader.js";
@@ -19,6 +19,7 @@ import {
createSetupEntryChannelPluginFixture,
globalAfterEach0,
globalAfterAll1,
updatePluginManifest,
} from "./loader.test-harness.js";
import { loadPluginManifestRegistry } from "./manifest-registry.js";
@@ -1693,6 +1694,156 @@ describe("loadOpenClawPlugins", () => {
});
});
it.each([
{
label: "per-hook timeout over the global timeout",
hooks: { timeoutMs: 250, timeouts: { after_tool_call: 25 } },
expectedTimeoutMs: 25,
},
{
label: "global timeout when no per-hook timeout is configured",
hooks: { timeoutMs: 40 },
expectedTimeoutMs: 40,
},
])("bounds agent tool-result middleware with $label", async ({ hooks, expectedTimeoutMs }) => {
useNoBundledPlugins();
const pluginId = `tool-result-middleware-timeout-${expectedTimeoutMs}`;
const plugin = writePlugin({
id: pluginId,
filename: `${pluginId}.cjs`,
body: `module.exports = { id: ${JSON.stringify(pluginId)}, register(api) {
api.registerAgentToolResultMiddleware(() => new Promise(() => {}), {
runtimes: ["openclaw"],
});
} };`,
});
updatePluginManifest(plugin, {
contracts: { agentToolResultMiddleware: ["openclaw"] },
});
const registry = loadRegistryFromSinglePlugin({
plugin,
pluginConfig: {
allow: [pluginId],
entries: {
[pluginId]: {
hooks,
},
},
},
});
const middleware = registry.agentToolResultMiddlewares[0];
if (!middleware) {
throw new Error("expected tool-result middleware registration");
}
vi.useFakeTimers();
try {
const middlewareRun = middleware.handler(
{
toolCallId: "call-1",
toolName: "exec",
args: {},
result: { content: [{ type: "text", text: "raw" }], details: {} },
},
{ runtime: "openclaw" },
);
const outcome = Promise.resolve(middlewareRun).then(
() => ({ status: "resolved" as const }),
(error: unknown) => ({
status: "rejected" as const,
message: error instanceof Error ? error.message : String(error),
}),
);
await vi.advanceTimersByTimeAsync(expectedTimeoutMs);
await expect(outcome).resolves.toEqual({
status: "rejected",
message: `agent tool result middleware for ${pluginId} timed out after ${expectedTimeoutMs}ms`,
});
} finally {
vi.useRealTimers();
}
});
it("leaves agent tool-result middleware unbounded when no timeout is configured", async () => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "tool-result-middleware-no-timeout",
filename: "tool-result-middleware-no-timeout.cjs",
body: `module.exports = { id: "tool-result-middleware-no-timeout", register(api) {
api.registerAgentToolResultMiddleware(() => new Promise(() => {}), {
runtimes: ["openclaw"],
});
} };`,
});
updatePluginManifest(plugin, {
contracts: { agentToolResultMiddleware: ["openclaw"] },
});
const registry = loadRegistryFromSinglePlugin({
plugin,
pluginConfig: { allow: ["tool-result-middleware-no-timeout"] },
});
const middleware = registry.agentToolResultMiddlewares[0];
if (!middleware) {
throw new Error("expected tool-result middleware registration");
}
vi.useFakeTimers();
try {
let settled = false;
void Promise.resolve(
middleware.handler(
{
toolCallId: "call-1",
toolName: "exec",
args: {},
result: { content: [{ type: "text", text: "raw" }], details: {} },
},
{ runtime: "openclaw" },
),
).finally(() => {
settled = true;
});
await vi.advanceTimersByTimeAsync(1_000);
expect(settled).toBe(false);
} finally {
vi.useRealTimers();
}
});
it("uses the registration option when typed hook policy has no timeout", async () => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "after-tool-call-option-timeout",
filename: "after-tool-call-option-timeout.cjs",
body: `module.exports = { id: "after-tool-call-option-timeout", register(api) {
api.on("after_tool_call", () => new Promise(() => {}), { timeoutMs: 30 });
} };`,
});
const registry = loadRegistryFromSinglePlugin({
plugin,
pluginConfig: { allow: ["after-tool-call-option-timeout"] },
});
const logger = { debug: vi.fn(), error: vi.fn(), warn: vi.fn() };
const runner = createHookRunner(registry, { logger });
vi.useFakeTimers();
try {
const run = runner.runAfterToolCall({ toolName: "exec", params: {} }, { toolName: "exec" });
await vi.advanceTimersByTimeAsync(30);
await expect(run).resolves.toBeUndefined();
expect(logger.error).toHaveBeenCalledWith(
"[hooks] after_tool_call handler from after-tool-call-option-timeout failed: timed out after 30ms",
);
} finally {
vi.useRealTimers();
}
});
it("blocks conversation typed hooks for non-bundled plugins unless explicitly allowed", () => {
useNoBundledPlugins();
const plugin = writePlugin({
+1 -1
View File
@@ -239,7 +239,7 @@ export function createPluginApiFactory(
registerCodexAppServerExtensionFactory(record, factory);
},
registerAgentToolResultMiddleware: (handler, options) => {
registerAgentToolResultMiddleware(record, handler, options);
registerAgentToolResultMiddleware(record, handler, options, params.hookPolicy);
},
registerSessionExtension: (extension) => registerSessionExtension(record, extension),
enqueueNextTurnInjection: (injection) => {
@@ -5,6 +5,7 @@ import type { AnyAgentTool } from "../agents/tools/common.js";
import { registerInternalHook, unregisterInternalHook } from "../hooks/internal-hooks.js";
import type { HookEntry } from "../hooks/types.js";
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
import { withTimeout } from "../utils/with-timeout.js";
import type { AgentToolResultMiddleware } from "./agent-tool-result-middleware-types.js";
import {
normalizeAgentToolResultMiddlewareRuntimeIds,
@@ -170,6 +171,7 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
record: PluginRecord,
handler: Parameters<OpenClawPluginApi["registerAgentToolResultMiddleware"]>[0],
options: Parameters<OpenClawPluginApi["registerAgentToolResultMiddleware"]>[1],
policy?: PluginTypedHookPolicy,
) => {
if (typeof (handler as unknown) !== "function") {
pushDiagnostic({
@@ -219,9 +221,15 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
existing.runtimes = uniqueValues([...existing.runtimes, ...runtimes]);
return;
}
const timeoutMs = resolveTypedHookTimeoutMs({ hookName: "after_tool_call", policy });
const safeHandler: AgentToolResultMiddleware = async (event, ctx) => {
try {
return await handler(event, ctx);
// fs-safe bounds only this await; it cannot cancel plugin work, so late side effects remain possible.
return await withTimeout(
Promise.resolve(handler(event, ctx)),
timeoutMs ?? 0,
`agent tool result middleware for ${record.id}`,
);
} catch (error) {
registryParams.logger.warn(
`[plugins] agent tool result middleware failed for ${record.id}`,