Skip to content

Fix sandbox terminal command display#309565

Merged
dileepyavan merged 1 commit intomainfrom
DileepY/309430
Apr 13, 2026
Merged

Fix sandbox terminal command display#309565
dileepyavan merged 1 commit intomainfrom
DileepY/309430

Conversation

@dileepyavan
Copy link
Copy Markdown
Member

@dileepyavan dileepyavan commented Apr 13, 2026

Summary

Fixes #309430

This prevents internal sandbox wrapper commands from leaking into the chat view for terminal tool invocations while running background.

The terminal command pipeline can apply multiple command-line rewrites. The sandbox rewriter correctly sets a clean forDisplay command while storing the wrapped command in toolEdited for execution. However, later execution-only rewrites, such as background detach wrapping, could replace forDisplay with a command that already included sandbox details. The chat UI then used that overwritten display command and showed the sandbox wrapper to the user.

This change makes the first explicit forDisplay value sticky during command rewriting:

  • execution continues to use the fully rewritten command via toolEdited
  • chat display continues to use the clean user-facing command via forDisplay
  • sandboxed invocations still show the sandbox label/icon without exposing the wrapper implementation

Testing

  • npm run transpile-client
  • ./scripts/test.sh --run src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts

Added regression coverage for a sandboxed async command with background detach enabled, verifying that chat displays echo hello while the executed command remains nohup sandbox-runtime echo hello &.

Copilot AI review requested due to automatic review settings April 13, 2026 17:57
@github-actions
Copy link
Copy Markdown
Contributor

Screenshot Changes

Base: 985aca65 Current: 865ded41

Changed (1)

agentSessionsViewer/NeedsInput/Light
Before After
before after

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

Fixes a terminal chat UX issue where internal sandbox wrapper commands could appear in the chat invocation text when additional execution-only command rewrites (eg. background detach) run after sandbox wrapping.

Changes:

  • Make the first forDisplay value produced during command rewriting “sticky” so later rewriters can’t overwrite it.
  • Add a regression test covering a sandboxed async command with background detach enabled, ensuring chat displays the clean command while execution uses the fully wrapped command.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts Adjusts command rewrite aggregation so forDisplayCommand is only set once (first non-undefined).
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts Adds regression coverage ensuring sandbox wrapper details don’t leak into chat display for detached async commands.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@dileepyavan dileepyavan merged commit 1eb3017 into main Apr 13, 2026
30 checks passed
@dileepyavan dileepyavan deleted the DileepY/309430 branch April 13, 2026 19:12
@vs-code-engineering vs-code-engineering Bot added this to the 1.117.0 milestone Apr 13, 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.

Full sandbox command being printed in chat view

3 participants