diff --git a/src/services/code-index/__tests__/config-manager.spec.ts b/src/services/code-index/__tests__/config-manager.spec.ts index 71b6e79e61..9855f301cb 100644 --- a/src/services/code-index/__tests__/config-manager.spec.ts +++ b/src/services/code-index/__tests__/config-manager.spec.ts @@ -60,6 +60,10 @@ describe("CodeIndexConfigManager", () => { expect(configManager.currentEmbedderProvider).toBe("openai") }) + it("returns the original context proxy", () => { + expect(configManager.getContextProxy()).toBe(mockContextProxy) + }) + it("loads Bedrock as the embedder provider with its optional profile", () => { mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEnabled: true, @@ -81,6 +85,34 @@ describe("CodeIndexConfigManager", () => { }) }) + describe("model dimension parsing", () => { + it.each([ + { raw: undefined, expected: undefined, warns: false }, + { raw: null, expected: undefined, warns: false }, + { raw: 1536, expected: 1536, warns: false }, + { raw: "768", expected: 768, warns: false }, + { raw: 0, expected: undefined, warns: true }, + { raw: -1, expected: undefined, warns: true }, + { raw: "invalid", expected: undefined, warns: true }, + { raw: NaN, expected: undefined, warns: true }, + ])("parses $raw with warning=$warns", async ({ raw, expected, warns }) => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}) + try { + mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEmbedderModelDimension: raw }) + const { currentConfig } = await configManager.loadConfiguration() + expect(currentConfig.modelDimension).toBe(expected) + expect(warn).toHaveBeenCalledTimes(warns ? 1 : 0) + if (warns) { + expect(warn).toHaveBeenCalledWith( + `Invalid codebaseIndexEmbedderModelDimension value: ${raw}. Must be a positive number.`, + ) + } + } finally { + warn.mockRestore() + } + }) + }) + describe("isFeatureEnabled", () => { it("should return false when codebaseIndexEnabled is false", async () => { mockContextProxy.getGlobalState.mockReturnValue({ @@ -192,6 +224,7 @@ describe("CodeIndexConfigManager", () => { qdrantUrl: "http://localhost:6333", qdrantApiKey: "", searchMinScore: 0.4, + searchMaxResults: 50, }) expect(result.requiresRestart).toBe(false) }) @@ -1553,6 +1586,59 @@ describe("CodeIndexConfigManager", () => { }) describe("doesConfigChangeRequireRestart", () => { + it.each([ + { missing: "enabled", expected: true }, + { missing: "configured", expected: true }, + { missing: "embedderProvider", expected: false }, + ] as const)( + "defaults missing previous $missing when comparing a ready configuration", + async ({ missing, expected }) => { + mockContextProxy.getGlobalState.mockReturnValue({ + codebaseIndexEnabled: true, + codebaseIndexEmbedderProvider: providerIdentifiers.openai, + codebaseIndexQdrantUrl: "http://localhost:6333", + }) + setupSecretMocks({ codeIndexOpenAiKey: "test-key" }) + configManager = new CodeIndexConfigManager(mockContextProxy) + const { configSnapshot } = await configManager.loadConfiguration() + const previous = { ...configSnapshot } + // Required in the type, but the comparison deliberately tolerates incomplete runtime snapshots. + Reflect.deleteProperty(previous, missing) + + expect(configManager.doesConfigChangeRequireRestart(previous)).toBe(expected) + }, + ) + + it("normalizes absent connection values on either side of a defensive comparison", () => { + const previous: PreviousConfigSnapshot = { + enabled: true, + configured: true, + embedderProvider: providerIdentifiers.openai, + } + // Normal loads fill every connection value; exercise the helper's defensive defaults directly. + expect(configManager["_hasConnectionSettingsChanged"](previous, { ...previous, openAiKey: "" })).toBe(false) + expect(configManager["_hasConnectionSettingsChanged"]({ ...previous, openAiKey: "" }, previous)).toBe(false) + expect( + configManager["_hasConnectionSettingsChanged"](previous, { ...previous, qdrantApiKey: "new-key" }), + ).toBe(true) + }) + + it("normalizes missing optional snapshot credentials in the previous load result", async () => { + // The snapshot type permits absent credentials even though normal readers initialize them. + configManager["config"] = Object.freeze({ + ...configManager["config"], + openAiOptions: undefined, + qdrantApiKey: undefined, + }) + + const { configSnapshot, requiresRestart } = await configManager.loadConfiguration() + + expect(configSnapshot.openAiKey).toBe("") + expect(configSnapshot.qdrantApiKey).toBe("") + expect(configSnapshot.configured).toBe(false) + expect(requiresRestart).toBe(false) + }) + it("should return true when enabling the feature", async () => { // Initial state: disabled mockContextProxy.getGlobalState.mockReturnValue({ @@ -1773,6 +1859,69 @@ describe("CodeIndexConfigManager", () => { }) describe("getConfig", () => { + it("publishes frozen configuration and options for every provider", async () => { + mockContextProxy.getGlobalState.mockReturnValue({ + codebaseIndexEnabled: true, + codebaseIndexEmbedderProvider: providerIdentifiers.openai, + codebaseIndexQdrantUrl: "http://localhost:6333", + codebaseIndexOpenAiCompatibleBaseUrl: "https://example.com/v1", + codebaseIndexSearchMaxResults: 23, + }) + mockContextProxy.getSecret.mockReturnValue("test-key") + + const { currentConfig } = await configManager.loadConfiguration() + expect(currentConfig).toEqual(configManager.getConfig()) + expect(currentConfig.searchMaxResults).toBe(23) + expect(Object.isFrozen(currentConfig)).toBe(true) + expect(Reflect.set(currentConfig, "modelId", "changed")).toBe(false) + for (const options of [ + currentConfig.openAiOptions, + currentConfig.ollamaOptions, + currentConfig.openAiCompatibleOptions, + currentConfig.geminiOptions, + currentConfig.mistralOptions, + currentConfig.vercelAiGatewayOptions, + currentConfig.bedrockOptions, + currentConfig.openRouterOptions, + ]) { + expect(options).toBeDefined() + if (!options) throw new Error("Expected provider options") + expect(Object.isFrozen(options)).toBe(true) + for (const key of Object.keys(options)) { + expect(Reflect.set(options, key, "changed")).toBe(false) + } + } + expect(configManager.getConfig()).toEqual(currentConfig) + }) + + it("preserves previously returned snapshots across reloads", async () => { + mockContextProxy.getSecret.mockReturnValue("old-key") + const { currentConfig: previous } = await configManager.loadConfiguration() + mockContextProxy.getSecret.mockReturnValue("new-key") + const { currentConfig: current, configSnapshot } = await configManager.loadConfiguration() + + expect(previous.openAiOptions?.openAiNativeApiKey).toBe("old-key") + expect(current.openAiOptions?.openAiNativeApiKey).toBe("new-key") + expect(current.openAiOptions).not.toBe(previous.openAiOptions) + expect(configSnapshot.openAiKey).toBe("old-key") + }) + + it("keeps the current snapshot when building the next snapshot fails", async () => { + const previous = configManager.getConfig() + mockContextProxy.getGlobalState.mockReturnValue({ + codebaseIndexEnabled: true, + codebaseIndexQdrantUrl: "http://changed:6333", + get codebaseIndexEmbedderModelDimension(): number { + throw new Error("Invalid stored dimension") + }, + }) + + await expect(configManager.loadConfiguration()).rejects.toThrow("Invalid stored dimension") + expect(configManager.getConfig()).toEqual(previous) + expect(configManager.isFeatureEnabled).toBe(false) + expect(configManager.getConfig().openAiOptions).toBe(previous.openAiOptions) + }) + it("should return the current configuration", () => { mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEnabled: true, @@ -2240,6 +2389,32 @@ describe("CodeIndexConfigManager", () => { expect(result.currentConfig.isConfigured).toBe(true) }) + it("keeps an explicitly empty Bedrock region unconfigured and restarts when restored", async () => { + const settings = { + codebaseIndexEnabled: true, + codebaseIndexQdrantUrl: "http://qdrant.local", + codebaseIndexEmbedderProvider: providerIdentifiers.bedrock, + codebaseIndexBedrockRegion: "", + codebaseIndexBedrockProfile: "test-profile", + } + mockContextProxy.getGlobalState.mockReturnValue(settings) + configManager = new CodeIndexConfigManager(mockContextProxy) + + const unchanged = await configManager.loadConfiguration() + expect(unchanged.currentConfig.bedrockOptions).toBeUndefined() + expect(unchanged.currentConfig.isConfigured).toBe(false) + expect(unchanged.configSnapshot.bedrockRegion).toBe("") + expect(unchanged.configSnapshot.bedrockProfile).toBe("") + expect(unchanged.requiresRestart).toBe(false) + + mockContextProxy.getGlobalState.mockReturnValue({ ...settings, codebaseIndexBedrockRegion: "eu-west-1" }) + const restored = await configManager.loadConfiguration() + expect(restored.currentConfig.bedrockOptions).toEqual({ region: "eu-west-1", profile: "test-profile" }) + expect(restored.currentConfig.isConfigured).toBe(true) + expect(restored.requiresRestart).toBe(true) + expect(unchanged.currentConfig.bedrockOptions).toBeUndefined() + }) + it("should return false from isConfigured for Bedrock when the Qdrant URL is missing", () => { mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEnabled: true, @@ -2288,17 +2463,14 @@ describe("CodeIndexConfigManager", () => { // private field to a value outside the union. mockContextProxy.getGlobalState.mockReturnValue({ codebaseIndexEnabled: true, + codebaseIndexQdrantUrl: "http://localhost:6333", }) mockContextProxy.getSecret.mockReturnValue(undefined) configManager = new CodeIndexConfigManager(mockContextProxy) - // The defensive `return false` is unreachable through the public API because - // EmbedderProvider is a closed union, so exercise it by forcing the private field - // to a value outside the union. `embedderProvider` is TypeScript `private`, not - // `#`-private, so a runtime property write reaches it. The double assertion is a - // last resort: the private field is not part of the public type surface, and - // `as any` is avoided to keep the file's no-explicit-any suppression budget flat. - ;(configManager as unknown as Record)["embedderProvider"] = "not-a-provider" + // Replace the snapshot with deliberately invalid runtime data to exercise + // the defensive branch without mutating the frozen production snapshot. + Reflect.set(configManager, "config", { ...configManager["config"], embedderProvider: "not-a-provider" }) expect(configManager.isConfigured()).toBe(false) }) }) diff --git a/src/services/code-index/config-manager.ts b/src/services/code-index/config-manager.ts index 5b2b2c63e2..3d8121476d 100644 --- a/src/services/code-index/config-manager.ts +++ b/src/services/code-index/config-manager.ts @@ -1,7 +1,12 @@ -import { ApiHandlerOptions } from "../../shared/api" import { ContextProxy } from "../../core/config/ContextProxy" +import type { CodebaseIndexConfig } from "@roo-code/types" import { EmbedderProvider } from "./interfaces/manager" -import { CodeIndexConfig, PreviousConfigSnapshot } from "./interfaces/config" +import { + CodeIndexConfig, + CodeIndexConfigSnapshot, + PreviousConfigSnapshot, + freezeCodeIndexConfigSnapshot, +} from "./interfaces/config" import { DEFAULT_SEARCH_MIN_SCORE, DEFAULT_MAX_SEARCH_RESULTS } from "./constants" import { getDefaultModelId, getModelDimension, getModelScoreThreshold } from "../../shared/embeddingModels" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" @@ -12,34 +17,20 @@ import { providerIdentifiers } from "@roo-code/types/provider-identifiers" */ export class CodeIndexConfigManager { private _isConfigurationLoaded = false + private config: CodeIndexConfigSnapshot + private readonly contextProxy: ContextProxy + + constructor(contextProxy: ContextProxy) { + this.contextProxy = contextProxy + // Initialize with current configuration to avoid false restart triggers + this.config = this._readConfiguration() + } /** Whether at least one full asynchronous configuration load has succeeded. */ public get isConfigurationLoaded(): boolean { return this._isConfigurationLoaded } - private codebaseIndexEnabled: boolean = false - private embedderProvider: EmbedderProvider = providerIdentifiers.openai - private modelId?: string - private modelDimension?: number - private openAiOptions?: ApiHandlerOptions - private ollamaOptions?: ApiHandlerOptions - private openAiCompatibleOptions?: { baseUrl: string; apiKey: string } - private geminiOptions?: { apiKey: string } - private mistralOptions?: { apiKey: string } - private vercelAiGatewayOptions?: { apiKey: string } - private bedrockOptions?: { region: string; profile?: string } - private openRouterOptions?: { apiKey: string; specificProvider?: string } - private qdrantUrl?: string = "http://localhost:6333" - private qdrantApiKey?: string - private searchMinScore?: number - private searchMaxResults?: number - - constructor(private readonly contextProxy: ContextProxy) { - // Initialize with current configuration to avoid false restart triggers - this._loadAndSetConfiguration() - } - /** * Gets the context proxy instance */ @@ -48,12 +39,13 @@ export class CodeIndexConfigManager { } /** - * Private method that handles loading configuration from storage and updating instance variables. - * This eliminates code duplication between initializeWithCurrentConfig() and loadConfiguration(). + * Builds a complete immutable snapshot without changing the current configuration. */ - private _loadAndSetConfiguration(): void { + private _readConfiguration(): CodeIndexConfigSnapshot { // Load configuration from storage - const codebaseIndexConfig = this.contextProxy?.getGlobalState("codebaseIndexConfig") ?? { + const codebaseIndexConfig: Readonly = this.contextProxy?.getGlobalState( + "codebaseIndexConfig", + ) ?? { codebaseIndexEnabled: false, codebaseIndexQdrantUrl: "http://localhost:6333", codebaseIndexEmbedderProvider: providerIdentifiers.openai, @@ -65,99 +57,133 @@ export class CodeIndexConfigManager { codebaseIndexBedrockProfile: "", } - const { - codebaseIndexEnabled, - codebaseIndexQdrantUrl, - codebaseIndexEmbedderProvider, - codebaseIndexEmbedderBaseUrl, - codebaseIndexEmbedderModelId, - codebaseIndexSearchMinScore, - codebaseIndexSearchMaxResults, - } = codebaseIndexConfig - - const openAiKey = this.contextProxy?.getSecret("codeIndexOpenAiKey") ?? "" const qdrantApiKey = this.contextProxy?.getSecret("codeIndexQdrantApiKey") ?? "" - // Fix: Read OpenAI Compatible settings from the correct location within codebaseIndexConfig - const openAiCompatibleBaseUrl = codebaseIndexConfig.codebaseIndexOpenAiCompatibleBaseUrl ?? "" - const openAiCompatibleApiKey = this.contextProxy?.getSecret("codebaseIndexOpenAiCompatibleApiKey") ?? "" - const geminiApiKey = this.contextProxy?.getSecret("codebaseIndexGeminiApiKey") ?? "" - const mistralApiKey = this.contextProxy?.getSecret("codebaseIndexMistralApiKey") ?? "" - const vercelAiGatewayApiKey = this.contextProxy?.getSecret("codebaseIndexVercelAiGatewayApiKey") ?? "" - const bedrockRegion = codebaseIndexConfig.codebaseIndexBedrockRegion ?? "us-east-1" - const bedrockProfile = codebaseIndexConfig.codebaseIndexBedrockProfile ?? "" - const openRouterApiKey = this.contextProxy?.getSecret("codebaseIndexOpenRouterApiKey") ?? "" - const openRouterSpecificProvider = codebaseIndexConfig.codebaseIndexOpenRouterSpecificProvider ?? "" - - // Update instance variables with configuration - this.codebaseIndexEnabled = codebaseIndexEnabled ?? false - this.qdrantUrl = codebaseIndexQdrantUrl - this.qdrantApiKey = qdrantApiKey ?? "" - this.searchMinScore = codebaseIndexSearchMinScore - this.searchMaxResults = codebaseIndexSearchMaxResults - - // Validate and set model dimension - const rawDimension = codebaseIndexConfig.codebaseIndexEmbedderModelDimension - if (rawDimension !== undefined && rawDimension !== null) { - const dimension = Number(rawDimension) - if (!isNaN(dimension) && dimension > 0) { - this.modelDimension = dimension - } else { - console.warn( - `Invalid codebaseIndexEmbedderModelDimension value: ${rawDimension}. Must be a positive number.`, - ) - this.modelDimension = undefined - } - } else { - this.modelDimension = undefined - } - this.openAiOptions = { openAiNativeApiKey: openAiKey } + // Build locally so a failed read cannot partially update the published snapshot. + const modelDimension = this._parseModelDimension(codebaseIndexConfig.codebaseIndexEmbedderModelDimension) // Set embedder provider with support for openai-compatible - if (codebaseIndexEmbedderProvider === providerIdentifiers.ollama) { - this.embedderProvider = providerIdentifiers.ollama - } else if (codebaseIndexEmbedderProvider === "openai-compatible") { - this.embedderProvider = "openai-compatible" - } else if (codebaseIndexEmbedderProvider === providerIdentifiers.gemini) { - this.embedderProvider = providerIdentifiers.gemini - } else if (codebaseIndexEmbedderProvider === providerIdentifiers.mistral) { - this.embedderProvider = providerIdentifiers.mistral - } else if (codebaseIndexEmbedderProvider === providerIdentifiers.vercelAiGateway) { - this.embedderProvider = providerIdentifiers.vercelAiGateway - } else if ((codebaseIndexEmbedderProvider as string) === providerIdentifiers.bedrock) { - this.embedderProvider = providerIdentifiers.bedrock - } else if (codebaseIndexEmbedderProvider === providerIdentifiers.openrouter) { - this.embedderProvider = providerIdentifiers.openrouter - } else if (codebaseIndexEmbedderProvider === "semble") { - this.embedderProvider = "semble" - } else { - this.embedderProvider = providerIdentifiers.openai + const embedderProvider = this._resolveEmbedderProvider(codebaseIndexConfig.codebaseIndexEmbedderProvider) + + const modelId = codebaseIndexConfig.codebaseIndexEmbedderModelId || undefined + + return freezeCodeIndexConfigSnapshot({ + codebaseIndexEnabled: codebaseIndexConfig.codebaseIndexEnabled ?? false, + qdrantUrl: codebaseIndexConfig.codebaseIndexQdrantUrl, + qdrantApiKey, + searchMinScore: codebaseIndexConfig.codebaseIndexSearchMinScore, + searchMaxResults: codebaseIndexConfig.codebaseIndexSearchMaxResults, + embedderProvider, + modelId, + modelDimension, + openAiOptions: this._readOpenAiOptions(), + ollamaOptions: this._readOllamaOptions(), + openAiCompatibleOptions: this._readOpenAiCompatibleOptions(), + geminiOptions: this._readGeminiOptions(), + mistralOptions: this._readMistralOptions(), + vercelAiGatewayOptions: this._readVercelAiGatewayOptions(), + bedrockOptions: this._readBedrockOptions(), + openRouterOptions: this._readOpenRouterOptions(), + }) + } + + private _readOpenAiOptions(): CodeIndexConfig["openAiOptions"] { + return { openAiNativeApiKey: this.contextProxy?.getSecret("codeIndexOpenAiKey") ?? "" } + } + + private _readOllamaOptions(): CodeIndexConfig["ollamaOptions"] { + const config = this.contextProxy?.getGlobalState("codebaseIndexConfig") + return { ollamaBaseUrl: config == null ? "" : config.codebaseIndexEmbedderBaseUrl } + } + + private _readGeminiOptions(): CodeIndexConfig["geminiOptions"] { + const apiKey = this.contextProxy?.getSecret("codebaseIndexGeminiApiKey") ?? "" + if (!apiKey) { + return undefined + } + + return { apiKey } + } + + private _readMistralOptions(): CodeIndexConfig["mistralOptions"] { + const apiKey = this.contextProxy?.getSecret("codebaseIndexMistralApiKey") ?? "" + if (!apiKey) { + return undefined + } + + return { apiKey } + } + + private _readVercelAiGatewayOptions(): CodeIndexConfig["vercelAiGatewayOptions"] { + const apiKey = this.contextProxy?.getSecret("codebaseIndexVercelAiGatewayApiKey") ?? "" + if (!apiKey) { + return undefined } - this.modelId = codebaseIndexEmbedderModelId || undefined + return { apiKey } + } - this.ollamaOptions = { - ollamaBaseUrl: codebaseIndexEmbedderBaseUrl, + private _readBedrockOptions(): CodeIndexConfig["bedrockOptions"] { + const config = this.contextProxy?.getGlobalState("codebaseIndexConfig") + const region = config?.codebaseIndexBedrockRegion ?? "us-east-1" + if (!region) { + return undefined } - this.openAiCompatibleOptions = - openAiCompatibleBaseUrl && openAiCompatibleApiKey - ? { - baseUrl: openAiCompatibleBaseUrl, - apiKey: openAiCompatibleApiKey, - } - : undefined - - this.geminiOptions = geminiApiKey ? { apiKey: geminiApiKey } : undefined - this.mistralOptions = mistralApiKey ? { apiKey: mistralApiKey } : undefined - this.vercelAiGatewayOptions = vercelAiGatewayApiKey ? { apiKey: vercelAiGatewayApiKey } : undefined - this.openRouterOptions = openRouterApiKey - ? { apiKey: openRouterApiKey, specificProvider: openRouterSpecificProvider || undefined } - : undefined - // Set bedrockOptions if region is provided (profile is optional) - this.bedrockOptions = bedrockRegion - ? { region: bedrockRegion, profile: bedrockProfile || undefined } - : undefined + return { region, profile: config?.codebaseIndexBedrockProfile || undefined } + } + + private _readOpenRouterOptions(): CodeIndexConfig["openRouterOptions"] { + const apiKey = this.contextProxy?.getSecret("codebaseIndexOpenRouterApiKey") ?? "" + if (!apiKey) { + return undefined + } + + const specificProvider = + this.contextProxy?.getGlobalState("codebaseIndexConfig")?.codebaseIndexOpenRouterSpecificProvider + return { apiKey, specificProvider: specificProvider || undefined } + } + + private _readOpenAiCompatibleOptions(): CodeIndexConfig["openAiCompatibleOptions"] { + const baseUrl = + this.contextProxy?.getGlobalState("codebaseIndexConfig")?.codebaseIndexOpenAiCompatibleBaseUrl ?? "" + const apiKey = this.contextProxy?.getSecret("codebaseIndexOpenAiCompatibleApiKey") ?? "" + + if (!baseUrl || !apiKey) { + return undefined + } + + return { baseUrl, apiKey } + } + + private _resolveEmbedderProvider(provider: unknown): EmbedderProvider { + switch (provider) { + case providerIdentifiers.ollama: + case "openai-compatible": + case providerIdentifiers.gemini: + case providerIdentifiers.mistral: + case providerIdentifiers.vercelAiGateway: + case providerIdentifiers.bedrock: + case providerIdentifiers.openrouter: + case "semble": + return provider + default: + return providerIdentifiers.openai + } + } + + private _parseModelDimension(rawDimension: unknown): number | undefined { + if (rawDimension === undefined || rawDimension === null) { + return undefined + } + + const dimension = Number(rawDimension) + if (dimension > 0) { + return dimension + } + + console.warn(`Invalid codebaseIndexEmbedderModelDimension value: ${rawDimension}. Must be a positive number.`) + return undefined } /** @@ -165,132 +191,87 @@ export class CodeIndexConfigManager { */ public async loadConfiguration(): Promise<{ configSnapshot: PreviousConfigSnapshot - currentConfig: { - isConfigured: boolean - embedderProvider: EmbedderProvider - modelId?: string - modelDimension?: number - openAiOptions?: ApiHandlerOptions - ollamaOptions?: ApiHandlerOptions - openAiCompatibleOptions?: { baseUrl: string; apiKey: string } - geminiOptions?: { apiKey: string } - mistralOptions?: { apiKey: string } - vercelAiGatewayOptions?: { apiKey: string } - bedrockOptions?: { region: string; profile?: string } - openRouterOptions?: { apiKey: string } - qdrantUrl?: string - qdrantApiKey?: string - searchMinScore?: number - } + currentConfig: CodeIndexConfig requiresRestart: boolean }> { // Capture the ACTUAL previous state before loading new configuration - const previousConfigSnapshot: PreviousConfigSnapshot = { - enabled: this.codebaseIndexEnabled, - configured: this.isConfigured(), - embedderProvider: this.embedderProvider, - modelId: this.modelId, - modelDimension: this.modelDimension, - openAiKey: this.openAiOptions?.openAiNativeApiKey ?? "", - ollamaBaseUrl: this.ollamaOptions?.ollamaBaseUrl ?? "", - openAiCompatibleBaseUrl: this.openAiCompatibleOptions?.baseUrl ?? "", - openAiCompatibleApiKey: this.openAiCompatibleOptions?.apiKey ?? "", - geminiApiKey: this.geminiOptions?.apiKey ?? "", - mistralApiKey: this.mistralOptions?.apiKey ?? "", - vercelAiGatewayApiKey: this.vercelAiGatewayOptions?.apiKey ?? "", - bedrockRegion: this.bedrockOptions?.region ?? "", - bedrockProfile: this.bedrockOptions?.profile ?? "", - openRouterApiKey: this.openRouterOptions?.apiKey ?? "", - openRouterSpecificProvider: this.openRouterOptions?.specificProvider ?? "", - qdrantUrl: this.qdrantUrl ?? "", - qdrantApiKey: this.qdrantApiKey ?? "", - } + const previousConfigSnapshot = this._getRestartComparisonSnapshot() // Refresh secrets from VSCode storage to ensure we have the latest values await this.contextProxy.refreshSecrets() - // Load new configuration from storage and update instance variables - this._loadAndSetConfiguration() + // Publish only after the complete snapshot has been built. + this.config = this._readConfiguration() const requiresRestart = this.doesConfigChangeRequireRestart(previousConfigSnapshot) const result = { configSnapshot: previousConfigSnapshot, - currentConfig: { - isConfigured: this.isConfigured(), - embedderProvider: this.embedderProvider, - modelId: this.modelId, - modelDimension: this.modelDimension, - openAiOptions: this.openAiOptions, - ollamaOptions: this.ollamaOptions, - openAiCompatibleOptions: this.openAiCompatibleOptions, - geminiOptions: this.geminiOptions, - mistralOptions: this.mistralOptions, - vercelAiGatewayOptions: this.vercelAiGatewayOptions, - bedrockOptions: this.bedrockOptions, - openRouterOptions: this.openRouterOptions, - qdrantUrl: this.qdrantUrl, - qdrantApiKey: this.qdrantApiKey, - searchMinScore: this.currentSearchMinScore, - }, + currentConfig: this.getConfig(), requiresRestart, } this._isConfigurationLoaded = true return result } + private _getRestartComparisonSnapshot(): PreviousConfigSnapshot { + return { + enabled: this.config.codebaseIndexEnabled, + configured: this.isConfigured(), + embedderProvider: this.config.embedderProvider, + modelId: this.config.modelId, + modelDimension: this.config.modelDimension, + openAiKey: this.config.openAiOptions?.openAiNativeApiKey ?? "", + ollamaBaseUrl: this.config.ollamaOptions?.ollamaBaseUrl ?? "", + openAiCompatibleBaseUrl: this.config.openAiCompatibleOptions?.baseUrl ?? "", + openAiCompatibleApiKey: this.config.openAiCompatibleOptions?.apiKey ?? "", + geminiApiKey: this.config.geminiOptions?.apiKey ?? "", + mistralApiKey: this.config.mistralOptions?.apiKey ?? "", + vercelAiGatewayApiKey: this.config.vercelAiGatewayOptions?.apiKey ?? "", + bedrockRegion: this.config.bedrockOptions?.region ?? "", + bedrockProfile: this.config.bedrockOptions?.profile ?? "", + openRouterApiKey: this.config.openRouterOptions?.apiKey ?? "", + openRouterSpecificProvider: this.config.openRouterOptions?.specificProvider ?? "", + qdrantUrl: this.config.qdrantUrl ?? "", + qdrantApiKey: this.config.qdrantApiKey ?? "", + } + } + /** * Checks if the service is properly configured based on the embedder type. */ public isConfigured(): boolean { - if (this.embedderProvider === "semble") { - // Semble requires no API keys or Qdrant — it's always configured + if (this.config.embedderProvider === "semble") { + // Semble requires no API keys or Qdrant. return true } - if (this.embedderProvider === providerIdentifiers.openai) { - const openAiKey = this.openAiOptions?.openAiNativeApiKey - const qdrantUrl = this.qdrantUrl - return !!(openAiKey && qdrantUrl) - } else if (this.embedderProvider === providerIdentifiers.ollama) { - // Ollama model ID has a default, so only base URL is strictly required for config - const ollamaBaseUrl = this.ollamaOptions?.ollamaBaseUrl - const qdrantUrl = this.qdrantUrl - return !!(ollamaBaseUrl && qdrantUrl) - } else if (this.embedderProvider === "openai-compatible") { - const baseUrl = this.openAiCompatibleOptions?.baseUrl - const apiKey = this.openAiCompatibleOptions?.apiKey - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(baseUrl && apiKey && qdrantUrl) - return isConfigured - } else if (this.embedderProvider === providerIdentifiers.gemini) { - const apiKey = this.geminiOptions?.apiKey - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(apiKey && qdrantUrl) - return isConfigured - } else if (this.embedderProvider === providerIdentifiers.mistral) { - const apiKey = this.mistralOptions?.apiKey - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(apiKey && qdrantUrl) - return isConfigured - } else if (this.embedderProvider === providerIdentifiers.vercelAiGateway) { - const apiKey = this.vercelAiGatewayOptions?.apiKey - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(apiKey && qdrantUrl) - return isConfigured - } else if (this.embedderProvider === providerIdentifiers.bedrock) { - // Only region is required for Bedrock (profile is optional) - const region = this.bedrockOptions?.region - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(region && qdrantUrl) - return isConfigured - } else if (this.embedderProvider === providerIdentifiers.openrouter) { - const apiKey = this.openRouterOptions?.apiKey - const qdrantUrl = this.qdrantUrl - const isConfigured = !!(apiKey && qdrantUrl) - return isConfigured + if (!this.config.qdrantUrl) { + return false + } + + switch (this.config.embedderProvider) { + case providerIdentifiers.openai: + return !!this.config.openAiOptions?.openAiNativeApiKey + case providerIdentifiers.ollama: + // The model ID has a default, so only the base URL is required. + return !!this.config.ollamaOptions?.ollamaBaseUrl + case "openai-compatible": + return !!(this.config.openAiCompatibleOptions?.baseUrl && this.config.openAiCompatibleOptions?.apiKey) + case providerIdentifiers.gemini: + return !!this.config.geminiOptions?.apiKey + case providerIdentifiers.mistral: + return !!this.config.mistralOptions?.apiKey + case providerIdentifiers.vercelAiGateway: + return !!this.config.vercelAiGatewayOptions?.apiKey + case providerIdentifiers.bedrock: + // The profile is optional. + return !!this.config.bedrockOptions?.region + case providerIdentifiers.openrouter: + return !!this.config.openRouterOptions?.apiKey + default: + return false } - return false // Should not happen if embedderProvider is always set correctly } /** @@ -310,132 +291,60 @@ export class CodeIndexConfigManager { * - Non-functional configuration tweaks */ doesConfigChangeRequireRestart(prev: PreviousConfigSnapshot): boolean { - const nowConfigured = this.isConfigured() - - // Handle null/undefined values safely - const prevEnabled = prev?.enabled ?? false - const prevConfigured = prev?.configured ?? false - const prevProvider = prev?.embedderProvider ?? providerIdentifiers.openai - const prevOpenAiKey = prev?.openAiKey ?? "" - const prevOllamaBaseUrl = prev?.ollamaBaseUrl ?? "" - const prevOpenAiCompatibleBaseUrl = prev?.openAiCompatibleBaseUrl ?? "" - const prevOpenAiCompatibleApiKey = prev?.openAiCompatibleApiKey ?? "" - const prevModelDimension = prev?.modelDimension - const prevGeminiApiKey = prev?.geminiApiKey ?? "" - const prevMistralApiKey = prev?.mistralApiKey ?? "" - const prevVercelAiGatewayApiKey = prev?.vercelAiGatewayApiKey ?? "" - const prevBedrockRegion = prev?.bedrockRegion ?? "" - const prevBedrockProfile = prev?.bedrockProfile ?? "" - const prevOpenRouterApiKey = prev?.openRouterApiKey ?? "" - const prevOpenRouterSpecificProvider = prev?.openRouterSpecificProvider ?? "" - const prevQdrantUrl = prev?.qdrantUrl ?? "" - const prevQdrantApiKey = prev?.qdrantApiKey ?? "" - - // 1. Transition from disabled/unconfigured to enabled/configured - if ((!prevEnabled || !prevConfigured) && this.codebaseIndexEnabled && nowConfigured) { - return true - } - - // 2. Transition from enabled to disabled - if (prevEnabled && !this.codebaseIndexEnabled) { - return true - } - - // 3. If wasn't ready before and isn't ready now, no restart needed - if ((!prevEnabled || !prevConfigured) && (!this.codebaseIndexEnabled || !nowConfigured)) { - return false - } - - // 4. CRITICAL CHANGES - Always restart for these - // Only check for critical changes if feature is enabled - if (!this.codebaseIndexEnabled) { - return false - } - - // Provider change - if (prevProvider !== this.embedderProvider) { - return true - } - - // Authentication changes (API keys) - const currentOpenAiKey = this.openAiOptions?.openAiNativeApiKey ?? "" - const currentOllamaBaseUrl = this.ollamaOptions?.ollamaBaseUrl ?? "" - const currentOpenAiCompatibleBaseUrl = this.openAiCompatibleOptions?.baseUrl ?? "" - const currentOpenAiCompatibleApiKey = this.openAiCompatibleOptions?.apiKey ?? "" - const currentModelDimension = this.modelDimension - const currentGeminiApiKey = this.geminiOptions?.apiKey ?? "" - const currentMistralApiKey = this.mistralOptions?.apiKey ?? "" - const currentVercelAiGatewayApiKey = this.vercelAiGatewayOptions?.apiKey ?? "" - const currentBedrockRegion = this.bedrockOptions?.region ?? "" - const currentBedrockProfile = this.bedrockOptions?.profile ?? "" - const currentOpenRouterApiKey = this.openRouterOptions?.apiKey ?? "" - const currentOpenRouterSpecificProvider = this.openRouterOptions?.specificProvider ?? "" - const currentQdrantUrl = this.qdrantUrl ?? "" - const currentQdrantApiKey = this.qdrantApiKey ?? "" - - if (prevOpenAiKey !== currentOpenAiKey) { - return true - } - - if (prevOllamaBaseUrl !== currentOllamaBaseUrl) { - return true - } - - if ( - prevOpenAiCompatibleBaseUrl !== currentOpenAiCompatibleBaseUrl || - prevOpenAiCompatibleApiKey !== currentOpenAiCompatibleApiKey - ) { - return true - } - - if (prevGeminiApiKey !== currentGeminiApiKey) { - return true - } - - if (prevMistralApiKey !== currentMistralApiKey) { - return true - } + const current = this._getRestartComparisonSnapshot() + const wasEnabled = prev?.enabled ?? false + const wasReady = wasEnabled && (prev?.configured ?? false) + const isReady = current.enabled && current.configured - if (prevVercelAiGatewayApiKey !== currentVercelAiGatewayApiKey) { + if (!wasReady && isReady) { return true } - if (prevBedrockRegion !== currentBedrockRegion || prevBedrockProfile !== currentBedrockProfile) { + if (wasEnabled && !current.enabled) { return true } - if (prevOpenRouterApiKey !== currentOpenRouterApiKey) { - return true - } - - // OpenRouter specific provider change - if (prevOpenRouterSpecificProvider !== currentOpenRouterSpecificProvider) { - return true - } - - // Check for model dimension changes (generic for all providers) - if (prevModelDimension !== currentModelDimension) { - return true + if (!current.enabled || (!wasReady && !isReady)) { + return false } - if (prevQdrantUrl !== currentQdrantUrl || prevQdrantApiKey !== currentQdrantApiKey) { + const previousProvider = prev?.embedderProvider ?? providerIdentifiers.openai + if (previousProvider !== current.embedderProvider || prev?.modelDimension !== current.modelDimension) { return true } - // Vector dimension changes (still important for compatibility) - if (this._hasVectorDimensionChanged(prevProvider, prev?.modelId)) { - return true - } + return ( + this._hasConnectionSettingsChanged(prev, current) || + this._hasVectorDimensionChanged(previousProvider, prev?.modelId) + ) + } - return false + private _hasConnectionSettingsChanged(prev: PreviousConfigSnapshot, current: PreviousConfigSnapshot): boolean { + const keys = [ + "openAiKey", + "ollamaBaseUrl", + "openAiCompatibleBaseUrl", + "openAiCompatibleApiKey", + "geminiApiKey", + "mistralApiKey", + "vercelAiGatewayApiKey", + "bedrockRegion", + "bedrockProfile", + "openRouterApiKey", + "openRouterSpecificProvider", + "qdrantUrl", + "qdrantApiKey", + ] as const satisfies readonly (keyof PreviousConfigSnapshot)[] + + return keys.some((key) => (prev?.[key] ?? "") !== (current[key] ?? "")) } /** * Checks if model changes result in vector dimension changes that require restart. */ private _hasVectorDimensionChanged(prevProvider: EmbedderProvider, prevModelId?: string): boolean { - const currentProvider = this.embedderProvider - const currentModelId = this.modelId ?? getDefaultModelId(currentProvider) + const currentProvider = this.config.embedderProvider + const currentModelId = this.config.modelId ?? getDefaultModelId(currentProvider) const resolvedPrevModelId = prevModelId ?? getDefaultModelId(prevProvider) // If model IDs are the same and provider is the same, no dimension change @@ -460,31 +369,31 @@ export class CodeIndexConfigManager { * Gets the current configuration state. */ public getConfig(): CodeIndexConfig { - return { + return Object.freeze({ isConfigured: this.isConfigured(), - embedderProvider: this.embedderProvider, - modelId: this.modelId, - modelDimension: this.modelDimension, - openAiOptions: this.openAiOptions, - ollamaOptions: this.ollamaOptions, - openAiCompatibleOptions: this.openAiCompatibleOptions, - geminiOptions: this.geminiOptions, - mistralOptions: this.mistralOptions, - vercelAiGatewayOptions: this.vercelAiGatewayOptions, - bedrockOptions: this.bedrockOptions, - openRouterOptions: this.openRouterOptions, - qdrantUrl: this.qdrantUrl, - qdrantApiKey: this.qdrantApiKey, + embedderProvider: this.config.embedderProvider, + modelId: this.config.modelId, + modelDimension: this.config.modelDimension, + openAiOptions: this.config.openAiOptions, + ollamaOptions: this.config.ollamaOptions, + openAiCompatibleOptions: this.config.openAiCompatibleOptions, + geminiOptions: this.config.geminiOptions, + mistralOptions: this.config.mistralOptions, + vercelAiGatewayOptions: this.config.vercelAiGatewayOptions, + bedrockOptions: this.config.bedrockOptions, + openRouterOptions: this.config.openRouterOptions, + qdrantUrl: this.config.qdrantUrl, + qdrantApiKey: this.config.qdrantApiKey, searchMinScore: this.currentSearchMinScore, searchMaxResults: this.currentSearchMaxResults, - } + }) } /** * Gets whether the code indexing feature is enabled */ public get isFeatureEnabled(): boolean { - return this.codebaseIndexEnabled + return this.config.codebaseIndexEnabled } /** @@ -495,10 +404,10 @@ export class CodeIndexConfigManager { } /** - * Gets the current embedder type (openai or ollama) + * Gets the current embedder provider */ public get currentEmbedderProvider(): EmbedderProvider { - return this.embedderProvider + return this.config.embedderProvider } /** @@ -506,8 +415,8 @@ export class CodeIndexConfigManager { */ public get qdrantConfig(): { url?: string; apiKey?: string } { return { - url: this.qdrantUrl, - apiKey: this.qdrantApiKey, + url: this.config.qdrantUrl, + apiKey: this.config.qdrantApiKey, } } @@ -515,7 +424,7 @@ export class CodeIndexConfigManager { * Gets the current model ID being used for embeddings. */ public get currentModelId(): string | undefined { - return this.modelId + return this.config.modelId } /** @@ -524,12 +433,12 @@ export class CodeIndexConfigManager { */ public get currentModelDimension(): number | undefined { // First try to get the model-specific dimension - const modelId = this.modelId ?? getDefaultModelId(this.embedderProvider) - const modelDimension = getModelDimension(this.embedderProvider, modelId) + const modelId = this.config.modelId ?? getDefaultModelId(this.config.embedderProvider) + const modelDimension = getModelDimension(this.config.embedderProvider, modelId) // Only use custom dimension if model doesn't have a built-in dimension - if (!modelDimension && this.modelDimension && this.modelDimension > 0) { - return this.modelDimension + if (!modelDimension && this.config.modelDimension && this.config.modelDimension > 0) { + return this.config.modelDimension } return modelDimension @@ -541,13 +450,13 @@ export class CodeIndexConfigManager { */ public get currentSearchMinScore(): number { // First check if user has configured a custom score threshold - if (this.searchMinScore !== undefined) { - return this.searchMinScore + if (this.config.searchMinScore !== undefined) { + return this.config.searchMinScore } // Fall back to model-specific threshold - const currentModelId = this.modelId ?? getDefaultModelId(this.embedderProvider) - const modelSpecificThreshold = getModelScoreThreshold(this.embedderProvider, currentModelId) + const currentModelId = this.config.modelId ?? getDefaultModelId(this.config.embedderProvider) + const modelSpecificThreshold = getModelScoreThreshold(this.config.embedderProvider, currentModelId) return modelSpecificThreshold ?? DEFAULT_SEARCH_MIN_SCORE } @@ -556,6 +465,6 @@ export class CodeIndexConfigManager { * Returns user setting if configured, otherwise returns default. */ public get currentSearchMaxResults(): number { - return this.searchMaxResults ?? DEFAULT_MAX_SEARCH_RESULTS + return this.config.searchMaxResults ?? DEFAULT_MAX_SEARCH_RESULTS } } diff --git a/src/services/code-index/interfaces/config.ts b/src/services/code-index/interfaces/config.ts index f52f98aaa0..dc6986d3c6 100644 --- a/src/services/code-index/interfaces/config.ts +++ b/src/services/code-index/interfaces/config.ts @@ -1,48 +1,65 @@ -import { ApiHandlerOptions } from "../../../shared/api" // Adjust path if needed import { EmbedderProvider } from "./manager" /** * Configuration state for the code indexing feature */ export interface CodeIndexConfig { - isConfigured: boolean - embedderProvider: EmbedderProvider - modelId?: string - modelDimension?: number // Generic dimension property for all providers - openAiOptions?: ApiHandlerOptions - ollamaOptions?: ApiHandlerOptions - openAiCompatibleOptions?: { baseUrl: string; apiKey: string } - geminiOptions?: { apiKey: string } - mistralOptions?: { apiKey: string } - vercelAiGatewayOptions?: { apiKey: string } - bedrockOptions?: { region: string; profile?: string } - openRouterOptions?: { apiKey: string; specificProvider?: string } - qdrantUrl?: string - qdrantApiKey?: string - searchMinScore?: number - searchMaxResults?: number + readonly isConfigured: boolean + readonly embedderProvider: EmbedderProvider + readonly modelId?: string + readonly modelDimension?: number // Generic dimension property for all providers + readonly openAiOptions?: Readonly<{ openAiNativeApiKey?: string }> + readonly ollamaOptions?: Readonly<{ ollamaBaseUrl?: string }> + readonly openAiCompatibleOptions?: Readonly<{ baseUrl: string; apiKey: string }> + readonly geminiOptions?: Readonly<{ apiKey: string }> + readonly mistralOptions?: Readonly<{ apiKey: string }> + readonly vercelAiGatewayOptions?: Readonly<{ apiKey: string }> + readonly bedrockOptions?: Readonly<{ region: string; profile?: string }> + readonly openRouterOptions?: Readonly<{ apiKey: string; specificProvider?: string }> + readonly qdrantUrl?: string + readonly qdrantApiKey?: string + readonly searchMinScore?: number + readonly searchMaxResults?: number +} + +/** + * Stored configuration snapshot. Search defaults and readiness are derived by the manager. + * Every nested options object contains only scalar values and is readonly as well. + */ +export interface CodeIndexConfigSnapshot extends Omit { + readonly codebaseIndexEnabled: boolean +} + +/** Freeze the snapshot and its flat options objects before publishing it. */ +export function freezeCodeIndexConfigSnapshot(snapshot: CodeIndexConfigSnapshot): CodeIndexConfigSnapshot { + for (const value of Object.values(snapshot)) { + if (value !== null && typeof value === "object") { + Object.freeze(value) + } + } + return Object.freeze(snapshot) } /** * Snapshot of previous configuration used to determine if a restart is required */ export type PreviousConfigSnapshot = { - enabled: boolean - configured: boolean - embedderProvider: EmbedderProvider - modelId?: string - modelDimension?: number // Generic dimension property - openAiKey?: string - ollamaBaseUrl?: string - openAiCompatibleBaseUrl?: string - openAiCompatibleApiKey?: string - geminiApiKey?: string - mistralApiKey?: string - vercelAiGatewayApiKey?: string - bedrockRegion?: string - bedrockProfile?: string - openRouterApiKey?: string - openRouterSpecificProvider?: string - qdrantUrl?: string - qdrantApiKey?: string + readonly enabled: boolean + readonly configured: boolean + readonly embedderProvider: EmbedderProvider + readonly modelId?: string + readonly modelDimension?: number // Generic dimension property + readonly openAiKey?: string + readonly ollamaBaseUrl?: string + readonly openAiCompatibleBaseUrl?: string + readonly openAiCompatibleApiKey?: string + readonly geminiApiKey?: string + readonly mistralApiKey?: string + readonly vercelAiGatewayApiKey?: string + readonly bedrockRegion?: string + readonly bedrockProfile?: string + readonly openRouterApiKey?: string + readonly openRouterSpecificProvider?: string + readonly qdrantUrl?: string + readonly qdrantApiKey?: string } diff --git a/src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts b/src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts index 3a826826d4..d9f9ae4b50 100644 --- a/src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts +++ b/src/services/code-index/vector-store/__tests__/vector-store-factory.spec.ts @@ -43,14 +43,14 @@ describe("VectorStoreFactory", () => { }) it.each([undefined, 0, -1])("rejects an unavailable or invalid manual dimension: %s", (dimension) => { - config.modelDimension = dimension + config = { ...config, modelDimension: dimension } expect(() => factory.create(config, "/workspace")).toThrow("serviceFactory.vectorDimensionNotDetermined") expect(QdrantVectorStore).not.toHaveBeenCalled() }) it("rejects Semble before resolving dimensions or creating a store", () => { - config.embedderProvider = "semble" + config = { ...config, embedderProvider: "semble" } expect(() => factory.create(config, "/workspace")).toThrow("Semble provider handles its own vector storage") expect(getModelDimension).not.toHaveBeenCalled() @@ -68,8 +68,7 @@ describe("VectorStoreFactory", () => { }) it("reports the dimension error before a missing Qdrant URL", () => { - config.modelDimension = undefined - config.qdrantUrl = undefined + config = { ...config, modelDimension: undefined, qdrantUrl: undefined } expect(() => factory.create(config, "/workspace")).toThrow("serviceFactory.vectorDimensionNotDetermined") })