From 82554e16c71658280424ad3ec4bd1ad9d13d5e8f Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Wed, 12 Aug 2026 18:50:28 +0530 Subject: [PATCH 1/4] feat(pm): /pm:* slash-command shims over the pm skill workflows Agentflare-Agent: claude-code_2-1-228_harness Agentflare-Branch: feat/pm-fast-follows --- .claude/commands/pm/groom.md | 9 +++++++++ .claude/commands/pm/health.md | 8 ++++++++ .claude/commands/pm/plan.md | 10 ++++++++++ .claude/commands/pm/standup.md | 8 ++++++++ 4 files changed, 35 insertions(+) create mode 100644 .claude/commands/pm/groom.md create mode 100644 .claude/commands/pm/health.md create mode 100644 .claude/commands/pm/plan.md create mode 100644 .claude/commands/pm/standup.md diff --git a/.claude/commands/pm/groom.md b/.claude/commands/pm/groom.md new file mode 100644 index 00000000..f927af3f --- /dev/null +++ b/.claude/commands/pm/groom.md @@ -0,0 +1,9 @@ +--- +description: Backlog grooming — ranked shortlist + stale/unassigned/blocked/duplicate flags (read-only) +argument-hint: [staleness-days] [rice|wsjf|value-effort] +--- + +Load the `pm` skill (Skill tool, name "pm") and run its `/pm:groom` workflow +exactly as written there. Arguments: "$ARGUMENTS" — optional staleness threshold +in days (default 14) and optional scoring framework (`rice` default, `wsjf`, +`value-effort`). Read-only — never mutate items. diff --git a/.claude/commands/pm/health.md b/.claude/commands/pm/health.md new file mode 100644 index 00000000..df0ea8ff --- /dev/null +++ b/.claude/commands/pm/health.md @@ -0,0 +1,8 @@ +--- +description: Team health scorecard — velocity, WIP, stuck items, bottlenecks (read-only) +argument-hint: [window-weeks] +--- + +Load the `pm` skill (Skill tool, name "pm") and run its `/pm:health` workflow +exactly as written there. Window argument: "$ARGUMENTS" (weeks; empty means the +default 4). Read-only — never mutate items. diff --git a/.claude/commands/pm/plan.md b/.claude/commands/pm/plan.md new file mode 100644 index 00000000..559dcaae --- /dev/null +++ b/.claude/commands/pm/plan.md @@ -0,0 +1,10 @@ +--- +description: Sprint bucketing — Now / Next / Later + needs-estimation over the backlog (read-only) +argument-hint: [~capacity] [rice|wsjf|value-effort] +--- + +Load the `pm` skill (Skill tool, name "pm") and run its `/pm:plan` workflow +exactly as written there. Arguments: "$ARGUMENTS" — optional capacity hint like +"~8" (caps the Now bucket) and optional scoring framework (`rice` default, +`wsjf`, `value-effort`). Read-only — this proposes a plan, it never assigns or +moves items. diff --git a/.claude/commands/pm/standup.md b/.claude/commands/pm/standup.md new file mode 100644 index 00000000..78987851 --- /dev/null +++ b/.claude/commands/pm/standup.md @@ -0,0 +1,8 @@ +--- +description: Daily activity digest — Done / In progress / Stuck over the current project's items (read-only) +argument-hint: [cutoff-hours] +--- + +Load the `pm` skill (Skill tool, name "pm") and run its `/pm:standup` workflow +exactly as written there. Cutoff argument: "$ARGUMENTS" (hours; empty means the +default 24). Read-only — never mutate items. From ea018f4e48b3dd969ec30fc4c6547fb68c447c06 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Wed, 12 Aug 2026 18:52:16 +0530 Subject: [PATCH 2/4] feat(pm): consolidate into a single /pm command with subcommand dispatch Agentflare-Agent: claude-code_2-1-228_harness Agentflare-Branch: feat/pm-fast-follows --- .claude/commands/pm.md | 29 +++++++++++++++++++++++++++++ .claude/commands/pm/groom.md | 9 --------- .claude/commands/pm/health.md | 8 -------- .claude/commands/pm/plan.md | 10 ---------- .claude/commands/pm/standup.md | 8 -------- 5 files changed, 29 insertions(+), 35 deletions(-) create mode 100644 .claude/commands/pm.md delete mode 100644 .claude/commands/pm/groom.md delete mode 100644 .claude/commands/pm/health.md delete mode 100644 .claude/commands/pm/plan.md delete mode 100644 .claude/commands/pm/standup.md diff --git a/.claude/commands/pm.md b/.claude/commands/pm.md new file mode 100644 index 00000000..4ae65365 --- /dev/null +++ b/.claude/commands/pm.md @@ -0,0 +1,29 @@ +--- +description: Product management over agentflare items — standup, groom, plan, health, portfolio (read-only), or PM dispatch mode +argument-hint: [args] +--- + +Parse "$ARGUMENTS": the first word is the subcommand; everything after it is +that subcommand's arguments. Dispatch: + +- **standup** `[cutoff-hours]` — load the `pm` skill (Skill tool, name "pm") + and run its `/pm:standup` workflow. Cutoff in hours, default 24. +- **groom** `[staleness-days] [rice|wsjf|value-effort]` — load the `pm` skill + and run its `/pm:groom` workflow. Staleness default 14 days; framework + default `rice`. +- **plan** `[~capacity] [rice|wsjf|value-effort]` — load the `pm` skill and + run its `/pm:plan` workflow. Capacity hint like `~8` caps the Now bucket. +- **health** `[window-weeks]` — load the `pm` skill and run its `/pm:health` + workflow. Window in weeks, default 4. +- **portfolio** `[standup|health] [per-subcommand args]` — load the `pm` + skill and run its `/pm:portfolio` workflow: the chosen report (default + `health`) rolled up across every project in the workspace. +- **mode on** — load the `pm-mode` skill and act as project manager (create + and dispatch work instead of implementing it yourself) until turned off. +- **mode off** — leave PM mode: stop following the `pm-mode` skill and return + to normal implementation behavior. Confirm in one line. + +The `pm` skill's workflows are read-only over items — never mutate item state +from any subcommand except what `pm-mode` itself explicitly directs while mode +is on. No subcommand or empty arguments → print a one-line usage summary of +the subcommands above and stop. diff --git a/.claude/commands/pm/groom.md b/.claude/commands/pm/groom.md deleted file mode 100644 index f927af3f..00000000 --- a/.claude/commands/pm/groom.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -description: Backlog grooming — ranked shortlist + stale/unassigned/blocked/duplicate flags (read-only) -argument-hint: [staleness-days] [rice|wsjf|value-effort] ---- - -Load the `pm` skill (Skill tool, name "pm") and run its `/pm:groom` workflow -exactly as written there. Arguments: "$ARGUMENTS" — optional staleness threshold -in days (default 14) and optional scoring framework (`rice` default, `wsjf`, -`value-effort`). Read-only — never mutate items. diff --git a/.claude/commands/pm/health.md b/.claude/commands/pm/health.md deleted file mode 100644 index df0ea8ff..00000000 --- a/.claude/commands/pm/health.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -description: Team health scorecard — velocity, WIP, stuck items, bottlenecks (read-only) -argument-hint: [window-weeks] ---- - -Load the `pm` skill (Skill tool, name "pm") and run its `/pm:health` workflow -exactly as written there. Window argument: "$ARGUMENTS" (weeks; empty means the -default 4). Read-only — never mutate items. diff --git a/.claude/commands/pm/plan.md b/.claude/commands/pm/plan.md deleted file mode 100644 index 559dcaae..00000000 --- a/.claude/commands/pm/plan.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -description: Sprint bucketing — Now / Next / Later + needs-estimation over the backlog (read-only) -argument-hint: [~capacity] [rice|wsjf|value-effort] ---- - -Load the `pm` skill (Skill tool, name "pm") and run its `/pm:plan` workflow -exactly as written there. Arguments: "$ARGUMENTS" — optional capacity hint like -"~8" (caps the Now bucket) and optional scoring framework (`rice` default, -`wsjf`, `value-effort`). Read-only — this proposes a plan, it never assigns or -moves items. diff --git a/.claude/commands/pm/standup.md b/.claude/commands/pm/standup.md deleted file mode 100644 index 78987851..00000000 --- a/.claude/commands/pm/standup.md +++ /dev/null @@ -1,8 +0,0 @@ ---- -description: Daily activity digest — Done / In progress / Stuck over the current project's items (read-only) -argument-hint: [cutoff-hours] ---- - -Load the `pm` skill (Skill tool, name "pm") and run its `/pm:standup` workflow -exactly as written there. Cutoff argument: "$ARGUMENTS" (hours; empty means the -default 24). Read-only — never mutate items. From 87d0d20ee5a9471ff56b9c417a6321d6ea450ab8 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Wed, 12 Aug 2026 20:16:22 +0530 Subject: [PATCH 3/4] feat(pm): portfolio roll-up and persisted handoff-history bottlenecks Project override (name or UUID) on the read-only item groom/standup/health actions, plus project(action=list), backs a new /pm:portfolio workflow that rolls reports up across every project in the workspace. Adds an item_assignment_events log (migration 0011), written by item::update whenever assignee_agent changes (covers claim too, since claim assigns through update). item_health now computes real bottlenecks (items handed between agents >=2x in the window) instead of the always-empty placeholder, with a caveat that history starts at this migration. Splits crates/agentflare-backend/src/item.rs (over the LOC gate's frozen limit) into item/mod.rs + item/tests.rs, and src/mcp_server/tests/item_tests.rs (over the plain limit) into item_tests.rs + item_tests_reporting.rs. Also fixes a pre-existing Windows-only clippy warning: an unused `use super::*` in a unix-only test file. Agentflare-Agent: claude-code_2-1-228_harness Agentflare-Branch: feat/pm-fast-follows --- .claude/commands/pm.md | 62 +- .claude/skills/pm/SKILL.md | 34 +- .../src/assignment_events.rs | 232 ++ crates/agentflare-backend/src/db.rs | 1 + crates/agentflare-backend/src/item.rs | 2139 ----------------- crates/agentflare-backend/src/item/mod.rs | 821 +++++++ crates/agentflare-backend/src/item/tests.rs | 1329 ++++++++++ crates/agentflare-backend/src/lib.rs | 1 + .../0011_item_assignment_events.sql | 15 + scripts/loc-gate.sh | 1 - src/mcp_server.rs | 46 +- src/mcp_server/item.rs | 66 +- .../tests/item_commit_failure_tests.rs | 1 + src/mcp_server/tests/item_tests.rs | 697 ------ src/mcp_server/tests/item_tests_reporting.rs | 782 ++++++ src/mcp_server/tests/mod.rs | 1 + src/mcp_server/types.rs | 12 +- 17 files changed, 3349 insertions(+), 2891 deletions(-) create mode 100644 crates/agentflare-backend/src/assignment_events.rs delete mode 100644 crates/agentflare-backend/src/item.rs create mode 100644 crates/agentflare-backend/src/item/mod.rs create mode 100644 crates/agentflare-backend/src/item/tests.rs create mode 100644 crates/agentflare-backend/src/migrations/0011_item_assignment_events.sql create mode 100644 src/mcp_server/tests/item_tests_reporting.rs diff --git a/.claude/commands/pm.md b/.claude/commands/pm.md index 4ae65365..ed0592f4 100644 --- a/.claude/commands/pm.md +++ b/.claude/commands/pm.md @@ -1,29 +1,43 @@ --- -description: Product management over agentflare items — standup, groom, plan, health, portfolio (read-only), or PM dispatch mode -argument-hint: [args] +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 is the subcommand; everything after it is -that subcommand's arguments. Dispatch: +Parse "$ARGUMENTS": the first word (if any) is the subcommand; everything after +it is that subcommand's arguments. -- **standup** `[cutoff-hours]` — load the `pm` skill (Skill tool, name "pm") - and run its `/pm:standup` workflow. Cutoff in hours, default 24. -- **groom** `[staleness-days] [rice|wsjf|value-effort]` — load the `pm` skill - and run its `/pm:groom` workflow. Staleness default 14 days; framework - default `rice`. -- **plan** `[~capacity] [rice|wsjf|value-effort]` — load the `pm` skill and - run its `/pm:plan` workflow. Capacity hint like `~8` caps the Now bucket. -- **health** `[window-weeks]` — load the `pm` skill and run its `/pm:health` - workflow. Window in weeks, default 4. -- **portfolio** `[standup|health] [per-subcommand args]` — load the `pm` - skill and run its `/pm:portfolio` workflow: the chosen report (default - `health`) rolled up across every project in the workspace. -- **mode on** — load the `pm-mode` skill and act as project manager (create - and dispatch work instead of implementing it yourself) until turned off. -- **mode off** — leave PM mode: stop following the `pm-mode` skill and return - to normal implementation behavior. Confirm in one line. +## Bare `/pm` (no arguments) — start the PM day -The `pm` skill's workflows are read-only over items — never mutate item state -from any subcommand except what `pm-mode` itself explicitly directs while mode -is on. No subcommand or empty arguments → print a one-line usage summary of -the subcommands above and stop. +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/db.rs b/crates/agentflare-backend/src/db.rs index e2fbf8ae..98cdf167 100644 --- a/crates/agentflare-backend/src/db.rs +++ b/crates/agentflare-backend/src/db.rs @@ -19,6 +19,7 @@ const MIGRATION_LIST: &[M<'static>] = &[ M::up(include_str!("migrations/0008_bridge_repos.sql")), M::up(include_str!("migrations/0009_vent_escalation.sql")), M::up(include_str!("migrations/0010_project_dirs.sql")), + M::up(include_str!("migrations/0011_item_assignment_events.sql")), ]; const MIGRATIONS: Migrations = Migrations::from_slice(MIGRATION_LIST); diff --git a/crates/agentflare-backend/src/item.rs b/crates/agentflare-backend/src/item.rs deleted file mode 100644 index dc783d28..00000000 --- a/crates/agentflare-backend/src/item.rs +++ /dev/null @@ -1,2139 +0,0 @@ -use rusqlite::Connection; -use serde::{Deserialize, Serialize}; - -use crate::error::Result; -use crate::events; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Item { - pub id: String, - pub project_id: String, - pub state_id: String, - pub name: String, - pub description: String, - pub priority: String, - pub parent_id: Option, - pub assignee_agent: Option, - pub sequence_id: i64, - pub sort_order: f64, - pub started_at: Option, - pub completed_at: Option, - pub archived_at: Option, - pub external_source: Option, - pub external_id: Option, - pub metadata: String, - pub created_at: i64, - pub updated_at: i64, - pub deleted_at: Option, -} - -#[derive(Debug, Deserialize)] -pub struct CreateItem { - pub project_id: String, - pub state_id: String, - pub name: String, - pub description: Option, - pub priority: Option, - pub parent_id: Option, - pub assignee_agent: Option, - pub sort_order: Option, - pub external_source: Option, - pub external_id: Option, - pub metadata: Option, - pub label_ids: Vec, - pub assignee_ids: Vec, - pub dependency_ids: Vec, -} - -#[derive(Debug, Deserialize, Default)] -pub struct UpdateItem { - pub name: Option, - pub description: Option, - pub priority: Option, - pub state_id: Option, - pub assignee_agent: Option, - pub sort_order: Option, - pub metadata: Option, -} - -fn now() -> i64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0) -} - -fn row_to_item(row: &rusqlite::Row) -> rusqlite::Result { - Ok(Item { - id: row.get(0)?, - project_id: row.get(1)?, - state_id: row.get(2)?, - name: row.get(3)?, - description: row.get(4)?, - priority: row.get(5)?, - parent_id: row.get(6)?, - assignee_agent: row.get(7)?, - sequence_id: row.get(8)?, - sort_order: row.get(9)?, - started_at: row.get(10)?, - completed_at: row.get(11)?, - archived_at: row.get(12)?, - external_source: row.get(13)?, - external_id: row.get(14)?, - metadata: row.get(15)?, - created_at: row.get(16)?, - updated_at: row.get(17)?, - deleted_at: row.get(18)?, - }) -} - -fn next_sequence_id(conn: &Connection, project_id: &str) -> rusqlite::Result { - conn.execute( - "INSERT INTO project_sequences (project_id, next_seq) VALUES (?1, 1) - ON CONFLICT(project_id) DO UPDATE SET next_seq = next_seq + 1", - rusqlite::params![project_id], - )?; - conn.query_row( - "SELECT next_seq FROM project_sequences WHERE project_id = ?1", - rusqlite::params![project_id], - |row| row.get(0), - ) -} - -fn workspace_id_for_project(conn: &Connection, project_id: &str) -> Result { - conn.query_row( - "SELECT workspace_id FROM projects WHERE id = ?1 AND deleted_at IS NULL", - rusqlite::params![project_id], - |row| row.get(0), - ) - .map_err(|e| match e { - rusqlite::Error::QueryReturnedNoRows => { - crate::error::Error::NotFound(project_id.to_string()) - } - other => other.into(), - }) -} - -pub fn create(conn: &Connection, input: CreateItem) -> Result { - let id = db_kit::ids::new_id(); - let ts = now(); - let sort_order = input.sort_order.unwrap_or(65535.0); - let description = input.description.unwrap_or_default(); - let priority = input.priority.unwrap_or_else(|| "none".to_string()); - let metadata = input.metadata.unwrap_or_else(|| "{}".to_string()); - let assignee_agent = input - .assignee_agent - .as_deref() - .map(agent_registry::canonicalize); - - let state = crate::state::get(conn, &input.state_id)?; - if state.project_id != input.project_id { - return Err(crate::error::Error::InvalidTransition(format!( - "state {} belongs to a different project than project {}", - input.state_id, input.project_id - ))); - } - - let tx = conn.unchecked_transaction()?; - let seq = next_sequence_id(&tx, &input.project_id)?; - tx.execute( - "INSERT INTO items (id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, external_source, external_id, metadata, created_at, updated_at) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)", - rusqlite::params![ - id, - input.project_id, - input.state_id, - input.name, - description, - priority, - input.parent_id, - assignee_agent, - seq, - sort_order, - input.external_source, - input.external_id, - metadata, - ts, - ts, - ], - )?; - for label_id in &input.label_ids { - add_label(&tx, &id, label_id)?; - } - for agent_id in &input.assignee_ids { - add_assignee(&tx, &id, agent_id)?; - } - for dep_id in &input.dependency_ids { - add_dependency(&tx, &id, dep_id)?; - } - tx.commit()?; - let item = get(conn, &id)?; - if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { - events::emit( - conn, - &wid, - "item", - "create", - serde_json::to_value(&item).unwrap_or_default(), - ); - } - Ok(item) -} - -pub fn get(conn: &Connection, id: &str) -> Result { - conn.query_row( - "SELECT id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, started_at, completed_at, archived_at, external_source, external_id, metadata, created_at, updated_at, deleted_at - FROM items WHERE id = ?1 AND deleted_at IS NULL", - rusqlite::params![id], - row_to_item, - ) - .map_err(|e| match e { - rusqlite::Error::QueryReturnedNoRows => crate::error::Error::NotFound(id.to_string()), - other => other.into(), - }) -} - -/// Resolve a user-supplied identifier to an item UUID. -/// Accepts a UUID (pass-through) or a numeric `sequence_id`. -/// When `project_id` is `Some`, scopes the sequence_id lookup to that project; -/// when `None`, searches across all projects (returns the first match). -pub fn resolve_id(conn: &Connection, project_id: Option<&str>, id_or_seq: &str) -> Result { - let numeric_part = id_or_seq.strip_prefix('#').unwrap_or(id_or_seq); - if let Ok(seq) = numeric_part.parse::() { - let sql = match project_id { - Some(_) => { - "SELECT id FROM items WHERE project_id = ?1 AND sequence_id = ?2 AND deleted_at IS NULL" - } - None => "SELECT id FROM items WHERE sequence_id = ?1 AND deleted_at IS NULL LIMIT 1", - }; - let params: Vec> = match project_id { - Some(pid) => vec![Box::new(pid.to_string()), Box::new(seq)], - None => vec![Box::new(seq)], - }; - let params_ref: Vec<&dyn rusqlite::types::ToSql> = - params.iter().map(|p| p.as_ref()).collect(); - conn.query_row(sql, params_ref.as_slice(), |row| row.get(0)) - .map_err(|e| match e { - rusqlite::Error::QueryReturnedNoRows => { - crate::error::Error::NotFound(format!("sequence_id #{seq}")) - } - other => other.into(), - }) - } else { - Ok(id_or_seq.to_string()) - } -} - -pub fn list_by_project(conn: &Connection, project_id: &str) -> Result> { - let mut stmt = conn.prepare( - "SELECT id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, started_at, completed_at, archived_at, external_source, external_id, metadata, created_at, updated_at, deleted_at - FROM items WHERE project_id = ?1 AND deleted_at IS NULL ORDER BY sort_order", - )?; - let rows = stmt.query_map(rusqlite::params![project_id], row_to_item)?; - Ok(rows.collect::>()?) -} - -pub fn list_by_label(conn: &Connection, project_id: &str, label_id: &str) -> Result> { - let mut stmt = conn.prepare( - "SELECT items.id, items.project_id, items.state_id, items.name, items.description, items.priority, items.parent_id, items.assignee_agent, items.sequence_id, items.sort_order, items.started_at, items.completed_at, items.archived_at, items.external_source, items.external_id, items.metadata, items.created_at, items.updated_at, items.deleted_at - FROM items - INNER JOIN item_labels ON item_labels.item_id = items.id - WHERE item_labels.label_id = ?1 AND items.project_id = ?2 AND items.deleted_at IS NULL - ORDER BY items.sort_order", - )?; - let rows = stmt.query_map(rusqlite::params![label_id, project_id], row_to_item)?; - Ok(rows.collect::>()?) -} - -/// List non-deleted items assigned to an agent (excludes completed/cancelled). -pub fn list_by_assignee_agent( - conn: &Connection, - project_id: &str, - agent: &str, -) -> Result> { - let mut stmt = conn.prepare( - "SELECT i.id, i.project_id, i.state_id, i.name, i.description, - i.priority, i.parent_id, i.assignee_agent, i.sequence_id, - i.sort_order, i.started_at, i.completed_at, i.archived_at, - i.external_source, i.external_id, i.metadata, - i.created_at, i.updated_at, i.deleted_at - FROM items i - JOIN states s ON s.id = i.state_id - WHERE i.project_id = ?1 - AND i.assignee_agent = ?2 - AND i.deleted_at IS NULL - AND s.group_name NOT IN ('completed', 'cancelled') - ORDER BY i.sort_order", - )?; - let rows = stmt.query_map(rusqlite::params![project_id, agent], row_to_item)?; - Ok(rows.collect::>()?) -} - -pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { - let ts = now(); - let assignee_agent = input - .assignee_agent - .as_deref() - .map(agent_registry::canonicalize); - let mut sets = vec!["updated_at = ?2".to_string()]; - let mut param_idx = 3; - if input.name.is_some() { - sets.push(format!("name = ?{param_idx}")); - param_idx += 1; - } - if input.description.is_some() { - sets.push(format!("description = ?{param_idx}")); - param_idx += 1; - } - if input.priority.is_some() { - sets.push(format!("priority = ?{param_idx}")); - param_idx += 1; - } - if input.state_id.is_some() { - sets.push(format!("state_id = ?{param_idx}")); - param_idx += 1; - } - if assignee_agent.is_some() { - sets.push(format!("assignee_agent = ?{param_idx}")); - param_idx += 1; - } - if input.sort_order.is_some() { - sets.push(format!("sort_order = ?{param_idx}")); - param_idx += 1; - } - if input.metadata.is_some() { - sets.push(format!("metadata = ?{param_idx}")); - } - let sql = format!( - "UPDATE items SET {} WHERE id = ?1 AND deleted_at IS NULL", - sets.join(", ") - ); - let mut stmt = conn.prepare(&sql)?; - let mut param_values: Vec> = Vec::new(); - param_values.push(Box::new(id.to_string())); - param_values.push(Box::new(ts)); - if let Some(ref name) = input.name { - param_values.push(Box::new(name.clone())); - } - if let Some(ref desc) = input.description { - param_values.push(Box::new(desc.clone())); - } - if let Some(ref pri) = input.priority { - param_values.push(Box::new(pri.clone())); - } - if let Some(ref sid) = input.state_id { - param_values.push(Box::new(sid.clone())); - } - if let Some(ref agent) = assignee_agent { - param_values.push(Box::new(agent.clone())); - } - if let Some(so) = input.sort_order { - param_values.push(Box::new(so)); - } - if let Some(ref metadata) = input.metadata { - param_values.push(Box::new(metadata.clone())); - } - let changed = stmt.execute(rusqlite::params_from_iter(param_values.iter()))?; - if changed == 0 { - return Err(crate::error::Error::NotFound(id.to_string())); - } - let item = get(conn, id)?; - if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { - events::emit( - conn, - &wid, - "item", - "update", - serde_json::to_value(&item).unwrap_or_default(), - ); - } - Ok(item) -} - -/// Moves an item to a different state within its project. Unlike `update()`, -/// this sets `started_at`/`completed_at` based on the *target* state's -/// group — deliberately not a transition state-machine (Plane itself allows -/// any state → any state; only timestamps follow group membership), so the -/// one real constraint enforced here is that `state_id` belongs to the same -/// project as the item. -pub fn update_state(conn: &Connection, id: &str, state_id: &str) -> Result { - let item = get(conn, id)?; - let state = crate::state::get(conn, state_id)?; - if state.project_id != item.project_id { - return Err(crate::error::Error::InvalidTransition(format!( - "state {state_id} belongs to a different project than item {id}" - ))); - } - let ts = now(); - let changed = match state.group_name.as_str() { - "started" => conn.execute( - "UPDATE items SET state_id = ?2, started_at = ?3, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", - rusqlite::params![id, state_id, ts], - )?, - "completed" => conn.execute( - "UPDATE items SET state_id = ?2, completed_at = ?3, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", - rusqlite::params![id, state_id, ts], - )?, - _ => conn.execute( - "UPDATE items SET state_id = ?2, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", - rusqlite::params![id, state_id, ts], - )?, - }; - if changed == 0 { - return Err(crate::error::Error::NotFound(id.to_string())); - } - let item = get(conn, id)?; - if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { - events::emit( - conn, - &wid, - "item", - "update", - serde_json::to_value(&item).unwrap_or_default(), - ); - } - Ok(item) -} - -pub fn delete(conn: &Connection, id: &str) -> Result<()> { - let item = get(conn, id)?; - let ts = now(); - let changed = conn.execute( - "UPDATE items SET deleted_at = ?1, updated_at = ?1 WHERE id = ?2 AND deleted_at IS NULL", - rusqlite::params![ts, id], - )?; - if changed == 0 { - return Err(crate::error::Error::NotFound(id.to_string())); - } - if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { - events::emit( - conn, - &wid, - "item", - "delete", - serde_json::json!({"id": item.id}), - ); - } - Ok(()) -} - -pub fn add_label(conn: &Connection, item_id: &str, label_id: &str) -> Result<()> { - // A label may only be attached to an item in the same scope: a project-scoped - // label must share the item's project; a workspace-level label (project_id NULL) - // must share the item's workspace. This mirrors Plane's project-membership check - // and, because item::create routes through here, guards that path too. - let item = get(conn, item_id)?; - let label = crate::label::get(conn, label_id)?; - let in_scope = match &label.project_id { - Some(project_id) => project_id == &item.project_id, - None => label.workspace_id == workspace_id_for_project(conn, &item.project_id)?, - }; - if !in_scope { - return Err(crate::error::Error::Validation(format!( - "label {label_id} is not in item {item_id}'s scope (project or workspace)" - ))); - } - conn.execute( - "INSERT OR IGNORE INTO item_labels (item_id, label_id) VALUES (?1, ?2)", - rusqlite::params![item_id, label_id], - )?; - Ok(()) -} - -pub fn remove_label(conn: &Connection, item_id: &str, label_id: &str) -> Result<()> { - conn.execute( - "DELETE FROM item_labels WHERE item_id = ?1 AND label_id = ?2", - rusqlite::params![item_id, label_id], - )?; - Ok(()) -} - -pub fn list_labels(conn: &Connection, item_id: &str) -> Result> { - let mut stmt = conn.prepare("SELECT label_id FROM item_labels WHERE item_id = ?1")?; - let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; - Ok(rows.collect::>()?) -} - -pub fn add_assignee(conn: &Connection, item_id: &str, agent_id: &str) -> Result<()> { - conn.execute( - "INSERT OR IGNORE INTO item_assignees (item_id, agent_id) VALUES (?1, ?2)", - rusqlite::params![item_id, agent_id], - )?; - Ok(()) -} - -pub fn remove_assignee(conn: &Connection, item_id: &str, agent_id: &str) -> Result<()> { - conn.execute( - "DELETE FROM item_assignees WHERE item_id = ?1 AND agent_id = ?2", - rusqlite::params![item_id, agent_id], - )?; - Ok(()) -} - -pub fn list_assignees(conn: &Connection, item_id: &str) -> Result> { - let mut stmt = conn.prepare("SELECT agent_id FROM item_assignees WHERE item_id = ?1")?; - let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; - Ok(rows.collect::>()?) -} - -pub fn add_dependency(conn: &Connection, item_id: &str, depends_on: &str) -> Result<()> { - conn.execute( - "INSERT OR IGNORE INTO item_dependencies (item_id, depends_on_item_id) VALUES (?1, ?2)", - rusqlite::params![item_id, depends_on], - )?; - Ok(()) -} - -pub fn remove_dependency(conn: &Connection, item_id: &str, depends_on: &str) -> Result<()> { - conn.execute( - "DELETE FROM item_dependencies WHERE item_id = ?1 AND depends_on_item_id = ?2", - rusqlite::params![item_id, depends_on], - )?; - Ok(()) -} - -pub fn list_dependencies(conn: &Connection, item_id: &str) -> Result> { - let mut stmt = - conn.prepare("SELECT depends_on_item_id FROM item_dependencies WHERE item_id = ?1")?; - let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; - Ok(rows.collect::>()?) -} - -/// Dependency edges for a set of items, with each edge's target state_group -/// already joined in — so a caller's blocking status is correct even when -/// the dependency target isn't itself in the same shortlist/limit window -/// (e.g. a completed dependency that fell outside `groom`'s cap must not -/// read back as an open blocker just because its state wasn't looked up). -/// `(item_id, depends_on_item_id, depends_on_state_group)`. -pub fn dependency_edges_for_items( - conn: &Connection, - item_ids: &[String], -) -> Result> { - if item_ids.is_empty() { - return Ok(vec![]); - } - let placeholders = item_ids.iter().map(|_| "?").collect::>().join(","); - let sql = format!( - "SELECT d.item_id, d.depends_on_item_id, s.group_name - FROM item_dependencies d - JOIN items i ON i.id = d.depends_on_item_id AND i.deleted_at IS NULL - JOIN states s ON s.id = i.state_id - WHERE d.item_id IN ({placeholders})" - ); - let mut stmt = conn.prepare(&sql)?; - let rows = stmt.query_map(rusqlite::params_from_iter(item_ids.iter()), |row| { - Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, String>(2)?, - )) - })?; - Ok(rows.collect::>()?) -} - -/// Fan-in counts: for each of `item_ids`, how many other (non-deleted) items -/// declare a dependency on it — project-wide, not limited to the same -/// shortlist/limit window a caller happens to be looking at. -pub fn dependency_fanin_for_items( - conn: &Connection, - item_ids: &[String], -) -> Result> { - if item_ids.is_empty() { - return Ok(std::collections::HashMap::new()); - } - let placeholders = item_ids.iter().map(|_| "?").collect::>().join(","); - let sql = format!( - "SELECT d.depends_on_item_id, COUNT(*) - FROM item_dependencies d - JOIN items i ON i.id = d.item_id AND i.deleted_at IS NULL - WHERE d.depends_on_item_id IN ({placeholders}) - GROUP BY d.depends_on_item_id" - ); - let mut stmt = conn.prepare(&sql)?; - let rows = stmt.query_map(rusqlite::params_from_iter(item_ids.iter()), |row| { - Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) - })?; - Ok(rows.collect::>()?) -} - -/// FTS5 search across items (name, description, metadata) within a project. -/// Returns BM25-ranked results, most relevant first. Query is sanitised -/// via `flare-search-kit` into safe FTS5 tokens (quoted, operators -/// neutralised) so user input like `PR-123` isn't misinterpreted as -/// column:value syntax. -/// -/// Falls back to a `LIKE` substring scan when FTS5 finds nothing. FTS5's -/// default tokenizer splits on `-`/`_`, so a compound identifier like -/// `agentflare-store` indexes as separate `agentflare`/`store` tokens — -/// a query for `flare-store` (or bare `flare`) would otherwise miss it, -/// since `flare` is a suffix, not a prefix, of `agentflare`. -pub fn search( - conn: &Connection, - project_id: &str, - query: &str, - limit: Option, -) -> Result> { - let limit = limit.unwrap_or(20); - let safe = - flare_search_kit::fts_query(query, flare_search_kit::MatchMode::All).unwrap_or_default(); - if safe.is_empty() { - return Ok(vec![]); - } - let mut stmt = conn.prepare( - "SELECT items.id, items.project_id, items.state_id, items.name, items.description, - items.priority, items.parent_id, items.assignee_agent, items.sequence_id, - items.sort_order, items.started_at, items.completed_at, items.archived_at, - items.external_source, items.external_id, items.metadata, - items.created_at, items.updated_at, items.deleted_at - FROM items_fts - JOIN items ON items.rowid = items_fts.rowid - WHERE items.project_id = ?1 - AND items_fts MATCH ?2 - AND items.deleted_at IS NULL - ORDER BY bm25(items_fts, 3.0, 1.0, 1.0) - LIMIT ?3", - )?; - let rows = stmt.query_map( - rusqlite::params![project_id, safe, flare_search_kit::clamped_limit(limit)], - row_to_item, - )?; - let results: Vec = rows.collect::>()?; - if !results.is_empty() { - return Ok(results); - } - - let like_pat = format!( - "%{}%", - query - .replace('\\', "\\\\") - .replace('%', "\\%") - .replace('_', "\\_") - ); - let mut like_stmt = conn.prepare( - "SELECT items.id, items.project_id, items.state_id, items.name, items.description, - items.priority, items.parent_id, items.assignee_agent, items.sequence_id, - items.sort_order, items.started_at, items.completed_at, items.archived_at, - items.external_source, items.external_id, items.metadata, - items.created_at, items.updated_at, items.deleted_at - FROM items - WHERE items.project_id = ?1 - AND items.deleted_at IS NULL - AND (items.name LIKE ?2 ESCAPE '\\' OR items.description LIKE ?2 ESCAPE '\\') - ORDER BY items.updated_at DESC - LIMIT ?3", - )?; - let like_rows = like_stmt.query_map( - rusqlite::params![project_id, like_pat, flare_search_kit::clamped_limit(limit)], - row_to_item, - )?; - Ok(like_rows.collect::>()?) -} - -/// Outcome of a claim attempt — the raw lease `Acquire` plus the handoff -/// freeze rule: while an item carries an `assignee_agent` that nobody has -/// claimed yet (a handoff sitting unaccepted), only that assignee may -/// acquire it. Once any claim has ever been taken (even a since-stale one), -/// the ordinary `Acquired`/`Held` staleness rules take back over — this -/// variant only covers the fresh, never-claimed window. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum ClaimOutcome { - Acquired, - Held { owner: String, age_secs: i64 }, - BlockedByAssignee { assignee: String }, -} - -/// Canonical agent identity of an owner id (`:` -> -/// canonical ``), matching `assignee_agent`'s canonical form — -/// `assignee_agent` is canonicalized on write (see `create`/`update`), but -/// `owner` is the raw caller-supplied id, so an alias like `claude:1` must -/// be canonicalized here too or it won't match `claude-code`. -/// -/// `pub` because `assignee_agent` legitimately carries the instance suffix -/// after a claim (`claim()` below stores the raw `owner`, on purpose — see -/// its own doc comment and the tests pinning that), so any caller outside -/// this module that reads `assignee_agent` back to resolve *which agent -/// type* it names (not which specific instance) needs the same stripping -/// this module already does internally, instead of re-deriving it. -pub fn agent_part(owner: &str) -> String { - agent_registry::canonicalize(owner.split(':').next().unwrap_or(owner)) -} - -/// Claims an item so other agents don't duplicate the work: on a fresh -/// acquire, sets the assignee and moves state into the project's "started" -/// group (which sets `started_at`, via `update_state`). A live claim held by -/// someone else returns `Held` and leaves the item untouched. An item -/// freshly handed off (assignee set, never yet claimed) to a *different* -/// agent than the caller returns `BlockedByAssignee` instead of letting the -/// caller silently steal it. Acquisition, the state transition, and the -/// assignee update are one transaction — a mid-sequence failure can't leave -/// `item_claims` saying "claimed" while the item itself never reflects it. -pub fn claim( - conn: &Connection, - item_id: &str, - owner: &str, - now: i64, - ttl_secs: i64, -) -> Result { - let tx = conn.unchecked_transaction()?; - let item = get(&tx, item_id)?; - if let Some(assignee) = &item.assignee_agent - && agent_part(assignee) != agent_part(owner) - && crate::claim::current_owner(&tx, item_id).is_none() - { - // Excludes completed/cancelled items: a done-and-released item is - // fair game for anyone to re-claim (e.g. reopened follow-up work) — - // the freeze only protects a handoff that's still open. - let state = crate::state::get(&tx, &item.state_id)?; - if !matches!(state.group_name.as_str(), "completed" | "cancelled") { - return Ok(ClaimOutcome::BlockedByAssignee { - assignee: assignee.clone(), - }); - } - } - let outcome = crate::claim::acquire(&tx, item_id, owner, now, ttl_secs)?; - let result = match outcome { - crate::claim::Acquire::Acquired => { - let started_state = crate::state::first_in_group(&tx, &item.project_id, "started")?; - update_state(&tx, item_id, &started_state.id)?; - update( - &tx, - item_id, - UpdateItem { - assignee_agent: Some(owner.to_string()), - ..Default::default() - }, - )?; - ClaimOutcome::Acquired - } - crate::claim::Acquire::Held { owner, age_secs } => ClaimOutcome::Held { owner, age_secs }, - }; - tx.commit()?; - Ok(result) -} - -/// Moves a claimed item into the project's "completed" group WITHOUT -/// releasing the claim lease yet. Deliberately split from the lease release -/// (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 -/// 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 { - // 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(&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) -} - -/// Moves a claimed item into the project's "in_review" group, same shape and -/// same claim-lease-stays-held contract as `mark_completed` above — used -/// instead of it when `done` results in an open PR (item #420). The work -/// isn't actually finished until that PR merges: landing straight on -/// "completed" would show the item as done while its PR is still red or -/// under review, which is the state-side half of the bug `mark_completed` -/// alone had (the other half was deleting the worktree too, fixed in -/// `mcp_server::item::item_done` by only cleaning up when no PR resulted). -/// -/// Auto-creates the "in_review" state on first use per project: it's in -/// `state::DEFAULT_STATES` for every project seeded after item #420, but -/// existing projects were seeded before it existed and have no such state -/// to find. -pub fn mark_in_review(conn: &Connection, item_id: &str, owner: &str) -> Result { - let tx = conn.unchecked_transaction()?; - if !crate::claim::is_owner(&tx, item_id, owner)? { - return Ok(false); - } - let item = get(&tx, item_id)?; - let review_state = match crate::state::first_in_group(&tx, &item.project_id, "in_review") { - Ok(s) => s, - Err(crate::error::Error::NotFound(_)) => crate::state::create( - &tx, - crate::state::CreateState { - project_id: item.project_id.clone(), - name: crate::state::IN_REVIEW_STATE_NAME.into(), - group_name: "in_review".into(), - sequence: crate::state::IN_REVIEW_STATE_SEQUENCE, - is_default: None, - color: Some(crate::state::IN_REVIEW_STATE_COLOR.into()), - }, - )?, - Err(e) => return Err(e), - }; - update_state(&tx, item_id, &review_state.id)?; - tx.commit()?; - Ok(true) -} - -/// Promotes an item from "in_review" to "completed" once its PR is -/// confirmed merged (`item_check_merge`, item #420). Unlike -/// `mark_completed`/`mark_in_review`, this is deliberately NOT owner-scoped: -/// `item_done` leaves the claim lease held (not released) when it moves an -/// item into "in_review" specifically so nobody else can claim it out from -/// under the pending review, so by the time anything reaches this function -/// no other owner could legally exist — whoever notices the merge and calls -/// `check_merge`, possibly a different session than the one that opened the -/// PR, is allowed to finish the transition. Releases whatever lease is -/// still held as part of the same commit. Returns `Ok(false)` (a no-op, -/// not an error) when the item isn't currently in "in_review" — callers -/// can call this speculatively without checking state first. -pub fn promote_in_review_to_completed(conn: &Connection, item_id: &str) -> Result { - let tx = conn.unchecked_transaction()?; - let item = get(&tx, item_id)?; - let state = crate::state::get(&tx, &item.state_id)?; - if state.group_name != "in_review" { - return Ok(false); - } - let completed_state = crate::state::first_in_group(&tx, &item.project_id, "completed")?; - update_state(&tx, item_id, &completed_state.id)?; - if let Some(owner) = crate::claim::current_owner(&tx, item_id) { - crate::claim::done(&tx, item_id, &owner, now())?; - } - tx.commit()?; - Ok(true) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::db; - use crate::project::{self, CreateProject}; - use crate::workspace::{self, CreateWorkspace}; - - fn seed_project(conn: &Connection, suffix: &str) -> (String, String) { - let ws = workspace::create( - conn, - CreateWorkspace { - name: format!("Test{suffix}"), - slug: format!("test{suffix}"), - owner_agent: None, - item_label: None, - }, - ) - .unwrap(); - let proj = project::create( - conn, - CreateProject { - workspace_id: ws.id.clone(), - name: format!("Test{suffix}"), - identifier: format!("T{suffix}"), - external_source: None, - external_id: None, - }, - ) - .unwrap(); - let states = crate::state::list_by_project(conn, &proj.id).unwrap(); - let state_id = states - .iter() - .find(|s| s.is_default) - .map(|s| s.id.clone()) - .unwrap(); - (proj.id, state_id) - } - - #[test] - fn create_and_get() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid, - state_id: sid, - name: "Test Item".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert_eq!(item.name, "Test Item"); - assert_eq!(item.sequence_id, 1); - let got = get(&conn, &item.id).unwrap(); - assert_eq!(got.id, item.id); - } - - #[test] - fn sequence_increments() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let i1 = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "First".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let i2 = create( - &conn, - CreateItem { - project_id: pid, - state_id: sid, - name: "Second".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert_eq!(i1.sequence_id, 1); - assert_eq!(i2.sequence_id, 2); - } - - #[test] - fn list_by_project_scopes() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let (pid2, _sid2) = seed_project(&conn, "2"); - create( - &conn, - CreateItem { - project_id: pid1.clone(), - state_id: sid1, - name: "Item 1".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert_eq!(list_by_project(&conn, &pid1).unwrap().len(), 1); - assert_eq!(list_by_project(&conn, &pid2).unwrap().len(), 0); - } - - #[test] - fn add_and_remove_labels() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let ws = crate::workspace::list(&conn) - .unwrap() - .into_iter() - .next() - .unwrap(); - let label = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: Some(pid), - workspace_id: ws.id, - name: "bug".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - add_label(&conn, &item.id, &label.id).unwrap(); - let labels = list_labels(&conn, &item.id).unwrap(); - assert_eq!(labels.len(), 1); - assert_eq!(labels[0], label.id); - remove_label(&conn, &item.id, &label.id).unwrap(); - assert!(list_labels(&conn, &item.id).unwrap().is_empty()); - } - - fn workspace_by_slug(conn: &Connection, slug: &str) -> String { - workspace::list(conn) - .unwrap() - .into_iter() - .find(|w| w.slug == slug) - .unwrap() - .id - } - - #[test] - fn add_label_rejects_label_from_another_project() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let (pid2, _sid2) = seed_project(&conn, "2"); - let item = create( - &conn, - CreateItem { - project_id: pid1, - state_id: sid1, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let foreign = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: Some(pid2), - workspace_id: workspace_by_slug(&conn, "test2"), - name: "bug".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - let err = add_label(&conn, &item.id, &foreign.id).unwrap_err(); - assert!(matches!(err, crate::error::Error::Validation(_))); - assert!(list_labels(&conn, &item.id).unwrap().is_empty()); - } - - #[test] - fn add_label_accepts_workspace_level_label_in_same_workspace() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let item = create( - &conn, - CreateItem { - project_id: pid1, - state_id: sid1, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - // Workspace-level label (project_id = None) in the item's workspace. - let global = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: None, - workspace_id: workspace_by_slug(&conn, "test1"), - name: "global".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - add_label(&conn, &item.id, &global.id).unwrap(); - assert_eq!(list_labels(&conn, &item.id).unwrap().len(), 1); - } - - #[test] - fn add_label_rejects_workspace_level_label_from_another_workspace() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let (_pid2, _sid2) = seed_project(&conn, "2"); - let item = create( - &conn, - CreateItem { - project_id: pid1, - state_id: sid1, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - // Workspace-level label (project_id = None) but in a *different* workspace. - let foreign_global = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: None, - workspace_id: workspace_by_slug(&conn, "test2"), - name: "global".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - let err = add_label(&conn, &item.id, &foreign_global.id).unwrap_err(); - assert!(matches!(err, crate::error::Error::Validation(_))); - assert!(list_labels(&conn, &item.id).unwrap().is_empty()); - } - - #[test] - fn add_and_remove_assignees() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid, - state_id: sid, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - add_assignee(&conn, &item.id, "agent:1").unwrap(); - add_assignee(&conn, &item.id, "agent:2").unwrap(); - let agents = list_assignees(&conn, &item.id).unwrap(); - assert_eq!(agents.len(), 2); - remove_assignee(&conn, &item.id, "agent:1").unwrap(); - assert_eq!(list_assignees(&conn, &item.id).unwrap().len(), 1); - } - - #[test] - fn add_and_remove_dependencies() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let i1 = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "A".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let i2 = create( - &conn, - CreateItem { - project_id: pid, - state_id: sid, - name: "B".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let i1_id = i1.id.clone(); - let i2_id = i2.id.clone(); - add_dependency(&conn, &i2_id, &i1_id).unwrap(); - let deps = list_dependencies(&conn, &i2_id).unwrap(); - assert_eq!(deps, vec![i1_id.clone()]); - remove_dependency(&conn, &i2_id, &i1_id).unwrap(); - assert!(list_dependencies(&conn, &i2.id).unwrap().is_empty()); - } - - #[test] - fn create_wires_up_label_assignee_and_dependency_ids() { - // Regression test: CreateItem.label_ids/assignee_ids/dependency_ids - // must actually be attached by create(), not silently dropped. - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let ws = crate::workspace::list(&conn) - .unwrap() - .into_iter() - .next() - .unwrap(); - let label = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: Some(pid.clone()), - workspace_id: ws.id, - name: "bug".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - let blocker = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "Blocker".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let item = create( - &conn, - CreateItem { - project_id: pid, - state_id: sid, - name: "Test".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![label.id.clone()], - assignee_ids: vec!["agent:1".into()], - dependency_ids: vec![blocker.id.clone()], - }, - ) - .unwrap(); - assert_eq!(list_labels(&conn, &item.id).unwrap(), vec![label.id]); - assert_eq!( - list_assignees(&conn, &item.id).unwrap(), - vec!["agent:1".to_string()] - ); - assert_eq!( - list_dependencies(&conn, &item.id).unwrap(), - vec![blocker.id] - ); - } - - fn state_in_group(conn: &Connection, project_id: &str, group: &str) -> String { - crate::state::list_by_project(conn, project_id) - .unwrap() - .into_iter() - .find(|s| s.group_name == group) - .unwrap() - .id - } - - #[test] - fn update_state_sets_started_at_when_moving_into_started_group() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert!(item.started_at.is_none()); - let started_state = state_in_group(&conn, &pid, "started"); - let updated = update_state(&conn, &item.id, &started_state).unwrap(); - assert!(updated.started_at.is_some()); - assert!(updated.completed_at.is_none()); - } - - #[test] - fn update_state_sets_completed_at_when_moving_into_completed_group() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let completed_state = state_in_group(&conn, &pid, "completed"); - let updated = update_state(&conn, &item.id, &completed_state).unwrap(); - assert!(updated.completed_at.is_some()); - } - - #[test] - fn update_state_leaves_timestamps_none_when_moving_into_backlog() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let backlog_state = state_in_group(&conn, &pid, "backlog"); - let updated = update_state(&conn, &item.id, &backlog_state).unwrap(); - assert!(updated.started_at.is_none()); - assert!(updated.completed_at.is_none()); - } - - #[test] - fn create_rejects_state_from_a_different_project() { - let conn = db::open_in_memory().unwrap(); - let (pid1, _sid1) = seed_project(&conn, "1"); - let (_pid2, sid2) = seed_project(&conn, "2"); - assert!(matches!( - create( - &conn, - CreateItem { - project_id: pid1, - state_id: sid2, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ), - Err(crate::error::Error::InvalidTransition(_)) - )); - } - - #[test] - fn update_state_rejects_state_from_a_different_project() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let (pid2, _sid2) = seed_project(&conn, "2"); - let item = create( - &conn, - CreateItem { - project_id: pid1, - state_id: sid1, - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let other_project_state = state_in_group(&conn, &pid2, "started"); - assert!(matches!( - update_state(&conn, &item.id, &other_project_state), - Err(crate::error::Error::InvalidTransition(_)) - )); - } - - const TTL: i64 = 14400; - - fn make_item(conn: &Connection, pid: &str, sid: &str) -> Item { - create( - conn, - CreateItem { - project_id: pid.to_string(), - state_id: sid.to_string(), - name: "Test".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap() - } - - #[test] - fn claim_acquires_sets_assignee_and_moves_to_started_state() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - let outcome = claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); - assert_eq!(outcome, ClaimOutcome::Acquired); - let updated = get(&conn, &item.id).unwrap(); - assert_eq!(updated.assignee_agent.as_deref(), Some("agent:1")); - assert_eq!(updated.state_id, state_in_group(&conn, &pid, "started")); - assert!(updated.started_at.is_some()); - } - - #[test] - fn claim_on_already_held_item_returns_held_and_leaves_item_unchanged() { - 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(); - let outcome = claim(&conn, &item.id, "agent:2", 1001, TTL).unwrap(); - assert!(matches!( - outcome, - ClaimOutcome::Held { ref owner, .. } if owner == "agent:1" - )); - let unchanged = get(&conn, &item.id).unwrap(); - assert_eq!(unchanged.assignee_agent.as_deref(), Some("agent:1")); - } - - #[test] - fn stale_claim_is_stealable_by_a_different_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(); - let outcome = claim(&conn, &item.id, "agent:2", 1000 + TTL + 1, TTL).unwrap(); - assert_eq!(outcome, ClaimOutcome::Acquired); - let updated = get(&conn, &item.id).unwrap(); - assert_eq!(updated.assignee_agent.as_deref(), Some("agent:2")); - } - - #[test] - fn claim_by_a_different_agent_than_the_handoff_assignee_is_blocked() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - // Simulate a handoff: assignee set, never claimed yet. - update( - &conn, - &item.id, - UpdateItem { - assignee_agent: Some("opencode".into()), - ..Default::default() - }, - ) - .unwrap(); - let outcome = claim(&conn, &item.id, "claude-code:1", 1000, TTL).unwrap(); - assert_eq!( - outcome, - ClaimOutcome::BlockedByAssignee { - assignee: "opencode".to_string() - } - ); - let unchanged = get(&conn, &item.id).unwrap(); - assert_eq!(unchanged.assignee_agent.as_deref(), Some("opencode")); - assert!(crate::claim::current_owner(&conn, &item.id).is_none()); - } - - #[test] - fn claim_by_the_handoff_assignee_itself_succeeds() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - update( - &conn, - &item.id, - UpdateItem { - assignee_agent: Some("opencode".into()), - ..Default::default() - }, - ) - .unwrap(); - let outcome = claim(&conn, &item.id, "opencode:1", 1000, TTL).unwrap(); - assert_eq!(outcome, ClaimOutcome::Acquired); - } - - #[test] - fn claim_by_the_handoff_assignee_via_an_alias_succeeds() { - // assignee_agent is canonicalized on write ("claude" -> "claude-code"), - // but `owner` is the raw caller-supplied id — an alias owner must - // still be recognized as the assignee, not blocked as an impostor. - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - update( - &conn, - &item.id, - UpdateItem { - assignee_agent: Some("claude".into()), - ..Default::default() - }, - ) - .unwrap(); - assert_eq!( - get(&conn, &item.id).unwrap().assignee_agent.as_deref(), - Some("claude-code") - ); - let outcome = claim(&conn, &item.id, "claude:1", 1000, TTL).unwrap(); - assert_eq!(outcome, ClaimOutcome::Acquired); - } - - #[test] - fn current_owner_returns_the_claim_owner() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - assert!(crate::claim::current_owner(&conn, &item.id).is_none()); - claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); - assert_eq!( - crate::claim::current_owner(&conn, &item.id).as_deref(), - Some("agent:1") - ); - } - - #[test] - fn current_owner_returns_none_after_done() { - 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(); - crate::claim::done(&conn, &item.id, "agent:1", 2000).unwrap(); - assert!(crate::claim::current_owner(&conn, &item.id).is_none()); - } - - #[test] - 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); - claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); - assert!(mark_completed(&conn, &item.id, "agent:1").unwrap()); - let done_item = get(&conn, &item.id).unwrap(); - assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed")); - assert!(done_item.completed_at.is_some()); - - // Lease is still held — concurrent claim must be rejected. - match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() { - ClaimOutcome::Held { .. } => {} - other => panic!("expected Held after mark_completed, got {other:?}"), - } - - // 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, ClaimOutcome::Acquired); - } - - #[test] - 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()); - } - - #[test] - fn mark_in_review_moves_to_in_review_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); - claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); - assert!(mark_in_review(&conn, &item.id, "agent:1").unwrap()); - let reviewed = get(&conn, &item.id).unwrap(); - assert_eq!(reviewed.state_id, state_in_group(&conn, &pid, "in_review")); - // Not actually finished yet -- completed_at must stay unset. - assert!(reviewed.completed_at.is_none()); - - // Lease is still held, same contract as mark_completed. - match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() { - ClaimOutcome::Held { .. } => {} - other => panic!("expected Held after mark_in_review, got {other:?}"), - } - } - - #[test] - fn mark_in_review_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_in_review(&conn, &item.id, "agent:2").unwrap()); - } - - #[test] - fn mark_in_review_backfills_the_state_for_a_project_seeded_before_it_existed() { - // Simulates a project created before item #420: delete the - // "in_review" state seed_defaults would otherwise have created, and - // confirm mark_in_review heals it instead of erroring. - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let old_review_state_id = state_in_group(&conn, &pid, "in_review"); - conn.execute( - "UPDATE states SET deleted_at = 1 WHERE id = ?1", - rusqlite::params![old_review_state_id], - ) - .unwrap(); - assert!(crate::state::first_in_group(&conn, &pid, "in_review").is_err()); - - let item = make_item(&conn, &pid, &sid); - claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); - assert!(mark_in_review(&conn, &item.id, "agent:1").unwrap()); - - let reviewed = get(&conn, &item.id).unwrap(); - let healed = crate::state::first_in_group(&conn, &pid, "in_review").unwrap(); - assert_eq!(reviewed.state_id, healed.id); - assert_ne!(healed.id, old_review_state_id); - } - - #[test] - fn promote_in_review_to_completed_moves_state_and_releases_the_lease() { - 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_in_review(&conn, &item.id, "agent:1").unwrap()); - - assert!(promote_in_review_to_completed(&conn, &item.id).unwrap()); - let done_item = get(&conn, &item.id).unwrap(); - assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed")); - assert!(done_item.completed_at.is_some()); - - // Lease was released -- a different agent can claim it now. - let outcome = claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap(); - assert_eq!(outcome, ClaimOutcome::Acquired); - } - - #[test] - fn promote_in_review_to_completed_is_a_noop_when_not_in_review() { - 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(); - // Still "started", never moved to in_review. - assert!(!promote_in_review_to_completed(&conn, &item.id).unwrap()); - let unchanged = get(&conn, &item.id).unwrap(); - assert_eq!(unchanged.state_id, state_in_group(&conn, &pid, "started")); - } - - #[test] - fn search_ranks_by_relevance() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "Database schema migration".into(), - description: Some("Add users table".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "Fix login button".into(), - description: Some("Update CSS for login page button".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Backup database".into(), - description: Some("PR-123 adds nightly DB backup".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let results = search(&conn, &pid, "PR-123", None).unwrap(); - assert_eq!(results.len(), 1); - assert!(results[0].description.contains("PR-123")); - - let db_results = search(&conn, &pid, "database", None).unwrap(); - assert_eq!(db_results.len(), 2); - // Both matched — "Database" is in name of item 1, "database" - // is in name of item 3. BM25 ranking may tie; verify both match. - assert!( - db_results[0].name.to_lowercase().contains("database") - || db_results[0] - .description - .to_lowercase() - .contains("database") - ); - } - - #[test] - fn search_empty_query_returns_nothing() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Test".into(), - description: Some("something".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - let results = search(&conn, &pid, "", None).unwrap(); - assert!(results.is_empty()); - } - - #[test] - fn search_scoped_to_project() { - let conn = db::open_in_memory().unwrap(); - let (pid1, sid1) = seed_project(&conn, "1"); - let (pid2, sid2) = seed_project(&conn, "2"); - create( - &conn, - CreateItem { - project_id: pid1.clone(), - state_id: sid1, - name: "Database setup".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - create( - &conn, - CreateItem { - project_id: pid2.clone(), - state_id: sid2, - name: "Database setup".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert_eq!(search(&conn, &pid1, "database", None).unwrap().len(), 1); - assert_eq!(search(&conn, &pid2, "database", None).unwrap().len(), 1); - } - - #[test] - fn search_falls_back_to_like_for_suffix_of_compound_token() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Implement agentflare-store v1".into(), - description: Some("unified local storage layer".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - - // FTS5 tokenizes "agentflare-store" as ["agentflare", "store"], so a - // bare "flare" query (a suffix, not a prefix, of "agentflare") finds - // nothing via MATCH — only the LIKE fallback can find it. - let results = search(&conn, &pid, "flare-store", None).unwrap(); - assert_eq!(results.len(), 1); - assert!(results[0].name.contains("agentflare-store")); - } - - #[test] - fn search_like_fallback_matches_literal_backslash_in_query() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: r"agentflare\filter setup".into(), - description: Some("unrelated".into()), - priority: None, - parent_id: None, - assignee_agent: None, - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - - // FTS5 tokenizes on the backslash the same way it does on a hyphen - // (see the suffix-of-compound-token test above), so "flare\filter" - // has no whole-token FTS match and only the LIKE fallback can find - // it. Before escaping backslashes first, `format!` left the query's - // real `\` in the pattern un-doubled, so SQLite's `ESCAPE '\\'` - // silently swallowed it as an (undefined) escape prefix for the - // next character instead of matching it literally — the fallback - // then missed a hit it should have found. - let results = search(&conn, &pid, r"flare\filter", None).unwrap(); - assert_eq!(results.len(), 1); - } - - #[test] - fn heartbeat_release_done_are_owner_scoped() { - 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!(!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!(crate::claim::heartbeat(&conn, &item.id, "agent:1", 1100).unwrap()); - assert!(crate::claim::done(&conn, &item.id, "agent:1", 1200).unwrap()); - } - - #[test] - fn resolve_id_passes_through_uuid_unchanged() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - - let resolved = resolve_id(&conn, Some(&pid), &item.id).unwrap(); - assert_eq!(resolved, item.id); - } - - #[test] - fn resolve_id_resolves_bare_numeric_sequence_id() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - - let resolved = resolve_id(&conn, Some(&pid), &item.sequence_id.to_string()).unwrap(); - assert_eq!(resolved, item.id); - } - - #[test] - fn resolve_id_resolves_hash_prefixed_sequence_id() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let item = make_item(&conn, &pid, &sid); - - let resolved = resolve_id(&conn, Some(&pid), &format!("#{}", item.sequence_id)).unwrap(); - assert_eq!(resolved, item.id); - } - - #[test] - fn resolve_id_numeric_not_found_returns_not_found_error() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - let _item = make_item(&conn, &pid, &sid); - - let err = resolve_id(&conn, Some(&pid), "999999").unwrap_err(); - assert!(matches!(err, crate::error::Error::NotFound(_)), "{err:?}"); - } - - #[test] - fn resolve_id_scopes_numeric_lookup_to_project() { - let conn = db::open_in_memory().unwrap(); - let (pid_a, sid_a) = seed_project(&conn, "a"); - let (pid_b, _sid_b) = seed_project(&conn, "b"); - let item = make_item(&conn, &pid_a, &sid_a); - - // The item's sequence_id exists in project A but not project B. - let err = resolve_id(&conn, Some(&pid_b), &item.sequence_id.to_string()).unwrap_err(); - assert!(matches!(err, crate::error::Error::NotFound(_)), "{err:?}"); - } - - #[test] - fn create_and_update_canonicalize_known_assignee_aliases() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, ""); - - let item = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "Test".into(), - description: None, - priority: None, - parent_id: None, - assignee_agent: Some("claude".into()), - sort_order: None, - external_source: None, - external_id: None, - metadata: None, - label_ids: vec![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - assert_eq!(item.assignee_agent.as_deref(), Some("claude-code")); - - let updated = update( - &conn, - &item.id, - UpdateItem { - assignee_agent: Some("Claude Code".into()), - ..Default::default() - }, - ) - .unwrap(); - assert_eq!(updated.assignee_agent.as_deref(), Some("claude-code")); - } - - #[test] - fn list_by_label_returns_only_items_carrying_that_label() { - let conn = db::open_in_memory().unwrap(); - let (pid, sid) = seed_project(&conn, "label"); - let ws_id = crate::project::get(&conn, &pid).unwrap().workspace_id; - let label = crate::label::create( - &conn, - crate::label::CreateLabel { - project_id: Some(pid.clone()), - workspace_id: ws_id, - name: "ready-for-work".into(), - color: None, - parent_id: None, - sort_order: None, - external_source: None, - external_id: None, - }, - ) - .unwrap(); - - let labeled = create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid.clone(), - name: "Labeled".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - create( - &conn, - CreateItem { - project_id: pid.clone(), - state_id: sid, - name: "Unlabeled".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![], - assignee_ids: vec![], - dependency_ids: vec![], - }, - ) - .unwrap(); - add_label(&conn, &labeled.id, &label.id).unwrap(); - - let found = list_by_label(&conn, &pid, &label.id).unwrap(); - assert_eq!(found.len(), 1); - assert_eq!(found[0].id, labeled.id); - } -} diff --git a/crates/agentflare-backend/src/item/mod.rs b/crates/agentflare-backend/src/item/mod.rs new file mode 100644 index 00000000..f223023f --- /dev/null +++ b/crates/agentflare-backend/src/item/mod.rs @@ -0,0 +1,821 @@ +use rusqlite::Connection; +use serde::{Deserialize, Serialize}; + +use crate::error::Result; +use crate::events; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Item { + pub id: String, + pub project_id: String, + pub state_id: String, + pub name: String, + pub description: String, + pub priority: String, + pub parent_id: Option, + pub assignee_agent: Option, + pub sequence_id: i64, + pub sort_order: f64, + pub started_at: Option, + pub completed_at: Option, + pub archived_at: Option, + pub external_source: Option, + pub external_id: Option, + pub metadata: String, + pub created_at: i64, + pub updated_at: i64, + pub deleted_at: Option, +} + +#[derive(Debug, Deserialize)] +pub struct CreateItem { + pub project_id: String, + pub state_id: String, + pub name: String, + pub description: Option, + pub priority: Option, + pub parent_id: Option, + pub assignee_agent: Option, + pub sort_order: Option, + pub external_source: Option, + pub external_id: Option, + pub metadata: Option, + pub label_ids: Vec, + pub assignee_ids: Vec, + pub dependency_ids: Vec, +} + +#[derive(Debug, Deserialize, Default)] +pub struct UpdateItem { + pub name: Option, + pub description: Option, + pub priority: Option, + pub state_id: Option, + pub assignee_agent: Option, + pub sort_order: Option, + pub metadata: Option, +} + +fn now() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +fn row_to_item(row: &rusqlite::Row) -> rusqlite::Result { + Ok(Item { + id: row.get(0)?, + project_id: row.get(1)?, + state_id: row.get(2)?, + name: row.get(3)?, + description: row.get(4)?, + priority: row.get(5)?, + parent_id: row.get(6)?, + assignee_agent: row.get(7)?, + sequence_id: row.get(8)?, + sort_order: row.get(9)?, + started_at: row.get(10)?, + completed_at: row.get(11)?, + archived_at: row.get(12)?, + external_source: row.get(13)?, + external_id: row.get(14)?, + metadata: row.get(15)?, + created_at: row.get(16)?, + updated_at: row.get(17)?, + deleted_at: row.get(18)?, + }) +} + +fn next_sequence_id(conn: &Connection, project_id: &str) -> rusqlite::Result { + conn.execute( + "INSERT INTO project_sequences (project_id, next_seq) VALUES (?1, 1) + ON CONFLICT(project_id) DO UPDATE SET next_seq = next_seq + 1", + rusqlite::params![project_id], + )?; + conn.query_row( + "SELECT next_seq FROM project_sequences WHERE project_id = ?1", + rusqlite::params![project_id], + |row| row.get(0), + ) +} + +fn workspace_id_for_project(conn: &Connection, project_id: &str) -> Result { + conn.query_row( + "SELECT workspace_id FROM projects WHERE id = ?1 AND deleted_at IS NULL", + rusqlite::params![project_id], + |row| row.get(0), + ) + .map_err(|e| match e { + rusqlite::Error::QueryReturnedNoRows => { + crate::error::Error::NotFound(project_id.to_string()) + } + other => other.into(), + }) +} + +pub fn create(conn: &Connection, input: CreateItem) -> Result { + let id = db_kit::ids::new_id(); + let ts = now(); + let sort_order = input.sort_order.unwrap_or(65535.0); + let description = input.description.unwrap_or_default(); + let priority = input.priority.unwrap_or_else(|| "none".to_string()); + let metadata = input.metadata.unwrap_or_else(|| "{}".to_string()); + let assignee_agent = input + .assignee_agent + .as_deref() + .map(agent_registry::canonicalize); + + let state = crate::state::get(conn, &input.state_id)?; + if state.project_id != input.project_id { + return Err(crate::error::Error::InvalidTransition(format!( + "state {} belongs to a different project than project {}", + input.state_id, input.project_id + ))); + } + + let tx = conn.unchecked_transaction()?; + let seq = next_sequence_id(&tx, &input.project_id)?; + tx.execute( + "INSERT INTO items (id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, external_source, external_id, metadata, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)", + rusqlite::params![ + id, + input.project_id, + input.state_id, + input.name, + description, + priority, + input.parent_id, + assignee_agent, + seq, + sort_order, + input.external_source, + input.external_id, + metadata, + ts, + ts, + ], + )?; + for label_id in &input.label_ids { + add_label(&tx, &id, label_id)?; + } + for agent_id in &input.assignee_ids { + add_assignee(&tx, &id, agent_id)?; + } + for dep_id in &input.dependency_ids { + add_dependency(&tx, &id, dep_id)?; + } + tx.commit()?; + let item = get(conn, &id)?; + if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { + events::emit( + conn, + &wid, + "item", + "create", + serde_json::to_value(&item).unwrap_or_default(), + ); + } + Ok(item) +} + +pub fn get(conn: &Connection, id: &str) -> Result { + conn.query_row( + "SELECT id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, started_at, completed_at, archived_at, external_source, external_id, metadata, created_at, updated_at, deleted_at + FROM items WHERE id = ?1 AND deleted_at IS NULL", + rusqlite::params![id], + row_to_item, + ) + .map_err(|e| match e { + rusqlite::Error::QueryReturnedNoRows => crate::error::Error::NotFound(id.to_string()), + other => other.into(), + }) +} + +/// Resolve a user-supplied identifier to an item UUID. +/// Accepts a UUID (pass-through) or a numeric `sequence_id`. +/// When `project_id` is `Some`, scopes the sequence_id lookup to that project; +/// when `None`, searches across all projects (returns the first match). +pub fn resolve_id(conn: &Connection, project_id: Option<&str>, id_or_seq: &str) -> Result { + let numeric_part = id_or_seq.strip_prefix('#').unwrap_or(id_or_seq); + if let Ok(seq) = numeric_part.parse::() { + let sql = match project_id { + Some(_) => { + "SELECT id FROM items WHERE project_id = ?1 AND sequence_id = ?2 AND deleted_at IS NULL" + } + None => "SELECT id FROM items WHERE sequence_id = ?1 AND deleted_at IS NULL LIMIT 1", + }; + let params: Vec> = match project_id { + Some(pid) => vec![Box::new(pid.to_string()), Box::new(seq)], + None => vec![Box::new(seq)], + }; + let params_ref: Vec<&dyn rusqlite::types::ToSql> = + params.iter().map(|p| p.as_ref()).collect(); + conn.query_row(sql, params_ref.as_slice(), |row| row.get(0)) + .map_err(|e| match e { + rusqlite::Error::QueryReturnedNoRows => { + crate::error::Error::NotFound(format!("sequence_id #{seq}")) + } + other => other.into(), + }) + } else { + Ok(id_or_seq.to_string()) + } +} + +pub fn list_by_project(conn: &Connection, project_id: &str) -> Result> { + let mut stmt = conn.prepare( + "SELECT id, project_id, state_id, name, description, priority, parent_id, assignee_agent, sequence_id, sort_order, started_at, completed_at, archived_at, external_source, external_id, metadata, created_at, updated_at, deleted_at + FROM items WHERE project_id = ?1 AND deleted_at IS NULL ORDER BY sort_order", + )?; + let rows = stmt.query_map(rusqlite::params![project_id], row_to_item)?; + Ok(rows.collect::>()?) +} + +pub fn list_by_label(conn: &Connection, project_id: &str, label_id: &str) -> Result> { + let mut stmt = conn.prepare( + "SELECT items.id, items.project_id, items.state_id, items.name, items.description, items.priority, items.parent_id, items.assignee_agent, items.sequence_id, items.sort_order, items.started_at, items.completed_at, items.archived_at, items.external_source, items.external_id, items.metadata, items.created_at, items.updated_at, items.deleted_at + FROM items + INNER JOIN item_labels ON item_labels.item_id = items.id + WHERE item_labels.label_id = ?1 AND items.project_id = ?2 AND items.deleted_at IS NULL + ORDER BY items.sort_order", + )?; + let rows = stmt.query_map(rusqlite::params![label_id, project_id], row_to_item)?; + Ok(rows.collect::>()?) +} + +/// List non-deleted items assigned to an agent (excludes completed/cancelled). +pub fn list_by_assignee_agent( + conn: &Connection, + project_id: &str, + agent: &str, +) -> Result> { + let mut stmt = conn.prepare( + "SELECT i.id, i.project_id, i.state_id, i.name, i.description, + i.priority, i.parent_id, i.assignee_agent, i.sequence_id, + i.sort_order, i.started_at, i.completed_at, i.archived_at, + i.external_source, i.external_id, i.metadata, + i.created_at, i.updated_at, i.deleted_at + FROM items i + JOIN states s ON s.id = i.state_id + WHERE i.project_id = ?1 + AND i.assignee_agent = ?2 + AND i.deleted_at IS NULL + AND s.group_name NOT IN ('completed', 'cancelled') + ORDER BY i.sort_order", + )?; + let rows = stmt.query_map(rusqlite::params![project_id, agent], row_to_item)?; + Ok(rows.collect::>()?) +} + +pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { + let ts = now(); + let assignee_agent = input + .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() { + sets.push(format!("name = ?{param_idx}")); + param_idx += 1; + } + if input.description.is_some() { + sets.push(format!("description = ?{param_idx}")); + param_idx += 1; + } + if input.priority.is_some() { + sets.push(format!("priority = ?{param_idx}")); + param_idx += 1; + } + if input.state_id.is_some() { + sets.push(format!("state_id = ?{param_idx}")); + param_idx += 1; + } + if assignee_agent.is_some() { + sets.push(format!("assignee_agent = ?{param_idx}")); + param_idx += 1; + } + if input.sort_order.is_some() { + sets.push(format!("sort_order = ?{param_idx}")); + param_idx += 1; + } + if input.metadata.is_some() { + sets.push(format!("metadata = ?{param_idx}")); + } + let sql = format!( + "UPDATE items SET {} WHERE id = ?1 AND deleted_at IS NULL", + sets.join(", ") + ); + let mut stmt = conn.prepare(&sql)?; + let mut param_values: Vec> = Vec::new(); + param_values.push(Box::new(id.to_string())); + param_values.push(Box::new(ts)); + if let Some(ref name) = input.name { + param_values.push(Box::new(name.clone())); + } + if let Some(ref desc) = input.description { + param_values.push(Box::new(desc.clone())); + } + if let Some(ref pri) = input.priority { + param_values.push(Box::new(pri.clone())); + } + if let Some(ref sid) = input.state_id { + param_values.push(Box::new(sid.clone())); + } + if let Some(ref agent) = assignee_agent { + param_values.push(Box::new(agent.clone())); + } + if let Some(so) = input.sort_order { + param_values.push(Box::new(so)); + } + if let Some(ref metadata) = input.metadata { + param_values.push(Box::new(metadata.clone())); + } + let changed = stmt.execute(rusqlite::params_from_iter(param_values.iter()))?; + 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( + conn, + &wid, + "item", + "update", + serde_json::to_value(&item).unwrap_or_default(), + ); + } + Ok(item) +} + +/// Moves an item to a different state within its project. Unlike `update()`, +/// this sets `started_at`/`completed_at` based on the *target* state's +/// group — deliberately not a transition state-machine (Plane itself allows +/// any state → any state; only timestamps follow group membership), so the +/// one real constraint enforced here is that `state_id` belongs to the same +/// project as the item. +pub fn update_state(conn: &Connection, id: &str, state_id: &str) -> Result { + let item = get(conn, id)?; + let state = crate::state::get(conn, state_id)?; + if state.project_id != item.project_id { + return Err(crate::error::Error::InvalidTransition(format!( + "state {state_id} belongs to a different project than item {id}" + ))); + } + let ts = now(); + let changed = match state.group_name.as_str() { + "started" => conn.execute( + "UPDATE items SET state_id = ?2, started_at = ?3, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", + rusqlite::params![id, state_id, ts], + )?, + "completed" => conn.execute( + "UPDATE items SET state_id = ?2, completed_at = ?3, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", + rusqlite::params![id, state_id, ts], + )?, + _ => conn.execute( + "UPDATE items SET state_id = ?2, updated_at = ?3 WHERE id = ?1 AND deleted_at IS NULL", + rusqlite::params![id, state_id, ts], + )?, + }; + if changed == 0 { + return Err(crate::error::Error::NotFound(id.to_string())); + } + let item = get(conn, id)?; + if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { + events::emit( + conn, + &wid, + "item", + "update", + serde_json::to_value(&item).unwrap_or_default(), + ); + } + Ok(item) +} + +pub fn delete(conn: &Connection, id: &str) -> Result<()> { + let item = get(conn, id)?; + let ts = now(); + let changed = conn.execute( + "UPDATE items SET deleted_at = ?1, updated_at = ?1 WHERE id = ?2 AND deleted_at IS NULL", + rusqlite::params![ts, id], + )?; + if changed == 0 { + return Err(crate::error::Error::NotFound(id.to_string())); + } + if let Ok(wid) = workspace_id_for_project(conn, &item.project_id) { + events::emit( + conn, + &wid, + "item", + "delete", + serde_json::json!({"id": item.id}), + ); + } + Ok(()) +} + +pub fn add_label(conn: &Connection, item_id: &str, label_id: &str) -> Result<()> { + // A label may only be attached to an item in the same scope: a project-scoped + // label must share the item's project; a workspace-level label (project_id NULL) + // must share the item's workspace. This mirrors Plane's project-membership check + // and, because item::create routes through here, guards that path too. + let item = get(conn, item_id)?; + let label = crate::label::get(conn, label_id)?; + let in_scope = match &label.project_id { + Some(project_id) => project_id == &item.project_id, + None => label.workspace_id == workspace_id_for_project(conn, &item.project_id)?, + }; + if !in_scope { + return Err(crate::error::Error::Validation(format!( + "label {label_id} is not in item {item_id}'s scope (project or workspace)" + ))); + } + conn.execute( + "INSERT OR IGNORE INTO item_labels (item_id, label_id) VALUES (?1, ?2)", + rusqlite::params![item_id, label_id], + )?; + Ok(()) +} + +pub fn remove_label(conn: &Connection, item_id: &str, label_id: &str) -> Result<()> { + conn.execute( + "DELETE FROM item_labels WHERE item_id = ?1 AND label_id = ?2", + rusqlite::params![item_id, label_id], + )?; + Ok(()) +} + +pub fn list_labels(conn: &Connection, item_id: &str) -> Result> { + let mut stmt = conn.prepare("SELECT label_id FROM item_labels WHERE item_id = ?1")?; + let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; + Ok(rows.collect::>()?) +} + +pub fn add_assignee(conn: &Connection, item_id: &str, agent_id: &str) -> Result<()> { + conn.execute( + "INSERT OR IGNORE INTO item_assignees (item_id, agent_id) VALUES (?1, ?2)", + rusqlite::params![item_id, agent_id], + )?; + Ok(()) +} + +pub fn remove_assignee(conn: &Connection, item_id: &str, agent_id: &str) -> Result<()> { + conn.execute( + "DELETE FROM item_assignees WHERE item_id = ?1 AND agent_id = ?2", + rusqlite::params![item_id, agent_id], + )?; + Ok(()) +} + +pub fn list_assignees(conn: &Connection, item_id: &str) -> Result> { + let mut stmt = conn.prepare("SELECT agent_id FROM item_assignees WHERE item_id = ?1")?; + let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; + Ok(rows.collect::>()?) +} + +pub fn add_dependency(conn: &Connection, item_id: &str, depends_on: &str) -> Result<()> { + conn.execute( + "INSERT OR IGNORE INTO item_dependencies (item_id, depends_on_item_id) VALUES (?1, ?2)", + rusqlite::params![item_id, depends_on], + )?; + Ok(()) +} + +pub fn remove_dependency(conn: &Connection, item_id: &str, depends_on: &str) -> Result<()> { + conn.execute( + "DELETE FROM item_dependencies WHERE item_id = ?1 AND depends_on_item_id = ?2", + rusqlite::params![item_id, depends_on], + )?; + Ok(()) +} + +pub fn list_dependencies(conn: &Connection, item_id: &str) -> Result> { + let mut stmt = + conn.prepare("SELECT depends_on_item_id FROM item_dependencies WHERE item_id = ?1")?; + let rows = stmt.query_map(rusqlite::params![item_id], |row| row.get::<_, String>(0))?; + Ok(rows.collect::>()?) +} + +/// Dependency edges for a set of items, with each edge's target state_group +/// already joined in — so a caller's blocking status is correct even when +/// the dependency target isn't itself in the same shortlist/limit window +/// (e.g. a completed dependency that fell outside `groom`'s cap must not +/// read back as an open blocker just because its state wasn't looked up). +/// `(item_id, depends_on_item_id, depends_on_state_group)`. +pub fn dependency_edges_for_items( + conn: &Connection, + item_ids: &[String], +) -> Result> { + if item_ids.is_empty() { + return Ok(vec![]); + } + let placeholders = item_ids.iter().map(|_| "?").collect::>().join(","); + let sql = format!( + "SELECT d.item_id, d.depends_on_item_id, s.group_name + FROM item_dependencies d + JOIN items i ON i.id = d.depends_on_item_id AND i.deleted_at IS NULL + JOIN states s ON s.id = i.state_id + WHERE d.item_id IN ({placeholders})" + ); + let mut stmt = conn.prepare(&sql)?; + let rows = stmt.query_map(rusqlite::params_from_iter(item_ids.iter()), |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + })?; + Ok(rows.collect::>()?) +} + +/// Fan-in counts: for each of `item_ids`, how many other (non-deleted) items +/// declare a dependency on it — project-wide, not limited to the same +/// shortlist/limit window a caller happens to be looking at. +pub fn dependency_fanin_for_items( + conn: &Connection, + item_ids: &[String], +) -> Result> { + if item_ids.is_empty() { + return Ok(std::collections::HashMap::new()); + } + let placeholders = item_ids.iter().map(|_| "?").collect::>().join(","); + let sql = format!( + "SELECT d.depends_on_item_id, COUNT(*) + FROM item_dependencies d + JOIN items i ON i.id = d.item_id AND i.deleted_at IS NULL + WHERE d.depends_on_item_id IN ({placeholders}) + GROUP BY d.depends_on_item_id" + ); + let mut stmt = conn.prepare(&sql)?; + let rows = stmt.query_map(rusqlite::params_from_iter(item_ids.iter()), |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) + })?; + Ok(rows.collect::>()?) +} + +/// FTS5 search across items (name, description, metadata) within a project. +/// Returns BM25-ranked results, most relevant first. Query is sanitised +/// via `flare-search-kit` into safe FTS5 tokens (quoted, operators +/// neutralised) so user input like `PR-123` isn't misinterpreted as +/// column:value syntax. +/// +/// Falls back to a `LIKE` substring scan when FTS5 finds nothing. FTS5's +/// default tokenizer splits on `-`/`_`, so a compound identifier like +/// `agentflare-store` indexes as separate `agentflare`/`store` tokens — +/// a query for `flare-store` (or bare `flare`) would otherwise miss it, +/// since `flare` is a suffix, not a prefix, of `agentflare`. +pub fn search( + conn: &Connection, + project_id: &str, + query: &str, + limit: Option, +) -> Result> { + let limit = limit.unwrap_or(20); + let safe = + flare_search_kit::fts_query(query, flare_search_kit::MatchMode::All).unwrap_or_default(); + if safe.is_empty() { + return Ok(vec![]); + } + let mut stmt = conn.prepare( + "SELECT items.id, items.project_id, items.state_id, items.name, items.description, + items.priority, items.parent_id, items.assignee_agent, items.sequence_id, + items.sort_order, items.started_at, items.completed_at, items.archived_at, + items.external_source, items.external_id, items.metadata, + items.created_at, items.updated_at, items.deleted_at + FROM items_fts + JOIN items ON items.rowid = items_fts.rowid + WHERE items.project_id = ?1 + AND items_fts MATCH ?2 + AND items.deleted_at IS NULL + ORDER BY bm25(items_fts, 3.0, 1.0, 1.0) + LIMIT ?3", + )?; + let rows = stmt.query_map( + rusqlite::params![project_id, safe, flare_search_kit::clamped_limit(limit)], + row_to_item, + )?; + let results: Vec = rows.collect::>()?; + if !results.is_empty() { + return Ok(results); + } + + let like_pat = format!( + "%{}%", + query + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_") + ); + let mut like_stmt = conn.prepare( + "SELECT items.id, items.project_id, items.state_id, items.name, items.description, + items.priority, items.parent_id, items.assignee_agent, items.sequence_id, + items.sort_order, items.started_at, items.completed_at, items.archived_at, + items.external_source, items.external_id, items.metadata, + items.created_at, items.updated_at, items.deleted_at + FROM items + WHERE items.project_id = ?1 + AND items.deleted_at IS NULL + AND (items.name LIKE ?2 ESCAPE '\\' OR items.description LIKE ?2 ESCAPE '\\') + ORDER BY items.updated_at DESC + LIMIT ?3", + )?; + let like_rows = like_stmt.query_map( + rusqlite::params![project_id, like_pat, flare_search_kit::clamped_limit(limit)], + row_to_item, + )?; + Ok(like_rows.collect::>()?) +} + +/// Outcome of a claim attempt — the raw lease `Acquire` plus the handoff +/// freeze rule: while an item carries an `assignee_agent` that nobody has +/// claimed yet (a handoff sitting unaccepted), only that assignee may +/// acquire it. Once any claim has ever been taken (even a since-stale one), +/// the ordinary `Acquired`/`Held` staleness rules take back over — this +/// variant only covers the fresh, never-claimed window. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ClaimOutcome { + Acquired, + Held { owner: String, age_secs: i64 }, + BlockedByAssignee { assignee: String }, +} + +/// Canonical agent identity of an owner id (`:` -> +/// canonical ``), matching `assignee_agent`'s canonical form — +/// `assignee_agent` is canonicalized on write (see `create`/`update`), but +/// `owner` is the raw caller-supplied id, so an alias like `claude:1` must +/// be canonicalized here too or it won't match `claude-code`. +/// +/// `pub` because `assignee_agent` legitimately carries the instance suffix +/// after a claim (`claim()` below stores the raw `owner`, on purpose — see +/// its own doc comment and the tests pinning that), so any caller outside +/// this module that reads `assignee_agent` back to resolve *which agent +/// type* it names (not which specific instance) needs the same stripping +/// this module already does internally, instead of re-deriving it. +pub fn agent_part(owner: &str) -> String { + agent_registry::canonicalize(owner.split(':').next().unwrap_or(owner)) +} + +/// Claims an item so other agents don't duplicate the work: on a fresh +/// acquire, sets the assignee and moves state into the project's "started" +/// group (which sets `started_at`, via `update_state`). A live claim held by +/// someone else returns `Held` and leaves the item untouched. An item +/// freshly handed off (assignee set, never yet claimed) to a *different* +/// agent than the caller returns `BlockedByAssignee` instead of letting the +/// caller silently steal it. Acquisition, the state transition, and the +/// assignee update are one transaction — a mid-sequence failure can't leave +/// `item_claims` saying "claimed" while the item itself never reflects it. +pub fn claim( + conn: &Connection, + item_id: &str, + owner: &str, + now: i64, + ttl_secs: i64, +) -> Result { + let tx = conn.unchecked_transaction()?; + let item = get(&tx, item_id)?; + if let Some(assignee) = &item.assignee_agent + && agent_part(assignee) != agent_part(owner) + && crate::claim::current_owner(&tx, item_id).is_none() + { + // Excludes completed/cancelled items: a done-and-released item is + // fair game for anyone to re-claim (e.g. reopened follow-up work) — + // the freeze only protects a handoff that's still open. + let state = crate::state::get(&tx, &item.state_id)?; + if !matches!(state.group_name.as_str(), "completed" | "cancelled") { + return Ok(ClaimOutcome::BlockedByAssignee { + assignee: assignee.clone(), + }); + } + } + let outcome = crate::claim::acquire(&tx, item_id, owner, now, ttl_secs)?; + let result = match outcome { + crate::claim::Acquire::Acquired => { + let started_state = crate::state::first_in_group(&tx, &item.project_id, "started")?; + update_state(&tx, item_id, &started_state.id)?; + update( + &tx, + item_id, + UpdateItem { + assignee_agent: Some(owner.to_string()), + ..Default::default() + }, + )?; + ClaimOutcome::Acquired + } + crate::claim::Acquire::Held { owner, age_secs } => ClaimOutcome::Held { owner, age_secs }, + }; + tx.commit()?; + Ok(result) +} + +/// Moves a claimed item into the project's "completed" group WITHOUT +/// releasing the claim lease yet. Deliberately split from the lease release +/// (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 +/// 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 { + // 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(&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) +} + +/// Moves a claimed item into the project's "in_review" group, same shape and +/// same claim-lease-stays-held contract as `mark_completed` above — used +/// instead of it when `done` results in an open PR (item #420). The work +/// isn't actually finished until that PR merges: landing straight on +/// "completed" would show the item as done while its PR is still red or +/// under review, which is the state-side half of the bug `mark_completed` +/// alone had (the other half was deleting the worktree too, fixed in +/// `mcp_server::item::item_done` by only cleaning up when no PR resulted). +/// +/// Auto-creates the "in_review" state on first use per project: it's in +/// `state::DEFAULT_STATES` for every project seeded after item #420, but +/// existing projects were seeded before it existed and have no such state +/// to find. +pub fn mark_in_review(conn: &Connection, item_id: &str, owner: &str) -> Result { + let tx = conn.unchecked_transaction()?; + if !crate::claim::is_owner(&tx, item_id, owner)? { + return Ok(false); + } + let item = get(&tx, item_id)?; + let review_state = match crate::state::first_in_group(&tx, &item.project_id, "in_review") { + Ok(s) => s, + Err(crate::error::Error::NotFound(_)) => crate::state::create( + &tx, + crate::state::CreateState { + project_id: item.project_id.clone(), + name: crate::state::IN_REVIEW_STATE_NAME.into(), + group_name: "in_review".into(), + sequence: crate::state::IN_REVIEW_STATE_SEQUENCE, + is_default: None, + color: Some(crate::state::IN_REVIEW_STATE_COLOR.into()), + }, + )?, + Err(e) => return Err(e), + }; + update_state(&tx, item_id, &review_state.id)?; + tx.commit()?; + Ok(true) +} + +/// Promotes an item from "in_review" to "completed" once its PR is +/// confirmed merged (`item_check_merge`, item #420). Unlike +/// `mark_completed`/`mark_in_review`, this is deliberately NOT owner-scoped: +/// `item_done` leaves the claim lease held (not released) when it moves an +/// item into "in_review" specifically so nobody else can claim it out from +/// under the pending review, so by the time anything reaches this function +/// no other owner could legally exist — whoever notices the merge and calls +/// `check_merge`, possibly a different session than the one that opened the +/// PR, is allowed to finish the transition. Releases whatever lease is +/// still held as part of the same commit. Returns `Ok(false)` (a no-op, +/// not an error) when the item isn't currently in "in_review" — callers +/// can call this speculatively without checking state first. +pub fn promote_in_review_to_completed(conn: &Connection, item_id: &str) -> Result { + let tx = conn.unchecked_transaction()?; + let item = get(&tx, item_id)?; + let state = crate::state::get(&tx, &item.state_id)?; + if state.group_name != "in_review" { + return Ok(false); + } + let completed_state = crate::state::first_in_group(&tx, &item.project_id, "completed")?; + update_state(&tx, item_id, &completed_state.id)?; + if let Some(owner) = crate::claim::current_owner(&tx, item_id) { + crate::claim::done(&tx, item_id, &owner, now())?; + } + tx.commit()?; + Ok(true) +} + +#[cfg(test)] +mod tests; diff --git a/crates/agentflare-backend/src/item/tests.rs b/crates/agentflare-backend/src/item/tests.rs new file mode 100644 index 00000000..68b7745c --- /dev/null +++ b/crates/agentflare-backend/src/item/tests.rs @@ -0,0 +1,1329 @@ +use super::*; +use crate::db; +use crate::project::{self, CreateProject}; +use crate::workspace::{self, CreateWorkspace}; + +fn seed_project(conn: &Connection, suffix: &str) -> (String, String) { + let ws = workspace::create( + conn, + CreateWorkspace { + name: format!("Test{suffix}"), + slug: format!("test{suffix}"), + owner_agent: None, + item_label: None, + }, + ) + .unwrap(); + let proj = project::create( + conn, + CreateProject { + workspace_id: ws.id.clone(), + name: format!("Test{suffix}"), + identifier: format!("T{suffix}"), + external_source: None, + external_id: None, + }, + ) + .unwrap(); + let states = crate::state::list_by_project(conn, &proj.id).unwrap(); + let state_id = states + .iter() + .find(|s| s.is_default) + .map(|s| s.id.clone()) + .unwrap(); + (proj.id, state_id) +} + +#[test] +fn create_and_get() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid, + state_id: sid, + name: "Test Item".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert_eq!(item.name, "Test Item"); + assert_eq!(item.sequence_id, 1); + let got = get(&conn, &item.id).unwrap(); + assert_eq!(got.id, item.id); +} + +#[test] +fn sequence_increments() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let i1 = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "First".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let i2 = create( + &conn, + CreateItem { + project_id: pid, + state_id: sid, + name: "Second".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert_eq!(i1.sequence_id, 1); + assert_eq!(i2.sequence_id, 2); +} + +#[test] +fn list_by_project_scopes() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let (pid2, _sid2) = seed_project(&conn, "2"); + create( + &conn, + CreateItem { + project_id: pid1.clone(), + state_id: sid1, + name: "Item 1".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert_eq!(list_by_project(&conn, &pid1).unwrap().len(), 1); + assert_eq!(list_by_project(&conn, &pid2).unwrap().len(), 0); +} + +#[test] +fn add_and_remove_labels() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let ws = crate::workspace::list(&conn) + .unwrap() + .into_iter() + .next() + .unwrap(); + let label = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: Some(pid), + workspace_id: ws.id, + name: "bug".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + add_label(&conn, &item.id, &label.id).unwrap(); + let labels = list_labels(&conn, &item.id).unwrap(); + assert_eq!(labels.len(), 1); + assert_eq!(labels[0], label.id); + remove_label(&conn, &item.id, &label.id).unwrap(); + assert!(list_labels(&conn, &item.id).unwrap().is_empty()); +} + +fn workspace_by_slug(conn: &Connection, slug: &str) -> String { + workspace::list(conn) + .unwrap() + .into_iter() + .find(|w| w.slug == slug) + .unwrap() + .id +} + +#[test] +fn add_label_rejects_label_from_another_project() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let (pid2, _sid2) = seed_project(&conn, "2"); + let item = create( + &conn, + CreateItem { + project_id: pid1, + state_id: sid1, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let foreign = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: Some(pid2), + workspace_id: workspace_by_slug(&conn, "test2"), + name: "bug".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + let err = add_label(&conn, &item.id, &foreign.id).unwrap_err(); + assert!(matches!(err, crate::error::Error::Validation(_))); + assert!(list_labels(&conn, &item.id).unwrap().is_empty()); +} + +#[test] +fn add_label_accepts_workspace_level_label_in_same_workspace() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let item = create( + &conn, + CreateItem { + project_id: pid1, + state_id: sid1, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + // Workspace-level label (project_id = None) in the item's workspace. + let global = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: None, + workspace_id: workspace_by_slug(&conn, "test1"), + name: "global".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + add_label(&conn, &item.id, &global.id).unwrap(); + assert_eq!(list_labels(&conn, &item.id).unwrap().len(), 1); +} + +#[test] +fn add_label_rejects_workspace_level_label_from_another_workspace() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let (_pid2, _sid2) = seed_project(&conn, "2"); + let item = create( + &conn, + CreateItem { + project_id: pid1, + state_id: sid1, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + // Workspace-level label (project_id = None) but in a *different* workspace. + let foreign_global = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: None, + workspace_id: workspace_by_slug(&conn, "test2"), + name: "global".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + let err = add_label(&conn, &item.id, &foreign_global.id).unwrap_err(); + assert!(matches!(err, crate::error::Error::Validation(_))); + assert!(list_labels(&conn, &item.id).unwrap().is_empty()); +} + +#[test] +fn add_and_remove_assignees() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid, + state_id: sid, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + add_assignee(&conn, &item.id, "agent:1").unwrap(); + add_assignee(&conn, &item.id, "agent:2").unwrap(); + let agents = list_assignees(&conn, &item.id).unwrap(); + assert_eq!(agents.len(), 2); + remove_assignee(&conn, &item.id, "agent:1").unwrap(); + assert_eq!(list_assignees(&conn, &item.id).unwrap().len(), 1); +} + +#[test] +fn add_and_remove_dependencies() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let i1 = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "A".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let i2 = create( + &conn, + CreateItem { + project_id: pid, + state_id: sid, + name: "B".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let i1_id = i1.id.clone(); + let i2_id = i2.id.clone(); + add_dependency(&conn, &i2_id, &i1_id).unwrap(); + let deps = list_dependencies(&conn, &i2_id).unwrap(); + assert_eq!(deps, vec![i1_id.clone()]); + remove_dependency(&conn, &i2_id, &i1_id).unwrap(); + assert!(list_dependencies(&conn, &i2.id).unwrap().is_empty()); +} + +#[test] +fn create_wires_up_label_assignee_and_dependency_ids() { + // Regression test: CreateItem.label_ids/assignee_ids/dependency_ids + // must actually be attached by create(), not silently dropped. + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let ws = crate::workspace::list(&conn) + .unwrap() + .into_iter() + .next() + .unwrap(); + let label = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: Some(pid.clone()), + workspace_id: ws.id, + name: "bug".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + let blocker = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "Blocker".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let item = create( + &conn, + CreateItem { + project_id: pid, + state_id: sid, + name: "Test".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![label.id.clone()], + assignee_ids: vec!["agent:1".into()], + dependency_ids: vec![blocker.id.clone()], + }, + ) + .unwrap(); + assert_eq!(list_labels(&conn, &item.id).unwrap(), vec![label.id]); + assert_eq!( + list_assignees(&conn, &item.id).unwrap(), + vec!["agent:1".to_string()] + ); + assert_eq!( + list_dependencies(&conn, &item.id).unwrap(), + vec![blocker.id] + ); +} + +fn state_in_group(conn: &Connection, project_id: &str, group: &str) -> String { + crate::state::list_by_project(conn, project_id) + .unwrap() + .into_iter() + .find(|s| s.group_name == group) + .unwrap() + .id +} + +#[test] +fn update_state_sets_started_at_when_moving_into_started_group() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert!(item.started_at.is_none()); + let started_state = state_in_group(&conn, &pid, "started"); + let updated = update_state(&conn, &item.id, &started_state).unwrap(); + assert!(updated.started_at.is_some()); + assert!(updated.completed_at.is_none()); +} + +#[test] +fn update_state_sets_completed_at_when_moving_into_completed_group() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let completed_state = state_in_group(&conn, &pid, "completed"); + let updated = update_state(&conn, &item.id, &completed_state).unwrap(); + assert!(updated.completed_at.is_some()); +} + +#[test] +fn update_state_leaves_timestamps_none_when_moving_into_backlog() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let backlog_state = state_in_group(&conn, &pid, "backlog"); + let updated = update_state(&conn, &item.id, &backlog_state).unwrap(); + assert!(updated.started_at.is_none()); + assert!(updated.completed_at.is_none()); +} + +#[test] +fn create_rejects_state_from_a_different_project() { + let conn = db::open_in_memory().unwrap(); + let (pid1, _sid1) = seed_project(&conn, "1"); + let (_pid2, sid2) = seed_project(&conn, "2"); + assert!(matches!( + create( + &conn, + CreateItem { + project_id: pid1, + state_id: sid2, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ), + Err(crate::error::Error::InvalidTransition(_)) + )); +} + +#[test] +fn update_state_rejects_state_from_a_different_project() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let (pid2, _sid2) = seed_project(&conn, "2"); + let item = create( + &conn, + CreateItem { + project_id: pid1, + state_id: sid1, + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let other_project_state = state_in_group(&conn, &pid2, "started"); + assert!(matches!( + update_state(&conn, &item.id, &other_project_state), + Err(crate::error::Error::InvalidTransition(_)) + )); +} + +const TTL: i64 = 14400; + +fn make_item(conn: &Connection, pid: &str, sid: &str) -> Item { + create( + conn, + CreateItem { + project_id: pid.to_string(), + state_id: sid.to_string(), + name: "Test".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap() +} + +#[test] +fn claim_acquires_sets_assignee_and_moves_to_started_state() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + let outcome = claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); + assert_eq!(outcome, ClaimOutcome::Acquired); + let updated = get(&conn, &item.id).unwrap(); + assert_eq!(updated.assignee_agent.as_deref(), Some("agent:1")); + assert_eq!(updated.state_id, state_in_group(&conn, &pid, "started")); + assert!(updated.started_at.is_some()); +} + +#[test] +fn claim_on_already_held_item_returns_held_and_leaves_item_unchanged() { + 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(); + let outcome = claim(&conn, &item.id, "agent:2", 1001, TTL).unwrap(); + assert!(matches!( + outcome, + ClaimOutcome::Held { ref owner, .. } if owner == "agent:1" + )); + let unchanged = get(&conn, &item.id).unwrap(); + assert_eq!(unchanged.assignee_agent.as_deref(), Some("agent:1")); +} + +#[test] +fn stale_claim_is_stealable_by_a_different_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(); + let outcome = claim(&conn, &item.id, "agent:2", 1000 + TTL + 1, TTL).unwrap(); + assert_eq!(outcome, ClaimOutcome::Acquired); + let updated = get(&conn, &item.id).unwrap(); + assert_eq!(updated.assignee_agent.as_deref(), Some("agent:2")); +} + +#[test] +fn claim_by_a_different_agent_than_the_handoff_assignee_is_blocked() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + // Simulate a handoff: assignee set, never claimed yet. + update( + &conn, + &item.id, + UpdateItem { + assignee_agent: Some("opencode".into()), + ..Default::default() + }, + ) + .unwrap(); + let outcome = claim(&conn, &item.id, "claude-code:1", 1000, TTL).unwrap(); + assert_eq!( + outcome, + ClaimOutcome::BlockedByAssignee { + assignee: "opencode".to_string() + } + ); + let unchanged = get(&conn, &item.id).unwrap(); + assert_eq!(unchanged.assignee_agent.as_deref(), Some("opencode")); + assert!(crate::claim::current_owner(&conn, &item.id).is_none()); +} + +#[test] +fn claim_by_the_handoff_assignee_itself_succeeds() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + update( + &conn, + &item.id, + UpdateItem { + assignee_agent: Some("opencode".into()), + ..Default::default() + }, + ) + .unwrap(); + let outcome = claim(&conn, &item.id, "opencode:1", 1000, TTL).unwrap(); + assert_eq!(outcome, ClaimOutcome::Acquired); +} + +#[test] +fn claim_by_the_handoff_assignee_via_an_alias_succeeds() { + // assignee_agent is canonicalized on write ("claude" -> "claude-code"), + // but `owner` is the raw caller-supplied id — an alias owner must + // still be recognized as the assignee, not blocked as an impostor. + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + update( + &conn, + &item.id, + UpdateItem { + assignee_agent: Some("claude".into()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + get(&conn, &item.id).unwrap().assignee_agent.as_deref(), + Some("claude-code") + ); + let outcome = claim(&conn, &item.id, "claude:1", 1000, TTL).unwrap(); + assert_eq!(outcome, ClaimOutcome::Acquired); +} + +#[test] +fn current_owner_returns_the_claim_owner() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + assert!(crate::claim::current_owner(&conn, &item.id).is_none()); + claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); + assert_eq!( + crate::claim::current_owner(&conn, &item.id).as_deref(), + Some("agent:1") + ); +} + +#[test] +fn current_owner_returns_none_after_done() { + 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(); + crate::claim::done(&conn, &item.id, "agent:1", 2000).unwrap(); + assert!(crate::claim::current_owner(&conn, &item.id).is_none()); +} + +#[test] +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); + claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); + assert!(mark_completed(&conn, &item.id, "agent:1").unwrap()); + let done_item = get(&conn, &item.id).unwrap(); + assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed")); + assert!(done_item.completed_at.is_some()); + + // Lease is still held — concurrent claim must be rejected. + match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() { + ClaimOutcome::Held { .. } => {} + other => panic!("expected Held after mark_completed, got {other:?}"), + } + + // 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, ClaimOutcome::Acquired); +} + +#[test] +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()); +} + +#[test] +fn mark_in_review_moves_to_in_review_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); + claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); + assert!(mark_in_review(&conn, &item.id, "agent:1").unwrap()); + let reviewed = get(&conn, &item.id).unwrap(); + assert_eq!(reviewed.state_id, state_in_group(&conn, &pid, "in_review")); + // Not actually finished yet -- completed_at must stay unset. + assert!(reviewed.completed_at.is_none()); + + // Lease is still held, same contract as mark_completed. + match claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap() { + ClaimOutcome::Held { .. } => {} + other => panic!("expected Held after mark_in_review, got {other:?}"), + } +} + +#[test] +fn mark_in_review_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_in_review(&conn, &item.id, "agent:2").unwrap()); +} + +#[test] +fn mark_in_review_backfills_the_state_for_a_project_seeded_before_it_existed() { + // Simulates a project created before item #420: delete the + // "in_review" state seed_defaults would otherwise have created, and + // confirm mark_in_review heals it instead of erroring. + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let old_review_state_id = state_in_group(&conn, &pid, "in_review"); + conn.execute( + "UPDATE states SET deleted_at = 1 WHERE id = ?1", + rusqlite::params![old_review_state_id], + ) + .unwrap(); + assert!(crate::state::first_in_group(&conn, &pid, "in_review").is_err()); + + let item = make_item(&conn, &pid, &sid); + claim(&conn, &item.id, "agent:1", 1000, TTL).unwrap(); + assert!(mark_in_review(&conn, &item.id, "agent:1").unwrap()); + + let reviewed = get(&conn, &item.id).unwrap(); + let healed = crate::state::first_in_group(&conn, &pid, "in_review").unwrap(); + assert_eq!(reviewed.state_id, healed.id); + assert_ne!(healed.id, old_review_state_id); +} + +#[test] +fn promote_in_review_to_completed_moves_state_and_releases_the_lease() { + 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_in_review(&conn, &item.id, "agent:1").unwrap()); + + assert!(promote_in_review_to_completed(&conn, &item.id).unwrap()); + let done_item = get(&conn, &item.id).unwrap(); + assert_eq!(done_item.state_id, state_in_group(&conn, &pid, "completed")); + assert!(done_item.completed_at.is_some()); + + // Lease was released -- a different agent can claim it now. + let outcome = claim(&conn, &item.id, "agent:2", 1200, TTL).unwrap(); + assert_eq!(outcome, ClaimOutcome::Acquired); +} + +#[test] +fn promote_in_review_to_completed_is_a_noop_when_not_in_review() { + 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(); + // Still "started", never moved to in_review. + assert!(!promote_in_review_to_completed(&conn, &item.id).unwrap()); + let unchanged = get(&conn, &item.id).unwrap(); + assert_eq!(unchanged.state_id, state_in_group(&conn, &pid, "started")); +} + +#[test] +fn search_ranks_by_relevance() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "Database schema migration".into(), + description: Some("Add users table".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "Fix login button".into(), + description: Some("Update CSS for login page button".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Backup database".into(), + description: Some("PR-123 adds nightly DB backup".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let results = search(&conn, &pid, "PR-123", None).unwrap(); + assert_eq!(results.len(), 1); + assert!(results[0].description.contains("PR-123")); + + let db_results = search(&conn, &pid, "database", None).unwrap(); + assert_eq!(db_results.len(), 2); + // Both matched — "Database" is in name of item 1, "database" + // is in name of item 3. BM25 ranking may tie; verify both match. + assert!( + db_results[0].name.to_lowercase().contains("database") + || db_results[0] + .description + .to_lowercase() + .contains("database") + ); +} + +#[test] +fn search_empty_query_returns_nothing() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Test".into(), + description: Some("something".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + let results = search(&conn, &pid, "", None).unwrap(); + assert!(results.is_empty()); +} + +#[test] +fn search_scoped_to_project() { + let conn = db::open_in_memory().unwrap(); + let (pid1, sid1) = seed_project(&conn, "1"); + let (pid2, sid2) = seed_project(&conn, "2"); + create( + &conn, + CreateItem { + project_id: pid1.clone(), + state_id: sid1, + name: "Database setup".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + create( + &conn, + CreateItem { + project_id: pid2.clone(), + state_id: sid2, + name: "Database setup".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert_eq!(search(&conn, &pid1, "database", None).unwrap().len(), 1); + assert_eq!(search(&conn, &pid2, "database", None).unwrap().len(), 1); +} + +#[test] +fn search_falls_back_to_like_for_suffix_of_compound_token() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Implement agentflare-store v1".into(), + description: Some("unified local storage layer".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + + // FTS5 tokenizes "agentflare-store" as ["agentflare", "store"], so a + // bare "flare" query (a suffix, not a prefix, of "agentflare") finds + // nothing via MATCH — only the LIKE fallback can find it. + let results = search(&conn, &pid, "flare-store", None).unwrap(); + assert_eq!(results.len(), 1); + assert!(results[0].name.contains("agentflare-store")); +} + +#[test] +fn search_like_fallback_matches_literal_backslash_in_query() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: r"agentflare\filter setup".into(), + description: Some("unrelated".into()), + priority: None, + parent_id: None, + assignee_agent: None, + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + + // FTS5 tokenizes on the backslash the same way it does on a hyphen + // (see the suffix-of-compound-token test above), so "flare\filter" + // has no whole-token FTS match and only the LIKE fallback can find + // it. Before escaping backslashes first, `format!` left the query's + // real `\` in the pattern un-doubled, so SQLite's `ESCAPE '\\'` + // silently swallowed it as an (undefined) escape prefix for the + // next character instead of matching it literally — the fallback + // then missed a hit it should have found. + let results = search(&conn, &pid, r"flare\filter", None).unwrap(); + assert_eq!(results.len(), 1); +} + +#[test] +fn heartbeat_release_done_are_owner_scoped() { + 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!(!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!(crate::claim::heartbeat(&conn, &item.id, "agent:1", 1100).unwrap()); + assert!(crate::claim::done(&conn, &item.id, "agent:1", 1200).unwrap()); +} + +#[test] +fn resolve_id_passes_through_uuid_unchanged() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + + let resolved = resolve_id(&conn, Some(&pid), &item.id).unwrap(); + assert_eq!(resolved, item.id); +} + +#[test] +fn resolve_id_resolves_bare_numeric_sequence_id() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + + let resolved = resolve_id(&conn, Some(&pid), &item.sequence_id.to_string()).unwrap(); + assert_eq!(resolved, item.id); +} + +#[test] +fn resolve_id_resolves_hash_prefixed_sequence_id() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let item = make_item(&conn, &pid, &sid); + + let resolved = resolve_id(&conn, Some(&pid), &format!("#{}", item.sequence_id)).unwrap(); + assert_eq!(resolved, item.id); +} + +#[test] +fn resolve_id_numeric_not_found_returns_not_found_error() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + let _item = make_item(&conn, &pid, &sid); + + let err = resolve_id(&conn, Some(&pid), "999999").unwrap_err(); + assert!(matches!(err, crate::error::Error::NotFound(_)), "{err:?}"); +} + +#[test] +fn resolve_id_scopes_numeric_lookup_to_project() { + let conn = db::open_in_memory().unwrap(); + let (pid_a, sid_a) = seed_project(&conn, "a"); + let (pid_b, _sid_b) = seed_project(&conn, "b"); + let item = make_item(&conn, &pid_a, &sid_a); + + // The item's sequence_id exists in project A but not project B. + let err = resolve_id(&conn, Some(&pid_b), &item.sequence_id.to_string()).unwrap_err(); + assert!(matches!(err, crate::error::Error::NotFound(_)), "{err:?}"); +} + +#[test] +fn create_and_update_canonicalize_known_assignee_aliases() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, ""); + + let item = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "Test".into(), + description: None, + priority: None, + parent_id: None, + assignee_agent: Some("claude".into()), + sort_order: None, + external_source: None, + external_id: None, + metadata: None, + label_ids: vec![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + assert_eq!(item.assignee_agent.as_deref(), Some("claude-code")); + + let updated = update( + &conn, + &item.id, + UpdateItem { + assignee_agent: Some("Claude Code".into()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!(updated.assignee_agent.as_deref(), Some("claude-code")); +} + +#[test] +fn list_by_label_returns_only_items_carrying_that_label() { + let conn = db::open_in_memory().unwrap(); + let (pid, sid) = seed_project(&conn, "label"); + let ws_id = crate::project::get(&conn, &pid).unwrap().workspace_id; + let label = crate::label::create( + &conn, + crate::label::CreateLabel { + project_id: Some(pid.clone()), + workspace_id: ws_id, + name: "ready-for-work".into(), + color: None, + parent_id: None, + sort_order: None, + external_source: None, + external_id: None, + }, + ) + .unwrap(); + + let labeled = create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid.clone(), + name: "Labeled".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + create( + &conn, + CreateItem { + project_id: pid.clone(), + state_id: sid, + name: "Unlabeled".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![], + assignee_ids: vec![], + dependency_ids: vec![], + }, + ) + .unwrap(); + add_label(&conn, &labeled.id, &label.id).unwrap(); + + let found = list_by_label(&conn, &pid, &label.id).unwrap(); + assert_eq!(found.len(), 1); + assert_eq!(found[0].id, labeled.id); +} 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/crates/agentflare-backend/src/migrations/0011_item_assignment_events.sql b/crates/agentflare-backend/src/migrations/0011_item_assignment_events.sql new file mode 100644 index 00000000..2d84c273 --- /dev/null +++ b/crates/agentflare-backend/src/migrations/0011_item_assignment_events.sql @@ -0,0 +1,15 @@ +-- Append-only log of item assignee transitions (claims, handoffs, manual +-- reassignments). Written by item::update whenever assignee_agent actually +-- changes; read by the health scorecard's bottleneck signal ("items handed +-- between agents repeatedly"). History starts at the upgrade that ships this +-- table — transitions before it are unrecorded. +CREATE TABLE IF NOT EXISTS item_assignment_events ( + id TEXT PRIMARY KEY, + item_id TEXT NOT NULL REFERENCES items(id), + from_owner TEXT, + to_owner TEXT NOT NULL, + created_at INTEGER NOT NULL +); + +CREATE INDEX IF NOT EXISTS idx_item_assignment_events_item_time + ON item_assignment_events (item_id, created_at); diff --git a/scripts/loc-gate.sh b/scripts/loc-gate.sh index 87d219d9..c07f19e7 100644 --- a/scripts/loc-gate.sh +++ b/scripts/loc-gate.sh @@ -7,7 +7,6 @@ FROZEN_LIMIT=2000 ALLOWLIST=( src/mcp_server.rs - crates/agentflare-backend/src/item.rs src/components.rs # Already 1604 lines on master before item #441's git-shim polish touched # it -- pre-existing debt, same situation as tick.rs/work.rs above. Frozen diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 455a256d..4f22a78b 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -959,6 +959,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 @@ -1462,7 +1493,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) @@ -1637,15 +1668,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_commit_failure_tests.rs b/src/mcp_server/tests/item_commit_failure_tests.rs index 3e78ee73..00e87e47 100644 --- a/src/mcp_server/tests/item_commit_failure_tests.rs +++ b/src/mcp_server/tests/item_commit_failure_tests.rs @@ -1,3 +1,4 @@ +#[cfg(unix)] use super::*; // Windows' FILE_ATTRIBUTE_READONLY on a *directory* doesn't prevent writes diff --git a/src/mcp_server/tests/item_tests.rs b/src/mcp_server/tests/item_tests.rs index 848f4833..a7be849c 100644 --- a/src/mcp_server/tests/item_tests.rs +++ b/src/mcp_server/tests/item_tests.rs @@ -816,700 +816,3 @@ fn item_groom_reads_size_through_double_encoded_metadata() { assert_eq!(entry["size"], "M"); assert_eq!(entry["unestimated"], false); } - -#[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_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 -/// 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/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 59b4d9fd..f900567e 100644 --- a/src/mcp_server/tests/mod.rs +++ b/src/mcp_server/tests/mod.rs @@ -352,5 +352,6 @@ mod artifact_tests; mod asset_tests; mod item_commit_failure_tests; mod item_tests; +mod item_tests_reporting; mod search_tests; mod state_tests; diff --git a/src/mcp_server/types.rs b/src/mcp_server/types.rs index ff996f2c..e41213d8 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, } From eaf7e5bae083f8a7d86d3b9998d2429d6f059a84 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 13 Aug 2026 10:25:28 +0530 Subject: [PATCH 4/4] test: remove item_health placeholder test superseded by real bottleneck detection item_health_reports_velocity_wip_and_bottleneck_placeholder asserted the old stub message ("no handoff history"), which no longer applies now that assignment_events wires up real bottleneck detection. The identical scenario is already covered by item_tests_reporting's item_health_reports_velocity_wip_and_empty_bottlenecks. Agentflare-Agent: claude-code_2-1-229_agent Agentflare-Branch: task/446-pm-skill-pack-fast-follows-pm-shims-port Agentflare-Item: 446 --- src/mcp_server/tests/item_tests.rs | 66 ------------------------------ 1 file changed, 66 deletions(-) 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