fix(codex): cover side-question native hooks by Kaspre · Pull Request #82559 · openclaw/openclaw

Kaspre · 2026-05-16T11:37:10Z

Problem

#82496 restored Codex native hook relay coverage for the normal app-server run path, but the public Codex /btw path forks a child Codex thread through runCodexAppServerSideQuestion. That child thread needs its own hook config and its app-server approval requests need to reach the same native relay policy surface.

There is also a related app-server approval gap: Codex native command approvals arrive as item/commandExecution/requestApproval, while OpenClaw policy is expressed as PreToolUse/before-tool-call policy. Without bridging that approval request through the active native relay, a Codex app-server command approval can bypass the same native PreToolUse enforcement the main hook path relies on.

Change and value

This PR keeps the existing native hook relay model and applies it to the missing surfaces:

Public Codex /btw side-question runs now opt into native hook relay config.
Side-question child threads receive hook config on both thread/fork and turn/start, preserving code-mode config.
Side-question command approval requests forward the active side-thread relay handle into the approval bridge.
Normal Codex app-server command approval requests also pass the active native relay handle into the approval bridge.
The approval bridge invokes nativeHook.invoke for command approvals before prompting. Native deny fails closed to Codex; explicit native allow can accept; native no-decision falls through to the existing plugin approval route without re-running before-tool-call policy.
Codex exec_command plus tool_input.cmd now normalizes to OpenClaw exec plus command before policy evaluation.

Who is affected, and who is not

Affected: Codex app-server users who depend on OpenClaw native hook relay policy for native Codex command execution, including /btw side conversations.

Not affected: non-Codex harnesses, internal direct side-question callers that do not opt into nativeHookRelay, and Codex native no-decision approval behavior. A no-op PreToolUse response is not treated as an approval; it still falls through to the existing app-server approval flow.

Implementation

runCodexAppServerSideQuestion now registers a relay, merges hook config with Codex runtime config, sends it to the side thread, passes the relay to approval handling, and unregisters it in cleanup. The main run path now also passes its active relay into the shared approval bridge.

handleCodexAppServerApprovalRequest now maps app-server command approval requests into a Codex PreToolUse relay payload. It fails closed when the expected relay cannot be invoked, rejects native denials before prompting, accepts only explicit native allow decisions, and preserves the previous plugin approval path for native no-decision.

Real behavior proof

Behavior or issue addressed: Codex /btw side-question child threads and app-server command approval requests now reach OpenClaw native PreToolUse policy instead of relying only on the main-run hook config.

Real environment tested: Local PR worktree /tmp/openclaw-pr-side-question-hooks at head 7f620eef8bb340ebac41dbe3650102478dc49e20, rebased onto the latest completed green full main CI base available at the time, 89532d3a92a8bec0121ce954b682e052c3be2f42 (CI run 25977237869, success). GitHub PR readback after the push showed current base 54c9820ed9c220180b9cc2785b2b9921bc12b46f, with hosted PR CI running for head 7f620eef8bb340ebac41dbe3650102478dc49e20.

Exact steps or command run after this patch:

pnpm test extensions/codex/src/app-server/approval-bridge.test.ts extensions/codex/src/app-server/side-question.test.ts extensions/codex/index.test.ts src/agents/harness/native-hook-relay.test.ts src/agents/tool-policy.test.ts
git diff --check

Evidence after fix:

vitest.unit-fast: 31 passed.
vitest.agents: 49 passed. It verifies Codex native hook relay behavior, including failing closed when native PreToolUse policy rewrites params and Codex exec_command/cmd input normalization before OpenClaw policy evaluation.
vitest.extensions: 3 files passed, 57 tests. It verifies native deny returns decline before plugin prompting, explicit native allow returns accept, missing relay fails closed, native no-decision falls through to plugin approval without re-running legacy before-tool-call policy, side-question hook config on fork and turn start, side-thread approval forwarding with the active relay handle, event selection, disabled clearing config, TTL sizing, cleanup after success/failure, and the public Codex harness opting side questions into nativeHookRelay: { enabled: true }.
git diff --check: passed.

Observed result after fix: Side-question child threads receive native hook relay config, and Codex app-server command approval requests are checked through the active native relay before any app-server/plugin approval path can allow them.

What was not tested: I did not claim live /btw shell execution proof from model-driven prompts because the local runtime attempts completed without producing an actual Codex shell tool call in logs. The broad local CI matrix was not run on this WSL host; hosted PR CI was retriggered on the pushed head.

Verification

