fix(verify): force LC_ALL=C at the git chokepoint so rollback stops matching English (#236)

[pbean]

Summary

Closes #236. safe_rollback (src/bmad_loop/verify.py) restores each preserve dir from a pre-reset stash snapshot and tolerates the benign case — a preserve dir holding only untracked files — by substring-matching git's English message:

rc, out = _git(repo, "checkout", snapshot, "--", d)
if rc != 0 and "did not match" not in out:
    raise GitError(...)

_run_git spawned git with the inherited environment and no locale override, so under a localized git (e.g. LANG=it_IT.UTF-8) the benign message is translated, the substring never matches, and the benign no-op raises GitError — a resolved re-drive that should auto-recover instead lands in the rollback-pause path. Reported by @Polloinfilzato in #233 as a locale-dependent test failure; the root cause is production code and the blast radius is the rollback path.

Fix

Force LC_ALL=C at the single git spawn point, _run_git, so every git message the orchestrator inspects stays stable English — making the whole class of locale-fragile message parsing impossible rather than patching the one instance.

env={**(env if env is not None else os.environ), "LC_ALL": "C"},

• LC_ALL=C is merged last so it wins over both the inherited environment and any explicit env. The two _git_env callers (throwaway GIT_INDEX_FILE snapshot staging; synthetic commit-tree identity) keep their vars because the caller's env is spread first.
• LC_ALL=C alone is sufficient — verified that LANGUAGE=it LC_ALL=C git … still prints English (gettext disables translation entirely in the C locale, so it wins over LANGUAGE).
• Scoped to git only: the verify command runner (operator test/lint gates) is deliberately left untouched.
• verify.py:587 today is the only site in src/ that branches on English git text; the merge machinery already decides via return codes / MERGE_HEAD / git diff --quiet.

Tests

Two portable regression tests spy on subprocess.run's env rather than the parent LANG, so the guarantee is pinned even on a CI box with no non-English locale catalogs installed (a plain LANG=it_IT test would emit English anyway and pass without the fix — false green):

• test_run_git_forces_c_locale — asserts LC_ALL=C reaches every git child.
• test_run_git_locale_merge_preserves_explicit_env — asserts the merge preserves a caller-supplied env key alongside the forced LC_ALL.

The existing "did not match" tolerance tests stay green and now hold under any operator locale.

Verification

• Full suite: 2709 passed, 1 skipped.
• trunk check clean on changed files.

Refs #233.

Summary by CodeRabbit

• Bug Fixes

  • Improved rollback reliability when Git is run in a non-English system locale.
  • Git output is now consistently interpreted in English, preventing benign rollback messages from being misclassified as errors.
  • Preserved custom environment settings while applying the locale standardization.

• Tests

  • Added coverage for locale handling and environment preservation during Git operations.

• Documentation

  • Updated the changelog with details of the rollback reliability fix.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d850d26d-5c5c-4fe2-a8ab-935a793baf99

📥 Commits

Reviewing files that changed from the base of the PR and between a7fba70 and 319ee9e.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/bmad_loop/verify.py
• tests/test_verify.py

---

Walkthrough

Git subprocesses now force LC_ALL=C while preserving supplied environment variables, keeping rollback output matching stable across locales. Tests cover both behaviors, and the changelog documents the fix.

Changes

Locale-stable Git handling

Layer / File(s)	Summary
Force English Git output src/bmad_loop/verify.py	_run_git merges the existing environment with LC_ALL=C, and safe_rollback documents the stable English matching behavior.
Validate locale behavior tests/test_verify.py, CHANGELOG.md	Tests verify locale forcing and environment preservation; the changelog records the rollback fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• safe_rollback matches git's English "did not match" — benign restore no-op raises GitError under a localized git #236 — Covers the _run_git locale override and environment-merging behavior implemented here.

Possibly related PRs

• bmad-code-org/bmad-loop#174 — Both changes modify shared _run_git subprocess behavior and its tests.

Suggested reviewers: dracic

Poem

I’m a rabbit with Git in my paws,
Now every message follows clear laws.
LC_ALL=C, the words stay bright,
Rollbacks can parse them just right.
Hop, test, and merge—what a delight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: forcing LC_ALL=C for git subprocesses to keep rollback parsing stable.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/lc-all-c-git-chokepoint-236

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}