`verify._git` merges stderr into stdout for every caller, so a git that warns at rc 0 corrupts every output-reading probe

[pbean]

verify._git (verify.py:145-147) returns (rc, (stdout + stderr).strip()). Every caller that reads the text rather than only the returncode therefore reads git's diagnostics as data. Git writes advisories to stderr while still exiting 0, so this is not a failure path — it is the normal path on a host whose git config the orchestrator does not control.

PR #441 (#423) fixed the two most dangerous instances — worktree_clean and the new path_tracked, both emptiness reads whose inversion is silent and load-bearing — by reading proc.stdout alone through _run_git. It deliberately did not widen to the rest of the family. This issue is that rest.

Reproduction (git 2.55.0)

$ git init -q r && cd r && git commit -qm init --allow-empty
$ git config core.fsyncMethod bogusvalue      # any unknown value; not an error
$ git rev-parse HEAD; echo "rc=$?"
77943fe624cd0aa1540d68f4571a18d8cf64f996
rc=0
# stderr, same invocation:
warning: ignoring unknown core.fsyncMethod value 'bogusvalue'

git ls-files --others --exclude-standard and git rev-list HEAD..HEAD both behave identically: empty stdout, rc 0, the warning on stderr. Other real-world producers of the same shape: a core.fsmonitor hook that cannot exec, a stale-index advisory, core.hooksPath pointing at a missing dir, and every warning: ignoring unknown <key> value for a config key a newer git dropped.

The affected readers

site	reads	corruption under a warning
rev_parse_head (verify.py:189)	return out	returns "<sha>\nwarning: …" — not a sha. Flows into same_commit() comparisons and into persisted baselines in run state, so a resume compares a warning-suffixed string against a clean one and reads "moved".
untracked_files (verify.py:393)	splits lines into a set	a phantom untracked path named warning: … on a pristine tree. Its contract is "what git clean -fd would remove", so the phantom becomes a rollback candidate.
commits_above (verify.py:468)	splits lines into a list	a phantom commit sha, handed to preserve_commits. Docstring says "Empty when HEAD is at or behind baseline" — under a warning it is never empty.
last_commit_for (verify.py:207)	return out	same as rev_parse_head; backs operator-park provenance.
current_branch (verify.py:846)	branch name	a branch name with a warning appended.
worktree list --porcelain (verify.py:939)	record parsing	an unparseable extra record.
commit_paths' status --porcelain leg (verify.py:2240)	if not out	reads non-empty on an unchanged path set, so it proceeds to git commit instead of returning None, and the commit fails with nothing to commit ⇒ GitError. Loud, not silent — listed for completeness.

verify.py:1116 is a second, inline instance of the same ls-files --others read, inside the patch-capture path: each returned rel is opened and appended to the patch, so the phantom line becomes a file that cannot be opened. That same function already reads proc.stdout alone for its git diff leg three lines above (verify.py:1113) — the fix shape is present in the file, just not applied consistently.

Shape of the fix

Same as #441's: read proc.stdout through _run_git for the value, and keep the stdout + stderr merge only on the error path, where stderr is the informative half. _git's merge is right for the "raise with a message" callers and wrong for the "parse the answer" callers; the split is per-caller, not a change to _git itself — _git_raw already exists as the precedent (verify.py:150, stdout verbatim for -z output).

Worth considering instead: a _git_out sibling returning (rc, stdout, stderr) so a reader cannot pick the merged form by accident, and leaving _git for rc-only callers. Whichever shape, all git subprocess calls must keep going through the _run_git chokepoint (AGENTS.md hard invariant).

Test angle

A fixture that sets core.fsyncMethod to an unknown value in the sandbox repo (via git config, so it survives GIT_CONFIG_NOSYSTEM) and asserts each probe still answers correctly. This makes every one of the rows above fail before the fix, which no current test does — the suite has no host-noise dimension at all.

Notes

• Pre-existing on main; not introduced by fix(verify): force literal git pathspecs, and read status stdout alone (#433, 6D) #441, which documented the family in its PR body and fixed only the two it was scoped to.
• Not a duplicate of safe_rollback matches git's English "did not match" — benign restore no-op raises GitError under a localized git #236 (locale pin) or _run_git's decode fault escapes the GitError taxonomy on -z output #377 (decode taxonomy), though both are the same chokepoint.

[coderabbitai]

🔗 Related PRs

#237 - fix(verify): force LC_ALL=C at the git chokepoint so rollback stops matching English (#236) [merged]
#341 - fix(recovery): refuse the rollback reset when the snapshot fails (#340) [merged]
#382 - fix(verify): decode verify-command output with errors="replace" (#378) [merged]
#385 - fix(install): scope the worktree git-add shield per-worktree (#384) [merged]
#387 - fix(verify): translate git's decode fault and add a bytes mode (#377) [merged]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

• Create Plan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!