refactor(task): append environment details into existing blocks#11437
refactor(task): append environment details into existing blocks#11437roomote[bot] wants to merge 2 commits intomainfrom
Conversation
Add appendEnvironmentDetails() helper that merges environment details into the last text block or tool_result instead of adding a standalone trailing text block. This avoids message shapes that can break interleaved-thinking models like DeepSeek reasoner, which expect specific message structures. Changes: - Add appendEnvironmentDetails() and removeEnvironmentDetailsBlocks() helpers - Update Task.resumeAfterDelegation() to use the helper - Update Task.recursivelyMakeClineRequests() to use the helper - Adapt types to use AI SDK UserContentPart instead of Anthropic SDK types - Add comprehensive unit tests (25 test cases) Re-applies #11198 with type adaptations for the AI SDK migration.
All previously flagged issues have been resolved in 963082f. No new issues found.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
src/core/task/Task.ts
Outdated
| // then append env details to the last text or tool_result block. | ||
| // This avoids creating standalone trailing text blocks which can break | ||
| // interleaved-thinking models like DeepSeek reasoner. | ||
| const contentWithoutEnvDetails = removeEnvironmentDetailsBlocks(lastUserMsg.content) |
There was a problem hiding this comment.
removeEnvironmentDetailsBlocks only removes standalone env_details text blocks (where the entire trimmed text starts/ends with the tags). Since appendEnvironmentDetails now embeds env details into the last text or tool_result block (e.g., "User message\n\n<environment_details>...") rather than as a standalone block, this cleanup can't find them on subsequent calls. In resumeAfterDelegation, which mutates the last user message in conversation history, this means env details will accumulate on each resume.
The PR already has stripAppendedEnvironmentDetails() that handles both standalone and appended env details, but it's only tested, never called. Both this line and the equivalent at line ~2802 should use stripAppendedEnvironmentDetails instead (and update the import accordingly).
| const contentWithoutEnvDetails = removeEnvironmentDetailsBlocks(lastUserMsg.content) | |
| const contentWithoutEnvDetails = stripAppendedEnvironmentDetails(lastUserMsg.content) |
Fix it with Roo Code or mention @roomote and request a fix.
src/core/task/Task.ts
Outdated
| } | ||
| return true | ||
| }) | ||
| const contentWithoutEnvDetails = removeEnvironmentDetailsBlocks(parsedUserContent) |
There was a problem hiding this comment.
Same issue as above: removeEnvironmentDetailsBlocks won't clean up env details that were previously appended into an existing block. Use stripAppendedEnvironmentDetails here as well.
| const contentWithoutEnvDetails = removeEnvironmentDetailsBlocks(parsedUserContent) | |
| const contentWithoutEnvDetails = stripAppendedEnvironmentDetails(parsedUserContent) |
Fix it with Roo Code or mention @roomote and request a fix.
src/core/task/Task.ts
Outdated
| import { MessageManager } from "../message-manager" | ||
| import { validateAndFixToolResultIds } from "./validateToolResultIds" | ||
| import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" | ||
| import { appendEnvironmentDetails, removeEnvironmentDetailsBlocks } from "./appendEnvironmentDetails" |
There was a problem hiding this comment.
The import needs to include stripAppendedEnvironmentDetails (which supersedes removeEnvironmentDetailsBlocks at both call sites). removeEnvironmentDetailsBlocks can be dropped from the import since stripAppendedEnvironmentDetails calls it internally.
| import { appendEnvironmentDetails, removeEnvironmentDetailsBlocks } from "./appendEnvironmentDetails" | |
| import { appendEnvironmentDetails, stripAppendedEnvironmentDetails } from "./appendEnvironmentDetails" |
Fix it with Roo Code or mention @roomote and request a fix.
…ulation Replace removeEnvironmentDetailsBlocks with stripAppendedEnvironmentDetails at both call sites in Task.ts. Since appendEnvironmentDetails now embeds env details into existing blocks rather than standalone blocks, removeEnvironmentDetailsBlocks cannot find them on subsequent calls (e.g., task resume), causing accumulation. stripAppendedEnvironmentDetails handles both standalone and appended env details.
Related GitHub Issue
Closes: #11200
Description
Re-applies the changes from #11198 (which was reverted) with adaptations for the AI SDK type migration that happened since.
The core change: instead of adding
<environment_details>as a standalone trailing text block, we now append it into the last text block ortool_resultblock. This avoids message shapes that break interleaved-thinking models like DeepSeek reasoner, which expect specific message structures.Key implementation details:
appendEnvironmentDetails()helper insrc/core/task/appendEnvironmentDetails.tsthat merges environment details into the last suitable content blockremoveEnvironmentDetailsBlocks()andstripAppendedEnvironmentDetails()helpers for cleaning up existing env detailsTask.resumeAfterDelegation()andTask.recursivelyMakeClineRequests()to use the new helpersContentBlockParam) to AI SDK types (UserContentPart,LegacyToolResultBlock,TextPart) to match the codebase's current type systemTest Procedure
cd src && npx vitest run core/task/__tests__/appendEnvironmentDetails.spec.tstsc --noEmit)Pre-Submission Checklist
Documentation Updates
Additional Notes
This is a re-application of #11198 with type adaptations. The original PR used Anthropic SDK types (
Anthropic.Messages.ContentBlockParam,Anthropic.Messages.ToolResultBlockParam), but the codebase has since migrated to AI SDK types (UserContentPart,LegacyToolResultBlock,TextPartfromai). The logic is identical; only the type signatures changed.