Conversation
RELATES_TO dependencies are purely informational and cannot create blocking deadlocks. The DFS cycle detection was incorrectly traversing RELATES_TO edges, causing false positive cycle rejections — e.g., creating B RELATES_TO A when A BLOCKS B existed would be rejected. Three changes: - DFS filter: `!= IS_BLOCKED_BY` → `== BLOCKS` (excludes RELATES_TO) - insertDependencyInTransaction: skip cycle check for RELATES_TO - createBatch: skip cycle check for RELATES_TO Adds 5 TDD tests covering the fix and regression guards. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract create, update, and delete operations from the 702-line god class into CreateItemHandler, UpdateItemHandler, and DeleteItemHandler. Consolidate duplicated hierarchy validation (cycle detection, depth computation) into shared ItemHierarchyValidator service. Extract JSON field helpers into ItemFieldExtractors. ManageItemsTool is now a ~170-line dispatcher. MCP interface unchanged — all existing tests pass without modification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add DatabaseManager.suspendedTransaction() extension function that wraps newSuspendedTransaction + try/catch into a single call. Apply across all 33 suspend repository methods in SQLiteNoteRepository (7), SQLiteRoleTransitionRepository (5), and SQLiteWorkItemRepository (21). Net reduction of ~80 lines of boilerplate. Consolidates the deprecated newSuspendedTransaction call to a single site for future Exposed migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract WorkItem, Note, and Dependency serialization into extension functions in EntityJsonSerializers.kt. Replace 17+ inline role.name.lowercase() / priority.name.lowercase() calls across 10 tool files with Role.toJsonString() / Priority.toJsonString() helpers. Removes duplicate private serializer functions from QueryItemsTool and QueryNotesTool, reducing ~40 lines of duplicated code. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add extractItemBoolean to ItemFieldExtractors for handler-context boolean extraction. Replace inline jsonObject["key"]?.jsonPrimitive ?.booleanOrNull patterns with optionalBoolean() across 5 tool files. Replace inline offset extraction with optionalInt() in QueryItemsTool. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tory Extract buildFilteredQuery() to eliminate exact duplicate filter construction between findByFilters() and countByFilters() — 20 lines of identical condition-building code consolidated into one method. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s, cycle detection - Add requireUUID() helper to BaseToolDefinition to eliminate !! non-null assertions - Replace extractUUID(required=true)!! with requireUUID() in 4 call sites - Add .coerceAtLeast(0) offset bounds check in SQLiteWorkItemRepository.findByFilters() - Add time range validation (createdAfter < createdBefore, etc.) in QueryItemsTool.executeSearch() - Replace repeat()-based ancestor walk with visited-set cycle detection in ItemHierarchyValidator Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep refactored extension methods, transaction wrapper, and toWorkItemOrNull naming from the decompose-manage-items refactoring that landed on main. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep hardening fixes (time range validation, visited-set cycle detection) that are the purpose of this PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
requireUUID()helper — Added toBaseToolDefinitionto eliminate!!non-null assertions onextractUUID(required=true)calls. Replaced 4 call sites acrossQueryNotesTool,QueryDependenciesTool, andGetNextStatusTool..coerceAtLeast(0)inSQLiteWorkItemRepository.findByFilters()for defense-in-depth (tool layer already coerces, but repository should not assume validated input).createdAfter > createdBefore, etc.) inQueryItemsTool.executeSearch()to prevent silent empty results.repeat(MAX_DEPTH + 1)with a visited-set loop inItemHierarchyValidator.validateAndComputeDepth()to correctly detect pre-existing cycles in the hierarchy.Test plan
./gradlew :current:clean :current:test)🤖 Generated with Claude Code