feat(cli): add lock-management surface (unlock subcommand, --lock-timeout, --break-lock)

Operators now have first-class, JSON-aware recovery for the
`<.socket>/apply.lock` advisory lock used by mutating subcommands.

  - `socket-patch unlock` — probe lock state. Exits 0 when free, 1 when
    held. `--release` deletes a free leftover lock file; refuses when
    held (use --break-lock on the mutating subcommand for that scenario).
  - `--lock-timeout=<secs>` — on apply/rollback/repair/remove, waits up
    to N seconds for the lock to free before reporting `lock_held`.
    Plumbs through to the existing `acquire(dir, Duration)` API.
  - `--break-lock` — on apply/rollback/repair/remove, removes the lock
    file before acquisition. Records a `lock_broken` warning event in
    the JSON envelope (and `warnings[]` for rollback's ad-hoc shape) so
    the action is auditable. Stderr also notes it in human mode.

Replaces the prior "rm <.socket>/apply.lock and retry" stderr hint with
a pointer at the new tools. Adds unit + integration tests covering each
new flag and subcommand against held / free / leftover-file scenarios.

Assisted-by: Claude Code:claude-opus-4-7

Author: mikolalysenko

crates/socket-patch-cli/src/args.rs

@@ -146,6 +146,26 @@ pub struct GlobalArgs {
     )]
     pub yes: bool,
 
+    /// Seconds to wait for `<.socket>/apply.lock` before giving up.
+    /// Default (`None`) and `0` both mean a single non-blocking try
+    /// — failing immediately if another process holds the lock. A
+    /// positive value retries with a 100 ms backoff until the lock
+    /// frees or the budget elapses. Only meaningful for the mutating
+    /// subcommands (`apply`, `rollback`, `repair`, `remove`); other
+    /// commands accept it silently.
+    #[arg(long = "lock-timeout", env = "SOCKET_LOCK_TIMEOUT")]
+    pub lock_timeout: Option<u64>,
+
+    /// Force-remove `<.socket>/apply.lock` before attempting
+    /// acquisition. Use when you are certain no other socket-patch
+    /// process is running (e.g. a previous run crashed in a way that
+    /// stripped the OS lock but left the file). Emits a
+    /// `lock_broken` warning event in the JSON envelope so the
+    /// action is auditable. Only meaningful for mutating
+    /// subcommands; other commands accept it silently.
+    #[arg(long = "break-lock", env = "SOCKET_BREAK_LOCK", default_value_t = false)]
+    pub break_lock: bool,
+
     /// Emit verbose debug logs to stderr.
     #[arg(long = "debug", env = "SOCKET_DEBUG", default_value_t = false)]
     pub debug: bool,
@@ -235,6 +255,8 @@ impl Default for GlobalArgs {
             silent: false,
             dry_run: false,
             yes: false,
+            lock_timeout: None,
+            break_lock: false,
             debug: false,
             no_telemetry: false,
         }

crates/socket-patch-cli/src/commands/apply.rs

@@ -12,11 +12,12 @@ use socket_patch_core::patch::apply::{
     apply_package_patch, verify_file_patch, ApplyResult, PatchSources, VerifyStatus,
 };
 
-use crate::commands::lock_cli::acquire_or_emit;
+use crate::commands::lock_cli::{acquire_or_emit, lock_broken_event};
 use socket_patch_core::utils::purl::strip_purl_qualifiers;
 use socket_patch_core::utils::telemetry::{track_patch_applied, track_patch_apply_failed};
 use std::collections::{HashMap, HashSet};
 use std::path::{Path, PathBuf};
+use std::time::Duration;
 use tempfile::TempDir;
 
 use crate::args::{apply_env_toggles, GlobalArgs};
