Agent Host changes for lszomoru/agents/branch-picker-uncommitted-action

Author: lszomoru

src/vs/platform/actionWidget/browser/actionList.ts

@@ -392,6 +392,7 @@ class ActionItemRenderer<T> implements IListRenderer<IActionListItem<T>, IAction
 			data.detail.textContent = '';
 			data.detail.style.display = 'none';
 		}
+		data.container.classList.toggle('has-detail', !!element.detail);
 
 		// Render optional inline toggle (shown as its own row below the detail)
 		dom.clearNode(data.inlineToggleContainer);

src/vs/platform/actionWidget/browser/actionWidget.css

@@ -315,15 +315,13 @@
 }
 
 /* Items with detail — show detail as subtext below the title */
-.action-widget .monaco-list .monaco-list-row.action {
-	&:has(.detail:not([style*="display: none"])) {
-		flex-wrap: wrap;
-		align-content: center;
-		padding-right: 6px;
-
-		.title {
-			line-height: 14px;
-		}
+.action-widget .monaco-list .monaco-list-row.action.has-detail {
+	flex-wrap: wrap;
+	align-content: center;
+	padding-right: 6px;
+
+	.title {
+		line-height: 14px;
 	}
 }
 
@@ -467,9 +465,16 @@
 	display: none;
 }
 
+.action-widget .monaco-list-row.action.has-detail.has-toolbar .action-list-item-toolbar {
+	display: flex;
+	visibility: hidden;
+	margin-right: 10px;
+}
+
 .action-widget .monaco-list-row.focused.action.has-toolbar .action-list-item-toolbar,
 .action-widget .monaco-list-row:hover.action.has-toolbar .action-list-item-toolbar {
 	display: flex;
+	visibility: visible;
 }
 
 .action-widget .monaco-list-row .action-list-item-toolbar .monaco-action-bar:not(.vertical) .action-label:not(.disabled):hover {

src/vs/platform/actionWidget/test/browser/actionList.test.ts

@@ -357,6 +357,72 @@ suite('ActionListWidget', () => {
 		});
 	});
 
