diff --git a/.claude/commands/pm.md b/.claude/commands/pm.md new file mode 100644 index 00000000..ed0592f4 --- /dev/null +++ b/.claude/commands/pm.md @@ -0,0 +1,43 @@ +--- +description: Act as the project's PM — bare /pm enables PM mode and runs the daily kickoff; subcommands give targeted reports (standup, groom, plan, health, portfolio) +argument-hint: [standup|groom|plan|health|portfolio|mode] [args] +--- + +Parse "$ARGUMENTS": the first word (if any) is the subcommand; everything after +it is that subcommand's arguments. + +## Bare `/pm` (no arguments) — start the PM day + +1. Load the `pm-mode` skill and stay in PM mode (create and dispatch work + instead of implementing it yourself) until `/pm mode off`. +2. Load the `pm` skill and run the daily kickoff, in this order: + a. **Standup** — `/pm:standup` workflow (last 24h): what shipped, what's in + flight per assignee, what's stuck. + b. **Intake triage** — `/pm:groom` workflow's flag lists only: new/unassigned + items, blocked items, likely duplicates, unestimated items. + c. **Blockers first** — for each stuck or blocked item, say what unblocks it + and who should act. + d. **Pull next** — the groom `pull_next` shortlist, cross-checked against + priorities. +3. Close with a **morning briefing**: ≤10 lines — Done / In flight / Stuck / + Recommended dispatches — then propose the concrete dispatch actions PM mode + allows (assignments, handoffs, item creation) and wait for approval before + executing any of them. + +## Subcommands (targeted, read-only reports — no PM mode change) + +- **standup** `[cutoff-hours]` — `pm` skill `/pm:standup`; cutoff default 24. +- **groom** `[staleness-days] [rice|wsjf|value-effort]` — `/pm:groom`; + staleness default 14, framework default `rice`. +- **plan** `[~capacity] [rice|wsjf|value-effort]` — `/pm:plan`; capacity hint + like `~8` caps the Now bucket. +- **health** `[window-weeks]` — `/pm:health`; window default 4. +- **portfolio** `[standup|health] [args]` — `/pm:portfolio`: the chosen report + (default `health`) rolled up across every project in the workspace. +- **mode on** — enable PM mode without the daily kickoff. +- **mode off** — leave PM mode: stop following `pm-mode`, return to normal + implementation behavior, confirm in one line. + +The `pm` skill's report workflows are read-only over items — item mutations +happen only through PM-mode dispatch actions the user has approved. Unknown +subcommand → one-line usage summary, then stop. diff --git a/.claude/skills/pm/SKILL.md b/.claude/skills/pm/SKILL.md index 95e42a9d..7af13d55 100644 --- a/.claude/skills/pm/SKILL.md +++ b/.claude/skills/pm/SKILL.md @@ -1,6 +1,6 @@ --- name: pm -description: Product management for the current agentflare project — run /pm:standup (daily activity digest), /pm:groom (backlog grooming + RICE/ICE/WSJF/Value-Effort prioritization), /pm:plan (Now/Next/Later sprint bucketing), or /pm:health (velocity + WIP + bottleneck scorecard). Read-only; operates on agentflare items via MCP. +description: Product management for the current agentflare project — run /pm:standup (daily activity digest), /pm:groom (backlog grooming + RICE/ICE/WSJF/Value-Effort prioritization), /pm:plan (Now/Next/Later sprint bucketing), /pm:health (velocity + WIP + bottleneck scorecard), or /pm:portfolio (cross-project roll-up). Read-only; operates on agentflare items via MCP. --- # PM Agent — product management over agentflare items @@ -17,8 +17,10 @@ All content authored from public PM methodologies (RICE, ICE, WSJF, Value-Effort ## Scope -One project only — whichever project the current repo resolves to. No -cross-project aggregation. +Default: one project — whichever project the current repo resolves to. +`/pm:portfolio` is the one exception: it loops the read-only reports across +every project in the workspace via the `project` override param (still +read-only, still one workspace). ## Workflows @@ -96,9 +98,27 @@ Arg: window in weeks (default 4). returns `velocity` (oldest→newest weekly series + `velocity_trend`: up/down/flat), `wip` (list + count), `stuck` (WIP older than `staleness_days`, default 7), and `bottlenecks`/`bottleneck_note`. -2. `bottlenecks` is currently always empty — agentflare has no persisted - handoff-history log distinct from item state yet, so this can't be - computed server-side. Print `bottleneck_note` verbatim ("no handoff - history") rather than inventing a signal. +2. `bottlenecks` lists items handed between different agents ≥2× inside the + window, computed server-side from the persisted assignment log (written on + every claim/reassignment). Print each entry as returned (`#N name — K + handoffs (owner chain)`), plus `bottleneck_note` — it carries the one + caveat that matters: history starts at the assignment-log migration, so + older transitions are invisible. 3. One-glance scorecard: Velocity · WIP · Stuck · Bottlenecks. 4. Print the time-signal caveat. Read-only. + +### /pm:portfolio — cross-project roll-up + +Args: which report (`health` default, or `standup`); the report's own args +pass through (window weeks / cutoff hours). + +1. One call: `project action="list"` — every project in the linked workspace. +2. For each project, one call: `item action="" + project=` — the `project` override is honored only by the + read-only reporting actions, so this stays mutation-free by construction. +3. Print one roll-up table, one row per project: + - health: project · velocity trend · WIP · stuck · bottleneck count. + - standup: project · done · in-progress · stuck counts. + Follow with a short "needs attention" list: any project with stuck items, + a `down` velocity trend, or non-empty bottlenecks, and why. +4. Print the time-signal caveat once (it applies to every row). Read-only. diff --git a/crates/agentflare-backend/src/assignment_events.rs b/crates/agentflare-backend/src/assignment_events.rs new file mode 100644 index 00000000..732d8c43 --- /dev/null +++ b/crates/agentflare-backend/src/assignment_events.rs @@ -0,0 +1,232 @@ +//! Append-only log of item assignee transitions — the persisted "handoff +//! history" the health scorecard's bottleneck signal reads. Rows are written +//! by [`crate::item::update`] whenever `assignee_agent` actually changes +//! (which also covers `claim`, since claiming assigns through `update`). +//! History starts at the migration that shipped the table; transitions +//! before it are unrecorded. + +use rusqlite::Connection; + +use crate::item::agent_part; + +fn now() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +/// One item's handoff activity within a window: how many times it moved +/// between *different* agents (instance suffixes stripped — `claude:1` → +/// `claude:2` is not a handoff), and the distinct owner chain in order of +/// first appearance. +#[derive(Debug)] +pub struct HandoffStat { + pub item_id: String, + pub handoffs: usize, + pub owners: Vec, +} + +/// Records one assignee transition. Called from `item::update` inside the +/// caller's transaction so the event commits (or rolls back) with the +/// assignment itself. +pub(crate) fn record( + conn: &Connection, + item_id: &str, + from_owner: Option<&str>, + to_owner: &str, +) -> rusqlite::Result<()> { + conn.execute( + "INSERT INTO item_assignment_events (id, item_id, from_owner, to_owner, created_at) + VALUES (?1, ?2, ?3, ?4, ?5)", + rusqlite::params![db_kit::ids::new_id(), item_id, from_owner, to_owner, now()], + )?; + Ok(()) +} + +/// Handoff stats per item for a project, over events at or after `since`. +/// Only items with at least one agent-to-agent handoff are returned; the +/// caller picks its own "repeatedly" threshold. +pub fn handoff_stats_since( + conn: &Connection, + project_id: &str, + since: i64, +) -> crate::error::Result> { + let mut stmt = conn.prepare( + "SELECT e.item_id, e.from_owner, e.to_owner + FROM item_assignment_events e + JOIN items i ON i.id = e.item_id + WHERE i.project_id = ?1 AND i.deleted_at IS NULL AND e.created_at >= ?2 + ORDER BY e.item_id, e.created_at", + )?; + let rows = stmt.query_map(rusqlite::params![project_id, since], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, Option>(1)?, + row.get::<_, String>(2)?, + )) + })?; + + let mut stats: Vec = Vec::new(); + for row in rows { + let (item_id, from_owner, to_owner) = row?; + let to_agent = agent_part(&to_owner); + if stats.last().map(|s| s.item_id.as_str()) != Some(item_id.as_str()) { + stats.push(HandoffStat { + item_id, + handoffs: 0, + owners: Vec::new(), + }); + } + let stat = stats.last_mut().expect("pushed above"); + if from_owner + .as_deref() + .is_some_and(|f| agent_part(f) != to_agent) + { + stat.handoffs += 1; + } + if let Some(from) = from_owner.as_deref().map(agent_part) + && !stat.owners.contains(&from) + { + stat.owners.push(from); + } + if !stat.owners.contains(&to_agent) { + stat.owners.push(to_agent); + } + } + stats.retain(|s| s.handoffs >= 1); + Ok(stats) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{item, project, state, workspace}; + + fn seed() -> (rusqlite::Connection, String, String) { + let conn = crate::db::open_in_memory().unwrap(); + let ws = workspace::create( + &conn, + workspace::CreateWorkspace { + name: "W".into(), + slug: "w".into(), + item_label: None, + owner_agent: None, + }, + ) + .unwrap(); + let proj = project::create( + &conn, + project::CreateProject { + workspace_id: ws.id, + name: "P".into(), + identifier: "P".into(), + external_source: None, + external_id: None, + }, + ) + .unwrap(); + let backlog = state::list_by_project(&conn, &proj.id) + .unwrap() + .into_iter() + .find(|s| s.group_name == "backlog") + .unwrap(); + (conn, proj.id, backlog.id) + } + + fn make_item(conn: &rusqlite::Connection, pid: &str, sid: &str) -> item::Item { + item::create( + conn, + item::CreateItem { + project_id: pid.to_string(), + state_id: sid.to_string(), + name: "I".into(), + description: None, + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: Vec::new(), + assignee_ids: Vec::new(), + dependency_ids: Vec::new(), + }, + ) + .unwrap() + } + + fn assign(conn: &rusqlite::Connection, id: &str, agent: &str) { + item::update( + conn, + id, + item::UpdateItem { + assignee_agent: Some(agent.to_string()), + ..Default::default() + }, + ) + .unwrap(); + } + + #[test] + fn update_records_a_transition_only_when_the_assignee_changes() { + let (conn, pid, sid) = seed(); + let it = make_item(&conn, &pid, &sid); + assign(&conn, &it.id, "alice"); // None -> alice + assign(&conn, &it.id, "alice"); // no change, no event + assign(&conn, &it.id, "bob"); // alice -> bob + let count: i64 = conn + .query_row( + "SELECT count(*) FROM item_assignment_events WHERE item_id = ?1", + [&it.id], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(count, 2); + } + + #[test] + fn claim_records_a_transition_through_update() { + let (conn, pid, sid) = seed(); + let it = make_item(&conn, &pid, &sid); + item::claim(&conn, &it.id, "alice:1", 1000, 600).unwrap(); + let count: i64 = conn + .query_row( + "SELECT count(*) FROM item_assignment_events WHERE item_id = ?1", + [&it.id], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(count, 1); + } + + #[test] + fn handoff_stats_count_agent_changes_not_first_assignment_or_instances() { + let (conn, pid, sid) = seed(); + let it = make_item(&conn, &pid, &sid); + assign(&conn, &it.id, "alice"); // first assignment — not a handoff + assign(&conn, &it.id, "alice:2"); // same agent, other instance — not a handoff + assign(&conn, &it.id, "bob"); // handoff 1 + assign(&conn, &it.id, "carol"); // handoff 2 + let stats = handoff_stats_since(&conn, &pid, 0).unwrap(); + assert_eq!(stats.len(), 1); + assert_eq!(stats[0].handoffs, 2); + assert_eq!(stats[0].owners, vec!["alice", "bob", "carol"]); + } + + #[test] + fn handoff_stats_respect_the_since_cutoff_and_skip_no_handoff_items() { + let (conn, pid, sid) = seed(); + let it = make_item(&conn, &pid, &sid); + assign(&conn, &it.id, "alice"); // only a first assignment + assert!(handoff_stats_since(&conn, &pid, 0).unwrap().is_empty()); + assign(&conn, &it.id, "bob"); + let far_future = now() + 10_000; + assert!( + handoff_stats_since(&conn, &pid, far_future) + .unwrap() + .is_empty() + ); + } +} diff --git a/crates/agentflare-backend/src/item/crud.rs b/crates/agentflare-backend/src/item/crud.rs index 9f9f2643..b2c30bb9 100644 --- a/crates/agentflare-backend/src/item/crud.rs +++ b/crates/agentflare-backend/src/item/crud.rs @@ -169,6 +169,13 @@ pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { .assignee_agent .as_deref() .map(agent_registry::canonicalize); + // Snapshot the outgoing assignee before the write so the assignment log + // can record the transition (only fetched when the assignee is changing). + let previous_assignee = if assignee_agent.is_some() { + Some(get(conn, id)?.assignee_agent) + } else { + None + }; let mut sets = vec!["updated_at = ?2".to_string()]; let mut param_idx = 3; if input.name.is_some() { @@ -231,6 +238,11 @@ pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { if changed == 0 { return Err(crate::error::Error::NotFound(id.to_string())); } + if let (Some(new_assignee), Some(old_assignee)) = (&assignee_agent, &previous_assignee) + && old_assignee.as_deref() != Some(new_assignee.as_str()) + { + crate::assignment_events::record(conn, id, old_assignee.as_deref(), new_assignee)?; + } let item = get(conn, id)?; if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { events::emit( diff --git a/crates/agentflare-backend/src/lib.rs b/crates/agentflare-backend/src/lib.rs index 3dbf2b7e..2065d7b3 100644 --- a/crates/agentflare-backend/src/lib.rs +++ b/crates/agentflare-backend/src/lib.rs @@ -1,5 +1,6 @@ pub mod ask_event; pub mod asset; +pub mod assignment_events; pub mod bridge_repo; pub mod claim; pub mod comment; diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 87c1d390..ca06898a 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -960,6 +960,37 @@ impl AgentflareMcp { Ok(project) } + /// Resolves the project a read-only reporting action should run against: + /// the `project` override (name, case-insensitive, or UUID — looked up in + /// the linked workspace) when given, else the repo's linked project. + /// Lookup-only for overrides: none of `resolve_project`'s link-file / + /// bridge-registration side effects apply to a project this repo merely + /// reports on. + pub(crate) fn resolve_project_for_read( + &self, + conn: &rusqlite::Connection, + project_override: Option<&str>, + ) -> Result { + let Some(wanted) = project_override.map(str::trim).filter(|s| !s.is_empty()) else { + return self.resolve_project(conn); + }; + let workspace_id = Self::resolve_workspace_id(conn)?; + let projects = agentflare_backend::project::list_by_workspace(conn, &workspace_id) + .map_err(map_backend_err)?; + projects + .into_iter() + .find(|p| p.id == wanted || p.name.eq_ignore_ascii_case(wanted)) + .ok_or_else(|| { + ErrorData::invalid_params( + format!( + "project '{wanted}' not found in the linked workspace — \ + use `project action=list` to see valid names/ids" + ), + None, + ) + }) + } + /// Refreshes this repo's row in the local GitHub bridge's repo registry /// (`bridge_repos`) — the reverse of `project.json`'s folder→project /// link, indexed by repo instead so the daemon (no reliable cwd, so it @@ -1463,7 +1494,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 scorecard; `bottlenecks` is currently always empty — no handoff log is persisted yet, see `bottleneck_note`. `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). `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." )] fn item(&self, Parameters(req): Parameters) -> Result { self.item_inner(req) @@ -1638,15 +1669,24 @@ impl AgentflareMcp { let project = self.resolve_project(conn)?; Ok(serde_json::to_string_pretty(&project).unwrap_or_default()) })?, + "list" => self.with_backend_db(|conn| { + // Ensure this repo's own project exists/links first so a fresh + // workspace still lists at least the current project. + let _ = self.resolve_project(conn)?; + let workspace_id = Self::resolve_workspace_id(conn)?; + let projects = agentflare_backend::project::list_by_workspace(conn, &workspace_id) + .map_err(map_backend_err)?; + Ok(serde_json::to_string_pretty(&projects).unwrap_or_default()) + })?, other => Err(ErrorData::invalid_params( - format!("unknown project action: '{other}' — expected info"), + format!("unknown project action: '{other}' — expected info|list"), None, )), } } #[tool( - description = "Show the workspace/project this repo is currently linked to (auto-created/linked on first use). The `action` field selects the operation (only `info` for now)." + description = "Workspace/project linkage. `info` shows the project this repo is linked to (auto-created/linked on first use); `list` shows every project in the linked workspace — the valid targets for the item tool's read-only `project` override (portfolio roll-ups)." )] fn project(&self, Parameters(req): Parameters) -> Result { self.project_inner(req) diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index ffe59520..b02f16ce 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -1068,7 +1068,7 @@ impl AgentflareMcp { // SQLite `IN (...)` parameter list built from it. let cap = req.limit.unwrap_or(15).clamp(0, MAX_GROOM_LIMIT) as usize; self.with_backend_db(|conn| { - let project = self.resolve_project(conn)?; + let project = self.resolve_project_for_read(conn, req.project.as_deref())?; let mut items = agentflare_backend::item::list_by_project(conn, &project.id) .map_err(map_backend_err)?; let states = agentflare_backend::state::list_by_project(conn, &project.id) @@ -1182,7 +1182,7 @@ impl AgentflareMcp { let cutoff_hours = req.cutoff_hours.unwrap_or(24).max(0); let stuck_days = req.staleness_days.unwrap_or(7).max(0); self.with_backend_db(|conn| { - let project = self.resolve_project(conn)?; + let project = self.resolve_project_for_read(conn, req.project.as_deref())?; let mut items = agentflare_backend::item::list_by_project(conn, &project.id) .map_err(map_backend_err)?; let states = agentflare_backend::state::list_by_project(conn, &project.id) @@ -1268,22 +1268,20 @@ impl AgentflareMcp { } /// One-call health scorecard: velocity (trailing weekly windows, updated_at - /// proxy per rubric.md), WIP, stuck, and a bottlenecks placeholder. + /// proxy per rubric.md), WIP, stuck, and bottlenecks (items handed between + /// agents ≥2× in the window, from the `item_assignment_events` log written + /// by `item::update` — history starts at that migration, so transitions + /// predating it are not counted). /// - /// No precomputed/event-populated rollup table backs velocity — checked - /// first: `events::emit` (agentflare-backend/src/events.rs) is outbound - /// webhook delivery only, not a persisted log, and there's no handoff- - /// history table either (`handoff` is assign + asset version + comment, - /// not a separate audit log). Building either is real new schema/migration - /// work; at this project's actual scale (~40 items) a live scan is - /// sub-millisecond (see the groom benchmark), so adding that - /// infrastructure now would be speculative. Revisit if item volume grows - /// enough that this scan is ever measured as slow — don't estimate it. + /// No precomputed rollup table backs velocity — at this project's actual + /// scale (~40 items) a live scan is sub-millisecond (see the groom + /// benchmark). Revisit if item volume grows enough that this scan is ever + /// measured as slow — don't estimate it. pub(super) fn item_health(&self, req: ItemRequest) -> Result { let window_weeks = req.window_weeks.unwrap_or(4).clamp(1, MAX_WINDOW_WEEKS); let stuck_days = req.staleness_days.unwrap_or(7).max(0); self.with_backend_db(|conn| { - let project = self.resolve_project(conn)?; + let project = self.resolve_project_for_read(conn, req.project.as_deref())?; let items = agentflare_backend::item::list_by_project(conn, &project.id) .map_err(map_backend_err)?; let states = agentflare_backend::state::list_by_project(conn, &project.id) @@ -1356,6 +1354,42 @@ impl AgentflareMcp { .cloned() .collect(); + let window_start = now - window_weeks.saturating_mul(7 * 86_400); + let handoff_stats = agentflare_backend::assignment_events::handoff_stats_since( + conn, + &project.id, + window_start, + ) + .map_err(map_backend_err)?; + let item_by_id: std::collections::HashMap<&str, &agentflare_backend::item::Item> = + items.iter().map(|i| (i.id.as_str(), i)).collect(); + let bottlenecks: Vec = handoff_stats + .iter() + .filter(|s| s.handoffs >= 2) + .map(|s| { + let label = item_by_id + .get(s.item_id.as_str()) + .map(|i| format!("#{} {}", i.sequence_id, i.name)) + .unwrap_or_else(|| s.item_id.clone()); + format!( + "{label} — {} handoffs ({})", + s.handoffs, + s.owners.join(" → ") + ) + }) + .collect(); + let bottleneck_note = if bottlenecks.is_empty() { + "no item was handed between agents ≥2× in the window (handoff history \ + is recorded from the assignment-log migration onward — earlier \ + transitions are not counted)" + .to_string() + } else { + format!( + "items handed between agents ≥2× in the last {window_weeks} week(s) — \ + repeated handoffs usually mean unclear ownership or a stuck dependency" + ) + }; + let resp = HealthResponse { window_weeks, velocity, @@ -1365,10 +1399,8 @@ impl AgentflareMcp { stuck_days, stuck_count: stuck.len(), stuck, - bottlenecks: Vec::new(), - bottleneck_note: "no handoff history — agentflare does not persist a handoff \ - log distinct from item state today" - .to_string(), + bottlenecks, + bottleneck_note, }; Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) })? diff --git a/src/mcp_server/tests/item_tests.rs b/src/mcp_server/tests/item_tests.rs index 1d2ea9c3..3230a722 100644 --- a/src/mcp_server/tests/item_tests.rs +++ b/src/mcp_server/tests/item_tests.rs @@ -1049,72 +1049,6 @@ fn item_standup_buckets_done_in_progress_grouped_and_stuck() { assert_eq!(standup["stuck_count"], 0); } -#[test] -fn item_health_reports_velocity_wip_and_bottleneck_placeholder() { - let (_tmp, s) = harness(); - let project_id: serde_json::Value = - serde_json::from_str(&s.item(Parameters(empty_item_create("bootstrap"))).unwrap()).unwrap(); - let project_id = project_id["project_id"].as_str().unwrap().to_string(); - let conn = backend_conn(&_tmp); - let states = agentflare_backend::state::list_by_project(&conn, &project_id).unwrap(); - let started_state = states - .iter() - .find(|st| st.group_name == "started") - .unwrap() - .id - .clone(); - let completed_state = states - .iter() - .find(|st| st.group_name == "completed") - .unwrap() - .id - .clone(); - drop(conn); - - let move_to = |name: &str, state_id: &str| { - let created: serde_json::Value = - serde_json::from_str(&s.item(Parameters(empty_item_create(name))).unwrap()).unwrap(); - s.item(Parameters(ItemRequest { - action: "update_state".into(), - id: Some(created["id"].as_str().unwrap().to_string()), - state_id: Some(state_id.to_string()), - ..Default::default() - })) - .unwrap(); - }; - move_to("Done 1", &completed_state); - move_to("Done 2", &completed_state); - move_to("WIP", &started_state); - - let health: serde_json::Value = serde_json::from_str( - &s.item(Parameters(ItemRequest { - action: "health".into(), - window_weeks: Some(2), - ..Default::default() - })) - .unwrap(), - ) - .unwrap(); - - let velocity = health["velocity"].as_array().unwrap(); - assert_eq!(velocity.len(), 2, "oldest -> newest, 2 requested windows"); - assert_eq!( - velocity[1]["completed_count"], 2, - "current week has both Done items" - ); - assert_eq!(velocity[0]["completed_count"], 0, "prior week is empty"); - assert_eq!(health["velocity_trend"], "up"); - assert_eq!(health["wip_count"], 1); - assert_eq!(health["stuck_count"], 0); - assert_eq!(health["bottlenecks"].as_array().unwrap().len(), 0); - assert!( - health["bottleneck_note"] - .as_str() - .unwrap() - .contains("no handoff history") - ); -} - /// Regression (CodeRabbit): an absurd `window_weeks` must be clamped, /// not used to size a `Vec` directly — otherwise a caller /// passing e.g. `i64::MAX` drives a near-infinite allocation while the diff --git a/src/mcp_server/tests/item_tests_reporting.rs b/src/mcp_server/tests/item_tests_reporting.rs new file mode 100644 index 00000000..fdcaff07 --- /dev/null +++ b/src/mcp_server/tests/item_tests_reporting.rs @@ -0,0 +1,782 @@ +use super::*; + +#[test] +fn item_groom_capacity_buckets_now_next_later_and_needs_estimation() { + let (_tmp, s) = harness(); + let sized = |name: &str, size: &str| ItemRequest { + action: "create".into(), + name: Some(name.into()), + metadata: Some(serde_json::json!({"size": size})), + ..Default::default() + }; + let ready_a: serde_json::Value = + serde_json::from_str(&s.item(Parameters(sized("Ready A", "S"))).unwrap()).unwrap(); + let ready_b: serde_json::Value = + serde_json::from_str(&s.item(Parameters(sized("Ready B", "S"))).unwrap()).unwrap(); + let dep: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Dep"))).unwrap()).unwrap(); + let blocked: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + dependency_ids: Some(vec![dep["id"].as_str().unwrap().to_string()]), + ..sized("Blocked", "M") + })) + .unwrap(), + ) + .unwrap(); + let unestimated: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Unsized"))).unwrap()).unwrap(); + + // No capacity: buckets omitted entirely (backward compatible). + let unbucketed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert!(unbucketed.get("now").is_none()); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + capacity: Some(1), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let ids = |key: &str| -> Vec { + groomed[key] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() + }; + let now = ids("now"); + let next = ids("next"); + assert_eq!(now.len(), 1, "capacity=1 caps now to 1 ready item"); + assert!( + now.contains(&ready_a["id"].as_str().unwrap().to_string()) + || now.contains(&ready_b["id"].as_str().unwrap().to_string()) + ); + // Whichever ready item didn't make `now` spills into `next`. + assert_eq!(now.len() + next.len(), 2); + assert_eq!(ids("later"), vec![blocked["id"].as_str().unwrap()]); + // "Dep" has no size either — unestimated, same as the dedicated "Unsized" item. + let mut needs_est = ids("needs_estimation"); + needs_est.sort_unstable(); + let mut expected = vec![ + dep["id"].as_str().unwrap().to_string(), + unestimated["id"].as_str().unwrap().to_string(), + ]; + expected.sort_unstable(); + assert_eq!(needs_est, expected); +} + +/// Regression (CodeRabbit): standup's "done" filter and health's +/// velocity bucketing must key off `completed_at`, not `updated_at` — +/// editing an already-completed item (e.g. fixing a typo) bumps +/// `updated_at` without re-completing it, and must not make old work +/// spuriously reappear as "just done" or shift which week it counts in. +#[test] +fn item_standup_and_health_use_completed_at_not_updated_at() { + let (_tmp, s) = harness(); + let created: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Old work"))).unwrap()).unwrap(); + let project_id = created["project_id"].as_str().unwrap().to_string(); + let id = created["id"].as_str().unwrap().to_string(); + let conn = backend_conn(&_tmp); + let completed_state = agentflare_backend::state::list_by_project(&conn, &project_id) + .unwrap() + .into_iter() + .find(|st| st.group_name == "completed") + .unwrap() + .id; + drop(conn); + s.item(Parameters(ItemRequest { + action: "update_state".into(), + id: Some(id.clone()), + state_id: Some(completed_state), + ..Default::default() + })) + .unwrap(); + + // Simulate: completed long ago, then edited just now (updated_at + // recent, completed_at old) — direct SQL, no clock control in tests. + let old_ts = 1_700_000_000_i64; // long before "now" in this fixture era + let conn = backend_conn(&_tmp); + conn.execute( + "UPDATE items SET completed_at = ?1 WHERE id = ?2", + rusqlite::params![old_ts, id], + ) + .unwrap(); + drop(conn); + s.item(Parameters(ItemRequest { + action: "update".into(), + id: Some(id.clone()), + description: Some("fixed a typo".into()), + ..Default::default() + })) + .unwrap(); + + let standup: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "standup".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert!( + !standup["done"] + .as_array() + .unwrap() + .iter() + .any(|i| i["id"] == id), + "editing an old completed item must not resurrect it in 'done'" + ); + + let health: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "health".into(), + window_weeks: Some(1), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!( + health["velocity"][0]["completed_count"], 0, + "an old completion must not count in this week's velocity just because it was edited" + ); +} + +#[test] +fn item_standup_buckets_done_in_progress_grouped_and_stuck() { + let (_tmp, s) = harness(); + let project_id: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("bootstrap"))).unwrap()).unwrap(); + let project_id = project_id["project_id"].as_str().unwrap().to_string(); + let conn = backend_conn(&_tmp); + let states = agentflare_backend::state::list_by_project(&conn, &project_id).unwrap(); + let started_state = states + .iter() + .find(|st| st.group_name == "started") + .unwrap() + .id + .clone(); + let completed_state = states + .iter() + .find(|st| st.group_name == "completed") + .unwrap() + .id + .clone(); + drop(conn); + + let move_to = |name: &str, assignee: Option<&str>, state_id: &str| -> serde_json::Value { + let created: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some(name.into()), + assignee_agent: assignee.map(String::from), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + s.item(Parameters(ItemRequest { + action: "update_state".into(), + id: Some(created["id"].as_str().unwrap().to_string()), + state_id: Some(state_id.to_string()), + ..Default::default() + })) + .unwrap(); + created + }; + + let wip_alice = move_to("WIP Alice", Some("alice"), &started_state); + let _wip_bob = move_to("WIP Bob", Some("bob"), &started_state); + let _wip_unassigned = move_to("WIP Unassigned", None, &started_state); + let done_item = move_to("Done item", Some("alice"), &completed_state); + + let standup: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "standup".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + assert_eq!(standup["done_count"], 1); + assert_eq!(standup["done"][0]["id"], done_item["id"]); + assert_eq!(standup["in_progress_count"], 3); + let groups: Vec<&str> = standup["in_progress"] + .as_array() + .unwrap() + .iter() + .map(|g| g["assignee"].as_str().unwrap()) + .collect(); + assert_eq!(groups, vec!["alice", "bob", "unassigned"]); + let alice_group = standup["in_progress"] + .as_array() + .unwrap() + .iter() + .find(|g| g["assignee"] == "alice") + .unwrap(); + assert_eq!(alice_group["items"][0]["id"], wip_alice["id"]); + // Nothing is 7+ days old in a freshly-created fixture. + assert_eq!(standup["stuck_count"], 0); +} + +#[test] +fn item_health_reports_velocity_wip_and_empty_bottlenecks() { + let (_tmp, s) = harness(); + let project_id: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("bootstrap"))).unwrap()).unwrap(); + let project_id = project_id["project_id"].as_str().unwrap().to_string(); + let conn = backend_conn(&_tmp); + let states = agentflare_backend::state::list_by_project(&conn, &project_id).unwrap(); + let started_state = states + .iter() + .find(|st| st.group_name == "started") + .unwrap() + .id + .clone(); + let completed_state = states + .iter() + .find(|st| st.group_name == "completed") + .unwrap() + .id + .clone(); + drop(conn); + + let move_to = |name: &str, state_id: &str| { + let created: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create(name))).unwrap()).unwrap(); + s.item(Parameters(ItemRequest { + action: "update_state".into(), + id: Some(created["id"].as_str().unwrap().to_string()), + state_id: Some(state_id.to_string()), + ..Default::default() + })) + .unwrap(); + }; + move_to("Done 1", &completed_state); + move_to("Done 2", &completed_state); + move_to("WIP", &started_state); + + let health: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "health".into(), + window_weeks: Some(2), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let velocity = health["velocity"].as_array().unwrap(); + assert_eq!(velocity.len(), 2, "oldest -> newest, 2 requested windows"); + assert_eq!( + velocity[1]["completed_count"], 2, + "current week has both Done items" + ); + assert_eq!(velocity[0]["completed_count"], 0, "prior week is empty"); + assert_eq!(health["velocity_trend"], "up"); + assert_eq!(health["wip_count"], 1); + assert_eq!(health["stuck_count"], 0); + assert_eq!(health["bottlenecks"].as_array().unwrap().len(), 0); + assert!( + health["bottleneck_note"] + .as_str() + .unwrap() + .contains("no item was handed between agents") + ); +} + +#[test] +fn read_only_reporting_actions_accept_a_project_override() { + let (_tmp, s) = harness(); + s.item(Parameters(empty_item_create("bootstrap"))).unwrap(); + + let projects: serde_json::Value = serde_json::from_str( + &s.project(Parameters(ProjectRequest { + action: "list".into(), + })) + .unwrap(), + ) + .unwrap(); + let name = projects[0]["name"].as_str().unwrap().to_string(); + + // Override by name is case-insensitive and hits the same project. + let health: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "health".into(), + project: Some(name.to_uppercase()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(health["wip_count"], 0); + + // Unknown override is a hard error, not a silent fallback to the + // repo-linked project. + let err = s + .item(Parameters(ItemRequest { + action: "standup".into(), + project: Some("no-such-project".into()), + ..Default::default() + })) + .unwrap_err(); + assert_eq!(err.code, rmcp::model::ErrorCode::INVALID_PARAMS); +} + +#[test] +fn item_health_flags_items_handed_between_agents_repeatedly() { + let (_tmp, s) = harness(); + let created: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Hot potato"))).unwrap()) + .unwrap(); + let id = created["id"].as_str().unwrap().to_string(); + let reassign = |agent: &str| { + s.item(Parameters(ItemRequest { + action: "update".into(), + id: Some(id.clone()), + assignee_agent: Some(agent.to_string()), + ..Default::default() + })) + .unwrap(); + }; + reassign("alice"); // first assignment — not a handoff + reassign("bob"); // handoff 1 + reassign("carol"); // handoff 2 + + let health: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "health".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let bottlenecks = health["bottlenecks"].as_array().unwrap(); + assert_eq!(bottlenecks.len(), 1, "got: {bottlenecks:?}"); + let line = bottlenecks[0].as_str().unwrap(); + assert!( + line.contains("Hot potato") + && line.contains("2 handoffs") + && line.contains("alice → bob → carol"), + "got: {line}" + ); + assert!( + health["bottleneck_note"] + .as_str() + .unwrap() + .contains("handed between agents") + ); +} + +/// Regression (CodeRabbit): an absurd `window_weeks` must be clamped, +/// not used to size a `Vec` directly — otherwise a caller +/// passing e.g. `i64::MAX` drives a near-infinite allocation while the +/// backend DB lock is held. +#[test] +fn item_health_clamps_window_weeks_to_a_sane_maximum() { + let (_tmp, s) = harness(); + let health: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "health".into(), + window_weeks: Some(i64::MAX), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(health["window_weeks"], 52); + assert_eq!(health["velocity"].as_array().unwrap().len(), 52); +} + +/// Regression (CodeRabbit): an absurd groom `limit` must be clamped — +/// bounds the O(n^2) duplicate-detection pass and the SQLite `IN (...)` +/// parameter list built from the shortlist. +#[test] +fn item_groom_clamps_limit_to_a_sane_maximum() { + let (_tmp, s) = harness(); + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + limit: Some(i64::MAX), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert!(groomed["items"].as_array().unwrap().len() <= 200); +} + +/// Real measured comparison, not an estimate: one `groom` call vs. the +/// `list` + N×`get` path it replaces, against a backlog-sized dataset (60 +/// items — close to this project's real ~40-item backlog) with dependency +/// edges so `groom`'s blocked/fan-in computation does real work too. Not a +/// hard perf gate (`#[ignore]`, run explicitly) — timing assertions in CI +/// are flaky; this is for a human to re-run and read the numbers. +#[test] +#[ignore = "manual benchmark — run with: cargo test item_groom_benchmark -- --ignored --nocapture"] +fn item_groom_benchmark() { + let (_tmp, s) = harness(); + let mut ids: Vec = Vec::with_capacity(60); + for n in 0..60 { + let priority = ["urgent", "high", "medium", "low", "none"][n % 5]; + let created: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some(format!("Benchmark item {n}")), + description: Some( + "Lorem ipsum dolor sit amet, consectetur adipiscing elit. ".repeat(20), + ), + priority: Some(priority.into()), + dependency_ids: if n > 0 && n % 7 == 0 { + Some(vec![ids[n - 1].clone()]) + } else { + None + }, + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + ids.push(created["id"].as_str().unwrap().to_string()); + } + + let groom_start = std::time::Instant::now(); + let groomed = s + .item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(); + let groom_elapsed = groom_start.elapsed(); + + let old_start = std::time::Instant::now(); + let listed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + state_group: Some("backlog,unstarted".into()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let shortlist_ids: Vec = listed["items"] + .as_array() + .unwrap() + .iter() + .take(15) + .map(|i| i["id"].as_str().unwrap().to_string()) + .collect(); + for id in &shortlist_ids { + s.item(Parameters(ItemRequest { + action: "get".into(), + id: Some(id.clone()), + ..Default::default() + })) + .unwrap(); + } + let old_elapsed = old_start.elapsed(); + + println!( + "groom (1 call): {groom_elapsed:?} | list+{}xget (old path): {old_elapsed:?} | speedup: {:.1}x", + shortlist_ids.len(), + old_elapsed.as_secs_f64() / groom_elapsed.as_secs_f64().max(1e-9) + ); + assert!(groomed.contains("pull_next")); +} + +#[test] +fn item_list_respects_limit_and_offset() { + let (_tmp, s) = harness(); + for name in ["A", "B", "C"] { + s.item(Parameters(empty_item_create(name))).unwrap(); + } + let listed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + limit: Some(1), + offset: Some(1), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let names: Vec<&str> = listed["items"] + .as_array() + .unwrap() + .iter() + .map(|i| i["name"].as_str().unwrap()) + .collect(); + assert_eq!(names, vec!["B"]); + assert_eq!(listed["total"], 3); + assert_eq!(listed["offset"], 1); + assert_eq!(listed["limit"], 1); + assert_eq!(listed["next_offset"], 2); + assert_eq!(listed["prev_offset"], 0); +} + +#[test] +fn item_list_pagination_edges_out_of_range_offset_and_zero_limit() { + let (_tmp, s) = harness(); + for name in ["A", "B", "C"] { + s.item(Parameters(empty_item_create(name))).unwrap(); + } + + let past_end: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + limit: Some(1), + offset: Some(100), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(past_end["items"].as_array().unwrap().len(), 0); + assert_eq!(past_end["next_offset"], serde_json::Value::Null); + assert_eq!(past_end["prev_offset"], 2); + + let zero_limit: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + limit: Some(0), + offset: Some(1), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(zero_limit["items"].as_array().unwrap().len(), 0); + assert_eq!(zero_limit["next_offset"], serde_json::Value::Null); + assert_eq!(zero_limit["prev_offset"], serde_json::Value::Null); +} + +#[test] +fn item_list_returns_lean_projection_with_readable_state() { + let (_tmp, s) = harness(); + s.item(Parameters(empty_item_create("Test"))).unwrap(); + let listed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "list".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let first = &listed["items"].as_array().unwrap()[0]; + assert_eq!(first["state"], "Backlog"); + assert_eq!(first["state_group"], "backlog"); + assert!(first.get("description").is_none()); + assert!(first.get("metadata").is_none()); + assert_eq!(listed["next_offset"], serde_json::Value::Null); + assert_eq!(listed["prev_offset"], serde_json::Value::Null); +} + +#[test] +fn resolve_workspace_id_creates_once_and_reuses() { + let (tmp, _s) = harness(); + let conn = backend_conn(&tmp); + let id1 = AgentflareMcp::resolve_workspace_id(&conn).unwrap(); + let id2 = AgentflareMcp::resolve_workspace_id(&conn).unwrap(); + assert_eq!(id1, id2); +} + +/// If `.agentflare/project.json` is deleted (wiped worktree, `rm -rf`, +/// etc.) while the project it pointed to still exists, resolving again +/// must reconnect to that same project — not silently fork a duplicate, +/// which would strand the original project's items. +#[test] +fn resolve_project_relinks_to_existing_project_when_link_file_is_deleted() { + let (tmp, s) = harness(); + let conn = backend_conn(&tmp); + let first = s.resolve_project(&conn).unwrap(); + + std::fs::remove_file(s.project_link_path()).unwrap(); + + let second = s.resolve_project(&conn).unwrap(); + assert_eq!( + first.id, second.id, + "must reconnect to the same project, not fork a duplicate" + ); + let all = agentflare_backend::project::list_by_workspace(&conn, &first.workspace_id).unwrap(); + assert_eq!( + all.len(), + 1, + "no duplicate project should have been created: {all:?}" + ); +} + +/// Two different repos can easily share a directory basename (or, for +/// non-git dirs, no distinguishing info at all beyond the name). They +/// must never be conflated into one project just because they'd derive +/// the same display identifier — each gets its own project, with the +/// second disambiguated by a suffix. +#[test] +fn resolve_project_does_not_conflate_different_repos_with_the_same_derived_name() { + let tmp = tempfile::tempdir().unwrap(); + let db_path = tmp.path().join("backend.db"); + let s1 = AgentflareMcp { + backend_db_override: Some(db_path.clone()), + backend_project_link_override: Some(tmp.path().join("link1.json")), + backend_repo_key_override: Some("path:/repo/one".to_string()), + ..Default::default() + }; + let s2 = AgentflareMcp { + backend_db_override: Some(db_path.clone()), + backend_project_link_override: Some(tmp.path().join("link2.json")), + backend_repo_key_override: Some("path:/repo/two".to_string()), + ..Default::default() + }; + let conn = agentflare_backend::db::open_db(&db_path).unwrap(); + let p1 = s1.resolve_project(&conn).unwrap(); + let p2 = s2.resolve_project(&conn).unwrap(); + assert_ne!( + p1.id, p2.id, + "different repos must never share a project even with the same derived name" + ); + assert_ne!( + p1.identifier, p2.identifier, + "the second project must get a disambiguating suffix" + ); + + // Each keeps resolving to its own project on repeat calls. + assert_eq!(s1.resolve_project(&conn).unwrap().id, p1.id); + assert_eq!(s2.resolve_project(&conn).unwrap().id, p2.id); +} + +/// Non-git projects need the same "root is stable no matter which +/// subdirectory you're in" guarantee git repos get for free from `git +/// rev-parse --show-toplevel` — otherwise the same project would split +/// across multiple `.agentflare/project.json` files depending on which +/// subdirectory a tool happened to be called from. +#[test] +fn find_root_from_walks_up_to_the_nearest_marker() { + // Bounding "home" at the tempdir's own parent contains the walk + // entirely within this test's constructed tree — passing some + // unrelated path here would NOT do that: the walk follows the real + // filesystem's `.parent()` chain regardless, so it would keep + // climbing past `root` into real ancestor directories (which may + // have their own real markers, e.g. this machine's actual + // `~/.agentflare`) until it happened to reach that unrelated path, + // which — not being a real ancestor — it never would, walking all + // the way to the filesystem root instead. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let home = root.parent().unwrap(); + std::fs::write(root.join("package.json"), "{}").unwrap(); + let deep = root.join("src").join("nested").join("deep"); + std::fs::create_dir_all(&deep).unwrap(); + + assert_eq!(AgentflareMcp::find_root_from(&deep, home), root); + assert_eq!(AgentflareMcp::find_root_from(root, home), root); +} + +#[test] +fn find_root_from_prefers_an_existing_agentflare_link_over_other_markers() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + let home = root.parent().unwrap(); + // A nested directory with its own marker (e.g. a sub-package) must + // not shadow an ancestor's existing project link — the + // .agentflare pass runs before the ROOT_MARKERS pass for + // exactly this reason. + std::fs::create_dir_all(root.join(".agentflare")).unwrap(); + let sub = root.join("packages").join("sub"); + std::fs::create_dir_all(&sub).unwrap(); + std::fs::write(sub.join("package.json"), "{}").unwrap(); + + assert_eq!(AgentflareMcp::find_root_from(&sub, home), root); + assert_eq!(AgentflareMcp::find_root_from(root, home), root); +} + +/// The boundary itself: a directory that IS `home` must never be +/// treated as a project root, even if it happens to contain a marker — +/// this is what keeps the global `~/.agentflare` data dir from ever +/// being mistaken for a per-repo link. +#[test] +fn find_root_from_never_resolves_to_home_itself() { + let home = tempfile::tempdir().unwrap(); + // Stands in for the real global data dir at ~/.agentflare. + std::fs::create_dir_all(home.path().join(".agentflare")).unwrap(); + let start = home.path().join("some_project"); + std::fs::create_dir_all(&start).unwrap(); + + // `start` itself has no marker, and home — one level up — does. If + // the walk checked markers at `home`, this would return `home`. It + // must instead stop short of ever inspecting `home` and fall back + // to `start`. + assert_eq!(AgentflareMcp::find_root_from(&start, home.path()), start); +} + +// No test for the "nothing found anywhere above" fallback: `find_root_from` +// walks all the way to the filesystem root, so a tempdir-based test would +// depend on what markers happen to exist above the OS temp directory on +// whatever machine runs this — not a property this test can control. The +// fallback itself is a single trivial `None => return start`. +#[test] +fn item_get_resolves_bare_and_hash_prefixed_sequence_id() { + let (_tmp, s) = harness(); + let created: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Test"))).unwrap()).unwrap(); + let uuid = created["id"].as_str().unwrap().to_string(); + let seq = created["sequence_id"].as_i64().unwrap(); + + let by_bare_seq: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "get".into(), + id: Some(seq.to_string()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(by_bare_seq["id"], uuid); + + let by_hash_seq: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "get".into(), + id: Some(format!("#{seq}")), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(by_hash_seq["id"], uuid); + + let by_uuid: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "get".into(), + id: Some(uuid.clone()), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(by_uuid["id"], uuid); +} + +#[test] +fn item_get_unknown_sequence_id_returns_not_found() { + let (_tmp, s) = harness(); + let err = s + .item(Parameters(ItemRequest { + action: "get".into(), + id: Some("999999".into()), + ..Default::default() + })) + .unwrap_err(); + assert_eq!(err.code, rmcp::model::ErrorCode::INVALID_PARAMS); +} diff --git a/src/mcp_server/tests/mod.rs b/src/mcp_server/tests/mod.rs index 4a62d078..7f360b16 100644 --- a/src/mcp_server/tests/mod.rs +++ b/src/mcp_server/tests/mod.rs @@ -352,6 +352,7 @@ mod artifact_tests; mod asset_tests; mod item_commit_failure_tests; mod item_tests; +mod item_tests_reporting; mod project_resolution_tests; mod search_tests; mod state_tests; diff --git a/src/mcp_server/types.rs b/src/mcp_server/types.rs index f21deabc..486bb07a 100644 --- a/src/mcp_server/types.rs +++ b/src/mcp_server/types.rs @@ -792,6 +792,11 @@ pub(crate) struct ItemRequest { )] #[serde(default)] pub(crate) summary: Option, + #[schemars( + description = "Project override — a project name (case-insensitive) or UUID in the linked workspace. Honored ONLY by the read-only reporting actions groom|standup|health, for portfolio roll-ups; every other action always uses the repo's linked project. Use `project action=list` to enumerate valid targets." + )] + #[serde(default)] + pub(crate) project: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full @@ -928,8 +933,9 @@ pub(crate) struct HealthResponse { pub(crate) stuck_days: i64, pub(crate) stuck_count: usize, pub(crate) stuck: Vec, - /// Empty today — agentflare has no persisted handoff log distinct from - /// item state, so this can't be computed yet (see `bottleneck_note`). + /// Items handed between different agents ≥2× in the window, from the + /// persisted assignment log (see `bottleneck_note` for the history-start + /// caveat). pub(crate) bottlenecks: Vec, pub(crate) bottleneck_note: String, } @@ -996,7 +1002,7 @@ pub(crate) struct WebhookRequest { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] pub(crate) struct ProjectRequest { - #[schemars(description = "Action: info")] + #[schemars(description = "Action: info|list")] pub(crate) action: String, }