Skip to content
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

Terminal shell integration doesn't work on remote windows #141599

Closed
DonJayamanne opened this issue Jan 26, 2022 · 19 comments · Fixed by #142568
Closed

Terminal shell integration doesn't work on remote windows #141599

DonJayamanne opened this issue Jan 26, 2022 · 19 comments · Fixed by #142568
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders remote Remote system operations issues terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link
Contributor

Does this issue occur when all extensions are disabled?: Yes/No

  • Version: 1.64.0-insider (user setup)
  • Commit: 4506091
  • Date: 2022-01-26T05:17:50.284Z
  • OS: Windows_NT x64 10.0.22000
  • WSL2 (ubuntu, with zsh shell)

Follow up on #141339 (comment)

@meganrogge
Copy link
Contributor

Just to be sure, you changed the directory and also ran some commands, correct?

You also should've seen
Screen Shot 2022-01-26 at 4 09 59 PM
in the terminal. Did you?

@meganrogge meganrogge added info-needed Issue requires more information from poster terminal-shell-integration Shell integration, command decorations, etc. WSL Issue when using WSL labels Jan 26, 2022
@meganrogge meganrogge added this to the January 2022 milestone Jan 26, 2022
@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2022

Can repro at least one issue here, we're injecting a Windows path:

image

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug windows VS Code on Windows issues and removed info-needed Issue requires more information from poster confirmation-pending labels Jan 27, 2022
@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2022

This is a bit of a risky change, I think we probably need to move the shell integration arg injecting over to the server so execInstallFolder uses the right OS. Or refactor how the function is organized to support pulling that path over from the other side if needed.

@Tyriar Tyriar added remote Remote system operations issues and removed windows VS Code on Windows issues WSL Issue when using WSL labels Jan 27, 2022
@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2022

I think this breaks arg injection on all remotes, except for the test resolver which we tested against :trollface:

@Tyriar Tyriar changed the title Terminal shell integration doesn't work on zsh in WSL Terminal shell integration doesn't work on remote windows Jan 27, 2022
@meganrogge
Copy link
Contributor

Also worked on ssh

@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2022

@meganrogge oh shell integration does on ssh? Are you sshing into localhost as that should work?

@Tyriar Tyriar modified the milestones: January 2022, February 2022 Jan 27, 2022
@meganrogge
Copy link
Contributor

meganrogge commented Jan 27, 2022

Yeah it works when I ssh into localhost
Screen Shot 2022-01-27 at 1 30 28 PM

@abbasyadollahi
Copy link

I believe I'm getting a similar issue when enabling the new experimental shell integration setting.
It seems it's searching for a file local to my drive although I'm currently running the terminal in a remote SSH window.
Any new terminal I open is met with the following:

zsh:1: no such file or directory: /Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/contrib/terminal/browser/media/shellIntegration-zsh.sh

@connor4312
Copy link
Member

I still hit this. The host OS is a Windows system with Powershell 7, using remote SSH to connect to a Linux machine (that should use zsh). I just have terminal.integrated.defaultProfile.windows: Powershell in my settings.

@connor4312 connor4312 reopened this Feb 24, 2022
@connor4312 connor4312 added the verification-found Issue verification failed label Feb 24, 2022
@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2022

@connor4312 this should only happen for the first terminal in the window which is tracked inhttps://github.com//issues/141753, please verify this for terminals created after the connection is done.

@Tyriar Tyriar closed this as completed Feb 25, 2022
@Tyriar Tyriar removed the verification-found Issue verification failed label Feb 25, 2022
@connor4312
Copy link
Member

This seems to happen whenever I click the "+" button https://memes.peet.io/img/22-02-0f460379-9dfb-4c56-829f-78e33458ef05.mp4

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2022

Can repro, I wasn't seeing it because I had "terminal.integrated.defaultProfile.linux": "bash" set

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2022

I see this in the log which is wrong:

WARN Shell integration cannot be enabled when custom args undefined are provided for pwsh.exe.

Cause is related to this debt item: #143965

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2022

That issue is caused because we currently only resolve backend OS/remote authority after the process is created.

@Tyriar
Copy link
Member

Tyriar commented Feb 25, 2022

I pushed a fix but could only test Windows -> Windows, make sure to verify on Windows -> non-Windows (eg. WSL) or the other way around.

@joyceerhl joyceerhl added the verified Verification succeeded label Feb 28, 2022
@joyceerhl
Copy link
Contributor

Shell integration works for me over Remote-SSH with the following setup:

  • Host OS: Windows, Powershell 7
  • Remote OS: Linux

image

@connor4312
Copy link
Member

Verified it works here too 🎉

@joyceerhl
Copy link
Contributor

I just noticed the following in the logs when using Remote-SSH with shell integration enabled

 WARN Shell integration cannot be enabled when custom args -c,wget --version > /dev/null
if [ $? -eq 0 ]
then
	wget --connect-timeout=7 --tries=1 --dns-timeout=7 -q  -O - http://<redacted>/latest/meta-data/instance-id
else
	curl --version > /dev/null
	if [ $? -eq 0 ]
	then
		curl --connect-timeout 7 -s  http://<redacted>/latest/meta-data/instance-id
	fi
fi
exit 0 are provided for sh.

Is this known/expected @Tyriar?

@Tyriar
Copy link
Member

Tyriar commented Feb 28, 2022

@joyceerhl yes that's expected as a warning just to call out if someone is looking that shell integration couldn't be enabled. I just improved that warnings format in a pr I have out as well.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders remote Remote system operations issues terminal-shell-integration Shell integration, command decorations, etc. verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants
@DonJayamanne @Tyriar @connor4312 @abbasyadollahi @meganrogge @joyceerhl and others