fix: preserve no-policy native hook fallback by steipete · Pull Request #88846 · openclaw/openclaw

steipete · 2026-06-01T00:59:05Z

Summary

Recreates fix: allow missing native hook relay without policy #88620 from the contributor branch because the fork stopped accepting maintainer pushes.
Keeps selected no-policy Codex PreToolUse relay hooks installed with an explicit unavailable no-op marker.
Preserves fail-closed behavior for unknown unavailable PreToolUse state and PermissionRequest.
Adds regression coverage for the CLI fallback, harness command marker, and Codex app-server config path.

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 — passed, 3 Vitest shards / 98 tests.
pnpm lint --threads=8 — passed.
/Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/main — clean, no accepted/actionable findings.
Checked Codex hook behavior in ../codex/codex-rs/hooks/src/events/pre_tool_use.rs, ../codex/codex-rs/hooks/src/events/permission_request.rs, and ../codex/codex-rs/hooks/src/engine/{dispatcher.rs,discovery.rs}.

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 <[email protected]>

clawsweeper · 2026-06-01T01:01:16Z

Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 9:05 PM ET / 01:05 UTC.

Summary
The PR adds an internal no-policy --pre-tool-use-unavailable noop marker to generated Codex PreToolUse relay commands, preserves fail-closed unavailable handling otherwise, and adds regression coverage for the app-server config, harness command builder, and CLI fallback.

PR surface: Source +26, Tests +86. Total +112 across 7 files.

Reproducibility: yes. source-reproducible: current main always denies unavailable PreToolUse, and the replaced proofed branch exercised the missing-relay CLI no-op marker, missing-marker deny, and PermissionRequest deny paths. I did not run tests locally because this review is read-only.

Review metrics: 1 noteworthy metric.

Fallback Surface: 1 internal CLI marker added; 1 PreToolUse unavailable path changed. The marker controls whether an unavailable Codex PreToolUse relay becomes a no-op or a denial, so it is the main security/compatibility decision before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

none.

Risk before merge

[P1] Merging changes security-sensitive Codex PreToolUse unavailable behavior; the no-op marker must remain limited to registrations with no before-tool policy or loop-detection work.
[P1] The PR carries the protected maintainer label and replaces another closed branch, so final security/CI signoff should stay human-owned even though the patch review found no blocking defect.

Maintainer options:

Accept The Guarded Fallback (recommended)
Merge after maintainer/security signoff confirms the marker is emitted only for no-policy PreToolUse and required CI remains green.
Ask For A Live Registered-Relay Proof
Require a live Codex app-server run that installs the generated hook and observes unavailable-marker behavior before merging.
Pause If All Outages Must Fail Closed
Pause or close if maintainership decides that even no-policy PreToolUse relay outages must continue to deny tool execution.

Next step before merge

[P2] Protected maintainer label plus security-sensitive fallback semantics make this a human signoff item, not an automated repair job.

Security
Cleared: No concrete security regression was found; the patch preserves fail-closed behavior unless the generated command explicitly records a no-policy PreToolUse fallback.

Review details

Best possible solution:

Land the guarded marker-based fallback after maintainer/security signoff and required CI, while keeping unknown PreToolUse state and PermissionRequest unavailable state fail-closed.

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

Yes, source-reproducible: current main always denies unavailable PreToolUse, and the replaced proofed branch exercised the missing-relay CLI no-op marker, missing-marker deny, and PermissionRequest deny paths. I did not run tests locally because this review is read-only.

Is this the best way to solve the issue?

Yes, this appears to be the best fix shape: carrying an explicit generated-command marker is safer than inferring policy state after relay lookup fails, and the unknown/default paths remain fail-closed.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P1: The PR targets a linked P1 regression where Codex PreToolUse relay outages can block valid agent callback workflows before ingress handling.
add merge-risk: 🚨 compatibility: The diff changes an existing unavailable-relay fallback path for upgraded Codex sessions by preserving selected no-policy hooks instead of clearing them.
add merge-risk: 🚨 security-boundary: The diff changes command-execution hook fallback behavior, so maintainers must confirm it cannot become a general PreToolUse bypass.
add proof: sufficient: Contributor real behavior proof is sufficient. The current PR points to the replaced branch, whose body includes terminal proof for the same touched files, and a direct diff check showed no touched-file difference between that proofed head and this PR head.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The current PR points to the replaced branch, whose body includes terminal proof for the same touched files, and a direct diff check showed no touched-file difference between that proofed head and this PR head.

Label justifications:

P1: The PR targets a linked P1 regression where Codex PreToolUse relay outages can block valid agent callback workflows before ingress handling.
merge-risk: 🚨 security-boundary: The diff changes command-execution hook fallback behavior, so maintainers must confirm it cannot become a general PreToolUse bypass.
merge-risk: 🚨 compatibility: The diff changes an existing unavailable-relay fallback path for upgraded Codex sessions by preserving selected no-policy hooks instead of clearing them.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The current PR points to the replaced branch, whose body includes terminal proof for the same touched files, and a direct diff check showed no touched-file difference between that proofed head and this PR head.
proof: sufficient: Contributor real behavior proof is sufficient. The current PR points to the replaced branch, whose body includes terminal proof for the same touched files, and a direct diff check showed no touched-file difference between that proofed head and this PR head.

Evidence reviewed

PR surface:

Source +26, Tests +86. Total +112 across 7 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	4	27	1	+26
Tests	3	87	1	+86
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	7	114	2	+112

What I checked:

