From 6600c82390dfdd4e8918d34132e5cc45d9ae7c56 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Mon, 28 Sep 2026 14:27:59 +0300 Subject: [PATCH] refactor(code-index): scope state ownership and workspace status delivery Centralize indexing status subscriptions in the extension scope and keep state ownership in workspace scopes. Honor the configured workspace root resolution when selecting the status source, with regression coverage for both selection strategies. --- src/__tests__/extension.spec.ts | 138 +++++++- .../CodebaseSearchTool.workspace.spec.ts | 3 + src/core/webview/ClineProvider.ts | 68 +--- .../webview/__tests__/ClineProvider.spec.ts | 9 + src/extension.ts | 10 + .../code-index-manager-registry.spec.ts | 22 +- .../__tests__/code-index-scope.spec.ts | 49 +++ .../code-index-status-manager.spec.ts | 311 ++++++++++++++++++ .../code-index-workspace-scope.spec.ts | 34 +- src/services/code-index/code-index-scope.ts | 29 ++ .../code-index/code-index-status-manager.ts | 79 +++++ .../code-index/code-index-workspace-scope.ts | 11 +- src/services/code-index/manager.ts | 11 +- 13 files changed, 698 insertions(+), 76 deletions(-) create mode 100644 src/services/code-index/__tests__/code-index-scope.spec.ts create mode 100644 src/services/code-index/__tests__/code-index-status-manager.spec.ts create mode 100644 src/services/code-index/code-index-scope.ts create mode 100644 src/services/code-index/code-index-status-manager.ts diff --git a/src/__tests__/extension.spec.ts b/src/__tests__/extension.spec.ts index 56ccd52588..d0a8b274b8 100644 --- a/src/__tests__/extension.spec.ts +++ b/src/__tests__/extension.spec.ts @@ -12,7 +12,7 @@ vi.mock("vscode", () => ({ tabGroups: { onDidChangeTabs: vi.fn(), }, - onDidChangeActiveTextEditor: vi.fn(), + onDidChangeActiveTextEditor: vi.fn().mockReturnValue({ dispose: vi.fn() }), }, workspace: { registerTextDocumentContentProvider: vi.fn(), @@ -205,6 +205,7 @@ vi.mock("../core/webview/ClineProvider", async () => { { // Static method used by extension.ts getVisibleInstance: vi.fn().mockReturnValue(mockInstance), + getAllInstances: vi.fn().mockReturnValue([]), sideBarId: "zoo-code.SidebarProvider", }, ), @@ -238,6 +239,141 @@ describe("extension.ts", () => { settingsUpdatedHandler = undefined }) + test("initializes the code index scope and registers it for extension cleanup", async () => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const init = vi.spyOn(CodeIndexScope.prototype, "init") + const dispose = vi.spyOn(CodeIndexScope.prototype, "dispose") + try { + const { activate } = await import("../extension") + await activate(mockContext) + + const scopes = mockContext.subscriptions.filter((entry) => entry instanceof CodeIndexScope) + expect(scopes).toHaveLength(1) + expect(init).toHaveBeenCalledExactlyOnceWith() + expect(init.mock.contexts[0]).toBe(scopes[0]) + expect(dispose).not.toHaveBeenCalled() + scopes[0].dispose() + expect(dispose).toHaveBeenCalledExactlyOnceWith() + } finally { + init.mockRestore() + dispose.mockRestore() + } + }) + + test("publishes indexing status to matching providers and logs delivery failures", async () => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + const provider = ClineProvider.getVisibleInstance()! + Object.defineProperty(provider, "workspacePath", { configurable: true, value: "/workspace" }) + const getAll = vi.mocked(ClineProvider.getAllInstances) + const post = vi.mocked(provider.postMessageToWebview) + getAll.mockReturnValue([provider, provider]) + post.mockRejectedValueOnce(new Error("delivery failed")).mockResolvedValue(undefined) + try { + const { activate } = await import("../extension") + await activate(mockContext) + const scope = mockContext.subscriptions.find((entry) => entry instanceof CodeIndexScope)! + const status = { + systemStatus: "Standby" as const, + message: "Ready", + processedItems: 0, + totalItems: 0, + currentItemUnit: "blocks", + workspacePath: "/workspace", + workspaceEnabled: true, + autoEnableDefault: true, + } + scope["statusManager"]!["publishStatus"](status) + await Promise.resolve() + expect(post).toHaveBeenCalledTimes(2) + expect(post).toHaveBeenCalledWith({ type: "indexingStatusUpdate", values: status }) + expect(log).toHaveBeenCalledWith( + "[CodeIndexStatusManager] Failed to publish indexing status:", + expect.objectContaining({ message: "delivery failed" }), + ) + scope.dispose() + } finally { + log.mockRestore() + getAll.mockReturnValue([]) + post.mockReset() + Reflect.deleteProperty(provider, "workspacePath") + } + }) + + test.each([ + { workspacePath: "/other-workspace", receivesStatus: false }, + { workspacePath: "/workspace", receivesStatus: true }, + { workspacePath: undefined, receivesStatus: true }, + { workspacePath: "", receivesStatus: true }, + ])( + "routes status for provider workspace=$workspacePath with receivesStatus=$receivesStatus", + async ({ workspacePath, receivesStatus }) => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const { ClineProvider } = await import("../core/webview/ClineProvider") + const provider = ClineProvider.getVisibleInstance()! + Object.defineProperty(provider, "workspacePath", { configurable: true, value: workspacePath }) + const getAll = vi.mocked(ClineProvider.getAllInstances) + getAll.mockReturnValue([provider]) + try { + const { activate } = await import("../extension") + await activate(mockContext) + const scope = mockContext.subscriptions.find((entry) => entry instanceof CodeIndexScope)! + const status = { + systemStatus: "Indexing" as const, + message: "Processing confidential.ts", + processedItems: 1, + totalItems: 2, + currentItemUnit: "files", + workspacePath: "/workspace", + workspaceEnabled: true, + autoEnableDefault: true, + } + scope["statusManager"]!["publishStatus"](status) + await Promise.resolve() + if (receivesStatus) { + expect(provider.postMessageToWebview).toHaveBeenCalledExactlyOnceWith({ + type: "indexingStatusUpdate", + values: status, + }) + } else { + expect(provider.postMessageToWebview).not.toHaveBeenCalled() + } + scope.dispose() + } finally { + getAll.mockReturnValue([]) + Reflect.deleteProperty(provider, "workspacePath") + } + }, + ) + + test.each([new Error("scope initialization failed"), "scope initialization failed"])( + "continues activation and logs code index scope initialization failure: %s", + async (error) => { + vi.resetModules() + const { CodeIndexScope } = await import("../services/code-index/code-index-scope") + const init = vi.spyOn(CodeIndexScope.prototype, "init").mockImplementationOnce(() => { + throw error + }) + try { + const { activate } = await import("../extension") + await expect(activate(mockContext)).resolves.toBeDefined() + + const vscode = await import("vscode") + const channel = vi.mocked(vscode.window.createOutputChannel).mock.results.at(-1)?.value + expect(channel?.appendLine).toHaveBeenCalledWith( + "[CodeIndexScope] Failed to initialize: scope initialization failed", + ) + expect(mockContext.subscriptions.filter((entry) => entry instanceof CodeIndexScope)).toHaveLength(1) + } finally { + init.mockRestore() + } + }, + ) + test("does not call dotenv.config when optional .env does not exist", async () => { vi.resetModules() vi.clearAllMocks() diff --git a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts index 03789f7d40..f2c4cc2aed 100644 --- a/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts +++ b/src/core/tools/__tests__/CodebaseSearchTool.workspace.spec.ts @@ -6,6 +6,7 @@ import type { ToolCallbacks } from "../BaseTool" import { CodebaseSearchTool } from "../CodebaseSearchTool" import { CodeIndexManagerRegistry } from "../../../services/code-index/code-index-manager-registry" import { CodeIndexManager } from "../../../services/code-index/manager" +import { CodeIndexStateManager } from "../../../services/code-index/state-manager" import { getWorkspacePath } from "../../../utils/path" import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode" @@ -15,6 +16,7 @@ vi.mock("vscode", () => ({ Uri: { file: vi.fn() }, })) vi.mock("../../../utils/path", () => ({ getWorkspacePath: vi.fn() })) +vi.mock("../../../services/code-index/state-manager") vi.mock("../../../services/code-index/manager", () => ({ CodeIndexManager: vi.fn().mockImplementation(function (workspacePath: string) { let initialized = false @@ -196,6 +198,7 @@ describe("CodebaseSearchTool workspace selection", () => { "/external-task", expect.objectContaining({ fsPath: "/external-task" }), provider.context, + expect.any(CodeIndexStateManager), ) expect(vscode.Uri.file).toHaveBeenCalledExactlyOnceWith("/external-task") const manager = CodeIndexManagerRegistry.getOrCreate(provider.context, "/external-task")! diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 185cfbb427..6fac386383 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -92,7 +92,6 @@ import { MarketplaceManager } from "../../services/marketplace" import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService" import type { CodeIndexManager } from "../../services/code-index/manager" import { CodeIndexManagerRegistry } from "../../services/code-index/code-index-manager-registry" -import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" import { MdmService } from "../../services/mdm/MdmService" import { SkillsManager } from "../../services/skills/SkillsManager" @@ -212,8 +211,6 @@ export class ClineProvider private taskScheduler = new TaskScheduler() private static readonly delegationTransitionLocks = new Map>() private cancelledDelegationChildIds = new Set() - private codeIndexStatusSubscription?: vscode.Disposable - private codeIndexManager?: CodeIndexManager private _workspaceTracker?: WorkspaceTracker // workSpaceTracker read-only for access outside this class protected mcpHub?: McpHub // Change from private to protected protected skillsManager?: SkillsManager @@ -1045,17 +1042,6 @@ export class ClineProvider // and executes code based on the message that is received. this.setWebviewMessageListener(webviewView.webview) - // Initialize code index status subscription for the current workspace. - this.updateCodeIndexStatusSubscription() - - // Listen for active editor changes to update code index status for the - // current workspace. - const activeEditorSubscription = vscode.window.onDidChangeActiveTextEditor(() => { - // Update subscription when workspace might have changed. - this.updateCodeIndexStatusSubscription() - }) - this.webviewDisposables.push(activeEditorSubscription) - // Listen for when the panel becomes visible. // https://github.com/microsoft/vscode-discussions/discussions/840 if ("onDidChangeViewState" in webviewView) { @@ -1093,8 +1079,6 @@ export class ClineProvider } else { this.log("Clearing webview resources for sidebar view") this.clearWebviewResources() - // Reset current workspace manager reference when view is disposed - this.codeIndexManager = undefined } }, null, @@ -3243,53 +3227,6 @@ export class ClineProvider return CodeIndexManagerRegistry.getOrCreate(this.context) } - /** - * Updates the code index status subscription to listen to the current workspace manager - */ - private updateCodeIndexStatusSubscription(): void { - // Get the current workspace manager - const currentManager = this.getCurrentWorkspaceCodeIndexManager() - - // If the manager hasn't changed, no need to update subscription - if (currentManager === this.codeIndexManager) { - return - } - - // Dispose the old subscription if it exists - if (this.codeIndexStatusSubscription) { - this.codeIndexStatusSubscription.dispose() - this.codeIndexStatusSubscription = undefined - } - - // Update the current workspace manager reference - this.codeIndexManager = currentManager - - // Subscribe to the new manager's progress updates if it exists - if (currentManager) { - this.codeIndexStatusSubscription = currentManager.onProgressUpdate((update: IndexProgressUpdate) => { - // Only send updates if this manager is still the current one - if (currentManager === this.getCurrentWorkspaceCodeIndexManager()) { - // Get the full status from the manager to ensure we have all fields correctly formatted - const fullStatus = currentManager.getCurrentStatus() - void this.postMessageToWebview({ - type: "indexingStatusUpdate", - values: fullStatus, - }) - } - }) - - if (this.view) { - this.webviewDisposables.push(this.codeIndexStatusSubscription) - } - - // Send initial status for the current workspace - void this.postMessageToWebview({ - type: "indexingStatusUpdate", - values: currentManager.getCurrentStatus(), - }) - } - } - /** * TaskProviderLike, TelemetryPropertiesProvider */ @@ -3777,6 +3714,11 @@ export class ClineProvider } } + /** Workspace explicitly associated with this provider, without an active-editor fallback. */ + public get workspacePath(): string | undefined { + return this.currentWorkspacePath + } + public get cwd() { return this.currentWorkspacePath || getWorkspacePath() } diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 72aa1bcb76..feef95d870 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -561,6 +561,15 @@ describe("ClineProvider", () => { }) }) + test("exposes only the explicitly associated workspace without a fallback", () => { + provider["currentWorkspacePath"] = "/workspace-a" + expect(provider.workspacePath).toBe("/workspace-a") + provider["currentWorkspacePath"] = "/workspace-b" + expect(provider.workspacePath).toBe("/workspace-b") + provider["currentWorkspacePath"] = undefined + expect(provider.workspacePath).toBeUndefined() + }) + test("constructor initializes correctly", () => { expect(provider).toBeInstanceOf(ClineProvider) // Since getVisibleInstance returns the last instance where view.visible is true diff --git a/src/extension.ts b/src/extension.ts index 8706de765b..1d71f834df 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -35,6 +35,7 @@ import { openAiCodexOAuthManager } from "./integrations/openai-codex/oauth" import { kimiCodeOAuthManager } from "./integrations/kimi-code/oauth" import { McpServerManager } from "./services/mcp/McpServerManager" import { CodeIndexManagerRegistry } from "./services/code-index/code-index-manager-registry" +import { CodeIndexScope } from "./services/code-index/code-index-scope" import { MdmService } from "./services/mdm/MdmService" import { migrateSettings } from "./utils/migrateSettings" import { autoImportSettings } from "./utils/autoImportSettings" @@ -195,6 +196,15 @@ export async function activate(context: vscode.ExtensionContext) { }), ) + const codeIndexScope = new CodeIndexScope(context) + context.subscriptions.push(codeIndexScope) + try { + codeIndexScope.init() + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + outputChannel.appendLine(`[CodeIndexScope] Failed to initialize: ${message}`) + } + // Initialize code index managers for all workspace folders. if (vscode.workspace.workspaceFolders) { for (const folder of vscode.workspace.workspaceFolders) { diff --git a/src/services/code-index/__tests__/code-index-manager-registry.spec.ts b/src/services/code-index/__tests__/code-index-manager-registry.spec.ts index 28ecb5d6f0..bfbf58da8f 100644 --- a/src/services/code-index/__tests__/code-index-manager-registry.spec.ts +++ b/src/services/code-index/__tests__/code-index-manager-registry.spec.ts @@ -3,6 +3,9 @@ import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from import { CodeIndexManager } from "../manager" import { CodeIndexManagerRegistry } from "../code-index-manager-registry" import { CodeIndexWorkspaceScope } from "../code-index-workspace-scope" +import { CodeIndexStateManager } from "../state-manager" + +vi.mock("../state-manager") vi.mock("vscode", () => ({ workspace: { workspaceFolders: undefined, getWorkspaceFolder: vi.fn() }, @@ -45,7 +48,7 @@ describe("CodeIndexManagerRegistry", () => { it("uses the first workspace when there is no active editor", () => { CodeIndexManagerRegistry.getOrCreate(context) - expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context, expect.any(CodeIndexStateManager)) }) it("prefers the active editor's workspace", () => { @@ -53,20 +56,20 @@ describe("CodeIndexManagerRegistry", () => { Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: editor }) vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(second) expect(CodeIndexManagerRegistry.getOrCreate(context)).toBeDefined() - expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context, expect.any(CodeIndexStateManager)) }) it("falls back to the first workspace for an editor outside all folders", () => { Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: makeTextEditor() }) CodeIndexManagerRegistry.getOrCreate(context) - expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith("/first", first.uri, context, expect.any(CodeIndexStateManager)) }) it("gives an explicit path priority over the active editor", () => { Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: makeTextEditor() }) vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(first) expect(CodeIndexManagerRegistry.getOrCreate(context, "/second")).toBeDefined() - expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith("/second", second.uri, context, expect.any(CodeIndexStateManager)) }) it("preserves the actual remote workspace URI", () => { @@ -76,7 +79,7 @@ describe("CodeIndexManagerRegistry", () => { value: [{ uri, name: "remote", index: 0 }], }) CodeIndexManagerRegistry.getOrCreate(context, "/remote") - expect(CodeIndexManager).toHaveBeenCalledWith("/remote", uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith("/remote", uri, context, expect.any(CodeIndexStateManager)) expect(vi.mocked(CodeIndexManager).mock.calls[0][1]).toBe(uri) expect(vscode.Uri.file).not.toHaveBeenCalled() }) @@ -87,7 +90,7 @@ describe("CodeIndexManagerRegistry", () => { vi.mocked(vscode.Uri.file).mockReturnValue(uri) CodeIndexManagerRegistry.getOrCreate(context, uri.fsPath) expect(vscode.Uri.file).toHaveBeenCalledWith(uri.fsPath) - expect(CodeIndexManager).toHaveBeenCalledWith(uri.fsPath, uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith(uri.fsPath, uri, context, expect.any(CodeIndexStateManager)) }) it("constructs a file URI for an explicit path not matching any open workspace folder", () => { @@ -96,7 +99,12 @@ describe("CodeIndexManagerRegistry", () => { vi.mocked(vscode.Uri.file).mockReturnValue(uri) CodeIndexManagerRegistry.getOrCreate(context, "/outside/project") expect(vscode.Uri.file).toHaveBeenCalledWith("/outside/project") - expect(CodeIndexManager).toHaveBeenCalledWith("/outside/project", uri, context) + expect(CodeIndexManager).toHaveBeenCalledWith( + "/outside/project", + uri, + context, + expect.any(CodeIndexStateManager), + ) }) it("reuses the same path and keeps different paths isolated", () => { diff --git a/src/services/code-index/__tests__/code-index-scope.spec.ts b/src/services/code-index/__tests__/code-index-scope.spec.ts new file mode 100644 index 0000000000..2a5692c817 --- /dev/null +++ b/src/services/code-index/__tests__/code-index-scope.spec.ts @@ -0,0 +1,49 @@ +import { makeExtensionContext } from "../../../test-utils/vscode" +import { CodeIndexScope } from "../code-index-scope" +import { CodeIndexStatusManager } from "../code-index-status-manager" +import { CodeIndexManagerRegistry } from "../code-index-manager-registry" + +vi.mock("../code-index-status-manager") +vi.mock("../code-index-manager-registry", () => ({ + CodeIndexManagerRegistry: { getOrCreate: vi.fn() }, +})) + +describe("CodeIndexScope", () => { + beforeEach(() => vi.clearAllMocks()) + + it("creates, initializes and disposes its status manager", () => { + const context = makeExtensionContext() + const scope = new CodeIndexScope(context) + expect(scope["_isInitialized"]).toBe(false) + expect(CodeIndexStatusManager).not.toHaveBeenCalled() + scope.init() + expect(scope["_isInitialized"]).toBe(true) + const manager = vi.mocked(CodeIndexStatusManager).mock.instances[0] + const [resolve] = vi.mocked(CodeIndexStatusManager).mock.calls[0] + resolve("/first") + expect(CodeIndexManagerRegistry.getOrCreate).toHaveBeenCalledExactlyOnceWith(context, "/first") + expect(manager.init).toHaveBeenCalledExactlyOnceWith() + expect(() => scope.init()).toThrow("already initialized") + scope.dispose() + expect(scope["_isInitialized"]).toBe(false) + scope.dispose() + expect(manager.dispose).toHaveBeenCalledExactlyOnceWith() + scope.init() + expect(CodeIndexStatusManager).toHaveBeenCalledTimes(2) + scope.dispose() + }) + + it("can dispose before initialization and retry failed initialization", () => { + const scope = new CodeIndexScope(makeExtensionContext()) + scope.dispose() + expect(CodeIndexStatusManager).not.toHaveBeenCalled() + vi.mocked(CodeIndexStatusManager.prototype.init).mockImplementationOnce(() => { + throw new Error("init failed") + }) + expect(() => scope.init()).toThrow("init failed") + expect(scope["_isInitialized"]).toBe(false) + expect(() => scope.init()).not.toThrow() + expect(scope["_isInitialized"]).toBe(true) + scope.dispose() + }) +}) diff --git a/src/services/code-index/__tests__/code-index-status-manager.spec.ts b/src/services/code-index/__tests__/code-index-status-manager.spec.ts new file mode 100644 index 0000000000..c73fb4f03b --- /dev/null +++ b/src/services/code-index/__tests__/code-index-status-manager.spec.ts @@ -0,0 +1,311 @@ +import * as vscode from "vscode" +import { + makeUri, + makeTextDocument, + makeTextEditor, + makeExtensionContext, + makeWorkspaceConfiguration, +} from "../../../test-utils/vscode" +import { CodeIndexManager } from "../manager" +import { CodeIndexStateManager } from "../state-manager" +import { CodeIndexStatusManager, type CodeIndexStatus } from "../code-index-status-manager" + +// Reload the real workspace resolver against this suite's VS Code mock, +// rather than the VS Code instance cached by the global test setup. +vi.hoisted(() => vi.resetModules()) + +vi.mock("../../../core/webview/ClineProvider", () => ({ ClineProvider: { getAllInstances: vi.fn(() => []) } })) +vi.mock("../manager") +vi.mock("../state-manager") + +vi.mock("vscode", () => ({ + window: { onDidChangeActiveTextEditor: vi.fn(), activeTextEditor: undefined }, + workspace: { workspaceFolders: undefined, getWorkspaceFolder: vi.fn(), getConfiguration: vi.fn() }, +})) + +function makeSource(workspacePath: string) { + const status: CodeIndexStatus = { + systemStatus: "Standby", + message: "Ready", + processedItems: 0, + totalItems: 0, + currentItemUnit: "blocks", + workspacePath, + workspaceEnabled: true, + autoEnableDefault: true, + } + let emit = () => {} + const subscription = { dispose: vi.fn() } + const event: CodeIndexManager["onProgressUpdate"] = (listener) => { + emit = () => listener(status) + return subscription + } + const manager = new CodeIndexManager( + workspacePath, + makeUri(workspacePath), + makeExtensionContext(), + new CodeIndexStateManager(), + ) + Object.defineProperty(manager, "onProgressUpdate", { value: vi.fn(event), configurable: true }) + return Object.assign(manager, { + getCurrentStatus: vi.fn(() => status), + dispose: vi.fn(), + subscription, + emit: () => emit(), + status, + }) +} + +describe("CodeIndexStatusManager", () => { + let editorChanged: () => void + let disposeEditor: ReturnType void>> + + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue(makeWorkspaceConfiguration()) + Object.defineProperty(vscode.window, "activeTextEditor", { configurable: true, value: undefined }) + Object.defineProperty(vscode.workspace, "workspaceFolders", { + configurable: true, + value: [{ uri: makeUri("/first"), name: "first", index: 0 }], + }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(undefined) + disposeEditor = vi.fn() + vi.mocked(vscode.window.onDidChangeActiveTextEditor).mockImplementation((listener) => { + editorChanged = () => listener(undefined) + return { dispose: disposeEditor } + }) + }) + + it("resolves the workspace before requesting its manager", () => { + const resolve = vi.fn(() => makeSource("/first")) + const manager = new CodeIndexStatusManager(resolve) + manager.init() + expect(resolve).toHaveBeenLastCalledWith("/first") + const uri = makeUri("/second/file.ts") + Object.defineProperty(vscode.window, "activeTextEditor", { + configurable: true, + value: makeTextEditor({ document: makeTextDocument({ uri }) }), + }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue({ + uri: makeUri("/second"), + name: "second", + index: 1, + }) + editorChanged() + expect(vscode.workspace.getWorkspaceFolder).toHaveBeenCalledWith(uri) + expect(resolve).toHaveBeenLastCalledWith("/second") + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(undefined) + editorChanged() + expect(resolve).toHaveBeenLastCalledWith("/first") + manager.dispose() + }) + + it.each([ + ["firstFolder", "/first"], + ["activeEditor", "/second"], + ])("publishes status for the configured %s workspace", (strategy, expectedPath) => { + vi.mocked(vscode.workspace.getConfiguration).mockReturnValue( + makeWorkspaceConfiguration({ "workspace.rootResolution": strategy }), + ) + const firstFolder = { uri: makeUri("/first"), name: "first", index: 0 } + const secondFolder = { uri: makeUri("/second"), name: "second", index: 1 } + Object.defineProperty(vscode.workspace, "workspaceFolders", { + configurable: true, + value: [firstFolder, secondFolder], + }) + Object.defineProperty(vscode.window, "activeTextEditor", { + configurable: true, + value: makeTextEditor({ document: makeTextDocument({ uri: makeUri("/second/file.ts") }) }), + }) + vi.mocked(vscode.workspace.getWorkspaceFolder).mockReturnValue(secondFolder) + const first = makeSource("/first") + const second = makeSource("/second") + const resolve = vi.fn((path: string) => (path === "/first" ? first : second)) + const publish = vi.fn() + const manager = new CodeIndexStatusManager(resolve) + manager["publishStatus"] = publish + try { + manager.init() + expect(resolve).toHaveBeenLastCalledWith(expectedPath) + const selected = expectedPath === "/first" ? first : second + const other = selected === first ? second : first + expect(publish).toHaveBeenCalledExactlyOnceWith(selected.status) + expect(other.onProgressUpdate).not.toHaveBeenCalled() + editorChanged() + selected.status.message = "Updated" + selected.emit() + expect(publish).toHaveBeenLastCalledWith(selected.status) + expect(publish).toHaveBeenCalledTimes(2) + expect(selected.onProgressUpdate).toHaveBeenCalledTimes(1) + } finally { + manager.dispose() + } + }) + + it.each([undefined, []])("does not request a manager without a workspace: %s", (folders) => { + Object.defineProperty(vscode.workspace, "workspaceFolders", { configurable: true, value: folders }) + const resolve = vi.fn() + const publish = vi.fn() + const manager = new CodeIndexStatusManager(resolve) + manager["publishStatus"] = publish + manager.init() + editorChanged() + expect(resolve).not.toHaveBeenCalled() + expect(publish).not.toHaveBeenCalled() + manager.dispose() + }) + + it("publishes initial and full updated status without resubscribing in the same workspace", () => { + const source = makeSource("/first") + const publish = vi.fn() + const manager = new CodeIndexStatusManager(() => source) + manager["publishStatus"] = publish + expect(source.onProgressUpdate).not.toHaveBeenCalled() + manager.init() + expect(publish).toHaveBeenCalledExactlyOnceWith(source.status) + source.status.message = "Updated" + source.emit() + expect(publish).toHaveBeenLastCalledWith(source.status) + expect(publish).toHaveBeenCalledTimes(2) + editorChanged() + expect(source.onProgressUpdate).toHaveBeenCalledTimes(1) + expect(publish).toHaveBeenCalledTimes(2) + manager.dispose() + }) + + it("switches subscriptions and rejects stale events before and after switching", () => { + const first = makeSource("/first") + const second = makeSource("/second") + let current = first + const publish = vi.fn() + const manager = new CodeIndexStatusManager(() => current) + manager["publishStatus"] = publish + manager.init() + current = second + first.emit() + expect(publish).toHaveBeenCalledTimes(1) + editorChanged() + expect(first.subscription.dispose).toHaveBeenCalledTimes(1) + expect(publish).toHaveBeenLastCalledWith(second.status) + first.emit() + expect(publish).toHaveBeenCalledTimes(2) + second.emit() + expect(publish).toHaveBeenCalledTimes(3) + manager.dispose() + expect(first.dispose).not.toHaveBeenCalled() + expect(second.dispose).not.toHaveBeenCalled() + }) + + it("handles missing workspaces and releases the subscription when a workspace disappears", () => { + const source = makeSource("/first") + let current: typeof source | undefined + const publish = vi.fn() + const manager = new CodeIndexStatusManager(() => current) + manager["publishStatus"] = publish + manager.init() + expect(publish).not.toHaveBeenCalled() + current = source + editorChanged() + expect(publish).toHaveBeenCalledExactlyOnceWith(source.status) + current = undefined + editorChanged() + expect(source.subscription.dispose).toHaveBeenCalledTimes(1) + source.emit() + expect(publish).toHaveBeenCalledTimes(1) + manager.dispose() + }) + + it("disposes idempotently, ignores stale progress and supports reinitialization", () => { + const source = makeSource("/first") + const publish = vi.fn() + const manager = new CodeIndexStatusManager(() => source) + manager["publishStatus"] = publish + manager.dispose() + manager.init() + manager.dispose() + manager.dispose() + expect(disposeEditor).toHaveBeenCalledTimes(1) + expect(source.subscription.dispose).toHaveBeenCalledTimes(1) + source.emit() + expect(publish).toHaveBeenCalledTimes(1) + manager.init() + expect(publish).toHaveBeenCalledTimes(2) + manager.dispose() + }) + + it("logs both unsubscribe failures and keeps disposal idempotent", () => { + const source = makeSource("/first") + const manager = new CodeIndexStatusManager(() => source) + const editorError = new Error("editor cleanup failed") + const progressError = new Error("progress cleanup failed") + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + try { + manager.init() + disposeEditor.mockImplementationOnce(() => { + throw editorError + }) + source.subscription.dispose.mockImplementationOnce(() => { + throw progressError + }) + expect(() => manager.dispose()).not.toThrow() + expect(log).toHaveBeenNthCalledWith( + 1, + "[CodeIndexStatusManager] Failed to dispose active editor subscription:", + editorError, + ) + expect(log).toHaveBeenNthCalledWith( + 2, + "[CodeIndexStatusManager] Failed to dispose progress subscription:", + progressError, + ) + manager.dispose() + expect(disposeEditor).toHaveBeenCalledTimes(1) + expect(source.subscription.dispose).toHaveBeenCalledTimes(1) + expect(log).toHaveBeenCalledTimes(2) + } finally { + log.mockRestore() + } + }) + + it("logs an unsubscribe failure and still subscribes to the next workspace", () => { + const first = makeSource("/first") + const second = makeSource("/second") + let current = first + const publish = vi.fn() + const manager = new CodeIndexStatusManager(() => current) + manager["publishStatus"] = publish + const log = vi.spyOn(console, "error").mockImplementation(() => {}) + try { + manager.init() + first.subscription.dispose.mockImplementationOnce(() => { + throw "cleanup failed" + }) + current = second + expect(() => editorChanged()).not.toThrow() + expect(log).toHaveBeenCalledExactlyOnceWith( + "[CodeIndexStatusManager] Failed to dispose progress subscription:", + "cleanup failed", + ) + expect(second.onProgressUpdate).toHaveBeenCalledTimes(1) + expect(publish).toHaveBeenLastCalledWith(second.status) + first.emit() + expect(publish).toHaveBeenCalledTimes(2) + } finally { + manager.dispose() + log.mockRestore() + } + }) + + it("propagates initialization failure without automatic cleanup", () => { + const source = makeSource("/first") + const publish = vi.fn().mockImplementationOnce(() => { + throw new Error("publish failed") + }) + const manager = new CodeIndexStatusManager(() => source) + manager["publishStatus"] = publish + expect(() => manager.init()).toThrow("publish failed") + expect(disposeEditor).not.toHaveBeenCalled() + expect(source.subscription.dispose).not.toHaveBeenCalled() + manager.dispose() + }) +}) diff --git a/src/services/code-index/__tests__/code-index-workspace-scope.spec.ts b/src/services/code-index/__tests__/code-index-workspace-scope.spec.ts index 2e20f07799..0379df7feb 100644 --- a/src/services/code-index/__tests__/code-index-workspace-scope.spec.ts +++ b/src/services/code-index/__tests__/code-index-workspace-scope.spec.ts @@ -1,6 +1,9 @@ import { makeExtensionContext, makeUri } from "../../../test-utils/vscode" import { CodeIndexManager } from "../manager" import { CodeIndexWorkspaceScope } from "../code-index-workspace-scope" +import { CodeIndexStateManager } from "../state-manager" + +vi.mock("../state-manager") vi.mock("../manager", () => ({ CodeIndexManager: vi.fn().mockImplementation(function () { @@ -27,11 +30,20 @@ describe("CodeIndexWorkspaceScope", () => { const scope = new CodeIndexWorkspaceScope(uri.fsPath, uri, context) expect(CodeIndexManager).not.toHaveBeenCalled() + expect(CodeIndexStateManager).not.toHaveBeenCalled() + expect(scope["_stateManager"]).toBeUndefined() expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") expect(scope.init()).toBeUndefined() const manager = scope.codeIndexManager - expect(CodeIndexManager).toHaveBeenCalledExactlyOnceWith(uri.fsPath, uri, context) + expect(CodeIndexStateManager).toHaveBeenCalledExactlyOnceWith() + expect(scope["_stateManager"]).toBe(vi.mocked(CodeIndexStateManager).mock.instances[0]) + expect(CodeIndexManager).toHaveBeenCalledExactlyOnceWith( + uri.fsPath, + uri, + context, + vi.mocked(CodeIndexStateManager).mock.instances[0], + ) expect(scope.codeIndexManager).toBe(manager) expect(() => scope.init()).toThrow("Code index workspace scope is already initialized") expect(scope.codeIndexManager).toBe(manager) @@ -45,6 +57,7 @@ describe("CodeIndexWorkspaceScope", () => { const manager = scope.codeIndexManager expect(scope.dispose()).toBeUndefined() + expect(scope["_stateManager"]).toBeUndefined() expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") scope.dispose() expect(manager.dispose).toHaveBeenCalledExactlyOnceWith() @@ -68,6 +81,24 @@ describe("CodeIndexWorkspaceScope", () => { expect(scope.codeIndexManager).toBe(current) expect(CodeIndexManager).toHaveBeenCalledTimes(2) expect(current.dispose).not.toHaveBeenCalled() + const calls = vi.mocked(CodeIndexManager).mock.calls + expect(CodeIndexStateManager).toHaveBeenCalledTimes(2) + expect(calls[1][3]).not.toBe(calls[0][3]) + expect(scope["_stateManager"]).toBe(calls[1][3]) + }) + + it("creates separate state managers for different workspace scopes", () => { + const context = makeExtensionContext() + const first = new CodeIndexWorkspaceScope("/first", makeUri("/first"), context) + const second = new CodeIndexWorkspaceScope("/second", makeUri("/second"), context) + first.init() + second.init() + + const calls = vi.mocked(CodeIndexManager).mock.calls + expect(CodeIndexStateManager).toHaveBeenCalledTimes(2) + expect(calls[0][3]).toBe(vi.mocked(CodeIndexStateManager).mock.instances[0]) + expect(calls[1][3]).toBe(vi.mocked(CodeIndexStateManager).mock.instances[1]) + expect(calls[1][3]).not.toBe(calls[0][3]) }) it("clears its reference even when manager disposal throws", () => { @@ -79,6 +110,7 @@ describe("CodeIndexWorkspaceScope", () => { throw error }) expect(() => scope.dispose()).toThrow(error) + expect(scope["_stateManager"]).toBeUndefined() expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") scope.dispose() expect(manager.dispose).toHaveBeenCalledTimes(1) diff --git a/src/services/code-index/code-index-scope.ts b/src/services/code-index/code-index-scope.ts new file mode 100644 index 0000000000..8b6b36c2cd --- /dev/null +++ b/src/services/code-index/code-index-scope.ts @@ -0,0 +1,29 @@ +import type * as vscode from "vscode" +import { CodeIndexManagerRegistry } from "./code-index-manager-registry" +import { CodeIndexStatusManager } from "./code-index-status-manager" + +export class CodeIndexScope implements vscode.Disposable { + private statusManager?: CodeIndexStatusManager + private _isInitialized = false + + public constructor(private readonly context: vscode.ExtensionContext) {} + + public init(): void { + if (this._isInitialized) { + throw new Error("Code index scope is already initialized") + } + const manager = new CodeIndexStatusManager((workspacePath) => + CodeIndexManagerRegistry.getOrCreate(this.context, workspacePath), + ) + manager.init() + this.statusManager = manager + this._isInitialized = true + } + + public dispose(): void { + const manager = this.statusManager + this.statusManager = undefined + this._isInitialized = false + manager?.dispose() + } +} diff --git a/src/services/code-index/code-index-status-manager.ts b/src/services/code-index/code-index-status-manager.ts new file mode 100644 index 0000000000..2461630eaa --- /dev/null +++ b/src/services/code-index/code-index-status-manager.ts @@ -0,0 +1,79 @@ +import * as vscode from "vscode" +import { ClineProvider } from "../../core/webview/ClineProvider" +import { getWorkspacePath } from "../../utils/path" +import type { CodeIndexManager } from "./manager" + +export type CodeIndexStatus = ReturnType + +/** Tracks the active workspace; owns subscriptions, never workspace managers. */ +export class CodeIndexStatusManager implements vscode.Disposable { + private currentManager?: CodeIndexManager + private progressSubscription?: vscode.Disposable + private editorSubscription?: vscode.Disposable + + public constructor( + private readonly getManagerForWorkspace: (workspacePath: string) => CodeIndexManager | undefined, + ) {} + + public init(): void { + this.editorSubscription = vscode.window.onDidChangeActiveTextEditor(() => this.updateSubscription()) + this.updateSubscription() + } + + private getCurrentManager(): CodeIndexManager | undefined { + const workspacePath = getWorkspacePath() + return workspacePath ? this.getManagerForWorkspace(workspacePath) : undefined + } + + private updateSubscription(): void { + const manager = this.getCurrentManager() + if (manager === this.currentManager) return + + this.disposeSubscription(this.progressSubscription, "progress") + this.progressSubscription = undefined + this.currentManager = manager + if (!manager) return + + this.progressSubscription = manager.onProgressUpdate(() => { + if (manager === this.currentManager && manager === this.getCurrentManager()) { + this.publishStatus(manager.getCurrentStatus()) + } + }) + this.publishStatus(manager.getCurrentStatus()) + } + + private publishStatus(status: CodeIndexStatus): void { + for (const provider of ClineProvider.getAllInstances()) { + void this.publishStatusToProvider(provider, status) + } + } + + private async publishStatusToProvider(provider: ClineProvider, status: CodeIndexStatus): Promise { + try { + const workspacePath = provider.workspacePath + if (workspacePath && workspacePath !== status.workspacePath) return + + await provider.postMessageToWebview({ type: "indexingStatusUpdate", values: status }) + } catch (error) { + console.error("[CodeIndexStatusManager] Failed to publish indexing status:", error) + } + } + + private disposeSubscription(subscription: vscode.Disposable | undefined, name: string): void { + try { + subscription?.dispose() + } catch (error) { + console.error(`[CodeIndexStatusManager] Failed to dispose ${name} subscription:`, error) + } + } + + public dispose(): void { + const editorSubscription = this.editorSubscription + const progressSubscription = this.progressSubscription + this.currentManager = undefined + this.editorSubscription = undefined + this.progressSubscription = undefined + this.disposeSubscription(editorSubscription, "active editor") + this.disposeSubscription(progressSubscription, "progress") + } +} diff --git a/src/services/code-index/code-index-workspace-scope.ts b/src/services/code-index/code-index-workspace-scope.ts index 683a783335..0f187fd749 100644 --- a/src/services/code-index/code-index-workspace-scope.ts +++ b/src/services/code-index/code-index-workspace-scope.ts @@ -1,10 +1,12 @@ import type * as vscode from "vscode" import { CodeIndexManager } from "./manager" +import { CodeIndexStateManager } from "./state-manager" /** Owns code-index services for one workspace; initialization remains with existing callers. */ export class CodeIndexWorkspaceScope implements vscode.Disposable { private _codeIndexManager?: CodeIndexManager + private _stateManager?: CodeIndexStateManager private _isInitialized = false public constructor( @@ -29,13 +31,20 @@ export class CodeIndexWorkspaceScope implements vscode.Disposable { if (this._isInitialized) { throw new Error("Code index workspace scope is already initialized") } - this._codeIndexManager = new CodeIndexManager(this.workspacePath, this.folderUri, this.context) + this._stateManager = new CodeIndexStateManager() + this._codeIndexManager = new CodeIndexManager( + this.workspacePath, + this.folderUri, + this.context, + this._stateManager, + ) this._isInitialized = true } public dispose(): void { const manager = this._codeIndexManager this._codeIndexManager = undefined + this._stateManager = undefined this._isInitialized = false manager?.dispose() } diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index ee181b17d6..bb186dc105 100644 --- a/src/services/code-index/manager.ts +++ b/src/services/code-index/manager.ts @@ -3,7 +3,7 @@ import { ContextProxy } from "../../core/config/ContextProxy" import { VectorStoreSearchResult } from "./interfaces" import { IndexingState } from "./interfaces/manager" import { CodeIndexConfigManager } from "./config-manager" -import { CodeIndexStateManager } from "./state-manager" +import type { CodeIndexStateManager } from "./state-manager" import { CodeIndexServiceFactory } from "./service-factory" import { CodeIndexSearchService } from "./search-service" import { CodeIndexOrchestrator } from "./orchestrator" @@ -35,11 +35,16 @@ export class CodeIndexManager { private readonly context: vscode.ExtensionContext /** @internal — construct only via {@link CodeIndexManagerRegistry} */ - public constructor(workspacePath: string, folderUri: vscode.Uri, context: vscode.ExtensionContext) { + public constructor( + workspacePath: string, + folderUri: vscode.Uri, + context: vscode.ExtensionContext, + stateManager: CodeIndexStateManager, + ) { this.workspacePath = workspacePath this._folderUri = folderUri this.context = context - this._stateManager = new CodeIndexStateManager() + this._stateManager = stateManager } // --- Public API ---