fix: correct terminal sandbox icon in thinking dropdown#304327
Merged
fix: correct terminal sandbox icon in thinking dropdown#304327
Conversation
When sandbox is enabled, the tool-level icon was set to terminalSecure (lock) at registration time, which leaked into every rendering path during streaming before we knew if a specific command was actually sandboxed. This caused unsandboxed commands (requestUnsandboxedExecution=true) to show the lock icon. Fix: - Set toolData.icon to always be Codicon.terminal (no lock). The per-command isSandboxWrapped flag in toolSpecificData is the authoritative source. - In the existing autorun in trackToolMetadata, update the icon element when the tool transitions out of streaming and toolSpecificData becomes available. - Store icon elements in toolIconsByCallId map for direct access. Fixes #303505
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an incorrect “sandbox/lock” icon being shown for unsandboxed Run in Terminal invocations in the chat “thinking” dropdown by decoupling the tool’s registered icon from per-invocation sandbox state, and updating the rendered icon once invocation-specific data becomes available.
Changes:
- Set
Run in Terminaltool registration icon to always useCodicon.terminal(no lock), regardless of sandbox enablement. - Track per-tool-call icon elements in the thinking UI and update the icon when a tool transitions out of streaming and terminal
toolSpecificDatais available.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts | Ensures the registered tool icon is always the non-lock terminal icon to avoid misleading streaming UI. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts | Adds tracking for icon DOM nodes and updates terminal icons post-streaming based on isSandboxWrapped. |
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts
Show resolved
Hide resolved
roblourens
approved these changes
Mar 24, 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.
When sandbox is enabled, the tool-level icon was set to terminalSecure (lock) at registration time, which leaked into every rendering path during streaming before we knew if a specific command was actually sandboxed. This caused unsandboxed commands (requestUnsandboxedExecution=true) to show the lock icon.
Fix:
Fixes #303505