Skip redundant steering message when kill_terminal disposes terminal#314130
Merged
meganrogge merged 1 commit intomainfrom May 4, 2026
Merged
Skip redundant steering message when kill_terminal disposes terminal#314130meganrogge merged 1 commit intomainfrom
meganrogge merged 1 commit intomainfrom
Conversation
meganrogge
force-pushed
the
meganrogge/fix-kill-terminal-steering
branch
from
4bb928a to
a681c68
Compare
Collaborator
Author
|
/requires-eval-assessment terminalbench2 gpt-5.4,claude-opus-4.6,claude-opus-4.7 |
vs-code-engineering
Bot
added
the
~requires-eval-assessment
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent duplicate “terminal exited” steering notifications when a background terminal is terminated via the kill_terminal tool, relying on kill_terminal’s tool result as the single source of final output.
Changes:
- In
runInTerminalTool, adds a guard in the terminalonDisposedhandler to skip sending a steering message when the execution is no longer tracked. - In
killTerminalTool, removes the tracked execution before disposing the terminal instance so theonDisposedhandler observes it as already removed. - Updates inline comments to document the intended interaction between
kill_terminaland background completion notifications.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts | Adds an onDisposed early-return intended to suppress redundant steering notifications after kill_terminal. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/killTerminalTool.ts | Reorders cleanup to remove execution tracking before disposing the terminal, to influence onDisposed behavior. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
When the agent calls kill_terminal, the onDisposed handler in
_registerCompletionNotification fires and sends a redundant steering
message that disrupts the tool-result processing loop.
Fix by adding a dedicated _killedByTool set that kill_terminal marks
before disposing the terminal. The onDisposed handler checks this set
and skips the steering message when present. This avoids relying on
_activeExecutions membership which can be removed by other cleanup
paths (e.g. _removeExecutionAssociations from onDidDisposeInstance).
The steering message still fires for user-initiated terminal closes
and natural process exits.
Fixes microsoft/vscode-internalbacklog#7606
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
meganrogge
force-pushed
the
meganrogge/fix-kill-terminal-steering
branch
from
a681c68 to
7c327f3
Compare
meganrogge
added
~requires-eval-assessment
Contributor
|
⏳ Queued vscode build for
|
connor4312
approved these changes
May 4, 2026
Contributor
|
🚀 Queued eval-assessment publish build for
|
Contributor
|
🔬 Queued eval-assessment benchmark for
Results will be posted back here when the run completes. |
Contributor
|
✅ Eval-assessment build published.
|
vs-code-engineering
Bot
removed
the
~requires-eval-assessment
Contributor
|
📊 Eval-assessment benchmark complete.
Analysis ResultsResolution Rate
Token Usage
Step Counts
|
Contributor
|
📊 Eval-assessment benchmark complete.
Analysis ResultsResolution Rate
Token Usage
Step Counts
|
Contributor
|
📊 Eval-assessment benchmark complete.
Analysis ResultsResolution Rate
Token Usage
Step Counts
|
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.
When the agent calls
kill_terminalon a backgrounded terminal, theonDisposedhandler in_registerCompletionNotificationfires and sends a redundant "terminal exited" steering message. This disrupts the tool-result processing loop, causing the agent to stop responding — found in 5/89 terminalbench2 eval tests.Fixes microsoft/vscode-internalbacklog#7606