diff --git a/src/vs/base/common/event.ts b/src/vs/base/common/event.ts index 5678ca05378..cf5fd39b738 100644 --- a/src/vs/base/common/event.ts +++ b/src/vs/base/common/event.ts @@ -267,7 +267,7 @@ export namespace Event { export function debounce(event: Event, merge: (last: T | undefined, event: T) => T, delay?: number | typeof MicrotaskDelay, leading?: boolean, flushOnListenerRemove?: boolean, leakWarningThreshold?: number, disposable?: DisposableStore): Event; export function debounce(event: Event, merge: (last: O | undefined, event: I) => O, delay?: number | typeof MicrotaskDelay, leading?: boolean, flushOnListenerRemove?: boolean, leakWarningThreshold?: number, disposable?: DisposableStore): Event; export function debounce(event: Event, merge: (last: O | undefined, event: I) => O, delay: number | typeof MicrotaskDelay = 100, leading = false, flushOnListenerRemove = false, leakWarningThreshold?: number, disposable?: DisposableStore): Event { - let subscription: IDisposable; + let subscription: IDisposable = Disposable.None; let output: O | undefined = undefined; let handle: Timeout | undefined | null = undefined; let numDebouncedCalls = 0; diff --git a/src/vs/base/test/common/event.test.ts b/src/vs/base/test/common/event.test.ts index 5e5ae1a6d95..5cd9b68de57 100644 --- a/src/vs/base/test/common/event.test.ts +++ b/src/vs/base/test/common/event.test.ts @@ -112,6 +112,14 @@ suite('Event utils dispose', function () { assertDisposablesCount([leaked]); // leaked is still there }); + + test('debounce-util can be disposed without listeners', function () { + const store = new DisposableStore(); + const emitter = ds.add(new Emitter()); + Event.debounce(emitter.event, l => 0, undefined, undefined, undefined, undefined, store); + + store.dispose(); + }); }); suite('Event', function () { diff --git a/src/vs/sessions/common/contextkeys.ts b/src/vs/sessions/common/contextkeys.ts index 151124e19ce..2d973c3fef4 100644 --- a/src/vs/sessions/common/contextkeys.ts +++ b/src/vs/sessions/common/contextkeys.ts @@ -121,7 +121,8 @@ export const SessionWorkspacePickerGroupContext = new RawContextKey('ses //#region < --- New Session Pickers --- > export const SessionWorkspacePickerVisibleContext = new RawContextKey('sessionWorkspacePickerVisible', false, localize('sessionWorkspacePickerVisible', "Whether the new-session view's workspace picker is rendered (as opposed to being replaced by the no-agent-host empty state)")); -export const SessionHarnessPickerVisibleContext = new RawContextKey('sessionHarnessPickerVisible', false, localize('sessionHarnessPickerVisible', "Whether the new-session view's harness (session type) picker is visible — it is hidden when at most one harness can serve the selected workspace")); +export const SessionHarnessPickerVisibleContext = new RawContextKey('sessionHarnessPickerVisible', false, localize('sessionHarnessPickerVisible', "Whether the new-session view's harness (session type) picker is rendered")); +export const SessionHarnessPickerInteractiveContext = new RawContextKey('sessionHarnessPickerInteractive', false, localize('sessionHarnessPickerInteractive', "Whether the new-session view's harness (session type) picker can be interacted with")); export const SessionIsolationPickerVisibleContext = new RawContextKey('sessionIsolationPickerVisible', false, localize('sessionIsolationPickerVisible', "Whether the new-session view's isolation picker is visible — it is shown only when the isolation option is enabled and the workspace has a git repository")); export const AgentHostSessionTypesAvailableContext = new RawContextKey('agentHostSessionTypesAvailable', false, localize('agentHostSessionTypesAvailable', "Whether at least one connected agent-host provider has advertised session types")); diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index 5bcc6c1fd5d..e9298bdf3c9 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -663,6 +663,9 @@ padding: 0 0 0 2px; line-height: 100% !important; align-self: center; + background-size: contain; + background-position: center; + background-repeat: no-repeat; } .sessions-chat-attachment-pill .monaco-icon-label .monaco-icon-label-container { diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index 987ce309a4b..cb02d27df0d 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -286,6 +286,10 @@ font-size: var(--vscode-codiconFontSize); } +.sessions-chat-picker-slot.sessions-workspace-category-picker-slot .action-label > .sessions-chat-dropdown-chevron { + font-size: var(--vscode-codiconFontSize-compact); +} + .sessions-workspace-category-picker .sessions-chat-dropdown-label { margin-left: 0; } diff --git a/src/vs/sessions/contrib/chat/browser/newChatContextAttachments.ts b/src/vs/sessions/contrib/chat/browser/newChatContextAttachments.ts index 4bdb86ee0cb..d8e46729591 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatContextAttachments.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatContextAttachments.ts @@ -31,6 +31,7 @@ import { ILanguageService } from '../../../../editor/common/languages/language.j import { getIconClasses } from '../../../../editor/common/services/getIconClasses.js'; import { basename } from '../../../../base/common/resources.js'; import { Schemas } from '../../../../base/common/network.js'; +import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js'; import { DEFAULT_LABELS_CONTAINER, ResourceLabels } from '../../../../workbench/browser/labels.js'; import { IChatRequestVariableEntry, isAgentHostCompletionVariableEntry, isPastedTextArtifact, OmittedState } from '../../../../workbench/contrib/chat/common/attachments/chatVariableEntries.js'; @@ -167,6 +168,13 @@ export class NewChatContextAttachments extends Disposable implements INewChatAtt const icon = dom.append(content, renderIcon(Codicon.repo)); icon.setAttribute('aria-hidden', 'true'); dom.append(content, dom.$('span.sessions-chat-attachment-name', undefined, entry.name)); + } else if (entry.icon) { + const icon = dom.append(content, renderIcon(entry.icon)); + icon.setAttribute('aria-hidden', 'true'); + if (entry.icon.color) { + icon.style.color = asCssVariable(entry.icon.color.id); + } + dom.append(content, dom.$('span.sessions-chat-attachment-name', undefined, entry.name)); } else { const label = this._resourceLabels.create(content, { supportIcons: true }); this._renderDisposables.add(label); diff --git a/src/vs/sessions/contrib/chat/browser/newChatInput.ts b/src/vs/sessions/contrib/chat/browser/newChatInput.ts index 5bcba276b8c..dc774b56faf 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInput.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInput.ts @@ -98,6 +98,7 @@ import { IChatSubmitRequestHandlerService } from '../../../../workbench/contrib/ import { INewChatModelPickerService, NewChatModelPickerService } from './newChatModelPicker.js'; import { ModelPicker, ModelPickerActionViewItem } from './modelPicker.js'; import { ISessionModelSelection, SessionModelSelection } from './sessionModelSelection.js'; +import { hasSendableModelSelection } from './sessionModelPickerState.js'; import { ISessionContext, SessionContext } from '../../../services/sessions/browser/sessionContext.js'; import { AGENT_SESSIONS_SCOPED_INPUT_HISTORY_SETTING } from './sessionsChatHistory.js'; import { IChatStatusItemService } from '../../../../workbench/contrib/chat/browser/chatStatus/chatStatusItemService.js'; @@ -510,7 +511,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation return true; } const modelSelection = this._modelSelection.state.read(reader); - return this.options.canSendRequest.read(reader) && modelSelection.hasSelectableModel && !modelSelection.pendingSelection; + return this.options.canSendRequest.read(reader) && hasSendableModelSelection(modelSelection); }); this._scopedInstantiationService = this._register(this.instantiationService.createChild(new ServiceCollection( [INewChatModelPickerService, this._newChatModelPickerService], @@ -526,6 +527,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation } })); } + this._register(this.storageService.onWillSaveState(() => this.saveState())); this._contextAttachments = this._register(this.instantiationService.createInstance(NewChatContextAttachments)); // Always use the mobile-aware picker. Its overrides bail to the // desktop behavior when `isPhoneLayout()` is false, so picking @@ -534,7 +536,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation // phone breakpoint after the chat input mounted. this.sessionTypePicker = this._register(this.instantiationService.createInstance(MobileSessionTypePicker, this.options.session, this.options.sessionTypePickerOptions)); this._register(this._contextAttachments.onDidChangeContext(() => { - this._updateDraftState(); + this._updateAndSaveDraftState(); this._updateSendButtonState(); this.focus(); })); @@ -1375,6 +1377,14 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation }; } + private _updateAndSaveDraftState(): void { + if (this._sending) { + return; + } + this._updateDraftState(); + this.saveState(); + } + private _toHistoryEntry(draft: IDraftState): IChatModelInputState { return { ...draft, @@ -1513,6 +1523,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation this._contextAttachments.setAttachments(draft.attachments.map(IChatRequestVariableEntry.fromExport)); } } + this._updateSendButtonState(); } private _getDraftState(): IDraftState | undefined { diff --git a/src/vs/sessions/contrib/chat/browser/newChatWidget.ts b/src/vs/sessions/contrib/chat/browser/newChatWidget.ts index c8d50975c55..da7b43eff53 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatWidget.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatWidget.ts @@ -210,7 +210,6 @@ export class NewChatWidget extends Disposable { loading, historyKey: constObservable(undefined), // no persisted history for the new-session view placeholder: localize('newSessionPromptPlaceholder', "Pitch your idea"), - sessionTypePickerOptions: { showChevron: false }, supportsBackground: true, deferredNotificationsEnabled, petHostPreferred: this.options.petHostPreferred, @@ -707,7 +706,6 @@ export class NewChatWidget extends Disposable { label: localize('newSessionWorkspacePicker.githubContext', "Issue/PR"), ariaLabel: localize('newSessionWorkspacePicker.githubContextAriaLabel', "Attach a GitHub issue or pull request to the new session"), tooltip: localize('newSessionWorkspacePicker.githubContextTooltip', "Attach an issue or pull request as context"), - icon: Codicon.add, hideIconWhenAttached: true, group: SESSION_WORKSPACE_GROUP_GITHUB, attachesContext: true, diff --git a/src/vs/sessions/contrib/chat/browser/sessionModelPickerState.ts b/src/vs/sessions/contrib/chat/browser/sessionModelPickerState.ts index a3d6f940534..40edee31f1c 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionModelPickerState.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionModelPickerState.ts @@ -47,6 +47,10 @@ export function hasSelectableModel( return models.length > 0 || options.showAutoModel; } +export function hasSendableModelSelection(state: ISessionModelSelectionState): boolean { + return state.hasSelectableModel && (!state.pendingSelection || state.options.showAutoModel); +} + export const EMPTY_MODEL_SELECTION_STATE: ISessionModelSelectionState = { currentModel: undefined, pendingSelection: undefined, diff --git a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts index 678ecec3630..0223794c434 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts @@ -18,7 +18,6 @@ import { ISessionsProvidersService } from '../../../services/sessions/browser/se import { autorun, IObservable, observableValue } from '../../../../base/common/observable.js'; import { ISession, SessionStatus } from '../../../services/sessions/common/session.js'; import { Emitter } from '../../../../base/common/event.js'; -import { isWeb } from '../../../../base/common/platform.js'; import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; @@ -31,7 +30,7 @@ import { IChatInputNotificationService } from '../../../../workbench/contrib/cha import { IChatEntitlementService } from '../../../../workbench/services/chat/common/chatEntitlementService.js'; import { markOnboardingTarget } from '../../../../workbench/contrib/onboarding/browser/spotlight/onboardingTarget.js'; import { reportNewChatPickerClosed } from './newChatPickerTelemetry.js'; -import { SessionHarnessPickerVisibleContext } from '../../../common/contextkeys.js'; +import { SessionHarnessPickerInteractiveContext, SessionHarnessPickerVisibleContext } from '../../../common/contextkeys.js'; import { isAllowSignedOutWhenUsableEnabled } from '../../../browser/sessionsAuthGate.js'; const STORAGE_KEY_LAST_SESSION_TYPE = 'sessions.userSelectedSessionType'; @@ -149,13 +148,8 @@ export class SessionTypePicker extends Disposable { private readonly _renderDisposables = this._register(new DisposableStore()); protected _triggerElement: HTMLElement | undefined; - /** - * Tracks whether the harness picker trigger is currently visible. Mirrors - * the `.hidden` state computed in {@link _updateTriggerLabel}, so the - * new-session-view onboarding tour can skip the harness step when only a - * single harness can serve the selected workspace. - */ private readonly _visibleKey: IContextKey; + private readonly _interactiveKey: IContextKey; constructor( private readonly _session: IObservable, @@ -176,6 +170,8 @@ export class SessionTypePicker extends Disposable { this._visibleKey = SessionHarnessPickerVisibleContext.bindTo(contextKeyService); this._register(toDisposable(() => this._visibleKey.reset())); + this._interactiveKey = SessionHarnessPickerInteractiveContext.bindTo(contextKeyService); + this._register(toDisposable(() => this._interactiveKey.reset())); // Restore the previously selected session type from storage this._picked = this._readStoredPick(); @@ -632,23 +628,27 @@ export class SessionTypePicker extends Disposable { private _updateTriggerLabel(): void { if (!this._triggerElement) { this._visibleKey.set(false); + this._interactiveKey.set(false); return; } dom.clearNode(this._triggerElement); - // In web (vscode.dev/agents) the host filter already scopes the - // workbench to a single agent host, so when that host advertises only - // one harness there is nothing to pick — hide the trigger entirely. - const hideForSingleHarness = isWeb && this._folderSessionTypes.length <= 1 && this._pickServedByFolder(this._picked); - if (this._folderSessionTypes.length === 0 || hideForSingleHarness) { + if (this._folderSessionTypes.length === 0) { this._triggerElement.classList.add('hidden'); + this._triggerElement.parentElement?.classList.remove('disabled'); this._visibleKey.set(false); + this._interactiveKey.set(false); return; } + const disabled = this._folderSessionTypes.length === 1 && this._pickServedByFolder(this._picked); this._triggerElement.classList.remove('hidden'); + this._triggerElement.parentElement?.classList.toggle('disabled', disabled); + this._triggerElement.tabIndex = disabled ? -1 : 0; + this._triggerElement.setAttribute('aria-disabled', String(disabled)); this._visibleKey.set(true); + this._interactiveKey.set(!disabled); const currentType = this._folderSessionTypes.find(t => t.providerId === this._picked?.providerId && t.sessionType.id === this._picked?.sessionTypeId)?.sessionType ?? this._folderSessionTypes.find(t => t.sessionType.id === this._picked?.sessionTypeId)?.sessionType; @@ -659,11 +659,13 @@ export class SessionTypePicker extends Disposable { const labelSpan = dom.append(this._triggerElement, dom.$('span.sessions-chat-dropdown-label')); labelSpan.textContent = modeLabel; - if (this._options?.showChevron !== false) { + if (!disabled && this._options?.showChevron !== false) { const chevron = dom.append(this._triggerElement, renderIcon(Codicon.chevronDownCompact)); chevron.classList.add('sessions-chat-dropdown-chevron'); } - this._triggerElement.ariaLabel = localize('sessionTypePicker.triggerAriaLabel', "Pick Session Type, {0}", modeLabel); + this._triggerElement.ariaLabel = disabled + ? localize('sessionTypePicker.disabledTriggerAriaLabel', "Session Type, {0}", modeLabel) + : localize('sessionTypePicker.triggerAriaLabel', "Pick Session Type, {0}", modeLabel); } } diff --git a/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts index 554bed5f1d7..bf3695ff1cb 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts @@ -104,7 +104,7 @@ export interface IWorkspacePickerTrigger { readonly label?: string; readonly ariaLabel: string; readonly tooltip?: string; - readonly icon: ThemeIcon; + readonly icon?: ThemeIcon; readonly hideIconWhenAttached?: boolean; readonly reflectsWorkspace?: boolean; readonly group?: string; @@ -136,6 +136,7 @@ interface IWorkspacePickerTriggerElements { icon?: HTMLElement; label?: HTMLElement; badge?: CountBadge; + chevron?: HTMLElement; } type IWorkspacePickerAction = IAction & { icon?: ThemeIcon; hoverContent?: string; onRemove?: () => void }; @@ -1444,7 +1445,7 @@ export class WorkspacePicker extends Disposable { trigger.classList.toggle('selected', (reflectsWorkspace && workspace !== undefined) || isSelectedCategory || badgeCount > 0 || relatedGitHubInfo !== undefined); const icon = (reflectsWorkspace ? workspace?.icon : undefined) ?? (relatedGitHubInfo ? Codicon.repo : (isSelectedCategory && workspace ? workspace.icon : options.icon)); - if (options.hideIconWhenAttached === true && badgeCount > 0) { + if (!icon || (options.hideIconWhenAttached === true && badgeCount > 0)) { contents.icon?.remove(); contents.icon = undefined; } else { @@ -1475,6 +1476,12 @@ export class WorkspacePicker extends Disposable { contents.badge?.dispose(); contents.badge = undefined; } + if (!contents.chevron) { + contents.chevron = renderIcon(Codicon.chevronDownCompact); + contents.chevron.classList.add('sessions-chat-dropdown-chevron'); + contents.chevron.setAttribute('aria-hidden', 'true'); + } + trigger.append(contents.chevron); return; } diff --git a/src/vs/sessions/contrib/chat/test/browser/modelPicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/modelPicker.test.ts index 9dea9f392ef..56463add8bd 100644 --- a/src/vs/sessions/contrib/chat/test/browser/modelPicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/modelPicker.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { ILanguageModelChatMetadataAndIdentifier } from '../../../../../workbench/contrib/chat/common/languageModels.js'; -import { hasSelectableModel, normalizeModelPickerOptions } from '../../browser/sessionModelPickerState.js'; +import { createModelSelectionState, hasSelectableModel, hasSendableModelSelection, normalizeModelPickerOptions } from '../../browser/sessionModelPickerState.js'; const aModel = { identifier: 'copilot-gpt-4o', metadata: {} } as ILanguageModelChatMetadataAndIdentifier; @@ -42,4 +42,24 @@ suite('ModelPicker selectability', () => { showManageModelsAction: false, })), true); }); + + test('allows an unresolved selection only when Auto is available', () => { + const pendingSelection = { reference: 'pending-model' }; + const options = { + useGroupedModelPicker: true, + showFeatured: true, + showUnavailableFeatured: false, + showManageModelsAction: false, + }; + const autoOptions = normalizeModelPickerOptions({ ...options, showAutoModel: true }); + const explicitModelOptions = normalizeModelPickerOptions({ ...options, showAutoModel: false }); + + assert.deepStrictEqual({ + auto: hasSendableModelSelection(createModelSelectionState([], autoOptions, undefined, pendingSelection)), + explicitModel: hasSendableModelSelection(createModelSelectionState([], explicitModelOptions, undefined, pendingSelection)), + }, { + auto: true, + explicitModel: false, + }); + }); }); diff --git a/src/vs/sessions/contrib/chat/test/browser/newChatInput.test.ts b/src/vs/sessions/contrib/chat/test/browser/newChatInput.test.ts index 6907c228539..4b2360ccbdf 100644 --- a/src/vs/sessions/contrib/chat/test/browser/newChatInput.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/newChatInput.test.ts @@ -5,6 +5,7 @@ import assert from 'assert'; import { DeferredPromise } from '../../../../../base/common/async.js'; +import { Codicon } from '../../../../../base/common/codicons.js'; import { DisposableStore, IDisposable, IReference } from '../../../../../base/common/lifecycle.js'; import { Schemas } from '../../../../../base/common/network.js'; import { URI } from '../../../../../base/common/uri.js'; @@ -32,6 +33,10 @@ const holdInputModelReference = Reflect.get(NewChatInputWidget.prototype, '_hold const getDraftState = Reflect.get(NewChatInputWidget.prototype, '_getDraftState') as (this: IDraftStateHarness) => { inputText: string; attachments: readonly IChatRequestVariableEntry[] } | undefined; const restoreState = Reflect.get(NewChatInputWidget.prototype, '_restoreState') as (this: IRestoreStateHarness) => void; const saveState = Reflect.get(NewChatInputWidget.prototype, 'saveState') as (this: IDraftStateHarness) => void; +const clearDraftState = Reflect.get(NewChatInputWidget.prototype, '_clearDraftState') as (this: IDraftStateHarness) => void; +const updateDraftState = Reflect.get(NewChatInputWidget.prototype, '_updateDraftState') as (this: IUpdateDraftStateHarness) => void; +const updateAndSaveDraftState = Reflect.get(NewChatInputWidget.prototype, '_updateAndSaveDraftState') as (this: IUpdateAndSaveDraftStateHarness) => void; +const updateSendButtonState = Reflect.get(NewChatInputWidget.prototype, '_updateSendButtonState') as (this: IUpdateSendButtonStateHarness) => void; const updateAttachmentRendering = Reflect.get(NewChatContextAttachments.prototype, '_updateRendering') as (this: IAttachmentRenderingHarness) => void; interface IDraftStateHarness { @@ -50,6 +55,37 @@ interface IRestoreStateHarness { readonly _contextAttachments: { setAttachments(entries: readonly IChatRequestVariableEntry[]): void; }; + _updateSendButtonState(): void; +} + +interface IUpdateDraftStateHarness extends IDraftStateHarness { + readonly _editor: { + getModel(): { getValue(): string } | null; + }; + readonly _contextAttachments: { + readonly attachments: readonly IChatRequestVariableEntry[]; + }; +} + +interface IUpdateAndSaveDraftStateHarness extends IUpdateDraftStateHarness { + readonly _sending: boolean; + _updateDraftState(): void; + saveState(): void; +} + +interface IUpdateSendButtonStateHarness { + readonly _sendButton: { enabled: boolean } | undefined; + readonly _sending: boolean; + readonly _editor: { + getModel(): { getValue(): string } | null; + }; + readonly _contextAttachments: { + readonly attachments: readonly IChatRequestVariableEntry[]; + }; + readonly options: { + readonly hasAdditionalSendContent?: { get(): boolean }; + }; + readonly _canSendRequest: { get(): boolean }; } interface IAttachmentRenderingHarness { @@ -178,11 +214,19 @@ suite('NewChatInputWidget', () => { value: repositoryRoot, }, ]; - const saveHarness: IDraftStateHarness = { + const saveHarness: IUpdateAndSaveDraftStateHarness = { storageService, - _draftState: { inputText: 'Fix this', attachments }, + _sending: false, + _editor: { getModel: () => ({ getValue: () => '' }) }, + _contextAttachments: { attachments }, + _updateDraftState() { + updateDraftState.call(this); + }, + saveState() { + saveState.call(this); + }, }; - saveState.call(saveHarness); + updateAndSaveDraftState.call(saveHarness); const restored: { inputText?: string; attachments?: readonly IChatRequestVariableEntry[] } = {}; const draft = getDraftState.call({ storageService }); @@ -190,6 +234,7 @@ suite('NewChatInputWidget', () => { _getDraftState: () => draft, _editor: { getModel: () => ({ setValue: value => restored.inputText = value }) }, _contextAttachments: { setAttachments: entries => restored.attachments = entries }, + _updateSendButtonState: () => { }, }); assert.deepStrictEqual({ @@ -198,13 +243,100 @@ suite('NewChatInputWidget', () => { folderValue: restored.attachments?.[0].value, repositoryValue: restored.attachments?.[1].value, }, { - inputText: 'Fix this', + inputText: '', attachmentIds: attachments.map(attachment => attachment.id), folderValue: folder, repositoryValue: repositoryRoot, }); }); + test('persists draft text when state is saved', () => { + let stored: string | undefined; + const storageService: IDraftStateHarness['storageService'] = { + get: () => stored, + store: (_key, value) => stored = value, + }; + const harness: IUpdateAndSaveDraftStateHarness = { + storageService, + _sending: false, + _editor: { getModel: () => ({ getValue: () => 'Fix this after reload' }) }, + _contextAttachments: { attachments: [] }, + _updateDraftState() { + updateDraftState.call(this); + }, + saveState() { + saveState.call(this); + }, + }; + + updateDraftState.call(harness); + saveState.call(harness); + + assert.deepStrictEqual(getDraftState.call({ storageService }), { + inputText: 'Fix this after reload', + attachments: [], + }); + }); + + test('does not re-persist a sent prompt when attachments clear during send', () => { + let stored: string | undefined; + let editorValue = 'Fix this'; + const storageService: IDraftStateHarness['storageService'] = { + get: () => stored, + store: (_key, value) => stored = value, + }; + const harness: IUpdateAndSaveDraftStateHarness = { + storageService, + _sending: true, + _editor: { getModel: () => ({ getValue: () => editorValue }) }, + _contextAttachments: { attachments: [] }, + _updateDraftState() { + updateDraftState.call(this); + }, + saveState() { + saveState.call(this); + }, + }; + + clearDraftState.call(harness); + updateAndSaveDraftState.call(harness); + editorValue = ''; + updateDraftState.call(harness); + + assert.deepStrictEqual(getDraftState.call({ storageService }), { + inputText: '', + attachments: [], + }); + }); + + test('enables send after restoring an unchanged retained input model', () => { + const sendButton = { enabled: false }; + const harness: IRestoreStateHarness & IUpdateSendButtonStateHarness = { + _getDraftState: () => ({ inputText: 'Fix this', attachments: [] }), + _sendButton: sendButton, + _sending: false, + _editor: { + getModel: () => ({ + getValue: () => 'Fix this', + setValue: () => { }, + }), + }, + _contextAttachments: { + attachments: [], + setAttachments: () => { }, + }, + options: {}, + _canSendRequest: { get: () => true }, + _updateSendButtonState() { + updateSendButtonState.call(this); + }, + }; + + restoreState.call(harness); + + assert.strictEqual(sendButton.enabled, true); + }); + test('renders GitHub context pills as openable with a keyboard-reachable remove button', async () => { const container = document.createElement('div'); const entry = toPasteVariableEntry('microsoft/vscode#332825', 'GitHub context: https://github.com/microsoft/vscode/pull/332825', { @@ -268,6 +400,66 @@ suite('NewChatInputWidget', () => { }); }); + test('renders issue and pull request attachment icons without nested focus targets', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const entries = [ + toPasteVariableEntry('microsoft/vscode#9014', 'Issue context', { + id: 'github-context:https://github.com/microsoft/vscode/issues/9014', + icon: { ...Codicon.issues, color: { id: 'charts.green' } }, + }), + toPasteVariableEntry('microsoft/vscode#123', 'Pull request context', { + id: 'github-context:https://github.com/microsoft/vscode/pull/123', + icon: Codicon.gitPullRequest, + }), + ]; + const renderDisposables = disposables.add(new DisposableStore()); + try { + updateAttachmentRendering.call({ + _container: container, + _attachedContext: entries, + _renderDisposables: renderDisposables, + _resourceLabels: { + clear: () => { }, + create: () => ({ + dispose: () => { }, + setLabel: () => { }, + setFile: () => { }, + }), + }, + openerService: { open: async () => true }, + removeAttachment: () => { }, + }); + + const pills = Array.from(container.querySelectorAll('.sessions-chat-attachment-pill')); + const focusTargets = pills.map(pill => pill.querySelector('.sessions-chat-attachment-open')); + focusTargets[0]?.focus(); + const issueButtonFocused = document.activeElement === focusTargets[0]; + focusTargets[1]?.focus(); + const pullRequestButtonFocused = document.activeElement === focusTargets[1]; + + assert.deepStrictEqual({ + pills: pills.map(pill => ({ + label: pill.querySelector('.sessions-chat-attachment-name')?.textContent, + icon: pill.querySelector('.codicon:not(.codicon-close-compact)')?.className, + color: pill.querySelector('.codicon:not(.codicon-close-compact)')?.style.color, + nestedLinks: pill.querySelectorAll('a').length, + })), + issueButtonFocused, + pullRequestButtonFocused, + }, { + pills: [ + { label: 'microsoft/vscode#9014', icon: 'codicon codicon-issues', color: 'var(--vscode-charts-green)', nestedLinks: 0 }, + { label: 'microsoft/vscode#123', icon: 'codicon codicon-git-pull-request', color: '', nestedLinks: 0 }, + ], + issueButtonFocused: true, + pullRequestButtonFocused: true, + }); + } finally { + container.remove(); + } + }); + test('renders additional folder and repository context as attachment pills', () => { const container = document.createElement('div'); const folder = URI.file('/workspace/docs'); diff --git a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.fixture.ts b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.fixture.ts index 059dc41fb6a..7356e20a375 100644 --- a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.fixture.ts +++ b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.fixture.ts @@ -327,7 +327,7 @@ export default defineThemedFixtureGroup({ path: 'sessions/chat/newWidget/' }, { }), NewSessionAttachedContext: defineComponentFixture({ labels: { kind: 'screenshot', blocksCi: true }, - expectedVisualDescriptions: ['The new-session workspace row shows Copilot, microsoft/vscode with a count badge showing 2, and Issue/PR with a count badge showing 1. The composer attachment row shows removable docs, microsoft/typescript, and microsoft/vscode#333053 context pills with compact dismiss icons.'], + expectedVisualDescriptions: ['The new-session workspace row shows Copilot, microsoft/vscode with a count badge showing 2, and Issue/PR with a count badge showing 1. The composer attachment row shows removable docs, microsoft/typescript, and microsoft/vscode#333053 context pills with compact dismiss icons. The folder icon is fully visible without cropping, and the GitHub issue pill includes an issue icon.'], render: context => renderNewChatWidget(context, { withWorkspace: true, withAttachedContext: true }), }), NewSessionRemoteWorkspace: defineComponentFixture({ diff --git a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts index 8ab06da47e3..e9c2adb165a 100644 --- a/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/newChatWidget.test.ts @@ -117,7 +117,7 @@ interface ISendHarness { interface IRenderWorkspacePickerHarness { readonly _workspacePickerVisibleKey: { set(value: boolean): void }; readonly _workspacePicker: { - renderCategoryTriggers(container: HTMLElement, triggers: readonly { readonly label?: string; readonly tooltip?: string; readonly attachesContext?: boolean }[]): HTMLElement; + renderCategoryTriggers(container: HTMLElement, triggers: readonly { readonly label?: string; readonly tooltip?: string; readonly icon?: { readonly id: string }; readonly attachesContext?: boolean }[]): HTMLElement; }; readonly _newChatInput: { readonly sessionTypePicker: { @@ -161,7 +161,7 @@ suite('NewChatWidget', () => { test('workspace row hosts a multiple-harness picker first', () => { const container = document.createElement('div'); const harnessLabels = ['Copilot', 'Claude']; - const workspaceTriggers: { readonly tooltip: string | undefined; readonly attachesContext: boolean | undefined }[] = []; + const workspaceTriggers: { readonly tooltip: string | undefined; readonly icon: string | undefined; readonly attachesContext: boolean | undefined }[] = []; const harness: IRenderWorkspacePickerHarness = { _workspacePickerVisibleKey: { set: () => { } }, _workspacePicker: { @@ -172,7 +172,7 @@ suite('NewChatWidget', () => { const item = document.createElement('div'); item.textContent = trigger.label ?? 'More'; row.appendChild(item); - workspaceTriggers.push({ tooltip: trigger.tooltip, attachesContext: trigger.attachesContext }); + workspaceTriggers.push({ tooltip: trigger.tooltip, icon: trigger.icon?.id, attachesContext: trigger.attachesContext }); } return row; }, @@ -207,8 +207,8 @@ suite('NewChatWidget', () => { ], ); assert.deepStrictEqual(workspaceTriggers, [ - { tooltip: 'Choose where the new session runs', attachesContext: false }, - { tooltip: 'Attach an issue or pull request as context', attachesContext: true }, + { tooltip: 'Choose where the new session runs', icon: 'project', attachesContext: false }, + { tooltip: 'Attach an issue or pull request as context', icon: undefined, attachesContext: true }, ]); }); diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts index 24934ecbc7a..afc2c81d668 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts @@ -24,6 +24,7 @@ import { IChatSessionsService } from '../../../../../workbench/contrib/chat/comm import { ILanguageModelsService } from '../../../../../workbench/contrib/chat/common/languageModels.js'; import { ChatEntitlement, IChatEntitlementService } from '../../../../../workbench/services/chat/common/chatEntitlementService.js'; import { TestStorageService } from '../../../../../workbench/test/common/workbenchTestServices.js'; +import { SessionHarnessPickerInteractiveContext, SessionHarnessPickerVisibleContext } from '../../../../common/contextkeys.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { IProviderSessionType, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { SessionTypeAuthRequirement, ISession, ISessionWorkspace, SessionStatus } from '../../../../services/sessions/common/session.js'; @@ -116,6 +117,7 @@ function createPicker( storage: IStorageService, options?: ISessionTypePickerOptions, actionWidgetService: Partial = { isVisible: false, hide: () => { }, show: () => { } }, + contextKeyService: IContextKeyService = new MockContextKeyService(), ): TestSessionTypePicker { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IActionWidgetService, actionWidgetService); @@ -135,7 +137,7 @@ function createPicker( }); instantiationService.stub(IConfigurationService, new TestConfigurationService()); instantiationService.stub(IChatInputNotificationService, { getActiveNotification: () => undefined }); - instantiationService.stub(IContextKeyService, new MockContextKeyService()); + instantiationService.stub(IContextKeyService, contextKeyService); return disposables.add(instantiationService.createInstance(TestSessionTypePicker, session, options)); } @@ -264,6 +266,60 @@ suite('SessionTypePicker', () => { }); }); + test('disables the trigger when the selected workspace has only one session type', () => { + management.setSessionTypes([ + sessionType('copilot', 'cloud', 'Cloud'), + ]); + const contextKeyService = new MockContextKeyService(); + const picker = createPicker(disposables, session, management, storage, undefined, undefined, contextKeyService); + session.set(createFakeSession('copilot', 'cloud', folder), undefined); + const container = document.createElement('div'); + picker.render(container); + const trigger = container.querySelector('.action-label'); + const singleType = { + hidden: trigger?.classList.contains('hidden'), + disabled: trigger?.getAttribute('aria-disabled'), + tabIndex: trigger?.tabIndex, + label: trigger?.getAttribute('aria-label'), + visible: contextKeyService.getContextKeyValue(SessionHarnessPickerVisibleContext.key), + interactive: contextKeyService.getContextKeyValue(SessionHarnessPickerInteractiveContext.key), + }; + + management.setSessionTypes([ + sessionType('copilot', 'cloud', 'Cloud'), + sessionType('local-agent-host', 'local', 'Local'), + ]); + + assert.deepStrictEqual({ + singleType, + multipleTypes: { + hidden: trigger?.classList.contains('hidden'), + disabled: trigger?.getAttribute('aria-disabled'), + tabIndex: trigger?.tabIndex, + label: trigger?.getAttribute('aria-label'), + visible: contextKeyService.getContextKeyValue(SessionHarnessPickerVisibleContext.key), + interactive: contextKeyService.getContextKeyValue(SessionHarnessPickerInteractiveContext.key), + }, + }, { + singleType: { + hidden: false, + disabled: 'true', + tabIndex: -1, + label: 'Session Type, Cloud', + visible: true, + interactive: false, + }, + multipleTypes: { + hidden: false, + disabled: 'false', + tabIndex: 0, + label: 'Pick Session Type, Cloud', + visible: true, + interactive: true, + }, + }); + }); + test('re-selecting the default (first) session type clears the stored pick', () => { management.setSessionTypes([ sessionType('local-1', 'local', 'Local'), diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts index f7769a2a2ac..47c1d73eb32 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts @@ -1225,14 +1225,15 @@ suite('WorkspacePicker - Category Triggers', () => { expanded: trigger.getAttribute('aria-expanded'), role: trigger.getAttribute('role'), tabIndex: trigger.tabIndex, + icons: Array.from(trigger.querySelectorAll('.codicon'), icon => icon.classList.item(1)), })), }, { selectedFolderUri: folderUri.toString(), triggers: [ - { label: 'local/project', ariaLabel: 'Folder: local/project', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0 }, - { label: 'Repo, Issue, or PR', ariaLabel: 'Choose a GitHub target', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0 }, - { label: 'Remote Setup', ariaLabel: 'Choose a remote setup', hidden: true, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0 }, - { label: undefined, ariaLabel: 'More workspace options', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0 }, + { label: 'local/project', ariaLabel: 'Folder: local/project', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0, icons: ['codicon-folder', 'codicon-chevron-down-compact'] }, + { label: 'Repo, Issue, or PR', ariaLabel: 'Choose a GitHub target', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0, icons: ['codicon-github', 'codicon-chevron-down-compact'] }, + { label: 'Remote Setup', ariaLabel: 'Choose a remote setup', hidden: true, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0, icons: ['codicon-radio-tower', 'codicon-chevron-down-compact'] }, + { label: undefined, ariaLabel: 'More workspace options', hidden: false, hasPopup: 'listbox', expanded: 'false', role: 'button', tabIndex: 0, icons: ['codicon-ellipsis', 'codicon-chevron-down-compact'] }, ], }); }); @@ -2117,7 +2118,7 @@ suite('WorkspacePicker - Category Triggers', () => { const contextTrigger = container.querySelector('.action-label'); const getContextTriggerSnapshot = () => ({ label: contextTrigger?.querySelector('.sessions-chat-dropdown-label')?.textContent, - icon: contextTrigger?.querySelector('.codicon')?.className, + icon: contextTrigger?.querySelector('.codicon:not(.sessions-chat-dropdown-chevron)')?.className, badge: contextTrigger?.querySelector('.monaco-count-badge')?.textContent, ariaLabel: contextTrigger?.getAttribute('aria-label'), }); @@ -2172,7 +2173,7 @@ suite('WorkspacePicker - Category Triggers', () => { }]); const contextTrigger = container.querySelector('.action-label'); const getContextTriggerSnapshot = () => ({ - icon: contextTrigger?.querySelector('.codicon')?.className, + icon: contextTrigger?.querySelector('.codicon:not(.sessions-chat-dropdown-chevron)')?.className, badge: contextTrigger?.querySelector('.monaco-count-badge')?.textContent, ariaLabel: contextTrigger?.getAttribute('aria-label'), }); diff --git a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewV2Tour.ts b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewV2Tour.ts index 58f4be8d9fb..d920e0d0ae2 100644 --- a/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewV2Tour.ts +++ b/src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewV2Tour.ts @@ -7,6 +7,7 @@ import { IObservable } from '../../../../../base/common/observable.js'; import { localize } from '../../../../../nls.js'; import { ISpotlightPayload, SPOTLIGHT_PRESENTATION_KIND } from '../../../../../workbench/contrib/onboarding/browser/spotlight/spotlightTypes.js'; import { IOnboardingScenario } from '../../../../../workbench/contrib/onboarding/common/onboardingScenario.js'; +import { SessionHarnessPickerInteractiveContext } from '../../../../common/contextkeys.js'; import { NEW_SESSION_ONBOARDING_SEEN_KEY } from './newSessionTour.js'; import { createNewSessionViewRecentTourWhen, createNewSessionViewWorkspaceStep } from './newSessionViewTourShared.js'; @@ -30,6 +31,7 @@ const newSessionViewV2Payload: ISpotlightPayload = { placement: 'above', missingTarget: WAIT_FOR_PICKER, openTarget: false, + when: SessionHarnessPickerInteractiveContext, allowTargetInteraction: true, }, { diff --git a/src/vs/sessions/contrib/onboardingTours/test/browser/newSessionViewV2Tour.test.ts b/src/vs/sessions/contrib/onboardingTours/test/browser/newSessionViewV2Tour.test.ts index 9569f0d5af4..e344001c872 100644 --- a/src/vs/sessions/contrib/onboardingTours/test/browser/newSessionViewV2Tour.test.ts +++ b/src/vs/sessions/contrib/onboardingTours/test/browser/newSessionViewV2Tour.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { observableValue } from '../../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; -import { AgentHostSessionTypesAvailableContext, IsNewChatSessionContext, SessionHasWorkspaceContext } from '../../../../common/contextkeys.js'; +import { AgentHostSessionTypesAvailableContext, IsNewChatSessionContext, SessionHarnessPickerInteractiveContext, SessionHasWorkspaceContext } from '../../../../common/contextkeys.js'; import { createNewSessionViewV2Tour, NEW_SESSION_VIEW_V2_TOUR_ID } from '../../browser/tours/newSessionViewV2Tour.js'; import { createNewSessionViewV3Tour } from '../../browser/tours/newSessionViewV3Tour.js'; import { NEW_SESSION_ONBOARDING_SEEN_KEY } from '../../browser/tours/newSessionTour.js'; @@ -34,6 +34,7 @@ suite('NewSessionViewV2Tour', () => { openTarget: step.openTarget, allowTargetInteraction: step.allowTargetInteraction, advanceWhenWorkspaceSelected: step.advanceWhen === SessionHasWorkspaceContext, + requiresInteractiveHarnessPicker: step.when === SessionHarnessPickerInteractiveContext, })), }, { id: NEW_SESSION_VIEW_V2_TOUR_ID, @@ -52,6 +53,7 @@ suite('NewSessionViewV2Tour', () => { openTarget: true, allowTargetInteraction: true, advanceWhenWorkspaceSelected: true, + requiresInteractiveHarnessPicker: false, }, { id: 'harnessPicker', @@ -60,6 +62,7 @@ suite('NewSessionViewV2Tour', () => { openTarget: false, allowTargetInteraction: true, advanceWhenWorkspaceSelected: false, + requiresInteractiveHarnessPicker: true, }, { id: 'modelPicker', @@ -68,6 +71,7 @@ suite('NewSessionViewV2Tour', () => { openTarget: true, allowTargetInteraction: true, advanceWhenWorkspaceSelected: false, + requiresInteractiveHarnessPicker: false, }, ], }); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 828b0f07a77..85044db6306 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -2561,6 +2561,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement protected readonly _onDidChangeCustomizations = this._register(new Emitter()); readonly onDidChangeCustomizations = this._onDidChangeCustomizations.event; + readonly onDidChangeModels: Event; /** Last-known root config state (schema + values), seeded from `RootState.config`. */ protected _rootConfig: RootConfigState | undefined; @@ -2775,6 +2776,10 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement @IWorkspaceTrustManagementService protected readonly _workspaceTrustManagementService: IWorkspaceTrustManagementService, ) { super(); + this.onDidChangeModels = Event.defer(Event.any( + this._languageModelsService.onDidChangeLanguageModels, + this._languageModelsService.onDidChangeModelVisibility, + ), false, this._store); this._downloadProgress = this._register(this._instantiationService.createInstance(AgentHostDownloadProgress)); this._register(toDisposable(() => { for (const cached of this._sessionCache.values()) { @@ -3913,13 +3918,6 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // -- Model selection ------------------------------------------------------ - get onDidChangeModels(): Event { - return Event.signal(Event.any( - this._languageModelsService.onDidChangeLanguageModels, - this._languageModelsService.onDidChangeModelVisibility, - )); - } - getModelsSnapshot(sessionId: string, desiredModelId?: string): ISessionModelsSnapshot { // Agent-host models are registered against the session's resource // scheme (the per-host/per-agent `targetChatSessionType`). Resolve the 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 867d8484262..7895ee4e7a1 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 @@ -441,7 +441,7 @@ function createSchemaDefaultConfigurationService(): TestConfigurationService { function createProvider(disposables: DisposableStore, agentHostService: MockAgentHostService, contributions = [ { type: 'agent-host-copilotcli', name: 'copilot', displayName: 'Copilot', description: 'test', icon: undefined }, -], options?: { sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; acquireOrLoadSession?: (resource: URI) => Promise; languageModelIds?: string[]; lookupLanguageModel?: (modelId: string) => ILanguageModelChatMetadata | undefined; hiddenLanguageModelIds?: ReadonlySet; languageModelVisibilityChanges?: Event; openSession?: boolean; configurationService?: IConfigurationService; activeSession?: IObservable; visibleSessions?: IObservable; activeClient?: Omit; activeClientAgents?: IObservable; activeClientScope?: (sessionType: string, roots: readonly URI[]) => IAgentCustomizationScope; storageService?: IStorageService; isSessionsWindow?: boolean; confirmDelete?: boolean; workspaceTrusted?: boolean; requestWorkspaceTrust?: (uri: URI) => Promise; workspaceTrustBarrier?: DeferredPromise; workspaceTrustError?: Error; setUrisTrust?: (uris: URI[], trusted: boolean) => Promise; gitHubService?: IGitHubService; devContainerAgentHostService?: IDevContainerAgentHostService; sessionsProvidersService?: ISessionsProvidersService; pathService?: IPathService; labelService?: ILabelService }): LocalAgentHostSessionsProvider { +], options?: { sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; acquireOrLoadSession?: (resource: URI) => Promise; languageModelIds?: string[]; lookupLanguageModel?: (modelId: string) => ILanguageModelChatMetadata | undefined; languageModelChanges?: Event; hiddenLanguageModelIds?: ReadonlySet; languageModelVisibilityChanges?: Event; openSession?: boolean; configurationService?: IConfigurationService; activeSession?: IObservable; visibleSessions?: IObservable; activeClient?: Omit; activeClientAgents?: IObservable; activeClientScope?: (sessionType: string, roots: readonly URI[]) => IAgentCustomizationScope; storageService?: IStorageService; isSessionsWindow?: boolean; confirmDelete?: boolean; workspaceTrusted?: boolean; requestWorkspaceTrust?: (uri: URI) => Promise; workspaceTrustBarrier?: DeferredPromise; workspaceTrustError?: Error; setUrisTrust?: (uris: URI[], trusted: boolean) => Promise; gitHubService?: IGitHubService; devContainerAgentHostService?: IDevContainerAgentHostService; sessionsProvidersService?: ISessionsProvidersService; pathService?: IPathService; labelService?: ILabelService }): LocalAgentHostSessionsProvider { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IAgentHostService, agentHostService); @@ -485,7 +485,7 @@ function createProvider(disposables: DisposableStore, agentHostService: MockAgen lookupLanguageModel: options?.lookupLanguageModel ?? (() => undefined), hasResolvedVendor: () => true, isModelHidden: (modelId: string) => options?.hiddenLanguageModelIds?.has(modelId) ?? false, - onDidChangeLanguageModels: Event.None, + onDidChangeLanguageModels: options?.languageModelChanges ?? Event.None, onDidChangeModelVisibility: options?.languageModelVisibilityChanges ?? Event.None, }); instantiationService.stub(ILabelService, options?.labelService ?? new MockLabelService()); @@ -1877,7 +1877,7 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); - test('getModelsSnapshot excludes hidden models and announces visibility changes', () => { + test('getModelsSnapshot excludes hidden models and announces visibility changes', async () => { const matchingModel = { ...createTestLanguageModel('matching'), targetChatSessionType: 'agent-host-copilotcli' }; const hiddenLanguageModelIds = new Set(['matching']); const visibilityChanges = disposables.add(new Emitter()); @@ -1896,9 +1896,31 @@ suite('LocalAgentHostSessionsProvider', () => { assert.deepStrictEqual(provider.getModelsSnapshot(session.sessionId).models, []); hiddenLanguageModelIds.delete('matching'); + const modelsChanged = Event.toPromise(provider.onDidChangeModels); visibilityChanges.fire(); - assert.strictEqual(changes, 1); - assert.deepStrictEqual(provider.getModelsSnapshot(session.sessionId).models.map(model => model.identifier), ['matching']); + const visibleModels = provider.getModelsSnapshot(session.sessionId).models.map(model => model.identifier); + await modelsChanged; + assert.deepStrictEqual({ changes, visibleModels }, { changes: 1, visibleModels: ['matching'] }); + }); + + test('announces language model changes after the model catalog settles', async () => { + const languageModelIds: string[] = []; + const languageModelChanges = disposables.add(new Emitter()); + const provider = createProvider(disposables, agentHost, undefined, { + languageModelIds, + languageModelChanges: languageModelChanges.event, + }); + + let modelIdsAtNotification: readonly string[] = []; + disposables.add(provider.onDidChangeModels(() => { + modelIdsAtNotification = [...languageModelIds]; + })); + const modelsChanged = Event.toPromise(provider.onDidChangeModels); + languageModelChanges.fire('agent-host-copilotcli'); + languageModelIds.push('matching'); + await modelsChanged; + + assert.deepStrictEqual(modelIdsAtNotification, ['matching']); }); test('getModelsSnapshot canonicalizes a matching logical-session model identifier', () => { diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index 46c54d515f2..20defd3f7cf 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -2639,7 +2639,9 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions const gitHubInfo = currentWorkspace?.folders .map(folder => folder.gitRepository?.gitHubInfo.get()) .find(info => info !== undefined); - const repoId = gitHubInfo ? `${gitHubInfo.owner}/${gitHubInfo.repo}` : undefined; + const repoId = gitHubInfo + ? `${gitHubInfo.owner}/${gitHubInfo.repo}` + : currentWorkspace?.folders.map(folder => githubRemoteRepoLabel(folder.root)).find(id => id !== undefined); const selection = await this.commandService.executeCommand(commandId, repoId); if (!selection) { return undefined; 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 26e374b5e51..460f421622f 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 @@ -8,6 +8,7 @@ import { Codicon } from '../../../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { timeout } from '../../../../../../base/common/async.js'; import { DisposableStore, IDisposable, ImmortalReference, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { ThemeIcon } from '../../../../../../base/common/themables.js'; import { URI } from '../../../../../../base/common/uri.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; import { mock, upcastPartial } from '../../../../../../base/test/common/mock.js'; @@ -35,7 +36,7 @@ import { IChatResponseModel } from '../../../../../../workbench/contrib/chat/com import { IChatAgentData } from '../../../../../../workbench/contrib/chat/common/participants/chatAgents.js'; import { IGitRepository, IGitService } from '../../../../../../workbench/contrib/git/common/gitService.js'; import { ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; -import { ChatModelSource, GITHUB_REMOTE_FILE_SCHEME, IChat, ISession, SessionStatus } from '../../../../../services/sessions/common/session.js'; +import { ChatModelSource, GITHUB_REMOTE_FILE_SCHEME, IChat, ISession, ISessionWorkspace, SESSION_WORKSPACE_GROUP_GITHUB, 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'; import { CloudSandboxAgentHostContribution, type ICloudSandboxProvisionedSession } from '../../../remoteAgentHost/browser/cloudSandboxAgentHostContribution.js'; @@ -59,6 +60,17 @@ import { computePullRequestIcon, GitHubPullRequestState, IGitHubPullRequest } fr // ---- Helpers ---------------------------------------------------------------- +interface IGitHubContextBrowseHarness { + readonly commandService: Pick; +} + +const browseForGitHubContext = Reflect.get(CopilotChatSessionsProvider.prototype, '_browseForGitHubContext') as ( + this: IGitHubContextBrowseHarness, + commandId: string, + icon: ThemeIcon, + currentWorkspace: ISessionWorkspace | undefined, +) => Promise; + function createMockAgentSession(resource: URI, opts?: { providerType?: string; title?: string; @@ -431,6 +443,58 @@ suite('CopilotChatSessionsProvider', () => { assert.strictEqual(provider.sessionTypes.length, 1); }); + test('scopes issue and pull request browsing to a selected GitHub repository', async () => { + const calls: { commandId: string; repoId: unknown }[] = []; + const harness: IGitHubContextBrowseHarness = { + commandService: new class extends mock() { + override async executeCommand(commandId: string, repoId?: unknown): Promise { + calls.push({ commandId, repoId }); + return { + repoId: 'cutelyaware/MC4D', + url: `https://github.com/cutelyaware/MC4D/${commandId === 'openIssue' ? 'issues/1' : 'pull/2'}`, + label: `cutelyaware/MC4D#${commandId === 'openIssue' ? '1' : '2'}`, + } as T; + } + }(), + }; + const repositoryRoot = URI.from({ + scheme: GITHUB_REMOTE_FILE_SCHEME, + authority: 'github', + path: '/cutelyaware/MC4D/HEAD', + }); + const workspace: ISessionWorkspace = { + uri: URI.parse('https://github.com/cutelyaware/MC4D'), + label: 'cutelyaware/MC4D', + icon: Codicon.repo, + group: SESSION_WORKSPACE_GROUP_GITHUB, + folders: [{ + root: repositoryRoot, + workingDirectory: repositoryRoot, + name: 'MC4D', + description: undefined, + gitRepository: undefined, + }], + requiresWorkspaceTrust: false, + isVirtualWorkspace: true, + }; + + const issue = await browseForGitHubContext.call(harness, 'openIssue', Codicon.issues, workspace); + const pullRequest = await browseForGitHubContext.call(harness, 'openPullRequest', Codicon.gitPullRequest, workspace); + + assert.deepStrictEqual({ + calls, + issue: { uri: issue?.uri.toString(), label: issue?.label, icon: issue?.icon.id }, + pullRequest: { uri: pullRequest?.uri.toString(), label: pullRequest?.label, icon: pullRequest?.icon.id }, + }, { + calls: [ + { commandId: 'openIssue', repoId: 'cutelyaware/MC4D' }, + { commandId: 'openPullRequest', repoId: 'cutelyaware/MC4D' }, + ], + issue: { uri: 'https://github.com/cutelyaware/MC4D/issues/1', label: 'cutelyaware/MC4D#1', icon: Codicon.issues.id }, + pullRequest: { uri: 'https://github.com/cutelyaware/MC4D/pull/2', label: 'cutelyaware/MC4D#2', icon: Codicon.gitPullRequest.id }, + }); + }); + test('sessionTypes excludes Local', () => { const provider = createProvider(disposables, model); assert.ok(!provider.sessionTypes.some(type => type.id === SessionType.Local)); diff --git a/test/componentFixtures/blocks-ci-screenshots.md b/test/componentFixtures/blocks-ci-screenshots.md index b5bec484243..02e2a56a761 100644 --- a/test/componentFixtures/blocks-ci-screenshots.md +++ b/test/componentFixtures/blocks-ci-screenshots.md @@ -181,25 +181,25 @@ ![screenshot](https://hediet-screenshots.azurewebsites.net/images/fe4b95bf8348637bba9f8c0dda791924e6c67fd7b5d173398f9b2c0bfc9f7071) #### sessions/chat/newWidget/newChatWidget/NewSessionAttachedContext/Dark -![screenshot](https://hediet-screenshots.azurewebsites.net/images/f3db1a2e75f5320f7047aac91b148f6600fea29ce8135d370a39fa964e64e873) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/1d76cd2c4bda9bed2203215ccd84ef0644895a8eddd5efc5934520dce3f5fce8) #### sessions/chat/newWidget/newChatWidget/NewSessionAttachedContext/Light -![screenshot](https://hediet-screenshots.azurewebsites.net/images/284dc3b398fe35444af49cd1726f3efdf071835f9a12b173c624b16f329dc738) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/fe415859b934c657863844d2bfbf30da4ae84a2b167301ba2517d317588c6ed8) #### sessions/chat/newWidget/newChatWidget/NewSessionGitHubContextPicker/Dark -![screenshot](https://hediet-screenshots.azurewebsites.net/images/d65a5c982df3188ca688e5b0c317ecc5f0be1451a0061f56dab251a43d708c21) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/a5e62a510e536d64b4d76dc940a3ad0b744caa7c3a58111492be9f20ea8db801) #### sessions/chat/newWidget/newChatWidget/NewSessionGitHubContextPicker/Light -![screenshot](https://hediet-screenshots.azurewebsites.net/images/a80dd54eb5c270bb59ec6f3830a107812b4cf7bc93371451af520cdef978da30) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/55683f61601d660f2f94c5c7a24179d782a0776464fa2680c5e00fa7b6fb2ee8) #### sessions/chat/newWidget/newChatWidget/NewSessionRemoteWorkspace/Dark -![screenshot](https://hediet-screenshots.azurewebsites.net/images/5a8e493276778f6d2a65a208b9fa0cb9f7bf7abafcf78d5adf8151b3459f0b9f) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/8c86c66d82f1ef4e19e918463bfb5827167a22e238d81f66d1e48870862fb229) #### sessions/chat/newWidget/newChatWidget/NewSessionRemoteWorkspace/Light -![screenshot](https://hediet-screenshots.azurewebsites.net/images/8858fb38373092200aee6675c0f6cbcb30ad4dce7c62acd8300d70ff64f8ae6b) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/6a96db096792e9d7f75306eaa04c7e89e26a39870871b8480cc39fb5cf2cfe71) #### sessions/chat/newWidget/newChatWidget/NewSessionWorkspacePicker/Dark -![screenshot](https://hediet-screenshots.azurewebsites.net/images/496be935d13b3be52e1e3ba5471317475a5fa1694a58daf9134e749c0fbe0acb) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/e9cbba25dc68f5f66bf85428b133461f2d1ebc236b1d6f82d08ae05aab9c33c7) #### sessions/chat/newWidget/newChatWidget/NewSessionWorkspacePicker/Light -![screenshot](https://hediet-screenshots.azurewebsites.net/images/1cba4648260e466818fe0c914cd29516cc465f322a112be65de67e2b49a20284) +![screenshot](https://hediet-screenshots.azurewebsites.net/images/c51cbace6b7e770064231bb3aac07f91ad41e4fcf4d5bd6aedfbdb00c775fa45)