diff --git a/src/mcp_prompts.rs b/src/mcp_prompts.rs index 044ad988..6a8eede1 100644 --- a/src/mcp_prompts.rs +++ b/src/mcp_prompts.rs @@ -175,12 +175,14 @@ fn get_handoff_command( assistant_text(format!( "Handoff command: `{command}`\n\n\ Grammar: first word is a subcommand (`inbox`, `thread`) or a recipient; the rest is the brief.\n\ - - ` ` → artifact_publish with recipient=, sender={me}, \ - a fresh thread_id (or the current one when continuing an exchange), name from the brief, \ - and content = the work product the brief points at (the preceding conversation content, \ - diff, review, or document — ask only if genuinely ambiguous). Prepend the brief to the \ - content so the recipient knows what is being asked. When answering an item from your \ - inbox, set reply_to= and reuse its thread_id.\n\ + - ` ` → call the `handoff` tool with recipient=, \ + name from the brief, content = the work product the brief points at (the preceding \ + conversation content, diff, review, or document — ask only if genuinely ambiguous), \ + and a thread_id when continuing an exchange. Prepend the brief to the content so the \ + recipient knows what is being asked (sender is set to your identity, {me}, \ + automatically). Use the `handoff` tool, not artifact_publish, so recipient can't be \ + omitted. When answering an item from your inbox, set reply_to= and \ + reuse its thread_id.\n\ - `inbox [me]` → artifact_list with recipient=; summarize sender, \ name, and brief for each.\n\ - `thread ` → artifact_list with thread_id=; present in chronological order with \ @@ -259,7 +261,7 @@ mod tests { let result = get_prompt(¶ms, None).unwrap(); let text = format!("{:?}", result.messages[0].content); assert!(text.contains("codex review the API design above"), "{text}"); - assert!(text.contains("artifact_publish"), "{text}"); + assert!(text.contains("`handoff` tool"), "{text}"); assert!(text.contains("recipient"), "{text}"); assert!(text.contains("reply_to"), "{text}"); } @@ -288,7 +290,7 @@ mod tests { let params = GetPromptRequestParams::new("handoff").with_arguments(args); let result = get_prompt(¶ms, Some("opencode")).unwrap(); let text = format!("{:?}", result.messages[0].content); - assert!(text.contains("sender=opencode"), "{text}"); + assert!(text.contains("identity, opencode"), "{text}"); assert!(text.contains("recipient="), "{text}"); assert!(!text.contains("claude-code"), "{text}"); } @@ -301,7 +303,7 @@ mod tests { let params = GetPromptRequestParams::new("handoff").with_arguments(args); let result = get_prompt(¶ms, None).unwrap(); let text = format!("{:?}", result.messages[0].content); - assert!(text.contains("sender=claude-code"), "{text}"); + assert!(text.contains("identity, claude-code"), "{text}"); } #[test] diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 0ed1dad5..7c451360 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -184,6 +184,35 @@ struct ArtifactPublishRequest { reply_to: Option, } +/// A handoff is an artifact routed to another agent's inbox. Unlike +/// `ArtifactPublishRequest`, `recipient` is a required field, not `Option` — +/// the schema itself makes an unaddressed handoff unrepresentable, so an +/// intended handoff can't silently land in no inbox. +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct HandoffRequest { + #[schemars(description = "Agent/runtime this handoff is addressed to — its inbox (artifact_list recipient=...). Required.")] + recipient: String, + #[schemars(description = "Short name/brief for the handoff, shown in the recipient's inbox")] + name: String, + #[schemars(description = "The work product being handed off (diff, review, document, ...). Prepend the brief so the recipient knows the ask.")] + content: String, + #[schemars(description = "html | markdown | mermaid | diagram | text (default: markdown)")] + #[serde(default)] + r#type: Option, + #[schemars(description = "Handoff thread to continue; omit to start a new one")] + #[serde(default)] + thread_id: Option, + #[schemars(description = "Artifact id this replies to (when answering an inbox item)")] + #[serde(default)] + reply_to: Option, + #[schemars(description = "Session ID for grouping (optional)")] + #[serde(default)] + session_id: Option, + #[schemars(description = "One-line description shown in the gallery")] + #[serde(default)] + description: Option, +} + #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct ArtifactListRequest { #[schemars(description = "Only artifacts from this session (omit for all)")] @@ -575,6 +604,64 @@ impl AgentflareMcp { Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) } + #[tool(description = "Hand a work product to another agent's inbox. Like artifact_publish, but recipient is REQUIRED, so the artifact is routed and shows up in that agent's artifact_list(recipient=...) inbox — it can't silently land nowhere. Use for agent-to-agent handoffs (reviews, diffs, docs); use artifact_publish for plain shareable pages. Sender is this runtime's own identity.")] + fn handoff( + &self, + Parameters(HandoffRequest { + recipient, + name, + content, + r#type, + thread_id, + reply_to, + session_id, + description, + }): Parameters, + ) -> Result { + if recipient.trim().is_empty() { + return Err(ErrorData::invalid_params( + "recipient is required for a handoff — without it the artifact lands in no inbox", + None, + )); + } + if name.trim().is_empty() { + return Err(ErrorData::invalid_params("name is required", None)); + } + if content.is_empty() { + return Err(ErrorData::invalid_params("content is required", None)); + } + let recipient = recipient.trim().to_string(); + let name = name.trim().to_string(); + let (store, base) = self.ensure_artifact_server()?; + let req = agentflare_artifacts::PublishRequest { + name, + artifact_type: agentflare_artifacts::ArtifactType::from( + r#type.as_deref().unwrap_or("markdown"), + ), + content, + session_id: session_id.unwrap_or_default(), + update_id: None, + label: None, + description, + favicon: None, + base_version: None, + sender: self.agent.clone(), + recipient: Some(recipient), + thread_id, + reply_to, + git: Self::git_provenance(), + }; + let resp = store.publish(&req).map_err(Self::artifact_error)?; + let result = serde_json::json!({ + "id": resp.id, + "version": resp.version, + "url": format!("{base}/{}", resp.id), + "index": format!("{base}/"), + "recipient": req.recipient, + }); + Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) + } + /// Best-effort git context of this process's cwd (the project the MCP /// server was launched in). None outside a repo; never fails a publish. pub(crate) fn git_provenance() -> Option { @@ -1719,6 +1806,81 @@ mod tests { assert_eq!(thread.as_array().unwrap().len(), 2); } + #[test] + fn handoff_tool_requires_recipient_and_routes_to_inbox() { + let tmp = tempfile::tempdir().unwrap(); + let s = AgentflareMcp { + artifacts_dir_override: Some(tmp.path().to_path_buf()), + agent: Some("claude-code".into()), + ..Default::default() + }; + + // A blank recipient is rejected — the whole reason this tool exists. + let err = s + .handoff(Parameters(HandoffRequest { + recipient: " ".into(), + name: "orphan".into(), + content: "for someone".into(), + ..Default::default() + })) + .unwrap_err(); + assert_eq!(err.code, rmcp::model::ErrorCode::INVALID_PARAMS); + + // A real handoff lands in the recipient's inbox; sender is our identity. + s.handoff(Parameters(HandoffRequest { + recipient: "opencode".into(), + name: "review-packet".into(), + content: "please review".into(), + ..Default::default() + })) + .unwrap(); + + let inbox: serde_json::Value = serde_json::from_str( + &s.artifact_list(Parameters(ArtifactListRequest { + recipient: Some("opencode".into()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(inbox.as_array().unwrap().len(), 1); + assert_eq!(inbox[0]["name"], "review-packet"); + assert_eq!(inbox[0]["sender"], "claude-code"); + assert_eq!(inbox[0]["recipient"], "opencode"); + } + + #[test] + fn handoff_trims_whitespace_padded_recipient() { + let tmp = tempfile::tempdir().unwrap(); + let s = AgentflareMcp { + artifacts_dir_override: Some(tmp.path().to_path_buf()), + agent: Some("claude-code".into()), + ..Default::default() + }; + + // A whitespace-padded recipient passes the emptiness check but must + // still be stored trimmed, or exact-match inbox lookups miss it. + s.handoff(Parameters(HandoffRequest { + recipient: " opencode ".into(), + name: "review-packet".into(), + content: "please review".into(), + ..Default::default() + })) + .unwrap(); + + let inbox: serde_json::Value = serde_json::from_str( + &s.artifact_list(Parameters(ArtifactListRequest { + recipient: Some("opencode".into()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(inbox.as_array().unwrap().len(), 1); + assert_eq!(inbox[0]["name"], "review-packet"); + assert_eq!(inbox[0]["recipient"], "opencode"); + } + #[test] fn artifact_diff_tool_returns_unified_diff() { let tmp = tempfile::tempdir().unwrap();