fix(checkpoints): recover stale locks in diff

[trac3r00]

Summary

• route checkpoint diff() git-add staging through the same stale index-lock recovery helper as checkpoint capture
• recover only matching store/indexes/.lock failures after 5 minutes and a positive no-open-holder lsof check
• quarantine stale locks with an atomic rename before unlink so a newly-created live lock at the original path is preserved

Test Plan

• python -m pytest tests/tools/test_checkpoint_manager.py::TestCheckpointIndexLocks -q -o 'addopts='
• python -m pytest tests/tools/test_checkpoint_manager.py -q -o 'addopts='
• git diff --check
• python -m py_compile tools/checkpoint_manager.py tests/tools/test_checkpoint_manager.py

[alt-glitch]

This was generated by AI during triage.

Related to #13232 and #52887: this covers the same stale checkpoint index-lock family, but uses matching-error recovery, an lsof holder check, and quarantine rename for the current per-project index path. The competing safety policy needs maintainer selection rather than a duplicate designation.

[trac3r00]

Thanks for the triage note. To help with the maintainer decision, here's how this PR differs from #13232 and #52887 — happy to close in favor of either if the maintainers prefer, but the recovery strategy here is meaningfully more conservative:

1. Targeted error match, not a blanket sweep. fix(checkpoint): self-heal stale index.lock from crashed git process #13232/fix(checkpoint): self-heal stale index.lock from crashed git process (salvage of #13232 by @thapecroth) #52887 unconditionally remove any index.lock older than 60s before every git call. This PR only recovers when git add has actually failed and the stderr matches this project's specific <index_file>.lock: ... Unable to create ... File exists pattern — it never touches a lock speculatively.
2. Open-process check via lsof, not just age. Age alone can't tell a genuinely stuck lock from a slow-but-alive git process (e.g. under IO pressure). This PR additionally requires lsof to confirm no process currently holds the lock file; if lsof is unavailable or ambiguous, it refuses to remove the lock rather than guessing.
3. TOCTOU-safe removal. Instead of unlink()ing the lock path directly (which can race a freshly-created live lock), this PR renames it to a quarantine path first and only unlinks the quarantine copy — so a new lock created between the check and the removal is never clobbered.
4. Covers diff() in addition to _take(). The shared helper (_git_add_recover_stale_index_lock) is wired into both the checkpoint-capture path and the diff path, since diff() also stages via git add -A and was hitting the same wedge.
5. Stale threshold is 5 minutes (vs. 60s), which is more conservative given shadow-repo git ops can occasionally take longer than the sub-second common case under load.

If the maintainers decide #13232/#52887 should land first, this PR's recovery-safety additions (lsof holder check + quarantine rename + diff() coverage) could still be layered on top as a follow-up rather than treated as a pure duplicate. Let me know which direction you'd like to take and I'll adjust.

[bob]

[teknium1]

Thanks for targeting the real per-project index-lock failure. Current main still stages directly through git add -A in tools/checkpoint_manager.py:859-860 and :1048-1051, with GIT_INDEX_FILE bound to store/indexes/<hash> at :269-270.

Problems

• PR tools/checkpoint_manager.py:456-471 checks lsof before renaming. A live Git process can create or replace the lock after that check and before lock_path.rename() at :465; the replacement is then moved to quarantine and unlinked. The claimed TOCTOU guarantee does not cover that interval.
• tests/tools/test_checkpoint_manager.py:1139-1148 creates a fresh lock only during unlink, after the rename. It does not test replacement before the rename.

Suggested changes

• Use an ownership/identity protocol that proves the quarantined file is the observed stale lock before deleting it, and add a test for replacement immediately before rename.
• Exercise the real diff() or _take() Git path with a planted stale per-project lock; the helper test currently mocks _run_git() at tests/tools/test_checkpoint_manager.py:1108-1131.
• Define and test behavior when lsof is unavailable; the helper intentionally declines recovery at PR tools/checkpoint_manager.py:398-399.

Automated hermes-sweeper review.

[teknium1]

The lsof result was obtained before this rename. A new Git process can create or replace lock_path in that interval; this rename then moves the fresh live lock to quarantine and line 471 deletes it. Please use an ownership/identity check that proves the quarantined inode is the stale lock, and add a replacement-race test.

[teknium1]

This test creates the fresh lock during quarantine unlink, after lock_path.rename() has completed. Add coverage that replaces the lock immediately before rename; that is the unsafe interval between the lsof check and the rename.