Fix Copilot CLI shim crash when copilot is not on PATH - #328911
Open
Hamjaster wants to merge 1 commit into
Open
Conversation
Find-RealCopilot compared the shim's directory against the resolved copilot binary's directory using Split-Path on both. When `copilot` is not installed, Get-Command returns nothing and $CopilotPath is $null. PowerShell's -or does short-circuit, but the first clause is false in that case, so the second one runs and Split-Path throws "Cannot bind argument to parameter 'Path' because it is null." That is the one situation the function most needs to handle: no real CLI on PATH, so the caller should get $null back and show the install prompt. Instead the shim died before reaching it. Take each directory only when its path is non-null, matching the guard style already used for the two Resolve-Path lines just above, and reuse the computed directory for $ScriptDir so the second unguarded Split-Path goes away too. Verified with PowerShell 7.6.4 that the condition is unchanged for every other combination (shim shadowing the real binary, identical paths, different directories, symlinked to the same file, and no script path at all); only the crashing case changes, and it now falls through to the existing branch that returns $null.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes microsoft/vscode-copilot-chat#5110
(That repo is archived and points here, so I'm filing the fix in this repo. The file moved to
extensions/copilot/and the bug came with it.)The problem in plain terms
On Windows, VS Code writes a small helper script that launches the Copilot CLI. Part of its job is to notice when the Copilot CLI isn't installed, so it can offer to install it for you.
That's exactly the case where it crashes.
The script looks up where
copilotlives on your system. If Copilot isn't installed, that lookup comes back empty. The script then tries to take "the folder containing" that empty result, and PowerShell refuses:So instead of "Copilot CLI isn't installed, want me to install it?", you get an error from the helper script itself. The install prompt it was supposed to show is right there in the code, a few lines further down, and never gets reached.
What this changes
Only take the folder of a path when there actually is a path. That's already how the two lines directly above it are written, so this makes the third line consistent with its neighbours.
With that, an uninstalled Copilot CLI reports "not found" the way the rest of the script expects, and you get the install prompt instead of a crash.
Why it wasn't caught
PowerShell's
-ordoes stop early, so it's easy to assume the later checks are safe. But the check that stops early is$CurrentScript -eq $CopilotPath, and when one side is a real path and the other is empty, that's false; the crashing check runs next. It only lines up when Copilot is genuinely missing, which is the path most people never hit because they have Copilot installed.How I checked it
I don't have Windows here, so I installed PowerShell 7.6.4 locally and ran the real
Find-RealCopilotfunction, pulled straight out of the file, from both the currentmainand this branch:$nullis what the caller wants; it's the value that triggers the "Install GitHub Copilot CLI? (y/N)" prompt.I also compared the old and new versions of the condition across every combination I could think of, to make sure I wasn't quietly changing behaviour somewhere else:
The only row that moves is the one that used to crash, and
Falsesends it into the existing branch that already handles a missing binary and returns$null.Notes for reviewers
Split-Path $CurrentScript -Parent(used for$ScriptDir) to reuse the value computed above. Same result, and it removes the other unguarded call in the same function rather than leaving a second one behind..ps1shipped as a resource rather than something the extension imports, so I verified by running the function directly rather than adding a test. Happy to add one if there's a harness for shell/PowerShell resources that I missed.copilot IS on PATHcase would resolvecopilot.cmd/copilot.exeviaPATHEXT; my local run used a plain executable, since the part being fixed is the null handling rather than the lookup itself.An AI coding agent helped me write this. I reviewed the change and ran the PowerShell verification above myself.