Skip to content

fix(codex): expose bound conversation dynamic tools#85213

Open
wizdomhall-hash wants to merge 1 commit into
openclaw:mainfrom
wizdomhall-hash:pr/macos-codex-plugin-tool-bridge
Open

fix(codex): expose bound conversation dynamic tools#85213
wizdomhall-hash wants to merge 1 commit into
openclaw:mainfrom
wizdomhall-hash:pr/macos-codex-plugin-tool-bridge

Conversation

@wizdomhall-hash

@wizdomhall-hash wizdomhall-hash commented May 22, 2026

Copy link
Copy Markdown

Summary

  • expose OpenClaw dynamic plugin tools to native bound Codex conversation threads
  • refresh managed Codex bindings when the dynamic-tool surface changes
  • route Codex app-server dynamic tool calls back through OpenClaw plugin handlers with bounded timeouts
  • keep bundled Codex host manifests from being shadowed by stale official global plugin installs

Motivation

On macOS, the Kynver AgentOS OpenClaw plugin could be loaded and runtime inspection could see its agent_os_* contracts, but native Codex conversation threads did not receive those dynamic tool specs as callable tools. That meant agent_os_get_context and related tools were visible to OpenClaw inspection but missing from the Codex-visible tool surface.

This PR recovers the working local Track 2 patch from a macOS OpenClaw/Codex install where the AgentOS tools became callable after the Codex bridge change.

Before / After

Before:

  • plugin runtime inspection could see imported Kynver AgentOS tool contracts
  • native bound Codex threads did not expose the imported dynamic tools to Codex tool calls

After:

  • managed Codex conversation bindings include the current OpenClaw dynamic-tool surface
  • tool-surface changes trigger binding refresh instead of leaving stale Codex-visible tools
  • Codex dynamic tool calls dispatch through OpenClaw plugin handlers rather than failing as missing tools

Verification

Focused tests run in the prepared recovery checkout:

node scripts/test-projects.mjs extensions/codex/src/conversation-binding.test.ts src/plugins/manifest-registry.test.ts src/plugins/tools.optional.test.ts

Result:

  • plugin shard: 125 passed
  • Codex extension shard: 19 passed

Additional checks:

git diff --cached --check -- .

The same whitespace check passed again after replaying the patch onto a fresh openclaw/openclaw clone. The fresh clone did not have dependencies installed, so the focused test command there stopped at missing vitest/package.json before tests started.

MacOS smoke from the patched local setup:

  • fresh OpenClaw/Codex tool discovery exposed agent_os_get_context
  • calling it reached the tool bridge successfully instead of failing as a missing tool

Scope

The patch is limited to the Codex conversation binding / dynamic tool bridge and supporting plugin registry behavior needed for that path. No secrets or local machine paths are included.

@openclaw-barnacle openclaw-barnacle Bot added extensions: codex size: XL triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 29, 2026, 4:10 AM ET / 08:10 UTC.

Summary
The branch projects OpenClaw plugin dynamic tools into native Codex bound conversations, dispatches app-server tool-call requests through plugin handlers, updates Codex binding sidecar metadata, adjusts official plugin manifest precedence, and changes plugin tool allowlist behavior/tests.

PR surface: Source +477, Tests +854. Total +1331 across 9 files.

Reproducibility: yes. source-reproducible. Current main starts bound Codex app-server threads without dynamicTools and returns a hardcoded failure for item/tool/call; I did not run a live failing gateway session in this read-only review.

Review metrics: 1 noteworthy metric.

  • Persisted Codex Binding Metadata: 1 field added. The branch persists threadBindingOrigin in Codex app-server sidecars, so upgrade behavior must be reconciled with current schemaVersion 2 binding state before merge.

Stored data model
Persistent data-model change detected: serialized state: extensions/codex/src/app-server/session-binding.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted real behavior proof showing a native bound Codex conversation receiving, invoking, dispatching, and returning a plugin dynamic-tool result.
  • Rebase onto current main and preserve schemaVersion 2 Codex binding lifecycle metadata.
  • Restore additive tools.alsoAllow semantics and reuse the current shared dynamic-tool execution helper.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body has tests and a narrative macOS smoke note, but no inspectable after-fix terminal output, logs, screenshot, recording, or linked artifact showing a real native bound Codex tool path; add redacted proof and update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun.

