gate nonpty terminal actions by judyks · Pull Request #327084 · microsoft/vscode

judyks · 2026-07-23T04:09:29Z

Summary
Fixes the inline terminal preview offering "Focus/Show Terminal" and a "view stale terminal" action for non-pty (SDK built-in / streaming) shell runs, where there is no real interactive terminal to focus.

The bug
The chat terminal preview (ChatTerminalToolProgressPart) decides whether to show the Focus/Show-Terminal action purely from whether a terminal instance exists (_toolbarHasInstance = !!terminalInstance), and its "open the terminal" fallback calls terminalService.openResource(terminalCommandUri). Neither has any notion of whether that terminal is a real interactive pty or an output-only, non-pty channel.

That distinction started to matter with the shell-output-streaming work (#327067): to stream output, a non-pty SDK shell run now emits a Terminal content block (agenthost-terminal://shell/copilotNonPtyShells/{toolCallId}, isPty: false). The adapter sets terminalToolSessionId for any Terminal block with no isPty guard, so a non-pty run now gets a terminalToolSessionId → a revived ITerminalInstance → _toolbarHasInstance === true → the Focus/Show action and the stale openResource fallback render. But these terminals are output-only: there is nothing interactive to focus, and "opening" them just surfaces an empty/stale terminal.

The protocol already carries the needed signal — isPty on ToolResultTerminalContent / TerminalState — but it was only consumed on the host side (agentHostPty uses it to set convertEol). It never reached IChatTerminalToolInvocationData, and the widget never gated any UI on it.

Approach
Propagate the existing isPty signal end-to-end to the renderer, and gate the instance-dependent affordances on it — rather than inventing a new heuristic in the UI. Output-only non-pty terminals keep their preview (command, decoration, streamed output) but drop the actions that only make sense for a real interactive pty.

Changes and why
stateToProgressAdapter.ts (buildTerminalToolSpecificData) and agentHostSessionHandler.ts (_reviveTerminalIfNeeded) — read isPty off the Terminal content block and carry it onto the invocation data, in both producers, via a shared getTerminalContentIsPty helper.
Why: the terminal payload is built in two places — the main builder (create / streaming refresh / finalize / history replay) and the async terminal-revive path — so both must set the flag for it to stay consistent across every lifecycle stage. ?? existing preserves it across streaming refreshes.

chatService.ts (IChatTerminalToolInvocationData) — add the isPty field.
Why: the renderer had no channel to learn a terminal was non-pty; this is the missing carrier.

chatTerminalToolProgressPart.ts — add a single _isNonPtyTerminal (isPty === false) check and use it to hide the Focus/Show Terminal action and early-return focusTerminal() for non-pty terminals. The early-return covers every caller — the toolbar button, the "Focus Most Recent Terminal" command, and the collapsible "Show" link — including the stale openResource(terminalCommandUri) fallback.
Why: for output-only non-pty terminals there is no interactive session to focus; showing the action or opening a stale/empty terminal is misleading. The command decoration and streamed output are unaffected — only the interactive affordances are withheld.

stateToProgressAdapter.test.ts — assert isPty is threaded from the block (false / true / unset) and that it survives a streaming-output refresh (the revive → refresh race).

Notes / not in scope
Command decoration and output streaming for these runs are handled by #327067 (exit code now lives on the terminal block result; output streams via the non-pty channel). This PR only removes the incorrect interactive affordances.
The isPty semantics match the protocol flag added in #327067: false = output-only non-pty channel; true/absent = pty-backed (custom-terminal or local) — those keep Focus/Show.
Known gap / follow-up
The isPty data-threading is unit-tested, but the widget gating behavior itself — Focus/Show hidden and focusTerminal() a no-op for non-pty cards — has no direct test. ChatTerminalToolProgressPart is never constructed in the test suite, and doing so needs substantial service/DOM mock setup, so it was left as a follow-up rather than done here. The gating is currently covered by the adapter tests plus reasoning; a widget-level test should be added to lock it in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5083fb1f-34d7-44bf-82a1-c4582e57683b

…tputStreaming

…treaming

…t over the terminal channel - Re-vendor agent-host-protocol: terminalComplete removed, exitCode/preview/truncated fold into terminal.result (TerminalCommandResult), isPty added to ToolResultTerminalContent and TerminalState - shell_exit now lands on the tool call's terminal block; history replay synthesizes a non-pty block keyed by toolCallId - Stream tool.execution_partial_result into an output-only agenthost-terminal channel (isPty false) instead of replacing Text content - Normalize LF to CRLF in AgentHostPty when the channel is plain text - Re-apply local CompletionItem.label extension clobbered by the protocol sync (still needs upstreaming)

The runtime emits cumulative plain-text snapshots (ANSI-stripped, throttled) that are capped to the leading ~10KB with a growing truncation marker, so a length-based delta goes stale once output exceeds the cap. Track the last emitted snapshot and prefix-check it: extend in place while the snapshot grows, reset the channel and rewrite when it was rewritten.

Pty-backed channels already carry CRLF because the pty converts output line endings; do the same for output-only channels at the point data enters the channel, so terminal frontends render both identically and AgentHostPty needs no isPty-specific handling. Reverts the client-side normalization.

vs-code-engineering · 2026-07-23T04:10:31Z

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@anthonykim1

Matched files:

src/vs/workbench/contrib/terminal/browser/agentHostPty.ts
src/vs/workbench/contrib/terminal/browser/agentHostTerminalService.ts

Copilot

Pull request overview

Adds non-PTY shell-output channels and prevents non-interactive terminal actions while updating the AHP terminal-result model.

Changes:

Streams SDK shell output through output-only terminal channels.
Propagates isPty to terminal UI and gates focus actions.
Replaces terminalComplete with terminal-embedded results and updates tests.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`agentHostTerminalService.ts`	Configures plain-text xterm output.
`agentHostPty.ts`	Tracks non-PTY terminal state.
`stateToProgressAdapter.test.ts`	Updates terminal-result tests.
`chatService.ts`	Adds terminal `isPty` metadata.
`chatTerminalToolProgressPart.ts`	Gates non-PTY focus actions.
`stateToProgressAdapter.ts`	Adapts embedded terminal results.
`agentHostSessionHandler.ts`	Propagates terminal type during revival.
`testAgentHostTerminalManager.ts`	Records output-terminal operations.
`mapSessionEvents.test.ts`	Tests result conversion.
`copilotShellTools.test.ts`	Updates terminal manager mock.
`copilotAgentSession.test.ts`	Tests non-PTY output streaming.
`copilotAgent.test.ts`	Updates terminal manager mock.
`agentHostTerminalManager.test.ts`	Tests output-only channels.
`mapSessionEvents.ts`	Converts SDK shell exits.
`copilotNonPtyShellTerminals.ts`	Implements cumulative output streams.
`copilotAgentSession.ts`	Connects partial results to streams.
`agentHostTerminalManager.ts`	Manages output-only terminals.
`sessionState.ts`	Updates protocol type exports.
`channels-terminal/state.ts`	Adds terminal `isPty`.
`channels-session/commands.ts`	Updates generated protocol documentation.
`channels-chat/state.ts`	Embeds command results in terminal content.
`.ahp-version`	Updates the synchronized protocol revision.

Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/terminal/browser/agentHostPty.ts:435

If an output terminal exits while the client is disconnected, the reconnect snapshot has state.exitCode but the missed TerminalExited action is never reconstructed here. The reconnected terminal is consequently left in a running state. Replay a defined snapshot exit code after restoring the content and action listener.

			this._isPlainTextOutput = state.isPty === false;

+	const terminalResult = tc.status === ToolCallStatus.Completed || tc.status === ToolCallStatus.Running
+		? tc.content?.find(isToolResultTerminalContent)?.result
 		: undefined;
-	if (terminalComplete?.exitCode !== undefined) {
-		return { exitCode: terminalComplete.exitCode };
+	if (terminalResult?.exitCode !== undefined) {
+		return { exitCode: terminalResult.exitCode };


 				language: 'shellscript',
 				terminalToolSessionId: sessionId,
 				terminalCommandUri,
+				isPty: getTerminalContentIsPty(tc.content) ?? existing?.isPty,


+export function buildNonPtyShellTerminalUri(toolCallId: string): string {
+	return `agenthost-terminal://shell/copilotNonPtyShells/${toolCallId}`;
+}


+	finalize(toolCallId: string, exitCode: number | undefined): void {
+		const stream = this._streams.get(toolCallId);
+		if (!stream || stream.finalized) {
+			return;
+		}
+		stream.finalized = true;
+		stream.lastEmitted = '';
+		this._terminalManager.finalizeOutputTerminal(stream.uri, exitCode);


 				this._supportsCommandDetection = true;
 				this._onSupportsCommandDetection.fire();
 			}
+			this._isPlainTextOutput = state.isPty === false;


…shells Non-pty (output-only) agent-host shell channels get a terminal instance only to render streamed output, so the inline terminal preview showed Focus/Show Terminal and the stale openResource fallback for them even though there is no interactive terminal to focus. Thread the terminal content block's isPty flag into IChatTerminalToolInvocationData (adapter + revive path) and gate the Focus/Show action and the stale command-resource fallback on it: isPty === false hides them; true/undefined keep them.

judyks · 2026-07-23T04:42:31Z

Summary

Fixes the inline terminal preview offering "Focus/Show Terminal" and a "view stale terminal" action for non-pty (SDK built-in / streaming) shell runs, where there is no real interactive terminal to focus.

The bug

The chat terminal preview (ChatTerminalToolProgressPart) decides whether to show the Focus/Show-Terminal action purely from whether a terminal instance exists (_toolbarHasInstance = !!terminalInstance), and its "open the terminal" fallback calls terminalService.openResource(terminalCommandUri). Neither has any notion of whether that terminal is a real interactive pty or an output-only, non-pty channel.

That distinction started to matter with the shell-output-streaming work (#327067): to stream output, a non-pty SDK shell run now emits a Terminal content block (agenthost-terminal://shell/copilotNonPtyShells/{toolCallId}, isPty: false). The adapter sets terminalToolSessionId for any Terminal block with no isPty guard, so a non-pty run now gets a terminalToolSessionId → a revived ITerminalInstance → _toolbarHasInstance === true → the Focus/Show action and the stale openResource fallback render. But these terminals are output-only: there is nothing interactive to focus, and "opening" them just surfaces an empty/stale terminal.

The protocol already carries the needed signal — isPty on ToolResultTerminalContent / TerminalState — but it was only consumed on the host side (agentHostPty uses it to set convertEol). It never reached IChatTerminalToolInvocationData, and the widget never gated any UI on it.

Approach

Propagate the existing isPty signal end-to-end to the renderer, and gate the instance-dependent affordances on it — rather than inventing a new heuristic in the UI. Output-only non-pty terminals keep their preview (command, decoration, streamed output) but drop the actions that only make sense for a real interactive pty.

Changes and why

stateToProgressAdapter.ts (buildTerminalToolSpecificData) and agentHostSessionHandler.ts (_reviveTerminalIfNeeded) — read isPty off the Terminal content block and carry it onto the invocation data, in both producers, via a shared getTerminalContentIsPty helper.
Why: the terminal payload is built in two places — the main builder (create / streaming refresh / finalize / history replay) and the async terminal-revive path — so both must set the flag for it to stay consistent across every lifecycle stage. ?? existing preserves it across streaming refreshes.
chatService.ts (IChatTerminalToolInvocationData) — add the isPty field.
Why: the renderer had no channel to learn a terminal was non-pty; this is the missing carrier.
chatTerminalToolProgressPart.ts — add a single _isNonPtyTerminal (isPty === false) check and use it to hide the Focus/Show Terminal action and early-return focusTerminal() for non-pty terminals. The early-return covers every caller — the toolbar button, the "Focus Most Recent Terminal" command, and the collapsible "Show" link — including the stale openResource(terminalCommandUri) fallback.
Why: for output-only non-pty terminals there is no interactive session to focus; showing the action or opening a stale/empty terminal is misleading. The command decoration and streamed output are unaffected — only the interactive affordances are withheld.
stateToProgressAdapter.test.ts — assert isPty is threaded from the block (false / true / unset) and that it survives a streaming-output refresh (the revive → refresh race).

Notes / not in scope

Command decoration and output streaming for these runs are handled by Stream built-in shell output over AHP terminal channel #327067 (exit code now lives on the terminal block result; output streams via the non-pty channel). This PR only removes the incorrect interactive affordances.
The isPty semantics match the protocol flag added in Stream built-in shell output over AHP terminal channel #327067: false = output-only non-pty channel; true/absent = pty-backed (custom-terminal or local) — those keep Focus/Show.

Known gap / follow-up

The isPty data-threading is unit-tested, but the widget gating behavior itself — Focus/Show hidden and focusTerminal() a no-op for non-pty cards — has no direct test. ChatTerminalToolProgressPart is never constructed in the test suite, and doing so needs substantial service/DOM mock setup, so it was left as a follow-up rather than done here. The gating is currently covered by the adapter tests plus reasoning; a widget-level test should be added to lock it in.

anthonykim1 and others added 11 commits July 16, 2026 23:04

Stream output for SDK built-in shell tool

fb5883e

better todo comment

27ce7bf

Bump Copilot SDK to 1.0.8 preview

c9aed0a

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5083fb1f-34d7-44bf-82a1-c4582e57683b

Exclude unused Koffi native modules

b3225ed

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5083fb1f-34d7-44bf-82a1-c4582e57683b

Merge remote-tracking branch 'upstream/main' into anthonykim1/shellOu…

86a2c4b

…tputStreaming

Merge branch 'anthonykim1/copilotJul22' into anthonykim1/shellOutputS…

9aaa7bf

…treaming

no need to complicate too much with lf crlf

8a821b4

Merge branch 'main' into anthonykim1/shellOutputStreaming

c30c63c

Copilot AI review requested due to automatic review settings July 23, 2026 04:09

Copilot started reviewing on behalf of judyks July 23, 2026 04:10 View session

judyks force-pushed the judykang/gate-nonpty-terminal-actions branch from 8127d63 to 494b56c Compare July 23, 2026 04:12

Copilot AI reviewed Jul 23, 2026

View reviewed changes

judyks force-pushed the judykang/gate-nonpty-terminal-actions branch from 494b56c to a868e4e Compare July 23, 2026 04:37

judyks marked this pull request as draft July 23, 2026 05:11

vs-code-engineering Bot assigned meganrogge Jul 23, 2026

judyks changed the title ~~Judykang/gate nonpty terminal actions~~ gate nonpty terminal actions Jul 23, 2026

judyks closed this Jul 23, 2026

judyks deleted the judykang/gate-nonpty-terminal-actions branch July 23, 2026 05:20

anthonykim1 unassigned meganrogge Jul 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

gate nonpty terminal actions#327084

gate nonpty terminal actions#327084
judyks wants to merge 12 commits into
microsoft:mainfrom
judyks:judykang/gate-nonpty-terminal-actions

judyks commented Jul 23, 2026 •

edited

Loading

Uh oh!

vs-code-engineering Bot commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

judyks commented Jul 23, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

judyks commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vs-code-engineering Bot commented Jul 23, 2026

📬 CODENOTIFY

@anthonykim1

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

judyks commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The bug

Approach

Changes and why

Notes / not in scope

Known gap / follow-up

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

judyks commented Jul 23, 2026 •

edited

Loading

judyks commented Jul 23, 2026 •

edited

Loading