Roo-Code/src/core/tools/applyDiffTool.ts
roomote[bot] b6bded9818
feat: add configurable delay for Go diagnostics to prevent premature error reporting (#5863)
* feat: add configurable delay for Go diagnostics to prevent premature error reporting

- Add diagnosticsDelayMs setting (default: 2000ms) to allow linters time to process
- Add diagnosticsEnabled setting to optionally disable diagnostic checking entirely
- Update DiffViewProvider.saveChanges() to use configurable delay before checking diagnostics
- Update all tool files (writeToFile, searchAndReplace, insertContent, applyDiff, multiApplyDiff) to pass diagnostic settings
- Add comprehensive tests for new diagnostic functionality
- Fixes issue where Go diagnostics errors were submitted to LLM before linter could clean up unused imports

Resolves #5859

* fix: add missing TypeScript type definitions for diagnostic settings

- Add diagnosticsDelayMs and diagnosticsEnabled to globalSettingsSchema
- Include properties in ExtensionState Pick type
- Add default values to EVALS_SETTINGS
- Fix VSCode mock to include DiagnosticSeverity for tests
- Resolves compilation errors in ClineProvider and webviewMessageHandler

* fix: update test mocks to support diagnostic settings in tool tests

- Add providerRef mock to insertContentTool and writeToFileTool tests
- Update mocks to include diagnosticsEnabled and diagnosticsDelayMs settings
- Fix test expectations to match new implementation with diagnostic configuration
- Resolves failing unit tests for insertContentTool.spec.ts and writeToFileTool.spec.ts

* fix: remove package-lock.json file (project uses pnpm)

* refactor: use existing writeDelayMs instead of diagnosticsDelayMs

- Remove diagnosticsDelayMs setting in favor of existing writeDelayMs
- Add min(0) validation for writeDelayMs in global settings schema
- Add error handling around delay function calls in DiffViewProvider
- Create DEFAULT_WRITE_DELAY_MS constant (1000ms) to replace repeated defaults
- Update all tool files to pass writeDelayMs instead of diagnosticsDelayMs
- Remove diagnosticsDelayMs from webview message handlers and types
- Update test files to use writeDelayMs instead of diagnosticsDelayMs

This refactoring consolidates diagnostic delay functionality to use the
existing writeDelayMs setting as requested in PR feedback.

* fix: resolve failing unit tests and TypeScript compilation errors

- Fix DiffViewProvider test to expect correct default delay (1000ms instead of 2000ms)
- Fix TypeScript type errors in ClineProvider test mock state object
- Correct terminalPowershellCounter and terminalZdotdir types to boolean
- Fix pinnedApiConfigs type from array to Record<string, boolean>

* fix: remove unrelated changes from ClineProvider.spec.ts

- Removed extensive unrelated property additions to mock state
- Kept only diagnosticsEnabled property which is related to Go diagnostics delay feature
- Removed unused DEFAULT_WRITE_DELAY_MS import
- Restored original structure and organization of mock state object

This addresses the feedback to remove unrelated changes while preserving
the necessary diagnostic functionality for the Go diagnostics delay feature.

* refactor: move DEFAULT_WRITE_DELAY_MS to packages/types/src/global-settings.ts

- Move DEFAULT_WRITE_DELAY_MS constant from src/shared/constants.ts to packages/types/src/global-settings.ts
- Update all import statements in affected files to use @roo-code/types
- Delete src/shared/constants.ts file as it is no longer needed
- Files updated:
  - src/integrations/editor/DiffViewProvider.ts
  - src/core/webview/ClineProvider.ts
  - src/core/tools/multiApplyDiffTool.ts
  - src/core/tools/applyDiffTool.ts
  - src/core/tools/searchAndReplaceTool.ts
  - src/core/tools/insertContentTool.ts
  - src/core/tools/writeToFileTool.ts

---------

Co-authored-by: Roo Code <roomote@roocode.com>
2025-07-18 16:12:00 -04:00

211 lines
6.8 KiB
TypeScript

import path from "path"
import fs from "fs/promises"
import { TelemetryService } from "@roo-code/telemetry"
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { ClineSayTool } from "../../shared/ExtensionMessage"
import { getReadablePath } from "../../utils/path"
import { Task } from "../task/Task"
import { ToolUse, RemoveClosingTag, AskApproval, HandleError, PushToolResult } from "../../shared/tools"
import { formatResponse } from "../prompts/responses"
import { fileExistsAtPath } from "../../utils/fs"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
export async function applyDiffToolLegacy(
cline: Task,
block: ToolUse,
askApproval: AskApproval,
handleError: HandleError,
pushToolResult: PushToolResult,
removeClosingTag: RemoveClosingTag,
) {
const relPath: string | undefined = block.params.path
let diffContent: string | undefined = block.params.diff
if (diffContent && !cline.api.getModel().id.includes("claude")) {
diffContent = unescapeHtmlEntities(diffContent)
}
const sharedMessageProps: ClineSayTool = {
tool: "appliedDiff",
path: getReadablePath(cline.cwd, removeClosingTag("path", relPath)),
diff: diffContent,
}
try {
if (block.partial) {
// Update GUI message
let toolProgressStatus
if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) {
toolProgressStatus = cline.diffStrategy.getProgressStatus(block)
}
if (toolProgressStatus && Object.keys(toolProgressStatus).length === 0) {
return
}
await cline
.ask("tool", JSON.stringify(sharedMessageProps), block.partial, toolProgressStatus)
.catch(() => {})
return
} else {
if (!relPath) {
cline.consecutiveMistakeCount++
cline.recordToolError("apply_diff")
pushToolResult(await cline.sayAndCreateMissingParamError("apply_diff", "path"))
return
}
if (!diffContent) {
cline.consecutiveMistakeCount++
cline.recordToolError("apply_diff")
pushToolResult(await cline.sayAndCreateMissingParamError("apply_diff", "diff"))
return
}
const accessAllowed = cline.rooIgnoreController?.validateAccess(relPath)
if (!accessAllowed) {
await cline.say("rooignore_error", relPath)
pushToolResult(formatResponse.toolError(formatResponse.rooIgnoreError(relPath)))
return
}
const absolutePath = path.resolve(cline.cwd, relPath)
const fileExists = await fileExistsAtPath(absolutePath)
if (!fileExists) {
cline.consecutiveMistakeCount++
cline.recordToolError("apply_diff")
const formattedError = `File does not exist at path: ${absolutePath}\n\n<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>`
await cline.say("error", formattedError)
pushToolResult(formattedError)
return
}
let originalContent: string | null = await fs.readFile(absolutePath, "utf-8")
// Apply the diff to the original content
const diffResult = (await cline.diffStrategy?.applyDiff(
originalContent,
diffContent,
parseInt(block.params.start_line ?? ""),
)) ?? {
success: false,
error: "No diff strategy available",
}
// Release the original content from memory as it's no longer needed
originalContent = null
if (!diffResult.success) {
cline.consecutiveMistakeCount++
const currentCount = (cline.consecutiveMistakeCountForApplyDiff.get(relPath) || 0) + 1
cline.consecutiveMistakeCountForApplyDiff.set(relPath, currentCount)
let formattedError = ""
TelemetryService.instance.captureDiffApplicationError(cline.taskId, currentCount)
if (diffResult.failParts && diffResult.failParts.length > 0) {
for (const failPart of diffResult.failParts) {
if (failPart.success) {
continue
}
const errorDetails = failPart.details ? JSON.stringify(failPart.details, null, 2) : ""
formattedError = `<error_details>\n${
failPart.error
}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
}
} else {
const errorDetails = diffResult.details ? JSON.stringify(diffResult.details, null, 2) : ""
formattedError = `Unable to apply diff to file: ${absolutePath}\n\n<error_details>\n${
diffResult.error
}${errorDetails ? `\n\nDetails:\n${errorDetails}` : ""}\n</error_details>`
}
if (currentCount >= 2) {
await cline.say("diff_error", formattedError)
}
cline.recordToolError("apply_diff", formattedError)
pushToolResult(formattedError)
return
}
cline.consecutiveMistakeCount = 0
cline.consecutiveMistakeCountForApplyDiff.delete(relPath)
// Show diff view before asking for approval
cline.diffViewProvider.editType = "modify"
await cline.diffViewProvider.open(relPath)
await cline.diffViewProvider.update(diffResult.content, true)
cline.diffViewProvider.scrollToFirstDiff()
// Check if file is write-protected
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
const completeMessage = JSON.stringify({
...sharedMessageProps,
diff: diffContent,
isProtected: isWriteProtected,
} satisfies ClineSayTool)
let toolProgressStatus
if (cline.diffStrategy && cline.diffStrategy.getProgressStatus) {
toolProgressStatus = cline.diffStrategy.getProgressStatus(block, diffResult)
}
const didApprove = await askApproval("tool", completeMessage, toolProgressStatus, isWriteProtected)
if (!didApprove) {
await cline.diffViewProvider.revertChanges() // Cline likely handles closing the diff view
return
}
// Call saveChanges to update the DiffViewProvider properties
const provider = cline.providerRef.deref()
const state = await provider?.getState()
const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
// Track file edit operation
if (relPath) {
await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
}
// Used to determine if we should wait for busy terminal to update before sending api request
cline.didEditFile = true
let partFailHint = ""
if (diffResult.failParts && diffResult.failParts.length > 0) {
partFailHint = `But unable to apply all diff parts to file: ${absolutePath}, silently use <read_file> tool to check newest file version and re-apply diffs\n`
}
// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
if (partFailHint) {
pushToolResult(partFailHint + message)
} else {
pushToolResult(message)
}
await cline.diffViewProvider.reset()
return
}
} catch (error) {
await handleError("applying diff", error)
await cline.diffViewProvider.reset()
return
}
}