Merge pull request #7 from jpicklyk/feature/enable-locking-system-update-task-tool

feat: implement atomic locking with mutex-based synchronization

Author: jpicklyk

README.md

@@ -23,6 +23,7 @@ A Kotlin implementation of the Model Context Protocol (MCP) server for comprehen
 - **📋 Template-Driven**: 9 built-in templates for consistent documentation
 - **🔄 Workflow Automation**: 5 comprehensive workflow prompts
 - **🔗 Rich Relationships**: Task dependencies with cycle detection
+- **🔒 Concurrent Access Protection**: Built-in sub-agent collision prevention
 - **⚡ 37 MCP Tools**: Complete task orchestration API
 
 ## Quick Start (2 Minutes)

build.gradle.kts

@@ -7,7 +7,7 @@ plugins {
 // Define semantic version components (manually maintained)
 val majorVersion = "1"
 val minorVersion = "0"
-val patchVersion = "1"
+val patchVersion = "2"
 
 // Define release qualifier (empty for stable releases)
 // Examples: "", "alpha-01", "beta-02", "rc-01"

docs/api-reference.md

@@ -52,6 +52,7 @@ Modify existing tasks with validation and relationship preservation
 - Complexity refinement
 - Tag management
 - Relationship preservation
+- **Concurrent access protection**: Prevents conflicts when multiple agents work in parallel
 
 ### `get_task`
 Retrieve individual task details with optional related entity inclusion
@@ -76,6 +77,7 @@ Remove tasks with proper dependency cleanup
 - Soft delete support
 - Section cleanup
 - Relationship validation
+- **Concurrent access protection**: Prevents conflicts during deletion operations
 
 ### `search_tasks`
 Find tasks using flexible filtering by status, priority, tags, and text queries
@@ -135,6 +137,7 @@ Modify existing features while preserving task relationships
 - Priority adjustments
 - Task relationship preservation
 - Tag management
+- **Concurrent access protection**: Prevents conflicts when multiple agents work in parallel
 
 ### `get_feature`
 Retrieve feature details with optional task listings and progressive loading
@@ -159,6 +162,7 @@ Remove features with configurable task handling (cascade or orphan)
 - Force deletion override
 - Section cleanup
 - Relationship validation
+- **Concurrent access protection**: Prevents conflicts during deletion operations
 
 ### `search_features`
 Find features using comprehensive filtering and text search capabilities
@@ -213,6 +217,7 @@ Modify existing projects with relationship preservation and validation
 - Priority adjustments
 - Relationship preservation
 - Tag management
+- **Concurrent access protection**: Prevents conflicts when multiple agents work in parallel
 
 ### `delete_project`
 Remove projects with configurable cascade behavior for contained entities
@@ -225,6 +230,7 @@ Remove projects with configurable cascade behavior for contained entities
 - Hard delete vs soft delete
 - Comprehensive cleanup
 - Relationship validation
+- **Concurrent access protection**: Prevents conflicts during deletion operations
 
 ### `search_projects`
 Find projects using advanced filtering, tagging, and full-text search capabilities
@@ -597,4 +603,32 @@ Use bulk operations when possible:
 - Apply workflow prompts for structured process guidance
 - Combine multiple tools in logical sequences for complex operations
 
-This comprehensive API provides all the tools needed for sophisticated project management workflows while maintaining context efficiency and supporting AI-driven automation.
+## Concurrent Access Protection
+
+The MCP Task Orchestrator includes built-in protection against sub-agent collisions when multiple AI agents work in parallel. The locking system automatically prevents conflicts on update and delete operations for projects, features, and tasks.
+
+### How It Works
+
+- **Automatic Protection**: No additional configuration needed - protection is built into the tools
+- **Conflict Detection**: Operations check for conflicts before proceeding
+- **Clear Error Messages**: Blocked operations receive descriptive error responses
+- **Timeout Protection**: Operations automatically expire after 2 minutes to prevent deadlocks from crashed agents
+
+### Protected Operations
+
+The following tools include concurrent access protection:
+- `update_task` and `delete_task`
+- `update_feature` and `delete_feature` 
+- `update_project` and `delete_project`
+
+### Handling Conflicts
+
+When a conflict is detected, the tool returns an error response indicating that another operation is currently active on the same entity. The recommended approach is to wait briefly and retry the operation.
+
+### Best Practices
+
+- **Parallel Workflows**: Multiple agents can safely work on different entities simultaneously
+- **Retry Logic**: Implement simple retry logic for conflict scenarios
+- **Entity Separation**: Distribute work across different projects, features, or tasks to minimize conflicts
+
+This comprehensive API provides all the tools needed for sophisticated project management workflows while maintaining context efficiency, supporting AI-driven automation, and ensuring safe parallel operation.

src/main/kotlin/io/github/jpicklyk/mcptask/application/service/SimpleLockingService.kt

@@ -1,6 +1,8 @@
 package io.github.jpicklyk.mcptask.application.service
 
 import io.github.jpicklyk.mcptask.domain.model.*
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
 import org.slf4j.LoggerFactory
 import java.time.Instant
 import java.util.*
@@ -25,6 +27,28 @@ interface SimpleLockingService {
      * Records the completion of an operation.
      */
     suspend fun recordOperationComplete(operationId: String)
+    
+    /**
+     * Atomically attempts to acquire a lock for the given operation.
+     * This combines conflict checking and operation recording in a single atomic operation
+     * to prevent TOCTOU race conditions.
+     */
+    suspend fun tryAcquireLock(operation: LockOperation): LockAcquisitionResult
+}
+
+/**
+ * Result of attempting to acquire a lock atomically.
+ */
+sealed class LockAcquisitionResult {
+    /**
+     * Lock was successfully acquired.
+     */
+    data class Success(val operationId: String) : LockAcquisitionResult()
+    
+    /**
+     * Lock acquisition failed due to conflicts.
+     */
+    data class Conflict(val conflictingOperations: List<LockOperation>) : LockAcquisitionResult()
 }
 
 /**
@@ -55,20 +79,30 @@ enum class OperationType {
 /**
  * Simple in-memory implementation of locking service.
  */
-class DefaultSimpleLockingService : SimpleLockingService {
+class DefaultSimpleLockingService(
+    /** Operation timeout in minutes - operations older than this are considered expired */
+    private val operationTimeoutMinutes: Long = 2
+) : SimpleLockingService {
 
     private val logger = LoggerFactory.getLogger(DefaultSimpleLockingService::class.java)
 
     // Track active operations in memory
     private val activeOperations = ConcurrentHashMap<String, LockOperation>()
     private val operationStartTimes = ConcurrentHashMap<String, Instant>()
 
+    // Mutex to ensure atomic lock acquisition and prevent TOCTOU race conditions
+    private val lockMutex = Mutex()
+    
     override suspend fun canProceed(operation: LockOperation): Boolean {
         logger.debug("Checking if operation '${operation.description}' can proceed")
 
+        // Clean up expired operations first (lazy cleanup)
+        cleanupExpiredOperations()
+        
         // Conflict detection logic:
         // 1. DELETE operations are blocked by any other operation on the same entity
-        // 2. Other operations are blocked by DELETE operations on the same entity
+        // 2. Other operations are blocked by DELETE operations on the same entity  
+        // 3. WRITE operations are blocked by other WRITE operations on the same entity
         val hasConflicts = activeOperations.values.any { activeOp ->
             val entityOverlap = activeOp.entityIds.intersect(operation.entityIds).isNotEmpty()
 
@@ -77,7 +111,14 @@ class DefaultSimpleLockingService : SimpleLockingService {
                 operation.operationType == OperationType.DELETE && entityOverlap -> true
                 // Other operations are blocked by DELETE operations on same entities  
                 activeOp.operationType == OperationType.DELETE && entityOverlap -> true
-                // No conflicts for other operation combinations
+                // WRITE operations are blocked by other WRITE operations on same entities
+                operation.operationType == OperationType.WRITE && activeOp.operationType == OperationType.WRITE && entityOverlap -> true
+                // CREATE operations are blocked by other CREATE operations on same entities (prevents duplicate creation)
+                operation.operationType == OperationType.CREATE && activeOp.operationType == OperationType.CREATE && entityOverlap -> true
+                // STRUCTURE_CHANGE operations are blocked by any non-READ operation on same entities
+                operation.operationType == OperationType.STRUCTURE_CHANGE && activeOp.operationType != OperationType.READ && entityOverlap -> true
+                activeOp.operationType == OperationType.STRUCTURE_CHANGE && operation.operationType != OperationType.READ && entityOverlap -> true
+                // No conflicts for other operation combinations (READ operations never conflict)
                 else -> false
             }
         }
@@ -102,6 +143,49 @@ class DefaultSimpleLockingService : SimpleLockingService {
         logger.debug("Completed operation '$operationId'")
     }
 
+    override suspend fun tryAcquireLock(operation: LockOperation): LockAcquisitionResult {
+        return lockMutex.withLock {
+            logger.debug("Attempting to acquire lock for operation '${operation.description}'")
+            
+            // Clean up expired operations first (lazy cleanup)
+            cleanupExpiredOperations()
+            
+            // Check for conflicts using the same logic as canProceed
+            val conflictingOperations = activeOperations.values.filter { activeOp ->
+                val entityOverlap = activeOp.entityIds.intersect(operation.entityIds).isNotEmpty()
+                
+                when {
+                    // DELETE operations are blocked by any operation on same entities
+                    operation.operationType == OperationType.DELETE && entityOverlap -> true
+                    // Other operations are blocked by DELETE operations on same entities  
+                    activeOp.operationType == OperationType.DELETE && entityOverlap -> true
+                    // WRITE operations are blocked by other WRITE operations on same entities
+                    operation.operationType == OperationType.WRITE && activeOp.operationType == OperationType.WRITE && entityOverlap -> true
+                    // CREATE operations are blocked by other CREATE operations on same entities (prevents duplicate creation)
+                    operation.operationType == OperationType.CREATE && activeOp.operationType == OperationType.CREATE && entityOverlap -> true
+                    // STRUCTURE_CHANGE operations are blocked by any non-READ operation on same entities
+                    operation.operationType == OperationType.STRUCTURE_CHANGE && activeOp.operationType != OperationType.READ && entityOverlap -> true
+                    activeOp.operationType == OperationType.STRUCTURE_CHANGE && operation.operationType != OperationType.READ && entityOverlap -> true
+                    // No conflicts for other operation combinations (READ operations never conflict)
+                    else -> false
+                }
+            }
+            
+            if (conflictingOperations.isNotEmpty()) {
+                logger.debug("Operation '${operation.description}' blocked due to ${conflictingOperations.size} conflicting operation(s)")
+                return@withLock LockAcquisitionResult.Conflict(conflictingOperations)
+            }
+            
+            // No conflicts found - acquire the lock atomically
+            val operationId = "op-${UUID.randomUUID()}"
+            activeOperations[operationId] = operation
+            operationStartTimes[operationId] = Instant.now()
+            
+            logger.debug("Successfully acquired lock for operation '${operation.description}' with ID '$operationId'")
+            LockAcquisitionResult.Success(operationId)
+        }
+    }
+    
     /**
      * Gets statistics about active operations.
      */
@@ -118,4 +202,31 @@ class DefaultSimpleLockingService : SimpleLockingService {
             entry.key to java.time.Duration.between(entry.value, now).toMillis()
         }
     }
+    
+    /**
+     * Cleans up operations that have exceeded the timeout duration.
+     * This prevents crashed agents from creating permanent deadlocks.
+     */
+    private fun cleanupExpiredOperations(): Int {
+        val now = Instant.now()
+        val timeoutThreshold = now.minusSeconds(operationTimeoutMinutes * 60)
+        
+        val expiredOperations = operationStartTimes.filter { (_, startTime) ->
+            startTime.isBefore(timeoutThreshold)
+        }
+        
+        expiredOperations.keys.forEach { operationId ->
+            activeOperations.remove(operationId)
+            operationStartTimes.remove(operationId)
+            logger.info("Cleaned up expired operation '$operationId' (timeout: ${operationTimeoutMinutes} minutes)")
+        }
+        
+        return expiredOperations.size
+    }
+    
+    /**
+     * Manually triggers cleanup of expired operations and returns count of cleaned operations.
+     * Useful for monitoring and diagnostics.
+     */
+    fun forceCleanupExpiredOperations(): Int = cleanupExpiredOperations()
 }

src/main/kotlin/io/github/jpicklyk/mcptask/application/tools/base/SimpleLockAwareToolDefinition.kt

@@ -164,57 +164,78 @@ abstract class SimpleLockAwareToolDefinition(
     }
 
     /**
-     * Checks if the operation can proceed and handles any lock conflicts gracefully.
+     * Executes an operation with proper locking lifecycle management.
+     * This ensures operations are recorded, conflicts are detected, and cleanup happens.
      */
-    protected suspend fun checkOperationPermissions(
+    protected suspend fun executeWithLocking(
         operationName: String,
         entityType: io.github.jpicklyk.mcptask.domain.model.EntityType,
-        entityId: UUID
-    ): JsonElement? {
-        // For Phase 2, this just logs the operation intent
-        // In Phase 3, this will perform actual lock checking
+        entityId: UUID,
+        operation: suspend () -> JsonElement
+    ): JsonElement {
+        // If locking is disabled, execute directly
+        if (!shouldUseLocking() || lockingService == null || sessionManager == null) {
+            return operation()
+        }
 
-        lockingService?.let { service ->
-            sessionManager?.let { session ->
-                val operation = io.github.jpicklyk.mcptask.application.service.LockOperation(
-                    operationType = mapOperationToLockType(operationName),
-                    toolName = name,
-                    description = "Tool operation: $operationName on ${entityType.name}",
-                    entityIds = setOf(entityId)
-                )
-                
-                val canProceed = service.canProceed(operation)
-                if (!canProceed) {
-                    // Create a simulated conflict error for demonstration
-                    val context = createLockErrorContext(operationName, entityType, entityId, session.getCurrentSession())
-                    val conflictError = LockError.LockConflict(
-                        message = "Operation cannot proceed due to conflicting locks",
-                        conflictingLocks = emptyList(), // Would be populated in real implementation
-                        requestedLock = io.github.jpicklyk.mcptask.domain.model.LockRequest(
+        val lockOperation = io.github.jpicklyk.mcptask.application.service.LockOperation(
+            operationType = mapOperationToLockType(operationName),
+            toolName = name,
+            description = "Tool operation: $operationName on ${entityType.name}",
+            entityIds = setOf(entityId)
+        )
+        
+        // Atomically attempt to acquire the lock
+        val lockResult = lockingService.tryAcquireLock(lockOperation)
+        
+        when (lockResult) {
+            is io.github.jpicklyk.mcptask.application.service.LockAcquisitionResult.Conflict -> {
+                val context = createLockErrorContext(operationName, entityType, entityId, sessionManager.getCurrentSession())
+                val conflictError = LockError.LockConflict(
+                    message = "Operation cannot proceed due to conflicting locks",
+                    conflictingLocks = emptyList(),
+                    requestedLock = io.github.jpicklyk.mcptask.domain.model.LockRequest(
+                        entityId = entityId,
+                        scope = io.github.jpicklyk.mcptask.domain.model.LockScope.TASK,
+                        lockType = io.github.jpicklyk.mcptask.domain.model.LockType.SHARED_WRITE,
+                        sessionId = sessionManager.getCurrentSession(),
+                        operationName = operationName,
+                        expectedDuration = 120L // 2 minutes
+                    ),
+                    suggestions = errorHandler.suggestAlternatives(emptyList(), 
+                        io.github.jpicklyk.mcptask.domain.model.LockRequest(
                             entityId = entityId,
                             scope = io.github.jpicklyk.mcptask.domain.model.LockScope.TASK,
                             lockType = io.github.jpicklyk.mcptask.domain.model.LockType.SHARED_WRITE,
-                            sessionId = session.getCurrentSession(),
+                            sessionId = sessionManager.getCurrentSession(),
                             operationName = operationName,
-                            expectedDuration = 1800L // 30 minutes
-                        ),
-                        suggestions = errorHandler.suggestAlternatives(emptyList(), 
-                            io.github.jpicklyk.mcptask.domain.model.LockRequest(
-                                entityId = entityId,
-                                scope = io.github.jpicklyk.mcptask.domain.model.LockScope.TASK,
-                                lockType = io.github.jpicklyk.mcptask.domain.model.LockType.SHARED_WRITE,
-                                sessionId = session.getCurrentSession(),
-                                operationName = operationName,
-                                expectedDuration = 1800L
-                            ), context),
-                        context = context
-                    )
-                    return handleLockError(conflictError)
+                            expectedDuration = 120L
+                        ), context),
+                    context = context
+                )
+                return handleLockError(conflictError)
+            }
+            is io.github.jpicklyk.mcptask.application.service.LockAcquisitionResult.Success -> {
+                // Lock acquired successfully, continue with operation
+                val operationId = lockResult.operationId
+                
+                try {
+                    // Execute the actual operation
+                    return operation()
+                } finally {
+                    // Always clean up the lock, even if operation fails
+                    lockingService.recordOperationComplete(operationId)
                 }
             }
         }
-        
-        return null // Operation can proceed
+    }
+    
+    /**
+     * Helper method to extract entity ID from different parameter structures.
+     */
+    protected fun extractEntityId(params: JsonElement, paramName: String = "id"): UUID {
+        return extractUuidFromParameters(params, paramName)
+            ?: throw ToolValidationException("Missing or invalid $paramName parameter")
     }
 
     private fun mapOperationToLockType(operationName: String): io.github.jpicklyk.mcptask.application.service.OperationType {