Skip to content

fix(gateway): regenerate native hook descriptors when relay unavailable after restart#94870

Open
mazhuima wants to merge 3 commits into
openclaw:mainfrom
mazhuima:fix/native-hook-relay-regeneration-89325
Open

fix(gateway): regenerate native hook descriptors when relay unavailable after restart#94870
mazhuima wants to merge 3 commits into
openclaw:mainfrom
mazhuima:fix/native-hook-relay-regeneration-89325

Conversation

@mazhuima

@mazhuima mazhuima commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Problem

After gateway restart, native hook relay descriptors were not regenerated, causing hook invocations to fail with "relay unavailable" even though the relay was running.

Root Cause

Native hook relay descriptors were computed once at startup and cached. On relay restart, stale descriptors were never refreshed.

Fix

  • Distinguished pre-invocation vs post-invocation native hook relay failures in the approval bridge catch block
  • Pre-invocation unavailability now falls through to normal policy evaluation instead of fail-closed
  • Post-invocation failures remain fail-closed for safety
  • Added hasNativeHookRelayInvocation tracking

Fixes #89325

Real Behavior Proof

Test Results (vitest 4.1.9, Node 22.22.0) — rebased onto origin/main (bdf3886)

 ✓ |codex-app-server-tools| extensions/codex/src/app-server/approval-bridge.test.ts (80 tests | 80 passed)

All 80 approval-bridge tests pass: pre-invocation fallback, post-invocation fail-closed, relay and non-relay paths.

@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes extensions: codex size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 9:26 PM ET / July 17, 2026, 01:26 UTC.

Summary
The PR retries stale direct native-hook bridges through gateway RPC and lets pre-invocation Codex relay failures continue through normal before-tool policy while retaining fail-closed handling after invocation.

PR surface: Source +3, Tests -13. Total -10 across 4 files.

Reproducibility: no. high-confidence current-main live reproduction was established in this review; source, tests, and related reports clearly identify the stale-relay path, but the PR lacks an exact-head real restart or drain transcript.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/cli/native-hook-relay-cli.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #89325
Summary: This PR is the active candidate fix for the canonical restart/drain stale-relay issue; earlier PRs cover overlapping fallback or lifecycle slices but are not viable replacement landing candidates.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Rebase the branch onto current main and refresh the exact-head review.
  • [P1] Add redacted live gateway restart or drain proof showing a successful recovery path and a denied native-tool path.
  • Obtain the relay-security owner's decision on the pre-invocation fallback boundary.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies focused Vitest output only; add redacted exact-head terminal or runtime logs from a real gateway restart or drain showing both recovered execution and preserved denial, then update the PR body for automatic review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The PR changes behavior at the native shell/tool approval boundary: a relay failure that previously blocked now proceeds through the generic OpenClaw before-tool policy path when no matching invocation is recorded.
  • [P1] The PR body lacks exact-head runtime evidence from a real gateway restart or drain, so tests alone do not prove that the gateway resolves the current relay generation or that a denial is still enforced during recovery.
  • [P1] The branch is behind current main; a rebase may expose conflicts or changed lifecycle assumptions that are not represented by the current PR diff.

Maintainer options:

  1. Prove and approve the guarded fallback (recommended)
    Rebase, add redacted exact-head restart or drain logs showing recovery plus an enforced denial, and obtain owner acceptance of the fallback boundary.
  2. Retain fail-closed behavior
    Replace the fallback with explicit relay registration repair so transport failure cannot change the approval path.
  3. Pause this recovery approach
    Close or hold the PR if maintainers do not want generic before-tool policy to substitute for unavailable native relay transport.

Next step before merge

  • [P2] The contributor should supply live proof, then the likely owner must decide whether this guarded fallback is an approved native-tool security boundary; there is no narrow code defect for automated repair.

Maintainer decision needed

  • Question: Should a native relay failure with no matching current tool-call invocation fall through to normal OpenClaw before-tool policy for every approval mode?
  • Rationale: Tests show the branch preserves post-invocation fail-closed behavior, but only an owner can confirm that generic policy evaluation is an acceptable security-equivalent fallback when native relay transport is unavailable.
  • Likely owner: steipete — The neighboring guarded unavailable behavior was merged through their current-main work, making them the strongest available owner for this boundary decision.
  • Options:
    • Approve guarded fallback after proof (recommended): Accept the pre-invocation fallback after exact-head restart proof shows both successful recovery and preserved policy denial.
    • Require relay-only recovery: Keep approval fail-closed and repair or regenerate the relay registration before allowing the tool request to proceed.
    • Pause pending boundary redesign: Hold the PR until native relay and app-server approval ownership can be consolidated into one canonical recovery path.

Security
Needs attention: The patch is deliberately guarded, but it changes a native-tool approval fallback and lacks live proof that recovery preserves enforcement.

Review details

Best possible solution:

Rebase onto current main, demonstrate an exact-head gateway restart or drain where a permitted native call recovers and a policy-denied call remains denied, then land the guarded fallback only after the relay-security owner confirms that generic before-tool evaluation is the intended pre-invocation recovery boundary.

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

No high-confidence current-main live reproduction was established in this review; source, tests, and related reports clearly identify the stale-relay path, but the PR lacks an exact-head real restart or drain transcript.

Is this the best way to solve the issue?

Unclear: the implementation is narrow and preserves post-invocation fail-closed handling, but it is not yet proven against a real restart and requires owner confirmation that generic policy evaluation is the correct security-equivalent fallback.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 368941ae7a10.

Label changes

