From b32a8498c0f91d5a24f2077e24c3ba29610eee6c Mon Sep 17 00:00:00 2001 From: Roomote Date: Sun, 23 Aug 2026 01:49:08 +0000 Subject: [PATCH 1/2] fix(services): handle background promise failures --- src/eslint.config.mjs | 1 + .../code-index/__tests__/manager.spec.ts | 19 +++++++++++++++ src/services/code-index/manager.ts | 8 ++++++- src/services/code-index/processors/scanner.ts | 10 ++++---- src/services/mcp/McpHub.ts | 4 +++- src/services/mcp/__tests__/McpHub.spec.ts | 24 +++++++++++++++++-- 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/eslint.config.mjs b/src/eslint.config.mjs index 6ae161683f..36a56c5df2 100644 --- a/src/eslint.config.mjs +++ b/src/eslint.config.mjs @@ -59,6 +59,7 @@ export default [ "core/tools/**/*.ts", "core/webview/**/*.ts", "integrations/**/*.ts", + "services/**/*.ts", ], languageOptions: { parserOptions: { diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index 627163f900..ce52593ed5 100644 --- a/src/services/code-index/__tests__/manager.spec.ts +++ b/src/services/code-index/__tests__/manager.spec.ts @@ -168,6 +168,25 @@ describe("CodeIndexManager - handleSettingsChange regression", () => { }) describe("handleSettingsChange", () => { + it("should log background indexing failures", async () => { + const indexingError = new Error("indexing startup failed") + const startIndexing = vi.fn().mockRejectedValue(indexingError) + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined) + Object.defineProperty(manager, "_orchestrator", { + value: { startIndexing, stopIndexing: vi.fn() }, + configurable: true, + }) + + manager["startIndexingInBackground"]() + await Promise.resolve() + + expect(consoleErrorSpy).toHaveBeenCalledWith( + "[CodeIndexManager] Background indexing failed:", + indexingError, + ) + consoleErrorSpy.mockRestore() + }) + it("should not throw when called on uninitialized manager (regression test)", async () => { // This is the core regression test: handleSettingsChange() should not throw // when called before the manager is initialized (during first-time configuration) diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index 245e8678f5..dd36a32d88 100644 --- a/src/services/code-index/manager.ts +++ b/src/services/code-index/manager.ts @@ -227,13 +227,19 @@ export class CodeIndexManager { (needsServiceRecreation && (!this._orchestrator || this._orchestrator.state !== "Indexing")) if (shouldStartOrRestartIndexing) { - this._orchestrator?.startIndexing() + this.startIndexingInBackground() } } return { requiresRestart } } + private startIndexingInBackground(): void { + void this._orchestrator?.startIndexing().catch((error) => { + console.error("[CodeIndexManager] Background indexing failed:", error) + }) + } + /** * Initiates the indexing process (initial scan and starts watcher). * Automatically recovers from error state if needed before starting. diff --git a/src/services/code-index/processors/scanner.ts b/src/services/code-index/processors/scanner.ts index 5d9ff5e362..4504e02e9b 100644 --- a/src/services/code-index/processors/scanner.ts +++ b/src/services/code-index/processors/scanner.ts @@ -216,10 +216,11 @@ export class DirectoryScanner implements IDirectoryScanner { activeBatchPromises.add(batchPromise) // Clean up completed promises to prevent memory accumulation - batchPromise.finally(() => { + const cleanupBatch = () => { activeBatchPromises.delete(batchPromise) pendingBatchCount-- - }) + } + void batchPromise.then(cleanupBatch, cleanupBatch) } } finally { release() @@ -306,10 +307,11 @@ export class DirectoryScanner implements IDirectoryScanner { activeBatchPromises.add(batchPromise) // Clean up completed promises to prevent memory accumulation - batchPromise.finally(() => { + const cleanupBatch = () => { activeBatchPromises.delete(batchPromise) pendingBatchCount-- - }) + } + void batchPromise.then(cleanupBatch, cleanupBatch) } finally { release() } diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 4c66a2ca60..1374e430fe 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -176,7 +176,9 @@ export class McpHub { if (secretStorage) { this.secretStorage = secretStorage } - this.watchMcpSettingsFile() + void this.watchMcpSettingsFile().catch((error) => { + console.error("[McpHub] Failed to watch MCP settings file:", error) + }) this.watchProjectMcpFile().catch(console.error) this.setupWorkspaceFoldersWatcher() this.initializationPromise = Promise.all([ diff --git a/src/services/mcp/__tests__/McpHub.spec.ts b/src/services/mcp/__tests__/McpHub.spec.ts index 576514c5df..96589d8dd6 100644 --- a/src/services/mcp/__tests__/McpHub.spec.ts +++ b/src/services/mcp/__tests__/McpHub.spec.ts @@ -10,6 +10,10 @@ import { ServerConfigSchema, McpHub } from "../McpHub" import { OAUTH_FLOW_TIMEOUT_MS } from "../constants" import { t } from "../../../i18n" +type McpHubPrivate = { + watchMcpSettingsFile: () => Promise +} + // Mock fs/promises before importing anything that uses it. // Named exports and the default export must share the same vi.fn() instances so that // `import * as fs` (used by McpHub.ts) and `import fs` (default) both see the same mocks. @@ -192,6 +196,22 @@ describe("McpHub", () => { } }) + it("should log settings watcher startup failures", async () => { + const watcherError = new Error("watcher startup failed") + const watchSpy = vi + .spyOn(McpHub.prototype as unknown as McpHubPrivate, "watchMcpSettingsFile") + .mockRejectedValueOnce(watcherError) + vi.mocked(console.error).mockClear() + + const failingHub = new McpHub(mockProvider as ClineProvider) + await failingHub.waitUntilReady() + await Promise.resolve() + + expect(console.error).toHaveBeenCalledWith("[McpHub] Failed to watch MCP settings file:", watcherError) + await failingHub.dispose() + watchSpy.mockRestore() + }) + describe("Discriminated union type handling", () => { it("should create connected connections with proper type", async () => { // Mock StdioClientTransport @@ -2526,7 +2546,7 @@ describe("McpHub", () => { }) mockSecretStorage.onDidChange.mockImplementation((_key: string, cb: () => void) => { - Promise.resolve().then(() => { + queueMicrotask(() => { mockSecretStorage.getOAuthData.mockResolvedValue({ expires_at: Date.now() + 10 * 60 * 1000, }) @@ -2811,7 +2831,7 @@ describe("McpHub", () => { }) mockSecretStorage.onDidChange.mockImplementation((_key: string, cb: () => void) => { - Promise.resolve().then(() => { + queueMicrotask(() => { // Dispose the hub before the token callback runs ;(mcpHub as any).isDisposed = true mockSecretStorage.getOAuthData.mockResolvedValue({ From b1bfc2d55690e79070de43aa119ace7788b4c89d Mon Sep 17 00:00:00 2001 From: Roomote Date: Sun, 23 Aug 2026 04:11:04 +0000 Subject: [PATCH 2/2] test(code-index): cover threshold batch cleanup --- .../processors/__tests__/scanner.spec.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/services/code-index/processors/__tests__/scanner.spec.ts b/src/services/code-index/processors/__tests__/scanner.spec.ts index dfb3071c78..72fefd676d 100644 --- a/src/services/code-index/processors/__tests__/scanner.spec.ts +++ b/src/services/code-index/processors/__tests__/scanner.spec.ts @@ -220,6 +220,36 @@ describe("DirectoryScanner", () => { expect(mockVectorStore.upsertPoints).toHaveBeenCalled() }) + it("should clean up a threshold-triggered batch", async () => { + const thresholdScanner = new DirectoryScanner( + mockEmbedder, + mockVectorStore, + mockCodeParser, + mockCacheManager, + mockIgnoreInstance, + 1, + ) + const { listFiles } = await import("../../../glob/list-files") + vi.mocked(listFiles).mockResolvedValue([["test/file1.js"], false]) + mockCodeParser.parseFile.mockResolvedValue([ + { + file_path: "test/file1.js", + content: "test content", + start_line: 1, + end_line: 5, + identifier: "test", + type: "function", + fileHash: "hash", + segmentHash: "threshold-segment", + }, + ]) + + await thresholdScanner.scanDirectory("/test") + + expect(mockEmbedder.createEmbeddings).toHaveBeenCalledTimes(1) + expect(mockVectorStore.upsertPoints).toHaveBeenCalledTimes(1) + }) + it("should delete points for removed files", async () => { ;(mockCacheManager.getAllHashes as any).mockReturnValue({ "old/file.js": "old-hash" })