@@ -167,16 +168,20 @@ pub async fn run(args: ApplyArgs) -> i32 {
     // `.socket/` directory. The guard releases on function return; see
     // `socket_patch_core::patch::apply_lock`.
     let socket_dir = manifest_path.parent().unwrap_or(Path::new("."));
-    let _lock = match acquire_or_emit(
+    let acquired = match acquire_or_emit(
         socket_dir,
         Command::Apply,
         args.common.json,
         args.common.silent,
         args.common.dry_run,
+        Duration::from_secs(args.common.lock_timeout.unwrap_or(0)),
+        args.common.break_lock,
     ) {
-        Ok(guard) => guard,
+        Ok(acquired) => acquired,
         Err(code) => return code,
     };
+    let _lock = acquired.guard;
+    let lock_was_broken = acquired.broke_lock;
 
     // Package-manager layout detection. yarn-berry PnP keeps packages
     // inside `.yarn/cache/*.zip` and resolves them via `.pnp.cjs` —
@@ -237,6 +242,9 @@ pub async fn run(args: ApplyArgs) -> i32 {
             if args.common.json {
                 let mut env = Envelope::new(Command::Apply);
                 env.dry_run = args.common.dry_run;
+                if lock_was_broken {
+                    env.record(lock_broken_event(socket_dir));
+                }
                 for result in &results {
                     env.record(result_to_event(result, args.common.dry_run));
                     // Sidecar records live on the envelope, not on

crates/socket-patch-cli/src/commands/lock_cli.rs

@@ -16,7 +16,31 @@ use std::time::Duration;
 
 use socket_patch_core::patch::apply_lock::{acquire, LockError, LockGuard};
 
-use crate::json_envelope::{Command, Envelope, EnvelopeError};
+use crate::json_envelope::{
+    Command, Envelope, EnvelopeError, PatchAction, PatchEvent,
+};
+
+/// Stable `errorCode` tag emitted as a `Skipped` warning event when
+/// `--break-lock` actually deletes a pre-existing lock file. Exposed
+/// for downstream consumers and integration tests that pattern-match
+/// on it.
+pub const LOCK_BROKEN_CODE: &str = "lock_broken";
+
+/// Outcome of a successful lock acquisition. Callers attach a
+/// `lock_broken` event to their own envelope when [`broke_lock`] is
+/// true, so the audit trail follows the same conventions as the
+/// rest of the command's output.
+///
+/// [`broke_lock`]: LockAcquired::broke_lock
+#[derive(Debug)]
+pub struct LockAcquired {
+    pub guard: LockGuard,
+    /// True iff `--break-lock` was set AND the helper actually
+    /// removed a pre-existing `apply.lock` file before acquiring.
+    /// False when the file didn't exist (nothing to break) — the
+    /// flag was a no-op in that case so no warning is warranted.
+    pub broke_lock: bool,
+}
 
 /// Try to acquire `<socket_dir>/apply.lock` and return the guard, or
 /// emit a failure envelope and a non-zero exit code.
@@ -26,23 +50,74 @@ use crate::json_envelope::{Command, Envelope, EnvelopeError};
 /// than a generic "lock failed". `dry_run` is plumbed through to the
 /// envelope's `dry_run` field for the (rare) case where lock
 /// contention happens during a dry-run apply.
+///
+/// `timeout = Duration::ZERO` keeps the historical non-blocking
+/// try-once shape. Positive values wait with a 100 ms backoff —
+/// see `socket_patch_core::patch::apply_lock::acquire`.
+///
+/// `break_lock = true` deletes `<socket_dir>/apply.lock` before the
+/// acquire attempt. The motivating case is a crashed prior run that
+/// left the file but no OS lock. When the file exists and is
+/// successfully removed the return value's `broke_lock` is true and
+/// the caller should attach a `lock_broken` warning event to their
+/// envelope.
 pub fn acquire_or_emit(
     socket_dir: &Path,
     command: Command,
     json: bool,
     silent: bool,
     dry_run: bool,
-) -> Result<LockGuard, i32> {
-    match acquire(socket_dir, Duration::ZERO) {
-        Ok(guard) => Ok(guard),
+    timeout: Duration,
+    break_lock: bool,
+) -> Result<LockAcquired, i32> {
+    let mut broke_lock = false;
+    if break_lock {
+        let path = socket_dir.join("apply.lock");
+        match std::fs::remove_file(&path) {
+            Ok(()) => {
+                broke_lock = true;
+                if !silent && !json {
+                    eprintln!(
+                        "Warning: --break-lock removed {} before acquisition.",
+                        path.display()
+                    );
+                }
+            }
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
+                // No file to break — silently proceed to the normal
+                // acquire path. Documented as a no-op so scripts can
+                // pass --break-lock unconditionally on retry.
+            }
+            Err(source) => {
+                let msg = format!(
+                    "failed to remove lock file at {}: {}",
+                    path.display(),
+                    source
+                );
+                emit(command, json, silent, dry_run, "lock_break_failed", &msg, None);
+                return Err(1);
+            }
+        }
+    }
+
+    match acquire(socket_dir, timeout) {
+        Ok(guard) => Ok(LockAcquired { guard, broke_lock }),
         Err(LockError::Held) => {
+            let msg = if timeout > Duration::ZERO {
+                format!(
+                    "another socket-patch process is operating in this directory (waited {}s)",
+                    timeout.as_secs()
+                )
+            } else {
+                "another socket-patch process is operating in this directory".to_string()
+            };
             emit(
                 command,
                 json,
                 silent,
                 dry_run,
                 "lock_held",
-                "another socket-patch process is operating in this directory",
+                &msg,
                 Some(socket_dir),
             );
             Err(1)
@@ -55,6 +130,27 @@ pub fn acquire_or_emit(
     }
 }
 
+/// Build the warning event that callers attach to their envelope
+/// when [`LockAcquired::broke_lock`] is true. Artifact-level (no
+/// PURL) since the action targets the `.socket/` directory itself,
+/// not a specific package.
+pub fn lock_broken_event(socket_dir: &Path) -> PatchEvent {
+    PatchEvent::artifact(PatchAction::Skipped).with_reason(
+        LOCK_BROKEN_CODE,
+        format!(
+            "--break-lock removed {}/apply.lock before acquisition",
+            socket_dir.display()
+        ),
+    )
+}
+
+/// Convenience: record the `lock_broken` warning event on an
+/// envelope. Mirrors the inline pattern at each call site so we
+/// don't drift on the action / errorCode pair.
+pub fn record_lock_broken(env: &mut Envelope, socket_dir: &Path) {
+    env.record(lock_broken_event(socket_dir));
+}
+
 fn emit(
     command: Command,
     json: bool,
@@ -71,10 +167,9 @@ fn emit(
         println!("{}", env.to_pretty_json());
     } else if !silent {
         eprintln!("Error: {message}.");
-        if let Some(dir) = hint_dir {
+        if hint_dir.is_some() {
             eprintln!(
-                "  If you are sure no other process is running, remove {}/apply.lock and retry.",
-                dir.display()
+                "  Run `socket-patch unlock` to inspect, or rerun with --break-lock if you're sure no holder exists."
             );
         }
     }
@@ -87,17 +182,43 @@ mod tests {
     #[test]
     fn acquire_or_emit_succeeds_on_fresh_dir() {
         let dir = tempfile::tempdir().unwrap();
-        let guard = acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap();
-        drop(guard);
+        let acquired = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            false,
+        )
+        .unwrap();
+        assert!(!acquired.broke_lock);
+        drop(acquired.guard);
     }
 
     #[test]
     fn acquire_or_emit_returns_one_on_contention() {
         let dir = tempfile::tempdir().unwrap();
-        let _first =
-            acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap();
-        let code =
-            acquire_or_emit(dir.path(), Command::Apply, false, true, false).unwrap_err();
+        let _first = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            false,
+        )
+        .unwrap();
+        let code = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            false,
+        )
+        .unwrap_err();
         assert_eq!(code, 1);
     }
 
@@ -110,8 +231,111 @@ mod tests {
             false,
             true,
             false,
+            Duration::ZERO,
+            false,
+        )
+        .unwrap_err();
+        assert_eq!(code, 1);
+    }
+
+    /// Positive timeout waits then errors `lock_held` — confirms the
+    /// budget is plumbed through to `acquire`. Mirrors the
+    /// `apply_lock::tests::timeout_held` shape so a regression in
+    /// either layer surfaces here.
+    #[test]
+    fn acquire_or_emit_honors_lock_timeout() {
+        let dir = tempfile::tempdir().unwrap();
+        let _first = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            false,
+        )
+        .unwrap();
+        let start = std::time::Instant::now();
+        let code = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::from_millis(250),
+            false,
         )
         .unwrap_err();
+        let elapsed = start.elapsed();
         assert_eq!(code, 1);
+        assert!(
+            elapsed >= Duration::from_millis(200),
+            "expected at least 200ms wait, got {:?}",
+            elapsed
+        );
+    }
+
+    /// `break_lock=true` against a pre-existing lock file with no
+    /// holder removes the file and acquires fresh. `broke_lock` flag
+    /// surfaces so callers can attach the warning event.
+    #[test]
+    fn acquire_or_emit_break_lock_removes_and_acquires() {
+        let dir = tempfile::tempdir().unwrap();
+        // Pre-stage a lock file with no holder — simulates the
+        // post-crash leftover scenario.
+        std::fs::write(dir.path().join("apply.lock"), b"").unwrap();
+
+        let acquired = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            true,
+        )
+        .unwrap();
+        assert!(
+            acquired.broke_lock,
+            "broke_lock should be true when a lock file existed and was removed"
+        );
+        // Lock file has been re-created by `acquire` and we hold it.
+        assert!(dir.path().join("apply.lock").is_file());
+    }
+
+    /// `break_lock=true` on a clean directory (no lock file) is a
+    /// no-op for the warning surface — `broke_lock` stays false so
+    /// callers don't emit a spurious event.
+    #[test]
+    fn acquire_or_emit_break_lock_is_noop_when_no_file() {
+        let dir = tempfile::tempdir().unwrap();
+        let acquired = acquire_or_emit(
+            dir.path(),
+            Command::Apply,
+            false,
+            true,
+            false,
+            Duration::ZERO,
+            true,
+        )
+        .unwrap();
+        assert!(
+            !acquired.broke_lock,
+            "broke_lock should be false when there was nothing to remove"
+        );
+    }
+
+    #[test]
+    fn lock_broken_event_uses_documented_code() {
+        let dir = tempfile::tempdir().unwrap();
+        let event = lock_broken_event(dir.path());
+        let v: serde_json::Value =
+            serde_json::from_str(&serde_json::to_string(&event).unwrap()).unwrap();
+        assert_eq!(v["action"], "skipped");
+        assert_eq!(v["errorCode"], LOCK_BROKEN_CODE);
+        assert!(
+            v.as_object().unwrap().get("purl").is_none(),
+            "lock_broken is an artifact-level event — no purl"
+        );
     }
 }