From 7824e558cb305d950ef4d02b5925d206b4722c99 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Mon, 11 May 2026 00:39:29 -0400
Subject: [PATCH] docs: refine fabro mcp implementation plan

---
 docs/plans/2026-05-11-add-fabro-mcp-server.md | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/docs/plans/2026-05-11-add-fabro-mcp-server.md b/docs/plans/2026-05-11-add-fabro-mcp-server.md
index 89a75fb23..dce6ea220 100644
--- a/docs/plans/2026-05-11-add-fabro-mcp-server.md
+++ b/docs/plans/2026-05-11-add-fabro-mcp-server.md
@@ -140,7 +140,9 @@ struct CreateRunSpec {
     cwd: Option<PathBuf>,
     run_id: Option<String>,
     goal: Option<String>,
+    #[serde(default)]
     inputs: HashMap<String, serde_json::Value>,
+    #[serde(default)]
     labels: HashMap<String, String>,
     dry_run: Option<bool>,
     auto_approve: Option<bool>,
@@ -790,8 +792,8 @@ use std::sync::Arc;
 
 use anyhow::Result;
 use rmcp::{
-    ServerHandler, serve_server,
-    handler::server::router::tool::ToolRouter,
+    Json, ServerHandler, serve_server,
+    handler::server::{router::tool::ToolRouter, wrapper::Parameters},
     model::{ServerCapabilities, ServerInfo},
     tool, tool_handler, tool_router,
     transport::stdio,
@@ -814,14 +816,19 @@ pub(crate) struct FabroMcpServer {
 pub(crate) async fn start(args: McpStartArgs, base_ctx: &CommandContext) -> Result<()> {
     let ctx = Arc::new(base_ctx.with_connection(&args.connection)?);
     let server = FabroMcpServer::new(ctx);
-    serve_server(server, stdio()).await?;
+    let service = serve_server(server, stdio()).await?;
+    service.waiting().await?;
     Ok(())
 }
 ```
 
-Implement `ServerHandler`:
+Implement `ServerHandler` through the `#[tool_handler]` impl, not a separate
+plain impl. `rmcp::serve_server(...)` returns after initialization with a
+running service handle; `fabro mcp start` must await `service.waiting()` so the
+stdio process stays alive for later `tools/list` and `tools/call` requests.
 
 ```rust
+#[tool_handler(router = self.tool_router)]
 impl ServerHandler for FabroMcpServer {
     fn get_info(&self) -> ServerInfo {
         ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
@@ -846,8 +853,6 @@ impl FabroMcpServer {
     }
 }
 
-#[tool_handler(router = self.tool_router)]
-impl ServerHandler for FabroMcpServer { ... }
 ```
 
 Each placeholder in `run_tools.rs` should return `Err("not implemented".to_string())` until later tasks, except it must compile and be listed.