diff --git a/src/database/knowledge_dao.rs b/src/database/knowledge_dao.rs index ffc368f..091cd9a 100644 --- a/src/database/knowledge_dao.rs +++ b/src/database/knowledge_dao.rs @@ -601,12 +601,41 @@ impl KnowledgeDao for SqliteKnowledgeDao { trace_db_call(cx, "delete", "delete_entity", |_span| { use schema::entities::dsl::*; let mut conn = self.connection.lock().expect("KnowledgeDao lock"); - diesel::delete(entities.filter(id.eq(entity_id))) - .execute(conn.deref_mut()) - .map(|_| ()) - .map_err(|e| anyhow::anyhow!("Delete error: {}", e)) + + // entity_facts has a CHECK constraint requiring + // `object_entity_id IS NOT NULL OR object_value IS NOT NULL`. + // The FK on object_entity_id is ON DELETE SET NULL — but + // facts that pointed at the deleted entity *only* via the + // entity reference (the common case for relational facts + // like "Alice is_friend_of Bob") have no object_value, so + // SET NULL would leave them with both NULLs and the CHECK + // aborts the whole DELETE. Pre-delete those facts in a + // transaction so the CASCADE / SET NULL chain on what + // remains can fire cleanly. + // + // Long-term fix is to change the FK to ON DELETE CASCADE + // via a table-rebuild migration, but the DAO-side workaround + // is sufficient and less invasive. + conn.transaction::<(), diesel::result::Error, _>(|conn| { + use schema::entity_facts::dsl as ef; + diesel::delete( + ef::entity_facts + .filter(ef::object_entity_id.eq(entity_id)) + .filter(ef::object_value.is_null()), + ) + .execute(conn)?; + + diesel::delete(entities.filter(id.eq(entity_id))).execute(conn)?; + Ok(()) + }) + .map_err(|e| anyhow::anyhow!("Delete error: {}", e)) + }) + .map_err(|e| { + // Surface the actual diesel error string before collapsing + // to the opaque DbErrorKind::QueryError. + log::warn!("delete_entity({}) failed: {}", entity_id, e); + DbError::new(DbErrorKind::QueryError) }) - .map_err(|_| DbError::new(DbErrorKind::QueryError)) } fn merge_entities( @@ -1334,6 +1363,77 @@ mod tests { ); } + #[test] + fn delete_entity_clears_relational_facts_that_would_violate_check() { + // entity_facts has a CHECK that at least one of object_entity_id / + // object_value is non-null. The FK on object_entity_id is + // ON DELETE SET NULL, which would leave purely-relational facts + // (subject + predicate + object_entity_id, no object_value) + // with both nulls and abort the delete. The DAO pre-deletes + // those rows in a transaction so the parent delete can succeed. + let cx = opentelemetry::Context::new(); + let conn = connection_with_fks_on(); + let alice = create_user(&conn, "alice"); + create_persona_row(&conn, alice, "default"); + + let mut dao = SqliteKnowledgeDao::from_connection(conn.clone()); + let bob = make_entity(&mut dao, "Bob"); + let carol = make_entity(&mut dao, "Carol"); + + // A relational fact where Carol is the object — exactly the + // shape the CHECK + SET NULL combination would otherwise break. + let (rel_fact, _) = dao + .upsert_fact( + &cx, + InsertEntityFact { + subject_entity_id: bob.id, + predicate: "is_friend_of".to_string(), + object_entity_id: Some(carol.id), + object_value: None, + source_photo: None, + source_insight_id: None, + confidence: 0.6, + status: "active".to_string(), + created_at: 0, + persona_id: "default".to_string(), + user_id: alice, + }, + ) + .unwrap(); + + // A typed fact where Bob is the subject — should survive. + add_fact(&mut dao, bob.id, "has_age", "30", alice, "default"); + + // Delete Carol — should succeed (relational fact pre-deleted). + dao.delete_entity(&cx, carol.id).unwrap(); + + assert!( + dao.get_entity_by_id(&cx, carol.id).unwrap().is_none(), + "Carol should be deleted" + ); + // The relational fact about Carol should be gone (pre-deleted by + // the DAO's transaction, not SET NULL'd). + let bob_facts = dao + .get_facts_for_entity( + &cx, + bob.id, + &PersonaFilter::Single { + user_id: alice, + persona_id: "default".to_string(), + }, + ) + .unwrap(); + assert!( + !bob_facts.iter().any(|f| f.id == rel_fact.id), + "relational fact pointing at Carol should be removed" + ); + // The typed fact survives. + assert!( + bob_facts.iter().any(|f| f.predicate == "has_age"), + "typed fact about Bob should survive Carol's deletion" + ); + } + #[test] fn upsert_entity_collapses_near_duplicate_by_embedding() { // The agent's pre-flight check uses FTS5 prefix tokens, which