feat: add deleteTask method with CASCADE cleanup verification#19
feat: add deleteTask method with CASCADE cleanup verification#19
Conversation
Add deleteTask() method to SQLiteStore and comprehensive tests to verify proper cleanup of task-related data via CASCADE constraints. Closes #14 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdded a public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/memory/sqlite-store.ts`:
- Around line 1340-1347: The deleteTask method currently fails when the task is
a parent because tasks.parent_task_id has no ON DELETE action; modify deleteTask
to handle child rows first by wrapping operations in a transaction: begin a
transaction, run an UPDATE on the tasks table to SET parent_task_id = NULL WHERE
parent_task_id = id (or reparent as desired), then DELETE FROM tasks WHERE id =
?, commit the transaction, and ensure the function returns a boolean based on
the delete result; alternatively, update the schema to define
tasks.parent_task_id with ON DELETE SET NULL/ CASCADE and adjust deleteTask
accordingly.
| /** | ||
| * Delete a task and all associated data | ||
| * CASCADE constraints automatically clean up task_embeddings and task_relationships | ||
| */ | ||
| deleteTask(id: string): boolean { | ||
| const result = this.db.prepare('DELETE FROM tasks WHERE id = ?').run(id); | ||
| return result.changes > 0; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "schema\|create\|migrate" src/memoryRepository: blackms/aistack
Length of output: 41
🏁 Script executed:
git ls-files src/memory/Repository: blackms/aistack
Length of output: 187
🏁 Script executed:
cat -n src/memory/sqlite-store.ts | head -100Repository: blackms/aistack
Length of output: 3428
🏁 Script executed:
grep -n "CREATE TABLE.*tasks" src/memory/sqlite-store.tsRepository: blackms/aistack
Length of output: 144
🏁 Script executed:
sed -n '100,500p' src/memory/sqlite-store.ts | cat -nRepository: blackms/aistack
Length of output: 16691
🏁 Script executed:
sed -n '1340,1347p' src/memory/sqlite-store.ts | cat -nRepository: blackms/aistack
Length of output: 405
🏁 Script executed:
sed -n '1320,1360p' src/memory/sqlite-store.ts | cat -nRepository: blackms/aistack
Length of output: 1396
🏁 Script executed:
rg "deleteTask" src/memory/sqlite-store.ts -B 2 -A 2Repository: blackms/aistack
Length of output: 296
🏁 Script executed:
rg "transaction" src/memory/sqlite-store.ts | head -20Repository: blackms/aistack
Length of output: 481
Handle parent/child FK constraint before deleting tasks.
tasks.parent_task_id has a foreign key with no ON DELETE action. When deleting a task that is a parent of other tasks, the FK constraint will throw an exception, causing deleteTask to fail and violate the boolean return contract. Explicitly handle children before deletion using a transaction, or update the schema to include ON DELETE SET NULL / CASCADE.
One possible fix (nullify children before delete)
- deleteTask(id: string): boolean {
- const result = this.db.prepare('DELETE FROM tasks WHERE id = ?').run(id);
- return result.changes > 0;
- }
+ deleteTask(id: string): boolean {
+ return this.transaction((db) => {
+ db.prepare('UPDATE tasks SET parent_task_id = NULL WHERE parent_task_id = ?').run(id);
+ const result = db.prepare('DELETE FROM tasks WHERE id = ?').run(id);
+ return result.changes > 0;
+ });
+ }🤖 Prompt for AI Agents
In `@src/memory/sqlite-store.ts` around lines 1340 - 1347, The deleteTask method
currently fails when the task is a parent because tasks.parent_task_id has no ON
DELETE action; modify deleteTask to handle child rows first by wrapping
operations in a transaction: begin a transaction, run an UPDATE on the tasks
table to SET parent_task_id = NULL WHERE parent_task_id = id (or reparent as
desired), then DELETE FROM tasks WHERE id = ?, commit the transaction, and
ensure the function returns a boolean based on the delete result; alternatively,
update the schema to define tasks.parent_task_id with ON DELETE SET NULL/
CASCADE and adjust deleteTask accordingly.
Add comprehensive unit tests covering all major methods: - Namespace operations (deleteByNamespace) - Tag operations (addTag, removeTag, getAllTags, searchByTags) - Relationship operations (createRelationship, getRelationships, etc.) - Version operations (getVersionHistory, restoreVersion) - Project operations (CRUD for projects, tasks, specifications) - Active agent operations (save, load, delete, update status) - Review loop operations (save, load, delete) - Agent identity operations (create, update, list, audit) - Resource metrics and deliverable checkpoints - Consensus checkpoint operations Also fix bug in addTag() - was using get() instead of getById() Coverage results: - Statements: 95.15% - Functions: 100% - Lines: 95.15% Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
deleteTask()method toSQLiteStorefor removing tasks and associated datadeleteTask()methodChanges
src/memory/sqlite-store.tsdeleteTask()methodtests/integration/task-cleanup.test.tstests/unit/sqlite-store.test.tsdeleteTask()CASCADE Behavior Verified
task_embeddingstask_relationshipsdrift_detection_eventsTest plan
npm run test:unit)npm run test:integration)deleteTask()method properly removes taskstask_embeddingstask_relationshipsdrift_detection_eventsconfirmed to persist after task deletion (by design)Closes #14
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.