[AI-assisted] Fix cached plugin tool wrapper bug - iterate through candidates to match by actual runtime tool name#78716
Conversation
openclaw-barnacle
Bot
added
size: S
triage: needs-real-behavior-proof
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff3ab330e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. Source inspection shows current main can select the first unnamed factory through shared plugin-level declaredNames, and the linked lossless-claw report describes that exact multi-tool shape failing with plugin tool runtime missing. Real behavior proof Next step before merge Security Review detailsBest possible solution: Land this runtime-name dispatch fix or an equivalent focused patch after maintainer approval, keeping contracts.tools as discovery metadata rather than per-factory runtime identity. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main can select the first unnamed factory through shared plugin-level declaredNames, and the linked lossless-claw report describes that exact multi-tool shape failing with plugin tool runtime missing. Is this the best way to solve the issue? Yes. Matching explicit named registrations first and then unnamed factories by returned runtime tool name is the narrow maintainable fix; changing manifest contracts or requiring new plugin author behavior would be broader. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against a1ac559ed7e6. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9defb48c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 556ba77d6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7563085dfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
openclaw-barnacle
Bot
added
proof: supplied
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
clawsweeper
Bot
added
the
proof: sufficient
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
removed
the
proof: sufficient
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
clawsweeper
Bot
added
the
proof: sufficient
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
b83680a to
5835285
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
removed
the
proof: sufficient
c91a3b7 to
8040057
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
…tch by actual runtime tool name
… with names and catching factory failures - First try to narrow candidates using names when available to preserve original behavior for named factories - Add try-catch around factory invocation to continue on failures, matching uncached path behavior - This prevents regression for plugins with multiple named factories while fixing the bug for unnamed factories
- P1: Restore catch (error) for cold-load failures (line 986) - P2: Preserve synchronous tool execution errors by moving return outside try blocks - P2: Try named candidates first, then fall back to all candidates (including unnamed) - P2: Keep error handling for factory/materialization failures
- Only try unnamed candidates in the fallback to avoid double invocation - This prevents performance issues and side effects from calling named factories twice
2615dc2 to
e40ce4f
Compare
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
|
Confirming real-world reproduction of #78671 from a production OpenClaw setup, in case it helps unblock the "needs-real-behavior-proof" gate. Environment:
Symptom (exactly as in #78671): [tools] lcm_describe failed: plugin tool runtime missing (lossless-claw): lcm_describe
What we've verified does NOT fix it:
The "plugin tool runtime missing" wording (rather than e.g. "subagent override denied") suggests the registration-time path described in #78671 — handlers are visible in palette but the wrong factory is selected at execution time. Happy to test a build with the patch applied against my live setup if that would help validate the fix. |
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
Fix cached descriptor-backed plugin tool dispatch for unnamed factories sharing manifest contracts. Thanks @zanni098!
3 participants
Summary
Fixes #78671
The bug was in
createCachedDescriptorPluginTool.execute: cached descriptor execution could treat plugin-leveldeclaredNamesas per-factory identity for unnamed factories. In a plugin that declares multiplecontracts.toolsand registers each tool through unnamed factories, the first factory could effectively claim sibling manifest tools and causeplugin tool runtime missing.This PR dispatches cached descriptor-backed tools by resolving the actual runtime tool name. It tries matching named factories first, then unnamed factories, and keeps tool execution outside factory/materialization error handling so real execution errors are preserved.
Real behavior proof
Behavior or issue addressed: Cached descriptor-backed plugin tool execution now reaches the runtime factory whose returned tool name matches the requested manifest tool. This fixes multi-tool plugins where shared manifest
contracts.toolsexpose sibling tools but unnamed factories previously causedplugin tool runtime missing.Real environment tested: Windows local OpenClaw checkout at
C:\Users\eishm\CascadeProjects\openclaw, PR branchfix/cached-plugin-tool-wrapper-declarednames, patched source build, temporaryOPENCLAW_HOME, and a disposable local OpenClaw plugin with one unrelated named factory plus two unnamed factories under sharedcontracts.tools.Exact steps or command run after this patch: Ran
pnpm test src/plugins/tools.optional.test.ts,pnpm exec oxfmt --check --threads=1 src/plugins/tools.ts src/plugins/tools.optional.test.ts CHANGELOG.md,pnpm build,pnpm check:changed, andnode --import tsx .proof/cached-plugin-dispatch-proof.mjs.After-fix evidence:
Observed result after fix: The cached descriptor for
proof_betaexecuted the second unnamed runtime factory and returnedproof-beta-ok. The prior failure mode,plugin tool runtime missing, did not occur. The unrelated named factory was not invoked during cachedproof_betaexecution.What was not tested: Production/private plugin data, secrets, and user conversations were not used. The runtime proof used a disposable local plugin with the same manifest and unnamed-factory shape described in #78671.