diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts index 83c72fb12f5a9..619bb84672fdf 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts @@ -16,7 +16,7 @@ import { INTEGRATION_ID } from '../../../../platform/endpoint/common/licenseAgre import { INativeEnvService } from '../../../../platform/env/common/envService'; import { IVSCodeExtensionContext } from '../../../../platform/extContext/common/extensionContext'; import { IFileSystemService } from '../../../../platform/filesystem/common/fileSystemService'; -import { RelativePattern } from '../../../../platform/filesystem/common/fileTypes'; +import { FileType, RelativePattern } from '../../../../platform/filesystem/common/fileTypes'; import { ILogService } from '../../../../platform/log/common/logService'; import { deriveCopilotCliOTelEnv } from '../../../../platform/otel/common/agentOTelEnv'; import { IOTelService } from '../../../../platform/otel/common/otelService'; @@ -55,6 +55,10 @@ import { ICopilotCLIMCPHandler, McpServerMappings, remapCustomAgentTools } from const COPILOT_CLI_WORKSPACE_JSON_FILE_KEY = 'github.copilot.cli.workspaceSessionFile'; +const AGENT_HOST_ENABLED_SETTING_ID = 'chat.agentHost.enabled'; +const AGENT_HOST_DEFAULT_SESSIONS_PROVIDER_SETTING_ID = 'chat.agentHost.defaultSessionsProvider'; +const DEFAULT_TO_COPILOT_HARNESS_SETTING_ID = 'chat.defaultToCopilotHarness'; +const AGENT_HOST_COPILOT_CLIENT_NAME = 'vscode-agent-host'; export const COPILOT_CLI_CHAT_PANEL_SYSTEM_MESSAGE = 'You are an AI assistant using Copilot CLI runtime in VS Code. You help users with software engineering tasks. When asked about your identity, you must state that you are an AI assistant using Copilot CLI runtime in VS Code.'; type SDKPackage = Awaited>; @@ -193,7 +197,9 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS void this.createCustomAgentLookup(); } })); - this.monitorSessionFiles(); + if (this.shouldMonitorSessionFiles()) { + this.monitorSessionFiles(); + } this._sessionManager = new Lazy>(async () => { try { const sdkPackage = await this.getSDKPackage(); @@ -231,6 +237,17 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS this._sessionTracker = this.instantiationService.createInstance(CopilotCLISessionWorkspaceTracker); } + private shouldMonitorSessionFiles(): boolean { + if (this.configurationService.getNonExtensionConfig(AGENT_HOST_ENABLED_SETTING_ID) !== true) { + return true; + } + + const defaultProviderSettingId = this._agentSessionsWorkspace.isAgentSessionsWorkspace + ? AGENT_HOST_DEFAULT_SESSIONS_PROVIDER_SETTING_ID + : DEFAULT_TO_COPILOT_HARNESS_SETTING_ID; + return this.configurationService.getNonExtensionConfig(defaultProviderSettingId) !== true; + } + private async getSDKPackage(): Promise { return this.copilotCLISDK.getPackage(); } @@ -398,6 +415,9 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS if (!metadata || token.isCancellationRequested) { return; } + if (metadata.clientName === AGENT_HOST_COPILOT_CLIENT_NAME) { + return; + } await this._sessionTracker.initialize(); return await this.constructSessionItem(metadata, token); } @@ -480,19 +500,19 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS return joinPath(userDataPath, 'agentSessionData'); } - /** - * Whether the agent host owns this session — it writes a per-session SQLite DB at - * `/agentSessionData//session.db` and we skip those to avoid double-listing sessions both - * surfaces read from the shared `~/.copilot/session-state/` directory. - */ - private async _isOwnedByAgentHost(sessionId: string, dataDir: URI | undefined): Promise { + private async _getAgentHostOwnedSessionIds(dataDir: URI | undefined): Promise> { if (!dataDir) { - return false; + return new Set(); + } + try { + const entries = await this.fileSystem.readDirectory(dataDir); + return new Set(entries + .filter(([, type]) => (type & FileType.Directory) !== 0) + .map(([name]) => name)); + } catch (error) { + this.logService.trace(`Failed to read Agent Host session data directory: ${error}`); + return new Set(); } - // Must mirror `SessionDataService._sanitizedSessionKey`. - const sanitized = sessionId.replace(/[^a-zA-Z0-9_.-]/g, '-'); - const dbPath = joinPath(dataDir, sanitized, 'session.db'); - return this.fileSystem.stat(dbPath).then(() => true, () => false); } async _getAllSessions(token: CancellationToken): Promise { @@ -505,11 +525,16 @@ export class CopilotCLISessionService extends Disposable implements ICopilotCLIS // Skip sessions the agent host already lists (both surfaces share `~/.copilot/session-state/`). const agentHostDataDir = this._getAgentHostSessionDataDir(); + const sessionsWithoutAgentHostClientName = sessionMetadataList.filter(metadata => metadata.clientName !== AGENT_HOST_COPILOT_CLIENT_NAME); + const agentHostOwnedSessionIds = sessionsWithoutAgentHostClientName.length > 0 + ? await this._getAgentHostOwnedSessionIds(agentHostDataDir) + : new Set(); // Convert SessionMetadata to ICopilotCLISession const diskSessions: ICopilotCLISessionItem[] = coalesce(await Promise.all( sessionMetadataList.map(async (metadata): Promise => { - if (await this._isOwnedByAgentHost(metadata.sessionId, agentHostDataDir)) { + const sanitizedSessionId = metadata.sessionId.replace(/[^a-zA-Z0-9_.-]/g, '-'); + if (metadata.clientName === AGENT_HOST_COPILOT_CLIENT_NAME || agentHostOwnedSessionIds.has(sanitizedSessionId)) { return; } const workingDirectory = metadata.context?.cwd ? URI.file(metadata.context.cwd) : undefined; diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts index a0b4553e3b9a4..23798d8337cc4 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotCliSessionService.spec.ts @@ -8,13 +8,16 @@ import { mkdir, mkdtemp, rm, writeFile as writeNodeFile } from 'node:fs/promises import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import type { ChatContext, ChatCustomAgent, ChatParticipantToolToken, Uri } from 'vscode'; +import type { ChatContext, ChatCustomAgent, ChatParticipantToolToken, Event as VSCodeEvent, FileSystemWatcher, Uri } from 'vscode'; import { CancellationToken } from 'vscode-languageserver-protocol'; import { IAuthenticationService } from '../../../../../platform/authentication/common/authentication'; import { NullChatDebugFileLoggerService } from '../../../../../platform/chat/common/chatDebugFileLoggerService'; import { IConfigurationService } from '../../../../../platform/configuration/common/configurationService'; +import { InMemoryConfigurationService } from '../../../../../platform/configuration/test/common/inMemoryConfigurationService'; import { NullNativeEnvService } from '../../../../../platform/env/common/nullEnvService'; +import { IVSCodeExtensionContext } from '../../../../../platform/extContext/common/extensionContext'; import { MockFileSystemService } from '../../../../../platform/filesystem/node/test/mockFileSystemService'; +import { FileType } from '../../../../../platform/filesystem/common/fileTypes'; import { MockGitService } from '../../../../../platform/ignore/node/test/mockGitService'; import { ILogService } from '../../../../../platform/log/common/logService'; import { NullMcpService } from '../../../../../platform/mcp/common/mcpService'; @@ -63,7 +66,40 @@ class MockLocalSession { export class NullAgentSessionsWorkspace implements IAgentSessionsWorkspace { _serviceBrand: undefined; - readonly isAgentSessionsWorkspace = false; + constructor(readonly isAgentSessionsWorkspace = false) { } +} + +const emptyFileSystemEvent: VSCodeEvent = () => toDisposable(() => { }); + +class TestFileSystemWatcher implements FileSystemWatcher { + ignoreCreateEvents = false; + ignoreChangeEvents = false; + ignoreDeleteEvents = false; + readonly onDidCreate = emptyFileSystemEvent; + readonly onDidChange = emptyFileSystemEvent; + readonly onDidDelete = emptyFileSystemEvent; + dispose(): void { } +} + +class TrackingFileSystemService extends MockFileSystemService { + createFileSystemWatcherCallCount = 0; + readDirectoryCallCount = 0; + + override createFileSystemWatcher(): FileSystemWatcher { + this.createFileSystemWatcherCallCount++; + return new TestFileSystemWatcher(); + } + + override async readDirectory(uri: URI): Promise<[string, FileType][]> { + this.readDirectoryCallCount++; + return super.readDirectory(uri); + } +} + +class TestExtensionContext extends mock() { + constructor(public override readonly globalStorageUri: Uri) { + super(); + } } class NullChatSessionWorkspaceFolderService extends mock() { @@ -116,6 +152,13 @@ function createCustomAgent(uri: URI, name: string): ChatCustomAgent { }; } +interface ICreateSessionServiceOptions { + readonly configurationService?: IConfigurationService; + readonly extensionContext?: IVSCodeExtensionContext; + readonly fileSystem?: MockFileSystemService; + readonly isAgentSessionsWorkspace?: boolean; +} + const createLocalFeatureFlagServiceCreator = () => () => ({}); describe('CopilotCLISessionService', () => { @@ -126,6 +169,8 @@ describe('CopilotCLISessionService', () => { let manager: MockCliSdkSessionManager; let metadataStore: MockChatSessionMetadataStore; let promptsService: MockPromptsService; + let configurationService: IConfigurationService; + let createSessionService: (options?: ICreateSessionServiceOptions) => CopilotCLISessionService; let tempStateHome: string | undefined; const originalXdgStateHome = process.env.XDG_STATE_HOME; beforeEach(async () => { @@ -182,12 +227,16 @@ describe('CopilotCLISessionService', () => { return disposables.add(new CopilotCLISession(workspaceInfo, agentName, sdkSession, [], undefined, logService, workspaceService, new MockChatSessionMetadataStore(), instantiationService, new NullRequestLogger(), new NullICopilotCLIImageSupport(), new FakeToolsService(), new FakeUserQuestionHandler(), accessor.get(IConfigurationService), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new MockGitService(), { _serviceBrand: undefined } as any, { _serviceBrand: undefined, resetTurnCredits() { }, getCreditsForTurn() { return undefined; }, setLastCopilotUsage() { } } as any, new NullTelemetryService())); } } as unknown as IInstantiationService; - const configurationService = accessor.get(IConfigurationService); + configurationService = accessor.get(IConfigurationService); const nullMcpServer = disposables.add(new NullMcpService()); const titleService = new NullCustomSessionTitleService(); metadataStore = new MockChatSessionMetadataStore(); promptsService = disposables.add(new MockPromptsService()); - service = disposables.add(new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), new MockFileSystemService(), new CopilotCLIMCPHandler(logService, authService, configurationService, nullMcpServer), cliAgents, workspaceService, titleService, configurationService, new MockSkillLocations(), delegationService, metadataStore, new NullAgentSessionsWorkspace(), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullPromptVariablesService(), new NullChatDebugFileLoggerService(), promptsService, new NullCopilotCLIModels(), new NullExperimentationService())); + createSessionService = (options = {}) => { + const serviceConfiguration = options.configurationService ?? configurationService; + return new CopilotCLISessionService(logService, sdk, instantiationService, new NullNativeEnvService(), options.fileSystem ?? new MockFileSystemService(), new CopilotCLIMCPHandler(logService, authService, serviceConfiguration, nullMcpServer), cliAgents, workspaceService, titleService, serviceConfiguration, new MockSkillLocations(), delegationService, metadataStore, new NullAgentSessionsWorkspace(options.isAgentSessionsWorkspace), new NullChatSessionWorkspaceFolderService(), new NullChatSessionWorktreeService(), new NoopOTelService(resolveOTelConfig({ env: {}, extensionVersion: '0.0.0', sessionId: 'test' })), new NullPromptVariablesService(), new NullChatDebugFileLoggerService(), promptsService, new NullCopilotCLIModels(), new NullExperimentationService(), options.extensionContext); + }; + service = disposables.add(createSessionService()); manager = await service.getSessionManager() as unknown as MockCliSdkSessionManager; }); @@ -204,6 +253,38 @@ describe('CopilotCLISessionService', () => { // --- Tests ---------------------------------------------------------------------------------- + describe('session file monitoring', () => { + it('skips the watcher when Agent Host is the default for the current window', async () => { + const cases = [ + { name: 'Agents window Agent Host default', isAgentSessionsWorkspace: true, agentHostEnabled: true, agentsDefault: true, editorDefault: false, expectedWatcherCount: 0 }, + { name: 'editor window Agent Host default', isAgentSessionsWorkspace: false, agentHostEnabled: true, agentsDefault: false, editorDefault: true, expectedWatcherCount: 0 }, + { name: 'Agents window Agent Host disabled', isAgentSessionsWorkspace: true, agentHostEnabled: false, agentsDefault: true, editorDefault: false, expectedWatcherCount: 1 }, + { name: 'editor window Agent Host disabled', isAgentSessionsWorkspace: false, agentHostEnabled: false, agentsDefault: false, editorDefault: true, expectedWatcherCount: 1 }, + { name: 'Agents window editor default only', isAgentSessionsWorkspace: true, agentHostEnabled: true, agentsDefault: false, editorDefault: true, expectedWatcherCount: 1 }, + { name: 'editor window Agents default only', isAgentSessionsWorkspace: false, agentHostEnabled: true, agentsDefault: true, editorDefault: false, expectedWatcherCount: 1 }, + ]; + + const results = []; + for (const testCase of cases) { + const testConfiguration = disposables.add(new InMemoryConfigurationService(configurationService)); + await Promise.all([ + testConfiguration.setNonExtensionConfig('chat.agentHost.enabled', testCase.agentHostEnabled), + testConfiguration.setNonExtensionConfig('chat.agentHost.defaultSessionsProvider', testCase.agentsDefault), + testConfiguration.setNonExtensionConfig('chat.defaultToCopilotHarness', testCase.editorDefault), + ]); + const fileSystem = new TrackingFileSystemService(); + disposables.add(createSessionService({ + configurationService: testConfiguration, + fileSystem, + isAgentSessionsWorkspace: testCase.isAgentSessionsWorkspace, + })); + results.push({ name: testCase.name, watcherCount: fileSystem.createFileSystemWatcherCallCount }); + } + + expect(results).toEqual(cases.map(testCase => ({ name: testCase.name, watcherCount: testCase.expectedWatcherCount }))); + }); + }); + describe('CopilotCLISessionService.getChatHistory', () => { it('refreshes cached custom agent mode instructions when custom agents change', async () => { const agentUri = URI.file('/workspace/.github/agents/review.agent.md'); @@ -653,6 +734,47 @@ describe('CopilotCLISessionService', () => { }); describe('CopilotCLISessionService.getAllSessions', () => { + it('filters Agent Host sessions without per-session stats', async () => { + const fileSystem = new TrackingFileSystemService(); + const globalStorageUri = URI.file('/user-data/User/globalStorage/github.copilot-chat'); + fileSystem.mockDirectory(URI.file('/user-data/agentSessionData'), [ + ['legacy-owned', FileType.Directory], + ['not-a-session', FileType.File], + ]); + const filteredService = disposables.add(createSessionService({ + extensionContext: new TestExtensionContext(globalStorageUri), + fileSystem, + })); + const filteredManager = await filteredService.getSessionManager() as unknown as MockCliSdkSessionManager; + const sessions = [ + { id: 'agent-host', clientName: 'vscode-agent-host' }, + { id: 'legacy:owned', clientName: 'vscode' }, + { id: 'extension-host', clientName: 'vscode' }, + { id: 'standalone', clientName: 'copilot-cli' }, + ]; + for (const { id, clientName } of sessions) { + const sdkSession = new MockCliSdkSession(id, new Date(0)); + sdkSession.clientName = clientName; + sdkSession.summary = id; + filteredManager.sessions.set(id, sdkSession); + } + + const result = await filteredService.getAllSessions(CancellationToken.None); + const targetedAgentHostItem = await filteredService.getSessionItem('agent-host', CancellationToken.None); + + expect({ + sessionIds: result.map(item => item.id), + targetedAgentHostItem, + readDirectoryCallCount: fileSystem.readDirectoryCallCount, + statCallCount: fileSystem.getStatCallCount(), + }).toEqual({ + sessionIds: ['extension-host', 'standalone'], + targetedAgentHostItem: undefined, + readDirectoryCallCount: 1, + statCallCount: 0, + }); + }); + it('will not list created sessions', async () => { const session = await service.createSession({ model: 'gpt-test', ...sessionOptionsFor(URI.file('/tmp')) }, CancellationToken.None); disposables.add(session); diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/testHelpers.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/testHelpers.ts index dd5b952407380..27db19de13a85 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/testHelpers.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/test/testHelpers.ts @@ -21,6 +21,7 @@ export class MockCliSdkSession { public events: {}[] = []; public title: string | undefined; public name: string | undefined; + public clientName: string | undefined; public readonly renameSession = async (name: string): Promise => { this.title = name; this.name = name; @@ -90,11 +91,11 @@ export class MockCliSdkSessionManager { return Promise.resolve(undefined); } listSessions() { - return Promise.resolve(Array.from(this.sessions.values()).map(s => ({ sessionId: s.sessionId, startTime: s.startTime, modifiedTime: s.startTime, summary: s.summary, name: s.name }))); + return Promise.resolve(Array.from(this.sessions.values()).map(s => ({ sessionId: s.sessionId, startTime: s.startTime, modifiedTime: s.startTime, summary: s.summary, name: s.name, clientName: s.clientName }))); } getSessionMetadata({ sessionId }: { sessionId: string }) { const session = this.sessions.get(sessionId); - return Promise.resolve(session ? { sessionId: session.sessionId, startTime: session.startTime, modifiedTime: session.startTime, summary: session.summary, name: session.name, isRemote: false } : undefined); + return Promise.resolve(session ? { sessionId: session.sessionId, startTime: session.startTime, modifiedTime: session.startTime, summary: session.summary, name: session.name, clientName: session.clientName, isRemote: false } : undefined); } deleteSession(id: string) { this.sessions.delete(id); return Promise.resolve(); } closeSession(_id: string) { return Promise.resolve(); } diff --git a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts index 3ac367ea9e291..1a72347791c3e 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts @@ -48,6 +48,7 @@ type ReasoningEffort = NonNullable; const ContextTiers = ['default', 'long_context'] as const; type ContextTier = NonNullable; +const AGENT_HOST_COPILOT_CLIENT_NAME = 'vscode-agent-host'; type UserInputHandler = NonNullable; type UserInputRequest = Parameters[0]; @@ -580,7 +581,7 @@ export class CopilotSessionLauncher implements ICopilotSessionLauncher { } return { ...byok, - clientName: 'vscode', + clientName: AGENT_HOST_COPILOT_CLIENT_NAME, enableMcpApps: true, enableFileHooks: true, enableConfigDiscovery: true, diff --git a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts index 718f422477f5a..a4070a59f4f2f 100644 --- a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts @@ -4,17 +4,23 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import type { CopilotClient, CopilotSession } from '@github/copilot-sdk'; import { Emitter, Event } from '../../../../base/common/event.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; +import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import type { IFileService } from '../../../files/common/files.js'; import { InstantiationService } from '../../../instantiation/common/instantiationService.js'; import { ServiceCollection } from '../../../instantiation/common/serviceCollection.js'; import { ILogService, NullLogService } from '../../../log/common/log.js'; import type { IByokLmBridgeConnection, IByokLmChatRequest, IByokLmChatResult, IByokLmModelInfo } from '../../common/agentHostByokLm.js'; import type { ModelSelection } from '../../common/state/protocol/state.js'; +import type { IAgentConfigurationService } from '../../node/agentConfigurationService.js'; +import { ActiveClientToolSet } from '../../node/activeClientState.js'; +import type { IAgentHostTerminalManager } from '../../node/agentHostTerminalManager.js'; import { ByokLmBridgeRegistry, IByokLmBridgeRegistry } from '../../node/byokLmBridgeRegistry.js'; import { ByokLmProxyService, IByokLmProxyService, type IByokLmProxyHandle } from '../../node/copilot/byokLmProxyService.js'; -import { CopilotSessionLauncher, getCopilotReasoningEffort, resolveByokSessionConfig } from '../../node/copilot/copilotSessionLauncher.js'; +import { CopilotSessionLauncher, getCopilotReasoningEffort, resolveByokSessionConfig, type CopilotSessionLaunchPlan, type ICopilotSessionRuntime } from '../../node/copilot/copilotSessionLauncher.js'; /** * Covers the BYOK provider/model synthesis the launcher feeds into @@ -253,6 +259,87 @@ suite('CopilotSessionLauncher BYOK proxy lifecycle', () => { }); }); +suite('CopilotSessionLauncher client identity', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('passes the Agent Host client name to create and resume', async () => { + const createConfigs: Parameters[0][] = []; + const resumeConfigs: Parameters[1][] = []; + const session = { + sessionId: 'session-1', + on: () => () => { }, + disconnect: async () => { }, + } as unknown as CopilotSession; + const client = { + createSession: async (config: Parameters[0]) => { + createConfigs.push(config); + return session; + }, + resumeSession: async (_sessionId: string, config: Parameters[1]) => { + resumeConfigs.push(config); + return session; + }, + }; + const configurationService = { + getRootValue: () => undefined, + } as Partial as IAgentConfigurationService; + const launcher = new CopilotSessionLauncher( + configurationService, + {} as IAgentHostTerminalManager, + new NullLogService(), + {} as IFileService, + { _serviceBrand: undefined, start: async () => { throw new Error('Unexpected proxy start'); }, dispose: () => { } }, + new ByokLmBridgeRegistry(), + ); + const runtime: ICopilotSessionRuntime = { + handlePermissionRequest: async () => { throw new Error('Unexpected permission request'); }, + handleExitPlanModeRequest: async () => { throw new Error('Unexpected exit plan mode request'); }, + handleUserInputRequest: async () => { throw new Error('Unexpected user input request'); }, + handleElicitationRequest: async () => { throw new Error('Unexpected elicitation request'); }, + handleMcpAuthRequest: async () => { throw new Error('Unexpected MCP auth request'); }, + requestUnsandboxedCommandConfirmation: async () => false, + handlePreToolUse: async () => { }, + handlePostToolUse: async () => { }, + createClientSdkTools: () => [], + createServerSdkTools: () => [], + }; + const basePlan = { + client, + sessionId: 'session-1', + workingDirectory: URI.file('/workspace'), + resolvedAgentName: undefined, + snapshot: { tools: [], plugins: [], mcpServers: {} }, + activeClientToolSet: new ActiveClientToolSet(), + shellManager: undefined, + githubToken: undefined, + }; + const createPlan: CopilotSessionLaunchPlan = { + ...basePlan, + kind: 'create', + model: undefined, + }; + const resumePlan: CopilotSessionLaunchPlan = { + ...basePlan, + kind: 'resume', + fallback: { model: undefined }, + }; + + const created = await launcher.launch(createPlan, runtime); + const resumed = await launcher.launch(resumePlan, runtime); + created.dispose(); + resumed.dispose(); + + assert.deepStrictEqual({ + createClientName: createConfigs[0].clientName, + resumeClientName: resumeConfigs[0].clientName, + }, { + createClientName: 'vscode-agent-host', + resumeClientName: 'vscode-agent-host', + }); + }); +}); + /** * Covers the reasoning-effort resolution fed into `createSession` and * `CopilotAgent._changeModel`: the host-level override (see diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/REMOTE_AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/remoteAgentHost/REMOTE_AGENT_HOST_SESSIONS_PROVIDER.md index c3a8fc6f1d7a1..78afc24ea3e72 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/REMOTE_AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/REMOTE_AGENT_HOST_SESSIONS_PROVIDER.md @@ -75,6 +75,7 @@ Decoupling these allows copilot sessions from different providers (local CLI, re - `clearConnection()` — Clears the connection when the host disconnects - Handles session notifications (`notify/sessionAdded`, `notify/sessionRemoved`) and state changes - Fires `onDidChangeSessionTypes` when the host's agent list changes +- Missing Copilot credentials open the standard product sign-in dialog before tokens are forwarded to the remote host. Authentication and transport failures propagate to the pending request; `false` is reserved for canceled or unavailable authentication. - Remote-host management options do not expose an IPC output channel; remote diagnostics use the host's forwarded logs when available. - SSH connection progress notifications are closed when the connect promise settles; keyboard-interactive prompt cancellation rejects the connect promise as cancellation and does not show an error notification. - SSH config host connections use resolved `IdentityFile` and `IdentityAgent` values from `ssh -G`; encrypted private keys are prompted for a passphrase through the same quick-input bridge as keyboard-interactive auth. diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts index 965c551854ee6..dbe49fa545913 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHost.contribution.ts @@ -969,11 +969,9 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc const authTokenCache = this._connections.get(address)?.authTokenCache; provider?.setAuthenticationPending(true); try { - await authenticateProtectedResources(agents, { + await this._instantiationService.invokeFunction(authenticateProtectedResources, agents, { authTokenCache, - authenticationService: this._authenticationService, logPrefix: '[RemoteAgentHost]', - logService: this._logService, authenticate: request => connection.authenticate(request), }); } catch (err) { @@ -989,18 +987,11 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc */ private async _resolveAuthenticationInteractively(address: string, connection: IAgentConnection, protectedResources: readonly ProtectedResourceMetadata[]): Promise { const authTokenCache = this._connections.get(address)?.authTokenCache; - try { - return await resolveAuthenticationInteractively(protectedResources, { - authTokenCache, - authenticationService: this._authenticationService, - logPrefix: '[RemoteAgentHost]', - logService: this._logService, - authenticate: request => connection.authenticate(request), - }); - } catch (err) { - this._logService.error('[RemoteAgentHost] Interactive authentication failed', err); - } - return false; + return this._instantiationService.invokeFunction(resolveAuthenticationInteractively, protectedResources, { + authTokenCache, + logPrefix: '[RemoteAgentHost]', + authenticate: request => connection.authenticate(request), + }); } } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts index 821b0d690e5ab..bcb7b7e014a65 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostAuth.ts @@ -4,17 +4,22 @@ *--------------------------------------------------------------------------------------------*/ import { fetchAuthorizationServerMetadata } from '../../../../../../base/common/oauth.js'; +import { CancellationError } from '../../../../../../base/common/errors.js'; import { URI } from '../../../../../../base/common/uri.js'; import { type McpOAuthClient, type ProtectedResourceMetadata } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; import { type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { ServicesAccessor } from '../../../../../../platform/instantiation/common/instantiation.js'; import { ILabelService } from '../../../../../../platform/label/common/label.js'; import { ILogService } from '../../../../../../platform/log/common/log.js'; +import { localize } from '../../../../../../nls.js'; import { IAuthenticationMcpAccessService } from '../../../../../services/authentication/browser/authenticationMcpAccessService.js'; import { IAuthenticationMcpService } from '../../../../../services/authentication/browser/authenticationMcpService.js'; import { IAuthenticationMcpUsageService } from '../../../../../services/authentication/browser/authenticationMcpUsageService.js'; import { AuthenticationSession, getDynamicAuthenticationProviderId, IAuthenticationService } from '../../../../../services/authentication/common/authentication.js'; import { IDynamicAuthenticationProviderStorageService } from '../../../../../services/authentication/common/dynamicAuthenticationProviderStorage.js'; +import { CHAT_SETUP_ACTION_ID } from '../../actions/chatActions.js'; +import { IChatSetupResult } from '../../chatSetup/chatSetup.js'; /** * Stable identity for an agent-host MCP server, used as the key for @@ -40,21 +45,60 @@ export function agentHostMcpServerId(authority: string, serverName: string, reso * when the connection is disposed. */ export class AgentHostAuthTokenCache { - private readonly _lastTokens = new Map(); + private readonly _completedTokens = new Map(); + private readonly _pendingAuthentications = new Map }>(); + private readonly _keyGenerations = new Map(); + private _globalGeneration = 0; /** - * Record that we just sent `token` for `resource` and `scopes`, and return - * whether this is a change from the last token sent. When `false`, callers - * should skip the `authenticate` RPC. + * Forwards a token once per resource/scope pair. Same-token callers share + * and await an in-flight authentication. */ - updateAndIsChanged(resource: string, scopes: readonly string[] | undefined, token: string): boolean { + async authenticate(resource: string, scopes: readonly string[] | undefined, token: string, authenticate: () => Promise): Promise { const key = this._key(resource, scopes); - const previous = this._lastTokens.get(key); - if (previous === token) { + const globalGeneration = this._globalGeneration; + const keyGeneration = this._keyGenerations.get(key) ?? 0; + const pending = this._pendingAuthentications.get(key); + if (pending) { + if (pending.token === token) { + await pending.promise; + if (!this._isCurrentGeneration(key, globalGeneration, keyGeneration)) { + throw new CancellationError(); + } + return false; + } + + try { + await pending.promise; + } catch { + // The newer token gets its own attempt regardless of the previous result. + } + if (!this._isCurrentGeneration(key, globalGeneration, keyGeneration)) { + throw new CancellationError(); + } + return this.authenticate(resource, scopes, token, authenticate); + } + + if (this._completedTokens.get(key) === token) { return false; } - this._lastTokens.set(key, token); - return true; + + const promise = (async () => { + await authenticate(); + if (!this._isCurrentGeneration(key, globalGeneration, keyGeneration)) { + throw new CancellationError(); + } + this._completedTokens.set(key, token); + })(); + this._pendingAuthentications.set(key, { token, promise }); + try { + await promise; + return true; + } finally { + if (this._pendingAuthentications.get(key)?.promise === promise) { + this._pendingAuthentications.delete(key); + } + } } /** @@ -65,20 +109,37 @@ export class AgentHostAuthTokenCache { clear(resource?: string, scopes?: readonly string[]): void { if (resource !== undefined) { if (scopes !== undefined) { - this._lastTokens.delete(this._key(resource, scopes)); + const key = this._key(resource, scopes); + this._invalidateKey(key); + this._completedTokens.delete(key); + this._pendingAuthentications.delete(key); return; } const prefix = `${resource}\x00`; - for (const key of [...this._lastTokens.keys()]) { + const keys = new Set([...this._completedTokens.keys(), ...this._pendingAuthentications.keys(), ...this._keyGenerations.keys()]); + for (const key of keys) { if (key.startsWith(prefix)) { - this._lastTokens.delete(key); + this._invalidateKey(key); + this._completedTokens.delete(key); + this._pendingAuthentications.delete(key); } } } else { - this._lastTokens.clear(); + this._globalGeneration++; + this._completedTokens.clear(); + this._pendingAuthentications.clear(); + this._keyGenerations.clear(); } } + private _invalidateKey(key: string): void { + this._keyGenerations.set(key, (this._keyGenerations.get(key) ?? 0) + 1); + } + + private _isCurrentGeneration(key: string, globalGeneration: number, keyGeneration: number): boolean { + return this._globalGeneration === globalGeneration && (this._keyGenerations.get(key) ?? 0) === keyGeneration; + } + private _key(resource: string, scopes: readonly string[] | undefined): string { return `${resource}\x00${scopes ? [...new Set(scopes)].sort().join('\x00') : ''}`; } @@ -150,9 +211,7 @@ export interface IAgentHostAuthenticateRequest { export interface IAgentHostAuthenticationOptions { readonly authTokenCache?: AgentHostAuthTokenCache; - readonly authenticationService: IAuthenticationService; readonly logPrefix: string; - readonly logService: ILogService; readonly authenticate: (request: IAgentHostAuthenticateRequest) => Promise; } @@ -177,14 +236,31 @@ export interface IAgentHostMcpAuthenticationOptionsBase { readonly authenticate: (request: IAgentHostAuthenticateRequest) => Promise; } +async function forwardAuthenticationToken( + options: Pick, + resource: string, + scopes: readonly string[], + token: string, +): Promise { + const request = { resource, scopes, token }; + if (options.authTokenCache) { + return options.authTokenCache.authenticate(resource, scopes, token, () => options.authenticate(request)); + } + await options.authenticate(request); + return true; +} + /** * Resolves and forwards bearer tokens for the protected resources declared by * the agents currently published from an agent host. */ export async function authenticateProtectedResources( + accessor: ServicesAccessor, agents: readonly AgentInfo[], options: IAgentHostAuthenticationOptions, ): Promise { + const authenticationService = accessor.get(IAuthenticationService); + const logService = accessor.get(ILogService); for (const agent of agents) { for (const resource of agent.protectedResources ?? []) { const resourceUri = URI.parse(resource.resource); @@ -193,27 +269,21 @@ export async function authenticateProtectedResources( resourceUri, resource.authorization_servers ?? [], scopes, - options.authenticationService, - options.logService, + authenticationService, + logService, options.logPrefix, ); if (!token) { - options.logService.info(`${options.logPrefix} No token resolved for resource: ${resource.resource}`); + logService.info(`${options.logPrefix} No token resolved for resource: ${resource.resource}`); continue; } - if (options.authTokenCache && !options.authTokenCache.updateAndIsChanged(resource.resource, scopes, token)) { - options.logService.trace(`${options.logPrefix} Auth token for ${resource.resource} unchanged; skipping authenticate RPC`); + const authenticated = await forwardAuthenticationToken(options, resource.resource, scopes, token); + if (!authenticated) { + logService.trace(`${options.logPrefix} Auth token for ${resource.resource} unchanged; skipping authenticate RPC`); continue; } - - options.logService.info(`${options.logPrefix} Authenticating for resource: ${resource.resource}`); - try { - await options.authenticate({ resource: resource.resource, scopes, token }); - } catch (err) { - options.authTokenCache?.clear(resource.resource, scopes); - throw err; - } + logService.info(`${options.logPrefix} Authenticating for resource: ${resource.resource}`); } } } @@ -223,44 +293,57 @@ export async function authenticateProtectedResources( * forwards the resulting token to the agent host connection. */ export async function resolveAuthenticationInteractively( + accessor: ServicesAccessor, protectedResources: readonly ProtectedResourceMetadata[], options: IAgentHostAuthenticationOptions, ): Promise { + const authenticationService = accessor.get(IAuthenticationService); + const commandService = accessor.get(ICommandService); + const logService = accessor.get(ILogService); for (const resource of protectedResources) { const resourceUri = URI.parse(resource.resource); const scopes = resource.scopes_supported ?? []; - const token = await resolveTokenForResource( + let token = await resolveTokenForResource( resourceUri, resource.authorization_servers ?? [], scopes, - options.authenticationService, - options.logService, + authenticationService, + logService, options.logPrefix, ); if (token) { - await options.authenticate({ resource: resource.resource, scopes, token }); - options.authTokenCache?.updateAndIsChanged(resource.resource, scopes, token); - options.logService.info(`${options.logPrefix} Interactive authentication succeeded for ${resource.resource}`); + await forwardAuthenticationToken(options, resource.resource, scopes, token); + logService.info(`${options.logPrefix} Interactive authentication succeeded for ${resource.resource}`); return true; } - for (const server of resource.authorization_servers ?? []) { - const serverUri = URI.parse(server); - const providerId = await options.authenticationService.getOrActivateProviderIdForServer(serverUri, resourceUri); - if (!providerId) { - continue; - } - - const session = await options.authenticationService.createSession(providerId, [...scopes], { - activateImmediate: true, - authorizationServer: serverUri, - }); - - await options.authenticate({ resource: resource.resource, scopes, token: session.accessToken }); - options.authTokenCache?.updateAndIsChanged(resource.resource, scopes, session.accessToken); - options.logService.info(`${options.logPrefix} Interactive authentication succeeded for ${resource.resource}`); - return true; + const setupResult = await commandService.executeCommand(CHAT_SETUP_ACTION_ID, undefined, { + forceSignInDialog: true, + additionalScopes: scopes, + dialogTitle: localize('agentHost.signInDialogTitle', "Sign in to use GitHub Copilot"), + disableChatViewReveal: true, + returnResult: true, + }); + if (setupResult?.success === undefined) { + return false; } + if (!setupResult.success) { + throw setupResult.error ?? new Error(localize('agentHost.signInFailed', "Failed to sign in to use GitHub Copilot.")); + } + token = await resolveTokenForResource( + resourceUri, + resource.authorization_servers ?? [], + scopes, + authenticationService, + logService, + options.logPrefix, + ); + if (!token) { + return false; + } + await forwardAuthenticationToken(options, resource.resource, scopes, token); + logService.info(`${options.logPrefix} Interactive authentication succeeded for ${resource.resource}`); + return true; } return false; @@ -423,8 +506,7 @@ async function authenticateMcpSession( updateAccess: boolean, agentHost: { readonly authority: string; readonly label: string } | undefined, ): Promise { - await options.authenticate({ resource: options.mcpServerUrl, scopes, token: session.accessToken }); - options.authTokenCache?.updateAndIsChanged(options.mcpServerUrl, scopes, session.accessToken); + await forwardAuthenticationToken(options, options.mcpServerUrl, scopes, session.accessToken); if (updateAccess) { authenticationMcpAccessService.updateAllowedMcpServers(providerId, session.account.label, [{ id: options.mcpServerId, name: options.mcpServerName, allowed: true, url: options.mcpServerUrl, agentHost }]); authenticationMcpService.updateAccountPreference(options.mcpServerId, providerId, session.account); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index b7d4a37396ff5..1e87cde05f195 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -312,11 +312,9 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr await this._seedTestToken(agents, testToken); return; } - await authenticateProtectedResources(agents, { + await this._instantiationService.invokeFunction(authenticateProtectedResources, agents, { authTokenCache: this._authTokenCache, - authenticationService: this._authenticationService, logPrefix: '[AgentHost]', - logService: this._logService, authenticate: request => this._agentHostService.authenticate(request), }); } catch (err) { @@ -336,37 +334,31 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr const testToken = this._getScenarioAutomationToken(); if (testToken !== undefined) { for (const resource of protectedResources) { - await this._agentHostService.authenticate({ resource: resource.resource, token: testToken }); - this._authTokenCache.updateAndIsChanged(resource.resource, resource.scopes_supported, testToken); + await this._authTokenCache.authenticate( + resource.resource, + resource.scopes_supported, + testToken, + () => this._agentHostService.authenticate({ resource: resource.resource, token: testToken }), + ); } return protectedResources.length > 0; } - try { - return await resolveAuthenticationInteractively(protectedResources, { - authTokenCache: this._authTokenCache, - authenticationService: this._authenticationService, - logPrefix: '[AgentHost]', - logService: this._logService, - authenticate: request => this._agentHostService.authenticate(request), - }); - } catch (err) { - this._logService.error('[AgentHost] Interactive authentication failed', err); - } - return false; + return this._instantiationService.invokeFunction(resolveAuthenticationInteractively, protectedResources, { + authTokenCache: this._authTokenCache, + logPrefix: '[AgentHost]', + authenticate: request => this._agentHostService.authenticate(request), + }); } private async _seedTestToken(agents: readonly AgentInfo[], token: string): Promise { for (const agent of agents) { for (const resource of agent.protectedResources ?? []) { - if (!this._authTokenCache.updateAndIsChanged(resource.resource, resource.scopes_supported, token)) { - continue; - } - try { - await this._agentHostService.authenticate({ resource: resource.resource, token }); - } catch (err) { - this._authTokenCache.clear(resource.resource); - throw err; - } + await this._authTokenCache.authenticate( + resource.resource, + resource.scopes_supported, + token, + () => this._agentHostService.authenticate({ resource: resource.resource, token }), + ); } } } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index 30cb64ecea2d9..c59f15ef056e9 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -91,7 +91,7 @@ import { buildHostLocalEventsPath } from '../../copilotCliEventsUri.js'; import { toolDataToDefinition } from './agentHostToolUtils.js'; import { IAgentHostUntitledProvisionalSessionService } from './agentHostUntitledProvisionalSessionService.js'; import { IAgentHostImportConversationStore } from './agentHostImportConversationStore.js'; -import { activeTurnToProgress, BOOLEAN_TRUE_OPTION_ID, completedToolCallToEditParts, completedToolCallToSerialized, convertProtocolAnswers, convertProtocolPlanReviewResult, createInputRequestCarousel, createInputRequestPlanReview, finalizeToolInvocation, formatTurnResponseDetails, getTerminalContentUri, getUrlInputRequestPresentation, isSubagentTool, makeAhpTerminalToolSessionId, messageAttachmentsToVariableData, messageToVariableData, parseAhpTerminalToolSessionId, rewriteAgentHostLinkTarget, stringOrMarkdownToString, systemNotificationToChatPart, toolCallAuthenticationServer, toolCallConfirmationMessages, toolCallStateToInvocation, turnsToHistory, updateRunningToolSpecificData, usageInfoToAutoModeResolution, usageInfoToChatUsage, usageInfoToQuotas, type IAgentHostToolInvocationOptions, type IToolCallFileEdit, type TurnModelLookup } from './stateToProgressAdapter.js'; +import { activeTurnToProgress, BOOLEAN_TRUE_OPTION_ID, completedToolCallToEditParts, completedToolCallToSerialized, convertProtocolAnswers, convertProtocolPlanReviewResult, createInputRequestCarousel, createInputRequestPlanReview, finalizeToolInvocation, formatTurnResponseDetails, getTerminalContentUri, getUrlInputRequestPresentation, isSubagentTool, makeAhpTerminalToolSessionId, messageAttachmentsToVariableData, messageToVariableData, parseAhpTerminalToolSessionId, rewriteAgentHostLinkTarget, stringOrMarkdownToString, systemNotificationToChatPart, toolCallAuthenticationServer, toolCallConfirmationMessages, toolCallStateToInvocation, toolCallStateToPreparedInvocation, toolCallStateToStreamingInvocation, turnsToHistory, updateRunningToolSpecificData, usageInfoToAutoModeResolution, usageInfoToChatUsage, usageInfoToQuotas, type IAgentHostToolInvocationOptions, type IToolCallFileEdit, type TurnModelLookup } from './stateToProgressAdapter.js'; import { resolveMcpServerAuthentication, agentHostMcpServerId } from './agentHostAuth.js'; export { toolDataToDefinition }; @@ -1207,6 +1207,8 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC imported ? { turns: imported.turns, model: imported.model } : undefined, ); } else { + await this._ensureRequiredAuthentication(); + // Eager-created session: take a refcounted subscription so the // handler observes state changes for the duration of the chat // session, then wire up the per-turn machinery that @@ -1792,7 +1794,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC session: URI, turnId: string, cancellationToken: CancellationToken, - protocolOptions?: ConfirmationOption[], + getProtocolOptions: () => ConfirmationOption[] | undefined, chatURI?: string, ): void { IChatToolInvocation.awaitConfirmation(invocation, cancellationToken).then(reason => { @@ -1800,6 +1802,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC // protocol option so we can forward `selectedOptionId` and // derive approve/deny from the option's kind. let selectedOption: ConfirmationOption | undefined; + const protocolOptions = getProtocolOptions(); if (reason.type === ToolConfirmKind.UserAction && reason.selectedButton && protocolOptions) { selectedOption = protocolOptions.find(o => o.id === reason.selectedButton); } @@ -2481,9 +2484,21 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC const toolCallId = initial.toolCallId; const subAgentInvocationId = opts.subAgentInvocationId; const adopted = opts.adoptInvocations?.get(toolCallId); - let invocation = adopted - ?? toolCallStateToInvocation(initial, subAgentInvocationId, opts.backendSession, this._config.connectionAuthority, opts.sessionResource.authority); - if (!adopted) { + let confirmationOptions = initial.status === ToolCallStatus.PendingConfirmation ? initial.options : undefined; + // Tools that stream their arguments (reliably: terminal/bash commands) + // are first observed in `Streaming`. Represent them with a native + // streaming `ChatToolInvocation` and later drive it through + // `transitionFromStreaming` (see the autorun below), so a single card + // spans the whole lifecycle instead of a settled placeholder plus a + // separate confirmation card (#314858). + let invocation: ChatToolInvocation; + if (adopted) { + invocation = adopted; + } else if (initial.status === ToolCallStatus.Streaming) { + invocation = toolCallStateToStreamingInvocation(initial, subAgentInvocationId); + opts.sink([invocation]); + } else { + invocation = toolCallStateToInvocation(initial, subAgentInvocationId, opts.backendSession, this._config.connectionAuthority, opts.sessionResource.authority); opts.sink([invocation]); } @@ -2550,46 +2565,35 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC this._observeSubagentSession(opts.sessionResource, opts.backendSession, toolCallId, childChatUri, rootInvocationId, invocation, opts.sink, store, subagentContext, perInvocationCredits, perInvocationModel); }; - // Initial confirmation hookup. The autorun below only handles - // *transitions* back into `PendingConfirmation` (server-driven - // re-confirmation), not the initial state, because - // `toolCallStateToInvocation` already created the invocation in - // `WaitingForConfirmation`. Without this explicit call, no listener - // would observe the user's confirmation answer. + // Hook up a tool first observed after it already entered confirmation. if (initial.status === ToolCallStatus.PendingConfirmation && !IChatToolInvocation.isComplete(invocation)) { - this._awaitToolConfirmation(invocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, initial.options, opts.chatURI); + this._awaitToolConfirmation(invocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, () => confirmationOptions, opts.chatURI); } tryObserveSubagent(initial); - // Stream subsequent status transitions. Re-confirmation is detected - // from a `tc.status` transition (Running → PendingConfirmation), not - // from comparing against `invocation.state`: the user's local - // confirmation flips `invocation.state` to `Executing` before the - // server echoes Running, and a state-comparison check would - // spuriously trigger re-confirmation in that gap. - let previousStatus: ToolCallStatus | undefined; + // Reuse the invocation whenever a tool enters confirmation to avoid duplicate cards. + let previousStatus: ToolCallStatus | undefined = initial.status; store.add(autorun(reader => { const tc = part$.read(reader).toolCall; const status = tc.status; const priorStatus = previousStatus; - const isReconfirmation = previousStatus !== undefined - && previousStatus !== ToolCallStatus.PendingConfirmation - && status === ToolCallStatus.PendingConfirmation; + if (status === ToolCallStatus.PendingConfirmation) { + confirmationOptions = tc.options; + } + const enteringConfirmation = status === ToolCallStatus.PendingConfirmation + && previousStatus !== ToolCallStatus.PendingConfirmation; previousStatus = status; - if (isReconfirmation) { - // Server bounced the call back to PendingConfirmation - // (e.g. write confirmation after edit). Settle the old - // invocation and replace it with a fresh one carrying the - // new confirmation messages. - invocation.didExecuteTool(undefined); - const confirmInvocation = toolCallStateToInvocation(tc, subAgentInvocationId, opts.backendSession, this._config.connectionAuthority, opts.sessionResource.authority); - opts.sink([confirmInvocation]); - invocation = confirmInvocation; - this._awaitToolConfirmation(confirmInvocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, tc.options, opts.chatURI); + if (enteringConfirmation) { + if (!IChatToolInvocation.isComplete(invocation)) { + const prepared = toolCallStateToPreparedInvocation(tc, opts.backendSession, this._config.connectionAuthority, opts.sessionResource.authority); + invocation.requestConfirmation(prepared); + this._awaitToolConfirmation(invocation, toolCallId, opts.backendSession, opts.turnId, opts.cancellationToken, () => confirmationOptions, opts.chatURI); + } } else if (status === ToolCallStatus.PendingConfirmation) { invocation.updateConfirmationMessages(toolCallConfirmationMessages(tc, this._config.connectionAuthority)); } else if (status === ToolCallStatus.AuthRequired) { + this._ensureLeftStreaming(invocation, tc, opts); invocation.setAuthenticationRequired(toolCallAuthenticationServer(tc, opts.sessionResource.authority), () => { this._dispatchAction(opts.backendSession, { type: ActionType.ChatToolCallComplete, @@ -2606,6 +2610,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC if (priorStatus === ToolCallStatus.AuthRequired) { invocation.setAuthenticationResolved(); } + this._ensureLeftStreaming(invocation, tc, opts); invocation.invocationMessage = stringOrMarkdownToString(tc.invocationMessage, this._config.connectionAuthority); this._reviveTerminalIfNeeded(invocation, tc, opts.backendSession); updateRunningToolSpecificData(invocation, tc, opts.backendSession, this._config.connectionAuthority); @@ -2617,6 +2622,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC // Revive terminal before finalizing — handles the case where // Running was skipped (e.g. throttling) and terminal content // only appears at Completed time. + this._ensureLeftStreaming(invocation, tc, opts); this._reviveTerminalIfNeeded(invocation, tc, opts.backendSession); const fileEdits = finalizeToolInvocation(invocation, tc, opts.backendSession, this._config.connectionAuthority); if (fileEdits.length > 0) { @@ -2634,6 +2640,19 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC })); } + /** Transitions an invocation from streaming once its AHP tool call is ready. */ + private _ensureLeftStreaming( + invocation: ChatToolInvocation, + tc: ToolCallState, + opts: IObserveTurnOptions, + ): void { + if (invocation.state.read(undefined).type !== IChatToolInvocation.StateKind.Streaming) { + return; + } + const prepared = toolCallStateToPreparedInvocation(tc, opts.backendSession, this._config.connectionAuthority, opts.sessionResource.authority); + invocation.transitionFromStreaming(prepared, undefined, undefined); + } + /** * Per-call setup for a client-provided tool. Eagerly creates a streaming * {@link ChatToolInvocation} so the UI has a handle, then invokes the @@ -3642,16 +3661,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC }; } - /** Creates a new backend session and subscribes to its state. */ - private async _createAndSubscribe(sessionResource: URI, model: ModelSelection | undefined, fork?: { session: URI; turnIndex: number; turnId: string }, config?: Record, importConversation?: { readonly turns: readonly Turn[]; readonly model?: ModelSelection }): Promise { - const workingDirectory = this._resolveRequestedWorkingDirectory(sessionResource); - const requestedSession = fork ? undefined : this._resolveSessionUri(sessionResource); - - this._logService.trace(`[AgentHost] Creating new session, model=${model?.id ?? '(default)'}, provider=${this._config.provider}${fork ? `, fork from ${fork.session.toString()} at index ${fork.turnIndex}` : ''}`); - - // Eagerly authenticate before creating the session if the agent - // declares required protected resources. This avoids a wasted - // round-trip where createSession fails with AuthRequired. + private async _ensureRequiredAuthentication(): Promise { const agentInfo = this._getRootState()?.agents.find(a => a.provider === this._config.provider); const protectedResources = agentInfo?.protectedResources ?? []; const hasRequiredAuth = protectedResources.some(r => r.required !== false); @@ -3661,6 +3671,17 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC throw new Error(localize('agentHost.authRequired', "Authentication is required to start a session. Please sign in and try again.")); } } + return protectedResources; + } + + /** Creates a new backend session and subscribes to its state. */ + private async _createAndSubscribe(sessionResource: URI, model: ModelSelection | undefined, fork?: { session: URI; turnIndex: number; turnId: string }, config?: Record, importConversation?: { readonly turns: readonly Turn[]; readonly model?: ModelSelection }): Promise { + const workingDirectory = this._resolveRequestedWorkingDirectory(sessionResource); + const requestedSession = fork ? undefined : this._resolveSessionUri(sessionResource); + + this._logService.trace(`[AgentHost] Creating new session, model=${model?.id ?? '(default)'}, provider=${this._config.provider}${fork ? `, fork from ${fork.session.toString()} at index ${fork.turnIndex}` : ''}`); + + const protectedResources = await this._ensureRequiredAuthentication(); const activeClient = this._getCurrentActiveClient(); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index 3525cb142a417..18bc453a64d14 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -32,7 +32,7 @@ import { ChatPlanReviewData } from '../../../common/model/chatProgressTypes/chat import { ChatQuestionCarouselData } from '../../../common/model/chatProgressTypes/chatQuestionCarouselData.js'; import { type IChatRequestVariableData } from '../../../common/model/chatModel.js'; import { AgentHostCompletionReferenceKind, restorePasteVariableEntryFromAttachment, toAgentHostCompletionVariableEntryFromMetadata, type IAgentFeedbackVariableEntry, type IChatRequestVariableEntry } from '../../../common/attachments/chatVariableEntries.js'; -import { type IToolConfirmationMessages, type IToolData, type IToolResult, type IToolResultInputOutputDetails, ToolDataSource, ToolInvocationPresentation } from '../../../common/tools/languageModelToolsService.js'; +import { type IToolConfirmationMessages, type IToolData, type IPreparedToolInvocation, type IToolResult, type IToolResultInputOutputDetails, ToolDataSource, ToolInvocationPresentation } from '../../../common/tools/languageModelToolsService.js'; import { MCP } from '../../../../mcp/common/modelContextProtocol.js'; import { basename } from '../../../../../../base/common/resources.js'; import { hasKey, type Mutable } from '../../../../../../base/common/types.js'; @@ -1124,8 +1124,13 @@ export function activeTurnToProgress(sessionResource: URI, activeTurn: ActiveTur break; case ResponsePartKind.ToolCall: { const tc = rp.toolCall; + const isOtherClientToolCall = tc.contributor?.kind === ToolCallContributorKind.Client + && toolInvocationOptions + && tc.contributor.clientId !== toolInvocationOptions.currentClientId; if (tc.status === ToolCallStatus.Completed || tc.status === ToolCallStatus.Cancelled) { parts.push(completedToolCallToSerialized(tc as ICompletedToolCall, undefined, sessionResource, connectionAuthority)); + } else if (tc.status === ToolCallStatus.Streaming && !isOtherClientToolCall) { + parts.push(toolCallStateToStreamingInvocation(tc, undefined)); } else if (tc.status === ToolCallStatus.Running || tc.status === ToolCallStatus.AuthRequired || tc.status === ToolCallStatus.Streaming || tc.status === ToolCallStatus.PendingConfirmation) { parts.push(toolCallStateToInvocation(tc, undefined, sessionResource, connectionAuthority, mcpServerAuthority, toolInvocationOptions)); } @@ -2142,6 +2147,46 @@ export function toolCallAuthenticationServer(tc: ToolCallState & { status: ToolC }; } +/** + * Creates a {@link ChatToolInvocation} in the native streaming state for a + * tool call that is still streaming its arguments (AHP + * {@link ToolCallStatus.Streaming}). The invocation is later driven out of the + * streaming state via {@link ChatToolInvocation.transitionFromStreaming} once + * the tool reaches confirmation/running, so a single card represents the whole + * lifecycle instead of a settled placeholder plus a replacement. + */ +export function toolCallStateToStreamingInvocation(tc: ToolCallState, subAgentInvocationId: string | undefined): ChatToolInvocation { + return ChatToolInvocation.createStreaming({ + toolCallId: tc.toolCallId, + toolId: tc.toolName, + toolData: { + id: tc.toolName, + source: ToolDataSource.Internal, + displayName: tc.displayName, + modelDescription: tc.toolName, + }, + subagentInvocationId: subAgentInvocationId, + }); +} + +/** + * Extracts the {@link IPreparedToolInvocation} display fields for a tool-call + * state, reusing {@link toolCallStateToInvocation} so the confirmation, + * terminal, and other `toolSpecificData` logic stays in one place. Used to + * transition a streaming invocation into its confirmation/running presentation + * without allocating a second visible card. + */ +export function toolCallStateToPreparedInvocation(tc: ToolCallState, sessionResource: URI, connectionAuthority: string, mcpServerAuthority = sessionResource.authority, options?: IAgentHostToolInvocationOptions): IPreparedToolInvocation { + const built = toolCallStateToInvocation(tc, undefined, sessionResource, connectionAuthority, mcpServerAuthority, options); + return { + invocationMessage: built.invocationMessage, + pastTenseMessage: built.pastTenseMessage, + confirmationMessages: built.confirmationMessages, + presentation: built.presentation, + toolSpecificData: built.toolSpecificData, + }; +} + /** * Updates a running tool invocation's `toolSpecificData` based on the * protocol tool call state. Handles terminal and subagent content detection. diff --git a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetup.ts b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetup.ts index 2d332be163558..6469175b54eec 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetup.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetup.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { ThemeIcon } from '../../../../../base/common/themables.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; @@ -58,6 +59,35 @@ export type ChatSetupResultValue = boolean /* success */ | undefined /* canceled export interface IChatSetupResult { readonly success: ChatSetupResultValue; readonly dialogSkipped: boolean; + readonly error?: Error; + readonly errorAlreadyHandled?: boolean; +} + +export class ChatSetupError extends Error { + constructor( + readonly originalError: Error, + readonly userNotified: boolean, + ) { + super(originalError.message, { cause: originalError }); + this.name = originalError.name; + } +} + +export interface IChatSetupRunOptions { + readonly disableChatViewReveal?: boolean; + readonly forceSignInDialog?: boolean; + readonly additionalScopes?: readonly string[]; + readonly forceAnonymous?: ChatSetupAnonymous; + readonly dialogIcon?: ThemeIcon; + readonly dialogTitle?: string; + readonly setupStrategy?: ChatSetupStrategy; + readonly disableCloseButton?: boolean; + readonly onSignInStarted?: () => void; +} + +export interface IChatSetupCommandOptions extends IChatSetupRunOptions { + readonly inputValue?: string; + readonly returnResult?: boolean; } export function refreshTokens(commandService: ICommandService): void { diff --git a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupContributions.ts b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupContributions.ts index ab618867bd46e..1a2e1bfbcf904 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupContributions.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupContributions.ts @@ -56,7 +56,7 @@ import { CHAT_CATEGORY, CHAT_SETUP_ACTION_ID, CHAT_SETUP_SUPPORT_ANONYMOUS_ACTIO import { ChatViewContainerId, IChatWidget, IChatWidgetService } from '../chat.js'; import { ChatInputNotificationSeverity, IChatInputNotificationService } from '../widget/input/chatInputNotificationService.js'; import { chatViewsWelcomeRegistry } from '../viewsWelcome/chatViewsWelcome.js'; -import { buildUpgradeUrlWithRedirect, ChatSetupAnonymous, ChatSetupStrategy, refreshTokens } from './chatSetup.js'; +import { buildUpgradeUrlWithRedirect, ChatSetupAnonymous, ChatSetupStrategy, IChatSetupCommandOptions, IChatSetupResult, refreshTokens } from './chatSetup.js'; import { ChatSetupController } from './chatSetupController.js'; import { GrowthSessionController, registerGrowthSession } from './chatSetupGrowthSession.js'; import { AICodeActionsHelper, AINewSymbolNamesProvider, ChatCodeActionsProvider, SetupAgent } from './chatSetupProviders.js'; @@ -241,7 +241,7 @@ export class ChatSetupContribution extends Disposable implements IWorkbenchContr }); } - override async run(accessor: ServicesAccessor, mode?: ChatModeKind | string, options?: { forceSignInDialog?: boolean; additionalScopes?: readonly string[]; forceAnonymous?: ChatSetupAnonymous; inputValue?: string; dialogIcon?: ThemeIcon; dialogTitle?: string; setupStrategy?: ChatSetupStrategy; disableCloseButton?: boolean; onSignInStarted?: () => void }): Promise { + override async run(accessor: ServicesAccessor, mode?: ChatModeKind | string, options?: IChatSetupCommandOptions): Promise { const widgetService = accessor.get(IChatWidgetService); const instantiationService = accessor.get(IInstantiationService); const dialogService = accessor.get(IDialogService); @@ -269,8 +269,12 @@ export class ChatSetupContribution extends Disposable implements IWorkbenchContr } const setup = ChatSetup.getInstance(instantiationService, context, controller); - const { success } = await setup.run(options); - if (success === false && !lifecycleService.willShutdown) { + const result = await setup.run(options); + if (options?.returnResult) { + return result; + } + const { success } = result; + if (success === false && !result.errorAlreadyHandled && !lifecycleService.willShutdown) { const { confirmed } = await dialogService.confirm({ type: Severity.Error, message: localize('setupErrorDialog', "Chat setup failed. Would you like to try again?"), diff --git a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupController.ts b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupController.ts index 27409c0063457..be58d1559f47a 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupController.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupController.ts @@ -27,7 +27,7 @@ import { IExtensionsWorkbenchService } from '../../../extensions/common/extensio import { ChatEntitlement, ChatEntitlementContext, ChatEntitlementRequests, isProUser } from '../../../../services/chat/common/chatEntitlementService.js'; import { CHAT_OPEN_ACTION_ID } from '../actions/chatActions.js'; import { ChatViewContainerId, ChatViewId } from '../chat.js'; -import { ChatSetupAnonymous, ChatSetupStep, ChatSetupResultValue, InstallChatEvent, InstallChatClassification, refreshTokens, maybeEnableAuthExtension } from './chatSetup.js'; +import { ChatSetupAnonymous, ChatSetupError, ChatSetupStep, ChatSetupResultValue, InstallChatEvent, InstallChatClassification, refreshTokens, maybeEnableAuthExtension } from './chatSetup.js'; import { IDefaultAccount } from '../../../../../base/common/defaultAccount.js'; import { IDefaultAccountService } from '../../../../../platform/defaultAccount/common/defaultAccount.js'; import { IProductService } from '../../../../../platform/product/common/productService.js'; @@ -158,10 +158,12 @@ export class ChatSetupController extends Disposable { let entitlements; let defaultAccount; + let signInError: Error | undefined; try { ({ defaultAccount, entitlements } = await this.requests.signIn(options)); } catch (e) { this.logService.error(`[chat setup] signIn: error ${e}`); + signInError = e instanceof Error ? e : new Error(String(e)); } if (!defaultAccount && !this.lifecycleService.willShutdown) { @@ -176,6 +178,9 @@ export class ChatSetupController extends Disposable { return this.signIn(options); } } + if (signInError) { + throw new ChatSetupError(signInError, true); + } return { defaultAccount, entitlement: entitlements?.entitlement }; } diff --git a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupRunner.ts b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupRunner.ts index 8e6786f314b60..02fab4f2650a6 100644 --- a/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupRunner.ts +++ b/src/vs/workbench/contrib/chat/browser/chatSetup/chatSetupRunner.ts @@ -9,7 +9,6 @@ import { IButton } from '../../../../../base/browser/ui/button/button.js'; import { Dialog, DialogContentsAlignment } from '../../../../../base/browser/ui/dialog/dialog.js'; import { coalesce } from '../../../../../base/common/arrays.js'; import { Codicon } from '../../../../../base/common/codicons.js'; -import { ThemeIcon } from '../../../../../base/common/themables.js'; import { toErrorMessage } from '../../../../../base/common/errorMessage.js'; import { MarkdownString } from '../../../../../base/common/htmlContent.js'; import { Lazy } from '../../../../../base/common/lazy.js'; @@ -29,7 +28,7 @@ import { IWorkbenchLayoutService } from '../../../../services/layout/browser/lay import { ChatEntitlement, ChatEntitlementContext, ChatEntitlementService, IChatEntitlementService, isProUser } from '../../../../services/chat/common/chatEntitlementService.js'; import { IChatWidgetService } from '../chat.js'; import { ChatSetupController } from './chatSetupController.js'; -import { IChatSetupResult, ChatSetupAnonymous, InstallChatEvent, InstallChatClassification, ChatSetupStrategy, ChatSetupResultValue } from './chatSetup.js'; +import { IChatSetupResult, ChatSetupAnonymous, ChatSetupError, InstallChatEvent, InstallChatClassification, ChatSetupStrategy, ChatSetupResultValue, IChatSetupRunOptions } from './chatSetup.js'; import { GitHubPaths, IDefaultAccountService } from '../../../../../platform/defaultAccount/common/defaultAccount.js'; import { IHostService } from '../../../../services/host/browser/host.js'; import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; @@ -82,7 +81,7 @@ export class ChatSetup { this.skipDialogOnce = true; } - async run(options?: { disableChatViewReveal?: boolean; forceSignInDialog?: boolean; additionalScopes?: readonly string[]; forceAnonymous?: ChatSetupAnonymous; dialogIcon?: ThemeIcon; dialogTitle?: string; setupStrategy?: ChatSetupStrategy; disableCloseButton?: boolean; onSignInStarted?: () => void }): Promise { + async run(options?: IChatSetupRunOptions): Promise { if (this.pendingRun) { return this.pendingRun; } @@ -96,7 +95,7 @@ export class ChatSetup { } } - private async doRun(options?: { disableChatViewReveal?: boolean; forceSignInDialog?: boolean; additionalScopes?: readonly string[]; forceAnonymous?: ChatSetupAnonymous; dialogIcon?: ThemeIcon; dialogTitle?: string; setupStrategy?: ChatSetupStrategy; disableCloseButton?: boolean; onSignInStarted?: () => void }): Promise { + private async doRun(options?: IChatSetupRunOptions): Promise { this.context.update({ later: false }); const dialogSkipped = this.skipDialogOnce; @@ -148,6 +147,8 @@ export class ChatSetup { } let success: ChatSetupResultValue = undefined; + let setupError: Error | undefined; + let errorAlreadyHandled = false; try { switch (setupStrategy) { case ChatSetupStrategy.SetupWithEnterpriseProvider: @@ -173,13 +174,19 @@ export class ChatSetup { } catch (error) { this.logService.error(`[chat setup] Error during setup: ${toErrorMessage(error)}`); success = false; + if (error instanceof ChatSetupError) { + setupError = error.originalError; + errorAlreadyHandled = error.userNotified; + } else { + setupError = error instanceof Error ? error : new Error(toErrorMessage(error)); + } } if (success) { this.context.update({ completed: true }); } - return { success, dialogSkipped }; + return { success, dialogSkipped, error: setupError, errorAlreadyHandled }; } /** @@ -222,7 +229,7 @@ export class ChatSetup { } } - private async showDialog(options?: { forceSignInDialog?: boolean; forceAnonymous?: ChatSetupAnonymous; dialogIcon?: ThemeIcon; dialogTitle?: string; disableCloseButton?: boolean; onSignInStarted?: () => void }): Promise { + private async showDialog(options?: IChatSetupRunOptions): Promise { const disposables = new DisposableStore(); const buttons = this.getButtons(options); @@ -250,7 +257,7 @@ export class ChatSetup { return buttons[button]?.[1] ?? ChatSetupStrategy.Canceled; } - private getButtons(options?: { forceSignInDialog?: boolean; forceAnonymous?: ChatSetupAnonymous }): Array<[string, ChatSetupStrategy, { styleButton?: (button: IButton) => void } | undefined]> { + private getButtons(options?: IChatSetupRunOptions): Array<[string, ChatSetupStrategy, { styleButton?: (button: IButton) => void } | undefined]> { type ContinueWithButton = [string, ChatSetupStrategy, { styleButton?: (button: IButton) => void } | undefined]; const styleButton = (...classes: string[]) => ({ styleButton: (button: IButton) => button.element.classList.add(...classes) }); @@ -287,7 +294,7 @@ export class ChatSetup { return buttons; } - private getDialogTitle(options?: { forceSignInDialog?: boolean; forceAnonymous?: ChatSetupAnonymous; dialogTitle?: string }): string { + private getDialogTitle(options?: IChatSetupRunOptions): string { if (options?.dialogTitle) { return options.dialogTitle; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 4eab0cd9fa505..b9c96d26ab6bb 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -157,6 +157,7 @@ export interface IChatListItemTemplate { readonly titleToolbar?: MenuWorkbenchToolBar; readonly header?: HTMLElement; readonly footerToolbar: MenuWorkbenchToolBar; + readonly footerToolbarContainer: HTMLElement; readonly footerDetailsContainer: HTMLElement; readonly avatarContainer: HTMLElement; readonly username: HTMLElement; @@ -837,7 +838,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer { @@ -1117,8 +1118,18 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer setGroupHover(true))); - templateData.elementDisposables.add(dom.addDisposableListener(templateData.rowContainer, dom.EventType.MOUSE_LEAVE, () => setGroupHover(false))); + const hoverTargets = isResponseVM(element) + ? [templateData.value, templateData.footerToolbarContainer] + : [templateData.rowContainer]; + const isHoverTarget = (target: EventTarget | null) => dom.isHTMLElement(target) && hoverTargets.some(hoverTarget => hoverTarget.contains(target)); + for (const hoverTarget of hoverTargets) { + templateData.elementDisposables.add(dom.addDisposableListener(hoverTarget, dom.EventType.MOUSE_ENTER, () => setGroupHover(true))); + templateData.elementDisposables.add(dom.addDisposableListener(hoverTarget, dom.EventType.MOUSE_LEAVE, e => { + if (!isHoverTarget(e.relatedTarget)) { + setGroupHover(false); + } + })); + } templateData.elementDisposables.add(toDisposable(() => setGroupHover(false))); } @@ -2181,9 +2192,6 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.experimental.renderMarkdownImmediately') === true; - // When incremental rendering is enabled, skip word-counting for markdown. // The morpher's own buffer + rAF loop is the sole rate limiter. const incrementalRendering = this.configService.getValue(ChatConfiguration.IncrementalRendering) === true; @@ -2201,7 +2209,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.experimental.renderMarkdownImmediately'); - const delay = renderImmediately ? MicrotaskDelay : 0; - this.viewModelDisposables.add(Event.runAndSubscribe(Event.accumulate(this.viewModel.onDidChange, delay), (events => { + this.viewModelDisposables.add(Event.runAndSubscribe(Event.accumulate(this.viewModel.onDidChange), (events => { if (!this.viewModel || this._store.isDisposed) { // See https://github.com/microsoft/vscode/issues/278969 return; diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index eb872cad9d4ee..30ef19e46e9ed 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -175,13 +175,8 @@ font-size: 14px; } -.monaco-list-row:not(.focused) .interactive-item-container:not(:hover) .header .monaco-toolbar, -.monaco-list:not(:focus-within) .monaco-list-row .interactive-item-container:not(:hover) .header .monaco-toolbar, -.monaco-list-row:not(.focused) .interactive-item-container:not(:hover) .header .monaco-toolbar .action-label, -.monaco-list:not(:focus-within) .monaco-list-row .interactive-item-container:not(:hover) .header .monaco-toolbar .action-label { - /* Also apply this rule to the .action-label directly to work around a strange issue- when the - toolbar is hidden without that second rule, tabbing from the list container into a list item doesn't work - and the tab key doesn't do anything. */ +.interactive-item-container:not(:hover):not(:focus-within) .header .monaco-toolbar, +.interactive-item-container:not(:hover):not(:focus-within) .header .monaco-toolbar .action-label { display: none; } @@ -278,25 +273,19 @@ /* Complete response only */ display: block; opacity: 0; - visibility: hidden; padding-top: 6px; height: 22px; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0.1s; + transition: opacity 0.1s ease-in-out; } -/* Show toolbar on hover and last response. Also show when the item has keyboard focus (focus-within) or when the surrounding list row is marked focused (monaco list keyboard navigation). */ -.interactive-item-container.interactive-response:not(.chat-response-loading):hover .chat-footer-toolbar, +/* Show toolbar on hover and last response. Keep it visible when keyboard focus moves inside the response. */ .interactive-item-container.interactive-response.chat-most-recent-response:not(.chat-response-loading) .chat-footer-toolbar, -.interactive-item-container.interactive-response:not(.chat-response-loading):hover .chat-footer-toolbar .chat-footer-details, .interactive-item-container.interactive-response:not(.chat-response-loading):focus-within .chat-footer-toolbar, .interactive-item-container.interactive-response:not(.chat-response-loading):focus-within .chat-footer-toolbar .chat-footer-details, -.monaco-list-row.focused .interactive-item-container.interactive-response:not(.chat-response-loading) .chat-footer-toolbar, -.monaco-list-row.focused .interactive-item-container.interactive-response:not(.chat-response-loading) .chat-footer-toolbar .chat-footer-details, .interactive-item-container.interactive-response:not(.chat-response-loading).group-hovered .chat-footer-toolbar, .interactive-item-container.interactive-response:not(.chat-response-loading).group-hovered .chat-footer-toolbar .chat-footer-details { opacity: 1; - visibility: visible; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0s; + transition: opacity 0.1s ease-in-out; } /* Style the internal toolbar element to use flexbox */ @@ -3810,20 +3799,17 @@ have to be updated for changes to the rules above, or to support more deeply nes font-size: var(--vscode-fontSize-label3); line-height: var(--vscode-fontSize-label3); opacity: 0.7; - visibility: visible; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0s; + transition: opacity 0.1s ease-in-out; } .interactive-item-container.interactive-request:not(.group-hovered) .chat-request-timestamp { opacity: 0; - visibility: hidden; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0.1s; + transition: opacity 0.1s ease-in-out; } - .interactive-item-container.interactive-request:not(.group-hovered) .chat-request-timestamp:focus-visible { + .interactive-item-container.interactive-request:not(.group-hovered) .chat-request-timestamp:focus { opacity: 0.7; - visibility: visible; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0s; + transition: opacity 0.1s ease-in-out; } .interactive-item-container.interactive-request .chat-attached-context { @@ -4290,20 +4276,6 @@ have to be updated for changes to the rules above, or to support more deeply nes .interactive-item-container .value .rendered-markdown { outline: 1px solid var(--vscode-focusBorder); } - - .request-hover:not(.has-no-actions) { - display: block; - } - - .checkpoint-container { - opacity: 1; - } - - .chat-request-timestamp { - opacity: 0.7; - visibility: visible; - transition: opacity 0.1s ease-in-out, visibility 0s linear 0s; - } } .interactive-request.editing .rendered-markdown, diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index e70e1d8fac373..0847905983712 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -286,11 +286,10 @@ export function isSupportedChatFileScheme(accessor: ServicesAccessor, scheme: st * Returns the effective default session type for a new chat in the VS Code * editor window. * - * When the agent host is enabled and `chat.defaultToCopilotHarness` is opted - * in, Agent Host Copilot CLI is the default so that first-time users land on - * Copilot instead of the local harness. Otherwise it falls back to - * {@link localChatSessionType} when local is enabled, or to the first visible - * non-local provider. + * Virtual workspaces always default to {@link localChatSessionType}. Otherwise, + * when the agent host is enabled and `chat.defaultToCopilotHarness` is opted in, + * Agent Host Copilot CLI is the default. It falls back to the local harness + * when enabled, or to the first visible non-local provider. */ export function getComputedDefaultSessionType( configurationService: IConfigurationService, @@ -298,6 +297,10 @@ export function getComputedDefaultSessionType( workspace: IWorkspace, agentHostEnabled: boolean ): string { + if (isVirtualWorkspace(workspace)) { + return localChatSessionType; + } + if (agentHostEnabled && configurationService.getValue(ChatConfiguration.DefaultToCopilotHarness)) { return SessionType.AgentHostCopilot; } diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts index d543eba0347f7..5deaa5d2b9b8a 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts @@ -135,28 +135,36 @@ export class ChatToolInvocation implements IChatToolInvocation { type: IChatToolInvocation.StateKind.WaitingForConfirmation, parameters: this.parameters, confirmationMessages: this.confirmationMessages, - confirm: reason => { - if (reason.type === ToolConfirmKind.Denied || reason.type === ToolConfirmKind.Skipped) { - this._state.set({ - type: IChatToolInvocation.StateKind.Cancelled, - reason: reason.type, - parameters: this.parameters, - confirmationMessages: this.confirmationMessages, - }, undefined); - } else { - this._state.set({ - type: IChatToolInvocation.StateKind.Executing, - confirmed: reason, - progress: this._progress, - parameters: this.parameters, - confirmationMessages: this.confirmationMessages, - }, undefined); - } - } + confirm: reason => this._confirm(reason), }); } } + /** + * Shared confirmation handler used by every `WaitingForConfirmation` state + * this invocation can enter (initial construction, transition out of + * streaming, and re-arming via {@link requestConfirmation}). Denials/skips + * cancel; anything else moves to executing. + */ + private _confirm(reason: ConfirmedReason): void { + if (reason.type === ToolConfirmKind.Denied || reason.type === ToolConfirmKind.Skipped) { + this._state.set({ + type: IChatToolInvocation.StateKind.Cancelled, + reason: reason.type, + parameters: this.parameters, + confirmationMessages: this.confirmationMessages, + }, undefined); + } else { + this._state.set({ + type: IChatToolInvocation.StateKind.Executing, + confirmed: reason, + progress: this._progress, + parameters: this.parameters, + confirmationMessages: this.confirmationMessages, + }, undefined); + } + } + /** * Update the partial input observable during streaming. */ @@ -246,28 +254,9 @@ export class ChatToolInvocation implements IChatToolInvocation { this.toolSpecificData = preparedInvocation.toolSpecificData; } - const confirm = (reason: ConfirmedReason) => { - if (reason.type === ToolConfirmKind.Denied || reason.type === ToolConfirmKind.Skipped) { - this._state.set({ - type: IChatToolInvocation.StateKind.Cancelled, - reason: reason.type, - parameters: this.parameters, - confirmationMessages: this.confirmationMessages, - }, undefined); - } else { - this._state.set({ - type: IChatToolInvocation.StateKind.Executing, - confirmed: reason, - progress: this._progress, - parameters: this.parameters, - confirmationMessages: this.confirmationMessages, - }, undefined); - } - }; - // Transition to the appropriate state if (autoConfirmed) { - confirm(autoConfirmed); + this._confirm(autoConfirmed); } else if (!this.confirmationMessages?.title) { this._state.set({ type: IChatToolInvocation.StateKind.Executing, @@ -281,11 +270,44 @@ export class ChatToolInvocation implements IChatToolInvocation { type: IChatToolInvocation.StateKind.WaitingForConfirmation, parameters: this.parameters, confirmationMessages: this.confirmationMessages, - confirm, + confirm: reason => this._confirm(reason), }, undefined); } } + /** Moves an active invocation into confirmation while preserving the same tool card. */ + public requestConfirmation(preparedInvocation: IPreparedToolInvocation): void { + const currentType = this._state.get().type; + if (currentType === IChatToolInvocation.StateKind.Streaming) { + this.transitionFromStreaming(preparedInvocation, this.parameters, undefined); + return; + } + if (currentType === IChatToolInvocation.StateKind.Completed + || currentType === IChatToolInvocation.StateKind.Cancelled + || currentType === IChatToolInvocation.StateKind.WaitingForConfirmation) { + return; + } + + if (preparedInvocation.invocationMessage) { + this.invocationMessage = preparedInvocation.invocationMessage; + } + this.pastTenseMessage = preparedInvocation.pastTenseMessage; + this.confirmationMessages = preparedInvocation.confirmationMessages; + this.presentation = preparedInvocation.presentation; + this.toolSpecificData = preparedInvocation.toolSpecificData; + + if (!this.confirmationMessages?.title) { + return; // nothing to confirm + } + + this._state.set({ + type: IChatToolInvocation.StateKind.WaitingForConfirmation, + parameters: this.parameters, + confirmationMessages: this.confirmationMessages, + confirm: reason => this._confirm(reason), + }, undefined); + } + private _setCompleted(result: IToolResult | undefined, postConfirmed?: ConfirmedReason | undefined) { if (postConfirmed && (postConfirmed.type === ToolConfirmKind.Denied || postConfirmed.type === ToolConfirmKind.Skipped)) { this._state.set({ diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts index 98df00d6d20ed..4fa74abe50a83 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostAuth.test.ts @@ -4,10 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { DeferredPromise } from '../../../../../../base/common/async.js'; import { Event } from '../../../../../../base/common/event.js'; +import { DisposableStore } from '../../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../../base/common/uri.js'; import { type ProtectedResourceMetadata } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; import { type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; +import { mock } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; @@ -16,7 +20,28 @@ import { IAuthenticationMcpService } from '../../../../../services/authenticatio import { IAuthenticationMcpUsageService } from '../../../../../services/authentication/browser/authenticationMcpUsageService.js'; import { IAuthenticationService, type IAuthenticationProvider } from '../../../../../services/authentication/common/authentication.js'; import { IDynamicAuthenticationProviderStorageService } from '../../../../../services/authentication/common/dynamicAuthenticationProviderStorage.js'; -import { authenticateProtectedResources, resolveAuthenticationInteractively, resolveTokenForResource, AgentHostAuthTokenCache, agentHostMcpServerId, resolveMcpServerAuthentication } from '../../../browser/agentSessions/agentHost/agentHostAuth.js'; +import { CHAT_SETUP_ACTION_ID } from '../../../browser/actions/chatActions.js'; +import { authenticateProtectedResources, resolveAuthenticationInteractively, resolveTokenForResource, AgentHostAuthTokenCache, agentHostMcpServerId, resolveMcpServerAuthentication, type IAgentHostAuthenticationOptions } from '../../../browser/agentSessions/agentHost/agentHostAuth.js'; + +class TestCommandService extends mock() { + readonly calls: { commandId: string; args: unknown[] }[] = []; + result: unknown = { success: true, dialogSkipped: false }; + onExecute: (() => void) | undefined; + + override async executeCommand(commandId: string, ...args: unknown[]): Promise { + this.calls.push({ commandId, args }); + this.onExecute?.(); + return this.result as R; + } +} + +function createAuthInstantiationService(disposables: Pick, authenticationService: IAuthenticationService, commandService = new TestCommandService()): TestInstantiationService { + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IAuthenticationService, authenticationService); + instantiationService.stub(ICommandService, commandService); + instantiationService.stub(ILogService, new NullLogService()); + return instantiationService; +} function createMockAuthService(overrides: { getOrActivateProviderIdForServer?: (serverUri: URI, resourceUri: URI) => Promise; @@ -160,49 +185,187 @@ suite('AgentHostAuthTokenCache', () => { ensureNoDisposablesAreLeakedInTestSuite(); - test('first token for a resource is reported as changed', () => { + test('forwards the first token and skips it after completion', async () => { + const cache = new AgentHostAuthTokenCache(); + let authenticateCalls = 0; + const authenticate = async () => { authenticateCalls++; }; + + const results = [ + await cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate), + await cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate), + ]; + + assert.deepStrictEqual({ results, authenticateCalls }, { results: [true, false], authenticateCalls: 1 }); + }); + + test('same-token callers await the in-flight authentication', async () => { const cache = new AgentHostAuthTokenCache(); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), true); + const authentication = new DeferredPromise(); + let authenticateCalls = 0; + const authenticate = async () => { + authenticateCalls++; + await authentication.p; + }; + let secondSettled = false; + + const first = cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate); + const second = cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate).then(result => { + secondSettled = true; + return result; + }); + await Promise.resolve(); + const beforeCompletion = { authenticateCalls, secondSettled }; + authentication.complete(); + + assert.deepStrictEqual({ + beforeCompletion, + results: await Promise.all([first, second]), + authenticateCalls, + }, { + beforeCompletion: { authenticateCalls: 1, secondSettled: false }, + results: [true, false], + authenticateCalls: 1, + }); + }); + + test('different tokens are serialized for the same resource and scopes', async () => { + const cache = new AgentHostAuthTokenCache(); + const firstAuthentication = new DeferredPromise(); + const calls: string[] = []; + + const first = cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + calls.push('tok1'); + await firstAuthentication.p; + }); + const second = cache.authenticate('https://api.example.com', ['read'], 'tok2', async () => { + calls.push('tok2'); + }); + await Promise.resolve(); + const beforeCompletion = [...calls]; + firstAuthentication.complete(); + await Promise.all([first, second]); + + assert.deepStrictEqual({ beforeCompletion, calls }, { beforeCompletion: ['tok1'], calls: ['tok1', 'tok2'] }); }); - test('repeating the same token for the same resource is reported as unchanged', () => { + test('a completed token waits for a newer in-flight authentication', async () => { const cache = new AgentHostAuthTokenCache(); - cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), false); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), false); + const newerAuthentication = new DeferredPromise(); + const calls: string[] = []; + await cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + calls.push('tok1'); + }); + const newer = cache.authenticate('https://api.example.com', ['read'], 'tok2', async () => { + calls.push('tok2'); + await newerAuthentication.p; + }); + let olderSettled = false; + const older = cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + calls.push('tok1'); + }).then(result => { + olderSettled = true; + return result; + }); + await Promise.resolve(); + const beforeCompletion = { calls: [...calls], olderSettled }; + newerAuthentication.complete(); + + assert.deepStrictEqual({ + beforeCompletion, + results: await Promise.all([newer, older]), + calls, + }, { + beforeCompletion: { calls: ['tok1', 'tok2'], olderSettled: false }, + results: [true, true], + calls: ['tok1', 'tok2', 'tok1'], + }); }); - test('a different token for the same resource is reported as changed', () => { + test('clear cancels queued authentication from the previous generation', async () => { const cache = new AgentHostAuthTokenCache(); - cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok2'), true); - // And the new token is now the cached one. - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok2'), false); + const firstAuthentication = new DeferredPromise(); + const calls: string[] = []; + const first = cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + calls.push('tok1'); + await firstAuthentication.p; + }); + const queued = cache.authenticate('https://api.example.com', ['read'], 'tok2', async () => { + calls.push('tok2'); + }); + cache.clear(); + await cache.authenticate('https://api.example.com', ['read'], 'tok3', async () => { + calls.push('tok3'); + }); + firstAuthentication.complete(); + + await assert.rejects(first); + await assert.rejects(queued); + assert.deepStrictEqual(calls, ['tok1', 'tok3']); }); - test('tokens for distinct scopes are tracked independently', () => { + test('scoped clear does not cancel unrelated in-flight authentication', async () => { const cache = new AgentHostAuthTokenCache(); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'read-token'), true); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['write'], 'write-token'), true); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'read-token'), false); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['write'], 'write-token'), false); + const unrelatedAuthentication = new DeferredPromise(); + let unrelatedCalls = 0; + const unrelated = cache.authenticate('https://other.example.com', ['read'], 'other-token', async () => { + unrelatedCalls++; + await unrelatedAuthentication.p; + }); + cache.clear('https://api.example.com', ['read']); + unrelatedAuthentication.complete(); + + assert.deepStrictEqual({ + result: await unrelated, + unrelatedCalls, + repeated: await cache.authenticate('https://other.example.com', ['read'], 'other-token', async () => { + unrelatedCalls++; + }), + }, { + result: true, + unrelatedCalls: 1, + repeated: false, + }); }); - test('tokens for distinct resources are tracked independently', () => { + test('tokens for distinct scopes and resources are tracked independently', async () => { const cache = new AgentHostAuthTokenCache(); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), true); - assert.strictEqual(cache.updateAndIsChanged('https://other.example.com', ['read'], 'tok1'), true); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), false); - assert.strictEqual(cache.updateAndIsChanged('https://other.example.com', ['read'], 'tok1'), false); + let authenticateCalls = 0; + const authenticate = async () => { authenticateCalls++; }; + + await Promise.all([ + cache.authenticate('https://api.example.com', ['read'], 'read-token', authenticate), + cache.authenticate('https://api.example.com', ['write'], 'write-token', authenticate), + cache.authenticate('https://other.example.com', ['read'], 'read-token', authenticate), + ]); + + assert.strictEqual(authenticateCalls, 3); }); - test('clear forgets every cached token', () => { + test('failed authentication is not cached', async () => { const cache = new AgentHostAuthTokenCache(); - cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'); - cache.updateAndIsChanged('https://other.example.com', ['read'], 'tok2'); + let authenticateCalls = 0; + await assert.rejects(cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + authenticateCalls++; + throw new Error('failed'); + }), /failed/); + await cache.authenticate('https://api.example.com', ['read'], 'tok1', async () => { + authenticateCalls++; + }); + + assert.strictEqual(authenticateCalls, 2); + }); + + test('clear forgets every completed token', async () => { + const cache = new AgentHostAuthTokenCache(); + let authenticateCalls = 0; + const authenticate = async () => { authenticateCalls++; }; + await cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate); + await cache.authenticate('https://other.example.com', ['read'], 'tok2', authenticate); cache.clear(); - assert.strictEqual(cache.updateAndIsChanged('https://api.example.com', ['read'], 'tok1'), true); - assert.strictEqual(cache.updateAndIsChanged('https://other.example.com', ['read'], 'tok2'), true); + await cache.authenticate('https://api.example.com', ['read'], 'tok1', authenticate); + await cache.authenticate('https://other.example.com', ['read'], 'tok2', authenticate); + + assert.strictEqual(authenticateCalls, 4); }); }); @@ -517,14 +680,13 @@ suite('resolveMcpServerAuthentication', () => { suite('authenticateProtectedResources', () => { - const log = new NullLogService(); const protectedResource: ProtectedResourceMetadata = { resource: 'https://api.example.com', authorization_servers: ['https://auth.example.com'], scopes_supported: ['read'], }; - ensureNoDisposablesAreLeakedInTestSuite(); + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); test('skips authenticate when the cached token is unchanged', async () => { const authService = createMockAuthService({ @@ -540,21 +702,18 @@ suite('authenticateProtectedResources', () => { const cache = new AgentHostAuthTokenCache(); const requests: { resource: string; scopes?: readonly string[]; token: string }[] = []; const agents = [{ protectedResources: [protectedResource] }] as unknown as readonly AgentInfo[]; + const instantiationService = createAuthInstantiationService(disposables, authService); - await authenticateProtectedResources(agents, { + await instantiationService.invokeFunction(authenticateProtectedResources, agents, { authTokenCache: cache, - authenticationService: authService, logPrefix: '[AgentHost]', - logService: log, authenticate: async request => { requests.push(request); }, }); - await authenticateProtectedResources(agents, { + await instantiationService.invokeFunction(authenticateProtectedResources, agents, { authTokenCache: cache, - authenticationService: authService, logPrefix: '[AgentHost]', - logService: log, authenticate: async request => { requests.push(request); }, @@ -566,16 +725,15 @@ suite('authenticateProtectedResources', () => { suite('resolveAuthenticationInteractively', () => { - const log = new NullLogService(); const protectedResource: ProtectedResourceMetadata = { resource: 'https://api.example.com', authorization_servers: ['https://auth.example.com'], scopes_supported: ['read'], }; - ensureNoDisposablesAreLeakedInTestSuite(); + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('uses an existing token before prompting for a new session', async () => { + test('uses an existing token before prompting and dedupes repeated checks', async () => { let createSessionCalls = 0; const authService = createMockAuthService({ getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), @@ -592,41 +750,97 @@ suite('resolveAuthenticationInteractively', () => { }, }); const requests: { resource: string; scopes?: readonly string[]; token: string }[] = []; + const cache = new AgentHostAuthTokenCache(); + const instantiationService = createAuthInstantiationService(disposables, authService); - const success = await resolveAuthenticationInteractively([protectedResource], { - authTokenCache: new AgentHostAuthTokenCache(), - authenticationService: authService, + const options: IAgentHostAuthenticationOptions = { + authTokenCache: cache, logPrefix: '[AgentHost]', - logService: log, authenticate: async request => { requests.push(request); }, - }); + }; + const results = [ + await instantiationService.invokeFunction(resolveAuthenticationInteractively, [protectedResource], options), + await instantiationService.invokeFunction(resolveAuthenticationInteractively, [protectedResource], options), + ]; - assert.strictEqual(success, true); - assert.deepStrictEqual(requests, [{ resource: protectedResource.resource, scopes: ['read'], token: 'existing-token' }]); - assert.strictEqual(createSessionCalls, 0); + assert.deepStrictEqual({ results, requests, createSessionCalls }, { + results: [true, true], + requests: [{ resource: protectedResource.resource, scopes: ['read'], token: 'existing-token' }], + createSessionCalls: 0, + }); }); - test('creates a session when no existing token is available', async () => { + test('uses the product sign-in flow and forwards its token', async () => { + let signedIn = false; + const commandService = new TestCommandService(); + commandService.onExecute = () => signedIn = true; const authService = createMockAuthService({ getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), - getSessions: () => Promise.resolve([]), - createSession: async () => ({ accessToken: 'new-token' }), + getSessions: () => Promise.resolve(signedIn ? [{ scopes: ['read'], accessToken: 'signed-in-token' }] : []), }); const requests: { resource: string; scopes?: readonly string[]; token: string }[] = []; + const instantiationService = createAuthInstantiationService(disposables, authService, commandService); - const success = await resolveAuthenticationInteractively([protectedResource], { + const success = await instantiationService.invokeFunction(resolveAuthenticationInteractively, [protectedResource], { authTokenCache: new AgentHostAuthTokenCache(), - authenticationService: authService, logPrefix: '[AgentHost]', - logService: log, authenticate: async request => { requests.push(request); }, }); - assert.strictEqual(success, true); - assert.deepStrictEqual(requests, [{ resource: protectedResource.resource, scopes: ['read'], token: 'new-token' }]); + assert.deepStrictEqual({ success, commandCalls: commandService.calls, requests }, { + success: true, + commandCalls: [{ + commandId: CHAT_SETUP_ACTION_ID, + args: [undefined, { + forceSignInDialog: true, + additionalScopes: ['read'], + dialogTitle: 'Sign in to use GitHub Copilot', + disableChatViewReveal: true, + returnResult: true, + }], + }], + requests: [{ resource: protectedResource.resource, scopes: ['read'], token: 'signed-in-token' }], + }); + }); + + test('does not fall back to direct provider login when product sign-in is canceled', async () => { + const commandService = new TestCommandService(); + commandService.result = { success: undefined, dialogSkipped: false }; + let createSessionCalls = 0; + const authService = createMockAuthService({ + getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), + getSessions: () => Promise.resolve([]), + createSession: async () => { + createSessionCalls++; + return { accessToken: 'unexpected-token' }; + }, + }); + const instantiationService = createAuthInstantiationService(disposables, authService, commandService); + + const success = await instantiationService.invokeFunction(resolveAuthenticationInteractively, [protectedResource], { + logPrefix: '[AgentHost]', + authenticate: async () => { }, + }); + + assert.deepStrictEqual({ success, createSessionCalls }, { success: false, createSessionCalls: 0 }); + }); + + test('propagates product sign-in failures', async () => { + const commandService = new TestCommandService(); + commandService.result = { success: false, dialogSkipped: false, error: new Error('Bad credentials') }; + const authService = createMockAuthService({ + getOrActivateProviderIdForServer: () => Promise.resolve('provider-1'), + getSessions: () => Promise.resolve([]), + }); + const instantiationService = createAuthInstantiationService(disposables, authService, commandService); + + await assert.rejects(instantiationService.invokeFunction(resolveAuthenticationInteractively, [protectedResource], { + logPrefix: '[AgentHost]', + authenticate: async () => { }, + }), /Bad credentials/); }); }); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index 6aab080b1f42e..a3fd0aad23bc7 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -27,11 +27,12 @@ import { AgentFeedbackAttachmentDisplayKind, AgentFeedbackAttachmentMetadataKey import { BrowserViewAttachmentDisplayKind, BrowserViewAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/browserViewAttachments.js'; import { ActionType, isSessionAction, isChatAction, type ActionEnvelope, type IRootConfigChangedAction, type SessionAction, type ChatAction as AgentHostChatAction, type TerminalAction, type INotification, type IToolCallConfirmedAction, type ITurnStartedAction, type ClientAnnotationsAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import type { IStateSnapshot } from '../../../../../../platform/agentHost/common/state/sessionProtocol.js'; -import { CustomizationType, McpAuthRequiredReason, McpServerStatus, type ClientPluginCustomization, type ToolDefinition } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; -import { ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, SessionLifecycle, SessionStatus, TurnState, ToolCallStatus, ToolCallConfirmationReason, ToolCallContributorKind, createSessionState, createChatState, createDefaultChatSummary, buildChatUri, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, createActiveTurn, isAhpRootChannel, PolicyState, ResponsePartKind, ROOT_STATE_URI, StateComponents, buildSubagentChatUri, ToolResultContentType, MessageAttachmentKind, MessageKind, type SessionState, type SessionSummary, type ChatState, type ISessionWithDefaultChat, RootState, type ToolCallState, type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; +import { ConfirmationOptionKind, CustomizationType, McpAuthRequiredReason, McpServerStatus, type ClientPluginCustomization, type ProtectedResourceMetadata, type ToolDefinition } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { ChatInputAnswerState, ChatInputAnswerValueKind, ChatInputQuestionKind, ChatInputResponseKind, SessionLifecycle, SessionStatus, TurnState, ToolCallStatus, ToolCallConfirmationReason, ToolCallContributorKind, ToolCallRiskAssessmentKind, ToolCallRiskAssessmentStatus, createSessionState, createChatState, createDefaultChatSummary, buildChatUri, buildDefaultChatUri, parseDefaultChatUri, isAhpChatChannel, createActiveTurn, isAhpRootChannel, PolicyState, ResponsePartKind, ROOT_STATE_URI, StateComponents, buildSubagentChatUri, ToolResultContentType, MessageAttachmentKind, MessageKind, type SessionState, type SessionSummary, type ChatState, type ISessionWithDefaultChat, RootState, type ToolCallState, type AgentInfo } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { CompletionItemKind as AhpCompletionItemKind, type CompletionsParams, type CompletionsResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { sessionReducer, chatReducer } from '../../../../../../platform/agentHost/common/state/sessionReducers.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; +import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; import { IProgress, IProgressNotificationOptions, IProgressService, IProgressStep } from '../../../../../../platform/progress/common/progress.js'; import { IAuthenticationService } from '../../../../../services/authentication/common/authentication.js'; import { ChatEntitlement, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; @@ -77,6 +78,7 @@ import { AgentHostNewSessionFolderService, IAgentHostNewSessionFolderService } f import { OpenAgentHostFolderPickerAction } from '../../../browser/agentSessions/agentHost/agentHostChatInputPicker.contribution.js'; import { MenuId, MenuRegistry, isIMenuItem, type IMenuItem } from '../../../../../../platform/actions/common/actions.js'; import { ChatContextKeys } from '../../../common/actions/chatContextKeys.js'; +import { CHAT_SETUP_ACTION_ID } from '../../../browser/actions/chatActions.js'; import { type ContextKeyValue } from '../../../../../../platform/contextkey/common/contextkey.js'; import { IAgentHostActiveClientService } from '../../../browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { SyncedCustomizationBundler } from '../../../browser/agentSessions/agentHost/syncedCustomizationBundler.js'; @@ -551,6 +553,16 @@ class MockChatAgentService extends mock() { } } +class MockCommandService extends mock() { + readonly calls: { commandId: string; args: unknown[] }[] = []; + result: unknown; + + override async executeCommand(commandId: string, ...args: unknown[]): Promise { + this.calls.push({ commandId, args }); + return this.result as R | undefined; + } +} + class MockChatWidgetService extends mock() { declare readonly _serviceBrand: undefined; @@ -687,6 +699,8 @@ function createTestServices(disposables: DisposableStore, workingDirectoryResolv ...chatDebugServiceOverride, }); instantiationService.stub(IDefaultAccountService, { onDidChangeDefaultAccount: Event.None, getDefaultAccount: async () => null }); + const commandService = new MockCommandService(); + instantiationService.stub(ICommandService, commandService); instantiationService.stub(IAuthenticationService, { onDidChangeSessions: Event.None, ...authServiceOverride }); instantiationService.stub(ILanguageModelsService, { deltaLanguageModelChatProviderDescriptors: () => { }, @@ -852,7 +866,7 @@ function createTestServices(disposables: DisposableStore, workingDirectoryResolv instantiationService.stub(IAgentHostActiveClientService, activeClientService); instantiationService.stub(IOpenerService, openerService as IOpenerService); - return { instantiationService, agentHostService, chatAgentService, chatWidgetService, chatService, openerService, activeClientService, seedActiveClient, chatSessionContributions, chatSessionItemControllers, newSessionFolderService, trustController, modelService, workingCopyService }; + return { instantiationService, agentHostService, chatAgentService, chatWidgetService, chatService, openerService, activeClientService, seedActiveClient, chatSessionContributions, chatSessionItemControllers, newSessionFolderService, trustController, modelService, workingCopyService, commandService }; } function createSessionListStore(disposables: DisposableStore, instantiationService: TestInstantiationService, connection: IAgentHostSessionListConnection): AgentHostSessionListStore { @@ -4344,13 +4358,15 @@ suite('AgentHostChatContribution', () => { await timeout(10); - // The tool call should have produced a ChatToolInvocation in WaitingForConfirmation state - // After toolCallStart (Streaming) and toolCallReady without confirmed (PendingConfirmation), - // the handler emits two progress events — we want the last one (with confirmation). + // After toolCallStart (Streaming) and toolCallReady without + // `confirmed` (PendingConfirmation), the streaming invocation is + // driven into WaitingForConfirmation in place — exactly ONE card, + // not a settled placeholder plus a replacement (#314858). const toolInvocations = collected.flat().filter(p => p.kind === 'toolInvocation'); - assert.ok(toolInvocations.length >= 1, 'Should have received tool confirmation progress'); - const permInvocation = toolInvocations[toolInvocations.length - 1] as IChatToolInvocation; + assert.strictEqual(toolInvocations.length, 1, 'Should have exactly one tool invocation card'); + const permInvocation = toolInvocations[0] as IChatToolInvocation; assert.strictEqual(permInvocation.kind, 'toolInvocation'); + assert.strictEqual(permInvocation.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); // Confirm the tool IChatToolInvocation.confirmWith(permInvocation, { type: ToolConfirmKind.UserAction }); @@ -4419,7 +4435,8 @@ suite('AgentHostChatContribution', () => { await timeout(10); const toolInvocations = collected.flat().filter(p => p.kind === 'toolInvocation'); - const permInvocation = toolInvocations[toolInvocations.length - 1] as IChatToolInvocation; + assert.strictEqual(toolInvocations.length, 1, 'Should have exactly one tool invocation card'); + const permInvocation = toolInvocations[0] as IChatToolInvocation; assert.strictEqual(permInvocation.toolSpecificData?.kind, 'input'); const inputData = permInvocation.toolSpecificData as IChatToolInputInvocationData; assert.deepStrictEqual(inputData.rawInput, { input: 'echo hello' }); @@ -4430,6 +4447,67 @@ suite('AgentHostChatContribution', () => { await turnPromise; })); + test('terminal command needing confirmation renders exactly one card (#314858)', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); + + const { turnPromise, collected, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); + + // Reproduce the REAL Copilot SDK sequence: `onToolStart` optimistically + // emits a `confirmed: not-needed` ready (moving the tool to Running) + // and only ~1s later does the permission callback fire a second ready + // with a confirmationTitle (Running → PendingConfirmation). This is + // the path that used to settle the running card and emit a second + // confirmation card (#314858). + fire({ type: 'chat/toolCallStart', session, turnId, toolCallId: 'tc-term-confirm', toolName: 'bash', displayName: 'Bash', _meta: { toolKind: 'terminal', language: 'shellscript' } } as ChatAction); + await timeout(1); + fire({ + type: 'chat/toolCallReady', session, turnId, toolCallId: 'tc-term-confirm', + invocationMessage: 'Running `rm -rf build`', toolInput: 'rm -rf build', confirmed: 'not-needed', + } as ChatAction); + await timeout(1); + + // One running terminal card so far. + const afterRunning = collected.flat().filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; + assert.strictEqual(afterRunning.length, 1, 'one running terminal card before confirmation'); + assert.strictEqual(afterRunning[0].state.get().type, IChatToolInvocation.StateKind.Executing); + + // Permission callback bounces the same tool back to confirmation. + fire({ + type: 'chat/toolCallReady', session, turnId, toolCallId: 'tc-term-confirm', + invocationMessage: 'Running `rm -rf build`', toolInput: 'rm -rf build', confirmationTitle: 'Run command?', + } as ChatAction); + await timeout(10); + + // Still exactly one card — now a terminal confirmation, not two. + const toolInvocations = collected.flat().filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; + assert.strictEqual(toolInvocations.length, 1, 'exactly one terminal confirmation card'); + const inv = toolInvocations[0]; + assert.strictEqual(inv.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); + assert.strictEqual(inv.toolSpecificData?.kind, 'terminal'); + + // Confirm → exactly one dispatch, then echo Running and complete. + IChatToolInvocation.confirmWith(inv, { type: ToolConfirmKind.UserAction }); + await timeout(10); + const confirmed = agentHostService.dispatchedActions.filter(a => + a.action.type === 'chat/toolCallConfirmed' && (a.action as IToolCallConfirmedAction).toolCallId === 'tc-term-confirm'); + assert.strictEqual(confirmed.length, 1, 'exactly one toolCallConfirmed dispatch'); + + agentHostService.fireAction({ + channel: confirmed[0].channel.toString(), action: confirmed[0].action, + serverSeq: 100, origin: { clientId: agentHostService.clientId, clientSeq: confirmed[0].clientSeq }, + }); + fire({ + type: 'chat/toolCallComplete', session, turnId, toolCallId: 'tc-term-confirm', + result: { success: true, pastTenseMessage: 'Ran command', content: [{ type: 'text', text: 'done\n' }] }, + } as ChatAction); + fire({ type: 'chat/turnComplete', endedAt: '2025-01-01T00:00:00.000Z', session, turnId } as ChatAction); + await turnPromise; + + // Still exactly one card across the whole turn. + const finalInvocations = collected.flat().filter(p => p.kind === 'toolInvocation'); + assert.strictEqual(finalInvocations.length, 1, 'still exactly one card across the whole turn'); + })); + test('read permission shows input-style confirmation data', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); @@ -4450,6 +4528,65 @@ suite('AgentHostChatContribution', () => { await turnPromise; })); + test('pending confirmation refreshes risk assessment and options', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); + const { turnPromise, collected, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); + + fire({ type: 'chat/toolCallStart', session, turnId, toolCallId: 'tc-risk', toolName: 'read_file', displayName: 'Read File' } as ChatAction); + fire({ + type: 'chat/toolCallReady', + session, + turnId, + toolCallId: 'tc-risk', + invocationMessage: 'Read file?', + confirmationTitle: 'Read file', + toolInput: '/foo.ts', + riskAssessment: { kind: ToolCallRiskAssessmentKind.Judge, status: ToolCallRiskAssessmentStatus.Loading }, + options: [{ id: 'allow-once', label: 'Allow Once', kind: ConfirmationOptionKind.Approve }], + } as ChatAction); + await timeout(10); + + const invocation = collected.flat().find(p => p.kind === 'toolInvocation') as ChatToolInvocation; + assert.deepStrictEqual(invocation.confirmationMessages, { + title: 'Read file', + message: 'Read file?', + approvalReason: { status: 'loading' }, + customOptions: [{ id: 'allow-once', label: 'Allow Once', kind: ConfirmationOptionKind.Approve }], + }); + + fire({ + type: 'chat/toolCallReady', + session, + turnId, + toolCallId: 'tc-risk', + invocationMessage: 'Read file?', + riskAssessment: { + kind: ToolCallRiskAssessmentKind.Judge, + status: ToolCallRiskAssessmentStatus.Complete, + reason: 'This reads a sensitive file.', + safety: 0.2, + }, + options: [{ id: 'allow-session', label: 'Allow in Session', kind: ConfirmationOptionKind.Approve }], + } as ChatAction); + await timeout(10); + + assert.deepStrictEqual(invocation.confirmationMessages, { + title: 'Read file', + message: 'Read file?', + approvalReason: { status: 'complete', explanation: 'This reads a sensitive file.', safety: 0.2 }, + customOptions: [{ id: 'allow-session', label: 'Allow in Session', kind: ConfirmationOptionKind.Approve }], + }); + + IChatToolInvocation.confirmWith(invocation, { type: ToolConfirmKind.UserAction, selectedButton: 'allow-session' }); + await timeout(10); + const confirmation = agentHostService.dispatchedActions.find(a => + a.action.type === 'chat/toolCallConfirmed' && (a.action as IToolCallConfirmedAction).toolCallId === 'tc-risk'); + assert.strictEqual((confirmation?.action as IToolCallConfirmedAction).selectedOptionId, 'allow-session'); + + fire({ type: 'chat/turnComplete', endedAt: '2025-01-01T00:00:00.000Z', session, turnId } as ChatAction); + await turnPromise; + })); + test('local confirmation does not race with pending tc.status: no spurious re-confirm before server echo', () => runWithFakedTimers({ useFakeTimers: true }, async () => { // Regression for a bug where the per-tool-call autorun read both // `part$` AND `invocation.state` and used a state-comparison check @@ -4462,13 +4599,15 @@ suite('AgentHostChatContribution', () => { // from a `tc.status` *transition*, not from invocation-state // comparison. // - // Baseline (bug-free) flow for a tool needing initial confirmation: - // toolCallStart → emit placeholder invocation (count=1) - // toolCallReady → status: Streaming → PendingConfirmation, - // settle placeholder, emit confirm invocation (count=2) + // Baseline (bug-free) flow for a tool needing initial confirmation + // (Option A single-card streaming, #314858): + // toolCallStart → emit streaming invocation (count=1) + // toolCallReady → Streaming → PendingConfirmation: drive the + // SAME invocation into WaitingForConfirmation + // via transitionFromStreaming (count stays 1) // user confirms → invocation.state: WaitingForConfirmation → Executing - // (count must NOT change — this is the regression) - // server echoes → tc.status: PendingConfirmation → Running (count=2) + // (count must NOT change) + // server echoes → tc.status: PendingConfirmation → Running (count=1) const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); const { turnPromise, collected, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); @@ -4481,6 +4620,7 @@ suite('AgentHostChatContribution', () => { await timeout(10); const beforeConfirm = collected.flat().filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; + assert.strictEqual(beforeConfirm.length, 1, 'a single confirmation card is emitted for the streaming tool'); const permInvocation = beforeConfirm[beforeConfirm.length - 1]; assert.strictEqual(permInvocation.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); @@ -4528,12 +4668,13 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(finalInvocations.length, beforeConfirm.length, 'no extra invocations across the full turn'); })); - test('genuine re-confirmation (Running → PendingConfirmation) emits a fresh confirmation invocation', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - // Companion to the regression test above: the *legitimate* case - // where the server bounces a tool call back to PendingConfirmation - // (e.g. result confirmation after an edit). Here we DO want a - // fresh invocation and a second `session/toolCallConfirmed` - // dispatch. + test('re-confirmation (Running → PendingConfirmation) reuses the same card and dispatches again', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // When a tool that already ran is bounced back to PendingConfirmation + // (Running → PendingConfirmation — the shape every Copilot permission + // prompt takes, since `onToolStart` optimistically readies the tool + // before the permission callback fires), the SAME card is reused: no + // second invocation, but a second `session/toolCallConfirmed` is + // still dispatched for the new confirmation. const { sessionHandler, agentHostService, chatAgentService } = createContribution(disposables); const { turnPromise, collected, session, turnId, fire } = await startTurn(sessionHandler, agentHostService, chatAgentService, disposables); @@ -4573,16 +4714,16 @@ suite('AgentHostChatContribution', () => { // transitions Running → PendingConfirmation. fire({ type: 'chat/toolCallReady', session, turnId, toolCallId: 'tc-recon', - invocationMessage: 'Confirm execution', toolInput: 'echo hi', + invocationMessage: 'Confirm execution', toolInput: 'echo hi', confirmationTitle: 'Confirm?', } as ChatAction); await timeout(10); - // We now expect a *fresh* invocation in WaitingForConfirmation. + // The SAME invocation is re-armed for confirmation — no new card. const invocationsAfterReconfirm = collected.flat().filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; - assert.strictEqual(invocationsAfterReconfirm.length, invocationCountAfterRunning + 1, 'a fresh invocation should be emitted on Running → PendingConfirmation transition'); + assert.strictEqual(invocationsAfterReconfirm.length, invocationCountAfterRunning, 'no fresh invocation is emitted on Running → PendingConfirmation; the card is reused'); const reconfirmInvocation = invocationsAfterReconfirm[invocationsAfterReconfirm.length - 1]; + assert.strictEqual(reconfirmInvocation, firstInvocation, 'the same invocation instance is reused'); assert.strictEqual(reconfirmInvocation.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); - assert.notStrictEqual(reconfirmInvocation, firstInvocation); // User confirms the re-confirmation; expect a second toolCallConfirmed dispatch. IChatToolInvocation.confirmWith(reconfirmInvocation, { type: ToolConfirmKind.UserAction }); @@ -7002,6 +7143,70 @@ suite('AgentHostChatContribution', () => { assert.deepStrictEqual((configChanged!.action as { config: Record }).config, config); })); + test('handler resolves authentication before sending to an eager-created session', async () => { + const authenticationRequests: ProtectedResourceMetadata[][] = []; + const { instantiationService, agentHostService, chatAgentService } = createTestServices(disposables); + const sessionHandler = disposables.add(instantiationService.createInstance(AgentHostSessionHandler, { + provider: 'copilot', + agentId: 'eager-auth-agent', + sessionType: 'agent-host-copilot', + fullName: 'Agent Host - Copilot', + description: 'test', + connection: agentHostService, + connectionAuthority: 'local', + resolveAuthentication: async protectedResources => { + authenticationRequests.push(protectedResources); + return true; + }, + })); + const protectedResource: ProtectedResourceMetadata = { + resource: 'https://api.github.com', + resource_name: 'GitHub', + authorization_servers: ['https://github.com/login/oauth'], + scopes_supported: ['read:user'], + required: true, + }; + agentHostService.setRootState({ + agents: [{ + provider: 'copilot', + displayName: 'Agent Host - Copilot', + description: 'test', + models: [], + protectedResources: [protectedResource], + }], + activeSessions: 0, + }); + + const sessionUri = AgentSession.uri('copilot', 'eager-auth'); + agentHostService.sessionStates.set(sessionUri.toString(), { + ...createSessionState({ resource: sessionUri.toString(), provider: 'copilot', title: 'Test', status: SessionStatus.Idle, createdAt: new Date().toISOString(), modifiedAt: new Date().toISOString() }), + lifecycle: SessionLifecycle.Ready, + turns: [], + }); + + const sessionResource = URI.from({ scheme: 'agent-host-copilot', path: '/eager-auth' }); + const chatSession = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None); + disposables.add(toDisposable(() => chatSession.dispose())); + + const registered = chatAgentService.registeredAgents.get('eager-auth-agent')!; + const turnPromise = registered.impl.invoke(makeRequest({ agentId: 'eager-auth-agent', message: 'Send after sign out', sessionResource }), () => { }, [], CancellationToken.None); + await timeout(10); + const turnDispatch = agentHostService.turnActions[0]; + assert.ok(turnDispatch); + const turnAction = turnDispatch.action as ITurnStartedAction; + agentHostService.fireAction({ channel: turnDispatch.channel.toString(), action: turnDispatch.action, serverSeq: 1, origin: { clientId: agentHostService.clientId, clientSeq: turnDispatch.clientSeq } }); + agentHostService.fireAction({ channel: turnDispatch.channel.toString(), action: { type: 'chat/turnComplete', turnId: turnAction.turnId } as ChatAction, serverSeq: 2, origin: undefined }); + await turnPromise; + + assert.deepStrictEqual({ + authenticationRequests, + turnActionCount: agentHostService.turnActions.length, + }, { + authenticationRequests: [[protectedResource]], + turnActionCount: 1, + }); + }); + test('handler does not clobber picker-set session config on eager-create path', () => runWithFakedTimers({ useFakeTimers: true }, async () => { // Repro for the VS Code chat-input picker bug: the user picks // "Worktree" via the chip, the picker dispatches @@ -7493,6 +7698,62 @@ suite('AgentHostChatContribution', () => { await timeout(10); })); + test('reconnect during streaming then confirmation renders exactly one card (#314858)', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // Reload/reconnect while a terminal tool is still streaming its + // arguments, then the tool requests confirmation. The reconnect + // snapshot must adopt a native streaming invocation so the live + // observer drives it in place through `transitionFromStreaming` — + // not an Executing placeholder that would trigger the duplicate + // confirmation card on `Streaming → PendingConfirmation`. + const { sessionHandler, agentHostService } = createContribution(disposables); + + const sessionUri = AgentSession.uri('copilot', 'reconnect-stream-confirm'); + const sessionState = makeSessionStateWithActiveTurn(sessionUri.toString()); + sessionState.activeTurn!.responseParts.push({ + kind: ResponsePartKind.ToolCall, + toolCall: { + toolCallId: 'tc-stream-confirm', + toolName: 'bash', + displayName: 'Bash', + invocationMessage: 'Running command', + status: ToolCallStatus.Streaming, + _meta: { toolKind: 'terminal' }, + }, + }); + agentHostService.sessionStates.set(sessionUri.toString(), sessionState); + + const sessionResource = URI.from({ scheme: 'agent-host-copilot', path: '/reconnect-stream-confirm' }); + const session = await sessionHandler.provideChatSessionContent(sessionResource, CancellationToken.None); + disposables.add(toDisposable(() => session.dispose())); + + // Snapshot has exactly one streaming tool card. + const initialToolInvocations = (session.progressObs?.get() ?? []).filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; + assert.strictEqual(initialToolInvocations.length, 1, 'exactly one streaming tool card on reconnect'); + assert.strictEqual(initialToolInvocations[0].state.get().type, IChatToolInvocation.StateKind.Streaming); + + // Server now moves the tool into confirmation. + agentHostService.fireAction({ + channel: sessionUri.toString(), + action: { type: 'chat/toolCallReady', turnId: 'turn-active', toolCallId: 'tc-stream-confirm', invocationMessage: 'Running `rm -rf build`', toolInput: 'rm -rf build', confirmationTitle: 'Run command?' } as ChatAction, + serverSeq: 1, + origin: undefined, + }); + await timeout(10); + + // Still exactly one card, now waiting for confirmation — no duplicate. + const afterConfirm = (session.progressObs?.get() ?? []).filter(p => p.kind === 'toolInvocation') as IChatToolInvocation[]; + assert.strictEqual(afterConfirm.length, 1, 'still exactly one card after Streaming → PendingConfirmation on reconnect'); + assert.strictEqual(afterConfirm[0].state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); + + // Clean up the awaitConfirmation promise before teardown. + agentHostService.fireAction({ + channel: sessionUri.toString(), action: { type: 'chat/turnComplete', turnId: 'turn-active' } as ChatAction, + serverSeq: 2, + origin: undefined, + }); + await timeout(10); + })); + test('no active turn loads completed history only with isComplete true', async () => { const { sessionHandler, agentHostService } = createContribution(disposables); @@ -8609,6 +8870,51 @@ suite('AgentHostChatContribution', () => { assert.deepStrictEqual(agentHostService.authenticateCalls, []); }); + + test('propagates interactive authentication errors for eager-created sessions', async () => { + const authService: Partial = { + onDidChangeSessions: Event.None, + getOrActivateProviderIdForServer: async () => 'github', + getSessions: (async () => []) as IAuthenticationService['getSessions'], + }; + const { instantiationService, agentHostService, chatAgentService, commandService } = createTestServices(disposables, undefined, authService); + commandService.result = { success: false, dialogSkipped: false, error: new Error('Bad credentials') }; + disposables.add(instantiationService.createInstance(AgentHostContribution)); + agentHostService.setRootState({ agents: protectedAgents(), activeSessions: 0 }); + await timeout(0); + + const sessionUri = AgentSession.uri('copilot', 'eager-auth-error'); + agentHostService.sessionStates.set(sessionUri.toString(), { + ...createSessionState({ resource: sessionUri.toString(), provider: 'copilot', title: 'Test', status: SessionStatus.Idle, createdAt: new Date().toISOString(), modifiedAt: new Date().toISOString() }), + lifecycle: SessionLifecycle.Ready, + turns: [], + }); + disposables.add(agentHostService.getSubscription(StateComponents.Session, sessionUri)); + + const sessionResource = URI.from({ scheme: 'agent-host-copilot', path: '/eager-auth-error' }); + const registered = chatAgentService.registeredAgents.get('agent-host-copilot')!; + await assert.rejects( + registered.impl.invoke(makeRequest({ message: 'Send after sign out', sessionResource }), () => { }, [], CancellationToken.None), + /Bad credentials/, + ); + + assert.deepStrictEqual({ + commandCalls: commandService.calls, + turnActions: agentHostService.turnActions, + }, { + commandCalls: [{ + commandId: CHAT_SETUP_ACTION_ID, + args: [undefined, { + forceSignInDialog: true, + additionalScopes: ['read:user'], + dialogTitle: 'Sign in to use GitHub Copilot', + disableChatViewReveal: true, + returnResult: true, + }], + }], + turnActions: [], + }); + }); }); // ---- MCP auth prompt dedupe (per conversation) ---------------------- diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts index e6f4fbabb38ba..bb151635ca22b 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts @@ -14,7 +14,7 @@ import { fromAgentHostUri, toAgentHostUri } from '../../../../../../platform/age import { buildSubagentChatUri, MessageKind, ToolCallContributorKind, ToolCallRiskAssessmentKind, ToolCallRiskAssessmentStatus, ToolCallStatus, ToolCallConfirmationReason, ToolResultContentType, TurnState, ResponsePartKind, readUsageInfoMeta, type ActiveTurn, type ICompletedToolCall, type ToolCallPendingConfirmationState, type ToolCallRunningState, type Turn, type ToolCallResponsePart, ToolCallCancellationReason, type Message } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { IChatToolInvocation, IChatToolInvocationSerialized, ToolConfirmKind, type IChatMarkdownContent, type IChatTerminalToolInvocationData, type IChatThinkingPart, type IChatUsage } from '../../../common/chatService/chatService.js'; import { isToolResultInputOutputDetails, type IToolResultInputOutputDetails, ToolDataSource, ToolInvocationPresentation } from '../../../common/tools/languageModelToolsService.js'; -import { turnsToHistory as rawTurnsToHistory, activeTurnToProgress as rawActiveTurnToProgress, toolCallStateToInvocation as rawToolCallStateToInvocation, finalizeToolInvocation as rawFinalizeToolInvocation, updateRunningToolSpecificData as rawUpdateRunningToolSpecificData, usageInfoToAutoModeResolution, usageInfoToQuotas, formatTurnResponseDetails, rewriteAgentHostLinkTarget, rewriteMarkdownLinks, type TurnModelLookup } from '../../../browser/agentSessions/agentHost/stateToProgressAdapter.js'; +import { turnsToHistory as rawTurnsToHistory, activeTurnToProgress as rawActiveTurnToProgress, toolCallStateToInvocation as rawToolCallStateToInvocation, toolCallStateToPreparedInvocation as rawToolCallStateToPreparedInvocation, toolCallStateToStreamingInvocation, finalizeToolInvocation as rawFinalizeToolInvocation, updateRunningToolSpecificData as rawUpdateRunningToolSpecificData, usageInfoToAutoModeResolution, usageInfoToQuotas, formatTurnResponseDetails, rewriteAgentHostLinkTarget, rewriteMarkdownLinks, type TurnModelLookup } from '../../../browser/agentSessions/agentHost/stateToProgressAdapter.js'; // ---- Helper factories ------------------------------------------------------- @@ -70,6 +70,10 @@ function toolCallStateToInvocation(tc: Parameters[0]) { + return rawToolCallStateToPreparedInvocation(tc, URI.file('/'), 'local'); +} + function finalizeToolInvocation(invocation: Parameters[0], tc: Parameters[1]) { return rawFinalizeToolInvocation(invocation, tc, URI.file('/'), 'local'); } @@ -1127,6 +1131,84 @@ suite('stateToProgressAdapter', () => { }); }); + suite('streaming tool invocations (#314858)', () => { + + type AnyToolCallState = Parameters[0]; + + test('toolCallStateToStreamingInvocation starts in the native Streaming state', () => { + const tc: AnyToolCallState = { toolCallId: 'tc-stream', toolName: 'bash', displayName: 'Bash', status: ToolCallStatus.Streaming }; + const invocation = toolCallStateToStreamingInvocation(tc, undefined); + assert.strictEqual(invocation.toolCallId, 'tc-stream'); + assert.strictEqual(invocation.toolId, 'bash'); + assert.strictEqual(invocation.state.get().type, IChatToolInvocation.StateKind.Streaming); + assert.strictEqual(IChatToolInvocation.isComplete(invocation), false); + }); + + test('transitionFromStreaming with a pending terminal prepared invocation yields a single terminal confirmation card', () => { + // A terminal command streamed its args, then requested confirmation. + const streaming = toolCallStateToStreamingInvocation({ toolCallId: 'tc-term', toolName: 'bash', displayName: 'Bash', status: ToolCallStatus.Streaming }, undefined); + const pending: AnyToolCallState = { + toolCallId: 'tc-term', + toolName: 'bash', + displayName: 'Bash', + invocationMessage: 'Running `rm -rf build`', + toolInput: 'rm -rf build', + status: ToolCallStatus.PendingConfirmation, + _meta: { toolKind: 'terminal' }, + confirmationTitle: 'Run command?', + }; + + const prepared = toolCallStateToPreparedInvocation(pending); + assert.strictEqual(prepared.confirmationMessages?.title, 'Run command?'); + assert.strictEqual(prepared.toolSpecificData?.kind, 'terminal'); + + streaming.transitionFromStreaming(prepared, undefined, undefined); + assert.strictEqual(streaming.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); + assert.strictEqual(streaming.toolSpecificData?.kind, 'terminal'); + }); + + test('transitionFromStreaming with a non-confirmation prepared invocation goes straight to Executing', () => { + const streaming = toolCallStateToStreamingInvocation({ toolCallId: 'tc-run', toolName: 'read_file', displayName: 'Read File', status: ToolCallStatus.Streaming }, undefined); + const running: AnyToolCallState = { toolCallId: 'tc-run', toolName: 'read_file', displayName: 'Read File', invocationMessage: 'Reading file', status: ToolCallStatus.Running, confirmed: ToolCallConfirmationReason.NotNeeded }; + + const prepared = toolCallStateToPreparedInvocation(running); + assert.strictEqual(prepared.confirmationMessages, undefined); + + streaming.transitionFromStreaming(prepared, undefined, undefined); + assert.strictEqual(streaming.state.get().type, IChatToolInvocation.StateKind.Executing); + }); + + test('requestConfirmation re-arms confirmation from Executing (Copilot Running → PendingConfirmation)', () => { + // Real Copilot flow: onToolStart readies the tool (Running/Executing) + // before the permission callback bounces it to PendingConfirmation. + // requestConfirmation must move the SAME invocation back to + // WaitingForConfirmation so a single card spans the lifecycle. + const streaming = toolCallStateToStreamingInvocation({ toolCallId: 'tc-term', toolName: 'bash', displayName: 'Bash', status: ToolCallStatus.Streaming }, undefined); + + // Streaming → Running (confirmed: not-needed) → Executing. + const running: AnyToolCallState = { toolCallId: 'tc-term', toolName: 'bash', displayName: 'Bash', invocationMessage: 'Running command', status: ToolCallStatus.Running, confirmed: ToolCallConfirmationReason.NotNeeded, _meta: { toolKind: 'terminal' } }; + streaming.transitionFromStreaming(toolCallStateToPreparedInvocation(running), undefined, undefined); + assert.strictEqual(streaming.state.get().type, IChatToolInvocation.StateKind.Executing); + + // Running → PendingConfirmation via the permission callback. + const pending: AnyToolCallState = { toolCallId: 'tc-term', toolName: 'bash', displayName: 'Bash', invocationMessage: 'Running `rm -rf build`', toolInput: 'rm -rf build', status: ToolCallStatus.PendingConfirmation, _meta: { toolKind: 'terminal' }, confirmationTitle: 'Run command?' }; + streaming.requestConfirmation(toolCallStateToPreparedInvocation(pending)); + assert.strictEqual(streaming.state.get().type, IChatToolInvocation.StateKind.WaitingForConfirmation); + assert.strictEqual(streaming.toolSpecificData?.kind, 'terminal'); + }); + + test('requestConfirmation no-ops on a completed invocation', () => { + const streaming = toolCallStateToStreamingInvocation({ toolCallId: 'tc-done', toolName: 'bash', displayName: 'Bash', status: ToolCallStatus.Streaming }, undefined); + streaming.transitionFromStreaming(toolCallStateToPreparedInvocation({ toolCallId: 'tc-done', toolName: 'bash', displayName: 'Bash', invocationMessage: 'run', status: ToolCallStatus.Running, confirmed: ToolCallConfirmationReason.NotNeeded }), undefined, undefined); + streaming.didExecuteTool(undefined); + assert.strictEqual(IChatToolInvocation.isComplete(streaming), true); + + const pending: AnyToolCallState = { toolCallId: 'tc-done', toolName: 'bash', displayName: 'Bash', invocationMessage: 'confirm', status: ToolCallStatus.PendingConfirmation, confirmationTitle: 'Confirm?' }; + streaming.requestConfirmation(toolCallStateToPreparedInvocation(pending)); + assert.strictEqual(IChatToolInvocation.isComplete(streaming), true, 'completed invocation is not re-armed'); + }); + }); + suite('finalizeToolInvocation', () => { test('rewrites markdown links in pastTenseMessage through the agent host scheme', () => { @@ -1621,6 +1703,31 @@ suite('stateToProgressAdapter', () => { }); }); + test('hydrates another client streaming tool with its cancel affordance', () => { + const toolCall: ToolCallResponsePart['toolCall'] = { + toolCallId: 'tc-other-client-streaming', + toolName: 'run_task', + displayName: 'Run Task', + status: ToolCallStatus.Streaming, + contributor: { kind: ToolCallContributorKind.Client, clientId: 'owner-client' }, + }; + const result = activeTurnToProgress(URI.file('/'), createActiveTurnState([ + { kind: ResponsePartKind.ToolCall, toolCall }, + ]), undefined, { + currentClientId: 'viewer-client', + cancelOtherClientToolCall: () => { }, + }); + const invocation = result[0] as IChatToolInvocation; + + assert.deepStrictEqual({ + state: invocation.state.get().type, + hasOtherClientData: !!invocation.otherClientToolCall, + }, { + state: IChatToolInvocation.StateKind.Executing, + hasOtherClientData: true, + }); + }); + test('creates confirmation invocations for pending tool confirmations', () => { const result = activeTurnToProgress(URI.file('/'), createActiveTurnState([ { diff --git a/src/vs/workbench/contrib/chat/test/common/constants.test.ts b/src/vs/workbench/contrib/chat/test/common/constants.test.ts index 17b0610b9b5fc..81cb94f99da0e 100644 --- a/src/vs/workbench/contrib/chat/test/common/constants.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/constants.test.ts @@ -394,18 +394,25 @@ suite('ChatConfiguration defaults', () => { }); }); - test('agent host default wins over a virtual workspace local override', () => { + test('virtual workspace defaults to local when the agent host default is enabled', () => { const configurationService = new TestConfigurationService({ [ChatConfiguration.DefaultToCopilotHarness]: true, [ChatConfiguration.EditorLocalAgentEnabled]: false, }); const chatSessionsService = createChatSessionsService(SessionType.AgentHostCopilot); + const storageService = disposables.add(new TestStorageService()); const workspace = createWorkspace(URI.parse('vscode-vfs://github/microsoft/vscode')); assert.deepStrictEqual({ computed: getComputedDefaultSessionType(configurationService, chatSessionsService, workspace, true), + rememberedAware: getDefaultNewChatSessionType(configurationService, chatSessionsService, storageService, workspace, true), + localVisible: isVisibleEditorChatSessionType(localChatSessionType, configurationService, chatSessionsService, workspace), + localRememberedUsable: isRememberedSessionTypeUsable(localChatSessionType, configurationService, chatSessionsService, workspace), }, { - computed: SessionType.AgentHostCopilot, + computed: localChatSessionType, + rememberedAware: localChatSessionType, + localVisible: true, + localRememberedUsable: true, }); }); diff --git a/src/vs/workbench/contrib/search/browser/search.contribution.ts b/src/vs/workbench/contrib/search/browser/search.contribution.ts index 824c95b5cc46b..eda06cfb7d49c 100644 --- a/src/vs/workbench/contrib/search/browser/search.contribution.ts +++ b/src/vs/workbench/contrib/search/browser/search.contribution.ts @@ -139,12 +139,6 @@ configurationRegistry.registerConfiguration({ nls.localize('search.mode.newEditor', "Search in a new search editor."), ] }, - 'search.useRipgrep': { - type: 'boolean', - description: nls.localize('useRipgrep', "This setting is deprecated and now falls back on \"search.usePCRE2\"."), - deprecationMessage: nls.localize('useRipgrepDeprecated', "Deprecated. Consider \"search.usePCRE2\" for advanced regex feature support."), - default: true - }, 'search.useIgnoreFiles': { type: 'boolean', markdownDescription: nls.localize('useIgnoreFiles', "Controls whether to use `.gitignore` and `.ignore` files when searching for files."), @@ -231,12 +225,6 @@ configurationRegistry.registerConfiguration({ default: false, description: nls.localize('search.showLineNumbers', "Controls whether to show line numbers for search results."), }, - 'search.usePCRE2': { - type: 'boolean', - default: false, - description: nls.localize('search.usePCRE2', "Whether to use the PCRE2 regex engine in text search. This enables using some advanced regex features like lookahead and backreferences. However, not all PCRE2 features are supported - only features that are also supported by JavaScript."), - deprecationMessage: nls.localize('usePCRE2Deprecated', "Deprecated. PCRE2 will be used automatically when using regex features that are only supported by PCRE2."), - }, 'search.actionsPosition': { type: 'string', enum: ['auto', 'right'], diff --git a/src/vs/workbench/services/search/common/queryBuilder.ts b/src/vs/workbench/services/search/common/queryBuilder.ts index ad3c4b9e1f53a..d602a6783b200 100644 --- a/src/vs/workbench/services/search/common/queryBuilder.ts +++ b/src/vs/workbench/services/search/common/queryBuilder.ts @@ -141,12 +141,6 @@ export class QueryBuilder { text(contentPattern: IPatternInfo, folderResources?: uri[], options: ITextQueryBuilderOptions = {}): ITextQuery { contentPattern = this.getContentPattern(contentPattern, options); - const searchConfig = this.configurationService.getValue(); - - const fallbackToPCRE = folderResources && folderResources.some(folder => { - const folderConfig = this.configurationService.getValue({ resource: folder }); - return !folderConfig.search?.useRipgrep; - }); const commonQuery = this.commonQuery(folderResources?.map(toWorkspaceFolder), options); return { @@ -155,7 +149,6 @@ export class QueryBuilder { contentPattern, previewOptions: options.previewOptions, maxFileSize: options.maxFileSize, - usePCRE2: searchConfig.search?.usePCRE2 || fallbackToPCRE || false, surroundingContext: options.surroundingContext, userDisabledExcludesAndIgnoreFiles: options.disregardExcludeSettings && options.disregardIgnoreFiles, diff --git a/src/vs/workbench/services/search/common/search.ts b/src/vs/workbench/services/search/common/search.ts index eb63046877da4..b83f767441402 100644 --- a/src/vs/workbench/services/search/common/search.ts +++ b/src/vs/workbench/services/search/common/search.ts @@ -131,7 +131,6 @@ export interface ITextQueryProps extends ICommonQueryPr previewOptions?: ITextSearchPreviewOptions; maxFileSize?: number; - usePCRE2?: boolean; surroundingContext?: number; userDisabledExcludesAndIgnoreFiles?: boolean; @@ -193,10 +192,6 @@ export interface INotebookPatternInfo { isInNotebookCellOutput?: boolean; } -export interface IExtendedExtensionSearchOptions { - usePCRE2?: boolean; -} - export interface IFileMatch { resource: U; results?: ITextSearchResult[]; @@ -430,7 +425,6 @@ export const enum SemanticSearchBehavior { export interface ISearchConfigurationProperties { exclude: glob.IExpression; - useRipgrep: boolean; /** * Use ignore file for file search. */ @@ -442,7 +436,6 @@ export interface ISearchConfigurationProperties { globalFindClipboard: boolean; useReplacePreview: boolean; showLineNumbers: boolean; - usePCRE2: boolean; actionsPosition: 'auto' | 'right'; maxResults: number | null; collapseResults: 'auto' | 'alwaysCollapse' | 'alwaysExpand'; diff --git a/src/vs/workbench/services/search/common/textSearchManager.ts b/src/vs/workbench/services/search/common/textSearchManager.ts index 24cc41e05b98e..0b74a125d1e05 100644 --- a/src/vs/workbench/services/search/common/textSearchManager.ts +++ b/src/vs/workbench/services/search/common/textSearchManager.ts @@ -11,7 +11,7 @@ import * as path from '../../../../base/common/path.js'; import * as resources from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { FolderQuerySearchTree } from './folderQuerySearchTree.js'; -import { DEFAULT_MAX_SEARCH_RESULTS, hasSiblingPromiseFn, IAITextQuery, IExtendedExtensionSearchOptions, IFileMatch, IFolderQuery, excludeToGlobPattern, IPatternInfo, ISearchCompleteStats, ITextQuery, ITextSearchContext, ITextSearchMatch, ITextSearchResult, ITextSearchStats, QueryGlobTester, QueryType, resolvePatternsForProvider, ISearchRange, DEFAULT_TEXT_SEARCH_PREVIEW_OPTIONS } from './search.js'; +import { DEFAULT_MAX_SEARCH_RESULTS, hasSiblingPromiseFn, IAITextQuery, IFileMatch, IFolderQuery, excludeToGlobPattern, IPatternInfo, ISearchCompleteStats, ITextQuery, ITextSearchContext, ITextSearchMatch, ITextSearchResult, ITextSearchStats, QueryGlobTester, QueryType, resolvePatternsForProvider, ISearchRange, DEFAULT_TEXT_SEARCH_PREVIEW_OPTIONS } from './search.js'; import { TextSearchComplete2, TextSearchMatch2, TextSearchProviderFolderOptions, TextSearchProvider2, TextSearchProviderOptions, TextSearchQuery2, TextSearchResult2, AITextSearchProvider, AISearchResult, AISearchKeyword } from './searchExtTypes.js'; export interface IFileUtils { @@ -180,9 +180,6 @@ export class TextSearchManager { previewOptions: this.query.previewOptions ?? DEFAULT_TEXT_SEARCH_PREVIEW_OPTIONS, surroundingContext: this.query.surroundingContext ?? 0, }; - if ('usePCRE2' in this.query) { - (searchOptions).usePCRE2 = this.query.usePCRE2; - } let result; if (this.queryProviderPair.query.type === QueryType.aiText) { diff --git a/src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts b/src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts index 266c24914a75d..8ebeeaee48532 100644 --- a/src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts +++ b/src/vs/workbench/services/search/node/ripgrepTextSearchEngine.ts @@ -13,7 +13,7 @@ import { splitGlobAware } from '../../../../base/common/glob.js'; import { createRegExp, escapeRegExpCharacters } from '../../../../base/common/strings.js'; import { URI } from '../../../../base/common/uri.js'; import { Progress } from '../../../../platform/progress/common/progress.js'; -import { DEFAULT_MAX_SEARCH_RESULTS, IExtendedExtensionSearchOptions, ITextSearchPreviewOptions, SearchError, SearchErrorCode, serializeSearchError, TextSearchMatch } from '../common/search.js'; +import { DEFAULT_MAX_SEARCH_RESULTS, ITextSearchPreviewOptions, SearchError, SearchErrorCode, serializeSearchError, TextSearchMatch } from '../common/search.js'; import { Range, TextSearchComplete2, TextSearchContext2, TextSearchMatch2, TextSearchProviderOptions, TextSearchQuery2, TextSearchResult2 } from '../common/searchExtTypes.js'; import { AST as ReAST, RegExpParser, RegExpVisitor } from 'vscode-regexpp'; import { anchorGlob, IOutputChannel, Maybe, rangeToSearchRange, searchRangeToRange } from './ripgrepSearchUtils.js'; @@ -478,10 +478,6 @@ export function getRgArgs(query: TextSearchQuery2, options: RipgrepTextSearchOpt query.isRegExp = true; } - if ((options).usePCRE2) { - args.push('--pcre2'); - } - // Allow $ to match /r/n args.push('--crlf'); diff --git a/src/vs/workbench/services/search/test/browser/queryBuilder.test.ts b/src/vs/workbench/services/search/test/browser/queryBuilder.test.ts index 72b1043fd52f4..2604b9d1b5367 100644 --- a/src/vs/workbench/services/search/test/browser/queryBuilder.test.ts +++ b/src/vs/workbench/services/search/test/browser/queryBuilder.test.ts @@ -23,9 +23,8 @@ import { extUriBiasedIgnorePathCase } from '../../../../../base/common/resources import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; const DEFAULT_EDITOR_CONFIG = {}; -const DEFAULT_USER_CONFIG = { useRipgrep: true, useIgnoreFiles: true, useGlobalIgnoreFiles: true, useParentIgnoreFiles: true }; +const DEFAULT_USER_CONFIG = { useIgnoreFiles: true, useGlobalIgnoreFiles: true, useParentIgnoreFiles: true }; const DEFAULT_QUERY_PROPS = {}; -const DEFAULT_TEXT_QUERY_PROPS = { usePCRE2: false }; suite('QueryBuilder', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -1211,11 +1210,6 @@ function makeExcludePatternFromPatterns(...patterns: string[]): { } function assertEqualTextQueries(actual: ITextQuery, expected: ITextQuery): void { - expected = { - ...DEFAULT_TEXT_QUERY_PROPS, - ...expected - }; - return assertEqualQueries(actual, expected); } diff --git a/src/vs/workbench/services/search/test/node/textSearch.integrationTest.ts b/src/vs/workbench/services/search/test/node/textSearch.integrationTest.ts index cacb8f8e0d0b8..4e72ef243e92a 100644 --- a/src/vs/workbench/services/search/test/node/textSearch.integrationTest.ts +++ b/src/vs/workbench/services/search/test/node/textSearch.integrationTest.ts @@ -79,7 +79,7 @@ flakySuite('TextSearch-integration', function () { return doSearchTest(config, 4); }); - test('Text: GameOfLife (unicode escape sequences, force PCRE2)', () => { + test('Text: GameOfLife (PCRE2 lookbehind)', () => { const config: ITextQuery = { type: QueryType.Text, folderQueries: ROOT_FOLDER_QUERY, @@ -93,7 +93,6 @@ flakySuite('TextSearch-integration', function () { const config: ITextQuery = { type: QueryType.Text, folderQueries: ROOT_FOLDER_QUERY, - usePCRE2: true, contentPattern: { pattern: 'Life(?!P)', isRegExp: true } }; diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts index a64b91c944165..bd90cd1debb0b 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatWidget.fixture.ts @@ -55,6 +55,7 @@ export interface IFixtureMessage { | { kind: 'terminalConfirmation'; command: string; title?: string; disclaimer?: string; requestUnsandboxedExecution?: boolean; requestUnsandboxedExecutionReason?: string; riskAssessment?: { risk: ToolRiskLevel; explanation: string }; riskLoading?: boolean; confirmation?: { commandLine: string; cwdLabel?: string; cdPrefix?: string } } | { kind: 'elicitation'; title: string; message: string; confirmation?: { commandLine: string; cwdLabel?: string; cdPrefix?: string }; riskAssessment?: { risk: ToolRiskLevel; explanation: string }; riskLoading?: boolean } >; + readonly details?: string; readonly responseComplete?: boolean; /** * Per-turn file changes surfaced via {@link IChatResponseFileChangesService}, @@ -70,6 +71,12 @@ export interface IChatWidgetFixtureOptions { readonly height?: number; /** Whether to render the main chat input. Defaults to `true`. */ readonly inputVisible?: boolean; + /** Chat list rendering style. Defaults to compact for existing fixtures. */ + readonly renderStyle?: 'default' | 'compact' | 'minimal'; + /** Whether to populate the response footer with an action. */ + readonly responseFooterAction?: boolean; + /** Whether to show request and response timing details. */ + readonly verbose?: boolean; /** * When `false`, registers a stub `IChatToolRiskAssessmentService` whose * `isEnabled()` returns `false`, exercising the "feature off" code path. @@ -192,6 +199,9 @@ export async function renderChatWidget(context: ComponentFixtureContext, options }); configService.setUserConfiguration('editor', { fontFamily: 'monospace', fontLigatures: false }); configService.setUserConfiguration(ChatConfiguration.ToolConfirmationCarousel, true); + if (options.verbose !== undefined) { + configService.setUserConfiguration(ChatConfiguration.Verbose, options.verbose); + } if (needsTurnPills) { configService.setUserConfiguration(ChatConfiguration.TurnStatusPills, options.turnStatusPills); } @@ -262,6 +272,9 @@ export async function renderChatWidget(context: ComponentFixtureContext, options model.acceptResponseProgress(request, toolInvocation); } } + if (message.details) { + response.setResult({ details: message.details }); + } if (message.responseComplete !== false) { response.complete(); } @@ -304,7 +317,11 @@ export async function renderChatWidget(context: ComponentFixtureContext, options menuService.addItem(MenuId.ChatExecute, { command: { id: 'workbench.action.chat.submit', title: 'Send', icon: Codicon.newLine }, group: 'navigation', order: 4 }); menuService.addItem(MenuId.ChatInputSecondary, { command: { id: 'workbench.action.chat.openSessionTargetPicker', title: 'Local' }, group: 'navigation', order: 0 }); menuService.addItem(MenuId.ChatInputSecondary, { command: { id: 'workbench.action.chat.openPermissionPicker', title: 'Default Approvals' }, group: 'navigation', order: 10 }); + if (options.responseFooterAction) { + menuService.addItem(MenuId.ChatMessageFooter, { command: { id: 'workbench.action.chat.copyResponse', title: 'Copy', icon: Codicon.copy }, group: 'navigation', order: 1 }); + } + const renderStyle = options.renderStyle === 'default' ? undefined : options.renderStyle ?? 'compact'; const inputOptions: IChatInputPartOptions = { renderFollowups: false, renderInputToolbarBelowInput: false, @@ -350,7 +367,7 @@ export async function renderChatWidget(context: ComponentFixtureContext, options { currentChatMode: () => ChatModeKind.Agent, defaultElementHeight: 120, - renderStyle: 'compact', + renderStyle, styles: { listForeground: 'var(--vscode-foreground)', listBackground: 'var(--vscode-editor-background)', @@ -379,6 +396,70 @@ const SIMPLE_QA: IFixtureMessage[] = [ }, ]; +const LAST_RESPONSE_HOVER: IFixtureMessage[] = [ + { + user: 'Summarize the changes', + assistant: [ + { kind: 'markdown', text: 'The response content ends here. The remaining row height is reserved space.' }, + ], + details: 'Claude Opus 4.8 - 2 credits', + }, +]; + +async function renderLastResponseHover(context: ComponentFixtureContext, target: 'content' | 'reserved-space'): Promise { + await renderChatWidget(context, { + messages: LAST_RESPONSE_HOVER, + height: 600, + inputVisible: false, + renderStyle: 'default', + responseFooterAction: true, + }); + + const response = context.container.querySelector('.interactive-response.chat-most-recent-response'); + const hoverTarget = target === 'content' ? response?.querySelector(':scope > .value') : response; + hoverTarget?.dispatchEvent(new MouseEvent('mouseenter')); +} + +const KEYBOARD_FOCUS: IFixtureMessage[] = [ + { + user: 'Summarize the changes', + assistant: [ + { kind: 'markdown', text: 'The first response has keyboard-accessible actions.' }, + ], + details: 'Claude Opus 4.8 - 2 credits', + }, + { + user: 'What should I do next?', + assistant: [ + { kind: 'markdown', text: 'Run the tests and review the diff.' }, + ], + details: 'Claude Opus 4.8 - 1 credit', + }, +]; + +async function renderKeyboardFocus(context: ComponentFixtureContext, target: 'response-action' | 'request-timestamp'): Promise { + await renderChatWidget(context, { + messages: KEYBOARD_FOCUS, + height: 600, + inputVisible: false, + renderStyle: 'default', + responseFooterAction: true, + verbose: target === 'request-timestamp', + }); + + const selector = target === 'response-action' + ? '.interactive-response:not(.chat-most-recent-response) .chat-footer-toolbar .action-label' + : '.interactive-request .chat-request-timestamp'; + const focusTarget = context.container.querySelector(selector); + if (!focusTarget) { + throw new Error(`Missing keyboard focus target: ${target}`); + } + focusTarget.focus(); + if (focusTarget.ownerDocument.activeElement !== focusTarget) { + throw new Error(`Could not focus keyboard target: ${target}`); + } +} + const PENDING_TOOL_APPROVAL: IFixtureMessage[] = [ { user: 'run git init', @@ -487,4 +568,8 @@ export default defineThemedFixtureGroup({ path: 'chat/widget/' }, { 'issue-309796-missing-backslash': defineComponentFixture({ render: ctx => renderChatWidget(ctx, { messages: ISSUE_309796_MISSING_BACKSLASH }) }), }), MultiTurn: defineComponentFixture({ render: ctx => renderChatWidget(ctx, { messages: MULTI_TURN }) }), + LastResponseContentHover: defineComponentFixture({ render: ctx => renderLastResponseHover(ctx, 'content') }), + LastResponseReservedSpaceHover: defineComponentFixture({ render: ctx => renderLastResponseHover(ctx, 'reserved-space') }), + ResponseActionKeyboardFocus: defineComponentFixture({ render: ctx => renderKeyboardFocus(ctx, 'response-action') }), + RequestTimestampKeyboardFocus: defineComponentFixture({ render: ctx => renderKeyboardFocus(ctx, 'request-timestamp') }), });