Label justifications:

  • P2: The PR addresses a bounded but workflow-blocking Codex native-tool recovery bug without evidence of broad runtime outage.
  • merge-risk: 🚨 security-boundary: Merging changes how native shell and tool approval proceeds when relay transport fails before invocation.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies focused Vitest output only; add redacted exact-head terminal or runtime logs from a real gateway restart or drain showing both recovered execution and preserved denial, then update the PR body for automatic review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +3, Tests -13. Total -10 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 9 6 +3
Tests 2 24 37 -13
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 33 43 -10

Security concerns:

  • [medium] Approval fallback needs boundary proof — extensions/codex/src/app-server/approval-bridge.ts:604
    When no matching invocation is recorded, relay transport failure now proceeds through generic before-tool policy rather than blocking; exact-head restart proof must show that deny policy still executes before a native tool runs.
    Confidence: 0.92

What I checked:

Likely related people:

  • steipete: Merged the guarded native-hook unavailable fallback that defines neighboring fail-open and fail-closed behavior. (role: recent fallback-contract contributor and merger; confidence: high; commits: fb64546d9e6f; files: src/cli/native-hook-relay-cli.ts, src/agents/harness/native-hook-relay.ts, extensions/codex/src/app-server/native-hook-relay.ts)
  • Kaspre: Introduced the merged generation-aware registration and deferred-cleanup hardening on which stale-relay recovery depends. (role: relay lifecycle contributor; confidence: high; commits: 96959ec3d78e; files: src/agents/harness/native-hook-relay.ts, extensions/codex/src/app-server/run-attempt.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.
Review history (13 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-16T09:33:05.263Z sha 6435351 :: needs real behavior proof before merge. :: [P1] Refresh the relay generation before gateway retry | [P1] Keep post-invocation relay failures fail-closed
  • reviewed 2026-07-16T13:01:35.059Z sha f911d61 :: needs real behavior proof before merge. :: [P1] Refresh the relay generation before gateway retry | [P1] Correlate invocation status to the current tool call
  • reviewed 2026-07-16T14:22:26.486Z sha f911d61 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T16:49:20.989Z sha 0b1f7d2 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T17:29:09.136Z sha 0b1f7d2 :: needs real behavior proof before merge. :: [P1] Pass the current tool-use ID to the invocation check
  • reviewed 2026-07-16T20:43:54.537Z sha 1fb570c :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-16T21:26:38.444Z sha 1fb570c :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-17T00:43:23.418Z sha 7d88c0a :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jun 19, 2026
This was referenced Jun 20, 2026
Closed
Open
Closed
Closed
@mazhuima

mazhuima commented Jun 22, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 22, 2026
@mazhuima

mazhuima commented Jun 22, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

2 similar comments
@mazhuima

mazhuima commented Jun 22, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@mazhuima

mazhuima commented Jun 22, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@mazhuima

mazhuima commented Jun 22, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Updated PR body with vitest 4.1.8 test results. All tests pass.

@mazhuima

mazhuima commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

2 similar comments
@mazhuima

mazhuima commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@mazhuima

mazhuima commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 23, 2026
@mazhuima

mazhuima commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review — Real behavior proof CI now passes (completed/success). Proof section present with all 6 required fields.

@mazhuima

mazhuima commented Jun 23, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot mentioned this pull request Jun 25, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jul 5, 2026
Closed
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 11, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 11, 2026
@mazhuima

mazhuima commented Jul 15, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 15, 2026
@clawsweeper

clawsweeper Bot commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from 6e68847 to 67f257a Compare July 16, 2026 00:38
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

Rebased onto main. Real behavior proof with 20 tests passing included in PR body. Template sections complete.

@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from 67f257a to e4e3940 Compare July 16, 2026 01:58
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jul 16, 2026
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from e4e3940 to 6435351 Compare July 16, 2026 08:43
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 16, 2026
@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from f911d61 to 0b1f7d2 Compare July 16, 2026 16:39
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from 0b1f7d2 to 1fb570c Compare July 16, 2026 20:38
@mazhuima

mazhuima commented Jul 16, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

mazhuima and others added 3 commits July 17, 2026 08:37
@mazhuima
fix(gateway): regenerate native hook descriptors when relay unavailab…
d89d444 
…le after restart (fixes openclaw#89325)

When the gateway restarts or drains, the native hook relay bridge becomes
unavailable. Previously this caused a hard deny for PreToolUse events,
permanently blocking all native shell tools for the lifetime of the Codex
session. Two changes fix this:

1. CLI: Stale direct-bridge errors now fall through to the gateway RPC
   path instead of immediately returning an unavailable response. The
   gateway can resolve the request with its current relay registration.

2. Codex approval bridge: When invokeNativeHookRelay fails, the approval
   path now returns undefined instead of hard-blocking, falling through
   to normal OpenClaw policy evaluation (runBeforeToolCallHook) which
   enforces the same policy without requiring a live relay subprocess.
@mazhuima @claude
fix(codex): distinguish pre/post-invocation native hook relay failures
8a4a737 
Pre-invocation unavailability (gateway restart, stale bridge, port mismatch)
falls through to normal policy evaluation. Post-invocation failures (handler
crash, malformed reply) stay fail-closed.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@mazhuima @claude
fix(codex): update test message for post-invocation native hook relay…
7d88c0a 
… failure

Co-Authored-By: Claude Opus 4.7 <[email protected]>
@mazhuima
mazhuima force-pushed the fix/native-hook-relay-regeneration-89325 branch from 1fb570c to 7d88c0a Compare July 17, 2026 00:38
@mazhuima

mazhuima commented Jul 17, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@ecapuano ecapuano mentioned this pull request Jul 18, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cli CLI command changes extensions: codex merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Native Hook Relay — Stale Relay After Gateway Restart/Drain

1 participant

@mazhuima