git diff --check
Focused local tests listed in ## Real behavior proof, refreshed after the latest rebase at head 7f620eef8bb340ebac41dbe3650102478dc49e20.
Hosted CI was retriggered on the rebased head.

clawsweeper · 2026-05-16T11:41:46Z

Codex review: needs real behavior proof before merge.

Summary
The PR extends Codex native hook relay coverage to /btw side questions, app-server command approvals, exec command normalization, and hook channel context propagation with focused tests and a changelog entry.

Reproducibility: yes. source-reproducible: current main leaves public Codex /btw side questions outside native relay opt-in and app-server command approvals without the active relay handle. I did not run live Codex app-server proof in this read-only review.

Real behavior proof
Needs real behavior proof before merge: The PR body reports focused Vitest/CI output and explicitly does not claim a live /btw shell execution proof, so contributor-visible real behavior proof is still needed before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Human review and real runtime proof remain; automation cannot supply the contributor's real Codex /btw or app-server approval evidence for them.

Security
Cleared: The diff changes a command-approval security boundary but tightens enforcement and adds no dependency, workflow, package-script, secret-handling, or supply-chain surface.

Review details

Best possible solution:

Land the scoped relay hardening after maintainer review and redacted live runtime evidence showing /btw or app-server command approval PreToolUse enforcement after the patch.

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

Yes, source-reproducible: current main leaves public Codex /btw side questions outside native relay opt-in and app-server command approvals without the active relay handle. I did not run live Codex app-server proof in this read-only review.

Is this the best way to solve the issue?

Yes on code direction: the PR extends the existing native hook relay seam and fails closed for unsafe approval-bridge outcomes. Merge should still wait for real runtime proof because this is a security-sensitive execution boundary.

What I checked:

Current main side-question gap: Current main enables native hook relay for normal Codex app-server attempts, but runSideQuestion calls runCodexAppServerSideQuestion with only pluginConfig, leaving public /btw side questions outside that relay opt-in. (extensions/codex/harness.ts:46, ee492092a71d)
Current main approval bridge gap: Current main app-server approval policy runs runBeforeToolCallHook directly and has no active native relay handle, so it cannot bridge command approvals through the same native PreToolUse relay path. (extensions/codex/src/app-server/approval-bridge.ts:293, ee492092a71d)
PR approval bridge implementation: At PR head, command approvals with an active pre_tool_use relay invoke invokeNativeHookRelay, dedupe already-observed tool uses, fail closed on relay errors or unreadable decisions, and fall through to plugin approval only on relay no-decision. (extensions/codex/src/app-server/approval-bridge.ts:300, 40fd3483435c)
PR side-question implementation: At PR head, side questions register a native relay, merge hook config into the forked side thread, pass the relay handle into app-server approvals, and unregister it during cleanup. (extensions/codex/src/app-server/side-question.ts:220, 40fd3483435c)
PR regression coverage: The PR adds approval-bridge tests for native deny, no-decision fallback, dedupe, unreadable/non-deny fail-closed behavior, and missing-relay fail-closed behavior. (extensions/codex/src/app-server/approval-bridge.test.ts:229, 40fd3483435c)
Prior review finding resolved: The latest PR head includes fix(codex): drop stale deadcode allowlist entry, removing the unrelated allowlist churn that the earlier bot review flagged. (scripts/deadcode-unused-files.allowlist.mjs, 40fd3483435c)

Likely related people:

joshavant: Authored the merged native Codex tool-policy relay foundation that this PR extends, including Codex harness relay enablement and approval bridge work. (role: recent area contributor; confidence: high; commits: e57b137aef41; files: extensions/codex/harness.ts, extensions/codex/src/app-server/approval-bridge.ts, extensions/codex/src/app-server/run-attempt.ts)
steipete: Recent current-main Codex guard work touches adjacent app-server runtime files, and this PR branch includes follow-up commits closing native-hook relay review gaps. (role: recent area contributor; confidence: medium; commits: 403fbd729638, 5daca725ea9e, f41d890f7ef9; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/dynamic-tools.ts, extensions/codex/src/app-server/side-question.ts)

Remaining risk / open question:

No redacted live /btw or app-server command approval run currently proves the after-fix native PreToolUse path in a real Codex runtime.

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

Kaspre · 2026-05-16T23:37:33Z

CI triage note for the current PR run on head 3397fbae2c42460f6f5530c76557f80b9cce0617:

The Node failures do not appear to be caused by this PR. I compared the failing PR jobs against the exact base commit used for the merge test, 0190f4ae1e505c63e0de51d6cc5b300d24f4a136, and the same base-side CI run already had the same failures:

