-
Notifications
You must be signed in to change notification settings - Fork 8.8k
fix(conpty): detect WSL switch when entered via command (fix #19420) #19425
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
base: main
Are you sure you want to change the base?
Conversation
…t#19420) Add WSL detection logic in ConptyConnection to notify Terminal Chat
@microsoft-github-policy-service agree |
Can you show a screen recording of this in action? (screen to gif is great, but I hear snipping Tool does screen recordings these days too) |
|
||
_LaunchAttachedClient(); | ||
|
||
if (!_commandline.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least, this indenting needs to match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there, thanks for trying to fix this! The _commandline
inside ConptyConnection only applies to the first process which is started. If you run wsl
under pwsh
, for example, it will still never be detected.
Considering that issue #19420 reports that it is "working correctly when [I] launch WSL from the dropdown" and not "when entered by the wsl
command", unfortunately I do not think this fixes the issue.
Thank you @zadjii-msft and @DHowett for the feedback! 🙏 That makes sense — my current approach only checks _commandline during startup, so it misses the scenario where WSL is launched from an existing shell (PowerShell/CMD). I’ll update the fix to detect WSL dynamically (e.g., via environment variables like WSL_DISTRO_NAME) and trigger _environmentChangedEventHandlers when the environment changes mid-session. Also fixing the indentation issue in the same commit. I’ll push an updated patch soon. Thanks again for the clear direction! |
Do you think that'll work? Why would Terminal's environment variables change just because the shell launched a sub-shell? |
Yeah, that makes sense — the Terminal’s environment wouldn’t actually change just because the shell launched a WSL subshell. My thought with WSL_DISTRO_NAME was to detect the active context, but you’re right — that wouldn’t reflect outside the WSL process. I’ll explore checking the attached process path or command (like wsl.exe or /init) instead — that might be a cleaner detection approach. ✨ |
Summary
Fixes #19420 — Terminal Chat did not detect WSL when launched by running
wsl
inside an existing Windows shell.Root cause
When a user launches
wsl
from an existing PowerShell/CMD session, the new WSL process runs as a child of the shell. Terminal previously only relied on the launched process at tab-creation to determine the environment and thus missed the manualwsl
case.Fix
ConptyConnection::Start()
. After launching the attached client, we check the commandline and set a_isWSLConnection
flag and raise an_environmentChangedEventHandlers
event whenwsl
is detected._isWSLConnection
flag and_environmentChangedEventHandlers
inConptyConnection
so other components (e.g., Terminal Chat) can subscribe and update context.Files changed
src/cascadia/TerminalConnection/ConptyConnection.h
(added _isWSLConnection flag and event)src/cascadia/TerminalConnection/ConptyConnection.cpp
(added detection logic after launching client)Testing
Manual:
wsl
, open Terminal Chat — chat should now detect WSL (or logs showConPtyDetectedWSLCommand
event).CI: Unit tests touched are logic-only; no new flaky tests added.
Notes