Skip to content

fix(git): tolerate noisy git rev-parse output in is_git_project - #1980

Open
bearomorphism wants to merge 2 commits into
commitizen-tools:masterfrom
bearomorphism:fix/1497-better-not-a-git-project-diagnostics
Open

fix(git): tolerate noisy git rev-parse output in is_git_project#1980
bearomorphism wants to merge 2 commits into
commitizen-tools:masterfrom
bearomorphism:fix/1497-better-not-a-git-project-diagnostics

Conversation

@bearomorphism

@bearomorphism bearomorphism commented May 9, 2026

Copy link
Copy Markdown
Collaborator

Description

Closes #1497.

Why

Users running commitizen via uv tool install commitizen inside Git Bash on Windows 11 encounter NotAGitProjectError even though running git rev-parse --is-inside-work-tree manually in the same shell returns true. Commands that require a git context — cz commit, cz bump, cz changelog — all raise the error, while read-only commands such as cz info and cz ls succeed because they never call is_git_project().

The root cause is the single-line check at commitizen/git.py:316 (master):

return c.out.strip() == "true"

Some shell wrappers — including Git Bash's colour-prompt infrastructure and certain terminal emulators on Windows — inject ANSI CSI escape sequences (e.g., \x1b[0m) into subprocess standard output. When that happens, c.out.strip() yields "\x1b[0mtrue" rather than the bare string "true", the strict equality check fails silently, and is_git_project() returns False, triggering the error even inside a valid work tree.

This was surfaced during the #1964 audit: "Depending on subprocess setup with uv tool install on git-bash for Windows, [the output] may not match exactly." Because the original reporter's exact environment couldn't be reproduced locally, the new --debug logging path provides the next-best diagnostic: any future reporter can run cz --debug commit and see the exact bytes returned by git rev-parse.

What changed

File Change
commitizen/git.py Add _ANSI_ESCAPE compiled regex; rewrite is_git_project() to trust the exit code first, strip ANSI codes, then exact-match "true"; add logger.debug for both failure paths
tests/test_git.py Add five targeted regression tests: inside-work-tree, outside-repo, ANSI-prefixed output (the #1497 case), bare-repo interior, and substring-true rejection (parametrised)

How it works

  • Exit-code-first gate: if git rev-parse returns non-zero, we are definitively outside any git context — is_git_project() returns False immediately, independent of output parsing. This handles the genuine "not a repo" case cleanly and makes the diagnostic log message (rc=128) unambiguous.
  • ANSI stripping before comparison: the module-level constant _ANSI_ESCAPE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]") (compiled once at import time) covers all CSI sequences — SGR colour codes, cursor commands, and resets. It is applied via .sub("", c.out) before .strip().lower().
  • Exact match, not substring (the non-obvious choice): after stripping ANSI codes the cleaned output is compared with == "true". An earlier draft used .lower().endswith("true"), which also accepts "untrue" or "nottrue" — strings git rev-parse would never emit, but which a defensive matcher should still reject. The ANSI-strip step is the only looseness introduced; the final comparison remains a full-string equality check.
  • logger.debug diagnostics: both failure paths (non-zero exit and unexpected textual output) emit a DEBUG-level message that includes rc, out, and err. Running cz --debug commit will now surface the exact bytes returned by git rev-parse, eliminating guesswork for future environment-specific reports.

Backward compatibility

  • is_git_project() is an internal helper with no public API surface; all callers are within commitizen/commands/.
  • In the standard path (exit code 0, output "true\n"), the new code is semantically identical to the original one-liner at commitizen/git.py:316.
  • is_staging_clean() and every other helper in commitizen/git.py are untouched.
  • All five new tests plus every previously-existing is_git_project / is_staging_clean test in tests/test_git.py continue to pass.

Checklist

Was generative AI tooling used to co-author this PR?

  • Yes (please specify the tool below)

Generated-by: Claude following the guidelines

Code Changes

  • Add test cases to all the changes you introduce
  • Run uv run poe all locally to ensure this change passes linter check and tests
  • Manually test the changes (see "Steps to Test" below)
  • Update the documentation for the changes

Documentation Changes

is_git_project() is an internal helper with no user-facing documentation page. A full docstring explaining the exit-code-first logic and ANSI-strip rationale has been added to the function itself.

Expected Behavior

Scenario Outcome
Inside a git work tree, clean subprocess output ("true\n") is_git_project() returns True — unchanged
Outside any git repo (git rev-parse exits 128) is_git_project() returns False; --debug shows rc=128 and the fatal error text
Inside a git work tree, output has ANSI colour prefix (e.g., "\x1b[0mtrue\r\n") is_git_project() returns True — regression fix for #1497
Inside .git of a bare repo (git rev-parse exits 0, output "false\n") is_git_project() returns False
Output is a string containing "true" as a substring ("untrue", "nottrue") is_git_project() returns False

