Remove apply.lock when a lock-holding run is interrupted (#808)

[Mikola Lysenko (mikolalysenko)]

Why

The maintainer decided on #808 that socket-patch must never leave a lock file in the project directory. .socket/apply.lock stays transient (Option C). A persistent lock, if ever needed, goes outside the project (home/cache dir or a configurable path).

Normal exits, error exits and panics already drop LockGuard, which deletes the lock. The one path that left apply.lock (and a .socket/ the run created) behind was an interrupted run: Ctrl-C, SIGTERM or SIGHUP while apply, vendor, scan --mode vendored, rollback, repair or remove held the lock. This PR closes that path.

What changed

• Core (apply_lock.rs): every held lock is registered in a process-global table. New cleanup_held_lock_on_interrupt() unlinks each registered lock file only if the path still names the inode this process locked (same identity check as Drop), then removes .socket/ if it is now empty (Unix only, non-recursive, only a dir literally named .socket).
  • Unix: fixed slots of preallocated C strings plus dev/ino, so the cleanup only calls stat/unlink/rmdir and is async-signal-safe.
  • Windows: a mutex-guarded table with a duplicate handle used for the identity check. No rmdir, since the name stays delete-pending until exit.
  • The guard leaves the table just before its own unlink, after the durability barrier, so an interrupt during a long fsync still cleans up.
• CLI (src/interrupt.rs, wired in main.rs): Unix sigaction handlers for SIGINT/SIGTERM/SIGHUP run the cleanup, reset to SIG_DFL and re-raise, so the process still dies by the signal. Signals ignored at startup stay ignored. Windows uses SetConsoleCtrlHandler for Ctrl-C/Ctrl-Break/close and returns FALSE so the default handler still ends the process.
• Failpoint: debug-only <name>[@n]~pause form and a new apply_lock.acquired failpoint, used by the e2e tests to pause while the lock is held.
• The Node addon installs no handler, so it gets no interrupt cleanup (documented: installing the handler is the host's job).

User-visible changes

An interrupted lock-holding run no longer leaves .socket/apply.lock (or an empty .socket/) in the project. Exit codes and signal deaths are unchanged; the prompt's cursor restore still runs first. Only SIGKILL or power loss can still leave the file, and the next lock-taking command reclaims it as before.

Docs updated

• CLI_CONTRACT.md: the Lock lifecycle paragraph covers interrupts, the uncatchable-kill case, and "never keeps a persistent lock file in the project". The --dry-run row says the lock is removed on exit or on interrupt.
• Module docs in apply_lock.rs and lock_cli.rs say the same, including the small Unix window between the handler's unlink and the process dying.
• CHANGELOG.md and migrating-to-v5.md are untouched.

Tests run (macOS, debug)

• cargo test -p socket-patch-core --lib -- apply_lock failpoint group_commit: 45 passed (3 new: cleanup removes file and empty .socket/ and the later drop copes; a replaced file is left alone; a dropped guard is unregistered).
• cargo test -p socket-patch-cli --lib -- lock_cli prompt: 41 passed.
• --test e2e_safety_lock: 32 passed, including 4 new Unix tests (SIGINT, SIGTERM, SIGHUP each remove apply.lock, the process dies by the signal, the manifest survives; SIGINT ignored at startup keeps the run alive, then SIGTERM still cleans up). With interrupt::install() disabled all 4 fail.
• --test cli_remove_silent 10, --test vendor_group_commit_e2e 11, --test repair covgap_commands_repair:: 9: all passed.
• Clippy shows no new warnings in touched files.

Not run: Windows. The Windows modules were compiled for x86_64-pc-windows-gnu in a scratch crate with the same pinned deps, but the Windows tests and the console handler need Windows CI. The empty-.socket/ prune is covered by the unit test only; no lock-taking command reaches the lock in an empty project.

Review findings fixed

• LockGuard::drop unregistered before the durability barrier, so an interrupt during a long fsync left apply.lock behind. It now unregisters just before the unlink (27db2d9).
• The Unix window between the handler's unlink and process death is now documented.

Out of scope: moving journal replay and the durability barrier out of the lock stays with #809 / #793.

Closes #808

🤖 Generated with Claude Code

---

Note

Medium Risk
Changes lock lifecycle and signal/console handlers on all platforms; incorrect interrupt cleanup could unlink the wrong file or briefly overlap holders, though identity gating and new tests mitigate this.

Overview
Ensures Ctrl-C, SIGTERM/SIGHUP, and Windows console interrupts no longer leave .socket/apply.lock (or an empty .socket/) in the project — only SIGKILL or power loss can still leave the file, which the next lock-taking command reclaims as before.

Core: Held locks register in a process-global table; new cleanup_held_lock_on_interrupt() unlinks each lock with the same inode identity check as LockGuard drop, then prunes an empty .socket/ (Unix: async-signal-safe slots; Windows: mutex table + POSIX-semantics delete so the name can go before process exit). Registration stays active through the durability barrier until after unlink so interrupts during long fsync still clean up.

CLI: New interrupt module installed at startup — Unix re-raises after cleanup; Windows console ctrl handler; signals already ignored stay ignored. Interactive prompt chains SIGINT so cursor restore runs before lock cleanup.

Testing/docs: Failpoints gain ~pause (e.g. apply_lock.acquired); Unix e2e tests signal a parked apply and assert no lock residue. CLI_CONTRACT.md and module docs describe interrupt vs uncatchable-kill behavior.

^{Reviewed by Cursor Bugbot for commit 5608a7e. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] Merged main, CI green (one Maven Central warm-up flake in the maven_reactor e2e leg passed on re-run); ready for review.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 5608a7e. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review at 5608a7e.

• CI: 395/395 non-skipped checks green (ci-ok success). sbt 1.9.9 / jdk 17 / agent, mill 0.12.17 and vlt install-proof (ubuntu-latest, 1.0.0-rc.8) failed on the first attempt and passed on one re-run; the same sbt 1.9.9/jdk 17 leg and the vlt workflow also failed intermittently on recent main pushes.
• Bugbot: reviewed 5608a7e, no unresolved findings.
• Mergeable: yes.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Tanmay Singla (@Tanmay182003) — final reviewer: not enqueueing yet. One non-merge commit landed after your approval at 1881e15:

• 5608a7e Test that an interrupt inside the guard's drop still removes apply.lock — adds a debug-only apply_lock.releasing failpoint that parks apply inside LockGuard::drop just before the identity probe and unlink, plus an end-to-end test (interrupted_holder::interrupt_during_release_removes_the_lock_…) covering Bugbot's "Interrupt can skip lock unlink" gap. It's test plus a debug-only failpoint, not a behavior change for release builds, but it does touch non-test code.

CI is green on 5608a7e (ci-ok, clippy). Could you take another look at the diff since your approval? Once you re-approve, it can go straight to the merge queue.

---

Generated by Claude Code