Mantis proof suggestion
A live transport transcript would materially prove the native bound Codex tool catalog, invocation, plugin dispatch, and returned result path that mocks cannot cover. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram live: verify current PR head in a native bound Codex conversation sees a plugin dynamic tool, invokes it, dispatches to the plugin handler, and returns the result with secrets, tokens, raw chat IDs, and account-specific data redacted.

Risk before merge

  • [P1] Existing users with tools.alsoAllow can silently lose unrelated default non-optional plugin tools if this allowlist change lands.
  • [P1] The branch is dirty against current main and must preserve schemaVersion 2 Codex binding metadata, including network-proxy and deferred dynamic-tool state, during rebase.
  • [P1] The contributor has not supplied inspectable after-fix terminal output, logs, screenshot, recording, or linked artifact showing a real native bound Codex tool path.
  • [P1] Dynamic-tool refresh can replace a managed bound Codex thread when fingerprints change, so session-context preservation needs current-main upgrade proof.
  • [P1] Routing app-server tool-call requests into plugin handlers crosses a security boundary; thread, turn, namespace, and conversation scoping need real-path proof before merge.

Maintainer options:

  1. Rebase And Preserve Current Contracts (recommended)
    Rebase onto current main, keep DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY additive, use dynamic-tool-execution.ts, and preserve schemaVersion 2 binding metadata before requesting proof.
  2. Accept The Upgrade Breaks Explicitly
    Maintainers could intentionally accept the allowlist/session-state behavior changes, but that needs explicit upgrade notes, focused compatibility tests, and owner approval.
  3. Replace With A Fresh Narrow PR
    If the branch cannot be rebased without replacing most of the Codex binding changes, close or pause it in favor of a focused current-main PR for the native-bound dynamic-tool path.

Next step before merge

  • [P1] Human review/contributor action is needed because the branch must rebase onto current Codex lifecycle code, fix the compatibility findings, and provide real behavior proof before merge.

Security
Cleared: No dependency, workflow, lockfile, or secret-handling issue was found; plugin-handler dispatch remains a security-boundary merge risk to prove.

Review findings

  • [P1] Preserve default plugin tools for alsoAllow — src/plugins/tools.ts:374-375
  • [P1] Reuse the shared dynamic-tool execution helper — extensions/codex/src/app-server/dynamic-tool-timeout.ts:11-13
Review details

Best possible solution:

Rebase onto current main, implement the bridge through the current Codex dynamic-tool execution/lifecycle helpers, preserve additive alsoAllow semantics and schemaVersion 2 binding metadata, then add redacted live native-bound Codex proof before merge.

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

Yes, source-reproducible. Current main starts bound Codex app-server threads without dynamicTools and returns a hardcoded failure for item/tool/call; I did not run a live failing gateway session in this read-only review.

Is this the best way to solve the issue?

No, not as written. The bridge is the right layer for the upstream Codex contract, but the stale helper duplication, allowlist regression, dirty branch, schema drift, and missing real behavior proof make this PR unsafe to merge now.

Full review comments:

  • [P1] Preserve default plugin tools for alsoAllow — src/plugins/tools.ts:374-375
    Current main treats the default plugin-tools allowlist sentinel as additive: alsoAllow with the sentinel plus optional_tool keeps default non-optional tools. This new branch returns no default tools for plugins without an explicit match whenever another allowlist entry is present, so existing configs can silently lose default plugin tools after upgrade.
    Confidence: 0.91
  • [P1] Reuse the shared dynamic-tool execution helper — extensions/codex/src/app-server/dynamic-tool-timeout.ts:11-13
    Current main already has extensions/codex/src/app-server/dynamic-tool-execution.ts for Codex dynamic-tool timeouts, protocol cleanup, diagnostics, and terminal-release handling. This PR adds a parallel helper with a lower 30s default and fewer behaviors, so bound conversations would drift from the app-server dynamic-tool contract instead of sharing the maintained path.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 96c8338d5bfa.

Label changes