Repository policy applied: Root AGENTS.md was read fully; its compatibility, fallback, PR review, and direct Codex-source requirements apply to this Codex hook relay change. (AGENTS.md:1, 732748c8c5cb)
Current-main fallback behavior: Current main renders every unavailable pre_tool_use relay as a PreToolUse denial, while PermissionRequest also fails closed and observational events no-op. (src/agents/harness/native-hook-relay.ts:752, 732748c8c5cb)
Generated marker is scoped to no local PreToolUse work: The PR adds preToolUseUnavailable: "noop" only for pre_tool_use when nativeHookRelayEventHasLocalWork is false, so registrations with before-tool policy or loop detection do not get the no-op marker. (src/agents/harness/native-hook-relay.ts:442, 76eb478ac9fe)
Codex app-server config keeps selected no-policy PreToolUse installed: The PR keeps selected no-policy PreToolUse in the Codex hook config and records the trusted hash for the generated command instead of clearing the selected event. (extensions/codex/src/app-server/native-hook-relay.ts:234, 76eb478ac9fe)
Unavailable fallback remains fail-closed by default: The PR makes unavailable PreToolUse no-op only for the explicit noop marker; missing or unknown PreToolUse state still denies, and PermissionRequest still denies. (src/agents/harness/native-hook-relay.ts:760, 76eb478ac9fe)
Regression tests cover marker generation and fallback behavior: Added tests cover no-policy command marker emission, explicit PreToolUse unavailable no-op handling, and continued default PreToolUse denial without the marker. (src/cli/native-hook-relay-cli.test.ts:280, 76eb478ac9fe)

Likely related people:

steipete: Recent main history for src/agents/harness/native-hook-relay.ts includes multiple native hook relay fixes and lint/refactor touches by this login, including bounded expiry and replacement-race work. (role: recent area contributor; confidence: high; commits: 61cf22f14711, 450642a89780, c4e1bb30da11; files: src/agents/harness/native-hook-relay.ts, extensions/codex/src/app-server/native-hook-relay.ts, src/cli/native-hook-relay-cli.ts)
pashpashpash: The native Codex hook relay surface appears to originate in the commit that bridged Codex-native tool events into OpenClaw plugin hooks. (role: introduced behavior; confidence: medium; commits: 7a958d920c88; files: src/agents/harness/native-hook-relay.ts, extensions/codex/src/app-server/native-hook-relay.ts)
amknight: Recent Codex app-server native relay generation and restart behavior was changed in the same app-server relay area. (role: recent adjacent contributor; confidence: medium; commits: 42e9504114f3; files: extensions/codex/src/app-server/native-hook-relay.ts)
Kaspre: Recent side-question native hook work touched the Codex native hook relay composition and approval fallback paths adjacent to this change. (role: recent adjacent contributor; confidence: medium; commits: 69a0c925b890; files: extensions/codex/src/app-server/side-question.ts, extensions/codex/src/app-server/native-hook-relay.ts, src/agents/harness/native-hook-relay.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.

steipete · 2026-06-01T01:23:59Z

Land-ready maintainer replacement for #88620.

Work done:

Recreated the contributor PR on a writable openclaw/openclaw branch because the fork branch rejected maintainer pushes with permission denied.
Squashed the final native-hook relay diff into one maintainer commit with Co-authored-by: woodym-dotcom <[email protected]>.
Preserved the Codex PreToolUse unavailable behavior: unknown policy state still denies, explicit generated no-policy marker no-ops.

Local 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 — passed, 3 Vitest shards / 98 tests.
pnpm lint --threads=8 — passed.
/Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/main — clean, no accepted/actionable findings.

Dependency/Codex contract checked:

../codex/codex-rs/hooks/src/events/pre_tool_use.rs
../codex/codex-rs/hooks/src/events/permission_request.rs
../codex/codex-rs/hooks/src/engine/dispatcher.rs
../codex/codex-rs/hooks/src/engine/discovery.rs

GitHub verification:

CI run 26729700996 — passed.
Real behavior proof run 26729874455 — passed.
OpenGrep PR Diff run 26729701010 — passed.
CodeQL high checks run 26729701003 — passed.
CodeQL Critical Quality selector run 26729701038 — passed/skipped non-selected shards.

Known gaps:

No additional live registered-relay E2E was run locally; this patch targets the unavailable-relay fallback path, and CI plus the existing real behavior proof cover the pushed branch.

openclaw-barnacle Bot added cli CLI command changes agents Agent runtime and tooling extensions: codex size: S maintainer Maintainer-authored PR labels Jun 1, 2026

steipete merged commit fb64546 into main Jun 1, 2026
217 of 223 checks passed

steipete deleted the maint/pr-88620-native-hook-relay branch June 1, 2026 01:24

This was referenced Jun 1, 2026

fix: allow missing native hook relay without policy #88620

Closed

[Bug]: PreToolUse hook relay outage blocks trusted callback commands before ingress execution #87543

Closed

clawsweeper Bot mentioned this pull request Jun 27, 2026

fix(harness): add Codex pre-tool loop relay opt-out #97224

Merged

nakamasato mentioned this pull request Jul 17, 2026

[Feature]: Scope Codex native hook relay spawns with per-tool hook matchers #109522

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: preserve no-policy native hook fallback#88846

fix: preserve no-policy native hook fallback#88846
steipete merged 1 commit into
mainfrom
maint/pr-88620-native-hook-relay

steipete commented Jun 1, 2026

Uh oh!

clawsweeper Bot commented Jun 1, 2026 •

edited

Loading

Uh oh!

steipete commented Jun 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jun 1, 2026

Summary

Verification

Uh oh!

clawsweeper Bot commented Jun 1, 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jun 1, 2026 •

edited

Loading