Skip to content

Remote agent host: fix model picker on new session page#306553

Merged
roblourens merged 5 commits intomainfrom
roblou/horizontal-jackal
Mar 31, 2026
Merged

Remote agent host: fix model picker on new session page#306553
roblourens merged 5 commits intomainfrom
roblou/horizontal-jackal

Conversation

@roblourens
Copy link
Copy Markdown
Member

@roblourens roblourens commented Mar 31, 2026

The model picker on the new session page didn't show models for remote agent host sessions. Two issues:

1. Models never populated: The remote agent host server initially reports agents with 0 models (before authentication). After auth succeeds, the local agent host calls refreshModels() to trigger a root state update with the full model list — but the remote agent host contribution was missing this call. Added refreshModels() after _authenticateWithConnection and in _authenticateAllConnections, matching the local agent host pattern.

2. Model picker not visible: The localModelPicker action's when clause only matched the default Copilot provider (activeSessionProviderId == 'default-copilot'). Widened it to also match remote agent host providers via ContextKeyExpr.regex(ActiveSessionProviderIdContext.key, /^agenthost-/). Also made getAvailableModels dynamic — it now derives targetChatSessionType from the active provider's untitled session resource scheme instead of hardcoding AgentSessionProviders.Background.

Also: Restores two changes that were accidentally reverted by a bad merge in an earlier PR — the delegate.setModel fix (#305345) and multi-select context menu bridge (#306332).

(Written by Copilot)

roblourens and others added 2 commits March 30, 2026 16:22
Co-authored-by: Copilot <copilot@github.com>
Previously, registerLanguageModelProvider only eagerly resolved models
when _hasStoredModelForVendor returned true. Otherwise it waited for
onDidChange. This created a hidden temporal coupling: if a provider
populated models before registration (like AgentHostLanguageModelProvider),
the onDidChange event fired with no listener attached, and the models
never appeared.

Fix: always call _resolveAllLanguageModels on registration. This is safe
(the provider guard handles unregistration races, and the per-vendor
sequencer prevents duplicate work).

Also use ActiveSessionProviderIdContext.key instead of a hardcoded string
in the remote agent host context key expression.

(Written by Copilot)
Copilot AI review requested due to automatic review settings March 31, 2026 01:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a temporal-coupling issue in the chat language model registry where some providers could populate models before being registered, causing the service to miss the update and never surface those models (notably impacting remote agent host sessions and the model picker).

Changes:

  • Always resolve language models when a provider is registered (instead of only when stored preferences exist).
  • Add regression tests covering eager resolution without stored preferences and the “models updated before registration” scenario.
  • Use ActiveSessionProviderIdContext.key in the remote agent host context expression and improve session context-menu bridging to support multi-selection.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/vs/workbench/contrib/chat/common/languageModels.ts Makes provider registration always kick off model resolution to avoid missed pre-registration updates.
src/vs/workbench/contrib/chat/test/common/languageModels.test.ts Adds/adjusts tests to validate eager model resolution behavior and cover the regression scenario.
src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsActions.ts Uses the centralized context key and improves command bridging to forward multi-selection session contexts.
Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/common/languageModels.ts:1006

  • registerLanguageModelProvider now always kicks off _resolveAllLanguageModels asynchronously. If the returned IDisposable is disposed (provider unregistered) while that async resolution is still in-flight, the queued work can still complete and repopulate _modelCache after unregistration (because _resolveAllLanguageModels captures provider before the sequencer and doesn’t re-check that the provider is still the currently registered one before applying results). Consider adding a guard/version check inside the queued task (e.g., re-read this._providers.get(vendorId) and ensure it matches the provider instance that started the resolve) so stale resolves are discarded after unregistration.
		this._providers.set(vendor, provider);

		// Always resolve models on registration so callers don't need to
		// worry about firing onDidChange after registering the provider.
		this._resolveAllLanguageModels(vendor, true);

		const modelChangeListener = provider.onDidChange(() => {
			this._resolveAllLanguageModels(vendor, true);
		});

roblourens and others added 2 commits March 30, 2026 19:16
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
@roblourens roblourens changed the title Always resolve language models on provider registration Remote agent host: fix model picker on new session page Mar 31, 2026
@roblourens roblourens marked this pull request as ready for review March 31, 2026 02:22
@roblourens roblourens enabled auto-merge (squash) March 31, 2026 02:23
@roblourens roblourens merged commit d13d7c7 into main Mar 31, 2026
18 checks passed
@roblourens roblourens deleted the roblou/horizontal-jackal branch March 31, 2026 02:47
@vs-code-engineering vs-code-engineering bot added this to the 1.115.0 milestone Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants