Skip to content

Commit 5ad27bd

Browse files
Eva (agent)clawsweeper[bot]
Eva (agent)
authored and
clawsweeper[bot]
committed
fix(plugins): bound before_compaction/after_compaction hooks with a default timeout
DEFAULT_VOID_HOOK_TIMEOUT_MS_BY_HOOK only listed agent_end, so the before_compaction and after_compaction void hooks ran fully unbounded when a plugin supplied no hook.timeoutMs. In the codex agent harness these hooks fire on the serialized notification queue, so a slow or hung handler froze processing of every later codex notification — including turn/completed — hanging the whole agent turn. Add defensive default timeout entries for both hooks, mirroring the existing agent_end pattern. The budget matches agent_end's 30s rather than the tighter modifying-hook defaults because compaction hooks can legitimately do real work (e.g. a memory flush). The runner is fail-open for void hooks, so a timed-out handler is logged and compaction proceeds.
1 parent c81271e commit 5ad27bd

2 files changed

Lines changed: 133 additions & 0 deletions

File tree

‎src/plugins/hooks.compaction-timeout.test.ts‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/**
2+
* Test: before_compaction & after_compaction void-hook default timeouts.
3+
*
4+
* Without a default budget these hooks run fully unbounded. In the codex
5+
* agent harness they fire on the serialized notification queue, so a hung
6+
* handler freezes every later codex notification — including turn/completed —
7+
* and the whole turn hangs. The runner seeds DEFAULT_VOID_HOOK_TIMEOUT_MS_BY_HOOK
8+
* with a defensive budget for both hooks; these tests assert a never-settling
9+
* handler is bounded by that default rather than hanging.
10+
*/
11+
import { beforeEach, describe, expect, it, vi } from "vitest";
12+
import { createHookRunner } from "./hooks.js";
13+
import { addTestHook, TEST_PLUGIN_AGENT_CTX } from "./hooks.test-helpers.js";
14+
import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js";
15+
import type { PluginHookRegistration } from "./types.js";
16+
17+
// The defensive default applied to before_compaction / after_compaction in
18+
// DEFAULT_VOID_HOOK_TIMEOUT_MS_BY_HOOK. Kept in sync with hooks.ts.
19+
const DEFAULT_COMPACTION_HOOK_TIMEOUT_MS = 30_000;
20+
21+
describe("compaction hook default timeouts", () => {
22+
let registry: PluginRegistry;
23+
24+
beforeEach(() => {
25+
registry = createEmptyPluginRegistry();
26+
});
27+
28+
it("bounds a never-settling before_compaction handler with the default timeout", async () => {
29+
vi.useFakeTimers();
30+
try {
31+
const handler = vi.fn(() => new Promise<void>(() => {}));
32+
addTestHook({
33+
registry,
34+
pluginId: "plugin-a",
35+
hookName: "before_compaction",
36+
handler: handler as PluginHookRegistration["handler"],
37+
});
38+
const logger = {
39+
error: vi.fn(),
40+
warn: vi.fn(),
41+
};
42+
43+
// No voidHookTimeoutMsByHook override — relies on the built-in default.
44+
const runner = createHookRunner(registry, { logger });
45+
const run = runner.runBeforeCompaction({ messageCount: 3 }, TEST_PLUGIN_AGENT_CTX);
46+
47+
await vi.advanceTimersByTimeAsync(DEFAULT_COMPACTION_HOOK_TIMEOUT_MS);
48+
49+
await expect(run).resolves.toBeUndefined();
50+
expect(logger.error).toHaveBeenCalledWith(
51+
`[hooks] before_compaction handler from plugin-a failed: timed out after ${DEFAULT_COMPACTION_HOOK_TIMEOUT_MS}ms`,
52+
);
53+
} finally {
54+
vi.useRealTimers();
55+
}
56+
});
57+
58+
it("bounds a never-settling after_compaction handler with the default timeout", async () => {
59+
vi.useFakeTimers();
60+
try {
61+
const handler = vi.fn(() => new Promise<void>(() => {}));
62+
addTestHook({
63+
registry,
64+
pluginId: "plugin-a",
65+
hookName: "after_compaction",
66+
handler: handler as PluginHookRegistration["handler"],
67+
});
68+
const logger = {
69+
error: vi.fn(),
70+
warn: vi.fn(),
71+
};
72+
73+
const runner = createHookRunner(registry, { logger });
74+
const run = runner.runAfterCompaction(
75+
{ messageCount: 2, compactedCount: 1 },
76+
TEST_PLUGIN_AGENT_CTX,
77+
);
78+
79+
await vi.advanceTimersByTimeAsync(DEFAULT_COMPACTION_HOOK_TIMEOUT_MS);
80+
81+
await expect(run).resolves.toBeUndefined();
82+
expect(logger.error).toHaveBeenCalledWith(
83+
`[hooks] after_compaction handler from plugin-a failed: timed out after ${DEFAULT_COMPACTION_HOOK_TIMEOUT_MS}ms`,
84+
);
85+
} finally {
86+
vi.useRealTimers();
87+
}
88+
});
89+
90+
it("lets a fast before_compaction handler complete without timing out", async () => {
91+
vi.useFakeTimers();
92+
try {
93+
const handler = vi.fn(
94+
async () =>
95+
await new Promise<void>((resolve) => {
96+
setTimeout(resolve, 20);
97+
}),
98+
);
99+
addTestHook({
100+
registry,
101+
pluginId: "plugin-a",
102+
hookName: "before_compaction",
103+
handler: handler as PluginHookRegistration["handler"],
104+
});
105+
const logger = {
106+
error: vi.fn(),
107+
warn: vi.fn(),
108+
};
109+
110+
const runner = createHookRunner(registry, { logger });
111+
const run = runner.runBeforeCompaction({ messageCount: 3 }, TEST_PLUGIN_AGENT_CTX);
112+
113+
await vi.advanceTimersByTimeAsync(20);
114+
115+
await expect(run).resolves.toBeUndefined();
116+
expect(logger.error).not.toHaveBeenCalled();
117+
} finally {
118+
vi.useRealTimers();
119+
}
120+
});
121+
});

‎src/plugins/hooks.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,18 @@ export type HookRunnerOptions = {
189189

190190
const DEFAULT_VOID_HOOK_TIMEOUT_MS_BY_HOOK: Partial<Record<PluginHookName, number>> = {
191191
agent_end: 30_000,
192+
// Defensive default for the compaction lifecycle hooks. Without a budget an
193+
// unresponsive handler runs fully unbounded, and in the codex agent harness
194+
// these hooks fire on the serialized notification queue
195+
// (event-projector handleItemStarted awaits before_compaction / after_compaction
196+
// for a contextCompaction item), so a hung handler freezes every later codex
197+
// notification — including turn/completed — and the whole turn hangs. These
198+
// hooks can legitimately do real work (e.g. a memory flush), so the budget
199+
// matches agent_end's 30s rather than the tighter modifying-hook defaults.
200+
// The runner is fail-open for void hooks, so a timed-out handler is logged
201+
// and compaction proceeds.
202+
before_compaction: 30_000,
203+
after_compaction: 30_000,
192204
};
193205
const DEFAULT_MODIFYING_HOOK_TIMEOUT_MS_BY_HOOK: Partial<Record<PluginHookName, number>> = {
194206
before_agent_run: 15_000,

0 commit comments

Comments
 (0)