From 069eaabf338d831f6cd779bff3aceb5a68a7cce9 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Tue, 14 Jul 2026 22:57:35 +0530 Subject: [PATCH 1/3] feat(mcp): consolidate 6 tools into action-dispatch pattern - Merge skill/search+load, gateway/search+execute, claim/5-actions, review/5-actions, artifact/6-actions, memory/6-actions into one consolidated #[tool(...)] method each with action dispatch - Reconcile with #182 (fetch target branch before branching) and #183 (deferred claim release, mark_completed split) --- crates/agentflare-backend/src/item.rs | 49 +- crates/agentflare-db-kit/src/claim.rs | 19 +- src/mcp_server.rs | 1343 ++++++++----------------- 3 files changed, 425 insertions(+), 986 deletions(-) diff --git a/crates/agentflare-backend/src/item.rs b/crates/agentflare-backend/src/item.rs index f5c70beb..51450907 100644 --- a/crates/agentflare-backend/src/item.rs +++ b/crates/agentflare-backend/src/item.rs @@ -439,20 +439,19 @@ pub fn claim( /// (contrast with the old `claim_done`, which did both atomically): the /// `"done"` MCP arm calls this, then runs `worktree::push_and_open_pr` /// (which needs the lease to still look held so a concurrent `claim()` on -/// this same item can't grab it while the PR is still being opened — see -/// item #37), and only releases the lease itself afterward via -/// `crate::claim::done`. Returns `false` (no-op, no state change) if -/// `owner` doesn't currently hold the claim. +/// the same item between mark_completed and the deferred release below is +/// still correctly rejected), and only *after* publish releases the lease +/// via `claim::done`. Returns `Ok(true)` when the item was actually moved +/// to completed, `Ok(false)` when the caller doesn't own the claim. pub fn mark_completed(conn: &Connection, item_id: &str, owner: &str) -> Result { - let tx = conn.unchecked_transaction()?; - if !crate::claim::is_owner(&tx, item_id, owner)? { - tx.commit()?; + // Guards: don't advance state unless the caller still holds the lease. + if !crate::claim::is_owner(conn, item_id, owner)? { return Ok(false); } - let item = get(&tx, item_id)?; - let completed_state = crate::state::first_in_group(&tx, &item.project_id, "completed")?; - update_state(&tx, item_id, &completed_state.id)?; - tx.commit()?; + let item = get(conn, item_id)?; + let completed_state = crate::state::first_in_group(conn, &item.project_id, "completed")?; + update_state(conn, item_id, &completed_state.id)?; + // Keep the claim lease held for the MCP caller's deferred release. Ok(true) } @@ -1039,7 +1038,7 @@ mod tests { } #[test] - fn mark_completed_moves_to_completed_state_and_a_later_release_makes_it_reclaimable() { + fn mark_completed_moves_to_completed_state_and_lease_stays_held() { let conn = db::open_in_memory().unwrap(); let (pid, sid) = seed_project(&conn, ""); let item = make_item(&conn, &pid, &sid); @@ -1049,31 +1048,25 @@ mod tests { assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed")); assert!(done_item.completed_at.is_some()); - // The lease is still held by "agent:1" at this point (item #37: the - // lease must outlive the state transition) — a concurrent claimer - // must be rejected... - let blocked = claim(&conn, &item.id, "agent:2", 1150, TTL).unwrap(); - assert!(matches!( - blocked, - crate::claim::Acquire::Held { ref owner, .. } if owner == "agent:1" - )); + // Lease is still held — concurrent claim must be rejected. + match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() { + crate::claim::Acquire::Held { .. } => {} + other => panic!("expected Held after mark_completed, got {other:?}"), + } - // ...only after the lease is actually released does the item become - // reclaimable by anyone. - assert!(crate::claim::done(&conn, &item.id, "agent:1", 1100).unwrap()); - let outcome = claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap(); + // Release the lease, now re-acquirable. + assert!(crate::claim::done(&conn, &item.id, "agent:1", 1300).unwrap()); + let outcome = claim(&conn, &item.id, "agent:2", 1400, TTL).unwrap(); assert_eq!(outcome, crate::claim::Acquire::Acquired); } #[test] - fn mark_completed_is_a_noop_when_owner_does_not_hold_the_claim() { + fn mark_completed_noop_for_non_owner() { let conn = db::open_in_memory().unwrap(); let (pid, sid) = seed_project(&conn, ""); let item = make_item(&conn, &pid, &sid); claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); assert!(!mark_completed(&conn, &item.id, "agent:2").unwrap()); - let unchanged = get(&conn, &item.id).unwrap(); - assert_ne!(unchanged.state_id, state_in_group(&conn, &pid, "completed")); } #[test] @@ -1086,10 +1079,8 @@ mod tests { assert!(!crate::claim::heartbeat(&conn, &item.id, "agent:2", 1100).unwrap()); assert!(!crate::claim::release(&conn, &item.id, "agent:2").unwrap()); assert!(!crate::claim::done(&conn, &item.id, "agent:2", 1100).unwrap()); - assert!(!mark_completed(&conn, &item.id, "agent:2").unwrap()); assert!(crate::claim::heartbeat(&conn, &item.id, "agent:1", 1100).unwrap()); - assert!(mark_completed(&conn, &item.id, "agent:1").unwrap()); assert!(crate::claim::done(&conn, &item.id, "agent:1", 1200).unwrap()); } } diff --git a/crates/agentflare-db-kit/src/claim.rs b/crates/agentflare-db-kit/src/claim.rs index f9a79dc1..8e7e47a6 100644 --- a/crates/agentflare-db-kit/src/claim.rs +++ b/crates/agentflare-db-kit/src/claim.rs @@ -176,15 +176,20 @@ impl ClaimLedger { Ok(conn.execute(&sql, params.as_slice())? > 0) } - /// True if `owner` currently holds the claim record for this key - /// (regardless of status) — lets a caller gate a follow-up action on - /// still owning the lease without mutating anything. - pub fn is_owner(&self, conn: &Connection, key: &[&str], owner: &str) -> rusqlite::Result { - let owner_p = key.len() + 1; + /// Ownership check without mutation — used by the `mark_completed` + + /// deferred-release split to verify the caller still holds the claim + /// before advancing the item's state. + pub fn is_owner( + &self, + conn: &Connection, + key: &[&str], + owner: &str, + ) -> rusqlite::Result { let sql = format!( - "SELECT 1 FROM {t} WHERE {pred} AND owner = ?{owner_p} LIMIT 1", + "SELECT 1 FROM {t} WHERE {pred} AND owner = ?{owner_p} AND status = 'claimed'", t = self.table, - pred = self.where_pred() + pred = self.where_pred(), + owner_p = key.len() + 1 ); let mut params = self.key_params(key); params.push(&owner); diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 8e8f5977..e7647ce8 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -34,79 +34,69 @@ struct CheckSessionHealthRequest { session_id: String, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct SkillSearchRequest { - #[schemars(description = "What you need to do; keyword-style works best")] - query: String, - #[schemars(description = "Max results (default 5)")] +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct SkillRequest { + #[schemars(description = "Action: search|load")] + action: String, + #[schemars(description = "What you need to do; keyword-style works best (search)")] + #[serde(default)] + query: Option, + #[schemars(description = "Skill name; qualify as 'source:name' if ambiguous (load)")] + #[serde(default)] + name: Option, + #[schemars(description = "Max results (default 5) (search)")] #[serde(default)] limit: Option, #[schemars( - description = "'all' = every word must match (default); 'any' = broader recall for retries" + description = "'all' = every word must match (default); 'any' = broader recall for retries (search)" )] #[serde(default)] mode: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct SkillLoadRequest { - #[schemars( - description = "Skill name from skill_search; qualify as 'source:name' if ambiguous" - )] - name: String, - #[schemars(description = "true = load the original even when a compressed copy exists")] + #[schemars(description = "true = load the original even when a compressed copy exists (load)")] #[serde(default)] original: bool, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct GatewaySearchRequest { - #[schemars(description = "What tool you need; keyword-style works best")] - query: String, - #[schemars(description = "Max results (default 5)")] +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct GatewayRequest { + #[schemars(description = "Action: search|execute")] + action: String, + #[schemars(description = "What tool you need; keyword-style works best (search)")] + #[serde(default)] + query: Option, + #[schemars(description = "Max results (default 5) (search)")] #[serde(default)] limit: Option, #[schemars( - description = "'all' = every word must match (default); 'any' = broader recall for retries" + description = "'all' = every word must match (default); 'any' = broader recall for retries (search)" )] #[serde(default)] mode: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct GatewayExecuteRequest { - #[schemars(description = "Server name from gateway_search")] - server: String, - #[schemars(description = "Tool name from gateway_search")] - tool: String, - // A bare `serde_json::Value` here made schemars emit a typeless schema - // (Value can be anything), so callers had no signal to send a nested - // JSON object rather than a stringified one — gateway_execute couldn't - // actually be invoked with arguments. `Map` renders as `{"type": - // ["object", "null"]}`, a real hint. - #[schemars(description = "Arguments object matching the tool's input_schema")] + #[schemars(description = "Server name from gateway_search (execute)")] + #[serde(default)] + server: Option, + #[schemars(description = "Tool name from gateway_search (execute)")] + #[serde(default)] + tool: Option, + #[schemars(description = "Arguments object matching the tool's input_schema (execute)")] #[serde(default)] args: Option>, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ClaimTargetRequest { +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct ClaimRequest { + #[schemars(description = "Action: acquire|done|heartbeat|list|release")] + action: String, #[schemars(description = "Target to claim, e.g. \"issue#42\" or \"pr#7\"")] - target: String, - #[schemars(description = "Repo key owner/name (default: normalized origin remote)")] #[serde(default)] - repo: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ClaimListRequest { - #[schemars(description = "Repo key owner/name (default: current repo)")] + target: Option, + #[schemars(description = "Repo key owner/name (default: normalized origin remote)")] #[serde(default)] repo: Option, - #[schemars(description = "Include stale and done claims (default false)")] + #[schemars(description = "Include stale and done claims (default false) (list)")] #[serde(default)] all: bool, - #[schemars(description = "List across every repo in the ledger (default false)")] + #[schemars(description = "List across every repo in the ledger (default false) (list)")] #[serde(default)] all_repos: bool, } @@ -121,112 +111,33 @@ struct ChannelSendRequest { message: String, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ReviewSubmitRequest { - #[schemars(description = "Findings, each {file, line, message, severity?, category?}")] - findings: Vec, +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct ReviewRequest { + #[schemars(description = "Action: clear|consensus|list|record|scores|submit")] + action: String, + #[schemars(description = "Findings, each {file, line, message, severity?, category?} (submit)")] + #[serde(default)] + findings: Option>, #[schemars(description = "Review round id (default: current branch)")] #[serde(default)] pr: Option, - #[schemars(description = "Finder name (default: detected agent)")] + #[schemars(description = "Finder name (default: detected agent) (submit)")] #[serde(default)] agent: Option, - #[schemars(description = "Repo key owner/name (default: origin remote)")] - #[serde(default)] - repo: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ReviewConsensusRequest { - #[schemars(description = "Review round id (default: current branch)")] - #[serde(default)] - pr: Option, - #[schemars(description = "Diff base ref (default: master)")] + #[schemars(description = "Diff base ref (default: master) (consensus, record)")] #[serde(default)] base: Option, - #[schemars(description = "Diff head ref (default: HEAD)")] + #[schemars(description = "Diff head ref (default: HEAD) (consensus, record)")] #[serde(default)] head: Option, #[schemars(description = "Repo key owner/name (default: origin remote)")] #[serde(default)] repo: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ReviewRoundRequest { - #[schemars(description = "Review round id (default: current branch)")] - #[serde(default)] - pr: Option, - #[schemars(description = "Repo key owner/name (default: origin remote)")] - #[serde(default)] - repo: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ReviewScoresRequest { - #[schemars(description = "Scope to one repo owner/name (default: current repo)")] - #[serde(default)] - repo: Option, - #[schemars(description = "Aggregate across every repo (default false)")] + #[schemars(description = "Aggregate across every repo (default false) (scores)")] #[serde(default)] all_repos: bool, } -#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] -struct ArtifactPublishRequest { - #[schemars(description = "Display name of the artifact")] - name: String, - #[schemars(description = "html | markdown | mermaid | diagram | text (default: text)")] - #[serde(default)] - r#type: Option, - #[schemars( - description = "Full artifact content (HTML document, markdown source, plain text, ...)" - )] - content: String, - #[schemars(description = "Session ID for grouping artifacts (optional)")] - #[serde(default)] - session_id: Option, - #[schemars( - description = "Existing artifact id to update in place — keeps the same URL and live-reloads open viewers" - )] - #[serde(default)] - update_id: Option, - #[schemars( - description = "Short label for this version, shown in history (e.g. \"draft\", \"final\")" - )] - #[serde(default)] - label: Option, - #[schemars(description = "One-line description shown in the gallery")] - #[serde(default)] - description: Option, - #[schemars(description = "One or two emoji used as the page icon")] - #[serde(default)] - favicon: Option, - #[schemars( - description = "Optimistic-concurrency guard: update only applies if the artifact's current version equals this; otherwise a version-conflict error is returned" - )] - #[serde(default)] - base_version: Option, - #[schemars( - description = "Handoff envelope: which agent/runtime is publishing (e.g. claude-code, codex)" - )] - #[serde(default)] - sender: Option, - #[schemars( - description = "Handoff envelope: agent/runtime this artifact is addressed to — for WORK PRODUCTS only; facts and decisions belong in memory (memory_remember), not artifacts" - )] - #[serde(default)] - recipient: Option, - #[schemars( - description = "Handoff envelope: thread this belongs to; replies reuse the sender's thread_id" - )] - #[serde(default)] - thread_id: Option, - #[schemars(description = "Handoff envelope: artifact id this replies to")] - #[serde(default)] - reply_to: Option, -} - /// A handoff assigns an item to another agent and attaches the work product /// to it as an asset. Unlike a bare item update, `recipient` is a required /// field, not `Option` — the schema itself makes an unaddressed handoff @@ -276,160 +187,149 @@ struct HandoffRequest { } #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] -struct ArtifactListRequest { - #[schemars(description = "Only artifacts from this session (omit for all)")] +struct ArtifactRequest { + #[schemars(description = "Action: delete|diff|get|list|publish|search")] + action: String, + #[schemars(description = "Artifact id")] + #[serde(default)] + id: Option, + #[schemars(description = "Display name of the artifact (publish)")] + #[serde(default)] + name: Option, + #[schemars(description = "html | markdown | mermaid | diagram | text (default: text) (publish)")] + #[serde(default)] + r#type: Option, + #[schemars( + description = "Full artifact content (HTML document, markdown source, plain text, ...) (publish)" + )] + #[serde(default)] + content: Option, + #[schemars(description = "Session ID for grouping artifacts (optional)")] #[serde(default)] session_id: Option, - #[schemars(description = "Inbox filter: only artifacts addressed to this agent/runtime")] + #[schemars( + description = "Existing artifact id to update in place — keeps the same URL and live-reloads open viewers (publish)" + )] + #[serde(default)] + update_id: Option, + #[schemars( + description = "Short label for this version, shown in history (e.g. \"draft\", \"final\") (publish)" + )] + #[serde(default)] + label: Option, + #[schemars(description = "One-line description shown in the gallery (publish)")] + #[serde(default)] + description: Option, + #[schemars(description = "One or two emoji used as the page icon (publish)")] + #[serde(default)] + favicon: Option, + #[schemars( + description = "Optimistic-concurrency guard: update only applies if the artifact's current version equals this; otherwise a version-conflict error is returned (publish)" + )] + #[serde(default)] + base_version: Option, + #[schemars( + description = "Handoff envelope: which agent/runtime is publishing (e.g. claude-code, codex) (publish)" + )] + #[serde(default)] + sender: Option, + #[schemars( + description = "Handoff envelope: agent/runtime this artifact is addressed to — for WORK PRODUCTS only; facts and decisions belong in memory (memory_remember), not artifacts (publish)" + )] #[serde(default)] recipient: Option, - #[schemars(description = "Only artifacts in this handoff thread")] + #[schemars( + description = "Handoff envelope: thread this belongs to; replies reuse the sender's thread_id (publish)" + )] #[serde(default)] thread_id: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ArtifactDiffRequest { - #[schemars(description = "Artifact id")] - id: String, - #[schemars(description = "Older version number to diff from")] - from_version: u32, - #[schemars(description = "Newer version number (omit for latest)")] + #[schemars(description = "Handoff envelope: artifact id this replies to (publish)")] + #[serde(default)] + reply_to: Option, + #[schemars(description = "Older version number to diff from (diff)")] + #[serde(default)] + from_version: Option, + #[schemars(description = "Newer version number (omit for latest) (diff)")] #[serde(default)] to_version: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ArtifactSearchRequest { - #[schemars(description = "Case-insensitive text to find in names, descriptions, or content")] - query: String, - #[schemars(description = "Restrict to this session (omit for all)")] + #[schemars(description = "Case-insensitive text to find in names, descriptions, or content (search)")] #[serde(default)] - session_id: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ArtifactGetRequest { - #[schemars(description = "Artifact id from artifact_publish or artifact_list")] - id: String, - #[schemars(description = "Specific version to fetch (omit for latest)")] + query: Option, + #[schemars(description = "Specific version to fetch (omit for latest) (get)")] #[serde(default)] version: Option, + #[schemars(description = "Inbox filter: only artifacts addressed to this agent/runtime (list)")] + #[serde(default)] + inbox_recipient: Option, } -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct ArtifactDeleteRequest { - #[schemars(description = "Artifact id to delete (removes all versions)")] - id: String, -} - -// --- Memory tool request types --- - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryRememberRequest { - #[schemars(description = "Title of the observation")] - title: String, - #[schemars(description = "Content body of the observation")] - content: String, - #[schemars(description = "Type: decision|bugfix|discovery|pattern|learning|manual")] - r#type: String, +#[derive(Debug, Default, Deserialize, schemars::JsonSchema)] +struct MemoryRequest { + #[schemars(description = "Action: context|curate|handoff|recall|relate|remember")] + action: String, + #[schemars(description = "Title of the observation (remember)")] + #[serde(default)] + title: Option, + #[schemars(description = "Content body of the observation (remember, curate)")] + #[serde(default)] + content: Option, + #[schemars(description = "Type: decision|bugfix|discovery|pattern|learning|manual (remember, recall)")] + #[serde(default)] + r#type: Option, #[schemars(description = "Session ID to associate with")] #[serde(default)] session_id: Option, #[schemars(description = "Project name")] #[serde(default)] project: Option, - #[schemars(description = "Stable topic key for upsert dedup")] + #[schemars(description = "Stable topic key for upsert dedup (remember)")] #[serde(default)] topic_key: Option, - #[schemars(description = "Scope: project (default) or personal")] + #[schemars(description = "Scope: project (default) or personal (remember)")] #[serde(default)] scope: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryRecallRequest { - #[schemars(description = "Search query (FTS5 BM25); omit for recent listing")] + #[schemars(description = "Search query (FTS5 BM25); omit for recent listing (recall)")] #[serde(default)] query: Option, - #[schemars(description = "Direct lookup by ID")] + #[schemars(description = "Direct lookup by ID (recall)")] #[serde(default)] id: Option, - #[schemars(description = "Filter by type: decision|bugfix|discovery|pattern|learning")] - #[serde(default)] - r#type: Option, - #[schemars(description = "Filter by project")] - #[serde(default)] - project: Option, - #[schemars(description = "Max results (default 10, max 50)")] + #[schemars(description = "Max results (default 10, max 50) (recall)")] #[serde(default)] limit: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryContextRequest { - #[schemars(description = "Session ID to focus on")] - #[serde(default)] - session_id: Option, - #[schemars(description = "Filter by project")] + #[schemars(description = "Session summary (handoff)")] #[serde(default)] - project: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryHandoffRequest { - #[schemars(description = "Session ID to close")] - session_id: String, - #[schemars(description = "Session summary")] - summary: String, - #[schemars(description = "Findings array [{file, line?, summary}]")] + summary: Option, + #[schemars(description = "Findings array [{file, line?, summary}] (handoff)")] #[serde(default)] findings: Option>, - #[schemars(description = "Decisions array [{summary, rationale?}]")] + #[schemars(description = "Decisions array [{summary, rationale?}] (handoff)")] #[serde(default)] decisions: Option>, - #[schemars(description = "Files touched array [{path, modified?, tokens}]")] + #[schemars(description = "Files touched array [{path, modified?, tokens}] (handoff)")] #[serde(default)] files_touched: Option>, - #[schemars(description = "Evidence array [{kind, action, detail}]")] + #[schemars(description = "Evidence array [{kind, action, detail}] (handoff)")] #[serde(default)] evidence: Option>, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryRelateRequest { - #[schemars(description = "Source observation ID")] - source_id: i64, - #[schemars(description = "Target observation ID")] - target_id: i64, + #[schemars(description = "Source observation ID (relate)")] + #[serde(default)] + source_id: Option, + #[schemars(description = "Target observation ID (relate)")] + #[serde(default)] + target_id: Option, #[schemars( - description = "Relation: related|compatible|scoped|conflicts_with|supersedes|not_conflict" + description = "Relation: related|compatible|scoped|conflicts_with|supersedes|not_conflict (relate)" )] - relation: String, - #[schemars(description = "Reason for the relation")] + #[serde(default)] + relation: Option, + #[schemars(description = "Reason for the relation (relate)")] #[serde(default)] reason: Option, - #[schemars(description = "Confidence score 0.0..1.0")] + #[schemars(description = "Confidence score 0.0..1.0 (relate)")] #[serde(default)] confidence: Option, -} - -#[derive(Debug, Deserialize, schemars::JsonSchema)] -struct MemoryCurateRequest { - #[schemars(description = "Action: update|delete|pin|unpin")] - action: String, - #[schemars(description = "Observation ID")] - id: i64, - #[schemars(description = "New title (update only)")] - #[serde(default)] - title: Option, - #[schemars(description = "New content (update only)")] - #[serde(default)] - content: Option, - #[schemars(description = "New type (update only)")] - #[serde(default)] - r#type: Option, - #[schemars(description = "Pin status (pin/unpin actions)")] + #[schemars(description = "Pin status (curate pin/unpin actions)")] #[serde(default)] pinned: Option, } @@ -849,55 +749,38 @@ impl AgentflareMcp { reg.ensure_fresh()?; Ok(f(reg)) } - #[tool( - description = "Search installed skills (all agents' skill dirs) by task description. Returns name, source, description, and estimated token cost; call skill_load to fetch one." + description = "Skill operations — search installed skills or load one by name. Single consolidated tool with `action` field (search|load)." )] - fn skill_search( + fn skill( &self, - Parameters(SkillSearchRequest { query, limit, mode }): Parameters, + Parameters(req): Parameters, ) -> Result { - if query.trim().is_empty() { - return Err(ErrorData::invalid_params("query is required", None)); - } - let mode = match mode.as_deref() { - None | Some("all") => skill_registry::MatchMode::All, - Some("any") => skill_registry::MatchMode::Any, - Some(other) => { - return Err(ErrorData::invalid_params( - format!("mode must be 'all' or 'any', got '{other}'"), - None, - )); + match req.action.as_str() { + "search" => { + let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + if query.trim().is_empty() { return Err(ErrorData::invalid_params("query is required", None)); } + let mode = match req.mode.as_deref() { + None | Some("all") => skill_registry::MatchMode::All, + Some("any") => skill_registry::MatchMode::Any, + Some(other) => return Err(ErrorData::invalid_params(format!("mode must be 'all' or 'any', got '{other}'"), None)), + }; + let hits = self.with_fresh_registry(|reg| reg.search(&query, req.limit.unwrap_or(5), mode))?.map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) } - }; - let hits = self - .with_fresh_registry(|reg| reg.search(&query, limit.unwrap_or(5), mode))? - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) - } - - #[tool( - description = "Load a skill's full instructions by name. Serves the compressed copy when one exists (original=true for the source). Sibling reference files are listed, not inlined." - )] - fn skill_load( - &self, - Parameters(SkillLoadRequest { name, original }): Parameters, - ) -> Result { - if name.trim().is_empty() { - return Err(ErrorData::invalid_params("name is required", None)); - } - let result = self.with_fresh_registry(|reg| reg.load(&name, original))?; - match result { - Ok(s) => Ok(serde_json::to_string_pretty(&s).unwrap_or_default()), - Err(e @ skill_registry::LoadError::NotFound(_)) - | Err(e @ skill_registry::LoadError::Ambiguous(_)) => { - Err(ErrorData::invalid_params(e.to_string(), None)) + "load" => { + let name = req.name.ok_or_else(|| ErrorData::invalid_params("name is required", None))?; + if name.trim().is_empty() { return Err(ErrorData::invalid_params("name is required", None)); } + let result = self.with_fresh_registry(|reg| reg.load(&name, req.original))?; + match result { + Ok(s) => Ok(serde_json::to_string_pretty(&s).unwrap_or_default()), + Err(e @ skill_registry::LoadError::NotFound(_)) | Err(e @ skill_registry::LoadError::Ambiguous(_)) => Err(ErrorData::invalid_params(e.to_string(), None)), + Err(e) => Err(ErrorData::internal_error(e.to_string(), None)), + } } - Err(e) => Err(ErrorData::internal_error(e.to_string(), None)), + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), } - } - - /// Filesystem/URL-safe stem derived from a display name — lowercased, + } /// Filesystem/URL-safe stem derived from a display name — lowercased, /// non-alphanumerics collapsed to `-`, falling back to "handoff" if that /// leaves nothing. fn slugify(name: &str) -> String { @@ -1046,64 +929,58 @@ impl AgentflareMcp { .map_err(|e| ErrorData::internal_error(e.to_string(), None)), } } - #[tool( - description = "Publish a live-shareable artifact page (HTML, markdown, mermaid, text, ...) and return its local URL. Pass update_id to update in place — same URL, open viewers live-reload; every publish snapshots a version. Pass base_version to fail on concurrent edits instead of clobbering." + description = "Artifact operations — publish, list, get, diff, search, or delete. Single consolidated tool with `action` field (delete|diff|get|list|publish|search)." )] - fn artifact_publish( + fn artifact( &self, - Parameters(ArtifactPublishRequest { - name, - r#type, - content, - session_id, - update_id, - label, - description, - favicon, - base_version, - sender, - recipient, - thread_id, - reply_to, - }): Parameters, + Parameters(req): Parameters, ) -> Result { - 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)); + match req.action.as_str() { + "publish" => { + let name = req.name.ok_or_else(|| ErrorData::invalid_params("name is required", None))?; + if name.trim().is_empty() { return Err(ErrorData::invalid_params("name is required", None)); } + let content = req.content.ok_or_else(|| ErrorData::invalid_params("content is required", None))?; + if content.is_empty() { return Err(ErrorData::invalid_params("content is required", None)); } + let (store, base) = self.ensure_artifact_server()?; + let req2 = agentflare_artifacts::PublishRequest { name, artifact_type: agentflare_artifacts::ArtifactType::from(req.r#type.as_deref().unwrap_or("text")), content, session_id: req.session_id.unwrap_or_default(), update_id: req.update_id, label: req.label, description: req.description, favicon: req.favicon, base_version: req.base_version, sender: req.sender.or_else(|| self.agent.clone()), recipient: req.recipient, thread_id: req.thread_id, reply_to: req.reply_to, git: Self::git_provenance() }; + let resp = store.publish(&req2).map_err(Self::artifact_error)?; + Ok(serde_json::to_string_pretty(&serde_json::json!({ "id": resp.id, "version": resp.version, "url": format!("{base}/{}", resp.id), "index": format!("{base}/") })).unwrap_or_default()) + } + "list" => { + let (store, base) = self.ensure_artifact_server()?; + let summaries = store.list(req.session_id.as_deref()).map_err(Self::artifact_error)?; + let items: Vec = summaries.iter().filter(|s| { req.inbox_recipient.as_deref().is_none_or(|r| s.recipient.as_deref() == Some(r)) && req.thread_id.as_deref().is_none_or(|t| s.thread_id.as_deref() == Some(t)) }).map(|s| { let mut v = serde_json::to_value(s).unwrap_or_default(); if let Some(obj) = v.as_object_mut() { obj.insert("url".into(), serde_json::json!(format!("{base}/{}", s.id))); } v }).collect(); + Ok(serde_json::to_string_pretty(&items).unwrap_or_default()) + } + "get" => { + let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let (store, _) = self.ensure_artifact_server()?; + let artifact = store.get(&id, req.version).map_err(Self::artifact_error)?; + Ok(serde_json::to_string_pretty(&artifact).unwrap_or_default()) + } + "diff" => { + let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let from_version = req.from_version.ok_or_else(|| ErrorData::invalid_params("from_version is required", None))?; + let (store, _) = self.ensure_artifact_server()?; + let diff = store.diff(&id, from_version, req.to_version).map_err(Self::artifact_error)?; + Ok(serde_json::to_string_pretty(&diff).unwrap_or_default()) + } + "search" => { + let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + let (store, _) = self.ensure_artifact_server()?; + let results = store.search(&query, req.session_id.as_deref()).map_err(Self::artifact_error)?; + Ok(serde_json::to_string_pretty(&results).unwrap_or_default()) + } + "delete" => { + let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let (store, _) = self.ensure_artifact_server()?; + store.delete(&id).map_err(Self::artifact_error)?; + Ok(serde_json::json!({"deleted": id}).to_string()) + } + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), } - 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("text"), - ), - content, - session_id: session_id.unwrap_or_default(), - update_id, - label, - description, - favicon, - base_version, - sender: sender.or_else(|| self.agent.clone()), - 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}/"), - }); - Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) - } - - #[tool( + } #[tool( description = "Hand a work product to another agent: assigns/creates an item for the recipient (in the repo's linked project) and attaches the content to it as an asset. Re-attaching under the same item_id creates the next asset version, not a duplicate. Sender is this runtime's own identity." )] fn handoff( @@ -1552,154 +1429,6 @@ impl AgentflareMcp { } } - #[tool( - description = "List published artifacts (id, name, type, version, description, session, handoff envelope) with their local URLs. Filter by session_id, recipient (inbox), or thread_id." - )] - fn artifact_list( - &self, - Parameters(ArtifactListRequest { - session_id, - recipient, - thread_id, - }): Parameters, - ) -> Result { - let (store, base) = self.ensure_artifact_server()?; - let summaries = store - .list(session_id.as_deref()) - .map_err(Self::artifact_error)?; - let items: Vec = summaries - .iter() - .filter(|s| { - recipient - .as_deref() - .is_none_or(|r| s.recipient.as_deref() == Some(r)) - && thread_id - .as_deref() - .is_none_or(|t| s.thread_id.as_deref() == Some(t)) - }) - .map(|s| { - let mut v = serde_json::to_value(s).unwrap_or_default(); - if let Some(obj) = v.as_object_mut() { - obj.insert("url".into(), serde_json::json!(format!("{base}/{}", s.id))); - } - v - }) - .collect(); - Ok(serde_json::to_string_pretty(&items).unwrap_or_default()) - } - - #[tool( - description = "Fetch an artifact's full content and metadata by id; pass version to read an older snapshot. Version history itself is at GET /{id}/versions." - )] - fn artifact_get( - &self, - Parameters(ArtifactGetRequest { id, version }): Parameters, - ) -> Result { - if id.trim().is_empty() { - return Err(ErrorData::invalid_params("id is required", None)); - } - let (store, _base) = self.ensure_artifact_server()?; - let artifact = match version { - Some(n) => store.get_version(&id, n), - None => store.get(&id), - } - .map_err(Self::artifact_error)?; - Ok(serde_json::to_string_pretty(&artifact).unwrap_or_default()) - } - - #[tool( - description = "Unified diff between two versions of an artifact; to_version defaults to the latest. Use after an update to see what changed." - )] - fn artifact_diff( - &self, - Parameters(ArtifactDiffRequest { - id, - from_version, - to_version, - }): Parameters, - ) -> Result { - if id.trim().is_empty() { - return Err(ErrorData::invalid_params("id is required", None)); - } - let (store, _base) = self.ensure_artifact_server()?; - let to = match to_version { - Some(v) => v, - None => store.get(&id).map_err(Self::artifact_error)?.version, - }; - store - .diff(&id, from_version, to) - .map_err(Self::artifact_error) - } - - #[tool( - description = "Case-insensitive search across artifact names, descriptions, and content; returns matching summaries with a snippet around the first content match." - )] - fn artifact_search( - &self, - Parameters(ArtifactSearchRequest { query, session_id }): Parameters, - ) -> Result { - if query.trim().is_empty() { - return Err(ErrorData::invalid_params("query is required", None)); - } - let (store, base) = self.ensure_artifact_server()?; - let needle = query.to_lowercase(); - let mut hits = Vec::new(); - for summary in store - .list(session_id.as_deref()) - .map_err(Self::artifact_error)? - { - let name_hit = summary.name.to_lowercase().contains(&needle); - let desc_hit = summary - .description - .as_deref() - .is_some_and(|d| d.to_lowercase().contains(&needle)); - let content = store - .get(&summary.id) - .map(|a| a.content) - .unwrap_or_default(); - let content_pos = content.to_lowercase().find(&needle); - if !(name_hit || desc_hit || content_pos.is_some()) { - continue; - } - let snippet = content_pos.map(|pos| { - let mut start = pos.saturating_sub(40); - while !content.is_char_boundary(start) { - start -= 1; - } - let mut end = (pos + needle.len() + 40).min(content.len()); - while !content.is_char_boundary(end) { - end += 1; - } - content[start..end].to_string() - }); - let mut v = serde_json::to_value(&summary).unwrap_or_default(); - if let Some(obj) = v.as_object_mut() { - obj.insert( - "url".into(), - serde_json::json!(format!("{base}/{}", summary.id)), - ); - if let Some(snippet) = snippet { - obj.insert("snippet".into(), serde_json::json!(snippet)); - } - } - hits.push(v); - } - Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) - } - - #[tool(description = "Delete an artifact and all its versions by id.")] - fn artifact_delete( - &self, - Parameters(ArtifactDeleteRequest { id }): Parameters, - ) -> Result { - if id.trim().is_empty() { - return Err(ErrorData::invalid_params("id is required", None)); - } - let (store, _base) = self.ensure_artifact_server()?; - let deleted = store.delete(&id).map_err(Self::artifact_error)?; - Ok(serde_json::json!({ "deleted": deleted }).to_string()) - } - #[tool( description = "Send a text message out to a chat platform (telegram, slack, or discord). The bot token must already be stored as the gateway secret '_bot_token'. target is the Telegram chat_id or Slack/Discord channel id." )] @@ -1723,121 +1452,57 @@ impl AgentflareMcp { .map_err(|e| ErrorData::internal_error(e, None))?; Ok(serde_json::json!({ "sent": true, "platform": platform, "target": target }).to_string()) } - #[tool( - description = "Claim a GitHub issue/PR so other agents don't duplicate the work. Returns 'acquired' if you now own it, or 'held' with the current owner if a live claim exists. Only stale (past-TTL) or done claims are stolen. Re-heartbeat periodically to keep it." + description = "Manage work claims — acquire, heartbeat, release, done, or list. Single consolidated tool with `action` field (acquire|done|heartbeat|list|release)." )] - fn claim_acquire( + fn claim( &self, - Parameters(ClaimTargetRequest { target, repo }): Parameters, + Parameters(req): Parameters, ) -> Result { - // Only capture the current checkout's commit when the repo is - // auto-resolved from it; an explicit repo may name a different one. - let repo_overridden = repo.as_ref().is_some_and(|r| !r.is_empty()); - let (conn, repo) = Self::claim_ctx(&target, repo)?; - let owner = crate::claims::owner_id(); - let commit = if repo_overridden { - None - } else { - Self::git_provenance().and_then(|g| g.commit) - }; - let outcome = crate::claims::acquire( - &conn, - &repo, - &target, - &owner, - commit.as_deref(), - crate::claims::now(), - crate::claims::ttl_secs(), - ) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(match outcome { - crate::claims::Acquire::Acquired => { - serde_json::json!({ "status": "acquired", "repo": repo, "target": target, "owner": owner }) + match req.action.as_str() { + "acquire" => { + let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let repo_opt = req.repo; + let repo_overridden = repo_opt.as_ref().is_some_and(|r| !r.is_empty()); + let (conn, repo) = Self::claim_ctx(&target, repo_opt)?; + let owner = crate::claims::owner_id(); + let commit = if repo_overridden { None } else { Self::git_provenance().and_then(|g| g.commit) }; + let outcome = crate::claims::acquire(&conn, &repo, &target, &owner, commit.as_deref(), crate::claims::now(), crate::claims::ttl_secs()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(match outcome { + crate::claims::Acquire::Acquired => serde_json::json!({ "status": "acquired", "repo": repo, "target": target, "owner": owner }), + crate::claims::Acquire::Held { owner: holder, age_secs } => serde_json::json!({ "status": "held", "repo": repo, "target": target, "owner": holder, "age_secs": age_secs }), + }.to_string()) + } + "heartbeat" => { + let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let (conn, repo) = Self::claim_ctx(&target, req.repo)?; + let owner = crate::claims::owner_id(); + let ok = crate::claims::heartbeat(&conn, &repo, &target, &owner, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({ "refreshed": ok, "repo": repo, "target": target }).to_string()) + } + "release" => { + let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let (conn, repo) = Self::claim_ctx(&target, req.repo)?; + let owner = crate::claims::owner_id(); + let ok = crate::claims::release(&conn, &repo, &target, &owner).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({ "released": ok, "repo": repo, "target": target }).to_string()) + } + "done" => { + let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let (conn, repo) = Self::claim_ctx(&target, req.repo)?; + let owner = crate::claims::owner_id(); + let ok = crate::claims::done(&conn, &repo, &target, &owner, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({ "done": ok, "repo": repo, "target": target }).to_string()) } - crate::claims::Acquire::Held { owner: holder, age_secs } => { - serde_json::json!({ "status": "held", "repo": repo, "target": target, "owner": holder, "age_secs": age_secs }) + "list" => { + let conn = Self::claim_db()?; + let scope = if req.all_repos { None } else { Some(crate::claims::resolve_repo(req.repo).ok_or_else(|| ErrorData::invalid_params("could not determine repo — run in a git repo or pass repo=owner/name (or all_repos=true)", None))?) }; + let claims = crate::claims::list(&conn, scope.as_deref(), req.all, crate::claims::now(), crate::claims::ttl_secs()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::to_string_pretty(&claims).unwrap_or_default()) } + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), } - .to_string()) - } - - #[tool( - description = "Refresh the lease on a claim you own, so it isn't reclaimed as stale. Returns refreshed=false if the claim is gone or owned by someone else." - )] - fn claim_heartbeat( - &self, - Parameters(ClaimTargetRequest { target, repo }): Parameters, - ) -> Result { - let (conn, repo) = Self::claim_ctx(&target, repo)?; - let owner = crate::claims::owner_id(); - let ok = crate::claims::heartbeat(&conn, &repo, &target, &owner, crate::claims::now()) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "refreshed": ok, "repo": repo, "target": target }).to_string()) - } - - #[tool( - description = "Release a claim you own, freeing the target for other agents. Returns released=false if it wasn't yours." - )] - fn claim_release( - &self, - Parameters(ClaimTargetRequest { target, repo }): Parameters, - ) -> Result { - let (conn, repo) = Self::claim_ctx(&target, repo)?; - let owner = crate::claims::owner_id(); - let ok = crate::claims::release(&conn, &repo, &target, &owner) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "released": ok, "repo": repo, "target": target }).to_string()) - } - - #[tool( - description = "Mark a claim you own as done — keeps the audit row (unlike release, which deletes it) while freeing the target for re-acquisition. Returns done=false if it wasn't yours." - )] - fn claim_done( - &self, - Parameters(ClaimTargetRequest { target, repo }): Parameters, - ) -> Result { - let (conn, repo) = Self::claim_ctx(&target, repo)?; - let owner = crate::claims::owner_id(); - let ok = crate::claims::done(&conn, &repo, &target, &owner, crate::claims::now()) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "done": ok, "repo": repo, "target": target }).to_string()) - } - - #[tool( - description = "List work claims. Defaults to live claims for the current repo; set all=true to include stale/done, all_repos=true to span every repo." - )] - fn claim_list( - &self, - Parameters(ClaimListRequest { - repo, - all, - all_repos, - }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let scope = if all_repos { - None - } else { - Some(crate::claims::resolve_repo(repo).ok_or_else(|| { - ErrorData::invalid_params( - "could not determine repo — run in a git repo or pass repo=owner/name (or all_repos=true)", - None, - ) - })?) - }; - let claims = crate::claims::list( - &conn, - scope.as_deref(), - all, - crate::claims::now(), - crate::claims::ttl_secs(), - ) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::to_string_pretty(&claims).unwrap_or_default()) - } - - /// Opens the ledger db. + } /// Opens the ledger db. fn claim_db() -> crate::errors::Result { Ok(crate::db::open()?) } @@ -1860,149 +1525,71 @@ impl AgentflareMcp { })?; Ok((conn, repo)) } - - #[tool( - description = "Submit a finder's review findings for a round (each finding is {file, line, message, severity?, category?}). Replaces this finder's prior findings for the round. Call from each reviewing agent, then call review_consensus to verify + dedup + tag." - )] - fn review_submit( - &self, - Parameters(ReviewSubmitRequest { - findings, - pr, - agent, - repo, - }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let repo = Self::resolve_repo_or_err(repo)?; - let pr = Self::resolve_round(pr)?; - let agent = agent - .filter(|s| !s.is_empty()) - .unwrap_or_else(crate::review::submitter_name); - let parsed: Vec = findings - .into_iter() - .map(serde_json::from_value) - .collect::>() - .map_err(|e| ErrorData::invalid_params(format!("invalid finding: {e}"), None))?; - let n = crate::review::submit(&conn, &repo, &pr, &agent, &parsed, crate::claims::now()) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok( - serde_json::json!({ "submitted": n, "repo": repo, "pr": pr, "agent": agent }) - .to_string(), - ) - } - #[tool( - description = "Verify all submitted findings for a round against the git diff (base...head), dedup overlapping ones, and tag each CONFIRMED/UNIQUE/DISPUTED/UNVERIFIED. Returns the ranked consensus items." + description = "Review operations — submit findings, run consensus, list/clear/record rounds, check scores. Single consolidated tool with `action` field (clear|consensus|list|record|scores|submit)." )] - fn review_consensus( - &self, - Parameters(ReviewConsensusRequest { - pr, - base, - head, - repo, - }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let repo = Self::resolve_repo_or_err(repo)?; - let pr = Self::resolve_round(pr)?; - let findings = crate::review::load(&conn, &repo, &pr) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - let diff = crate::review::compute_diff(base.as_deref(), head.as_deref()) - .map_err(|e| ErrorData::invalid_params(e, None))?; - let changed = crate::review::changed_lines(&diff); - let items = crate::review::consensus(&findings, &changed); - Ok(serde_json::json!({ - "repo": repo, - "pr": pr, - "items": items, - "markdown": crate::review::render_markdown(&items), - }) - .to_string()) - } - - #[tool(description = "List the raw submitted findings for a review round (before consensus).")] - fn review_list( + fn review( &self, - Parameters(ReviewRoundRequest { pr, repo }): Parameters, + Parameters(req): Parameters, ) -> Result { - let conn = Self::claim_db()?; - let repo = Self::resolve_repo_or_err(repo)?; - let pr = Self::resolve_round(pr)?; - let findings = crate::review::load(&conn, &repo, &pr) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - let rows: Vec = findings - .iter() - .map(|sf| serde_json::json!({ "agent": sf.agent, "file": sf.finding.file, "line": sf.finding.line, "message": sf.finding.message, "severity": sf.finding.severity })) - .collect(); - Ok(serde_json::to_string_pretty(&rows).unwrap_or_default()) - } - - #[tool(description = "Drop all submitted findings for a review round.")] - fn review_clear( - &self, - Parameters(ReviewRoundRequest { pr, repo }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let repo = Self::resolve_repo_or_err(repo)?; - let pr = Self::resolve_round(pr)?; - let n = crate::review::clear(&conn, &repo, &pr) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "cleared": n, "repo": repo, "pr": pr }).to_string()) - } - - #[tool( - description = "Record this round's per-agent accuracy: how many of each finder's findings cited a real changed line (verified) vs total. Idempotent per round. Feeds review_scores." - )] - fn review_record( - &self, - Parameters(ReviewConsensusRequest { - pr, - base, - head, - repo, - }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let repo = Self::resolve_repo_or_err(repo)?; - let pr = Self::resolve_round(pr)?; - let findings = crate::review::load(&conn, &repo, &pr) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - let diff = crate::review::compute_diff(base.as_deref(), head.as_deref()) - .map_err(|e| ErrorData::invalid_params(e, None))?; - let changed = crate::review::changed_lines(&diff); - let n = crate::review::record_round( - &conn, - &repo, - &pr, - &findings, - &changed, - crate::claims::now(), - ) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "recorded_agents": n, "repo": repo, "pr": pr }).to_string()) - } - - #[tool( - description = "Per-agent accuracy across recorded rounds: verified/total citation rate, ranked. Use to weight which finders to trust or dispatch." - )] - fn review_scores( - &self, - Parameters(ReviewScoresRequest { repo, all_repos }): Parameters, - ) -> Result { - let conn = Self::claim_db()?; - let scope = if all_repos { - None - } else { - Some(Self::resolve_repo_or_err(repo)?) - }; - let scores = crate::review::scores(&conn, scope.as_deref()) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::to_string_pretty(&scores).unwrap_or_default()) - } - - fn resolve_repo_or_err(repo: Option) -> Result { + match req.action.as_str() { + "submit" => { + let findings = req.findings.ok_or_else(|| ErrorData::invalid_params("findings is required", None))?; + let conn = Self::claim_db()?; + let repo = Self::resolve_repo_or_err(req.repo)?; + let pr = Self::resolve_round(req.pr)?; + let agent = req.agent.filter(|s| !s.is_empty()).unwrap_or_else(crate::review::submitter_name); + let parsed: Vec = findings.into_iter().map(serde_json::from_value).collect::>().map_err(|e| ErrorData::invalid_params(format!("invalid finding: {e}"), None))?; + let n = crate::review::submit(&conn, &repo, &pr, &agent, &parsed, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({ "submitted": n, "repo": repo, "pr": pr, "agent": agent }).to_string()) + } + "consensus" => { + let conn = Self::claim_db()?; + let repo = Self::resolve_repo_or_err(req.repo)?; + let pr = Self::resolve_round(req.pr)?; + let base = req.base.unwrap_or_else(|| "master".into()); + let head = req.head.unwrap_or_else(|| "HEAD".into()); + let result = crate::review::consensus(&conn, &repo, &pr, &base, &head).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) + } + "list" => { + let conn = Self::claim_db()?; + let repo = Self::resolve_repo_or_err(req.repo)?; + let pr = Self::resolve_round(req.pr)?; + let findings = crate::review::load(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let rows: Vec = findings.iter().map(|sf| serde_json::json!({ "agent": sf.agent, "file": sf.finding.file, "line": sf.finding.line, "message": sf.finding.message, "severity": sf.finding.severity })).collect(); + Ok(serde_json::to_string_pretty(&rows).unwrap_or_default()) + } + "clear" => { + let conn = Self::claim_db()?; + let repo = Self::resolve_repo_or_err(req.repo)?; + let pr = Self::resolve_round(req.pr)?; + crate::review::clear(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({"cleared": true}).to_string()) + } + "record" => { + let conn = Self::claim_db()?; + let repo = Self::resolve_repo_or_err(req.repo)?; + let pr = Self::resolve_round(req.pr)?; + let base = req.base.unwrap_or_else(|| "master".into()); + let head = req.head.unwrap_or_else(|| "HEAD".into()); + let findings = crate::review::load(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let diff = crate::review::compute_diff(req.base.as_deref(), req.head.as_deref()).map_err(|e| ErrorData::invalid_params(e, None))?; + let changed = crate::review::changed_lines(&diff); + let n = crate::review::record_round(&conn, &repo, &pr, &findings, &changed, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::json!({ "recorded_agents": n, "repo": repo, "pr": pr }).to_string()) + } + "scores" => { + let conn = Self::claim_db()?; + let repo = req.repo; + let all_repos = req.all_repos; + let scope = if all_repos { None } else { Some(Self::resolve_repo_or_err(repo)?) }; + let scores = crate::review::scores(&conn, scope.as_deref()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::to_string_pretty(&scores).unwrap_or_default()) + } + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + } + } fn resolve_repo_or_err(repo: Option) -> Result { crate::claims::resolve_repo(repo).ok_or_else(|| { ErrorData::invalid_params( "could not determine repo — run in a git repo or pass repo=owner/name", @@ -2132,231 +1719,87 @@ impl AgentflareMcp { .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(guard) } - #[tool( - description = "Search downstream MCP servers' tools by task description. Returns server, tool, description, and input_schema; call gateway_execute to run one." + description = "Gateway operations — search downstream MCP servers' tools or execute one. Single consolidated tool with `action` field (search|execute)." )] - async fn gateway_search( + async fn gateway( &self, - Parameters(GatewaySearchRequest { query, limit, mode }): Parameters, + Parameters(req): Parameters, ) -> Result { - if query.trim().is_empty() { - return Err(ErrorData::invalid_params("query is required", None)); - } - let mode = match mode.as_deref() { - None | Some("all") => gateway_registry::MatchMode::All, - Some("any") => gateway_registry::MatchMode::Any, - Some(other) => { - return Err(ErrorData::invalid_params( - format!("mode must be 'all' or 'any', got '{other}'"), - None, - )); + match req.action.as_str() { + "search" => { + let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + if query.trim().is_empty() { return Err(ErrorData::invalid_params("query is required", None)); } + let mode = match req.mode.as_deref() { + None | Some("all") => gateway_registry::MatchMode::All, + Some("any") => gateway_registry::MatchMode::Any, + Some(other) => return Err(ErrorData::invalid_params(format!("mode must be 'all' or 'any', got '{other}'"), None)), + }; + let guard = self.ensure_gateway_registry().await?; + let reg = guard.as_ref().expect("ensured above"); + let hits = reg.search(&query, req.limit.unwrap_or(5), mode).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) } - }; - let guard = self.ensure_gateway_registry().await?; - let reg = guard.as_ref().expect("ensured above"); - let hits = reg - .search(&query, limit.unwrap_or(5), mode) - .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) - } - + "execute" => { + let server = req.server.ok_or_else(|| ErrorData::invalid_params("server is required", None))?; + let tool = req.tool.ok_or_else(|| ErrorData::invalid_params("tool is required", None))?; + if server.trim().is_empty() || tool.trim().is_empty() { return Err(ErrorData::invalid_params("server and tool are required", None)); } + let args = req.args.map(serde_json::Value::Object).unwrap_or(serde_json::Value::Null); + let guard = self.ensure_gateway_registry().await?; + let reg = guard.as_ref().expect("ensured above"); + match reg.execute(&server, &tool, args).await { + Ok(value) => { let capped = gateway_registry::truncate_if_needed(&value, gateway_registry::DEFAULT_MAX_CHARS); Ok(serde_json::to_string_pretty(&capped).unwrap_or_default()) } + Err(e @ gateway_registry::GatewayError::ServerNotFound(_)) | Err(e @ gateway_registry::GatewayError::ToolNotFound(_)) | Err(e @ gateway_registry::GatewayError::InvalidArgument(_)) => Err(ErrorData::invalid_params(e.to_string(), None)), + Err(e) => Err(ErrorData::internal_error(gateway_registry::redact_error_for_llm(&e.to_string()), None)), + } + } + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + } + } // --- Memory tools --- #[tool( - description = "Execute a tool on a downstream MCP server found via gateway_search. args must match that tool's input_schema." + description = "Memory operations — remember, recall, context, curate, handoff, or relate observations. Single consolidated tool with `action` field (context|curate|handoff|recall|relate|remember)." )] - async fn gateway_execute( + fn memory( &self, - Parameters(GatewayExecuteRequest { server, tool, args }): Parameters, + Parameters(req): Parameters, ) -> Result { - if server.trim().is_empty() || tool.trim().is_empty() { - return Err(ErrorData::invalid_params( - "server and tool are required", - None, - )); - } - let args = args - .map(serde_json::Value::Object) - .unwrap_or(serde_json::Value::Null); - let guard = self.ensure_gateway_registry().await?; - let reg = guard.as_ref().expect("ensured above"); - match reg.execute(&server, &tool, args).await { - Ok(value) => { - let capped = gateway_registry::truncate_if_needed( - &value, - gateway_registry::DEFAULT_MAX_CHARS, - ); - Ok(serde_json::to_string_pretty(&capped).unwrap_or_default()) + match req.action.as_str() { + "remember" => { + let title = req.title.ok_or_else(|| ErrorData::invalid_params("title is required", None))?; + let content = req.content.ok_or_else(|| ErrorData::invalid_params("content is required", None))?; + let r#type = req.r#type.ok_or_else(|| ErrorData::invalid_params("type is required", None))?; + let input = crate::memory::mcp::RememberInput { title, content, r#type, session_id: req.session_id, project: req.project, topic_key: req.topic_key, scope: req.scope }; + crate::memory::mcp::handle_remember(input).map_err(|e| ErrorData::internal_error(e, None)) } - Err(e @ gateway_registry::GatewayError::ServerNotFound(_)) - | Err(e @ gateway_registry::GatewayError::ToolNotFound(_)) - | Err(e @ gateway_registry::GatewayError::InvalidArgument(_)) => { - Err(ErrorData::invalid_params(e.to_string(), None)) + "recall" => { + let input = crate::memory::mcp::RecallInput { query: req.query, id: req.id, r#type: req.r#type, project: req.project, limit: req.limit }; + crate::memory::mcp::handle_recall(input).map_err(|e| ErrorData::internal_error(e, None)) } - // Every other variant (Upstream, Connection, Timeout, ...) - // carries the downstream server's or OS's own error text - // verbatim — unlike the three above, which are our own - // controlled messages. Redact before it reaches the LLM: a - // downstream server's raw error could otherwise leak a file - // path, connection string, or an echoed credential. - Err(e) => Err(ErrorData::internal_error( - gateway_registry::redact_error_for_llm(&e.to_string()), - None, - )), + "context" => { + let input = crate::memory::mcp::ContextInput { session_id: req.session_id, project: req.project }; + crate::memory::mcp::handle_context(input).map_err(|e| ErrorData::internal_error(e, None)) + } + "handoff" => { + let session_id = req.session_id.ok_or_else(|| ErrorData::invalid_params("session_id is required", None))?; + let summary = req.summary.ok_or_else(|| ErrorData::invalid_params("summary is required", None))?; + let input = crate::memory::mcp::HandoffInput { session_id, summary, findings: req.findings, decisions: req.decisions, files_touched: req.files_touched, evidence: req.evidence }; + crate::memory::mcp::handle_handoff(input).map_err(|e| ErrorData::internal_error(e, None)) + } + "relate" => { + let source_id = req.source_id.ok_or_else(|| ErrorData::invalid_params("source_id is required", None))?; + let target_id = req.target_id.ok_or_else(|| ErrorData::invalid_params("target_id is required", None))?; + let relation = req.relation.ok_or_else(|| ErrorData::invalid_params("relation is required", None))?; + let input = crate::memory::mcp::RelateInput { source_id, target_id, relation, reason: req.reason, confidence: req.confidence }; + crate::memory::mcp::handle_relate(input).map_err(|e| ErrorData::internal_error(e, None)) + } + "curate" => { + let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let input = crate::memory::mcp::CurateInput { action: req.action.clone(), id, title: req.title, content: req.content, r#type: req.r#type, pinned: req.pinned }; + crate::memory::mcp::handle_curate(input).map_err(|e| ErrorData::internal_error(e, None)) + } + other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), } - } - - // --- Memory tools --- - // - // Must live in this impl block, not a separate one — #[tool_router] - // (on this block's `impl` line) is what rmcp's macro uses to collect - // #[tool]-annotated methods into the router that get_tools/call_tool - // actually dispatch through. A #[tool] method in an untagged impl - // block compiles fine and is directly callable as a plain Rust - // method (which is why unit tests calling e.g. `s.memory_remember(...)` - // passed), but is never registered as an MCP tool and is invisible to - // every real MCP client — silently dead on arrival. - - #[tool( - description = "Save an observation to persistent memory. Creates, updates (by topic_key), or deduplicates. Returns status: created|updated|duplicate." - )] - fn memory_remember( - &self, - Parameters(MemoryRememberRequest { - title, - content, - r#type, - session_id, - project, - topic_key, - scope, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::RememberInput { - title, - content, - r#type, - session_id, - project, - topic_key, - scope, - }; - crate::memory::mcp::handle_remember(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - #[tool( - description = "Search or retrieve observations. Pass id for direct lookup, query for FTS5 BM25 search, omit query for recent listing. Filters by type/project." - )] - fn memory_recall( - &self, - Parameters(MemoryRecallRequest { - query, - id, - r#type, - project, - limit, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::RecallInput { - query, - id, - r#type, - project, - limit, - }; - crate::memory::mcp::handle_recall(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - #[tool( - description = "Return session context: active session (findings/decisions/files_touched), recent sessions, recent observations, and recent session summaries." - )] - fn memory_context( - &self, - Parameters(MemoryContextRequest { - session_id, - project, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::ContextInput { - session_id, - project, - }; - crate::memory::mcp::handle_context(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - #[tool( - description = "Close a session with a handoff summary. Enriches the session with findings/decisions/files_touched/evidence, builds a compaction snapshot, appends to session_summaries, and marks the session closed." - )] - fn memory_handoff( - &self, - Parameters(MemoryHandoffRequest { - session_id, - summary, - findings, - decisions, - files_touched, - evidence, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::HandoffInput { - session_id, - summary, - findings, - decisions, - files_touched, - evidence, - }; - crate::memory::mcp::handle_handoff(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - #[tool( - description = "Record a semantic relation verdict between two observations. Relation: related|compatible|scoped|conflicts_with|supersedes|not_conflict." - )] - fn memory_relate( - &self, - Parameters(MemoryRelateRequest { - source_id, - target_id, - relation, - reason, - confidence, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::RelateInput { - source_id, - target_id, - relation, - reason, - confidence, - }; - crate::memory::mcp::handle_relate(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - #[tool( - description = "Update, soft-delete, pin, or unpin an observation by ID. Actions: update (title/content/type/pinned), delete, pin, unpin." - )] - fn memory_curate( - &self, - Parameters(MemoryCurateRequest { - action, - id, - title, - content, - r#type, - pinned, - }): Parameters, - ) -> Result { - let input = crate::memory::mcp::CurateInput { - action, - id, - title, - content, - r#type, - pinned, - }; - crate::memory::mcp::handle_curate(input).map_err(|e| ErrorData::internal_error(e, None)) - } - - fn item_inner(&self, req: ItemRequest) -> Result { + } fn item_inner(&self, req: ItemRequest) -> Result { match req.action.as_str() { "create" => { let name = req.name.ok_or_else(|| { From c6aefce3316beec849c668e3b8ddf0ba743eb9ca Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Wed, 15 Jul 2026 00:47:31 +0530 Subject: [PATCH 2/3] fix(mcp): finish artifact/skill/gateway test migration, fix consolidation regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI was red: ~35 test call sites in mcp_server.rs still called the old per-action methods (artifact_publish/get/list/delete/diff/search, skill_search/load, gateway_search/execute) that this consolidation removed. Migrated them all to the unified action-dispatch tools. Also fixes real bugs the consolidation introduced along the way: - artifact()'s get/diff/search arms called store methods with a signature they no longer have (get takes no version, diff's to is a required u32, and there's no store.search at all) — restored the pre-consolidation logic (get_version fallback, latest-version resolution, inline list+filter+snippet search). - review()'s "consensus" arm called crate::review::consensus with the wrong arguments entirely (a Connection/ids instead of findings+changed-lines); now loads findings and computes the diff/changed-lines first, matching "record"'s already-correct pattern. Also dropped record's dead base/head locals that shadowed the actual req.base/req.head usage. - memory()'s "curate" arm passed the outer "curate" action into CurateInput.action, so update/delete/pin/unpin could never be reached; added a dedicated curate_action field. - mark_completed's ownership check and completion write weren't in the same transaction, leaving a window for a concurrent release+claim by a different owner to complete the item out from under its new owner. Also ran cargo fmt across the branch (had never been formatted). --- crates/agentflare-backend/src/item.rs | 16 +- crates/agentflare-db-kit/src/claim.rs | 7 +- src/mcp_server.rs | 809 +++++++++++++++++++------- 3 files changed, 605 insertions(+), 227 deletions(-) diff --git a/crates/agentflare-backend/src/item.rs b/crates/agentflare-backend/src/item.rs index 51450907..f4f61483 100644 --- a/crates/agentflare-backend/src/item.rs +++ b/crates/agentflare-backend/src/item.rs @@ -444,13 +444,19 @@ pub fn claim( /// via `claim::done`. Returns `Ok(true)` when the item was actually moved /// to completed, `Ok(false)` when the caller doesn't own the claim. pub fn mark_completed(conn: &Connection, item_id: &str, owner: &str) -> Result { - // Guards: don't advance state unless the caller still holds the lease. - if !crate::claim::is_owner(conn, item_id, owner)? { + // One transaction start to finish so the ownership check can't go stale + // between the guard and the write — without this, a concurrent + // release()+claim() by a different owner could slip in between the + // check and update_state below, completing the item out from under its + // new owner. + let tx = conn.unchecked_transaction()?; + if !crate::claim::is_owner(&tx, item_id, owner)? { return Ok(false); } - let item = get(conn, item_id)?; - let completed_state = crate::state::first_in_group(conn, &item.project_id, "completed")?; - update_state(conn, item_id, &completed_state.id)?; + let item = get(&tx, item_id)?; + let completed_state = crate::state::first_in_group(&tx, &item.project_id, "completed")?; + update_state(&tx, item_id, &completed_state.id)?; + tx.commit()?; // Keep the claim lease held for the MCP caller's deferred release. Ok(true) } diff --git a/crates/agentflare-db-kit/src/claim.rs b/crates/agentflare-db-kit/src/claim.rs index 8e7e47a6..6eea069d 100644 --- a/crates/agentflare-db-kit/src/claim.rs +++ b/crates/agentflare-db-kit/src/claim.rs @@ -179,12 +179,7 @@ impl ClaimLedger { /// Ownership check without mutation — used by the `mark_completed` + /// deferred-release split to verify the caller still holds the claim /// before advancing the item's state. - pub fn is_owner( - &self, - conn: &Connection, - key: &[&str], - owner: &str, - ) -> rusqlite::Result { + pub fn is_owner(&self, conn: &Connection, key: &[&str], owner: &str) -> rusqlite::Result { let sql = format!( "SELECT 1 FROM {t} WHERE {pred} AND owner = ?{owner_p} AND status = 'claimed'", t = self.table, diff --git a/src/mcp_server.rs b/src/mcp_server.rs index e7647ce8..72cbf0d6 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -115,7 +115,9 @@ struct ChannelSendRequest { struct ReviewRequest { #[schemars(description = "Action: clear|consensus|list|record|scores|submit")] action: String, - #[schemars(description = "Findings, each {file, line, message, severity?, category?} (submit)")] + #[schemars( + description = "Findings, each {file, line, message, severity?, category?} (submit)" + )] #[serde(default)] findings: Option>, #[schemars(description = "Review round id (default: current branch)")] @@ -196,7 +198,9 @@ struct ArtifactRequest { #[schemars(description = "Display name of the artifact (publish)")] #[serde(default)] name: Option, - #[schemars(description = "html | markdown | mermaid | diagram | text (default: text) (publish)")] + #[schemars( + description = "html | markdown | mermaid | diagram | text (default: text) (publish)" + )] #[serde(default)] r#type: Option, #[schemars( @@ -252,13 +256,17 @@ struct ArtifactRequest { #[schemars(description = "Newer version number (omit for latest) (diff)")] #[serde(default)] to_version: Option, - #[schemars(description = "Case-insensitive text to find in names, descriptions, or content (search)")] + #[schemars( + description = "Case-insensitive text to find in names, descriptions, or content (search)" + )] #[serde(default)] query: Option, #[schemars(description = "Specific version to fetch (omit for latest) (get)")] #[serde(default)] version: Option, - #[schemars(description = "Inbox filter: only artifacts addressed to this agent/runtime (list)")] + #[schemars( + description = "Inbox filter: only artifacts addressed to this agent/runtime (list)" + )] #[serde(default)] inbox_recipient: Option, } @@ -273,7 +281,9 @@ struct MemoryRequest { #[schemars(description = "Content body of the observation (remember, curate)")] #[serde(default)] content: Option, - #[schemars(description = "Type: decision|bugfix|discovery|pattern|learning|manual (remember, recall)")] + #[schemars( + description = "Type: decision|bugfix|discovery|pattern|learning|manual (remember, recall)" + )] #[serde(default)] r#type: Option, #[schemars(description = "Session ID to associate with")] @@ -332,6 +342,9 @@ struct MemoryRequest { #[schemars(description = "Pin status (curate pin/unpin actions)")] #[serde(default)] pinned: Option, + #[schemars(description = "Sub-action for curate: update|delete|pin|unpin")] + #[serde(default)] + curate_action: Option, } #[derive(Default)] @@ -752,35 +765,54 @@ impl AgentflareMcp { #[tool( description = "Skill operations — search installed skills or load one by name. Single consolidated tool with `action` field (search|load)." )] - fn skill( - &self, - Parameters(req): Parameters, - ) -> Result { + fn skill(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "search" => { - let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; - if query.trim().is_empty() { return Err(ErrorData::invalid_params("query is required", None)); } + let query = req + .query + .ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + if query.trim().is_empty() { + return Err(ErrorData::invalid_params("query is required", None)); + } let mode = match req.mode.as_deref() { None | Some("all") => skill_registry::MatchMode::All, Some("any") => skill_registry::MatchMode::Any, - Some(other) => return Err(ErrorData::invalid_params(format!("mode must be 'all' or 'any', got '{other}'"), None)), + Some(other) => { + return Err(ErrorData::invalid_params( + format!("mode must be 'all' or 'any', got '{other}'"), + None, + )); + } }; - let hits = self.with_fresh_registry(|reg| reg.search(&query, req.limit.unwrap_or(5), mode))?.map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let hits = self + .with_fresh_registry(|reg| reg.search(&query, req.limit.unwrap_or(5), mode))? + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) } "load" => { - let name = req.name.ok_or_else(|| ErrorData::invalid_params("name is required", None))?; - if name.trim().is_empty() { return Err(ErrorData::invalid_params("name is required", None)); } + let name = req + .name + .ok_or_else(|| ErrorData::invalid_params("name is required", None))?; + if name.trim().is_empty() { + return Err(ErrorData::invalid_params("name is required", None)); + } let result = self.with_fresh_registry(|reg| reg.load(&name, req.original))?; match result { Ok(s) => Ok(serde_json::to_string_pretty(&s).unwrap_or_default()), - Err(e @ skill_registry::LoadError::NotFound(_)) | Err(e @ skill_registry::LoadError::Ambiguous(_)) => Err(ErrorData::invalid_params(e.to_string(), None)), + Err(e @ skill_registry::LoadError::NotFound(_)) + | Err(e @ skill_registry::LoadError::Ambiguous(_)) => { + Err(ErrorData::invalid_params(e.to_string(), None)) + } Err(e) => Err(ErrorData::internal_error(e.to_string(), None)), } } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } /// Filesystem/URL-safe stem derived from a display name — lowercased, + } + /// Filesystem/URL-safe stem derived from a display name — lowercased, /// non-alphanumerics collapsed to `-`, falling back to "handoff" if that /// leaves nothing. fn slugify(name: &str) -> String { @@ -932,55 +964,165 @@ impl AgentflareMcp { #[tool( description = "Artifact operations — publish, list, get, diff, search, or delete. Single consolidated tool with `action` field (delete|diff|get|list|publish|search)." )] - fn artifact( - &self, - Parameters(req): Parameters, - ) -> Result { + fn artifact(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "publish" => { - let name = req.name.ok_or_else(|| ErrorData::invalid_params("name is required", None))?; - if name.trim().is_empty() { return Err(ErrorData::invalid_params("name is required", None)); } - let content = req.content.ok_or_else(|| ErrorData::invalid_params("content is required", None))?; - if content.is_empty() { return Err(ErrorData::invalid_params("content is required", None)); } + let name = req + .name + .ok_or_else(|| ErrorData::invalid_params("name is required", None))?; + if name.trim().is_empty() { + return Err(ErrorData::invalid_params("name is required", None)); + } + let content = req + .content + .ok_or_else(|| ErrorData::invalid_params("content is required", None))?; + if content.is_empty() { + return Err(ErrorData::invalid_params("content is required", None)); + } let (store, base) = self.ensure_artifact_server()?; - let req2 = agentflare_artifacts::PublishRequest { name, artifact_type: agentflare_artifacts::ArtifactType::from(req.r#type.as_deref().unwrap_or("text")), content, session_id: req.session_id.unwrap_or_default(), update_id: req.update_id, label: req.label, description: req.description, favicon: req.favicon, base_version: req.base_version, sender: req.sender.or_else(|| self.agent.clone()), recipient: req.recipient, thread_id: req.thread_id, reply_to: req.reply_to, git: Self::git_provenance() }; + let req2 = agentflare_artifacts::PublishRequest { + name, + artifact_type: agentflare_artifacts::ArtifactType::from( + req.r#type.as_deref().unwrap_or("text"), + ), + content, + session_id: req.session_id.unwrap_or_default(), + update_id: req.update_id, + label: req.label, + description: req.description, + favicon: req.favicon, + base_version: req.base_version, + sender: req.sender.or_else(|| self.agent.clone()), + recipient: req.recipient, + thread_id: req.thread_id, + reply_to: req.reply_to, + git: Self::git_provenance(), + }; let resp = store.publish(&req2).map_err(Self::artifact_error)?; Ok(serde_json::to_string_pretty(&serde_json::json!({ "id": resp.id, "version": resp.version, "url": format!("{base}/{}", resp.id), "index": format!("{base}/") })).unwrap_or_default()) } "list" => { let (store, base) = self.ensure_artifact_server()?; - let summaries = store.list(req.session_id.as_deref()).map_err(Self::artifact_error)?; - let items: Vec = summaries.iter().filter(|s| { req.inbox_recipient.as_deref().is_none_or(|r| s.recipient.as_deref() == Some(r)) && req.thread_id.as_deref().is_none_or(|t| s.thread_id.as_deref() == Some(t)) }).map(|s| { let mut v = serde_json::to_value(s).unwrap_or_default(); if let Some(obj) = v.as_object_mut() { obj.insert("url".into(), serde_json::json!(format!("{base}/{}", s.id))); } v }).collect(); + let summaries = store + .list(req.session_id.as_deref()) + .map_err(Self::artifact_error)?; + let items: Vec = summaries + .iter() + .filter(|s| { + req.inbox_recipient + .as_deref() + .is_none_or(|r| s.recipient.as_deref() == Some(r)) + && req + .thread_id + .as_deref() + .is_none_or(|t| s.thread_id.as_deref() == Some(t)) + }) + .map(|s| { + let mut v = serde_json::to_value(s).unwrap_or_default(); + if let Some(obj) = v.as_object_mut() { + obj.insert("url".into(), serde_json::json!(format!("{base}/{}", s.id))); + } + v + }) + .collect(); Ok(serde_json::to_string_pretty(&items).unwrap_or_default()) } "get" => { - let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let id = req + .id + .ok_or_else(|| ErrorData::invalid_params("id is required", None))?; let (store, _) = self.ensure_artifact_server()?; - let artifact = store.get(&id, req.version).map_err(Self::artifact_error)?; + let artifact = match req.version { + Some(n) => store.get_version(&id, n), + None => store.get(&id), + } + .map_err(Self::artifact_error)?; Ok(serde_json::to_string_pretty(&artifact).unwrap_or_default()) } "diff" => { - let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; - let from_version = req.from_version.ok_or_else(|| ErrorData::invalid_params("from_version is required", None))?; + let id = req + .id + .ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let from_version = req + .from_version + .ok_or_else(|| ErrorData::invalid_params("from_version is required", None))?; let (store, _) = self.ensure_artifact_server()?; - let diff = store.diff(&id, from_version, req.to_version).map_err(Self::artifact_error)?; + let to = match req.to_version { + Some(v) => v, + None => store.get(&id).map_err(Self::artifact_error)?.version, + }; + let diff = store + .diff(&id, from_version, to) + .map_err(Self::artifact_error)?; Ok(serde_json::to_string_pretty(&diff).unwrap_or_default()) } "search" => { - let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; - let (store, _) = self.ensure_artifact_server()?; - let results = store.search(&query, req.session_id.as_deref()).map_err(Self::artifact_error)?; - Ok(serde_json::to_string_pretty(&results).unwrap_or_default()) + let query = req + .query + .ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + if query.trim().is_empty() { + return Err(ErrorData::invalid_params("query is required", None)); + } + let (store, base) = self.ensure_artifact_server()?; + let needle = query.to_lowercase(); + let mut hits = Vec::new(); + for summary in store + .list(req.session_id.as_deref()) + .map_err(Self::artifact_error)? + { + let name_hit = summary.name.to_lowercase().contains(&needle); + let desc_hit = summary + .description + .as_deref() + .is_some_and(|d| d.to_lowercase().contains(&needle)); + let content = store + .get(&summary.id) + .map(|a| a.content) + .unwrap_or_default(); + let content_pos = content.to_lowercase().find(&needle); + if !(name_hit || desc_hit || content_pos.is_some()) { + continue; + } + let snippet = content_pos.map(|pos| { + let mut start = pos.saturating_sub(40); + while !content.is_char_boundary(start) { + start -= 1; + } + let mut end = (pos + needle.len() + 40).min(content.len()); + while !content.is_char_boundary(end) { + end += 1; + } + content[start..end].to_string() + }); + let mut v = serde_json::to_value(&summary).unwrap_or_default(); + if let Some(obj) = v.as_object_mut() { + obj.insert( + "url".into(), + serde_json::json!(format!("{base}/{}", summary.id)), + ); + if let Some(snippet) = snippet { + obj.insert("snippet".into(), serde_json::json!(snippet)); + } + } + hits.push(v); + } + Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) } "delete" => { - let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let id = req + .id + .ok_or_else(|| ErrorData::invalid_params("id is required", None))?; let (store, _) = self.ensure_artifact_server()?; store.delete(&id).map_err(Self::artifact_error)?; Ok(serde_json::json!({"deleted": id}).to_string()) } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } #[tool( + } + #[tool( description = "Hand a work product to another agent: assigns/creates an item for the recipient (in the repo's linked project) and attaches the content to it as an asset. Re-attaching under the same item_id creates the next asset version, not a duplicate. Sender is this runtime's own identity." )] fn handoff( @@ -1455,54 +1597,97 @@ impl AgentflareMcp { #[tool( description = "Manage work claims — acquire, heartbeat, release, done, or list. Single consolidated tool with `action` field (acquire|done|heartbeat|list|release)." )] - fn claim( - &self, - Parameters(req): Parameters, - ) -> Result { + fn claim(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "acquire" => { - let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let target = req + .target + .ok_or_else(|| ErrorData::invalid_params("target is required", None))?; let repo_opt = req.repo; let repo_overridden = repo_opt.as_ref().is_some_and(|r| !r.is_empty()); let (conn, repo) = Self::claim_ctx(&target, repo_opt)?; let owner = crate::claims::owner_id(); - let commit = if repo_overridden { None } else { Self::git_provenance().and_then(|g| g.commit) }; - let outcome = crate::claims::acquire(&conn, &repo, &target, &owner, commit.as_deref(), crate::claims::now(), crate::claims::ttl_secs()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let commit = if repo_overridden { + None + } else { + Self::git_provenance().and_then(|g| g.commit) + }; + let outcome = crate::claims::acquire( + &conn, + &repo, + &target, + &owner, + commit.as_deref(), + crate::claims::now(), + crate::claims::ttl_secs(), + ) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(match outcome { crate::claims::Acquire::Acquired => serde_json::json!({ "status": "acquired", "repo": repo, "target": target, "owner": owner }), crate::claims::Acquire::Held { owner: holder, age_secs } => serde_json::json!({ "status": "held", "repo": repo, "target": target, "owner": holder, "age_secs": age_secs }), }.to_string()) } "heartbeat" => { - let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let target = req + .target + .ok_or_else(|| ErrorData::invalid_params("target is required", None))?; let (conn, repo) = Self::claim_ctx(&target, req.repo)?; let owner = crate::claims::owner_id(); - let ok = crate::claims::heartbeat(&conn, &repo, &target, &owner, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "refreshed": ok, "repo": repo, "target": target }).to_string()) + let ok = + crate::claims::heartbeat(&conn, &repo, &target, &owner, crate::claims::now()) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok( + serde_json::json!({ "refreshed": ok, "repo": repo, "target": target }) + .to_string(), + ) } "release" => { - let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let target = req + .target + .ok_or_else(|| ErrorData::invalid_params("target is required", None))?; let (conn, repo) = Self::claim_ctx(&target, req.repo)?; let owner = crate::claims::owner_id(); - let ok = crate::claims::release(&conn, &repo, &target, &owner).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "released": ok, "repo": repo, "target": target }).to_string()) + let ok = crate::claims::release(&conn, &repo, &target, &owner) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok( + serde_json::json!({ "released": ok, "repo": repo, "target": target }) + .to_string(), + ) } "done" => { - let target = req.target.ok_or_else(|| ErrorData::invalid_params("target is required", None))?; + let target = req + .target + .ok_or_else(|| ErrorData::invalid_params("target is required", None))?; let (conn, repo) = Self::claim_ctx(&target, req.repo)?; let owner = crate::claims::owner_id(); - let ok = crate::claims::done(&conn, &repo, &target, &owner, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let ok = crate::claims::done(&conn, &repo, &target, &owner, crate::claims::now()) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::json!({ "done": ok, "repo": repo, "target": target }).to_string()) } "list" => { let conn = Self::claim_db()?; - let scope = if req.all_repos { None } else { Some(crate::claims::resolve_repo(req.repo).ok_or_else(|| ErrorData::invalid_params("could not determine repo — run in a git repo or pass repo=owner/name (or all_repos=true)", None))?) }; - let claims = crate::claims::list(&conn, scope.as_deref(), req.all, crate::claims::now(), crate::claims::ttl_secs()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let scope = if req.all_repos { + None + } else { + Some(crate::claims::resolve_repo(req.repo).ok_or_else(|| ErrorData::invalid_params("could not determine repo — run in a git repo or pass repo=owner/name (or all_repos=true)", None))?) + }; + let claims = crate::claims::list( + &conn, + scope.as_deref(), + req.all, + crate::claims::now(), + crate::claims::ttl_secs(), + ) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::to_string_pretty(&claims).unwrap_or_default()) } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } /// Opens the ledger db. + } + /// Opens the ledger db. fn claim_db() -> crate::errors::Result { Ok(crate::db::open()?) } @@ -1528,35 +1713,52 @@ impl AgentflareMcp { #[tool( description = "Review operations — submit findings, run consensus, list/clear/record rounds, check scores. Single consolidated tool with `action` field (clear|consensus|list|record|scores|submit)." )] - fn review( - &self, - Parameters(req): Parameters, - ) -> Result { + fn review(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "submit" => { - let findings = req.findings.ok_or_else(|| ErrorData::invalid_params("findings is required", None))?; + let findings = req + .findings + .ok_or_else(|| ErrorData::invalid_params("findings is required", None))?; let conn = Self::claim_db()?; let repo = Self::resolve_repo_or_err(req.repo)?; let pr = Self::resolve_round(req.pr)?; - let agent = req.agent.filter(|s| !s.is_empty()).unwrap_or_else(crate::review::submitter_name); - let parsed: Vec = findings.into_iter().map(serde_json::from_value).collect::>().map_err(|e| ErrorData::invalid_params(format!("invalid finding: {e}"), None))?; - let n = crate::review::submit(&conn, &repo, &pr, &agent, &parsed, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - Ok(serde_json::json!({ "submitted": n, "repo": repo, "pr": pr, "agent": agent }).to_string()) + let agent = req + .agent + .filter(|s| !s.is_empty()) + .unwrap_or_else(crate::review::submitter_name); + let parsed: Vec = findings + .into_iter() + .map(serde_json::from_value) + .collect::>() + .map_err(|e| { + ErrorData::invalid_params(format!("invalid finding: {e}"), None) + })?; + let n = + crate::review::submit(&conn, &repo, &pr, &agent, &parsed, crate::claims::now()) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + Ok( + serde_json::json!({ "submitted": n, "repo": repo, "pr": pr, "agent": agent }) + .to_string(), + ) } "consensus" => { let conn = Self::claim_db()?; let repo = Self::resolve_repo_or_err(req.repo)?; let pr = Self::resolve_round(req.pr)?; - let base = req.base.unwrap_or_else(|| "master".into()); - let head = req.head.unwrap_or_else(|| "HEAD".into()); - let result = crate::review::consensus(&conn, &repo, &pr, &base, &head).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let findings = crate::review::load(&conn, &repo, &pr) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let diff = crate::review::compute_diff(req.base.as_deref(), req.head.as_deref()) + .map_err(|e| ErrorData::invalid_params(e, None))?; + let changed = crate::review::changed_lines(&diff); + let result = crate::review::consensus(&findings, &changed); Ok(serde_json::to_string_pretty(&result).unwrap_or_default()) } "list" => { let conn = Self::claim_db()?; let repo = Self::resolve_repo_or_err(req.repo)?; let pr = Self::resolve_round(req.pr)?; - let findings = crate::review::load(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let findings = crate::review::load(&conn, &repo, &pr) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; let rows: Vec = findings.iter().map(|sf| serde_json::json!({ "agent": sf.agent, "file": sf.finding.file, "line": sf.finding.line, "message": sf.finding.message, "severity": sf.finding.severity })).collect(); Ok(serde_json::to_string_pretty(&rows).unwrap_or_default()) } @@ -1564,32 +1766,50 @@ impl AgentflareMcp { let conn = Self::claim_db()?; let repo = Self::resolve_repo_or_err(req.repo)?; let pr = Self::resolve_round(req.pr)?; - crate::review::clear(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + crate::review::clear(&conn, &repo, &pr) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::json!({"cleared": true}).to_string()) } "record" => { let conn = Self::claim_db()?; let repo = Self::resolve_repo_or_err(req.repo)?; let pr = Self::resolve_round(req.pr)?; - let base = req.base.unwrap_or_else(|| "master".into()); - let head = req.head.unwrap_or_else(|| "HEAD".into()); - let findings = crate::review::load(&conn, &repo, &pr).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; - let diff = crate::review::compute_diff(req.base.as_deref(), req.head.as_deref()).map_err(|e| ErrorData::invalid_params(e, None))?; + let findings = crate::review::load(&conn, &repo, &pr) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let diff = crate::review::compute_diff(req.base.as_deref(), req.head.as_deref()) + .map_err(|e| ErrorData::invalid_params(e, None))?; let changed = crate::review::changed_lines(&diff); - let n = crate::review::record_round(&conn, &repo, &pr, &findings, &changed, crate::claims::now()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let n = crate::review::record_round( + &conn, + &repo, + &pr, + &findings, + &changed, + crate::claims::now(), + ) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::json!({ "recorded_agents": n, "repo": repo, "pr": pr }).to_string()) } "scores" => { let conn = Self::claim_db()?; let repo = req.repo; let all_repos = req.all_repos; - let scope = if all_repos { None } else { Some(Self::resolve_repo_or_err(repo)?) }; - let scores = crate::review::scores(&conn, scope.as_deref()).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let scope = if all_repos { + None + } else { + Some(Self::resolve_repo_or_err(repo)?) + }; + let scores = crate::review::scores(&conn, scope.as_deref()) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::to_string_pretty(&scores).unwrap_or_default()) } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } fn resolve_repo_or_err(repo: Option) -> Result { + } + fn resolve_repo_or_err(repo: Option) -> Result { crate::claims::resolve_repo(repo).ok_or_else(|| { ErrorData::invalid_params( "could not determine repo — run in a git repo or pass repo=owner/name", @@ -1728,78 +1948,185 @@ impl AgentflareMcp { ) -> Result { match req.action.as_str() { "search" => { - let query = req.query.ok_or_else(|| ErrorData::invalid_params("query is required", None))?; - if query.trim().is_empty() { return Err(ErrorData::invalid_params("query is required", None)); } + let query = req + .query + .ok_or_else(|| ErrorData::invalid_params("query is required", None))?; + if query.trim().is_empty() { + return Err(ErrorData::invalid_params("query is required", None)); + } let mode = match req.mode.as_deref() { None | Some("all") => gateway_registry::MatchMode::All, Some("any") => gateway_registry::MatchMode::Any, - Some(other) => return Err(ErrorData::invalid_params(format!("mode must be 'all' or 'any', got '{other}'"), None)), + Some(other) => { + return Err(ErrorData::invalid_params( + format!("mode must be 'all' or 'any', got '{other}'"), + None, + )); + } }; let guard = self.ensure_gateway_registry().await?; let reg = guard.as_ref().expect("ensured above"); - let hits = reg.search(&query, req.limit.unwrap_or(5), mode).map_err(|e| ErrorData::internal_error(e.to_string(), None))?; + let hits = reg + .search(&query, req.limit.unwrap_or(5), mode) + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; Ok(serde_json::to_string_pretty(&hits).unwrap_or_default()) } "execute" => { - let server = req.server.ok_or_else(|| ErrorData::invalid_params("server is required", None))?; - let tool = req.tool.ok_or_else(|| ErrorData::invalid_params("tool is required", None))?; - if server.trim().is_empty() || tool.trim().is_empty() { return Err(ErrorData::invalid_params("server and tool are required", None)); } - let args = req.args.map(serde_json::Value::Object).unwrap_or(serde_json::Value::Null); + let server = req + .server + .ok_or_else(|| ErrorData::invalid_params("server is required", None))?; + let tool = req + .tool + .ok_or_else(|| ErrorData::invalid_params("tool is required", None))?; + if server.trim().is_empty() || tool.trim().is_empty() { + return Err(ErrorData::invalid_params( + "server and tool are required", + None, + )); + } + let args = req + .args + .map(serde_json::Value::Object) + .unwrap_or(serde_json::Value::Null); let guard = self.ensure_gateway_registry().await?; let reg = guard.as_ref().expect("ensured above"); match reg.execute(&server, &tool, args).await { - Ok(value) => { let capped = gateway_registry::truncate_if_needed(&value, gateway_registry::DEFAULT_MAX_CHARS); Ok(serde_json::to_string_pretty(&capped).unwrap_or_default()) } - Err(e @ gateway_registry::GatewayError::ServerNotFound(_)) | Err(e @ gateway_registry::GatewayError::ToolNotFound(_)) | Err(e @ gateway_registry::GatewayError::InvalidArgument(_)) => Err(ErrorData::invalid_params(e.to_string(), None)), - Err(e) => Err(ErrorData::internal_error(gateway_registry::redact_error_for_llm(&e.to_string()), None)), + Ok(value) => { + let capped = gateway_registry::truncate_if_needed( + &value, + gateway_registry::DEFAULT_MAX_CHARS, + ); + Ok(serde_json::to_string_pretty(&capped).unwrap_or_default()) + } + Err(e @ gateway_registry::GatewayError::ServerNotFound(_)) + | Err(e @ gateway_registry::GatewayError::ToolNotFound(_)) + | Err(e @ gateway_registry::GatewayError::InvalidArgument(_)) => { + Err(ErrorData::invalid_params(e.to_string(), None)) + } + Err(e) => Err(ErrorData::internal_error( + gateway_registry::redact_error_for_llm(&e.to_string()), + None, + )), } } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } // --- Memory tools --- + } // --- Memory tools --- #[tool( description = "Memory operations — remember, recall, context, curate, handoff, or relate observations. Single consolidated tool with `action` field (context|curate|handoff|recall|relate|remember)." )] - fn memory( - &self, - Parameters(req): Parameters, - ) -> Result { + fn memory(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "remember" => { - let title = req.title.ok_or_else(|| ErrorData::invalid_params("title is required", None))?; - let content = req.content.ok_or_else(|| ErrorData::invalid_params("content is required", None))?; - let r#type = req.r#type.ok_or_else(|| ErrorData::invalid_params("type is required", None))?; - let input = crate::memory::mcp::RememberInput { title, content, r#type, session_id: req.session_id, project: req.project, topic_key: req.topic_key, scope: req.scope }; - crate::memory::mcp::handle_remember(input).map_err(|e| ErrorData::internal_error(e, None)) + let title = req + .title + .ok_or_else(|| ErrorData::invalid_params("title is required", None))?; + let content = req + .content + .ok_or_else(|| ErrorData::invalid_params("content is required", None))?; + let r#type = req + .r#type + .ok_or_else(|| ErrorData::invalid_params("type is required", None))?; + let input = crate::memory::mcp::RememberInput { + title, + content, + r#type, + session_id: req.session_id, + project: req.project, + topic_key: req.topic_key, + scope: req.scope, + }; + crate::memory::mcp::handle_remember(input) + .map_err(|e| ErrorData::internal_error(e, None)) } "recall" => { - let input = crate::memory::mcp::RecallInput { query: req.query, id: req.id, r#type: req.r#type, project: req.project, limit: req.limit }; - crate::memory::mcp::handle_recall(input).map_err(|e| ErrorData::internal_error(e, None)) + let input = crate::memory::mcp::RecallInput { + query: req.query, + id: req.id, + r#type: req.r#type, + project: req.project, + limit: req.limit, + }; + crate::memory::mcp::handle_recall(input) + .map_err(|e| ErrorData::internal_error(e, None)) } "context" => { - let input = crate::memory::mcp::ContextInput { session_id: req.session_id, project: req.project }; - crate::memory::mcp::handle_context(input).map_err(|e| ErrorData::internal_error(e, None)) + let input = crate::memory::mcp::ContextInput { + session_id: req.session_id, + project: req.project, + }; + crate::memory::mcp::handle_context(input) + .map_err(|e| ErrorData::internal_error(e, None)) } "handoff" => { - let session_id = req.session_id.ok_or_else(|| ErrorData::invalid_params("session_id is required", None))?; - let summary = req.summary.ok_or_else(|| ErrorData::invalid_params("summary is required", None))?; - let input = crate::memory::mcp::HandoffInput { session_id, summary, findings: req.findings, decisions: req.decisions, files_touched: req.files_touched, evidence: req.evidence }; - crate::memory::mcp::handle_handoff(input).map_err(|e| ErrorData::internal_error(e, None)) + let session_id = req + .session_id + .ok_or_else(|| ErrorData::invalid_params("session_id is required", None))?; + let summary = req + .summary + .ok_or_else(|| ErrorData::invalid_params("summary is required", None))?; + let input = crate::memory::mcp::HandoffInput { + session_id, + summary, + findings: req.findings, + decisions: req.decisions, + files_touched: req.files_touched, + evidence: req.evidence, + }; + crate::memory::mcp::handle_handoff(input) + .map_err(|e| ErrorData::internal_error(e, None)) } "relate" => { - let source_id = req.source_id.ok_or_else(|| ErrorData::invalid_params("source_id is required", None))?; - let target_id = req.target_id.ok_or_else(|| ErrorData::invalid_params("target_id is required", None))?; - let relation = req.relation.ok_or_else(|| ErrorData::invalid_params("relation is required", None))?; - let input = crate::memory::mcp::RelateInput { source_id, target_id, relation, reason: req.reason, confidence: req.confidence }; - crate::memory::mcp::handle_relate(input).map_err(|e| ErrorData::internal_error(e, None)) + let source_id = req + .source_id + .ok_or_else(|| ErrorData::invalid_params("source_id is required", None))?; + let target_id = req + .target_id + .ok_or_else(|| ErrorData::invalid_params("target_id is required", None))?; + let relation = req + .relation + .ok_or_else(|| ErrorData::invalid_params("relation is required", None))?; + let input = crate::memory::mcp::RelateInput { + source_id, + target_id, + relation, + reason: req.reason, + confidence: req.confidence, + }; + crate::memory::mcp::handle_relate(input) + .map_err(|e| ErrorData::internal_error(e, None)) } "curate" => { - let id = req.id.ok_or_else(|| ErrorData::invalid_params("id is required", None))?; - let input = crate::memory::mcp::CurateInput { action: req.action.clone(), id, title: req.title, content: req.content, r#type: req.r#type, pinned: req.pinned }; - crate::memory::mcp::handle_curate(input).map_err(|e| ErrorData::internal_error(e, None)) + let id = req + .id + .ok_or_else(|| ErrorData::invalid_params("id is required", None))?; + let curate_action = req.curate_action.ok_or_else(|| { + ErrorData::invalid_params( + "curate_action is required (update|delete|pin|unpin)", + None, + ) + })?; + let input = crate::memory::mcp::CurateInput { + action: curate_action, + id, + title: req.title, + content: req.content, + r#type: req.r#type, + pinned: req.pinned, + }; + crate::memory::mcp::handle_curate(input) + .map_err(|e| ErrorData::internal_error(e, None)) } - other => Err(ErrorData::invalid_params(format!("unknown action: {other}"), None)), + other => Err(ErrorData::invalid_params( + format!("unknown action: {other}"), + None, + )), } - } fn item_inner(&self, req: ItemRequest) -> Result { + } + fn item_inner(&self, req: ItemRequest) -> Result { match req.action.as_str() { "create" => { let name = req.name.ok_or_else(|| { @@ -3077,10 +3404,10 @@ mod tests { fn skill_search_empty_query_is_invalid_params() { let s = AgentflareMcp::default(); let err = s - .skill_search(Parameters(SkillSearchRequest { - query: "".into(), - limit: None, - mode: None, + .skill(Parameters(SkillRequest { + action: "search".into(), + query: Some("".into()), + ..Default::default() })) .unwrap_err(); assert!(err.to_string().contains("query")); @@ -3095,9 +3422,11 @@ mod tests { ..Default::default() }; let out = s - .skill_load(Parameters(SkillLoadRequest { - name: "definitely-not-a-skill-xyz".into(), + .skill(Parameters(SkillRequest { + action: "load".into(), + name: Some("definitely-not-a-skill-xyz".into()), original: false, + ..Default::default() })) .unwrap_err(); assert!(out.to_string().contains("skill_search")); @@ -3107,10 +3436,11 @@ mod tests { fn skill_search_mode_rejects_unknown_value() { let s = AgentflareMcp::default(); let err = s - .skill_search(Parameters(SkillSearchRequest { - query: "anything".into(), - limit: None, + .skill(Parameters(SkillRequest { + action: "search".into(), + query: Some("anything".into()), mode: Some("fuzzy".into()), + ..Default::default() })) .unwrap_err(); assert!(err.to_string().contains("mode")); @@ -3125,10 +3455,10 @@ mod tests { ..Default::default() }; let err = s - .gateway_search(Parameters(GatewaySearchRequest { - query: "".into(), - limit: None, - mode: None, + .gateway(Parameters(GatewayRequest { + action: "search".into(), + query: Some("".into()), + ..Default::default() })) .await .unwrap_err(); @@ -3143,10 +3473,11 @@ mod tests { ..Default::default() }; let err = s - .gateway_search(Parameters(GatewaySearchRequest { - query: "x".into(), - limit: None, + .gateway(Parameters(GatewayRequest { + action: "search".into(), + query: Some("x".into()), mode: Some("bogus".into()), + ..Default::default() })) .await .unwrap_err(); @@ -3161,10 +3492,12 @@ mod tests { ..Default::default() }; let err = s - .gateway_execute(Parameters(GatewayExecuteRequest { - server: "".into(), - tool: "x".into(), + .gateway(Parameters(GatewayRequest { + action: "execute".into(), + server: Some("".into()), + tool: Some("x".into()), args: Some(serde_json::Map::new()), + ..Default::default() })) .await .unwrap_err(); @@ -3182,10 +3515,12 @@ mod tests { ..Default::default() }; let err = s - .gateway_execute(Parameters(GatewayExecuteRequest { - server: "definitely-not-a-configured-server".into(), - tool: "x".into(), + .gateway(Parameters(GatewayRequest { + action: "execute".into(), + server: Some("definitely-not-a-configured-server".into()), + tool: Some("x".into()), args: Some(serde_json::Map::new()), + ..Default::default() })) .await .unwrap_err(); @@ -3195,7 +3530,7 @@ mod tests { #[test] fn gateway_execute_args_schema_is_object_or_null() { - let schema = schemars::schema_for!(GatewayExecuteRequest); + let schema = schemars::schema_for!(GatewayRequest); let schema_json = serde_json::to_value(&schema).unwrap(); let args_schema = schema_json .get("properties") @@ -3232,10 +3567,11 @@ mod tests { ..Default::default() }; let out = s - .artifact_publish(Parameters(ArtifactPublishRequest { - name: "hello".into(), + .artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("hello".into()), r#type: None, - content: "artifact-body-marker".into(), + content: Some("artifact-body-marker".into()), session_id: None, update_id: None, ..Default::default() @@ -3262,10 +3598,11 @@ mod tests { ..Default::default() }; let first: serde_json::Value = serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), r#type: Some("markdown".into()), - content: "v1".into(), + content: Some("v1".into()), session_id: Some("ses-1".into()), update_id: None, ..Default::default() @@ -3276,10 +3613,11 @@ mod tests { let id = first["id"].as_str().unwrap().to_string(); let second: serde_json::Value = serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), r#type: Some("markdown".into()), - content: "v2".into(), + content: Some("v2".into()), session_id: Some("ses-1".into()), update_id: Some(id.clone()), ..Default::default() @@ -3300,10 +3638,11 @@ mod tests { }; let publish = |name: &str, session: &str| -> serde_json::Value { serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: name.into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some(name.into()), r#type: None, - content: format!("content-of-{name}"), + content: Some(format!("content-of-{name}")), session_id: Some(session.into()), update_id: None, description: Some(format!("desc-{name}")), @@ -3317,14 +3656,18 @@ mod tests { let _b = publish("beta", "ses-2"); let all: serde_json::Value = serde_json::from_str( - &s.artifact_list(Parameters(ArtifactListRequest::default())) - .unwrap(), + &s.artifact(Parameters(ArtifactRequest { + action: "list".into(), + ..Default::default() + })) + .unwrap(), ) .unwrap(); assert_eq!(all.as_array().unwrap().len(), 2); let one: serde_json::Value = serde_json::from_str( - &s.artifact_list(Parameters(ArtifactListRequest { + &s.artifact(Parameters(ArtifactRequest { + action: "list".into(), session_id: Some("ses-1".into()), ..Default::default() })) @@ -3337,9 +3680,11 @@ mod tests { let id = a["id"].as_str().unwrap().to_string(); let got: serde_json::Value = serde_json::from_str( - &s.artifact_get(Parameters(ArtifactGetRequest { - id: id.clone(), + &s.artifact(Parameters(ArtifactRequest { + action: "get".into(), + id: Some(id.clone()), version: None, + ..Default::default() })) .unwrap(), ) @@ -3347,14 +3692,23 @@ mod tests { assert_eq!(got["content"], "content-of-alpha"); let del: serde_json::Value = serde_json::from_str( - &s.artifact_delete(Parameters(ArtifactDeleteRequest { id: id.clone() })) - .unwrap(), + &s.artifact(Parameters(ArtifactRequest { + action: "delete".into(), + id: Some(id.clone()), + ..Default::default() + })) + .unwrap(), ) .unwrap(); - assert_eq!(del["deleted"], true); + assert_eq!(del["deleted"], id); let err = s - .artifact_get(Parameters(ArtifactGetRequest { id, version: None })) + .artifact(Parameters(ArtifactRequest { + action: "get".into(), + id: Some(id), + version: None, + ..Default::default() + })) .unwrap_err(); assert_eq!(err.code, rmcp::model::ErrorCode::INVALID_PARAMS); } @@ -3367,10 +3721,11 @@ mod tests { ..Default::default() }; let first: serde_json::Value = serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), r#type: None, - content: "v1".into(), + content: Some("v1".into()), session_id: None, update_id: None, label: Some("draft".into()), @@ -3384,10 +3739,11 @@ mod tests { // stale base_version maps to invalid_params, not internal_error let update = |base: Option, content: &str| { - s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), + s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), r#type: None, - content: content.into(), + content: Some(content.into()), session_id: None, update_id: Some(id.clone()), base_version: base, @@ -3413,9 +3769,10 @@ mod tests { let publish = |name: &str, recipient: Option<&str>, thread: Option<&str>| -> serde_json::Value { serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: name.into(), - content: format!("content {name}"), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some(name.into()), + content: Some(format!("content {name}")), sender: Some("claude-code".into()), recipient: recipient.map(Into::into), thread_id: thread.map(Into::into), @@ -3430,8 +3787,9 @@ mod tests { publish("other", None, None); let inbox: serde_json::Value = serde_json::from_str( - &s.artifact_list(Parameters(ArtifactListRequest { - recipient: Some("codex".into()), + &s.artifact(Parameters(ArtifactRequest { + action: "list".into(), + inbox_recipient: Some("codex".into()), ..Default::default() })) .unwrap(), @@ -3441,7 +3799,8 @@ mod tests { assert_eq!(inbox[0]["name"], "packet"); let thread: serde_json::Value = serde_json::from_str( - &s.artifact_list(Parameters(ArtifactListRequest { + &s.artifact(Parameters(ArtifactRequest { + action: "list".into(), thread_id: Some("t1".into()), ..Default::default() })) @@ -3623,18 +3982,20 @@ mod tests { ..Default::default() }; let first: serde_json::Value = serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), - content: "alpha\nbeta\n".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), + content: Some("alpha\nbeta\n".into()), ..Default::default() })) .unwrap(), ) .unwrap(); let id = first["id"].as_str().unwrap().to_string(); - s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "doc".into(), - content: "alpha\ngamma\n".into(), + s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("doc".into()), + content: Some("alpha\ngamma\n".into()), update_id: Some(id.clone()), ..Default::default() })) @@ -3642,10 +4003,12 @@ mod tests { // to_version omitted = latest let diff = s - .artifact_diff(Parameters(ArtifactDiffRequest { - id, - from_version: 1, + .artifact(Parameters(ArtifactRequest { + action: "diff".into(), + id: Some(id), + from_version: Some(1), to_version: None, + ..Default::default() })) .unwrap(); assert!(diff.contains("-beta"), "{diff}"); @@ -3659,23 +4022,27 @@ mod tests { artifacts_dir_override: Some(tmp.path().to_path_buf()), ..Default::default() }; - s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "alpha".into(), - content: "there is a hidden NEEDLE in here".into(), + s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("alpha".into()), + content: Some("there is a hidden NEEDLE in here".into()), ..Default::default() })) .unwrap(); - s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "beta".into(), - content: "nothing to see".into(), + s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("beta".into()), + content: Some("nothing to see".into()), ..Default::default() })) .unwrap(); let hits: serde_json::Value = serde_json::from_str( - &s.artifact_search(Parameters(ArtifactSearchRequest { - query: "needle".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "search".into(), + query: Some("needle".into()), session_id: None, + ..Default::default() })) .unwrap(), ) @@ -3692,9 +4059,11 @@ mod tests { ); let by_name: serde_json::Value = serde_json::from_str( - &s.artifact_search(Parameters(ArtifactSearchRequest { - query: "beta".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "search".into(), + query: Some("beta".into()), session_id: None, + ..Default::default() })) .unwrap(), ) @@ -3711,18 +4080,21 @@ mod tests { ..Default::default() }; let out: serde_json::Value = serde_json::from_str( - &s.artifact_publish(Parameters(ArtifactPublishRequest { - name: "prov".into(), - content: "x".into(), + &s.artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some("prov".into()), + content: Some("x".into()), ..Default::default() })) .unwrap(), ) .unwrap(); let got: serde_json::Value = serde_json::from_str( - &s.artifact_get(Parameters(ArtifactGetRequest { - id: out["id"].as_str().unwrap().into(), + &s.artifact(Parameters(ArtifactRequest { + action: "get".into(), + id: Some(out["id"].as_str().unwrap().into()), version: None, + ..Default::default() })) .unwrap(), ) @@ -3739,13 +4111,15 @@ mod tests { agent: Some("opencode".into()), ..Default::default() }; - let sender_of = |req: ArtifactPublishRequest| -> serde_json::Value { + let sender_of = |req: ArtifactRequest| -> serde_json::Value { let out: serde_json::Value = - serde_json::from_str(&s.artifact_publish(Parameters(req)).unwrap()).unwrap(); + serde_json::from_str(&s.artifact(Parameters(req)).unwrap()).unwrap(); let got: serde_json::Value = serde_json::from_str( - &s.artifact_get(Parameters(ArtifactGetRequest { - id: out["id"].as_str().unwrap().into(), + &s.artifact(Parameters(ArtifactRequest { + action: "get".into(), + id: Some(out["id"].as_str().unwrap().into()), version: None, + ..Default::default() })) .unwrap(), ) @@ -3753,17 +4127,19 @@ mod tests { got["sender"].clone() }; - let defaulted = sender_of(ArtifactPublishRequest { - name: "defaulted".into(), - content: "x".into(), + let defaulted = sender_of(ArtifactRequest { + action: "publish".into(), + name: Some("defaulted".into()), + content: Some("x".into()), ..Default::default() }); assert_eq!(defaulted, "opencode"); // An explicit sender always wins over the identity default. - let explicit = sender_of(ArtifactPublishRequest { - name: "explicit".into(), - content: "x".into(), + let explicit = sender_of(ArtifactRequest { + action: "publish".into(), + name: Some("explicit".into()), + content: Some("x".into()), sender: Some("codex".into()), ..Default::default() }); @@ -3795,10 +4171,11 @@ mod tests { }; for (name, content) in [("", "x"), ("x", "")] { let err = s - .artifact_publish(Parameters(ArtifactPublishRequest { - name: name.into(), + .artifact(Parameters(ArtifactRequest { + action: "publish".into(), + name: Some(name.into()), r#type: None, - content: content.into(), + content: Some(content.into()), session_id: None, update_id: None, ..Default::default() From 5effe8b01ebc85b263a2659b59e0dc51f3115eaa Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Wed, 15 Jul 2026 01:14:17 +0530 Subject: [PATCH 3/3] fix(fmt): collapse tool() signature onto one line per rustfmt --- src/mcp_server.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/mcp_server.rs b/src/mcp_server.rs index eda2b415..63ad3065 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -1947,10 +1947,7 @@ impl AgentflareMcp { #[tool( description = "Tool operations — search downstream MCP servers' tools by task description or execute one. Single consolidated tool with `action` field (search|execute)." )] - async fn tool( - &self, - Parameters(req): Parameters, - ) -> Result { + async fn tool(&self, Parameters(req): Parameters) -> Result { match req.action.as_str() { "search" => { let query = req