[myk1yt]

Supersedes #1705 (stale-bot closed; conflicts now resolved vs current main)

Related GitHub Issue

Closes: #1307

Description

When a turn executes multiple tool_use blocks and at least one tool returns images, userMessageContent ended up as tool_result, image, tool_result, image. Anthropic rejects that user message with a hard 400 (tool_use ids were found without tool_result blocks immediately after) because tool_result blocks must be the first, contiguous content of the user message. Both consumers (Task.flushPendingToolResultsToHistory and the main request assembly) passed the array through verbatim.

Fix at the single choke point every tool_result push flows through (Task.pushToolResultToUserContent, covering all 8 call sites: the main tool-result callback, MCP-native dispatch, rejection/validation-error paths, and the task-interruption pre-injection path): after appending a new tool_result, any image blocks already in userMessageContent are relocated after it. Result shape: [trA, trB, imgA, imgB, text] — Anthropic accepts non-tool_result content after the contiguous result run, so this is spec-compliant and matches the issue's expected shape.

Why not at the image-push sites (as the issue suggested): if tool A returns images and a later tool result has no images, [trA, imgA, trB] still interleaves — grouping at the result push is the only complete minimal fix, and it keeps the consumer untouched.

Behavior preserved: multiple images keep per-tool order; single-tool-with-images and image-free turns are byte-identical; images with no tool results are never relocated.

Test Procedure

• src/core/task/__tests__/Task.spec.ts (+1 regression test, 1 test updated to the new correct order with full block equality — stricter, not weaker): tr1 + 2 images + image-less tr2 → asserts [tr1, tr2, img1, img2].
• src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts (+2 integration tests through the real presentAssistantMessage): two image-returning tools → [trA, trB, imgA, imgB]; first tool returns images, second returns none → images relocated after the second result. Both fail on main.
• cd src && ./node_modules/.bin/vitest run core/assistant-message → 67/67 passed.
• cd src && ./node_modules/.bin/vitest run core/task/__tests__/Task.spec.ts → 151/151 passed.
• ESLint on all 3 changed files → clean.

Pre-Submission Checklist

• Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
• Scope: My changes are focused on the linked issue (one major feature/fix per PR).
• Self-Review: I have performed a thorough self-review of my code.
• Testing: New and/or updated tests have been added to cover my changes (if applicable).
• Visual Snapshot (UI changes only): N/A — no UI changes.
• Documentation Impact: No documentation updates are required.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Visual Snapshots

N/A.

Videos (interaction / animation only)

N/A.

Documentation Updates

• No documentation updates are required.

Additional Notes

Pre-existing follow-up (out of scope): a repetition-limit feedback text block can still land between tool results (presentAssistantMessage.ts ~684); this PR improves that case too (was tr, text, img, tr before). Happy to address separately if it ever manifests.

Get in Touch

GitHub: @myk1yt — please tag me here; I monitor notifications.

[coderabbitai]

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Tool results are now kept together at the start of a message, ahead of text and images. Text and images retain their original relative order. Images from parallel tool responses stay in the same order as their corresponding results, including when a tool returns no images, so image placement no longer disrupts the sequence of tool results in the message.

Task.pushToolResultToUserContent now places tool-result blocks before other content and preserves order within both groups. Tests cover text and image blocks, including parallel tool responses and a later tool that returns no images.

Changes: Tool Result Ordering

Layer / File(s)	Summary
Keep tool results contiguous src/core/task/Task.ts, src/core/task/__tests__/Task.spec.ts	The method groups tool-result blocks before other content and preserves order within each group. Tests cover interleaved text and images, including a later tool result with no images.
Test parallel tool responses src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts, src/eslint-suppressions.json	The image-handling tests call the real task method and verify tool-result and image ordering for parallel tool responses. The configured suppression count decreases from 16 to 14.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

• Zoo-Code-Org/Zoo-Code#2113: Changes the same method and adds Anthropic history normalization; this PR further ensures accumulated tool results form a contiguous prefix.

Merge Risk: Minimal · up to 0d593

No actionable merge-blocking issue is established. The change groups tool results before images and text, with tests covering the reported ordering regression; merge after normal checks pass.

Pre-merge checks: 8

