Skip to content

Commit 96959ec

Browse files
KaspreKaspre
authored
fix(codex): defer native-hook-relay unregister to avoid cleanup race
Keep successful Codex native hook relays alive through a bounded grace window so late hook callbacks still reach OpenClaw enforcement, while interrupted, aborted, timed-out, and failed turns unregister immediately.\n\nCo-authored-by: Kaspre <[email protected]>
1 parent 0abedd5 commit 96959ec

6 files changed

Lines changed: 269 additions & 19 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ Docs: https://docs.openclaw.ai
7171
- WebChat: keep the run-complete indicator in progress until deferred history replay renders the assistant reply, so Done no longer appears before response text. (#85374) Thanks @neeravmakwana.
7272
- Agents/tools: give timed-out or cancelled process trees a bounded SIGTERM cleanup window before SIGKILL while preserving tree-aware cancellation. Fixes #66399. (#85865) Thanks @IWhatsskill.
7373
- Agents/compaction: skip agent-harness preflight for provider-owned CLI runtime sessions so over-threshold Claude CLI sessions continue through normal compaction instead of failing on a missing harness. Fixes #84857. (#84878) Thanks @zhangguiping-xydt.
74+
- Codex/app-server: keep successful native hook relays available through a short post-turn grace window so late Codex hook subprocesses can finish policy enforcement without clearing a replacement relay. (#83987) Thanks @Kaspre.
7475
- Control UI/config: save form-mode edits from the source config snapshot so runtime-only provider defaults like empty `models.providers.<id>.baseUrl` are not written back and rejected. Fixes #85831. Thanks @garyd9.
7576
- Browser/existing-session: launch Chrome DevTools MCP with usage statistics disabled by default so its telemetry watchdog stays off unless an operator explicitly opts in. (#85886) Thanks @rohitjavvadi.
7677
- Telegram: normalize legacy durable group retry targets before retry sends, polls, and pins so group retries keep using the real chat id. (#85656) Thanks @luoyanglang.

‎extensions/codex/src/app-server/event-projector.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ export class CodexAppServerEventProjector {
176176
private readonly options: CodexAppServerEventProjectorOptions = {},
177177
) {}
178178

179+
getCompletedTurnStatus(): CodexTurn["status"] | undefined {
180+
return this.completedTurn?.status;
181+
}
182+
179183
async handleNotification(notification: CodexServerNotification): Promise<void> {
180184
const params = isJsonObject(notification.params) ? notification.params : undefined;
181185
if (!params) {

‎extensions/codex/src/app-server/run-attempt.test.ts‎

Lines changed: 117 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SessionManager } from "@earendil-works/pi-coding-agent";
55
import {
66
abortAgentHarnessRun,
77
embeddedAgentLog,
8+
invokeNativeHookRelay,
89
nativeHookRelayTesting,
910
onAgentEvent,
1011
queueAgentHarnessMessage,
@@ -860,6 +861,7 @@ describe("runCodexAppServerAttempt", () => {
860861
await closeCodexSandboxExecServersForTests();
861862
resetCodexAppServerClientFactoryForTest();
862863
testing.resetOpenClawCodingToolsFactoryForTests();
864+
testing.clearPendingCodexNativeHookRelayUnregistersForTests();
863865
resetCodexRateLimitCacheForTests();
864866
nativeHookRelayTesting.clearNativeHookRelaysForTests();
865867
clearPluginCommands();
@@ -6777,6 +6779,8 @@ describe("runCodexAppServerAttempt", () => {
67776779
expect(preToolUseState?.enabled).toBe(true);
67786780
expect(preToolUseState?.trusted_hash).toMatch(/^sha256:[a-f0-9]{64}$/);
67796781
const relayId = extractRelayIdFromThreadRequest(startRequest?.params);
6782+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeDefined();
6783+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
67806784
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
67816785
});
67826786

@@ -6839,6 +6843,7 @@ describe("runCodexAppServerAttempt", () => {
68396843

68406844
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
68416845
await run;
6846+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
68426847
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
68436848
});
68446849

@@ -6942,6 +6947,7 @@ describe("runCodexAppServerAttempt", () => {
69426947

69436948
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
69446949
await run;
6950+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
69456951
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
69466952
});
69476953

@@ -7004,6 +7010,7 @@ describe("runCodexAppServerAttempt", () => {
70047010

70057011
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
70067012
await run;
7013+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
70077014
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
70087015
});
70097016

@@ -7034,6 +7041,7 @@ describe("runCodexAppServerAttempt", () => {
70347041

70357042
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
70367043
await run;
7044+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
70377045
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
70387046
});
70397047

@@ -7066,6 +7074,7 @@ describe("runCodexAppServerAttempt", () => {
70667074

70677075
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
70687076
await run;
7077+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
70697078
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
70707079
});
70717080

@@ -7099,6 +7108,7 @@ describe("runCodexAppServerAttempt", () => {
70997108

71007109
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
71017110
await run;
7111+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
71027112
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
71037113
});
71047114

@@ -7140,6 +7150,7 @@ describe("runCodexAppServerAttempt", () => {
71407150
await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" });
71417151
completed = true;
71427152
await run;
7153+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
71437154
expect(
71447155
nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId),
71457156
).toBeUndefined();
@@ -7152,7 +7163,7 @@ describe("runCodexAppServerAttempt", () => {
71527163
}
71537164
});
71547165

7155-
it("reuses the Codex native hook relay id across runs for the same session", async () => {
7166+
it("keeps a replacement Codex native hook relay registered when prior cleanup is pending", async () => {
71567167
const sessionFile = path.join(tempDir, "session.jsonl");
71577168
const workspaceDir = path.join(tempDir, "workspace");
71587169
const firstHarness = createStartedThreadHarness();
@@ -7171,9 +7182,22 @@ describe("runCodexAppServerAttempt", () => {
71717182
(request) => request.method === "thread/start",
71727183
);
71737184
const firstRelayId = extractRelayIdFromThreadRequest(firstStartRequest?.params);
7174-
expect(
7175-
nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(firstRelayId),
7176-
).toBeUndefined();
7185+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(firstRelayId)?.runId).toBe(
7186+
"run-1",
7187+
);
7188+
await expect(
7189+
invokeNativeHookRelay({
7190+
provider: "codex",
7191+
relayId: firstRelayId,
7192+
event: "pre_tool_use",
7193+
rawPayload: {
7194+
hook_event_name: "PreToolUse",
7195+
tool_name: "Bash",
7196+
tool_use_id: "late-call-1",
7197+
tool_input: { command: "python3 -c 'print(\"x\")'" },
7198+
},
7199+
}),
7200+
).resolves.toMatchObject({ exitCode: 0 });
71777201

71787202
const secondHarness = createResumeHarness();
71797203
const secondParams = createParams(sessionFile, workspaceDir);
@@ -7196,8 +7220,17 @@ describe("runCodexAppServerAttempt", () => {
71967220
expect(resumedRegistration?.runId).toBe("run-2");
71977221
expect(resumedRegistration?.allowedEvents).toEqual(["pre_tool_use"]);
71987222

7223+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
7224+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(firstRelayId)?.runId).toBe(
7225+
"run-2",
7226+
);
7227+
71997228
await secondHarness.completeTurn({ threadId: "thread-existing", turnId: "turn-1" });
72007229
await secondRun;
7230+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(firstRelayId)?.runId).toBe(
7231+
"run-2",
7232+
);
7233+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
72017234
expect(
72027235
nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(firstRelayId),
72037236
).toBeUndefined();
@@ -7215,6 +7248,13 @@ describe("runCodexAppServerAttempt", () => {
72157248
expect(relayId).not.toContain("cu-pr-relay-smoke");
72167249
});
72177250

7251+
it("extends native hook relay cleanup grace for configured hook timeouts", () => {
7252+
expect(testing.resolveCodexNativeHookRelayUnregisterGraceMs(undefined)).toBe(10_000);
7253+
expect(testing.resolveCodexNativeHookRelayUnregisterGraceMs(5)).toBe(10_000);
7254+
expect(testing.resolveCodexNativeHookRelayUnregisterGraceMs(9)).toBe(14_000);
7255+
expect(testing.resolveCodexNativeHookRelayUnregisterGraceMs(60)).toBe(65_000);
7256+
});
7257+
72187258
it("sends clearing Codex native hook config when the relay is disabled", async () => {
72197259
const sessionFile = path.join(tempDir, "session.jsonl");
72207260
const workspaceDir = path.join(tempDir, "workspace");
@@ -7455,6 +7495,20 @@ describe("runCodexAppServerAttempt", () => {
74557495

74567496
expect(result.aborted).toBe(true);
74577497
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
7498+
await expect(
7499+
invokeNativeHookRelay({
7500+
provider: "codex",
7501+
relayId,
7502+
event: "pre_tool_use",
7503+
rawPayload: {
7504+
hook_event_name: "PreToolUse",
7505+
tool_name: "Bash",
7506+
tool_input: { command: "pnpm test" },
7507+
},
7508+
}),
7509+
).rejects.toThrow("native hook relay not found");
7510+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
7511+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
74587512
});
74597513

74607514
it("refreshes Codex account rate limits when a failed turn omits reset details", async () => {
@@ -8229,18 +8283,20 @@ describe("runCodexAppServerAttempt", () => {
82298283
expect(result.timedOut).toBe(false);
82308284
});
82318285

8232-
it("releases completion when Codex raw-events an interrupted turn marker", async () => {
8286+
it("releases completion and native hook relay state when Codex raw-events an interrupted turn marker", async () => {
82338287
const harness = createStartedThreadHarness();
82348288
const run = runCodexAppServerAttempt(
82358289
createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")),
8236-
{ turnTerminalIdleTimeoutMs: 60_000 },
8290+
{ nativeHookRelay: { enabled: true }, turnTerminalIdleTimeoutMs: 60_000 },
82378291
);
82388292
let resolved = false;
82398293
void run.then(() => {
82408294
resolved = true;
82418295
});
82428296

82438297
await harness.waitForMethod("turn/start");
8298+
const startRequest = harness.requests.find((request) => request.method === "thread/start");
8299+
const relayId = extractRelayIdFromThreadRequest(startRequest?.params);
82448300
await harness.notify({
82458301
method: "rawResponseItem/completed",
82468302
params: {
@@ -8266,6 +8322,61 @@ describe("runCodexAppServerAttempt", () => {
82668322
expect(result.timedOut).toBe(false);
82678323
expect(result.promptError).toBeNull();
82688324
expect(harness.request.mock.calls.some(([method]) => method === "turn/interrupt")).toBe(false);
8325+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
8326+
await expect(
8327+
invokeNativeHookRelay({
8328+
provider: "codex",
8329+
relayId,
8330+
event: "pre_tool_use",
8331+
rawPayload: {
8332+
hook_event_name: "PreToolUse",
8333+
tool_name: "Bash",
8334+
tool_input: { command: "pnpm test" },
8335+
},
8336+
}),
8337+
).rejects.toThrow("native hook relay not found");
8338+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
8339+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
8340+
});
8341+
8342+
it("cleans up native hook relay state when Codex completes the turn as interrupted", async () => {
8343+
const harness = createStartedThreadHarness();
8344+
const run = runCodexAppServerAttempt(
8345+
createParams(path.join(tempDir, "session.jsonl"), path.join(tempDir, "workspace")),
8346+
{ nativeHookRelay: { enabled: true }, turnTerminalIdleTimeoutMs: 60_000 },
8347+
);
8348+
8349+
await harness.waitForMethod("turn/start");
8350+
const startRequest = harness.requests.find((request) => request.method === "thread/start");
8351+
const relayId = extractRelayIdFromThreadRequest(startRequest?.params);
8352+
await harness.notify({
8353+
method: "turn/completed",
8354+
params: {
8355+
threadId: "thread-1",
8356+
turnId: "turn-1",
8357+
turn: { id: "turn-1", status: "interrupted", items: [] },
8358+
},
8359+
});
8360+
8361+
const result = await run;
8362+
expect(result.aborted).toBe(false);
8363+
expect(result.timedOut).toBe(false);
8364+
expect(result.promptError).toBeNull();
8365+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
8366+
await expect(
8367+
invokeNativeHookRelay({
8368+
provider: "codex",
8369+
relayId,
8370+
event: "pre_tool_use",
8371+
rawPayload: {
8372+
hook_event_name: "PreToolUse",
8373+
tool_name: "Bash",
8374+
tool_input: { command: "pnpm test" },
8375+
},
8376+
}),
8377+
).rejects.toThrow("native hook relay not found");
8378+
testing.flushPendingCodexNativeHookRelayUnregistersForTests();
8379+
expect(nativeHookRelayTesting.getNativeHookRelayRegistrationForTests(relayId)).toBeUndefined();
82698380
});
82708381

82718382
it("keeps upstream cancellation aborted when Codex completes the turn as interrupted", async () => {

0 commit comments

Comments
 (0)