fix: allow missing native hook relay without policy by woodym-dotcom · Pull Request #88620 · openclaw/openclaw

woodym-dotcom · 2026-05-31T12:55:46Z

Summary

Preserve fail-closed PreToolUse behavior whenever unavailable fallback policy state is unknown.
Add an explicit generated-command marker, --pre-tool-use-unavailable noop, for the no-policy PreToolUse case.
Keep PermissionRequest fail-closed and keep PostToolUse / before_agent_finalize observational.
Add regression coverage for stale/missing PreToolUse default-deny, explicit no-policy no-op fallback, and generated command marker behavior.

Review feedback addressed

ClawSweeper concern: fallback used process-local hasBeforeToolCallPolicy() after relay/gateway failure.
Fix: the standalone CLI no longer infers policy state. It denies unavailable PreToolUse unless the original generated hook command explicitly carried the no-policy marker.
The marker is only emitted when the originating registration has no local PreToolUse work, including loop detection.

Real behavior proof

Behavior or issue addressed: Native hook relay unavailable fallback for Codex hooks after the fix. Explicit no-policy PreToolUse marker returns no-op; missing/unknown PreToolUse state denies; PermissionRequest denies.

Real environment tested: Raspberry Pi local checkout of this PR head 26d360b, Node v22.22.2, source CLI run with node --import tsx src/entry.ts, isolated temporary HOME and OPENCLAW_HOME so no user gateway credentials or local OpenClaw config were reused.

Exact steps or command run after this patch:

Verified the source CLI reports OpenClaw 2026.5.30 (26d360b).
Ran hooks relay for missing pre_tool_use with --pre-tool-use-unavailable noop.
Ran hooks relay for missing pre_tool_use without the marker.
Ran hooks relay for missing permission_request.

Evidence after fix:

$ HOME="$proof_home" OPENCLAW_HOME="$proof_home/.openclaw" \
  node --import tsx src/entry.ts --version
OpenClaw 2026.5.30 (26d360b)

$ printf '{}' | HOME="$proof_home" OPENCLAW_HOME="$proof_home/.openclaw" \
  node --import tsx src/entry.ts hooks relay \
  --provider codex \
  --relay-id missing \
  --generation generation-1 \
  --event pre_tool_use \
  --pre-tool-use-unavailable noop \
  --timeout 1000

stdout: <empty>
stderr:
native hook relay unavailable: gateway nativeHook.invoke requires credentials before opening a websocket
Fix: configure gateway.auth token/password, pair this device, or pass --token/--password.

$ printf '{}' | HOME="$proof_home" OPENCLAW_HOME="$proof_home/.openclaw" \
  node --import tsx src/entry.ts hooks relay \
  --provider codex \
  --relay-id missing \
  --generation generation-1 \
  --event pre_tool_use \
  --timeout 1000

stdout:
{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"Native hook relay unavailable"}}

stderr:
native hook relay unavailable: gateway nativeHook.invoke requires credentials before opening a websocket
Fix: configure gateway.auth token/password, pair this device, or pass --token/--password.

$ printf '{}' | HOME="$proof_home" OPENCLAW_HOME="$proof_home/.openclaw" \
  node --import tsx src/entry.ts hooks relay \
  --provider codex \
  --relay-id missing \
  --generation generation-1 \
  --event permission_request \
  --timeout 1000

