diff --git a/src/core/mentions/index.ts b/src/core/mentions/index.ts index 601cf4a41f..2820d2c2fb 100644 --- a/src/core/mentions/index.ts +++ b/src/core/mentions/index.ts @@ -248,7 +248,7 @@ async function getFileOrFolderContent( async function getWorkspaceProblems(cwd: string): Promise { // Check if diagnostics are enabled const config = vscode.workspace.getConfiguration("roo-cline") - const includeDiagnostics = config.get("includeDiagnostics", true) + const includeDiagnostics = config.get("includeDiagnostics", false) if (!includeDiagnostics) { return "Diagnostics are disabled in settings." diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 51f0d2f90d..240a611786 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1443,8 +1443,8 @@ export class ClineProvider maxReadFileLine: maxReadFileLine ?? -1, maxConcurrentFileReads: maxConcurrentFileReads ?? 5, includeDiagnostics: includeDiagnostics ?? false, - maxDiagnosticsCount: maxDiagnosticsCount ?? 5, - diagnosticsFilter: diagnosticsFilter ?? ["error", "warning"], + maxDiagnosticsCount: maxDiagnosticsCount ?? 50, + diagnosticsFilter: diagnosticsFilter ?? [], settingsImportedAt: this.settingsImportedAt, terminalCompressProgressBar: terminalCompressProgressBar ?? true, hasSystemPromptOverride, @@ -1597,8 +1597,8 @@ export class ClineProvider maxReadFileLine: stateValues.maxReadFileLine ?? -1, maxConcurrentFileReads: stateValues.maxConcurrentFileReads ?? 5, includeDiagnostics: stateValues.includeDiagnostics ?? false, - maxDiagnosticsCount: stateValues.maxDiagnosticsCount ?? 5, - diagnosticsFilter: stateValues.diagnosticsFilter ?? ["error", "warning"], + maxDiagnosticsCount: stateValues.maxDiagnosticsCount ?? 50, + diagnosticsFilter: stateValues.diagnosticsFilter ?? [], historyPreviewCollapsed: stateValues.historyPreviewCollapsed ?? false, cloudUserInfo, cloudIsAuthenticated, diff --git a/src/core/webview/__tests__/diagnosticsSettings.integration.test.ts b/src/core/webview/__tests__/diagnosticsSettings.integration.test.ts new file mode 100644 index 0000000000..df335767eb --- /dev/null +++ b/src/core/webview/__tests__/diagnosticsSettings.integration.test.ts @@ -0,0 +1,180 @@ +import * as vscode from "vscode" +import { ClineProvider } from "../ClineProvider" +import { ContextProxy } from "../../config/ContextProxy" + +// Mock vscode +jest.mock("vscode", () => ({ + workspace: { + getConfiguration: jest.fn(() => ({ + get: jest.fn(), + })), + }, + ExtensionContext: jest.fn(), + OutputChannel: jest.fn(), + Uri: { + file: jest.fn((path: string) => ({ fsPath: path })), + }, + ExtensionMode: { + Development: 1, + Production: 2, + Test: 3, + }, +})) + +// Mock other dependencies +jest.mock("../../config/ContextProxy") +jest.mock("../../../services/code-index/manager") +jest.mock("../../../services/mcp/McpServerManager") +jest.mock("../../config/CustomModesManager") +jest.mock("../../../services/marketplace/MarketplaceManager") + +describe("Diagnostics Settings Integration", () => { + let provider: ClineProvider + let mockContext: any + let mockOutputChannel: any + let mockContextProxy: jest.Mocked + + beforeEach(() => { + // Setup mocks + mockContext = { + globalState: { + get: jest.fn(), + update: jest.fn(), + keys: jest.fn(() => []), + }, + secrets: { + get: jest.fn(), + store: jest.fn(), + }, + globalStorageUri: { fsPath: "/test/storage" }, + extensionUri: { fsPath: "/test/extension" }, + extension: { + packageJSON: { + version: "1.0.0", + name: "test-extension", + }, + }, + } + + mockOutputChannel = { + appendLine: jest.fn(), + } + + mockContextProxy = { + getValue: jest.fn(), + setValue: jest.fn(), + getValues: jest.fn(() => ({ + includeDiagnostics: false, + maxDiagnosticsCount: 50, + diagnosticsFilter: [], + })), + setValues: jest.fn(), + getProviderSettings: jest.fn(() => ({})), + setProviderSettings: jest.fn(), + resetAllState: jest.fn(), + extensionUri: { fsPath: "/test/extension" }, + globalStorageUri: { fsPath: "/test/storage" }, + extensionMode: vscode.ExtensionMode.Test, + } as any + + // Create provider instance + provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", mockContextProxy) + }) + + afterEach(() => { + jest.clearAllMocks() + }) + + describe("Settings Persistence", () => { + it("should persist includeDiagnostics setting", async () => { + // Simulate setting update + await provider.setValue("includeDiagnostics", true) + + expect(mockContextProxy.setValue).toHaveBeenCalledWith("includeDiagnostics", true) + }) + + it("should persist maxDiagnosticsCount setting", async () => { + // Simulate setting update + await provider.setValue("maxDiagnosticsCount", 100) + + expect(mockContextProxy.setValue).toHaveBeenCalledWith("maxDiagnosticsCount", 100) + }) + + it("should persist diagnosticsFilter setting", async () => { + // Simulate setting update + const filters = ["eslint", "typescript"] + await provider.setValue("diagnosticsFilter", filters) + + expect(mockContextProxy.setValue).toHaveBeenCalledWith("diagnosticsFilter", filters) + }) + + it("should retrieve diagnostics settings from state", () => { + // Setup mock return values + mockContextProxy.getValue.mockImplementation((key: string) => { + const values: Record = { + includeDiagnostics: true, + maxDiagnosticsCount: 75, + diagnosticsFilter: ["error", "warning"], + } + return values[key] + }) + + // Retrieve settings + const includeDiagnostics = provider.getValue("includeDiagnostics") + const maxDiagnosticsCount = provider.getValue("maxDiagnosticsCount") + const diagnosticsFilter = provider.getValue("diagnosticsFilter") + + expect(includeDiagnostics).toBe(true) + expect(maxDiagnosticsCount).toBe(75) + expect(diagnosticsFilter).toEqual(["error", "warning"]) + }) + + it("should update multiple diagnostics settings at once", async () => { + const newSettings = { + includeDiagnostics: true, + maxDiagnosticsCount: 100, + diagnosticsFilter: ["eslint/no-unused-vars", "typescript"], + } + + await provider.setValues(newSettings as any) + + expect(mockContextProxy.setValues).toHaveBeenCalledWith(expect.objectContaining(newSettings)) + }) + }) + + describe("Default Values", () => { + it("should use correct default values when settings are undefined", () => { + mockContextProxy.getValue.mockReturnValue(undefined) + mockContextProxy.getValues.mockReturnValue({}) + + const state = provider.getValues() + + // These defaults should match what's in the code + expect(state.includeDiagnostics ?? false).toBe(false) + expect(state.maxDiagnosticsCount ?? 50).toBe(50) + expect(state.diagnosticsFilter ?? []).toEqual([]) + }) + }) + + describe("Settings Validation", () => { + it("should validate maxDiagnosticsCount range", async () => { + // Test valid range + await provider.setValue("maxDiagnosticsCount", 100) + expect(mockContextProxy.setValue).toHaveBeenCalledWith("maxDiagnosticsCount", 100) + + // Note: Validation should be done in the UI component + // The provider itself doesn't validate ranges + }) + + it("should handle empty diagnosticsFilter", async () => { + await provider.setValue("diagnosticsFilter", []) + expect(mockContextProxy.setValue).toHaveBeenCalledWith("diagnosticsFilter", []) + }) + + it("should handle diagnosticsFilter with multiple values", async () => { + const filters = ["eslint", "typescript", "dart Error", "custom-linter"] + await provider.setValue("diagnosticsFilter", filters) + expect(mockContextProxy.setValue).toHaveBeenCalledWith("diagnosticsFilter", filters) + }) + }) +}) diff --git a/src/integrations/diagnostics/__tests__/diagnosticsToProblemsString.test.ts b/src/integrations/diagnostics/__tests__/diagnosticsToProblemsString.test.ts new file mode 100644 index 0000000000..e546f7d0c4 --- /dev/null +++ b/src/integrations/diagnostics/__tests__/diagnosticsToProblemsString.test.ts @@ -0,0 +1,198 @@ +import * as vscode from "vscode" +import { diagnosticsToProblemsString } from "../index" + +// Mock vscode +jest.mock("vscode", () => ({ + workspace: { + getConfiguration: jest.fn(() => ({ + get: jest.fn((key: string, defaultValue: any) => { + const config: Record = { + includeDiagnostics: false, + maxDiagnosticsCount: 50, + diagnosticsFilter: [], + } + return config[key] ?? defaultValue + }), + })), + fs: { + stat: jest.fn(), + }, + openTextDocument: jest.fn(), + }, + DiagnosticSeverity: { + Error: 0, + Warning: 1, + Information: 2, + Hint: 3, + }, + FileType: { + File: 1, + Directory: 2, + }, + Uri: { + file: (path: string) => ({ fsPath: path }), + }, + Range: jest.fn((startLine: number, startChar: number, endLine: number, endChar: number) => ({ + start: { line: startLine, character: startChar }, + end: { line: endLine, character: endChar }, + })), + Position: jest.fn((line: number, char: number) => ({ line, character: char })), +})) + +describe("diagnosticsToProblemsString", () => { + const mockCwd = "/test/workspace" + + beforeEach(() => { + jest.clearAllMocks() + }) + + it("should return empty string when includeDiagnostics is false", async () => { + const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [ + [ + vscode.Uri.file("/test/workspace/file.ts"), + [ + { + range: new vscode.Range(0, 0, 0, 10), + message: "Test error", + severity: vscode.DiagnosticSeverity.Error, + } as vscode.Diagnostic, + ], + ], + ] + + const result = await diagnosticsToProblemsString(diagnostics, [vscode.DiagnosticSeverity.Error], mockCwd, { + includeDiagnostics: false, + }) + + expect(result).toBe("") + }) + + it("should include diagnostics when includeDiagnostics is true", async () => { + const mockDocument = { + lineAt: jest.fn(() => ({ text: "const x = 1" })), + } + ;(vscode.workspace.openTextDocument as jest.Mock).mockResolvedValue(mockDocument) + ;(vscode.workspace.fs.stat as jest.Mock).mockResolvedValue({ type: vscode.FileType.File }) + + const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [ + [ + vscode.Uri.file("/test/workspace/file.ts"), + [ + { + range: new vscode.Range(0, 0, 0, 10), + message: "Test error", + severity: vscode.DiagnosticSeverity.Error, + source: "typescript", + } as vscode.Diagnostic, + ], + ], + ] + + const result = await diagnosticsToProblemsString(diagnostics, [vscode.DiagnosticSeverity.Error], mockCwd, { + includeDiagnostics: true, + }) + + expect(result).toContain("file.ts") + expect(result).toContain("Test error") + expect(result).toContain("typescript") + }) + + it("should respect maxDiagnosticsCount", async () => { + const mockDocument = { + lineAt: jest.fn(() => ({ text: "const x = 1" })), + } + ;(vscode.workspace.openTextDocument as jest.Mock).mockResolvedValue(mockDocument) + ;(vscode.workspace.fs.stat as jest.Mock).mockResolvedValue({ type: vscode.FileType.File }) + + const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [ + [ + vscode.Uri.file("/test/workspace/file.ts"), + Array(10) + .fill(null) + .map( + (_, i) => + ({ + range: new vscode.Range(i, 0, i, 10), + message: `Test error ${i}`, + severity: vscode.DiagnosticSeverity.Error, + }) as vscode.Diagnostic, + ), + ], + ] + + const result = await diagnosticsToProblemsString(diagnostics, [vscode.DiagnosticSeverity.Error], mockCwd, { + includeDiagnostics: true, + maxDiagnosticsCount: 3, + }) + + expect(result).toContain("Test error 0") + expect(result).toContain("Test error 1") + expect(result).toContain("Test error 2") + expect(result).not.toContain("Test error 3") + expect(result).toContain("7 more diagnostics omitted") + }) + + it("should apply diagnosticsFilter correctly", async () => { + const mockDocument = { + lineAt: jest.fn(() => ({ text: "const x = 1" })), + } + ;(vscode.workspace.openTextDocument as jest.Mock).mockResolvedValue(mockDocument) + ;(vscode.workspace.fs.stat as jest.Mock).mockResolvedValue({ type: vscode.FileType.File }) + + const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [ + [ + vscode.Uri.file("/test/workspace/file.ts"), + [ + { + range: new vscode.Range(0, 0, 0, 10), + message: "ESLint error", + severity: vscode.DiagnosticSeverity.Error, + source: "eslint", + code: "no-unused-vars", + } as vscode.Diagnostic, + { + range: new vscode.Range(1, 0, 1, 10), + message: "TypeScript error", + severity: vscode.DiagnosticSeverity.Error, + source: "typescript", + code: "2322", + } as vscode.Diagnostic, + ], + ], + ] + + const result = await diagnosticsToProblemsString(diagnostics, [vscode.DiagnosticSeverity.Error], mockCwd, { + includeDiagnostics: true, + diagnosticsFilter: ["typescript 2322"], + }) + + expect(result).toContain("TypeScript error") + expect(result).not.toContain("ESLint error") + }) + + it("should handle file read errors gracefully", async () => { + ;(vscode.workspace.openTextDocument as jest.Mock).mockRejectedValue(new Error("File not found")) + ;(vscode.workspace.fs.stat as jest.Mock).mockResolvedValue({ type: vscode.FileType.File }) + + const diagnostics: [vscode.Uri, vscode.Diagnostic[]][] = [ + [ + vscode.Uri.file("/test/workspace/file.ts"), + [ + { + range: new vscode.Range(0, 0, 0, 10), + message: "Test error", + severity: vscode.DiagnosticSeverity.Error, + } as vscode.Diagnostic, + ], + ], + ] + + const result = await diagnosticsToProblemsString(diagnostics, [vscode.DiagnosticSeverity.Error], mockCwd, { + includeDiagnostics: true, + }) + + expect(result).toContain("file.ts") + expect(result).toContain("(unavailable)") + expect(result).toContain("Test error") + }) +}) diff --git a/src/integrations/diagnostics/index.ts b/src/integrations/diagnostics/index.ts index 0765f630a6..df4d139183 100644 --- a/src/integrations/diagnostics/index.ts +++ b/src/integrations/diagnostics/index.ts @@ -81,20 +81,15 @@ export async function diagnosticsToProblemsString( }, ): Promise { // Use provided options or fall back to VSCode configuration - const includeDiagnostics = - options?.includeDiagnostics ?? - vscode.workspace.getConfiguration("roo-cline").get("includeDiagnostics", false) + const config = vscode.workspace.getConfiguration("roo-cline") + const includeDiagnostics = options?.includeDiagnostics ?? config.get("includeDiagnostics", false) if (!includeDiagnostics) { return "" } - const maxDiagnosticsCount = - options?.maxDiagnosticsCount ?? - vscode.workspace.getConfiguration("roo-cline").get("maxDiagnosticsCount", 5) - const diagnosticsFilter = - options?.diagnosticsFilter ?? - vscode.workspace.getConfiguration("roo-cline").get("diagnosticsFilter", ["error", "warning"]) + const maxDiagnosticsCount = options?.maxDiagnosticsCount ?? config.get("maxDiagnosticsCount", 50) + const diagnosticsFilter = options?.diagnosticsFilter ?? config.get("diagnosticsFilter", []) const documents = new Map() const fileStats = new Map() @@ -112,10 +107,10 @@ export async function diagnosticsToProblemsString( const code = typeof d.code === "object" ? d.code.value : d.code const filterKey = source ? `${source} ${code || ""}`.trim() : `${code || ""}`.trim() - // Check if this diagnostic should be filtered out - return !diagnosticsFilter.some((filter) => { - // Support partial matching - return filterKey.includes(filter) || d.message.includes(filter) + // Check if this diagnostic matches any filter (exact match) + return diagnosticsFilter.some((filter) => { + // Exact matching for filter key + return filterKey === filter || (filter && filterKey.startsWith(filter + " ")) }) }) .sort((a, b) => a.range.start.line - b.range.start.line) diff --git a/src/package.json b/src/package.json index 77ce1a1240..8212f3da17 100644 --- a/src/package.json +++ b/src/package.json @@ -3,7 +3,7 @@ "displayName": "%extension.displayName%", "description": "%extension.description%", "publisher": "RooVeterinaryInc", - "version": "3.20.3", + "version": "3.20.4", "icon": "assets/icons/icon.png", "galleryBanner": { "color": "#617A91", @@ -347,7 +347,7 @@ }, "roo-cline.includeDiagnostics": { "type": "boolean", - "default": true, + "default": false, "description": "%settings.includeDiagnostics.description%" }, "roo-cline.maxDiagnosticsCount": { diff --git a/webview-ui/src/components/settings/DiagnosticsSettings.tsx b/webview-ui/src/components/settings/DiagnosticsSettings.tsx index bfe3f2173f..51ae94a65e 100644 --- a/webview-ui/src/components/settings/DiagnosticsSettings.tsx +++ b/webview-ui/src/components/settings/DiagnosticsSettings.tsx @@ -1,4 +1,4 @@ -import { HTMLAttributes } from "react" +import { HTMLAttributes, ChangeEvent } from "react" import { useAppTranslation } from "@/i18n/TranslationContext" import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react" import { AlertCircle } from "lucide-react" @@ -48,7 +48,9 @@ export const DiagnosticsSettings = ({
setCachedStateField("includeDiagnostics", e.target.checked)} + onChange={(e: ChangeEvent) => + setCachedStateField("includeDiagnostics", e.target.checked) + } data-testid="include-diagnostics-checkbox">