+	test('keeps detail row geometry stable when its toolbar becomes visible', () => {
+		const widget = createActionListWidget(disposables, {
+			items: [
+				action('plain'),
+				{ ...action('detail'), detail: 'Description', toolbarActions: [toAction({ id: 'toolbar', label: 'Toolbar', run: () => { } })] },
+				...Array.from({ length: 20 }, (_, index) => action(`filler-${index}`)),
+			],
+		});
+		const wrapper = document.createElement('div');
+		wrapper.classList.add('action-widget');
+		widget.domNode.parentElement?.insertBefore(wrapper, widget.domNode);
+		wrapper.appendChild(widget.domNode);
+		disposables.add({ dispose: () => wrapper.remove() });
+
+		const rows = Array.from(widget.domNode.querySelectorAll<HTMLElement>('.monaco-list-row'));
+		const detailRow = rows[1];
+		const detail = detailRow.querySelector<HTMLElement>('.detail')!;
+		const toolbar = detailRow.querySelector<HTMLElement>('.action-list-item-toolbar')!;
+		const verticalScrollbar = widget.domNode.querySelector<HTMLElement>('.scrollbar.vertical')!;
+		const initial = {
+			rowHeight: detailRow.getBoundingClientRect().height,
+			detailTop: detail.getBoundingClientRect().top,
+			toolbarDisplay: mainWindow.getComputedStyle(toolbar).display,
+			toolbarVisibility: mainWindow.getComputedStyle(toolbar).visibility,
+			toolbarMarginRight: mainWindow.getComputedStyle(toolbar).marginRight,
+		};
+		detailRow.classList.add('focused');
+		const focused = {
+			rowHeight: detailRow.getBoundingClientRect().height,
+			detailTop: detail.getBoundingClientRect().top,
+			toolbarDisplay: mainWindow.getComputedStyle(toolbar).display,
+			toolbarVisibility: mainWindow.getComputedStyle(toolbar).visibility,
+			toolbarMarginRight: mainWindow.getComputedStyle(toolbar).marginRight,
+			clearsScrollbar: detailRow.getBoundingClientRect().right - toolbar.getBoundingClientRect().right >= verticalScrollbar.getBoundingClientRect().width,
+		};
+
+		assert.deepStrictEqual({
+			rows: rows.slice(0, 2).map(row => ({
+				hasDetail: row.classList.contains('has-detail'),
+				hasToolbar: row.classList.contains('has-toolbar'),
+			})),
+			initial,
+			focused,
+		}, {
+			rows: [
+				{ hasDetail: false, hasToolbar: false },
+				{ hasDetail: true, hasToolbar: true },
+			],
+			initial: {
+				rowHeight: 48,
+				detailTop: initial.detailTop,
+				toolbarDisplay: 'flex',
+				toolbarVisibility: 'hidden',
+				toolbarMarginRight: '6px',
+			},
+			focused: {
+				rowHeight: 48,
+				detailTop: initial.detailTop,
+				toolbarDisplay: 'flex',
+				toolbarVisibility: 'visible',
+				toolbarMarginRight: '6px',
+				clearsScrollbar: true,
+			},
+		});
+	});
+
 	test('keeps titled separator above first filtered match', () => {
 		const widget = createActionListWidget(disposables, {
 			items: [

src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionConfigPicker.ts

@@ -11,6 +11,7 @@ import { ActionListItemKind, IActionListDelegate, IActionListItem } from '../../
 import { IActionWidgetService } from '../../../../../platform/actionWidget/browser/actionWidget.js';
 import { BaseActionViewItem } from '../../../../../base/browser/ui/actionbar/actionViewItems.js';
 import { Checkbox } from '../../../../../base/browser/ui/toggle/toggle.js';
+import { toAction } from '../../../../../base/common/actions.js';
 import { Delayer } from '../../../../../base/common/async.js';
 import { Codicon } from '../../../../../base/common/codicons.js';
 import { Disposable, DisposableMap, DisposableStore, IDisposable, MutableDisposable } from '../../../../../base/common/lifecycle.js';
@@ -35,10 +36,14 @@ import { markOnboardingTarget } from '../../../../../workbench/contrib/onboardin
 import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../../workbench/common/contributions.js';
 import { type IChatInputPickerOptions } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPickerActionItem.js';
 import { IChatInputPickerResponsiveState } from '../../../../../workbench/contrib/chat/browser/widget/input/chatInputPickerResponsiveLayout.js';
+import { IViewsService } from '../../../../../workbench/services/views/common/viewsService.js';
+import { IAgentWorkbenchLayoutService } from '../../../../browser/workbench.js';
 import { Menus } from '../../../../browser/menus.js';
 import { SessionProviderIdContext, IsPhoneLayoutContext, IsQuickChatSessionContext } from '../../../../common/contextkeys.js';
 import { IWorkbenchLayoutService } from '../../../../../workbench/services/layout/browser/layoutService.js';
 import { reportNewChatPickerClosed } from '../../../chat/browser/newChatPickerTelemetry.js';
+import { ISessionChangesService } from '../../../changes/browser/sessionChangesService.js';
+import { CHANGES_VIEW_ID } from '../../../changes/common/changes.js';
 import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js';
 import { IActiveSession } from '../../../../services/sessions/common/sessionsManagement.js';
 import { ISessionContext } from '../../../../services/sessions/browser/sessionContext.js';
@@ -144,7 +149,7 @@ function getBranchUncommittedChanges(branchName: string, repositoryBranchName: s
 		: undefined;
 }
 
-function toActionItems(property: string, items: readonly IConfigPickerItem[], currentValue: unknown | undefined, policyRestricted?: boolean, repositoryBranchName?: string, repositoryUncommittedChanges?: number): IActionListItem<IConfigPickerItem>[] {
+function toActionItems(property: string, items: readonly IConfigPickerItem[], currentValue: unknown | undefined, policyRestricted?: boolean, repositoryBranchName?: string, repositoryUncommittedChanges?: number, onShowChanges?: () => Promise<void>): IActionListItem<IConfigPickerItem>[] {
 	const actionItems: IActionListItem<IConfigPickerItem>[] = items.map(item => {
 		const disabled = property === SessionConfigKey.AutoApprove && isAutoApproveValuePolicyRestricted(item.value, policyRestricted === true);
 		const checked = isSelectedValue(currentValue, item.value);
@@ -162,6 +167,14 @@ function toActionItems(property: string, items: readonly IConfigPickerItem[], cu
 			ariaDescription: uncommittedChangesDescription,
 			disabled,
 			item: { ...item, checked },
+			toolbarActions: uncommittedChanges !== undefined && onShowChanges
+				? [toAction({
+					id: 'sessions.agentHost.showBranchChanges',
+					label: localize('agentHostSessionConfig.branchItemShowChanges', "Show Changes"),
+					class: ThemeIcon.asClassName(Codicon.diffMultiple),
+					run: onShowChanges,
+				})]
+				: undefined,
 		};
 	});
 
@@ -364,8 +377,10 @@ export class AgentHostSessionConfigPicker extends Disposable {
 		@IHoverService protected readonly _hoverService: IHoverService,
 		@ISessionsProvidersService protected readonly _sessionsProvidersService: ISessionsProvidersService,
 		@ITelemetryService protected readonly _telemetryService: ITelemetryService,
-		@IWorkbenchLayoutService protected readonly _layoutService: IWorkbenchLayoutService,
+		@IAgentWorkbenchLayoutService protected readonly _layoutService: IAgentWorkbenchLayoutService,
 		@IStorageService protected readonly _storageService: IStorageService,
+		@ISessionChangesService private readonly _sessionChangesService: ISessionChangesService,
+		@IViewsService protected readonly _viewsService: IViewsService,
 	) {
 		super();
 
@@ -785,7 +800,10 @@ export class AgentHostSessionConfigPicker extends Disposable {
 		const repositoryState = property === SessionConfigKey.Branch
 			? this._getRepositoryBranchState(sessionId)
 			: undefined;
-		const actionItems = toActionItems(property, items, currentValue, policyRestricted, repositoryState?.branchName, repositoryState?.uncommittedChanges);
+		const onShowChanges = property === SessionConfigKey.Branch
+			? () => this._showChanges()
+			: undefined;
+		const actionItems = toActionItems(property, items, currentValue, policyRestricted, repositoryState?.branchName, repositoryState?.uncommittedChanges, onShowChanges);
 
 		const delegate: IActionListDelegate<IConfigPickerItem> = {
 			onSelect: async item => {
@@ -816,7 +834,7 @@ export class AgentHostSessionConfigPicker extends Disposable {
 					const filteredRawItems = await this._getItems(provider, sessionId, property, schema, query);
 					const { items: filteredItems, policyRestricted: filteredPolicyRestricted } = applyAutoApproveFiltering(filteredRawItems, property, this._configurationService);
 					const filteredRepositoryState = this._getRepositoryBranchState(sessionId);
-					return toActionItems(property, filteredItems, provider.getSessionConfig(sessionId)?.values[property] ?? schema.default, filteredPolicyRestricted, filteredRepositoryState.branchName, filteredRepositoryState.uncommittedChanges);
+					return toActionItems(property, filteredItems, provider.getSessionConfig(sessionId)?.values[property] ?? schema.default, filteredPolicyRestricted, filteredRepositoryState.branchName, filteredRepositoryState.uncommittedChanges, onShowChanges);
 				})
 				: undefined,
 			onHide: () => trigger.focus(),
@@ -842,6 +860,20 @@ export class AgentHostSessionConfigPicker extends Disposable {
 		);
 	}
 
+	private async _showChanges(): Promise<void> {
+		this._actionWidgetService.hide();
+		const session = this._session.get();
+		if (this._layoutService.isSinglePaneLayoutEnabled && session) {
+			const suppression = this._layoutService.suppressEditorPartAutoVisibility();
+			try {
+				await this._sessionChangesService.openChangesEditor(session.resource);
+			} finally {
+				suppression.dispose();
+			}
+		}
+		await this._viewsService.openView(CHANGES_VIEW_ID, true);
+	}
+
 	protected _getRepositoryBranchState(sessionId: string): { branchName: string | undefined; uncommittedChanges: number | undefined } {
 		const session = this._session.get();
 		const repository = session?.sessionId === sessionId

src/vs/sessions/contrib/providers/agentHost/test/browser/agentHost/agentHostSessionConfigPicker.test.ts

@@ -23,16 +23,21 @@ 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 { IWorkbenchLayoutService } from '../../../../../../../workbench/services/layout/browser/layoutService.js';
+import { IView } from '../../../../../../../workbench/common/views.js';
+import { IViewsService } from '../../../../../../../workbench/services/views/common/viewsService.js';
+import { IAgentWorkbenchLayoutService } from '../../../../../../browser/workbench.js';
 import { Menus } from '../../../../../../browser/menus.js';
 import { IAgentHostSessionsProvider, LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../../../../common/agentHostSessionsProvider.js';
+import { ISessionChangesService } from '../../../../../../contrib/changes/browser/sessionChangesService.js';
+import { CHANGES_VIEW_ID } from '../../../../../../contrib/changes/common/changes.js';
 import { ISessionsProvidersService } from '../../../../../../services/sessions/browser/sessionsProvidersService.js';
 import { IActiveSession } from '../../../../../../services/sessions/common/sessionsManagement.js';
 import { ISessionWorkspace } from '../../../../../../services/sessions/common/session.js';
 import { ISessionsProvider } from '../../../../../../services/sessions/common/sessionsProvider.js';
 import { AgentHostSessionConfigPicker, IConfigPickerItem, PickerActionViewItem } from '../../../browser/agentHostSessionConfigPicker.js';
 
 const SESSION_ID = 'local-agent-host:s1';
+const SESSION_RESOURCE = URI.parse('agent-session:/s1');
 
 function makeWorkspace(uncommittedChanges: number | undefined, branchName = 'main'): ISessionWorkspace {
 	const root = URI.file('/repo');
@@ -195,6 +200,7 @@ function branchState(container: HTMLElement): { icon: string | undefined; ariaLa
 class CapturingActionWidgetHolder {
 	delegate: IActionListDelegate<IConfigPickerItem> | undefined;
 	items: readonly IActionListItem<IConfigPickerItem>[] = [];
+	readonly events: string[] = [];
 }
 
 function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeakedInTestSuite>, 'add'>) {
@@ -205,7 +211,7 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
 	const instantiationService = store.add(new TestInstantiationService());
 	instantiationService.stub(IActionWidgetService, {
 		isVisible: false,
-		hide: () => { },
+		hide: () => actionWidget.events.push('hide'),
 		show: (_user, _supportsPreview, items: readonly IActionListItem<IConfigPickerItem>[], delegate: IActionListDelegate<IConfigPickerItem>) => {
 			actionWidget.items = items;
 			actionWidget.delegate = delegate;
@@ -219,9 +225,26 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
 	instantiationService.stub(IContextKeyService, new (class extends mock<IContextKeyService>() {
 		override readonly onDidChangeContext = Event.None;
 	})());
-	instantiationService.stub(IWorkbenchLayoutService, new (class extends mock<IWorkbenchLayoutService>() {
+	instantiationService.stub(IAgentWorkbenchLayoutService, new (class extends mock<IAgentWorkbenchLayoutService>() {
 		// No `phone-layout` class → `isPhoneLayout` is false → isolation renders as a checkbox.
 		override readonly mainContainer = document.createElement('div');
+		override readonly isSinglePaneLayoutEnabled = true;
+		override suppressEditorPartAutoVisibility() {
+			actionWidget.events.push('suppressEditorPartAutoVisibility');
+			return { dispose: () => actionWidget.events.push('releaseEditorPartAutoVisibility') };
+		}
+	})());
+	instantiationService.stub(ISessionChangesService, new (class extends mock<ISessionChangesService>() {
+		override async openChangesEditor(sessionResource: URI): Promise<undefined> {
+			actionWidget.events.push(`openChangesEditor:${sessionResource.toString()}`);
+			return undefined;
+		}
+	})());
+	instantiationService.stub(IViewsService, new (class extends mock<IViewsService>() {
+		override async openView<T extends IView>(id: string, focus?: boolean): Promise<T | null> {
+			actionWidget.events.push(`openView:${id}:${focus}`);
+			return null;
+		}
 	})());
 	instantiationService.set(ISessionsProvidersService, new (class extends mock<ISessionsProvidersService>() {
 		override readonly onDidChangeProviders = Event.None;
@@ -236,6 +259,7 @@ function setupServices(store: Pick<ReturnType<typeof ensureNoDisposablesAreLeake
 	const sessionObs = observableValue<IActiveSession | undefined>('activeSession', {
 		providerId: LOCAL_AGENT_HOST_PROVIDER_ID,
 		sessionId: SESSION_ID,
+		resource: SESSION_RESOURCE,
 		workspace,
 	} as IActiveSession);
 	return { instantiationService, provider, sessionObs, workspaceObs, actionWidget };
@@ -400,6 +424,7 @@ suite('Agent Host Session Config Picker', () => {
 			checked: item.item?.checked,
 			detail: item.detail,
 			ariaDescription: item.ariaDescription,
+			toolbarActions: item.toolbarActions?.map(action => ({ id: action.id, label: action.label })),
 		}));
 
 		services.workspaceObs.set(makeWorkspace(1, 'dev'), undefined);
@@ -412,13 +437,19 @@ suite('Agent Host Session Config Picker', () => {
 			ariaDescription: singularItem?.ariaDescription,
 		};
 
+		services.workspaceObs.set(makeWorkspace(0, 'dev'), undefined);
+		branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!
+			.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
+		await new Promise(resolve => setTimeout(resolve));
+		const cleanToolbarActions = services.actionWidget.items.find(item => item.label === 'dev')?.toolbarActions;
+
 		services.provider.completions = [{ value: 'main', label: 'main' }];
 		branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!
 			.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
 		await new Promise(resolve => setTimeout(resolve));
 		const singleResultKinds = services.actionWidget.items.map(item => item.kind);
 
-		assert.deepStrictEqual({ plural, singular, singleResultKinds }, {
+		assert.deepStrictEqual({ plural, singular, cleanToolbarActions, singleResultKinds }, {
 			plural: [
 				{
 					kind: ActionListItemKind.Action,
@@ -427,6 +458,7 @@ suite('Agent Host Session Config Picker', () => {
 					checked: true,
 					detail: undefined,
 					ariaDescription: undefined,
+					toolbarActions: undefined,
 				},
 				{
 					kind: ActionListItemKind.Separator,
@@ -435,6 +467,7 @@ suite('Agent Host Session Config Picker', () => {
 					checked: undefined,
 					detail: undefined,
 					ariaDescription: undefined,
+					toolbarActions: undefined,
 				},
 				{
 					kind: ActionListItemKind.Action,
@@ -443,16 +476,45 @@ suite('Agent Host Session Config Picker', () => {
 					checked: false,
 					detail: '2 uncommitted files',
 					ariaDescription: '2 uncommitted files',
+					toolbarActions: [{
+						id: 'sessions.agentHost.showBranchChanges',
+						label: 'Show Changes',
+					}],
 				},
 			],
 			singular: {
 				detail: '1 uncommitted file',
 				ariaDescription: '1 uncommitted file',
 			},
+			cleanToolbarActions: undefined,
 			singleResultKinds: [ActionListItemKind.Action],
 		});
 	});
 
+	test('dirty branch action selects the Changes tab before focusing the Changes view', async () => {
+		const services = setupServices(store);
+		services.provider.config = makeDynamicBranchConfig('main');
+		services.provider.completions = [
+			{ value: 'main', label: 'main' },
+			{ value: 'dev', label: 'dev' },
+		];
+		services.workspaceObs.set(makeWorkspace(1, 'dev'), undefined);
+		const { container } = renderPicker(store, services);
+
+		branchSlot(container)!.querySelector<HTMLElement>('a.action-label')!.click();
+		await new Promise(resolve => setTimeout(resolve));
+		const action = services.actionWidget.items.find(item => item.label === 'dev')?.toolbarActions?.[0];
+		await action?.run();
+
+		assert.deepStrictEqual(services.actionWidget.events, [
+			'hide',
+			'suppressEditorPartAutoVisibility',
+			`openChangesEditor:${SESSION_RESOURCE.toString()}`,
+			'releaseEditorPartAutoVisibility',
+			`openView:${CHANGES_VIEW_ID}:true`,
+		]);
+	});
+
 	test('a picker recreated on a session switch still renders the provider-seeded chips (disabled) while resolving', () => {
 		const services = setupServices(store);
 		const { provider } = services;