feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python#3386
feat(scripts): port create-new-feature, setup-plan and setup-tasks to Python#3386marcelsafin wants to merge 9 commits into
Conversation
… Python Ports the three core workflow scripts to Python as part of github#3280, following the check-prerequisites PoC pattern from github#3302. Adds resolve_template() to the shared common.py module and parity tests that run bash and Python side by side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ports three core Spec Kit workflow scripts (create-new-feature, setup-plan, setup-tasks) to Python while preserving the existing shell implementations, and adds parity tests to ensure the Python ports match Bash/PowerShell output and exit behavior.
Changes:
- Added Python implementations for
create-new-feature,setup-plan, andsetup-tasks, plus sharedresolve_template()logic inscripts/python/common.py. - Added shared parity-test utilities (
tests/parity_helpers.py) to run Bash/PowerShell/Python twins in temporary repos with normalized output comparisons. - Added parity test suites for the three newly-ported scripts, including JSON/text output parity and PowerShell JSON parity where available.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_setup_tasks_python_parity.py | Adds Bash/Python + PowerShell/Python parity tests for setup-tasks. |
| tests/test_setup_plan_python_parity.py | Adds Bash/Python + PowerShell/Python parity tests for setup-plan. |
| tests/test_create_new_feature_python_parity.py | Adds parity tests for create-new-feature including numbering, truncation, errors, and persistence. |
| tests/parity_helpers.py | Introduces reusable helpers for parity tests (repo scaffolding, command builders, normalization, subprocess runner). |
| scripts/python/setup_tasks.py | Python port of setup-tasks including prereq checks + template resolution and JSON/text output. |
| scripts/python/setup_plan.py | Python port of setup-plan including template copy behavior and JSON-mode stderr status messages. |
| scripts/python/create_new_feature.py | Python port of create-new-feature including naming rules, numbering/timestamp modes, truncation, and feature.json persistence. |
| scripts/python/common.py | Adds resolve_template() and preset ordering helper to mirror Bash template resolution priority. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def run( | ||
| cmd: list[str], repo: Path, env: dict[str, str] | None = None | ||
| ) -> subprocess.CompletedProcess[str]: | ||
| return subprocess.run( | ||
| cmd, | ||
| cwd=repo, | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| env=env or clean_env(), | ||
| ) |
There was a problem hiding this comment.
Fixed in 83a5dca: env if env is not None else clean_env().
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| def _sorted_preset_ids(presets_dir: Path) -> list[str]: | ||
| registry = presets_dir / ".registry" | ||
| if registry.is_file(): | ||
| try: | ||
| data = json.loads(registry.read_text(encoding="utf-8")) | ||
| except (OSError, json.JSONDecodeError): | ||
| data = None | ||
| if isinstance(data, dict): | ||
| presets = data.get("presets", {}) | ||
| if isinstance(presets, dict): | ||
| return [ | ||
| pid | ||
| for pid, meta in sorted( | ||
| presets.items(), | ||
| key=lambda kv: kv[1].get("priority", 10) | ||
| if isinstance(kv[1], dict) | ||
| else 10, | ||
| ) | ||
| if isinstance(meta, dict) and meta.get("enabled", True) is not False | ||
| ] | ||
| try: | ||
| return sorted(p.name for p in presets_dir.iterdir() if p.is_dir()) | ||
| except OSError: | ||
| return [] |
There was a problem hiding this comment.
Fixed in d4e4c5e: the whole registry read/sort is now wrapped in a broad except that falls back to the directory scan, and the scan skips hidden dirs, matching the bash behavior. Added a parity test with an unorderable priority value plus a hidden preset dir.
… hidden preset dirs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ships with the scripts they reference; the remaining templates got their py: lines in github#3403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| def ps_cmd(repo: Path, script: str, *args: str) -> list[str]: | ||
| return [ | ||
| POWERSHELL_EXE, | ||
| "-NoProfile", | ||
| "-File", | ||
| str(repo / ".specify" / "scripts" / "powershell" / f"{script}.ps1"), | ||
| *args, | ||
| ] |
There was a problem hiding this comment.
All ps_cmd call sites are guarded by HAS_POWERSHELL skips, but the failure mode was obscure. Added an assert with a clear message so a missing guard fails loudly.
| scripts: | ||
| sh: scripts/bash/setup-plan.sh --json | ||
| ps: scripts/powershell/setup-plan.ps1 -Json | ||
| py: scripts/python/setup_plan.py --json |
There was a problem hiding this comment.
Good catch. resolve_skill_placeholders now accepts py and prefixes the resolved interpreter (quoted when the path contains spaces), matching process_template. Covered in tests/test_skill_placeholder_py.py.
| scripts: | ||
| sh: scripts/bash/setup-tasks.sh --json | ||
| ps: scripts/powershell/setup-tasks.ps1 -Json | ||
| py: scripts/python/setup_tasks.py --json |
There was a problem hiding this comment.
Fixed together with the plan.md thread: the skills resolver handles py with interpreter prefixing now.
resolve_skill_placeholders only accepted sh/ps, so a py init option
fell into the fallback path and {SCRIPT} rendered without an
interpreter prefix. Accept py and prefix the resolved interpreter,
matching process_template. Also guard ps_cmd against a missing
PowerShell with a clear assert.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| @requires_bash | ||
| @pytest.mark.parametrize( | ||
| "args", | ||
| [ | ||
| (), | ||
| (" ",), | ||
| ("--short-name",), | ||
| ("--number",), | ||
| ], | ||
| ids=["missing_description", "whitespace_description", "short_name_no_value", "number_no_value"], | ||
| ) | ||
| def test_python_argument_errors_match_bash(repo: Path, args: tuple[str, ...]) -> None: | ||
| bash = run(bash_cmd(repo, SCRIPT, *args), repo) | ||
| py = run(py_cmd(repo, SCRIPT, *args), repo) | ||
|
|
||
| assert py.returncode == bash.returncode == 1 | ||
| assert py.stdout == bash.stdout == "" | ||
| assert normalize_script_names(py.stderr, repo, SCRIPT) == normalize_script_names( | ||
| bash.stderr, repo, SCRIPT | ||
| ) | ||
|
|
There was a problem hiding this comment.
Added test_python_invalid_number_fails_cleanly: exit code 1, empty stdout, and the exact error message are pinned, with a comment explaining the deliberate deviation from the bash arithmetic error.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| scripts: | ||
| sh: scripts/bash/setup-plan.sh --json | ||
| ps: scripts/powershell/setup-plan.ps1 -Json | ||
| py: scripts/python/setup_plan.py --json |
There was a problem hiding this comment.
| scripts: | ||
| sh: scripts/bash/setup-tasks.sh --json | ||
| ps: scripts/powershell/setup-tasks.ps1 -Json | ||
| py: scripts/python/setup_tasks.py --json |
There was a problem hiding this comment.
Same as the plan.md thread: covered by the shared-infra fix in #3403.
| def _help_text(argv0: str) -> str: | ||
| return f"""{_usage(argv0)} | ||
|
|
||
| Options: | ||
| --json Output in JSON format | ||
| --dry-run Compute feature name and paths without creating directories or files | ||
| --allow-existing-branch Reuse an existing feature directory if it already exists | ||
| --short-name <name> Provide a custom short name (2-4 words) for the feature | ||
| --number N Specify branch number manually (overrides auto-detection) | ||
| --timestamp Use timestamp prefix (YYYYMMDD-HHMMSS) instead of sequential numbering | ||
| --help, -h Show this help message |
There was a problem hiding this comment.
Right, --no-git does not exist in any variant. That was a description error on my part, not missing code: the flag list should have said --allow-existing-branch only. PR description fixed. The bash twin has no --no-git either, so implementing it here would break parity scope.
| elif arg in {"--help", "-h"}: | ||
| sys.stdout.write(_help_text(sys.argv[0])) | ||
| return 0 | ||
| # Other arguments are collected but unused, matching setup-plan.sh. |
There was a problem hiding this comment.
Reworded in 7408065. The comment now says the extra args are 'accepted and silently ignored', which matches what the loop and setup-plan.sh actually do.
The loop accepts and silently ignores extra positional args (it doesn't build a collected list); match the wording to what the code and setup-plan.sh actually do. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| script_variant = init_opts.get("script") | ||
| if script_variant not in {"sh", "ps"}: | ||
| if script_variant not in {"sh", "ps", "py"}: | ||
| fallback_order = [] |
There was a problem hiding this comment.
Fixed in 6e8f2e8. The fallback now also triggers when the configured variant is missing from the command's scripts mapping, not only when the variant name is unknown, so {SCRIPT} is never left unresolved. Regression test added: script=py with an sh/ps-only template resolves to the sh variant.
| scripts: | ||
| sh: scripts/bash/setup-plan.sh --json | ||
| ps: scripts/powershell/setup-plan.ps1 -Json | ||
| py: scripts/python/setup_plan.py --json | ||
| --- |
There was a problem hiding this comment.
Good catch, the description overstated it. Updated the PR body: the port is additive except for the scripts.py entries in plan.md/tasks.md and the py variant support in resolve_skill_placeholders.
…tter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/specify_cli/agents.py:418
- When a Python-configured project encounters a command without a
pyentry, this fallback selectssh(orpson Windows). Shared infra installs only the selected script variant, so ascript=pyproject does not contain.specify/scripts/bash/powershell; commands such asclarify.md, which currently has onlysh/ps, therefore resolve{SCRIPT}to a path that does not exist. Please ensure those commands provide/use their installed Python twin, or install the chosen fallback variant rather than merely replacing the placeholder.
if script_variant not in {"sh", "ps", "py"} or script_variant not in scripts:
fallback_order = []
default_variant = (
"ps" if platform.system().lower().startswith("win") else "sh"
)
| if branch_number: | ||
| try: | ||
| number = int(branch_number, 10) | ||
| except ValueError: | ||
| print( | ||
| f"Error: --number must be an integer, got '{branch_number}'", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 |
…10# parity The bash twin uses $((10#$BRANCH_NUMBER)), which rejects signed and whitespace-padded values. Python's int() accepted them (e.g. -1), producing a malformed -01-... prefix that sequential scans ignore. Restrict --number to unsigned decimal digits before conversion, and pin the parity with a bash-comparison test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Ports the three core workflow scripts to Python, per #3280. Follows the pattern established by the check-prerequisites PoC (#3302): additive port with parity tests that run the bash and Python versions side by side and compare output. Two small non-additive touches:
templates/commands/plan.mdandtemplates/commands/tasks.mdgain ascripts.pyentry, andresolve_skill_placeholdersacceptspyas a script variant (with interpreter prefixing and a fallback when a command's frontmatter lacks the configured variant).Scope:
scripts/python/create_new_feature.py— port ofcreate-new-feature.sh, including branch-name generation (stop words, acronym detection, word limits), feature numbering (specs scan + timestamp-dir handling), the 244-char ref-length truncation,--allow-existing-branchmode, andfeature.jsonpersistence (byte-identical to the jq output).scripts/python/setup_plan.py— port ofsetup-plan.sh, including plan template resolution and the status messages that go to stderr in--jsonmode.scripts/python/setup_tasks.py— port ofsetup-tasks.sh, including plan/spec validation and the tasks-template requirement.scripts/python/common.py— addsresolve_template()(overrides → presets via registry → extensions → core), shared by the three scripts above.tests/parity_helpers.py— shared fixtures/helpers for parity tests, extracted from the pattern intest_check_prerequisites_python_parity.py.pwshis unavailable).One deliberate deviation:
--number abcproduces a clean error message in Python where bash surfaces a raw arithmetic error (10#abc: value too great for base). The parity test asserts exit code and empty stdout for that case, not the stderr text.Testing
uv run specify --helpuv sync && uv run pytest(3801 passed, 110 skipped)AI Disclosure
Ported and tested with GitHub Copilot CLI. I reviewed the diff and verified parity behavior manually.
Fixes #3280