Skip to content

Commit 320e759

Browse files
Copilotpatniko
andauthored
Fix Windows subprocess resolution for .cmd/.bat CLI executables (#338)
* Initial plan * Fix Windows CLI resolution for .cmd/.bat files using shutil.which() Co-authored-by: patniko <26906478+patniko@users.noreply.github.com> * Address code review feedback: move imports to top and use correct patch paths Co-authored-by: patniko <26906478+patniko@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: patniko <26906478+patniko@users.noreply.github.com> Co-authored-by: Patrick Nikoletich <patniko@github.com>
1 parent 149c346 commit 320e759

File tree

2 files changed

+70
-0
lines changed

2 files changed

+70
-0
lines changed

python/copilot/client.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import inspect
1717
import os
1818
import re
19+
import shutil
1920
import subprocess
2021
import threading
2122
from dataclasses import asdict, is_dataclass
@@ -1035,6 +1036,14 @@ async def _start_cli_server(self) -> None:
10351036
RuntimeError: If the server fails to start or times out.
10361037
"""
10371038
cli_path = self.options["cli_path"]
1039+
1040+
# Resolve the full path on Windows (handles .cmd/.bat files)
1041+
# On Windows, subprocess.Popen doesn't use PATHEXT to resolve extensions,
1042+
# so we need to use shutil.which() to find the actual executable
1043+
resolved_path = shutil.which(cli_path)
1044+
if resolved_path:
1045+
cli_path = resolved_path
1046+
10381047
args = ["--headless", "--log-level", self.options["log_level"]]
10391048

10401049
# Add auth-related flags

python/test_client.py

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.py instead.
55
"""
66

7+
from unittest.mock import MagicMock, patch
8+
79
import pytest
810

911
from copilot import CopilotClient
@@ -136,3 +138,62 @@ def test_use_logged_in_user_with_cli_url_raises(self):
136138
CopilotClient(
137139
{"cli_url": "localhost:8080", "use_logged_in_user": False, "log_level": "error"}
138140
)
141+
142+
143+
class TestCLIPathResolution:
144+
"""Test that CLI path resolution works correctly, especially on Windows."""
145+
146+
@pytest.mark.asyncio
147+
async def test_cli_path_resolved_with_which(self):
148+
"""Test that shutil.which() is used to resolve the CLI path."""
149+
# Create a mock resolved path
150+
mock_resolved_path = "/usr/local/bin/copilot"
151+
152+
with patch("copilot.client.shutil.which", return_value=mock_resolved_path):
153+
with patch("copilot.client.subprocess.Popen") as mock_popen:
154+
# Mock the process and its stdout for TCP mode
155+
mock_process = MagicMock()
156+
mock_process.stdout.readline.return_value = b"listening on port 8080\n"
157+
mock_popen.return_value = mock_process
158+
159+
client = CopilotClient(
160+
{"cli_path": "copilot", "use_stdio": False, "log_level": "error"}
161+
)
162+
163+
try:
164+
await client._start_cli_server()
165+
166+
# Verify that subprocess.Popen was called with the resolved path
167+
mock_popen.assert_called_once()
168+
args = mock_popen.call_args[0][0]
169+
assert args[0] == mock_resolved_path
170+
finally:
171+
if client._process:
172+
client._process = None
173+
174+
@pytest.mark.asyncio
175+
async def test_cli_path_not_resolved_when_which_returns_none(self):
176+
"""Test that original path is used when shutil.which() returns None."""
177+
original_path = "/custom/path/to/copilot"
178+
179+
with patch("copilot.client.shutil.which", return_value=None):
180+
with patch("copilot.client.subprocess.Popen") as mock_popen:
181+
# Mock the process and its stdout for TCP mode
182+
mock_process = MagicMock()
183+
mock_process.stdout.readline.return_value = b"listening on port 8080\n"
184+
mock_popen.return_value = mock_process
185+
186+
client = CopilotClient(
187+
{"cli_path": original_path, "use_stdio": False, "log_level": "error"}
188+
)
189+
190+
try:
191+
await client._start_cli_server()
192+
193+
# Verify that subprocess.Popen was called with the original path
194+
mock_popen.assert_called_once()
195+
args = mock_popen.call_args[0][0]
196+
assert args[0] == original_path
197+
finally:
198+
if client._process:
199+
client._process = None

0 commit comments

Comments
 (0)