diff --git a/server/src/http/gateway/mcp.rs b/server/src/http/gateway/mcp.rs index 06ad1f2f..840fdf4f 100644 --- a/server/src/http/gateway/mcp.rs +++ b/server/src/http/gateway/mcp.rs @@ -350,7 +350,25 @@ pub async fn gateway_mcp_post_handler( Some(project) => { proxy_and_respond_for_project(&state, project, &bytes, rpc.id).await } - None => proxy_and_respond(&state, &bytes, rpc.id).await, + None if is_create_tool(tool_name) => { + // Story 1225 AC 3: with >1 project registered, a + // create call omitting `project` is ambiguous — fail + // loudly instead of silently filing into whichever + // project happens to be active. + let project_count = state.projects.read().await.len(); + if project_count > 1 { + to_json_response(JsonRpcResponse::error( + rpc.id, + -32602, + format!( + "'{tool_name}' requires an explicit `project` argument when more than one project is registered (see list_projects) — the active project is not used implicitly for creates." + ), + )) + } else { + proxy_and_respond_with_resolved_project(&state, &bytes, rpc.id).await + } + } + None => proxy_and_respond_with_resolved_project(&state, &bytes, rpc.id).await, } } } @@ -376,6 +394,50 @@ async fn proxy_and_respond(state: &GatewayState, bytes: &[u8], id: Option } } +/// Returns `true` for tool names that create a new work item or resource +/// (`create_bug`, `create_story`, `create_worktree`, ...), used to gate +/// implicit active-project routing behind an explicit `project` argument +/// when the destination is ambiguous (story 1225 AC 3). +fn is_create_tool(tool_name: &str) -> bool { + tool_name.starts_with("create_") +} + +/// Proxy a request to the active project and, on success, annotate the +/// response with which project actually handled it (story 1225 AC 3) — so a +/// caller who omitted `project` can see whether their call landed where they +/// expected instead of silently trusting `active_project`. +async fn proxy_and_respond_with_resolved_project( + state: &GatewayState, + bytes: &[u8], + id: Option, +) -> Response { + let active = state.active_project.read().await.clone(); + match state.proxy_active_mcp(bytes).await { + Ok(resp_body) => Response::builder() + .status(StatusCode::OK) + .header("Content-Type", "application/json") + .body(Body::from(annotate_resolved_project(&resp_body, &active))), + Err(e) => to_json_response(JsonRpcResponse::error( + id, + -32603, + format!("proxy error: {e}"), + )), + } +} + +/// Insert a `resolved_project` field into a JSON-RPC `result` object, +/// leaving the bytes unchanged if they don't parse as JSON or `result` isn't +/// an object (e.g. an error response, which has no `result` at all). +fn annotate_resolved_project(bytes: &[u8], project: &str) -> Vec { + let Ok(mut value) = serde_json::from_slice::(bytes) else { + return bytes.to_vec(); + }; + if let Some(result) = value.get_mut("result").and_then(|r| r.as_object_mut()) { + result.insert("resolved_project".to_string(), json!(project)); + } + serde_json::to_vec(&value).unwrap_or_else(|_| bytes.to_vec()) +} + /// Proxy a request to an explicitly named project (story 1208 AC 1) rather /// than whatever project is currently active, so a single ops/LLM session /// can address any registered project per-call without a prior @@ -1310,6 +1372,7 @@ async fn handle_pipeline_get(state: &GatewayState, id: Option) -> JsonRpc mod tests { use super::*; use crate::service::gateway::config::{GatewayConfig, ProjectEntry}; + use poem::EndpointExt as _; use std::collections::BTreeMap; use std::sync::Arc; @@ -1688,4 +1751,232 @@ mod tests { inject_project_arg_schema(&mut tools); assert_eq!(tools[0], json!({"name": "no_schema"})); } + + // ── project arg is honored end-to-end, not silently misrouted (story 1225) ─ + + /// Register a fake sled connection for `name` whose background task + /// answers every `mcp_request` with a canned response naming itself, so + /// tests can assert *which* project actually handled a routed call. + async fn spawn_fake_sled(name: &str) -> gateway::SledConnection { + let (tx, mut rx) = + tokio::sync::mpsc::unbounded_channel::(); + let in_flight: Arc< + tokio::sync::Mutex< + std::collections::HashMap>, + >, + > = Arc::new(tokio::sync::Mutex::new(std::collections::HashMap::new())); + let in_flight_task = Arc::clone(&in_flight); + let label = name.to_string(); + tokio::spawn(async move { + while let Some(env) = rx.recv().await { + if let Some(sender) = in_flight_task.lock().await.remove(&env.req_id) { + let _ = sender.send(json!({ + "jsonrpc": "2.0", + "id": 1, + "result": { "content": [{ "type": "text", "text": format!("handled_by:{label}") }] } + })); + } + } + }); + gateway::SledConnection { + tx, + last_heartbeat_ms: Arc::new(std::sync::atomic::AtomicI64::new( + chrono::Utc::now().timestamp_millis(), + )), + in_flight, + } + } + + async fn make_two_project_state(dir: &std::path::Path) -> Arc { + let mut projects = BTreeMap::new(); + projects.insert("alpha".to_string(), ProjectEntry::with_url("http://a:3001")); + projects.insert("beta".to_string(), ProjectEntry::with_url("http://b:3002")); + let config = GatewayConfig { + projects, + sled_tokens: BTreeMap::new(), + release_channels: BTreeMap::new(), + }; + let state = Arc::new(GatewayState::new(config, dir.to_path_buf(), 3000).unwrap()); + assert_eq!(*state.active_project.read().await, "alpha"); + state + .register_sled_connection("alpha".to_string(), spawn_fake_sled("alpha").await) + .await; + state + .register_sled_connection("beta".to_string(), spawn_fake_sled("beta").await) + .await; + state + } + + #[tokio::test] + async fn create_tool_with_explicit_project_lands_in_non_active_project() { + let dir = tempfile::tempdir().unwrap(); + let state = make_two_project_state(dir.path()).await; + let app = poem::Route::new() + .at("/mcp", poem::post(gateway_mcp_post_handler)) + .data(state); + let cli = poem::test::TestClient::new(app); + + let rpc_body = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "create_bug", + "arguments": { "project": "beta", "title": "x", "description": "y" } + } + })) + .unwrap(); + let resp = cli + .post("/mcp") + .header("content-type", "application/json") + .body(rpc_body) + .send() + .await; + let parsed: Value = resp.0.into_body().into_json().await.unwrap(); + assert_eq!( + parsed["result"]["content"][0]["text"], "handled_by:beta", + "explicit project=beta must land in beta even though alpha is active: {parsed}" + ); + } + + #[tokio::test] + async fn read_tool_with_explicit_project_reads_from_non_active_project() { + let dir = tempfile::tempdir().unwrap(); + let state = make_two_project_state(dir.path()).await; + let app = poem::Route::new() + .at("/mcp", poem::post(gateway_mcp_post_handler)) + .data(state); + let cli = poem::test::TestClient::new(app); + + let rpc_body = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "get_pipeline_status", + "arguments": { "project": "beta" } + } + })) + .unwrap(); + let resp = cli + .post("/mcp") + .header("content-type", "application/json") + .body(rpc_body) + .send() + .await; + let parsed: Value = resp.0.into_body().into_json().await.unwrap(); + assert_eq!( + parsed["result"]["content"][0]["text"], "handled_by:beta", + "explicit project=beta must read from beta even though alpha is active: {parsed}" + ); + } + + #[tokio::test] + async fn create_tool_without_project_errors_when_multiple_projects_registered() { + let dir = tempfile::tempdir().unwrap(); + let state = make_two_project_state(dir.path()).await; + let app = poem::Route::new() + .at("/mcp", poem::post(gateway_mcp_post_handler)) + .data(state); + let cli = poem::test::TestClient::new(app); + + let rpc_body = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "create_bug", + "arguments": { "title": "x", "description": "y" } + } + })) + .unwrap(); + let resp = cli + .post("/mcp") + .header("content-type", "application/json") + .body(rpc_body) + .send() + .await; + let parsed: Value = resp.0.into_body().into_json().await.unwrap(); + assert!( + parsed["error"]["message"] + .as_str() + .unwrap() + .contains("requires an explicit `project`"), + "create call omitting `project` with >1 project registered must error, not \ + silently file into the active project: {parsed}" + ); + } + + #[tokio::test] + async fn read_tool_without_project_annotates_resolved_project() { + let dir = tempfile::tempdir().unwrap(); + let state = make_two_project_state(dir.path()).await; + let app = poem::Route::new() + .at("/mcp", poem::post(gateway_mcp_post_handler)) + .data(state); + let cli = poem::test::TestClient::new(app); + + let rpc_body = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "get_pipeline_status", + "arguments": {} + } + })) + .unwrap(); + let resp = cli + .post("/mcp") + .header("content-type", "application/json") + .body(rpc_body) + .send() + .await; + let parsed: Value = resp.0.into_body().into_json().await.unwrap(); + assert_eq!( + parsed["result"]["content"][0]["text"], "handled_by:alpha", + "sanity: implicit routing hit the active project: {parsed}" + ); + assert_eq!( + parsed["result"]["resolved_project"], "alpha", + "omitting `project` must echo which project was actually resolved: {parsed}" + ); + } + + #[test] + fn annotate_resolved_project_inserts_field_into_result_object() { + let bytes = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "result": { "content": [{ "type": "text", "text": "ok" }] } + })) + .unwrap(); + let annotated = annotate_resolved_project(&bytes, "alpha"); + let parsed: Value = serde_json::from_slice(&annotated).unwrap(); + assert_eq!(parsed["result"]["resolved_project"], "alpha"); + assert_eq!(parsed["result"]["content"][0]["text"], "ok"); + } + + #[test] + fn annotate_resolved_project_leaves_error_response_untouched() { + let bytes = serde_json::to_vec(&json!({ + "jsonrpc": "2.0", + "id": 1, + "error": { "code": -32603, "message": "boom" } + })) + .unwrap(); + let annotated = annotate_resolved_project(&bytes, "alpha"); + let parsed: Value = serde_json::from_slice(&annotated).unwrap(); + assert_eq!(parsed["error"]["message"], "boom"); + assert!(parsed.get("result").is_none()); + } + + #[test] + fn is_create_tool_matches_create_prefixed_names_only() { + assert!(is_create_tool("create_bug")); + assert!(is_create_tool("create_story")); + assert!(is_create_tool("create_worktree")); + assert!(!is_create_tool("get_pipeline_status")); + assert!(!is_create_tool("show")); + } }