Repository navigation
fix(worktree-reaper): fail closed when /proc cannot be scanned before removal (#5740) - #6731
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 7 minutes. View limit details
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. |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Salvage note from duplicate PR #6733 (lane code-q; closed as duplicate of this PR per the duplicate-PR protocol — diffed against this merged PR before closing, not just compared by file list). Race context: lanes code-z and code-q claimed issue #5740 in fallback mode 24 seconds apart (03:29:00Z / 03:29:24Z) — the deployed fleet binary has no Diff verdict: the two implementations satisfy the same contract with different mechanics — this PR raises One unique item in the duplicate, recorded here as the salvage: end-to-end failure-injection coverage through |
Fixes #5740
The reaper's
/procscan failed open:proc_cwds()returned an empty list whenos.listdir("/proc")raised, andtree_occupied()read that as "unoccupied" — so the fresh pre-removal check inreap_treecould pass on an inspection error and delete a worktree whose occupancy was simply unknown.proc_cwds()now raisesProcScanError(anOSErrorsubclass, so it still reads as an inspection failure) instead of returning[]. A successful scan with zero matches still returns[]— the two states are now distinct all the way down.reap_treecatchesProcScanErrorat the pre-removal re-check: candidate kept with reasonoccupied-unknown-at-removal, the error recorded on the record, a WARNING printed — and nogit worktree remove, branch deletion, or path deletion runs.main()catches it at the run's initial scan: clear ERROR, summary printed, exit 1, nothing evaluated or removed.Tests (
infra/worktree-reaper/tests/test_worktree_reaper.py, the documented command in its docstring): newProcScanFailClosedTests— injectedos.listdir("/proc")failure raisesProcScanError;reap_treeunder the same injection returns False, keeps the candidate, assertsrun_gitwas never called and the tree survives; successful-empty-scan and confirmed-matching-process behaviour pinned as controls. 12/12 pass on the laptop.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.