Resolve cwd for analyzers/rewriters in single place

Part of #274496

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineAnalyzer/commandLineFileWriteAnalyzer.ts

@@ -8,7 +8,6 @@ import { URI } from '../../../../../../../base/common/uri.js';
 import { localize } from '../../../../../../../nls.js';
 import { IConfigurationService } from '../../../../../../../platform/configuration/common/configuration.js';
 import { IWorkspaceContextService } from '../../../../../../../platform/workspace/common/workspace.js';
-import { IHistoryService } from '../../../../../../services/history/common/history.js';
 import { TerminalChatAgentToolsSettingId } from '../../../common/terminalChatAgentToolsConfiguration.js';
 import type { TreeSitterCommandParser } from '../../treeSitterCommandParser.js';
 import type { ICommandLineAnalyzer, ICommandLineAnalyzerOptions, ICommandLineAnalyzerResult } from './commandLineAnalyzer.js';
@@ -18,7 +17,6 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand
 		private readonly _treeSitterCommandParser: TreeSitterCommandParser,
 		private readonly _log: (message: string, ...args: unknown[]) => void,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
-		@IHistoryService private readonly _historyService: IHistoryService,
 		@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
 	) {
 		super();
@@ -34,12 +32,7 @@ export class CommandLineFileWriteAnalyzer extends Disposable implements ICommand
 		// TODO: Handle environment variables https://github.com/microsoft/vscode/issues/274166
 		// TODO: Handle command substitions/complex destinations https://github.com/microsoft/vscode/issues/274167
 		if (capturedFileWrites.length) {
-			let cwd = options.cwd;
-			if (!cwd) {
-				const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot();
-				const workspaceFolder = activeWorkspaceRootUri ? this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri) ?? undefined : undefined;
-				cwd = workspaceFolder?.uri;
-			}
+			const cwd = options.cwd;
 			if (cwd) {
 				fileWrites = capturedFileWrites.map(e => URI.joinPath(cwd, e));
 			} else {

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/commandLineRewriter/commandLineCdPrefixRewriter.ts

@@ -5,19 +5,15 @@
 
 import { Disposable } from '../../../../../../../base/common/lifecycle.js';
 import { OperatingSystem } from '../../../../../../../base/common/platform.js';
-import type { URI } from '../../../../../../../base/common/uri.js';
-import { IWorkspaceContextService } from '../../../../../../../platform/workspace/common/workspace.js';
 import { isPowerShell } from '../../runInTerminalHelpers.js';
 import type { ICommandLineRewriter, ICommandLineRewriterOptions, ICommandLineRewriterResult } from './commandLineRewriter.js';
 
 export class CommandLineCdPrefixRewriter extends Disposable implements ICommandLineRewriter {
-	constructor(
-		@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
-	) {
-		super();
-	}
-
 	rewrite(options: ICommandLineRewriterOptions): ICommandLineRewriterResult | undefined {
+		if (!options.cwd) {
+			return undefined;
+		}
+
 		const isPwsh = isPowerShell(options.shell, options.os);
 
 		// Re-write the command if it starts with `cd <dir> && <suffix>` or `cd <dir>; <suffix>`
@@ -31,34 +27,21 @@ export class CommandLineCdPrefixRewriter extends Disposable implements ICommandL
 		const cdDir = cdPrefixMatch?.groups?.dir;
 		const cdSuffix = cdPrefixMatch?.groups?.suffix;
 		if (cdDir && cdSuffix) {
-			let cwd: URI | undefined = options.cwd;
-
-			// If a terminal is not available, use the workspace root
-			if (!cwd) {
-				const workspaceFolders = this._workspaceContextService.getWorkspace().folders;
-				if (workspaceFolders.length === 1) {
-					cwd = workspaceFolders[0].uri;
-				}
+			// Remove any surrounding quotes
+			let cdDirPath = cdDir;
+			if (cdDirPath.startsWith('"') && cdDirPath.endsWith('"')) {
+				cdDirPath = cdDirPath.slice(1, -1);
 			}
-
-			// Re-write the command if it matches the cwd
-			if (cwd) {
-				// Remove any surrounding quotes
-				let cdDirPath = cdDir;
-				if (cdDirPath.startsWith('"') && cdDirPath.endsWith('"')) {
-					cdDirPath = cdDirPath.slice(1, -1);
-				}
-				// Normalize trailing slashes
-				cdDirPath = cdDirPath.replace(/(?:[\\\/])$/, '');
-				let cwdFsPath = cwd.fsPath.replace(/(?:[\\\/])$/, '');
-				// Case-insensitive comparison on Windows
-				if (options.os === OperatingSystem.Windows) {
-					cdDirPath = cdDirPath.toLowerCase();
-					cwdFsPath = cwdFsPath.toLowerCase();
-				}
-				if (cdDirPath === cwdFsPath) {
-					return { rewritten: cdSuffix, reasoning: 'Removed redundant cd command' };
-				}
+			// Normalize trailing slashes
+			cdDirPath = cdDirPath.replace(/(?:[\\\/])$/, '');
+			let cwdFsPath = options.cwd.fsPath.replace(/(?:[\\\/])$/, '');
+			// Case-insensitive comparison on Windows
+			if (options.os === OperatingSystem.Windows) {
+				cdDirPath = cdDirPath.toLowerCase();
+				cwdFsPath = cwdFsPath.toLowerCase();
+			}
+			if (cdDirPath === cwdFsPath) {
+				return { rewritten: cdSuffix, reasoning: 'Removed redundant cd command' };
 			}
 		}
 		return undefined;

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts

@@ -48,6 +48,8 @@ import { LocalChatSessionUri } from '../../../../chat/common/chatUri.js';
 import type { ICommandLineRewriter } from './commandLineRewriter/commandLineRewriter.js';
 import { CommandLineCdPrefixRewriter } from './commandLineRewriter/commandLineCdPrefixRewriter.js';
 import { CommandLinePwshChainOperatorRewriter } from './commandLineRewriter/commandLinePwshChainOperatorRewriter.js';
+import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js';
+import { IHistoryService } from '../../../../../services/history/common/history.js';
 
 // #region Tool data
 
@@ -272,15 +274,17 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 	}
 
 	constructor(
-		@IInstantiationService private readonly _instantiationService: IInstantiationService,
+		@IChatService private readonly _chatService: IChatService,
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
+		@IHistoryService private readonly _historyService: IHistoryService,
+		@IInstantiationService private readonly _instantiationService: IInstantiationService,
 		@ILanguageModelToolsService private readonly _languageModelToolsService: ILanguageModelToolsService,
+		@IRemoteAgentService private readonly _remoteAgentService: IRemoteAgentService,
 		@IStorageService private readonly _storageService: IStorageService,
+		@ITerminalChatService private readonly _terminalChatService: ITerminalChatService,
 		@ITerminalLogService private readonly _logService: ITerminalLogService,
 		@ITerminalService private readonly _terminalService: ITerminalService,
-		@ITerminalChatService private readonly _terminalChatService: ITerminalChatService,
-		@IRemoteAgentService private readonly _remoteAgentService: IRemoteAgentService,
-		@IChatService private readonly _chatService: IChatService,
+		@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
 	) {
 		super();
 
@@ -331,11 +335,17 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 	async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise<IPreparedToolInvocation | undefined> {
 		const args = context.parameters as IRunInTerminalInputParams;
 
-		const instance = context.chatSessionId ? this._sessionTerminalAssociations.get(context.chatSessionId)?.instance : undefined;
 		const os = await this._osBackend;
 		const shell = await this._profileFetcher.getCopilotShell();
 		const language = os === OperatingSystem.Windows ? 'pwsh' : 'sh';
-		const cwd = await instance?.getCwdResource();
+
+		const instance = context.chatSessionId ? this._sessionTerminalAssociations.get(context.chatSessionId)?.instance : undefined;
+		let cwd = await instance?.getCwdResource();
+		if (!cwd) {
+			const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot();
+			const workspaceFolder = activeWorkspaceRootUri ? this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri) ?? undefined : undefined;
+			cwd = workspaceFolder?.uri;
+		}
 
 		const terminalToolSessionId = generateUuid();
 		// Generate a custom command ID to link the command between renderer and pty host

src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/commandLineCdPrefixRewriter.test.ts

@@ -5,13 +5,10 @@
 
 import { strictEqual } from 'assert';
 import { isWindows, OperatingSystem } from '../../../../../../base/common/platform.js';
+import { URI } from '../../../../../../base/common/uri.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
 import type { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
-import { TestContextService } from '../../../../../test/common/workbenchTestServices.js';
-import { IWorkspaceContextService, toWorkspaceFolder } from '../../../../../../platform/workspace/common/workspace.js';
 import { workbenchInstantiationService } from '../../../../../test/browser/workbenchTestServices.js';
-import { URI } from '../../../../../../base/common/uri.js';
-import { Workspace } from '../../../../../../platform/workspace/test/common/testWorkspace.js';
 import { CommandLineCdPrefixRewriter } from '../../browser/tools/commandLineRewriter/commandLineCdPrefixRewriter.js';
 import type { ICommandLineRewriterOptions } from '../../browser/tools/commandLineRewriter/commandLineRewriter.js';
 
@@ -19,7 +16,6 @@ suite('CommandLineCdPrefixRewriter', () => {
 	const store = ensureNoDisposablesAreLeakedInTestSuite();
 
 	let instantiationService: TestInstantiationService;
-	let workspaceService: TestContextService;
 	let rewriter: CommandLineCdPrefixRewriter;
 
 	function createRewriteOptions(command: string, cwd: URI | undefined, shell: string, os: OperatingSystem): ICommandLineRewriterOptions {
@@ -31,22 +27,16 @@ suite('CommandLineCdPrefixRewriter', () => {
 		};
 	}
 
-	function setWorkspaceFolders(folders: URI[]) {
-		workspaceService.setWorkspace(new Workspace(folders.reduce((prev, curr) => {
-			return `${prev},${curr}`;
-		}, ''), folders.map(e => toWorkspaceFolder(e))));
-	}
-
 	setup(() => {
 		instantiationService = workbenchInstantiationService({}, store);
-		workspaceService = instantiationService.get(IWorkspaceContextService) as TestContextService;
 		rewriter = store.add(instantiationService.createInstance(CommandLineCdPrefixRewriter));
 	});
 
 	suite('cd <cwd> && <suffix> -> <suffix>', () => {
 		(!isWindows ? suite : suite.skip)('Posix', () => {
-			function t(commandLine: string, cwd: URI | undefined, workspaceFolders: URI[], shell: string, expectedResult: string | undefined) {
-				setWorkspaceFolders(workspaceFolders);
+			const cwd = URI.file('/test/workspace');
+
+			function t(commandLine: string, shell: string, expectedResult: string | undefined) {
 				const options = createRewriteOptions(commandLine, cwd, shell, OperatingSystem.Linux);
 				const result = rewriter.rewrite(options);
 				strictEqual(result?.rewritten, expectedResult);
@@ -55,21 +45,20 @@ suite('CommandLineCdPrefixRewriter', () => {
 				}
 			}
 
-			test('should return undefined when no cd prefix pattern matches', () => t('echo hello world', undefined, [], 'bash', undefined));
-			test('should return undefined when cd pattern does not have suffix', () => t('cd /some/path', undefined, [], 'bash', undefined));
-			test('should rewrite command with ; separator when directory matches cwd', () => t('cd /test/workspace; npm test', undefined, [URI.file('/test/workspace')], 'pwsh', 'npm test'));
-			test('should rewrite command with && separator when directory matches cwd', () => t('cd /test/workspace && npm install', undefined, [URI.file('/test/workspace')], 'bash', 'npm install'));
-			test('should rewrite command when the path is wrapped in double quotes', () => t('cd "/test/workspace" && npm install', undefined, [URI.file('/test/workspace')], 'bash', 'npm install'));
-			test('should not rewrite command when directory does not match cwd', () => t('cd /different/path && npm install', undefined, [URI.file('/test/workspace')], 'bash', undefined));
-			test('should return undefined when no workspace folders available', () => t('cd /some/path && npm install', undefined, [], 'bash', undefined));
-			test('should return undefined when multiple workspace folders available', () => t('cd /some/path && npm install', undefined, [URI.file('/workspace1'), URI.file('/workspace2')], 'bash', undefined));
-			test('should handle commands with complex suffixes', () => t('cd /test/workspace && npm install && npm test && echo "done"', undefined, [URI.file('/test/workspace')], 'bash', 'npm install && npm test && echo "done"'));
-			test('should ignore any trailing forward slash', () => t('cd /test/workspace/ && npm install', undefined, [URI.file('/test/workspace')], 'bash', 'npm install'));
+			test('should return undefined when no cd prefix pattern matches', () => t('echo hello world', 'bash', undefined));
+			test('should return undefined when cd pattern does not have suffix', () => t('cd /some/path', 'bash', undefined));
+			test('should rewrite command with ; separator when directory matches cwd', () => t('cd /test/workspace; npm test', 'pwsh', 'npm test'));
+			test('should rewrite command with && separator when directory matches cwd', () => t('cd /test/workspace && npm install', 'bash', 'npm install'));
+			test('should rewrite command when the path is wrapped in double quotes', () => t('cd "/test/workspace" && npm install', 'bash', 'npm install'));
+			test('should not rewrite command when directory does not match cwd', () => t('cd /different/path && npm install', 'bash', undefined));
+			test('should handle commands with complex suffixes', () => t('cd /test/workspace && npm install && npm test && echo "done"', 'bash', 'npm install && npm test && echo "done"'));
+			test('should ignore any trailing forward slash', () => t('cd /test/workspace/ && npm install', 'bash', 'npm install'));
 		});
 
 		(isWindows ? suite : suite.skip)('Windows', () => {
-			function t(commandLine: string, cwd: URI | undefined, workspaceFolders: URI[], shell: string, expectedResult: string | undefined) {
-				setWorkspaceFolders(workspaceFolders);
+			const cwd = URI.file('C:\\test\\workspace');
+
+			function t(commandLine: string, shell: string, expectedResult: string | undefined) {
 				const options = createRewriteOptions(commandLine, cwd, shell, OperatingSystem.Windows);
 				const result = rewriter.rewrite(options);
 				strictEqual(result?.rewritten, expectedResult);
@@ -78,20 +67,16 @@ suite('CommandLineCdPrefixRewriter', () => {
 				}
 			}
 
-			test('should ignore any trailing back slash', () => t('cd c:\\test\\workspace\\ && npm install', undefined, [URI.file('c:\\test\\workspace')], 'cmd', 'npm install'));
-			test('should prioritize cwd option over workspace service', () => t('cd C:\\cwd\\workspace && npm test', URI.file('C:\\cwd\\workspace'), [URI.file('C:\\workspace\\service')], 'cmd', 'npm test'));
-			test('should prioritize cwd option over workspace service - PowerShell style', () => t('cd C:\\cwd\\workspace; npm test', URI.file('C:\\cwd\\workspace'), [URI.file('C:\\workspace\\service')], 'pwsh', 'npm test'));
-			test('should not rewrite when cwd differs from cd path', () => t('cd C:\\different\\path && npm test', URI.file('C:\\cwd\\workspace'), [URI.file('C:\\workspace\\service')], 'cmd', undefined));
-			test('should fallback to workspace service when cwd is undefined', () => t('cd C:\\workspace\\service && npm test', undefined, [URI.file('C:\\workspace\\service')], 'cmd', 'npm test'));
-			test('should prioritize cwd over workspace service even when both match cd path', () => t('cd C:\\shared\\workspace && npm build', URI.file('C:\\shared\\workspace'), [URI.file('C:\\shared\\workspace')], 'cmd', 'npm build'));
-			test('should handle case-insensitive comparison on Windows', () => t('cd c:\\cwd\\workspace && npm test', URI.file('C:\\Cwd\\Workspace'), [], 'cmd', 'npm test'));
-			test('should handle quoted paths with cwd priority', () => t('cd "C:\\cwd\\workspace" && npm test', URI.file('C:\\cwd\\workspace'), [URI.file('C:\\different\\workspace')], 'cmd', 'npm test'));
-			test('should handle cd /d flag when directory matches cwd', () => t('cd /d C:\\test\\workspace && echo hello', undefined, [URI.file('C:\\test\\workspace')], 'pwsh', 'echo hello'));
-			test('should handle cd /d flag with quoted paths when directory matches cwd', () => t('cd /d "C:\\test\\workspace" && echo hello', undefined, [URI.file('C:\\test\\workspace')], 'pwsh', 'echo hello'));
-			test('should handle cd /d flag with quoted paths from issue example', () => t('cd /d "d:\\microsoft\\vscode" && .\\scripts\\test.bat', undefined, [URI.file('d:\\microsoft\\vscode')], 'pwsh', '.\\scripts\\test.bat'));
-			test('should not rewrite cd /d when directory does not match cwd', () => t('cd /d C:\\different\\path ; echo hello', undefined, [URI.file('C:\\test\\workspace')], 'pwsh', undefined));
-			test('should handle cd /d flag with cwd priority', () => t('cd /d C:\\cwd\\workspace && npm test', URI.file('C:\\cwd\\workspace'), [URI.file('C:\\workspace\\service')], 'pwsh', 'npm test'));
-			test('should handle cd /d flag with semicolon separator', () => t('cd /d C:\\test\\workspace; echo hello', undefined, [URI.file('C:\\test\\workspace')], 'pwsh', 'echo hello'));
+			test('should ignore any trailing back slash', () => t('cd c:\\test\\workspace\\ && npm install', 'cmd', 'npm install'));
+			test('should rewrite command with && separator when directory matches cwd', () => t('cd C:\\test\\workspace && npm test', 'cmd', 'npm test'));
+			test('should rewrite command with ; separator when directory matches cwd - PowerShell style', () => t('cd C:\\test\\workspace; npm test', 'pwsh', 'npm test'));
+			test('should not rewrite when cwd differs from cd path', () => t('cd C:\\different\\path && npm test', 'cmd', undefined));
+			test('should handle case-insensitive comparison on Windows', () => t('cd c:\\test\\workspace && npm test', 'cmd', 'npm test'));
+			test('should handle quoted paths', () => t('cd "C:\\test\\workspace" && npm test', 'cmd', 'npm test'));
+			test('should handle cd /d flag when directory matches cwd', () => t('cd /d C:\\test\\workspace && echo hello', 'pwsh', 'echo hello'));
+			test('should handle cd /d flag with quoted paths when directory matches cwd', () => t('cd /d "C:\\test\\workspace" && echo hello', 'pwsh', 'echo hello'));
+			test('should not rewrite cd /d when directory does not match cwd', () => t('cd /d C:\\different\\path ; echo hello', 'pwsh', undefined));
+			test('should handle cd /d flag with semicolon separator', () => t('cd /d C:\\test\\workspace; echo hello', 'pwsh', 'echo hello'));
 		});
 	});
 });