Skip to content

Make sure handleCommandFinished is fired for PartialCommandDetectionCapability #251655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 17, 2025

Conversation

meganrogge
Copy link
Contributor

When the Basic shell integration strategy is chosen due to either a screen reader being present when using pwsh (which lacks PSReadline support) or some other clashing configuration, partialCommandDetectionCapability vs CommandDetectionCapability is used.

RunInTerminalTool relies on onDidEndTerminalShellExecution firing in order to consider the terminal command running step completed. If that does not occur, the tool will continue to hang and wait for a done event that never comes.

onDidEndTerminalShellExecution waits for onCommandFinished to be fired from its capability. In the case of the partialCommandDetectionCapability, that happens only onEnter IE when enter is sent to the terminal. That does not occur when the chat agent sends the command, so no onCommandFinished gets fired and thus chat hangs and never believes the command has completed even though it has.

The fix is to listen to terminalInstance.onDidExecuteText.

Repro I used to verify:
Enable NVDA on windows and screen reader mode. Set the default terminal profile to pwsh. Instruct chat agent to run ls in the terminal. Watch it hang and never complete. Same steps using this PR allows it to complete, see screenshot below. Also verified this PR doesn't cause issues when not in screen reader mode. I believe this will fix other peoples' reports too, captured in other issues.

image

fix #251436

@meganrogge meganrogge requested a review from Tyriar June 16, 2025 21:01
@meganrogge meganrogge self-assigned this Jun 16, 2025
@meganrogge meganrogge added this to the June 2025 milestone Jun 16, 2025
@meganrogge
Copy link
Contributor Author

to do/ defer: test

@meganrogge meganrogge enabled auto-merge (squash) June 16, 2025 22:08
@meganrogge meganrogge merged commit a0d633f into main Jun 17, 2025
8 checks passed
@meganrogge meganrogge deleted the merogge/cursor branch June 17, 2025 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chat agent hangs even for simple commands
2 participants