Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

Greptile Summary

This PR bumps the minimum required Codex app-server version from 0.118.0 to 0.124.0 and tightens the SemVer comparison so same-version prereleases (e.g. 0.124.0-alpha.2) no longer satisfy the stable protocol floor, while newer-series prereleases (e.g. 0.125.0-alpha.1) are still accepted. It also adds MCP PreToolUse, PostToolUse, and PermissionRequest relay test coverage and removes "MCP hook parity" from the v1 unsupported table.

Confidence Score: 5/5

Safe to merge — the version gate logic is correct, test coverage is thorough, and documentation is consistent.

The parseVersionForComparison change correctly implements standard SemVer prerelease precedence (prerelease < stable when numeric parts are equal). All test fixtures are updated consistently, new edge-case tests are added for both the prerelease gate and MCP relay payloads, and the regex in readCodexVersionFromUserAgent already captured prerelease identifiers so no further changes were needed there. No logic errors or security concerns were identified.

No files require special attention.

_{Reviews (1): Last reviewed commit: "codex harness mcp hook parity" | Re-trigger Greptile}

Addressed the build-metadata edge in 8be0a253c4. Same-base build-suffixed versions now fail the stable 0.124.0 floor, with tests for 0.124.0+alpha.2 rejection and 0.125.0+custom acceptance.

Follow-up after moving the Codex floor to 0.125.0: the runtime gate, docs, and Codex live-smoke package pin now all point at 0.125.0.

I also addressed the payload-size concern from the security review by adding per-string and aggregate string budgets to native hook relay JSON validation. The other point about forwarding native tool results to plugin hooks is intentional for parity: OpenClaw after_tool_call plugins are privileged hook participants and need the tool result to enforce policy, audit, or maintain state. Redacting that path in the relay would make Codex-native tools weaker than OpenClaw-owned tools. If we want sanitized telemetry later, that should be a separate surface from the plugin hook contract.

I also checked a set of real third-party plugins against the MCP hook question.

The useful direct analogue is Knostic Shield: its L3 path registers before_tool_call, scans tool params for dangerous command/secret patterns, and blocks. I added a relay test for that exact class of behavior using a Codex MCP PreToolUse payload, so a native MCP call like mcp__shell__run_command with a destructive command is denied before execution through the same OpenClaw hook surface.

Most of the other sampled plugins are not native MCP hook consumers. WeCom uses before_tool_call, but only to rewrite OpenClaw's message dynamic tool params. MCP Adapter exposes external MCP servers as OpenClaw-owned tools, so it exercises the dynamic-tool parity path rather than the Codex-native MCP relay. A2A Gateway, Codex App Server bridge, Web Search Plus, and Apify are useful broader contract tests, but they do not prove native MCP hook interception.

Addressed the remaining Aisle relay DoS finding too. Native hook JSON validation now counts object key lengths against the relay string budget, and the PermissionRequest fallback key no longer joins every tool-input key into an unbounded string. The new test covers a large object-key payload on a native MCP PermissionRequest.

On the Aisle PermissionRequest fail-open note: I checked Codex core before changing behavior. Codex PermissionRequest hooks return an optional decision; if hooks return no decision, Codex falls through to its normal guardian/user approval path. In current Codex, tools/orchestrator.rs and mcp_tool_call.rs both treat None from run_permission_request_hooks as "continue to normal approval," not "allow."

So the relay intentionally returns a no-op when OpenClaw chooses to defer or approval is unavailable. That preserves the v1 contract of allow, deny, or defer to Codex's own approval path. I updated the relay comments/log wording to make this explicit. The object-key DoS finding was a real hardening issue and is fixed in the current head.

Current head addresses the two Aisle hardening findings that were reported against earlier commits.

• Pending PermissionRequest decisions are now keyed by relay/run/tool identity plus a stable content fingerprint, so a reused tool id with different tool input cannot inherit an old pending decision.
• Invocation history now retains bounded payload snapshots instead of full native hook payloads. The active invocation still receives the full validated payload, but retained debug history is capped.

I added regression coverage for both cases in src/agents/harness/native-hook-relay.test.ts.

Codex maintainer pass pushed a rebased fixup on top of this PR.

What changed:

• Rebased onto current main and resolved the obsolete channel-entry-contract.test.ts conflict by keeping the .cjs sidecar fix that already landed in fix(github-copilot): preserve reasoning IDs for Copilot Codex models #71684.
• Added one more hardening fix for the Aisle JSON-budget finding: native hook relay validation now checks the pending work budget before enqueueing array/object children, so broad payloads cannot allocate an oversized pending stack before the 20k node cap trips.
• Added regression coverage that proves values beyond the JSON node budget are not even read.

Local verification:

• pnpm test src/agents/harness/native-hook-relay.test.ts extensions/codex/src/app-server/client.test.ts extensions/codex/src/app-server/native-hook-relay.test.ts src/plugin-sdk/channel-entry-contract.test.ts
• pnpm tsgo:core:test && pnpm tsgo:extensions:test
• targeted core + extension oxlint on touched files
• pnpm check:changed

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

for (const key of Object.keys(toolInput).toSorted()) {
  if (processed >= MAX_PERMISSION_FALLBACK_KEYS) break;
  ...
}

for (const key of Object.keys(value).toSorted()) {
  updateJsonHash(hash, value[key]);
}

function permissionRequestToolInputKeyFingerprint(toolInput: Record<string, unknown>): string {
  const keys: string[] = [];
  for (const key in toolInput) {
    if (!Object.prototype.hasOwnProperty.call(toolInput, key)) continue;
    keys.push(key);
    if (keys.length >= MAX_PERMISSION_FALLBACK_KEYS) break;
  }
  keys.sort();
  ​// then apply MAX_PERMISSION_FALLBACK_KEY_CHARS truncation
  ...
}

Analyzed PR: #71707 at commit c7b4402

_{Last updated on: 2026-04-25T20:27:27Z}

Landed. Thanks @pashpashpash!

• Source SHA: c7b4402
• Merge commit: 34fb966
• Gate: PR CI green on source SHA; local pnpm check:changed; focused relay/Codex/plugin-sdk tests; core and extension type/lint checks. The transient parity-gate red was a QA session lock in the Opus mock half and cleared on rerun.