fix: preserve no-policy native hook fallback

steipete · woodym-dotcom · steipete · commit 76eb478ac9fe · 2026-06-01T01:55:07.000+01:00
Keep selected no-policy Codex PreToolUse relay hooks installed with an explicit unavailable no-op marker, while unknown unavailable PreToolUse and PermissionRequest still fail closed. Refs #87543. Replaces #88620. Co-authored-by: woodym-dotcom <266261448+woodym-dotcom@users.noreply.github.com>
diff --git a/extensions/codex/src/app-server/native-hook-relay.test.ts b/extensions/codex/src/app-server/native-hook-relay.test.ts
@@ -185,6 +185,41 @@ describe("Codex native hook relay config", () => {
     });
   });
 
+  it("keeps selected no-policy PreToolUse installed with an unavailable no-op marker", () => {
+    expect(
+      buildCodexNativeHookRelayConfig({
+        relay: createRelay({ inactiveEvents: ["pre_tool_use"] }),
+        events: ["pre_tool_use"],
+      }),
+    ).toEqual({
+      "features.hooks": true,
+      "hooks.PreToolUse": [
+        {
+          hooks: [
+            {
+              type: "command",
+              command:
+                "openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --pre-tool-use-unavailable noop",
+              timeout: 5,
+              async: false,
+              statusMessage: "OpenClaw native hook relay",
+            },
+          ],
+        },
+      ],
+      "hooks.state": {
+        "/<session-flags>/config.toml:pre_tool_use:0:0": {
+          enabled: true,
+          trusted_hash: expect.stringMatching(/^sha256:[a-f0-9]{64}$/),
+        },
+        "<session-flags>/config.toml:pre_tool_use:0:0": {
+          enabled: true,
+          trusted_hash: expect.stringMatching(/^sha256:[a-f0-9]{64}$/),
+        },
+      },
+    });
+  });
+
   it("clears omitted hook events when requested", () => {
     expect(
       buildCodexNativeHookRelayConfig({
@@ -276,7 +311,11 @@ function createRelay(options?: {
     expiresAtMs: Date.now() + 1000,
     shouldRelayEvent: (event) => !inactiveEvents.has(event),
     commandForEvent: (event) =>
-      `openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event ${event}`,
+      `openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event ${event}${
+        event === "pre_tool_use" && inactiveEvents.has(event)
+          ? " --pre-tool-use-unavailable noop"
+          : ""
+      }`,
     renew: () => undefined,
     unregister: () => undefined,
   };
diff --git a/extensions/codex/src/app-server/native-hook-relay.ts b/extensions/codex/src/app-server/native-hook-relay.ts
@@ -231,7 +231,11 @@ export function buildCodexNativeHookRelayConfig(params: {
   for (const event of CODEX_NATIVE_HOOK_RELAY_EVENTS) {
     const codexEvent = CODEX_HOOK_EVENT_BY_NATIVE_EVENT[event];
     const selected = selectedEvents.has(event);
-    if (!selected || !params.relay.shouldRelayEvent(event)) {
+    const shouldRelay = params.relay.shouldRelayEvent(event);
+    // Keep no-policy PreToolUse commands installed with an explicit no-op marker;
+    // otherwise a stale relay fallback cannot distinguish no policy from unknown policy.
+    const selectedNoopPreToolUse = selected && event === "pre_tool_use" && !shouldRelay;
+    if (!selected || (!shouldRelay && !selectedNoopPreToolUse)) {
       if (selected || params.clearOmittedEvents) {
         config[`hooks.${codexEvent}`] = [] satisfies JsonValue;
       }
diff --git a/src/agents/harness/native-hook-relay.test.ts b/src/agents/harness/native-hook-relay.test.ts
@@ -525,6 +525,10 @@ describe("native hook relay registry", () => {
     expect(relay.shouldRelayEvent("post_tool_use")).toBe(false);
     expect(relay.shouldRelayEvent("before_agent_finalize")).toBe(false);
     expect(relay.shouldRelayEvent("permission_request")).toBe(true);
+    expect(relay.commandForEvent("pre_tool_use")).toBe(
+      "/usr/local/bin/node '/opt/Open Claw/openclaw.mjs' hooks relay --provider codex --relay-id " +
+        `${relay.relayId} --generation ${relay.generation} --event pre_tool_use --pre-tool-use-unavailable noop --timeout 1234`,
+    );
   });
 
   it("builds pre-tool relay commands only when before-tool policy is active", () => {
@@ -3196,4 +3200,19 @@ describe("native hook relay command builder", () => {
       "openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event permission_request --timeout 5000",
     );
   });
+
+  it("includes explicit unavailable noop mode only for PreToolUse", () => {
+    expect(
+      buildNativeHookRelayCommand({
+        provider: "codex",
+        relayId: "relay-1",
+        generation: "generation-1",
+        event: "pre_tool_use",
+        preToolUseUnavailable: "noop",
+        executable: "openclaw",
+      }),
+    ).toBe(
+      "openclaw hooks relay --provider codex --relay-id relay-1 --generation generation-1 --event pre_tool_use --pre-tool-use-unavailable noop --timeout 5000",
+    );
+  });
 });
diff --git a/src/agents/harness/native-hook-relay.ts b/src/agents/harness/native-hook-relay.ts
@@ -439,6 +439,10 @@ export function registerNativeHookRelay(
         relayId,
         generation: registration.generation,
         event,
+        preToolUseUnavailable:
+          event === "pre_tool_use" && !nativeHookRelayEventHasLocalWork(registration, event)
+            ? "noop"
+            : undefined,
         nice: params.command?.nice,
         timeoutMs: params.command?.timeoutMs,
         executable: params.command?.executable,
@@ -517,6 +521,7 @@ export function buildNativeHookRelayCommand(params: {
   relayId: string;
   generation?: string;
   event: NativeHookRelayEvent;
+  preToolUseUnavailable?: "noop";
   timeoutMs?: number;
   executable?: string;
   nice?: number | false;
@@ -541,6 +546,9 @@ export function buildNativeHookRelayCommand(params: {
     ...(params.generation ? ["--generation", params.generation] : []),
     "--event",
     params.event,
+    ...(params.event === "pre_tool_use" && params.preToolUseUnavailable
+      ? ["--pre-tool-use-unavailable", params.preToolUseUnavailable]
+      : []),
     "--timeout",
     String(timeoutMs),
   ]);
@@ -752,13 +760,20 @@ export async function invokeNativeHookRelayBridge(
 export function renderNativeHookRelayUnavailableResponse(params: {
   provider: unknown;
   event: unknown;
+  preToolUseUnavailable?: unknown;
   message?: string;
 }): NativeHookRelayProcessResponse {
   const provider = readNativeHookRelayProvider(params.provider);
   const event = readNativeHookRelayEvent(params.event);
   const adapter = getNativeHookRelayProviderAdapter(provider);
   const message = params.message?.trim() || "Native hook relay unavailable";
   if (event === "pre_tool_use") {
+    // The standalone CLI cannot reconstruct the originating registration after
+    // relay lookup fails, so unavailable PreToolUse must fail closed unless the
+    // generated command explicitly recorded that no before-tool policy existed.
+    if (params.preToolUseUnavailable === "noop") {
+      return adapter.renderNoopResponse(event);
+    }
     return adapter.renderPreToolUseBlockResponse(message);
   }
   if (event === "permission_request") {
diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts
@@ -530,6 +530,10 @@ export function registerHooksCli(program: Command): void {
     .requiredOption("--relay-id <id>", "Native hook relay id")
     .option("--generation <generation>", "Native hook relay registration generation")
     .requiredOption("--event <event>", "Native hook event")
+    .option(
+      "--pre-tool-use-unavailable <mode>",
+      "PreToolUse fallback mode when the originating relay is unavailable",
+    )
     .option("--timeout <ms>", "Gateway timeout in ms", "5000")
     .action(async (opts: NativeHookRelayCliOptions) =>
       runHooksCliAction(async () => {
diff --git a/src/cli/native-hook-relay-cli.test.ts b/src/cli/native-hook-relay-cli.test.ts
@@ -305,6 +305,34 @@ describe("native hook relay CLI", () => {
     expect(stderr.text()).toContain("native hook relay unavailable");
   });
 
+  it("keeps PreToolUse unavailable handling observational only with an explicit no-policy marker", async () => {
+    const callGateway = vi.fn(async () => {
+      throw new Error("gateway closed");
+    });
+    const stdout = createWritableTextBuffer();
+    const stderr = createWritableTextBuffer();
+
+    const exitCode = await runNativeHookRelayCli(
+      {
+        provider: "codex",
+        relayId: "relay-1",
+        generation: "generation-1",
+        event: "pre_tool_use",
+        preToolUseUnavailable: "noop",
+      },
+      {
+        stdin: createReadableTextStream("{}"),
+        stdout,
+        stderr,
+        callGateway: callGateway as never,
+      },
+    );
+
+    expect(exitCode).toBe(0);
+    expect(stdout.text()).toBe("");
+    expect(stderr.text()).toContain("native hook relay unavailable");
+  });
+
   it("fails closed for PermissionRequest when the gateway relay is unavailable", async () => {
     const callGateway = vi.fn(async () => {
       throw new Error("gateway closed");
diff --git a/src/cli/native-hook-relay-cli.ts b/src/cli/native-hook-relay-cli.ts
@@ -16,6 +16,7 @@ export type NativeHookRelayCliOptions = {
   relayId?: string;
   generation?: string;
   event?: string;
+  preToolUseUnavailable?: string;
   timeout?: string;
 };
 
@@ -76,6 +77,7 @@ export async function runNativeHookRelayCli(
       const response = renderNativeHookRelayUnavailableResponse({
         provider,
         event,
+        preToolUseUnavailable: opts.preToolUseUnavailable,
         message: "Native hook relay unavailable",
       });
       writeText(stdout, response.stdout);
@@ -101,6 +103,7 @@ export async function runNativeHookRelayCli(
     const response = renderNativeHookRelayUnavailableResponse({
       provider,
       event,
+      preToolUseUnavailable: opts.preToolUseUnavailable,
       message: "Native hook relay unavailable",
     });
     writeText(stdout, response.stdout);
