install._shield_git is a bare subprocess.run git spawn, which AGENTS.md's chokepoint invariant
otherwise forbids ("All git subprocess calls go through the _run_git chokepoint in verify.py").
It stands outside deliberately, and its docstring names the reason: _run_git returned
CompletedProcess[str] and raised on a non-zero rc, neither of which the shield can use.
#377 removed that blocker. verify.git_bytes now returns CompletedProcess[bytes] with the rc
intact, inheriting the LC_ALL=C pin (#236) and the _git_timeout_s bound (#156). The shield's two
stated requirements are both met.
Why this was not folded into #385
Absorption is not a pure swap — the exception guards move underneath it:
So each of the 8 call sites needs its guard re-derived from what it actually wants to tolerate,
which is review work of its own rather than a mechanical substitution.
Scope
- Replace
_shield_git with verify.git_bytes; install currently imports no verify, so this
adds that edge (nothing structural forbids it — the docstring already says so).
- Re-derive the exception guards at all 8 sites; add
GitError where the intent is "tolerate git
being unavailable".
_SHIELD_GIT_TIMEOUT_S becomes dead once the chokepoint's limits.git_timeout_s bound applies —
note that this is a behavior change: the shield would start honouring the operator's configured
timeout instead of a hardcoded 120s mirror.- A test that the shield's git calls carry
LC_ALL=C, which standing outside the chokepoint costs
today.
Follow-up to #384 / #385 and #377.
install._shield_gitis a baresubprocess.rungit spawn, which AGENTS.md's chokepoint invariantotherwise forbids ("All git subprocess calls go through the
_run_gitchokepoint inverify.py").It stands outside deliberately, and its docstring names the reason:
_run_gitreturnedCompletedProcess[str]and raised on a non-zero rc, neither of which the shield can use.#377 removed that blocker.
verify.git_bytesnow returnsCompletedProcess[bytes]with the rcintact, inheriting the
LC_ALL=Cpin (#236) and the_git_timeout_sbound (#156). The shield's twostated requirements are both met.
Why this was not folded into #385
Absorption is not a pure swap — the exception guards move underneath it:
install.py:933guardsexcept (subprocess.SubprocessError, OSError)around therev-parse --absolute-git-dir --git-common-dirpair. Throughgit_bytes, a timeout arrives asGitError,which is not a
SubprocessError, so that guard would silently stop catching timeouts — theexact class of hole _run_git translates only TimeoutExpired, so a spawn-level OSError bypasses every except GitError guard #343 and _run_git's decode fault escapes the GitError taxonomy on -z output #377 were both about.
_shield_inherited_excludes(install.py:820) and_worktree_local_exclude's tail (install.py:1008)carry the same tuple shape and need the same review.
GitSpawnErroris aGitError, not anOSError, so theOSErrorhalf of each tuple stopscovering spawn faults too.
So each of the 8 call sites needs its guard re-derived from what it actually wants to tolerate,
which is review work of its own rather than a mechanical substitution.
Scope
_shield_gitwithverify.git_bytes;installcurrently imports noverify, so thisadds that edge (nothing structural forbids it — the docstring already says so).
GitErrorwhere the intent is "tolerate gitbeing unavailable".
_SHIELD_GIT_TIMEOUT_Sbecomes dead once the chokepoint'slimits.git_timeout_sbound applies —note that this is a behavior change: the shield would start honouring the operator's configured
timeout instead of a hardcoded 120s mirror.
LC_ALL=C, which standing outside the chokepoint coststoday.
Follow-up to #384 / #385 and #377.