Skip to content

fix: apply text transforms to tool calls#97775

Closed
ZengWen-DT wants to merge 1 commit into
openclaw:mainfrom
ZengWen-DT:fix/97761-text-transforms-toolcalls
Closed

fix: apply text transforms to tool calls#97775
ZengWen-DT wants to merge 1 commit into
openclaw:mainfrom
ZengWen-DT:fix/97761-text-transforms-toolcalls

Conversation

@ZengWen-DT

Copy link
Copy Markdown
Contributor

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 read looked 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_end argument payloads.

Evidence

Focused test:

$ node scripts/run-vitest.mjs src/agents/plugin-text-transforms.test.ts
Test Files  1 passed (1)
Tests  6 passed (6)

Formatting and whitespace:

$ pnpm exec oxfmt --check --threads=1 packages/agent-core/src/agent-loop.ts packages/agent-core/src/agent.ts packages/agent-core/src/types.ts src/agents/embedded-agent-runner/run/attempt.ts src/agents/plugin-text-transforms.ts src/agents/plugin-text-transforms.test.ts
All matched files use the correct format.

$ git diff --check

Real behavior proof with local DeepSeek environment:

$ pnpm openclaw --profile issue-97761-proof agent --local --agent main \
  --session-key agent:main:proof-97761-toolcall-baseline-transform \
  --model deepseek/deepseek-v4-pro \
  --message 'Use the read tool exactly once to read the workspace file named "John Smith.txt". Then answer with only the file content, no extra words. Do not use exec.' \
  --json --timeout 180

finalAssistantVisibleText: OPENCLAW_97761_TOOLCALL_RESTORED
toolSummary: calls=1 tools=[read] failures=0

Session proof showed the executed read call used the restored path:

{
  "name": "read",
  "arguments": {
    "path": "<proof-workspace>/John Smith.txt"
  }
}

Pre-PR structured review:

$ .agents/skills/autoreview/scripts/autoreview --mode local
autoreview clean: no accepted/actionable findings reported
overall: patch is correct (0.79)

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Jun 29, 2026
@clawsweeper

clawsweeper Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 29, 2026, 11:19 AM ET / 15:19 UTC.

Summary
The PR applies plugin text-transform output replacements to streamed tool-call payloads, final assistant-message tool-call arguments, and focused regression tests.

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.

  • Public agent-core hooks: 1 added. AgentOptions and AgentLoopConfig are re-exported through openclaw/plugin-sdk/agent-core, so maintainers should notice the new API surface before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97761
Summary: The linked issue is the canonical report for the provider textTransforms tool-call output gap; this PR is a candidate fix, while two parallel open PRs address the same root cause with narrower coverage or weaker proof.

Members:

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] The new transformAssistantMessage option is exported through openclaw/plugin-sdk/agent-core, so external consumers may treat it as a supported contract unless maintainers deliberately accept that API surface.

Maintainer options:

  1. Accept the new hook deliberately
    Maintainers can merge this shape if transformAssistantMessage is intended to become an additive public agent-core/plugin-sdk finalized-message hook.
  2. Keep final restoration internal
    Rework the final-result restoration so embedded runs still transform tool arguments before execution without adding a new exported agent-core option.

Next step before merge

  • [P2] The remaining action is maintainer judgment on the new public agent-core/plugin-sdk hook rather than a narrow automated repair.

Security
Cleared: No concrete security or supply-chain concern found; the diff changes TypeScript agent transform code and tests without dependency, workflow, permission, package, or secret-handling changes.

Review details

Best 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 changes

Label justifications:

  • P2: The PR fixes a bounded agent/provider text-transform bug with external tool-output impact but limited implementation scope.
  • merge-risk: 🚨 compatibility: The PR adds an optional finalized-message transform hook to the public agent-core/plugin-sdk contract.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a local DeepSeek run and a session tool-call argument record showing the restored read path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a local DeepSeek run and a session tool-call argument record showing the restored read path.
Evidence reviewed

PR surface:

Source +145, Tests +187. Total +332 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 5 152 7 +145
Tests 1 188 1 +187
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 340 8 +332

What I checked:

  • Current main text-transform gap: Current main rewrites text deltas, text endings, partial/message/error fields, and final stream results, but the existing content transform does not rewrite tool-call argument records. (src/agents/plugin-text-transforms.ts:48, 825d9a66623a)
  • Tool execution uses the final response result: The agent loop takes await response.result() as the final assistant message and executes tool calls from that message content, so event-only restoration would not be enough. (packages/agent-core/src/agent-loop.ts:519, 825d9a66623a)
  • Current tool-call stream contract: The assistant stream protocol carries tool-call deltas in delta and finalized tool calls in toolCall with ToolCall.arguments as the executable argument record. (packages/llm-core/src/types.ts:383, 825d9a66623a)
  • PR implementation covers stream and result boundaries: The PR recursively rewrites string leaves in tool-call arguments, rewrites toolcall_delta and toolcall_end payloads, and exposes transformPluginMessageText for final assistant-message restoration. (src/agents/plugin-text-transforms.ts:59, 051e827c6977)
  • PR adds public finalized-message hook: The PR adds transformAssistantMessage to AgentLoopConfig, which is exported through the agent-core surface. (packages/agent-core/src/types.ts:198, 051e827c6977)
  • Agent-core is plugin SDK-facing: openclaw/plugin-sdk/agent-core is a package export and src/plugin-sdk/agent-core.ts re-exports packages/agent-core/src/index.js, so AgentOptions and AgentLoopConfig changes become public SDK-facing surface. (package.json:1464, 825d9a66623a)

Likely related people:

  • steipete: Git history and GitHub commit metadata show this account introduced plugin text transforms and later refactored the shared stream iterator wrapper used by the transform helper. (role: feature introducer and adjacent refactor author; confidence: high; commits: 202f80792ed1, 4e2541e5fbbf; files: src/agents/plugin-text-transforms.ts, src/agents/plugin-text-transforms.test.ts)
  • joshavant: Current checkout blame for the inspected transform and agent-core files points through the recent merged inbound-media PR; the commit is adjacent rather than feature-specific, so this is a weaker routing signal. (role: recent current-line contributor; confidence: low; commits: 7c47904bb42d; files: src/agents/plugin-text-transforms.ts, packages/agent-core/src/agent-loop.ts, src/plugin-sdk/agent-core.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 proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 29, 2026
@steipete

steipete commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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 tbx_01kwdfxwxtsjz86x6ctvyc94bz, and all 67 exact-head hosted checks: https://github.com/openclaw/openclaw/actions/runs/28483994373

Closing this broader duplicate as superseded by the landed fix.

@steipete steipete closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] textTransforms should apply output replacements to toolcall_delta / toolcall_end events

2 participants