feat: add configurable delay for Go diagnostics to prevent premature error reporting (#5863)

roomote[bot] · roomote · web-flow · commit b6bded9818f0 · 2025-07-18T16:12:00.000-04:00
* 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>
diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts
@@ -15,6 +15,13 @@ import { modeConfigSchema } from "./mode.js"
 import { customModePromptsSchema, customSupportPromptsSchema } from "./mode.js"
 import { languagesSchema } from "./vscode.js"
 
+/**
+ * Default delay in milliseconds after writes to allow diagnostics to detect potential problems.
+ * This delay is particularly important for Go and other languages where tools like goimports
+ * need time to automatically clean up unused imports.
+ */
+export const DEFAULT_WRITE_DELAY_MS = 1000
+
 /**
  * GlobalSettings
  */
@@ -37,7 +44,7 @@ export const globalSettingsSchema = z.object({
 	alwaysAllowWrite: z.boolean().optional(),
 	alwaysAllowWriteOutsideWorkspace: z.boolean().optional(),
 	alwaysAllowWriteProtected: z.boolean().optional(),
-	writeDelayMs: z.number().optional(),
+	writeDelayMs: z.number().min(0).optional(),
 	alwaysAllowBrowser: z.boolean().optional(),
 	alwaysApproveResubmit: z.boolean().optional(),
 	requestDelaySeconds: z.number().optional(),
@@ -87,6 +94,8 @@ export const globalSettingsSchema = z.object({
 	terminalZdotdir: z.boolean().optional(),
 	terminalCompressProgressBar: z.boolean().optional(),
 
+	diagnosticsEnabled: z.boolean().optional(),
+
 	rateLimitSeconds: z.number().optional(),
 	diffEnabled: z.boolean().optional(),
 	fuzzyMatchThreshold: z.number().optional(),
@@ -226,6 +235,8 @@ export const EVALS_SETTINGS: RooCodeSettings = {
 	terminalCompressProgressBar: true,
 	terminalShellIntegrationDisabled: true,
 
+	diagnosticsEnabled: true,
+
 	diffEnabled: true,
 	fuzzyMatchThreshold: 1,
 
diff --git a/src/core/tools/__tests__/insertContentTool.spec.ts b/src/core/tools/__tests__/insertContentTool.spec.ts
@@ -71,6 +71,14 @@ describe("insertContentTool", () => {
 			cwd: "/",
 			consecutiveMistakeCount: 0,
 			didEditFile: false,
+			providerRef: {
+				deref: vi.fn().mockReturnValue({
+					getState: vi.fn().mockResolvedValue({
+						diagnosticsEnabled: true,
+						writeDelayMs: 1000,
+					}),
+				}),
+			},
 			rooIgnoreController: {
 				validateAccess: vi.fn().mockReturnValue(true),
 			},
diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts
@@ -132,6 +132,14 @@ describe("writeToFileTool", () => {
 		mockCline.consecutiveMistakeCount = 0
 		mockCline.didEditFile = false
 		mockCline.diffStrategy = undefined
+		mockCline.providerRef = {
+			deref: vi.fn().mockReturnValue({
+				getState: vi.fn().mockResolvedValue({
+					diagnosticsEnabled: true,
+					writeDelayMs: 1000,
+				}),
+			}),
+		}
 		mockCline.rooIgnoreController = {
 			validateAccess: vi.fn().mockReturnValue(true),
 		}
@@ -376,7 +384,7 @@ describe("writeToFileTool", () => {
 				userEdits: userEditsValue,
 				finalContent: "modified content",
 			})
-			// Manually set the property on the mock instance because the original saveChanges is not called
+			// Set the userEdits property on the diffViewProvider mock to simulate user edits
 			mockCline.diffViewProvider.userEdits = userEditsValue
 
 			await executeWriteFileTool({}, { fileExists: true })
diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts
@@ -2,6 +2,7 @@ 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"
@@ -170,7 +171,11 @@ export async function applyDiffToolLegacy(
 			}
 
 			// Call saveChanges to update the DiffViewProvider properties
-			await cline.diffViewProvider.saveChanges()
+			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) {
diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts
@@ -10,6 +10,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { fileExistsAtPath } from "../../utils/fs"
 import { insertGroups } from "../diff/insert-groups"
+import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 export async function insertContentTool(
 	cline: Task,
@@ -155,7 +156,11 @@ export async function insertContentTool(
 		}
 
 		// Call saveChanges to update the DiffViewProvider properties
-		await cline.diffViewProvider.saveChanges()
+		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) {
diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts
@@ -2,6 +2,7 @@ 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"
@@ -553,7 +554,11 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
 				}
 
 				// Call saveChanges to update the DiffViewProvider properties
-				await cline.diffViewProvider.saveChanges()
+				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
 				await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
diff --git a/src/core/tools/searchAndReplaceTool.ts b/src/core/tools/searchAndReplaceTool.ts
@@ -11,6 +11,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage"
 import { getReadablePath } from "../../utils/path"
 import { fileExistsAtPath } from "../../utils/fs"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
+import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 /**
  * Tool for performing search and replace operations on files
@@ -227,7 +228,11 @@ export async function searchAndReplaceTool(
 		}
 
 		// Call saveChanges to update the DiffViewProvider properties
-		await cline.diffViewProvider.saveChanges()
+		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) {
diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts
@@ -13,6 +13,7 @@ import { getReadablePath } from "../../utils/path"
 import { isPathOutsideWorkspace } from "../../utils/pathUtils"
 import { detectCodeOmission } from "../../integrations/editor/detect-omission"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
+import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 export async function writeToFileTool(
 	cline: Task,
@@ -213,7 +214,11 @@ export async function writeToFileTool(
 			}
 
 			// Call saveChanges to update the DiffViewProvider properties
-			await cline.diffViewProvider.saveChanges()
+			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) {
diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts
@@ -42,6 +42,7 @@ import { ExtensionMessage, MarketplaceInstalledMetadata } from "../../shared/Ext
 import { Mode, defaultModeSlug } from "../../shared/modes"
 import { experimentDefault, experiments, EXPERIMENT_IDS } from "../../shared/experiments"
 import { formatLanguage } from "../../shared/language"
+import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 import { Terminal } from "../../integrations/terminal/Terminal"
 import { downloadTask } from "../../integrations/misc/export-markdown"
 import { getTheme } from "../../integrations/theme/getTheme"
@@ -1436,6 +1437,7 @@ export class ClineProvider
 			profileThresholds,
 			alwaysAllowFollowupQuestions,
 			followupAutoApproveTimeoutMs,
+			diagnosticsEnabled,
 		} = await this.getState()
 
 		const telemetryKey = process.env.POSTHOG_API_KEY
@@ -1489,7 +1491,7 @@ export class ClineProvider
 			remoteBrowserHost,
 			remoteBrowserEnabled: remoteBrowserEnabled ?? false,
 			cachedChromeHostUrl: cachedChromeHostUrl,
-			writeDelayMs: writeDelayMs ?? 1000,
+			writeDelayMs: writeDelayMs ?? DEFAULT_WRITE_DELAY_MS,
 			terminalOutputLineLimit: terminalOutputLineLimit ?? 500,
 			terminalShellIntegrationTimeout: terminalShellIntegrationTimeout ?? Terminal.defaultShellIntegrationTimeout,
 			terminalShellIntegrationDisabled: terminalShellIntegrationDisabled ?? false,
@@ -1555,6 +1557,7 @@ export class ClineProvider
 			hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
 			alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
 			followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
+			diagnosticsEnabled: diagnosticsEnabled ?? true,
 		}
 	}
 
@@ -1638,6 +1641,7 @@ export class ClineProvider
 			alwaysAllowFollowupQuestions: stateValues.alwaysAllowFollowupQuestions ?? false,
 			alwaysAllowUpdateTodoList: stateValues.alwaysAllowUpdateTodoList ?? false,
 			followupAutoApproveTimeoutMs: stateValues.followupAutoApproveTimeoutMs ?? 60000,
+			diagnosticsEnabled: stateValues.diagnosticsEnabled ?? true,
 			allowedMaxRequests: stateValues.allowedMaxRequests,
 			autoCondenseContext: stateValues.autoCondenseContext ?? true,
 			autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100,
@@ -1656,7 +1660,7 @@ export class ClineProvider
 			remoteBrowserEnabled: stateValues.remoteBrowserEnabled ?? false,
 			cachedChromeHostUrl: stateValues.cachedChromeHostUrl as string | undefined,
 			fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0,
-			writeDelayMs: stateValues.writeDelayMs ?? 1000,
+			writeDelayMs: stateValues.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS,
 			terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500,
 			terminalShellIntegrationTimeout:
 				stateValues.terminalShellIntegrationTimeout ?? Terminal.defaultShellIntegrationTimeout,
diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts
@@ -540,6 +540,7 @@ describe("ClineProvider", () => {
 			sharingEnabled: false,
 			profileThresholds: {},
 			hasOpenedModeSelector: false,
+			diagnosticsEnabled: true,
 		}
 
 		const message: ExtensionMessage = {
diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts
@@ -1044,6 +1044,10 @@ export const webviewMessageHandler = async (
 			await updateGlobalState("writeDelayMs", message.value)
 			await provider.postStateToWebview()
 			break
+		case "diagnosticsEnabled":
+			await updateGlobalState("diagnosticsEnabled", message.bool ?? true)
+			await provider.postStateToWebview()
+			break
 		case "terminalOutputLineLimit":
 			await updateGlobalState("terminalOutputLineLimit", message.value)
 			await provider.postStateToWebview()
diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts
@@ -4,13 +4,15 @@ import * as fs from "fs/promises"
 import * as diff from "diff"
 import stripBom from "strip-bom"
 import { XMLBuilder } from "fast-xml-parser"
+import delay from "delay"
 
 import { createDirectoriesForFile } from "../../utils/fs"
 import { arePathsEqual, getReadablePath } from "../../utils/path"
 import { formatResponse } from "../../core/prompts/responses"
 import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
 import { ClineSayTool } from "../../shared/ExtensionMessage"
 import { Task } from "../../core/task/Task"
+import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
 
 import { DecorationController } from "./DecorationController"
 
@@ -179,7 +181,7 @@ export class DiffViewProvider {
 		}
 	}
 
-	async saveChanges(): Promise<{
+	async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
 		newProblemsMessage: string | undefined
 		userEdits: string | undefined
 		finalContent: string | undefined
@@ -214,18 +216,35 @@ export class DiffViewProvider {
 		// and can address them accordingly. If problems don't change immediately after
 		// applying a fix, won't be notified, which is generally fine since the
 		// initial fix is usually correct and it may just take time for linters to catch up.
-		const postDiagnostics = vscode.languages.getDiagnostics()
-
-		const newProblems = await diagnosticsToProblemsString(
-			getNewDiagnostics(this.preDiagnostics, postDiagnostics),
-			[
-				vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention)
-			],
-			this.cwd,
-		) // Will be empty string if no errors.
-
-		const newProblemsMessage =
-			newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : ""
+		
+		let newProblemsMessage = ""
+		
+		if (diagnosticsEnabled) {
+			// Add configurable delay to allow linters time to process and clean up issues
+			// like unused imports (especially important for Go and other languages)
+			// Ensure delay is non-negative
+			const safeDelayMs = Math.max(0, writeDelayMs)
+			
+			try {
+				await delay(safeDelayMs)
+			} catch (error) {
+				// Log error but continue - delay failure shouldn't break the save operation
+				console.warn(`Failed to apply write delay: ${error}`)
+			}
+			
+			const postDiagnostics = vscode.languages.getDiagnostics()
+
+			const newProblems = await diagnosticsToProblemsString(
+				getNewDiagnostics(this.preDiagnostics, postDiagnostics),
+				[
+					vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention)
+				],
+				this.cwd,
+			) // Will be empty string if no errors.
+
+			newProblemsMessage =
+				newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : ""
+		}
 
 		// If the edited content has different EOL characters, we don't want to
 		// show a diff with all the EOL differences.
diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts
diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts
diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts

Original file line number	Diff line number	Diff line change
`@@ -540,6 +540,7 @@ describe("ClineProvider", () => {`
`540`	`540`	`sharingEnabled: false,`
`541`	`541`	`profileThresholds: {},`
`542`	`542`	`hasOpenedModeSelector: false,`
	`543`	`+ diagnosticsEnabled: true,`
`543`	`544`	`}`
`544`	`545`
`545`	`546`	`const message: ExtensionMessage = {`