_shield_undo_extension's docstring promises it "never raises, because every caller is already reporting a degrade and this exists to say what it could not undo". Round 9 (#384, CodeRabbit CR-3) made that true for the sibling-worktree scan by widening its except to (OSError, RuntimeError, UnicodeError). The try one block down was not given the same treatment, and it decodes git's stderr:
try:
undone = git_bytes(worktree, "config", "--unset-all", "extensions.worktreeConfig")
if undone.returncode in (0, 5):
return ""
detail = os.fsdecode(undone.stderr).strip() or f"git exited {undone.returncode}"
except GitError as e:
detail = str(e)os.fsdecode is guarded for GitError only, so a codec fault from it escapes the function.
Measured, not reasoned. With os.fsdecode made to fail the way a lone invalid byte does on Windows, and the unset made to return a non-zero rc so the decode is reached, the exception escapes _shield_undo_extension:
!! ESCAPED a function whose docstring says it never raises:
UnicodeDecodeError 'utf-8' codec can't decode byte 0x62 in position 0
Impact is bounded and Windows-only. POSIX os.fsdecode uses surrogateescape and does not raise on decode; Windows uses utf-8/surrogatepass, which the repo already treats as a real fault shape — _worktree_local_exclude's tail carries UnicodeError for exactly that reason, and that tail does catch this. So it is not a crash. The cost is the same one CR-3 named: the escape replaces both the original activation-or-enable fault and the retained-flag disclosure with the tail's generic reason, losing precisely what rounds 6, 7 and 10 added.
Same class as CR-3, at the sibling block — the recurring shape of #384's review history, which is a taxonomy fixed at one site and left unfixed at its siblings.
Fix shape: widen the second except to match the first, i.e. except (GitError, UnicodeError). OSError/RuntimeError are not needed there (nothing in that block resolves paths), so this is not a copy of the other tuple.
Testability note: a regression test cannot use real bytes on POSIX, since surrogateescape never raises — it has to inject the decode fault, the way round 9's CR-3 test injects the resolve() fault. Ablation: restore the bare except GitError and confirm the test fails.
Found while auditing the guarantee wording published in d31c2b1 (Codex round 10 on #385) against the paths that can still violate it. Pre-existing rather than introduced there, and deliberately left out of that pass to keep it scoped to the two reported findings.
_shield_undo_extension's docstring promises it "never raises, because every caller is already reporting a degrade and this exists to say what it could not undo". Round 9 (#384, CodeRabbit CR-3) made that true for the sibling-worktree scan by widening itsexceptto(OSError, RuntimeError, UnicodeError). Thetryone block down was not given the same treatment, and it decodes git's stderr:os.fsdecodeis guarded forGitErroronly, so a codec fault from it escapes the function.Measured, not reasoned. With
os.fsdecodemade to fail the way a lone invalid byte does on Windows, and the unset made to return a non-zero rc so the decode is reached, the exception escapes_shield_undo_extension:Impact is bounded and Windows-only. POSIX
os.fsdecodeusessurrogateescapeand does not raise on decode; Windows uses utf-8/surrogatepass, which the repo already treats as a real fault shape —_worktree_local_exclude's tail carriesUnicodeErrorfor exactly that reason, and that tail does catch this. So it is not a crash. The cost is the same one CR-3 named: the escape replaces both the original activation-or-enable fault and the retained-flag disclosure with the tail's generic reason, losing precisely what rounds 6, 7 and 10 added.Same class as CR-3, at the sibling block — the recurring shape of #384's review history, which is a taxonomy fixed at one site and left unfixed at its siblings.
Fix shape: widen the second
exceptto match the first, i.e.except (GitError, UnicodeError).OSError/RuntimeErrorare not needed there (nothing in that block resolves paths), so this is not a copy of the other tuple.Testability note: a regression test cannot use real bytes on POSIX, since
surrogateescapenever raises — it has to inject the decode fault, the way round 9's CR-3 test injects theresolve()fault. Ablation: restore the bareexcept GitErrorand confirm the test fails.Found while auditing the guarantee wording published in d31c2b1 (Codex round 10 on #385) against the paths that can still violate it. Pre-existing rather than introduced there, and deliberately left out of that pass to keep it scoped to the two reported findings.