diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteDependencyRepository.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteDependencyRepository.kt index 6ea45ae..ae17341 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteDependencyRepository.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteDependencyRepository.kt @@ -33,8 +33,9 @@ class SQLiteDependencyRepository(private val databaseManager: DatabaseManager) : } private fun insertDependencyInTransaction(dependency: Dependency): Dependency { - // Check for cyclic dependencies before creating - if (checkCyclicDependencyInternal(dependency.fromItemId, dependency.toItemId)) { + // Only check cycles for blocking dependency types — RELATES_TO is informational + if (dependency.type != DependencyType.RELATES_TO && + checkCyclicDependencyInternal(dependency.fromItemId, dependency.toItemId)) { throw ValidationException("Creating this dependency would result in a circular dependency") } @@ -128,8 +129,10 @@ class SQLiteDependencyRepository(private val databaseManager: DatabaseManager) : // Phase 3: Incremental cycle detection - check and insert each dependency sequentially. // Each dependency is inserted before checking the next, so subsequent checks see earlier // batch members in the graph. Transaction rollback handles atomicity on failure. + // RELATES_TO deps are informational and cannot create blocking cycles — skip check. for (dep in dependencies) { - if (checkCyclicDependencyInternal(dep.fromItemId, dep.toItemId)) { + if (dep.type != DependencyType.RELATES_TO && + checkCyclicDependencyInternal(dep.fromItemId, dep.toItemId)) { throw ValidationException( "Creating these dependencies would result in a circular dependency chain" ) @@ -169,13 +172,13 @@ class SQLiteDependencyRepository(private val databaseManager: DatabaseManager) : visiting.add(currentItemId) - // Follow outgoing BLOCKS edges (not IS_BLOCKED_BY) + // Follow outgoing BLOCKS edges only (RELATES_TO is informational, not blocking) val outgoing = DependenciesTable.selectAll() .where { DependenciesTable.fromItemId eq currentItemId } .map { mapRowToDependency(it) } for (dep in outgoing) { - if (dep.type != DependencyType.IS_BLOCKED_BY) { + if (dep.type == DependencyType.BLOCKS) { if (dep.toItemId == fromItemId) return true if (hasCycle(dep.toItemId)) return true } diff --git a/current/src/test/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/database/repository/SQLiteDependencyRepositoryTest.kt b/current/src/test/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/database/repository/SQLiteDependencyRepositoryTest.kt index bc7d33c..0e5b3c9 100644 --- a/current/src/test/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/database/repository/SQLiteDependencyRepositoryTest.kt +++ b/current/src/test/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/database/repository/SQLiteDependencyRepositoryTest.kt @@ -280,6 +280,64 @@ class SQLiteDependencyRepositoryTest { assertEquals(2, deps.size) } + // --- RELATES_TO should not participate in cycle detection --- + + @Test + fun `hasCyclicDependency - RELATES_TO edge does not create blocking path`() = runBlocking { + // A RELATES_TO B is informational — it should not create a blocking path + depRepository.create(Dependency(fromItemId = itemA, toItemId = itemB, type = DependencyType.RELATES_TO)) + + // "Would adding B BLOCKS A create a cycle?" — No, because RELATES_TO is not a blocking edge + val isCyclic = depRepository.hasCyclicDependency(itemB, itemA) + assertFalse(isCyclic, "RELATES_TO should not be treated as a blocking path in cycle detection") + } + + @Test + fun `create succeeds when only RELATES_TO exists in reverse direction`() = runBlocking { + // A RELATES_TO B is informational + depRepository.create(Dependency(fromItemId = itemA, toItemId = itemB, type = DependencyType.RELATES_TO)) + + // B BLOCKS A should succeed — RELATES_TO doesn't create a blocking cycle + val dep = depRepository.create(Dependency(fromItemId = itemB, toItemId = itemA, type = DependencyType.BLOCKS)) + assertNotNull(dep) + assertEquals(itemB, dep.fromItemId) + assertEquals(itemA, dep.toItemId) + } + + @Test + fun `RELATES_TO at end of BLOCKS chain does not cause false cycle`() = runBlocking { + // Build a BLOCKS chain: A -> B -> C + depRepository.create(Dependency(fromItemId = itemA, toItemId = itemB, type = DependencyType.BLOCKS)) + depRepository.create(Dependency(fromItemId = itemB, toItemId = itemC, type = DependencyType.BLOCKS)) + + // C RELATES_TO A is informational — not a blocking cycle back to A + val dep = depRepository.create(Dependency(fromItemId = itemC, toItemId = itemA, type = DependencyType.RELATES_TO)) + assertNotNull(dep, "RELATES_TO closing a BLOCKS chain should not be rejected as a cycle") + } + + @Test + fun `mixed BLOCKS and IS_BLOCKED_BY cycle detection still works after fix`() = runBlocking { + // A BLOCKS B — real blocking relationship + depRepository.create(Dependency(fromItemId = itemA, toItemId = itemB, type = DependencyType.BLOCKS)) + + // "Would adding B BLOCKS A create a cycle?" — Yes, this is a real blocking cycle + val isCyclic = depRepository.hasCyclicDependency(itemB, itemA) + assertTrue(isCyclic, "Real BLOCKS cycle should still be detected") + } + + @Test + fun `createBatch succeeds with RELATES_TO closing a BLOCKS chain`() = runBlocking { + // Build a BLOCKS chain: A -> B -> C + depRepository.create(Dependency(fromItemId = itemA, toItemId = itemB, type = DependencyType.BLOCKS)) + depRepository.create(Dependency(fromItemId = itemB, toItemId = itemC, type = DependencyType.BLOCKS)) + + // Batch-create C RELATES_TO A — should succeed since RELATES_TO is informational + val batch = depRepository.createBatch( + listOf(Dependency(fromItemId = itemC, toItemId = itemA, type = DependencyType.RELATES_TO)) + ) + assertEquals(1, batch.size, "Batch with RELATES_TO closing a BLOCKS chain should succeed") + } + // --- findByItemIds --- @Test