Fix terminal progress for built-in SDK bash tool#320251
Open
roblourens wants to merge 4 commits into
Open
Conversation
When the Custom Terminal tool is disabled and the Agent Host SDK's built-in bash tool runs commands, no Terminal content block is emitted. The live Running/Completed path in toolCallStateToInvocation only set terminal toolSpecificData when getTerminalContentUri returned a URI, so built-in bash invocations never got the terminal pill and fell back to the generic tool widget showing just the first line of the command. Match the fallback already present in the history-replay path and in finalizeToolInvocation: also treat the tool as terminal when getToolKind(tc) === 'terminal'. AHP-only fields (terminalToolSessionId, terminalCommandUri) stay undefined for built-in bash. Also extend updateRunningToolSpecificData to refresh terminalCommandOutput as text content streams in, spreading the existing toolSpecificData so the async _reviveTerminalIfNeeded population of AHP fields is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t-look-at-the-terminal-tool-for-7a64b2fd
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes terminal progress rendering for the Agent Host SDK’s built-in bash tool when the Custom Terminal tool is disabled, ensuring terminal-style tool invocations still show the terminal pill (expandable command + output) even without an AHP Terminal content block.
Changes:
- Treat tool calls as terminal invocations when
getToolKind(tc) === 'terminal', even if no terminal content URI is present. - Refresh
terminalCommandOutputduring running updates as text content streams in, while preserving any asynchronously revived AHP terminal fields. - Add targeted unit tests covering built-in bash terminal
toolSpecificData, CRLF normalization, streaming refresh, and AHP-field preservation.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts | Ensures terminal tool invocations are created/updated based on toolKind === 'terminal' and keeps terminal output current during streaming. |
| src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts | Adds test coverage for built-in bash terminal invocation behavior, output normalization, streaming refresh, and field preservation. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Five sites in stateToProgressAdapter were inlining nearly identical
IChatTerminalToolInvocationData payloads (pending confirmation, live
create, streaming refresh, finalize, history replay), each with subtly
different field-preservation rules and four near-duplicate isTerminal
predicates. Extract two helpers:
- isTerminalToolCall(tc, single source of truth forexistingKind?)
the three terminal-detection signals (existing kind, _meta.toolKind,
Terminal content block) with rationale documented in JSDoc.
- buildTerminalToolSpecificData(tc, sessionResource, buildsexisting?)
the payload status-agnostically with consistent fallback semantics so
AHP fields populated asynchronously by _reviveTerminalIfNeeded survive
later refreshes. Completion-only fields (terminalCommandState) are
layered on by the caller.
updateRunningToolSpecificData now takes a sessionResource: URI parameter
to thread through to the helper; the production caller in
agentHostSessionHandler passes opts.backendSession and the test wrapper
passes URI.file('/').
No behavior change. All 59 stateToProgressAdapter tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The dedupe refactor was dropping terminalCommandId (and would silently
drop any other field on IChatTerminalToolInvocationData not explicitly
handled by the helper). Spread `existing` into the returned object so
prior-pass fields survive unless we have a fresh value to override them
this matches the original site-specific code that spreadwith
`existing` directly.
Also rename a misleading test ('picks up text output on completion')
that actually exercises the running-state codepath (per Copilot review
comment on PR #320251).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dmitrivMS
approved these changes
Jun 6, 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.
Fix #318458
When the Custom Terminal tool is disabled and the Agent Host SDK's built-in
bashtool runs commands, noTerminalcontent block is emitted. The live Running/Completed path intoolCallStateToInvocationonly set terminaltoolSpecificDatawhengetTerminalContentUrireturned a URI, so built-in bash invocations never got the terminal pill and fell back to the generic tool widget showing just the first line of the command.Fix
Match the fallback already present in the history-replay path (
completedToolCallToSerialized) and infinalizeToolInvocation: also treat the tool as a terminal tool whengetToolKind(tc) === 'terminal'. AHP-only fields (terminalToolSessionId,terminalCommandUri) stay undefined for built-in bash where they don't apply.Also extends
updateRunningToolSpecificDatato refreshterminalCommandOutputas text content streams in, spreading the existingtoolSpecificDataso the async_reviveTerminalIfNeededpopulation of AHP fields is preserved if a Terminal block does arrive.Tests
Adds 3 new tests in
stateToProgressAdapter.test.tscovering: built-in bash terminaltoolSpecificData(no Terminal content block), CRLF output normalization on completion, streaming output refresh, and AHP-field preservation on refresh. All 60 stateToProgressAdapter and 514 agentSessions tests pass.