From 8d494af740e97c7a1171fd7d6e8ea7ae19210064 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 13 Aug 2026 19:01:26 +0530 Subject: [PATCH] feat: surface flare doctor --reclaim as an MCP action (item action=doctor) Adds a doctor action to the item MCP tool wrapping flare_git_core::doctor::scan/reclaim (item #235's CLI-only worktree hygiene scan/reclaim) so an MCP-only agent that hits the git worktree remove/prune shim denial has a discoverable path to worktree cleanup instead of a dead end. Also cross-references the new action from the shim's teardown-deny message and the item tool's own description. Agentflare-Agent: claude-code Agentflare-Branch: task/465-surface-flare-doctor-reclaim-via-mcp-wor Agentflare-Item: 465 --- crates/flare-git-core/src/classify.rs | 2 +- src/mcp_server.rs | 6 +- src/mcp_server/builtin_tools.rs | 2 +- src/mcp_server/item_doctor.rs | 55 +++++++++++ src/mcp_server/tests/item_doctor_tests.rs | 107 ++++++++++++++++++++++ src/mcp_server/tests/mod.rs | 1 + src/mcp_server/types.rs | 14 ++- 7 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 src/mcp_server/item_doctor.rs create mode 100644 src/mcp_server/tests/item_doctor_tests.rs diff --git a/crates/flare-git-core/src/classify.rs b/crates/flare-git-core/src/classify.rs index 49c2edc2..e360016b 100644 --- a/crates/flare-git-core/src/classify.rs +++ b/crates/flare-git-core/src/classify.rs @@ -450,7 +450,7 @@ pub fn classify_pure( Some("remove") | Some("prune") ); let reason = if teardown { - "'git worktree remove/prune' is orchestrator-managed by agentflare — to tear down an item's worktree call `item(action=\"check_merge\", id=)` once its PR merges, or `item(action=\"release\", id=)`; to prune stale worktrees run `agentflare git worktree audit --prune`.".to_string() + "'git worktree remove/prune' is orchestrator-managed by agentflare — to tear down an item's worktree call `item(action=\"check_merge\", id=)` once its PR merges, or `item(action=\"release\", id=)`; to prune stale worktrees run `agentflare git worktree audit --prune`, or from an MCP-only session call `item(action=\"doctor\", reclaim=true)` (same scan/reclaim as `agentflare git doctor --reclaim`).".to_string() } else { "'git worktree' is orchestrator-managed by agentflare — call `item(action=\"claim\", id=)` to provision one. (Not the standalone `claim`/`mcp__flare__claim` tool -- that only takes a scope lock and does not create a worktree.)".to_string() }; diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 755a03bb..aa9337b9 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -11,6 +11,7 @@ mod flare_docs; mod flare_git; mod handoff; pub(crate) mod item; +mod item_doctor; mod memory_tool; mod project_resolution; mod review; @@ -1234,9 +1235,10 @@ impl AgentflareMcp { "groom" => self.item_groom(req), "standup" => self.item_standup(req), "health" => self.item_health(req), + "doctor" => self.item_doctor(req), other => Err(ErrorData::invalid_params( format!( - "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health" + "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health|doctor" ), None, )), @@ -1244,7 +1246,7 @@ impl AgentflareMcp { } #[tool( - description = "Manage work items in the repo's linked project. Single consolidated tool with `action` field (create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health). `groom` returns a priority+staleness-ranked shortlist with description, stale/unassigned/blocked/duplicate flags, and a pull_next list — all in one call, no per-item `get` round trips needed. `standup` returns done/in_progress(grouped by assignee)/stuck buckets computed server-side. `health` returns a velocity/WIP/stuck/bottlenecks scorecard (`bottlenecks` = items handed between agents ≥2× in the window; history starts at the assignment-log migration). The read-only reporting actions groom|standup|health accept a `project` override (name or UUID from `project action=list`) for portfolio roll-ups. `done` moves an item to \"in_review\" (not \"completed\") when it results in an open PR, and leaves the worktree in place for follow-up commits; call `check_merge` once the PR is confirmed merged to promote it to \"completed\" and clean up the worktree. Pass `summary` on `done` with what you changed and why — it becomes the PR body; omitting it leaves the PR with a generic placeholder description." + description = "Manage work items in the repo's linked project. Single consolidated tool with `action` field (create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health|doctor). `groom` returns a priority+staleness-ranked shortlist with description, stale/unassigned/blocked/duplicate flags, and a pull_next list — all in one call, no per-item `get` round trips needed. `standup` returns done/in_progress(grouped by assignee)/stuck buckets computed server-side. `health` returns a velocity/WIP/stuck/bottlenecks scorecard (`bottlenecks` = items handed between agents ≥2× in the window; history starts at the assignment-log migration). The read-only reporting actions groom|standup|health accept a `project` override (name or UUID from `project action=list`) for portfolio roll-ups. `done` moves an item to \"in_review\" (not \"completed\") when it results in an open PR, and leaves the worktree in place for follow-up commits; call `check_merge` once the PR is confirmed merged to promote it to \"completed\" and clean up the worktree. Pass `summary` on `done` with what you changed and why — it becomes the PR body; omitting it leaves the PR with a generic placeholder description. `doctor` is the MCP equivalent of `agentflare git doctor`: scans every worktree in this repo for dirty/stale/orphaned/duplicate-branch/missing-upstream health flags (respects `staleness_days`, default 14) and, with `reclaim=true`, deletes the clean stale/orphaned ones (never the main worktree; add `force=true` to also delete dirty ones) — this is the tool to reach for a `git worktree remove/prune` shim denial, not a specific item's `check_merge`/`release`." )] fn item(&self, Parameters(req): Parameters) -> Result { self.item_inner(req) diff --git a/src/mcp_server/builtin_tools.rs b/src/mcp_server/builtin_tools.rs index c70b12a3..6a7c7c18 100644 --- a/src/mcp_server/builtin_tools.rs +++ b/src/mcp_server/builtin_tools.rs @@ -88,7 +88,7 @@ const BUILTIN_TOOLS: &[(&str, &str)] = &[ ), ( "item", - "Manage work items in the repo's linked project. Single consolidated tool with `action` field (create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health).", + "Manage work items in the repo's linked project. Single consolidated tool with `action` field (create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health|doctor). `doctor` scans/reclaims stale or orphaned worktrees in this repo -- the MCP equivalent of `agentflare git doctor --reclaim`.", ), ( "comment", diff --git a/src/mcp_server/item_doctor.rs b/src/mcp_server/item_doctor.rs new file mode 100644 index 00000000..aef3e0d3 --- /dev/null +++ b/src/mcp_server/item_doctor.rs @@ -0,0 +1,55 @@ +//! `item(action="doctor")` -- split out of `item.rs` to keep it under the +//! repo's LOC gate. MCP surface for `agentflare git doctor [--reclaim] +//! [--force]` (item #235) -- until now that worktree-hygiene scan/reclaim +//! only existed as a CLI command, so an agent hitting the `git worktree +//! remove/prune` shim denial (see `classify::classify_pure`'s +//! teardown-deny message) had no MCP-reachable way to actually clean up +//! stale/orphaned worktrees (item #465). + +use super::*; + +impl AgentflareMcp { + /// `sequence_id` (as a string, matching `LaneHealth`) -> its state's + /// `group_name` -- used by `item_doctor` to flag a worktree as orphaned + /// when the item behind it is done but the worktree wasn't cleaned up. + /// Mirrors `cli::git::item_state_groups`'s query, adapted to the MCP + /// server's own DB-connection accessor. Best-effort like that sibling: + /// any DB error just yields an empty map, so orphan detection silently + /// finds nothing rather than failing the whole scan. + fn item_state_groups_for_doctor(&self) -> std::collections::HashMap { + self.with_backend_db(|conn| { + let mut stmt = match conn.prepare( + "SELECT i.sequence_id, s.group_name FROM items i \ + JOIN states s ON i.state_id = s.id WHERE i.deleted_at IS NULL", + ) { + Ok(s) => s, + Err(_) => return std::collections::HashMap::new(), + }; + match stmt.query_map([], |r| Ok((r.get::<_, i64>(0)?, r.get::<_, String>(1)?))) { + Ok(rows) => rows + .filter_map(|r| r.ok()) + .map(|(seq, group)| (seq.to_string(), group)) + .collect(), + Err(_) => std::collections::HashMap::new(), + } + }) + .unwrap_or_default() + } + + pub(crate) fn item_doctor(&self, req: ItemRequest) -> Result { + let repo_root = self.worktree_repo_root(); + let staleness_days = req.staleness_days.unwrap_or(14).max(0) as u64; + let item_states = self.item_state_groups_for_doctor(); + let report = flare_git_core::doctor::scan(&repo_root, staleness_days, &item_states); + let reclaimed = if req.reclaim.unwrap_or(false) { + flare_git_core::doctor::reclaim(&repo_root, &report, req.force.unwrap_or(false)) + } else { + Vec::new() + }; + let mut value = serde_json::to_value(&report).unwrap_or_default(); + if let Some(obj) = value.as_object_mut() { + obj.insert("reclaimed".to_string(), serde_json::json!(reclaimed)); + } + Ok(serde_json::to_string_pretty(&value).unwrap_or_default()) + } +} diff --git a/src/mcp_server/tests/item_doctor_tests.rs b/src/mcp_server/tests/item_doctor_tests.rs new file mode 100644 index 00000000..e1ba2123 --- /dev/null +++ b/src/mcp_server/tests/item_doctor_tests.rs @@ -0,0 +1,107 @@ +use super::*; + +#[test] +fn item_doctor_scans_without_reclaiming_by_default() { + // Item #465: `agentflare git doctor` (item #235) was CLI-only -- an + // MCP-only agent hitting the `git worktree remove/prune` shim denial + // had no reachable way to run it. `reclaim` defaults to false, matching + // the CLI's own `--reclaim` opt-in flag. + let tmp = tempfile::tempdir().unwrap(); + let repo_dir = tempfile::tempdir().unwrap(); + let repo_root = repo_dir.path().to_path_buf(); + let run_git = |args: &[&str]| { + std::process::Command::new("git") + .args(args) + .current_dir(&repo_root) + .output() + .unwrap() + }; + run_git(&["init", "-b", "master"]); + run_git(&["config", "user.email", "test@test.com"]); + run_git(&["config", "user.name", "Test"]); + run_git(&["commit", "--allow-empty", "-m", "initial"]); + + let s = AgentflareMcp { + backend_db_override: Some(tmp.path().join("backend.db")), + backend_project_link_override: Some(tmp.path().join("project.json")), + worktree_repo_root_override: Some(repo_root), + ..Default::default() + }; + + let report: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "doctor".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(report["lanes"].as_array().unwrap().len(), 1, "{report:?}"); + assert_eq!( + report["reclaimed"].as_array().unwrap().len(), + 0, + "reclaim defaults to false — nothing must be deleted: {report:?}" + ); +} + +#[test] +fn item_doctor_reclaims_a_stale_clean_linked_worktree() { + let tmp = tempfile::tempdir().unwrap(); + let repo_dir = tempfile::tempdir().unwrap(); + let repo_root = repo_dir.path().to_path_buf(); + let run_git = |dir: &std::path::Path, args: &[&str]| { + std::process::Command::new("git") + .args(args) + .current_dir(dir) + .output() + .unwrap() + }; + run_git(&repo_root, &["init", "-b", "master"]); + run_git(&repo_root, &["config", "user.email", "test@test.com"]); + run_git(&repo_root, &["config", "user.name", "Test"]); + run_git(&repo_root, &["commit", "--allow-empty", "-m", "initial"]); + + let linked_path = repo_dir.path().join("linked-wt"); + run_git( + &repo_root, + &[ + "worktree", + "add", + "-b", + "linked-branch", + linked_path.to_str().unwrap(), + ], + ); + assert!(linked_path.exists()); + + let s = AgentflareMcp { + backend_db_override: Some(tmp.path().join("backend.db")), + backend_project_link_override: Some(tmp.path().join("project.json")), + worktree_repo_root_override: Some(repo_root), + ..Default::default() + }; + + let report: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "doctor".into(), + // 0 days makes every lane immediately stale, so the clean + // linked worktree is reclaim-eligible without needing to fake + // an old commit timestamp. + staleness_days: Some(0), + reclaim: Some(true), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let reclaimed = report["reclaimed"].as_array().unwrap(); + assert_eq!( + reclaimed, + &vec![serde_json::json!("linked-branch")], + "{report:?}" + ); + assert!( + !linked_path.exists(), + "reclaim=true must delete a clean stale linked worktree" + ); +} diff --git a/src/mcp_server/tests/mod.rs b/src/mcp_server/tests/mod.rs index 070f33e1..fe189146 100644 --- a/src/mcp_server/tests/mod.rs +++ b/src/mcp_server/tests/mod.rs @@ -351,6 +351,7 @@ mod action_tests; mod artifact_tests; mod asset_tests; mod item_commit_failure_tests; +mod item_doctor_tests; mod item_pr_failure_tests; mod item_tests; mod item_tests_reporting; diff --git a/src/mcp_server/types.rs b/src/mcp_server/types.rs index 486bb07a..0995e7bb 100644 --- a/src/mcp_server/types.rs +++ b/src/mcp_server/types.rs @@ -697,7 +697,7 @@ pub(crate) fn base64_encode(bytes: &[u8]) -> String { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] pub(crate) struct ItemRequest { #[schemars( - description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health" + description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|check_merge|cancel|add_label|remove_label|groom|standup|health|doctor" )] pub(crate) action: String, #[schemars( @@ -765,7 +765,7 @@ pub(crate) struct ItemRequest { #[serde(default)] pub(crate) query: Option, #[schemars( - description = "Days since updated_at before an item counts as stale/stuck (groom: default 14; standup/health: default 7)" + description = "Days since updated_at before an item counts as stale/stuck (groom: default 14; standup/health: default 7). Also used by doctor as the worktree-staleness threshold (default 14)." )] #[serde(default)] pub(crate) staleness_days: Option, @@ -797,6 +797,16 @@ pub(crate) struct ItemRequest { )] #[serde(default)] pub(crate) project: Option, + #[schemars( + description = "doctor only: also delete clean stale/orphaned worktrees (never the main worktree, and never a dirty one unless `force` is set). Default false: scan-only." + )] + #[serde(default)] + pub(crate) reclaim: Option, + #[schemars( + description = "doctor only: with reclaim=true, also delete lanes flagged dirty (uncommitted changes). Default false." + )] + #[serde(default)] + pub(crate) force: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full