Skip to content

fix(providers): skip unreadable Anthropic payload tool schemas#90289

Closed
vincentkoc wants to merge 1 commit into
mainfrom
fuzz-tool-schema-next199-20260604
Closed

fix(providers): skip unreadable Anthropic payload tool schemas#90289
vincentkoc wants to merge 1 commit into
mainfrom
fuzz-tool-schema-next199-20260604

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 4, 2026

Copy link
Copy Markdown
Member

Summary

  • guard Anthropic-family OpenAI-compatible payload tool conversion against throwing tool-schema getters
  • skip only unreadable tool definitions while preserving healthy payload tools
  • add focused wrapper coverage for poisoned parameters getters and input_schema precedence

Verification

  • node scripts/run-vitest.mjs run src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.test.ts
  • node scripts/run-oxlint.mjs src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.ts src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.test.ts
  • ./node_modules/.bin/oxfmt --check --threads=1 src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.ts src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.test.ts
  • git diff --check origin/main...HEAD
  • .agents/skills/autoreview/scripts/autoreview --mode local
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Real behavior proof

Behavior addressed: Anthropic-family OpenAI-compatible payload wrapping could throw while normalizing a malformed plugin/provider tool whose schema field was exposed through a throwing getter.

Real environment tested: local linked OpenClaw worktree on fuzz-tool-schema-next199-20260604, rebased onto origin/main at 045145c7008, Node wrapper test harness.

Exact steps or command run after this patch: node scripts/run-vitest.mjs run src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.test.ts.

Evidence after fix: focused Vitest passed 1 file / 2 tests; oxlint, oxfmt, diff check, local autoreview, and branch autoreview all passed.

Observed result after fix: the wrapper no longer throws on a poisoned parameters getter, drops the unreadable tool, preserves healthy tools, and uses valid input_schema without reading the poisoned fallback.

What was not tested: broad pnpm check:changed did not run because Blacksmith Testbox was unauthenticated and Crabbox sparse-sync preflight failed before remote execution with less than 1 GiB free in the local Crabbox sparse-sync cache.

@vincentkoc vincentkoc self-assigned this Jun 4, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 4, 2026, 6:32 AM ET / 10:32 UTC.

Summary
The PR wraps Anthropic-family OpenAI-compatible tool conversion field reads in a safe accessor and adds focused Vitest coverage for poisoned schema getters and input_schema precedence.

PR surface: Source +35, Tests +100. Total +135 across 2 files.

Reproducibility: yes. Source inspection shows current main directly reads schema fields during wrapper normalization, so a tool object with a throwing getter on parameters or similar fields will throw; I did not execute it because this review is read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
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:

  • Run or wait for the normal broad changed/CI gate before merging the draft.

Risk before merge

  • [P1] The PR body says the broad changed gate did not run; normal CI or a maintainer-run broad check should still gate merge for this draft PR.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the fix in the existing Anthropic-family payload compatibility wrapper with the focused regression tests, then merge only after the draft PR has normal maintainer and CI validation.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No ClawSweeper repair lane is needed: the protected maintainer draft PR has no actionable review findings, and the remaining action is normal maintainer/CI validation.

Security
Cleared: The diff only changes a provider payload wrapper and its focused test; it does not touch dependencies, workflows, secrets, package metadata, install scripts, or release automation.

Review details

Best possible solution:

Keep the fix in the existing Anthropic-family payload compatibility wrapper with the focused regression tests, then merge only after the draft PR has normal maintainer and CI validation.

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

Yes. Source inspection shows current main directly reads schema fields during wrapper normalization, so a tool object with a throwing getter on parameters or similar fields will throw; I did not execute it because this review is read-only.

Is this the best way to solve the issue?

Yes. The PR fixes the narrow owner path that performs the Anthropic-to-OpenAI tool payload conversion, preserves healthy tools, and avoids a new config/API surface.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 88eb40549188.

Label changes

Label changes:

  • add P2: This is a normal-priority provider payload bug fix with limited blast radius and focused tests, not an urgent live regression or emergency.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external contributor proof gate does not apply because this is a maintainer-labeled MEMBER PR, though the PR body includes focused local wrapper validation evidence.

Label justifications:

  • P2: This is a normal-priority provider payload bug fix with limited blast radius and focused tests, not an urgent live regression or emergency.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external contributor proof gate does not apply because this is a maintainer-labeled MEMBER PR, though the PR body includes focused local wrapper validation evidence.
Evidence reviewed

PR surface:

Source +35, Tests +100. Total +135 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 47 12 +35
Tests 1 100 0 +100
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 147 12 +135

What I checked:

Likely related people:

  • steipete: Current wrapper lines blame to the repository snapshot commit by Peter Steinberger, and earlier history shows Peter introduced the model-compat based Anthropic tool payload normalization path. (role: introduced behavior and recent area contributor; confidence: high; commits: 5a869eea5a28, e5c06dd64a16; files: src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.ts, src/agents/pi-embedded-runner/extra-params.ts)
  • opriz: Authored the Kimi tool-format fix that defines the sibling invariant for when Anthropic tool payloads must remain native instead of being converted to OpenAI function shape. (role: adjacent owner; confidence: medium; commits: 51bae7512048; files: src/agents/provider-capabilities.ts, src/agents/pi-embedded-runner-extraparams.test.ts)
  • vincentkoc: Authored the release baseline commit that touched the current wrapper, plugin SDK export, and model catalog compatibility surfaces now involved in this PR. (role: recent area contributor; confidence: medium; commits: 2e08f0f4221f; files: src/llm/providers/stream-wrappers/anthropic-family-tool-payload-compat.ts, src/plugin-sdk/provider-stream.ts, packages/model-catalog-core/src/model-catalog-types.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: 🐚 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. labels Jun 4, 2026
This was referenced Jun 14, 2026
Merged
Merged
@steipete

steipete commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Landed the same issue with broader contract coverage in #92908 (19130e0dc2f4801d8fab8f2dadf2573d47d80628).

The landed fix keeps healthy siblings, quarantines nested/non-serializable/structurally invalid schemas, reconciles function/custom/string/allowed_tools choices without broadening access, preserves provider metadata, adds release-shard coverage, and passed live OpenAI plus full CI.

I could not update this branch because maintainer edits are disabled, so I recreated the fix on a maintainer branch and preserved your authorship with Co-authored-by: Vincent Koc <[email protected]>.

Thanks, @vincentkoc. Enabling Allow edits by maintainers on future PRs lets us apply fixups directly instead of recreating the branch.

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

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vincentkoc @steipete