Skip to content

chat: Warn about BYOK models in remote Copilot sessions - #339941

Merged
Vritant Bhardwaj (vritant24) merged 5 commits into
microsoft:mainfrom
vritant24:agents/byok-models-notification-alert
Oct 6, 2026
Merged

Vritant Bhardwaj (vritant24) merged 5 commits into
microsoft:mainfrom
vritant24:agents/byok-models-notification-alert

Conversation

@vritant24

@vritant24 Vritant Bhardwaj (vritant24) commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Summary

BYOK models are not supported by the Copilot harness in remote sessions. Add a chat-input warning in the editor so users with configured BYOK models understand this limitation.

  • Require an actual resolved BYOK model, the Copilot harness, and a live connection to the selected remote host or remote workspace.
  • Never show the warning in the Agents window, for local sessions, or for other harnesses.
  • Limit the warning to the first eligible input/session per editor-window instance. Model changes and reconnections do not reset dismissal.
  • Wait for a concrete session resource before displaying the warning so startup does not prematurely consume the one-time notice.
  • Offer Don't Show Again, persisted per profile across reloads.
  • Pass the rendering input context to the existing notification onDidShow callback to enforce first-input ownership.
  • Update accessibility help and add regression coverage for connection state, startup, message sending, and profile-isolated muting.

No linked issue was provided.

Testing

Passed:

  • npm run compile-client — completed with 0 errors.
  • ./scripts/test.sh --run src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostRemoteByokNotification.test.ts --run src/vs/workbench/contrib/chat/test/browser/widget/input/chatInputNotificationWidget.test.ts --run src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts — 110 tests passed.
  • node build/hygiene.ts src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostRemoteByokNotification.ts src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostRemoteByokNotification.test.ts — passed.
  • git diff --cached --check and the normal pre-commit hygiene hook — passed.

Manual verification in the OSS editor using TestResolver and a synthetic BYOK provider registered through the real language-model service:

  • A connected remote Copilot input displays the warning when the provider resolves.
  • Keyboard dismissal restores focus to the chat input.
  • Starting another chat and re-registering the provider do not repeat the warning.
  • After ordinary dismissal, Developer: Reload Window allows the warning to appear again once the session initializes.
  • Don't Show Again hides the warning and persists through new chats, provider changes, and Developer: Reload Window.

For manual verification in a fresh, unmuted OSS editor profile, use the built-in TestResolver (./scripts/code.sh --remote=test+test); Remote-SSH does not support OSS:

  1. Connect the remote workspace and select the Agent Host Copilot harness.
  2. With no BYOK models, confirm no warning. Add the first BYOK model and wait for discovery; the warning should appear without reloading.
  3. Confirm local sessions, other harnesses, disconnected hosts, and the Agents window do not show it.
  4. Confirm other chat inputs/sessions do not repeat the warning in the same window.
  5. Select Don't Show Again, reload the window, and confirm it stays hidden.

Not fully verified manually: real provider authentication, SSH/WSL/Dev Container/Codespaces transports, actual network interruption, multiple concurrent windows, full application restart, physical screen-reader output, and the complete theme/layout matrix. Automated connection-state and widget tests do not substitute for those environment-specific checks.

Show an editor-only warning for configured BYOK models when the selected Copilot harness is connected remotely. Limit it to the first eligible input per window and support persistent profile-scoped muting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 23:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Cached remote environment data can incorrectly show the warning for an already disconnected workspace.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Adds an editor-only warning when configured BYOK models are unavailable in connected remote Copilot sessions.

Changes:

  • Adds connection-, model-, harness-, and window-aware warning logic with persistent muting.
  • Passes input context to notification visibility callbacks.
  • Adds accessibility guidance, regression tests, and a manual checklist.
File Description
agentHostRemoteByokNotification.ts Implements warning eligibility and lifecycle.
agentHost.contribution.ts Registers the contribution.
chatInputNotificationService.ts Extends the visibility callback API.
chatInputNotificationWidget.ts Supplies rendering context to callbacks.
agentHostRemoteByokNotification.test.ts Tests warning behavior.
chatInputNotificationWidget.test.ts Tests callback context delivery.
chatAccessibilityHelp.ts Documents accessible interaction.
chatAccessibilityHelp.test.ts Tests help visibility.
BYOK-REMOTE-NOTIFICATION-TEST-CHECKLIST.md Provides manual validation steps.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Use current remote transport state instead of cached environment data when deciding whether to show the warning. Require the rendering context for notification visibility callbacks and cover startup disconnects and reconnections.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
roblourens previously approved these changes Oct 6, 2026
Avoid consuming the one-time remote warning before the chat has a stable session resource. Cover startup, message sending, and profile-isolated muting, and remove the manual checklist.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
roblourens previously approved these changes Oct 6, 2026
@vritant24 Vritant Bhardwaj (vritant24) added the ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch label Oct 6, 2026
@vs-code-engineering

Copy link
Copy Markdown
Contributor

This PR will be automatically cherry-picked to release/1.141 when merged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

# Conflicts:
#	src/vs/workbench/contrib/chat/test/browser/accessibility/chatAccessibilityHelp.test.ts
#	src/vs/workbench/contrib/chat/test/electron-browser/agentSessionsDraftHandoff.test.ts
@vritant24
Vritant Bhardwaj (vritant24) merged commit 75d6718 into microsoft:main Oct 6, 2026
54 of 56 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.141.0 milestone Oct 6, 2026
@vs-code-engineering vs-code-engineering Bot added release-cherry-pick Automated cherry-pick between release and main branches and removed ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch labels Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-cherry-pick Automated cherry-pick between release and main branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants