Conversation
Split the nested `chat.tools.terminal.sandbox.network` object setting into three flat settings: - `chat.tools.terminal.sandbox.network.allowedDomains` - `chat.tools.terminal.sandbox.network.deniedDomains` - `chat.tools.terminal.sandbox.network.allowTrustedDomains` Add configuration migration to automatically migrate existing values. The old setting is preserved as deprecated with a pointer to the new ones. Fixes #304232
Contributor
There was a problem hiding this comment.
Pull request overview
This PR flattens the terminal sandbox network configuration from a single nested object setting into three separate settings, and introduces a configuration migration to automatically move existing user values over while keeping the legacy key deprecated for compatibility.
Changes:
- Replace
chat.tools.terminal.sandbox.networkwith three flat settings for allowed domains, denied domains, and trusted-domain inclusion. - Update terminal sandbox runtime/config consumers and related tool messaging to use the new setting IDs.
- Add a configuration migration that splits old object values into the new keys and clears the deprecated setting.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts | Updates a config-change test to target the new allowed-domains key. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/terminalSandboxService.test.ts | Updates sandbox service tests to use the new flattened keys. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts | Switches runtime reads and change detection from the nested object to the three flat settings. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandbox.ts | Removes now-unused network/settings interfaces formerly tied to the nested object setting. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts | Defines the new settings and introduces a deprecated placeholder for the legacy key. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/sandboxOutputAnalyzer.ts | Updates guidance text to reference the new allowed-domains setting ID. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts | Updates re-registration triggers to watch the new keys. |
| src/vs/workbench/contrib/terminal/terminalContribExports.ts | Exposes new (and deprecated) setting IDs for other terminal layers. |
| src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts | Registers a configuration migration to split the legacy object into the new flat settings. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts:196
- Reading only the new flattened settings here means existing user configs that still use the deprecated object key (
chat.tools.terminal.sandbox.network) will be treated as “no allowed domains” until the configuration migration has executed. Because configuration migrations are applied by a workbench contribution (registered at a later phase), there can be a window where the sandbox runs with the wrong network rules. Consider falling back to reading the deprecated object value when the new keys are unset, so behavior is correct even before/without migration.
const allowedDomainsSetting = this._configurationService.getValue<string[]>(TerminalChatAgentToolsSettingId.TerminalSandboxNetworkAllowedDomains) ?? [];
const deniedDomainsSetting = this._configurationService.getValue<string[]>(TerminalChatAgentToolsSettingId.TerminalSandboxNetworkDeniedDomains) ?? [];
const allowTrustedDomains = this._configurationService.getValue<boolean>(TerminalChatAgentToolsSettingId.TerminalSandboxNetworkAllowTrustedDomains) ?? false;
...bench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts
Show resolved
Hide resolved
...rkbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts
Show resolved
Hide resolved
joshspicer
approved these changes
Mar 23, 2026
pwang347
approved these changes
Mar 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split the nested
chat.tools.terminal.sandbox.networkobject setting into three flat settings:chat.tools.terminal.sandbox.network.allowedDomainschat.tools.terminal.sandbox.network.deniedDomainschat.tools.terminal.sandbox.network.allowTrustedDomainsAdd configuration migration to automatically migrate existing values. The old setting is preserved as deprecated with a pointer to the new ones.
Fixes #304232