fix(mcp): return explicit JSON-RPC errors instead of misleading Ok strings for missing entities and silent graph modification drops
This commit is contained in:
1 parent
cc190816fe
commit
f81723a539
4 files changed
+27
-9
No files matched your search
@@ -169,7 +169,7 @@ impl McpTool for CreateRelationsHandler {
|
|||||||
});
|
});
|
||||||
if !missing_nodes.is_empty() {
|
if !missing_nodes.is_empty() {
|
||||||
let missing: Vec<_> = missing_nodes.into_iter().collect();
|
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())
|
Ok("Relations created".to_string())
|
||||||
}
|
}
|
||||||
@@ -200,7 +200,7 @@ impl McpTool for AddObservationsHandler {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
if !missing_entities.is_empty() {
|
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())
|
Ok("Observations added".to_string())
|
||||||
}
|
}
|
||||||
@@ -260,14 +260,20 @@ impl McpTool for DeleteObservationsHandler {
|
|||||||
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
||||||
let req: DeleteObservationsTool =
|
let req: DeleteObservationsTool =
|
||||||
serde_json::from_value(args).map_err(|e| e.to_string())?;
|
serde_json::from_value(args).map_err(|e| e.to_string())?;
|
||||||
|
let mut missing = Vec::new();
|
||||||
state.modify_graph(|master| {
|
state.modify_graph(|master| {
|
||||||
for d in req.deletions {
|
for d in req.deletions {
|
||||||
if let Some(e) = master.entities.get_mut(&d.entity_name) {
|
if let Some(e) = master.entities.get_mut(&d.entity_name) {
|
||||||
let to_rem: HashSet<_> = d.observations.into_iter().collect();
|
let to_rem: HashSet<_> = d.observations.into_iter().collect();
|
||||||
e.observations.retain(|o| !to_rem.contains(o));
|
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())
|
Ok("Observations deleted".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -507,11 +513,17 @@ impl McpTool for CondenseEntityHandler {
|
|||||||
|
|
||||||
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
||||||
let req: CondenseEntityTool = serde_json::from_value(args).map_err(|e| e.to_string())?;
|
let req: CondenseEntityTool = serde_json::from_value(args).map_err(|e| e.to_string())?;
|
||||||
|
let mut missing = false;
|
||||||
state.modify_graph(|master| {
|
state.modify_graph(|master| {
|
||||||
if let Some(e) = master.entities.get_mut(&req.entity_name) {
|
if let Some(e) = master.entities.get_mut(&req.entity_name) {
|
||||||
e.observations = req.summarized_observations;
|
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())
|
Ok("Entity condensed".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -530,6 +542,7 @@ impl McpTool for MergeEntitiesHandler {
|
|||||||
|
|
||||||
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
async fn execute(&self, args: Value, state: Arc<MemoryState>) -> Result<String, String> {
|
||||||
let req: MergeEntitiesTool = serde_json::from_value(args).map_err(|e| e.to_string())?;
|
let req: MergeEntitiesTool = serde_json::from_value(args).map_err(|e| e.to_string())?;
|
||||||
|
let mut missing = false;
|
||||||
state.modify_graph(|master| {
|
state.modify_graph(|master| {
|
||||||
if let Some(src) = master.entities.remove(&req.source_entity) {
|
if let Some(src) = master.entities.remove(&req.source_entity) {
|
||||||
if let Some(tgt) = master.entities.get_mut(&req.target_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();
|
new_tgt.name = req.target_entity.clone();
|
||||||
master.entities.insert(req.target_entity.clone(), new_tgt);
|
master.entities.insert(req.target_entity.clone(), new_tgt);
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
missing = true;
|
||||||
}
|
}
|
||||||
let mut seen = std::collections::HashSet::new();
|
let mut seen = std::collections::HashSet::new();
|
||||||
master.relations.retain_mut(|r| {
|
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())
|
Ok("Entities merged".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -297,7 +297,7 @@ impl McpTool for ResolveTechDebtHandler {
|
|||||||
if found {
|
if found {
|
||||||
Ok("Tech debt resolved".to_string())
|
Ok("Tech debt resolved".to_string())
|
||||||
} else {
|
} else {
|
||||||
Ok("Tech debt not found".to_string())
|
Err("Tech debt not found".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ impl McpTool for DeleteTaskHandler {
|
|||||||
][0]
|
][0]
|
||||||
.clone())
|
.clone())
|
||||||
} else {
|
} else {
|
||||||
Ok("Task not found.".to_string())
|
Err("Task not found.".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -264,7 +264,7 @@ impl McpTool for UpdateTaskStatusHandler {
|
|||||||
} else if found {
|
} else if found {
|
||||||
Ok("Task status updated.".to_string())
|
Ok("Task status updated.".to_string())
|
||||||
} else {
|
} else {
|
||||||
Ok("Task not found.".to_string())
|
Err("Task not found.".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -344,7 +344,7 @@ impl McpTool for SetAcceptanceCriteriaHandler {
|
|||||||
if success {
|
if success {
|
||||||
Ok("Acceptance criteria set successfully.".to_string())
|
Ok("Acceptance criteria set successfully.".to_string())
|
||||||
} else {
|
} 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 {
|
} else if already_met {
|
||||||
Ok("Acceptance criteria was already met.".to_string())
|
Ok("Acceptance criteria was already met.".to_string())
|
||||||
} else {
|
} 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 {
|
if found {
|
||||||
Ok("Milestone updated".to_string())
|
Ok("Milestone updated".to_string())
|
||||||
} else {
|
} else {
|
||||||
Ok("Milestone not found".to_string())
|
Err("Milestone not found".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -193,7 +193,7 @@ impl McpTool for DeleteSnippetHandler {
|
|||||||
drop(idx.delete_document(&req.name));
|
drop(idx.delete_document(&req.name));
|
||||||
Ok("Snippet deleted.".to_string())
|
Ok("Snippet deleted.".to_string())
|
||||||
} else {
|
} else {
|
||||||
Ok("Snippet not found.".to_string())
|
Err("Snippet not found.".to_string())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user