Skip to content

Commit ca5764c

Browse files
committed
fix: return JSON-RPC error responses for failed MCP tool calls
Previously, run_mcp_tool_call errors were propogated with `?`, which exited MCP server loop entirely. MCP hosts then observed a "Connection closed" error because the process crashed. Wrap the tools/call dispatch in a match so errors are returned as JSON-RPC -32000 (server error) responses. The server stays alive and continues processing subsequent requests and propogates message in JSON response. Adds a new integration test to validate that a failure response doesn't close the MCP connection. AI assistance disclosure: - agent_name: GitHub Copilot CLI - agent_version: 1.0.3 - model_used: Claude Opus 4.6 - human_testing: Ran cargo test -q and observed all tests passing. Manual build and pass json sequence with unauthed search (that previously would crash mcp) followed by news query (that works without auth) and observed server live through failure response.
1 parent 9ba2c06 commit ca5764c

3 files changed

Lines changed: 115 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ Before `1.0.0`, breaking changes may still ship in minor releases.
77

88
## [Unreleased]
99

10+
### Fixed
11+
- MCP server no longer crashes when a tool call fails; instead propogate errors as JSON-RPC responses.
12+
1013
## [0.5.3]
1114

1215
### Added

src/main.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,11 +2076,18 @@ async fn run_mcp(args: McpArgs, profile: Option<&str>) -> Result<(), KagiError>
20762076
]
20772077
}
20782078
}),
2079-
"tools/call" => serde_json::json!({
2080-
"jsonrpc": "2.0",
2081-
"id": id,
2082-
"result": run_mcp_tool_call(&request, profile).await?,
2083-
}),
2079+
"tools/call" => match run_mcp_tool_call(&request, profile).await {
2080+
Ok(result) => serde_json::json!({
2081+
"jsonrpc": "2.0",
2082+
"id": id,
2083+
"result": result,
2084+
}),
2085+
Err(error) => serde_json::json!({
2086+
"jsonrpc": "2.0",
2087+
"id": id,
2088+
"error": {"code": -32000, "message": error.to_string()},
2089+
}),
2090+
},
20842091
_ => serde_json::json!({
20852092
"jsonrpc": "2.0",
20862093
"id": id,

tests/integration-cli.rs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -997,3 +997,103 @@ fn mcp_news_search_tool_call_returns_clusters() {
997997
assert_eq!(clusters[0]["items"][0]["paywall"], true);
998998
assert_eq!(clusters[1]["items"].as_array().unwrap().len(), 2);
999999
}
1000+
1001+
#[test]
1002+
fn mcp_tool_call_error_returns_json_rpc_error_and_keeps_server_alive() {
1003+
let server = MockServer::start();
1004+
1005+
// Mock search endpoint to return a 500 error, simulating a backend failure.
1006+
let _search = server.mock(|when, then| {
1007+
when.method(GET).path("/search");
1008+
then.status(500).body("Internal Server Error");
1009+
});
1010+
1011+
// Mock news endpoints so kagi_news succeeds — proving the server survived.
1012+
let _latest = server.mock(|when, then| {
1013+
when.method(GET).path("/api/batches/latest");
1014+
then.status(200)
1015+
.header("content-type", "application/json")
1016+
.json_body(news_latest_batch());
1017+
});
1018+
let _metadata = server.mock(|when, then| {
1019+
when.method(GET).path("/api/categories/metadata");
1020+
then.status(200)
1021+
.header("content-type", "application/json")
1022+
.json_body(news_category_metadata());
1023+
});
1024+
let _categories = server.mock(|when, then| {
1025+
when.method(GET)
1026+
.path("/api/batches/batch-1/categories");
1027+
then.status(200)
1028+
.header("content-type", "application/json")
1029+
.json_body(news_batch_categories());
1030+
});
1031+
let _stories = server.mock(|when, then| {
1032+
when.method(GET)
1033+
.path("/api/batches/batch-1/categories/category-1/stories");
1034+
then.status(200)
1035+
.header("content-type", "application/json")
1036+
.json_body(news_stories());
1037+
});
1038+
1039+
let tempdir = TempDir::new().expect("tempdir");
1040+
let env = session_env(&server);
1041+
1042+
// Send a search tool call (will fail) followed by a news tool call (should succeed).
1043+
let failing_request = json!({
1044+
"jsonrpc": "2.0",
1045+
"id": 1,
1046+
"method": "tools/call",
1047+
"params": {
1048+
"name": "kagi_search",
1049+
"arguments": { "query": "test" }
1050+
}
1051+
});
1052+
let succeeding_request = json!({
1053+
"jsonrpc": "2.0",
1054+
"id": 2,
1055+
"method": "tools/call",
1056+
"params": {
1057+
"name": "kagi_news",
1058+
"arguments": { "category": "tech", "lang": "en", "limit": 3 }
1059+
}
1060+
});
1061+
1062+
let stdin = format!(
1063+
"{}\n{}\n",
1064+
serde_json::to_string(&failing_request).unwrap(),
1065+
serde_json::to_string(&succeeding_request).unwrap(),
1066+
);
1067+
1068+
let output = run_kagi_with_stdin(&["mcp"], &stdin, &env_refs(&env), tempdir.path());
1069+
1070+
assert_success(&output);
1071+
let stdout = String::from_utf8_lossy(&output.stdout);
1072+
let responses: Vec<Value> = stdout
1073+
.lines()
1074+
.map(|line| serde_json::from_str(line).expect("each line is valid JSON"))
1075+
.collect();
1076+
1077+
assert_eq!(responses.len(), 2, "expected two JSON-RPC responses");
1078+
1079+
// First response: the failed tool call should be a JSON-RPC error, not a crash.
1080+
let error_resp = &responses[0];
1081+
assert_eq!(error_resp["id"], 1);
1082+
assert!(
1083+
error_resp.get("error").is_some(),
1084+
"expected JSON-RPC error for failed tool call, got: {error_resp}"
1085+
);
1086+
assert_eq!(error_resp["error"]["code"], -32000);
1087+
assert!(
1088+
error_resp["error"]["message"].as_str().unwrap().len() > 0,
1089+
"error message should be non-empty"
1090+
);
1091+
1092+
// Second response: the server stayed alive and processed the next request.
1093+
let success_resp = &responses[1];
1094+
assert_eq!(success_resp["id"], 2);
1095+
assert!(
1096+
success_resp.get("result").is_some(),
1097+
"expected successful result for second tool call, got: {success_resp}"
1098+
);
1099+
}

0 commit comments

Comments
 (0)