Skip to content

Comments

refactor: Update BuildCmdWrapper to use args splatting and add unit tests#67

Merged
GordonBeeming merged 1 commit intomainfrom
gb/issue64
Feb 19, 2026
Merged

refactor: Update BuildCmdWrapper to use args splatting and add unit tests#67
GordonBeeming merged 1 commit intomainfrom
gb/issue64

Conversation

@GordonBeeming
Copy link
Owner

This pull request improves the way command-line arguments are forwarded from Windows batch wrapper scripts to PowerShell functions, ensuring that quoted arguments are handled correctly. It also adds unit tests to verify the new behavior.

Improvements to command-line argument forwarding:

  • Modified the BuildCmdWrapper method in ShellIntegration to use PowerShell's @args splatting and the -- separator when forwarding arguments, which preserves quoted arguments as single values.

Testing enhancements:

  • Added new unit tests in ShellIntegrationTests.cs to verify that the wrapper script uses @args and the -- separator for both pwsh and Windows PowerShell.

Other changes:

  • Removed the .worktrees/issue-65 subproject commit reference.

Copilot AI review requested due to automatic review settings February 19, 2026 10:28
Copy link
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

Refactors the Windows .cmd wrapper generation to forward arguments into PowerShell in a way that preserves quoting, and adds unit tests around the wrapper content to prevent regressions.

Changes:

  • Update ShellIntegration.BuildCmdWrapper to forward args using PowerShell @args splatting (and a -- separator) instead of embedding %* inside the -Command string.
  • Add unit tests validating the generated wrapper contains the new forwarding approach.
  • Remove the .worktrees/issue-65 subproject commit reference file.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
app/Infrastructure/ShellIntegration.cs Changes Windows CMD wrapper content to forward args via @args and --, and exposes the wrapper builder for unit tests.
tests/CopilotHere.UnitTests/ShellIntegrationTests.cs Adds tests asserting the wrapper includes @args forwarding (and partially asserts -- %*).
.worktrees/issue-65 Removes an extraneous worktree/subproject commit reference.

Comment on lines +334 to +346
internal static string BuildCmdWrapper(string functionName, string psPath)
{
// Prefer pwsh, fall back to powershell.
// Keep wrapper simple and rely on PowerShell script for all logic.
// Pass arguments after `--` and forward via PowerShell `@args` so quoted
// values (for example --prompt "What is 1 + 1 ?") stay as a single arg.
return "@echo off\r\n" +
"setlocal\r\n" +
"set \"SCRIPT=%USERPROFILE%\\.copilot_here.ps1\"\r\n" +
"where pwsh >nul 2>nul\r\n" +
"if %ERRORLEVEL%==0 (\r\n" +
$" pwsh -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" +
$" pwsh -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" +
") else (\r\n" +
$" powershell -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" +
$" powershell -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" +
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

BuildCmdWrapper takes psPath but never uses it (the wrapper still hardcodes %USERPROFILE%\.copilot_here.ps1). Either remove the unused parameter (and adjust call sites/tests) or use psPath to build the script path in the generated CMD so the wrapper reflects the actual installed location.

Copilot uses AI. Check for mistakes.
Comment on lines +344 to +346
$" pwsh -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" +
") else (\r\n" +
$" powershell -NoProfile -ExecutionPolicy Bypass -Command \". '%USERPROFILE%\\.copilot_here.ps1'; {functionName} %*\"\r\n" +
$" powershell -NoProfile -ExecutionPolicy Bypass -Command \"& {{ . '%USERPROFILE%\\.copilot_here.ps1'; {functionName} @args }}\" -- %*\r\n" +
Copy link

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

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

This change alters user-facing behavior (Windows CMD wrapper argument forwarding) but the repo’s versioning rule requires bumping the version consistently across all version locations so users re-download updates. Please increment the version in copilot_here.sh, copilot_here.ps1, Directory.Build.props, app/Infrastructure/BuildInfo.cs, and update the “Current version” in .github/copilot-instructions.md (see .github/copilot-instructions.md Script Versioning section).

Copilot uses AI. Check for mistakes.
@GordonBeeming GordonBeeming merged commit b335af8 into main Feb 19, 2026
31 checks passed
@GordonBeeming GordonBeeming deleted the gb/issue64 branch February 19, 2026 21:00
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.

1 participant