From 4ae0e3ea9dc4ee0fc682f76d574229a6ea1ded1c Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 18:21:01 +0530 Subject: [PATCH 01/13] worktree: fix review gaps for #133 CARGO_TARGET_DIR isolation Document that ambient CARGO_TARGET_DIR env var outranks the per-worktree .cargo/config.toml (Cargo precedence CLI > env > config), so #133's ambient-env case remains open. Also warn on the re-claim fast path. --- src/worktree.rs | 120 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/src/worktree.rs b/src/worktree.rs index b6151d5f..352452b9 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -87,6 +87,64 @@ pub fn ensure_worktrees_ignored(repo_root: &Path) { } } +/// Warns (does not fail) when an ambient `CARGO_TARGET_DIR` is set in the +/// environment at claim time. A shared `CARGO_TARGET_DIR` across worktrees +/// is a silent correctness bug: Cargo's fingerprint hash omits the worktree +/// path, so two worktrees of the same repo reuse each other's stale local +/// crate artifacts (cargo #12516/#14053/#7740; OpenBlob #522). +/// +/// NOTE: this is a mitigation, NOT a fix for the ambient-env case of #133. +/// Per Cargo's precedence (CLI flag > env var > config file), an ambient +/// `CARGO_TARGET_DIR` *always* wins over the `.cargo/config.toml` that +/// `isolate_worktree_target_dir` writes — so when the env var is set, the +/// worktree's isolated `target/` is silently shadowed and the bug persists. +/// Nothing in code can force Cargo to prefer the config file over the env var; +/// the only safe remedies are unsetting the var or trusting CI. #133 therefore +/// remains OPEN for the ambient-env case. +fn warn_if_ambient_target_dir() { + if std::env::var_os("CARGO_TARGET_DIR").is_some() { + eprintln!( + "worktree: ambient CARGO_TARGET_DIR is set — it is SHARED across worktrees and \ + can leak stale artifacts between divergent checkouts. Prefer trusting CI for \ + local test builds, or unset it and rely on the worktree's isolated target dir." + ); + } +} + +/// Writes a per-worktree `.cargo/config.toml` so the worktree's `target/` +/// resolves locally instead of inheriting a shared `CARGO_TARGET_DIR`. +/// +/// Caveat: this only takes effect when `CARGO_TARGET_DIR` is *unset* in the +/// ambient environment. Per Cargo's precedence (CLI flag > env var > config +/// file), an ambient `CARGO_TARGET_DIR` still overrides this file — so this +/// isolates ONLY the default-target case, not the ambient-env case that #133 +/// originally described. `warn_if_ambient_target_dir` is the only mitigation +/// for that case. No config-file change can outrank the env var. +/// +/// Local workspace crates must NOT be shared across worktrees (silent +/// contamination); registry deps are safe but are better served by a shared +/// sccache. A relative `target-dir = "target"` resolves per-checkout, giving +/// each worktree its own isolated cache. Soft-fails (eprintln) — never blocks +/// a claim. +fn isolate_worktree_target_dir(worktree_path: &Path) { + let cargo_dir = worktree_path.join(".cargo"); + let _ = std::fs::create_dir_all(&cargo_dir); + let config_path = cargo_dir.join("config.toml"); + if config_path.exists() { + return; // don't clobber an intentional worktree-local override + } + let content = "[build]\n# Isolated per worktree (see item #133). Registry deps are\n\ + # better shared via sccache (RUSTC_WRAPPER + SCCACHE_BASEDIRS),\n\ + # not a shared CARGO_TARGET_DIR, which leaks artifacts across worktrees.\n\ + target-dir = \"target\"\n"; + if let Err(e) = std::fs::write(&config_path, content) { + eprintln!( + "worktree: could not write isolated .cargo/config.toml for {}: {e}", + worktree_path.display() + ); + } +} + /// Creates an isolated git worktree for `item` against `target_branch`. /// /// Deliberately takes an already-resolved `target_branch` instead of a @@ -107,12 +165,18 @@ pub fn create_worktree( .join("task") .join(item.sequence_id.to_string()); if already_isolated_for(&branch, repo_root) { + // Re-claiming an existing worktree: nothing to create, but still + // ensure its target dir is isolated (idempotent, no-op if present), + // and re-warn since the ambient env can still be shadowing it. + warn_if_ambient_target_dir(); + isolate_worktree_target_dir(&worktree_path); return Some(worktree_path); } ensure_worktrees_ignored(repo_root); if let Some(parent) = worktree_path.parent() { let _ = std::fs::create_dir_all(parent); } + warn_if_ambient_target_dir(); if let Some(p) = progress { p.send( 0.0, @@ -170,6 +234,7 @@ pub fn create_worktree( if let Some(p) = progress { p.send(1.0, Some(1.0), Some("Worktree created".into())); } + isolate_worktree_target_dir(&worktree_path); Some(worktree_path) } Err(e) => { @@ -454,6 +519,61 @@ mod tests { assert!(!already_isolated_for("task/1", &repo.path)); } + #[test] + fn isolate_worktree_target_dir_writes_relative_target_dir() { + let tmp = TempDir::new().unwrap(); + let wt = tmp.path().join(".worktrees").join("task").join("1"); + std::fs::create_dir_all(&wt).unwrap(); + isolate_worktree_target_dir(&wt); + let config = wt.join(".cargo").join("config.toml"); + assert!(config.exists(), "expected .cargo/config.toml in worktree"); + let content = std::fs::read_to_string(&config).unwrap(); + assert!( + content.contains("target-dir = \"target\""), + "must set a relative, per-checkout target dir, got: {content}" + ); + assert!( + !content.contains("target-dir = \"/") + && !content.contains("target-dir = \"~") + && !content.contains("CARGO_TARGET_DIR ="), + "must not set an absolute/shared target dir" + ); + } + + #[test] + fn isolate_worktree_target_dir_does_not_clobber_existing_config() { + let tmp = TempDir::new().unwrap(); + let wt = tmp.path().join(".worktrees").join("task").join("1"); + let cargo_dir = wt.join(".cargo"); + std::fs::create_dir_all(&cargo_dir).unwrap(); + let config = cargo_dir.join("config.toml"); + std::fs::write( + &config, + "[build]\ntarget-dir = \"/some/intentional/path\"\n", + ) + .unwrap(); + isolate_worktree_target_dir(&wt); + let content = std::fs::read_to_string(&config).unwrap(); + assert!( + content.contains("/some/intentional/path"), + "existing worktree-local config must be preserved" + ); + } + + #[test] + fn warn_if_ambient_target_dir_warns_when_set() { + // Just asserts the function runs without panicking whether or not the + // var is set; the warning is an ephemeral eprintln, not assertable here. + unsafe { + std::env::set_var("CARGO_TARGET_DIR", "/tmp/shared"); + } + warn_if_ambient_target_dir(); + unsafe { + std::env::remove_var("CARGO_TARGET_DIR"); + } + warn_if_ambient_target_dir(); + } + #[test] fn already_isolated_for_true_inside_the_worktree_it_created() { let repo = init_repo(); From 8004d1b4f7c39b41141b12aef9b9056c14515b35 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 18:37:11 +0530 Subject: [PATCH 02/13] refactor: rename ponytail/caveman to optimize/flare-code across CLI, init, components, MCP, and auth_runner --- src/auth_runner.rs | 2 +- src/cli/caveman.rs | 85 --------------- src/cli/mod.rs | 6 - src/cli/ponytail.rs | 253 ------------------------------------------- src/components.rs | 65 +++-------- src/init.rs | 48 ++++---- src/mcp_prompts.rs | 64 +++++------ src/optimize/code.rs | 1 + 8 files changed, 71 insertions(+), 453 deletions(-) delete mode 100644 src/cli/caveman.rs delete mode 100644 src/cli/ponytail.rs diff --git a/src/auth_runner.rs b/src/auth_runner.rs index 8c32d674..6e02a52c 100644 --- a/src/auth_runner.rs +++ b/src/auth_runner.rs @@ -41,7 +41,7 @@ pub fn run(agent: &str, args: &[String], json: bool) { if !json { eprintln!("retrying with new profile ({remaining} retries left)..."); } - // ponytail: short backoff, linear increase if rate limits persist + // flare-code: short backoff, linear increase if rate limits persist thread::sleep(Duration::from_secs(1 + (MAX_RETRIES - remaining) as u64)); } ExitKind::Failure(code) => { diff --git a/src/cli/caveman.rs b/src/cli/caveman.rs deleted file mode 100644 index e9e271e1..00000000 --- a/src/cli/caveman.rs +++ /dev/null @@ -1,85 +0,0 @@ -//! DEPRECATED — use `agentflare flare output` instead. -use clap::{Args, Subcommand}; -use std::path::PathBuf; - -#[derive(Subcommand)] -pub enum CavemanAction { - Compress { - source: PathBuf, - target: Option, - #[arg(long)] - spec_file: Option, - #[arg(long)] - backup: Option, - }, -} - -#[derive(Args)] -pub struct CavemanArgs { - #[command(subcommand)] - pub action: CavemanAction, -} - -impl CavemanArgs { - pub fn run(self) { - match self.action { - CavemanAction::Compress { - source, - target, - spec_file, - backup, - } => { - let target = target.unwrap_or_else(|| source.clone()); - let prompt = match &spec_file { - Some(path) => match std::fs::read_to_string(path) { - Ok(spec) => crate::optimize::Prompt::Custom(spec), - Err(e) => { - eprintln!("failed to read spec file {}: {e}", path.display()); - std::process::exit(1); - } - }, - None => crate::optimize::Prompt::Generic, - }; - let backup_mode = match backup.as_deref() { - Some("sibling") => crate::optimize::BackupMode::Sibling, - Some("out-of-tree") | None => crate::optimize::BackupMode::OutOfTree, - Some(other) => { - eprintln!("--backup must be 'sibling' or 'out-of-tree', got '{other}'"); - std::process::exit(1); - } - }; - let result = crate::optimize::compress( - &crate::optimize::RealLlm, - &source, - &target, - prompt, - backup_mode, - ); - match result { - Ok(report) => { - let pct = 100usize.saturating_sub( - 100 * report.compressed_bytes / report.original_bytes.max(1), - ); - println!( - "{}→{}B ▼{pct}%", - report.original_bytes, report.compressed_bytes - ); - println!( - "{}", - crate::cli::optimize::record_and_marker( - report.original_path.clone(), - report.original_bytes as u64, - report.compressed_bytes as u64, - crate::optimize::retrieve::now_unix(), - ) - ); - } - Err(e) => { - eprintln!("{e}"); - std::process::exit(1); - } - } - } - } - } -} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index d92dd435..3319a619 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -2,7 +2,6 @@ mod agents; mod alias; mod artifacts; mod auth; -mod caveman; mod channel; mod claim; mod coaching; @@ -15,7 +14,6 @@ mod init; mod mcp; mod memory; mod optimize; -mod ponytail; mod review; mod run; mod uninstall; @@ -59,8 +57,6 @@ pub enum Commands { Handoff(handoff::HandoffArgs), #[command(alias = "flare", visible_alias = "opt")] Optimize(optimize::OptimizeArgs), - Ponytail(ponytail::PonytailArgs), - Caveman(caveman::CavemanArgs), Channel(channel::ChannelArgs), Claim(claim::ClaimArgs), Review(review::ReviewArgs), @@ -86,8 +82,6 @@ impl Commands { Self::Artifacts(cmd) => cmd.run(), Self::Handoff(cmd) => cmd.run(), Self::Optimize(cmd) => cmd.run(), - Self::Ponytail(cmd) => cmd.run(), - Self::Caveman(cmd) => cmd.run(), Self::Channel(cmd) => cmd.run(), Self::Claim(cmd) => cmd.run(), Self::Review(cmd) => cmd.run(), diff --git a/src/cli/ponytail.rs b/src/cli/ponytail.rs deleted file mode 100644 index 222a8df8..00000000 --- a/src/cli/ponytail.rs +++ /dev/null @@ -1,253 +0,0 @@ -//! DEPRECATED — use `agentflare flare code` instead. -use clap::{Args, Subcommand}; -use std::io::Read; - -const DEFAULT_EXCLUDE_AGENT_TYPES: &str = - "explore|investigat|search|review|readonly|read-only|verify"; - -fn should_inject_for(agent_type: &str, override_matcher: Option<&str>) -> bool { - if agent_type.is_empty() { - return true; - } - let (pattern, is_allowlist) = match override_matcher { - Some(m) => (m, true), - None => (DEFAULT_EXCLUDE_AGENT_TYPES, false), - }; - let re = match regex::Regex::new(&format!("(?i){pattern}")) { - Ok(r) => r, - Err(_) => { - eprintln!("[ponytail] invalid PONYTAIL_SUBAGENT_MATCHER regex — injecting everywhere"); - return true; - } - }; - let matched = re.is_match(agent_type); - if is_allowlist { matched } else { !matched } -} - -fn subagent_should_inject() -> bool { - let override_matcher = std::env::var("PONYTAIL_SUBAGENT_MATCHER").ok(); - let (tx, rx) = std::sync::mpsc::channel(); - std::thread::spawn(move || { - let mut input = String::new(); - let _ = std::io::stdin().read_to_string(&mut input); - let _ = tx.send(input); - }); - let input = match rx.recv_timeout(std::time::Duration::from_millis(1000)) { - Ok(s) => s, - Err(_) => { - eprintln!("[ponytail] SubagentStart stdin timeout — injecting"); - return true; - } - }; - let data: serde_json::Value = match serde_json::from_str(&input) { - Ok(v) => v, - Err(_) => return true, - }; - let agent_type = data - .get("agent_type") - .and_then(|v| v.as_str()) - .unwrap_or(""); - should_inject_for(agent_type, override_matcher.as_deref()) -} - -#[derive(Subcommand)] -pub enum PonytailAction { - Status, - Set { - mode: String, - }, - Default { - mode: String, - }, - Off, - Review, - Audit, - Debt, - Gain, - Info, - Playbook, - NoHallucination, - Hook { - #[command(subcommand)] - event: PonytailHookEvent, - }, -} - -#[derive(Subcommand)] -pub enum PonytailHookEvent { - SessionStart, - SubagentStart, - PromptSubmit, - Statusline, -} - -#[derive(Args)] -pub struct PonytailArgs { - #[command(subcommand)] - pub action: PonytailAction, -} - -fn report_message(mode: &str) -> String { - if mode == "off" { - "ponytail is off. Use /ponytail lite|full|ultra to activate.".to_string() - } else { - format!("PONYTAIL MODE ACTIVE — level: {mode}") - } -} - -fn emit_hook(event: &str, off_guard: bool) { - let mode = - crate::optimize::code::active_mode().unwrap_or_else(crate::optimize::code::default_mode); - if off_guard && mode == "off" { - crate::optimize::code::clear_active(); - println!("OK"); - return; - } - let instructions = crate::optimize::code::build_instructions(&mode, None); - let platform = crate::optimize::code::detect_platform(); - let output = crate::optimize::code::format_hook_output(event, &instructions.body, &platform); - println!("{output}"); -} - -impl PonytailArgs { - pub fn run(self) { - match self.action { - PonytailAction::Status => { - let mode = crate::optimize::code::active_mode() - .unwrap_or_else(crate::optimize::code::default_mode); - println!("{mode}"); - } - PonytailAction::Set { mode } => { - let normalized = crate::optimize::code::normalize_config_mode(&mode) - .unwrap_or_else(|| { - eprintln!("error: invalid mode: {mode}"); - std::process::exit(1); - }); - crate::optimize::code::set_active(normalized).unwrap_or_else(|e| { - eprintln!("error: {e}"); - std::process::exit(1); - }); - println!("{normalized}"); - } - PonytailAction::Default { mode } => { - let normalized = crate::optimize::code::normalize_config_mode(&mode) - .unwrap_or_else(|| { - eprintln!("error: invalid mode: {mode}"); - std::process::exit(1); - }); - crate::optimize::code::set_default_mode(normalized).unwrap_or_else(|e| { - eprintln!("error: {e}"); - std::process::exit(1); - }); - crate::optimize::code::set_active(normalized).ok(); - println!("default: {normalized}"); - } - PonytailAction::Off => { - crate::optimize::code::clear_active(); - println!("off"); - } - PonytailAction::Review => println!("{}", crate::optimize::code::SKILL_REVIEW), - PonytailAction::Audit => println!("{}", crate::optimize::code::SKILL_AUDIT), - PonytailAction::Debt => println!("{}", crate::optimize::code::SKILL_DEBT), - PonytailAction::Gain => println!("{}", crate::optimize::code::SKILL_GAIN), - PonytailAction::Info => println!("{}", crate::optimize::code::SKILL_HELP), - PonytailAction::Playbook => println!("{}", crate::optimize::code::SKILL_PLAYBOOK), - PonytailAction::NoHallucination => { - println!("{}", crate::optimize::code::SKILL_NO_HALLUCINATION) - } - PonytailAction::Hook { event } => match event { - PonytailHookEvent::SessionStart => { - crate::optimize::code::clear_session(); - let mode = crate::optimize::code::active_mode() - .unwrap_or_else(crate::optimize::code::default_mode); - if mode != "off" { - crate::optimize::code::set_active(&mode).ok(); - } - emit_hook("SessionStart", true); - } - PonytailHookEvent::SubagentStart => { - if subagent_should_inject() { - emit_hook("SubagentStart", true); - } - } - PonytailHookEvent::PromptSubmit => { - let mut input = String::new(); - std::io::stdin().read_line(&mut input).ok(); - if let Some(action) = crate::optimize::code::detect_switch_action(&input) { - match action { - crate::optimize::code::SwitchAction::SetMode(m) => { - crate::optimize::code::set_active(&m).ok(); - } - crate::optimize::code::SwitchAction::SetSession(m) => { - crate::optimize::code::set_session(&m).ok(); - } - crate::optimize::code::SwitchAction::SetDefault(m) => { - crate::optimize::code::set_default_mode(&m).ok(); - crate::optimize::code::set_active(&m).ok(); - } - crate::optimize::code::SwitchAction::Off => { - crate::optimize::code::clear_active(); - } - crate::optimize::code::SwitchAction::Report => { - let mode = crate::optimize::code::active_mode() - .unwrap_or_else(crate::optimize::code::default_mode); - let platform = crate::optimize::code::detect_platform(); - let ctx = report_message(&mode); - let output = crate::optimize::code::format_hook_output( - "UserPromptSubmit", - &ctx, - &platform, - ); - println!("{output}"); - return; - } - } - } - println!("OK"); - } - PonytailHookEvent::Statusline => { - let mode = crate::optimize::code::active_mode() - .unwrap_or_else(crate::optimize::code::default_mode); - if mode == "off" || mode.is_empty() { - return; - } - if mode == "full" { - print!("\x1b[38;5;108m[PONYTAIL]\x1b[0m"); - } else { - let upper = mode.to_uppercase(); - print!("\x1b[38;5;108m[PONYTAIL:{upper}]\x1b[0m"); - } - } - }, - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn should_inject_for_excludes_read_only_agent_types_by_default() { - assert!(!should_inject_for("cavecrew-investigator", None)); - assert!(!should_inject_for("Explore", None)); - assert!(!should_inject_for("cavecrew-reviewer", None)); - } - - #[test] - fn should_inject_for_includes_code_writing_agent_types_by_default() { - assert!(should_inject_for("general-purpose", None)); - assert!(should_inject_for("cavecrew-builder", None)); - } - - #[test] - fn should_inject_for_treats_empty_agent_type_as_inject() { - assert!(should_inject_for("", None)); - assert!(should_inject_for("", Some("builder"))); - } - - #[test] - fn should_inject_for_falls_back_to_inject_on_invalid_override_regex() { - assert!(should_inject_for("anything", Some("[invalid("))); - } -} diff --git a/src/components.rs b/src/components.rs index 80348fd6..ebf63298 100644 --- a/src/components.rs +++ b/src/components.rs @@ -288,8 +288,7 @@ pub fn get_components(host: &str) -> Vec { let claude_code_only = host == "claude-code"; let host_owned = host.to_string(); let leanctx_log = crate::state::state_dir().join("leanctx-install.log"); - let ponytail_config = home().join(".config").join("ponytail").join("config.json"); - let caveman_config = home().join(".config").join("caveman").join("config.json"); + let optimize_code_config = crate::optimize::code::config_path(); #[cfg_attr(not(feature = "skill-overrides-sync"), allow(unused_mut))] let mut components = vec![ @@ -540,11 +539,11 @@ pub fn get_components(host: &str) -> Vec { }), }, Component { - id: "ponytail-mode", + id: "optimize-code-mode", needs_consent: false, - describe: "pin Ponytail to ultra mode".to_string(), + describe: "pin flare code to ultra mode".to_string(), check: { - let path = ponytail_config.clone(); + let path = optimize_code_config.clone(); Box::new(move || { if !claude_code_only { return true; @@ -557,43 +556,12 @@ pub fn get_components(host: &str) -> Vec { }) }, apply: { - let path = ponytail_config.clone(); + let path = optimize_code_config.clone(); Box::new(move || { if write_pinned_mode(&path) { - "Ponytail pinned to ultra".to_string() + "flare code pinned to ultra".to_string() } else { - "Ponytail mode already set".to_string() - } - }) - }, - }, - Component { - id: "caveman-mode", - needs_consent: false, - describe: "pin Caveman to ultra mode".to_string(), - check: { - let path = caveman_config.clone(); - Box::new(move || { - if !claude_code_only { - return true; - } - if !plugin_enabled(&claude_settings(), "caveman@caveman") { - return true; // nothing to pin yet - } - fs::read_to_string(&path) - .ok() - .and_then(|s| serde_json::from_str::(&s).ok()) - .and_then(|v| v.get("defaultMode").and_then(|m| m.as_str()).map(String::from)) - == Some("ultra".to_string()) - }) - }, - apply: { - let path = caveman_config.clone(); - Box::new(move || { - if write_pinned_mode(&path) { - "Caveman pinned to ultra".to_string() - } else { - "Caveman mode already set".to_string() + "flare code mode already set".to_string() } }) }, @@ -610,7 +578,7 @@ pub fn get_components(host: &str) -> Vec { // (registered above) become the on-demand detail source. // Claude-Code-only: other hosts have no equivalent per-skill override // mechanism. Not consent-gated (a local config tweak, same trust - // level as ponytail-mode/caveman-mode above) so it also re-syncs on + // level as optimize-code-mode above) so it also re-syncs on // every session-start as new skills appear, not just during `init`. #[cfg(feature = "skill-overrides-sync")] { @@ -666,8 +634,7 @@ mod tests { "leanctx", "agentflare-mcp", "ponytail-plugin", - "ponytail-mode", - "caveman-mode", + "optimize-code-mode", ]; #[cfg(feature = "skill-overrides-sync")] let expected: Vec<&str> = vec![ @@ -676,8 +643,7 @@ mod tests { "leanctx", "agentflare-mcp", "ponytail-plugin", - "ponytail-mode", - "caveman-mode", + "optimize-code-mode", "skill-overrides-sync", ]; @@ -737,11 +703,11 @@ mod tests { } #[test] - fn non_claude_code_hosts_never_need_the_claude_cli_for_ponytail_or_caveman() { + fn non_claude_code_hosts_never_need_the_claude_cli_for_ponytail_plugin() { // Regression check for the host-gating bug caught during manual - // testing: these two components must report "satisfied" (no + // testing: the ponytail plugin component must report "satisfied" (no // pending nag, no attempted install) on every host except - // claude-code, since Ponytail/Caveman have no equivalent elsewhere. + // claude-code, since the plugin has no equivalent elsewhere. for host in [ "codex", "cursor", @@ -756,15 +722,10 @@ mod tests { .iter() .find(|c| c.id == "ponytail-plugin") .unwrap(); - let caveman_mode = components.iter().find(|c| c.id == "caveman-mode").unwrap(); assert!( (ponytail_plugin.check)(), "ponytail-plugin should be satisfied on '{host}'" ); - assert!( - (caveman_mode.check)(), - "caveman-mode should be satisfied on '{host}'" - ); } } diff --git a/src/init.rs b/src/init.rs index f9650de9..d6da55cf 100644 --- a/src/init.rs +++ b/src/init.rs @@ -206,13 +206,13 @@ pub fn run(agent: &str, yes: bool) { "claude-code" => { wire_claude_code(); if confirm_ponytail_migration(agent, yes) { - wire_ponytail_hooks(agent); + wire_optimize_hooks(agent); } } "cursor" => { wire_cursor(); if confirm_ponytail_migration(agent, yes) { - wire_ponytail_hooks(agent); + wire_optimize_hooks(agent); } } "codex" => { @@ -227,7 +227,7 @@ pub fn run(agent: &str, yes: bool) { println!(" work together (plugin handles hooks, agentflare provides"); println!(" skill engine)."); } - wire_ponytail_opencode(); + wire_optimize_opencode(); } _ => {} } @@ -286,8 +286,8 @@ fn confirm_gateway_integrations(agent: &str, yes: bool) { /// all-or-nothing "SessionStart present? skip everything" gate. /// `marker` is a plain substring of `"hook "`, matching both current /// flagless commands and older installs that still carry `--agent ` -/// (upgrades stay idempotent either way). It must not match ponytail's own -/// hook commands (`"" ponytail hook X"`), so both can coexist per event. +/// (upgrades stay idempotent either way). It must not match optimize code's own +/// hook commands (`"" optimize code hook X"`), so both can coexist per event. fn add_hook_entry( hooks_obj: &mut Map, event: &str, @@ -578,16 +578,16 @@ fn wire_opencode() { } } -pub fn wire_ponytail_hooks(agent: &str) { +pub fn wire_optimize_hooks(agent: &str) { match agent { - "claude-code" | "cowork" => wire_ponytail_claude_code(), - "cursor" | "cursor-cli" => wire_ponytail_cursor(), - "opencode" => wire_ponytail_opencode(), + "claude-code" | "cowork" => wire_optimize_claude_code(), + "cursor" | "cursor-cli" => wire_optimize_cursor(), + "opencode" => wire_optimize_opencode(), _ => println!(" info auto-wiring not supported for {agent}. Manual config required."), } } -fn wire_ponytail_claude_code() { +fn wire_optimize_claude_code() { let path = home().join(".claude").join("settings.json"); let mut settings: Value = fs::read_to_string(&path) .ok() @@ -601,10 +601,10 @@ fn wire_ponytail_claude_code() { let already_wired = settings .get("hooks") .and_then(|h| h.get("SessionStart")) - .map(|v| v.to_string().contains("ponytail")) + .map(|v| v.to_string().contains("optimize")) .unwrap_or(false); if already_wired { - println!(" skip ponytail hooks already wired in ~/.claude/settings.json"); + println!(" skip optimize code hooks already wired in ~/.claude/settings.json"); return; } @@ -613,20 +613,20 @@ fn wire_ponytail_claude_code() { let hooks_obj = hooks.as_object_mut().unwrap(); hooks_obj.entry("SessionStart").or_insert_with(|| json!([])).as_array_mut().unwrap().push(json!({ - "hooks": [{ "type": "command", "command": format!("\"{bin}\" ponytail hook session-start"), "timeout": 10 }] + "hooks": [{ "type": "command", "command": format!("\"{bin}\" optimize code hook session-start"), "timeout": 10 }] })); hooks_obj.entry("SubagentStart").or_insert_with(|| json!([])).as_array_mut().unwrap().push(json!({ - "hooks": [{ "type": "command", "command": format!("\"{bin}\" ponytail hook subagent-start"), "timeout": 5 }] + "hooks": [{ "type": "command", "command": format!("\"{bin}\" optimize code hook subagent-start"), "timeout": 5 }] })); hooks_obj.entry("UserPromptSubmit").or_insert_with(|| json!([])).as_array_mut().unwrap().push(json!({ - "hooks": [{ "type": "command", "command": format!("\"{bin}\" ponytail hook prompt-submit"), "timeout": 5 }] + "hooks": [{ "type": "command", "command": format!("\"{bin}\" optimize code hook prompt-submit"), "timeout": 5 }] })); obj.insert( "statusLine".to_string(), json!({ "type": "command", - "command": format!("\"{bin}\" ponytail hook statusline") + "command": format!("\"{bin}\" optimize code hook statusline") }), ); @@ -637,19 +637,19 @@ fn wire_ponytail_claude_code() { &path, serde_json::to_string_pretty(&settings).unwrap() + "\n", ) { - Ok(_) => println!(" ok ponytail hooks wired in ~/.claude/settings.json"), + Ok(_) => println!(" ok optimize code hooks wired in ~/.claude/settings.json"), Err(e) => println!(" fail writing ~/.claude/settings.json: {e}"), } } -fn wire_ponytail_cursor() { +fn wire_optimize_cursor() { let path = cwd().join(".cursor").join("hooks.json"); let bin = agentflare_binary(); if path.exists() { let existing = fs::read_to_string(&path).unwrap_or_default(); - if existing.contains("ponytail") { - println!(" skip ponytail hooks already wired in .cursor/hooks.json"); + if existing.contains("optimize") { + println!(" skip optimize code hooks already wired in .cursor/hooks.json"); return; } } @@ -675,7 +675,7 @@ fn wire_ponytail_cursor() { .as_array_mut() .unwrap() .push(json!({ - "command": format!("\"{bin}\" ponytail hook session-start"), + "command": format!("\"{bin}\" optimize code hook session-start"), "type": "command", "timeout": 30 })); @@ -685,7 +685,7 @@ fn wire_ponytail_cursor() { .as_array_mut() .unwrap() .push(json!({ - "command": format!("\"{bin}\" ponytail hook prompt-submit"), + "command": format!("\"{bin}\" optimize code hook prompt-submit"), "type": "command", "timeout": 10 })); @@ -697,12 +697,12 @@ fn wire_ponytail_cursor() { &path, serde_json::to_string_pretty(&content).unwrap() + "\n", ) { - Ok(_) => println!(" ok ponytail hooks wired in .cursor/hooks.json"), + Ok(_) => println!(" ok optimize code hooks wired in .cursor/hooks.json"), Err(e) => println!(" fail writing .cursor/hooks.json: {e}"), } } -fn wire_ponytail_opencode() { +fn wire_optimize_opencode() { println!(" info OpenCode uses plugin system for hooks, not config."); println!(" Keep @dietrichgebert/ponytail in plugin list."); println!(" The plugin's built-in hooks work alongside agentflare."); diff --git a/src/mcp_prompts.rs b/src/mcp_prompts.rs index 511a6e7b..5e1ac9be 100644 --- a/src/mcp_prompts.rs +++ b/src/mcp_prompts.rs @@ -1,7 +1,7 @@ -//! MCP "Prompts" for ponytail — surfaces `/ponytail*` as native Claude Code +//! MCP "Prompts" for flare code — surfaces `/optimize*` as native Claude Code //! slash commands via the MCP protocol (same mechanism lean-ctx uses for its //! own `/lean-ctx*` commands), routed entirely through agentflare's own -//! ponytail port. No dependency on the DietrichGebert/ponytail marketplace +//! optimize port. No dependency on the DietrichGebert/ponytail marketplace //! plugin. use rmcp::model::{ @@ -20,7 +20,7 @@ const SUB_SKILLS: &[(&str, &str)] = &[ ), ( "debt", - "Harvest `ponytail:` shortcut comments into a tracked ledger", + "Harvest `flare-code:` shortcut comments into a tracked ledger", ), ( "gain", @@ -28,7 +28,7 @@ const SUB_SKILLS: &[(&str, &str)] = &[ ), ( "help", - "Quick-reference card for all ponytail modes, skills, and commands", + "Quick-reference card for all flare code modes, skills, and commands", ), ( "playbook", @@ -43,8 +43,8 @@ const SUB_SKILLS: &[(&str, &str)] = &[ pub fn list_prompts() -> Vec { let mut prompts = vec![ Prompt::new( - "ponytail", - Some("Switch or report Ponytail lazy-dev mode"), + "optimize", + Some("Switch or report flare code lazy-dev mode"), Some(vec![PromptArgument::new("mode") .with_description("lite|full|ultra|off|status (omit to report current mode)")]), ), @@ -66,7 +66,7 @@ pub fn list_prompts() -> Vec { prompts.extend( SUB_SKILLS .iter() - .map(|(name, desc)| Prompt::new(format!("ponytail-{name}"), Some(*desc), None)), + .map(|(name, desc)| Prompt::new(format!("optimize-{name}"), Some(*desc), None)), ); prompts } @@ -81,14 +81,14 @@ pub fn get_prompt( if request.name == "handoff" { return Some(get_handoff_command(request, agent)); } - if request.name == "ponytail" { - return Some(get_ponytail_mode(request)); + if request.name == "optimize" { + return Some(get_optimize_mode(request)); } - let skill = request.name.strip_prefix("ponytail-")?; + let skill = request.name.strip_prefix("optimize-")?; SUB_SKILLS .iter() .any(|(name, _)| *name == skill) - .then(|| get_ponytail_skill(skill)) + .then(|| get_optimize_skill(skill)) } fn assistant_text(msg: impl Into) -> GetPromptResult { @@ -98,7 +98,7 @@ fn assistant_text(msg: impl Into) -> GetPromptResult { )]) } -fn get_ponytail_mode(request: &GetPromptRequestParams) -> GetPromptResult { +fn get_optimize_mode(request: &GetPromptRequestParams) -> GetPromptResult { let mode_arg = request .arguments .as_ref() @@ -112,24 +112,24 @@ fn get_ponytail_mode(request: &GetPromptRequestParams) -> GetPromptResult { let mode = crate::optimize::code::active_mode() .unwrap_or_else(crate::optimize::code::default_mode); return assistant_text(if mode == "off" { - "ponytail is off. Use /ponytail mode=lite|full|ultra to activate.".to_string() + "flare code is off. Use /optimize mode=lite|full|ultra to activate.".to_string() } else { - format!("PONYTAIL MODE ACTIVE — level: {mode}") + format!("FLARE CODE MODE ACTIVE — level: {mode}") }); } if mode_arg == "off" { crate::optimize::code::clear_active(); - return assistant_text("ponytail is now off."); + return assistant_text("flare code is now off."); } match crate::optimize::code::normalize_config_mode(&mode_arg) { Some(normalized) => match crate::optimize::code::set_active(normalized) { Ok(()) => { assistant_text(crate::optimize::code::build_instructions(normalized, None).body) } - Err(e) => assistant_text(format!("Failed to persist ponytail mode: {e}")), + Err(e) => assistant_text(format!("Failed to persist flare code mode: {e}")), }, None => assistant_text(format!( - "Unknown ponytail mode '{mode_arg}'. Use lite|full|ultra|off|status." + "Unknown flare code mode '{mode_arg}'. Use lite|full|ultra|off|status." )), } } @@ -228,9 +228,9 @@ fn get_handoff_command(request: &GetPromptRequestParams, agent: Option<&str>) -> )) } -fn get_ponytail_skill(skill: &str) -> GetPromptResult { +fn get_optimize_skill(skill: &str) -> GetPromptResult { if let Err(e) = crate::optimize::code::set_active(skill) { - return assistant_text(format!("Failed to persist ponytail mode: {e}")); + return assistant_text(format!("Failed to persist flare code mode: {e}")); } let body = crate::optimize::code::sub_skills::get(skill).unwrap_or_default(); assistant_text(body) @@ -241,13 +241,13 @@ mod tests { use super::*; #[test] - fn lists_ponytail_and_all_sub_skills() { + fn lists_optimize_and_all_sub_skills() { let prompts = list_prompts(); let names: Vec<&str> = prompts.iter().map(|p| p.name.as_str()).collect(); - assert!(names.contains(&"ponytail")); - assert!(names.contains(&"ponytail-review")); - assert!(names.contains(&"ponytail-no-hallucination")); - // ponytail + artifact + handoff + one per sub-skill + assert!(names.contains(&"optimize")); + assert!(names.contains(&"optimize-review")); + assert!(names.contains(&"optimize-no-hallucination")); + // optimize + artifact + handoff + one per sub-skill assert_eq!(names.len(), 3 + SUB_SKILLS.len()); } @@ -379,28 +379,28 @@ mod tests { } #[test] - fn ponytail_review_returns_full_skill_body() { - let result = get_prompt(&GetPromptRequestParams::new("ponytail-review"), None).unwrap(); + fn optimize_review_returns_full_skill_body() { + let result = get_prompt(&GetPromptRequestParams::new("optimize-review"), None).unwrap(); let PromptMessage { content, .. } = &result.messages[0]; let text = format!("{content:?}"); - assert!(text.contains("ponytail-review")); + assert!(text.contains("review")); } #[test] - fn bare_ponytail_without_mode_reports_without_crashing() { - let result = get_prompt(&GetPromptRequestParams::new("ponytail"), None).unwrap(); + fn bare_optimize_without_mode_reports_without_crashing() { + let result = get_prompt(&GetPromptRequestParams::new("optimize"), None).unwrap(); assert_eq!(result.messages.len(), 1); } #[test] - fn ponytail_with_unknown_mode_reports_error_text() { + fn optimize_with_unknown_mode_reports_error_text() { use rmcp::model::JsonObject; let mut args = JsonObject::new(); args.insert("mode".to_string(), serde_json::json!("bogus-mode")); - let params = GetPromptRequestParams::new("ponytail").with_arguments(args); + let params = GetPromptRequestParams::new("optimize").with_arguments(args); let result = get_prompt(¶ms, None).unwrap(); let PromptMessage { content, .. } = &result.messages[0]; let text = format!("{content:?}"); - assert!(text.contains("Unknown ponytail mode")); + assert!(text.contains("Unknown flare code mode")); } } diff --git a/src/optimize/code.rs b/src/optimize/code.rs index 10d0e37b..75813f4f 100644 --- a/src/optimize/code.rs +++ b/src/optimize/code.rs @@ -1,4 +1,5 @@ //! Flare code minimalism layer — lazy senior dev rules for AI agents. +pub use flare_code::config::config_path; pub use flare_code::sub_skills::{ SKILL_AUDIT, SKILL_DEBT, SKILL_GAIN, SKILL_HELP, SKILL_NO_HALLUCINATION, SKILL_PLAYBOOK, SKILL_REVIEW, From 2546473cf4a8b8b7cd4525f3388b7e8fa3413de8 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 19:07:13 +0530 Subject: [PATCH 03/13] AGENTS.md: enforce lean-ctx for all code intelligence --- AGENTS.md | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8d38e117..1e274bcd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,15 +29,35 @@ a separate sidecar not in agentflare's read path. ## Context compression — lean-ctx -Prefer [lean-ctx](https://github.com/yvgude/lean-ctx) tools over native equivalents: -read files with its compressed reader, run shell commands through its -compression wrapper, search code with its search/callgraph tools, orient in -unfamiliar code with its composed-context command before exploring. +**MANDATORY for code intelligence — do NOT use native Grep / Read-on-full-file / +shell `cat`/`grep`/`rg`/`find` to search or read code. Route ALL of it through +lean-ctx instead.** lean-ctx is in shadow mode: native file/search/shell calls +auto-route to `ctx_*` — but the rule below is the contract so agents without +shadow routing (Aider, plain shells) still comply. + +- **Code search** → `ctx_search` (action=regex | semantic | symbol), NOT Grep/grep/rg. + - exact symbol: `ctx_search(action=symbol, name=...)` + - by meaning: `ctx_search(action=semantic, query=...)` (uses the on-demand + dense index — no pre-build needed) + - by pattern: `ctx_search(action=regex, pattern=...)` +- **Callers/callees** → `ctx_callgraph` (NOT grep for "who calls X"). +- **Orient in unfamiliar code** → `ctx_compose` FIRST (one call vs + search→read→search chain). +- **Read files** → `ctx_read` (compressed reader), prefer mode=anchored/full. + Recover a compressed read verbatim via `ctx_read mode=raw`. +- **Shell** → `ctx_shell` (auto-compresses output). + +Native `cat`/`grep`/`rg`/`find`/`Read`-whole-file are ONLY for: writing files, +git status/diff you will act on, and non-code text. Everything code-intelligence +goes through lean-ctx so the index stays the single source of truth. ```bash npm install -g lean-ctx-bin && lean-ctx onboard ``` +If `ctx_*` tools are genuinely unavailable in your runtime, fall back to the +native Grep/Read — but that is the exception, and you must say so. + ## Cross-session memory agentflare ships persistent memory in the binary itself — no separate From 24c7a7d0b7bf00f6c33de16ee12f80312300e457 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 21:58:24 +0530 Subject: [PATCH 04/13] fix: correct opencode.json LSP config schema Key: rust (not rust-analyzer). Built-in server, no custom command/extensions needed. Settings go in initialization options. --- opencode.json | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/opencode.json b/opencode.json index 41852d11..5826a88f 100644 --- a/opencode.json +++ b/opencode.json @@ -1,8 +1,7 @@ { "lsp": { - "rust-analyzer": { - "enabled": true, - "settings": { + "rust": { + "initialization": { "rust-analyzer.checkOnSave": true, "rust-analyzer.check.command": "clippy", "rust-analyzer.cargo.features": "all", From 8387bb75e291f846267fecd2258263c1bd701e99 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 22:00:26 +0530 Subject: [PATCH 05/13] fix: add command to built-in rust LSP override Built-in servers need command when overridden in config --- opencode.json | 1 + 1 file changed, 1 insertion(+) diff --git a/opencode.json b/opencode.json index 5826a88f..9c532ab9 100644 --- a/opencode.json +++ b/opencode.json @@ -1,6 +1,7 @@ { "lsp": { "rust": { + "command": ["rust-analyzer"], "initialization": { "rust-analyzer.checkOnSave": true, "rust-analyzer.check.command": "clippy", From 08034c2a3b07c82588d155108bb64d1cedfb6dc7 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 22:42:55 +0530 Subject: [PATCH 06/13] feat(item): add groom action for one-call backlog grooming Adds item action=\groom\: returns a priority+recency-ranked shortlist with full description plus server-computed stale/unassigned/blocked_by/ depended_on_by_count/possible_duplicates/size/unestimated signals and a pull_next list, in one MCP round trip instead of list + N x get. - dependencies_for_items(): bulk dependency-edge query for the shortlist - UpdateItem/item(update) now accepts metadata, so size:S|M|L can be set on existing items (was create-only) - also fixes a latent param_idx bug in item::update where sort_order never advanced the placeholder index, silently reusing it for any field added after it - groom parses metadata.size instead of regexing description prose - /pm:groom, /pm:plan, and the read-recipe/rubric skill docs now call groom directly instead of the old list+get+hand-scoring path - benchmark test (ignored by default) comparing groom vs list+15xget --- .claude/skills/pm/SKILL.md | 40 ++- .claude/skills/pm/reference/read-recipe.md | 19 +- .claude/skills/pm/reference/rubric.md | 7 +- crates/agentflare-backend/src/item.rs | 29 ++ src/mcp_server.rs | 324 ++++++++++++++++++++- src/mcp_server/item.rs | 173 +++++++++++ 6 files changed, 567 insertions(+), 25 deletions(-) diff --git a/.claude/skills/pm/SKILL.md b/.claude/skills/pm/SKILL.md index 86b6b71a..78614a5b 100644 --- a/.claude/skills/pm/SKILL.md +++ b/.claude/skills/pm/SKILL.md @@ -10,8 +10,8 @@ description: Product management for the current agentflare project — run /pm:s These workflows NEVER mutate items. Do not call `item` with any of: create, update, update_state, delete, claim, heartbeat, release, done, cancel, add_label, remove_label — nor `comment` create/edit/delete. You may only read -(`item` list/get/search, `comment` list, `handoff` inbox, `memory`). Output is -suggestions for a human, never actions taken. +(`item` list/get/search/groom, `comment` list, `handoff` inbox, `memory`). +Output is suggestions for a human, never actions taken. All content authored from public PM methodologies (RICE, ICE, MoSCoW, Now/Next/Later). No third-party notices required. @@ -42,24 +42,34 @@ Read-only: never change item state. Arg: staleness threshold in days (default 14). -1. Read open items: `item action="list" state_group="backlog,unstarted"`. -2. Shortlist the top candidates by `priority` (urgent>high>medium>low>none), - cap 15, and `item action="get"` each for description/labels. -3. Score each shortlisted item with reference/rubric.md (RICE, ICE fallback). - Print a ranked table: rank · FIX-NN · name · score · one-line reason. -4. Flag lists (from the full open list, no get needed): - - **Stale**: updated_at older than <threshold> days. - - **Unassigned**: assignee_agent is null. - - **Likely duplicates**: items whose names are near-identical (same key tokens). - - **Unestimated**: no size/effort signal (from the shortlist gets). -5. **Pull next**: top 3 ranked items that are unassigned and not stale. -6. Print the time-signal caveat. Read-only. +1. One call: `item action="groom" state_group="backlog,unstarted" staleness_days= limit=15`. + This replaces the old `list` + N×`get` + hand-computed flags — the server + already returns the shortlist (priority + recency ranked, full description) + with `stale`, `unassigned`, `blocked_by`, `depended_on_by_count`, + `possible_duplicates`, `size`/`unestimated` precomputed per item, plus + `pull_next` and the summary counts. Do not re-derive these by eyeballing + timestamps or text — they're already computed. +2. Score each shortlisted item with reference/rubric.md (RICE using the + returned `size` where present, ICE fallback where `unestimated=true`) — + your judgment is only needed for Reach and Confidence, which the server + can't infer from free text. Print a ranked table: rank · FIX-NN · name · + score · one-line reason. +3. Flag lists — read straight from the response, no recomputation: + - **Stale**: items with `stale=true`. + - **Unassigned**: items with `unassigned=true` (`unassigned_count` for the total). + - **Blocked**: items with non-empty `blocked_by`. + - **Likely duplicates**: items with non-empty `possible_duplicates`. + - **Unestimated**: items with `unestimated=true` (`unestimated_count` for the + total) — recommend adding `metadata={"size":"S"|"M"|"L"}` via `item(update)`. +4. **Pull next**: the response's `pull_next` (top 3 unassigned/not-stale/unblocked + by rank) — cross-check against your RICE ranking and note if they diverge. +5. Print the time-signal caveat. Read-only — `groom` only reads. ### /pm:plan — Now / Next / Later bucketing Arg: capacity hint like "~8" (optional; caps the Now bucket). -1. Reuse the groom ranking (steps 1–3 of /pm:groom). +1. Reuse the groom ranking (steps 1–2 of /pm:groom). 2. Bucket by rank and readiness: - **Now**: highest-ranked items that are ready (have an estimate, not blocked by an open dependency). Cap to the capacity hint if provided. diff --git a/.claude/skills/pm/reference/read-recipe.md b/.claude/skills/pm/reference/read-recipe.md index 775b6010..352de991 100644 --- a/.claude/skills/pm/reference/read-recipe.md +++ b/.claude/skills/pm/reference/read-recipe.md @@ -10,13 +10,24 @@ Call `item` with `action="list"`. Add filters as needed: - `assignee_agent`: matches that agent PLUS unassigned items, open-first. The list projection has ONLY: id, name, state, state_group, priority, -assignee_agent, parent_id, sequence_id, updated_at. +assignee_agent, parent_id, sequence_id, updated_at. Use `list` for +standup/health, which only need this thin projection. + +## Grooming/plan: one call, not list+N×get + +`item action="groom"` (optional `state_group`, `staleness_days` default 14, +`limit` default 15) returns the priority+recency-ranked shortlist with full +description AND precomputed `stale`/`unassigned`/`blocked_by`/ +`depended_on_by_count`/`possible_duplicates`/`size`/`unestimated`, plus +`pull_next` and summary counts — computed server-side in one round trip. +Do not fall back to `list` + per-item `get` for grooming/planning; that was +the old N+1 path this action replaces. ## Detail fetch (only when needed) -`item action="get" id=` returns the full item incl. description, metadata, -labels, timestamps. Grooming/plan fetch detail ONLY for the shortlisted items -(cap at the top 15) to stay bounded. +`item action="get" id=` returns one full item incl. description, metadata, +labels, timestamps — for a single ad-hoc lookup outside grooming, not for +building a shortlist (use `groom` for that). ## Time signals — approximate, state this in output diff --git a/.claude/skills/pm/reference/rubric.md b/.claude/skills/pm/reference/rubric.md index 8858c5f4..f32eada3 100644 --- a/.claude/skills/pm/reference/rubric.md +++ b/.claude/skills/pm/reference/rubric.md @@ -8,8 +8,11 @@ Map each factor to a fixed 1–5 from readable signals. Show the reason inline. `customer`, `revenue`, `priority:high|urgent`. 1 trivial … 5 critical. - Confidence — how well-specified the item is (has a clear description/acceptance). 1 vague … 5 crisp. -- Effort — size. From a `size:S|M|L` label or an estimate in the body. - 1 = large/expensive … 5 = tiny. If unknown, mark UNESTIMATED (see below). +- Effort — size. From `groom`'s `size` field (parsed server-side from + `metadata.size`, set via `item(update)` with `metadata={"size":"S"|"M"|"L"}`). + 1 = large/expensive … 5 = tiny. `groom` sets `unestimated=true` when + `size` is absent — treat that as UNESTIMATED (see below), don't guess a + size from description prose. Print each score as: `RICE 9.6 — R4 I5 C3 / E? (UNESTIMATED)` with one-line why. diff --git a/crates/agentflare-backend/src/item.rs b/crates/agentflare-backend/src/item.rs index 2db61b0e..cfa36b58 100644 --- a/crates/agentflare-backend/src/item.rs +++ b/crates/agentflare-backend/src/item.rs @@ -53,6 +53,7 @@ pub struct UpdateItem { pub state_id: Option, pub assignee_agent: Option, pub sort_order: Option, + pub metadata: Option, } fn now() -> i64 { @@ -247,6 +248,10 @@ pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { } 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", @@ -274,6 +279,9 @@ pub fn update(conn: &Connection, id: &str, input: UpdateItem) -> Result { 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())); @@ -440,6 +448,27 @@ pub fn list_dependencies(conn: &Connection, item_id: &str) -> Result Ok(rows.collect::>()?) } +/// Dependency edges `(item_id, depends_on_item_id)` for a set of items in one +/// query, instead of N `list_dependencies` round trips — used by `groom` to +/// compute blocked/fan-in signals for a whole shortlist at once. +pub fn dependencies_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 item_id, depends_on_item_id FROM item_dependencies WHERE 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)?)) + })?; + 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 diff --git a/src/mcp_server.rs b/src/mcp_server.rs index f16793a8..e7394858 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -556,7 +556,7 @@ fn base64_encode(bytes: &[u8]) -> String { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct ItemRequest { #[schemars( - description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label" + description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom" )] action: String, #[schemars( @@ -586,7 +586,9 @@ struct ItemRequest { )] #[serde(default)] assignee_agent: Option, - #[schemars(description = "Domain-specific fields as a JSON object (create)")] + #[schemars( + description = "Domain-specific fields as a JSON object (create, update). Set {\"size\": \"S\"|\"M\"|\"L\"} so `groom` can score effort instead of reporting the item unestimated." + )] #[serde(default)] metadata: Option, #[schemars(description = "Label IDs to attach on creation (create)")] @@ -614,6 +616,11 @@ struct ItemRequest { #[schemars(description = "FTS5 search query (search)")] #[serde(default)] query: Option, + #[schemars( + description = "Days since updated_at before an item counts as stale (groom); default 14" + )] + #[serde(default)] + staleness_days: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full @@ -632,6 +639,48 @@ struct ItemSummary { updated_at: i64, } +/// One shortlisted item plus the decision-support signals `groom` computes +/// server-side (staleness, blocking, fan-in, near-duplicates) so the caller +/// doesn't have to re-derive them by eyeballing timestamps and free text. +#[derive(Debug, serde::Serialize)] +struct GroomItem { + id: String, + sequence_id: i64, + name: String, + description: String, + state: String, + state_group: String, + priority: String, + assignee_agent: Option, + updated_at: i64, + stale: bool, + unassigned: bool, + /// Parsed from `metadata.size` ("S"|"M"|"L"); `None` when absent — see `unestimated`. + size: Option, + /// True when `metadata.size` is missing — add a size label to enable real RICE scoring. + unestimated: bool, + /// IDs this item depends on that are still open (not completed/cancelled). + blocked_by: Vec, + /// How many other items declare a dependency on this one. + depended_on_by_count: i64, + /// Other shortlisted items with a near-identical name (token-Jaccard ≥ 0.5). + possible_duplicates: Vec, +} + +/// One-call groom result: priority+staleness-ranked shortlist with all the +/// flags a human/agent needs to make pull-next decisions, computed in Rust +/// instead of costing N `get` round trips + manual LLM staleness/dup checks. +#[derive(Debug, serde::Serialize)] +struct GroomResponse { + staleness_days: i64, + stale_count: usize, + unassigned_count: usize, + unestimated_count: usize, + items: Vec, + /// Top unassigned, not-stale, unblocked items from the shortlist. + pull_next: Vec, +} + #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct CommentRequest { #[schemars(description = "Action: create|edit|delete|list")] @@ -2269,9 +2318,10 @@ impl AgentflareMcp { "search" => self.item_search(req), "add_label" => self.item_add_label(req), "remove_label" => self.item_remove_label(req), + "groom" => self.item_groom(req), other => Err(ErrorData::invalid_params( format!( - "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label" + "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom" ), None, )), @@ -2279,7 +2329,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|cancel|add_label|remove_label). See each field's description for when it's required." + 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|cancel|add_label|remove_label|groom). `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. See each field's description for when it's required." )] fn item(&self, Parameters(req): Parameters) -> Result { self.item_inner(req) @@ -4423,6 +4473,272 @@ mod tests { assert_eq!(names, vec!["Open", "Done"]); } + #[test] + fn item_groom_flags_unassigned_and_computes_pull_next() { + let (_tmp, s) = harness(); + let foo: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Foo"))).unwrap()).unwrap(); + s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Bar".into()), + assignee_agent: Some("someone".into()), + ..Default::default() + })) + .unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let items = groomed["items"].as_array().unwrap(); + let foo_entry = items + .iter() + .find(|i| i["name"] == "Foo") + .expect("Foo present"); + assert_eq!(foo_entry["unassigned"], true); + assert_eq!(foo_entry["stale"], false); + let bar_entry = items + .iter() + .find(|i| i["name"] == "Bar") + .expect("Bar present"); + assert_eq!(bar_entry["unassigned"], false); + + let pull_next: Vec<&str> = groomed["pull_next"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert!(pull_next.contains(&foo["id"].as_str().unwrap())); + assert_eq!(groomed["unassigned_count"], 1); + } + + #[test] + fn item_groom_flags_blocked_by_open_dependency() { + let (_tmp, s) = harness(); + let dep: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Dep"))).unwrap()).unwrap(); + let dep_id = dep["id"].as_str().unwrap().to_string(); + let blocked: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Blocked".into()), + dependency_ids: Some(vec![dep_id.clone()]), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let items = groomed["items"].as_array().unwrap(); + let blocked_entry = items + .iter() + .find(|i| i["id"] == blocked["id"]) + .expect("Blocked present"); + let blocked_by: Vec<&str> = blocked_entry["blocked_by"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert_eq!(blocked_by, vec![dep_id.as_str()]); + + let dep_entry = items.iter().find(|i| i["id"] == dep["id"]).unwrap(); + assert_eq!(dep_entry["depended_on_by_count"], 1); + + let pull_next: Vec<&str> = groomed["pull_next"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert!(!pull_next.contains(&blocked["id"].as_str().unwrap())); + } + + #[test] + fn item_groom_detects_near_duplicate_names() { + let (_tmp, s) = harness(); + let a: serde_json::Value = serde_json::from_str( + &s.item(Parameters(empty_item_create( + "FIX-08 backlog low unassigned stale", + ))) + .unwrap(), + ) + .unwrap(); + let b: serde_json::Value = serde_json::from_str( + &s.item(Parameters(empty_item_create( + "FIX-09 backlog low unassigned stale duplicateish", + ))) + .unwrap(), + ) + .unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let items = groomed["items"].as_array().unwrap(); + let a_entry = items.iter().find(|i| i["id"] == a["id"]).unwrap(); + let dups: Vec<&str> = a_entry["possible_duplicates"] + .as_array() + .unwrap() + .iter() + .map(|v| v.as_str().unwrap()) + .collect(); + assert!(dups.contains(&b["id"].as_str().unwrap())); + } + + #[test] + fn item_update_sets_metadata() { + let (_tmp, s) = harness(); + let created: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Sized"))).unwrap()) + .unwrap(); + let updated: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "update".into(), + id: Some(created["id"].as_str().unwrap().to_string()), + metadata: Some(serde_json::json!({"size": "M"})), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + assert_eq!(updated["metadata"], serde_json::json!({"size": "M"}).to_string()); + } + + #[test] + fn item_groom_reads_size_and_flags_unestimated() { + let (_tmp, s) = harness(); + let sized: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Sized".into()), + metadata: Some(serde_json::json!({"size": "L"})), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let bare: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Bare"))).unwrap()).unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let items = groomed["items"].as_array().unwrap(); + let sized_entry = items.iter().find(|i| i["id"] == sized["id"]).unwrap(); + assert_eq!(sized_entry["size"], "L"); + assert_eq!(sized_entry["unestimated"], false); + let bare_entry = items.iter().find(|i| i["id"] == bare["id"]).unwrap(); + assert_eq!(bare_entry["size"], serde_json::Value::Null); + assert_eq!(bare_entry["unestimated"], true); + assert_eq!(groomed["unestimated_count"], 1); + } + + /// 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 + .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(); diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 8edeb60b..d45ac37d 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -153,6 +153,7 @@ impl AgentflareMcp { state_id: None, assignee_agent: req.assignee_agent, sort_order: None, + metadata: req.metadata.map(|v| v.to_string()), }; let item = agentflare_backend::item::update(conn, &id, input).map_err(map_backend_err)?; @@ -445,4 +446,176 @@ impl AgentflareMcp { ) })? } + + /// One-call groom: filtered + priority/staleness-ranked shortlist with + /// full description plus stale/unassigned/blocked/duplicate signals + /// computed server-side. Replaces the `list` + N×`get` round trips a + /// manual groom otherwise costs. + pub(super) fn item_groom(&self, req: ItemRequest) -> Result { + if req.limit.is_some_and(|l| l < 0) { + return Err(ErrorData::invalid_params("limit must be non-negative", None)); + } + let staleness_days = req.staleness_days.unwrap_or(14).max(0); + let cap = req.limit.unwrap_or(15).max(0) as usize; + self.with_backend_db(|conn| { + let project = self.resolve_project(conn)?; + 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) + .map_err(map_backend_err)?; + let state_by_id: std::collections::HashMap<&str, &agentflare_backend::state::State> = + states.iter().map(|s| (s.id.as_str(), s)).collect(); + + let wanted_groups: Vec<&str> = req + .state_group + .as_deref() + .unwrap_or("backlog,unstarted") + .split(',') + .map(str::trim) + .collect(); + items.retain(|i| { + state_by_id + .get(i.state_id.as_str()) + .map(|s| wanted_groups.contains(&s.group_name.as_str())) + .unwrap_or(false) + }); + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let stale_cutoff = now - staleness_days.saturating_mul(86_400); + + fn priority_rank(p: &str) -> u8 { + match p { + "urgent" => 5, + "high" => 4, + "medium" => 3, + "low" => 2, + _ => 1, + } + } + // Priority first, then most-recently-touched within a priority tier. + items.sort_by(|a, b| { + priority_rank(&b.priority) + .cmp(&priority_rank(&a.priority)) + .then(b.updated_at.cmp(&a.updated_at)) + }); + let shortlist: Vec<_> = items.into_iter().take(cap).collect(); + + let ids: Vec = shortlist.iter().map(|i| i.id.clone()).collect(); + let edges = agentflare_backend::item::dependencies_for_items(conn, &ids) + .map_err(map_backend_err)?; + let group_of = |id: &str| -> &str { + shortlist + .iter() + .find(|i| i.id == id) + .and_then(|i| state_by_id.get(i.state_id.as_str())) + .map(|s| s.group_name.as_str()) + .unwrap_or("") + }; + let mut fanin: std::collections::HashMap = std::collections::HashMap::new(); + let mut blocked_by: std::collections::HashMap> = + std::collections::HashMap::new(); + for (item_id, depends_on) in &edges { + *fanin.entry(depends_on.clone()).or_insert(0) += 1; + if !matches!(group_of(depends_on), "completed" | "cancelled") { + blocked_by + .entry(item_id.clone()) + .or_default() + .push(depends_on.clone()); + } + } + + // Near-duplicate names within the shortlist (token-Jaccard, no + // embeddings needed at this backlog scale). + fn name_tokens(name: &str) -> std::collections::HashSet { + name.to_lowercase() + .split(|c: char| !c.is_alphanumeric()) + .filter(|s| s.len() > 2) + .map(str::to_string) + .collect() + } + let token_sets: Vec<_> = shortlist.iter().map(|i| name_tokens(&i.name)).collect(); + let mut duplicates: std::collections::HashMap> = + std::collections::HashMap::new(); + for a in 0..shortlist.len() { + for b in (a + 1)..shortlist.len() { + let (sa, sb) = (&token_sets[a], &token_sets[b]); + if sa.is_empty() || sb.is_empty() { + continue; + } + let inter = sa.intersection(sb).count() as f64; + let union = sa.union(sb).count() as f64; + if union > 0.0 && inter / union >= 0.5 { + duplicates + .entry(shortlist[a].id.clone()) + .or_default() + .push(shortlist[b].id.clone()); + duplicates + .entry(shortlist[b].id.clone()) + .or_default() + .push(shortlist[a].id.clone()); + } + } + } + + // `size` lives in the free-form `metadata` JSON blob (`{"size": "S"|"M"|"L"}`) + // rather than a regex over description prose — sets via `item(update)`. + fn parsed_size(metadata: &str) -> Option { + serde_json::from_str::(metadata) + .ok()? + .get("size")? + .as_str() + .filter(|s| matches!(*s, "S" | "M" | "L")) + .map(str::to_string) + } + + let groom_items: Vec = shortlist + .into_iter() + .map(|i| { + let state = state_by_id.get(i.state_id.as_str()); + let stale = i.updated_at < stale_cutoff; + let unassigned = i.assignee_agent.is_none(); + let size = parsed_size(&i.metadata); + let unestimated = size.is_none(); + GroomItem { + blocked_by: blocked_by.get(&i.id).cloned().unwrap_or_default(), + depended_on_by_count: *fanin.get(&i.id).unwrap_or(&0), + possible_duplicates: duplicates.get(&i.id).cloned().unwrap_or_default(), + id: i.id, + sequence_id: i.sequence_id, + name: i.name, + description: i.description, + state: state.map(|s| s.name.clone()).unwrap_or_default(), + state_group: state.map(|s| s.group_name.clone()).unwrap_or_default(), + priority: i.priority, + assignee_agent: i.assignee_agent, + updated_at: i.updated_at, + stale, + unassigned, + size, + unestimated, + } + }) + .collect(); + + let pull_next: Vec = groom_items + .iter() + .filter(|i| i.unassigned && !i.stale && i.blocked_by.is_empty()) + .take(3) + .map(|i| i.id.clone()) + .collect(); + + let resp = GroomResponse { + staleness_days, + stale_count: groom_items.iter().filter(|i| i.stale).count(), + unassigned_count: groom_items.iter().filter(|i| i.unassigned).count(), + unestimated_count: groom_items.iter().filter(|i| i.unestimated).count(), + items: groom_items, + pull_next, + }; + Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) + })? + } } From 69cb0d6f8ee9bad407eabe5f44d790da6d341be6 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 23:02:26 +0530 Subject: [PATCH 07/13] fix(item): tolerate double-encoded metadata in groom size parsing Dogfooding item(create) with metadata={"size":"S"} via a live MCP call stored a JSON string containing JSON instead of the object itself, so groom's parsed_size() silently reported these items as unestimated. parsed_size() now unwraps one extra string-encoding layer before giving up. Regression test reproduces the exact stored shape. --- src/mcp_server.rs | 40 ++++++++++++++++++++++++++++++++++++++++ src/mcp_server/item.rs | 13 +++++++++++-- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/mcp_server.rs b/src/mcp_server.rs index e7394858..3b38f6e6 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -4661,6 +4661,46 @@ mod tests { assert_eq!(groomed["unestimated_count"], 1); } + /// Regression: some callers double-encode an object-typed `metadata` param + /// as a JSON string containing JSON — reproduced live via item(create) + /// with metadata={"size":"S"}, which stored `"{\"size\": \"S\"}"` (a + /// string) rather than the object itself. `groom` must still read `size` + /// through that extra layer instead of silently reporting `unestimated`. + #[test] + fn item_groom_reads_size_through_double_encoded_metadata() { + let (_tmp, s) = harness(); + let double_encoded: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Double-encoded".into()), + metadata: Some(serde_json::Value::String( + serde_json::json!({"size": "M"}).to_string(), + )), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + let entry = groomed["items"] + .as_array() + .unwrap() + .iter() + .find(|i| i["id"] == double_encoded["id"]) + .unwrap(); + assert_eq!(entry["size"], "M"); + assert_eq!(entry["unestimated"], false); + } + /// 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 diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index d45ac37d..2fff486f 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -563,8 +563,17 @@ impl AgentflareMcp { // `size` lives in the free-form `metadata` JSON blob (`{"size": "S"|"M"|"L"}`) // rather than a regex over description prose — sets via `item(update)`. fn parsed_size(metadata: &str) -> Option { - serde_json::from_str::(metadata) - .ok()? + let mut value = serde_json::from_str::(metadata).ok()?; + // Defensive: some callers double-encode an object-typed param as a + // JSON string containing JSON (observed live — item(create) with + // metadata={"size":"S"} stored `"{\"size\": \"S\"}"` instead of the + // object). Unwrap one extra layer before giving up. + if let serde_json::Value::String(inner) = &value + && let Ok(reparsed) = serde_json::from_str::(inner) + { + value = reparsed; + } + value .get("size")? .as_str() .filter(|s| matches!(*s, "S" | "M" | "L")) From 8b690c88d2607d339f94df6931b2b7ea714331d9 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 23:10:28 +0530 Subject: [PATCH 08/13] feat(item): add capacity-based Now/Next/Later bucketing to groom Extends item action="groom" with an optional `capacity` param that additionally buckets the shortlist into now/next/later/needs_estimation, reusing the rank/blocked_by/unestimated signals groom already computes. Omitted from the response when capacity is unset (backward compatible). Extracted priority_rank/parsed_size/dependency_signals/near_duplicates/ capacity_buckets into named module-level functions in item.rs - the handler was accreting cognitive complexity with every addition and this keeps it as an orchestration function. /pm:plan now calls groom(capacity=N) directly instead of re-bucketing groom's shortlist itself. --- .claude/skills/pm/SKILL.md | 18 ++-- src/mcp_server.rs | 92 ++++++++++++++++ src/mcp_server/item.rs | 210 +++++++++++++++++++++++-------------- 3 files changed, 235 insertions(+), 85 deletions(-) diff --git a/.claude/skills/pm/SKILL.md b/.claude/skills/pm/SKILL.md index 78614a5b..a9a4b2ec 100644 --- a/.claude/skills/pm/SKILL.md +++ b/.claude/skills/pm/SKILL.md @@ -69,15 +69,15 @@ Arg: staleness threshold in days (default 14). Arg: capacity hint like "~8" (optional; caps the Now bucket). -1. Reuse the groom ranking (steps 1–2 of /pm:groom). -2. Bucket by rank and readiness: - - **Now**: highest-ranked items that are ready (have an estimate, not blocked - by an open dependency). Cap to the capacity hint if provided. - - **Next**: next tier by rank. - - **Later**: the tail + anything low-confidence. -3. Separately list **Needs estimation** (unestimated items) — cannot be planned. -4. Print each bucket as an ordered list of `FIX-NN · name · score`. -5. Print the time-signal caveat. Read-only — this proposes a plan, it does not +1. One call: `item action="groom" state_group="backlog,unstarted" capacity=`. + The server does the bucketing: `now` (top-`capacity` ready items — unblocked, + has a `size`), `next` (remaining ready items), `later` (blocked items), + `needs_estimation` (unestimated — excluded from planning). No hand-bucketing. +2. Score each item with reference/rubric.md for the printed rationale (RICE + using `size`, ICE fallback for `unestimated` ones) — your judgment covers + Reach/Confidence, the buckets themselves are already computed. +3. Print each bucket as an ordered list of `FIX-NN · name · score`. +4. Print the time-signal caveat. Read-only — this proposes a plan, it does not assign or move items. ### /pm:health — team health scorecard diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 3b38f6e6..b2b1c5f4 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -621,6 +621,11 @@ struct ItemRequest { )] #[serde(default)] staleness_days: Option, + #[schemars( + description = "Now-bucket size (groom only) — when set, additionally buckets the shortlist into now/next/later/needs_estimation for sprint planning" + )] + #[serde(default)] + capacity: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full @@ -679,6 +684,16 @@ struct GroomResponse { items: Vec, /// Top unassigned, not-stale, unblocked items from the shortlist. pull_next: Vec, + /// Only populated when the `capacity` request field is set. + #[serde(skip_serializing_if = "Option::is_none")] + now: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + next: Option>, + #[serde(skip_serializing_if = "Option::is_none")] + later: Option>, + /// Unestimated items — excluded from now/next/later, can't be planned yet. + #[serde(skip_serializing_if = "Option::is_none")] + needs_estimation: Option>, } #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] @@ -4701,6 +4716,83 @@ mod tests { 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); + } + /// 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 diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 2fff486f..258caf11 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -6,6 +6,122 @@ use super::*; +fn priority_rank(p: &str) -> u8 { + match p { + "urgent" => 5, + "high" => 4, + "medium" => 3, + "low" => 2, + _ => 1, + } +} + +/// `size` lives in the free-form `metadata` JSON blob (`{"size": "S"|"M"|"L"}`) +/// rather than a regex over description prose — sets via `item(update)`. +fn parsed_size(metadata: &str) -> Option { + let mut value = serde_json::from_str::(metadata).ok()?; + // Defensive: some callers double-encode an object-typed param as a JSON + // string containing JSON (observed live — item(create) with + // metadata={"size":"S"} stored `"{\"size\": \"S\"}"` instead of the + // object). Unwrap one extra layer before giving up. + if let serde_json::Value::String(inner) = &value + && let Ok(reparsed) = serde_json::from_str::(inner) + { + value = reparsed; + } + value + .get("size")? + .as_str() + .filter(|s| matches!(*s, "S" | "M" | "L")) + .map(str::to_string) +} + +/// Fan-in count and open-dependency blocking per item, from a flat edge list. +fn dependency_signals<'a>( + edges: &[(String, String)], + state_group_of: impl Fn(&str) -> &'a str, +) -> ( + std::collections::HashMap, + std::collections::HashMap>, +) { + let mut fanin: std::collections::HashMap = std::collections::HashMap::new(); + let mut blocked_by: std::collections::HashMap> = + std::collections::HashMap::new(); + for (item_id, depends_on) in edges { + *fanin.entry(depends_on.clone()).or_insert(0) += 1; + if !matches!(state_group_of(depends_on), "completed" | "cancelled") { + blocked_by + .entry(item_id.clone()) + .or_default() + .push(depends_on.clone()); + } + } + (fanin, blocked_by) +} + +/// Near-duplicate names within a shortlist (token-Jaccard ≥ 0.5) — no +/// embeddings needed at this backlog scale. +fn near_duplicates( + shortlist: &[agentflare_backend::item::Item], +) -> std::collections::HashMap> { + fn name_tokens(name: &str) -> std::collections::HashSet { + name.to_lowercase() + .split(|c: char| !c.is_alphanumeric()) + .filter(|s| s.len() > 2) + .map(str::to_string) + .collect() + } + let token_sets: Vec<_> = shortlist.iter().map(|i| name_tokens(&i.name)).collect(); + let mut duplicates: std::collections::HashMap> = + std::collections::HashMap::new(); + for a in 0..shortlist.len() { + for b in (a + 1)..shortlist.len() { + let (sa, sb) = (&token_sets[a], &token_sets[b]); + if sa.is_empty() || sb.is_empty() { + continue; + } + let inter = sa.intersection(sb).count() as f64; + let union = sa.union(sb).count() as f64; + if union > 0.0 && inter / union >= 0.5 { + duplicates + .entry(shortlist[a].id.clone()) + .or_default() + .push(shortlist[b].id.clone()); + duplicates + .entry(shortlist[b].id.clone()) + .or_default() + .push(shortlist[a].id.clone()); + } + } + } + duplicates +} + +/// Now/Next/Later planning buckets. Unestimated items are excluded outright +/// (can't be planned without a size); of the rest, blocked items go to +/// `later`, and ready items split into `now` (first `capacity`, in existing +/// rank order) and `next` (the remainder). +fn capacity_buckets( + items: &[GroomItem], + capacity: i64, +) -> (Vec, Vec, Vec, Vec) { + let capacity = capacity.max(0) as usize; + let mut needs_estimation = Vec::new(); + let mut later = Vec::new(); + let mut ready = Vec::new(); + for i in items { + if i.unestimated { + needs_estimation.push(i.id.clone()); + } else if !i.blocked_by.is_empty() { + later.push(i.id.clone()); + } else { + ready.push(i.id.clone()); + } + } + let next = ready.split_off(capacity.min(ready.len())); + (ready, next, later, needs_estimation) +} + impl AgentflareMcp { pub(super) fn item_create(&self, req: ItemRequest) -> Result { let name = req @@ -486,15 +602,6 @@ impl AgentflareMcp { .unwrap_or(0); let stale_cutoff = now - staleness_days.saturating_mul(86_400); - fn priority_rank(p: &str) -> u8 { - match p { - "urgent" => 5, - "high" => 4, - "medium" => 3, - "low" => 2, - _ => 1, - } - } // Priority first, then most-recently-touched within a priority tier. items.sort_by(|a, b| { priority_rank(&b.priority) @@ -506,79 +613,15 @@ impl AgentflareMcp { let ids: Vec = shortlist.iter().map(|i| i.id.clone()).collect(); let edges = agentflare_backend::item::dependencies_for_items(conn, &ids) .map_err(map_backend_err)?; - let group_of = |id: &str| -> &str { + let (fanin, blocked_by) = dependency_signals(&edges, |id| { shortlist .iter() .find(|i| i.id == id) .and_then(|i| state_by_id.get(i.state_id.as_str())) .map(|s| s.group_name.as_str()) .unwrap_or("") - }; - let mut fanin: std::collections::HashMap = std::collections::HashMap::new(); - let mut blocked_by: std::collections::HashMap> = - std::collections::HashMap::new(); - for (item_id, depends_on) in &edges { - *fanin.entry(depends_on.clone()).or_insert(0) += 1; - if !matches!(group_of(depends_on), "completed" | "cancelled") { - blocked_by - .entry(item_id.clone()) - .or_default() - .push(depends_on.clone()); - } - } - - // Near-duplicate names within the shortlist (token-Jaccard, no - // embeddings needed at this backlog scale). - fn name_tokens(name: &str) -> std::collections::HashSet { - name.to_lowercase() - .split(|c: char| !c.is_alphanumeric()) - .filter(|s| s.len() > 2) - .map(str::to_string) - .collect() - } - let token_sets: Vec<_> = shortlist.iter().map(|i| name_tokens(&i.name)).collect(); - let mut duplicates: std::collections::HashMap> = - std::collections::HashMap::new(); - for a in 0..shortlist.len() { - for b in (a + 1)..shortlist.len() { - let (sa, sb) = (&token_sets[a], &token_sets[b]); - if sa.is_empty() || sb.is_empty() { - continue; - } - let inter = sa.intersection(sb).count() as f64; - let union = sa.union(sb).count() as f64; - if union > 0.0 && inter / union >= 0.5 { - duplicates - .entry(shortlist[a].id.clone()) - .or_default() - .push(shortlist[b].id.clone()); - duplicates - .entry(shortlist[b].id.clone()) - .or_default() - .push(shortlist[a].id.clone()); - } - } - } - - // `size` lives in the free-form `metadata` JSON blob (`{"size": "S"|"M"|"L"}`) - // rather than a regex over description prose — sets via `item(update)`. - fn parsed_size(metadata: &str) -> Option { - let mut value = serde_json::from_str::(metadata).ok()?; - // Defensive: some callers double-encode an object-typed param as a - // JSON string containing JSON (observed live — item(create) with - // metadata={"size":"S"} stored `"{\"size\": \"S\"}"` instead of the - // object). Unwrap one extra layer before giving up. - if let serde_json::Value::String(inner) = &value - && let Ok(reparsed) = serde_json::from_str::(inner) - { - value = reparsed; - } - value - .get("size")? - .as_str() - .filter(|s| matches!(*s, "S" | "M" | "L")) - .map(str::to_string) - } + }); + let duplicates = near_duplicates(&shortlist); let groom_items: Vec = shortlist .into_iter() @@ -616,6 +659,17 @@ impl AgentflareMcp { .map(|i| i.id.clone()) .collect(); + // Only computed when `capacity` is set — omitted from the response + // otherwise (backward compatible). + let (now, next, later, needs_estimation) = match req.capacity { + Some(capacity) => { + let (now, next, later, needs_estimation) = + capacity_buckets(&groom_items, capacity); + (Some(now), Some(next), Some(later), Some(needs_estimation)) + } + None => (None, None, None, None), + }; + let resp = GroomResponse { staleness_days, stale_count: groom_items.iter().filter(|i| i.stale).count(), @@ -623,6 +677,10 @@ impl AgentflareMcp { unestimated_count: groom_items.iter().filter(|i| i.unestimated).count(), items: groom_items, pull_next, + now, + next, + later, + needs_estimation, }; Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) })? From 65d5670a5786b403f640c8d3df37ccd5d8386dab Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 23:15:55 +0530 Subject: [PATCH 09/13] feat(item): add standup action for server-side daily digest bucketing Adds item action="standup": returns done (completed within cutoff_hours, default 24)/in_progress (grouped by assignee, "unassigned" as its own group)/stuck (in-progress older than staleness_days, default 7) computed server-side from one state-filtered read, instead of the caller bucketing a flat list result by hand. /pm:standup and the read-recipe skill doc now call it directly. --- .claude/skills/pm/SKILL.md | 16 +-- .claude/skills/pm/reference/read-recipe.md | 10 +- src/mcp_server.rs | 123 ++++++++++++++++++++- src/mcp_server/item.rs | 95 ++++++++++++++++ 4 files changed, 231 insertions(+), 13 deletions(-) diff --git a/.claude/skills/pm/SKILL.md b/.claude/skills/pm/SKILL.md index a9a4b2ec..e367044b 100644 --- a/.claude/skills/pm/SKILL.md +++ b/.claude/skills/pm/SKILL.md @@ -10,7 +10,7 @@ description: Product management for the current agentflare project — run /pm:s These workflows NEVER mutate items. Do not call `item` with any of: create, update, update_state, delete, claim, heartbeat, release, done, cancel, add_label, remove_label — nor `comment` create/edit/delete. You may only read -(`item` list/get/search/groom, `comment` list, `handoff` inbox, `memory`). +(`item` list/get/search/groom/standup, `comment` list, `handoff` inbox, `memory`). Output is suggestions for a human, never actions taken. All content authored from public PM methodologies (RICE, ICE, MoSCoW, Now/Next/Later). No third-party notices required. @@ -29,13 +29,13 @@ health additionally use `reference/rubric.md`. Arg: cutoff (default: items with `updated_at` within the last 24h). -1. Read items: `item action="list" state_group="started,completed"`. -2. Bucket: - - **Done** — state_group=completed, updated_at ≥ cutoff. - - **In progress** — state_group=started (all), grouped by assignee_agent. - - **Stuck** — in-progress items whose updated_at is older than 7 days. -3. For each item print `FIX-NN · · `. -4. Print the read-recipe time-signal caveat. +1. One call: `item action="standup" cutoff_hours=`. The + server returns `done` (completed within cutoff_hours), `in_progress` + (grouped by assignee, "unassigned" as its own group), and `stuck` + (in-progress items older than `staleness_days`, default 7) — already + bucketed, no hand-sorting a flat `list` result. +2. For each item print `FIX-NN · · `. +3. Print the read-recipe time-signal caveat. Read-only: never change item state. ### /pm:groom — backlog grooming + prioritization diff --git a/.claude/skills/pm/reference/read-recipe.md b/.claude/skills/pm/reference/read-recipe.md index 352de991..8c7d54de 100644 --- a/.claude/skills/pm/reference/read-recipe.md +++ b/.claude/skills/pm/reference/read-recipe.md @@ -10,8 +10,14 @@ Call `item` with `action="list"`. Add filters as needed: - `assignee_agent`: matches that agent PLUS unassigned items, open-first. The list projection has ONLY: id, name, state, state_group, priority, -assignee_agent, parent_id, sequence_id, updated_at. Use `list` for -standup/health, which only need this thin projection. +assignee_agent, parent_id, sequence_id, updated_at. + +## Standup: one call, not list+bucket + +`item action="standup"` (optional `cutoff_hours` default 24, `staleness_days` +default 7 for the stuck threshold) returns `done`/`in_progress` (grouped by +assignee)/`stuck` pre-bucketed server-side, plus counts. Use this instead of +`list` + hand-sorting for standup. ## Grooming/plan: one call, not list+N×get diff --git a/src/mcp_server.rs b/src/mcp_server.rs index b2b1c5f4..0e28fb03 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -556,7 +556,7 @@ fn base64_encode(bytes: &[u8]) -> String { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct ItemRequest { #[schemars( - description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom" + description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup" )] action: String, #[schemars( @@ -626,6 +626,11 @@ struct ItemRequest { )] #[serde(default)] capacity: Option, + #[schemars( + description = "Hours back a completed item counts as \"done\" (standup); default 24" + )] + #[serde(default)] + cutoff_hours: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full @@ -696,6 +701,37 @@ struct GroomResponse { needs_estimation: Option>, } +/// Lean per-item row for `standup` — no description, matches `ItemSummary`'s +/// thin-projection philosophy since standup doesn't need item bodies. +#[derive(Debug, serde::Serialize)] +struct StandupItem { + id: String, + sequence_id: i64, + name: String, + priority: String, + assignee_agent: Option, + updated_at: i64, +} + +#[derive(Debug, serde::Serialize)] +struct StandupGroup { + /// The literal string "unassigned" when `assignee_agent` is null. + assignee: String, + items: Vec, +} + +#[derive(Debug, serde::Serialize)] +struct StandupResponse { + cutoff_hours: i64, + stuck_days: i64, + done: Vec, + done_count: usize, + in_progress: Vec, + in_progress_count: usize, + stuck: Vec, + stuck_count: usize, +} + #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct CommentRequest { #[schemars(description = "Action: create|edit|delete|list")] @@ -2334,9 +2370,10 @@ impl AgentflareMcp { "add_label" => self.item_add_label(req), "remove_label" => self.item_remove_label(req), "groom" => self.item_groom(req), + "standup" => self.item_standup(req), other => Err(ErrorData::invalid_params( format!( - "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom" + "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup" ), None, )), @@ -2344,7 +2381,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|cancel|add_label|remove_label|groom). `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. See each field's description for when it's required." + 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|cancel|add_label|remove_label|groom|standup). `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. See each field's description for when it's required." )] fn item(&self, Parameters(req): Parameters) -> Result { self.item_inner(req) @@ -4793,6 +4830,86 @@ mod tests { assert_eq!(needs_est, expected); } + #[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); + } + /// 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 diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 258caf11..b9b46522 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -685,4 +685,99 @@ impl AgentflareMcp { Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) })? } + + /// One-call standup: done/in-progress(grouped by assignee)/stuck, computed + /// server-side from a single state-filtered read instead of the caller + /// bucketing a flat `list` result by hand. + pub(super) fn item_standup(&self, req: ItemRequest) -> Result { + 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 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) + .map_err(map_backend_err)?; + let state_by_id: std::collections::HashMap<&str, &agentflare_backend::state::State> = + states.iter().map(|s| (s.id.as_str(), s)).collect(); + items.retain(|i| { + state_by_id + .get(i.state_id.as_str()) + .map(|s| matches!(s.group_name.as_str(), "started" | "completed")) + .unwrap_or(false) + }); + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let done_cutoff = now - cutoff_hours.saturating_mul(3_600); + let stuck_cutoff = now - stuck_days.saturating_mul(86_400); + + fn to_standup_item(i: &agentflare_backend::item::Item) -> StandupItem { + StandupItem { + id: i.id.clone(), + sequence_id: i.sequence_id, + name: i.name.clone(), + priority: i.priority.clone(), + assignee_agent: i.assignee_agent.clone(), + updated_at: i.updated_at, + } + } + + let mut done: Vec = items + .iter() + .filter(|i| { + state_by_id + .get(i.state_id.as_str()) + .map(|s| s.group_name == "completed") + .unwrap_or(false) + && i.updated_at >= done_cutoff + }) + .map(to_standup_item) + .collect(); + done.sort_by_key(|i| std::cmp::Reverse(i.updated_at)); + + let in_progress_items: Vec<_> = items + .iter() + .filter(|i| { + state_by_id + .get(i.state_id.as_str()) + .map(|s| s.group_name == "started") + .unwrap_or(false) + }) + .collect(); + + let stuck: Vec = in_progress_items + .iter() + .filter(|i| i.updated_at < stuck_cutoff) + .map(|i| to_standup_item(i)) + .collect(); + + let mut by_assignee: std::collections::BTreeMap> = + std::collections::BTreeMap::new(); + for i in &in_progress_items { + by_assignee + .entry(i.assignee_agent.clone().unwrap_or_else(|| "unassigned".into())) + .or_default() + .push(to_standup_item(i)); + } + let in_progress: Vec = by_assignee + .into_iter() + .map(|(assignee, items)| StandupGroup { assignee, items }) + .collect(); + + let resp = StandupResponse { + cutoff_hours, + stuck_days, + done_count: done.len(), + done, + in_progress_count: in_progress_items.len(), + in_progress, + stuck_count: stuck.len(), + stuck, + }; + Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) + })? + } } From db2234ab785ee8b0593c1df7f04753c067db0953 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 23:23:21 +0530 Subject: [PATCH 10/13] feat(item): add health action for velocity/WIP/stuck scorecard Adds item action="health": trailing weekly velocity series (oldest to newest) with an up/down/flat trend, WIP list+count, stuck items (WIP older than staleness_days, default 7), and a bottlenecks field. Velocity is a live scan over list_by_project, not a precomputed/event- populated rollup table: events::emit (agentflare-backend/src/events.rs) turned out to be 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 migration work; at this project's actual scale a live scan is sub-millisecond (see the groom benchmark), so that infrastructure would be speculative today. bottlenecks is therefore always empty, with bottleneck_note explaining why, matching the skill's own documented "if none available, print no handoff history" fallback. /pm:health now calls the action directly instead of hand-computing the weekly buckets. --- .claude/skills/pm/SKILL.md | 21 ++++--- src/mcp_server.rs | 99 +++++++++++++++++++++++++++-- src/mcp_server/item.rs | 123 +++++++++++++++++++++++++++++++++---- 3 files changed, 218 insertions(+), 25 deletions(-) diff --git a/.claude/skills/pm/SKILL.md b/.claude/skills/pm/SKILL.md index e367044b..6bdac01d 100644 --- a/.claude/skills/pm/SKILL.md +++ b/.claude/skills/pm/SKILL.md @@ -10,7 +10,7 @@ description: Product management for the current agentflare project — run /pm:s These workflows NEVER mutate items. Do not call `item` with any of: create, update, update_state, delete, claim, heartbeat, release, done, cancel, add_label, remove_label — nor `comment` create/edit/delete. You may only read -(`item` list/get/search/groom/standup, `comment` list, `handoff` inbox, `memory`). +(`item` list/get/search/groom/standup/health, `comment` list, `handoff` inbox, `memory`). Output is suggestions for a human, never actions taken. All content authored from public PM methodologies (RICE, ICE, MoSCoW, Now/Next/Later). No third-party notices required. @@ -84,12 +84,13 @@ Arg: capacity hint like "~8" (optional; caps the Now bucket). Arg: window in weeks (default 4). -1. Velocity: `item action="list" state_group="completed"`; per rubric.md, count - items whose `updated_at` falls in each trailing 7-day window; show the series - and the trend arrow. -2. WIP: `item action="list" state_group="started"`; report the count and list. -3. Stuck: WIP items with `updated_at` older than 7 days. -4. Bottlenecks: read `handoff` history (read-only) for items handed off - repeatedly; if none available, print "no handoff history". -5. One-glance scorecard: Velocity · WIP · Stuck · Bottlenecks. -6. Print the time-signal caveat. Read-only. +1. One call: `item action="health" window_weeks=`. The server + 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. +3. One-glance scorecard: Velocity · WIP · Stuck · Bottlenecks. +4. Print the time-signal caveat. Read-only. diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 0e28fb03..1277ca3f 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -556,7 +556,7 @@ fn base64_encode(bytes: &[u8]) -> String { #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct ItemRequest { #[schemars( - description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup" + description = "Action: create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup|health" )] action: String, #[schemars( @@ -631,6 +631,9 @@ struct ItemRequest { )] #[serde(default)] cutoff_hours: Option, + #[schemars(description = "Trailing weekly windows for velocity (health); default 4")] + #[serde(default)] + window_weeks: Option, } /// Lean per-item projection for `item(list)` — the raw 19-field `Item` (full @@ -703,7 +706,7 @@ struct GroomResponse { /// Lean per-item row for `standup` — no description, matches `ItemSummary`'s /// thin-projection philosophy since standup doesn't need item bodies. -#[derive(Debug, serde::Serialize)] +#[derive(Debug, Clone, serde::Serialize)] struct StandupItem { id: String, sequence_id: i64, @@ -732,6 +735,31 @@ struct StandupResponse { stuck_count: usize, } +#[derive(Debug, serde::Serialize)] +struct VelocityWeek { + week_start: i64, + week_end: i64, + completed_count: usize, +} + +#[derive(Debug, serde::Serialize)] +struct HealthResponse { + window_weeks: i64, + /// Oldest → newest. + velocity: Vec, + /// "up" | "down" | "flat" — last window vs. the one before it. + velocity_trend: String, + wip_count: usize, + wip: Vec, + stuck_days: i64, + stuck_count: usize, + stuck: Vec, + /// Empty today — agentflare has no persisted handoff log distinct from + /// item state, so this can't be computed yet (see `bottleneck_note`). + bottlenecks: Vec, + bottleneck_note: String, +} + #[derive(Debug, Default, Deserialize, schemars::JsonSchema)] struct CommentRequest { #[schemars(description = "Action: create|edit|delete|list")] @@ -2371,9 +2399,10 @@ impl AgentflareMcp { "remove_label" => self.item_remove_label(req), "groom" => self.item_groom(req), "standup" => self.item_standup(req), + "health" => self.item_health(req), other => Err(ErrorData::invalid_params( format!( - "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup" + "unknown item action: '{other}' — expected create|get|list|search|update|update_state|delete|claim|heartbeat|release|done|cancel|add_label|remove_label|groom|standup|health" ), None, )), @@ -2381,7 +2410,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|cancel|add_label|remove_label|groom|standup). `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. See each field's description for when it's required." + 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|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`. See each field's description for when it's required." )] fn item(&self, Parameters(req): Parameters) -> Result { self.item_inner(req) @@ -4910,6 +4939,68 @@ mod tests { 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")); + } + /// 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 diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index b9b46522..9629c47f 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -97,6 +97,17 @@ fn near_duplicates( duplicates } +fn to_standup_item(i: &agentflare_backend::item::Item) -> StandupItem { + StandupItem { + id: i.id.clone(), + sequence_id: i.sequence_id, + name: i.name.clone(), + priority: i.priority.clone(), + assignee_agent: i.assignee_agent.clone(), + updated_at: i.updated_at, + } +} + /// Now/Next/Later planning buckets. Unestimated items are excluded outright /// (can't be planned without a size); of the rest, blocked items go to /// `later`, and ready items split into `now` (first `capacity`, in existing @@ -714,17 +725,6 @@ impl AgentflareMcp { let done_cutoff = now - cutoff_hours.saturating_mul(3_600); let stuck_cutoff = now - stuck_days.saturating_mul(86_400); - fn to_standup_item(i: &agentflare_backend::item::Item) -> StandupItem { - StandupItem { - id: i.id.clone(), - sequence_id: i.sequence_id, - name: i.name.clone(), - priority: i.priority.clone(), - assignee_agent: i.assignee_agent.clone(), - updated_at: i.updated_at, - } - } - let mut done: Vec = items .iter() .filter(|i| { @@ -780,4 +780,105 @@ impl AgentflareMcp { Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) })? } + + /// One-call health scorecard: velocity (trailing weekly windows, updated_at + /// proxy per rubric.md), WIP, stuck, and a bottlenecks placeholder. + /// + /// 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. + pub(super) fn item_health(&self, req: ItemRequest) -> Result { + let window_weeks = req.window_weeks.unwrap_or(4).max(1); + let stuck_days = req.staleness_days.unwrap_or(7).max(0); + self.with_backend_db(|conn| { + let project = self.resolve_project(conn)?; + 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) + .map_err(map_backend_err)?; + let state_by_id: std::collections::HashMap<&str, &agentflare_backend::state::State> = + states.iter().map(|s| (s.id.as_str(), s)).collect(); + let group_of = |i: &agentflare_backend::item::Item| -> &str { + state_by_id + .get(i.state_id.as_str()) + .map(|s| s.group_name.as_str()) + .unwrap_or("") + }; + + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + + let completed: Vec<&agentflare_backend::item::Item> = items + .iter() + .filter(|i| group_of(i) == "completed") + .collect(); + let mut velocity: Vec = (0..window_weeks) + .map(|w| { + let week_end = now - w.saturating_mul(7 * 86_400); + let week_start = week_end - 7 * 86_400; + // Upper bound inclusive: an item completed in the same + // second as this call must not be excluded from "this week". + let completed_count = completed + .iter() + .filter(|i| i.updated_at > week_start && i.updated_at <= week_end) + .count(); + VelocityWeek { + week_start, + week_end, + completed_count, + } + }) + .collect(); + velocity.reverse(); // oldest -> newest + let velocity_trend = match velocity.len() { + n if n >= 2 => { + let last = velocity[n - 1].completed_count; + let prev = velocity[n - 2].completed_count; + match last.cmp(&prev) { + std::cmp::Ordering::Greater => "up", + std::cmp::Ordering::Less => "down", + std::cmp::Ordering::Equal => "flat", + } + } + _ => "flat", + } + .to_string(); + + let wip: Vec = items + .iter() + .filter(|i| group_of(i) == "started") + .map(to_standup_item) + .collect(); + let stuck_cutoff = now - stuck_days.saturating_mul(86_400); + let stuck: Vec = wip + .iter() + .filter(|i| i.updated_at < stuck_cutoff) + .cloned() + .collect(); + + let resp = HealthResponse { + window_weeks, + velocity, + velocity_trend, + wip_count: wip.len(), + wip, + 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(), + }; + Ok(serde_json::to_string_pretty(&resp).unwrap_or_default()) + })? + } } From 6c199143ba8a9f86088514fe672915fa70b88f5c Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Thu, 16 Jul 2026 23:39:28 +0530 Subject: [PATCH 11/13] docs(pm): document sizing methodology as a deliberate read-only exception --- .claude/skills/pm/reference/rubric.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.claude/skills/pm/reference/rubric.md b/.claude/skills/pm/reference/rubric.md index f32eada3..289c23d5 100644 --- a/.claude/skills/pm/reference/rubric.md +++ b/.claude/skills/pm/reference/rubric.md @@ -21,6 +21,24 @@ Print each score as: `RICE 9.6 — R4 I5 C3 / E? (UNESTIMATED)` with one-line wh When items lack any effort/size signal, use ICE = Impact × Confidence × Ease (1–5 each) and label the table "ICE (no effort estimates present)". +## Sizing an unestimated item (mutating — outside the read-only workflows) + +`groom`/`plan` never call `item(update)` — sizing is a deliberate exception, +done only when a human directly asks you to size specific items, not as an +automatic step in any workflow. + +When asked, don't guess uniformly: +- **Self-contained description** (states its own size, or is a trivially + small single fix) — size directly from the text. +- **Judgment call** (the estimate depends on how much of this already exists, + how tangled the current code is, or how much is genuinely new) — verify + against the actual codebase first (`ctx_compose`/`ctx_search`/`ctx_read`) + before committing a size via `item(update) metadata={"size":...}`. Trusting + an item's own scope claims without checking is how a real L gets shipped as + M — found live in this project (#100 claimed reusable ledgers that don't + exist anywhere in the codebase; verifying caught it, the description alone + would not have). + ## Unestimated handling Never fail. Score what you can, mark the missing factor `?`, and list all From 9f552877829fd022a5205ec2b77798ff8aa507d9 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Fri, 17 Jul 2026 00:02:32 +0530 Subject: [PATCH 12/13] fix: rustfmt + update stale caveman_cli.rs test to the renamed CLI path - cargo fmt --all: wraps a few lines the local dev-profile check didn't flag (CI's fmt job uses --check with no width override) - tests/caveman_cli.rs called the pre-rename `agentflare caveman compress` subcommand, which no longer exists after the ponytail/caveman -> optimize/flare-code rename; updated to `agentflare optimize output compress` (FlareAction::Output { action: OutputAction::Compress }, src/cli/optimize.rs) and renamed the test function to match Verified with the exact CI command set locally: - cargo fmt --all --check: clean - cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic: clean - cargo test --workspace: 543 passed, 1 ignored, 0 failed (plus all other workspace crates' test suites, all passing) --- src/mcp_server.rs | 41 ++++++++++++++++++++++++----------------- src/mcp_server/item.rs | 11 +++++++++-- tests/caveman_cli.rs | 6 ++++-- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 1277ca3f..302ad63c 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -4692,8 +4692,7 @@ mod tests { fn item_update_sets_metadata() { let (_tmp, s) = harness(); let created: serde_json::Value = - serde_json::from_str(&s.item(Parameters(empty_item_create("Sized"))).unwrap()) - .unwrap(); + serde_json::from_str(&s.item(Parameters(empty_item_create("Sized"))).unwrap()).unwrap(); let updated: serde_json::Value = serde_json::from_str( &s.item(Parameters(ItemRequest { action: "update".into(), @@ -4704,7 +4703,10 @@ mod tests { .unwrap(), ) .unwrap(); - assert_eq!(updated["metadata"], serde_json::json!({"size": "M"}).to_string()); + assert_eq!( + updated["metadata"], + serde_json::json!({"size": "M"}).to_string() + ); } #[test] @@ -4862,10 +4864,9 @@ mod tests { #[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: 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(); @@ -4942,10 +4943,9 @@ mod tests { #[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: 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(); @@ -4964,10 +4964,9 @@ mod tests { 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(); + 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()), @@ -4992,13 +4991,21 @@ mod tests { 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[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")); + assert!( + health["bottleneck_note"] + .as_str() + .unwrap() + .contains("no handoff history") + ); } /// Real measured comparison, not an estimate: one `groom` call vs. the diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 9629c47f..453e1c96 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -580,7 +580,10 @@ impl AgentflareMcp { /// manual groom otherwise costs. pub(super) fn item_groom(&self, req: ItemRequest) -> Result { if req.limit.is_some_and(|l| l < 0) { - return Err(ErrorData::invalid_params("limit must be non-negative", None)); + return Err(ErrorData::invalid_params( + "limit must be non-negative", + None, + )); } let staleness_days = req.staleness_days.unwrap_or(14).max(0); let cap = req.limit.unwrap_or(15).max(0) as usize; @@ -758,7 +761,11 @@ impl AgentflareMcp { std::collections::BTreeMap::new(); for i in &in_progress_items { by_assignee - .entry(i.assignee_agent.clone().unwrap_or_else(|| "unassigned".into())) + .entry( + i.assignee_agent + .clone() + .unwrap_or_else(|| "unassigned".into()), + ) .or_default() .push(to_standup_item(i)); } diff --git a/tests/caveman_cli.rs b/tests/caveman_cli.rs index 6e0d64b3..b4f23559 100644 --- a/tests/caveman_cli.rs +++ b/tests/caveman_cli.rs @@ -33,7 +33,7 @@ fn agentflare_bin() -> PathBuf { } #[test] -fn caveman_compress_generic_uses_the_stubbed_claude_cli() { +fn optimize_output_compress_generic_uses_the_stubbed_claude_cli() { let dir = tempfile::tempdir().unwrap(); let stub_dir = dir.path().join("bin"); std::fs::create_dir_all(&stub_dir).unwrap(); @@ -50,8 +50,10 @@ fn caveman_compress_generic_uses_the_stubbed_claude_cli() { existing_path ); + // `caveman compress` was renamed to `optimize output compress` + // (FlareAction::Output { action: OutputAction::Compress }, src/cli/optimize.rs). let output = Command::new(agentflare_bin()) - .args(["caveman", "compress"]) + .args(["optimize", "output", "compress"]) .arg(&source) .env("PATH", new_path) .env_remove("ANTHROPIC_API_KEY") From 09e95c2fd6dfc5a928ce65cd8ded3b5538e9968f Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Fri, 17 Jul 2026 00:22:44 +0530 Subject: [PATCH 13/13] fix: address CodeRabbit findings on groom/standup/health - dependency_edges_for_items (was dependencies_for_items) now joins the dependency target's true state_group in SQL, instead of looking it up via a shortlist-scoped linear scan. Fixes a real bug: a completed dependency that fell outside the default state_group filter (e.g. "backlog,unstarted" excludes "completed") read back as "" from the old lookup and was treated as still-open, falsely blocking its dependent. - dependency_fanin_for_items counts dependents project-wide instead of only within the shortlist, fixing an undercounted depended_on_by_count when a dependent fell outside the shortlist/limit window. - standup's "done" filter and health's velocity bucketing now key off completed_at instead of updated_at. Editing an already-completed item (e.g. fixing a typo) bumps updated_at without re-completing it; using updated_at made old work spuriously reappear as "just done" or shift which week it counted toward. - window_weeks (health) and limit (groom) are now clamped (52, 200) instead of unbounded - an unbounded window_weeks drove a Vec allocation of that literal size while holding the backend DB lock. - Fixed two doc inaccuracies: read-recipe.md claimed item(get) returns labels (it doesn't - separate join table); staleness_days' schema description only mentioned groom's default, not standup/health's. 6 new regression tests, one per fix. cargo test --workspace: 548 passed, 1 ignored, 0 failed. cargo clippy --locked --workspace --all-targets --all-features: clean. cargo fmt --all --check: clean. --- .claude/skills/pm/reference/read-recipe.md | 5 +- crates/agentflare-backend/src/item.rs | 50 ++++- src/mcp_server.rs | 236 ++++++++++++++++++++- src/mcp_server/item.rs | 76 ++++--- 4 files changed, 324 insertions(+), 43 deletions(-) diff --git a/.claude/skills/pm/reference/read-recipe.md b/.claude/skills/pm/reference/read-recipe.md index 8c7d54de..9d7dfe12 100644 --- a/.claude/skills/pm/reference/read-recipe.md +++ b/.claude/skills/pm/reference/read-recipe.md @@ -32,8 +32,9 @@ the old N+1 path this action replaces. ## Detail fetch (only when needed) `item action="get" id=` returns one full item incl. description, metadata, -labels, timestamps — for a single ad-hoc lookup outside grooming, not for -building a shortlist (use `groom` for that). +timestamps — for a single ad-hoc lookup outside grooming, not for building a +shortlist (use `groom` for that). Labels are a separate join, not part of +this response. ## Time signals — approximate, state this in output diff --git a/crates/agentflare-backend/src/item.rs b/crates/agentflare-backend/src/item.rs index cfa36b58..dc75d0fa 100644 --- a/crates/agentflare-backend/src/item.rs +++ b/crates/agentflare-backend/src/item.rs @@ -448,23 +448,59 @@ pub fn list_dependencies(conn: &Connection, item_id: &str) -> Result Ok(rows.collect::>()?) } -/// Dependency edges `(item_id, depends_on_item_id)` for a set of items in one -/// query, instead of N `list_dependencies` round trips — used by `groom` to -/// compute blocked/fan-in signals for a whole shortlist at once. -pub fn dependencies_for_items( +/// 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> { +) -> Result> { if item_ids.is_empty() { return Ok(vec![]); } let placeholders = item_ids.iter().map(|_| "?").collect::>().join(","); let sql = format!( - "SELECT item_id, depends_on_item_id FROM item_dependencies WHERE item_id IN ({placeholders})" + "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::<_, String>(1)?)) + Ok((row.get::<_, String>(0)?, row.get::<_, i64>(1)?)) })?; Ok(rows.collect::>()?) } diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 302ad63c..7da9d4ac 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -606,7 +606,7 @@ struct ItemRequest { #[serde(default)] state_group: Option, #[schemars( - description = "Max items to return (list: omit for no limit; search: omit for 20, capped at 1000)" + description = "Max items to return (list: omit for no limit; search: omit for 20, capped at 1000; groom: omit for 15, capped at 200)" )] #[serde(default)] limit: Option, @@ -617,7 +617,7 @@ struct ItemRequest { #[serde(default)] query: Option, #[schemars( - description = "Days since updated_at before an item counts as stale (groom); default 14" + description = "Days since updated_at before an item counts as stale/stuck (groom: default 14; standup/health: default 7)" )] #[serde(default)] staleness_days: Option, @@ -631,7 +631,7 @@ struct ItemRequest { )] #[serde(default)] cutoff_hours: Option, - #[schemars(description = "Trailing weekly windows for velocity (health); default 4")] + #[schemars(description = "Trailing weekly windows for velocity (health); default 4, max 52")] #[serde(default)] window_weeks: Option, } @@ -4599,6 +4599,119 @@ mod tests { assert_eq!(groomed["unassigned_count"], 1); } + /// Regression (CodeRabbit): a completed dependency must never read back + /// as an open blocker just because it fell outside the shortlist's + /// default state_group filter (completed items aren't in + /// "backlog,unstarted", so the naive shortlist-scoped lookup used to + /// return "" for its state and treat that as "still open"). + #[test] + fn item_groom_does_not_block_on_a_completed_dependency_outside_the_shortlist() { + let (_tmp, s) = harness(); + let dep: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Dep"))).unwrap()).unwrap(); + let project_id = dep["project_id"].as_str().unwrap().to_string(); + let blocked: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Blocked".into()), + dependency_ids: Some(vec![dep["id"].as_str().unwrap().to_string()]), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + 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(dep["id"].as_str().unwrap().to_string()), + state_id: Some(completed_state), + ..Default::default() + })) + .unwrap(); + + // Default state_group is "backlog,unstarted" — Dep (now completed) + // falls outside the shortlist entirely. + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let items = groomed["items"].as_array().unwrap(); + assert!( + !items.iter().any(|i| i["id"] == dep["id"]), + "completed Dep should not be in the default shortlist" + ); + let blocked_entry = items.iter().find(|i| i["id"] == blocked["id"]).unwrap(); + assert_eq!( + blocked_entry["blocked_by"].as_array().unwrap().len(), + 0, + "a completed dependency must not block, even when it's outside the shortlist" + ); + } + + /// Regression (CodeRabbit): fan-in must count dependents project-wide, + /// not just other items that happen to share the same shortlist. + #[test] + fn item_groom_fanin_counts_dependents_outside_the_shortlist() { + let (_tmp, s) = harness(); + let target: serde_json::Value = + serde_json::from_str(&s.item(Parameters(empty_item_create("Target"))).unwrap()) + .unwrap(); + let project_id = target["project_id"].as_str().unwrap().to_string(); + let dependent: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "create".into(), + name: Some("Dependent".into()), + dependency_ids: Some(vec![target["id"].as_str().unwrap().to_string()]), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + + 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); + // Move the dependent out of the default shortlist filter — Target's + // fan-in must still count it. + s.item(Parameters(ItemRequest { + action: "update_state".into(), + id: Some(dependent["id"].as_str().unwrap().to_string()), + state_id: Some(completed_state), + ..Default::default() + })) + .unwrap(); + + let groomed: serde_json::Value = serde_json::from_str( + &s.item(Parameters(ItemRequest { + action: "groom".into(), + ..Default::default() + })) + .unwrap(), + ) + .unwrap(); + let items = groomed["items"].as_array().unwrap(); + assert!(!items.iter().any(|i| i["id"] == dependent["id"])); + let target_entry = items.iter().find(|i| i["id"] == target["id"]).unwrap(); + assert_eq!(target_entry["depended_on_by_count"], 1); + } + #[test] fn item_groom_flags_blocked_by_open_dependency() { let (_tmp, s) = harness(); @@ -4861,6 +4974,85 @@ mod tests { 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(); @@ -5008,6 +5200,44 @@ mod tests { ); } + /// 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 diff --git a/src/mcp_server/item.rs b/src/mcp_server/item.rs index 453e1c96..1eb20731 100644 --- a/src/mcp_server/item.rs +++ b/src/mcp_server/item.rs @@ -6,6 +6,16 @@ use super::*; +/// Bounds `groom`'s shortlist size — caps the O(n^2) duplicate-detection +/// pass and the SQLite `IN (...)` parameter list built from it. +const MAX_GROOM_LIMIT: i64 = 200; + +/// Bounds `health`'s velocity window — without this, a caller-supplied +/// `window_weeks` (e.g. `i64::MAX`) would build a `Vec` of +/// that literal size regardless of how much data actually exists, while +/// holding the backend DB lock. +const MAX_WINDOW_WEEKS: i64 = 52; + fn priority_rank(p: &str) -> u8 { match p { "urgent" => 5, @@ -36,27 +46,24 @@ fn parsed_size(metadata: &str) -> Option { .map(str::to_string) } -/// Fan-in count and open-dependency blocking per item, from a flat edge list. -fn dependency_signals<'a>( - edges: &[(String, String)], - state_group_of: impl Fn(&str) -> &'a str, -) -> ( - std::collections::HashMap, - std::collections::HashMap>, -) { - let mut fanin: std::collections::HashMap = std::collections::HashMap::new(); +/// Open-dependency blocking per item, from edges that already carry the +/// dependency target's true state_group (joined server-side in +/// `dependency_edges_for_items` — so blocking status is correct even when +/// the target isn't in the same shortlist/limit window as the item). +fn blocked_by_map( + edges: &[(String, String, String)], +) -> std::collections::HashMap> { let mut blocked_by: std::collections::HashMap> = std::collections::HashMap::new(); - for (item_id, depends_on) in edges { - *fanin.entry(depends_on.clone()).or_insert(0) += 1; - if !matches!(state_group_of(depends_on), "completed" | "cancelled") { + for (item_id, depends_on, target_group) in edges { + if !matches!(target_group.as_str(), "completed" | "cancelled") { blocked_by .entry(item_id.clone()) .or_default() .push(depends_on.clone()); } } - (fanin, blocked_by) + blocked_by } /// Near-duplicate names within a shortlist (token-Jaccard ≥ 0.5) — no @@ -586,7 +593,9 @@ impl AgentflareMcp { )); } let staleness_days = req.staleness_days.unwrap_or(14).max(0); - let cap = req.limit.unwrap_or(15).max(0) as usize; + // Bounds the shortlist's O(n^2) duplicate-detection pass and the + // 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 mut items = agentflare_backend::item::list_by_project(conn, &project.id) @@ -625,16 +634,11 @@ impl AgentflareMcp { let shortlist: Vec<_> = items.into_iter().take(cap).collect(); let ids: Vec = shortlist.iter().map(|i| i.id.clone()).collect(); - let edges = agentflare_backend::item::dependencies_for_items(conn, &ids) + let edges = agentflare_backend::item::dependency_edges_for_items(conn, &ids) + .map_err(map_backend_err)?; + let blocked_by = blocked_by_map(&edges); + let fanin = agentflare_backend::item::dependency_fanin_for_items(conn, &ids) .map_err(map_backend_err)?; - let (fanin, blocked_by) = dependency_signals(&edges, |id| { - shortlist - .iter() - .find(|i| i.id == id) - .and_then(|i| state_by_id.get(i.state_id.as_str())) - .map(|s| s.group_name.as_str()) - .unwrap_or("") - }); let duplicates = near_duplicates(&shortlist); let groom_items: Vec = shortlist @@ -728,18 +732,22 @@ impl AgentflareMcp { let done_cutoff = now - cutoff_hours.saturating_mul(3_600); let stuck_cutoff = now - stuck_days.saturating_mul(86_400); - let mut done: Vec = items + // completed_at, not updated_at: editing an already-completed item + // (e.g. fixing a typo) bumps updated_at without re-completing it — + // using updated_at here would make old work spuriously reappear + // in a "done recently" digest. + let mut done_items: Vec<&agentflare_backend::item::Item> = items .iter() .filter(|i| { state_by_id .get(i.state_id.as_str()) .map(|s| s.group_name == "completed") .unwrap_or(false) - && i.updated_at >= done_cutoff + && i.completed_at.is_some_and(|t| t >= done_cutoff) }) - .map(to_standup_item) .collect(); - done.sort_by_key(|i| std::cmp::Reverse(i.updated_at)); + done_items.sort_by_key(|i| std::cmp::Reverse(i.completed_at)); + let done: Vec = done_items.into_iter().map(to_standup_item).collect(); let in_progress_items: Vec<_> = items .iter() @@ -801,7 +809,7 @@ impl AgentflareMcp { /// infrastructure now would be speculative. 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).max(1); + 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)?; @@ -831,11 +839,17 @@ impl AgentflareMcp { .map(|w| { let week_end = now - w.saturating_mul(7 * 86_400); let week_start = week_end - 7 * 86_400; - // Upper bound inclusive: an item completed in the same - // second as this call must not be excluded from "this week". + // completed_at, not updated_at (see the standup fix above — + // same reason: editing a completed item must not move it + // between velocity weeks). Upper bound inclusive: an item + // completed in the same second as this call must not be + // excluded from "this week". let completed_count = completed .iter() - .filter(|i| i.updated_at > week_start && i.updated_at <= week_end) + .filter(|i| { + i.completed_at + .is_some_and(|t| t > week_start && t <= week_end) + }) .count(); VelocityWeek { week_start,