Mode picker in Sessions app by sandy081 · Pull Request #298199 · microsoft/vscode

sandy081 · 2026-02-27T08:55:39Z

Mode picker in Sessions app

…chat sessions

…tom agent target

…lback

…agent selection

…nd update item structure

…in SessionsManagementService

…cker

…model

…-02-26T22-26-52 # Conflicts: # src/vs/sessions/contrib/chat/browser/newChatViewPane.ts

Copilot

Pull request overview

This PR adds a mode picker to the Sessions app, allowing users to select between the default "Agent" mode and any custom agents when starting a new local/Background session. The mode picker filters custom agents based on the session type's target and integrates with the existing chat mode infrastructure.

Changes:

Adds a new ModePicker widget for selecting chat modes in the Sessions app
Extends INewSession interface to track the selected mode via modeId property
Updates SessionsManagementService to resolve mode information and pass it to chat requests

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
src/vs/sessions/contrib/chat/browser/modePicker.ts	New self-contained mode picker widget that displays Agent and filtered custom modes for local sessions
src/vs/sessions/contrib/chat/browser/newChatViewPane.ts	Integrates ModePicker into the new chat widget, shows/hides based on session type, syncs mode changes to session
src/vs/sessions/contrib/chat/browser/newSession.ts	Adds modeId property to INewSession interface with implementations in LocalNewSession (stores) and RemoteNewSession (no-op)
src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts	Resolves mode from session's modeId, constructs modeInstructions with tool references, sets mode on input model for UI consistency

Comments suppressed due to low confidence (1)

src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts:309

Non-null assertion operator (!) is used on resolvedMode at line 309 without a null check. While rawModeInstructions being truthy implies resolvedMode exists, this assumption is fragile. If the conditional logic changes, this could cause a runtime error. Consider using optional chaining instead: resolvedMode?.name.get().

			name: resolvedMode!.name.get(),

Copilot · 2026-02-27T09:01:58Z

src/vs/sessions/contrib/chat/browser/modePicker.ts

+	private _updateTriggerLabel(): void {
+		if (!this._triggerElement) {
+			return;
+		}
+
+		dom.clearNode(this._triggerElement);
+
+		const icon = this._selectedMode.icon.get();
+		if (icon) {
+			dom.append(this._triggerElement, renderIcon(icon));
+		}
+
+		const labelSpan = dom.append(this._triggerElement, dom.$('span.sessions-chat-dropdown-label'));
+		labelSpan.textContent = this._selectedMode.label.get();
+		dom.append(this._triggerElement, renderIcon(Codicon.chevronDown));
+	}


The ModePicker does not visually indicate when no custom modes are available, unlike the CloudModelPicker which adds a 'disabled' class (see modelPicker.ts line 202). While the picker will show only the default Agent mode, it would be better UX to add visual feedback when the picker is effectively showing only one option. Consider adding disabled styling when modes.length is 1.

Copilot · 2026-02-27T09:01:59Z

src/vs/sessions/contrib/chat/browser/modePicker.ts

+		if (modes.length === 0) {
+			return;
+		}


The early return when modes.length is 0 prevents the picker from being shown, but this condition should never be true since _getAvailableModes always includes at least ChatMode.Agent (line 109). This check is defensive but may hide bugs where the default Agent mode is unexpectedly missing. Consider removing this check or adding logging to detect if this condition ever occurs.

Suggested change

if (modes.length === 0) {

return;

}

Copilot · 2026-02-27T09:01:59Z

src/vs/sessions/contrib/chat/browser/newSession.ts

 	}

