diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index cdb020ad80..7bb17e3b60 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -62,6 +62,7 @@ import { TurnBoundaryCompactionEngine } from './compactionEngine.js'; import { CHANNEL_STARTUP_PROFILE_META_KEY, CHANNEL_STARTUP_PROFILE_VERSION, + WORKTREE_MCP_DEFER_META_KEY, } from './bridgeTypes.js'; import { ApprovalMode, @@ -1046,6 +1047,24 @@ describe('createAcpSessionBridge', () => { }); }); + it('marks worktree session creation to defer MCP discovery', async () => { + const handle = makeChannel(); + const bridge = makeBridge({ + sessionScope: 'thread', + channelFactory: async () => handle.channel, + }); + + await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + worktree: { slug: 'task-a', path: WS_B, branch: 'worktree-task-a' }, + }); + + expect(handle.agent.newSessionCalls[0]?._meta).toMatchObject({ + [WORKTREE_MCP_DEFER_META_KEY]: true, + }); + await bridge.shutdown(); + }); + it('does not fail initialization when span enrichment throws', async () => { const handle = makeChannel({ initializeImpl: async () => ({ @@ -11519,6 +11538,15 @@ describe('createAcpSessionBridge', () => { aborted: false, }); expect(shellSpy).toHaveBeenCalledTimes(1); + expect(shellSpy).toHaveBeenCalledWith( + 'echo hello', + WS_A, + expect.any(Function), + expect.any(AbortSignal), + false, + { terminalWidth: 120, terminalHeight: 40 }, + { streamStdout: true }, + ); const it = events[Symbol.asyncIterator](); const first = await it.next(); expect(first.value?.type).toBe('user_shell_command'); @@ -11528,6 +11556,115 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); shellSpy.mockRestore(); }); + + it('executes direct shell in each session effective cwd', async () => { + const shellSpy = mockShellExecute(); + const handle = makeChannel({ + extMethodImpl: async (method, params) => { + if (method === SERVE_CONTROL_EXT_METHODS.sessionCd) { + return { + previousCwd: WS_A, + newCwd: (params as { path: string }).path, + warnings: [], + }; + } + return {}; + }, + }); + const bridge = makeBridge({ + sessionShellCommandEnabled: true, + channelFactory: async () => handle.channel, + }); + const firstSession = await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + sessionScope: 'thread', + }); + const secondSession = await bridge.spawnOrAttach({ + workspaceCwd: WS_A, + sessionScope: 'thread', + }); + + await Promise.all([ + bridge.changeSessionCwd(firstSession.sessionId, { path: WS_A }), + bridge.changeSessionCwd(secondSession.sessionId, { path: WS_B }), + ]); + await Promise.all([ + bridge.executeShellCommand( + firstSession.sessionId, + 'echo first', + undefined, + { clientId: firstSession.clientId }, + ), + bridge.executeShellCommand( + secondSession.sessionId, + 'echo second', + undefined, + { clientId: secondSession.clientId }, + ), + ]); + + expect(shellSpy).toHaveBeenCalledTimes(2); + expect( + shellSpy.mock.calls.map(([command, cwd]) => [command, cwd]), + ).toEqual( + expect.arrayContaining([ + ['echo first', WS_A], + ['echo second', WS_B], + ]), + ); + + await bridge.shutdown(); + shellSpy.mockRestore(); + }); + + it('waits for a pending cwd change before executing direct shell', async () => { + const shellSpy = mockShellExecute(); + const cdResult = deferred<{ + previousCwd: string; + newCwd: string; + warnings: string[]; + }>(); + const handle = makeChannel({ + extMethodImpl: async (method) => { + if (method === SERVE_CONTROL_EXT_METHODS.sessionCd) { + return cdResult.promise; + } + return {}; + }, + }); + const bridge = makeBridge({ + sessionShellCommandEnabled: true, + channelFactory: async () => handle.channel, + }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + + const cd = bridge.changeSessionCwd(session.sessionId, { path: WS_B }); + await vi.waitFor(() => + expect(handle.agent.extMethodCalls).toContainEqual({ + method: SERVE_CONTROL_EXT_METHODS.sessionCd, + params: { + sessionId: session.sessionId, + path: WS_B, + }, + }), + ); + const shell = bridge.executeShellCommand( + session.sessionId, + 'echo after-cd', + undefined, + { clientId: session.clientId }, + ); + + await Promise.resolve(); + expect(shellSpy).not.toHaveBeenCalled(); + cdResult.resolve({ previousCwd: WS_A, newCwd: WS_B, warnings: [] }); + await Promise.all([cd, shell]); + + expect(shellSpy.mock.calls[0]?.[1]).toBe(WS_B); + + await bridge.shutdown(); + shellSpy.mockRestore(); + }); }); describe('setSessionApprovalMode (#4175 Wave 4 PR 17)', () => { diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 1ecf00acc4..e895ec8f20 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -109,6 +109,7 @@ import { LOAD_REPLAY_VERSION, PROMPT_CANCEL_METHOD, TODO_STOP_GUARD_QUEUE_RELEASE_METHOD, + WORKTREE_MCP_DEFER_META_KEY, } from './bridgeTypes.js'; import { getChannelStartupProfileAttributes } from './channel-startup-profile.js'; import type { @@ -470,6 +471,7 @@ interface ChannelInfo { interface SessionEntry { sessionId: string; workspaceCwd: string; + effectiveCwd: string; createdAt: string; displayName?: string; /** Id of the session that spawned this one (via `create_sub_session`). @@ -493,6 +495,8 @@ interface SessionEntry { recordingDegraded: boolean; /** Set synchronously while agent-owned state and its writer lease close. */ closing: boolean; + /** Tail of cwd changes that direct shell commands must not overtake. */ + cwdChangeQueue: Promise; /** * Tail of the per-session prompt queue. Each new prompt chains off the * resolved (or rejected) state of this promise so prompts run one at a @@ -2645,17 +2649,24 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { async () => { // This legacy-named helper sanitizes and injects trace metadata // for any ACP request, not only prompts. + const request = telemetry.injectPromptContext({ + cwd: boundWorkspace, + mcpServers: [], + ...(sourceType + ? { _meta: sessionSourceRequestMeta(sourceType, sourceId) } + : {}), + }); const response = await withTimeout( ci.connection.newSession( - telemetry.injectPromptContext({ - cwd: boundWorkspace, - mcpServers: [], - ...(sourceType - ? { - _meta: sessionSourceRequestMeta(sourceType, sourceId), - } - : {}), - }), + worktree + ? { + ...request, + _meta: { + ...(isRecord(request._meta) ? request._meta : {}), + [WORKTREE_MCP_DEFER_META_KEY]: true, + }, + } + : request, ), initTimeoutMs, 'newSession', @@ -3879,6 +3890,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { const entry: SessionEntry = { sessionId, workspaceCwd, + effectiveCwd: workspaceCwd, createdAt: new Date().toISOString(), ...(options.parentSessionId ? { parentSessionId: options.parentSessionId } @@ -3897,6 +3909,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { }), recordingDegraded: false, closing: false, + cwdChangeQueue: Promise.resolve(), promptQueue: Promise.resolve(), pendingPromptCount: 0, pendingPromptList: [], @@ -6394,6 +6407,7 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { // State update inside the queue lambda — always executes when // the extMethod settles, regardless of caller timeout. + entry.effectiveCwd = extResult.newCwd; if (extResult.previousCwd !== extResult.newCwd) { entry.events.publish({ type: 'session_cwd_changed', @@ -6415,6 +6429,10 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { () => undefined, () => undefined, ); + entry.cwdChangeQueue = cdPromise.then( + () => undefined, + () => undefined, + ); // Timeout is caller-facing only: surfaces a deadline exceeded error // to the HTTP client without advancing the queue prematurely. @@ -7773,7 +7791,11 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return { exitCode: null, output: '', aborted: true }; } - const cwd = entry.workspaceCwd; + await entry.cwdChangeQueue; + if (signal?.aborted) { + return { exitCode: null, output: '', aborted: true }; + } + const cwd = entry.effectiveCwd; entry.events.publish({ type: 'user_shell_command', diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index e39fd207a5..dac7660bdf 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -171,6 +171,7 @@ export const LOAD_REPLAY_VERSION = 1 as const; export const CHANNEL_STARTUP_PROFILE_META_KEY = 'qwen.daemon.channelStartupProfile'; export const CHANNEL_STARTUP_PROFILE_VERSION = 1 as const; +export const WORKTREE_MCP_DEFER_META_KEY = 'qwen.session.deferMcpDiscovery'; export interface ChannelStartupProfileV1 { v: typeof CHANNEL_STARTUP_PROFILE_VERSION; diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index c371e3a6d1..793803a71d 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -868,6 +868,7 @@ import { CHANNEL_STARTUP_PROFILE_VERSION, PROMPT_CANCEL_METHOD, TODO_STOP_GUARD_QUEUE_RELEASE_METHOD, + WORKTREE_MCP_DEFER_META_KEY, } from '@qwen-code/acp-bridge/bridgeTypes'; import { initializeAcpStartupProfiler, @@ -3733,6 +3734,24 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('defers MCP discovery for a worktree session until relocation', async () => { + const innerConfig = await setupSessionMocks('worktree-mcp-session'); + const { agent, agentPromise } = await bootAcpAgent(); + + await agent.newSession({ + cwd: '/tmp', + mcpServers: [], + _meta: { [WORKTREE_MCP_DEFER_META_KEY]: true }, + }); + + expect(innerConfig.initialize).toHaveBeenCalledWith( + expect.objectContaining({ skipMcpDiscovery: true }), + ); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('serializes a working-directory change and hard-suspends Todo Stop Guard', async () => { const sessionId = '11111111-1111-1111-1111-111111111111'; const targetDir = await fs.mkdtemp( @@ -3778,6 +3797,45 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('reports an MCP refresh warning after changing the working directory', async () => { + const sessionId = '11111111-1111-1111-1111-111111111111'; + const targetDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'qwen-mcp-refresh-cwd-'), + ); + const canonicalTargetDir = await fs.realpath(targetDir); + const innerConfig = await setupSessionMocks(sessionId); + Object.assign(innerConfig, { + getTargetDir: vi.fn().mockReturnValue('/tmp'), + isRestrictiveSandbox: vi.fn().mockReturnValue(false), + relocateWorkingDirectory: vi.fn().mockResolvedValue({ + mcpRefreshError: new Error('MCP failed'), + }), + }); + Object.assign(innerConfig.getGeminiClient(), { + addWorkingDirectoryChangedContext: vi.fn().mockResolvedValue(undefined), + }); + const { agent, agentPromise } = await bootAcpAgent(); + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + try { + await expect( + agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionCd, { + sessionId, + path: targetDir, + }), + ).resolves.toEqual({ + previousCwd: '/tmp', + newCwd: canonicalTargetDir, + warnings: ['MCP refresh failed: MCP failed'], + }); + } finally { + await fs.rm(targetDir, { recursive: true, force: true }); + } + + mockConnectionState.resolve(); + await agentPromise; + }); + it('rechecks a no-op working-directory change after a concurrent relocation', async () => { const sessionId = '11111111-1111-1111-1111-111111111111'; const oldDir = await fs.mkdtemp( @@ -11885,6 +11943,43 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it('keeps ACP stdio MCP cwd implicit so session relocation can rebind it', async () => { + await setupSessionMocks('session-stdio-cwd'); + const agentPromise = runAcpAgent( + mockConfig, + makeSessionSettings(), + mockArgv, + ); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ + cwd: '/tmp', + mcpServers: [ + { + name: 'local', + command: 'node', + args: ['server.js'], + env: [], + } as unknown as McpServer, + ], + }); + + const sessionMcpServers = vi.mocked(loadCliConfig).mock.calls[0]?.[6]; + const localConfig = sessionMcpServers?.['local'] as unknown as { + _args: unknown[]; + }; + expect(localConfig._args).toEqual(['node', ['server.js'], {}]); + expect(localConfig._args[3]).toBeUndefined(); + + mockConnectionState.resolve(); + await agentPromise; + }); + it('passes undefined (not []) as the extension override to loadCliConfig', async () => { await setupSessionMocks('session-ext-override'); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index 1448c27abb..ba108029cd 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -306,6 +306,7 @@ import { LOAD_REPLAY_VERSION, PROMPT_CANCEL_METHOD, TODO_STOP_GUARD_QUEUE_RELEASE_METHOD, + WORKTREE_MCP_DEFER_META_KEY, type ClientMcpOverWsRuntimeConfig, type BridgeLoadReplayEnvelope, } from '@qwen-code/acp-bridge/bridgeTypes'; @@ -632,6 +633,11 @@ function isChannelSessionRequest(params: { _meta?: unknown }): boolean { return isObjectRecord(value) && value['sourceType'] === 'channel'; } +function shouldDeferMcpDiscovery(params: { _meta?: unknown }): boolean { + const meta = isObjectRecord(params._meta) ? params._meta : undefined; + return meta?.[WORKTREE_MCP_DEFER_META_KEY] === true; +} + function getLoadReplayPageSize(params: LoadSessionRequest): number | undefined { const meta = isObjectRecord(params._meta) ? params._meta : undefined; const value = meta?.[LOAD_REPLAY_PAGE_SIZE_META_KEY]; @@ -4354,7 +4360,17 @@ class QwenAgent implements Agent { ); this.settings = settings; const config = await profiler.time('config_setup', () => - this.newSessionConfig(cwd, mcpServers, settings, isChannelSession), + this.newSessionConfig( + cwd, + mcpServers, + settings, + isChannelSession, + undefined, + undefined, + shouldDeferMcpDiscovery(params) + ? { skipMcpDiscovery: true } + : undefined, + ), ); let session: Session; try { @@ -8940,6 +8956,15 @@ class QwenAgent implements Agent { }`, ); } + if (relocation.mcpRefreshError) { + warnings.push( + `MCP refresh failed: ${ + relocation.mcpRefreshError instanceof Error + ? relocation.mcpRefreshError.message + : String(relocation.mcpRefreshError) + }`, + ); + } try { await config @@ -10699,7 +10724,6 @@ class QwenAgent implements Agent { stdioServer.command, stdioServer.args, env, - cwd, ); continue; } diff --git a/packages/cli/src/ui/commands/cdCommand.test.ts b/packages/cli/src/ui/commands/cdCommand.test.ts index 2bc53ca4e0..28c5592c1c 100644 --- a/packages/cli/src/ui/commands/cdCommand.test.ts +++ b/packages/cli/src/ui/commands/cdCommand.test.ts @@ -339,6 +339,24 @@ describe('cdCommand', () => { }); }); + it('reports a successful move when MCP refresh fails afterward', async () => { + relocateWorkingDirectory.mockResolvedValue({ + mcpRefreshError: new Error('MCP failed'), + }); + + const result = (await cdCommand.action?.( + context, + '../next', + )) as MessageActionReturn; + const realNextDir = await realpath(nextDir); + + expect(result).toEqual({ + type: 'message', + messageType: 'warning', + content: `Moved to ${realNextDir}. MCP refresh failed: MCP failed`, + }); + }); + it('asks for confirmation before moving to an untrusted directory', async () => { context = createMockCommandContext({ invocation: { diff --git a/packages/cli/src/ui/commands/cdCommand.ts b/packages/cli/src/ui/commands/cdCommand.ts index 803827af3d..08d0e1ee06 100644 --- a/packages/cli/src/ui/commands/cdCommand.ts +++ b/packages/cli/src/ui/commands/cdCommand.ts @@ -179,6 +179,15 @@ export const cdCommand: SlashCommand = { }`, ); } + if (relocation.mcpRefreshError) { + warnings.push( + `MCP refresh failed: ${ + relocation.mcpRefreshError instanceof Error + ? relocation.mcpRefreshError.message + : String(relocation.mcpRefreshError) + }`, + ); + } } catch (error) { return { type: 'message' as const, diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 5c049d26a7..d9ec4de2aa 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -1874,6 +1874,30 @@ describe('Server Config (config.ts)', () => { expect(Object.keys(result!)).not.toContain('playwright'); }); + it('binds command MCP servers without an explicit cwd to the session target directory', () => { + const explicitCwd = path.resolve('/explicit/mcp'); + const config = new Config({ + ...baseParams, + targetDir: path.resolve('/session/worktree'), + mcpServers: { + implicit: { command: 'node', args: ['server.js'] }, + explicit: { command: 'node', cwd: explicitCwd }, + remote: { httpUrl: 'https://example.test/mcp' }, + sdk: { type: 'sdk', command: 'placeholder' }, + }, + }); + + expect(config.getMcpServers()).toMatchObject({ + implicit: { cwd: path.resolve('/session/worktree') }, + explicit: { cwd: explicitCwd }, + remote: { httpUrl: 'https://example.test/mcp' }, + sdk: { type: 'sdk', command: 'placeholder' }, + }); + expect(config.getSettingsMcpServers()?.['implicit']?.cwd).toBeUndefined(); + expect(config.getMcpServers()?.['remote']?.cwd).toBeUndefined(); + expect(config.getMcpServers()?.['sdk']?.cwd).toBeUndefined(); + }); + it('isMcpServerDisabled supports glob patterns in excludedMcpServers', () => { const config = new Config({ ...baseParams, @@ -5182,6 +5206,65 @@ describe('Server Config (config.ts)', () => { cwdSpy.mockRestore(); }); + it('relocateWorkingDirectory should reconcile MCP servers with the new session cwd', async () => { + const config = new Config({ + ...baseParams, + mcpServers: { local: { command: 'node', args: ['server.js'] } }, + }); + await config.initialize(); + const manager = ( + config.getToolRegistry() as unknown as { + __mcpManagerMock: { discoverAllMcpToolsIncremental: Mock }; + } + ).__mcpManagerMock; + await config.waitForMcpReady(); + manager.discoverAllMcpToolsIncremental.mockClear(); + 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); + + await expect(config.relocateWorkingDirectory(newDir)).resolves.toEqual({}); + + expect(config.getMcpServers()?.['local']?.cwd).toBe(newDir); + expect(manager.discoverAllMcpToolsIncremental).toHaveBeenCalledOnce(); + expect(manager.discoverAllMcpToolsIncremental).toHaveBeenCalledWith(config); + + chdirSpy.mockRestore(); + cwdSpy.mockRestore(); + }); + + it('relocateWorkingDirectory should report MCP reconcile 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'), + ); + 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.mcpRefreshError).toEqual(new Error('MCP failed')); + + chdirSpy.mockRestore(); + cwdSpy.mockRestore(); + }); + it('relocateWorkingDirectory should continue after recording flush fails', async () => { const config = new Config(baseParams); const newDir = path.resolve('/path/to/other'); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 68c013ca4b..6b76bd3852 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -4504,7 +4504,10 @@ export class Config { newDir: string, expectedCanonicalDir?: string, opts?: { skipProcessChdir?: boolean; skipArtifactMigration?: boolean }, - ): Promise<{ memoryRefreshError?: unknown }> { + ): Promise<{ + memoryRefreshError?: unknown; + mcpRefreshError?: unknown; + }> { if ( !opts?.skipArtifactMigration && this.chatRecordingService?.hasWriteOwnership() @@ -4569,12 +4572,25 @@ export class Config { this.fileHistoryService = undefined; this.getFileReadCache().clear(); + let memoryRefreshError: unknown; try { await this.refreshHierarchicalMemory(); - return {}; } catch (error) { - return { memoryRefreshError: error }; + memoryRefreshError = error; } + + let mcpRefreshError: unknown; + try { + await this.waitForMcpReady(); + await this.refreshMcpServers(); + } catch (error) { + mcpRefreshError = error; + } + + return { + ...(memoryRefreshError !== undefined && { memoryRefreshError }), + ...(mcpRefreshError !== undefined && { mcpRefreshError }), + }; } /** @@ -5052,7 +5068,18 @@ export class Config { // The UI layer should check isMcpServerDisabled() to determine // whether to show a server as disabled. - return mcpServers; + return Object.fromEntries( + Object.entries(mcpServers).map(([name, server]) => [ + name, + server.command !== undefined && + server.httpUrl === undefined && + server.url === undefined && + server.type !== 'sdk' && + server.cwd === undefined + ? { ...server, cwd: this.targetDir } + : server, + ]), + ); } getExcludedMcpServers(): string[] | undefined { @@ -5267,6 +5294,10 @@ export class Config { this.recentlyRemovedMcpServers.add(name); } } + await this.refreshMcpServers(); + } + + private async refreshMcpServers(): Promise { if (!this.initialized) { // No tool registry yet — boot-time discovery will pick up the new map. this.debugLogger.debug( @@ -5274,13 +5305,12 @@ export class Config { ); return; } + if (this.mcpReconcileInProgress) { // Coalesce: a pass is already running. Mark that the desired state // advanced so its drain loop runs again with the latest config, and // await that in-flight pass — NOT a resolved promise — so this caller - // does not proceed (e.g. the hot-reload listener emitting approval events - // and logging "complete") before its coalesced change is actually - // reconciled, and so it observes a shared reconcile failure. + // does not proceed before its coalesced change is actually reconciled. this.mcpReconcilePending = true; this.debugLogger.debug( '[mcp-hot-reload] reconcile already in flight — coalescing into a follow-up pass', @@ -5289,8 +5319,7 @@ export class Config { } this.mcpReconcileInProgress = true; const registry = this.getToolRegistry(); - // Run pass 1 + its drain loop as a single promise, assigned BEFORE the - // first await so a coalesced caller arriving mid-flight can await it. + // Assign before the first await so a coalesced caller can await this pass. const runReconcile = (async () => { try { this.debugLogger.debug( @@ -5299,9 +5328,8 @@ export class Config { await registry .getMcpClientManager() .discoverAllMcpToolsIncremental(this); - // Drain any change that arrived while this pass was in flight. The pool - // path returns the in-flight promise rather than queuing, so awaiting - // is not enough — re-run once more to pick up the latest config. + // The pool path returns an in-flight promise, so re-run after any + // coalesced change to ensure the latest effective config is applied. let pass = 1; while (this.mcpReconcilePending) { this.mcpReconcilePending = false; @@ -5325,17 +5353,13 @@ export class Config { throw err; } finally { this.mcpReconcileInProgress = false; - // Clear the coalesce flag too: if a pass threw, a pending follow-up - // would otherwise stay stuck `true` and make the next (unrelated) - // reconcile run an extra no-op drain pass. The next real settings - // change re-triggers reconcile anyway. + // A failed pass must not leak a pending drain into the next reconcile. this.mcpReconcilePending = false; this.mcpReconcilePromise = undefined; } })(); this.mcpReconcilePromise = runReconcile; - // Propagate failure to this caller (and, via the shared promise, to any - // coalesced callers). Existing callers rely on the throw. + // Propagate failure to this caller and every coalesced caller. await runReconcile; } diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index bc5fd4ac85..ef1e33b346 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -9,7 +9,7 @@ import { McpClientManager, type McpClientManagerOptions, } from './mcp-client-manager.js'; -import { McpClient } from './mcp-client.js'; +import { McpClient, populateMcpServerCommand } from './mcp-client.js'; import type { ToolRegistry } from './tool-registry.js'; import { MCPServerConfig, type Config } from '../config/config.js'; import type { PromptRegistry } from '../prompts/prompt-registry.js'; @@ -53,6 +53,7 @@ function mkManager( isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -122,6 +123,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -134,6 +136,11 @@ describe('McpClientManager', () => { options: { pool: fakePool }, }); await manager.discoverAllMcpTools(mockConfig); + expect(populateMcpServerCommand).toHaveBeenCalledWith( + { srv: {} }, + undefined, + '/session/worktree', + ); expect(acquireSpy).toHaveBeenCalledTimes(1); expect(acquireSpy).toHaveBeenCalledWith( 'srv', @@ -185,6 +192,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srvA: {}, srvB: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -220,6 +228,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ gated: {}, ok: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -254,6 +263,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer }), getResourceRegistry: () => ({ removeResourcesByServer }), getWorkspaceContext: () => ({}), @@ -283,6 +293,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer }), getResourceRegistry: () => ({ removeResourcesByServer }), getWorkspaceContext: () => ({}), @@ -342,6 +353,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -401,6 +413,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -447,6 +460,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -500,6 +514,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -559,6 +574,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -592,6 +608,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -634,6 +651,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -695,6 +713,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -762,6 +781,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -812,6 +832,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -854,6 +875,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -880,6 +902,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -914,6 +937,7 @@ describe('McpClientManager', () => { 'without-instructions': {}, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -943,6 +967,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => false, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -969,6 +994,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => false, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -999,6 +1025,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'pending-server': { scope: 'project' } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -1029,6 +1056,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'approved-server': { scope: 'project' } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), @@ -1061,6 +1089,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {}, 'another-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1099,6 +1128,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1133,6 +1163,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1175,6 +1206,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1237,6 +1269,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1308,6 +1341,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1377,6 +1411,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'test-server': {} }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1428,6 +1463,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1467,6 +1503,7 @@ describe('McpClientManager', () => { broken: { command: 'node', args: [], discoveryTimeoutMs: 50 }, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1539,6 +1576,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ oauth: serverConfig }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1589,6 +1627,7 @@ describe('McpClientManager', () => { disabled: { command: 'node', args: [] }, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1634,6 +1673,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ foo: { command: 'node', args: [] } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1693,6 +1733,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ foo: { command: 'node', args } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer }), @@ -1757,6 +1798,7 @@ describe('McpClientManager', () => { // identical; only the per-session filter changes. getMcpServers: () => ({ foo: { command: 'node', includeTools } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1812,6 +1854,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ foo: { command: 'node', args } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer }), @@ -1873,6 +1916,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ 'broken-auth': { command: 'node', args: [] } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1932,6 +1976,7 @@ describe('McpClientManager', () => { huge: { command: 'node', args: [], discoveryTimeoutMs: 10_000_000 }, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -1989,6 +2034,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ wsServer: { tcp: 'ws://example.test' } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2031,6 +2077,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ slow: serverConfig }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2152,6 +2199,7 @@ describe('McpClientManager', () => { slow: { command: 'node', args: [], discoveryTimeoutMs: 100 }, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2208,6 +2256,7 @@ describe('McpClientManager', () => { slow: { command: 'node', args: [], discoveryTimeoutMs: 100 }, }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2276,6 +2325,7 @@ describe('McpClientManager', () => { isTrustedFolder: () => true, getMcpServers: () => ({ srv: { command: 'node', args: [] } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2342,6 +2392,7 @@ describe('McpClientManager — PR 14 guardrails', () => { isTrustedFolder: () => true, getMcpServers: () => servers, getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2673,6 +2724,7 @@ describe('McpClientManager — PR 14 guardrails', () => { isTrustedFolder: () => true, getMcpServers: () => mcpServers, getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -2844,6 +2896,7 @@ describe('McpClientManager — PR 14 guardrails', () => { isTrustedFolder: () => true, getMcpServers: () => ({ foo: { command: 'node', args } }), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer }), @@ -3485,6 +3538,7 @@ describe('McpClientManager — PR 14b push events + hysteresis', () => { isTrustedFolder: () => true, getMcpServers: () => servers, getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }) as unknown as PromptRegistry, getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), @@ -4014,6 +4068,7 @@ describe('McpClientManager — addRuntimeMcpServer / removeRuntimeMcpServer (T2. isTrustedFolder: () => true, getMcpServers: () => ({}), getMcpServerCommand: () => undefined, + getTargetDir: () => '/session/worktree', getResourceRegistry: () => ({ removeResourcesByServer: vi.fn() }), getPromptRegistry: () => ({ removePromptsByServer: vi.fn() }), getWorkspaceContext: () => ({}), diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index d396fe6979..5f44625d49 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -1059,6 +1059,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); // mark the bulk pass active @@ -1226,6 +1227,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); const serverConfig = servers[serverName]; if (!serverConfig) { @@ -1264,6 +1266,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); const serverConfig = servers[serverName]; if (!serverConfig) { @@ -1564,6 +1567,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); // diff against the // current `pooledConnections` instead of releasing all then @@ -2121,6 +2125,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); // suppress per-server @@ -2624,6 +2629,7 @@ export class McpClientManager { const servers = populateMcpServerCommand( this.cliConfig.getMcpServers() || {}, this.cliConfig.getMcpServerCommand(), + this.cliConfig.getTargetDir(), ); const serverConfig = servers[serverName]; if (this.cliConfig.isMcpServerDisabled(serverName)) { diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index 4eba6b6aba..dca5c7b49c 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -2340,11 +2340,13 @@ lOTTGqPpwFUbw2EMOOpFYuIyzGMIpUNMBjE2gvJiqFQ= it('should discover tools via mcpServerCommand', () => { const commandString = 'command --arg1 value1'; - const out = populateMcpServerCommand({}, commandString); + const cwd = '/session/worktree'; + const out = populateMcpServerCommand({}, commandString, cwd); expect(out).toEqual({ mcp: { command: 'command', args: ['--arg1', 'value1'], + cwd, }, }); }); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 39af68fa39..c504807547 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -1159,7 +1159,11 @@ export async function discoverMcpTools( ): Promise { mcpDiscoveryState = MCPDiscoveryState.IN_PROGRESS; try { - mcpServers = populateMcpServerCommand(mcpServers, mcpServerCommand); + mcpServers = populateMcpServerCommand( + mcpServers, + mcpServerCommand, + cliConfig.getTargetDir(), + ); const discoveryPromises = Object.entries(mcpServers).map( ([mcpServerName, mcpServerConfig]) => @@ -1183,6 +1187,7 @@ export async function discoverMcpTools( export function populateMcpServerCommand( mcpServers: Record, mcpServerCommand: string | undefined, + cwd?: string, ): Record { if (mcpServerCommand) { const cmd = mcpServerCommand; @@ -1194,6 +1199,7 @@ export function populateMcpServerCommand( mcpServers['mcp'] = { command: args[0], args: args.slice(1), + cwd, }; } return mcpServers;