Skip to content

Prevent enable si showing up when using pwsh from chat with accessibility mode#309606

Merged
anthonykim1 merged 8 commits intomainfrom
anthonykim1/enableShellIntegrationChatAccessible
Apr 15, 2026
Merged

Prevent enable si showing up when using pwsh from chat with accessibility mode#309606
anthonykim1 merged 8 commits intomainfrom
anthonykim1/enableShellIntegrationChatAccessible

Conversation

@anthonykim1
Copy link
Copy Markdown
Contributor

@anthonykim1 anthonykim1 commented Apr 13, 2026

Resolves: #307698

Before:
image

After:

afterEnableFix.mp4

Two parts to this:

  • timeout bump for A11y: We add extra 3 second when waiting for shell integration with accessible mode toggled, since we remove and load our bundled PSReadLine in our shell integration script. This would allow us for the first command to chat gets proper command detection.
    • Without the timeout bump we get
image
  • Deferred banner: Instead of deciding whether to show "enable shell integration banner" right before running command, we should wait until command finishes,
    • Without the deferred banner we get the enable shell integration message again

@anthonykim1 anthonykim1 added this to the 1.117.0 milestone Apr 13, 2026
@anthonykim1 anthonykim1 self-assigned this Apr 13, 2026
Copilot AI review requested due to automatic review settings April 13, 2026 20:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts RunInTerminalTool’s “Enable shell integration” guidance so it doesn’t appear when shell integration is already enabled via settings, addressing the misleading warning seen in accessibility/screen reader scenarios.

Changes:

  • Import TerminalSettingId to read the shell integration enablement setting.
  • Gate the “Enable shell integration…” info message on both shellIntegrationQuality === None and terminal.integrated.shellIntegration.enabled !== true.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts Suppresses the “Enable shell integration…” prompt when shell integration is already enabled in configuration.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 13, 2026

Screenshot Changes

Base: 7f3ec374 Current: 8384ad7f

Changed (1)

editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

blocks-ci screenshots changed

Replace the contents of test/componentFixtures/blocks-ci-screenshots.md with:

Updated blocks-ci-screenshots.md
<!-- auto-generated by CI — do not edit manually -->

#### editor/codeEditor/CodeEditor/Dark
![screenshot](https://hediet-screenshots.azurewebsites.net/images/cb32a3e854b5734fe5aaca2318f2e0a42ee821b05ea97883ea42c5ba95edb3c3)

#### editor/codeEditor/CodeEditor/Light
![screenshot](https://hediet-screenshots.azurewebsites.net/images/42624fbba5e0db7f32c224b5eb9c5dd3b08245697ae2e7d2a88be0d7c287129b)

@anthonykim1
Copy link
Copy Markdown
Contributor Author

image @meganrogge was right, powershell 7 did not show the `enable shell integration warning` whereas windows powershell did.

@anthonykim1 anthonykim1 marked this pull request as ready for review April 15, 2026 22:24
commandDetection!,
executionOptions.persistentSession
);
if (toolTerminal.shellIntegrationQuality === ShellIntegrationQuality.None) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shell integration banner decision is deferred to after command execution completes (see below). This gives SI time to arrive — on Windows PowerShell 5.1, cold-loading PSReadLine can push HasRichCommandDetection past the initial wait timeout, and checking here would incorrectly show the banner.

@anthonykim1 anthonykim1 merged commit 7133939 into main Apr 15, 2026
25 of 26 checks passed
@anthonykim1 anthonykim1 deleted the anthonykim1/enableShellIntegrationChatAccessible branch April 15, 2026 23: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.

Enable shell integration on Chat view even when shell integration is already enabled

3 participants