fix: apply text transforms to tool calls#97775
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 11:19 AM ET / 15:19 UTC. Summary PR surface: Source +145, Tests +187. Total +332 across 6 files. Reproducibility: yes. at source level: current main restores ordinary output text but does not restore final tool-call argument records before the agent loop validates and executes tools. I did not rerun the live DeepSeek scenario in this read-only review. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land a proof-positive tool-call argument restoration after maintainers either accept the finalized-message hook as public API or keep that final transform internal to the embedded runner/wrapper stack. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main restores ordinary output text but does not restore final tool-call argument records before the agent loop validates and executes tools. I did not rerun the live DeepSeek scenario in this read-only review. Is this the best way to solve the issue? Mostly yes: the patch fixes the right stream and final-message boundaries and avoids rewriting tool names or ids. The unresolved question is whether the new public finalized-message hook is the best permanent API shape. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 825d9a66623a. Label changesLabel justifications:
Evidence reviewedPR surface: Source +145, Tests +187. Total +332 across 6 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
Thanks for the fix. #97769 has landed the canonical #97761 repair at the existing stream text-transform boundary, without adding a new agent-loop finalizer API. The landed version also handles the cross-wrapper edge case where a replacement token spans multiple raw tool-call deltas: output restoration runs after malformed-argument repair and HTML decoding, so repaired structured arguments cannot overwrite the restored executable values. Raw provider JSON fragments remain unchanged. Canonical landed commit: 5a73361 Proof: 32 focused tests including the split-token regression, docs/format checks, clean autoreview, Blacksmith Testbox Closing this broader duplicate as superseded by the landed fix. |
Closes #97761
What Problem This Solves
Fixes an issue where plugin text output transforms restored assistant text but not tool-call argument payloads. When a transform masked prompt text before the provider call, tool calls could still execute with the masked value, so tools such as
readlooked for the wrong path.AI-assisted.
Why This Change Was Made
The stream text-transform wrapper now applies output replacements to tool-call deltas, finalized tool calls, and nested tool-call argument strings, while embedded agent runs also apply the same output transform once at the finalized assistant-message boundary before tool validation and execution. Risk note: output transforms can be non-idempotent, so embedded runs disable final-result transformation in the stream wrapper and use one loop-level finalizer to avoid double replacements.
User Impact
Plugins that use text transforms to mask or rewrite provider-visible text can now rely on restored tool arguments before OpenClaw executes tools. Users should no longer see tool failures caused by transformed names or paths remaining in
toolcall_delta/toolcall_endargument payloads.Evidence
Focused test:
Formatting and whitespace:
Real behavior proof with local DeepSeek environment:
Session proof showed the executed
readcall used the restored path:{ "name": "read", "arguments": { "path": "<proof-workspace>/John Smith.txt" } }Pre-PR structured review: