Skip to content

Strip redundant cd <workingDirectory> && prefix from agent host shell tool calls#312019

Merged
roblourens merged 5 commits intomainfrom
roblou/agents/cleanup-bash-commands-plan
Apr 22, 2026
Merged

Strip redundant cd <workingDirectory> && prefix from agent host shell tool calls#312019
roblourens merged 5 commits intomainfrom
roblou/agents/cleanup-bash-commands-plan

Conversation

@roblourens
Copy link
Copy Markdown
Member

The Copilot CLI sometimes emits shell commands prefixed with a redundant cd <workingDirectory> && … even though the agent already runs in that directory. The extension-host CLI strips this for display; this PR brings the same cleanup to the agent host so all AHP clients see the simplified command.

Three live paths

All needed patching, and all share a new helper stripRedundantCdPrefix in src/vs/platform/agentHost/common/commandLineHelpers.ts:

  1. mapSessionEvents.ts — history replay via getMessages().
  2. copilotAgentSession.ts onToolStart — live tool execution. Mutates parameters.command in place; because _activeToolCalls stores the same object, getPastTenseMessage in onToolComplete automatically sees the rewritten command.
  3. copilotToolDisplay.ts getPermissionDisplay — shell permission requests (both 'shell' and 'custom-tool' kinds).

Testing

  • 32 new unit tests across commandLineHelpers, copilotToolDisplay, mapSessionEvents, and copilotAgentSession. 109 tests passing total in the agentHost module.
  • Validate-by-revert confirmed: temporarily disabling stripRedundantCdPrefix in each call site made the corresponding positive-path tests fail with the expected "expected stripped, got prefixed" assertion errors. Negative-path tests stayed green.
  • Opt-in real-SDK integration test in toolApprovalRealSdk.integrationTest.ts ("strips redundant cd <workingDirectory> && prefix from shell tool calls"). Gated by AGENT_HOST_REAL_SDK=1, runs through the full WebSocket → CopilotAgentSessionsession/toolCallReady action path with a live model. Mock-agent integration tests can't reach this code because the mock IAgent bypasses CopilotAgentSession entirely.

Notes

  • The agent host now has its own copy of extractCdPrefix. Three copies now exist (workbench runInTerminalHelpers.ts, extension copilotCLITools.ts, agent host commandLineHelpers.ts) — worth a future consolidation pass, not in scope here.
  • Display-only presentationOverrides were considered as an alternative; chose to rewrite at the AHP boundary so every client sees the simplified command without coordinating override metadata.
  • The pwsh &&; rewriting and sandbox/background-detach behavior the workbench has are out of scope; this only addresses the redundant cd prefix.

(Written by Copilot)

…ll tool calls

The Copilot CLI sometimes emits shell commands prefixed with a redundant
`cd <` even though the agent already runs in thatworkingDirectory> &&
directory. The extension-host CLI strips this for display; this change
brings the same cleanup to the agent host so all clients see the
simplified command.

Three live paths needed patching:
- mapSessionEvents.ts (history replay via getMessages())
- copilotAgentSession.ts onToolStart (live tool execution)
- copilotToolDisplay.ts getPermissionDisplay (shell permission requests)

All three share a new helper, stripRedundantCdPrefix, in
src/vs/platform/agentHost/common/commandLineHelpers.ts.

Tests: 32 new unit tests across commandLineHelpers, copilotToolDisplay,
mapSessionEvents, and copilotAgentSession (109 passing total in the
agentHost module). Also added an opt-in real-SDK integration test in
toolApprovalRealSdk.integrationTest.ts that exercises the full
 toolCallReady path with a live model
(mock-agent integration tests can't reach this code because the mock
IAgent bypasses CopilotAgentSession entirely).

(Written by Copilot)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 22, 2026 22:02
@github-actions
Copy link
Copy Markdown
Contributor

Screenshot Changes

Base: 1f9cd94d Current: 2a785c80

Changed (2)

editor/inlineChatAffordance/InlineChatOverlay/Light
Before After
before after
editor/inlineCompletions/other/JumpToHint/Dark
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

