Downgrade worktree remove fallback log to INFO - #665
Conversation
There was a problem hiding this comment.
Single-line change, downgrading logger.warning → logger.info at gateway/worktree_manager.py:513.
Assessment: No issues found.
The change is correct. The fallback path at line 505 handles the git worktree remove failure deterministically via shutil.rmtree + surgical admin dir cleanup. The failure is an expected race condition (Docker unmounting /dev/null bind over .git during container teardown), not an actionable operational concern. WARNING is inappropriate for expected-and-recovered conditions; INFO is the right level.
The surrounding code is sound: ignore_errors=True on shutil.rmtree prevents secondary failures, the admin dir is resolved before the removal attempt (line 495), and the stderr from the failed git command is still captured in the INFO log for debugging if needed.
No security, correctness, or robustness concerns.
— Authored by egg
|
egg review completed. View run logs |
Summary
Context
When a container is torn down, Docker unmounts the
/dev/nullbind mount over the worktree's.gitfile. Depending on timing,git worktree removemay run before the mount is fully cleaned up, causing git to report "is not a working tree". The fallback path handles this correctly via surgical cleanup (shutil.rmtree+ admin dir removal), so the worktree is always fully removed.Since this is an expected race condition with successful recovery, WARNING is unnecessarily noisy. INFO is the appropriate level.
Issue: none
Test plan:
gateway/worktree_manager.py:513test_worktree_manager.pycover the fallback pathAuthored-by: egg