Skip to content

Commit 72fbd7a

Browse files
hannesrudolphdaniel-lxs
authored andcommitted
fix: ensure 'Include diagnostic messages in context' checkbox persists unchecked state
- Add missing includeDiagnosticMessages property to getStateToPostToWebview - Remove duplicate case statement in webviewMessageHandler - Add comprehensive unit tests for checkbox persistence behavior
1 parent d5457aa commit 72fbd7a

File tree

2 files changed

+182
-2
lines changed

2 files changed

+182
-2
lines changed

src/core/webview/ClineProvider.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,7 +1440,8 @@ export class ClineProvider
14401440
profileThresholds,
14411441
alwaysAllowFollowupQuestions,
14421442
followupAutoApproveTimeoutMs,
1443-
diagnosticsEnabled,
1443+
includeDiagnosticMessages,
1444+
maxDiagnosticMessages,
14441445
} = await this.getState()
14451446

14461447
const telemetryKey = process.env.POSTHOG_API_KEY
@@ -1561,7 +1562,8 @@ export class ClineProvider
15611562
hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
15621563
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
15631564
followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
1564-
diagnosticsEnabled: diagnosticsEnabled ?? true,
1565+
includeDiagnosticMessages: includeDiagnosticMessages ?? DEFAULT_INCLUDE_DIAGNOSTIC_MESSAGES,
1566+
maxDiagnosticMessages: maxDiagnosticMessages ?? DEFAULT_MAX_DIAGNOSTIC_MESSAGES,
15651567
}
15661568
}
15671569

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import { describe, it, expect, beforeEach, vi } from "vitest"
2+
import { ClineProvider } from "../ClineProvider"
3+
import { ContextProxy } from "../../config/ContextProxy"
4+
import {
5+
DEFAULT_INCLUDE_DIAGNOSTIC_MESSAGES,
6+
DEFAULT_MAX_DIAGNOSTIC_MESSAGES,
7+
} from "../../constants/diagnosticSettings"
8+
import { TelemetryService } from "@roo-code/telemetry"
9+
10+
// Mock fs/promises to avoid file system operations
11+
vi.mock("fs/promises", () => ({
12+
mkdir: vi.fn().mockResolvedValue(undefined),
13+
writeFile: vi.fn().mockResolvedValue(undefined),
14+
readFile: vi.fn().mockResolvedValue("{}"),
15+
unlink: vi.fn().mockResolvedValue(undefined),
16+
rmdir: vi.fn().mockResolvedValue(undefined),
17+
}))
18+
19+
// Mock WorkspaceTracker to avoid vscode.window dependencies
20+
vi.mock("../../../integrations/workspace/WorkspaceTracker", () => {
21+
return {
22+
default: vi.fn().mockImplementation(() => ({
23+
initializeFilePaths: vi.fn(),
24+
dispose: vi.fn(),
25+
})),
26+
}
27+
})
28+
29+
// Mock vscode module
30+
vi.mock("vscode", () => ({
31+
window: {
32+
tabGroups: {
33+
onDidChangeTabs: vi.fn(() => ({ dispose: vi.fn() })),
34+
},
35+
createTextEditorDecorationType: vi.fn(() => ({ dispose: vi.fn() })),
36+
showErrorMessage: vi.fn(),
37+
showInformationMessage: vi.fn(),
38+
showWarningMessage: vi.fn(),
39+
},
40+
workspace: {
41+
workspaceFolders: [],
42+
onDidChangeConfiguration: vi.fn(() => ({ dispose: vi.fn() })),
43+
onDidSaveTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
44+
onDidChangeTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
45+
onDidOpenTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
46+
onDidCloseTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
47+
getConfiguration: vi.fn(() => ({
48+
get: vi.fn(),
49+
update: vi.fn(),
50+
})),
51+
},
52+
Uri: {
53+
file: vi.fn(),
54+
joinPath: vi.fn(),
55+
},
56+
env: {
57+
uriScheme: "vscode",
58+
language: "en",
59+
appName: "Visual Studio Code",
60+
},
61+
ExtensionMode: {
62+
Production: 1,
63+
Development: 2,
64+
Test: 3,
65+
},
66+
version: "1.85.0",
67+
}))
68+
69+
describe("Diagnostic Settings Persistence", () => {
70+
let provider: ClineProvider
71+
let mockContextProxy: ContextProxy
72+
let mockContext: any
73+
let mockOutputChannel: any
74+
75+
beforeEach(() => {
76+
// Initialize TelemetryService if not already initialized
77+
if (!TelemetryService.hasInstance()) {
78+
TelemetryService.createInstance([])
79+
}
80+
81+
// Mock VSCode context
82+
mockContext = {
83+
extension: {
84+
packageJSON: {
85+
version: "1.0.0",
86+
},
87+
},
88+
globalState: {
89+
get: vi.fn(),
90+
update: vi.fn(),
91+
keys: vi.fn().mockReturnValue([]),
92+
},
93+
secrets: {
94+
get: vi.fn(),
95+
store: vi.fn(),
96+
delete: vi.fn(),
97+
},
98+
globalStorageUri: {
99+
fsPath: "/test/path",
100+
},
101+
}
102+
103+
// Mock output channel
104+
mockOutputChannel = {
105+
appendLine: vi.fn(),
106+
}
107+
108+
// Create mock context proxy
109+
mockContextProxy = new ContextProxy(mockContext)
110+
111+
// Create provider instance
112+
provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", mockContextProxy)
113+
})
114+
115+
describe("getState", () => {
116+
it("should return default values when includeDiagnosticMessages is not set", async () => {
117+
const state = await provider.getState()
118+
119+
expect(state.includeDiagnosticMessages).toBe(DEFAULT_INCLUDE_DIAGNOSTIC_MESSAGES)
120+
expect(state.maxDiagnosticMessages).toBe(DEFAULT_MAX_DIAGNOSTIC_MESSAGES)
121+
})
122+
123+
it("should return saved false value for includeDiagnosticMessages", async () => {
124+
// Set the value to false
125+
await mockContextProxy.setValue("includeDiagnosticMessages", false)
126+
127+
const state = await provider.getState()
128+
129+
expect(state.includeDiagnosticMessages).toBe(false)
130+
})
131+
132+
it("should return saved values for diagnostic settings", async () => {
133+
// Set custom values
134+
await mockContextProxy.setValue("includeDiagnosticMessages", false)
135+
await mockContextProxy.setValue("maxDiagnosticMessages", 25)
136+
137+
const state = await provider.getState()
138+
139+
expect(state.includeDiagnosticMessages).toBe(false)
140+
expect(state.maxDiagnosticMessages).toBe(25)
141+
})
142+
})
143+
144+
describe("getStateToPostToWebview", () => {
145+
it("should include diagnostic settings in webview state", async () => {
146+
// Set custom values
147+
await mockContextProxy.setValue("includeDiagnosticMessages", false)
148+
await mockContextProxy.setValue("maxDiagnosticMessages", 30)
149+
150+
const webviewState = await provider.getStateToPostToWebview()
151+
152+
// Verify the settings are included in the webview state
153+
expect(webviewState.includeDiagnosticMessages).toBe(false)
154+
expect(webviewState.maxDiagnosticMessages).toBe(30)
155+
})
156+
157+
it("should use default values when settings are not saved", async () => {
158+
const webviewState = await provider.getStateToPostToWebview()
159+
160+
// Verify defaults are used
161+
expect(webviewState.includeDiagnosticMessages).toBe(DEFAULT_INCLUDE_DIAGNOSTIC_MESSAGES)
162+
expect(webviewState.maxDiagnosticMessages).toBe(DEFAULT_MAX_DIAGNOSTIC_MESSAGES)
163+
})
164+
165+
it("should persist false value correctly", async () => {
166+
// This is the key test - ensure false values are not overridden by defaults
167+
await mockContextProxy.setValue("includeDiagnosticMessages", false)
168+
169+
// Get state multiple times to simulate navigation
170+
const state1 = await provider.getStateToPostToWebview()
171+
expect(state1.includeDiagnosticMessages).toBe(false)
172+
173+
// Simulate navigating away and back
174+
const state2 = await provider.getStateToPostToWebview()
175+
expect(state2.includeDiagnosticMessages).toBe(false)
176+
})
177+
})
178+
})

0 commit comments

Comments
 (0)