Conversation
Prevent sandbox-wrapped command lines from leaking as output when commands produce no actual output. Adds stripCommandEchoAndPrompt() to isolate real output from marker-based terminal buffer captures. Also adds configurable idle poll interval and shell integration timeout=0 support for faster test execution.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes run_in_terminal tool output sanitization when marker-based capture includes the echoed sandbox-wrapper command and the next prompt (most visible when the actual command produces no stdout/stderr), and adds configuration/testing support to reduce idle-detection latency in tests.
Changes:
- Add
stripCommandEchoAndPrompt()and apply it to marker-based output inNoneExecuteStrategyandBasicExecuteStrategy. - Introduce
chat.tools.terminal.idlePollIntervaland thread it through all execute strategies; extendtrackIdleOnPromptwith a configurable fallback. - Update command-edit note detection to ignore cosmetic rewrites, allow
terminal.integrated.shellIntegration.timeout=0, and add unit/integration coverage.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/strategyHelpers.ts | Adds output stripping helper for command echo + prompt removal. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts | Uses configurable idle polling + strips echo/prompt from marker output. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts | Uses configurable idle polling + strips echo/prompt from marker output. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts | Uses configurable idle polling for idle-on-prompt fallback. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts | Adds optional promptFallbackMs to trackIdleOnPrompt. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts | Avoids “tool simplified the command” note for cosmetic rewrites by comparing display-normalized forms. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts | Adds chat.tools.terminal.idlePollInterval setting metadata. |
| src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts | Allows terminal.integrated.shellIntegration.timeout=0 to produce a 0ms wait. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/strategyHelpers.test.ts | Unit tests for stripCommandEchoAndPrompt(). |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/noneExecuteStrategy.test.ts | Unit tests ensuring no-output cases don’t leak sandbox wrapper text. |
| extensions/vscode-api-tests/src/singlefolder-tests/chat.runInTerminal.test.ts | Integration coverage for shell integration on/off and sandbox scenarios. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/noneExecuteStrategy.test.ts:109
- Same issue as above: NoneExecuteStrategy now requires an IConfigurationService, but the test calls the constructor with only three arguments, which will fail to compile / mis-wire dependencies. Please adjust this instantiation to provide IConfigurationService (ideally via instantiationService + TestConfigurationService).
const logService = createLogService();
const strategy = store.add(new NoneExecuteStrategy(instance, () => false, logService));
const cts = store.add(new CancellationTokenSource());
.../workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/strategyHelpers.ts
Outdated
Show resolved
Hide resolved
.../workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/strategyHelpers.ts
Outdated
Show resolved
Hide resolved
...vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/noneExecuteStrategy.test.ts
Show resolved
Hide resolved
…utput Anchor prompt-detection regexes to specific prompt shapes instead of broadly matching any line ending with $, #, %, or >. This prevents stripping real command output like "100%", "<div>", or "item #".
In CI, ^C cancellations leave stale prompt fragments before the actual command echo line. The leading-strip loop now continues scanning past unmatched lines until it finds the command echo, instead of breaking on the first non-matching line.
- Add trailing prompt patterns for hostname:path user$ (no @ sign) - Handle wrapped prompt fragments like "er$" at line boundaries - Add stripCommandEchoAndPrompt to RichExecuteStrategy marker fallback - Context-aware wrapped prompt continuation detection
…tripping - Add bubblewrap and socat to Linux CI apt-get install - Make sandbox test assertions platform-aware (macFileSystem vs linuxFileSystem) - Make /etc/shells test accept both macOS and Linux first-line format - Broaden wrapped prompt fragment regex to handle path chars (ts/testWorkspace$) - Fix continuation pattern to match user@host:path wrapped lines - Apply stripCommandEchoAndPrompt to getOutput() in BasicExecuteStrategy (basic shell integration lacks reliable 133;C markers so getOutput() can include command echo) - Keep RichExecuteStrategy getOutput() unstripped (rich integration has reliable markers)
…railing prompt regex matching
…ssage - Handle /usr/bin/bash (Linux) vs /bin/bash (macOS) in /tmp write test - Handle 'Read-only file system' (Linux) vs 'Operation not permitted' (macOS) - Add 'Read-only file system' to outputLooksSandboxBlocked heuristic - Replace newlines with spaces (not empty) to handle terminal wrapping - Extract outputLooksSandboxBlocked as exported function with unit tests
…andbox-no-output-leak
Add execPath to IRemoteAgentEnvironment so the server sends its actual process.execPath to the client. The sandbox service now uses this instead of hardcoding appRoot + '/node', which only works in production builds.
…dle partial command echoes - setupRecreatingStartMarker returns IDisposable to stop marker recreation before sending commands (prevents marker jumping on PSReadLine re-renders) - noneExecuteStrategy waits for cursor to move past start line after sendText before starting idle detection (prevents end marker at same line as start) - findCommandEcho supports suffix matching for partial command echoes from wrapped getOutput() results (shell integration ON with long commands) - Suffix matching requires mid-word split to avoid false positives on output that happens to be a suffix of the command (e.g. echo output) - Integration tests: use ; separator on Windows, add && conversion test, handle Windows exit code quirks with cmd /c
.../workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/strategyHelpers.ts
Outdated
Show resolved
Hide resolved
.../workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/strategyHelpers.ts
Outdated
Show resolved
Hide resolved
...kbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/noneExecuteStrategy.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/services/remote/common/remoteAgentEnvironmentChannel.ts
Show resolved
Hide resolved
- Strip sensitive data from debug logs (log metadata only) - Use array join instead of O(n^2) string concat in stripNewLinesAndBuildMapping - Add 5s timeout to cursor-move wait to prevent indefinite hangs - Align shellIntegrationTimeout descriptions (0 = skip the wait)
These are required for terminal sandbox integration tests.
Shell integration cannot be injected into /bin/sh, causing loss of exit code detection. This matches the existing cmd.exe -> powershell override pattern.
… lines - Extend bracketed prompt patterns from isUnixAt to isUnix so prompts like [W007DV9PF9-1:~/path] are recognized (CI macOS prompt format) - Cap trailing prompt stripping at 2 non-empty lines to prevent over-stripping legitimate output - Add unit tests for bracketed prompt without @ format
auto-merge was automatically disabled
March 22, 2026 08:23
Pull request was converted to draft
Split trailing prompt patterns into two categories: - Complete prompts (user@host:~ $, PS C:\>, etc.) stop stripping immediately — anything above is command output, not a wrapped prompt - Fragment patterns (er$, ] $, [host:~/path...) allow continued stripping to reassemble wrapped prompts This prevents falsely stripping output lines that happen to end with $ or # when a real complete prompt sits below them. Added adversarial tests verifying correct behavior for output containing prompt-like characters.
lszomoru
approved these changes
Mar 22, 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.
Fixes #303531
Problem
When the
run_in_terminaltool executes commands (especially sandbox-wrapped ones) that produce no output, the raw command echo and shell prompt leak into the tool result. The LLM and user see long, confusing strings like the full sandbox wrapper command instead of the expected "Command produced no output".Additionally, several related issues with terminal command execution reliability were discovered and fixed:
execPathwas incorrectly resolved in remote environmentsRoot Cause
Without shell integration (
NoneExecuteStrategy) or when shell integration markers misfire, the tool captures the terminal buffer between xterm markers usinggetContentsAsText(). This raw capture includes:sendTextwrote, including the full sandbox wrapper)Previously there was no stripping of (1) and (3), so the entire command echo was returned as "output".
Changes
1. Strip command echo and prompt from terminal output (
strategyHelpers.ts)New
stripCommandEchoAndPrompt()function that removes leading command echo lines and trailing prompt lines from marker-based output:findCommandEcho()— Locates the command text in the output, handling terminal line wrapping by stripping newlines and building an index mapping. Supports suffix matching forgetOutput()cases where only the wrapped continuation appearsuser@host:path $,] $,PS C:\>,C:\>,❯,>>>, and wrapped multi-line promptsApplied in
NoneExecuteStrategy,BasicExecuteStrategy, andRichExecuteStrategy(defensively, sincegetOutput()can also include echoes on some platforms).2. Fix premature idle detection (
noneExecuteStrategy.ts)After
sendText(), wait for the terminal cursor to move past the start marker line before beginning idle detection. Without this, the idle poll could resolve immediately on the existing prompt before the shell started processing the command.3. Fix start marker recreation (
strategyHelpers.ts)setupRecreatingStartMarker()now returns anIDisposablethat stops the recreation loop. Callers dispose it before sending a command so that prompt re-renders (e.g. PSReadLine transient prompts) don't move the start marker past the command output.4. Sandbox reliability improvements
sandboxOutputAnalyzer.ts) — When exit code is unavailable (no shell integration), detect sandbox failures by pattern-matching output for "Operation not permitted", "Permission denied", "Read-only file system", etc.execPathresolution for remote environments (terminalSandboxService.ts) — UseremoteEnv.execPathdirectly instead of constructing a path to anodebinary5. Configurable idle poll interval
chat.tools.terminal.idlePollIntervalsetting (default: 1000ms, range: 50–10000ms)6. Cosmetic fix: suppress spurious "tool simplified the command" message
CommandLinePreventHistoryRewriterprepends a space to prevent shell history recording. This was triggering a "Note: The tool simplified the command to..." message. The comparison now normalizes display forms before diffing.7. Address PR feedback: logging, performance, timeout
stripNewLinesAndBuildMappingshellIntegrationTimeoutdescriptions (0 = skip the wait)8. Install bubblewrap and socat in Linux CI pipelines
These packages are required for terminal sandbox integration tests on Linux.
9. Force
/bin/bashover/bin/shfor copilot terminal profileWhen the default terminal profile resolves to
/bin/sh, shell integration cannot be injected (pty host warns: "Shell integration cannot be enabled for executable /bin/sh"). This causes loss of exit code detection and degrades sandbox failure analysis.Added a
/bin/sh→/bin/bashfallback ingetCopilotProfile(), matching the existingcmd.exe→powershell.exeoverride pattern. This fixes both CI environments (where the user's shell may be/bin/sh) and real users whose default profile resolves to/bin/sh.Tests
Unit tests
strategyHelpers.test.ts— 20+ test cases forstripCommandEchoAndPrompt()covering bash, zsh, PowerShell, wrapped commands, sandbox-wrapped commands, edge casessandboxOutputAnalyzer.test.ts— Tests foroutputLooksSandboxBlocked()heuristic detectionnoneExecuteStrategy.test.ts— Strategy-level tests verifying "Command produced no output" and no sandbox wrapper leaksIntegration tests
chat.runInTerminal.test.ts— Comprehensive tests running under both shell integration modes (SI on/off), covering echo output, no-output commands, multi-line output, non-zero exit codes, special characters, and sandbox scenarioscc @Tyriar @meganrogge