Skip to content

Commit 68dabb7

Browse files
joshavantjoshavant
committed
fix: bound native hook relay lifetime
1 parent 80f1ae6 commit 68dabb7

6 files changed

Lines changed: 428 additions & 77 deletions

File tree

‎extensions/codex/src/app-server/native-hook-relay.test.ts‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { describe, expect, it } from "vitest";
55
import {
66
buildCodexNativeHookRelayConfig,
77
buildCodexNativeHookRelayDisabledConfig,
8+
resolveCodexNativeHookRelayCommandTimeoutMs,
89
resolveCodexNativeHookRelayUnregisterGraceMs,
910
} from "./native-hook-relay.js";
1011

@@ -23,7 +24,7 @@ describe("Codex native hook relay config", () => {
2324
{
2425
type: "command",
2526
command:
26-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use",
27+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --timeout 6000",
2728
timeout: 7,
2829
async: false,
2930
statusMessage: "OpenClaw native hook relay",
@@ -37,7 +38,7 @@ describe("Codex native hook relay config", () => {
3738
{
3839
type: "command",
3940
command:
40-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event post_tool_use",
41+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event post_tool_use --timeout 6000",
4142
timeout: 7,
4243
async: false,
4344
statusMessage: "OpenClaw native hook relay",
@@ -51,7 +52,7 @@ describe("Codex native hook relay config", () => {
5152
{
5253
type: "command",
5354
command:
54-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request",
55+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request --timeout 6000",
5556
timeout: 7,
5657
async: false,
5758
statusMessage: "OpenClaw native hook relay",
@@ -65,7 +66,7 @@ describe("Codex native hook relay config", () => {
6566
{
6667
type: "command",
6768
command:
68-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event before_agent_finalize",
69+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event before_agent_finalize --timeout 6000",
6970
timeout: 7,
7071
async: false,
7172
statusMessage: "OpenClaw native hook relay",
@@ -128,7 +129,7 @@ describe("Codex native hook relay config", () => {
128129
{
129130
type: "command",
130131
command:
131-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request",
132+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request --timeout 4000",
132133
timeout: 5,
133134
async: false,
134135
statusMessage: "OpenClaw native hook relay",
@@ -163,7 +164,7 @@ describe("Codex native hook relay config", () => {
163164
{
164165
type: "command",
165166
command:
166-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use",
167+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --timeout 4000",
167168
timeout: 5,
168169
async: false,
169170
statusMessage: "OpenClaw native hook relay",
@@ -200,7 +201,7 @@ describe("Codex native hook relay config", () => {
200201
{
201202
type: "command",
202203
command:
203-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --pre-tool-use-unavailable noop",
204+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --pre-tool-use-unavailable noop --timeout 4000",
204205
timeout: 5,
205206
async: false,
206207
statusMessage: "OpenClaw native hook relay",
@@ -238,7 +239,7 @@ describe("Codex native hook relay config", () => {
238239
{
239240
type: "command",
240241
command:
241-
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request",
242+
"openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request --timeout 4000",
242243
timeout: 5,
243244
async: false,
244245
statusMessage: "OpenClaw native hook relay",
@@ -266,6 +267,12 @@ describe("Codex native hook relay config", () => {
266267
});
267268
});
268269

270+
it("reserves relay timeout margin before Codex can kill the hook subprocess", () => {
271+
expect(resolveCodexNativeHookRelayCommandTimeoutMs(undefined)).toBe(4000);
272+
expect(resolveCodexNativeHookRelayCommandTimeoutMs(1)).toBe(750);
273+
expect(resolveCodexNativeHookRelayCommandTimeoutMs(7)).toBe(6000);
274+
});
275+
269276
it("omits matchers so Codex MCP tool names reach the relay with a stable trust hash", () => {
270277
const config = buildCodexNativeHookRelayConfig({
271278
relay: createRelay(),
@@ -311,12 +318,12 @@ function createRelay(options?: {
311318
allowedEvents: ["pre_tool_use", "post_tool_use", "permission_request", "before_agent_finalize"],
312319
expiresAtMs: Date.now() + 1000,
313320
shouldRelayEvent: (event) => !inactiveEvents.has(event),
314-
commandForEvent: (event) =>
321+
commandForEvent: (event, commandOptions) =>
315322
`openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event ${event}${
316323
event === "pre_tool_use" && inactiveEvents.has(event)
317324
? " --pre-tool-use-unavailable noop"
318325
: ""
319-
}`,
326+
}${commandOptions?.timeoutMs ? ` --timeout ${commandOptions.timeoutMs}` : ""}`,
320327
renew: () => undefined,
321328
unregister: () => undefined,
322329
};

‎extensions/codex/src/app-server/native-hook-relay.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ const CODEX_NATIVE_HOOK_RELAY_EVENTS_WITH_APP_SERVER_APPROVALS =
2929
const CODEX_NATIVE_HOOK_RELAY_MIN_TTL_MS = 30 * 60_000;
3030
/** Extra relay lifetime after the expected turn budget, preventing late hook drops. */
3131
export const CODEX_NATIVE_HOOK_RELAY_TTL_GRACE_MS = 5 * 60_000;
32+
const CODEX_NATIVE_HOOK_RELAY_COMMAND_MIN_PARENT_MARGIN_MS = 250;
33+
const CODEX_NATIVE_HOOK_RELAY_COMMAND_MAX_PARENT_MARGIN_MS = 1_000;
3234
const CODEX_NATIVE_HOOK_RELAY_UNREGISTER_GRACE_MS = 10_000;
3335
const CODEX_NATIVE_HOOK_RELAY_UNREGISTER_EXTRA_GRACE_MS = 5_000;
3436

@@ -263,8 +265,10 @@ export function buildCodexNativeHookRelayConfig(params: {
263265
}
264266
continue;
265267
}
266-
const command = params.relay.commandForEvent(event);
267268
const timeout = normalizeHookTimeoutSec(params.hookTimeoutSec);
269+
const command = params.relay.commandForEvent(event, {
270+
timeoutMs: resolveCodexNativeHookRelayCommandTimeoutMs(timeout),
271+
});
268272
config[`hooks.${codexEvent}`] = [
269273
{
270274
hooks: [
@@ -311,6 +315,18 @@ function normalizeHookTimeoutSec(value: number | undefined): number {
311315
return typeof value === "number" && Number.isFinite(value) && value > 0 ? Math.ceil(value) : 5;
312316
}
313317

318+
export function resolveCodexNativeHookRelayCommandTimeoutMs(
319+
hookTimeoutSec: number | undefined,
320+
): number {
321+
const parentTimeoutMs =
322+
finiteSecondsToTimerSafeMilliseconds(normalizeHookTimeoutSec(hookTimeoutSec)) ?? 5_000;
323+
const parentMarginMs = Math.min(
324+
CODEX_NATIVE_HOOK_RELAY_COMMAND_MAX_PARENT_MARGIN_MS,
325+
Math.max(CODEX_NATIVE_HOOK_RELAY_COMMAND_MIN_PARENT_MARGIN_MS, Math.floor(parentTimeoutMs / 5)),
326+
);
327+
return Math.max(1, parentTimeoutMs - parentMarginMs);
328+
}
329+
314330
function codexCommandHookTrustedHash(params: {
315331
event: NativeHookRelayEvent;
316332
command: string;

‎src/agents/harness/native-hook-relay.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,14 @@ describe("native hook relay registry", () => {
249249
"/usr/local/bin/node '/opt/Open Claw/openclaw.mjs' hooks relay --provider codex --relay-id " +
250250
`${relay.relayId} --generation ${relay.generation} --event pre_tool_use --timeout 1234`,
251251
);
252+
expect(relay.commandForEvent("pre_tool_use", { timeoutMs: 900 })).toBe(
253+
"/usr/local/bin/node '/opt/Open Claw/openclaw.mjs' hooks relay --provider codex --relay-id " +
254+
`${relay.relayId} --generation ${relay.generation} --event pre_tool_use --timeout 900`,
255+
);
256+
expect(relay.commandForEvent("pre_tool_use", { timeoutMs: 2_000 })).toBe(
257+
"/usr/local/bin/node '/opt/Open Claw/openclaw.mjs' hooks relay --provider codex --relay-id " +
258+
`${relay.relayId} --generation ${relay.generation} --event pre_tool_use --timeout 1234`,
259+
);
252260
});
253261

254262
it("rejects relay registrations when expiry would exceed Date range", () => {

‎src/agents/harness/native-hook-relay.ts‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ export type NativeHookRelayRegistration = {
111111
export type NativeHookRelayRegistrationHandle = NativeHookRelayRegistration & {
112112
generation?: string;
113113
shouldRelayEvent: (event: NativeHookRelayEvent) => boolean;
114-
commandForEvent: (event: NativeHookRelayEvent) => string;
114+
commandForEvent: (
115+
event: NativeHookRelayEvent,
116+
options?: NativeHookRelayCommandForEventOptions,
117+
) => string;
115118
renew: (ttlMs?: number) => void;
116119
unregister: () => void;
117120
};
@@ -140,6 +143,10 @@ export type NativeHookRelayCommandOptions = {
140143
timeoutMs?: number;
141144
};
142145

146+
export type NativeHookRelayCommandForEventOptions = {
147+
timeoutMs?: number;
148+
};
149+
143150
export type InvokeNativeHookRelayParams = {
144151
provider: unknown;
145152
relayId: unknown;
@@ -436,7 +443,7 @@ export function registerNativeHookRelay(
436443
const handle: ActiveNativeHookRelayRegistrationHandle = {
437444
...registration,
438445
shouldRelayEvent: (event) => nativeHookRelayEventHasLocalWork(registration, event),
439-
commandForEvent: (event) =>
446+
commandForEvent: (event, options) =>
440447
buildNativeHookRelayCommand({
441448
provider: params.provider,
442449
relayId,
@@ -447,7 +454,10 @@ export function registerNativeHookRelay(
447454
? "noop"
448455
: undefined,
449456
nice: params.command?.nice,
450-
timeoutMs: params.command?.timeoutMs,
457+
timeoutMs: resolveNativeHookRelayCommandTimeoutMs(
458+
params.command?.timeoutMs,
459+
options?.timeoutMs,
460+
),
451461
executable: params.command?.executable,
452462
nodeExecutable: params.command?.nodeExecutable,
453463
}),
@@ -519,6 +529,21 @@ function resolveNativeHookRelayNicePrefix(value: number | false | undefined): st
519529
return ["nice", "-n", String(nice)];
520530
}
521531

532+
function resolveNativeHookRelayCommandTimeoutMs(
533+
configuredTimeoutMs: number | undefined,
534+
overrideTimeoutMs: number | undefined,
535+
): number | undefined {
536+
const configured = normalizeOptionalPositiveInteger(configuredTimeoutMs);
537+
const override = normalizeOptionalPositiveInteger(overrideTimeoutMs);
538+
if (configured === undefined) {
539+
return override;
540+
}
541+
if (override === undefined) {
542+
return configured;
543+
}
544+
return Math.min(configured, override);
545+
}
546+
522547
export function buildNativeHookRelayCommand(params: {
523548
provider: NativeHookRelayProvider;
524549
relayId: string;
@@ -2157,6 +2182,12 @@ function normalizePositiveInteger(value: number | undefined, fallback: number):
21572182
: fallback;
21582183
}
21592184

2185+
function normalizeOptionalPositiveInteger(value: number | undefined): number | undefined {
2186+
return typeof value === "number" && Number.isFinite(value) && value > 0
2187+
? Math.floor(value)
2188+
: undefined;
2189+
}
2190+
21602191
function shellQuoteArgs(args: readonly string[]): string {
21612192
return args.map((arg) => shellQuoteArg(arg, process.platform)).join(" ");
21622193
}

‎src/cli/native-hook-relay-cli.test.ts‎

Lines changed: 123 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Native hook relay CLI tests cover relay command registration and runtime delegation.
2+
import { PassThrough } from "node:stream";
23
import { describe, expect, it, vi } from "vitest";
34
import {
45
createReadableTextStream,
@@ -8,7 +9,7 @@ import {
89

910
describe("native hook relay CLI", () => {
1011
it("reads Codex hook JSON from stdin and forwards it to the gateway relay", async () => {
11-
const callGateway = vi.fn(async () => ({ stdout: "", stderr: "", exitCode: 0 }));
12+
const callGateway = vi.fn(async (_opts: unknown) => ({ stdout: "", stderr: "", exitCode: 0 }));
1213
const stdout = createWritableTextBuffer();
1314
const stderr = createWritableTextBuffer();
1415

@@ -50,9 +51,14 @@ describe("native hook relay CLI", () => {
5051
tool_input: { command: "pnpm test" },
5152
},
5253
},
53-
timeoutMs: 1234,
54+
timeoutMs: expect.any(Number),
55+
signal: expect.any(AbortSignal),
5456
scopes: ["operator.admin"],
5557
});
58+
const call = callGateway.mock.calls[0]?.[0] as { timeoutMs?: number } | undefined;
59+
expect(call).toBeDefined();
60+
expect(call?.timeoutMs).toBeGreaterThan(0);
61+
expect(call?.timeoutMs).toBeLessThanOrEqual(1234);
5662
});
5763

5864
it("renders provider-compatible stdout, stderr, and exit code from the gateway response", async () => {
@@ -260,6 +266,115 @@ describe("native hook relay CLI", () => {
260266
expect(callGateway).not.toHaveBeenCalled();
261267
});
262268

269+
it.each([
270+
{
271+
event: "pre_tool_use",
272+
preToolUseUnavailable: "noop",
273+
stdout: null,
274+
},
275+
{
276+
event: "pre_tool_use",
277+
stdout: {
278+
hookSpecificOutput: {
279+
hookEventName: "PreToolUse",
280+
permissionDecision: "deny",
281+
permissionDecisionReason: "Native hook relay timed out",
282+
},
283+
},
284+
},
285+
{
286+
event: "permission_request",
287+
stdout: {
288+
hookSpecificOutput: {
289+
hookEventName: "PermissionRequest",
290+
decision: {
291+
behavior: "deny",
292+
message: "Native hook relay timed out",
293+
},
294+
},
295+
},
296+
},
297+
{
298+
event: "post_tool_use",
299+
stdout: null,
300+
},
301+
])(
302+
"bounds valid $event hook input that never reaches EOF",
303+
async (testCase) => {
304+
const invokeBridge = vi.fn();
305+
const callGateway = vi.fn();
306+
const stdin = createHeldOpenTextStream("{}");
307+
const stdout = createWritableTextBuffer();
308+
const stderr = createWritableTextBuffer();
309+
310+
const exitCode = await runNativeHookRelayCli(
311+
{
312+
provider: "codex",
313+
relayId: "relay-1",
314+
generation: "generation-1",
315+
event: testCase.event,
316+
preToolUseUnavailable: testCase.preToolUseUnavailable,
317+
timeout: "25",
318+
},
319+
{
320+
stdin,
321+
stdout,
322+
stderr,
323+
invokeBridge: invokeBridge as never,
324+
callGateway: callGateway as never,
325+
},
326+
);
327+
328+
expect(exitCode).toBe(0);
329+
if (testCase.stdout) {
330+
expect(JSON.parse(stdout.text())).toEqual(testCase.stdout);
331+
} else {
332+
expect(stdout.text()).toBe("");
333+
}
334+
expect(stderr.text()).toContain("native hook relay timed out");
335+
expect(stdin.destroyed).toBe(true);
336+
expect(invokeBridge).not.toHaveBeenCalled();
337+
expect(callGateway).not.toHaveBeenCalled();
338+
},
339+
1_000,
340+
);
341+
342+
it("applies the relay deadline to gateway fallback", async () => {
343+
const invokeBridge = vi.fn(async () => {
344+
throw new Error("bridge unavailable");
345+
});
346+
const callGateway = vi.fn(async () => await new Promise<never>(() => {}));
347+
const stdout = createWritableTextBuffer();
348+
const stderr = createWritableTextBuffer();
349+
350+
const exitCode = await runNativeHookRelayCli(
351+
{
352+
provider: "codex",
353+
relayId: "relay-1",
354+
generation: "generation-1",
355+
event: "post_tool_use",
356+
timeout: "25",
357+
},
358+
{
359+
stdin: createReadableTextStream("{}"),
360+
stdout,
361+
stderr,
362+
invokeBridge: invokeBridge as never,
363+
callGateway: callGateway as never,
364+
},
365+
);
366+
367+
expect(exitCode).toBe(0);
368+
expect(stdout.text()).toBe("");
369+
expect(stderr.text()).toContain("native hook relay timed out");
370+
expect(callGateway).toHaveBeenCalledWith(
371+
expect.objectContaining({
372+
method: "nativeHook.invoke",
373+
signal: expect.any(AbortSignal),
374+
}),
375+
);
376+
}, 1_000);
377+
263378
it("rejects oversized hook input without touching the gateway", async () => {
264379
const callGateway = vi.fn();
265380
const stderr = createWritableTextBuffer();
@@ -417,3 +532,9 @@ describe("native hook relay CLI", () => {
417532
expect(stderr.text()).toContain("native hook relay unavailable");
418533
});
419534
});
535+
536+
function createHeldOpenTextStream(text: string): PassThrough {
537+
const stream = new PassThrough();
538+
stream.write(text);
539+
return stream;
540+
}

0 commit comments

Comments
 (0)