This PR standardizes the “redundant cd <workingDirectory> && prefix” cleanup inside the agent host so that shell tool invocations shown to AHP clients match the already-stripped display in the extension-host CLI.

Changes:

  • Added extractCdPrefix / stripRedundantCdPrefix helper in commandLineHelpers.ts and applied it across history replay, live tool events, and permission display.
  • Threaded workingDirectory through CopilotAgentCopilotAgentSession and into mapSessionEvents / getPermissionDisplay so stripping can be conditional on the session CWD.
  • Added unit tests across the helper + affected call sites, plus a gated real-SDK integration test.
Show a summary per file
File Description
src/vs/platform/agentHost/common/commandLineHelpers.ts New helper for detecting/stripping redundant cd prefixes from shell commands.
src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts Strips redundant cd prefixes during history replay when workingDirectory is known.
src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts Strips redundant cd prefixes for shell/custom-tool permission request display.
src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts Strips redundant cd prefixes for live tool_start events and threads workingDirectory through.
src/vs/platform/agentHost/node/copilot/copilotAgent.ts Passes resolved workingDirectory into CopilotAgentSession creation.
src/vs/platform/agentHost/test/common/commandLineHelpers.test.ts New unit tests for extractCdPrefix and stripRedundantCdPrefix.
src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts New tests validating history-replay rewriting behavior.
src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts New tests validating permission-display rewriting behavior.
src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts New tests validating live tool-start rewrite + past-tense messaging.
src/vs/platform/agentHost/test/node/protocol/toolApprovalRealSdk.integrationTest.ts New gated real-SDK integration test asserting stripped command reaches session/toolCallReady.

Copilot's findings

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

Comment thread src/vs/platform/agentHost/common/commandLineHelpers.ts Outdated
Two Copilot review comments on PR #312019:

1. `commandLineHelpers.ts` `sameDirectory` did string-compare the raw
   extracted directory against `workingDirectory.fsPath`. On Windows,
   `URI.file('/repo/project').fsPath` is `\repo\project` while the
   model often emits `cd / separator mismatch maderepo/` project &&
   the prefix slip through. (This was the root cause of the Windows /
   Browser + Windows / Electron CI failures: 3 stripRedundantCdPrefix
   tests asserted on stripping behavior that only worked on POSIX.)

   Fix: route both sides through `URI.file(...)` (after trimming
   trailing separators) and compare via `extUriBiasedIgnorePathCase`,
   which handles separator normalization and case-insensitivity on
   Windows / macOS. Added two Windows-only test cases (forward-slash
   extracted vs native backslash wd, and pure-backslash same-direction)
   to cover the regression.

2. `toolApprovalRealSdk.integrationTest.ts` used a substring check
   miss quoted variants like `cd "<` or pwsh-stylewd>" &&
   `cd <`. Replaced with an anchored regex that tolerateswd>;
   optional surrounding quotes and either chain operator.

(Written by Copilot)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@roblourens roblourens marked this pull request as ready for review April 22, 2026 22:39
@roblourens roblourens enabled auto-merge (squash) April 22, 2026 22:39
@roblourens roblourens marked this pull request as draft April 22, 2026 22:42
auto-merge was automatically disabled April 22, 2026 22:42

Pull request was converted to draft

roblourens and others added 2 commits April 22, 2026 15:45
The merge of origin/main into the branch accidentally reverted the
title change from main ("Run in terminal" -> "Run in terminal?").
Restore both occurrences in getPermissionDisplay's shell + custom-tool
branches.

(Written by Copilot)

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

# Conflicts:
#	src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts
@roblourens roblourens marked this pull request as ready for review April 22, 2026 22:55
@roblourens roblourens enabled auto-merge (squash) April 22, 2026 22:55
@roblourens roblourens merged commit 98839ce into main Apr 22, 2026
26 checks passed
@roblourens roblourens deleted the roblou/agents/cleanup-bash-commands-plan branch April 22, 2026 23:20
@vs-code-engineering vs-code-engineering Bot added this to the 1.118.0 milestone Apr 22, 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.

3 participants