stdout:
{"hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"deny","message":"Native hook relay unavailable"}}}

stderr:
native hook relay unavailable: gateway nativeHook.invoke requires credentials before opening a websocket
Fix: configure gateway.auth token/password, pair this device, or pass --token/--password.

Observed result after fix: The amended branch fails closed when PreToolUse policy state is unknown, no-ops only when the generated command carries the explicit no-policy marker, and keeps PermissionRequest denied.

What was not tested: Full local Vitest did not complete in the sparse/partial checkout; GitHub CI is the full test gate. The proof does not exercise a live registered relay because this change is specifically the unavailable-relay fallback path.

Additional validation

$ node --experimental-strip-types --check src/cli/native-hook-relay-cli.ts
cli_rc:0
$ node --experimental-strip-types --check src/agents/harness/native-hook-relay.ts
harness_rc:0
$ node --experimental-strip-types --check src/cli/hooks-cli.ts
hooks_rc:0
$ git diff --check
# no output

clawsweeper · 2026-05-31T12:58:09Z

Codex review: found issues before merge. Reviewed May 31, 2026, 10:53 AM ET / 14:53 UTC.

Summary
The branch adds a hidden hooks relay --pre-tool-use-unavailable noop mode, threads it into native hook relay unavailable fallback handling, and adds regression coverage for missing PreToolUse and PermissionRequest relay behavior.

PR surface: Source +22, Tests +47. Total +69 across 5 files.

Reproducibility: yes. from source inspection: current main denies any unavailable PreToolUse relay, and the PR head still does not emit the new marker through the production Codex config path that skips inactive events.

Review metrics: 1 noteworthy metric.

Hidden fallback CLI option: 1 added. The new internal relay mode changes a permission-fallback surface, so maintainers should verify compatibility and policy semantics before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🐚 platinum hermit
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Fix or remove the no-policy marker path, then add a Codex config-generation regression test showing the installed command behavior for no-policy and policy-active PreToolUse cases.

Risk before merge

[P2] The no-op marker is added to commandForEvent, but production Codex config skips commandForEvent when no PreToolUse work exists, so the intended generated-command fallback is currently unreachable outside direct/manual CLI calls.
[P2] This changes a PreToolUse permission fallback surface; policy-active, stale, and unknown relay states must remain deny-by-default with tests proving the generated config path, not only the standalone CLI parser.

Maintainer options:

Make the marker production-reachable (recommended)
If maintainers want the no-policy no-op fallback, update the Codex config generation path and tests so the installed PreToolUse command actually carries the marker while policy-active registrations still fail closed.
Keep fail-closed and drop the marker
If the extra fallback is not worth the hook/config complexity, remove the hidden no-op mode and keep unavailable PreToolUse uniformly deny-by-default.
Accept CLI-only fallback behavior
Maintainers could intentionally accept that only manually invoked flagged relay commands no-op, but the PR should then stop claiming generated-command production coverage.

Next step before merge

[P2] Human review should choose the fallback policy direction before repair, because making the marker production-reachable may intentionally change hook installation and permission-boundary behavior.

Security
Cleared: No supply-chain or credential-handling regression was found, but the permission-boundary fallback semantics remain a merge-risk until the config-path finding is fixed.

Review findings

[P2] Wire the no-policy marker through the Codex config path — src/agents/harness/native-hook-relay.ts:442-445

Review details

Best possible solution:

Do not merge until the no-policy fallback is either wired through the actual Codex hook config path with tests preserving policy-active fail-closed behavior, or removed in favor of the existing fail-closed semantics.

Do we have a high-confidence way to reproduce the issue?

Yes, from source inspection: current main denies any unavailable PreToolUse relay, and the PR head still does not emit the new marker through the production Codex config path that skips inactive events.

Is this the best way to solve the issue?

No. The amended fail-closed default is safer than the first revision, but the marker is attached below a path that production does not call for no-policy PreToolUse, so the proposed implementation is not the narrow complete fix.

Full review comments:

[P2] Wire the no-policy marker through the Codex config path — src/agents/harness/native-hook-relay.ts:442-445
The marker is added in commandForEvent, but production only calls commandForEvent after buildCodexNativeHookRelayConfig accepts relay.shouldRelayEvent(event). For the no-policy PreToolUse case, that predicate is false, so the generated Codex hook command never receives --pre-tool-use-unavailable noop; the new no-op fallback is only reachable through direct/manual CLI calls and tests.
Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against b334e7ef29ba.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes exact after-fix terminal output from a real source CLI run for the unavailable fallback cases; it does not remove the separate production config-path finding.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes exact after-fix terminal output from a real source CLI run for the unavailable fallback cases; it does not remove the separate production config-path finding.
remove status: 📣 needs proof: Current PR status label is status: ⏳ waiting on author.

Label justifications:

P2: This is a normal-priority agent/CLI fallback fix with limited blast radius, but it affects a permission-sensitive runtime path.
merge-risk: 🚨 compatibility: Changing unavailable PreToolUse behavior can alter existing Codex hook sessions and stale generated-command upgrade behavior.
merge-risk: 🚨 security-boundary: The PR changes when a tool-policy hook failure returns no-op versus deny, which is part of the tool-execution safety boundary.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🐚 platinum hermit and patch quality is 🧂 unranked krab.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body includes exact after-fix terminal output from a real source CLI run for the unavailable fallback cases; it does not remove the separate production config-path finding.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes exact after-fix terminal output from a real source CLI run for the unavailable fallback cases; it does not remove the separate production config-path finding.

Evidence reviewed

PR surface:

Source +22, Tests +47. Total +69 across 5 files.

View PR surface stats

Area	Files	Added	Net
Source	3	22	+22
Tests	2	47	+47
Docs	0	0	0
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	5	69	+69

What I checked:

Current main fail-closed fallback: On current main, unavailable pre_tool_use renders a Codex permissionDecision: deny, while permission_request also denies and observational events no-op. (src/agents/harness/native-hook-relay.ts:752, b334e7ef29ba)
PR marker only attaches to commandForEvent: The PR adds the no-op marker only when commandForEvent("pre_tool_use") is called for a registration with no local PreToolUse work. (src/agents/harness/native-hook-relay.ts:442, 26d360bcef43)
Production config skips inactive event commands: The Codex config builder continues before calling commandForEvent whenever relay.shouldRelayEvent(event) is false, which is exactly the no-policy PreToolUse state that would add the marker. (extensions/codex/src/app-server/native-hook-relay.ts:234, 26d360bcef43)
Policy-state predicate: hasBeforeToolCallPolicy() treats either a before-tool hook or trusted tool policy as local PreToolUse work, so policy-active registrations should not receive the no-op marker. (src/agents/agent-tools.before-tool-call.ts:181, b334e7ef29ba)
Codex hook contract: Codex parses PreToolUse permissionDecision: deny as a blocking result and treats empty successful output as no blocking decision, matching the fallback shape under review. (../codex/codex-rs/hooks/src/engine/output_parser.rs:121, cdde711fac00)
Contributor proof and checks: The PR body includes terminal output for source CLI fallback cases, and public check-run data shows the latest Real behavior proof check succeeded for 26d360b. (26d360bcef43)

Likely related people:

Sebastien Tardif: git blame and git log -S on the native-hook relay and CLI fallback files point to commit 81b9da0 in this shallow checkout; earlier feature history is unavailable here, so routing confidence is limited. (role: current source provenance in shallow checkout; confidence: low; commits: 81b9da0bb0ff; files: src/agents/harness/native-hook-relay.ts, src/cli/native-hook-relay-cli.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

woodym-dotcom · 2026-05-31T14:33:36Z

Addressed the review feedback in 26d360b: unavailable PreToolUse now fails closed by default, and the no-policy no-op path only applies when the generated hook command carries an explicit marker from the originating registration. @clawsweeper re-review

clawsweeper · 2026-05-31T14:33:39Z

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

woodym-dotcom · 2026-05-31T14:38:25Z

Added a Real behavior proof section with exact after-fix source CLI output from branch 26d360b. @clawsweeper re-review

clawsweeper · 2026-05-31T14:38:28Z

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

woodym-dotcom · 2026-05-31T14:39:59Z

Updated Real behavior proof with required fields: behavior, environment, steps, evidence, observedResult, notTested. @clawsweeper re-review

clawsweeper · 2026-05-31T14:40:02Z

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

woodym-dotcom · 2026-05-31T14:42:24Z

Updated Real behavior proof to use the exact required field names. @clawsweeper re-review

clawsweeper · 2026-05-31T14:42:26Z

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

woodym-dotcom · 2026-05-31T14:46:33Z

Real behavior proof gate is now passing for head 26d360bcef43f7061482f209c38f5ab3eee9e47a after the PR body field-name update: https://github.com/openclaw/openclaw/actions/runs/26715621525/job/78733585205

@clawsweeper re-review

clawsweeper · 2026-05-31T14:46:36Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26715723903
Updated: 2026-05-31T14:54:23.467Z

steipete · 2026-06-01T00:33:35Z

Thanks for the iteration here. Closing this as not planned.

The latest head does wire the no-policy marker through the Codex config path, but after maintainer review we do not want to add a new PreToolUse unavailable fallback for the no-policy case without an observed stale generated-command user path. Current behavior already clears PreToolUse from generated Codex config when OpenClaw has no before-tool policy or loop-detection work, and unavailable or unknown PreToolUse relay state should stay fail-closed.

Supported path: keep the existing fail-closed fallback, and if we find a real stale-config upgrade/runtime state that blocks basic tool use despite no policy being active, handle that as a targeted migration/doctor/config-clearing fix with a concrete repro.

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. Verification: - pnpm test extensions/codex/src/app-server/native-hook-relay.test.ts src/agents/harness/native-hook-relay.test.ts src/cli/native-hook-relay-cli.test.ts - pnpm lint --threads=8 - autoreview --mode branch --base origin/main - GitHub CI run 26729700996, Real behavior proof 26729874455, OpenGrep 26729701010, CodeQL high 26729701003 Co-authored-by: woodym-dotcom <[email protected]>

steipete · 2026-06-01T01:25:31Z

Thanks @woodym-dotcom. I landed the final version via #88846 because this fork branch stopped accepting maintainer pushes with permission denied after the maintainer fixup.

Landed commit: fb64546
Replacement PR: #88846

The landed commit preserves your contributor credit with:

Co-authored-by: woodym-dotcom [email protected]

For future PRs, please keep "Allow edits by maintainers" enabled; GitHub still reported this PR as maintainer-editable, but the fork rejected both force-with-lease and normal pushes.

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 openclaw#87543. Replaces openclaw#88620. Verification: - pnpm test extensions/codex/src/app-server/native-hook-relay.test.ts src/agents/harness/native-hook-relay.test.ts src/cli/native-hook-relay-cli.test.ts - pnpm lint --threads=8 - autoreview --mode branch --base origin/main - GitHub CI run 26729700996, Real behavior proof 26729874455, OpenGrep 26729701010, CodeQL high 26729701003 Co-authored-by: woodym-dotcom <[email protected]>

openclaw-barnacle Bot added cli CLI command changes agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 31, 2026

openclaw-barnacle Bot added size: S and removed size: XS labels May 31, 2026

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 31, 2026

clawsweeper Bot mentioned this pull request May 31, 2026

fix(cli): make native hook relay resilient to stale bridges #88718

Closed

steipete self-assigned this May 31, 2026

openclaw-barnacle Bot added the extensions: codex label Jun 1, 2026

woodym-dotcom and others added 3 commits June 1, 2026 01:30

fix: allow missing native hook relay without policy

b2045b8

fix: preserve native hook relay policy fallback

055fcb7

fix: wire no-policy native hook fallback

a70f362

steipete force-pushed the fix/native-hook-relay-missing-pretooluse branch from 3b74cd0 to a70f362 Compare June 1, 2026 00:32

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 1, 2026

steipete closed this Jun 1, 2026

steipete mentioned this pull request Jun 1, 2026

fix: preserve no-policy native hook fallback #88846

Merged

Uh oh!

Uh oh!

Conversation

woodym-dotcom commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review feedback addressed

Real behavior proof

Additional validation

Uh oh!

clawsweeper Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

woodym-dotcom commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026

Uh oh!

woodym-dotcom commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026

Uh oh!

woodym-dotcom commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026

Uh oh!

woodym-dotcom commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026

Uh oh!

woodym-dotcom commented May 31, 2026

Uh oh!

clawsweeper Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jun 1, 2026

Uh oh!

steipete commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

woodym-dotcom commented May 31, 2026 •

edited

Loading

clawsweeper Bot commented May 31, 2026 •

edited

Loading

clawsweeper Bot commented May 31, 2026 •

edited

Loading