+	setModeId(_modeId: string | undefined): void {
+		// No-op for remote sessions


The setModeId method is a no-op for RemoteNewSession (line 241-243), but there's no documentation explaining why remote sessions don't support mode selection. Consider adding a comment explaining the reasoning, such as whether mode selection is not supported by the remote session provider or if modes are determined server-side.

Suggested change

// No-op for remote sessions

// Intentionally a no-op: remote sessions do not support client-side mode selection.

// Any mode or behavior differences are determined by the remote session provider/server.

Copilot · 2026-02-27T09:01:59Z

src/vs/sessions/contrib/chat/browser/modePicker.ts

+
+		const trigger = dom.append(slot, dom.$('a.action-label'));
+		trigger.tabIndex = 0;
+		trigger.role = 'button';


The trigger element has tabIndex and role set for keyboard accessibility (lines 73-74), but there's no aria-label or aria-describedby attribute to provide context for screen reader users. Consider adding an aria-label such as "Select chat mode" to improve accessibility, similar to how other pickers in the codebase provide accessible labels.

Suggested change

trigger.role = 'button';

trigger.role = 'button';

trigger.setAttribute('aria-label', localize('sessions.modePicker.ariaLabel', "Select chat mode"));

Copilot · 2026-02-27T09:02:00Z

src/vs/sessions/contrib/chat/browser/modePicker.ts

+		@IChatSessionsService private readonly chatSessionsService: IChatSessionsService,
+		@ICommandService private readonly commandService: ICommandService,
+	) {
+		super();


The ModePicker does not listen to changes in available modes. If custom agents are added, removed, or modified after the picker is created, the picker will not update its list of available modes until the user reopens the picker. Consider adding a listener to chatModeService.onDidChangeChatModes to refresh the available modes dynamically.

Suggested change

super();

super();

this._register(this.chatModeService.onDidChangeChatModes(() => {

// Refresh the trigger label when available chat modes change

if (this._triggerElement) {

this._updateTriggerLabel();

}

}));

Copilot · 2026-02-27T09:02:00Z

src/vs/sessions/contrib/sessions/browser/sessionsManagementService.ts

+			// Set the selected mode on the input model so the mode picker reflects it
+			if (session.modeId) {
+				const resolvedMode = this.chatModeService.findModeById(session.modeId);
+				if (resolvedMode) {
+					model.inputModel.setState({
+						mode: { id: resolvedMode.id, kind: resolvedMode.kind }
+					});
+				}


The mode resolution logic is duplicated: it's first resolved at lines 302-313 to build sendOptions, and then resolved again at lines 360-365 to set the input model state. This could lead to inconsistencies if the mode is deleted between the two calls. Consider consolidating this logic by reusing the resolvedMode variable from the first resolution, or move the input model state setting earlier to avoid redundant lookups.

This issue also appears on line 309 of the same file.

Copilot · 2026-02-27T09:02:00Z

src/vs/sessions/contrib/chat/browser/modePicker.ts

+	private _getAvailableModes(): IChatMode[] {
+		const customAgentTarget = this.chatSessionsService.getCustomAgentTargetForSessionType(AgentSessionProviders.Background);
+		const effectiveTarget = customAgentTarget && customAgentTarget !== Target.Undefined ? customAgentTarget : Target.GitHubCopilot;
+		const modes = this.chatModeService.getModes();
+
+		// Always include the default Agent mode
+		const result: IChatMode[] = [ChatMode.Agent];
+
+		// Add custom modes matching the target
+		for (const mode of modes.custom) {
+			const target = mode.target.get();
+			if (target === effectiveTarget || target === Target.Undefined) {
+				result.push(mode);
+			}
+		}
+
+		return result;
+	}


The mode filtering logic does not check the visibility.userInvocable property of custom modes. According to the codebase conventions (see chatModes.ts line 116 and line 162), modes with userInvocable set to false should not be shown in user-facing UI. The picker should filter out modes where visibility.userInvocable is false to prevent users from selecting modes that are only intended for agent invocation.

sandy081 added 9 commits February 27, 2026 01:48

feat: add mode picker functionality and integrate mode management in …

a1f1da5

…chat sessions

feat: integrate IChatSessionsService and enhance mode picker with cus…

c850537

…tom agent target

feat: update mode picker to use MenuItemAction and remove setMode cal…

5ff8465

…lback

fix: update available modes logic to use effective target for custom …

f111e7a

…agent selection

feat: enhance mode picker to support configuration of custom agents a…

7871ff8

…nd update item structure

feat: integrate ILanguageModelToolsService and enhance mode handling …

d72a55a

…in SessionsManagementService

feat: add group property to configure custom agents action in mode pi…

cb8b60e

…cker

feat: update session mode handling to reflect selected mode in input …

cb1ad72

…model

Merge remote-tracking branch 'origin/main' into copilot-worktree-2026…

bc86a2e

…-02-26T22-26-52 # Conflicts: # src/vs/sessions/contrib/chat/browser/newChatViewPane.ts

sandy081 self-assigned this Feb 27, 2026

Copilot AI review requested due to automatic review settings February 27, 2026 08:55

Copilot started reviewing on behalf of sandy081 February 27, 2026 08:56 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mode picker in Sessions app#298199

Mode picker in Sessions app#298199
sandy081 wants to merge 9 commits intomainfrom
copilot-worktree-2026-02-26T22-26-52

sandy081 commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// No-op for remote sessions
	// Intentionally a no-op: remote sessions do not support client-side mode selection.
	// Any mode or behavior differences are determined by the remote session provider/server.

	trigger.role = 'button';
	trigger.role = 'button';
	trigger.setAttribute('aria-label', localize('sessions.modePicker.ariaLabel', "Select chat mode"));

-		super();
+		super();
+		this._register(this.chatModeService.onDidChangeChatModes(() => {
+			// Refresh the trigger label when available chat modes change
+			if (this._triggerElement) {
+				this._updateTriggerLabel();
+			}
+		}));

Conversation

sandy081 commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants