fix(agents): bound finalize hook hangs after compaction retry

Behavior: a plugin before_agent_finalize hook that never resolves could previously freeze an agent run forever after a successful compaction retry, with no errors and no recovery from a gateway restart; this was the frozen-runner mechanism behind #84777. before_agent_finalize now has the same 15s default budget as sibling modifying hooks and fails open with the original final answer, converting the freeze into a bounded delay.

Surface: plugin hook runner defaults (src/plugins/hooks.ts), docs/plugins/hooks.md.

Refs #84777.

(cherry picked from commit b2baf799b4)
This commit is contained in:
Ayaan Zaidi
2026-07-07 02:54:05 +00:00
committed by Dallin Romney
parent c342750cb9
commit 5537d286dd
4 changed files with 99 additions and 0 deletions
+2
View File
@@ -394,6 +394,8 @@ final assistant answer. It is not the `/stop` cancellation path and does not
run when the user aborts a turn. Return `{ action: "revise", reason }` to ask
the harness for one more model pass before finalization, `{ action:
"finalize", reason? }` to force finalization, or omit a result to continue.
Handlers have a 15s default budget; on timeout, OpenClaw logs the failure and
continues with the original final answer.
Codex native `Stop` hooks are relayed into this hook as OpenClaw
`before_agent_finalize` decisions.
@@ -2,12 +2,27 @@
// lifecycle events, and deferred reply cleanup.
import { describe, expect, it, vi } from "vitest";
import { createInlineCodeState } from "../../packages/markdown-core/src/code-spans.js";
import { createHookRunner } from "../plugins/hooks.js";
import { createMockPluginRegistry, TEST_PLUGIN_AGENT_CTX } from "../plugins/hooks.test-helpers.js";
import { handleAgentEnd, handleAgentStart } from "./embedded-agent-subscribe.handlers.lifecycle.js";
import type { EmbeddedAgentSubscribeContext } from "./embedded-agent-subscribe.handlers.types.js";
const { emitAgentEventMock } = vi.hoisted(() => ({
emitAgentEventMock: vi.fn(),
}));
const DEFAULT_BEFORE_AGENT_FINALIZE_TIMEOUT_MS = 15_000;
const BEFORE_AGENT_FINALIZE_EVENT = {
runId: "run-1",
sessionId: "session-1",
sessionKey: "agent:main:session-1",
turnId: "turn-1",
provider: "openai",
model: "freeze-e2e",
cwd: "/repo",
transcriptPath: "/tmp/session.jsonl",
stopHookActive: false,
lastAssistantMessage: "done",
};
vi.mock("../infra/agent-events.js", () => ({
emitAgentEvent: emitAgentEventMock,
@@ -18,6 +33,7 @@ function createContext(
overrides?: {
onAgentEvent?: (event: unknown) => void;
onBeforeLifecycleTerminal?: () => void | Promise<void>;
onBeforeTerminalDelivery?: () => void | Promise<void>;
onBlockReply?: ((payload: unknown) => void) | undefined;
onBlockReplyFlush?: () => void | Promise<void>;
resolveTerminalStopReason?: () => string | undefined;
@@ -35,6 +51,7 @@ function createContext(
sessionKey: "agent:main:main",
onAgentEvent: overrides?.onAgentEvent,
onBeforeLifecycleTerminal: overrides?.onBeforeLifecycleTerminal,
onBeforeTerminalDelivery: overrides?.onBeforeTerminalDelivery,
resolveTerminalStopReason: overrides?.resolveTerminalStopReason,
...(onBlockReply ? { onBlockReply } : {}),
onBlockReplyFlush: overrides?.onBlockReplyFlush,
@@ -842,6 +859,59 @@ describe("handleAgentEnd", () => {
await endPromise;
});
it("resolves compaction retry after a timed-out terminal hook finalizes the original answer", async () => {
vi.useFakeTimers();
try {
const logger = { error: vi.fn(), warn: vi.fn(), debug: vi.fn() };
const runner = createHookRunner(
createMockPluginRegistry([
{
hookName: "before_agent_finalize",
handler: vi.fn(() => new Promise(() => {})),
},
]),
{ logger },
);
const onBeforeTerminalDelivery = vi.fn(async () => {
await runner.runBeforeAgentFinalize(BEFORE_AGENT_FINALIZE_EVENT, TEST_PLUGIN_AGENT_CTX);
return undefined;
});
const ctx = createContext(
{
role: "assistant",
content: [{ type: "text", text: "done" }],
stopReason: "stop",
},
{ onBeforeTerminalDelivery },
);
ctx.state.assistantTexts = ["done"];
ctx.state.pendingCompactionRetry = 1;
const endPromise = handleAgentEnd(ctx);
await Promise.resolve();
expect(onBeforeTerminalDelivery).toHaveBeenCalledTimes(1);
expect(ctx.resolveCompactionRetry).not.toHaveBeenCalled();
expect(ctx.flushBlockReplyBuffer).not.toHaveBeenCalledWith({ final: true });
await vi.advanceTimersByTimeAsync(DEFAULT_BEFORE_AGENT_FINALIZE_TIMEOUT_MS);
await endPromise;
expect(logger.error).toHaveBeenCalledWith(
"[hooks] before_agent_finalize handler from test-plugin failed: timed out after 15000ms",
);
expect(ctx.clearDeferredAssistantEvents).not.toHaveBeenCalled();
expect(ctx.clearDeferredBlockReplies).not.toHaveBeenCalled();
expect(ctx.flushDeferredAssistantEvents).toHaveBeenCalledTimes(1);
expect(ctx.flushDeferredBlockReplies).toHaveBeenCalledTimes(1);
expect(ctx.flushBlockReplyBuffer).toHaveBeenCalledWith({ final: true });
expect(ctx.resolveCompactionRetry).toHaveBeenCalledTimes(1);
expect(ctx.maybeResolveCompactionWait).not.toHaveBeenCalled();
} finally {
vi.useRealTimers();
}
});
it("runs the before-lifecycle callback before the lifecycle end event", async () => {
const order: string[] = [];
const onAgentEvent = vi.fn(() => {
@@ -15,6 +15,7 @@ const EVENT = {
stopHookActive: false,
lastAssistantMessage: "done",
};
const DEFAULT_BEFORE_AGENT_FINALIZE_TIMEOUT_MS = 15_000;
describe("before_agent_finalize hook runner", () => {
it("returns undefined when no hooks are registered", async () => {
@@ -202,6 +203,28 @@ describe("before_agent_finalize hook runner", () => {
});
});
it("times out hung handlers and continues with the original final answer", async () => {
vi.useFakeTimers();
try {
const handler = vi.fn(() => new Promise(() => {}));
const logger = { error: vi.fn(), warn: vi.fn(), debug: vi.fn() };
const runner = createHookRunner(
createMockPluginRegistry([{ hookName: "before_agent_finalize", handler }]),
{ logger },
);
const run = runner.runBeforeAgentFinalize(EVENT, TEST_PLUGIN_AGENT_CTX);
await vi.advanceTimersByTimeAsync(DEFAULT_BEFORE_AGENT_FINALIZE_TIMEOUT_MS);
await expect(run).resolves.toBeUndefined();
expect(logger.error).toHaveBeenCalledWith(
"[hooks] before_agent_finalize handler from test-plugin failed: timed out after 15000ms",
);
} finally {
vi.useRealTimers();
}
});
it("hasHooks reports correctly", () => {
const runner = createHookRunner(
createMockPluginRegistry([{ hookName: "before_agent_finalize", handler: vi.fn() }]),
+4
View File
@@ -228,6 +228,10 @@ const DEFAULT_MODIFYING_HOOK_TIMEOUT_MS_BY_HOOK: Partial<Record<PluginHookName,
// The runner is fail-open for this hook name, so a timed-out handler is
// logged and the run proceeds without its modifications.
before_agent_start: 15_000,
// Terminal finalization hooks sit on the runner's completion path. A hung
// handler must not freeze final delivery or keep compaction retry recovery
// unresolved; timeout fail-opens with the original final answer.
before_agent_finalize: 15_000,
before_prompt_build: 15_000,
resolve_exec_env: 15_000,
};