Skip to content

feat(extensions): port git extension scripts to Python#3400

Open
marcelsafin wants to merge 7 commits into
github:mainfrom
marcelsafin:feat/3282-port-git-extension
Open

feat(extensions): port git extension scripts to Python#3400
marcelsafin wants to merge 7 commits into
github:mainfrom
marcelsafin:feat/3282-port-git-extension

Conversation

@marcelsafin

Copy link
Copy Markdown
Contributor

Summary

Fixes #3282 — part of #3277. Last script port in the porting phase: extensions/git/scripts/{bash,powershell}/ now has Python twins in extensions/git/scripts/python/.

Changes

  • git_common.pyhas_git, effective_branch_name, check_feature_branch (port of git-common.sh)
  • initialize_repo.py — repo init with configurable initial commit message
  • auto_commit.py — per-event auto-commit, mirroring the line-based git-config.yml parser including the default/explicit-false precedence rules
  • create_new_feature_branch.py — full port: slug generation with stop-word filtering and acronym handling, sequential numbering across specs/local branches/remote refs, branch_template/branch_prefix with scoped numbering, GIT_BRANCH_NAME override, 244-byte truncation, SPECIFY_INIT_DIR via the core common.py resolver (same refusal as bash when core scripts are missing)

Same conventions as the merged check-prerequisites PoC and the #3280/#3281 ports: stdlib only, standalone scripts, no runtime wiring changes.

Testing

43 new tests in tests/extensions/git/test_git_extension_python_parity.py. The parity tests run each bash script and its Python twin in identical twin projects and assert matching stdout, exit codes, stderr, and resulting git state (branches created, commit messages). Covers branch creation and commit behavior per the issue's acceptance criteria, plus template validation errors, existing-branch handling, and no-git degradation.

  • uv run pytest tests/extensions/git/ — 97 passed
  • Full suite: 3847 passed, 105 skipped
  • uvx ruff check — clean

Checklist

  • Tests pass locally
  • Lint passes
  • I did use AI assistance for this contribution

Ports git-common, initialize-repo, auto-commit, and
create-new-feature-branch to extensions/git/scripts/python/,
mirroring the bash/PowerShell twins. Parity tests run each bash
script and its Python twin in identical projects and compare
output, exit codes, and resulting git state.

Fixes github#3282

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 7, 2026 20:28
@marcelsafin marcelsafin requested a review from mnriem as a code owner July 7, 2026 20:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Python equivalents for the extensions/git workflow scripts, aiming to preserve behavior while reducing dual-maintenance across bash/PowerShell. It also introduces a parity-focused test suite to validate the Python ports against the existing bash implementations.

Changes:

  • Added Python ports for create-new-feature-branch, initialize-repo, auto-commit, and shared git helpers.
  • Introduced a parity test suite that runs bash vs Python “twin projects” and compares outcomes.
  • Added unit tests for git_common.py helper behavior.

Reviewed changes

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

Show a summary per file
File Description
tests/extensions/git/test_git_extension_python_parity.py Adds parity tests (bash vs Python) plus unit tests for git_common.py.
extensions/git/scripts/python/git_common.py Implements git detection + feature-branch validation helpers.
extensions/git/scripts/python/initialize_repo.py Initializes a repo and makes an initial commit using optional config-driven message.
extensions/git/scripts/python/auto_commit.py Parses git-config.yml auto-commit settings and commits changes per event.
extensions/git/scripts/python/create_new_feature_branch.py Implements feature-branch naming/numbering/template logic and branch creation, mirroring bash behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to +52
Configuration:
branch_template Optional git-config.yml template with {{author}}, {{app}}, {{number}}, {{slug}}
branch_prefix Optional shorthand namespace expanded before {{number}}-{{slug}}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The help text renders correctly: HELP_TEXT is an f-string, so {{number}} in the source emits {number} at runtime. Verified: python3 create_new_feature_branch.py --help prints "template with {author}, {app}, {number}, {slug}".

Comment on lines +413 to +416
feature_description = " ".join(args.description_parts).strip()
if not feature_description:
_err(USAGE)
return 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ee4f9c8. The script now mirrors the bash order: usage error for missing description, then the specific whitespace message after trimming.

Comment on lines +275 to +280
def test_empty_description_errors(self, tmp_path: Path):
bash_proj, py_proj = _twin_projects(tmp_path)
b = _run_bash("create-new-feature-branch.sh", bash_proj, "--json", " ")
p = _run_py("create-new-feature-branch", py_proj, "--json", " ")
assert b.returncode == p.returncode == 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ee4f9c8. The test now asserts stderr parity and the specific error message.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment on lines +76 to +87
def _assert_parity(
bash_result: subprocess.CompletedProcess,
py_result: subprocess.CompletedProcess,
*,
stdout: bool = True,
) -> None:
assert py_result.returncode == bash_result.returncode, (
f"exit codes diverge: bash={bash_result.returncode} py={py_result.returncode}\n"
f"bash stderr: {bash_result.stderr}\npy stderr: {py_result.stderr}"
)
if stdout:
assert py_result.stdout == bash_result.stdout

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. _assert_parity now compares stderr by default alongside exit code and stdout. All existing parity tests pass with the stricter assertion, so no call sites needed an opt-out. 5241529

