Skip to content

fix(agents): preserve fallback tool-call hints#99851

Merged
vincentkoc merged 1 commit into
mainfrom
fix/claude-fallback-toolcall-hint
Jul 4, 2026
Merged

fix(agents): preserve fallback tool-call hints#99851
vincentkoc merged 1 commit into
mainfrom
fix/claude-fallback-toolcall-hint

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jul 4, 2026

Copy link
Copy Markdown
Member

Follow-up to #99653.

What Problem This Solves

Claude CLI history import normalizes native tool_use blocks to OpenClaw's provider-neutral toolcall shape. Cross-provider fallback formatting still checked only for raw tool_use, so tool-call hints disappeared from the fallback context while tool results remained.

Why This Change Was Made

The fallback formatter now uses the shared isToolCallBlock contract instead of duplicating one provider spelling.

Tests cover both sides of the boundary:

  • the formatter receives canonical toolcall;
  • a real Claude JSONL fixture starts with native tool_use, passes through importer normalization, and still renders the compact tool-call hint.

User Impact

When Claude CLI falls back to another model, the replacement model retains the prior tool-call sequence instead of seeing missing calls or orphaned results.

Evidence

  • node scripts/run-vitest.mjs src/agents/command/attempt-execution.test.ts
    • 1 file, 58 tests passed
  • Changed-file oxfmt, oxlint --deny-warnings, and git diff --check passed
  • Autoreview: clean, no actionable findings, confidence 0.98
  • Three independent best-fix/security/parser reviews: clean
  • Blacksmith Testbox tbx_01kwnwx0qgqrsmkyagjw77f6af

@vincentkoc vincentkoc self-assigned this Jul 4, 2026
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS maintainer Maintainer-authored PR labels Jul 4, 2026
@vincentkoc vincentkoc marked this pull request as ready for review July 4, 2026 06:31
@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 2:39 AM ET / 06:39 UTC.

Summary
The PR changes Claude CLI fallback prelude formatting to recognize provider-neutral toolcall blocks through the shared predicate and adds regression coverage for direct and imported Claude JSONL paths.

PR surface: Source +4, Tests +4. Total +8 across 2 files.

Reproducibility: yes. at source level: current main normalizes native Claude CLI tool_use to toolcall, while the fallback formatter only recognizes raw tool_use. I did not run tests because this cleanup review is read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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

  • [P2] Merging intentionally changes fallback retry session context by adding compact tool-call names for canonical toolcall blocks that current main omits, so maintainers should accept that small context-behavior change before landing.

Maintainer options:

  1. Accept the retry-context change (recommended)
    Merge after required checks if maintainers agree fallback retries should receive compact tool-call names for canonical toolcall blocks.
  2. Request live fallback proof first
    Ask for a small before/after Claude CLI fallback run if maintainers want runtime proof beyond source and focused regression coverage.

Next step before merge

  • No repair lane is needed; the remaining action is maintainer review of the small fallback-context behavior change and required checks.

Security
Cleared: The diff only changes local fallback formatting and tests; it does not alter dependencies, workflows, secrets handling, install scripts, package metadata, or downloaded/executed code.

Review details

Best possible solution:

Land the narrow shared-predicate formatter fix after required checks complete and maintainers accept the fallback retry context behavior change.

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

Yes at source level: current main normalizes native Claude CLI tool_use to toolcall, while the fallback formatter only recognizes raw tool_use. I did not run tests because this cleanup review is read-only.

Is this the best way to solve the issue?

Yes: using the existing isToolCallBlock contract is the narrowest maintainable fix at the formatter boundary. A local spelling list would duplicate the shared tool-content contract and invite drift.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR fixes agent fallback context continuity with focused scope and limited blast radius.
  • merge-risk: 🚨 session-state: The diff changes what prior tool-call context is seeded into fallback retries, which can affect session continuity and model behavior even with green tests.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external contributor proof gate is not applicable because this is a member-authored maintainer-labeled PR; the PR body still includes focused test and Testbox validation evidence.
Evidence reviewed

PR surface:

Source +4, Tests +4. Total +8 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 6 2 +4
Tests 1 6 2 +4
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 12 4 +8

What I checked:

Likely related people:

  • RomneyDa: Git blame points the current fallback formatter, Claude CLI importer normalization, and shared tool-content predicate to commit 13652f6, whose merged PR metadata maps to RomneyDa. (role: introduced current fallback/importer surface; confidence: high; commits: 13652f6654da; files: src/agents/command/attempt-execution.helpers.ts, src/gateway/cli-session-history.claude.ts, src/chat/tool-content.ts)
  • ZOOWH: Merged predecessor work in fix(cli): hide synthetic Claude reseed prompts #99653 is the explicit follow-up context for this PR and changed the surrounding Claude CLI reseed/history behavior. (role: recent related Claude CLI history contributor; confidence: medium; commits: 9aa4250c58ad; files: src/gateway/cli-session-history.claude.ts)
  • vincentkoc: Current main includes vincentkoc's merged adjacent Claude CLI reseed attachment preservation in fix(gateway): preserve legacy reseed attachments #99839, and this PR follows the same importer/fallback area. (role: recent adjacent area contributor; confidence: medium; commits: cafbd745c1b1, 1ea52f5b9984; files: src/gateway/cli-session-history.claude.ts, src/agents/command/attempt-execution.helpers.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. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jul 4, 2026
@vincentkoc

vincentkoc commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Land-ready proof for head 1ea52f5b99842cafe1b1d863c385f61a82ad3645:

  • Exact red/green reproduction: current base omitted the canonical (tool call: Bash) hint; this head preserves it.
  • Focused suite: node scripts/run-vitest.mjs src/agents/command/attempt-execution.test.ts - 58/58 passed.
  • Testbox tbx_01kwnwx0qgqrsmkyagjw77f6af: pnpm check:changed passed.
  • Hosted exact-head CI: 41 successful jobs, 9 skipped, 0 failures.
  • CI run: https://github.com/openclaw/openclaw/actions/runs/28697807614
  • Fresh autoreview: clean, no accepted/actionable findings; confidence 0.98.
  • Three independent current-main reviews found no blockers and agreed the shared provider-neutral predicate is the best fix.
  • Repo-native review artifacts validated with zero findings, and scripts/pr prepare-run 99851 passed.
  • main advanced by the unrelated Control UI CSS-only fix: stop rubber-band bounce of the Control UI shell in the Mac app web view #99830 during final preparation; the prepare guard verified no overlap and the PR remains cleanly mergeable.

No known proof gaps remain for the changed surface.

@vincentkoc vincentkoc merged commit 03fafe2 into main Jul 4, 2026
197 of 206 checks passed
@vincentkoc vincentkoc deleted the fix/claude-fallback-toolcall-hint branch July 4, 2026 06:42
@vincentkoc

vincentkoc commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
@vincentkoc
fix(agents): preserve fallback tool-call hints (openclaw#99851)
49b68c0
@github-actions github-actions Bot mentioned this pull request Jul 4, 2026
Open
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

agents Agent runtime and tooling maintainer Maintainer-authored PR merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS 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.

1 participant

@vincentkoc