Skip to content

Commit bbe8631

Browse files
mnriemCopilot
andauthored
feat(cli): add py script type & Python interpreter resolution (#3278) (#3285)
* feat(cli): add `py` script type & Python interpreter resolution (#3278) Introduce a third script variant alongside `sh`/`ps` as the foundation for unifying workflow scripts under a single Python implementation. - Add `"py": "Python"` to `SCRIPT_TYPE_CHOICES`; `VALID_SCRIPT_TYPES` consumers (init workflow step, init command, _helpers) pick it up automatically since they derive from that mapping. - Add `IntegrationBase.resolve_python_interpreter()` (project venv → `python3` → `python`, falling back to `python3`). - Prefix the resolved interpreter when `process_template()` expands `{SCRIPT}` for the `py` script type so `.py` scripts run portably (notably on Windows); thread `project_root` through callers so venv preference works. - Make `install_scripts()` mark copied `.py` files executable too. Includes positive and negative unit tests for interpreter resolution, `py` template processing, the new choice, and script installation. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): return repo-relative venv interpreter & correct docstring Address PR review feedback on #3285: - `resolve_python_interpreter()` now returns the venv interpreter as a path relative to the project root (`.venv/bin/python` / `.venv/Scripts/python.exe`) instead of an absolute/joined path, so the generated `{SCRIPT}` invocation stays portable and runnable from the repo root regardless of where the project lives. - Update `install_scripts()` docstring to note `.py` scripts are now made executable alongside `.sh`. - Update tests to assert the repo-relative interpreter path. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(cli): fall back to sys.executable for interpreter resolution When neither python3 nor python is discoverable on PATH (and no project venv is found), resolve_python_interpreter() now returns the running interpreter (sys.executable) so the generated {SCRIPT} invocation works in the current environment, falling back to "python3" only if that is also unavailable. Update unit tests accordingly. * fix(cli): quote py interpreter path when it contains whitespace For the `py` script type, the resolved interpreter may be an absolute path containing spaces (notably `sys.executable` under Windows `Program Files`). Quote it when it contains whitespace so the `{SCRIPT}` invocation isn't split into multiple arguments. Add positive/negative tests for the quoting behavior. * test: guard executable-bit assertions from Windows chmod semantics The Windows CI job failed because `os.chmod` does not set POSIX executable bits on Windows, so `install_scripts()` cannot make `.py`/ `.sh` files executable there (nor is it needed — the interpreter is invoked explicitly). Split the install_scripts test so file-copy behavior is still verified cross-platform, and skip the executable-bit assertions on win32 (matching the repo's existing pattern). --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3b30e40 commit bbe8631

8 files changed

Lines changed: 269 additions & 4 deletions

File tree

src/specify_cli/_agent_config.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,8 @@ def _build_agent_config() -> dict[str, dict[str, Any]]:
1717

1818
DEFAULT_INIT_INTEGRATION = "copilot"
1919

20-
SCRIPT_TYPE_CHOICES: dict[str, str] = {"sh": "POSIX Shell (bash/zsh)", "ps": "PowerShell"}
20+
SCRIPT_TYPE_CHOICES: dict[str, str] = {
21+
"sh": "POSIX Shell (bash/zsh)",
22+
"ps": "PowerShell",
23+
"py": "Python",
24+
}

src/specify_cli/integrations/base.py

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import re
1818
import shlex
1919
import shutil
20+
import sys
2021
from abc import ABC
2122
from dataclasses import dataclass
2223
from pathlib import Path
@@ -495,8 +496,8 @@ def install_scripts(
495496
496497
Copies files from this integration's ``scripts/`` directory to
497498
``.specify/integrations/<key>/scripts/`` in the project. Shell
498-
scripts are made executable. All copied files are recorded in
499-
*manifest*.
499+
(``.sh``) and Python (``.py``) scripts are made executable. All
500+
copied files are recorded in *manifest*.
500501
501502
Returns the list of files created.
502503
"""
@@ -513,7 +514,7 @@ def install_scripts(
513514
continue
514515
dst_script = scripts_dest / src_script.name
515516
shutil.copy2(src_script, dst_script)
516-
if dst_script.suffix == ".sh":
517+
if dst_script.suffix in (".sh", ".py"):
517518
dst_script.chmod(dst_script.stat().st_mode | 0o111)
518519
self.record_file_in_manifest(dst_script, project_root, manifest)
519520
created.append(dst_script)
@@ -538,13 +539,55 @@ def resolve_command_refs(content: str, separator: str = ".") -> str:
538539
content,
539540
)
540541

542+
@staticmethod
543+
def resolve_python_interpreter(project_root: Path | None = None) -> str:
544+
"""Resolve a portable Python interpreter command for ``{SCRIPT}``.
545+
546+
Used to build the invocation string for the ``py`` script type so
547+
that ``.py`` workflow scripts run consistently across platforms
548+
(notably Windows, where ``.py`` files are not directly executable).
549+
550+
Resolution order:
551+
552+
1. A project virtual environment (``.venv``) interpreter, if one
553+
exists under *project_root* (POSIX ``bin/python`` or Windows
554+
``Scripts/python.exe``). The returned path is **relative to the
555+
project root** (e.g. ``.venv/bin/python``) so generated
556+
``{SCRIPT}`` invocations stay portable and runnable from the
557+
repo root regardless of where the project lives.
558+
2. ``python3`` on ``PATH``.
559+
3. ``python`` on ``PATH``.
560+
561+
Falls back to the running interpreter (``sys.executable``) when
562+
``PATH`` resolution fails so the generated command is guaranteed
563+
to work in the current environment, and finally to ``"python3"``
564+
if even that is unavailable.
565+
"""
566+
if project_root is not None:
567+
# (existence check path, repo-root-relative invocation string)
568+
venv_candidates = (
569+
(project_root / ".venv" / "bin" / "python", ".venv/bin/python"),
570+
(
571+
project_root / ".venv" / "Scripts" / "python.exe",
572+
".venv/Scripts/python.exe",
573+
),
574+
)
575+
for candidate, relative in venv_candidates:
576+
if candidate.exists():
577+
return relative
578+
for name in ("python3", "python"):
579+
if shutil.which(name):
580+
return name
581+
return sys.executable or "python3"
582+
541583
@staticmethod
542584
def process_template(
543585
content: str,
544586
agent_name: str,
545587
script_type: str,
546588
arg_placeholder: str = "$ARGUMENTS",
547589
invoke_separator: str = ".",
590+
project_root: Path | None = None,
548591
) -> str:
549592
"""Process a raw command template into agent-ready content.
550593
@@ -578,6 +621,17 @@ def process_template(
578621

579622
# 2. Replace {SCRIPT}
580623
if script_command:
624+
# For the Python script type, prefix the resolved interpreter so
625+
# the command is portable (``.py`` files are not directly
626+
# executable on Windows).
627+
if script_type == "py":
628+
interpreter = IntegrationBase.resolve_python_interpreter(project_root)
629+
# Quote the interpreter if it contains whitespace (e.g. an
630+
# absolute ``sys.executable`` path under Windows
631+
# ``Program Files``) so it isn't split into multiple args.
632+
if any(ch.isspace() for ch in interpreter):
633+
interpreter = f'"{interpreter}"'
634+
script_command = f"{interpreter} {script_command}"
581635
content = content.replace("{SCRIPT}", script_command)
582636

583637
# 3. Strip scripts: section from frontmatter
@@ -784,6 +838,7 @@ def setup(
784838
raw = src_file.read_text(encoding="utf-8")
785839
processed = self.process_template(
786840
raw, self.key, script_type, arg_placeholder,
841+
project_root=project_root,
787842
)
788843
dst_name = self.command_filename(src_file.stem)
789844
dst_file = self.write_file_and_record(
@@ -986,6 +1041,7 @@ def setup(
9861041
description = self._extract_description(raw)
9871042
processed = self.process_template(
9881043
raw, self.key, script_type, arg_placeholder,
1044+
project_root=project_root,
9891045
)
9901046
_, body = self._split_frontmatter(processed)
9911047
toml_content = self._render_toml(description, body)
@@ -1186,6 +1242,7 @@ def setup(
11861242

11871243
processed = self.process_template(
11881244
raw, self.key, script_type, arg_placeholder,
1245+
project_root=project_root,
11891246
)
11901247
_, body = self._split_frontmatter(processed)
11911248
yaml_content = self._render_yaml(
@@ -1381,6 +1438,7 @@ def setup(
13811438
# Process body through the standard template pipeline
13821439
processed_body = self.process_template(
13831440
raw, self.key, script_type, arg_placeholder,
1441+
project_root=project_root,
13841442
invoke_separator=self.invoke_separator,
13851443
)
13861444
# Strip the processed frontmatter — we rebuild it for skills.

src/specify_cli/integrations/copilot/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,6 +370,7 @@ def _setup_default(
370370
raw = src_file.read_text(encoding="utf-8")
371371
processed = self.process_template(
372372
raw, self.key, script_type, arg_placeholder,
373+
project_root=project_root,
373374
)
374375
dst_name = self.command_filename(src_file.stem)
375376
dst_file = self.write_file_and_record(

src/specify_cli/integrations/forge/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ def setup(
134134
processed = self.process_template(
135135
raw, self.key, script_type, arg_placeholder,
136136
invoke_separator=self.invoke_separator,
137+
project_root=project_root,
137138
)
138139

139140
# FORGE-SPECIFIC: Ensure any remaining $ARGUMENTS placeholders are

src/specify_cli/integrations/generic/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ def setup(
123123
raw = src_file.read_text(encoding="utf-8")
124124
processed = self.process_template(
125125
raw, self.key, script_type, arg_placeholder,
126+
project_root=project_root,
126127
)
127128
dst_name = self.command_filename(src_file.stem)
128129
dst_file = self.write_file_and_record(

src/specify_cli/integrations/hermes/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ def setup(
140140
script_type,
141141
arg_placeholder,
142142
invoke_separator=self.invoke_separator,
143+
project_root=project_root,
143144
)
144145
# Strip the processed frontmatter — we rebuild it for skills.
145146
if processed_body.startswith("---"):

tests/integrations/test_base.py

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Tests for IntegrationOption, IntegrationBase, MarkdownIntegration, and primitives."""
22

3+
import sys
4+
35
import pytest
46

57
from specify_cli.integrations.base import (
@@ -299,3 +301,186 @@ def test_placeholder_with_digits(self):
299301
text = "__SPECKIT_COMMAND_V2_PLAN__"
300302
result = IntegrationBase.resolve_command_refs(text, ".")
301303
assert result == "/speckit.v2.plan"
304+
305+
306+
class TestResolvePythonInterpreter:
307+
def test_returns_python_on_path(self, monkeypatch):
308+
# Positive: when python3 is on PATH it is preferred over python.
309+
def fake_which(name):
310+
return f"/usr/bin/{name}" if name in ("python3", "python") else None
311+
312+
monkeypatch.setattr(
313+
"specify_cli.integrations.base.shutil.which", fake_which
314+
)
315+
assert IntegrationBase.resolve_python_interpreter() == "python3"
316+
317+
def test_falls_back_to_python_when_no_python3(self, monkeypatch):
318+
def fake_which(name):
319+
return "/usr/bin/python" if name == "python" else None
320+
321+
monkeypatch.setattr(
322+
"specify_cli.integrations.base.shutil.which", fake_which
323+
)
324+
assert IntegrationBase.resolve_python_interpreter() == "python"
325+
326+
def test_falls_back_to_sys_executable_when_nothing_found(self, monkeypatch):
327+
# Negative: nothing on PATH and no venv -> the running interpreter
328+
# (sys.executable) is used so the command works in this environment.
329+
monkeypatch.setattr(
330+
"specify_cli.integrations.base.shutil.which", lambda name: None
331+
)
332+
monkeypatch.setattr(
333+
"specify_cli.integrations.base.sys.executable", "/opt/py/bin/python"
334+
)
335+
assert IntegrationBase.resolve_python_interpreter() == "/opt/py/bin/python"
336+
337+
def test_falls_back_to_python3_when_no_interpreter_at_all(self, monkeypatch):
338+
# Negative edge: neither PATH nor sys.executable resolves.
339+
monkeypatch.setattr(
340+
"specify_cli.integrations.base.shutil.which", lambda name: None
341+
)
342+
monkeypatch.setattr(
343+
"specify_cli.integrations.base.sys.executable", ""
344+
)
345+
assert IntegrationBase.resolve_python_interpreter() == "python3"
346+
347+
def test_prefers_project_venv_posix(self, monkeypatch, tmp_path):
348+
venv_python = tmp_path / ".venv" / "bin" / "python"
349+
venv_python.parent.mkdir(parents=True)
350+
venv_python.write_text("")
351+
# Even if python3 is on PATH, the project venv wins. The returned
352+
# path is relative to the project root for portability.
353+
monkeypatch.setattr(
354+
"specify_cli.integrations.base.shutil.which",
355+
lambda name: "/usr/bin/python3",
356+
)
357+
result = IntegrationBase.resolve_python_interpreter(tmp_path)
358+
assert result == ".venv/bin/python"
359+
360+
def test_prefers_project_venv_windows(self, monkeypatch, tmp_path):
361+
venv_python = tmp_path / ".venv" / "Scripts" / "python.exe"
362+
venv_python.parent.mkdir(parents=True)
363+
venv_python.write_text("")
364+
monkeypatch.setattr(
365+
"specify_cli.integrations.base.shutil.which", lambda name: None
366+
)
367+
result = IntegrationBase.resolve_python_interpreter(tmp_path)
368+
assert result == ".venv/Scripts/python.exe"
369+
370+
def test_ignores_missing_venv(self, monkeypatch, tmp_path):
371+
# Negative: no venv directory -> PATH resolution is used instead.
372+
monkeypatch.setattr(
373+
"specify_cli.integrations.base.shutil.which",
374+
lambda name: "/usr/bin/python3" if name == "python3" else None,
375+
)
376+
assert IntegrationBase.resolve_python_interpreter(tmp_path) == "python3"
377+
378+
379+
class TestProcessTemplatePyScriptType:
380+
CONTENT = (
381+
"---\n"
382+
"scripts:\n"
383+
" sh: scripts/bash/check-prerequisites.sh --json\n"
384+
" ps: scripts/powershell/check-prerequisites.ps1 -Json\n"
385+
" py: scripts/python/check-prerequisites.py --json\n"
386+
"---\n"
387+
"Run {SCRIPT} now."
388+
)
389+
390+
def test_py_prefixes_interpreter(self, monkeypatch):
391+
# Positive: py script type prefixes a resolved interpreter and the
392+
# script path is rewritten to the .specify location.
393+
monkeypatch.setattr(
394+
"specify_cli.integrations.base.shutil.which",
395+
lambda name: "/usr/bin/python3" if name == "python3" else None,
396+
)
397+
result = IntegrationBase.process_template(self.CONTENT, "agent", "py")
398+
assert "python3 .specify/scripts/python/check-prerequisites.py --json" in result
399+
# The scripts: frontmatter block is stripped.
400+
assert "scripts:" not in result
401+
402+
def test_sh_does_not_prefix_interpreter(self):
403+
# Negative: non-py script types are never prefixed with an interpreter.
404+
result = IntegrationBase.process_template(self.CONTENT, "agent", "sh")
405+
assert ".specify/scripts/bash/check-prerequisites.sh --json" in result
406+
assert "python" not in result
407+
408+
def test_py_quotes_interpreter_with_spaces(self, monkeypatch):
409+
# An interpreter path containing whitespace (e.g. Windows
410+
# ``Program Files``) must be quoted so it isn't split into args.
411+
monkeypatch.setattr(
412+
"specify_cli.integrations.base.shutil.which", lambda name: None
413+
)
414+
monkeypatch.setattr(
415+
"specify_cli.integrations.base.sys.executable",
416+
r"C:\Program Files\Python\python.exe",
417+
)
418+
result = IntegrationBase.process_template(self.CONTENT, "agent", "py")
419+
assert (
420+
'"C:\\Program Files\\Python\\python.exe" '
421+
".specify/scripts/python/check-prerequisites.py --json"
422+
) in result
423+
424+
def test_py_does_not_quote_interpreter_without_spaces(self, monkeypatch):
425+
# Negative: a whitespace-free interpreter is left unquoted.
426+
monkeypatch.setattr(
427+
"specify_cli.integrations.base.shutil.which",
428+
lambda name: "/usr/bin/python3" if name == "python3" else None,
429+
)
430+
result = IntegrationBase.process_template(self.CONTENT, "agent", "py")
431+
assert '"' not in result.split("check-prerequisites.py")[0]
432+
433+
def test_py_uses_project_venv(self, monkeypatch, tmp_path):
434+
venv_python = tmp_path / ".venv" / "bin" / "python"
435+
venv_python.parent.mkdir(parents=True)
436+
venv_python.write_text("")
437+
result = IntegrationBase.process_template(
438+
self.CONTENT, "agent", "py", project_root=tmp_path
439+
)
440+
assert ".venv/bin/python .specify/scripts/python/check-prerequisites.py" in result
441+
442+
443+
class TestInstallScriptsPython:
444+
def _make_integration_with_scripts(self, monkeypatch, tmp_path):
445+
scripts_src = tmp_path / "bundled_scripts"
446+
scripts_src.mkdir()
447+
(scripts_src / "common.py").write_text("print('hi')\n")
448+
(scripts_src / "common.sh").write_text("echo hi\n")
449+
(scripts_src / "notes.txt").write_text("not executable\n")
450+
integration = StubIntegration()
451+
monkeypatch.setattr(
452+
integration, "integration_scripts_dir", lambda: scripts_src
453+
)
454+
return integration
455+
456+
def test_copies_all_script_files(self, monkeypatch, tmp_path):
457+
# Cross-platform: every bundled file is copied into the project.
458+
integration = self._make_integration_with_scripts(monkeypatch, tmp_path)
459+
project_root = tmp_path / "proj"
460+
project_root.mkdir()
461+
manifest = IntegrationManifest("stub", project_root.resolve())
462+
463+
created = integration.install_scripts(project_root, manifest)
464+
names = {p.name for p in created}
465+
assert {"common.py", "common.sh", "notes.txt"} == names
466+
467+
@pytest.mark.skipif(
468+
sys.platform == "win32", reason="chmod exec bit not reliable on Windows"
469+
)
470+
def test_marks_py_and_sh_executable(self, monkeypatch, tmp_path):
471+
integration = self._make_integration_with_scripts(monkeypatch, tmp_path)
472+
project_root = tmp_path / "proj"
473+
project_root.mkdir()
474+
manifest = IntegrationManifest("stub", project_root.resolve())
475+
476+
integration.install_scripts(project_root, manifest)
477+
478+
dest = project_root / ".specify" / "integrations" / "stub" / "scripts"
479+
py_file = dest / "common.py"
480+
sh_file = dest / "common.sh"
481+
txt_file = dest / "notes.txt"
482+
# Positive: .py and .sh are executable.
483+
assert py_file.stat().st_mode & 0o111
484+
assert sh_file.stat().st_mode & 0o111
485+
# Negative: a non-script file is not made executable.
486+
assert not (txt_file.stat().st_mode & 0o111)

tests/test_commands_package.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,20 @@ def test_agent_config_importable():
2424
assert "sh" in SCRIPT_TYPE_CHOICES
2525

2626

27+
def test_script_type_choices_includes_python():
28+
from specify_cli._agent_config import SCRIPT_TYPE_CHOICES
29+
assert SCRIPT_TYPE_CHOICES.get("py") == "Python"
30+
# The three supported variants are sh, ps, and py.
31+
assert {"sh", "ps", "py"} <= set(SCRIPT_TYPE_CHOICES)
32+
33+
34+
def test_workflow_init_valid_script_types_includes_python():
35+
from specify_cli.workflows.steps.init import VALID_SCRIPT_TYPES
36+
assert "py" in VALID_SCRIPT_TYPES
37+
# Negative: an unknown variant is not accepted.
38+
assert "rb" not in VALID_SCRIPT_TYPES
39+
40+
2741
def test_agent_config_re_exported_from_init():
2842
from specify_cli import AGENT_CONFIG, SCRIPT_TYPE_CHOICES
2943
assert isinstance(AGENT_CONFIG, dict)

0 commit comments

Comments
 (0)