in_auto_commit = False
in_event = False

content = config_file.read_text(encoding="utf-8")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed. read_text is now wrapped in try/except OSError and returns (False, ""), so an unreadable config disables auto-commit the same way a missing one does. Added test_unreadable_config_skips_auto_commit (chmod 000, asserts exit 0, no traceback, no commit). 5241529

An unreadable config file raised OSError with a full traceback from
_parse_auto_commit_config. Treat it like a missing config: auto-commit
stays disabled. Covered by a chmod-000 test (skipped on non-POSIX and
as root).

_assert_parity now also compares stderr so warning or usage-text
regressions between the bash and Python twins fail the suite. All
existing parity tests pass with the stricter assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 8, 2026 19:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +437 to +439
if core is not None and hasattr(core, "get_repo_root"):
repo_root = core.get_repo_root()
else:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 5479027. core.get_repo_root is now called with script_file=Path(__file__), and a TypeError fallback to the no-arg form keeps older cores working. cwd-outside-repo callers now resolve the same repo_root as the bash twin.

…utside-repo callers

Without script_file, core.get_repo_root() falls back to Path.cwd() when
SPECIFY_INIT_DIR is unset and no .specify root is found upward — the
bash twin instead falls back to the script's install location
(.specify/scripts/...). Pass script_file so both twins resolve the
same repo_root; TypeError fallback keeps older cores working.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 10, 2026 06:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment on lines +287 to +296
_, py_proj = _twin_projects(tmp_path)
elsewhere = tmp_path / "elsewhere"
elsewhere.mkdir()
p = _run_py(
"create-new-feature-branch", py_proj,
"--json", "--dry-run", "init dir feature",
env_extra={"SPECIFY_INIT_DIR": str(py_proj)},
)
assert p.returncode == 0
assert json.loads(p.stdout)["FEATURE_NUM"] == "001"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 8b3cc64. _run_py got a run_cwd override and the test now runs the script from elsewhere, so repo-root discovery fails unless SPECIFY_INIT_DIR does the resolution. That was the original intent of the unused directory.

("2026031-143022-slug", False),
],
)
def test_check_feature_branch(self, git_common, branch: str, expected: bool, capsys):

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in 8b3cc64.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 10, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

email = lines[0] if lines else ""
author = email.split("@")[0]
if not author:
author = os.environ.get("USER", "unknown") or "unknown"
Comment on lines +328 to +331
try:
lines = config_file.read_text(encoding="utf-8").splitlines()
except OSError:
return ""
Comment on lines +35 to +38
try:
lines = config_file.read_text(encoding="utf-8").splitlines()
except OSError:
return default
Comment on lines +55 to +60
try:
content = config_file.read_text(encoding="utf-8")
except OSError:
# Unreadable config is treated like a missing one: auto-commit
# stays disabled instead of crashing with a traceback.
return False, ""
…back

- Catch (OSError, UnicodeDecodeError) when reading git-config.yml in
  create_new_feature_branch.py, initialize_repo.py, and auto_commit.py
  so invalid UTF-8 config falls back to defaults instead of crashing
  with a traceback.
- Fall back to USERNAME (then "unknown") when USER is unset when
  deriving the branch author token, matching the PowerShell twin's
  Windows-friendly fallback chain.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 10, 2026 20:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines +297 to +309
def test_specify_init_dir_resolves_target_project(self, tmp_path: Path):
_, py_proj = _twin_projects(tmp_path)
elsewhere = tmp_path / "elsewhere"
elsewhere.mkdir()
# Run from outside the project so SPECIFY_INIT_DIR is what resolves it.
p = _run_py(
"create-new-feature-branch", py_proj,
"--json", "--dry-run", "init dir feature",
env_extra={"SPECIFY_INIT_DIR": str(py_proj)},
run_cwd=elsewhere,
)
assert p.returncode == 0
assert json.loads(p.stdout)["FEATURE_NUM"] == "001"
f"creation for {branch_name}"
)

_err(f"# To persist: export SPECIFY_FEATURE={shlex.quote(branch_name)}")
Comment on lines +615 to +617
print(
"# To persist in your shell: export "
f"SPECIFY_FEATURE={shlex.quote(branch_name)}"
…_INIT_DIR test

- Add a shared _persist_hint() helper in create_new_feature_branch.py
  and use it for both the JSON-mode stderr hint and the human-readable
  stdout hint, so there is a single place emitting the SPECIFY_FEATURE
  persistence guidance. On Windows (os.name == "nt") it prints
  PowerShell $env:VAR = "..." syntax; elsewhere it keeps the existing
  POSIX export VAR=... syntax (parity with the bash twin).
- Rework test_specify_init_dir_resolves_target_project so SPECIFY_INIT_DIR
  is the only thing that can produce the observed result: the script now
  runs from a separate host_proj (no existing specs, so script/cwd-based
  discovery would yield 001) while SPECIFY_INIT_DIR points at a different
  target_proj that already has an existing spec (007-existing, so the
  override must yield 008). The old version pointed SPECIFY_INIT_DIR at
  the same project the script was installed in, so it passed even if the
  env var were ignored.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 10, 2026 20:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

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.

Port git extension scripts to Python

3 participants