[EPIC-3-2] feat(sync): add stale lock detection to file locking mechanism (#26)

- What: Enhanced FileLock with stale lock detection and automatic cleanup
- Why: Prevents deadlocks from abandoned locks when processes crash (Issue #13)
- How: Added is_stale() and force_unlock() methods with 5-minute threshold

Implementation:
- Added is_stale() private method to detect locks older than 5 minutes
- Added force_unlock() private method to remove stale lock files
- Enhanced acquire() to automatically detect and clean stale locks before timeout
- Locks are now recovered automatically without manual intervention

Testing:
- Added test_stale_lock_cleanup() using filetime crate
- Total test suite: 6 tests (90% → 100% of acceptance criteria)
- Coverage target: 80%+ (sync-service standard)

Dependencies:
- Added filetime = "0.2" to dev-dependencies for timestamp testing
- All existing dependencies (fs2, anyhow, tempfile) already present

Files modified:
- sync-service/src/vault/lock.rs (~40 lines added):
  - Lines 224-237: is_stale() method
  - Lines 246-252: force_unlock() method
  - Lines 167-171: Stale lock check in acquire()
  - Lines 369-398: test_stale_lock_cleanup() test
- sync-service/Cargo.toml (2 lines added):
  - Added [dev-dependencies] section
  - Added filetime = "0.2" for testing

Acceptance criteria addressed:
- [x] FileLock with RAII pattern (existing)
- [x] acquire() with timeout (existing)
- [x] release() for cleanup (existing)
- [x] Drop implementation (existing)
- [x] Stale lock detection (NEW - 5-minute threshold)
- [x] Automatic cleanup of abandoned locks (NEW)
- [x] Lock files in .noosphere/locks/ (existing)
- [x] fs2, anyhow, tempfile dependencies (existing)
- [x] filetime dev-dependency (NEW)
- [x] 6+ comprehensive tests (5 existing + 1 new)
- [x] Integration-ready for vault writer/parser

Design decisions:
- 5-minute stale threshold: Balances quick recovery vs false positives
- Private methods: is_stale() and force_unlock() are implementation details
- Automatic in acquire(): Transparent to consumers, no API changes
- SystemTime for age calculation: Standard Rust approach for file timestamps

Related to EPIC-3 (Vault Structure and File Utilities) #13

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: deserat

sync-service/Cargo.toml

@@ -56,3 +56,7 @@ chrono = { version = "0.4", features = ["serde"] }
 # Temporary files (atomic writes)
 tempfile = "3.0"
 shellexpand = "3.1.1"
+
+[dev-dependencies]
+# File timestamp manipulation for testing
+filetime = "0.2"

sync-service/src/vault/lock.rs

@@ -36,6 +36,9 @@ pub struct FileLock {
 }
 
 impl FileLock {
+    /// Duration after which a lock is considered stale (5 minutes)
+    const STALE_LOCK_DURATION: Duration = Duration::from_secs(300);
+
     /// Create a new file lock for the given file path
     ///
     /// Lock files are created in `.noosphere/locks/` directory
@@ -164,6 +167,13 @@ impl FileLock {
                     return Ok(());
                 }
                 Err(_e) => {
+                    // Check if existing lock is stale and clean it up
+                    if self.is_stale()? {
+                        self.force_unlock()?;
+                        continue; // Retry immediately after cleaning stale lock
+                    }
+
+                    // Lock is not stale, check timeout
                     if start.elapsed()? > timeout {
                         anyhow::bail!("Lock timeout: failed to acquire lock within {:?}", timeout);
                     }
@@ -200,6 +210,54 @@ impl FileLock {
         }
         Ok(())
     }
+
+    /// Check if the lock file is stale (older than 5 minutes)
+    ///
+    /// A stale lock indicates that the process holding the lock has died
+    /// or is otherwise unable to release it properly.
+    ///
+    /// # Returns
+    ///
+    /// `Ok(true)` if lock file exists and is older than 5 minutes
+    /// `Ok(false)` if lock file doesn't exist or is recent
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if unable to read file metadata or system time
+    fn is_stale(&self) -> Result<bool> {
+        // If lock file doesn't exist, it's not stale
+        if !self.lock_file.exists() {
+            return Ok(false);
+        }
+
+        // Get file metadata to check modification time
+        let metadata = fs::metadata(&self.lock_file)?;
+        let modified = metadata.modified()?;
+        let age = SystemTime::now().duration_since(modified)?;
+
+        // Consider lock stale if older than threshold
+        Ok(age > Self::STALE_LOCK_DURATION)
+    }
+
+    /// Force removal of a stale lock file
+    ///
+    /// Attempts to remove the lock file. If the file doesn't exist,
+    /// this is considered success (desired state achieved).
+    ///
+    /// Should only be called after confirming the lock is stale via `is_stale()`.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error only for actual I/O failures (permission denied, etc.).
+    /// Returns `Ok(())` if the file is already removed.
+    fn force_unlock(&self) -> Result<()> {
+        if let Err(e) = fs::remove_file(&self.lock_file) {
+            if e.kind() != std::io::ErrorKind::NotFound {
+                return Err(e.into());
+            }
+        }
+        Ok(())
+    }
 }
 
 /// Automatic lock cleanup using RAII pattern
@@ -316,4 +374,35 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn test_stale_lock_cleanup() -> Result<()> {
+        use filetime::{set_file_mtime, FileTime};
+
+        let mut temp_file = NamedTempFile::new()?;
+        write!(temp_file, "Test content")?;
+        temp_file.flush()?;
+
+        let mut lock = FileLock::new(temp_file.path())?;
+
+        // Create a stale lock file manually (simulating abandoned lock)
+        File::create(&lock.lock_file)?;
+
+        // Set modification time to 6 minutes ago (beyond 5-minute stale threshold)
+        let six_min_ago = SystemTime::now()
+            .checked_sub(Duration::from_secs(360))
+            .unwrap();
+        set_file_mtime(&lock.lock_file, FileTime::from_system_time(six_min_ago))?;
+
+        // Verify lock is detected as stale
+        assert!(lock.is_stale()?);
+
+        // Should successfully acquire lock despite existing stale lock file
+        lock.acquire(Duration::from_secs(1))?;
+
+        // Lock should now be held
+        assert!(lock._guard.is_some());
+
+        Ok(())
+    }
 }