From f81723a539e003e7f55cf2f332d84feb1e4d6646 Mon Sep 17 00:00:00 2001 From: Riz Ashraf Date: Sat, 26 Sep 2026 08:24:56 +0100 Subject: [PATCH] fix(mcp): return explicit JSON-RPC errors instead of misleading Ok strings for missing entities and silent graph modification drops --- server/src/handlers/graph.rs | 22 ++++++++++++++++++++-- server/src/handlers/meta.rs | 2 +- server/src/handlers/tasks.rs | 10 +++++----- server/src/handlers/workspaces.rs | 2 +- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/server/src/handlers/graph.rs b/server/src/handlers/graph.rs index fc593d6..e9771e7 100644 --- a/server/src/handlers/graph.rs +++ b/server/src/handlers/graph.rs @@ -169,7 +169,7 @@ impl McpTool for CreateRelationsHandler { }); if !missing_nodes.is_empty() { let missing: Vec<_> = missing_nodes.into_iter().collect(); - return Ok(format!("Warning: Relations dropped due to missing entities: {}", missing.join(", "))); + return Err(format!("Error: Relations dropped due to missing entities: {}", missing.join(", "))); } Ok("Relations created".to_string()) } @@ -200,7 +200,7 @@ impl McpTool for AddObservationsHandler { } }); if !missing_entities.is_empty() { - return Ok(format!("Warning: Observations dropped for missing entities: {}", missing_entities.join(", "))); + return Err(format!("Error: Observations dropped for missing entities: {}", missing_entities.join(", "))); } Ok("Observations added".to_string()) } @@ -260,14 +260,20 @@ impl McpTool for DeleteObservationsHandler { async fn execute(&self, args: Value, state: Arc) -> Result { let req: DeleteObservationsTool = serde_json::from_value(args).map_err(|e| e.to_string())?; + let mut missing = Vec::new(); state.modify_graph(|master| { for d in req.deletions { if let Some(e) = master.entities.get_mut(&d.entity_name) { let to_rem: HashSet<_> = d.observations.into_iter().collect(); e.observations.retain(|o| !to_rem.contains(o)); + } else { + missing.push(d.entity_name); } } }); + if !missing.is_empty() { + return Err(format!("Error: Entities not found: {}", missing.join(", "))); + } Ok("Observations deleted".to_string()) } } @@ -507,11 +513,17 @@ impl McpTool for CondenseEntityHandler { async fn execute(&self, args: Value, state: Arc) -> Result { let req: CondenseEntityTool = serde_json::from_value(args).map_err(|e| e.to_string())?; + let mut missing = false; state.modify_graph(|master| { if let Some(e) = master.entities.get_mut(&req.entity_name) { e.observations = req.summarized_observations; + } else { + missing = true; } }); + if missing { + return Err(format!("Error: Entity '{}' not found", req.entity_name)); + } Ok("Entity condensed".to_string()) } } @@ -530,6 +542,7 @@ impl McpTool for MergeEntitiesHandler { async fn execute(&self, args: Value, state: Arc) -> Result { let req: MergeEntitiesTool = serde_json::from_value(args).map_err(|e| e.to_string())?; + let mut missing = false; state.modify_graph(|master| { if let Some(src) = master.entities.remove(&req.source_entity) { if let Some(tgt) = master.entities.get_mut(&req.target_entity) { @@ -541,6 +554,8 @@ impl McpTool for MergeEntitiesHandler { new_tgt.name = req.target_entity.clone(); master.entities.insert(req.target_entity.clone(), new_tgt); } + } else { + missing = true; } let mut seen = std::collections::HashSet::new(); master.relations.retain_mut(|r| { @@ -558,6 +573,9 @@ impl McpTool for MergeEntitiesHandler { } }); }); + if missing { + return Err(format!("Error: Source entity '{}' not found", req.source_entity)); + } Ok("Entities merged".to_string()) } } diff --git a/server/src/handlers/meta.rs b/server/src/handlers/meta.rs index 87a57c2..4d8804a 100644 --- a/server/src/handlers/meta.rs +++ b/server/src/handlers/meta.rs @@ -297,7 +297,7 @@ impl McpTool for ResolveTechDebtHandler { if found { Ok("Tech debt resolved".to_string()) } else { - Ok("Tech debt not found".to_string()) + Err("Tech debt not found".to_string()) } } } diff --git a/server/src/handlers/tasks.rs b/server/src/handlers/tasks.rs index 48d390f..78743a8 100644 --- a/server/src/handlers/tasks.rs +++ b/server/src/handlers/tasks.rs @@ -121,7 +121,7 @@ impl McpTool for DeleteTaskHandler { ][0] .clone()) } else { - Ok("Task not found.".to_string()) + Err("Task not found.".to_string()) } } } @@ -264,7 +264,7 @@ impl McpTool for UpdateTaskStatusHandler { } else if found { Ok("Task status updated.".to_string()) } else { - Ok("Task not found.".to_string()) + Err("Task not found.".to_string()) } } } @@ -344,7 +344,7 @@ impl McpTool for SetAcceptanceCriteriaHandler { if success { Ok("Acceptance criteria set successfully.".to_string()) } else { - Ok("Task not found.".to_string()) + Err("Task not found.".to_string()) } } } @@ -393,7 +393,7 @@ impl McpTool for VerifyAcceptanceCriteriaHandler { } else if already_met { Ok("Acceptance criteria was already met.".to_string()) } else { - Ok("Acceptance criteria or task not found.".to_string()) + Err("Acceptance criteria or task not found.".to_string()) } } } @@ -452,7 +452,7 @@ impl McpTool for UpdateMilestoneHandler { if found { Ok("Milestone updated".to_string()) } else { - Ok("Milestone not found".to_string()) + Err("Milestone not found".to_string()) } } } diff --git a/server/src/handlers/workspaces.rs b/server/src/handlers/workspaces.rs index f0eb8c0..2c19138 100644 --- a/server/src/handlers/workspaces.rs +++ b/server/src/handlers/workspaces.rs @@ -193,7 +193,7 @@ impl McpTool for DeleteSnippetHandler { drop(idx.delete_document(&req.name)); Ok("Snippet deleted.".to_string()) } else { - Ok("Snippet not found.".to_string()) + Err("Snippet not found.".to_string()) } } }