Check name	Status	Explanation
Title check		The title 'fix(anthropic): keep tool_result blocks contiguous by grouping image blocks separately' matches the main change: Task.pushToolResultToUserContent now keeps tool_result blocks first and cont…
Description check		The description follows the template's main sections: it links the issue (Closes: #1307), describes the implementation and trade-offs, gives a detailed test procedure, and completes the pre-submission…
Linked Issues check		Issue #1307 requires contiguous tool_result blocks before image and text blocks for multi-tool turns. The reviewed head updates Task.pushToolResultToUserContent to group all tool_result blocks f…
Out of Scope Changes check		The changed implementation in src/core/task/Task.ts directly fixes issue #1307. The changes in presentAssistantMessage-images.spec.ts and Task.spec.ts verify the required ordering and preserve t…
Regression Evidence		The changed Task.pushToolResultToUserContent behavior has focused unit coverage in src/core/task/__tests__/Task.spec.ts. The tests cover unique results, duplicate rejection, error results, pre-exi…
Security Boundaries		No changed path meets a security-boundary failure condition. The only production change is in Task.pushToolResultToUserContent: it deduplicates existing tool_result blocks, appends the supplied bl…
Persistence Integrity		No changed persistence path exists. The PR changes only in-memory ordering in Task.pushToolResultToUserContent and tests. The existing flush path awaits saveApiConversationHistory, clones history …
Lifecycle Resource Cleanup		No changed lifecycle path can leak a resource or duplicate work. The only production change is Task.pushToolResultToUserContent in src/core/task/Task.ts:591-612; it synchronously filters and splic…

Full details: Title check

Explanation

The title 'fix(anthropic): keep tool_result blocks contiguous by grouping image blocks separately' matches the main change: Task.pushToolResultToUserContent now keeps tool_result blocks first and contiguous, relocating image blocks after them, with tests covering this. It is a Conventional Commits style title, specific, and under 100 characters, though it is slightly long at 86 characters and 10 words and uses somewhat technical phrasing.

Full details: Description check

Explanation

The description follows the template's main sections: it links the issue (Closes: #1307), describes the implementation and trade-offs, gives a detailed test procedure, and completes the pre-submission checklist. Visual snapshots and videos are marked N/A, and the documentation section is answered. The description is clear and substantive. Minor gaps: the Discord username field from the template is replaced by a GitHub handle, and the Description section does not restate the 'why' from the issue in a separate sentence, though it covers the problem. Some sentences are long and use contractions-free but compound structures, so they only loosely follow the requested Simplified Technical English style.

Full details: Linked Issues check

Explanation

Issue #1307 requires contiguous tool_result blocks before image and text blocks for multi-tool turns. The reviewed head updates Task.pushToolResultToUserContent to group all tool_result blocks first while preserving result order and non-result order. The change also handles a later image-free result. Tests cover two image-returning tools and an image-returning tool followed by an image-free tool through presentAssistantMessage, plus direct Task regression cases. The supplied PR evidence reports both test suites passing.

Full details: Out of Scope Changes check

Explanation

The changed implementation in src/core/task/Task.ts directly fixes issue #1307. The changes in presentAssistantMessage-images.spec.ts and Task.spec.ts verify the required ordering and preserve tool-use order. The test binding, type updates, and eslint-suppressions.json count update support those test changes. No unrelated production behavior or unrelated feature is shown. The noted repetition-limit text ordering remains a separate follow-up and is not required by issue #1307.

Full details: Regression Evidence

Explanation

The changed Task.pushToolResultToUserContent behavior has focused unit coverage in src/core/task/__tests__/Task.spec.ts. The tests cover unique results, duplicate rejection, error results, pre-existing text and images, multiple results, a later result without images, interleaved text and images, and preservation of block order. presentAssistantMessage-images.spec.ts also uses the real Task implementation and covers two image-producing tools plus an image-producing tool followed by an image-free tool. The pull request has no UI changes, so no Playwright snapshot is required.

Full details: Security Boundaries

Explanation

No changed path meets a security-boundary failure condition. The only production change is in Task.pushToolResultToUserContent: it deduplicates existing tool_result blocks, appends the supplied block, and partitions content by block type. It does not read or expose secrets or PII, execute input, or change approval or allowlist controls. The other changes are tests and an ESLint suppression update.

Full details: Persistence Integrity

Explanation

No changed persistence path exists. The PR changes only in-memory ordering in Task.pushToolResultToUserContent and tests. The existing flush path awaits saveApiConversationHistory, clones history before saving, clears pending content only after success, and retains it after failure (Task.ts:1583-1608). The persistence writer uses a temporary file, rename commit, and rollback (safeWriteJson.ts:96-191). No changed operation is unawaited, non-atomic, or missing partial-failure handling.

Full details: Lifecycle Resource Cleanup

Explanation

No changed lifecycle path can leak a resource or duplicate work. The only production change is Task.pushToolResultToUserContent in src/core/task/Task.ts:591-612; it synchronously filters and splices an in-memory array after a tool result is appended. It adds no listener, watcher, provider, timer, task, subscription, or cancellation/restart work. The existing callers only invoke this method, and the remaining changes are tests and lint metadata.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Awaiting fresh human maintainer or CODEOWNER approval.

Automated review is complete for the latest commit but does not replace human approval.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!