Label justifications:

  • P1: The PR targets a broken Codex/plugin dynamic-tool workflow and currently carries merge-blocking compatibility and session-state risks.
  • merge-risk: 🚨 compatibility: The diff can remove default plugin tools from existing tools.alsoAllow setups by changing shared allowlist semantics.
  • merge-risk: 🚨 session-state: Dynamic-tool refresh may create a new managed Codex thread and leave prior bound-thread context behind unless current-main upgrade behavior is proven.
  • merge-risk: 🚨 security-boundary: The PR routes app-server item/tool/call requests into plugin handlers, so thread, turn, namespace, and conversation scoping must be preserved.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • 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 has tests and a narrative macOS smoke note, but no inspectable after-fix terminal output, logs, screenshot, recording, or linked artifact showing a real native bound Codex tool path; add redacted proof and update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun.
Evidence reviewed

PR surface:

Source +477, Tests +854. Total +1331 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 6 498 21 +477
Tests 3 859 5 +854
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 1357 26 +1331

What I checked:

  • Current main still rejects bound dynamic-tool calls: Current main registers an app-server request handler for bound Codex conversations, but item/tool/call returns a fixed failure saying native binding does not expose dynamic OpenClaw tools yet. (extensions/codex/src/conversation-binding.ts:701, 96c8338d5bfa)
  • Current main starts bound threads without dynamicTools: The bound thread/start payload contains cwd/model/policy/developer instructions but no dynamicTools payload, so the requested tool catalog is absent on main. (extensions/codex/src/conversation-binding.ts:467, 96c8338d5bfa)
  • PR adds the requested bridge path: The PR head builds a conversation dynamic-tool bridge, passes its specs/fingerprint to thread/start, and dispatches matching item/tool/call requests through handleDynamicToolCallWithTimeout. (extensions/codex/src/conversation-binding.ts:148, 1e6cbfe0f0e1)
  • Codex upstream supports this client contract: Upstream Codex accepts dynamic tools on thread/start and later sends DynamicToolCall requests that clients answer with content items and success status. (../codex/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs:394, ccdfb4f342a2)
  • PR regresses additive alsoAllow semantics: The PR returns an empty list when the default plugin-tools sentinel is combined with another allowlist entry and a plugin has no explicit match, dropping default non-optional plugin tools from existing configs. (src/plugins/tools.ts:374, 1e6cbfe0f0e1)
  • Current main covers additive alsoAllow behavior: Current main explicitly tests that DEFAULT_PLUGIN_TOOLS_ALLOWLIST_ENTRY plus optional_tool keeps both the default other_tool and the selected optional tool. (src/plugins/tools.optional.test.ts:1410, 96c8338d5bfa)

Likely related people:

  • vincentkoc: Recent main history includes Codex binding normalization, network-proxy profiles, and shared thread binding flow that this branch must rebase through. (role: recent Codex binding contributor; confidence: high; commits: 8afc1f770bbe, 5c3469549159, 815ffb3bb2e1; files: extensions/codex/src/conversation-binding.ts, extensions/codex/src/app-server/session-binding.ts)
  • joshavant: Recent work touched native Codex session continuity, exec approval floors, and binding paths that interact with dynamic-tool refresh and session preservation. (role: recent Codex session-state contributor; confidence: medium; commits: b5f8191887b9, 80227005a016, e0405ecc9bd6; files: extensions/codex/src/conversation-binding.ts, extensions/codex/src/app-server/session-binding.ts)
  • steipete: Recent main history touched plugin tool descriptors/allowlist-adjacent behavior and the current Codex dynamic-tool execution helper the PR should reuse. (role: shared plugin tool and dynamic-tool contributor; confidence: high; commits: f62a22ce560e, d99c824ac1db, 7139f473332f; files: src/plugins/tools.ts, src/plugins/tools.optional.test.ts, extensions/codex/src/app-server/dynamic-tool-execution.ts)
  • rohitjavvadi: The merged plugin API compatibility PR touched the same manifest-registry surface that this branch also changes. (role: adjacent plugin manifest compatibility contributor; confidence: medium; commits: 376b03f8eaba; files: src/plugins/manifest-registry.ts, src/plugins/manifest-registry.test.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.

@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. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@BingqingLyu

This comment was marked as spam.

Sign in to view
@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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed 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. labels Jun 19, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 16, 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 stale Marked as stale due to inactivity triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(codex): expose bound conversation dynamic tools This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jul 17, 2026
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

extensions: codex merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XL 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.

2 participants

@wizdomhall-hash @BingqingLyu