From e4bf46675c893125c0c413ac2be65defccf00afd Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Fri, 11 Sep 2026 01:12:08 +0200 Subject: [PATCH 1/8] automations: fix: revert broad provider configuration changes Revert 1328607eb90d69b7a66fb2e8d4fdca49dcc0f9d3 (#335018) so the worktree availability fix and unrelated provider configuration changes can be reviewed separately. Preserve subsequent main changes and the independent ActionList PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../node/agentHostAutomationService.ts | 10 +- .../platform/agentHost/node/agentService.ts | 23 +- .../agentHost/node/codex/codexAgent.ts | 51 +-- .../node/agentHostAutomationService.test.ts | 26 +- .../agentHost/test/node/agentService.test.ts | 131 +++--- .../node/codex/codexPrewarmEviction.test.ts | 349 +++------------ src/vs/sessions/SESSIONS.md | 2 - .../automations/browser/automationDialog.ts | 80 +--- .../automations/browser/automationTools.ts | 29 +- .../test/browser/automationDialog.test.ts | 271 +----------- .../test/browser/automationTools.test.ts | 236 +---------- .../contrib/chat/browser/branchPicker.ts | 38 +- .../browser/agentHostSessionConfigPicker.ts | 26 +- .../browser/baseAgentHostSessionsProvider.ts | 45 +- .../agentHostSessionConfigPicker.test.ts | 75 +--- .../localAgentHostSessionsProvider.test.ts | 41 -- .../browser/copilotChatSessionsActions.ts | 23 +- .../browser/copilotChatSessionsProvider.ts | 90 ++-- .../copilotChatSessionsProvider.test.ts | 397 +----------------- .../test/browser/sandboxPicker.test.ts | 15 - .../remoteAgentHostSessionsProvider.test.ts | 57 +-- .../sessions/common/sessionsProvider.ts | 12 - 22 files changed, 278 insertions(+), 1749 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostAutomationService.ts b/src/vs/platform/agentHost/node/agentHostAutomationService.ts index 70d3807c5efb7f..9365df4d904a6d 100644 --- a/src/vs/platform/agentHost/node/agentHostAutomationService.ts +++ b/src/vs/platform/agentHost/node/agentHostAutomationService.ts @@ -754,12 +754,10 @@ export class AgentHostAutomationService extends Disposable implements IAgentHost await this._execution.cancelSession(session); return; } - // 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 } : {}), - }; + // 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; await this._execution.startSession(session, message); } catch (error) { try { diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 35d60a2329e871..bb4125004ff11f 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -2926,7 +2926,10 @@ export class AgentService extends Disposable implements IAgentService { config = { ...config, importConversation: { ...config.importConversation, turns: importedTurns } }; } - // Providers must see pending isolation before creation can schedule eager prewarming. + // Resolve host-owned isolation before provider creation. Providers such as + // Codex may schedule eager prewarming from createSession; marking a + // client-chosen worktree session pending first prevents that prewarm from + // materializing in the picked folder before the host creates the worktree. const initializeSideEffects = this._sideEffects.initialize(); const sessionConfig = await this._resolveCreatedSessionConfig(provider, config); const deferWorktreeCreation = sessionConfig?.values?.[SessionConfigKey.Isolation] === 'worktree' && !config?.importConversation; @@ -3442,15 +3445,15 @@ export class AgentService extends Disposable implements IAgentService { } private async _createProviderSession(provider: IAgent, config: IAgentCreateSessionConfig | undefined, deferWorktreeCreation: boolean): Promise { - const session = config?.session ?? this._mintSessionUri(provider); - const pendingSessionId = deferWorktreeCreation ? AgentSession.id(session) : undefined; - if (pendingSessionId) { - this._worktree.notePending(pendingSessionId); + const requestedSessionId = deferWorktreeCreation && config?.session ? AgentSession.id(config.session) : undefined; + if (requestedSessionId) { + this._worktree.notePending(requestedSessionId); } let created: IAgentCreateSessionResult | undefined; try { const providerConfig = config ? this._toProviderConfig(config) : undefined; + const session = config?.session ?? this._mintSessionUri(provider); const defaultChatUri = URI.parse(buildDefaultChatUri(session)); const boundConfig: IAgentCreateSessionConfig = { ...(providerConfig ?? {}), session }; const result = await provider.chats.createChat(defaultChatUri, this._chatContext(session, defaultChatUri), this._toCreateChatOptions(boundConfig)); @@ -3461,6 +3464,9 @@ export class AgentService extends Disposable implements IAgentService { ...(result?.provisional ? { provisional: true } : {}), ...(result ? { chat: result } : {}), }; + if (deferWorktreeCreation && created.provisional) { + this._worktree.notePending(AgentSession.id(created.session)); + } await this._persistDefaultChatBacking(created); return created; } catch (err) { @@ -3469,8 +3475,9 @@ export class AgentService extends Disposable implements IAgentService { } throw err; } finally { - if (pendingSessionId && !created?.provisional) { - this._worktree.clearPending(pendingSessionId); + const returnedPendingSessionId = created?.provisional ? AgentSession.id(created.session) : undefined; + if (requestedSessionId && requestedSessionId !== returnedPendingSessionId) { + this._worktree.clearPending(requestedSessionId); } } } @@ -3486,8 +3493,6 @@ export class AgentService extends Disposable implements IAgentService { await provider.chats.disposeChat(defaultChatUri, this._chatContext(session, defaultChatUri)); } catch (disposeError) { this._logService.error(disposeError, `[AgentService] Failed to roll back default chat of provider session ${session.toString()}`); - } finally { - this._worktree.clearPending(AgentSession.id(session)); } } diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 5192d66e85a3db..e04651a38fe3a9 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -69,11 +69,10 @@ 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 } from '../shared/worktreeIsolation.js'; +import { IAgentHostWorktreeIsolation, type IAgentHostWorktreePendingState } from '../shared/worktreeIsolation.js'; import { getServerToolDisplay } from '../shared/serverToolGroups.js'; import { IAgentSdkDownloader, IAgentSdkPackage } from '../agentSdkDownloader.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; @@ -1253,6 +1252,7 @@ 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 private readonly _worktree: IAgentHostWorktreeIsolation, + @IAgentHostWorktreeIsolation 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,50 +1927,13 @@ 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, selectedAgent] = await Promise.all([ + const [workspaceAgents, workspaceSkills] = 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, selectedAgent, this._fileService); + const customization = await codexCustomizationConfig(workspaceAgents.agents, plugins, session.agent, this._fileService); const developerInstructions = [ customization.developerInstructions, session.managedWorkingDirectory ? AGENT_HOST_WORKSPACELESS_INSTRUCTIONS : '', @@ -2002,7 +1965,7 @@ export class CodexAgent extends Disposable implements IAgent { })), ]; const signature = JSON.stringify({ - agent: selectedAgent?.uri, + agent: session.agent?.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 e78dad5f1ebe97..16b499309de718 100644 --- a/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostAutomationService.test.ts @@ -725,16 +725,13 @@ suite('AgentHostAutomationService', () => { }))); }); - for (const messageOverride of ['none', 'model', 'agent', 'both']) { - test(`records the Automation session selections on its first turn with ${messageOverride} message overrides`, async () => { + for (const hasMessageModel of [false, true]) { + test(hasMessageModel ? 'preserves an explicit Automation message model' : 'records the Automation model configuration on its first turn', async () => { const session = URI.parse('mock:/model-configuration-run'); const model = { id: 'mock-model', config: { thinkingLevel: 'low', contextSize: 272_000 } }; - 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 messageModel = hasMessageModel ? { id: 'other-model', config: { thinkingLevel: 'high' } } : 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(); @@ -743,7 +740,6 @@ suite('AgentHostAutomationService', () => { const service = createService({ createSession: async template => { createdModel = template.model; - createdAgent = template.agent; stateManager.createSession({ resource: session.toString(), provider: 'mock', @@ -771,13 +767,9 @@ 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({ @@ -789,24 +781,12 @@ 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/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index ddca28beb62c47..9261bdfac0ace8 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -1184,81 +1184,70 @@ suite('AgentService (node dispatcher)', () => { }); }); - for (const sessionIdSource of ['client', 'host']) { - test(`marks worktree isolation pending before a provisional provider can prewarm with a ${sessionIdSource} session ID`, async () => { - const workingDirectory = URI.file('/workspace/repo'); - const gitService = createNoopGitService(); - gitService.getRepositoryRoot = async () => workingDirectory; - gitService.revParse = async () => 'head'; - gitService.getCurrentBranch = async () => 'feature'; - gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'main' }); - let failProviderDataWrite = false; - class FailingProviderDataDatabase extends TestSessionDatabase { - override async setMetadata(key: string, value: string): Promise { - if (failProviderDataWrite && key === 'defaultChatProviderData') { - throw new Error('provider data write failed'); + test('marks worktree isolation pending before a provisional provider can prewarm', async () => { + const session = AgentSession.uri('codex', 'pending-before-create'); + const workingDirectory = URI.file('/workspace/repo'); + const gitService = createNoopGitService(); + gitService.getRepositoryRoot = async () => workingDirectory; + gitService.revParse = async () => 'head'; + gitService.getCurrentBranch = async () => 'feature'; + gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'main' }); + const localService = disposables.add(createTestAgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const isolation = disposables.add(new WorktreeIsolation( + { _serviceBrand: undefined, generateBranchName: async () => 'agents/test' }, + gitService, + nullSessionDataService, + new NullLogService(), + )); + setTestAgentHostWorktreeIsolation(localService, isolation); + const pendingDuringCreate: boolean[] = []; + const providerCreateConfigs: Array | undefined> = []; + let failCreate = false; + class PrewarmingAgent extends MockAgent { + override readonly chats: IAgentChats = withChatOverrides(getChatSurface(this), base => ({ + createChat: async (chat, context, options) => { + const { configurationResource } = resolveAgentChatContext(context, chat); + pendingDuringCreate.push(isWorkingDirectoryPending(localService, configurationResource.toString())); + providerCreateConfigs.push(options?.config); + if (failCreate) { + throw new Error('create failed'); } - return super.setMetadata(key, value); - } - } - const sessionDataService = createSessionDataService(new FailingProviderDataDatabase()); - const localService = disposables.add(createTestAgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); - const isolation = disposables.add(new WorktreeIsolation( - { _serviceBrand: undefined, generateBranchName: async () => 'agents/test' }, - gitService, - sessionDataService, - new NullLogService(), - )); - setTestAgentHostWorktreeIsolation(localService, isolation); - const creatingSessions: URI[] = []; - const pendingDuringCreate: boolean[] = []; - const providerCreateConfigs: Array | undefined> = []; - let failCreate = false; - class PrewarmingAgent extends MockAgent { - override readonly chats: IAgentChats = withChatOverrides(getChatSurface(this), base => ({ - createChat: async (chat, context, options) => { - const { configurationResource } = resolveAgentChatContext(context, chat); - creatingSessions.push(configurationResource); - pendingDuringCreate.push(isWorkingDirectoryPending(localService, configurationResource)); - providerCreateConfigs.push(options?.config); - if (failCreate) { - throw new Error('create failed'); - } - return { ...await expectCreatedChat(base.createChat(chat, context, options)), provisional: true, providerData: 'blob' }; - }, - })); - } - const agent = disposables.add(new PrewarmingAgent('codex')); - registerTestAgentProvider(localService, agent); + return { ...await expectCreatedChat(base.createChat(chat, context, options)), provisional: true }; + }, + })); + } + const agent = new PrewarmingAgent('codex'); + disposables.add(toDisposable(() => agent.dispose())); + registerTestAgentProvider(localService, agent); - const createSession = (id: string) => localService.createSession({ - provider: 'codex', - session: sessionIdSource === 'client' ? AgentSession.uri('codex', id) : undefined, - workingDirectories: [workingDirectory], - config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, - }); - const session = await createSession('pending-before-create'); - failCreate = true; - await assert.rejects(createSession('failed-before-create'), /create failed/); - failCreate = false; - failProviderDataWrite = true; - await assert.rejects(createSession('failed-after-create'), /provider data write failed/); + await localService.createSession({ + provider: 'codex', + session, + workingDirectories: workingDirectory ? [workingDirectory] : undefined, + config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, + }); - assert.deepStrictEqual({ - providerSession: creatingSessions[0], - pendingDuringCreate, - providerCreateConfigs, - pendingAfterAttempts: creatingSessions.map(session => isWorkingDirectoryPending(localService, session)), - rolledBackSessions: agent.disposeSessionCalls, - }, { - providerSession: session, - pendingDuringCreate: [true, true, true], - providerCreateConfigs: [{}, {}, {}], - pendingAfterAttempts: [true, false, false], - rolledBackSessions: [creatingSessions[2]], - }); + const failedSession = AgentSession.uri('codex', 'failed-before-create'); + failCreate = true; + await assert.rejects(localService.createSession({ + provider: 'codex', + session: failedSession, + workingDirectories: workingDirectory ? [workingDirectory] : undefined, + config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, + }), /create failed/); + + assert.deepStrictEqual({ + pendingDuringCreate, + providerCreateConfigs, + pendingAfterCreate: isWorkingDirectoryPending(localService, session.toString()), + pendingAfterFailure: isWorkingDirectoryPending(localService, failedSession.toString()), + }, { + pendingDuringCreate: [true, true], + providerCreateConfigs: [{}, {}], + pendingAfterCreate: true, + pendingAfterFailure: false, }); - } + }); test('createSession validates, exposes, and persists multi-root metadata', async () => { const db = new TestSessionDatabase(); 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 ba89954822e1fa..18e8157ecb58a0 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexPrewarmEviction.test.ts @@ -12,7 +12,6 @@ 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'; @@ -38,7 +37,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, type IWorktreeMetadata } from '../../../node/shared/worktreeIsolation.js'; +import { IAgentHostWorktreeIsolation, NullAgentHostWorktreeIsolation } 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'; @@ -46,7 +45,6 @@ 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'; @@ -170,8 +168,6 @@ interface ICreateAgentOptions { readonly database?: TestSessionDatabase; readonly checkpointService?: IAgentHostCheckpointService; readonly customizationEnablementService?: IAgentHostCustomizationEnablementService; - readonly worktreeIsolation?: IAgentHostWorktreeIsolation; - readonly gitService?: Partial; } class TestCodexLogService extends NullLogService { @@ -223,14 +219,6 @@ 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(); @@ -249,12 +237,7 @@ 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, options.worktreeIsolation ?? new NullAgentHostWorktreeIsolation()); - instantiationService.stub(IAgentHostGitService, { - getRepositoryRoot: async () => undefined, - getWorktreeRoots: async () => [], - ...options.gitService, - }); + instantiationService.stub(IAgentHostWorktreeIsolation, new NullAgentHostWorktreeIsolation()); instantiationService.stub(IAgentHostStateManager, stateManager); instantiationService.stub(IAgentHostCustomizationEnablementService, options.customizationEnablementService ?? createNoopCustomizationEnablementService()); instantiationService.stub(IAgentHostGitHubEndpointService, createTestGitHubEndpointService()); @@ -309,8 +292,7 @@ async function createSession(agent: CodexAgent, options: IAgentCreateChatOptions } async function assertPrewarmEvictedOnSend(disposables: Pick, completePrewarmBeforeSend: boolean): Promise { - const worktreeIsolation = new TestCodexWorktreeIsolation(); - const agent = await createAgent(disposables, { worktreeIsolation }); + const agent = await createAgent(disposables); const peer = disposables.add(createTestPeer()); const client = new CodexAppServerClient(peer.transport); agent['_connection'] = { @@ -324,12 +306,7 @@ async function assertPrewarmEvictedOnSend(disposables: Pick { peer.exit(); }); - 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; + 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; - 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 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() }, }); - } - - 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; - - 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); - } - - 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; + const chat = URI.parse(buildDefaultChatUri(session)); - 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(); - } - }); - } + 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 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(), - }); - } + 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; - assert.deepStrictEqual(adopted, cases.map(({ selected, expected }) => ({ - selectedAgent: selected?.toString(), - resolvedAgent: expected?.toString(), - reappliedAgent: expected?.toString(), - workingDirectory: worktree.toString(), - }))); + 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, }); - } + peer.exit(); + }); 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/SESSIONS.md b/src/vs/sessions/SESSIONS.md index e83a2004fe26c1..dbeaac3640536f 100644 --- a/src/vs/sessions/SESSIONS.md +++ b/src/vs/sessions/SESSIONS.md @@ -107,8 +107,6 @@ Chat origin and interactivity describe whether a chat is user-created, tool-crea Capabilities describe operations supported by the backing provider and remain observable when support may change during hydration. Provider-specific checks belong in the provider; shared services and UI consume the capability contract. -Providers that own repository access expose workspace-specific worktree support and branch choices through `ISessionsProvider.getWorktreeOptions`. Consumers use that authority for remote workspaces rather than asking the local Git extension; providers without that operation retain local Git discovery. - ### Changes Sessions and chats expose provider-neutral file changes and changesets. Transport, reconciliation, and backend metadata stay in the provider. Presentation stays in the owning changes and layout contributions. diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 254d88ac3d8c32..fe7f2f898b3580 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as DOM from '../../../../base/browser/dom.js'; -import { raceCancellationError, raceTimeout, RunOnceScheduler } from '../../../../base/common/async.js'; +import { raceCancellationError, raceTimeout } from '../../../../base/common/async.js'; import { BaseActionViewItem, IBaseActionViewItemOptions } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { IButton } from '../../../../base/browser/ui/button/button.js'; @@ -42,7 +42,7 @@ import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/co import { defaultCheckboxStyles, defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { hasNativeContextMenu } from '../../../../platform/window/common/window.js'; import { IWorkspacePickerItem, WorkspacePicker } from '../../chat/browser/sessionWorkspacePicker.js'; -import { BranchPicker, IBranchPickerBranch, type IBranchPickerState } from '../../chat/browser/branchPicker.js'; +import { BranchPicker, IBranchPickerBranch } from '../../chat/browser/branchPicker.js'; import { MobileSessionTypePicker } from '../../chat/browser/mobile/mobileSessionTypePicker.js'; import { isMobilePickerSheetTarget } from '../../../browser/parts/mobile/mobilePickerSheet.js'; import { ISession, ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_LOCAL } from '../../../services/sessions/common/session.js'; @@ -65,7 +65,6 @@ import { NewChatModelPickerService, INewChatModelPickerService } from '../../cha import { createNewSessionConfigToolbar, createNewSessionControlToolbar } from '../../chat/browser/newSessionConfigToolbars.js'; import { ISessionModelSelection, SessionModelSelection } from '../../chat/browser/sessionModelSelection.js'; import { ISessionContext, SessionContext } from '../../../services/sessions/browser/sessionContext.js'; -import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; import { VisibleSession } from '../../../services/sessions/browser/visibleSessions.js'; import { setActiveSessionContextKeys } from '../../../services/sessions/common/sessionContextKeys.js'; import { SessionUsesCombinedConfigPickerContext } from '../../../common/contextkeys.js'; @@ -561,15 +560,11 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private readonly renderDisposables = this._register(new DisposableStore()); private readonly branchRepoDisposable = this._register(new MutableDisposable()); private readonly branchRequest = this._register(new MutableDisposable()); - private readonly repositoryReloadScheduler = this._register(new RunOnceScheduler(() => { - void this.reloadRepository(this.isolationModel.folderUri); - }, 0)); private branchRequestId = 0; private readonly branchPicker: BranchPicker; private branchLoadState: BranchLoadState = 'noFolder'; private repository: IGitRepository | undefined; private branches: readonly string[] = []; - private filterBranches: IBranchPickerState['filterBranches']; private detachedCommit: string | undefined; private worktreeCapabilityResolved = false; @@ -584,7 +579,6 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private readonly visible: IObservable | undefined, @IGitService private readonly gitService: IGitService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, - @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, @ILogService private readonly pickerLogService: ILogService, @IInstantiationService instantiationService: IInstantiationService, ) { @@ -603,7 +597,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.renderBranchControl(); }, onRetry: () => { - this.scheduleRepositoryReload(); + void this.reloadRepository(this.isolationModel.folderUri); }, isolation: { label: localize('automation.form.isolation.worktree', "New Worktree"), @@ -619,7 +613,6 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { override render(container: HTMLElement): void { this.renderDisposables.clear(); this.branchRepoDisposable.clear(); - this.repositoryReloadScheduler.cancel(); this.cancelBranchRequest(); DOM.clearNode(container); container.style.marginLeft = 'auto'; @@ -636,21 +629,16 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.refreshTargetCapability(); this.renderBranchControl(); this.renderDisposables.add(autorun(reader => { - this.workspaceFolder.read(reader); + const folderUri = this.workspaceFolder.read(reader); this.refreshTargetAndRender(); - this.scheduleRepositoryReload(); + void this.reloadRepository(folderUri); })); this.renderDisposables.add(this.onDidChangeTarget(() => { this.refreshTargetAndRender(); - this.scheduleRepositoryReload(); - })); - this.renderDisposables.add(this.sessionsManagementService.onDidChangeSessionTypes(() => { - this.refreshTargetAndRender(); - this.scheduleRepositoryReload(); })); + this.renderDisposables.add(this.sessionsManagementService.onDidChangeSessionTypes(() => this.refreshTargetAndRender())); this.renderDisposables.add({ dispose: () => { - this.repositoryReloadScheduler.cancel(); this.cancelBranchRequest(); } }); @@ -702,17 +690,16 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { branches.unshift({ name: selectedBranch, selected: true, - unavailable: !this.filterBranches, + unavailable: true, }); } const worktreeUnavailableReason = this.getWorktreeUnavailableReason(); const isolationState: 'enabled' | 'disabled' | 'hidden' = - worktreeUnavailableReason === undefined || this.isolationModel.isolationMode === 'worktree' ? 'enabled' : 'disabled'; + worktreeUnavailableReason === undefined ? 'enabled' : 'disabled'; this.branchPicker.update({ label: presentation.label, branches, - filterBranches: this.filterBranches, status: this.branchLoadState === 'loadingRepository' || this.branchLoadState === 'loadingBranches' ? 'loading' : this.branchLoadState === 'error' @@ -826,7 +813,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { if (!this.isolationModel.supportsWorktreeConfiguration) { return localize('automation.form.isolation.worktreeUnavailable', "Not supported by the selected session type"); } - if (this.isolationModel.isolationMode === 'worktree' && this.isolationModel.selectedBranch) { + if (this.isolationModel.selectedBranch) { return undefined; } switch (this.branchLoadState) { @@ -853,21 +840,12 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.branchRequest.clear(); } - private scheduleRepositoryReload(): void { - this.cancelBranchRequest(); - this.branchRepoDisposable.clear(); - this.branchLoadState = this.isolationModel.folderUri ? 'loadingRepository' : 'noFolder'; - this.renderBranchControl(); - this.repositoryReloadScheduler.schedule(); - } - private async reloadRepository(folder: URI | undefined): Promise { const requestId = ++this.branchRequestId; this.cancelBranchRequest(); this.branchRepoDisposable.clear(); this.repository = undefined; this.branches = []; - this.filterBranches = undefined; this.detachedCommit = undefined; if (!folder) { this.branchLoadState = 'noFolder'; @@ -879,46 +857,6 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.renderBranchControl(); const cts = new CancellationTokenSource(); this.branchRequest.value = cts; - const target = this.sessionsManagementService.getSessionTypesForFolder(folder).find(candidate => - candidate.sessionType.id === this.state.sessionTypeId - && (this.state.providerId === undefined || candidate.providerId === this.state.providerId) - ); - const provider = target && this.sessionsProvidersService.getProvider(target.providerId); - if (provider?.getWorktreeOptions && target) { - try { - const options = await provider.getWorktreeOptions(folder, target.sessionType.id, cts.token); - if (requestId !== this.branchRequestId || cts.token.isCancellationRequested) { - return; - } - this.isolationModel.setHeadBranch(options?.currentBranch); - this.branches = normalizeAutomationBranchNames(options?.branches ?? []); - this.branchLoadState = !options ? 'noRepository' : this.branches.length > 0 ? 'ready' : 'empty'; - if (options) { - this.isolationModel.setSupportsWorktreeConfiguration(options.supportsWorktree); - const loadBranches = options.loadBranches; - this.filterBranches = loadBranches ? async (query, token) => { - try { - const branches = await loadBranches(query, token); - const selectedBranch = this.isolationModel.selectedBranch ?? this.isolationModel.headBranch; - return normalizeAutomationBranchNames(branches).map(name => ({ name, selected: name === selectedBranch })); - } catch (error) { - if (!token.isCancellationRequested) { - this.pickerLogService.error('[AutomationDialog] Failed to filter worktree branches.', error); - } - throw error; - } - } : undefined; - } - } catch (error) { - if (requestId !== this.branchRequestId || cts.token.isCancellationRequested) { - return; - } - this.pickerLogService.error('[AutomationDialog] Failed to load worktree options from the session provider.', error); - this.branchLoadState = 'error'; - } - this.renderBranchControl(); - return; - } let repo: IGitRepository | undefined; try { repo = await this.gitService.openRepository(folder); diff --git a/src/vs/sessions/contrib/automations/browser/automationTools.ts b/src/vs/sessions/contrib/automations/browser/automationTools.ts index 6a948d1ce08210..40d3754581ab02 100644 --- a/src/vs/sessions/contrib/automations/browser/automationTools.ts +++ b/src/vs/sessions/contrib/automations/browser/automationTools.ts @@ -3,10 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { raceCancellationError } from '../../../../base/common/async.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { Codicon } from '../../../../base/common/codicons.js'; -import { isCancellationError } from '../../../../base/common/errors.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; @@ -26,7 +24,6 @@ import { ChatPermissionLevel } from '../../../../workbench/contrib/chat/common/c import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress } from '../../../../workbench/contrib/chat/common/tools/languageModelToolsService.js'; import { ISession } from '../../../services/sessions/common/session.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; -import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; export const ListAutomationsToolId = 'vscode_listAutomations'; export const ConfigureAutomationToolId = 'vscode_configureAutomation'; @@ -368,7 +365,6 @@ export class ConfigureAutomationTool implements IToolImpl { @IAutomationService private readonly automationService: IAutomationService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, @IConfigurationService private readonly configurationService: IConfigurationService, - @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, ) { } getToolData(): IToolData { @@ -582,14 +578,14 @@ The change uses the current tool-approval policy. When approval is required, the try { if (proposal.kind === 'create') { const target = proposal.validateTargetAvailability - ? await this.resolveAvailableTarget(proposal.initialValues.target, token) + ? this.resolveAvailableTarget(proposal.initialValues.target) : proposal.initialValues.target; return await this.applyCreate({ ...proposal.initialValues, target }, token); } const target = proposal.initialValues.target ? proposal.validateTargetAvailability - ? await this.resolveAvailableTarget(proposal.initialValues.target, token) + ? this.resolveAvailableTarget(proposal.initialValues.target) : proposal.initialValues.target : undefined; const patch = target ? { ...proposal.initialValues, target } : proposal.initialValues; @@ -604,9 +600,6 @@ The change uses the current tool-approval policy. When approval is required, the if (error instanceof AutomationToolMutationBlockedError) { return error.result; } - if (token.isCancellationRequested && isCancellationError(error)) { - return automationToolCancelled(); - } if (error instanceof AutomationToolInputError) { return automationToolError(error.message); } @@ -667,7 +660,7 @@ The change uses the current tool-approval policy. When approval is required, the return undefined; } - private async resolveAvailableTarget(target: AutomationTarget, token: CancellationToken): Promise { + private resolveAvailableTarget(target: AutomationTarget): AutomationTarget { const candidates = target.kind === 'quickChat' ? this.sessionsManagementService.getQuickChatSessionTypes() : this.sessionsManagementService.getSessionTypesForFolder(target.folderUri); @@ -677,20 +670,8 @@ The change uses the current tool-approval policy. When approval is required, the ? `The quick-chat target "${target.providerId}/${target.sessionTypeId}" is not available.` : 'The proposed workspace target is not available for the selected provider and session type.'); } - if (target.kind === 'workspace' && target.isolation.kind === 'worktree') { - if (!candidate.sessionType.supportsWorktreeConfiguration) { - throw new AutomationToolInputError(`Session type "${candidate.sessionType.id}" does not support worktree isolation.`); - } - const provider = this.sessionsProvidersService.getProvider(candidate.providerId); - if (!provider) { - throw new AutomationToolInputError(`Sessions provider "${candidate.providerId}" is no longer available.`); - } - if (provider.getWorktreeOptions) { - const options = await raceCancellationError(provider.getWorktreeOptions(target.folderUri, candidate.sessionType.id, token), token); - if (!options?.supportsWorktree) { - throw new AutomationToolInputError('The selected workspace does not support worktree isolation.'); - } - } + if (target.kind === 'workspace' && target.isolation.kind === 'worktree' && !candidate.sessionType.supportsWorktreeConfiguration) { + throw new AutomationToolInputError(`Session type "${candidate.sessionType.id}" does not support worktree isolation.`); } return { ...target, diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index 5be110e8345580..0270555a0f3788 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -9,7 +9,6 @@ import { StandardKeyboardEvent } from '../../../../../base/browser/keyboardEvent import { Dialog } from '../../../../../base/browser/ui/dialog/dialog.js'; import { SelectBox } from '../../../../../base/browser/ui/selectBox/selectBox.js'; import { DeferredPromise, timeout } from '../../../../../base/common/async.js'; -import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { Action, IAction } from '../../../../../base/common/actions.js'; @@ -41,8 +40,7 @@ import { IAutomationSessionTemplate } from '../../../../../workbench/contrib/cha import { GitRefType, IGitRepository, IGitService } from '../../../../../workbench/contrib/git/common/gitService.js'; import { IHostService } from '../../../../../workbench/services/host/browser/host.js'; import { ISession, ISessionWorkspace, SessionTypeAuthRequirement } from '../../../../services/sessions/common/session.js'; -import { IAutomationSessionConfiguration, type ISessionsProvider, type ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; -import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; +import { IAutomationSessionConfiguration } from '../../../../services/sessions/common/sessionsProvider.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { AutomationIsolationGroupActionViewItem, AutomationSessionDraftSynchronizer, canSelectAutomationWorkspace, getAutomationTargetHint, IFormState, IValidationState, isAutomationDialogPopupTarget, registerAutomationDialogKeyboardNavigation, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from '../../browser/automationDialog.js'; import { AutomationIsolationModel } from '../../common/isolationGroupModel.js'; @@ -112,8 +110,6 @@ class RecordingActionWidgetService extends mock() { labels: readonly string[] = []; details: ReadonlyArray['detail']> = []; ariaLabels: readonly string[] = []; - filter: ((query: string) => Promise) | undefined; - filterVisible = false; private selectItem: ((label: string) => void) | undefined; private hideWidget: ((didCancel?: boolean) => void) | undefined; @@ -126,7 +122,7 @@ class RecordingActionWidgetService extends mock() { _container: HTMLElement | undefined, _actionBarActions: readonly IAction[], accessibilityProvider?: Partial>>, - listOptions?: IActionListOptions, + _listOptions?: IActionListOptions, ): void { this.isVisible = true; this.labels = items.map(item => item.label ?? ''); @@ -135,17 +131,6 @@ class RecordingActionWidgetService extends mock() { const label = accessibilityProvider?.getAriaLabel?.(item); return typeof label === 'string' ? label : label?.get() ?? ''; }); - this.filterVisible = listOptions?.showFilter === true; - this.filter = delegate.onFilter ? async query => { - const filteredItems = await delegate.onFilter?.(query, CancellationToken.None) ?? []; - this.labels = filteredItems.map(item => item.label ?? ''); - this.selectItem = label => { - const item = filteredItems.find(candidate => candidate.label === label)?.item; - if (item) { - delegate.onSelect(item); - } - }; - } : undefined; this.selectItem = label => { const item = items.find(candidate => candidate.label === label)?.item; if (item) { @@ -754,11 +739,8 @@ suite('Automation branch picker', () => { function createItem(options?: { readonly state?: IFormState; readonly getRefs?: IGitRepository['getRefs']; - readonly getWorktreeOptions?: ISessionsProvider['getWorktreeOptions']; - readonly supportsWorktreeConfiguration?: boolean; readonly failOpenRepositoryOnce?: boolean; readonly providerInitiallyUnavailable?: boolean; - readonly onDidChangeTarget?: Event; readonly revalidate?: () => void; readonly visible?: boolean; }): { @@ -813,14 +795,11 @@ suite('Automation branch picker', () => { id: state.sessionTypeId ?? 'copilotcli', label: 'Copilot', icon: Codicon.copilot, - supportsWorktreeConfiguration: options?.supportsWorktreeConfiguration ?? true, + supportsWorktreeConfiguration: state.sessionTypeId === 'copilotcli', authRequirement: SessionTypeAuthRequirement.GitHub, }, }] : [], })); - instantiationService.stub(ISessionsProvidersService, { getProvider: () => undefined }, 'getProvider', upcastPartial({ - getWorktreeOptions: options?.getWorktreeOptions, - })); instantiationService.stub(ILogService, new NullLogService()); const action = disposables.add(new Action('test.automationIsolation', 'Automation Isolation')); @@ -830,7 +809,7 @@ suite('Automation branch picker', () => { state, model, model.folderUriObs, - options?.onDidChangeTarget ?? Event.None, + Event.None, options?.revalidate ?? (() => { }), undefined, visible, @@ -1050,8 +1029,7 @@ suite('Automation branch picker', () => { test('normalizes unsupported Worktree targets back to Folder mode', async () => { const { container, model } = createItem({ - state: createFormState({ sessionTypeId: 'cloud', branch: 'feature/saved' }), - supportsWorktreeConfiguration: false, + state: createFormState({ sessionTypeId: 'claude', branch: 'feature/saved' }), }); await timeout(0); @@ -1068,241 +1046,21 @@ suite('Automation branch picker', () => { }); }); - for (const sessionTypeId of ['copilotcli', 'claude', 'codex', 'custom']) { - test(`selects provider-owned worktree branches for ${sessionTypeId} without local Git`, async () => { - const folderUri = URI.parse('vscode-agent-host://remote/workspace'); - const requests: { folderUri: URI; sessionTypeId: string }[] = []; - const { container, model, actionWidgetService, getOpenRepositoryAttempts } = createItem({ - state: createFormState({ providerId: 'remote-provider', sessionTypeId, folderUri, isolationMode: 'workspace' }), - getWorktreeOptions: async (folderUri, sessionTypeId) => { - requests.push({ folderUri, sessionTypeId }); - return { supportsWorktree: true, currentBranch: 'main', branches: ['release', 'main', 'copilot-worktree-generated'] }; - }, - }); - await timeout(0); - container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); - container.querySelector('.automation-form-branch-slot')!.click(); - actionWidgetService.select('release'); - - assert.deepStrictEqual({ - requests, - localGitRequests: getOpenRepositoryAttempts(), - branches: actionWidgetService.labels, - mode: model.isolationMode, - branch: model.persistedBranch, - checked: container.querySelector('.monaco-checkbox')?.getAttribute('aria-checked'), - }, { - requests: [{ folderUri, sessionTypeId }], - localGitRequests: 0, - branches: ['main', 'release'], - mode: 'worktree', - branch: 'release', - checked: 'true', - }); - }); - } - - for (const options of [undefined, { supportsWorktree: false, currentBranch: 'main', branches: ['main'] }] satisfies (ISessionWorktreeOptions | undefined)[]) { - test(`disables provider worktrees when ${options ? 'the schema does not support them' : 'no repository exists'}`, async () => { - const { container, model, getOpenRepositoryAttempts } = createItem({ - state: createFormState({ isolationMode: 'workspace', branch: undefined }), - getWorktreeOptions: async () => options, - }); - await timeout(0); - container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); - - assert.deepStrictEqual({ - mode: model.isolationMode, - branch: model.persistedBranch, - localGitRequests: getOpenRepositoryAttempts(), - }, { mode: 'workspace', branch: undefined, localGitRequests: 0 }); - }); - } - - test('allows a saved worktree target to opt back into Folder when its workspace no longer supports worktrees', async () => { - const { container, model } = createItem({ - state: createFormState({ isolationMode: 'worktree', branch: 'release' }), - getWorktreeOptions: async () => ({ supportsWorktree: false, currentBranch: 'main', branches: ['main'] }), - }); - await timeout(0); - const before = { mode: model.isolationMode, branch: model.persistedBranch }; - container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); - const after = { mode: model.isolationMode, branch: model.persistedBranch }; - container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); - - assert.deepStrictEqual({ - before, - after, - cannotReenable: model.isolationMode, - }, { - before: { mode: 'worktree', branch: undefined }, - after: { mode: 'workspace', branch: undefined }, - cannotReenable: 'workspace', - }); - }); - - for (const discovery of ['noRepository', 'loading', 'error', 'empty'] as const) { - test(`does not re-enable a saved Worktree target after opting out while repository discovery is ${discovery}`, async () => { - const pending = new DeferredPromise(); - const { container, model } = createItem({ - state: createFormState({ isolationMode: 'worktree', branch: 'release' }), - getWorktreeOptions: async () => { - switch (discovery) { - case 'noRepository': return undefined; - case 'loading': return pending.p; - case 'error': throw new Error('Repository unavailable'); - case 'empty': return { supportsWorktree: true, currentBranch: undefined, branches: [] }; - } - }, - }); - await timeout(0); - const toggle = container.querySelector('.sessions-chat-isolation-checkbox .action-label')!; - const initialMode = model.isolationMode; - toggle.click(); - const afterOptingOut = model.isolationMode; - toggle.click(); - const afterReenableAttempt = { - mode: model.isolationMode, - selectedBranch: model.selectedBranch, - persistedBranch: model.persistedBranch, - disabled: container.querySelector('.sessions-chat-isolation-checkbox')?.classList.contains('disabled'), - }; - await pending.complete({ supportsWorktree: true, currentBranch: 'main', branches: ['main', 'release'] }); - - assert.deepStrictEqual({ initialMode, afterOptingOut, afterReenableAttempt }, { - initialMode: 'worktree', - afterOptingOut: 'workspace', - afterReenableAttempt: { mode: 'workspace', selectedBranch: 'release', persistedBranch: undefined, disabled: true }, - }); - }); - } - - test('retries provider branch failures without falling back to local Git', async () => { - let attempts = 0; - const { container, actionWidgetService, getOpenRepositoryAttempts } = createItem({ - getWorktreeOptions: async () => { - if (++attempts === 1) { - throw new Error('Remote host unavailable'); - } - return { supportsWorktree: true, currentBranch: 'remote-main', branches: ['remote-main'] }; - }, - }); - await timeout(0); - container.querySelector('.automation-form-branch-slot')!.click(); - actionWidgetService.select('Retry Loading Branches'); - await timeout(0); - - assert.deepStrictEqual({ - attempts, - localGitRequests: getOpenRepositoryAttempts(), - branch: container.querySelector('.automation-form-branch-name')?.textContent, - }, { attempts: 2, localGitRequests: 0, branch: 'remote-main' }); - }); - - test('coalesces target and session-type notifications into one provider repository lookup', async () => { - const targetChanged = disposables.add(new Emitter()); - const requests: string[] = []; - const { state, model, setProviderAvailable } = createItem({ - onDidChangeTarget: targetChanged.event, - getWorktreeOptions: async (_folder, sessionTypeId) => { - requests.push(sessionTypeId); - return { supportsWorktree: true, currentBranch: `${sessionTypeId}-main`, branches: [`${sessionTypeId}-main`] }; - }, - }); - await timeout(0); - state.sessionTypeId = 'claude'; - targetChanged.fire(); - setProviderAvailable(); - await timeout(0); - - assert.deepStrictEqual({ - requests, - branch: model.persistedBranch, - }, { requests: ['copilotcli', 'claude'], branch: 'claude-main' }); - }); - - test('queries the provider for branches beyond its initial completion window', async () => { - const queries: string[] = []; - let resolutionCount = 0; - const { container, actionWidgetService, model } = createItem({ - state: createFormState({ branch: 'saved/base' }), - getWorktreeOptions: async () => { - resolutionCount++; - return { - supportsWorktree: true, currentBranch: 'main', branches: ['main'], - loadBranches: async query => { - queries.push(query); - return ['release/long-lived']; - }, - }; - }, + test('enables Worktree branches for agent-host Copilot CLI', async () => { + const { container } = createItem({ + state: createFormState({ providerId: 'local-agent-host', sessionTypeId: 'copilotcli' }), }); await timeout(0); - container.querySelector('.automation-form-branch-slot')!.click(); - const savedBranchDetail = actionWidgetService.details[0]; - await actionWidgetService.filter?.('release'); - const filteredBranches = actionWidgetService.labels; - actionWidgetService.select('release/long-lived'); - container.querySelector('.automation-form-branch-slot')!.click(); + const trigger = container.querySelector('.automation-form-branch-slot'); + assert.ok(trigger); assert.deepStrictEqual({ - resolutionCount, - queries, - filterVisible: actionWidgetService.filterVisible, - filteredBranches, - selected: model.persistedBranch, - savedBranchDetail, - reopenedDetails: actionWidgetService.details, + disabled: trigger.getAttribute('aria-disabled'), + label: trigger.querySelector('.automation-form-branch-name')?.textContent, }, { - resolutionCount: 1, - queries: ['release'], - filterVisible: true, - filteredBranches: ['release/long-lived'], - selected: 'release/long-lived', - savedBranchDetail: undefined, - reopenedDetails: [undefined, undefined], - }); - }); - - test('does not select stale results while provider branch search is pending', async () => { - const branches = new DeferredPromise(); - const { container, actionWidgetService, model } = createItem({ - getWorktreeOptions: async () => ({ - supportsWorktree: true, currentBranch: 'main', branches: ['main'], - loadBranches: async () => branches.p, - }), - }); - await timeout(0); - container.querySelector('.automation-form-branch-slot')!.click(); - const filtering = actionWidgetService.filter?.('release'); - actionWidgetService.select('main'); - const pending = { - labels: actionWidgetService.labels, - selected: model.selectedBranch, - pickerVisible: actionWidgetService.isVisible, - }; - await branches.complete(['release']); - await filtering; - actionWidgetService.select('release'); - - assert.deepStrictEqual({ pending, selected: model.selectedBranch }, { - pending: { labels: ['Loading branches…'], selected: undefined, pickerVisible: true }, - selected: 'release', - }); - }); - - test('surfaces provider branch search failures as retry actions', async () => { - const { container, actionWidgetService } = createItem({ - getWorktreeOptions: async () => ({ - supportsWorktree: true, currentBranch: 'main', branches: ['main'], - loadBranches: async () => { throw new Error('Remote branch search failed'); }, - }), + disabled: 'false', + label: 'main', }); - await timeout(0); - container.querySelector('.automation-form-branch-slot')!.click(); - await actionWidgetService.filter?.('release'); - - assert.deepStrictEqual(actionWidgetService.labels, ['Retry Loading Branches']); }); test('preserves Worktree intent while the provider is discovered late', async () => { @@ -1326,7 +1084,6 @@ suite('Automation branch picker', () => { }); setProviderAvailable(); - await timeout(0); assert.deepStrictEqual({ mode: model.isolationMode, diff --git a/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts index 6a6e5a801271ee..060c6ef4990b22 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts @@ -11,9 +11,7 @@ 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 { ConfirmationOptionKind } from '../../../../../platform/agentHost/common/state/protocol/channels-chat/state.js'; -import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { NullLogService } from '../../../../../platform/log/common/log.js'; import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; @@ -24,8 +22,6 @@ import { ChatAutomationsEnabledContext, CHAT_AUTOMATIONS_ENABLED_SETTING } from import { IToolImpl, IToolInvocation, IToolResult, ToolProgress } from '../../../../../workbench/contrib/chat/common/tools/languageModelToolsService.js'; import { IChat, ISession, ISessionType, ISessionWorkspace } from '../../../../services/sessions/common/session.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; -import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; -import type { ISessionsProvider, ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; import { AutomationService } from '../../browser/automationService.js'; import { ConfigureAutomationTool, ConfigureAutomationToolId, DeleteAutomationTool, DeleteAutomationToolId, ListAutomationsTool, ListAutomationsToolId, RunAutomationTool, RunAutomationToolId } from '../../browser/automationTools.js'; import { AUTOMATION_STORAGE_KEY, IAutomationStorageCompareAndSwapResult, IAutomationStorageService } from '../../common/automationStorageService.js'; @@ -345,20 +341,6 @@ suite('AutomationTools', () => { return teardown.add(new AutomationService(storageService, new NullLogService(), automationStorageService)); } - function createConfigureTool( - automationService: IAutomationService, - sessionsManagementService: ISessionsManagementService, - configurationService: IConfigurationService, - provider?: ISessionsProvider, - ): ConfigureAutomationTool { - const instantiationService = teardown.add(new TestInstantiationService()); - instantiationService.stub(IAutomationService, automationService); - instantiationService.stub(ISessionsManagementService, sessionsManagementService); - instantiationService.stub(IConfigurationService, configurationService); - instantiationService.stub(ISessionsProvidersService, { getProvider: () => undefined }, 'getProvider', provider); - return instantiationService.createInstance(ConfigureAutomationTool); - } - test('tool data is gated by AI and Automations context keys', () => { const automationService = new FakeAutomationService(); const configurationService = createConfigurationService(); @@ -369,7 +351,7 @@ suite('AutomationTools', () => { ).getToolData(); const listData = new ListAutomationsTool(automationService, configurationService).getToolData(); const deleteData = new DeleteAutomationTool(automationService, configurationService).getToolData(); - const configureData = createConfigureTool( + const configureData = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), configurationService, @@ -415,7 +397,7 @@ suite('AutomationTools', () => { }); test('configureAutomation tool data requires explicit creation intent', () => { - const modelDescription = createConfigureTool( + const modelDescription = new ConfigureAutomationTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -778,7 +760,7 @@ suite('AutomationTools', () => { test('configureAutomation prepares normal create and update confirmations', async () => { const existing = createAutomation(); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( new FakeAutomationService([existing]), new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -837,7 +819,7 @@ suite('AutomationTools', () => { sessionTypeId: 'copilot', }; const schedule: IAutomationSchedule = { interval: 'daily', scheduleHour: 8, scheduleMinute: 30, scheduleDay: 1 }; - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(createSession({ quickChat: true }), true), createConfigurationService(), @@ -875,7 +857,7 @@ suite('AutomationTools', () => { test('configureAutomation applies a partial guarded update and returns clickable result data', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -924,7 +906,7 @@ suite('AutomationTools', () => { test('configureAutomation accepts a provider mode returned by listAutomations', async () => { const existing = createAutomation({ mode: 'autopilot' }); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -955,7 +937,7 @@ suite('AutomationTools', () => { }, }); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -989,7 +971,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(); const listed = await invoke(new ListAutomationsTool(automationService, configurationService), {}); const returnedTemplate = JSON.parse(getText(listed)).automations[0].sessionTemplate; - const tool = createConfigureTool(automationService, new FakeSessionsManagementService(undefined), configurationService); + const tool = new ConfigureAutomationTool(automationService, new FakeSessionsManagementService(undefined), configurationService); await invoke(tool, { automationId: existing.id, sessionTemplate: returnedTemplate }); assert.deepStrictEqual(automationService.updated, [{ id: existing.id, patch: { sessionTemplate } }]); @@ -997,7 +979,7 @@ suite('AutomationTools', () => { test('configureAutomation validates and bounds model-specific configuration', async () => { const automationService = new FakeAutomationService(); - const tool = createConfigureTool(automationService, new FakeSessionsManagementService(undefined), createConfigurationService()); + const tool = new ConfigureAutomationTool(automationService, new FakeSessionsManagementService(undefined), createConfigurationService()); const errors: IToolResult['toolResultError'][] = []; for (const sessionTemplate of [ { modelConfiguration: { thinkingLevel: 'low' } }, @@ -1036,7 +1018,7 @@ suite('AutomationTools', () => { }, }); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1063,7 +1045,7 @@ suite('AutomationTools', () => { throw new AutomationSessionTemplateAuthorityError(); } }([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1080,7 +1062,7 @@ suite('AutomationTools', () => { test('configureAutomation rejects editable changes made while awaiting approval', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1109,7 +1091,7 @@ suite('AutomationTools', () => { test('configureAutomation permits runtime metadata changes while awaiting approval', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1140,7 +1122,7 @@ suite('AutomationTools', () => { test('configureAutomation validates explicit targets before writing', async () => { const automationService = new FakeAutomationService(); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService( undefined, @@ -1173,185 +1155,11 @@ suite('AutomationTools', () => { }); }); - for (const operation of ['create', 'update'] as const) { - for (const supportsWorktree of [true, false, undefined]) { - test(`configureAutomation ${operation} validates workspace worktree support: ${supportsWorktree}`, async () => { - const existing = createAutomation(); - const automationService = new FakeAutomationService([existing]); - const folderUri = URI.parse('vscode-agent-host://remote/workspace'); - const providerId = 'remote-provider'; - const sessionTypeId = 'claude'; - const requests: Array<{ folderUri: URI; sessionTypeId: string; token: CancellationToken }> = []; - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType(providerId, sessionTypeId, true)]), - createConfigurationService(), - upcastPartial({ - id: providerId, - getWorktreeOptions: async (folderUri, sessionTypeId, token) => { - requests.push({ folderUri, sessionTypeId, token }); - return supportsWorktree === undefined ? undefined : { supportsWorktree, currentBranch: 'main', branches: ['main'] }; - }, - }), - ); - const result = await invoke(tool, { - ...(operation === 'create' - ? { name: 'Worktree automation', prompt: 'Review the repository', schedule: { interval: 'manual' } } - : { automationId: existing.id }), - target: { kind: 'workspace', folderUri: folderUri.toString(), providerId, sessionTypeId, isolation: 'worktree', branch: 'main' }, - }); - const target: AutomationTarget = { kind: 'workspace', folderUri, providerId, sessionTypeId, isolation: { kind: 'worktree', branch: 'main' } }; - - assert.deepStrictEqual({ - requests: requests.map(request => ({ ...request, folderUri: request.folderUri.toString() })), - error: result.toolResultError, - created: automationService.created.map(automation => automation.target), - updated: automationService.updated.map(update => update.patch.target), - }, { - requests: [{ folderUri: folderUri.toString(), sessionTypeId, token: CancellationToken.None }], - error: supportsWorktree ? undefined : 'The selected workspace does not support worktree isolation.', - created: supportsWorktree && operation === 'create' ? [target] : [], - updated: supportsWorktree && operation === 'update' ? [target] : [], - }); - }); - } - } - - test('configureAutomation preserves worktree support for providers without workspace option discovery', async () => { - const automationService = new FakeAutomationService(); - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType('fallback-provider', 'copilotcli', true)]), - createConfigurationService(), - upcastPartial({ id: 'fallback-provider' }), - ); - const result = await invoke(tool, { - name: 'Fallback worktree', prompt: 'Review the repository', schedule: { interval: 'manual' }, - target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, - }); - - assert.deepStrictEqual({ - error: result.toolResultError, - targets: automationService.created.map(automation => automation.target), - }, { - error: undefined, - targets: [{ kind: 'workspace', folderUri: FOLDER, providerId: 'fallback-provider', sessionTypeId: 'copilotcli', isolation: { kind: 'worktree', branch: 'main' } }], - }); - }); - - test('configureAutomation rejects worktree targets whose provider has disappeared', async () => { - const automationService = new FakeAutomationService(); - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType('unavailable-provider', 'codex', true)]), - createConfigurationService(), - ); - const result = await invoke(tool, { - name: 'Unavailable worktree', prompt: 'Do not save', schedule: { interval: 'manual' }, - target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, - }); - - assert.deepStrictEqual({ - error: result.toolResultError, - created: automationService.created, - }, { - error: 'Sessions provider "unavailable-provider" is no longer available.', - created: [], - }); - }); - - test('configureAutomation leaves non-worktree and unrelated partial updates independent of repository availability', async () => { - const existing = createAutomation({ - target: { kind: 'workspace', folderUri: FOLDER, providerId: 'remote-provider', sessionTypeId: 'custom', isolation: { kind: 'worktree', branch: 'main' } }, - }); - const automationService = new FakeAutomationService([existing]); - let lookups = 0; - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'custom', true)]), - createConfigurationService(), - upcastPartial({ - id: 'remote-provider', - getWorktreeOptions: async () => { lookups++; return undefined; }, - }), - ); - const results: IToolResult[] = []; - for (const isolation of ['default', 'folder']) { - results.push(await invoke(tool, { - name: 'Folder automation', prompt: 'Review the repository', schedule: { interval: 'manual' }, - target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation }, - })); - } - results.push(await invoke(tool, { automationId: existing.id, enabled: false })); - - assert.deepStrictEqual({ - lookups, - errors: results.map(result => result.toolResultError), - created: automationService.created.length, - updated: automationService.updated, - }, { lookups: 0, errors: [undefined, undefined, undefined], created: 2, updated: [{ id: existing.id, patch: { enabled: false } }] }); - }); - - test('configureAutomation cancellation interrupts workspace validation without writing', async () => { - const automationService = new FakeAutomationService(); - const started = new DeferredPromise(); - const worktreeOptions = new DeferredPromise(); - const cancellation = teardown.add(new CancellationTokenSource()); - let requestToken: CancellationToken | undefined; - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'codex', true)]), - createConfigurationService(), - upcastPartial({ - id: 'remote-provider', - getWorktreeOptions: async (_folderUri, _sessionTypeId, token) => { - requestToken = token; - await started.complete(); - return worktreeOptions.p; - }, - }), - ); - const pending = invoke(tool, { - name: 'Cancelled worktree', prompt: 'Do not save', schedule: { interval: 'manual' }, - target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, - }, SESSION_RESOURCE, cancellation.token); - await started.p; - cancellation.cancel(); - const result = await pending; - await worktreeOptions.complete({ supportsWorktree: true, currentBranch: 'main', branches: ['main'] }); - - assert.deepStrictEqual({ - status: JSON.parse(getText(result)).status, - cancelled: requestToken?.isCancellationRequested, - created: automationService.created, - }, { status: 'cancelled', cancelled: true, created: [] }); - }); - - test('configureAutomation propagates workspace lookup errors instead of accepting the target', async () => { - const automationService = new FakeAutomationService(); - const error = new Error('Remote repository lookup failed'); - const tool = createConfigureTool( - automationService, - new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'copilotcli', true)]), - createConfigurationService(), - upcastPartial({ - id: 'remote-provider', - getWorktreeOptions: async () => { throw error; }, - }), - ); - - await assert.rejects(invoke(tool, { - name: 'Failed lookup', prompt: 'Do not save', schedule: { interval: 'manual' }, - target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, - }), candidate => candidate === error); - assert.deepStrictEqual(automationService.created, []); - }); - test('configureAutomation rechecks cancellation immediately before writing', async () => { const automationService = new FakeAutomationService(); const tokenSource = new CancellationTokenSource(); tokenSource.cancel(); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1385,7 +1193,7 @@ suite('AutomationTools', () => { [providerSessionType('local-agent-host', 'copilot')], ); sessionsManagementService.beforeGetFolderSessionTypes = () => configurationService.setUserConfiguration(CHAT_AUTOMATIONS_ENABLED_SETTING, false); - const tool = createConfigureTool(automationService, sessionsManagementService, configurationService); + const tool = new ConfigureAutomationTool(automationService, sessionsManagementService, configurationService); const result = await invoke(tool, { name: 'Disabled', @@ -1415,7 +1223,7 @@ suite('AutomationTools', () => { automationStorageService.readBarrier = readBarrier; const automationService = createStorageBackedService(undefined, automationStorageService); const tokenSource = teardown.add(new CancellationTokenSource()); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1483,7 +1291,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(); automationStorageService.beforeCompareAndSwap = () => configurationService.setUserConfiguration(CHAT_AUTOMATIONS_ENABLED_SETTING, false); const automationService = createStorageBackedService(raw, automationStorageService); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(undefined), configurationService, @@ -1507,7 +1315,7 @@ suite('AutomationTools', () => { const tokenSource = teardown.add(new CancellationTokenSource()); automationStorageService.beforeCompareAndSwap = () => tokenSource.cancel(); const automationService = createStorageBackedService(undefined, automationStorageService); - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1536,7 +1344,7 @@ suite('AutomationTools', () => { }); test('configureAutomation rejects stale IDs and malformed targets', async () => { - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1584,7 +1392,7 @@ suite('AutomationTools', () => { }); test('configureAutomation bounds opaque provider configuration', async () => { - const tool = createConfigureTool( + const tool = new ConfigureAutomationTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1632,7 +1440,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(false); const runner = new RecordingAutomationRunner(automationService); const listResult = await invoke(new ListAutomationsTool(automationService, configurationService), {}); - const configureResult = await invoke(createConfigureTool( + const configureResult = await invoke(new ConfigureAutomationTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), configurationService, diff --git a/src/vs/sessions/contrib/chat/browser/branchPicker.ts b/src/vs/sessions/contrib/chat/browser/branchPicker.ts index 788e5c3e055803..9368cad17cdae0 100644 --- a/src/vs/sessions/contrib/chat/browser/branchPicker.ts +++ b/src/vs/sessions/contrib/chat/browser/branchPicker.ts @@ -7,8 +7,6 @@ import * as dom from '../../../../base/browser/dom.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; -import { Delayer } from '../../../../base/common/async.js'; -import { CancellationToken } from '../../../../base/common/cancellation.js'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { localize } from '../../../../nls.js'; import { IActionWidgetService } from '../../../../platform/actionWidget/browser/actionWidget.js'; @@ -34,7 +32,6 @@ export interface IBranchPickerState { readonly missing?: boolean; readonly showChevron?: boolean; readonly isolation?: IBranchPickerIsolationState; - readonly filterBranches?: (query: string, token: CancellationToken) => Promise; } /** @@ -83,7 +80,6 @@ interface IBranchPickerItem { */ export class BranchPicker extends Disposable { private readonly _renderDisposables = this._register(new DisposableStore()); - private readonly _filterDelayer = this._register(new Delayer[]>(200)); private _state: IBranchPickerState = { label: localize('branchPicker.select', "Branch"), branches: [], @@ -211,13 +207,8 @@ export class BranchPicker extends Disposable { } const trigger = this._triggerElement; - const filterBranches = this._state.filterBranches; - let filterPending = false; const delegate: IActionListDelegate = { onSelect: item => { - if (filterPending) { - return; - } this._actionWidgetService.hide(); if (item.kind === 'retry') { this._options.onRetry?.(); @@ -225,27 +216,7 @@ export class BranchPicker extends Disposable { this._options.onSelectBranch(item.name); } }, - onFilter: filterBranches ? (query, token) => { - filterPending = true; - this._actionWidgetService.updateItems(this._getItems({ ...this._state, status: 'loading' })); - return this._filterDelayer.trigger(async () => { - try { - const branches = await filterBranches(query, token); - return this._getItems({ ...this._state, branches, status: branches.length > 0 ? 'ready' : 'empty' }); - } catch (error) { - if (token.isCancellationRequested) { - throw error; - } - return this._getItems({ ...this._state, status: 'error' }); - } finally { - if (!token.isCancellationRequested) { - filterPending = false; - } - } - }); - } : undefined, onHide: () => { - this._filterDelayer.cancel(); this._isOpen = false; trigger?.setAttribute('aria-expanded', 'false'); if (trigger?.isConnected) { @@ -275,20 +246,19 @@ export class BranchPicker extends Disposable { }, getWidgetAriaLabel: () => localize('branchPicker.ariaLabel', "Branch Picker"), }, - filterBranches || branchCount > FILTER_THRESHOLD + branchCount > FILTER_THRESHOLD ? { showFilter: true, filterPlaceholder: localize('branchPicker.filter', "Filter branches…") } : undefined, ); } - private _getItems(state = this._state): readonly IActionListItem[] { - switch (state.status) { + private _getItems(): readonly IActionListItem[] { + switch (this._state.status) { case 'loading': return [{ kind: ActionListItemKind.Action, label: localize('branchPicker.loading', "Loading branches…"), disabled: true, - showAlways: true, item: { kind: 'branch' }, }]; case 'error': @@ -307,7 +277,7 @@ export class BranchPicker extends Disposable { item: { kind: 'branch' }, }]; case 'ready': - return state.branches.map(branch => ({ + return this._state.branches.map(branch => ({ kind: ActionListItemKind.Action, label: branch.name, detail: branch.unavailable ? localize('branchPicker.unavailable', "Unavailable locally") : undefined, diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts index b3beb7acb7fdf8..d40db1d14e75a1 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 { omitAutomationSessionTemplateConfigValues, SessionConfigKey } from '../../../../../platform/agentHost/common/sessionConfigKeys.js'; +import { 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,15 +100,6 @@ 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, - ), }], }); } @@ -388,7 +379,6 @@ 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, @@ -487,9 +477,7 @@ export class AgentHostSessionConfigPicker extends Disposable { // chips must remain interactive. const isLoading = provider.isSessionConfigResolving(session.sessionId).get(); - const properties = this._orderProperties(Object.entries(this._options.includeRepositoryConfiguration === false - ? omitAutomationSessionTemplateConfigValues(resolvedConfig.schema.properties) - : resolvedConfig.schema.properties)); + const properties = this._orderProperties(Object.entries(resolvedConfig.schema.properties)); let renderedIsolationCheckbox = false; for (const [property, schema] of properties) { @@ -1442,15 +1430,7 @@ 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, {})); - }, - )); - 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 })); + return new PickerActionViewItem(scopedInstantiationService.createInstance(MobileAgentHostSessionConfigPicker, session)); }, )); this._register(actionViewItemService.register( diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 4d836201c40a09..b03ccaf8891c84 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -65,7 +65,7 @@ import { linkKey } from '../../../../common/sessionLinks.js'; import { ChatInteractivity, ChatModelSource, ChatOriginKind, DEFAULT_CHAT_CAPABILITIES, effectiveChatInteractivity, getGitHubPullRequestRefs, getHighestPriorityPullRequestIcon, IChat, IChatCapabilities, IGitHubInfo, IGitHubIssueRef, IGitHubPullRequestRef, isActiveSessionStatus, ISession, ISessionAgentRef, ISessionArtifact, ISessionCapabilities, ISessionChangesSummary, ISessionChatCustomization, ISessionChangeset, ISessionCreationReference, ISessionFileChange, ISessionTurnFileChange, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, ISideChatSelection, sessionFileChangesEqual, sessionWorkspaceEqual, SessionRemoteConnectionFailureReason, SessionRemoteConnectionStatus, SessionStatus, SessionTypeAuthRequirement, toSessionId, TURN_CHANGES_CHANGESET_ID } from '../../../../services/sessions/common/session.js'; import { dedupeLinks, partitionSessionArtifacts } from './agentHostSessionArtifacts.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; -import { IAutomationSessionConfiguration, IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot, ISessionsProviderCreateSessionOptions, ISessionWorktreeConfiguration, type ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; +import { IAutomationSessionConfiguration, IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot, ISessionsProviderCreateSessionOptions, ISessionWorktreeConfiguration } from '../../../../services/sessions/common/sessionsProvider.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; import { computePullRequestRefPresentation } from '../../../github/browser/pullRequestIconStatus.js'; import { IPullRequestIconCache } from '../../../github/browser/pullRequestIconCache.js'; @@ -3230,8 +3230,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement .filter(agent => this._shouldAdvertiseAgent(agent.provider)) .map((agent): ISessionType => ({ id: agent.provider, - // Isolation is host-owned; the workspace schema determines the available choices. - supportsWorktreeConfiguration: true, + supportsWorktreeConfiguration: agent.provider === CopilotCLISessionType.id, authRequirement: resolveAgentAuthRequirement(agent), // The chat session contribution and language models for an agent-host // agent are registered under its resource scheme (`agent-host-`), @@ -4242,46 +4241,6 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement return this._getNewSession(sessionId)?.getConfigValues(); } - async getWorktreeOptions(folderUri: URI, sessionTypeId: string, token: CancellationToken): Promise { - const connection = this.connection; - if (!connection) { - throw new Error(`[${this.id}] Cannot resolve worktree options without an agent host connection`); - } - const resolved = await raceCancellationError(connection.resolveSessionConfig({ - provider: sessionTypeId, - workingDirectory: folderUri, - config: { [SessionConfigKey.Isolation]: 'folder' }, - }), token); - const branchSchema = resolved.schema.properties[SessionConfigKey.Branch]; - if (!branchSchema) { - return undefined; - } - const isolationSchema = resolved.schema.properties[SessionConfigKey.Isolation]; - const currentBranch = resolved.values[SessionConfigKey.Branch]; - const loadBranches = branchSchema.enumDynamic ? async (query: string, token: CancellationToken): Promise => { - if (this.connection !== connection) { - throw new Error(`[${this.id}] The agent host connection changed while loading branches`); - } - const result = await raceCancellationError(connection.sessionConfigCompletions({ - provider: sessionTypeId, - workingDirectory: folderUri, - config: resolved.values, - property: SessionConfigKey.Branch, - query: query || undefined, - }), token); - return result.items.map(item => item.value); - } : undefined; - const branches = loadBranches - ? await loadBranches('', token) - : (branchSchema.enum ?? []).filter((branch): branch is string => typeof branch === 'string'); - return { - supportsWorktree: isolationSchema?.enum?.includes('worktree') === true && !isolationSchema.readOnly, - currentBranch: typeof currentBranch === 'string' ? currentBranch : undefined, - branches, - ...(loadBranches ? { loadBranches } : {}), - }; - } - async setIsolationMode(sessionId: string, mode: string): Promise { const policyRestricted = isAutoApprovePolicyRestricted(this._baseConfigurationService); const value = normalizeSessionConfigValue( 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 d2045e8dd0757c..b793902b64f4e3 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,7 +29,6 @@ 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'; @@ -331,8 +330,8 @@ function setupServices( } /** Create and render a fresh picker instance, as the toolbar does on a rebuild. */ -function renderPicker(store: Pick, 'add'>, services: ReturnType, options?: ConstructorParameters[1]) { - const picker = store.add(services.instantiationService.createInstance(AgentHostSessionConfigPicker, services.sessionObs, options)); +function renderPicker(store: Pick, 'add'>, services: ReturnType) { + const picker = store.add(services.instantiationService.createInstance(AgentHostSessionConfigPicker, services.sessionObs)); const container = document.createElement('div'); picker.render(container); return { picker, container }; @@ -342,62 +341,6 @@ 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); @@ -556,7 +499,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', @@ -818,7 +761,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({ @@ -882,7 +825,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); @@ -987,7 +930,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'), @@ -1293,7 +1236,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); @@ -1323,7 +1266,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); @@ -1343,7 +1286,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/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index ddb3d7a5c00c5e..d862779568b640 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -753,47 +753,6 @@ suite('LocalAgentHostSessionsProvider', () => { ]); }); - test('advertises host-owned worktree configuration for every agent', () => { - agentHost.setAgents(['copilotcli', 'claude', 'codex', 'custom'].map(provider => ({ - provider, displayName: provider, description: '', models: [], - }))); - const configurationService = new TestConfigurationService({ [AgentHostCodexAgentEnabledSettingId]: true }); - const provider = createProvider(disposables, agentHost, undefined, { configurationService, isSessionsWindow: true }); - - assert.deepStrictEqual(provider.sessionTypes.map(type => ({ - id: type.id, - supportsWorktreeConfiguration: type.supportsWorktreeConfiguration, - })), ['copilotcli', 'claude', 'codex', 'custom'].map(id => ({ - id, supportsWorktreeConfiguration: true, - }))); - }); - - test('resolves workspace-specific worktree support from the host schema', async () => { - const provider = createProvider(disposables, agentHost); - const results = []; - for (const supportsWorktree of [true, false]) { - agentHost.resolveSessionConfigResult = { - schema: { - type: 'object', - properties: { - isolation: { type: 'string', title: 'Isolation', enum: supportsWorktree ? ['folder', 'worktree'] : ['folder'] }, - branch: { type: 'string', title: 'Branch', enum: ['main', 'release'] }, - }, - }, - values: { isolation: 'folder', branch: 'main' }, - }; - results.push(await provider.getWorktreeOptions(URI.file('/workspace'), 'claude', CancellationToken.None)); - } - agentHost.resolveSessionConfigResult = { schema: { type: 'object', properties: {} }, values: {} }; - results.push(await provider.getWorktreeOptions(URI.file('/workspace'), 'claude', CancellationToken.None)); - - assert.deepStrictEqual(results, [ - { supportsWorktree: true, currentBranch: 'main', branches: ['main', 'release'] }, - { supportsWorktree: false, currentBranch: 'main', branches: ['main', 'release'] }, - undefined, - ]); - }); - test('shares the root-state listener across session adapters', () => { agentHost.setAgents([{ provider: 'copilotcli', displayName: 'Copilot', description: '', models: [], capabilities: {} } as AgentInfo]); const provider = createProvider(disposables, agentHost); diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts index 1be1b1c3d91498..c88931381a9a5f 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts @@ -58,11 +58,6 @@ 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), }], }); } @@ -154,16 +149,14 @@ class CopilotPickerActionViewItemContribution extends Disposable implements IWor 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.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); + }, + )); 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 499f46479acfa0..9f41b1551cc31e 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, observableFromEvent, observableFromPromise, observableSignal, observableValue, observableValueOpts, runOnChange, transaction } from '../../../../../base/common/observable.js'; +import { autorun, constObservable, derived, derivedOpts, IObservable, IObservableSignal, IReader, ISettableObservable, ITransaction, 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,7 +78,6 @@ 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); @@ -270,8 +269,8 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { private readonly _status = observableValue(this, SessionStatus.Untitled); readonly status: IObservable = this._status; - private readonly _permissionLevelPreference = observableValue(this, ChatPermissionLevel.Default); - readonly permissionLevel: IObservable; + private readonly _permissionLevel = observableValue(this, ChatPermissionLevel.Default); + readonly permissionLevel: IObservable = this._permissionLevel; private readonly _workspaceData = observableValue(this, undefined); readonly workspace: IObservable = this._workspaceData; @@ -338,7 +337,6 @@ 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; } @@ -369,11 +367,6 @@ 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; @@ -540,8 +533,8 @@ class CopilotCLISession extends Disposable implements ICopilotChatSession { this._modeObservable.set({ id: modeId, kind: modeKind }, undefined); } - setPermissionLevel(level: string): void { - this._permissionLevelPreference.set(level, undefined); + setPermissionLevel(level: ChatPermissionLevel): void { + this._permissionLevel.set(level, undefined); } setTitle(title: string): void { @@ -679,8 +672,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; @@ -742,13 +735,11 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession this.sessionType = target; this.icon = CopilotCloudSessionType.icon; this.createdAt = new Date(); - 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._useSandbox.set(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 => { @@ -767,8 +758,8 @@ export class RemoteNewSession extends Disposable implements ICopilotChatSession this.mainChat = observableValue(this, buildChatFromSession(this)); } - setPermissionLevel(_level: ChatPermissionLevel): void { - // Remote sessions do not support client-side permission selection. + setPermissionLevel(level: ChatPermissionLevel): void { + throw new Error('Method not implemented.'); } // -- New session configuration methods -- @@ -791,7 +782,6 @@ 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. @@ -858,14 +848,6 @@ 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); } @@ -1796,28 +1778,21 @@ 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 (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; + if (mode) { + config[SessionConfigKey.Mode] = mode; } else { - 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(); + delete config[SessionConfigKey.Mode]; } - const configuredMode = config[SessionConfigKey.Mode]; - const mode = typeof configuredMode === 'string' ? configuredMode : undefined; - const configuredPermissionLevel = config[SessionConfigKey.AutoApprove]; - const permissionLevel = typeof configuredPermissionLevel === 'string' ? configuredPermissionLevel : undefined; + config[SessionConfigKey.AutoApprove] = permissionLevel; const agentUri = session.chatMode?.uri?.get(); const agent = agentUri ? { uri: agentUri.toString() } @@ -1837,8 +1812,16 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions throw new Error('CopilotChatSessionsProvider does not support quick chats'); } - /** The initial permission preference for a brand-new session. */ + /** + * Resolves the initial permission level for a brand-new session from + * `chat.permissions.default`, clamped to `Default` when enterprise policy + * disables global auto-approval. + */ 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; } @@ -1870,7 +1853,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions } } const permissionLevel = template?.config?.[SessionConfigKey.AutoApprove] ?? configuration.permissionLevel; - if (session instanceof CopilotCLISession && typeof permissionLevel === 'string') { + if (!(session instanceof RemoteNewSession) && isChatPermissionLevel(permissionLevel)) { session.setPermissionLevel(permissionLevel); } } @@ -1974,6 +1957,15 @@ 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 d717a7f479a2e4..4937305eec2e3d 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 { type IConfigurationChangeEvent, IConfigurationService, IConfigurationValue } from '../../../../../../platform/configuration/common/configuration.js'; +import { 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 { type ChatSessionOptionsMap, ChatSessionStatus, type IChatSession, IChatSessionProviderOptionGroup, IChatSessionsService, SessionType } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; +import { ChatSessionStatus, 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 { type IChatModel, IChatResponseModel } from '../../../../../../workbench/contrib/chat/common/model/chatModel.js'; +import { 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 { type IAutomationSessionConfiguration, ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; +import { 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,23 +418,11 @@ 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?: ICreateProviderForSendTestsOptions, + opts?: { onDidCommitSession?: Event<{ original: URI; committed: URI }>; configurationService?: TestConfigurationService; agentHostEnabled?: boolean; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined; notifications?: string[]; chatModeService?: IChatModeService; languageModelsService?: Partial }, ): TestSandboxCopilotProvider { const instantiationService = disposables.add(new TestInstantiationService()); @@ -463,10 +451,9 @@ function createProviderForSendTests( setSessionOption: () => true, getSessionOption: () => undefined, onDidChangeOptionGroups: Event.None, - ...opts?.chatSessionsService, }); instantiationService.stub(IChatService, { - acquireOrLoadSession: opts?.acquireOrLoadSession ?? (async () => undefined), + acquireOrLoadSession: async () => undefined, sendRequest: sendRequest, removeHistoryEntry: async (resource: URI) => { model.removeSession(resource); }, setChatSessionTitle: () => { }, @@ -2450,111 +2437,14 @@ suite('CopilotChatSessionsProvider', () => { assert.strictEqual(session?.permissionLevel.get(), ChatPermissionLevel.Autopilot); }); - test('clamps the effective default without rewriting its permission preference', async () => { + test('clamps to Default when chat.tools.global.autoApprove policy is false', () => { 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.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 }, - }); + assert.strictEqual(session?.permissionLevel.get(), ChatPermissionLevel.Default); }); test('falls back to Default when chat.permissions.default is unset', () => { @@ -2660,54 +2550,6 @@ 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, { @@ -2740,168 +2582,6 @@ 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'); @@ -2975,17 +2655,10 @@ 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; @@ -3008,13 +2681,11 @@ 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, }); }); @@ -3335,7 +3006,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; cloudSendResult?: ChatSendResult } = {}) { + function createSandboxProvider(opts: { enabled?: boolean; provision?: () => Promise; getOptionGroups?: () => IChatSessionProviderOptionGroup[] | undefined } = {}) { const configurationService = new TestConfigurationService(); configurationService.setUserConfiguration(CloudSandboxEnabledSettingId, opts.enabled ?? true); configurationService.setUserConfiguration(RemoteAgentHostsEnabledSettingId, true); @@ -3344,8 +3015,8 @@ suite('CopilotChatSessionsProvider', () => { const notifications: string[] = []; const provider = createProviderForSendTests(disposables, model, async (_resource, message) => { cloudSends.push(message); - // Leave routing-only requests pending unless the test provides a result. - return opts.cloudSendResult ?? new Promise(() => { }); + // Never settles: these tests only assert which path the send took. + return new Promise(() => { }); }, { configurationService, getOptionGroups: opts.getOptionGroups, notifications }); const provisionRequests: ICloudSandboxCreateSessionRequest[] = []; @@ -3432,52 +3103,6 @@ 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 dc29cf781b7c14..64e2f8059ea7e6 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,7 +9,6 @@ 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'; @@ -21,8 +20,6 @@ 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'; @@ -30,7 +27,6 @@ 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; @@ -47,17 +43,6 @@ 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); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 898e3f554c2599..dd5071d0cf29bf 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -5,7 +5,6 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; -import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; @@ -114,7 +113,7 @@ class MockAgentConnection extends mock() { return uri; } - override async resolveSessionConfig(_request: Parameters[0]): Promise { + override async resolveSessionConfig(): Promise { await Promise.resolve(); if (this.failResolveSessionConfig) { throw new Error('resolveSessionConfig unavailable'); @@ -435,60 +434,6 @@ suite('RemoteAgentHostSessionsProvider', () => { ]); }); - test('resolves worktree branches on the selected remote host for every agent', async () => { - const agentIds = ['copilotcli', 'claude', 'codex', 'custom']; - connection.setAgents(agentIds.map(provider => ({ provider, displayName: provider, description: '', models: [] }))); - const provider = createProvider(disposables, connection); - const folderUri = URI.parse('vscode-agent-host://localhost__4321/home/user/project'); - const resolveRequests: Parameters[0][] = []; - const completionRequests: Parameters[0][] = []; - connection.resolveSessionConfig = async request => { - resolveRequests.push(request); - return { - schema: { - type: 'object', - properties: { - isolation: { type: 'string', title: 'Isolation', enum: ['folder', 'worktree'] }, - branch: { type: 'string', title: 'Branch', enumDynamic: true }, - }, - }, - values: { isolation: 'folder', branch: 'remote-head' }, - }; - }; - connection.sessionConfigCompletions = async request => { - completionRequests.push(request); - return { - items: ['remote-head', 'release'] - .filter(name => !request.query || name.includes(request.query)) - .map(value => ({ value, label: value })), - }; - }; - const results = []; - for (const agentId of agentIds) { - const options = await provider.getWorktreeOptions(folderUri, agentId, CancellationToken.None); - results.push({ - supportsWorktree: options?.supportsWorktree, - currentBranch: options?.currentBranch, - branches: options?.branches, - search: await options?.loadBranches?.('release', CancellationToken.None), - }); - } - - assert.deepStrictEqual({ - capabilities: provider.sessionTypes.map(type => type.supportsWorktreeConfiguration), - results, - resolveRequests, - completionRequests, - }, { - capabilities: agentIds.map(() => true), - results: agentIds.map(() => ({ supportsWorktree: true, currentBranch: 'remote-head', branches: ['remote-head', 'release'], search: ['release'] })), - resolveRequests: agentIds.map(provider => ({ provider, workingDirectory: folderUri, config: { isolation: 'folder' } })), - completionRequests: agentIds.flatMap(provider => [undefined, 'release'].map(query => ({ - provider, workingDirectory: folderUri, config: { isolation: 'folder', branch: 'remote-head' }, property: 'branch', query, - }))), - }); - }); - test('session-type labels omit host suffix on web', () => { const provider = createProvider(disposables, connection, { address: '10.0.0.1:8080', connectionName: 'My Host', isWebPlatform: true }); diff --git a/src/vs/sessions/services/sessions/common/sessionsProvider.ts b/src/vs/sessions/services/sessions/common/sessionsProvider.ts index 641dbe0a50c84b..72f3db9fe9d960 100644 --- a/src/vs/sessions/services/sessions/common/sessionsProvider.ts +++ b/src/vs/sessions/services/sessions/common/sessionsProvider.ts @@ -73,15 +73,6 @@ export interface ISessionWorktreeConfiguration { readonly branch?: string; } -/** Repository branch choices and worktree support at a provider-owned workspace. */ -export interface ISessionWorktreeOptions { - readonly supportsWorktree: boolean; - readonly currentBranch: string | undefined; - readonly branches: readonly string[]; - /** Present when the initial branches are a partial result set. */ - readonly loadBranches?: (query: string, token: CancellationToken) => Promise; -} - /** * Presentation options for the sessions-core model picker. A provider returns * these from {@link ISessionsProvider.getModelPickerOptions} so it controls how @@ -433,9 +424,6 @@ export interface ISessionsProvider { */ setWorktreeConfiguration?(sessionId: string, configuration: ISessionWorktreeConfiguration): Promise; - /** Resolves worktree choices without creating a session; undefined means no repository was found. */ - getWorktreeOptions?(folderUri: URI, sessionTypeId: string, token: CancellationToken): Promise; - /** * Set whether the worktree branch tracks its upstream for a session. * @param sessionId The ID of the session. From afac4abfbeb5bf5362a0d7233e0cdb5e5224cea3 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Tue, 8 Sep 2026 11:39:25 +0200 Subject: [PATCH 2/8] agentHost: fix: mark generated worktree sessions pending early Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../platform/agentHost/node/agentService.ts | 23 ++- .../agentHost/test/node/agentService.test.ts | 131 ++++++++++-------- 2 files changed, 80 insertions(+), 74 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index bb4125004ff11f..35d60a2329e871 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -2926,10 +2926,7 @@ export class AgentService extends Disposable implements IAgentService { config = { ...config, importConversation: { ...config.importConversation, turns: importedTurns } }; } - // Resolve host-owned isolation before provider creation. Providers such as - // Codex may schedule eager prewarming from createSession; marking a - // client-chosen worktree session pending first prevents that prewarm from - // materializing in the picked folder before the host creates the worktree. + // Providers must see pending isolation before creation can schedule eager prewarming. const initializeSideEffects = this._sideEffects.initialize(); const sessionConfig = await this._resolveCreatedSessionConfig(provider, config); const deferWorktreeCreation = sessionConfig?.values?.[SessionConfigKey.Isolation] === 'worktree' && !config?.importConversation; @@ -3445,15 +3442,15 @@ export class AgentService extends Disposable implements IAgentService { } private async _createProviderSession(provider: IAgent, config: IAgentCreateSessionConfig | undefined, deferWorktreeCreation: boolean): Promise { - const requestedSessionId = deferWorktreeCreation && config?.session ? AgentSession.id(config.session) : undefined; - if (requestedSessionId) { - this._worktree.notePending(requestedSessionId); + const session = config?.session ?? this._mintSessionUri(provider); + const pendingSessionId = deferWorktreeCreation ? AgentSession.id(session) : undefined; + if (pendingSessionId) { + this._worktree.notePending(pendingSessionId); } let created: IAgentCreateSessionResult | undefined; try { const providerConfig = config ? this._toProviderConfig(config) : undefined; - const session = config?.session ?? this._mintSessionUri(provider); const defaultChatUri = URI.parse(buildDefaultChatUri(session)); const boundConfig: IAgentCreateSessionConfig = { ...(providerConfig ?? {}), session }; const result = await provider.chats.createChat(defaultChatUri, this._chatContext(session, defaultChatUri), this._toCreateChatOptions(boundConfig)); @@ -3464,9 +3461,6 @@ export class AgentService extends Disposable implements IAgentService { ...(result?.provisional ? { provisional: true } : {}), ...(result ? { chat: result } : {}), }; - if (deferWorktreeCreation && created.provisional) { - this._worktree.notePending(AgentSession.id(created.session)); - } await this._persistDefaultChatBacking(created); return created; } catch (err) { @@ -3475,9 +3469,8 @@ export class AgentService extends Disposable implements IAgentService { } throw err; } finally { - const returnedPendingSessionId = created?.provisional ? AgentSession.id(created.session) : undefined; - if (requestedSessionId && requestedSessionId !== returnedPendingSessionId) { - this._worktree.clearPending(requestedSessionId); + if (pendingSessionId && !created?.provisional) { + this._worktree.clearPending(pendingSessionId); } } } @@ -3493,6 +3486,8 @@ export class AgentService extends Disposable implements IAgentService { await provider.chats.disposeChat(defaultChatUri, this._chatContext(session, defaultChatUri)); } catch (disposeError) { this._logService.error(disposeError, `[AgentService] Failed to roll back default chat of provider session ${session.toString()}`); + } finally { + this._worktree.clearPending(AgentSession.id(session)); } } diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 9261bdfac0ace8..ddca28beb62c47 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -1184,70 +1184,81 @@ suite('AgentService (node dispatcher)', () => { }); }); - test('marks worktree isolation pending before a provisional provider can prewarm', async () => { - const session = AgentSession.uri('codex', 'pending-before-create'); - const workingDirectory = URI.file('/workspace/repo'); - const gitService = createNoopGitService(); - gitService.getRepositoryRoot = async () => workingDirectory; - gitService.revParse = async () => 'head'; - gitService.getCurrentBranch = async () => 'feature'; - gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'main' }); - const localService = disposables.add(createTestAgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); - const isolation = disposables.add(new WorktreeIsolation( - { _serviceBrand: undefined, generateBranchName: async () => 'agents/test' }, - gitService, - nullSessionDataService, - new NullLogService(), - )); - setTestAgentHostWorktreeIsolation(localService, isolation); - const pendingDuringCreate: boolean[] = []; - const providerCreateConfigs: Array | undefined> = []; - let failCreate = false; - class PrewarmingAgent extends MockAgent { - override readonly chats: IAgentChats = withChatOverrides(getChatSurface(this), base => ({ - createChat: async (chat, context, options) => { - const { configurationResource } = resolveAgentChatContext(context, chat); - pendingDuringCreate.push(isWorkingDirectoryPending(localService, configurationResource.toString())); - providerCreateConfigs.push(options?.config); - if (failCreate) { - throw new Error('create failed'); + for (const sessionIdSource of ['client', 'host']) { + test(`marks worktree isolation pending before a provisional provider can prewarm with a ${sessionIdSource} session ID`, async () => { + const workingDirectory = URI.file('/workspace/repo'); + const gitService = createNoopGitService(); + gitService.getRepositoryRoot = async () => workingDirectory; + gitService.revParse = async () => 'head'; + gitService.getCurrentBranch = async () => 'feature'; + gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'main' }); + let failProviderDataWrite = false; + class FailingProviderDataDatabase extends TestSessionDatabase { + override async setMetadata(key: string, value: string): Promise { + if (failProviderDataWrite && key === 'defaultChatProviderData') { + throw new Error('provider data write failed'); } - return { ...await expectCreatedChat(base.createChat(chat, context, options)), provisional: true }; - }, - })); - } - const agent = new PrewarmingAgent('codex'); - disposables.add(toDisposable(() => agent.dispose())); - registerTestAgentProvider(localService, agent); - - await localService.createSession({ - provider: 'codex', - session, - workingDirectories: workingDirectory ? [workingDirectory] : undefined, - config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, - }); + return super.setMetadata(key, value); + } + } + const sessionDataService = createSessionDataService(new FailingProviderDataDatabase()); + const localService = disposables.add(createTestAgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const isolation = disposables.add(new WorktreeIsolation( + { _serviceBrand: undefined, generateBranchName: async () => 'agents/test' }, + gitService, + sessionDataService, + new NullLogService(), + )); + setTestAgentHostWorktreeIsolation(localService, isolation); + const creatingSessions: URI[] = []; + const pendingDuringCreate: boolean[] = []; + const providerCreateConfigs: Array | undefined> = []; + let failCreate = false; + class PrewarmingAgent extends MockAgent { + override readonly chats: IAgentChats = withChatOverrides(getChatSurface(this), base => ({ + createChat: async (chat, context, options) => { + const { configurationResource } = resolveAgentChatContext(context, chat); + creatingSessions.push(configurationResource); + pendingDuringCreate.push(isWorkingDirectoryPending(localService, configurationResource)); + providerCreateConfigs.push(options?.config); + if (failCreate) { + throw new Error('create failed'); + } + return { ...await expectCreatedChat(base.createChat(chat, context, options)), provisional: true, providerData: 'blob' }; + }, + })); + } + const agent = disposables.add(new PrewarmingAgent('codex')); + registerTestAgentProvider(localService, agent); - const failedSession = AgentSession.uri('codex', 'failed-before-create'); - failCreate = true; - await assert.rejects(localService.createSession({ - provider: 'codex', - session: failedSession, - workingDirectories: workingDirectory ? [workingDirectory] : undefined, - config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, - }), /create failed/); + const createSession = (id: string) => localService.createSession({ + provider: 'codex', + session: sessionIdSource === 'client' ? AgentSession.uri('codex', id) : undefined, + workingDirectories: [workingDirectory], + config: { [SessionConfigKey.Isolation]: 'worktree', [SessionConfigKey.Branch]: 'main' }, + }); + const session = await createSession('pending-before-create'); + failCreate = true; + await assert.rejects(createSession('failed-before-create'), /create failed/); + failCreate = false; + failProviderDataWrite = true; + await assert.rejects(createSession('failed-after-create'), /provider data write failed/); - assert.deepStrictEqual({ - pendingDuringCreate, - providerCreateConfigs, - pendingAfterCreate: isWorkingDirectoryPending(localService, session.toString()), - pendingAfterFailure: isWorkingDirectoryPending(localService, failedSession.toString()), - }, { - pendingDuringCreate: [true, true], - providerCreateConfigs: [{}, {}], - pendingAfterCreate: true, - pendingAfterFailure: false, + assert.deepStrictEqual({ + providerSession: creatingSessions[0], + pendingDuringCreate, + providerCreateConfigs, + pendingAfterAttempts: creatingSessions.map(session => isWorkingDirectoryPending(localService, session)), + rolledBackSessions: agent.disposeSessionCalls, + }, { + providerSession: session, + pendingDuringCreate: [true, true, true], + providerCreateConfigs: [{}, {}, {}], + pendingAfterAttempts: [true, false, false], + rolledBackSessions: [creatingSessions[2]], + }); }); - }); + } test('createSession validates, exposes, and persists multi-root metadata', async () => { const db = new TestSessionDatabase(); From fd1a5ccfea305975d3ee90e7e59a59026160bd97 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Tue, 8 Sep 2026 11:39:37 +0200 Subject: [PATCH 3/8] automations: fix: advertise worktrees for all Agent Host agents Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../browser/baseAgentHostSessionsProvider.ts | 3 ++- .../localAgentHostSessionsProvider.test.ts | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index b03ccaf8891c84..bf33c7cd905590 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -3230,7 +3230,8 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement .filter(agent => this._shouldAdvertiseAgent(agent.provider)) .map((agent): ISessionType => ({ id: agent.provider, - supportsWorktreeConfiguration: agent.provider === CopilotCLISessionType.id, + // Isolation is host-owned; the workspace schema determines the available choices. + supportsWorktreeConfiguration: true, authRequirement: resolveAgentAuthRequirement(agent), // The chat session contribution and language models for an agent-host // agent are registered under its resource scheme (`agent-host-`), diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index d862779568b640..71d27f60bbbbfd 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -753,6 +753,21 @@ suite('LocalAgentHostSessionsProvider', () => { ]); }); + test('advertises host-owned worktree configuration for every agent', () => { + agentHost.setAgents(['copilotcli', 'claude', 'codex', 'custom'].map(provider => ({ + provider, displayName: provider, description: '', models: [], + }))); + const configurationService = new TestConfigurationService({ [AgentHostCodexAgentEnabledSettingId]: true }); + const provider = createProvider(disposables, agentHost, undefined, { configurationService, isSessionsWindow: true }); + + assert.deepStrictEqual(provider.sessionTypes.map(type => ({ + id: type.id, + supportsWorktreeConfiguration: type.supportsWorktreeConfiguration, + })), ['copilotcli', 'claude', 'codex', 'custom'].map(id => ({ + id, supportsWorktreeConfiguration: true, + }))); + }); + test('shares the root-state listener across session adapters', () => { agentHost.setAgents([{ provider: 'copilotcli', displayName: 'Copilot', description: '', models: [], capabilities: {} } as AgentInfo]); const provider = createProvider(disposables, agentHost); From a498f2a7a8886490a234f2f9546ead9c59f45ccb Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Tue, 8 Sep 2026 11:39:42 +0200 Subject: [PATCH 4/8] automations: fix: resolve worktree branches on the selected host Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- src/vs/sessions/SESSIONS.md | 2 + .../automations/browser/automationDialog.ts | 55 ++++- .../test/browser/automationDialog.test.ts | 188 ++++++++++++++++-- .../contrib/chat/browser/branchPicker.ts | 38 +++- .../browser/baseAgentHostSessionsProvider.ts | 42 +++- .../localAgentHostSessionsProvider.test.ts | 26 +++ .../remoteAgentHostSessionsProvider.test.ts | 57 +++++- .../sessions/common/sessionsProvider.ts | 12 ++ 8 files changed, 398 insertions(+), 22 deletions(-) diff --git a/src/vs/sessions/SESSIONS.md b/src/vs/sessions/SESSIONS.md index dbeaac3640536f..e83a2004fe26c1 100644 --- a/src/vs/sessions/SESSIONS.md +++ b/src/vs/sessions/SESSIONS.md @@ -107,6 +107,8 @@ Chat origin and interactivity describe whether a chat is user-created, tool-crea Capabilities describe operations supported by the backing provider and remain observable when support may change during hydration. Provider-specific checks belong in the provider; shared services and UI consume the capability contract. +Providers that own repository access expose workspace-specific worktree support and branch choices through `ISessionsProvider.getWorktreeOptions`. Consumers use that authority for remote workspaces rather than asking the local Git extension; providers without that operation retain local Git discovery. + ### Changes Sessions and chats expose provider-neutral file changes and changesets. Transport, reconciliation, and backend metadata stay in the provider. Presentation stays in the owning changes and layout contributions. diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index fe7f2f898b3580..0c4cbcfae61adc 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -42,7 +42,7 @@ import { IWorkspaceTrustRequestService } from '../../../../platform/workspace/co import { defaultCheckboxStyles, defaultInputBoxStyles, defaultSelectBoxStyles } from '../../../../platform/theme/browser/defaultStyles.js'; import { hasNativeContextMenu } from '../../../../platform/window/common/window.js'; import { IWorkspacePickerItem, WorkspacePicker } from '../../chat/browser/sessionWorkspacePicker.js'; -import { BranchPicker, IBranchPickerBranch } from '../../chat/browser/branchPicker.js'; +import { BranchPicker, IBranchPickerBranch, type IBranchPickerState } from '../../chat/browser/branchPicker.js'; import { MobileSessionTypePicker } from '../../chat/browser/mobile/mobileSessionTypePicker.js'; import { isMobilePickerSheetTarget } from '../../../browser/parts/mobile/mobilePickerSheet.js'; import { ISession, ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_LOCAL } from '../../../services/sessions/common/session.js'; @@ -65,6 +65,7 @@ import { NewChatModelPickerService, INewChatModelPickerService } from '../../cha import { createNewSessionConfigToolbar, createNewSessionControlToolbar } from '../../chat/browser/newSessionConfigToolbars.js'; import { ISessionModelSelection, SessionModelSelection } from '../../chat/browser/sessionModelSelection.js'; import { ISessionContext, SessionContext } from '../../../services/sessions/browser/sessionContext.js'; +import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; import { VisibleSession } from '../../../services/sessions/browser/visibleSessions.js'; import { setActiveSessionContextKeys } from '../../../services/sessions/common/sessionContextKeys.js'; import { SessionUsesCombinedConfigPickerContext } from '../../../common/contextkeys.js'; @@ -565,6 +566,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private branchLoadState: BranchLoadState = 'noFolder'; private repository: IGitRepository | undefined; private branches: readonly string[] = []; + private filterBranches: IBranchPickerState['filterBranches']; private detachedCommit: string | undefined; private worktreeCapabilityResolved = false; @@ -579,6 +581,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private readonly visible: IObservable | undefined, @IGitService private readonly gitService: IGitService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, + @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, @ILogService private readonly pickerLogService: ILogService, @IInstantiationService instantiationService: IInstantiationService, ) { @@ -635,8 +638,12 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { })); this.renderDisposables.add(this.onDidChangeTarget(() => { this.refreshTargetAndRender(); + void this.reloadRepository(this.isolationModel.folderUri); + })); + this.renderDisposables.add(this.sessionsManagementService.onDidChangeSessionTypes(() => { + this.refreshTargetAndRender(); + void this.reloadRepository(this.isolationModel.folderUri); })); - this.renderDisposables.add(this.sessionsManagementService.onDidChangeSessionTypes(() => this.refreshTargetAndRender())); this.renderDisposables.add({ dispose: () => { this.cancelBranchRequest(); @@ -690,7 +697,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { branches.unshift({ name: selectedBranch, selected: true, - unavailable: true, + unavailable: !this.filterBranches, }); } const worktreeUnavailableReason = this.getWorktreeUnavailableReason(); @@ -700,6 +707,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.branchPicker.update({ label: presentation.label, branches, + filterBranches: this.filterBranches, status: this.branchLoadState === 'loadingRepository' || this.branchLoadState === 'loadingBranches' ? 'loading' : this.branchLoadState === 'error' @@ -846,6 +854,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.branchRepoDisposable.clear(); this.repository = undefined; this.branches = []; + this.filterBranches = undefined; this.detachedCommit = undefined; if (!folder) { this.branchLoadState = 'noFolder'; @@ -857,6 +866,46 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.renderBranchControl(); const cts = new CancellationTokenSource(); this.branchRequest.value = cts; + const target = this.sessionsManagementService.getSessionTypesForFolder(folder).find(candidate => + candidate.sessionType.id === this.state.sessionTypeId + && (this.state.providerId === undefined || candidate.providerId === this.state.providerId) + ); + const provider = target && this.sessionsProvidersService.getProvider(target.providerId); + if (provider?.getWorktreeOptions && target) { + try { + const options = await provider.getWorktreeOptions(folder, target.sessionType.id, cts.token); + if (requestId !== this.branchRequestId || cts.token.isCancellationRequested) { + return; + } + this.isolationModel.setHeadBranch(options?.currentBranch); + this.branches = normalizeAutomationBranchNames(options?.branches ?? []); + this.branchLoadState = !options ? 'noRepository' : this.branches.length > 0 ? 'ready' : 'empty'; + if (options) { + this.isolationModel.setSupportsWorktreeConfiguration(options.supportsWorktree); + const loadBranches = options.loadBranches; + this.filterBranches = loadBranches ? async (query, token) => { + try { + const branches = await loadBranches(query, token); + const selectedBranch = this.isolationModel.selectedBranch ?? this.isolationModel.headBranch; + return normalizeAutomationBranchNames(branches).map(name => ({ name, selected: name === selectedBranch })); + } catch (error) { + if (!token.isCancellationRequested) { + this.pickerLogService.error('[AutomationDialog] Failed to filter worktree branches.', error); + } + throw error; + } + } : undefined; + } + } catch (error) { + if (requestId !== this.branchRequestId || cts.token.isCancellationRequested) { + return; + } + this.pickerLogService.error('[AutomationDialog] Failed to load worktree options from the session provider.', error); + this.branchLoadState = 'error'; + } + this.renderBranchControl(); + return; + } let repo: IGitRepository | undefined; try { repo = await this.gitService.openRepository(folder); diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index 0270555a0f3788..940ded39cb4703 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -9,6 +9,7 @@ import { StandardKeyboardEvent } from '../../../../../base/browser/keyboardEvent import { Dialog } from '../../../../../base/browser/ui/dialog/dialog.js'; import { SelectBox } from '../../../../../base/browser/ui/selectBox/selectBox.js'; import { DeferredPromise, timeout } from '../../../../../base/common/async.js'; +import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { StandardMouseEvent } from '../../../../../base/browser/mouseEvent.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { Action, IAction } from '../../../../../base/common/actions.js'; @@ -40,7 +41,8 @@ import { IAutomationSessionTemplate } from '../../../../../workbench/contrib/cha import { GitRefType, IGitRepository, IGitService } from '../../../../../workbench/contrib/git/common/gitService.js'; import { IHostService } from '../../../../../workbench/services/host/browser/host.js'; import { ISession, ISessionWorkspace, SessionTypeAuthRequirement } from '../../../../services/sessions/common/session.js'; -import { IAutomationSessionConfiguration } from '../../../../services/sessions/common/sessionsProvider.js'; +import { IAutomationSessionConfiguration, type ISessionsProvider, type ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; +import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { AutomationIsolationGroupActionViewItem, AutomationSessionDraftSynchronizer, canSelectAutomationWorkspace, getAutomationTargetHint, IFormState, IValidationState, isAutomationDialogPopupTarget, registerAutomationDialogKeyboardNavigation, shouldPassThroughAutomationDialogCommand, updateSaveButtonState } from '../../browser/automationDialog.js'; import { AutomationIsolationModel } from '../../common/isolationGroupModel.js'; @@ -110,6 +112,8 @@ class RecordingActionWidgetService extends mock() { labels: readonly string[] = []; details: ReadonlyArray['detail']> = []; ariaLabels: readonly string[] = []; + filter: ((query: string) => Promise) | undefined; + filterVisible = false; private selectItem: ((label: string) => void) | undefined; private hideWidget: ((didCancel?: boolean) => void) | undefined; @@ -122,7 +126,7 @@ class RecordingActionWidgetService extends mock() { _container: HTMLElement | undefined, _actionBarActions: readonly IAction[], accessibilityProvider?: Partial>>, - _listOptions?: IActionListOptions, + listOptions?: IActionListOptions, ): void { this.isVisible = true; this.labels = items.map(item => item.label ?? ''); @@ -131,6 +135,17 @@ class RecordingActionWidgetService extends mock() { const label = accessibilityProvider?.getAriaLabel?.(item); return typeof label === 'string' ? label : label?.get() ?? ''; }); + this.filterVisible = listOptions?.showFilter === true; + this.filter = delegate.onFilter ? async query => { + const filteredItems = await delegate.onFilter?.(query, CancellationToken.None) ?? []; + this.labels = filteredItems.map(item => item.label ?? ''); + this.selectItem = label => { + const item = filteredItems.find(candidate => candidate.label === label)?.item; + if (item) { + delegate.onSelect(item); + } + }; + } : undefined; this.selectItem = label => { const item = items.find(candidate => candidate.label === label)?.item; if (item) { @@ -739,6 +754,8 @@ suite('Automation branch picker', () => { function createItem(options?: { readonly state?: IFormState; readonly getRefs?: IGitRepository['getRefs']; + readonly getWorktreeOptions?: ISessionsProvider['getWorktreeOptions']; + readonly supportsWorktreeConfiguration?: boolean; readonly failOpenRepositoryOnce?: boolean; readonly providerInitiallyUnavailable?: boolean; readonly revalidate?: () => void; @@ -795,11 +812,14 @@ suite('Automation branch picker', () => { id: state.sessionTypeId ?? 'copilotcli', label: 'Copilot', icon: Codicon.copilot, - supportsWorktreeConfiguration: state.sessionTypeId === 'copilotcli', + supportsWorktreeConfiguration: options?.supportsWorktreeConfiguration ?? true, authRequirement: SessionTypeAuthRequirement.GitHub, }, }] : [], })); + instantiationService.stub(ISessionsProvidersService, { getProvider: () => undefined }, 'getProvider', upcastPartial({ + getWorktreeOptions: options?.getWorktreeOptions, + })); instantiationService.stub(ILogService, new NullLogService()); const action = disposables.add(new Action('test.automationIsolation', 'Automation Isolation')); @@ -1029,7 +1049,8 @@ suite('Automation branch picker', () => { test('normalizes unsupported Worktree targets back to Folder mode', async () => { const { container, model } = createItem({ - state: createFormState({ sessionTypeId: 'claude', branch: 'feature/saved' }), + state: createFormState({ sessionTypeId: 'cloud', branch: 'feature/saved' }), + supportsWorktreeConfiguration: false, }); await timeout(0); @@ -1046,23 +1067,163 @@ suite('Automation branch picker', () => { }); }); - test('enables Worktree branches for agent-host Copilot CLI', async () => { - const { container } = createItem({ - state: createFormState({ providerId: 'local-agent-host', sessionTypeId: 'copilotcli' }), + for (const sessionTypeId of ['copilotcli', 'claude', 'codex', 'custom']) { + test(`selects provider-owned worktree branches for ${sessionTypeId} without local Git`, async () => { + const folderUri = URI.parse('vscode-agent-host://remote/workspace'); + const requests: { folderUri: URI; sessionTypeId: string }[] = []; + const { container, model, actionWidgetService, getOpenRepositoryAttempts } = createItem({ + state: createFormState({ providerId: 'remote-provider', sessionTypeId, folderUri, isolationMode: 'workspace' }), + getWorktreeOptions: async (folderUri, sessionTypeId) => { + requests.push({ folderUri, sessionTypeId }); + return { supportsWorktree: true, currentBranch: 'main', branches: ['release', 'main', 'copilot-worktree-generated'] }; + }, + }); + await timeout(0); + container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); + container.querySelector('.automation-form-branch-slot')!.click(); + actionWidgetService.select('release'); + + assert.deepStrictEqual({ + requests, + localGitRequests: getOpenRepositoryAttempts(), + branches: actionWidgetService.labels, + mode: model.isolationMode, + branch: model.persistedBranch, + checked: container.querySelector('.monaco-checkbox')?.getAttribute('aria-checked'), + }, { + requests: [{ folderUri, sessionTypeId }], + localGitRequests: 0, + branches: ['main', 'release'], + mode: 'worktree', + branch: 'release', + checked: 'true', + }); + }); + } + + for (const options of [undefined, { supportsWorktree: false, currentBranch: 'main', branches: ['main'] }] satisfies (ISessionWorktreeOptions | undefined)[]) { + test(`disables provider worktrees when ${options ? 'the schema does not support them' : 'no repository exists'}`, async () => { + const { container, model, getOpenRepositoryAttempts } = createItem({ + state: createFormState({ isolationMode: 'workspace', branch: undefined }), + getWorktreeOptions: async () => options, + }); + await timeout(0); + container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); + + assert.deepStrictEqual({ + mode: model.isolationMode, + branch: model.persistedBranch, + localGitRequests: getOpenRepositoryAttempts(), + }, { mode: 'workspace', branch: undefined, localGitRequests: 0 }); + }); + } + + test('retries provider branch failures without falling back to local Git', async () => { + let attempts = 0; + const { container, actionWidgetService, getOpenRepositoryAttempts } = createItem({ + getWorktreeOptions: async () => { + if (++attempts === 1) { + throw new Error('Remote host unavailable'); + } + return { supportsWorktree: true, currentBranch: 'remote-main', branches: ['remote-main'] }; + }, }); await timeout(0); - const trigger = container.querySelector('.automation-form-branch-slot'); - assert.ok(trigger); + container.querySelector('.automation-form-branch-slot')!.click(); + actionWidgetService.select('Retry Loading Branches'); + await timeout(0); assert.deepStrictEqual({ - disabled: trigger.getAttribute('aria-disabled'), - label: trigger.querySelector('.automation-form-branch-name')?.textContent, + attempts, + localGitRequests: getOpenRepositoryAttempts(), + branch: container.querySelector('.automation-form-branch-name')?.textContent, + }, { attempts: 2, localGitRequests: 0, branch: 'remote-main' }); + }); + + test('queries the provider for branches beyond its initial completion window', async () => { + const queries: string[] = []; + let resolutionCount = 0; + const { container, actionWidgetService, model } = createItem({ + state: createFormState({ branch: 'saved/base' }), + getWorktreeOptions: async () => { + resolutionCount++; + return { + supportsWorktree: true, currentBranch: 'main', branches: ['main'], + loadBranches: async query => { + queries.push(query); + return ['release/long-lived']; + }, + }; + }, + }); + await timeout(0); + container.querySelector('.automation-form-branch-slot')!.click(); + const savedBranchDetail = actionWidgetService.details[0]; + await actionWidgetService.filter?.('release'); + const filteredBranches = actionWidgetService.labels; + actionWidgetService.select('release/long-lived'); + container.querySelector('.automation-form-branch-slot')!.click(); + + assert.deepStrictEqual({ + resolutionCount, + queries, + filterVisible: actionWidgetService.filterVisible, + filteredBranches, + selected: model.persistedBranch, + savedBranchDetail, + reopenedDetails: actionWidgetService.details, }, { - disabled: 'false', - label: 'main', + resolutionCount: 1, + queries: ['release'], + filterVisible: true, + filteredBranches: ['release/long-lived'], + selected: 'release/long-lived', + savedBranchDetail: undefined, + reopenedDetails: [undefined, undefined], }); }); + test('does not select stale results while provider branch search is pending', async () => { + const branches = new DeferredPromise(); + const { container, actionWidgetService, model } = createItem({ + getWorktreeOptions: async () => ({ + supportsWorktree: true, currentBranch: 'main', branches: ['main'], + loadBranches: async () => branches.p, + }), + }); + await timeout(0); + container.querySelector('.automation-form-branch-slot')!.click(); + const filtering = actionWidgetService.filter?.('release'); + actionWidgetService.select('main'); + const pending = { + labels: actionWidgetService.labels, + selected: model.selectedBranch, + pickerVisible: actionWidgetService.isVisible, + }; + await branches.complete(['release']); + await filtering; + actionWidgetService.select('release'); + + assert.deepStrictEqual({ pending, selected: model.selectedBranch }, { + pending: { labels: ['Loading branches…'], selected: undefined, pickerVisible: true }, + selected: 'release', + }); + }); + + test('surfaces provider branch search failures as retry actions', async () => { + const { container, actionWidgetService } = createItem({ + getWorktreeOptions: async () => ({ + supportsWorktree: true, currentBranch: 'main', branches: ['main'], + loadBranches: async () => { throw new Error('Remote branch search failed'); }, + }), + }); + await timeout(0); + container.querySelector('.automation-form-branch-slot')!.click(); + await actionWidgetService.filter?.('release'); + + assert.deepStrictEqual(actionWidgetService.labels, ['Retry Loading Branches']); + }); + test('preserves Worktree intent while the provider is discovered late', async () => { const { container, model, setProviderAvailable } = createItem({ state: createFormState({ branch: 'feature/saved' }), @@ -1084,6 +1245,7 @@ suite('Automation branch picker', () => { }); setProviderAvailable(); + await timeout(0); assert.deepStrictEqual({ mode: model.isolationMode, diff --git a/src/vs/sessions/contrib/chat/browser/branchPicker.ts b/src/vs/sessions/contrib/chat/browser/branchPicker.ts index 9368cad17cdae0..788e5c3e055803 100644 --- a/src/vs/sessions/contrib/chat/browser/branchPicker.ts +++ b/src/vs/sessions/contrib/chat/browser/branchPicker.ts @@ -7,6 +7,8 @@ import * as dom from '../../../../base/browser/dom.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; +import { Delayer } from '../../../../base/common/async.js'; +import { CancellationToken } from '../../../../base/common/cancellation.js'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { localize } from '../../../../nls.js'; import { IActionWidgetService } from '../../../../platform/actionWidget/browser/actionWidget.js'; @@ -32,6 +34,7 @@ export interface IBranchPickerState { readonly missing?: boolean; readonly showChevron?: boolean; readonly isolation?: IBranchPickerIsolationState; + readonly filterBranches?: (query: string, token: CancellationToken) => Promise; } /** @@ -80,6 +83,7 @@ interface IBranchPickerItem { */ export class BranchPicker extends Disposable { private readonly _renderDisposables = this._register(new DisposableStore()); + private readonly _filterDelayer = this._register(new Delayer[]>(200)); private _state: IBranchPickerState = { label: localize('branchPicker.select', "Branch"), branches: [], @@ -207,8 +211,13 @@ export class BranchPicker extends Disposable { } const trigger = this._triggerElement; + const filterBranches = this._state.filterBranches; + let filterPending = false; const delegate: IActionListDelegate = { onSelect: item => { + if (filterPending) { + return; + } this._actionWidgetService.hide(); if (item.kind === 'retry') { this._options.onRetry?.(); @@ -216,7 +225,27 @@ export class BranchPicker extends Disposable { this._options.onSelectBranch(item.name); } }, + onFilter: filterBranches ? (query, token) => { + filterPending = true; + this._actionWidgetService.updateItems(this._getItems({ ...this._state, status: 'loading' })); + return this._filterDelayer.trigger(async () => { + try { + const branches = await filterBranches(query, token); + return this._getItems({ ...this._state, branches, status: branches.length > 0 ? 'ready' : 'empty' }); + } catch (error) { + if (token.isCancellationRequested) { + throw error; + } + return this._getItems({ ...this._state, status: 'error' }); + } finally { + if (!token.isCancellationRequested) { + filterPending = false; + } + } + }); + } : undefined, onHide: () => { + this._filterDelayer.cancel(); this._isOpen = false; trigger?.setAttribute('aria-expanded', 'false'); if (trigger?.isConnected) { @@ -246,19 +275,20 @@ export class BranchPicker extends Disposable { }, getWidgetAriaLabel: () => localize('branchPicker.ariaLabel', "Branch Picker"), }, - branchCount > FILTER_THRESHOLD + filterBranches || branchCount > FILTER_THRESHOLD ? { showFilter: true, filterPlaceholder: localize('branchPicker.filter', "Filter branches…") } : undefined, ); } - private _getItems(): readonly IActionListItem[] { - switch (this._state.status) { + private _getItems(state = this._state): readonly IActionListItem[] { + switch (state.status) { case 'loading': return [{ kind: ActionListItemKind.Action, label: localize('branchPicker.loading', "Loading branches…"), disabled: true, + showAlways: true, item: { kind: 'branch' }, }]; case 'error': @@ -277,7 +307,7 @@ export class BranchPicker extends Disposable { item: { kind: 'branch' }, }]; case 'ready': - return this._state.branches.map(branch => ({ + return state.branches.map(branch => ({ kind: ActionListItemKind.Action, label: branch.name, detail: branch.unavailable ? localize('branchPicker.unavailable', "Unavailable locally") : undefined, diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index bf33c7cd905590..4d836201c40a09 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -65,7 +65,7 @@ import { linkKey } from '../../../../common/sessionLinks.js'; import { ChatInteractivity, ChatModelSource, ChatOriginKind, DEFAULT_CHAT_CAPABILITIES, effectiveChatInteractivity, getGitHubPullRequestRefs, getHighestPriorityPullRequestIcon, IChat, IChatCapabilities, IGitHubInfo, IGitHubIssueRef, IGitHubPullRequestRef, isActiveSessionStatus, ISession, ISessionAgentRef, ISessionArtifact, ISessionCapabilities, ISessionChangesSummary, ISessionChatCustomization, ISessionChangeset, ISessionCreationReference, ISessionFileChange, ISessionTurnFileChange, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, ISideChatSelection, sessionFileChangesEqual, sessionWorkspaceEqual, SessionRemoteConnectionFailureReason, SessionRemoteConnectionStatus, SessionStatus, SessionTypeAuthRequirement, toSessionId, TURN_CHANGES_CHANGESET_ID } from '../../../../services/sessions/common/session.js'; import { dedupeLinks, partitionSessionArtifacts } from './agentHostSessionArtifacts.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; -import { IAutomationSessionConfiguration, IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot, ISessionsProviderCreateSessionOptions, ISessionWorktreeConfiguration } from '../../../../services/sessions/common/sessionsProvider.js'; +import { IAutomationSessionConfiguration, IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot, ISessionsProviderCreateSessionOptions, ISessionWorktreeConfiguration, type ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; import { computePullRequestRefPresentation } from '../../../github/browser/pullRequestIconStatus.js'; import { IPullRequestIconCache } from '../../../github/browser/pullRequestIconCache.js'; @@ -4242,6 +4242,46 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement return this._getNewSession(sessionId)?.getConfigValues(); } + async getWorktreeOptions(folderUri: URI, sessionTypeId: string, token: CancellationToken): Promise { + const connection = this.connection; + if (!connection) { + throw new Error(`[${this.id}] Cannot resolve worktree options without an agent host connection`); + } + const resolved = await raceCancellationError(connection.resolveSessionConfig({ + provider: sessionTypeId, + workingDirectory: folderUri, + config: { [SessionConfigKey.Isolation]: 'folder' }, + }), token); + const branchSchema = resolved.schema.properties[SessionConfigKey.Branch]; + if (!branchSchema) { + return undefined; + } + const isolationSchema = resolved.schema.properties[SessionConfigKey.Isolation]; + const currentBranch = resolved.values[SessionConfigKey.Branch]; + const loadBranches = branchSchema.enumDynamic ? async (query: string, token: CancellationToken): Promise => { + if (this.connection !== connection) { + throw new Error(`[${this.id}] The agent host connection changed while loading branches`); + } + const result = await raceCancellationError(connection.sessionConfigCompletions({ + provider: sessionTypeId, + workingDirectory: folderUri, + config: resolved.values, + property: SessionConfigKey.Branch, + query: query || undefined, + }), token); + return result.items.map(item => item.value); + } : undefined; + const branches = loadBranches + ? await loadBranches('', token) + : (branchSchema.enum ?? []).filter((branch): branch is string => typeof branch === 'string'); + return { + supportsWorktree: isolationSchema?.enum?.includes('worktree') === true && !isolationSchema.readOnly, + currentBranch: typeof currentBranch === 'string' ? currentBranch : undefined, + branches, + ...(loadBranches ? { loadBranches } : {}), + }; + } + async setIsolationMode(sessionId: string, mode: string): Promise { const policyRestricted = isAutoApprovePolicyRestricted(this._baseConfigurationService); const value = normalizeSessionConfigValue( diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 71d27f60bbbbfd..ddb3d7a5c00c5e 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -768,6 +768,32 @@ suite('LocalAgentHostSessionsProvider', () => { }))); }); + test('resolves workspace-specific worktree support from the host schema', async () => { + const provider = createProvider(disposables, agentHost); + const results = []; + for (const supportsWorktree of [true, false]) { + agentHost.resolveSessionConfigResult = { + schema: { + type: 'object', + properties: { + isolation: { type: 'string', title: 'Isolation', enum: supportsWorktree ? ['folder', 'worktree'] : ['folder'] }, + branch: { type: 'string', title: 'Branch', enum: ['main', 'release'] }, + }, + }, + values: { isolation: 'folder', branch: 'main' }, + }; + results.push(await provider.getWorktreeOptions(URI.file('/workspace'), 'claude', CancellationToken.None)); + } + agentHost.resolveSessionConfigResult = { schema: { type: 'object', properties: {} }, values: {} }; + results.push(await provider.getWorktreeOptions(URI.file('/workspace'), 'claude', CancellationToken.None)); + + assert.deepStrictEqual(results, [ + { supportsWorktree: true, currentBranch: 'main', branches: ['main', 'release'] }, + { supportsWorktree: false, currentBranch: 'main', branches: ['main', 'release'] }, + undefined, + ]); + }); + test('shares the root-state listener across session adapters', () => { agentHost.setAgents([{ provider: 'copilotcli', displayName: 'Copilot', description: '', models: [], capabilities: {} } as AgentInfo]); const provider = createProvider(disposables, agentHost); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index dd5071d0cf29bf..898e3f554c2599 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -5,6 +5,7 @@ import assert from 'assert'; import { DeferredPromise, timeout } from '../../../../../../base/common/async.js'; +import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { Codicon } from '../../../../../../base/common/codicons.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; @@ -113,7 +114,7 @@ class MockAgentConnection extends mock() { return uri; } - override async resolveSessionConfig(): Promise { + override async resolveSessionConfig(_request: Parameters[0]): Promise { await Promise.resolve(); if (this.failResolveSessionConfig) { throw new Error('resolveSessionConfig unavailable'); @@ -434,6 +435,60 @@ suite('RemoteAgentHostSessionsProvider', () => { ]); }); + test('resolves worktree branches on the selected remote host for every agent', async () => { + const agentIds = ['copilotcli', 'claude', 'codex', 'custom']; + connection.setAgents(agentIds.map(provider => ({ provider, displayName: provider, description: '', models: [] }))); + const provider = createProvider(disposables, connection); + const folderUri = URI.parse('vscode-agent-host://localhost__4321/home/user/project'); + const resolveRequests: Parameters[0][] = []; + const completionRequests: Parameters[0][] = []; + connection.resolveSessionConfig = async request => { + resolveRequests.push(request); + return { + schema: { + type: 'object', + properties: { + isolation: { type: 'string', title: 'Isolation', enum: ['folder', 'worktree'] }, + branch: { type: 'string', title: 'Branch', enumDynamic: true }, + }, + }, + values: { isolation: 'folder', branch: 'remote-head' }, + }; + }; + connection.sessionConfigCompletions = async request => { + completionRequests.push(request); + return { + items: ['remote-head', 'release'] + .filter(name => !request.query || name.includes(request.query)) + .map(value => ({ value, label: value })), + }; + }; + const results = []; + for (const agentId of agentIds) { + const options = await provider.getWorktreeOptions(folderUri, agentId, CancellationToken.None); + results.push({ + supportsWorktree: options?.supportsWorktree, + currentBranch: options?.currentBranch, + branches: options?.branches, + search: await options?.loadBranches?.('release', CancellationToken.None), + }); + } + + assert.deepStrictEqual({ + capabilities: provider.sessionTypes.map(type => type.supportsWorktreeConfiguration), + results, + resolveRequests, + completionRequests, + }, { + capabilities: agentIds.map(() => true), + results: agentIds.map(() => ({ supportsWorktree: true, currentBranch: 'remote-head', branches: ['remote-head', 'release'], search: ['release'] })), + resolveRequests: agentIds.map(provider => ({ provider, workingDirectory: folderUri, config: { isolation: 'folder' } })), + completionRequests: agentIds.flatMap(provider => [undefined, 'release'].map(query => ({ + provider, workingDirectory: folderUri, config: { isolation: 'folder', branch: 'remote-head' }, property: 'branch', query, + }))), + }); + }); + test('session-type labels omit host suffix on web', () => { const provider = createProvider(disposables, connection, { address: '10.0.0.1:8080', connectionName: 'My Host', isWebPlatform: true }); diff --git a/src/vs/sessions/services/sessions/common/sessionsProvider.ts b/src/vs/sessions/services/sessions/common/sessionsProvider.ts index 72f3db9fe9d960..641dbe0a50c84b 100644 --- a/src/vs/sessions/services/sessions/common/sessionsProvider.ts +++ b/src/vs/sessions/services/sessions/common/sessionsProvider.ts @@ -73,6 +73,15 @@ export interface ISessionWorktreeConfiguration { readonly branch?: string; } +/** Repository branch choices and worktree support at a provider-owned workspace. */ +export interface ISessionWorktreeOptions { + readonly supportsWorktree: boolean; + readonly currentBranch: string | undefined; + readonly branches: readonly string[]; + /** Present when the initial branches are a partial result set. */ + readonly loadBranches?: (query: string, token: CancellationToken) => Promise; +} + /** * Presentation options for the sessions-core model picker. A provider returns * these from {@link ISessionsProvider.getModelPickerOptions} so it controls how @@ -424,6 +433,9 @@ export interface ISessionsProvider { */ setWorktreeConfiguration?(sessionId: string, configuration: ISessionWorktreeConfiguration): Promise; + /** Resolves worktree choices without creating a session; undefined means no repository was found. */ + getWorktreeOptions?(folderUri: URI, sessionTypeId: string, token: CancellationToken): Promise; + /** * Set whether the worktree branch tracks its upstream for a session. * @param sessionId The ID of the session. From 3ef1c18e5ebc32197a7f7cddc48a1708dccde712 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Tue, 8 Sep 2026 11:39:47 +0200 Subject: [PATCH 5/8] automations: fix: allow leaving unavailable worktree isolation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../automations/browser/automationDialog.ts | 2 +- .../test/browser/automationDialog.test.ts | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 0c4cbcfae61adc..1f8892428c2be3 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -702,7 +702,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { } const worktreeUnavailableReason = this.getWorktreeUnavailableReason(); const isolationState: 'enabled' | 'disabled' | 'hidden' = - worktreeUnavailableReason === undefined ? 'enabled' : 'disabled'; + worktreeUnavailableReason === undefined || this.isolationModel.isolationMode === 'worktree' ? 'enabled' : 'disabled'; this.branchPicker.update({ label: presentation.label, diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index 940ded39cb4703..83d397bcb1aff2 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -1118,6 +1118,28 @@ suite('Automation branch picker', () => { }); } + test('allows a saved worktree target to opt back into Folder when its workspace no longer supports worktrees', async () => { + const { container, model } = createItem({ + state: createFormState({ isolationMode: 'worktree', branch: 'release' }), + getWorktreeOptions: async () => ({ supportsWorktree: false, currentBranch: 'main', branches: ['main'] }), + }); + await timeout(0); + const before = { mode: model.isolationMode, branch: model.persistedBranch }; + container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); + const after = { mode: model.isolationMode, branch: model.persistedBranch }; + container.querySelector('.sessions-chat-isolation-checkbox .action-label')!.click(); + + assert.deepStrictEqual({ + before, + after, + cannotReenable: model.isolationMode, + }, { + before: { mode: 'worktree', branch: undefined }, + after: { mode: 'workspace', branch: undefined }, + cannotReenable: 'workspace', + }); + }); + test('retries provider branch failures without falling back to local Git', async () => { let attempts = 0; const { container, actionWidgetService, getOpenRepositoryAttempts } = createItem({ From 00eea67533c542a74494edddcc473cd762a702e4 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Tue, 8 Sep 2026 11:39:51 +0200 Subject: [PATCH 6/8] automations: fix: coalesce repository option reloads Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../automations/browser/automationDialog.ts | 25 ++++++++++++++----- .../test/browser/automationDialog.test.ts | 25 ++++++++++++++++++- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index 1f8892428c2be3..a3a802759ce0ce 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as DOM from '../../../../base/browser/dom.js'; -import { raceCancellationError, raceTimeout } from '../../../../base/common/async.js'; +import { raceCancellationError, raceTimeout, RunOnceScheduler } from '../../../../base/common/async.js'; import { BaseActionViewItem, IBaseActionViewItemOptions } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { IButton } from '../../../../base/browser/ui/button/button.js'; @@ -561,6 +561,9 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { private readonly renderDisposables = this._register(new DisposableStore()); private readonly branchRepoDisposable = this._register(new MutableDisposable()); private readonly branchRequest = this._register(new MutableDisposable()); + private readonly repositoryReloadScheduler = this._register(new RunOnceScheduler(() => { + void this.reloadRepository(this.isolationModel.folderUri); + }, 0)); private branchRequestId = 0; private readonly branchPicker: BranchPicker; private branchLoadState: BranchLoadState = 'noFolder'; @@ -600,7 +603,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.renderBranchControl(); }, onRetry: () => { - void this.reloadRepository(this.isolationModel.folderUri); + this.scheduleRepositoryReload(); }, isolation: { label: localize('automation.form.isolation.worktree', "New Worktree"), @@ -616,6 +619,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { override render(container: HTMLElement): void { this.renderDisposables.clear(); this.branchRepoDisposable.clear(); + this.repositoryReloadScheduler.cancel(); this.cancelBranchRequest(); DOM.clearNode(container); container.style.marginLeft = 'auto'; @@ -632,20 +636,21 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.refreshTargetCapability(); this.renderBranchControl(); this.renderDisposables.add(autorun(reader => { - const folderUri = this.workspaceFolder.read(reader); + this.workspaceFolder.read(reader); this.refreshTargetAndRender(); - void this.reloadRepository(folderUri); + this.scheduleRepositoryReload(); })); this.renderDisposables.add(this.onDidChangeTarget(() => { this.refreshTargetAndRender(); - void this.reloadRepository(this.isolationModel.folderUri); + this.scheduleRepositoryReload(); })); this.renderDisposables.add(this.sessionsManagementService.onDidChangeSessionTypes(() => { this.refreshTargetAndRender(); - void this.reloadRepository(this.isolationModel.folderUri); + this.scheduleRepositoryReload(); })); this.renderDisposables.add({ dispose: () => { + this.repositoryReloadScheduler.cancel(); this.cancelBranchRequest(); } }); @@ -848,6 +853,14 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { this.branchRequest.clear(); } + private scheduleRepositoryReload(): void { + this.cancelBranchRequest(); + this.branchRepoDisposable.clear(); + this.branchLoadState = this.isolationModel.folderUri ? 'loadingRepository' : 'noFolder'; + this.renderBranchControl(); + this.repositoryReloadScheduler.schedule(); + } + private async reloadRepository(folder: URI | undefined): Promise { const requestId = ++this.branchRequestId; this.cancelBranchRequest(); diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index 83d397bcb1aff2..1635daf7261565 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -758,6 +758,7 @@ suite('Automation branch picker', () => { readonly supportsWorktreeConfiguration?: boolean; readonly failOpenRepositoryOnce?: boolean; readonly providerInitiallyUnavailable?: boolean; + readonly onDidChangeTarget?: Event; readonly revalidate?: () => void; readonly visible?: boolean; }): { @@ -829,7 +830,7 @@ suite('Automation branch picker', () => { state, model, model.folderUriObs, - Event.None, + options?.onDidChangeTarget ?? Event.None, options?.revalidate ?? (() => { }), undefined, visible, @@ -1162,6 +1163,28 @@ suite('Automation branch picker', () => { }, { attempts: 2, localGitRequests: 0, branch: 'remote-main' }); }); + test('coalesces target and session-type notifications into one provider repository lookup', async () => { + const targetChanged = disposables.add(new Emitter()); + const requests: string[] = []; + const { state, model, setProviderAvailable } = createItem({ + onDidChangeTarget: targetChanged.event, + getWorktreeOptions: async (_folder, sessionTypeId) => { + requests.push(sessionTypeId); + return { supportsWorktree: true, currentBranch: `${sessionTypeId}-main`, branches: [`${sessionTypeId}-main`] }; + }, + }); + await timeout(0); + state.sessionTypeId = 'claude'; + targetChanged.fire(); + setProviderAvailable(); + await timeout(0); + + assert.deepStrictEqual({ + requests, + branch: model.persistedBranch, + }, { requests: ['copilotcli', 'claude'], branch: 'claude-main' }); + }); + test('queries the provider for branches beyond its initial completion window', async () => { const queries: string[] = []; let resolutionCount = 0; From cc892bc126bdcd597ed5a9fe775d862e9ebe1032 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Thu, 10 Sep 2026 17:58:55 +0200 Subject: [PATCH 7/8] automations: fix: validate workspace worktree support in tools Consult provider-owned workspace capabilities before accepting an explicit worktree target. Preserve legacy providers and unrelated edits, propagate lookup failures, and cancel pending validation without writing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../automations/browser/automationTools.ts | 29 ++- .../test/browser/automationTools.test.ts | 236 ++++++++++++++++-- 2 files changed, 238 insertions(+), 27 deletions(-) diff --git a/src/vs/sessions/contrib/automations/browser/automationTools.ts b/src/vs/sessions/contrib/automations/browser/automationTools.ts index 40d3754581ab02..6a948d1ce08210 100644 --- a/src/vs/sessions/contrib/automations/browser/automationTools.ts +++ b/src/vs/sessions/contrib/automations/browser/automationTools.ts @@ -3,8 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { raceCancellationError } from '../../../../base/common/async.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { Codicon } from '../../../../base/common/codicons.js'; +import { isCancellationError } from '../../../../base/common/errors.js'; import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; @@ -24,6 +26,7 @@ import { ChatPermissionLevel } from '../../../../workbench/contrib/chat/common/c import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress } from '../../../../workbench/contrib/chat/common/tools/languageModelToolsService.js'; import { ISession } from '../../../services/sessions/common/session.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; +import { ISessionsProvidersService } from '../../../services/sessions/browser/sessionsProvidersService.js'; export const ListAutomationsToolId = 'vscode_listAutomations'; export const ConfigureAutomationToolId = 'vscode_configureAutomation'; @@ -365,6 +368,7 @@ export class ConfigureAutomationTool implements IToolImpl { @IAutomationService private readonly automationService: IAutomationService, @ISessionsManagementService private readonly sessionsManagementService: ISessionsManagementService, @IConfigurationService private readonly configurationService: IConfigurationService, + @ISessionsProvidersService private readonly sessionsProvidersService: ISessionsProvidersService, ) { } getToolData(): IToolData { @@ -578,14 +582,14 @@ The change uses the current tool-approval policy. When approval is required, the try { if (proposal.kind === 'create') { const target = proposal.validateTargetAvailability - ? this.resolveAvailableTarget(proposal.initialValues.target) + ? await this.resolveAvailableTarget(proposal.initialValues.target, token) : proposal.initialValues.target; return await this.applyCreate({ ...proposal.initialValues, target }, token); } const target = proposal.initialValues.target ? proposal.validateTargetAvailability - ? this.resolveAvailableTarget(proposal.initialValues.target) + ? await this.resolveAvailableTarget(proposal.initialValues.target, token) : proposal.initialValues.target : undefined; const patch = target ? { ...proposal.initialValues, target } : proposal.initialValues; @@ -600,6 +604,9 @@ The change uses the current tool-approval policy. When approval is required, the if (error instanceof AutomationToolMutationBlockedError) { return error.result; } + if (token.isCancellationRequested && isCancellationError(error)) { + return automationToolCancelled(); + } if (error instanceof AutomationToolInputError) { return automationToolError(error.message); } @@ -660,7 +667,7 @@ The change uses the current tool-approval policy. When approval is required, the return undefined; } - private resolveAvailableTarget(target: AutomationTarget): AutomationTarget { + private async resolveAvailableTarget(target: AutomationTarget, token: CancellationToken): Promise { const candidates = target.kind === 'quickChat' ? this.sessionsManagementService.getQuickChatSessionTypes() : this.sessionsManagementService.getSessionTypesForFolder(target.folderUri); @@ -670,8 +677,20 @@ The change uses the current tool-approval policy. When approval is required, the ? `The quick-chat target "${target.providerId}/${target.sessionTypeId}" is not available.` : 'The proposed workspace target is not available for the selected provider and session type.'); } - if (target.kind === 'workspace' && target.isolation.kind === 'worktree' && !candidate.sessionType.supportsWorktreeConfiguration) { - throw new AutomationToolInputError(`Session type "${candidate.sessionType.id}" does not support worktree isolation.`); + if (target.kind === 'workspace' && target.isolation.kind === 'worktree') { + if (!candidate.sessionType.supportsWorktreeConfiguration) { + throw new AutomationToolInputError(`Session type "${candidate.sessionType.id}" does not support worktree isolation.`); + } + const provider = this.sessionsProvidersService.getProvider(candidate.providerId); + if (!provider) { + throw new AutomationToolInputError(`Sessions provider "${candidate.providerId}" is no longer available.`); + } + if (provider.getWorktreeOptions) { + const options = await raceCancellationError(provider.getWorktreeOptions(target.folderUri, candidate.sessionType.id, token), token); + if (!options?.supportsWorktree) { + throw new AutomationToolInputError('The selected workspace does not support worktree isolation.'); + } + } } return { ...target, diff --git a/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts index 060c6ef4990b22..6a6e5a801271ee 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationTools.test.ts @@ -11,7 +11,9 @@ 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 { ConfirmationOptionKind } from '../../../../../platform/agentHost/common/state/protocol/channels-chat/state.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { NullLogService } from '../../../../../platform/log/common/log.js'; import { InMemoryStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; @@ -22,6 +24,8 @@ import { ChatAutomationsEnabledContext, CHAT_AUTOMATIONS_ENABLED_SETTING } from import { IToolImpl, IToolInvocation, IToolResult, ToolProgress } from '../../../../../workbench/contrib/chat/common/tools/languageModelToolsService.js'; import { IChat, ISession, ISessionType, ISessionWorkspace } from '../../../../services/sessions/common/session.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; +import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; +import type { ISessionsProvider, ISessionWorktreeOptions } from '../../../../services/sessions/common/sessionsProvider.js'; import { AutomationService } from '../../browser/automationService.js'; import { ConfigureAutomationTool, ConfigureAutomationToolId, DeleteAutomationTool, DeleteAutomationToolId, ListAutomationsTool, ListAutomationsToolId, RunAutomationTool, RunAutomationToolId } from '../../browser/automationTools.js'; import { AUTOMATION_STORAGE_KEY, IAutomationStorageCompareAndSwapResult, IAutomationStorageService } from '../../common/automationStorageService.js'; @@ -341,6 +345,20 @@ suite('AutomationTools', () => { return teardown.add(new AutomationService(storageService, new NullLogService(), automationStorageService)); } + function createConfigureTool( + automationService: IAutomationService, + sessionsManagementService: ISessionsManagementService, + configurationService: IConfigurationService, + provider?: ISessionsProvider, + ): ConfigureAutomationTool { + const instantiationService = teardown.add(new TestInstantiationService()); + instantiationService.stub(IAutomationService, automationService); + instantiationService.stub(ISessionsManagementService, sessionsManagementService); + instantiationService.stub(IConfigurationService, configurationService); + instantiationService.stub(ISessionsProvidersService, { getProvider: () => undefined }, 'getProvider', provider); + return instantiationService.createInstance(ConfigureAutomationTool); + } + test('tool data is gated by AI and Automations context keys', () => { const automationService = new FakeAutomationService(); const configurationService = createConfigurationService(); @@ -351,7 +369,7 @@ suite('AutomationTools', () => { ).getToolData(); const listData = new ListAutomationsTool(automationService, configurationService).getToolData(); const deleteData = new DeleteAutomationTool(automationService, configurationService).getToolData(); - const configureData = new ConfigureAutomationTool( + const configureData = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), configurationService, @@ -397,7 +415,7 @@ suite('AutomationTools', () => { }); test('configureAutomation tool data requires explicit creation intent', () => { - const modelDescription = new ConfigureAutomationTool( + const modelDescription = createConfigureTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -760,7 +778,7 @@ suite('AutomationTools', () => { test('configureAutomation prepares normal create and update confirmations', async () => { const existing = createAutomation(); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( new FakeAutomationService([existing]), new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -819,7 +837,7 @@ suite('AutomationTools', () => { sessionTypeId: 'copilot', }; const schedule: IAutomationSchedule = { interval: 'daily', scheduleHour: 8, scheduleMinute: 30, scheduleDay: 1 }; - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(createSession({ quickChat: true }), true), createConfigurationService(), @@ -857,7 +875,7 @@ suite('AutomationTools', () => { test('configureAutomation applies a partial guarded update and returns clickable result data', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -906,7 +924,7 @@ suite('AutomationTools', () => { test('configureAutomation accepts a provider mode returned by listAutomations', async () => { const existing = createAutomation({ mode: 'autopilot' }); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -937,7 +955,7 @@ suite('AutomationTools', () => { }, }); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -971,7 +989,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(); const listed = await invoke(new ListAutomationsTool(automationService, configurationService), {}); const returnedTemplate = JSON.parse(getText(listed)).automations[0].sessionTemplate; - const tool = new ConfigureAutomationTool(automationService, new FakeSessionsManagementService(undefined), configurationService); + const tool = createConfigureTool(automationService, new FakeSessionsManagementService(undefined), configurationService); await invoke(tool, { automationId: existing.id, sessionTemplate: returnedTemplate }); assert.deepStrictEqual(automationService.updated, [{ id: existing.id, patch: { sessionTemplate } }]); @@ -979,7 +997,7 @@ suite('AutomationTools', () => { test('configureAutomation validates and bounds model-specific configuration', async () => { const automationService = new FakeAutomationService(); - const tool = new ConfigureAutomationTool(automationService, new FakeSessionsManagementService(undefined), createConfigurationService()); + const tool = createConfigureTool(automationService, new FakeSessionsManagementService(undefined), createConfigurationService()); const errors: IToolResult['toolResultError'][] = []; for (const sessionTemplate of [ { modelConfiguration: { thinkingLevel: 'low' } }, @@ -1018,7 +1036,7 @@ suite('AutomationTools', () => { }, }); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1045,7 +1063,7 @@ suite('AutomationTools', () => { throw new AutomationSessionTemplateAuthorityError(); } }([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1062,7 +1080,7 @@ suite('AutomationTools', () => { test('configureAutomation rejects editable changes made while awaiting approval', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1091,7 +1109,7 @@ suite('AutomationTools', () => { test('configureAutomation permits runtime metadata changes while awaiting approval', async () => { const existing = createAutomation(); const automationService = new FakeAutomationService([existing]); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1122,7 +1140,7 @@ suite('AutomationTools', () => { test('configureAutomation validates explicit targets before writing', async () => { const automationService = new FakeAutomationService(); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService( undefined, @@ -1155,11 +1173,185 @@ suite('AutomationTools', () => { }); }); + for (const operation of ['create', 'update'] as const) { + for (const supportsWorktree of [true, false, undefined]) { + test(`configureAutomation ${operation} validates workspace worktree support: ${supportsWorktree}`, async () => { + const existing = createAutomation(); + const automationService = new FakeAutomationService([existing]); + const folderUri = URI.parse('vscode-agent-host://remote/workspace'); + const providerId = 'remote-provider'; + const sessionTypeId = 'claude'; + const requests: Array<{ folderUri: URI; sessionTypeId: string; token: CancellationToken }> = []; + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType(providerId, sessionTypeId, true)]), + createConfigurationService(), + upcastPartial({ + id: providerId, + getWorktreeOptions: async (folderUri, sessionTypeId, token) => { + requests.push({ folderUri, sessionTypeId, token }); + return supportsWorktree === undefined ? undefined : { supportsWorktree, currentBranch: 'main', branches: ['main'] }; + }, + }), + ); + const result = await invoke(tool, { + ...(operation === 'create' + ? { name: 'Worktree automation', prompt: 'Review the repository', schedule: { interval: 'manual' } } + : { automationId: existing.id }), + target: { kind: 'workspace', folderUri: folderUri.toString(), providerId, sessionTypeId, isolation: 'worktree', branch: 'main' }, + }); + const target: AutomationTarget = { kind: 'workspace', folderUri, providerId, sessionTypeId, isolation: { kind: 'worktree', branch: 'main' } }; + + assert.deepStrictEqual({ + requests: requests.map(request => ({ ...request, folderUri: request.folderUri.toString() })), + error: result.toolResultError, + created: automationService.created.map(automation => automation.target), + updated: automationService.updated.map(update => update.patch.target), + }, { + requests: [{ folderUri: folderUri.toString(), sessionTypeId, token: CancellationToken.None }], + error: supportsWorktree ? undefined : 'The selected workspace does not support worktree isolation.', + created: supportsWorktree && operation === 'create' ? [target] : [], + updated: supportsWorktree && operation === 'update' ? [target] : [], + }); + }); + } + } + + test('configureAutomation preserves worktree support for providers without workspace option discovery', async () => { + const automationService = new FakeAutomationService(); + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType('fallback-provider', 'copilotcli', true)]), + createConfigurationService(), + upcastPartial({ id: 'fallback-provider' }), + ); + const result = await invoke(tool, { + name: 'Fallback worktree', prompt: 'Review the repository', schedule: { interval: 'manual' }, + target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, + }); + + assert.deepStrictEqual({ + error: result.toolResultError, + targets: automationService.created.map(automation => automation.target), + }, { + error: undefined, + targets: [{ kind: 'workspace', folderUri: FOLDER, providerId: 'fallback-provider', sessionTypeId: 'copilotcli', isolation: { kind: 'worktree', branch: 'main' } }], + }); + }); + + test('configureAutomation rejects worktree targets whose provider has disappeared', async () => { + const automationService = new FakeAutomationService(); + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType('unavailable-provider', 'codex', true)]), + createConfigurationService(), + ); + const result = await invoke(tool, { + name: 'Unavailable worktree', prompt: 'Do not save', schedule: { interval: 'manual' }, + target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, + }); + + assert.deepStrictEqual({ + error: result.toolResultError, + created: automationService.created, + }, { + error: 'Sessions provider "unavailable-provider" is no longer available.', + created: [], + }); + }); + + test('configureAutomation leaves non-worktree and unrelated partial updates independent of repository availability', async () => { + const existing = createAutomation({ + target: { kind: 'workspace', folderUri: FOLDER, providerId: 'remote-provider', sessionTypeId: 'custom', isolation: { kind: 'worktree', branch: 'main' } }, + }); + const automationService = new FakeAutomationService([existing]); + let lookups = 0; + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'custom', true)]), + createConfigurationService(), + upcastPartial({ + id: 'remote-provider', + getWorktreeOptions: async () => { lookups++; return undefined; }, + }), + ); + const results: IToolResult[] = []; + for (const isolation of ['default', 'folder']) { + results.push(await invoke(tool, { + name: 'Folder automation', prompt: 'Review the repository', schedule: { interval: 'manual' }, + target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation }, + })); + } + results.push(await invoke(tool, { automationId: existing.id, enabled: false })); + + assert.deepStrictEqual({ + lookups, + errors: results.map(result => result.toolResultError), + created: automationService.created.length, + updated: automationService.updated, + }, { lookups: 0, errors: [undefined, undefined, undefined], created: 2, updated: [{ id: existing.id, patch: { enabled: false } }] }); + }); + + test('configureAutomation cancellation interrupts workspace validation without writing', async () => { + const automationService = new FakeAutomationService(); + const started = new DeferredPromise(); + const worktreeOptions = new DeferredPromise(); + const cancellation = teardown.add(new CancellationTokenSource()); + let requestToken: CancellationToken | undefined; + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'codex', true)]), + createConfigurationService(), + upcastPartial({ + id: 'remote-provider', + getWorktreeOptions: async (_folderUri, _sessionTypeId, token) => { + requestToken = token; + await started.complete(); + return worktreeOptions.p; + }, + }), + ); + const pending = invoke(tool, { + name: 'Cancelled worktree', prompt: 'Do not save', schedule: { interval: 'manual' }, + target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, + }, SESSION_RESOURCE, cancellation.token); + await started.p; + cancellation.cancel(); + const result = await pending; + await worktreeOptions.complete({ supportsWorktree: true, currentBranch: 'main', branches: ['main'] }); + + assert.deepStrictEqual({ + status: JSON.parse(getText(result)).status, + cancelled: requestToken?.isCancellationRequested, + created: automationService.created, + }, { status: 'cancelled', cancelled: true, created: [] }); + }); + + test('configureAutomation propagates workspace lookup errors instead of accepting the target', async () => { + const automationService = new FakeAutomationService(); + const error = new Error('Remote repository lookup failed'); + const tool = createConfigureTool( + automationService, + new FakeSessionsManagementService(undefined, false, [providerSessionType('remote-provider', 'copilotcli', true)]), + createConfigurationService(), + upcastPartial({ + id: 'remote-provider', + getWorktreeOptions: async () => { throw error; }, + }), + ); + + await assert.rejects(invoke(tool, { + name: 'Failed lookup', prompt: 'Do not save', schedule: { interval: 'manual' }, + target: { kind: 'workspace', folderUri: FOLDER.toString(), isolation: 'worktree', branch: 'main' }, + }), candidate => candidate === error); + assert.deepStrictEqual(automationService.created, []); + }); + test('configureAutomation rechecks cancellation immediately before writing', async () => { const automationService = new FakeAutomationService(); const tokenSource = new CancellationTokenSource(); tokenSource.cancel(); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1193,7 +1385,7 @@ suite('AutomationTools', () => { [providerSessionType('local-agent-host', 'copilot')], ); sessionsManagementService.beforeGetFolderSessionTypes = () => configurationService.setUserConfiguration(CHAT_AUTOMATIONS_ENABLED_SETTING, false); - const tool = new ConfigureAutomationTool(automationService, sessionsManagementService, configurationService); + const tool = createConfigureTool(automationService, sessionsManagementService, configurationService); const result = await invoke(tool, { name: 'Disabled', @@ -1223,7 +1415,7 @@ suite('AutomationTools', () => { automationStorageService.readBarrier = readBarrier; const automationService = createStorageBackedService(undefined, automationStorageService); const tokenSource = teardown.add(new CancellationTokenSource()); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1291,7 +1483,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(); automationStorageService.beforeCompareAndSwap = () => configurationService.setUserConfiguration(CHAT_AUTOMATIONS_ENABLED_SETTING, false); const automationService = createStorageBackedService(raw, automationStorageService); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(undefined), configurationService, @@ -1315,7 +1507,7 @@ suite('AutomationTools', () => { const tokenSource = teardown.add(new CancellationTokenSource()); automationStorageService.beforeCompareAndSwap = () => tokenSource.cancel(); const automationService = createStorageBackedService(undefined, automationStorageService); - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), createConfigurationService(), @@ -1344,7 +1536,7 @@ suite('AutomationTools', () => { }); test('configureAutomation rejects stale IDs and malformed targets', async () => { - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1392,7 +1584,7 @@ suite('AutomationTools', () => { }); test('configureAutomation bounds opaque provider configuration', async () => { - const tool = new ConfigureAutomationTool( + const tool = createConfigureTool( new FakeAutomationService(), new FakeSessionsManagementService(undefined), createConfigurationService(), @@ -1440,7 +1632,7 @@ suite('AutomationTools', () => { const configurationService = createConfigurationService(false); const runner = new RecordingAutomationRunner(automationService); const listResult = await invoke(new ListAutomationsTool(automationService, configurationService), {}); - const configureResult = await invoke(new ConfigureAutomationTool( + const configureResult = await invoke(createConfigureTool( automationService, new FakeSessionsManagementService(createSession({ workspace: FOLDER })), configurationService, From 790568f194ff986449c9c1fb62142149fc8e09d7 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Thu, 10 Sep 2026 17:59:11 +0200 Subject: [PATCH 8/8] automations: fix: prevent invalid worktree re-enablement Keep the saved-branch exception only while Worktree is already selected. After opting out, require available repository branches before enabling isolation again. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e4e70d7c-fbe2-487b-a6c5-177af2abae2f --- .../automations/browser/automationDialog.ts | 2 +- .../test/browser/automationDialog.test.ts | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/automations/browser/automationDialog.ts b/src/vs/sessions/contrib/automations/browser/automationDialog.ts index a3a802759ce0ce..254d88ac3d8c32 100644 --- a/src/vs/sessions/contrib/automations/browser/automationDialog.ts +++ b/src/vs/sessions/contrib/automations/browser/automationDialog.ts @@ -826,7 +826,7 @@ export class AutomationIsolationGroupActionViewItem extends BaseActionViewItem { if (!this.isolationModel.supportsWorktreeConfiguration) { return localize('automation.form.isolation.worktreeUnavailable', "Not supported by the selected session type"); } - if (this.isolationModel.selectedBranch) { + if (this.isolationModel.isolationMode === 'worktree' && this.isolationModel.selectedBranch) { return undefined; } switch (this.branchLoadState) { diff --git a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts index 1635daf7261565..5be110e8345580 100644 --- a/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts +++ b/src/vs/sessions/contrib/automations/test/browser/automationDialog.test.ts @@ -1141,6 +1141,42 @@ suite('Automation branch picker', () => { }); }); + for (const discovery of ['noRepository', 'loading', 'error', 'empty'] as const) { + test(`does not re-enable a saved Worktree target after opting out while repository discovery is ${discovery}`, async () => { + const pending = new DeferredPromise(); + const { container, model } = createItem({ + state: createFormState({ isolationMode: 'worktree', branch: 'release' }), + getWorktreeOptions: async () => { + switch (discovery) { + case 'noRepository': return undefined; + case 'loading': return pending.p; + case 'error': throw new Error('Repository unavailable'); + case 'empty': return { supportsWorktree: true, currentBranch: undefined, branches: [] }; + } + }, + }); + await timeout(0); + const toggle = container.querySelector('.sessions-chat-isolation-checkbox .action-label')!; + const initialMode = model.isolationMode; + toggle.click(); + const afterOptingOut = model.isolationMode; + toggle.click(); + const afterReenableAttempt = { + mode: model.isolationMode, + selectedBranch: model.selectedBranch, + persistedBranch: model.persistedBranch, + disabled: container.querySelector('.sessions-chat-isolation-checkbox')?.classList.contains('disabled'), + }; + await pending.complete({ supportsWorktree: true, currentBranch: 'main', branches: ['main', 'release'] }); + + assert.deepStrictEqual({ initialMode, afterOptingOut, afterReenableAttempt }, { + initialMode: 'worktree', + afterOptingOut: 'workspace', + afterReenableAttempt: { mode: 'workspace', selectedBranch: 'release', persistedBranch: undefined, disabled: true }, + }); + }); + } + test('retries provider branch failures without falling back to local Git', async () => { let attempts = 0; const { container, actionWidgetService, getOpenRepositoryAttempts } = createItem({