Improve terminal unsandbox retry confirmation

extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts

@@ -40,8 +40,10 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string {
 			await terminalConfig.update('windowsUseConptyDll', true, vscode.ConfigurationTarget.Global);
 		}
 
-		// Register a dummy default model required for participant requests
-		disposables.push(vscode.lm.registerLanguageModelChatProvider('copilot', {
+		// Register a dummy default model required for participant requests.
+		// The test extension contributes both `test-lm-vendor` and `copilot`; focused
+		// chat runs may select either vendor before the participant request is routed.
+		const testLanguageModelProvider: vscode.LanguageModelChatProvider = {
 			async provideLanguageModelChatInformation(_options, _token) {
 				return [{
 					id: 'test-lm',
@@ -61,7 +63,11 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string {
 			async provideTokenCount(_model, _text, _token) {
 				return 1;
 			},
-		}));
+		};
+		disposables.push(
+			vscode.lm.registerLanguageModelChatProvider('test-lm-vendor', testLanguageModelProvider),
+			vscode.lm.registerLanguageModelChatProvider('copilot', testLanguageModelProvider),
+		);
 
 		// Enable global auto-approve + skip the confirmation dialog via test-mode context key
 		const chatToolsConfig = vscode.workspace.getConfiguration('chat.tools.global');
@@ -99,6 +105,7 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string {
 		timeout?: number;
 		requestUnsandboxedExecution?: boolean;
 		requestUnsandboxedExecutionReason?: string;
+		autoAcceptConfirmation?: boolean;
 	}
 
 	let participantRegistered = false;
@@ -158,11 +165,30 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string {
 		pendingCommand = command;
 		pendingOptions = opts;
 
-		await vscode.commands.executeCommand('workbench.action.chat.newChat');
-		vscode.commands.executeCommand('workbench.action.chat.open', { query: '@participant test' });
+		let acceptConfirmationInterval: ReturnType<typeof setInterval> | undefined;
+		const acceptPendingConfirmation = async () => {
+			try {
+				await vscode.commands.executeCommand('workbench.action.chat.acceptTool');
+				await vscode.commands.executeCommand('workbench.action.chat.acceptElicitation');
+			} catch {
+				// The commands are no-ops when no confirmation is visible.
+			}
+		};
+
+		try {
+			await vscode.commands.executeCommand('workbench.action.chat.newChat');
+			if (opts.autoAcceptConfirmation) {
+				acceptConfirmationInterval = setInterval(() => void acceptPendingConfirmation(), 200);
+			}
+			await vscode.commands.executeCommand('workbench.action.chat.open', { query: '@participant test' });
 
-		const result = await resultPromise.p;
-		return extractTextContent(result);
+			const result = await resultPromise.p;
+			return extractTextContent(result);
+		} finally {
+			if (acceptConfirmationInterval) {
+				clearInterval(acceptConfirmationInterval);
+			}
+		}
 	}
 
 	test('tool should be registered with expected schema', async function () {
@@ -334,12 +360,14 @@ function extractTextContent(result: vscode.LanguageModelToolResult): string {
 			this.timeout(60000);
 
 			const marker = `SANDBOX_TMP_${Date.now()}`;
-			const output = await invokeRunInTerminal(`echo "${marker}" > /tmp/${marker}.txt`);
+			const output = await invokeRunInTerminal(`echo "${marker}" > /tmp/${marker}.txt && cat /tmp/${marker}.txt && rm /tmp/${marker}.txt`, {
+				autoAcceptConfirmation: true,
+			});
 
 			const trimmed = output.trim();
 			assert.ok(trimmed.startsWith('Note: The tool simplified the command to'), `Unexpected output: ${JSON.stringify(trimmed)}`);
 			assert.ok(trimmed.includes(`/tmp/${marker}.txt`), `Unexpected output: ${JSON.stringify(trimmed)}`);
-			assert.ok(trimmed.endsWith('Command produced no output'), `Unexpected output: ${JSON.stringify(trimmed)}`);
+			assert.ok(trimmed.endsWith(marker), `Unexpected output: ${JSON.stringify(trimmed)}`);
 		});
 
 		test('can read files outside the workspace', async function () {

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

@@ -789,12 +789,13 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 			analyzersIsAutoApproveAllowed
 		);
 
-		const isFinalAutoApproved = (
+		const isAutoApprovedByRules = (
 			// Is the setting enabled and the user has opted-in
 			isAutoApproveAllowed &&
 			// Would be auto-approved based on rules
 			wouldBeAutoApproved
-		) || commandLineAnalyzerResults.some(e => e.forceAutoApproval);
+		);
+		const isFinalAutoApproved = isAutoApprovedByRules || commandLineAnalyzerResults.some(e => e.forceAutoApproval);
 
 		// Pass auto approve info if the command:
 		// - Was auto approved
@@ -874,7 +875,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 
 		// If forceConfirmationReason is set, always show confirmation regardless of auto-approval
 		const shouldShowConfirmation = (!isFinalAutoApproved && !isSessionAutoApproved) || context.forceConfirmationReason !== undefined;
-		(toolSpecificData as IRunInTerminalToolInvocationData).requiresConfirmationForRetry = shouldShowConfirmation;
+		(toolSpecificData as IRunInTerminalToolInvocationData).requiresConfirmationForRetry = (!isAutoApprovedByRules && !isSessionAutoApproved) || context.forceConfirmationReason !== undefined;
 		const confirmationMessage = requiresUnsandboxConfirmation
 			? new MarkdownString(localize(
 				'runInTerminal.unsandboxed.confirmationMessage',
@@ -1026,9 +1027,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 			};
 
 			const part = new ChatElicitationRequestPart(
-				blockedDomains?.length
-					? localize('runInTerminal.unsandboxed.domain', "Run `{0}` command outside the [sandbox]({1}) to access {2}?", shellType, TERMINAL_SANDBOX_DOCUMENTATION_URL, this._formatBlockedDomainsForTitle(blockedDomains))
-					: localize('runInTerminal.unsandboxed', "Run `{0}` command outside the [sandbox]({1})?", shellType, TERMINAL_SANDBOX_DOCUMENTATION_URL),
+				this._getAutomaticUnsandboxRetryTitle(shellType, blockedDomains),
 				new MarkdownString(localize(
 					'runInTerminal.unsandboxed.autoRetry.confirmationMessage',
 					"`{0}`",
@@ -1058,6 +1057,12 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
 		});
 	}
 
+	private _getAutomaticUnsandboxRetryTitle(shellType: string, blockedDomains: string[] | undefined): MarkdownString {
+		return blockedDomains?.length
+			? new MarkdownString(localize('runInTerminal.unsandboxed.autoRetry.domain', "Run `{0}` command outside the sandbox to access {1}?", shellType, this._formatBlockedDomainsForTitle(blockedDomains)))
+			: new MarkdownString(localize('runInTerminal.unsandboxed.autoRetry', "Run `{0}` command outside the sandbox?", shellType));
+	}
+
 	private _acceptAutomaticUnsandboxRetryToolInvocationUpdate(sessionResource: URI | undefined, toolCallId: string, toolSpecificData: IChatTerminalToolInvocationData, isComplete: boolean, toolResultMessage?: string | IMarkdownString): void {
 		const chatModel = sessionResource && this._chatService.getSession(sessionResource);
 		if (!(chatModel instanceof ChatModel)) {

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

@@ -281,6 +281,10 @@ suite('RunInTerminalTool', () => {
 		return (tool as unknown as Record<string, (sessionResource: URI | undefined, command: string, shell: string, blockedDomains: string[] | undefined, requiresConfirmationForRetry: boolean | undefined, token: CancellationToken) => Promise<boolean>>)['_confirmAutomaticUnsandboxRetry'](sessionResource, command, shell, blockedDomains, requiresConfirmationForRetry, CancellationToken.None);
 	}
 
+	function getAutomaticUnsandboxRetryTitle(tool: RunInTerminalTool, shellType: string, blockedDomains: string[] | undefined): IMarkdownString {
+		return (tool as unknown as Record<string, (shellType: string, blockedDomains: string[] | undefined) => IMarkdownString>)['_getAutomaticUnsandboxRetryTitle'](shellType, blockedDomains);
+	}
+
 	suite('sandbox invocation messaging', () => {
 		test('should instruct models to use $TMPDIR instead of /tmp when sandboxed', async () => {
 			sandboxEnabled = true;
@@ -500,6 +504,39 @@ suite('RunInTerminalTool', () => {
 
 			strictEqual(shouldRetry, false);
 		});
+
+		test('should use retry confirmation title without sandbox link', () => {
+			const title = getAutomaticUnsandboxRetryTitle(runInTerminalTool, 'bash', undefined);
+
+			strictEqual(title.value, 'Run `bash` command outside the sandbox?');
+		});
+
+		test('should use retry confirmation title without sandbox link for blocked domains', () => {
+			const title = getAutomaticUnsandboxRetryTitle(runInTerminalTool, 'bash', ['example.com']);
+
+			strictEqual(title.value, 'Run `bash` command outside the sandbox to access `example.com`?');
+		});
+
+		test('should show retry elicitation when sandbox force-approved command would otherwise require confirmation', async () => {
+			setAutoApprove({});
+			sandboxEnabled = true;
+			sandboxPrereqResult = {
+				enabled: true,
+				sandboxConfigPath: '/tmp/vscode-sandbox-settings.json',
+				failedCheck: undefined,
+			};
+
+			const preparedInvocation = await executeToolTest({ command: 'rm dangerous-file.txt' });
+
+			assertAutoApproved(preparedInvocation);
+			const terminalData = preparedInvocation!.toolSpecificData as ITestRunInTerminalToolInvocationData;
+			strictEqual(terminalData.commandLine.isSandboxWrapped, true);
+			strictEqual(terminalData.requiresConfirmationForRetry, true);
+
+			const shouldRetry = await confirmAutomaticUnsandboxRetry(runInTerminalTool, undefined, 'rm dangerous-file.txt', 'bash', undefined, terminalData.requiresConfirmationForRetry);
+
+			strictEqual(shouldRetry, false);
+		});
 	});
 
 	suite('default auto-approve rules', () => {