Steps to Test This Pull Request

git fetch fork fix/1497-better-not-a-git-project-diagnostics
git checkout fork/fix/1497-better-not-a-git-project-diagnostics

# 1. Targeted regression test.
uv run pytest tests/test_git.py -k is_git_project -v

# 2. Reproduce-the-bug-then-verify-the-fix sequence.
# Simulate the ANSI-prefixed output that Git Bash on Windows injects before "true".
python - <<'EOF'
from unittest.mock import patch
from commitizen import cmd, git

# #1497 failure mode: shell wrapper prepends an ANSI SGR reset
noisy = cmd.Command("\x1b[0mtrue\r\n", "", b"", b"", 0)
with patch("commitizen.cmd.run", return_value=noisy):
    result = git.is_git_project()
    assert result is True, f"Expected True, got {result!r}"
print("OK: ANSI-prefixed 'true' is accepted")

# Defensive check: garbage substrings ending in "true" must still be rejected
for bad in ("untrue", "nottrue", "test true false"):
    fake = cmd.Command(f"{bad}\n", "", b"", b"", 0)
    with patch("commitizen.cmd.run", return_value=fake):
        result = git.is_git_project()
        assert result is False, f"Expected False for {bad!r}, got {result!r}"
print("OK: substring-'true' strings are all rejected")
EOF

Additional Context

This fix was surfaced during the #1964 issue audit. The triage comment on #1497 identified commitizen/git.py:314–316 (master) as the likely culprit — the strict == "true" equality check fails when a shell wrapper injects ANSI colour codes into subprocess output, a documented behaviour of Git Bash on Windows when colour is enabled globally. Because the exact failure environment couldn't be reproduced locally, the new logger.debug path at both non-zero-exit and unexpected-output branches is the primary diagnostic improvement: it closes the feedback loop for any future reporter without requiring a code change.

@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.24%. Comparing base (4b93a50) to head (b03ce74).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1980   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files          61       61           
  Lines        2779     2790   +11     
=======================================
+ Hits         2730     2741   +11     
  Misses         49       49           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

The previous `is_git_project` implementation compared
`git rev-parse --is-inside-work-tree` output to the literal string
`"true"` after stripping. Shell wrappers that prepend ANSI colour
codes or extra whitespace (reported on git-bash + `uv tool install`
on Windows, commitizen-tools#1497) flipped the check to `False` even when git itself
considered the directory a valid work tree.

Trust the exit code as the primary signal: if `git rev-parse` exits
non-zero, we're not in a git context. Then use a loose
`endswith("true")` match to distinguish a work tree from a bare-repo
interior. Add `logger.debug` lines so users running with `--debug`
can see exactly what git returned when the check fails.

Closes commitizen-tools#1497

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bearomorphism
bearomorphism force-pushed the fix/1497-better-not-a-git-project-diagnostics branch from ac261a7 to ee91310 Compare May 9, 2026 12:30
@bearomorphism

Copy link
Copy Markdown
Collaborator Author

Updated per @bearomorphism's review (thanks for catching the loose match!):

Replaced c.out.strip().lower().endswith("true") with an ANSI-strip + exact-match approach:

cleaned = _ANSI_ESCAPE.sub("", c.out).strip().lower()
inside_work_tree = cleaned == "true"

Now correctly rejects untrue, nottrue, test true false, etc., while still accepting the original target case (\x1b[0mtrue\r\n). Three new parametrised tests cover the rejection cases.

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 fixes false NotAGitProjectError reports on Windows/Git Bash by making is_git_project() tolerant of ANSI escape sequences/noisy git rev-parse stdout while preserving strict "true" matching semantics.

Changes:

  • Strip ANSI CSI escape sequences from git rev-parse --is-inside-work-tree output and gate on exit code first.
  • Add DEBUG logging to help diagnose unexpected git rev-parse outputs/exit codes under cz --debug.
  • Add regression tests covering inside/outside repo, ANSI-prefixed output, bare/non-worktree output, and substring-true rejection.

Reviewed changes

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

File Description
commitizen/git.py Make is_git_project() robust to ANSI/noisy stdout and add debug diagnostics.
tests/test_git.py Add targeted regression tests for is_git_project() behaviors and edge cases.

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

Comment thread tests/test_git.py Outdated
Comment thread tests/test_git.py Outdated
* the 'not inside work tree' case covers both .git/ subdirs of normal repos AND the root of bare repos (which have no .git directory at all)

* the substring-rejection test verifies strict full-string equality after ANSI/whitespace strip, not whole-word matching

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment