From 2160dc5cb3b0ae282c960dcedb3528d89b2f3422 Mon Sep 17 00:00:00 2001 From: BeniBenj Date: Mon, 2 Mar 2026 12:20:54 +0100 Subject: [PATCH] make sure action are disabled when cloud is picked --- .../contrib/chat/browser/chat.contribution.ts | 27 +++- .../contrib/chat/browser/runScriptAction.ts | 31 +++- .../browser/sessionsManagementService.ts | 10 ++ .../browser/sessionsTerminalContribution.ts | 24 +-- .../sessionsTerminalContribution.test.ts | 143 +++++++++--------- 5 files changed, 150 insertions(+), 85 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts index 0ace6677cb8..37fe6dc5884 100644 --- a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts @@ -17,7 +17,7 @@ import { IViewContainersRegistry, IViewsRegistry, ViewContainerLocation, Extensi import { Registry } from '../../../../platform/registry/common/platform.js'; import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js'; import { AgentSessionProviders } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; -import { ISessionsManagementService, IsNewChatSessionContext } from '../../sessions/browser/sessionsManagementService.js'; +import { IsActiveSessionBackgroundProviderContext, ISessionsManagementService, IsNewChatSessionContext } from '../../sessions/browser/sessionsManagementService.js'; import { Menus } from '../../../browser/menus.js'; import { BranchChatSessionAction } from './branchChatSessionAction.js'; import { RunScriptContribution } from './runScriptAction.js'; @@ -51,7 +51,7 @@ export class OpenSessionWorktreeInVSCodeAction extends Action2 { id: Menus.TitleBarRight, group: 'navigation', order: 10, - when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated()) + when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated(), IsActiveSessionBackgroundProviderContext) }] }); } @@ -92,6 +92,29 @@ export class OpenSessionWorktreeInVSCodeAction extends Action2 { } registerAction2(OpenSessionWorktreeInVSCodeAction); +// Disabled placeholder shown in the titlebar when the active session does not support opening in VS Code +class OpenSessionWorktreeInVSCodeNotAvailableAction extends Action2 { + constructor() { + super({ + id: 'chat.openSessionWorktreeInVSCode.notAvailable', + title: localize2('openInVSCode', 'Open in VS Code'), + tooltip: localize('openInVSCodeNotAvailableTooltip', "Open in VS Code is not available for this session type"), + icon: Codicon.vscodeInsiders, + precondition: ContextKeyExpr.false(), + menu: [{ + id: Menus.TitleBarRight, + group: 'navigation', + order: 9, + when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated(), IsActiveSessionBackgroundProviderContext.toNegated()) + }] + }); + } + + override run(): void { } +} + +registerAction2(OpenSessionWorktreeInVSCodeNotAvailableAction); + class NewChatInSessionsWindowAction extends Action2 { constructor() { diff --git a/src/vs/sessions/contrib/chat/browser/runScriptAction.ts b/src/vs/sessions/contrib/chat/browser/runScriptAction.ts index bacdbac4330..e5c024cb2ba 100644 --- a/src/vs/sessions/contrib/chat/browser/runScriptAction.ts +++ b/src/vs/sessions/contrib/chat/browser/runScriptAction.ts @@ -9,13 +9,13 @@ import { Disposable } from '../../../../base/common/lifecycle.js'; import { autorun, derivedOpts, IObservable } from '../../../../base/common/observable.js'; import { localize, localize2 } from '../../../../nls.js'; import { MenuId, registerAction2, Action2, MenuRegistry } from '../../../../platform/actions/common/actions.js'; +import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { IQuickInputService, IQuickPickItem, IQuickPickSeparator } from '../../../../platform/quickinput/common/quickInput.js'; import { IWorkbenchContribution } from '../../../../workbench/common/contributions.js'; import { SessionsCategories } from '../../../common/categories.js'; -import { IActiveSessionItem, ISessionsManagementService } from '../../sessions/browser/sessionsManagementService.js'; +import { IActiveSessionItem, IsActiveSessionBackgroundProviderContext, ISessionsManagementService } from '../../sessions/browser/sessionsManagementService.js'; import { Menus } from '../../../browser/menus.js'; import { ISessionsConfigurationService, ITaskEntry, TaskStorageTarget } from './sessionsConfigurationService.js'; -import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { IsAuxiliaryWindowContext } from '../../../../workbench/common/contextkeys.js'; import { SessionsWelcomeVisibleContext } from '../../../common/contextkeys.js'; @@ -284,7 +284,7 @@ export class RunScriptContribution extends Disposable implements IWorkbenchContr } } -// Register the Run split button submenu on the workbench title bar +// Register the Run split button submenu on the workbench title bar (background sessions only) MenuRegistry.appendMenuItem(Menus.TitleBarRight, { submenu: RunScriptDropdownMenuId, isSplitButton: true, @@ -292,5 +292,28 @@ MenuRegistry.appendMenuItem(Menus.TitleBarRight, { icon: Codicon.play, group: 'navigation', order: 8, - when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated()) + when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated(), IsActiveSessionBackgroundProviderContext) }); + +// Disabled placeholder shown in the titlebar when the active session does not support running scripts +class RunScriptNotAvailableAction extends Action2 { + constructor() { + super({ + id: 'workbench.action.agentSessions.runScript.notAvailable', + title: localize2('run', "Run"), + tooltip: localize('runScriptNotAvailableTooltip', "Run Script is not available for this session type"), + icon: Codicon.play, + precondition: ContextKeyExpr.false(), + menu: [{ + id: Menus.TitleBarRight, + group: 'navigation', + order: 8, + when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated(), IsActiveSessionBackgroundProviderContext.toNegated()) + }] + }); + } + + override run(): void { } +} + +registerAction2(RunScriptNotAvailableAction); diff --git a/src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts b/src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts index 85f4a6cbd63..546c0b73faf 100644 --- a/src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts +++ b/src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts @@ -7,6 +7,7 @@ import { Disposable, DisposableStore, IDisposable, MutableDisposable } from '../ import { CancellationToken } from '../../../../base/common/cancellation.js'; import { IObservable, observableValue } from '../../../../base/common/observable.js'; import { URI } from '../../../../base/common/uri.js'; +import { localize } from '../../../../nls.js'; import { createDecorator, IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { IContextKey, IContextKeyService, RawContextKey } from '../../../../platform/contextkey/common/contextkey.js'; import { ILogService } from '../../../../platform/log/common/log.js'; @@ -27,6 +28,12 @@ import { GITHUB_REMOTE_FILE_SCHEME } from '../../fileTreeView/browser/githubFile export const IsNewChatSessionContext = new RawContextKey('isNewChatSession', true); +/** + * True when the active session uses the Background provider type (copilotcli). + * Used to gate actions that require a local worktree (run script, open in VS Code, terminal). + */ +export const IsActiveSessionBackgroundProviderContext = new RawContextKey('isActiveSessionBackgroundProvider', false, localize('isActiveSessionBackgroundProvider', "Whether the active session uses the background agent provider")); + //#region Active Session Service const LAST_SELECTED_SESSION_KEY = 'agentSessions.lastSelectedSession'; @@ -105,6 +112,7 @@ export class SessionsManagementService extends Disposable implements ISessionsMa private readonly _newSession = this._register(new MutableDisposable()); private lastSelectedSession: URI | undefined; private readonly isNewChatSessionContext: IContextKey; + private readonly _isBackgroundProvider: IContextKey; constructor( @IStorageService private readonly storageService: IStorageService, @@ -124,6 +132,7 @@ export class SessionsManagementService extends Disposable implements ISessionsMa // Bind context key to active session state. // isNewSession is false when there are any established sessions in the model. this.isNewChatSessionContext = IsNewChatSessionContext.bindTo(contextKeyService); + this._isBackgroundProvider = IsActiveSessionBackgroundProviderContext.bindTo(contextKeyService); // Load last selected session this.lastSelectedSession = this.loadLastSelectedSession(); @@ -464,6 +473,7 @@ export class SessionsManagementService extends Disposable implements ISessionsMa this.logService.trace('[ActiveSessionService] Active session cleared'); } + this._isBackgroundProvider.set(activeSessionItem?.providerType === AgentSessionProviders.Background); this._activeSession.set(activeSessionItem, undefined); } diff --git a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts index 38c8e785ebb..809716d687b 100644 --- a/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts +++ b/src/vs/sessions/contrib/terminal/browser/sessionsTerminalContribution.ts @@ -13,6 +13,7 @@ import { Action2, registerAction2 } from '../../../../platform/actions/common/ac import { ILogService } from '../../../../platform/log/common/log.js'; import { IWorkbenchContribution, getWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../workbench/common/contributions.js'; import { IAgentSessionsService } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsService.js'; +import { AgentSessionProviders } from '../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; import { ITerminalService } from '../../../../workbench/contrib/terminal/browser/terminal.js'; import { IPathService } from '../../../../workbench/services/path/common/pathService.js'; import { Menus } from '../../../browser/menus.js'; @@ -22,11 +23,15 @@ import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextke import { SessionsWelcomeVisibleContext } from '../../../common/contextkeys.js'; /** - * Returns the cwd URI for the given session: worktree for non-cloud agent - * sessions, repository otherwise, or `undefined` when neither is available. + * Returns the cwd URI for the given session: worktree or repository path for + * background sessions only. Returns `undefined` for non-background sessions + * (Cloud, Local, etc.) which have no local worktree, or when no path is available. */ function getSessionCwd(session: IActiveSessionItem | undefined): URI | undefined { - return session?.worktree ?? session?.repository; + if (session?.providerType !== AgentSessionProviders.Background) { + return undefined; + } + return session.worktree ?? session.repository; } /** @@ -48,14 +53,14 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben @ITerminalService private readonly _terminalService: ITerminalService, @IAgentSessionsService private readonly _agentSessionsService: IAgentSessionsService, @ILogService private readonly _logService: ILogService, + @IPathService private readonly _pathService: IPathService, ) { super(); - // React to active session worktree/repository path changes + // React to active session changes — use worktree/repo for background sessions, home dir otherwise this._register(autorun(reader => { const session = this._sessionsManagementService.activeSession.read(reader); - const targetPath = getSessionCwd(session); - this._onActivePathChanged(targetPath); + this._onActiveSessionChanged(session); })); // When a session is archived, close all terminals for its worktree @@ -103,11 +108,12 @@ export class SessionsTerminalContribution extends Disposable implements IWorkben } } - private async _onActivePathChanged(targetPath: URI | undefined): Promise { - if (!targetPath) { + private async _onActiveSessionChanged(session: IActiveSessionItem | undefined): Promise { + if (!session) { return; } + const targetPath = getSessionCwd(session) ?? await this._pathService.userHome(); const targetFsPath = targetPath.fsPath; if (this._lastTargetFsPath?.toLowerCase() === targetFsPath.toLowerCase()) { return; @@ -143,7 +149,7 @@ class OpenSessionInTerminalAction extends Action2 { menu: [{ id: Menus.TitleBarRight, group: 'navigation', - order: 9, + order: 10, when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated()) }] }); diff --git a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts index 5df597398ff..ebc29e7e13f 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -18,6 +18,10 @@ import { IAgentSession, IAgentSessionsModel } from '../../../../../workbench/con import { AgentSessionProviders } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessions.js'; import { IActiveSessionItem, ISessionsManagementService } from '../../../sessions/browser/sessionsManagementService.js'; import { SessionsTerminalContribution } from '../../browser/sessionsTerminalContribution.js'; +import { TestPathService } from '../../../../../workbench/test/browser/workbenchTestServices.js'; +import { IPathService } from '../../../../../workbench/services/path/common/pathService.js'; + +const HOME_DIR = URI.file('/home/user'); function makeAgentSession(opts: { repository?: URI; @@ -39,10 +43,11 @@ function makeAgentSession(opts: { } as unknown as IActiveSessionItem & IAgentSession; } -function makeNonAgentSession(opts: { repository?: URI; worktree?: URI }): IActiveSessionItem { +function makeNonAgentSession(opts: { repository?: URI; worktree?: URI; providerType?: string }): IActiveSessionItem { return { repository: opts.repository, worktree: opts.worktree, + providerType: opts.providerType ?? AgentSessionProviders.Local, } as IActiveSessionItem; } @@ -111,6 +116,8 @@ suite('SessionsTerminalContribution', () => { } as unknown as IAgentSessionsModel; }); + instantiationService.stub(IPathService, new TestPathService(HOME_DIR)); + contribution = store.add(instantiationService.createInstance(SessionsTerminalContribution)); }); @@ -120,11 +127,11 @@ suite('SessionsTerminalContribution', () => { ensureNoDisposablesAreLeakedInTestSuite(); - // --- getSessionCwd logic (via active session changes) --- + // --- Background provider: uses worktree/repository path --- - test('creates a terminal when active session has a worktree (non-cloud agent)', async () => { + test('creates a terminal at the worktree for a background session', async () => { const worktreeUri = URI.file('/worktree'); - const session = makeAgentSession({ worktree: worktreeUri, repository: URI.file('/repo'), providerType: AgentSessionProviders.Local }); + const session = makeAgentSession({ worktree: worktreeUri, repository: URI.file('/repo'), providerType: AgentSessionProviders.Background }); activeSessionObs.set(session, undefined); await tick(); @@ -132,20 +139,9 @@ suite('SessionsTerminalContribution', () => { assert.strictEqual(createdTerminals[0].cwd.fsPath, worktreeUri.fsPath); }); - test('reate a terminal with repository for cloud agent sessions', async () => { + test('falls back to repository when worktree is undefined for a background session', async () => { const repoUri = URI.file('/repo'); - const workTree = URI.file('/worktree'); - const session = makeAgentSession({ worktree: workTree, repository: repoUri, providerType: AgentSessionProviders.Cloud }); - activeSessionObs.set(session, undefined); - await tick(); - - assert.strictEqual(createdTerminals.length, 1); - assert.strictEqual(createdTerminals[0].cwd.fsPath, workTree.fsPath); - }); - - test('creates a terminal with repository for non-agent sessions', async () => { - const repoUri = URI.file('/repo'); - const session = makeNonAgentSession({ repository: repoUri }); + const session = makeAgentSession({ repository: repoUri, providerType: AgentSessionProviders.Background }); activeSessionObs.set(session, undefined); await tick(); @@ -153,38 +149,87 @@ suite('SessionsTerminalContribution', () => { assert.strictEqual(createdTerminals[0].cwd.fsPath, repoUri.fsPath); }); - test('does not create a terminal when no path is available', async () => { - const session = makeNonAgentSession({}); + test('does not create a terminal when background session has no paths', async () => { + const session = makeAgentSession({ providerType: AgentSessionProviders.Background }); activeSessionObs.set(session, undefined); await tick(); assert.strictEqual(createdTerminals.length, 0); }); + // --- Non-background providers: use home directory --- + + test('uses home directory for a cloud agent session', async () => { + const session = makeAgentSession({ worktree: URI.file('/worktree'), repository: URI.file('/repo'), providerType: AgentSessionProviders.Cloud }); + activeSessionObs.set(session, undefined); + await tick(); + + assert.strictEqual(createdTerminals.length, 1); + assert.strictEqual(createdTerminals[0].cwd.fsPath, HOME_DIR.fsPath); + }); + + test('uses home directory for a local agent session', async () => { + const session = makeAgentSession({ worktree: URI.file('/worktree'), providerType: AgentSessionProviders.Local }); + activeSessionObs.set(session, undefined); + await tick(); + + assert.strictEqual(createdTerminals.length, 1); + assert.strictEqual(createdTerminals[0].cwd.fsPath, HOME_DIR.fsPath); + }); + + test('uses home directory for a non-agent session', async () => { + const session = makeNonAgentSession({ repository: URI.file('/repo') }); + activeSessionObs.set(session, undefined); + await tick(); + + assert.strictEqual(createdTerminals.length, 1); + assert.strictEqual(createdTerminals[0].cwd.fsPath, HOME_DIR.fsPath); + }); + + test('does not recreate terminal when multiple non-background sessions share the home directory', async () => { + const session1 = makeAgentSession({ providerType: AgentSessionProviders.Cloud }); + activeSessionObs.set(session1, undefined); + await tick(); + assert.strictEqual(createdTerminals.length, 1); + + // Different non-background session — same home dir, no new terminal + const session2 = makeAgentSession({ providerType: AgentSessionProviders.Local }); + activeSessionObs.set(session2, undefined); + await tick(); + assert.strictEqual(createdTerminals.length, 1); + }); + + test('does not create a terminal when there is no active session', async () => { + activeSessionObs.set(undefined, undefined); + await tick(); + + assert.strictEqual(createdTerminals.length, 0); + }); + test('does not recreate terminal for the same path', async () => { const worktreeUri = URI.file('/worktree'); - const session1 = makeAgentSession({ worktree: worktreeUri, providerType: AgentSessionProviders.Local }); + const session1 = makeAgentSession({ worktree: worktreeUri, providerType: AgentSessionProviders.Background }); activeSessionObs.set(session1, undefined); await tick(); assert.strictEqual(createdTerminals.length, 1); // Setting a different session with the same worktree should not create a new terminal - const session2 = makeAgentSession({ worktree: worktreeUri, providerType: AgentSessionProviders.Local }); + const session2 = makeAgentSession({ worktree: worktreeUri, providerType: AgentSessionProviders.Background }); activeSessionObs.set(session2, undefined); await tick(); assert.strictEqual(createdTerminals.length, 1); }); - test('creates new terminal when switching to a different path', async () => { + test('creates new terminal when switching to a different background path', async () => { const worktree1 = URI.file('/worktree1'); const worktree2 = URI.file('/worktree2'); - activeSessionObs.set(makeAgentSession({ worktree: worktree1, providerType: AgentSessionProviders.Local }), undefined); + activeSessionObs.set(makeAgentSession({ worktree: worktree1, providerType: AgentSessionProviders.Background }), undefined); await tick(); - activeSessionObs.set(makeAgentSession({ worktree: worktree2, providerType: AgentSessionProviders.Local }), undefined); + activeSessionObs.set(makeAgentSession({ worktree: worktree2, providerType: AgentSessionProviders.Background }), undefined); await tick(); assert.strictEqual(createdTerminals.length, 2); @@ -290,64 +335,22 @@ suite('SessionsTerminalContribution', () => { assert.strictEqual(createdTerminals.length, 2, 'should create a new terminal after the old one was disposed'); }); - // --- agent session with worktree preferred over repository for non-cloud --- - - test('prefers worktree over repository for local agent session', async () => { - const worktreeUri = URI.file('/worktree'); - const repoUri = URI.file('/repo'); - const session = makeAgentSession({ - worktree: worktreeUri, - repository: repoUri, - providerType: AgentSessionProviders.Local, - }); - activeSessionObs.set(session, undefined); - await tick(); - - assert.strictEqual(createdTerminals[0].cwd.fsPath, worktreeUri.fsPath); - }); - - test('falls back to repository when worktree is undefined for agent session', async () => { - const repoUri = URI.file('/repo'); - const session = makeAgentSession({ - repository: repoUri, - providerType: AgentSessionProviders.Local, - }); - activeSessionObs.set(session, undefined); - await tick(); - - assert.strictEqual(createdTerminals[0].cwd.fsPath, repoUri.fsPath); - }); - - test('does not use repository for cloud agent session when worktree exists', async () => { - const worktreeUri = URI.file('/worktree'); - const repoUri = URI.file('/repo'); - const session = makeAgentSession({ - worktree: worktreeUri, - repository: repoUri, - providerType: AgentSessionProviders.Cloud, - }); - activeSessionObs.set(session, undefined); - await tick(); - - assert.strictEqual(createdTerminals[0].cwd.fsPath, worktreeUri.fsPath); - }); - // --- switching back to previously used path reuses terminal --- - test('switching back to a previously used path reuses the existing terminal', async () => { + test('switching back to a previously used background path reuses the existing terminal', async () => { const cwd1 = URI.file('/cwd1'); const cwd2 = URI.file('/cwd2'); - activeSessionObs.set(makeAgentSession({ worktree: cwd1, providerType: AgentSessionProviders.Local }), undefined); + activeSessionObs.set(makeAgentSession({ worktree: cwd1, providerType: AgentSessionProviders.Background }), undefined); await tick(); assert.strictEqual(createdTerminals.length, 1); - activeSessionObs.set(makeAgentSession({ worktree: cwd2, providerType: AgentSessionProviders.Local }), undefined); + activeSessionObs.set(makeAgentSession({ worktree: cwd2, providerType: AgentSessionProviders.Background }), undefined); await tick(); assert.strictEqual(createdTerminals.length, 2); // Switch back to cwd1 - should reuse terminal, not create a new one - activeSessionObs.set(makeAgentSession({ worktree: cwd1, providerType: AgentSessionProviders.Local }), undefined); + activeSessionObs.set(makeAgentSession({ worktree: cwd1, providerType: AgentSessionProviders.Background }), undefined); await tick(); assert.strictEqual(createdTerminals.length, 2, 'should reuse the terminal for cwd1'); });