diff --git a/docs/plugins/hooks.md b/docs/plugins/hooks.md index a9424ead8ca8..3a20f24e42e9 100644 --- a/docs/plugins/hooks.md +++ b/docs/plugins/hooks.md @@ -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. diff --git a/src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts b/src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts index 59fa76c14336..acdc06cacee9 100644 --- a/src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts +++ b/src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts @@ -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; + onBeforeTerminalDelivery?: () => void | Promise; onBlockReply?: ((payload: unknown) => void) | undefined; onBlockReplyFlush?: () => void | Promise; 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(() => { diff --git a/src/plugins/hooks.before-agent-finalize.test.ts b/src/plugins/hooks.before-agent-finalize.test.ts index 215594d3018d..1ff50711a36a 100644 --- a/src/plugins/hooks.before-agent-finalize.test.ts +++ b/src/plugins/hooks.before-agent-finalize.test.ts @@ -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() }]), diff --git a/src/plugins/hooks.ts b/src/plugins/hooks.ts index 57abb8ce3ffa..902c32b39fc0 100644 --- a/src/plugins/hooks.ts +++ b/src/plugins/hooks.ts @@ -228,6 +228,10 @@ const DEFAULT_MODIFYING_HOOK_TIMEOUT_MS_BY_HOOK: Partial