checks-node-core-fast: same src/pairing/setup-code.test.ts failure (prefer gateway.tailscale.mode=serve expectation).
checks-node-agentic-control-plane-auth-node: same src/gateway/server.auth.browser-hardening.test.ts failure (origin not allowed expectation vs pairing-required result).
checks-node-core: aggregate failure from the non-dist shard failure above.

The macos-swift failure is also outside this PR's changed surface; it reports SwiftFormat drift in existing macOS files such as ConfigureRemoteCommand.swift, AppState.swift, DashboardWindowController.swift, and RemoteTunnelManager.swift. The exact base run skipped that job, so I cannot say it reproduced on base in the same way, but it is unrelated to the Codex/native-hook files changed here.

The PR-specific changed-surface validation remains green: remote GCP pnpm check:changed passed on 3397fbae2c42460f6f5530c76557f80b9cce0617, and the targeted Codex/native-hook tests passed locally.

Kaspre · 2026-05-16T23:51:18Z

Superseded by the latest rebase/push.

Current head: 7f620eef8bb340ebac41dbe3650102478dc49e20
Current PR base at readback: 54c9820ed9c220180b9cc2785b2b9921bc12b46f

The earlier failures were tied to the previous stale/red base. After rebasing off that base and pushing the refreshed head, the PR check rollup is now green and GitHub reports the PR as MERGEABLE / CLEAN.

Kaspre · 2026-05-17T01:40:20Z

Size note for reviewers: this PR is now labeled XL, and I understand that is a meaningful review burden.

The raw diff size is mostly regression coverage rather than runtime code. At current head 7f620eef8bb340ebac41dbe3650102478dc49e20, the PR is 16 files / +1570 / -34, but roughly 1100 added lines are tests. The runtime change is closer to 470 added lines, spread across the Codex side-question setup, app-server approval bridge, native relay policy handling, and small context plumbing.

The reason I kept these pieces together is that they are one enforcement boundary: native PreToolUse policy should cover Codex command execution even when the command originates from the app-server approval path or a /btw side-question child thread. Splitting only one surface out would leave the guarantee partial:

/btw child threads need native hook relay config.
app-server command approvals need to invoke the active native relay before plugin/app approval can allow execution.
relay policy must fail closed when Codex cannot apply rewritten tool args.

That said, I am happy to split this if maintainers prefer smaller review units. The clean split would be:

app-server command approval bridge to native relay, including fail-closed semantics.
/btw side-question relay config plus channel/context preservation.

The current PR is green and keeps the full behavior covered in one place, but I do not want the XL size to get in the way of review if a split would be easier.

steipete · 2026-05-17T11:02:11Z

Behavior addressed: /btw side-question native hooks and app-server command approvals now use the Codex native hook relay safely; unsupported rewrites fail closed; channel ids are preserved for side-question policy.

Real environment tested: local source checkout plus GitHub Actions on PR head 40fd348.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.test.ts -- -t "passes normalized channel context to app-server dynamic tool result hooks"
pnpm check:test-types
node_modules/.bin/tsgo -p tsconfig.extensions.json --pretty false
pnpm deadcode:unused-files
git diff --check
codex-review --mode branch
GitHub Actions PR checks for 40fd348

Evidence after fix: focused local regression/type/deadcode checks passed; Codex review reported no actionable findings; GitHub PR checks reported no failing, pending, or in-progress checks.

Observed result after fix: native hook relay context is threaded through Codex app-server runs and /btw side questions, app-server command approvals use the active relay path, and the stale deadcode allowlist entry was removed after rebasing onto current main.

What was not tested: no live model-driven /btw shell execution; covered by app-server/native relay unit tests and CI. Codex review sandbox could not run loopback bridge tests because its sandbox rejects 127.0.0.1 listen with EPERM.

* fix(codex): cover side-question native hooks * fix(codex): enforce native approvals for app-server requests * fix(codex): preserve approval fallback after native relay noop * fix(codex): satisfy approval relay json typing * fix(codex): run approval relay in report mode * fix(codex): keep relay pre-tool decisions deny-only * fix(codex): remove dead relay approval branch * fix(codex): dedupe app-server relay approvals * fix(codex): fail closed on native relay rewrites * fix(codex): preserve side-question provider context * fix(codex): route side-question replies to origin * fix(codex): preserve native hook channel context * test(codex): align native relay rewrite assertion * fix(codex): align side-question hook config * fix(codex): route side-question approvals safely * test(codex): fix side-question hook typing * fix(codex): preserve side-question hook policy context * fix(codex): close native hook relay review gaps * fix(codex): keep dynamic tool hook channel context * fix(codex): preserve native finalize hook channel context * fix(codex): scope dynamic tool result hooks by channel * fix(codex): drop stale deadcode allowlist entry --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>

