From ef7cdda4892cb0246a347f9a26257d4836203f3d Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Thu, 24 Sep 2026 18:33:12 +0300 Subject: [PATCH 1/2] refactor(code-index): introduce workspace scope behind registry Refs Zoo-Code-Org/Zoo-Code#1594 --- .../code-index-manager-registry.spec.ts | 38 +++++++ .../code-index-workspace-scope.spec.ts | 100 ++++++++++++++++++ .../code-index/code-index-manager-registry.ts | 33 +++--- .../code-index/code-index-workspace-scope.ts | 42 ++++++++ 4 files changed, 201 insertions(+), 12 deletions(-) create mode 100644 src/services/code-index/__tests__/code-index-workspace-scope.spec.ts create mode 100644 src/services/code-index/code-index-workspace-scope.ts 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 9879ff8ea9..e8cc9c4871 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 @@ -2,6 +2,7 @@ import * as vscode from "vscode" import { makeExtensionContext, makeTextDocument, makeTextEditor, makeUri } from "../../../test-utils/vscode" import { CodeIndexManager } from "../manager" import { CodeIndexManagerRegistry } from "../code-index-manager-registry" +import { CodeIndexWorkspaceScope } from "../code-index-workspace-scope" vi.mock("vscode", () => ({ workspace: { workspaceFolders: undefined, getWorkspaceFolder: vi.fn() }, @@ -114,11 +115,48 @@ describe("CodeIndexManagerRegistry", () => { expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([manager]) }) + it("logs each disposal failure, continues cleanup and permits recreation", () => { + const logError = vi.spyOn(console, "error").mockImplementation(() => {}) + const first = CodeIndexManagerRegistry.getOrCreate(context, "/first")! + const second = CodeIndexManagerRegistry.getOrCreate(context, "/second")! + const third = CodeIndexManagerRegistry.getOrCreate(context, "/third")! + const error = new Error("cleanup failed") + vi.mocked(first.dispose).mockImplementationOnce(() => { + throw error + }) + vi.mocked(second.dispose).mockImplementationOnce(() => { + throw "second cleanup failed" + }) + + expect(() => CodeIndexManagerRegistry.disposeAll()).not.toThrow() + for (const manager of [first, second, third]) { + expect(manager.dispose).toHaveBeenCalledExactlyOnceWith() + } + expect(logError).toHaveBeenCalledTimes(2) + expect(logError).toHaveBeenNthCalledWith( + 1, + "[CodeIndexManagerRegistry] Failed to dispose workspace scope for /first:", + error, + ) + expect(logError).toHaveBeenNthCalledWith( + 2, + "[CodeIndexManagerRegistry] Failed to dispose workspace scope for /second:", + "second cleanup failed", + ) + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) + CodeIndexManagerRegistry.disposeAll() + expect(logError).toHaveBeenCalledTimes(2) + expect(first.dispose).toHaveBeenCalledTimes(1) + expect(CodeIndexManagerRegistry.getOrCreate(context, "/first")).not.toBe(first) + }) + it("disposes every manager, supports repeated cleanup and recreates instances", () => { + const disposeScope = vi.spyOn(CodeIndexWorkspaceScope.prototype, "dispose") const a = CodeIndexManagerRegistry.getOrCreate(context, "/first")! const b = CodeIndexManagerRegistry.getOrCreate(context, "/second")! CodeIndexManagerRegistry.disposeAll() CodeIndexManagerRegistry.disposeAll() + expect(disposeScope).toHaveBeenCalledTimes(2) expect(a.dispose).toHaveBeenCalledTimes(1) expect(b.dispose).toHaveBeenCalledTimes(1) expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) 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 new file mode 100644 index 0000000000..2e20f07799 --- /dev/null +++ b/src/services/code-index/__tests__/code-index-workspace-scope.spec.ts @@ -0,0 +1,100 @@ +import { makeExtensionContext, makeUri } from "../../../test-utils/vscode" +import { CodeIndexManager } from "../manager" +import { CodeIndexWorkspaceScope } from "../code-index-workspace-scope" + +vi.mock("../manager", () => ({ + CodeIndexManager: vi.fn().mockImplementation(function () { + return { initialize: vi.fn(), dispose: vi.fn() } + }), +})) + +describe("CodeIndexWorkspaceScope", () => { + beforeEach(() => vi.clearAllMocks()) + + it("guards generic values and preserves defined falsy values", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + expect(() => scope["ensureInitialized"](42)).toThrow("Code index workspace scope is not initialized") + scope.init() + expect(() => scope["ensureInitialized"](undefined)).toThrow("Code index workspace scope is not initialized") + expect(scope["ensureInitialized"](false)).toBe(false) + expect(scope["ensureInitialized"](0)).toBe(0) + expect(scope["ensureInitialized"]("")).toBe("") + }) + + it("creates its manager only on init without starting configuration initialization", () => { + const context = makeExtensionContext() + const uri = makeUri("/workspace") + const scope = new CodeIndexWorkspaceScope(uri.fsPath, uri, context) + + expect(CodeIndexManager).not.toHaveBeenCalled() + 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(scope.codeIndexManager).toBe(manager) + expect(() => scope.init()).toThrow("Code index workspace scope is already initialized") + expect(scope.codeIndexManager).toBe(manager) + expect(CodeIndexManager).toHaveBeenCalledTimes(1) + expect(manager.initialize).not.toHaveBeenCalled() + }) + + it("synchronously disposes its manager", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + scope.init() + const manager = scope.codeIndexManager + + expect(scope.dispose()).toBeUndefined() + expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") + scope.dispose() + expect(manager.dispose).toHaveBeenCalledExactlyOnceWith() + }) + + it("can dispose before dependency creation without constructing a manager", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + scope.dispose() + expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") + expect(CodeIndexManager).not.toHaveBeenCalled() + }) + + it("creates a fresh manager when initialized after disposal", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + scope.init() + const previous = scope.codeIndexManager + scope.dispose() + scope.init() + const current = scope.codeIndexManager + expect(current).not.toBe(previous) + expect(scope.codeIndexManager).toBe(current) + expect(CodeIndexManager).toHaveBeenCalledTimes(2) + expect(current.dispose).not.toHaveBeenCalled() + }) + + it("clears its reference even when manager disposal throws", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + scope.init() + const manager = scope.codeIndexManager + const error = new Error("disposal failed") + vi.mocked(manager.dispose).mockImplementationOnce(() => { + throw error + }) + expect(() => scope.dispose()).toThrow(error) + expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") + scope.dispose() + expect(manager.dispose).toHaveBeenCalledTimes(1) + }) + + it("remains uninitialized when construction fails and permits retry", () => { + const scope = new CodeIndexWorkspaceScope("/workspace", makeUri("/workspace"), makeExtensionContext()) + const error = new Error("construction failed") + vi.mocked(CodeIndexManager).mockImplementationOnce(function () { + throw error + }) + + expect(() => scope.init()).toThrow(error) + expect(() => scope.codeIndexManager).toThrow("Code index workspace scope is not initialized") + expect(scope.init()).toBeUndefined() + expect(scope.codeIndexManager).toBe(vi.mocked(CodeIndexManager).mock.results[1].value) + expect(CodeIndexManager).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/services/code-index/code-index-manager-registry.ts b/src/services/code-index/code-index-manager-registry.ts index 635ec62647..057745e1fd 100644 --- a/src/services/code-index/code-index-manager-registry.ts +++ b/src/services/code-index/code-index-manager-registry.ts @@ -1,9 +1,10 @@ import * as vscode from "vscode" -import { CodeIndexManager } from "./manager" +import type { CodeIndexManager } from "./manager" +import { CodeIndexWorkspaceScope } from "./code-index-workspace-scope" -/** Resolves workspaces and owns their cached CodeIndexManager instances. */ +/** Owns workspace scopes while preserving the manager-facing API. */ export class CodeIndexManagerRegistry { - private static instances = new Map() + private static codeIndexWorkspaceScopes = new Map() public static getOrCreate(context: vscode.ExtensionContext, workspacePath?: string): CodeIndexManager | undefined { const folder = this.resolveWorkspaceFolder(workspacePath) @@ -12,27 +13,35 @@ export class CodeIndexManagerRegistry { return undefined } - const existing = this.instances.get(resolvedPath) + const existing = this.codeIndexWorkspaceScopes.get(resolvedPath) if (existing) { - return existing + return existing.codeIndexManager } // Preserve real workspace URIs, including remote schemes and authorities. const folderUri = folder?.uri ?? vscode.Uri.file(resolvedPath) - const manager = new CodeIndexManager(resolvedPath, folderUri, context) - this.instances.set(resolvedPath, manager) - return manager + const codeIndexWorkspaceScope = new CodeIndexWorkspaceScope(resolvedPath, folderUri, context) + codeIndexWorkspaceScope.init() + this.codeIndexWorkspaceScopes.set(resolvedPath, codeIndexWorkspaceScope) + return codeIndexWorkspaceScope.codeIndexManager } public static getAllInstances(): CodeIndexManager[] { - return Array.from(this.instances.values()) + return Array.from(this.codeIndexWorkspaceScopes.values(), (scope) => scope.codeIndexManager) } public static disposeAll(): void { - for (const instance of this.instances.values()) { - instance.dispose() + for (const [workspacePath, codeIndexWorkspaceScope] of this.codeIndexWorkspaceScopes) { + try { + codeIndexWorkspaceScope.dispose() + } catch (error) { + console.error( + `[CodeIndexManagerRegistry] Failed to dispose workspace scope for ${workspacePath}:`, + error, + ) + } } - this.instances.clear() + this.codeIndexWorkspaceScopes.clear() } private static resolveWorkspaceFolder(workspacePath?: string): vscode.WorkspaceFolder | undefined { diff --git a/src/services/code-index/code-index-workspace-scope.ts b/src/services/code-index/code-index-workspace-scope.ts new file mode 100644 index 0000000000..683a783335 --- /dev/null +++ b/src/services/code-index/code-index-workspace-scope.ts @@ -0,0 +1,42 @@ +import type * as vscode from "vscode" + +import { CodeIndexManager } from "./manager" + +/** Owns code-index services for one workspace; initialization remains with existing callers. */ +export class CodeIndexWorkspaceScope implements vscode.Disposable { + private _codeIndexManager?: CodeIndexManager + private _isInitialized = false + + public constructor( + private readonly workspacePath: string, + private readonly folderUri: vscode.Uri, + private readonly context: vscode.ExtensionContext, + ) {} + + public get codeIndexManager(): CodeIndexManager { + return this.ensureInitialized(this._codeIndexManager) + } + + private ensureInitialized(value: T | undefined): T { + if (!this._isInitialized || value === undefined) { + throw new Error("Code index workspace scope is not initialized") + } + return value + } + + /** Creates the manager without loading configuration or starting indexing. */ + public init(): void { + if (this._isInitialized) { + throw new Error("Code index workspace scope is already initialized") + } + this._codeIndexManager = new CodeIndexManager(this.workspacePath, this.folderUri, this.context) + this._isInitialized = true + } + + public dispose(): void { + const manager = this._codeIndexManager + this._codeIndexManager = undefined + this._isInitialized = false + manager?.dispose() + } +} From 3bed48390144ea0f4f861d2d6d8e34ee54ba5315 Mon Sep 17 00:00:00 2001 From: gubin-dev Date: Sun, 27 Sep 2026 12:34:50 +0300 Subject: [PATCH 2/2] test(code-index): cover registry retry after construction failure --- .../code-index-manager-registry.spec.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 e8cc9c4871..28ecb5d6f0 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 @@ -108,6 +108,22 @@ describe("CodeIndexManagerRegistry", () => { expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([a, b]) }) + it("does not cache a scope when manager construction fails and permits retry for the same path", () => { + const error = new Error("construction failed") + vi.mocked(CodeIndexManager).mockImplementationOnce(function () { + throw error + }) + + expect(() => CodeIndexManagerRegistry.getOrCreate(context, "/first")).toThrow(error) + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) + + const manager = CodeIndexManagerRegistry.getOrCreate(context, "/first") + expect(manager).toBe(vi.mocked(CodeIndexManager).mock.results[1].value) + expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([manager]) + expect(CodeIndexManagerRegistry.getOrCreate(context, "/first")).toBe(manager) + expect(CodeIndexManager).toHaveBeenCalledTimes(2) + }) + it("returns a snapshot that cannot mutate the cache", () => { expect(CodeIndexManagerRegistry.getAllInstances()).toEqual([]) const manager = CodeIndexManagerRegistry.getOrCreate(context)