_run_git's decode fault escapes the GitError taxonomy on -z output

[pbean]

Found while sweeping for other copies of #374. This is the same genus but at the _run_git chokepoint, which makes it strictly more severe: the exception escapes untyped, defeating the very taxonomy that function exists to provide.

The defect

verify._run_git decodes git's stdout with text=True (locale codec, errors='strict') and translates only two exception classes:

    except subprocess.TimeoutExpired as exc:
        raise GitError(...)
    except OSError as exc:
        raise GitSpawnError(...)

Its own docstring states the contract:

Two failures are raised by subprocess.run before any return code exists — a timeout (#156) and a spawn-level OSError (#343) — so left uncaught either would bypass every except GitError guard and crash the run.

A UnicodeDecodeError is a third such failure, raised inside subprocess.run before any return code exists, and it is not in the taxonomy — so it does exactly what the docstring says must not happen.

Why ordinary git commands are safe (and which are not)

Git quotes non-ASCII paths by default (core.quotePath), so git status --porcelain returns pure ASCII (?? "weird-\377-name.txt") and decodes fine. -z disables that quoting and emits raw path bytes — which is exactly what _git_raw exists for:

Like _git but returns stdout verbatim (no strip, no stderr merge) — for NUL-delimited (-z) output...

Reproduction

A repo containing one file whose name is not valid UTF-8 (POSIX filenames are arbitrary bytes):

verify._git(repo, "status", "--porcelain")     -> rc=0 ok   out='?? "weird-\\377-name.txt"'
verify._git_raw(repo, "status", "--porcelain", "-z")
    -> RAISED UnicodeDecodeError -> 'utf-8' codec can't decode byte 0xff in position 9
       is GitError? False

Affected call sites

All three -z invocations:

line	call	reached from
verify.py:779	status --porcelain -z -uall in dirty_paths	clean_incoming_collisions (merge pre-flight)
verify.py:804	diff --name-only -z in branch_incoming_paths	clean_incoming_collisions
verify.py:2013	ls-files -z in commit_paths	decisions.py:161, cli.py:1859

clean_incoming_collisions raises GitError deliberately when it finds stray paths, so its callers guard on GitError — and this fault sails straight past that guard.

Suggested fix

Add the decode fault to the existing taxonomy rather than trying to make non-UTF-8 paths work:

    except UnicodeDecodeError as exc:
        raise GitError(f"git {cmd[3]} returned undecodable output in {repo}: {exc}") from exc

That is minimal, keeps CompletedProcess[str], and matches the module's documented design — observation guards degrade, unguarded paths fail typed. Making such paths actually work (errors="surrogateescape") is a larger call, because surrogates would then flow into journal/JSON writes that are themselves UTF-8; that deserves its own decision.

Pre-existing; not introduced by #373 or #376.

[coderabbitai]

🔗 Related PRs

#174 - fix(verify): git timeouts become GitError; rollback dirty check degrades instead of crashing the run (#156) [merged]
#182 - fix(worktree): degrade post-merge teardown failures instead of crashing the run (#139) [merged]
#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]
#344 - fix(verify): translate spawn OSError to GitSpawnError in _run_git (#343) [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!

[pbean]

Sweeping the rest of the codebase for this genus turned up two more _run_git call sites, both reproduced. Both share this issue's root cause and its fix, so recording them here rather than filing separately.

capture_diff (verify.py:931, :959) — highest blast radius

git diff <baseline> -- emits file content verbatim, not just paths, so core.quotePath gives no protection at all. An ordinary latin-1 file is enough:

data.csv = b"name;city\nJos\xe9;Malm\xf6\n"
git diff, text=True -> UnicodeDecodeError: 'utf-8' codec can't decode byte 0xe9

This matters more than the -z sites because capture_diff is the forensic capture for a FAILED unit — the only surviving copy of dropped work. Its caller guards except verify.GitError (workspace.py:187), and because the raise is not a GitError, capture_err is never set — so the on_teardown_degraded "worktree and branch preserved (uncaptured changes)" valve, whose whole purpose is to stop teardown from destroying uncaptured work, is bypassed.

worktree_list (verify.py:759) — unguarded caller

git worktree list --porcelain does not quote the path. Verified against a worktree whose directory name carries byte 0xff:

raw bytes contain 0xff? True
text=True -> UnicodeDecodeError: 'utf-8' codec can't decode byte 0xff in position 127

It escapes runs.py:583 (except verify.GitError) → runs.py:612 → cli.py:179 _reconcile_stale, which is unguarded and runs at run and sweep start (cli.py:915, :1147) with auto_clean_on_finish defaulting to True — a traceback before the engine starts.

Note on the safe sites

status --porcelain and ls-files --others without -z are safe only because git C-quotes them to ASCII. That is a property of git's defaults, not of our code — adding -z to one of them later silently reintroduces this. Worth a test pinning it.

Also worth noting the fix choice: errors="surrogateescape" would round-trip correctly for dirty_paths, whose strings are fed straight back to git checkout -- / unlink. Translating to GitError is the smaller change and makes the existing ~50 guards start working; making the paths actually usable is the larger one.

[pbean]

Heads-up for whoever takes this: install.py's raw-git surface grew while #384 was being fixed, and the new code is a deliberate customer of the bytes-mode chokepoint this issue would add.

#385 introduces install._shield_git, which runs git -C <path> … capturing bytes and never check=True. Its docstring names this issue as the blocker for routing it through verify._run_git:

Deliberately NOT routed through verify._run_git, the chokepoint AGENTS.md otherwise requires. Nothing structural forbids it — neither module imports the other — but _run_git hands back CompletedProcess[str], i.e. the strict decode this helper exists to avoid, and it raises GitError rather than returning the rc the shield reads as an answer. Adopting it needs a bytes mode on the chokepoint first; that is #377 […]. The timeout the chokepoint would bring is carried here meanwhile, so standing outside it no longer costs one.

Two properties any absorption has to preserve, both load-bearing rather than incidental:

1. Bytes, not strict decode — the same genus as this issue. A repo path carrying bytes invalid in the locale encoding must not raise UnicodeDecodeError out of the git-query arm, whose whole contract is "skip silently"; that type is in neither arm's exception tuple (_worktree_local_exclude's git-query arm can still raise UnicodeDecodeError #374). Callers os.fsdecode at the point of use.
2. rc is an answer, not a fault — the shield asks git questions whose "no" is a non-zero rc: config --get of an unset key exits 1, and that is what tells it to enable the extension or probe the XDG default. A chokepoint that raises GitError on non-zero cannot serve these call sites; the bytes mode needs a "return the CompletedProcess" shape, not just a different codec.

Call-site count as of #386 (the validate follow-up, which adds the last one): 8 _shield_git(...) calls in install.py — the extensions.worktreeConfig probe, the core.bare / core.worktree refusal checks, the extension enable, the core.excludesFile read, the rev-parse --absolute-git-dir --git-common-dir pair, the --worktree core.excludesFile activation, and rev-parse --git-path info/exclude in legacy_exclude_pollution.

_shield_git already carries the 120s timeout the chokepoint would bring (_SHIELD_GIT_TIMEOUT_S, mirroring LimitsPolicy.git_timeout_s's default rather than reading it — install holds no Policy), so standing outside _run_git currently costs the taxonomy and LC_ALL=C, not the timeout.