{{ message }}
Repository navigation
Complete running terminal command when its tab is closed - #488
Open
Lars Vogel (vogella) wants to merge 1 commit into
Open
Lars Vogel (vogella) wants to merge 1 commit into
Lars Vogel (vogella) wants to merge 1 commit into
Conversation
Lars Vogel (vogella)
requested review from
Ethan Hou (ethanyhou),
Sheng Chen (jdneo) and
xinyi-gong
as code owners
October 14, 2026 20:22
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Both implementations retain stale output that can prematurely complete the next command.
2 open findings
What changed in this PR
Completes pending terminal futures when users close active Copilot terminal tabs.
Changes:
- Returns a terminal-closed result for interrupted foreground commands.
- Clears stale terminal tab references in both implementations.
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Lars Vogel (vogella)
force-pushed
the
terminal-close-completes-command
branch
2 times, most recently
from
October 15, 2026 16:04
ec944b7 to
ec954b9
Compare
Comment on lines
+380
to
+381
Closing the Copilot terminal tab while a foreground command was running left the command's future incomplete, so the agent's run_in_terminal call never returned and the chat turn hung until cancelled. When the disposed widget is the current Copilot tab, the dispose listener now clears the tab, view control and output buffer and completes the pending command. The listener takes no lock because it runs on the UI thread while lock holders wait on syncExec, so the shared fields are volatile and revealTerminal() works on a snapshot of the tab item. Assisted-by: multiple AI agents and layers of automated tooling 🤖
Lars Vogel (vogella)
force-pushed
the
terminal-close-completes-command
branch
from
October 16, 2026 20:46
ec954b9 to
95bb2a5
Compare
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 user closed the Copilot terminal tab while an agent's foreground command was still running, the dispose listener only cleared the view control and never completed the command's future. Since that future has no timeout, the run_in_terminal tool call never returned and the chat turn hung until the user cancelled it. When the disposed widget is the current Copilot tab, the listener now clears the tab, view control and output buffer and completes the pending command with a "terminal was closed" message, in both the Eclipse terminal and the TM terminal implementation, so the agent gets a result and can continue. The listener deliberately takes no lock, since it runs on the UI thread while lock holders wait on syncExec, so the shared fields are volatile and revealTerminal() works on a snapshot of the tab item.