diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 9365df4d904a6d..70d3807c5efb7f 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -754,10 +754,12 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost await this._execution.cancelSession(session); return; } - // Clients restore the last turn's model configuration, not the SDK's creation defaults. - const message: Message = definition.message.model === undefined && definition.session.model !== undefined - ? { ...definition.message, model: definition.session.model } - : definition.message; + // Turn selections override creation defaults in both the provider and restored clients. + const message: Message = { + ...definition.message, + ...(definition.message.model === undefined && definition.session.model !== undefined ? { model: definition.session.model } : {}), + ...(definition.message.agent === undefined && definition.session.agent !== undefined ? { agent: definition.session.agent } : {}), + }; await this._execution.startSession(session, message); } catch (error) { try { diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index e04651a38fe3a9..5192d66e85a3db 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -69,10 +69,11 @@ import { IAgentHostProxyResolver } from '../agentHostProxyResolver.js'; import { MODEL_REFRESH_BASE_DELAY_MS, MODEL_REFRESH_MAX_ATTEMPTS, MODEL_REFRESH_MAX_DELAY_MS, modelRefreshBackoff } from '../shared/modelRefreshRetry.js'; import { AGENT_HOST_WORKSPACELESS_INSTRUCTIONS } from '../shared/workspacelessInstructions.js'; import { IAgentHostCheckpointService } from '../../common/agentHostCheckpointService.js'; +import { IAgentHostGitService, tryResolvePrimaryWorktreeRoot } from '../../common/agentHostGitService.js'; import { ISessionDataService } from '../../common/sessionDataService.js'; import { ICopilotApiService } from '../shared/copilotApiService.js'; import { extractForwardedErrorInfo } from '../shared/proxyChatError.js'; -import { IAgentHostWorktreeIsolation, type IAgentHostWorktreePendingState } from '../shared/worktreeIsolation.js'; +import { IAgentHostWorktreeIsolation } from '../shared/worktreeIsolation.js'; import { getServerToolDisplay } from '../shared/serverToolGroups.js'; import { IAgentSdkDownloader, IAgentSdkPackage } from '../agentSdkDownloader.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; @@ -1252,7 +1253,6 @@ export class CodexAgent extends Disposable implements IAgent { private readonly _metadataStore: CodexSessionMetadataStore; private _lastSignInRequest: string | undefined; private _lastSignOutRequest: string | undefined; - private readonly _worktree: IAgentHostWorktreePendingState; /** * The agent host's server-tool host (feedback "comments" today, more in the @@ -1280,12 +1280,12 @@ export class CodexAgent extends Disposable implements IAgent { @IAgentHostOTelService private readonly _otelService: IAgentHostOTelService, @IAgentHostCustomizationEnablementService private readonly _customizationEnablementService: IAgentHostCustomizationEnablementService, @IAgentHostSessionTitleSignal sessionTitleSignal: IAgentHostSessionTitleSignal, - @IAgentHostWorktreeIsolation worktree: IAgentHostWorktreeIsolation, + @IAgentHostWorktreeIsolation private readonly _worktree: IAgentHostWorktreeIsolation, @ISessionDataService private readonly _sessionDataService: ISessionDataService, @ITelemetryService private readonly _telemetryService: ITelemetryService, + @IAgentHostGitService private readonly _gitService: IAgentHostGitService, ) { super(); - this._worktree = worktree; this._metadataStore = this._instantiationService.createInstance(CodexSessionMetadataStore); this._githubMcpServerEnabled = this._isGitHubMcpServerEnabled(); this._publishAccountInfo({ status: 'unknown' }); @@ -1927,13 +1927,50 @@ export class CodexAgent extends Disposable implements IAgent { return resolved.filter(candidate => candidate !== undefined); } + /** Resolve native workspace agents in the host-owned worktree without rewriting their persisted selection identity. */ + private async _resolveSelectedAgent(session: ICodexSession): Promise { + const agent = session.agent; + if (!agent || !session.workingDirectory) { + return agent; + } + const agentUri = URI.parse(agent.uri); + const agentsDirectory = extUriBiasedIgnorePathCase.dirname(agentUri); + const sourceRoot = extUriBiasedIgnorePathCase.dirname(extUriBiasedIgnorePathCase.dirname(agentsDirectory)); + if (!extUriBiasedIgnorePathCase.isEqual(agentsDirectory, URI.joinPath(sourceRoot, '.github', 'agents')) + || extUriBiasedIgnorePathCase.isEqual(sourceRoot, session.workingDirectory)) { + return agent; + } + const worktree = await this._worktree.readWorktreeMetadata(session.configurationResource); + if (!worktree?.repositoryRoot || !worktree.worktreePath + || !extUriBiasedIgnorePathCase.isEqual(session.workingDirectory, worktree.worktreePath)) { + return agent; + } + if (!extUriBiasedIgnorePathCase.isEqual(sourceRoot, worktree.repositoryRoot)) { + try { + const checkoutRoot = await this._gitService.getRepositoryRoot(sourceRoot); + if (!checkoutRoot || !extUriBiasedIgnorePathCase.isEqual(checkoutRoot, sourceRoot) + || !extUriBiasedIgnorePathCase.isEqual(await tryResolvePrimaryWorktreeRoot(this._gitService, checkoutRoot), worktree.repositoryRoot)) { + return agent; + } + } catch (error) { + this._logService.warn('[Codex] Failed to resolve the selected workspace agent repository', error); + return agent; + } + } + return { + ...agent, + uri: URI.joinPath(worktree.worktreePath, '.github', 'agents', extUriBiasedIgnorePathCase.basename(agentUri)).toString(), + }; + } + private async _buildCustomizationLaunch(session: ICodexSession): Promise { const plugins = this._enabledClientPlugins(session); - const [workspaceAgents, workspaceSkills] = await Promise.all([ + const [workspaceAgents, workspaceSkills, selectedAgent] = await Promise.all([ discoverCodexWorkspaceAgents(this._customizationWorkingDirectories(session), this._fileService), discoverCodexWorkspaceSkills(this._customizationWorkingDirectories(session), this._fileService), + this._resolveSelectedAgent(session), ]); - const customization = await codexCustomizationConfig(workspaceAgents.agents, plugins, session.agent, this._fileService); + const customization = await codexCustomizationConfig(workspaceAgents.agents, plugins, selectedAgent, this._fileService); const developerInstructions = [ customization.developerInstructions, session.managedWorkingDirectory ? AGENT_HOST_WORKSPACELESS_INSTRUCTIONS : '', @@ -1965,7 +2002,7 @@ export class CodexAgent extends Disposable implements IAgent { })), ]; const signature = JSON.stringify({ - agent: session.agent?.uri, + agent: selectedAgent?.uri, agentRoles: customization.agentRoles, developerInstructions, selectedCapabilityRoots: selectedCapabilityRoots.map(root => root.location.path), diff --git a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts index 16b499309de718..e78dad5f1ebe97 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -725,13 +725,16 @@ suite('AgentHostAutomationService', () => { }))); }); - for (const hasMessageModel of [false, true]) { - test(hasMessageModel ? 'preserves an explicit Automation message model' : 'records the Automation model configuration on its first turn', async () => { + for (const messageOverride of ['none', 'model', 'agent', 'both']) { + test(`records the Automation session selections on its first turn with ${messageOverride} message overrides`, async () => { const session = URI.parse('mock:/model-configuration-run'); const model = { id: 'mock-model', config: { thinkingLevel: 'low', contextSize: 272_000 } }; - const messageModel = hasMessageModel ? { id: 'other-model', config: { thinkingLevel: 'high' } } : undefined; + const agent = { uri: 'file:///workspace/.github/agents/reviewer.agent.md' }; + const messageModel = messageOverride === 'model' || messageOverride === 'both' ? { id: 'other-model', config: { thinkingLevel: 'high' } } : undefined; + const messageAgent = messageOverride === 'agent' || messageOverride === 'both' ? { uri: 'file:///other/agents/reviewer.agent.md' } : undefined; const completed = new DeferredPromise(); let createdModel: AutomationDefinition['session']['model']; + let createdAgent: AutomationDefinition['session']['agent']; disposables.add(stateManager.onDidEmitEnvelope(envelope => { if (envelope.action.type === ActionType.AutomationRunLifecycleChanged && envelope.action.lifecycle.status === AutomationRunStatus.Completed) { void completed.complete(); @@ -740,6 +743,7 @@ suite('AgentHostAutomationService', () => { const service = createService({ createSession: async template => { createdModel = template.model; + createdAgent = template.agent; stateManager.createSession({ resource: session.toString(), provider: 'mock', @@ -767,9 +771,13 @@ suite('AgentHostAutomationService', () => { }); const automation = definition(); automation.session.model = model; + automation.session.agent = agent; if (messageModel) { automation.message.model = messageModel; } + if (messageAgent) { + automation.message.agent = messageAgent; + } await service.completeMigration(); await service.handleCreate({ ...createAction(), definition: automation }); await service.runAutomation({ @@ -781,12 +789,24 @@ suite('AgentHostAutomationService', () => { assert.deepStrictEqual({ createdModel, + createdAgent, recordedModel: stateManager.getChatState(buildDefaultChatUri(session))?.turns[0]?.message.model, + recordedAgent: stateManager.getChatState(buildDefaultChatUri(session))?.turns[0]?.message.agent, savedModel: stateManager.getAutomationCatalogState()?.entries[0].definition.session.model, + savedAgent: stateManager.getAutomationCatalogState()?.entries[0].definition.session.agent, + savedMessage: stateManager.getAutomationCatalogState()?.entries[0].definition.message, }, { createdModel: model, + createdAgent: agent, recordedModel: messageModel ?? model, + recordedAgent: messageAgent ?? agent, savedModel: model, + savedAgent: agent, + savedMessage: { + ...definition().message, + ...(messageModel ? { model: messageModel } : {}), + ...(messageAgent ? { agent: messageAgent } : {}), + }, }); }); } diff --git a/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts b/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts index 18e8157ecb58a0..ba89954822e1fa 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts @@ -12,6 +12,7 @@ import { DeferredPromise } from '../../../../../base/common/async.js'; import { VSBuffer } from '../../../../../base/common/buffer.js'; import { Emitter, Event } from '../../../../../base/common/event.js'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; +import { ResourceMap } from '../../../../../base/common/map.js'; import { Schemas } from '../../../../../base/common/network.js'; import { URI } from '../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../base/common/uuid.js'; @@ -37,7 +38,7 @@ import { CustomizationEnablementKind, CustomizationType, McpServerStatus, type C import { ISessionDataService } from '../../../common/sessionDataService.js'; import { SessionServerToolName } from '../../../common/serverToolNames.js'; import { AgentConfigurationService, IAgentConfigurationService } from '../../../node/agentConfigurationService.js'; -import { IAgentHostWorktreeIsolation, NullAgentHostWorktreeIsolation } from '../../../node/shared/worktreeIsolation.js'; +import { IAgentHostWorktreeIsolation, NullAgentHostWorktreeIsolation, type IWorktreeMetadata } from '../../../node/shared/worktreeIsolation.js'; import { IAgentHostCustomizationEnablementService, type CustomizationEnablementResolution } from '../../../node/agentHostCustomizationEnablementService.js'; import { AgentHostStateManager, IAgentHostStateManager } from '../../../node/agentHostStateManager.js'; import { IAgentHostSessionTitleSignal } from '../../../node/agentHostSessionTitleSignal.js'; @@ -45,6 +46,7 @@ import { IAgentHostGitHubEndpointService } from '../../../node/agentHostGitHubEn import { IAgentHostProxyResolver } from '../../../node/agentHostProxyResolver.js'; import { IAgentSdkDownloader } from '../../../node/agentSdkDownloader.js'; import { IAgentHostCheckpointService, NULL_CHECKPOINT_SERVICE } from '../../../common/agentHostCheckpointService.js'; +import { IAgentHostGitService } from '../../../common/agentHostGitService.js'; import { IAgentHostOTelService } from '../../../common/otel/agentHostOTelService.js'; import { CodexAgent, toCodexModelSelectionId } from '../../../node/codex/codexAgent.js'; import { CodexAppServerClient, type ICodexAppServerTransport } from '../../../node/codex/codexAppServerClient.js'; @@ -168,6 +170,8 @@ interface ICreateAgentOptions { readonly database?: TestSessionDatabase; readonly checkpointService?: IAgentHostCheckpointService; readonly customizationEnablementService?: IAgentHostCustomizationEnablementService; + readonly worktreeIsolation?: IAgentHostWorktreeIsolation; + readonly gitService?: Partial; } class TestCodexLogService extends NullLogService { @@ -219,6 +223,14 @@ class TestCodexConfigurationService extends AgentConfigurationService { } } +class TestCodexWorktreeIsolation extends NullAgentHostWorktreeIsolation { + readonly metadata = new ResourceMap(); + + override async readWorktreeMetadata(sessionUri: URI): Promise { + return this.metadata.get(sessionUri); + } +} + async function createAgent(disposables: Pick, options: ICreateAgentOptions = {}): Promise { const models = [{ id: 'gpt-test', name: 'GPT Test', model_picker_enabled: true, supported_endpoints: ['/responses'], vendor: 'OpenAI' }] as CCAModel[]; const instantiationService = new TestInstantiationService(); @@ -237,7 +249,12 @@ async function createAgent(disposables: Pick, options: I instantiationService.stub(ICopilotApiService, { _serviceBrand: undefined, models: async () => models }); instantiationService.stub(ICodexProxyService, { _serviceBrand: undefined }); instantiationService.stub(IAgentConfigurationService, configurationService); - instantiationService.stub(IAgentHostWorktreeIsolation, new NullAgentHostWorktreeIsolation()); + instantiationService.stub(IAgentHostWorktreeIsolation, options.worktreeIsolation ?? new NullAgentHostWorktreeIsolation()); + instantiationService.stub(IAgentHostGitService, { + getRepositoryRoot: async () => undefined, + getWorktreeRoots: async () => [], + ...options.gitService, + }); instantiationService.stub(IAgentHostStateManager, stateManager); instantiationService.stub(IAgentHostCustomizationEnablementService, options.customizationEnablementService ?? createNoopCustomizationEnablementService()); instantiationService.stub(IAgentHostGitHubEndpointService, createTestGitHubEndpointService()); @@ -292,7 +309,8 @@ async function createSession(agent: CodexAgent, options: IAgentCreateChatOptions } async function assertPrewarmEvictedOnSend(disposables: Pick, completePrewarmBeforeSend: boolean): Promise { - const agent = await createAgent(disposables); + const worktreeIsolation = new TestCodexWorktreeIsolation(); + const agent = await createAgent(disposables, { worktreeIsolation }); const peer = disposables.add(createTestPeer()); const client = new CodexAppServerClient(peer.transport); agent['_connection'] = { @@ -306,7 +324,12 @@ async function assertPrewarmEvictedOnSend(disposables: Pick { peer.exit(); }); - test('resumes an established thread when the selected workspace agent changes', async () => { - const agent = await createAgent(disposables); - agent['_schedulePrewarm'] = () => { }; - agent['_refreshSkillHookCustomizations'] = async () => { }; - agent['_refreshSkillExtraRoots'] = async () => { }; - const peer = disposables.add(createTestPeer()); - agent['_connection'] = { - kind: 'ready', - client: new CodexAppServerClient(peer.transport), - usageSource: 'github', - child: { kill: () => true }, - } as never; + for (const isolated of [false, true]) { + test(`reapplied workspace agent selection keeps updated instructions${isolated ? ' in an isolated worktree' : ''}`, async () => { + const worktreeIsolation = new TestCodexWorktreeIsolation(); + const agent = await createAgent(disposables, { worktreeIsolation }); + agent['_schedulePrewarm'] = () => { }; + agent['_refreshSkillHookCustomizations'] = async () => { }; + agent['_refreshSkillExtraRoots'] = async () => { }; + const peer = disposables.add(createTestPeer()); + agent['_connection'] = { + kind: 'ready', + client: new CodexAppServerClient(peer.transport), + usageSource: 'github', + child: { kill: () => true }, + } as never; + + const repo = URI.file('/repo-workspace-agent-edit'); + const workingDirectory = isolated ? URI.file('/repo-workspace-agent-worktree') : repo; + const sourceAgentUri = URI.joinPath(repo, '.github', 'agents', 'reviewer.agent.md'); + const agentUri = URI.joinPath(workingDirectory, '.github', 'agents', 'reviewer.agent.md'); + const selectedAgent = Object.freeze({ uri: sourceAgentUri.toString() }); + const firstInstructions = isolated ? 'Use the worktree instructions.' : 'Use the original instructions.'; + await agent['_fileService'].writeFile(sourceAgentUri, VSBuffer.fromString('---\nname: Reviewer\ndescription: Reviews changes\n---\nUse the original instructions.')); + if (isolated) { + await agent['_fileService'].writeFile(agentUri, VSBuffer.fromString(`---\nname: Reviewer\ndescription: Reviews changes\n---\n${firstInstructions}`)); + } + const { session } = await createSession(agent, { + workingDirectories: [repo], + model: { id: COPILOT_TEST_MODEL }, + }); + const chat = URI.parse(buildDefaultChatUri(session)); + const context = chatContext(session, chat); + if (isolated) { + worktreeIsolation.metadata.set(session, { branchName: 'agents/reviewer', repositoryRoot: repo, worktreePath: workingDirectory }); + } + + await agent.chats.changeAgent(chat, selectedAgent, context); + const firstSend = agent.chats.sendMessage(chat, 'first', [workingDirectory], undefined, 'turn-1'); + const start = await readNextRequest(peer.outbound); + peer.push({ id: start.id, result: { thread: { id: 'thread-workspace-agent' } } }); + const firstTurn = await readNextRequest(peer.outbound); + peer.push({ id: firstTurn.id, result: {} }); + await firstSend; + + await agent['_fileService'].writeFile(agentUri, VSBuffer.fromString('---\nname: Reviewer\ndescription: Reviews changes\n---\nUse the updated instructions.')); + await agent.chats.changeAgent(chat, selectedAgent, context); + const secondSend = agent.chats.sendMessage(chat, 'second', [workingDirectory], undefined, 'turn-2'); + const unsubscribe = await readNextRequest(peer.outbound); + peer.push({ id: unsubscribe.id, result: {} }); + const resume = await readNextRequest(peer.outbound); + const resumedAgents = resume.params.config?.['agents'] as Record; + const resumedRoleFile = await fs.promises.readFile(resumedAgents.Reviewer.config_file, 'utf8'); + peer.push({ id: resume.id, result: { thread: { id: 'thread-workspace-agent', cwd: workingDirectory.fsPath }, cwd: workingDirectory.fsPath } }); + const inventory = await readNextRequest(peer.outbound); + peer.push({ id: inventory.id, result: { data: [], nextCursor: null } }); + const secondTurn = await readNextRequest(peer.outbound); + peer.push({ id: secondTurn.id, result: {} }); + await secondSend; - const repo = URI.file('/repo-workspace-agent-edit'); - const agentUri = URI.joinPath(repo, '.github', 'agents', 'reviewer.agent.md'); - await agent['_fileService'].writeFile(agentUri, VSBuffer.fromString('---\nname: Reviewer\ndescription: Reviews changes\n---\nUse the original instructions.')); - const { session } = await createSession(agent, { - workingDirectories: [repo], - model: { id: COPILOT_TEST_MODEL }, - agent: { uri: agentUri.toString() }, + assert.deepStrictEqual({ + start: { method: start.method, cwd: start.params.cwd, developerInstructions: start.params.developerInstructions }, + firstTurn: { method: firstTurn.method, developerInstructions: firstTurn.params.collaborationMode?.settings.developer_instructions }, + unsubscribe: { method: unsubscribe.method, threadId: unsubscribe.params.threadId }, + resume: { method: resume.method, developerInstructions: resume.params.developerInstructions }, + secondTurn: { method: secondTurn.method, developerInstructions: secondTurn.params.collaborationMode?.settings.developer_instructions }, + resumedRoleFile, + selectedAgent: agent['_sessions'].get(AgentSession.id(session))?.agent, + needsResume: agent['_sessions'].get(AgentSession.id(session))?.needsResume, + }, { + start: { method: 'thread/start', cwd: workingDirectory.fsPath, developerInstructions: `${firstInstructions}\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + firstTurn: { method: 'turn/start', developerInstructions: `${firstInstructions}\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + unsubscribe: { method: 'thread/unsubscribe', threadId: 'thread-workspace-agent' }, + resume: { method: 'thread/resume', developerInstructions: `Use the updated instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + secondTurn: { method: 'turn/start', developerInstructions: `Use the updated instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + resumedRoleFile: 'name = "Reviewer"\ndescription = "Reviews changes"\ndeveloper_instructions = "Use the updated instructions."\n', + selectedAgent, + needsResume: false, + }); + peer.exit(); }); - const chat = URI.parse(buildDefaultChatUri(session)); + } - const firstSend = agent.chats.sendMessage(chat, 'first', [repo], undefined, 'turn-1'); - const start = await readNextRequest(peer.outbound); - peer.push({ id: start.id, result: { thread: { id: 'thread-workspace-agent' } } }); - const firstTurn = await readNextRequest(peer.outbound); - peer.push({ id: firstTurn.id, result: {} }); - await firstSend; + for (const { reapplySelection, linkedSource } of [ + { reapplySelection: false, linkedSource: false }, + { reapplySelection: true, linkedSource: false }, + { reapplySelection: false, linkedSource: true }, + { reapplySelection: true, linkedSource: true }, + ]) { + test(`worktree agent instructions survive provider reload (reapplySelection=${reapplySelection}, linkedSource=${linkedSource})`, async () => { + const database = new TestSessionDatabase(); + const worktreeIsolation = new TestCodexWorktreeIsolation(); + const repo = URI.file('/repo-restored-agent'); + const worktree = URI.file('/repo-restored-agent-worktree'); + const sourceRoot = linkedSource ? URI.file('/repo-restored-agent-linked-source') : repo; + const sourceAgentUri = URI.joinPath(sourceRoot, '.github', 'agents', 'reviewer.agent.md'); + const worktreeAgentUri = URI.joinPath(worktree, '.github', 'agents', 'reviewer.agent.md'); + const selectedAgent = Object.freeze({ uri: sourceAgentUri.toString() }); + const gitService: Partial = { + getRepositoryRoot: async () => sourceRoot, + getWorktreeRoots: async () => [repo, sourceRoot, worktree], + }; + const agentA = await createAgent(disposables, { database, worktreeIsolation, gitService }); + agentA['_schedulePrewarm'] = () => { }; + agentA['_refreshSkillHookCustomizations'] = async () => { }; + agentA['_refreshSkillExtraRoots'] = async () => { }; + const peerA = disposables.add(createTestPeer()); + agentA['_connection'] = { + kind: 'ready', + client: new CodexAppServerClient(peerA.transport), + usageSource: 'github', + child: { kill: () => true }, + } as never; + let peerB: ITestPeer | undefined; - await agent['_fileService'].writeFile(agentUri, VSBuffer.fromString('---\nname: Reviewer\ndescription: Reviews changes\n---\nUse the updated instructions.')); - const secondSend = agent.chats.sendMessage(chat, 'second', [repo], undefined, 'turn-2'); - const unsubscribe = await readNextRequest(peer.outbound); - peer.push({ id: unsubscribe.id, result: {} }); - const resume = await readNextRequest(peer.outbound); - const resumedAgents = resume.params.config?.['agents'] as Record; - const resumedRoleFile = await fs.promises.readFile(resumedAgents.Reviewer.config_file, 'utf8'); - peer.push({ id: resume.id, result: { thread: { id: 'thread-workspace-agent', cwd: repo.fsPath }, cwd: repo.fsPath } }); - const inventory = await readNextRequest(peer.outbound); - peer.push({ id: inventory.id, result: { data: [], nextCursor: null } }); - const secondTurn = await readNextRequest(peer.outbound); - peer.push({ id: secondTurn.id, result: {} }); - await secondSend; + try { + await agentA['_fileService'].writeFile(sourceAgentUri, VSBuffer.fromString('---\nname: Reviewer\n---\nUse the source instructions.')); + await agentA['_fileService'].writeFile(worktreeAgentUri, VSBuffer.fromString('---\nname: Reviewer\n---\nUse the worktree instructions.')); + const created = await createSession(agentA, { workingDirectories: [sourceRoot], model: { id: COPILOT_TEST_MODEL } }); + const chat = defaultChatOf(created.session); + const context = chatContext(created.session, chat); + worktreeIsolation.metadata.set(created.session, { branchName: 'agents/reviewer', repositoryRoot: repo, worktreePath: worktree }); + await agentA.chats.changeAgent(chat, selectedAgent, context); + + const firstSend = agentA.chats.sendMessage(chat, 'first', [worktree], undefined, 'turn-1'); + const start = await readNextRequest(peerA.outbound); + peerA.push({ id: start.id, result: { thread: { id: 'thread-restored-worktree-agent' } } }); + const firstTurn = await readNextRequest(peerA.outbound); + peerA.push({ id: firstTurn.id, result: {} }); + await firstSend; + await new Promise(resolve => setImmediate(resolve)); + const overlay = await agentA['_metadataStore'].read(created.session); + + const agentB = await createAgent(disposables, { database, worktreeIsolation, gitService }); + agentB['_refreshSkillHookCustomizations'] = async () => { }; + agentB['_refreshSkillExtraRoots'] = async () => { }; + peerB = disposables.add(createTestPeer()); + agentB['_connection'] = { + kind: 'ready', + client: new CodexAppServerClient(peerB.transport), + usageSource: 'github', + child: { kill: () => true }, + } as never; + await agentB['_fileService'].writeFile(worktreeAgentUri, VSBuffer.fromString('---\nname: Reviewer\n---\nUse the restored worktree instructions.')); + await agentB.materializeChat(chat, context, created.providerData); + if (reapplySelection) { + await agentB.chats.changeAgent(chat, selectedAgent, context); + } - assert.deepStrictEqual({ - start: { method: start.method, developerInstructions: start.params.developerInstructions }, - firstTurn: { method: firstTurn.method, developerInstructions: firstTurn.params.collaborationMode?.settings.developer_instructions }, - unsubscribe: { method: unsubscribe.method, threadId: unsubscribe.params.threadId }, - resume: { method: resume.method, developerInstructions: resume.params.developerInstructions }, - secondTurn: { method: secondTurn.method, developerInstructions: secondTurn.params.collaborationMode?.settings.developer_instructions }, - resumedRoleFile, - needsResume: agent['_sessions'].get(AgentSession.id(session))?.needsResume, - }, { - start: { method: 'thread/start', developerInstructions: `Use the original instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, - firstTurn: { method: 'turn/start', developerInstructions: `Use the original instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, - unsubscribe: { method: 'thread/unsubscribe', threadId: 'thread-workspace-agent' }, - resume: { method: 'thread/resume', developerInstructions: `Use the updated instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, - secondTurn: { method: 'turn/start', developerInstructions: `Use the updated instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, - resumedRoleFile: 'name = "Reviewer"\ndescription = "Reviews changes"\ndeveloper_instructions = "Use the updated instructions."\n', - needsResume: false, + const secondSend = agentB.chats.sendMessage(chat, 'second', undefined, undefined, 'turn-2', undefined, undefined, context); + const read = await readNextRequest(peerB.outbound); + peerB.push({ id: read.id, result: { thread: { id: 'thread-restored-worktree-agent', modelProvider: 'vscode-proxy' } } }); + const unsubscribe = await readNextRequest(peerB.outbound); + peerB.push({ id: unsubscribe.id, result: {} }); + const resume = await readNextRequest(peerB.outbound); + peerB.push({ id: resume.id, result: { thread: { id: 'thread-restored-worktree-agent', cwd: worktree.fsPath }, cwd: worktree.fsPath } }); + const inventory = await readNextRequest(peerB.outbound); + peerB.push({ id: inventory.id, result: { data: [], nextCursor: null } }); + const secondTurn = await readNextRequest(peerB.outbound); + peerB.push({ id: secondTurn.id, result: {} }); + await secondSend; + + assert.deepStrictEqual({ + overlay: { cwd: overlay.cwd?.toString(), agent: overlay.agent }, + start: { method: start.method, cwd: start.params.cwd, developerInstructions: start.params.developerInstructions }, + firstTurn: { method: firstTurn.method, developerInstructions: firstTurn.params.collaborationMode?.settings.developer_instructions }, + read: { method: read.method, threadId: read.params.threadId }, + unsubscribe: { method: unsubscribe.method, threadId: unsubscribe.params.threadId }, + resume: { method: resume.method, threadId: resume.params.threadId, developerInstructions: resume.params.developerInstructions }, + secondTurn: { method: secondTurn.method, threadId: secondTurn.params.threadId, developerInstructions: secondTurn.params.collaborationMode?.settings.developer_instructions }, + selectedAgent: agentB['_sessions'].get(AgentSession.id(created.session))?.agent, + }, { + overlay: { cwd: worktree.toString(), agent: selectedAgent }, + start: { method: 'thread/start', cwd: worktree.fsPath, developerInstructions: `Use the worktree instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + firstTurn: { method: 'turn/start', developerInstructions: `Use the worktree instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + read: { method: 'thread/read', threadId: 'thread-restored-worktree-agent' }, + unsubscribe: { method: 'thread/unsubscribe', threadId: 'thread-restored-worktree-agent' }, + resume: { method: 'thread/resume', threadId: 'thread-restored-worktree-agent', developerInstructions: `Use the restored worktree instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + secondTurn: { method: 'turn/start', threadId: 'thread-restored-worktree-agent', developerInstructions: `Use the restored worktree instructions.\n\n${CODEX_FILE_LINK_INSTRUCTIONS}` }, + selectedAgent, + }); + } finally { + peerA.exit(); + peerB?.exit(); + } }); - peer.exit(); - }); + } + + for (const prewarmClaimed of [false, true]) { + test(`only recorded workspace agents are resolved across cwd adoption (prewarmClaimed=${prewarmClaimed})`, async () => { + const worktreeIsolation = new TestCodexWorktreeIsolation(); + const repo = URI.file('/repo-workspace-agent'); + const worktree = URI.file('/repo-workspace-agent-worktree'); + const linkedSource = URI.file('/repo-workspace-agent-linked'); + const pluginRoot = URI.joinPath(repo, 'plugins', 'reviewer'); + const externalRoot = URI.file('/repo-workspace-agent-external'); + const sourceAgent = URI.joinPath(repo, '.github', 'agents', 'reviewer.agent.md'); + const worktreeAgent = URI.joinPath(worktree, '.github', 'agents', 'reviewer.agent.md'); + const linkedAgent = URI.joinPath(linkedSource, '.github', 'agents', 'reviewer.agent.md'); + const pluginAgent = URI.joinPath(pluginRoot, 'agents', 'reviewer.agent.md'); + const nestedAgent = URI.joinPath(pluginRoot, '.github', 'agents', 'reviewer.agent.md'); + const externalAgent = URI.joinPath(externalRoot, '.github', 'agents', 'reviewer.agent.md'); + const repositoryRoots = new ResourceMap([[linkedSource, linkedSource], [pluginRoot, repo], [externalRoot, externalRoot]]); + const worktreeRoots = new ResourceMap([[linkedSource, [repo, linkedSource, worktree]], [externalRoot, [externalRoot]]]); + const agent = await createAgent(disposables, { + worktreeIsolation, + gitService: { + getRepositoryRoot: async directory => repositoryRoots.get(directory), + getWorktreeRoots: async directory => worktreeRoots.get(directory) ?? [], + }, + }); + agent['_schedulePrewarm'] = () => { }; + const metadata: IWorktreeMetadata = { branchName: 'agents/reviewer', repositoryRoot: repo, worktreePath: worktree }; + const cases = [ + { selected: sourceAgent, expected: worktreeAgent, metadata }, + { selected: linkedAgent, expected: worktreeAgent, metadata }, + { selected: pluginAgent, expected: pluginAgent, metadata }, + { selected: nestedAgent, expected: nestedAgent, metadata }, + { selected: externalAgent, expected: externalAgent, metadata }, + { selected: worktreeAgent, expected: worktreeAgent, metadata }, + { selected: sourceAgent, expected: sourceAgent, metadata: undefined }, + { selected: sourceAgent, expected: sourceAgent, metadata: { ...metadata, worktreePath: URI.file('/other-worktree') } }, + { selected: undefined, expected: undefined, metadata }, + ]; + const adopted: Array<{ selectedAgent: string | undefined; resolvedAgent: string | undefined; reappliedAgent: string | undefined; workingDirectory: string | undefined }> = []; + + for (const { selected, metadata } of cases) { + const { session } = await createSession(agent, { + workingDirectories: [repo], + model: { id: COPILOT_TEST_MODEL }, + }); + const chat = defaultChatOf(session); + const context = chatContext(session, chat); + const selectedAgent = selected ? { uri: selected.toString() } : undefined; + if (metadata) { + worktreeIsolation.metadata.set(session, metadata); + } + await agent.chats.changeAgent(chat, selectedAgent, context); + const entry = agent['_sessions'].get(AgentSession.id(session))!; + if (prewarmClaimed) { + agent['_claimPrewarm'](entry); + } + await agent['_adoptWorkingDirectoryBeforeSend'](entry, worktree); + const resolvedAgent = await agent['_resolveSelectedAgent'](entry); + await agent.chats.changeAgent(chat, selectedAgent, context); + adopted.push({ + selectedAgent: entry.agent?.uri, + resolvedAgent: resolvedAgent?.uri, + reappliedAgent: (await agent['_resolveSelectedAgent'](entry))?.uri, + workingDirectory: entry.workingDirectory?.toString(), + }); + } + + assert.deepStrictEqual(adopted, cases.map(({ selected, expected }) => ({ + selectedAgent: selected?.toString(), + resolvedAgent: expected?.toString(), + reappliedAgent: expected?.toString(), + workingDirectory: worktree.toString(), + }))); + }); + } test('fresh multi-root start selects only existing secondary skill directories', async () => { const agent = await createAgent(disposables, { multiRootEnabled: true }); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts index d40db1d14e75a1..b3beb7acb7fdf8 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts @@ -59,7 +59,7 @@ import { AgentHostModePicker } from './agentHostModePicker.js'; import { MobileAgentHostModePicker } from './mobile/mobileAgentHostModePicker.js'; import { AgentHostPermissionPickerActionItem } from './agentHostPermissionPickerActionItem.js'; import { AgentHostPermissionPickerDelegate, isWellKnownAutoApproveSchema, isWellKnownClaudePermissionModeSchema, isWellKnownCodexApprovalsSchema, isWellKnownModeSchema } from './agentHostPermissionPickerDelegate.js'; -import { SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { omitAutomationSessionTemplateConfigValues, SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { AGENT_HOST_CHECKOUT_CHANGESET_OPERATION_ID } from '../../../../../platform/agentHost/common/agentHostChangesetOperationService.js'; import { CheckoutOperationPreAction, checkoutOperationMeta, isCheckoutOperationDirtyWorkingTreeErrorData } from '../../../../../platform/agentHost/common/meta/agentCheckoutOperationMeta.js'; import { ProtocolError } from '../../../../../platform/agentHost/common/state/sessionProtocol.js'; @@ -100,6 +100,15 @@ registerAction2(class extends Action2 { ContextKeyExpr.or(IsActiveSessionLocalAgentHost, IsActiveSessionRemoteAgentHost), IsQuickChatSessionContext.negate(), ), + }, { + id: Menus.NewSessionControl, + group: 'navigation', + order: 4, + when: ContextKeyExpr.and( + ContextKeyExpr.or(IsActiveSessionLocalAgentHost, IsActiveSessionRemoteAgentHost), + ChatContextKeys.enabled, + ChatContextKeys.inAutomationsDialog, + ), }], }); } @@ -379,6 +388,7 @@ export class AgentHostSessionConfigPicker extends Disposable { constructor( protected readonly _session: IObservable, + private readonly _options: { readonly includeRepositoryConfiguration?: boolean } = {}, @IActionWidgetService protected readonly _actionWidgetService: IActionWidgetService, @IConfigurationService protected readonly _configurationService: IConfigurationService, @IContextKeyService protected readonly _contextKeyService: IContextKeyService, @@ -477,7 +487,9 @@ export class AgentHostSessionConfigPicker extends Disposable { // chips must remain interactive. const isLoading = provider.isSessionConfigResolving(session.sessionId).get(); - const properties = this._orderProperties(Object.entries(resolvedConfig.schema.properties)); + const properties = this._orderProperties(Object.entries(this._options.includeRepositoryConfiguration === false + ? omitAutomationSessionTemplateConfigValues(resolvedConfig.schema.properties) + : resolvedConfig.schema.properties)); let renderedIsolationCheckbox = false; for (const [property, schema] of properties) { @@ -1430,7 +1442,15 @@ class AgentHostSessionConfigPickerContribution extends Disposable implements IWo 'sessions.agentHost.sessionConfigPicker', (_action, _options, scopedInstantiationService) => { const { session } = scopedInstantiationService.invokeFunction(accessor => accessor.get(ISessionContext)); - return new PickerActionViewItem(scopedInstantiationService.createInstance(MobileAgentHostSessionConfigPicker, session)); + return new PickerActionViewItem(scopedInstantiationService.createInstance(MobileAgentHostSessionConfigPicker, session, {})); + }, + )); + this._register(actionViewItemService.register( + Menus.NewSessionControl, + 'sessions.agentHost.sessionConfigPicker', + (_action, _options, scopedInstantiationService) => { + const { session } = scopedInstantiationService.invokeFunction(accessor => accessor.get(ISessionContext)); + return new PickerActionViewItem(scopedInstantiationService.createInstance(MobileAgentHostSessionConfigPicker, session, { includeRepositoryConfiguration: false })); }, )); this._register(actionViewItemService.register( diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts index b793902b64f4e3..d2045e8dd0757c 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts @@ -29,6 +29,7 @@ import { TestInstantiationService } from '../../../../../../../platform/instanti import { IStorageService } from '../../../../../../../platform/storage/common/storage.js'; import { ITelemetryService } from '../../../../../../../platform/telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../../../../../platform/telemetry/common/telemetryUtils.js'; +import { ChatContextKeys } from '../../../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { IView } from '../../../../../../../workbench/common/views.js'; import { IViewsService } from '../../../../../../../workbench/services/views/common/viewsService.js'; import { IAgentWorkbenchLayoutService } from '../../../../../../browser/workbench.js'; @@ -330,8 +331,8 @@ function setupServices( } /** Create and render a fresh picker instance, as the toolbar does on a rebuild. */ -function renderPicker(store: Pick, 'add'>, services: ReturnType) { - const picker = store.add(services.instantiationService.createInstance(AgentHostSessionConfigPicker, services.sessionObs)); +function renderPicker(store: Pick, 'add'>, services: ReturnType, options?: ConstructorParameters[1]) { + const picker = store.add(services.instantiationService.createInstance(AgentHostSessionConfigPicker, services.sessionObs, options)); const container = document.createElement('div'); picker.render(container); return { picker, container }; @@ -341,6 +342,62 @@ suite('Agent Host Session Config Picker', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); + test('offers provider-owned configuration in the automation controls menu', () => { + const item = MenuRegistry.getMenuItems(Menus.NewSessionControl) + .filter(isIMenuItem) + .find(item => item.command.id === 'sessions.agentHost.sessionConfigPicker'); + + assert.deepStrictEqual({ + order: item?.order, + automationScoped: item?.when?.keys().includes(ChatContextKeys.inAutomationsDialog.key), + }, { order: 4, automationScoped: true }); + }); + + test('edits automation enum and boolean options without duplicating repository or transient controls', async () => { + const services = setupServices(store); + const repositoryConfig = makeRepoConfig('main'); + services.provider.set({ + schema: { + type: 'object', + properties: { + ...repositoryConfig.schema.properties, + [SessionConfigKey.WorktreeBranchTrack]: { type: 'boolean', title: 'Branch Tracking' }, + [SessionConfigKey.AgentMerge]: { type: 'boolean', title: 'Agent Merge' }, + [SessionConfigKey.Permissions]: { type: 'boolean', title: 'Permissions' }, + [SessionConfigKey.ShellInitScripts]: { type: 'boolean', title: 'Shell Scripts' }, + detail: { type: 'string', title: 'Detail', enum: ['low', 'high'] }, + feature: { type: 'boolean', title: 'Feature' }, + }, + }, + values: { ...repositoryConfig.values, detail: 'low', feature: false }, + }, false); + const { container } = renderPicker(store, services, { includeRepositoryConfiguration: false }); + const triggers = container.querySelectorAll('a.action-label'); + triggers[0].click(); + await new Promise(resolve => setTimeout(resolve)); + services.actionWidget.delegate?.onSelect({ value: 'high', label: 'high' }); + await new Promise(resolve => setTimeout(resolve)); + container.querySelectorAll('a.action-label')[1].click(); + await new Promise(resolve => setTimeout(resolve)); + services.actionWidget.delegate?.onSelect({ value: 'true', label: 'On' }); + await new Promise(resolve => setTimeout(resolve)); + + assert.deepStrictEqual({ + triggers: triggers.length, + worktree: isolationSlot(container), + devContainer: container.querySelector('.sessions-chat-dev-container-checkbox'), + updates: services.provider.setSessionConfigValueArguments, + }, { + triggers: 2, + worktree: null, + devContainer: null, + updates: [ + { sessionId: SESSION_ID, property: 'detail', value: 'high' }, + { sessionId: SESSION_ID, property: 'feature', value: true }, + ], + }); + }); + test('restores pointer and keyboard focus without leaving pointer focus visible', async () => { const services = setupServices(store); const { container } = renderPicker(store, services); @@ -499,7 +556,7 @@ suite('Agent Host Session Config Picker', () => { test('generic auto-approve chips retain their contextual accessible name', () => { const services = setupServices(store); - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); const trigger = document.createElement('span'); picker.renderTriggerForTest(trigger, SessionConfigKey.AutoApprove, { title: 'Approval Mode', @@ -761,7 +818,7 @@ suite('Agent Host Session Config Picker', () => { } }); services.provider.config = makeDynamicBranchConfig('main', 'folder'); - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); await picker.setSessionConfigValueForTest(services.provider, SessionConfigKey.Branch, 'dev'); outcomes.push({ @@ -825,7 +882,7 @@ suite('Agent Host Session Config Picker', () => { } }); services.provider.config = makeDynamicBranchConfig('main', 'folder'); - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); const container = document.createElement('div'); picker.render(container); @@ -930,7 +987,7 @@ suite('Agent Host Session Config Picker', () => { test('serializes interleaved branch and isolation selections before deciding checkout', async () => { const services = setupServices(store); services.provider.config = makeDynamicBranchConfig('main', 'worktree'); - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); await Promise.all([ picker.setSessionConfigValueForTest(services.provider, SessionConfigKey.Branch, 'featureA'), @@ -1236,7 +1293,7 @@ suite('Agent Host Session Config Picker', () => { test('does not render configuration controls when the workspace has no Git repository', () => { const services = setupServices(store); services.provider.config = makeNoGitConfig(); - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); const container = document.createElement('div'); picker.render(container); @@ -1266,7 +1323,7 @@ suite('Agent Host Session Config Picker', () => { }, values: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.WorktreeBranchTrack]: false, [SessionConfigKey.WorktreeCreateNewBranch]: true }, } as ResolveSessionConfigResult; - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); const container = document.createElement('div'); picker.render(container); @@ -1286,7 +1343,7 @@ suite('Agent Host Session Config Picker', () => { }, values: { [SessionConfigKey.SandboxEnabled]: 'off' }, }; - const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs)); + const picker = store.add(services.instantiationService.createInstance(AlwaysRenderConfigPicker, services.sessionObs, {})); const container = document.createElement('div'); picker.render(container); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts index c88931381a9a5f..1be1b1c3d91498 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts @@ -58,6 +58,11 @@ registerAction2(class extends Action2 { group: 'navigation', order: 3, when: ContextKeyExpr.and(IsNewChatSessionContext, IsActiveSessionCopilotChatCloud, ChatContextKeys.enabled), + }, { + id: Menus.NewSessionControl, + group: 'navigation', + order: 3, + when: ContextKeyExpr.and(IsNewChatSessionContext, IsActiveSessionCopilotChatCloud, ChatContextKeys.enabled, ChatContextKeys.inAutomationsDialog), }], }); } @@ -149,14 +154,16 @@ class CopilotPickerActionViewItemContribution extends Disposable implements IWor return new PickerActionViewItem(picker); }, )); - this._register(actionViewItemService.register( - Menus.NewSessionRepositoryConfig, 'sessions.defaultCopilot.sandboxPicker', - (_action, _options, scopedInstantiationService) => { - const { session } = scopedInstantiationService.invokeFunction(accessor => accessor.get(ISessionContext)); - const picker = scopedInstantiationService.createInstance(SandboxPicker, session); - return new PickerActionViewItem(picker); - }, - )); + for (const menu of [Menus.NewSessionRepositoryConfig, Menus.NewSessionControl]) { + this._register(actionViewItemService.register( + menu, 'sessions.defaultCopilot.sandboxPicker', + (_action, _options, scopedInstantiationService) => { + const { session } = scopedInstantiationService.invokeFunction(accessor => accessor.get(ISessionContext)); + const picker = scopedInstantiationService.createInstance(SandboxPicker, session); + return new PickerActionViewItem(picker); + }, + )); + } this._register(actionViewItemService.register( Menus.NewSessionConfig, 'sessions.defaultCopilot.modePicker', (_action, _options, scopedInstantiationService) => { diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index 9f41b1551cc31e..499f46479acfa0 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -11,7 +11,7 @@ import { IMarkdownString, MarkdownString, markdownStringEqual } from '../../../. import { Disposable, DisposableStore, IDisposable, DisposableMap, MutableDisposable } from '../../../../../base/common/lifecycle.js'; import { Schemas } from '../../../../../base/common/network.js'; import { isWeb } from '../../../../../base/common/platform.js'; -import { autorun, constObservable, derived, derivedOpts, IObservable, IObservableSignal, IReader, ISettableObservable, ITransaction, observableFromPromise, observableSignal, observableValue, observableValueOpts, runOnChange, transaction } from '../../../../../base/common/observable.js'; +import { autorun, constObservable, derived, derivedOpts, IObservable, IObservableSignal, IReader, ISettableObservable, ITransaction, observableFromEvent, observableFromPromise, observableSignal, observableValue, observableValueOpts, runOnChange, transaction } from '../../../../../base/common/observable.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; import { URI } from '../../../../../base/common/uri.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; @@ -78,6 +78,7 @@ const STORAGE_KEY_ISOLATION_MODE = 'sessions.isolationPicker.selectedMode'; /** Remembers the cloud sandbox choice across new sessions, like the isolation picker above. */ const STORAGE_KEY_USE_SANDBOX = 'sessions.cloudSandboxPicker.useSandbox'; +const CLOUD_SANDBOX_CONFIG_KEY = 'useSandbox'; function getGitHubRepositoryId(repository: string): string | undefined { const match = /^(?:(?:https?|ssh|git):\/\/(?:git@)?github\.com\/|git@github\.com:)?(?[^/:\s]+)\/(?[^/\s]+?)(?:\.git)?\/?$/i.exec(repository); @@ -269,8 +270,8 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { private readonly _status = observableValue(this, SessionStatus.Untitled); readonly status: IObservable = this._status; - private readonly _permissionLevel = observableValue(this, ChatPermissionLevel.Default); - readonly permissionLevel: IObservable = this._permissionLevel; + private readonly _permissionLevelPreference = observableValue(this, ChatPermissionLevel.Default); + readonly permissionLevel: IObservable; private readonly _workspaceData = observableValue(this, undefined); readonly workspace: IObservable = this._workspaceData; @@ -337,6 +338,7 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { readonly selectedOptions = new Map(); get selectedModelId(): string | undefined { return this._modelId; } + get permissionLevelPreference(): string { return this._permissionLevelPreference.get(); } get chatMode(): IChatMode | undefined { return this._mode; } get query(): string | undefined { return this._query; } get attachedContext(): IChatRequestVariableEntry[] | undefined { return this._attachedContext; } @@ -367,6 +369,11 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { ) { super(); this.modelConfiguration = this._register(new AutomationModelConfiguration(languageModelsService, initialAutomationSessionConfiguration?.sessionTemplate)); + const policyRestricted = observableFromEvent(this, configurationService.onDidChangeConfiguration, () => configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue === false); + this.permissionLevel = derived(this, reader => { + const preference = this._permissionLevelPreference.read(reader); + return !policyRestricted.read(reader) && isChatPermissionLevel(preference) ? preference : ChatPermissionLevel.Default; + }); this.sessionId = toSessionId(providerId, resource); this.providerId = providerId; this.sessionType = AgentSessionProviders.Background; @@ -533,8 +540,8 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { this._modeObservable.set({ id: modeId, kind: modeKind }, undefined); } - setPermissionLevel(level: ChatPermissionLevel): void { - this._permissionLevel.set(level, undefined); + setPermissionLevel(level: string): void { + this._permissionLevelPreference.set(level, undefined); } setTitle(title: string): void { @@ -672,8 +679,8 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession readonly gitHubInfo: IObservable = constObservable(undefined); readonly branch: IObservable = constObservable(undefined); readonly isolationMode: IObservable = constObservable(undefined); - private readonly _useSandbox = observableValue(this, false); - readonly useSandbox: IObservable = this._useSandbox; + private readonly _useSandbox = observableValue(this, false); + readonly useSandbox: IObservable = this._useSandbox; readonly branches: IObservable = constObservable([]); readonly gitRepository?: IGitRepository | undefined; @@ -735,11 +742,13 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession this.sessionType = target; this.icon = CopilotCloudSessionType.icon; this.createdAt = new Date(); - this._useSandbox.set(storageService.getBoolean(STORAGE_KEY_USE_SANDBOX, StorageScope.PROFILE, false), undefined); + const useSandbox = initialAutomationSessionConfiguration?.sessionTemplate?.config?.[CLOUD_SANDBOX_CONFIG_KEY]; + this._useSandbox.set(typeof useSandbox === 'boolean' ? useSandbox : storageService.getBoolean(STORAGE_KEY_USE_SANDBOX, StorageScope.PROFILE, false), undefined); this._updateWhenClauseKeys(); this._register(this.chatSessionsService.onDidChangeOptionGroups(() => { this._updateWhenClauseKeys(); + this._updateModelOption(); this._onDidChangeOptionGroups.fire(); })); this._register(this.contextKeyService.onDidChangeContext(e => { @@ -758,8 +767,8 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession this.mainChat = observableValue(this, buildChatFromSession(this)); } - setPermissionLevel(level: ChatPermissionLevel): void { - throw new Error('Method not implemented.'); + setPermissionLevel(_level: ChatPermissionLevel): void { + // Remote sessions do not support client-side permission selection. } // -- New session configuration methods -- @@ -782,6 +791,7 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession setModelId(modelId: string | undefined, source: ChatModelSource): void { this._modelId = modelId; + this._updateModelOption(); // One update, and both halves of it: a model and where it came from are only meaningful as // a pair, so naming a source for a model the observable never reports would leave the // picker and the conversation disagreeing. @@ -848,6 +858,14 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession // --- Internals --- + private _updateModelOption(): void { + const group = this._getOptionGroups()?.find(isModelOptionGroup); + const item = group?.items.find(item => item.id === this._modelId); + if (group && item) { + this.setOptionValue(group.id, item); + } + } + private _getOptionGroups(): IChatSessionProviderOptionGroup[] | undefined { return this.chatSessionsService.getOptionGroupsForSessionType(this.target); } @@ -1778,21 +1796,28 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions const modelConfiguration = session.modelConfiguration.captureModelConfiguration(modelId); const initialConfiguration = session.initialAutomationSessionConfiguration; const initialTemplate = initialConfiguration?.sessionTemplate; - const initialMode = initialConfiguration?.mode ?? initialTemplate?.config?.[SessionConfigKey.Mode]; - const mode = session instanceof CopilotCLISession - ? session.mode.get()?.id - : typeof initialMode === 'string' ? initialMode : undefined; - const initialPermissionLevel = initialConfiguration?.permissionLevel ?? initialTemplate?.config?.[SessionConfigKey.AutoApprove]; - const permissionLevel = session instanceof RemoteNewSession && isChatPermissionLevel(initialPermissionLevel) - ? initialPermissionLevel - : session.permissionLevel.get(); const config = { ...initialTemplate?.config }; - if (mode) { - config[SessionConfigKey.Mode] = mode; + if (session instanceof CopilotCLISession) { + const mode = session.mode.get()?.id; + if (mode) { + config[SessionConfigKey.Mode] = mode; + } else { + delete config[SessionConfigKey.Mode]; + } + config[SessionConfigKey.AutoApprove] = session.permissionLevelPreference; } else { - delete config[SessionConfigKey.Mode]; + if (config[SessionConfigKey.Mode] === undefined && initialConfiguration?.mode !== undefined) { + config[SessionConfigKey.Mode] = initialConfiguration.mode; + } + if (config[SessionConfigKey.AutoApprove] === undefined) { + config[SessionConfigKey.AutoApprove] = initialConfiguration?.permissionLevel ?? session.permissionLevel.get(); + } + config[CLOUD_SANDBOX_CONFIG_KEY] = session.useSandbox.get(); } - config[SessionConfigKey.AutoApprove] = permissionLevel; + const configuredMode = config[SessionConfigKey.Mode]; + const mode = typeof configuredMode === 'string' ? configuredMode : undefined; + const configuredPermissionLevel = config[SessionConfigKey.AutoApprove]; + const permissionLevel = typeof configuredPermissionLevel === 'string' ? configuredPermissionLevel : undefined; const agentUri = session.chatMode?.uri?.get(); const agent = agentUri ? { uri: agentUri.toString() } @@ -1812,16 +1837,8 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions throw new Error('CopilotChatSessionsProvider does not support quick chats'); } - /** - * Resolves the initial permission level for a brand-new session from - * `chat.permissions.default`, clamped to `Default` when enterprise policy - * disables global auto-approval. - */ + /** The initial permission preference for a brand-new session. */ private _defaultPermissionLevel(): ChatPermissionLevel { - const policyRestricted = this.configurationService.inspect(ChatConfiguration.GlobalAutoApprove).policyValue === false; - if (policyRestricted) { - return ChatPermissionLevel.Default; - } const level = this.configurationService.getValue(ChatConfiguration.DefaultPermissionLevel); return isChatPermissionLevel(level) ? level : ChatPermissionLevel.Default; } @@ -1853,7 +1870,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions } } const permissionLevel = template?.config?.[SessionConfigKey.AutoApprove] ?? configuration.permissionLevel; - if (!(session instanceof RemoteNewSession) && isChatPermissionLevel(permissionLevel)) { + if (session instanceof CopilotCLISession && typeof permissionLevel === 'string') { session.setPermissionLevel(permissionLevel); } } @@ -1957,15 +1974,6 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions } } newSession.setModelId(modelId, source); - // Cloud sessions additionally persist the selection as the value of - // the `models` option group so the extension host honours it. - if (newSession instanceof RemoteNewSession) { - const { modelOption } = newSession.getModelOptionsSnapshot(); - const item = modelOption?.group.items.find(i => i.id === modelId); - if (item) { - newSession.setOptionValue(modelOption!.group.id, item); - } - } return; } diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts index 4937305eec2e3d..d717a7f479a2e4 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts @@ -17,7 +17,7 @@ import { mock, upcastPartial } from '../../../../../../base/test/common/mock.js' import { autorun, constObservable, ISettableObservable, observableValue } from '../../../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { runWithFakedTimers } from '../../../../../../base/test/common/timeTravelScheduler.js'; -import { IConfigurationService, IConfigurationValue } from '../../../../../../platform/configuration/common/configuration.js'; +import { type IConfigurationChangeEvent, IConfigurationService, IConfigurationValue } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { ICommandService } from '../../../../../../platform/commands/common/commands.js'; import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; @@ -32,15 +32,15 @@ import { IAgentSession, IAgentSessionsModel } from '../../../../../../workbench/ import { IAgentSessionsService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsService.js'; import { AgentSessionProviders } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; import { IChatService, ChatSendResult, IChatSendRequestData, IChatSendRequestOptions } from '../../../../../../workbench/contrib/chat/common/chatService/chatService.js'; -import { ChatSessionStatus, IChatSessionProviderOptionGroup, IChatSessionsService, SessionType } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; +import { type ChatSessionOptionsMap, ChatSessionStatus, type IChatSession, IChatSessionProviderOptionGroup, IChatSessionsService, SessionType } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { IChatWidget, IChatWidgetService } from '../../../../../../workbench/contrib/chat/browser/chat.js'; import { ILanguageModelChatMetadata, ILanguageModelChatMetadataAndIdentifier, ILanguageModelsService } from '../../../../../../workbench/contrib/chat/common/languageModels.js'; import { ILanguageModelToolsService } from '../../../../../../workbench/contrib/chat/common/tools/languageModelToolsService.js'; -import { IChatResponseModel } from '../../../../../../workbench/contrib/chat/common/model/chatModel.js'; +import { type IChatModel, IChatResponseModel } from '../../../../../../workbench/contrib/chat/common/model/chatModel.js'; import { ChatMode, CustomChatMode, IChatMode, IChatModes, IChatModeService } from '../../../../../../workbench/contrib/chat/common/chatModes.js'; import { IChatAgentData } from '../../../../../../workbench/contrib/chat/common/participants/chatAgents.js'; import { IGitService } from '../../../../../../workbench/contrib/git/common/gitService.js'; -import { ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; +import { type IAutomationSessionConfiguration, ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; import { ChatModelSource, GITHUB_REMOTE_FILE_SCHEME, IChat, ISession, ISessionChangesSummary, ISessionFileChange, ISessionWorkspace, SESSION_WORKSPACE_GROUP_GITHUB, SESSION_WORKSPACE_GROUP_LOCAL, SessionStatus } from '../../../../../services/sessions/common/session.js'; import { CloudSandboxEnabledSettingId, type ICloudSandboxCreateSessionRequest } from '../../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; import { RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; @@ -418,11 +418,23 @@ class TestSandboxCopilotProvider extends CopilotChatSessionsProvider { } } +interface ICreateProviderForSendTestsOptions { + readonly onDidCommitSession?: Event<{ original: URI; committed: URI }>; + readonly configurationService?: TestConfigurationService; + readonly agentHostEnabled?: boolean; + readonly getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; + readonly notifications?: string[]; + readonly chatModeService?: IChatModeService; + readonly languageModelsService?: Partial; + readonly chatSessionsService?: Partial; + readonly acquireOrLoadSession?: IChatService['acquireOrLoadSession']; +} + function createProviderForSendTests( disposables: DisposableStore, model: MockAgentSessionsModel, sendRequest: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise, - opts?: { onDidCommitSession?: Event<{ original: URI; committed: URI }>; configurationService?: TestConfigurationService; agentHostEnabled?: boolean; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; notifications?: string[]; chatModeService?: IChatModeService; languageModelsService?: Partial }, + opts?: ICreateProviderForSendTestsOptions, ): TestSandboxCopilotProvider { const instantiationService = disposables.add(new TestInstantiationService()); @@ -451,9 +463,10 @@ function createProviderForSendTests( setSessionOption: () => true, getSessionOption: () => undefined, onDidChangeOptionGroups: Event.None, + ...opts?.chatSessionsService, }); instantiationService.stub(IChatService, { - acquireOrLoadSession: async () => undefined, + acquireOrLoadSession: opts?.acquireOrLoadSession ?? (async () => undefined), sendRequest: sendRequest, removeHistoryEntry: async (resource: URI) => { model.removeSession(resource); }, setChatSessionTitle: () => { }, @@ -2437,14 +2450,111 @@ suite('CopilotChatSessionsProvider', () => { assert.strictEqual(session?.permissionLevel.get(), ChatPermissionLevel.Autopilot); }); - test('clamps to Default when chat.tools.global.autoApprove policy is false', () => { + test('clamps the effective default without rewriting its permission preference', async () => { const configurationService = makeConfig({ defaultLevel: ChatPermissionLevel.Autopilot, policyRestricted: true }); const provider = createProviderForSendTests(disposables, model, () => new Promise(() => { }), { configurationService }); const sessionInfo = provider.createNewSession(workspace, CopilotCLISessionType.id); const session = provider.getSession(sessionInfo.sessionId); + const captured = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); - assert.strictEqual(session?.permissionLevel.get(), ChatPermissionLevel.Default); + assert.deepStrictEqual({ + effective: session?.permissionLevel.get(), + preference: captured?.sessionTemplate?.config?.autoApprove, + }, { + effective: ChatPermissionLevel.Default, + preference: ChatPermissionLevel.Autopilot, + }); + }); + + for (const permissionLevel of [ChatPermissionLevel.AutoApprove, ChatPermissionLevel.Autopilot]) { + for (const canonical of [true, false]) { + test(`clamps restored ${canonical ? 'canonical' : 'legacy'} ${permissionLevel} before sending without changing the saved preference`, async () => { + const configurationService = makeConfig({ policyRestricted: true }); + let sentPermissionLevel: ChatPermissionLevel | undefined; + const provider = createProviderForSendTests(disposables, model, async (_resource, _message, options) => { + sentPermissionLevel = options?.modeInfo?.permissionLevel; + return { kind: 'rejected', reason: 'Request recorded' }; + }, { configurationService }); + const sessionInfo = provider.createNewSession(workspace, CopilotCLISessionType.id, { + automationConfiguration: canonical + ? { sessionTemplate: { config: { autoApprove: permissionLevel } }, permissionLevel: ChatPermissionLevel.Default } + : { permissionLevel }, + }); + const effective = provider.getSession(sessionInfo.sessionId)?.permissionLevel.get(); + const captured = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + + await assert.rejects(provider.sendRequest(sessionInfo.sessionId, sessionInfo.mainChat.get().resource, { query: 'hello' }), /Request recorded/); + + assert.deepStrictEqual({ + effective, + sentPermissionLevel, + preference: captured?.sessionTemplate?.config?.autoApprove, + legacyPreference: captured?.permissionLevel, + }, { + effective: ChatPermissionLevel.Default, + sentPermissionLevel: ChatPermissionLevel.Default, + preference: permissionLevel, + legacyPreference: permissionLevel, + }); + }); + } + } + + test('updates effective approvals when policy changes while preserving intent until an explicit edit', async () => { + const policy = { policyRestricted: false }; + const configurationService = makeConfig(policy); + const provider = createProviderForSendTests(disposables, model, async () => ({ kind: 'rejected', reason: 'Unexpected send' }), { configurationService }); + const sessionInfo = provider.createNewSession(workspace, CopilotCLISessionType.id, { + automationConfiguration: { sessionTemplate: { config: { autoApprove: ChatPermissionLevel.Autopilot } } }, + }); + const session = provider.getSession(sessionInfo.sessionId)!; + const effective: ChatPermissionLevel[] = []; + disposables.add(autorun(reader => { effective.push(session.permissionLevel.read(reader)); })); + + const updatePolicy = (restricted: boolean) => { + policy.policyRestricted = restricted; + configurationService.onDidChangeConfigurationEmitter.fire(upcastPartial({ + affectsConfiguration: key => key === ChatConfiguration.GlobalAutoApprove, + })); + }; + updatePolicy(true); + const restricted = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + updatePolicy(false); + const unrestricted = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + provider.setPermissionLevel(sessionInfo.sessionId, ChatPermissionLevel.Default); + updatePolicy(true); + updatePolicy(false); + const edited = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + + assert.deepStrictEqual({ + effective, + preferences: [restricted, unrestricted, edited].map(configuration => configuration?.sessionTemplate?.config?.autoApprove), + }, { + effective: [ChatPermissionLevel.Autopilot, ChatPermissionLevel.Default, ChatPermissionLevel.Autopilot, ChatPermissionLevel.Default], + preferences: [ChatPermissionLevel.Autopilot, ChatPermissionLevel.Autopilot, ChatPermissionLevel.Default], + }); + }); + + test('preserves an unknown approval preference until the user selects a supported level', async () => { + const provider = createProviderForSendTests(disposables, model, async () => ({ kind: 'rejected', reason: 'Unexpected send' })); + const sessionInfo = provider.createNewSession(workspace, CopilotCLISessionType.id, { + automationConfiguration: { sessionTemplate: { config: { autoApprove: 'future-approvals', providerOption: true } } }, + }); + const initialEffective = provider.getSession(sessionInfo.sessionId)?.permissionLevel.get(); + const initial = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + provider.setPermissionLevel(sessionInfo.sessionId, ChatPermissionLevel.AutoApprove); + const edited = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + + assert.deepStrictEqual({ + initialEffective, + initialConfig: initial?.sessionTemplate?.config, + editedConfig: edited?.sessionTemplate?.config, + }, { + initialEffective: ChatPermissionLevel.Default, + initialConfig: { autoApprove: 'future-approvals', providerOption: true }, + editedConfig: { autoApprove: ChatPermissionLevel.AutoApprove, providerOption: true }, + }); }); test('falls back to Default when chat.permissions.default is unset', () => { @@ -2550,6 +2660,54 @@ suite('CopilotChatSessionsProvider', () => { }); } + test('round trips native fallback model options without taking ordinary composer defaults', async () => { + const modelMetadata: ILanguageModelChatMetadata = { + extension: new ExtensionIdentifier('test'), + id: 'model', name: 'Model', vendor: 'copilot', family: 'test', version: '1', + maxInputTokens: 1, maxOutputTokens: 1, isDefaultForLocation: {}, + targetChatSessionType: CopilotCLISessionType.id, + configurationSchema: { + type: 'object', + properties: { thinkingLevel: { type: 'string', enum: ['low', 'medium', 'high'], default: 'medium' } }, + }, + }; + let sentOptions: IChatSendRequestOptions | undefined; + const writes: Record[] = []; + const provider = createProviderForSendTests(disposables, model, async (_resource, _message, options) => { + sentOptions = options; + return { kind: 'rejected', reason: 'Request recorded' }; + }, { + languageModelsService: { + getLanguageModelIds: () => ['copilot/model'], + lookupLanguageModel: identifier => identifier === 'copilot/model' ? modelMetadata : undefined, + hasResolvedVendor: () => true, + getModelConfiguration: () => ({ thinkingLevel: 'high' }), + setModelConfiguration: async (_modelId, values) => { writes.push(values); }, + }, + }); + const modelConfiguration = { thinkingLevel: 'low', futureOption: true }; + const original = provider.createNewSession(workspace, CopilotCLISessionType.id, { + automationConfiguration: { sessionTemplate: { modelId: 'copilot/model', modelConfiguration } }, + }); + const captured = await provider.getAutomationSessionConfiguration(original.sessionId); + const session = provider.createNewSession(workspace, CopilotCLISessionType.id, { automationConfiguration: captured }); + const recaptured = await provider.getAutomationSessionConfiguration(session.sessionId); + await assert.rejects(provider.sendRequest(session.sessionId, session.mainChat.get().resource, { query: 'hello' }), /Request recorded/); + + assert.deepStrictEqual({ + captured: [captured, recaptured].map(configuration => ({ + modelId: configuration?.sessionTemplate?.modelId, + modelConfiguration: configuration?.sessionTemplate?.modelConfiguration, + })), + sent: { modelId: sentOptions?.userSelectedModelId, modelConfiguration: sentOptions?.userSelectedModelConfiguration }, + writes, + }, { + captured: [{ modelId: 'copilot/model', modelConfiguration }, { modelId: 'copilot/model', modelConfiguration }], + sent: { modelId: 'copilot/model', modelConfiguration: { thinkingLevel: 'low' } }, + writes: [], + }); + }); + test('rejects Automation model configuration without a model before creating a fallback draft', () => { const provider = createProviderForSendTests(disposables, model, async () => ({ kind: 'rejected', reason: 'Unexpected send' })); assert.throws(() => provider.createNewSession(workspace, CopilotCLISessionType.id, { @@ -2582,6 +2740,168 @@ suite('CopilotChatSessionsProvider', () => { }); }); + suite('Automation cloud session configuration', () => { + const workspace = URI.from({ scheme: GITHUB_REMOTE_FILE_SCHEME, path: '/owner/repo/HEAD' }); + + for (const selection of ['canonical', 'legacy', 'explicit'] as const) { + for (const delayed of [false, true]) { + test(`applies the ${selection} model option with ${delayed ? 'late' : 'ready'} Cloud options before sending`, async () => { + const defaultModel = { id: 'default-model', name: 'Default Model', default: true }; + const selectedModel = { id: 'selected-model', name: 'Selected Model' }; + const modelGroup: IChatSessionProviderOptionGroup = { id: 'models', name: 'Models', items: [defaultModel, selectedModel] }; + let optionGroups = delayed ? undefined : [modelGroup]; + const optionsChanged = disposables.add(new Emitter()); + const sessionOptions: ChatSessionOptionsMap = new Map(); + const sentOptionMaps: ChatSessionOptionsMap[] = []; + let sentModelId: string | undefined; + const provider = createProviderForSendTests(disposables, model, async (_resource, _message, options) => { + sentModelId = options?.userSelectedModelId; + sentOptionMaps.push(new Map(sessionOptions)); + return { kind: 'rejected', reason: 'Request recorded' }; + }, { + getOptionGroups: () => optionGroups, + chatSessionsService: { + onDidChangeOptionGroups: optionsChanged.event, + setSessionOption: (_resource, optionId, value) => { + sessionOptions.set(optionId, value); + return true; + }, + getSessionOption: (_resource, optionId) => sessionOptions.get(optionId), + getOrCreateChatSession: async resource => { + sessionOptions.set('models', defaultModel); + if (delayed) { + optionGroups = [modelGroup]; + optionsChanged.fire(AgentSessionProviders.Cloud); + } + return upcastPartial({ sessionResource: resource }); + }, + updateSessionOptions: (_resource, updates) => { + for (const [key, value] of updates) { + sessionOptions.set(key, value); + } + return true; + }, + }, + acquireOrLoadSession: async () => new ImmortalReference(upcastPartial({ + inputModel: upcastPartial({ setState: () => { } }), + })), + }); + const automationConfiguration: IAutomationSessionConfiguration = selection === 'canonical' + ? { sessionTemplate: { modelId: selectedModel.id }, modelId: defaultModel.id } + : selection === 'legacy' ? { modelId: selectedModel.id } : {}; + const session = provider.createNewSession(workspace, CopilotCloudSessionType.id, { automationConfiguration }); + if (selection === 'explicit') { + provider.setModel(session.sessionId, session.mainChat.get().resource, selectedModel.id, ChatModelSource.Chosen); + } + const initialOption = sessionOptions.get('models'); + const captured = await provider.getAutomationSessionConfiguration(session.sessionId); + const chat = await provider.createNewChat(session.sessionId); + const preparedOption = sessionOptions.get('models'); + await assert.rejects(provider.sendRequest(session.sessionId, chat.resource, { query: 'hello' }), /Request recorded/); + + assert.deepStrictEqual({ + initialOption, + preparedOption, + capturedModel: captured?.sessionTemplate?.modelId, + sentModelId, + sentModelOptions: sentOptionMaps.map(options => options.get('models')), + }, { + initialOption: delayed ? undefined : selectedModel, + preparedOption: selectedModel, + capturedModel: selectedModel.id, + sentModelId: selectedModel.id, + sentModelOptions: [selectedModel], + }); + }); + } + } + + for (const config of [ + { mode: ChatModeKind.Ask, autoApprove: ChatPermissionLevel.Autopilot }, + { mode: 'future-mode', autoApprove: 'future-approvals' }, + ]) { + test(`preserves canonical Cloud ${config.mode} preferences over legacy aliases`, async () => { + const provider = createProviderForSendTests(disposables, model, async () => ({ kind: 'rejected', reason: 'Unexpected send' })); + const providerOption = { future: ['value'], unset: null }; + const sessionInfo = provider.createNewSession(workspace, CopilotCloudSessionType.id, { + automationConfiguration: { + sessionTemplate: { config: { ...config, providerOption } }, + mode: ChatModeKind.Agent, + permissionLevel: ChatPermissionLevel.Default, + }, + }); + const session = provider.getSession(sessionInfo.sessionId)!; + const captured = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + + assert.deepStrictEqual({ + mode: session.mode.get(), + permissionLevel: session.permissionLevel.get(), + captured, + }, { + mode: undefined, + permissionLevel: ChatPermissionLevel.Default, + captured: { + sessionTemplate: { config: { ...config, providerOption, useSandbox: false } }, + modelId: undefined, + mode: config.mode, + permissionLevel: config.autoApprove, + }, + }); + }); + } + + for (const useSandbox of [false, true]) { + test(`keeps legacy Cloud Sandbox=${useSandbox} behavior until its first template capture`, async () => { + let sentPermissionLevel: ChatPermissionLevel | undefined; + const provider = createProviderForSendTests(disposables, model, async (_resource, _message, options) => { + sentPermissionLevel = options?.modeInfo?.permissionLevel; + return { kind: 'rejected', reason: 'Request recorded' }; + }); + const ordinary = provider.createNewSession(workspace, CopilotCloudSessionType.id); + provider.getSession(ordinary.sessionId)!.setUseSandbox(useSandbox); + const sessionInfo = provider.createNewSession(workspace, CopilotCloudSessionType.id, { + automationConfiguration: { mode: ChatModeKind.Ask, permissionLevel: ChatPermissionLevel.AutoApprove }, + }); + provider.setMode(sessionInfo.sessionId, ChatModeKind.Ask); + provider.setPermissionLevel(sessionInfo.sessionId, ChatPermissionLevel.AutoApprove); + const restoredUseSandbox = provider.getSession(sessionInfo.sessionId)?.useSandbox.get(); + const captured = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + await assert.rejects(provider.sendRequest(sessionInfo.sessionId, sessionInfo.mainChat.get().resource, { query: 'hello' }), /Request recorded/); + + assert.deepStrictEqual({ + useSandbox: restoredUseSandbox, + sentPermissionLevel, + config: captured?.sessionTemplate?.config, + }, { + useSandbox, + sentPermissionLevel: ChatPermissionLevel.Default, + config: { mode: ChatModeKind.Ask, autoApprove: ChatPermissionLevel.AutoApprove, useSandbox }, + }); + }); + } + + test('does not enable unsupported Cloud worktree or branch configuration', async () => { + const provider = createProviderForSendTests(disposables, model, async () => ({ kind: 'rejected', reason: 'Unexpected send' })); + const sessionInfo = provider.createNewSession(workspace, CopilotCloudSessionType.id, { automationConfiguration: {} }); + await provider.setIsolationMode(sessionInfo.sessionId, 'worktree'); + await provider.setBranch(sessionInfo.sessionId, 'feature/saved'); + const session = provider.getSession(sessionInfo.sessionId)!; + const captured = await provider.getAutomationSessionConfiguration(sessionInfo.sessionId); + + assert.deepStrictEqual({ + supportsWorktree: CopilotCloudSessionType.supportsWorktreeConfiguration ?? false, + isolationMode: session.isolationMode.get(), + branch: session.branch.get(), + config: captured?.sessionTemplate?.config, + }, { + supportsWorktree: false, + isolationMode: undefined, + branch: undefined, + config: { autoApprove: ChatPermissionLevel.Default, useSandbox: false }, + }); + }); + }); + suite('Automation custom agent restoration', () => { const workspace = URI.file('/test/repo'); @@ -2655,10 +2975,17 @@ suite('CopilotChatSessionsProvider', () => { const discoveryStarted = new DeferredPromise(); let modes: readonly IChatMode[] = []; let sentOptions: IChatSendRequestOptions | undefined; + const sessionOptions: ChatSessionOptionsMap = new Map(); const provider = createProviderForSendTests(disposables, model, async (_resource, _message, options) => { sentOptions = options; return { kind: 'rejected', reason: 'Request recorded' }; }, { + chatSessionsService: { + setSessionOption: (_resource, optionId, value) => { + sessionOptions.set(optionId, value); + return true; + }, + }, chatModeService: createModeService(() => modes, async () => { await discoveryStarted.complete(); await ready.p; @@ -2681,11 +3008,13 @@ suite('CopilotChatSessionsProvider', () => { sentBeforeDiscovery, instructions: sentOptions?.modeInfo?.modeInstructions?.content, agent: sentOptions?.modeInfo?.modeInstructions?.name, + nativeAgentOption: sessionOptions.get('agent'), isBuiltin: sentOptions?.modeInfo?.isBuiltin, }, { sentBeforeDiscovery: false, instructions: 'Instructions for reviewer', agent: 'reviewer', + nativeAgentOption: 'reviewer', isBuiltin: false, }); }); @@ -3006,7 +3335,7 @@ suite('CopilotChatSessionsProvider', () => { // `repoNwo` has to strip back down to `owner/repo`. const repoWorkspace = URI.from({ scheme: GITHUB_REMOTE_FILE_SCHEME, path: '/osortega/simple-server/HEAD' }); - function createSandboxProvider(opts: { enabled?: boolean; provision?: () => Promise; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined } = {}) { + function createSandboxProvider(opts: { enabled?: boolean; provision?: () => Promise; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; cloudSendResult?: ChatSendResult } = {}) { const configurationService = new TestConfigurationService(); configurationService.setUserConfiguration(CloudSandboxEnabledSettingId, opts.enabled ?? true); configurationService.setUserConfiguration(RemoteAgentHostsEnabledSettingId, true); @@ -3015,8 +3344,8 @@ suite('CopilotChatSessionsProvider', () => { const notifications: string[] = []; const provider = createProviderForSendTests(disposables, model, async (_resource, message) => { cloudSends.push(message); - // Never settles: these tests only assert which path the send took. - return new Promise(() => { }); + // Leave routing-only requests pending unless the test provides a result. + return opts.cloudSendResult ?? new Promise(() => { }); }, { configurationService, getOptionGroups: opts.getOptionGroups, notifications }); const provisionRequests: ICloudSandboxCreateSessionRequest[] = []; @@ -3103,6 +3432,52 @@ suite('CopilotChatSessionsProvider', () => { }]; } + for (const useSandbox of [false, true]) { + for (const enabled of [false, true]) { + test(`restores Automation Sandbox=${useSandbox} independently of the composer with the feature ${enabled ? 'enabled' : 'disabled'}`, async () => { + const provisioned = provisionedSession(); + const { provider, provisionRequests, cloudSends } = createSandboxProvider({ + enabled, + provision: async () => provisioned, + cloudSendResult: { kind: 'rejected', reason: 'Cloud request recorded' }, + }); + const original = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id, { + automationConfiguration: { sessionTemplate: { config: { futureCloudOption: true } } }, + }); + provider.getSession(original.sessionId)!.setUseSandbox(useSandbox); + const saved = await provider.getAutomationSessionConfiguration(original.sessionId); + provider.deleteNewSession(original.sessionId); + const ordinary = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id); + provider.getSession(ordinary.sessionId)!.setUseSandbox(!useSandbox); + + const restored = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id, { automationConfiguration: saved }); + const restoredUseSandbox = provider.getSession(restored.sessionId)?.useSandbox.get(); + const recaptured = await provider.getAutomationSessionConfiguration(restored.sessionId); + const laterOrdinary = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id); + const send = provider.sendRequest(restored.sessionId, restored.mainChat.get().resource, { query: 'fix it' }); + if (useSandbox && enabled) { + await send; + } else { + await assert.rejects(send, /Cloud request recorded/); + } + + assert.deepStrictEqual({ + restoredUseSandbox, + recapturedConfig: recaptured?.sessionTemplate?.config, + ordinaryUseSandbox: provider.getSession(laterOrdinary.sessionId)?.useSandbox.get(), + provisionRequests, + cloudSends, + }, { + restoredUseSandbox: useSandbox, + recapturedConfig: { futureCloudOption: true, autoApprove: ChatPermissionLevel.Default, useSandbox }, + ordinaryUseSandbox: !useSandbox, + provisionRequests: useSandbox && enabled ? [{ repoNwo: 'osortega/simple-server', prompt: 'fix it' }] : [], + cloudSends: useSandbox && enabled ? [] : ['fix it'], + }); + }); + } + } + test('carries the composer model into the sandbox before the first turn', async () => { // Mission Control starts no run, so a session that has never run has no model to // restore: without this the first turn would silently take the agent host default. diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/sandboxPicker.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/sandboxPicker.test.ts index 64e2f8059ea7e6..dc29cf781b7c14 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/sandboxPicker.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/sandboxPicker.test.ts @@ -9,6 +9,7 @@ import { constObservable, observableValue } from '../../../../../../base/common/ import { URI } from '../../../../../../base/common/uri.js'; import { mock, upcastPartial } from '../../../../../../base/test/common/mock.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { isIMenuItem, MenuRegistry } from '../../../../../../platform/actions/common/actions.js'; import { CloudSandboxEnabledSettingId } from '../../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; import { RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -20,6 +21,8 @@ import { InMemoryStorageService, IStorageService } from '../../../../../../platf import { ITelemetryService } from '../../../../../../platform/telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../../../../platform/telemetry/common/telemetryUtils.js'; import { AgentSessionProviders } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; +import { ChatContextKeys } from '../../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; +import { Menus } from '../../../../../browser/menus.js'; import { IChatSessionsService } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ISessionsProvider } from '../../../../../services/sessions/common/sessionsProvider.js'; import { IActiveSession } from '../../../../../services/sessions/common/sessionsManagement.js'; @@ -27,6 +30,7 @@ import { GITHUB_REMOTE_FILE_SCHEME, ISessionFolder, ISessionWorkspace } from '.. import { ISessionsProvidersService } from '../../../../../services/sessions/browser/sessionsProvidersService.js'; import { CopilotChatSessionsProvider, ICopilotChatSession, RemoteNewSession } from '../../browser/copilotChatSessionsProvider.js'; import { SandboxPicker } from '../../browser/sandboxPicker.js'; +import '../../browser/copilotChatSessionsActions.js'; class TestSessionsProvidersService extends mock() { override readonly onDidChangeProviders = Event.None; @@ -43,6 +47,17 @@ class TestSessionsProvidersService extends mock() { suite('Copilot SandboxPicker', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + test('offers Sandbox in automation controls while respecting AI visibility', () => { + const item = MenuRegistry.getMenuItems(Menus.NewSessionControl) + .filter(isIMenuItem) + .find(item => item.command.id === 'sessions.defaultCopilot.sandboxPicker'); + + assert.deepStrictEqual({ + automationScoped: item?.when?.keys().includes(ChatContextKeys.inAutomationsDialog.key), + aiScoped: item?.when?.keys().includes(ChatContextKeys.enabled.key), + }, { automationScoped: true, aiScoped: true }); + }); + function createPicker(options: { settingEnabled?: boolean; remoteHostsEnabled?: boolean; hasRepository?: boolean; useSandbox?: boolean; committedSession?: boolean } = {}) { const configurationService = new TestConfigurationService(); configurationService.setUserConfiguration(CloudSandboxEnabledSettingId, options.settingEnabled ?? true);