diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/ItemHierarchyValidator.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/ItemHierarchyValidator.kt index 2a68fd8..4355a9b 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/ItemHierarchyValidator.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/service/ItemHierarchyValidator.kt @@ -45,13 +45,14 @@ class ItemHierarchyValidator { } // Guard: ancestor cycle check — walk up from parentId, ensure itemId is not an ancestor + // Uses a visited set to detect pre-existing cycles in the hierarchy (not just depth-bounded) + val visited = mutableSetOf() var cursor: UUID? = parentId - repeat(MAX_DEPTH + 1) { - val cursorId = cursor ?: return@repeat - val ancestorResult = repo.getById(cursorId) - val ancestor = when (ancestorResult) { + while (cursor != null && visited.size <= MAX_DEPTH) { + if (!visited.add(cursor)) break // Pre-existing cycle — stop walking + val ancestor = when (val ancestorResult = repo.getById(cursor)) { is Result.Success -> ancestorResult.data - is Result.Error -> return@repeat + is Result.Error -> break } if (ancestor.id == itemId) { throw ToolValidationException( diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/BaseToolDefinition.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/BaseToolDefinition.kt index 2114559..27e52cc 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/BaseToolDefinition.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/BaseToolDefinition.kt @@ -277,6 +277,20 @@ abstract class BaseToolDefinition : ToolDefinition { // UUID extraction (v3-specific) // ────────────────────────────────────────────── + /** + * Extracts and parses a required UUID parameter, returning a non-null UUID. + * + * Wraps [extractUUID] with `required=true` so call sites avoid `!!` non-null assertions. + * + * @param params The input parameters + * @param name The parameter name + * @return The parsed UUID (never null) + * @throws ToolValidationException if missing or not a valid UUID + */ + protected fun requireUUID(params: JsonElement, name: String): UUID = + extractUUID(params, name, required = true) + ?: throw ToolValidationException("'$name' is required and must be a valid UUID") + /** * Extracts and parses a UUID parameter from a string value. * diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/dependency/QueryDependenciesTool.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/dependency/QueryDependenciesTool.kt index 8a705f4..53d8ff7 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/dependency/QueryDependenciesTool.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/dependency/QueryDependenciesTool.kt @@ -90,7 +90,7 @@ Returns dependencies with counts breakdown and optional graph traversal data. } override suspend fun execute(params: JsonElement, context: ToolExecutionContext): JsonElement { - val itemId = extractUUID(params, "itemId", required = true)!! + val itemId = requireUUID(params, "itemId") val direction = optionalString(params, "direction") ?: "all" val typeFilter = optionalString(params, "type")?.let { DependencyType.fromString(it) } val includeItemInfo = optionalBoolean(params, "includeItemInfo", defaultValue = false) diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/items/QueryItemsTool.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/items/QueryItemsTool.kt index 0df654d..86a8e7d 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/items/QueryItemsTool.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/items/QueryItemsTool.kt @@ -343,6 +343,17 @@ Operations: get, search, overview val offset = (optionalInt(params, "offset") ?: 0).coerceAtLeast(0) val includeAncestors = optionalBoolean(params, "includeAncestors", false) + // Validate time ranges — reject inverted ranges early + if (createdAfter != null && createdBefore != null && createdAfter > createdBefore) { + return errorResponse("createdAfter must be before createdBefore", ErrorCodes.VALIDATION_ERROR) + } + if (modifiedAfter != null && modifiedBefore != null && modifiedAfter > modifiedBefore) { + return errorResponse("modifiedAfter must be before modifiedBefore", ErrorCodes.VALIDATION_ERROR) + } + if (roleChangedAfter != null && roleChangedBefore != null && roleChangedAfter > roleChangedBefore) { + return errorResponse("roleChangedAfter must be before roleChangedBefore", ErrorCodes.VALIDATION_ERROR) + } + // Parse role val role = roleStr?.let { Role.fromString(it) diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/notes/QueryNotesTool.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/notes/QueryNotesTool.kt index 6dc3ab3..48a543d 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/notes/QueryNotesTool.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/notes/QueryNotesTool.kt @@ -124,7 +124,7 @@ Read-only query operations for Notes (get, list). // ────────────────────────────────────────────── private suspend fun executeGet(params: JsonElement, context: ToolExecutionContext): JsonElement { - val id = extractUUID(params, "id", required = true)!! + val id = requireUUID(params, "id") val noteRepo = context.noteRepository() return when (val result = noteRepo.getById(id)) { @@ -146,7 +146,7 @@ Read-only query operations for Notes (get, list). // ────────────────────────────────────────────── private suspend fun executeList(params: JsonElement, context: ToolExecutionContext): JsonElement { - val itemId = extractUUID(params, "itemId", required = true)!! + val itemId = requireUUID(params, "itemId") val role = optionalString(params, "role") val includeBody = optionalBoolean(params, "includeBody", defaultValue = true) val noteRepo = context.noteRepository() diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetNextStatusTool.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetNextStatusTool.kt index 18b3470..afa5642 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetNextStatusTool.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetNextStatusTool.kt @@ -58,7 +58,7 @@ Read-only status progression recommendation for a WorkItem. } override suspend fun execute(params: JsonElement, context: ToolExecutionContext): JsonElement { - val itemId = extractUUID(params, "itemId", required = true)!! + val itemId = requireUUID(params, "itemId") // Fetch the WorkItem val itemResult = context.workItemRepository().getById(itemId) diff --git a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteWorkItemRepository.kt b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteWorkItemRepository.kt index ac78836..5be37ee 100644 --- a/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteWorkItemRepository.kt +++ b/current/src/main/kotlin/io/github/jpicklyk/mcptask/current/infrastructure/repository/SQLiteWorkItemRepository.kt @@ -217,7 +217,7 @@ class SQLiteWorkItemRepository(private val databaseManager: DatabaseManager) : W val items = baseQuery .orderBy(sortColumn, order) .limit(limit) - .offset(offset.toLong()) + .offset(offset.coerceAtLeast(0).toLong()) .mapNotNull { toWorkItemOrNull(it) } Result.Success(items)