Fix terminal hang when process exits without shell integration sequences#312827
Merged
meganrogge merged 2 commits intomainfrom Apr 27, 2026
Merged
Fix terminal hang when process exits without shell integration sequences#312827meganrogge merged 2 commits intomainfrom
meganrogge merged 2 commits intomainfrom
Conversation
When a terminal process exits (e.g., with exit code 1) without emitting shell integration prompt sequences, trackIdleOnPrompt blocks forever because the Executing state cancels both schedulers and no more data arrives to reschedule them. Fix by adding onExit to the Promise.race in both rich and basic execute strategies. When the process exits, the strategy resolves immediately with the available output and exit code instead of hanging indefinitely. Also skip the post-race waitForIdle in basic strategy when process has already exited, since no more data will arrive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a hang in the terminal-based run_in_terminal execution flow when a process exits without emitting expected shell-integration “prompt” sequences, by allowing the execute strategies to complete based on the terminal process exit event.
Changes:
- Add
onExitto the completionPromise.racein both rich/basic execute strategies to avoid waiting forever on prompt-idle detection. - Skip post-race
waitForIdlein the basic strategy when the terminal process has already exited. - Add a regression test for the rich strategy covering “process exits without shell integration sequences”.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts | Completes execution on terminal process exit and falls back to the exit event for exit code. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts | Completes execution on terminal process exit, skips idle wait on exit, and falls back to the exit event for exit code. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/richExecuteStrategy.test.ts | Adds a test ensuring rich strategy completes when the terminal process exits without shell integration events. |
Copilot's findings
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/richExecuteStrategy.ts:161
- When falling back to
onExitfor the exit code,exitCodeOrErrormay be anITerminalLaunchErrorthat includes a numericcode. Currently this path only handles numbers, so you can lose a real exit code. Consider using the error’scode(when present) and including themessageinadditionalInformationfor easier diagnosis.
// Determine exit code from shell integration or from the process exit event
let exitCode = finishedCommand?.exitCode;
if (exitCode === undefined && onDoneResult && onDoneResult.type === 'processExit') {
exitCode = isNumber(onDoneResult.exitCodeOrError) ? onDoneResult.exitCodeOrError : undefined;
}
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/basicExecuteStrategy.ts:212
- When falling back to
onExitfor the exit code,exitCodeOrErrormay be anITerminalLaunchErrorwith a numericcode. The current implementation only accepts numbers and will drop the error’scode. Consider using the error’scode(when present) and surfacing themessageinadditionalInformation.
// Determine exit code from shell integration or from the process exit event
let exitCode = finishedCommand?.exitCode;
if (exitCode === undefined && onDoneResult && onDoneResult.type === 'processExit') {
exitCode = isNumber(onDoneResult.exitCodeOrError) ? onDoneResult.exitCodeOrError : undefined;
}
- Files reviewed: 3/3 changed files
- Comments generated: 3
… test - Add isTerminalLaunchError/formatExitCodeOrError/extractExitCode helpers to both rich and basic execute strategies to properly handle onExit emitting ITerminalLaunchError objects instead of just numbers - Add unit test for BasicExecuteStrategy process-exit-without-shell-integration - Add ITerminalLaunchError test case to RichExecuteStrategy tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
roblourens
approved these changes
Apr 27, 2026
This was referenced Apr 27, 2026
Closed
Collaborator
Author
|
cc @anthonykim1 |
This was referenced Apr 29, 2026
Closed
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.
Problem
When a terminal process exits without emitting shell integration prompt sequences,
trackIdleOnPromptblocks forever because theExecutingstate cancels both schedulers and no more data arrives to reschedule them. This causesrun_in_terminalto never complete, eventually timing out after 1 hour.Evidence from eval run
24890545491(VS Code, terminalbench, gpt-5.4)Analysis of the VS Code logs (
vsc-output/productLogs/terminal.log) found 21 tasks with terminal hangs — commands that started with therichexecute strategy but never completed (no correspondingFinishedlog line). Of these, 7 timed out entirely (X_AGENT_STILL_RESPONDING), and 14 others lost significant time before recovering or failing for other reasons.Tasks with terminal hangs that timed out:
Tasks with terminal hangs that partially recovered:
All show the same pattern:
Using 'rich' execute strategylogged with no subsequentFinishedline for the hung command.Also confirmed in eval run
24966302375(terminalbench2)A newer eval run showed all 19 out of 19 timeouts caused by the same terminal hang pattern. 15 of those 19 tasks passed in the CLI (run
22941092844) with the same model. See #312853 for details.Fix
Add
onExitto thePromise.racein bothrichExecuteStrategyandbasicExecuteStrategy. When the process exits, the strategy resolves immediately with the available output and exit code instead of hanging indefinitely.Also:
waitForIdlein basic strategy when the process already exitedITerminalLaunchErrorproperly in exit code extraction and loggingTest
Added tests for both strategies:
completes when terminal process exits without shell integration sequences— simulates process exit withoutonCommandFinished, verifying the strategy completes with the correct exit codehandles ITerminalLaunchError on process exit— verifiesITerminalLaunchErrorobjects are properly handledLimitations
This fix handles the process exit case. For long-running commands where the process stays alive but shell integration breaks,
onExitwon't fire until the process finishes. A complementary fix to downgrade fromrichtobasicstrategy mid-flight is tracked in #312853.