-
-
Notifications
You must be signed in to change notification settings - Fork 3
Fix PowerShell path resolution using GetFullPath instead of Join-Path #7
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
Conversation
…ilure Co-authored-by: GordonBeeming <5680199+GordonBeeming@users.noreply.github.com>
- Join-Path concatenates even when second path is absolute, causing path duplication on Windows - GetFullPath correctly handles both relative and absolute paths - Absolute paths are returned unchanged, relative paths are resolved to current directory - Fixes Windows test failure where paths like "C:\Users\RUNNERC:\Users\..." were created - Updated version to 2025-11-10 Co-authored-by: Gordon Beeming <me@gordonbeeming.com> Co-authored-by: GordonBeeming <5680199+GordonBeeming@users.noreply.github.com>
|
@copilot it looks like it's still broken https://github.com/GordonBeeming/copilot_here/actions/runs/19237870130/job/54993155692?pr=7 |
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.
Pull Request Overview
This PR improves path resolution safety in the PowerShell script by replacing Join-Path with [System.IO.Path]::GetFullPath() for more reliable absolute path resolution.
- Replaces
Join-Path (Get-Location).PathwithGetFullPath()in three functions - Updates version from 2025-11-05.1 to 2025-11-10
- Adds explanatory comments for the path resolution changes
… directory mismatch - GetFullPath() uses process current directory, not PowerShell location - On Windows CI, process current directory can differ from PowerShell's Get-Location - This caused paths to be resolved relative to wrong directory (e.g., C:\Users\RUNNER instead of D:\a\copilot_here\copilot_here) - Fix: Use two-parameter GetFullPath($path, $basePath) with explicit base from Get-Location - Applied to all three locations: Resolve-MountPath, Save-MountToConfig, Remove-MountFromConfig Co-authored-by: Gordon Beeming <me@gordonbeeming.com> Co-authored-by: GordonBeeming <5680199+GordonBeeming@users.noreply.github.com>
Fixed in commit 15b4c93. The issue was that Changed to use the two-parameter overload |
Root Cause Analysis
The issue was that
[System.IO.Path]::GetFullPath($path)uses the process's current directory ([System.Environment]::CurrentDirectory), not PowerShell's current location (Get-Location).In Windows GitHub Actions CI:
D:\a\copilot_here\copilot_here(checkout directory)C:\Users\RUNNER(user home, possibly set by CI environment)When
GetFullPathwas called with a relative path, it resolved relative toC:\Users\RUNNERinstead of the PowerShell location, causing malformed paths like:Solution
Use the two-parameter overload:
[System.IO.Path]::GetFullPath($path, $basePath)with explicit base path from(Get-Location).Path. This ensures relative paths are resolved relative to PowerShell's location, not the process current directory.Changed in three functions:
Resolve-MountPath(line 62)Save-MountToConfig(line 158)Remove-MountFromConfig(line 221)Co-authored-by: Gordon Beeming me@gordonbeeming.com
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.