Skip to content

Make sure command finished event is fired when chat agent runs a command #251507

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

Closed
wants to merge 2 commits into from

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jun 15, 2025

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 also listen to onWriteParsed and check for if the cursorX === 0, IE the cursor has reset after a write has happened, thus the command has finished.

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 15, 2025 00:16
@meganrogge meganrogge self-assigned this Jun 15, 2025
@meganrogge meganrogge added this to the June 2025 milestone Jun 15, 2025
@meganrogge meganrogge enabled auto-merge (squash) June 15, 2025 00:33
Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com>
@Tyriar Tyriar disabled auto-merge June 16, 2025 14:09
@Tyriar
Copy link
Member

Tyriar commented Jun 16, 2025

Disabled auto merge so I could approve before testing.

@meganrogge
Copy link
Contributor Author

Closing in favor of #251655

@meganrogge meganrogge closed this Jun 17, 2025
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