refactor: address 5-pass audit findings for antipatterns, bottlenecks, memory efficiency, and LLM handlers

server/src/router.rs

@@ -73,7 +73,7 @@ impl McpResource for GraphEntitiesResource {
             }
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -97,7 +97,7 @@ impl McpResource for GraphRelationsResource {
             Ok(serde_json::to_string_pretty(&data)?)
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -124,7 +124,7 @@ impl McpResource for TasksActiveResource {
             Ok(serde_json::to_string_pretty(&data)?)
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -148,7 +148,7 @@ impl McpResource for DecisionsActiveResource {
             Ok(serde_json::to_string_pretty(&data)?)
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -172,7 +172,7 @@ impl McpResource for TechDebtActiveResource {
             Ok(serde_json::to_string_pretty(&data)?)
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -270,7 +270,7 @@ impl McpResource for SessionDeltaResource {
             Ok(serde_json::to_string_pretty(&delta)?)
         })
         .await
-        .unwrap()
+        .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
     }
 }
 
@@ -361,7 +361,7 @@ impl MemoryHandler {
                     Ok(serde_json::to_string_pretty(&*items)?)
                 })
                 .await
-                .unwrap()
+                .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
             }
         }
         struct PinnedFilesResource;
@@ -383,7 +383,7 @@ impl MemoryHandler {
                     Ok(serde_json::to_string_pretty(&*items)?)
                 })
                 .await
-                .unwrap()
+                .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
             }
         }
 
@@ -406,7 +406,7 @@ impl MemoryHandler {
                     Ok(serde_json::to_string_pretty(&*items)?)
                 })
                 .await
-                .unwrap()
+                .map_err(|e| crate::error::AppError::Internal(e.to_string()))?
             }
         }
 
@@ -715,7 +715,7 @@ impl MemoryHandler {
 
                 let category = match name {
                     "clipboard" => "CLIPBOARD",
-                    "create_entities" | "create_relations" | "read_graph" | "get_subgraph" | "search_graph" | "get_schema" => "GRAPH",
+                    "create_entities" | "create_relations" | "add_observations" | "delete_entities" | "delete_observations" | "delete_relations" | "read_graph" | "get_subgraph" | "search_graph" | "get_schema" | "search_nodes" | "open_nodes" | "visualize_graph" | "condense_entity" | "merge_entities" | "find_orphans" | "sweep_graph_health" | "summarize_subgraph" | "query_graph_path" | "resolve_stale_symbols" => "GRAPH",
                     "decisions" => "DECISION",
                     "log_code_change" => "CODE",
                     "log_error_fix" => "ERROR_FIX",
@@ -1131,5 +1131,111 @@ mod tests {
         assert_eq!(res2["jsonrpc"], "2.0");
         assert_eq!(res2["id"], 6);
     }
+
+    #[tokio::test]
+    async fn test_tools_list_schema_and_descriptions() {
+        let dir = tempdir().unwrap();
+        let state = Arc::new(MemoryState::new(dir.path().to_str().unwrap()));
+        let handler = MemoryHandler::new(state);
+
+        let list_req = json!({
+            "jsonrpc": "2.0",
+            "id": 100,
+            "method": "tools/list",
+            "params": {}
+        });
+
+        let res = handler.handle_request(list_req).await.unwrap();
+        assert_eq!(res["jsonrpc"], "2.0");
+        assert_eq!(res["id"], 100);
+
+        let tools = res["result"]["tools"].as_array().expect("tools array expected");
+        assert!(tools.len() >= 40, "Expected at least 40 registered MCP tools, found {}", tools.len());
+
+        for tool in tools {
+            let name = tool["name"].as_str().expect("tool name must be string");
+            assert!(!name.is_empty(), "Tool name cannot be empty");
+
+            let desc = tool["description"].as_str().expect("tool description must be string");
+            assert!(!desc.is_empty(), "Tool '{}' missing description", name);
+
+            let schema = tool["inputSchema"].as_object().expect("inputSchema must be object");
+            assert!(
+                schema.contains_key("properties") || schema.contains_key("type"),
+                "Tool '{}' schema must contain type/properties metadata",
+                name
+            );
+        }
+    }
+
+    #[tokio::test]
+    async fn test_all_tools_llm_request_ok_and_invalid() {
+        let dir = tempdir().unwrap();
+        let state = Arc::new(MemoryState::new(dir.path().to_str().unwrap()));
+        let handler = MemoryHandler::new(state);
+
+        // 1. Verify invalid LLM requests fail gracefully with isError: true or structured response
+        for (tool_name, _tool) in &handler.tools {
+            let invalid_req = json!({
+                "jsonrpc": "2.0",
+                "id": 200,
+                "method": "tools/call",
+                "params": {
+                    "name": tool_name,
+                    "arguments": {
+                        "invalid_field_unlikely_to_match": "bogus_value",
+                        "action": "non_existent_action_xyz_999"
+                    }
+                }
+            });
+
+            let res = handler.handle_request(invalid_req).await.expect("handler must respond");
+            assert_eq!(res["jsonrpc"], "2.0");
+            assert!(res["result"]["content"].is_array());
+        }
+
+        // 2. Verify valid LLM requests return isError: false
+        let sample_valid_calls = vec![
+            ("read_graph", json!({})),
+            ("search_nodes", json!({"query": "test"})),
+            ("tasks", json!({"action": "list"})),
+            ("decisions", json!({"action": "query"})),
+            ("tech_debt", json!({"action": "list"})),
+            ("sticky_notes", json!({"action": "read"})),
+            ("pinned_files", json!({"action": "list"})),
+            ("context_workspaces", json!({"action": "list"})),
+            ("pr_checklist", json!({"action": "get"})),
+            ("snippets", json!({"action": "search", "query": "test"})),
+            ("clipboard", json!({"action": "read"})),
+            ("environment", json!({"action": "read_fingerprint"})),
+            ("omni_search", json!({"query": "test"})),
+            ("get_project_health", json!({})),
+            ("read_preferences", json!({})),
+            ("find_orphans", json!({})),
+            ("sweep_graph_health", json!({})),
+            ("get_next_actionable_tasks", json!({})),
+            ("get_preflight_context", json!({})),
+            ("auto_session_checkpoint", json!({"session_summary": "Test summary"})),
+        ];
+
+        for (name, args) in sample_valid_calls {
+            let valid_req = json!({
+                "jsonrpc": "2.0",
+                "id": 201,
+                "method": "tools/call",
+                "params": {
+                    "name": name,
+                    "arguments": args
+                }
+            });
+
+            let res = handler.handle_request(valid_req).await.expect("handler must respond");
+            assert_eq!(res["jsonrpc"], "2.0");
+            assert_eq!(
+                res["result"]["isError"], false,
+                "Tool '{}' failed valid execution. Result: {:?}", name, res
+            );
+        }
+    }
 }