mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-10 22:41:14 +00:00
fix: progressive error for write_to_file missing content to break retry loops
This commit is contained in:
parent
44fd975b17
commit
0c071f25f1
5 changed files with 230 additions and 3 deletions
5
.changeset/fix-write-to-file-progressive-error.md
Normal file
5
.changeset/fix-write-to-file-progressive-error.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"roo-cline": patch
|
||||
---
|
||||
|
||||
Fix infinite retry loop when write_to_file fails with missing content parameter by providing progressive, tiered error guidance with context window awareness
|
||||
|
|
@ -0,0 +1,123 @@
|
|||
import { formatResponse } from "../responses"
|
||||
|
||||
describe("formatResponse.writeToFileMissingContentError", () => {
|
||||
describe("first failure (tier 1)", () => {
|
||||
it("should include the file path in the error message", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("src/index.ts")
|
||||
})
|
||||
|
||||
it("should include the base error explanation", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("'content' parameter was empty")
|
||||
expect(result).toContain("output token limits")
|
||||
})
|
||||
|
||||
it("should include helpful suggestions", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("Suggestions")
|
||||
expect(result).toContain("apply_diff")
|
||||
})
|
||||
|
||||
it("should include the tool use instructions reminder", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("Reminder: Instructions for Tool Use")
|
||||
})
|
||||
|
||||
it("should mention breaking down the task into smaller steps", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("breaking down the task into smaller steps")
|
||||
})
|
||||
|
||||
it("should suggest using apply_diff or edit for existing files", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).toContain("prefer apply_diff or edit to make targeted edits")
|
||||
})
|
||||
|
||||
it("should not include CRITICAL language on first failure", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).not.toContain("CRITICAL")
|
||||
})
|
||||
|
||||
it("should work with different file paths", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("components/MyComponent.tsx", 1)
|
||||
expect(result).toContain("components/MyComponent.tsx")
|
||||
})
|
||||
})
|
||||
|
||||
describe("second failure (tier 2)", () => {
|
||||
it("should indicate this is the 2nd failed attempt", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
expect(result).toContain("2nd failed attempt")
|
||||
})
|
||||
|
||||
it("should strongly suggest alternative approaches", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
expect(result).toContain("must use a different strategy")
|
||||
expect(result).toContain("Recommended approaches")
|
||||
})
|
||||
|
||||
it("should tell model not to retry full write again", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
expect(result).toContain("Do NOT attempt to write the full file content")
|
||||
})
|
||||
|
||||
it("should not include CRITICAL language on second failure", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2)
|
||||
expect(result).not.toContain("CRITICAL")
|
||||
})
|
||||
})
|
||||
|
||||
describe("third+ failure (tier 3)", () => {
|
||||
it("should include CRITICAL language on third failure", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
expect(result).toContain("CRITICAL")
|
||||
expect(result).toContain("3 times in a row")
|
||||
})
|
||||
|
||||
it("should tell model to NOT retry write_to_file", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
expect(result).toContain("do NOT retry write_to_file")
|
||||
})
|
||||
|
||||
it("should include required action strategies", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3)
|
||||
expect(result).toContain("Required action")
|
||||
expect(result).toContain("apply_diff or edit")
|
||||
expect(result).toContain("50-100 lines")
|
||||
})
|
||||
|
||||
it("should show correct count for higher failure counts", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 5)
|
||||
expect(result).toContain("5 times in a row")
|
||||
})
|
||||
})
|
||||
|
||||
describe("context window awareness", () => {
|
||||
it("should include context warning when usage exceeds 50%", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1, 60)
|
||||
expect(result).toContain("60% full")
|
||||
expect(result).toContain("output capacity is reduced")
|
||||
})
|
||||
|
||||
it("should not include context warning when usage is 50% or below", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1, 50)
|
||||
expect(result).not.toContain("% full")
|
||||
})
|
||||
|
||||
it("should not include context warning when usage is undefined", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 1)
|
||||
expect(result).not.toContain("% full")
|
||||
})
|
||||
|
||||
it("should include context warning in tier 2 messages", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 2, 75)
|
||||
expect(result).toContain("75% full")
|
||||
})
|
||||
|
||||
it("should include context warning in tier 3 messages", () => {
|
||||
const result = formatResponse.writeToFileMissingContentError("src/index.ts", 3, 90)
|
||||
expect(result).toContain("90% full")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -66,6 +66,66 @@ Otherwise, if you have not completed the task and do not need additional informa
|
|||
return `Missing value for required parameter '${paramName}'. Please retry with complete response.\n\n${instructions}`
|
||||
},
|
||||
|
||||
/**
|
||||
* Progressive error for write_to_file when the 'content' parameter is missing.
|
||||
* Uses tiered messaging to break infinite retry loops:
|
||||
* - Tier 1 (first failure): Helpful suggestions
|
||||
* - Tier 2 (second failure): Stronger guidance to use alternative tools
|
||||
* - Tier 3+ (third+ failure): Critical stop with explicit instructions
|
||||
*
|
||||
* @param relPath - The file path that was being written to
|
||||
* @param failureCount - How many consecutive times this has failed (1-based)
|
||||
* @param contextUsagePercent - Optional context window usage percentage
|
||||
*/
|
||||
writeToFileMissingContentError: (relPath: string, failureCount: number, contextUsagePercent?: number): string => {
|
||||
const toolUseInstructionsReminder = getToolInstructionsReminder()
|
||||
|
||||
const baseError =
|
||||
`[write_to_file for '${relPath}'] The 'content' parameter was empty. ` +
|
||||
`This usually means the file content was too large for the model's output token limits, ` +
|
||||
`causing it to be truncated or dropped entirely.`
|
||||
|
||||
const contextWarning =
|
||||
contextUsagePercent !== undefined && contextUsagePercent > 50
|
||||
? `\n\nContext window is ${contextUsagePercent}% full — output capacity is reduced.`
|
||||
: ""
|
||||
|
||||
// Tier 3: Critical stop after 3+ failures
|
||||
if (failureCount >= 3) {
|
||||
return (
|
||||
`CRITICAL: ${baseError}${contextWarning}\n\n` +
|
||||
`This has now failed ${failureCount} times in a row — do NOT retry write_to_file for this file.\n\n` +
|
||||
`Required action:\n` +
|
||||
`1. **Use apply_diff or edit** to make targeted changes to the file instead of rewriting it entirely\n` +
|
||||
`2. **If creating a new file**, write a minimal skeleton first (just imports and function/class signatures, no implementations — keep it under 50-100 lines), then use apply_diff or edit to fill in each section incrementally\n` +
|
||||
`3. **Break the task into smaller steps** — write one function or section at a time`
|
||||
)
|
||||
}
|
||||
|
||||
// Tier 2: Stronger guidance after 2nd failure
|
||||
if (failureCount >= 2) {
|
||||
return (
|
||||
`${baseError}${contextWarning}\n\n` +
|
||||
`This is the 2nd failed attempt — you must use a different strategy.\n\n` +
|
||||
`Recommended approaches:\n` +
|
||||
`1. **Use write_to_file with a minimal skeleton** (just the structure — imports, class/function signatures, no implementations), then use apply_diff or edit to fill in each section incrementally\n` +
|
||||
`2. **Use apply_diff or edit with smaller chunks** — if the file already exists, make targeted edits instead of rewriting the entire file\n` +
|
||||
`3. **Break the task into smaller steps** — write one function or section at a time\n\n` +
|
||||
`Do NOT attempt to write the full file content in a single write_to_file call again.`
|
||||
)
|
||||
}
|
||||
|
||||
// Tier 1: Helpful guidance on first failure
|
||||
return (
|
||||
`${baseError}${contextWarning}\n\n` +
|
||||
`Suggestions:\n` +
|
||||
`- If the file is large, try breaking down the task into smaller steps. Write a skeleton first, then fill in sections using apply_diff or edit.\n` +
|
||||
`- If the file already exists, prefer apply_diff or edit to make targeted edits instead of rewriting the entire file.\n` +
|
||||
`- Ensure the 'content' parameter contains the complete file content before closing the tool tag.\n\n` +
|
||||
toolUseInstructionsReminder
|
||||
)
|
||||
},
|
||||
|
||||
invalidMcpToolArgumentError: (serverName: string, toolName: string) =>
|
||||
JSON.stringify({
|
||||
status: "error",
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { type ClineSayTool, DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
|
|||
|
||||
import { Task } from "../task/Task"
|
||||
import { formatResponse } from "../prompts/responses"
|
||||
import { getApiMetrics } from "../../shared/getApiMetrics"
|
||||
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
|
||||
import { fileExistsAtPath, createDirectoriesForFile } from "../../utils/fs"
|
||||
import { stripLineNumbers, everyLineHasLineNumbers } from "../../integrations/misc/extract-text"
|
||||
|
|
@ -42,7 +43,29 @@ export class WriteToFileTool extends BaseTool<"write_to_file"> {
|
|||
if (newContent === undefined) {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("write_to_file")
|
||||
pushToolResult(await task.sayAndCreateMissingParamError("write_to_file", "content"))
|
||||
|
||||
// Use progressive error with context window awareness to break retry loops
|
||||
const contextWindow = task.api.getModel().info.contextWindow ?? 128_000
|
||||
const tokenUsage = getApiMetrics(task.combineMessages(task.clineMessages.slice(1)))
|
||||
const totalTokens = (tokenUsage.totalTokensIn ?? 0) + (tokenUsage.totalTokensOut ?? 0)
|
||||
const contextUsagePercent = contextWindow > 0 ? Math.round((totalTokens / contextWindow) * 100) : undefined
|
||||
|
||||
const errorMessage = formatResponse.writeToFileMissingContentError(
|
||||
relPath || "unknown",
|
||||
task.consecutiveMistakeCount,
|
||||
contextUsagePercent,
|
||||
)
|
||||
|
||||
await task.say(
|
||||
"error",
|
||||
`Roo tried to use write_to_file for '${relPath || "unknown"}' without value for required parameter 'content'. ${
|
||||
task.consecutiveMistakeCount >= 2
|
||||
? "This has happened multiple times -- Roo will try a different approach."
|
||||
: "Retrying..."
|
||||
}`,
|
||||
)
|
||||
|
||||
pushToolResult(formatResponse.toolError(errorMessage))
|
||||
await task.diffViewProvider.reset()
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,6 +36,10 @@ vi.mock("../../prompts/responses", () => ({
|
|||
toolError: vi.fn((msg) => `Error: ${msg}`),
|
||||
rooIgnoreError: vi.fn((path) => `Access denied: ${path}`),
|
||||
createPrettyPatch: vi.fn(() => "mock-diff"),
|
||||
writeToFileMissingContentError: vi.fn(
|
||||
(relPath: string, failureCount: number, contextUsagePercent?: number) =>
|
||||
`Progressive error for ${relPath} (attempt ${failureCount})`,
|
||||
),
|
||||
},
|
||||
}))
|
||||
|
||||
|
|
@ -62,6 +66,16 @@ vi.mock("../../../integrations/misc/extract-text", () => ({
|
|||
),
|
||||
}))
|
||||
|
||||
vi.mock("../../../shared/getApiMetrics", () => ({
|
||||
getApiMetrics: vi.fn().mockReturnValue({
|
||||
totalTokensIn: 50000,
|
||||
totalTokensOut: 10000,
|
||||
totalCacheWrites: 0,
|
||||
totalCacheReads: 0,
|
||||
totalCost: 0,
|
||||
}),
|
||||
}))
|
||||
|
||||
vi.mock("vscode", () => ({
|
||||
window: {
|
||||
showWarningMessage: vi.fn().mockResolvedValue(undefined),
|
||||
|
|
@ -171,8 +185,10 @@ describe("writeToFileTool", () => {
|
|||
}),
|
||||
}
|
||||
mockCline.api = {
|
||||
getModel: vi.fn().mockReturnValue({ id: "claude-3" }),
|
||||
getModel: vi.fn().mockReturnValue({ id: "claude-3", info: { contextWindow: 200_000 } }),
|
||||
}
|
||||
mockCline.clineMessages = []
|
||||
mockCline.combineMessages = vi.fn().mockReturnValue([])
|
||||
mockCline.fileContextTracker = {
|
||||
trackFileContext: vi.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
|
|
@ -328,7 +344,7 @@ describe("writeToFileTool", () => {
|
|||
})
|
||||
|
||||
it("unescapes HTML entities for non-Claude models", async () => {
|
||||
mockCline.api.getModel.mockReturnValue({ id: "gpt-4" })
|
||||
mockCline.api.getModel.mockReturnValue({ id: "gpt-4", info: { contextWindow: 128_000 } })
|
||||
|
||||
await executeWriteFileTool({ content: "<test>" })
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue