Skip to content

Commit 487e74b

Browse files
committed
fix: Ensure handleExternalSettingsChange only restarts service when manager is initialized
1 parent 47e2fd4 commit 487e74b

File tree

2 files changed

+119
-2
lines changed

2 files changed

+119
-2
lines changed
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import * as vscode from "vscode"
2+
import { CodeIndexManager } from "../manager"
3+
import { ContextProxy } from "../../../core/config/ContextProxy"
4+
5+
// Mock only the essential dependencies
6+
jest.mock("../../../utils/path", () => ({
7+
getWorkspacePath: jest.fn(() => "/test/workspace"),
8+
}))
9+
10+
jest.mock("../state-manager", () => ({
11+
CodeIndexStateManager: jest.fn().mockImplementation(() => ({
12+
onProgressUpdate: jest.fn(),
13+
getCurrentStatus: jest.fn(),
14+
dispose: jest.fn(),
15+
})),
16+
}))
17+
18+
describe("CodeIndexManager - handleExternalSettingsChange regression", () => {
19+
let mockContext: jest.Mocked<vscode.ExtensionContext>
20+
let manager: CodeIndexManager
21+
22+
beforeEach(() => {
23+
// Clear all instances before each test
24+
CodeIndexManager.disposeAll()
25+
26+
mockContext = {
27+
subscriptions: [],
28+
workspaceState: {} as any,
29+
globalState: {} as any,
30+
extensionUri: {} as any,
31+
extensionPath: "/test/extension",
32+
asAbsolutePath: jest.fn(),
33+
storageUri: {} as any,
34+
storagePath: "/test/storage",
35+
globalStorageUri: {} as any,
36+
globalStoragePath: "/test/global-storage",
37+
logUri: {} as any,
38+
logPath: "/test/log",
39+
extensionMode: vscode.ExtensionMode.Test,
40+
secrets: {} as any,
41+
environmentVariableCollection: {} as any,
42+
extension: {} as any,
43+
languageModelAccessInformation: {} as any,
44+
}
45+
46+
manager = CodeIndexManager.getInstance(mockContext)!
47+
})
48+
49+
afterEach(() => {
50+
CodeIndexManager.disposeAll()
51+
})
52+
53+
describe("handleExternalSettingsChange", () => {
54+
it("should not throw when called on uninitialized manager (regression test)", async () => {
55+
// This is the core regression test: handleExternalSettingsChange() should not throw
56+
// when called before the manager is initialized (during first-time configuration)
57+
58+
// Ensure manager is not initialized
59+
expect(manager.isInitialized).toBe(false)
60+
61+
// Mock a minimal config manager that simulates first-time configuration
62+
const mockConfigManager = {
63+
loadConfiguration: jest.fn().mockResolvedValue({ requiresRestart: true }),
64+
}
65+
;(manager as any)._configManager = mockConfigManager
66+
67+
// Mock the feature state to simulate valid configuration that would normally trigger restart
68+
jest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
69+
jest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
70+
71+
// The key test: this should NOT throw "CodeIndexManager not initialized" error
72+
await expect(manager.handleExternalSettingsChange()).resolves.not.toThrow()
73+
74+
// Verify that loadConfiguration was called (the method should still work)
75+
expect(mockConfigManager.loadConfiguration).toHaveBeenCalled()
76+
})
77+
78+
it("should work normally when manager is initialized", async () => {
79+
// Mock a minimal config manager
80+
const mockConfigManager = {
81+
loadConfiguration: jest.fn().mockResolvedValue({ requiresRestart: true }),
82+
}
83+
;(manager as any)._configManager = mockConfigManager
84+
85+
// Simulate an initialized manager by setting the required properties
86+
;(manager as any)._orchestrator = { stopWatcher: jest.fn() }
87+
;(manager as any)._searchService = {}
88+
;(manager as any)._cacheManager = {}
89+
90+
// Verify manager is considered initialized
91+
expect(manager.isInitialized).toBe(true)
92+
93+
// Mock the methods that would be called during restart
94+
const stopWatcherSpy = jest.spyOn(manager, "stopWatcher").mockImplementation()
95+
const startIndexingSpy = jest.spyOn(manager, "startIndexing").mockResolvedValue()
96+
97+
// Mock the feature state
98+
jest.spyOn(manager, "isFeatureEnabled", "get").mockReturnValue(true)
99+
jest.spyOn(manager, "isFeatureConfigured", "get").mockReturnValue(true)
100+
101+
await manager.handleExternalSettingsChange()
102+
103+
// Verify that the restart sequence was called
104+
expect(mockConfigManager.loadConfiguration).toHaveBeenCalled()
105+
expect(stopWatcherSpy).toHaveBeenCalled()
106+
expect(startIndexingSpy).toHaveBeenCalled()
107+
})
108+
109+
it("should handle case when config manager is not set", async () => {
110+
// Ensure config manager is not set (edge case)
111+
;(manager as any)._configManager = undefined
112+
113+
// This should not throw an error
114+
await expect(manager.handleExternalSettingsChange()).resolves.not.toThrow()
115+
})
116+
})
117+
})

src/services/code-index/manager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,8 @@ export class CodeIndexManager {
262262
const isFeatureEnabled = this.isFeatureEnabled
263263
const isFeatureConfigured = this.isFeatureConfigured
264264

265-
// If configuration changes require a restart, restart the service
266-
if (requiresRestart && isFeatureEnabled && isFeatureConfigured) {
265+
// If configuration changes require a restart and the manager is initialized, restart the service
266+
if (requiresRestart && isFeatureEnabled && isFeatureConfigured && this.isInitialized) {
267267
this.stopWatcher()
268268
await this.startIndexing()
269269
}

0 commit comments

Comments
 (0)