diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts index cf163e26a66..3d92bcfd9f7 100644 --- 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(), @@ -86,6 +93,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(), @@ -224,6 +233,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 index c980ee17ec6..e23d7aaa33c 100644 --- 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 index f223d4b0fcf..1b8582c9cc4 100644 --- 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 index f5b4ab7dd3d..ad4bb0590f8 100644 --- 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 index b76769fcf02..2b312244006 100644 --- 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 index 8057f779495..b41d409dbb7 100644 --- 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 index 967d5339bad..b6ec3ed39b0 100644 --- 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 index 84f8ef807e2..fd9d158f3f7 100644 --- 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 index 107122dcb46..ceb66717670 100644 --- 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 index dd9ee12bfcb..5272c334510 100644 --- 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 index 2efb2cbdff1..1bb76734db3 100644 --- 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 index 225e076297e..f4133029c99 100644 --- a/src/integrations/editor/DiffViewProvider.ts +++ b/src/integrations/editor/DiffViewProvider.ts @@ -4,6 +4,7 @@ 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" @@ -11,6 +12,7 @@ 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 index ad1950345bd..a4aded95bb9 100644 --- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts +++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts @@ -1,6 +1,12 @@ import { DiffViewProvider, DIFF_VIEW_URI_SCHEME, DIFF_VIEW_LABEL_CHANGES } from "../DiffViewProvider" import * as vscode from "vscode" import * as path from "path" +import delay from "delay" + +// Mock delay +vi.mock("delay", () => ({ + default: vi.fn().mockResolvedValue(undefined), +})) // Mock fs/promises vi.mock("fs/promises", () => ({ @@ -45,6 +51,12 @@ vi.mock("vscode", () => ({ languages: { getDiagnostics: vi.fn(() => []), }, + DiagnosticSeverity: { + Error: 0, + Warning: 1, + Information: 2, + Hint: 3, + }, WorkspaceEdit: vi.fn().mockImplementation(() => ({ replace: vi.fn(), delete: vi.fn(), @@ -327,4 +339,83 @@ describe("DiffViewProvider", () => { ).toBeUndefined() }) }) + + describe("saveChanges method with diagnostic settings", () => { + beforeEach(() => { + // Setup common mocks for saveChanges tests + ;(diffViewProvider as any).relPath = "test.ts" + ;(diffViewProvider as any).newContent = "new content" + ;(diffViewProvider as any).activeDiffEditor = { + document: { + getText: vi.fn().mockReturnValue("new content"), + isDirty: false, + save: vi.fn().mockResolvedValue(undefined), + }, + } + ;(diffViewProvider as any).preDiagnostics = [] + + // Mock vscode functions + vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any) + vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([]) + }) + + it("should apply diagnostic delay when diagnosticsEnabled is true", async () => { + const mockDelay = vi.mocked(delay) + mockDelay.mockClear() + + // Mock closeAllDiffViews + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + + const result = await diffViewProvider.saveChanges(true, 3000) + + // Verify delay was called with correct duration + expect(mockDelay).toHaveBeenCalledWith(3000) + expect(vscode.languages.getDiagnostics).toHaveBeenCalled() + expect(result.newProblemsMessage).toBe("") + }) + + it("should skip diagnostics when diagnosticsEnabled is false", async () => { + const mockDelay = vi.mocked(delay) + mockDelay.mockClear() + + // Mock closeAllDiffViews + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + + const result = await diffViewProvider.saveChanges(false, 2000) + + // Verify delay was NOT called and diagnostics were NOT checked + expect(mockDelay).not.toHaveBeenCalled() + expect(vscode.languages.getDiagnostics).not.toHaveBeenCalled() + expect(result.newProblemsMessage).toBe("") + }) + + it("should use default values when no parameters provided", async () => { + const mockDelay = vi.mocked(delay) + mockDelay.mockClear() + + // Mock closeAllDiffViews + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + + const result = await diffViewProvider.saveChanges() + + // Verify default behavior (enabled=true, delay=2000ms) + expect(mockDelay).toHaveBeenCalledWith(1000) + expect(vscode.languages.getDiagnostics).toHaveBeenCalled() + expect(result.newProblemsMessage).toBe("") + }) + + it("should handle custom delay values", async () => { + const mockDelay = vi.mocked(delay) + mockDelay.mockClear() + + // Mock closeAllDiffViews + ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined) + + const result = await diffViewProvider.saveChanges(true, 5000) + + // Verify custom delay was used + expect(mockDelay).toHaveBeenCalledWith(5000) + expect(vscode.languages.getDiagnostics).toHaveBeenCalled() + }) + }) }) diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 98f3aa7d293..060ec8cb152 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -214,6 +214,7 @@ export type ExtensionState = Pick< | "terminalZshP10k" | "terminalZdotdir" | "terminalCompressProgressBar" + | "diagnosticsEnabled" | "diffEnabled" | "fuzzyMatchThreshold" // | "experiments" // Optional in GlobalSettings, required here. diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 5d6ec0f41ca..e983ce720d9 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -107,6 +107,7 @@ export interface WebviewMessage { | "updateMcpTimeout" | "fuzzyMatchThreshold" | "writeDelayMs" + | "diagnosticsEnabled" | "enhancePrompt" | "enhancedPrompt" | "draggedImages"