From 82b2e4b494d915e6148c680daaf2d643ee67fc43 Mon Sep 17 00:00:00 2001 From: Qwen Code Bot Date: Thu, 30 Jul 2026 07:36:12 +0000 Subject: [PATCH] refactor(core): centralize effective MCP server resolution (#8068) Extract McpClientManager.getEffectiveMcpServers() as the single source of truth for the populateMcpServerCommand recipe, replacing six duplicated call sites that this PR otherwise had to edit in lockstep. Also cover relocateWorkingDirectory's combined memory + MCP refresh failure path so both errors stay surfaced. --- packages/core/src/config/config.test.ts | 34 +++++++++++++ packages/core/src/tools/mcp-client-manager.ts | 50 ++++++++----------- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index d9ec4de2aa..dbc8ec3aba 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -5596,6 +5596,40 @@ describe('Server Config (config.ts)', () => { cwdSpy.mockRestore(); }); + it('relocateWorkingDirectory should report both memory and MCP refresh failures after moving', async () => { + const config = new Config({ + ...baseParams, + mcpServers: { local: { command: 'node' } }, + }); + await config.initialize(); + const manager = ( + config.getToolRegistry() as unknown as { + __mcpManagerMock: { discoverAllMcpToolsIncremental: Mock }; + } + ).__mcpManagerMock; + await config.waitForMcpReady(); + manager.discoverAllMcpToolsIncremental.mockRejectedValueOnce( + new Error('MCP failed'), + ); + vi.mocked(loadServerHierarchicalMemory).mockRejectedValueOnce( + new Error('memory failed'), + ); + const newDir = path.resolve('/path/to/other'); + const chdirSpy = vi.spyOn(process, 'chdir').mockImplementation(() => { + // Keep the test process in its original directory. + }); + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(newDir); + + const result = await config.relocateWorkingDirectory(newDir); + + expect(config.getTargetDir()).toBe(newDir); + expect(result.memoryRefreshError).toEqual(new Error('memory failed')); + expect(result.mcpRefreshError).toEqual(new Error('MCP failed')); + + chdirSpy.mockRestore(); + cwdSpy.mockRestore(); + }); + it('refreshHierarchicalMemory should include empty memory prompt when no managed auto-memory index exists', async () => { const config = new Config(baseParams); diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 5f44625d49..06f9630f6a 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -1034,6 +1034,20 @@ export class McpClientManager { } } + /** + * Single source of truth for the effective server map: the configured + * servers plus the `mcpServerCommand`-derived `mcp` server, each stamped + * with the session target dir as its cwd. Every discovery entry point + * resolves servers through here so the recipe cannot diverge. + */ + private getEffectiveMcpServers(): Record { + return populateMcpServerCommand( + this.cliConfig.getMcpServers() || {}, + this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), + ); + } + /** * Initiates the tool discovery process for all configured MCP servers. * It connects to each server, discovers its available tools, and registers @@ -1056,11 +1070,7 @@ export class McpClientManager { } await this.stop(); - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); // mark the bulk pass active // so per-server `emitRefusedBatchIfAny` calls (which the inner @@ -1224,11 +1234,7 @@ export class McpClientManager { serverName: string, cliConfig: Config, ): Promise { - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); const serverConfig = servers[serverName]; if (!serverConfig) { return; @@ -1263,11 +1269,7 @@ export class McpClientManager { serverName: string, cliConfig: Config, ): Promise { - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); const serverConfig = servers[serverName]; if (!serverConfig) { return; @@ -1564,11 +1566,7 @@ export class McpClientManager { const sessionId = this.cliConfig.getSessionId(); const promptRegistry = this.cliConfig.getPromptRegistry(); const resourceRegistry = this.cliConfig.getResourceRegistry(); - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); // diff against the // current `pooledConnections` instead of releasing all then // re-acquiring everything. Pre-fix every incremental discovery @@ -2122,11 +2120,7 @@ export class McpClientManager { return this.discoverAllMcpToolsViaPool(cliConfig); } - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); // suppress per-server // length-1 batches inside this incremental pass — the @@ -2626,11 +2620,7 @@ export class McpClientManager { uri: string, options?: { signal?: AbortSignal }, ): Promise { - const servers = populateMcpServerCommand( - this.cliConfig.getMcpServers() || {}, - this.cliConfig.getMcpServerCommand(), - this.cliConfig.getTargetDir(), - ); + const servers = this.getEffectiveMcpServers(); const serverConfig = servers[serverName]; if (this.cliConfig.isMcpServerDisabled(serverName)) { throw new Error(`MCP server '${serverName}' is disabled.`);