openclaw-barnacle Bot added extensions: codex size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 16, 2026

Kaspre mentioned this pull request May 16, 2026

fix(codex): enforce native pre-tool policy relay #82377

Closed

Kaspre mentioned this pull request May 16, 2026

[Bug]: Codex harness bypass persists after #82078 — native-hook-relay registration silently skipped, hooks block never sent in thread/start #82372

Closed

Kaspre force-pushed the fix/codex-side-question-native-hooks branch from ab19e92 to 0adce0b Compare May 16, 2026 20:14

openclaw-barnacle Bot added agents Agent runtime and tooling size: XL proof: supplied External PR includes structured after-fix real behavior proof. and removed size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 16, 2026

clawsweeper Bot added the P1 High-priority user-facing bug, regression, or broken workflow. label May 16, 2026

Kaspre force-pushed the fix/codex-side-question-native-hooks branch from 3397fba to fd44d16 Compare May 16, 2026 23:47

Kaspre force-pushed the fix/codex-side-question-native-hooks branch from fd44d16 to 7f620ee Compare May 17, 2026 01:02

clawsweeper Bot added the impact:security Security boundary, credential, authz, sandbox, or sensitive-data risk. label May 17, 2026

steipete force-pushed the fix/codex-side-question-native-hooks branch 2 times, most recently from a710a83 to 2d717e7 Compare May 17, 2026 09:04

openclaw-barnacle Bot added the scripts Repository scripts label May 17, 2026

clawsweeper Bot added mantis: telegram-visible-proof Mantis should capture Telegram visible proof. impact:message-loss Channel message delivery can be lost, duplicated, or misrouted. labels May 17, 2026

Kaspre added 6 commits May 17, 2026 11:38

fix(codex): cover side-question native hooks

de72684

fix(codex): enforce native approvals for app-server requests

685f1b9

fix(codex): preserve approval fallback after native relay noop

ad005b6

fix(codex): satisfy approval relay json typing

479e675

fix(codex): run approval relay in report mode

f9e040b

fix(codex): keep relay pre-tool decisions deny-only

67dfd9a

Kaspre and others added 10 commits May 17, 2026 11:38

test(codex): align native relay rewrite assertion

e5f229b

fix(codex): align side-question hook config

5daca72

fix(codex): route side-question approvals safely

3eec0a1

test(codex): fix side-question hook typing

b467054

fix(codex): preserve side-question hook policy context

d253476

fix(codex): close native hook relay review gaps

f41d890

fix(codex): keep dynamic tool hook channel context

c664eb4

fix(codex): preserve native finalize hook channel context

221bbec

fix(codex): scope dynamic tool result hooks by channel

88ad771

fix(codex): drop stale deadcode allowlist entry

40fd348

steipete force-pushed the fix/codex-side-question-native-hooks branch from 117340b to 40fd348 Compare May 17, 2026 10:53

openclaw-barnacle Bot removed the scripts Repository scripts label May 17, 2026

steipete merged commit 69a0c92 into openclaw:main May 17, 2026
112 of 114 checks passed

github-actions Bot mentioned this pull request May 17, 2026

📡 Upstream Digest — 2026-05-17 13:00 UTC curtismercier/openclaw-mods#881

Open

clawsweeper Bot mentioned this pull request May 23, 2026

fix(codex): defer native-hook-relay unregister to avoid cleanup race #83987

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(codex): cover side-question native hooks#82559

fix(codex): cover side-question native hooks#82559
steipete merged 22 commits into
openclaw:mainfrom
Kaspre:fix/codex-side-question-native-hooks

Kaspre commented May 16, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 16, 2026 •

edited

Loading

Uh oh!

Kaspre commented May 16, 2026

Uh oh!

Kaspre commented May 16, 2026 •

edited

Loading

Uh oh!

Kaspre commented May 17, 2026

Uh oh!

steipete commented May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

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

Uh oh!

Problem

Change and value

Who is affected, and who is not

Implementation

Real behavior proof

Verification

Uh oh!

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

Uh oh!

Uh oh!

Kaspre commented May 16, 2026

Uh oh!

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

Uh oh!

Uh oh!

Kaspre commented May 17, 2026

Uh oh!

steipete commented May 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kaspre commented May 16, 2026 •

edited

Loading

clawsweeper Bot commented May 16, 2026 •

edited

Loading

Kaspre commented May 16, 2026 •

edited

Loading