From 8cda4458314266640b041bba5ac2ac465e1f9a66 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Fri, 25 Sep 2026 17:24:06 +0000 Subject: [PATCH 1/3] fix(copilot): detect copilot.exe on Windows instead of assuming copilot.cmd _copilot_executable() hardcoded the npm-shim name copilot.cmd on Windows, so workflow command-step dispatch failed for any Copilot CLI install that puts copilot.exe on PATH instead (standalone installer, winget, scoop). Probe PATH for copilot.exe, then copilot.cmd, before falling back to the historical default. Fixes #4755 Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous) --- .../integrations/copilot/__init__.py | 19 +++++++-- tests/integrations/test_extra_args.py | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 1a8285d7c4..9aa549c777 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -49,11 +49,22 @@ def _copilot_executable() -> str: """Return the executable name for Copilot CLI on this platform. - On Windows, subprocess invocation is reliable with `copilot.cmd`. + On Windows, the Copilot CLI may be installed as `copilot.exe` (e.g. a + standalone installer, winget, scoop) or as a `copilot.cmd` npm shim. + Probe `PATH` for whichever is actually present instead of assuming the + npm-style shim. """ - if os.name == "nt": - return "copilot.cmd" - return "copilot" + if os.name != "nt": + return "copilot" + + for candidate in ("copilot.exe", "copilot.cmd", "copilot"): + if shutil.which(candidate): + return candidate + + # Nothing found on PATH — keep the historical default so the + # resulting "command not found" error still references the + # previously expected name. + return "copilot.cmd" def _allow_all() -> bool: diff --git a/tests/integrations/test_extra_args.py b/tests/integrations/test_extra_args.py index 0ab68cb43a..44c488263c 100644 --- a/tests/integrations/test_extra_args.py +++ b/tests/integrations/test_extra_args.py @@ -639,6 +639,45 @@ def test_executable_env_var_copilot_unset_uses_platform_default(monkeypatch): assert args[0] == _copilot_executable() +def test_copilot_executable_windows_prefers_exe_on_path(monkeypatch): + """On Windows, `_copilot_executable()` must detect a `copilot.exe` + install rather than assuming the npm `copilot.cmd` shim (#4755).""" + import shutil + + from specify_cli.integrations.copilot import _copilot_executable + + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr( + shutil, "which", lambda name: r"C:\tools\copilot.exe" if name == "copilot.exe" else None + ) + assert _copilot_executable() == "copilot.exe" + + +def test_copilot_executable_windows_falls_back_to_cmd_shim(monkeypatch): + """A Windows install exposing only `copilot.cmd` (npm shim) still works.""" + import shutil + + from specify_cli.integrations.copilot import _copilot_executable + + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr( + shutil, "which", lambda name: r"C:\tools\copilot.cmd" if name == "copilot.cmd" else None + ) + assert _copilot_executable() == "copilot.cmd" + + +def test_copilot_executable_windows_nothing_on_path_keeps_historical_default(monkeypatch): + """Nothing found on PATH keeps the historical `copilot.cmd` default so + the resulting error still names the previously expected executable.""" + import shutil + + from specify_cli.integrations.copilot import _copilot_executable + + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr(shutil, "which", lambda name: None) + assert _copilot_executable() == "copilot.cmd" + + def test_executable_env_var_copilot_dispatch_command(monkeypatch): """CopilotIntegration.dispatch_command honours the executable env var.""" import subprocess From 4f88e14a3148e5b2f7dab2d7c7d95aa86eaceb3a Mon Sep 17 00:00:00 2001 From: chelsealong Date: Mon, 28 Sep 2026 15:06:38 +0000 Subject: [PATCH 2/3] docs(copilot): update _resolve_executable docstring for PATH probing --- src/specify_cli/integrations/copilot/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 9aa549c777..9f94dcca1a 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -304,9 +304,10 @@ def _resolve_executable(self) -> str: """Return the Copilot CLI executable, respecting the env-var override. Checks ``SPECKIT_INTEGRATION_COPILOT_EXECUTABLE`` first. Falls - back to the platform-specific default from ``_copilot_executable()`` - (``copilot.cmd`` on Windows, ``copilot`` elsewhere) so that - existing behaviour is preserved when the env var is unset. + back to the platform-specific default from ``_copilot_executable()``: + on Windows this probes ``PATH`` for ``copilot.exe`` then + ``copilot.cmd``, only falling back to ``copilot.cmd`` when neither is + found; elsewhere it is always ``copilot``. """ env_name = "SPECKIT_INTEGRATION_COPILOT_EXECUTABLE" override = os.environ.get(env_name, "").strip() From 910e7253b69edfc1533127c2c11e3f38b8a0de41 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Mon, 28 Sep 2026 16:36:42 +0000 Subject: [PATCH 3/3] fix(copilot): drop unlaunchable bare-name PATH probe, make tests deterministic Copilot bot review flagged three issues: the bare "copilot" candidate in _copilot_executable() can match a .bat/.com via PATHEXT that CreateProcess can't actually launch; test_copilot_exec_args hardcoded copilot.cmd for os.name == "nt" instead of deriving it from _copilot_executable(), making it non-deterministic on a real Windows runner; and the "prefers .exe" test only mocked .exe being present, so it didn't actually lock in preference over .cmd. --- .../integrations/copilot/__init__.py | 2 +- tests/integrations/test_extra_args.py | 20 ++++++++++++++++--- tests/test_workflows.py | 5 ++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index 9f94dcca1a..ace60fe217 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -57,7 +57,7 @@ def _copilot_executable() -> str: if os.name != "nt": return "copilot" - for candidate in ("copilot.exe", "copilot.cmd", "copilot"): + for candidate in ("copilot.exe", "copilot.cmd"): if shutil.which(candidate): return candidate diff --git a/tests/integrations/test_extra_args.py b/tests/integrations/test_extra_args.py index 44c488263c..6babdee392 100644 --- a/tests/integrations/test_extra_args.py +++ b/tests/integrations/test_extra_args.py @@ -647,9 +647,8 @@ def test_copilot_executable_windows_prefers_exe_on_path(monkeypatch): from specify_cli.integrations.copilot import _copilot_executable monkeypatch.setattr(os, "name", "nt") - monkeypatch.setattr( - shutil, "which", lambda name: r"C:\tools\copilot.exe" if name == "copilot.exe" else None - ) + paths = {"copilot.exe": r"C:\tools\copilot.exe", "copilot.cmd": r"C:\tools\copilot.cmd"} + monkeypatch.setattr(shutil, "which", lambda name: paths.get(name)) assert _copilot_executable() == "copilot.exe" @@ -678,6 +677,21 @@ def test_copilot_executable_windows_nothing_on_path_keeps_historical_default(mon assert _copilot_executable() == "copilot.cmd" +def test_copilot_executable_windows_ignores_unlaunchable_bare_name(monkeypatch): + """A bare `copilot` match (e.g. a `.bat`/`.com` resolved via `PATHEXT`) + must not be returned: `CreateProcess` doesn't consult `PATHEXT`, so a + bare name detected this way can't actually be launched.""" + import shutil + + from specify_cli.integrations.copilot import _copilot_executable + + monkeypatch.setattr(os, "name", "nt") + monkeypatch.setattr( + shutil, "which", lambda name: r"C:\tools\copilot.bat" if name == "copilot" else None + ) + assert _copilot_executable() == "copilot.cmd" + + def test_executable_env_var_copilot_dispatch_command(monkeypatch): """CopilotIntegration.dispatch_command honours the executable env var.""" import subprocess diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d8abcc0f55..bd1332edbe 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1024,11 +1024,10 @@ def test_codex_exec_args(self): def test_copilot_exec_args(self, monkeypatch): monkeypatch.delenv("SPECKIT_COPILOT_ALLOW_ALL_TOOLS", raising=False) monkeypatch.delenv("SPECKIT_ALLOW_ALL_TOOLS", raising=False) - from specify_cli.integrations.copilot import CopilotIntegration + from specify_cli.integrations.copilot import CopilotIntegration, _copilot_executable impl = CopilotIntegration() args = impl.build_exec_args("do stuff", model="claude-sonnet-4-20250514") - expected_exec = "copilot.cmd" if os.name == "nt" else "copilot" - assert args[0] == expected_exec + assert args[0] == _copilot_executable() assert "-p" in args assert "--yolo" in args assert "--model" in args