From 2e0827e829129cc0c0744aa6a9ebe7b0cdbedec1 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Tue, 28 Jul 2026 10:26:29 +0530 Subject: [PATCH 1/2] feat(skill): finish EPIC #272 Task 6 write-path + Task 9 provisioning Task 6 (proactive advisory): proactive_suggestions()/settings read-path already existed (PR #302) but had no way to actually set a snooze or dismiss - save_settings() was dead code. Add `skill snooze [--days N]` and `skill dismiss ` wired to it. Task 9 (repo stack -> auto provisioning): new `skill provision ` subcommand. Detects stack via manifest files (Cargo.toml/package.json/ tsconfig.json/pyproject.toml/requirements.txt/go.mod), ranks indexed skills against it via BM25 search, prints a dry-run report (skills, confidence, token cost) with zero DB writes, and on --yes re-tags the matched entries under source "provisioned:" via the existing scan+rebuild entry-creation path. --- src/cli/skill.rs | 251 ++++++++++++++++++++++++++++++++++++++++- src/skill_proactive.rs | 104 ++++++++++++++++- 2 files changed, 349 insertions(+), 6 deletions(-) diff --git a/src/cli/skill.rs b/src/cli/skill.rs index 7a05e566..e72cd684 100644 --- a/src/cli/skill.rs +++ b/src/cli/skill.rs @@ -7,7 +7,7 @@ use skill::source::parse_source; use skill::types::{ AgentConfig, AgentId, DiscoverOptions, InstallMode, InstallOptions, InstallScope, }; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; #[derive(Subcommand)] @@ -45,6 +45,25 @@ pub enum SkillAction { #[command(subcommand)] action: HubAction, }, + /// Suppress proactive skill-advisory suggestions for a skill. + Snooze { + name: String, + /// Days to suppress suggestions for. + #[arg(long, default_value_t = 7)] + days: i64, + }, + /// Permanently suppress proactive skill-advisory suggestions for a skill. + Dismiss { + name: String, + }, + /// Detect a target repo's stack and recommend indexed skills for it. + /// Prints a dry-run report only, unless `--yes` is passed. + Provision { + path: String, + /// Actually provision the recommended skills (default: dry run). + #[arg(long)] + yes: bool, + }, } #[derive(Subcommand)] @@ -648,6 +667,148 @@ fn run_hub(action: HubAction) -> Result> { } } +fn run_snooze(name: &str, days: i64) { + crate::skill_proactive::snooze(name, days); + println!("✓ snoozed '{name}' for {days} day(s)"); +} + +fn run_dismiss(name: &str) { + crate::skill_proactive::dismiss(name); + println!("✓ dismissed '{name}' — will not be suggested again"); +} + +/// Manifest files/layout that identify a repo's stack. Pure and side-effect +/// free so it's directly testable without touching the skill DB. +fn detect_stack(path: &Path) -> Vec<&'static str> { + let mut stack = Vec::new(); + if path.join("Cargo.toml").is_file() { + stack.push("rust"); + } + if path.join("package.json").is_file() { + stack.push("node"); + stack.push("javascript"); + } + if path.join("tsconfig.json").is_file() { + stack.push("typescript"); + } + if path.join("pyproject.toml").is_file() || path.join("requirements.txt").is_file() { + stack.push("python"); + } + if path.join("go.mod").is_file() { + stack.push("go"); + } + stack +} + +/// Best-scoring hit per skill name across every stack tag, ranked +/// descending. Takes a raw `Connection` (not `Registry`) so it's testable +/// against an in-memory DB seeded via `skill_registry::db::rebuild`. +fn rank_candidates( + conn: &rusqlite::Connection, + stack: &[&str], +) -> Vec { + let mut best: HashMap = HashMap::new(); + for tag in stack { + let Ok(hits) = skill_registry::search::search(conn, tag, 5, skill_registry::MatchMode::Any) + else { + continue; + }; + for hit in hits { + best.entry(hit.name.clone()) + .and_modify(|existing| { + if hit.score > existing.score { + *existing = hit.clone(); + } + }) + .or_insert(hit); + } + } + let mut ranked: Vec<_> = best.into_values().collect(); + ranked.sort_by(|a, b| { + b.score + .partial_cmp(&a.score) + .unwrap_or(std::cmp::Ordering::Equal) + }); + ranked +} + +fn run_provision(path: &str, yes: bool) -> Result<(), Box> { + let repo_path = Path::new(path); + let stack = detect_stack(repo_path); + if stack.is_empty() { + println!( + "no recognized stack detected at '{path}' (looked for Cargo.toml/package.json/pyproject.toml/requirements.txt/go.mod)" + ); + return Ok(()); + } + println!("detected stack: {}", stack.join(", ")); + + let db_path = crate::paths::skills_db_path(); + let mut conn = skill_registry::db::open_db(&db_path)?; + let ranked = rank_candidates(&conn, &stack); + if ranked.is_empty() { + println!( + "no matching skills found for detected stack — run `agentflare skill list`/`search` first to populate the index" + ); + return Ok(()); + } + + let total_tokens: i64 = ranked.iter().map(|h| h.est_tokens).sum(); + println!( + "recommended skills ({} candidates, ~{total_tokens} tokens):", + ranked.len() + ); + for hit in &ranked { + println!( + " {} — {} (confidence {:.0}%, ~{} tokens)", + hit.name, + hit.description, + hit.score.min(1.0) * 100.0, + hit.est_tokens + ); + } + + if !yes { + println!("\n(dry run — pass --yes to install)"); + return Ok(()); + } + + let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let sources = + skill_registry::sources::default_sources(&home, &cwd, &["claude-code".to_string()]); + let scan = skill_registry::sources::scan_sources(&sources); + let mut all = scan.entries; + let mut existing: HashSet<(String, String)> = all + .iter() + .map(|e| (e.name.clone(), e.source.clone())) + .collect(); + + let provisioned_source = format!("provisioned:{path}"); + let mut installed = 0; + for hit in &ranked { + let Some(entry) = all + .iter() + .find(|e| e.name == hit.name && e.source == hit.source) + .cloned() + else { + continue; + }; + let provisioned = skill_registry::sources::SkillEntry { + source: provisioned_source.clone(), + ..entry + }; + if existing.insert((provisioned.name.clone(), provisioned.source.clone())) { + all.push(provisioned); + installed += 1; + } + } + + skill_registry::db::rebuild(&mut conn, &all)?; + println!("✓ provisioned {installed} skill(s) for '{path}' under source '{provisioned_source}'"); + Ok(()) +} + impl SkillArgs { pub fn run(self) { let rt = tokio::runtime::Runtime::new().unwrap(); @@ -704,6 +865,14 @@ impl SkillArgs { std::process::exit(1); } }, + SkillAction::Snooze { name, days } => run_snooze(&name, days), + SkillAction::Dismiss { name } => run_dismiss(&name), + SkillAction::Provision { path, yes } => { + if let Err(e) = run_provision(&path, yes) { + eprintln!("provision error: {e}"); + std::process::exit(1); + } + } } } } @@ -818,4 +987,84 @@ mod tests { ); }); } + + #[test] + fn detect_stack_identifies_rust_repo() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("Cargo.toml"), "[package]\nname = \"x\"\n").unwrap(); + assert_eq!(detect_stack(dir.path()), vec!["rust"]); + } + + #[test] + fn detect_stack_identifies_node_repo() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("package.json"), "{}").unwrap(); + assert_eq!(detect_stack(dir.path()), vec!["node", "javascript"]); + } + + #[test] + fn detect_stack_is_empty_for_unrecognized_repo() { + let dir = tempfile::tempdir().unwrap(); + assert!(detect_stack(dir.path()).is_empty()); + } + + fn fixture_entry( + name: &str, + description: &str, + tags: &str, + ) -> skill_registry::sources::SkillEntry { + skill_registry::sources::SkillEntry { + name: name.to_string(), + source: "test".to_string(), + path: PathBuf::from(format!("/fixtures/{name}/SKILL.md")), + description: description.to_string(), + body: String::new(), + neg_text: String::new(), + tags: tags.to_string(), + est_tokens: 42, + mtime: 1, + bandit_alpha: 1.0, + bandit_beta: 1.0, + shadow_path: None, + } + } + + #[test] + fn rank_candidates_differs_by_detected_stack() { + let mut conn = skill_registry::db::open_in_memory().unwrap(); + skill_registry::db::rebuild( + &mut conn, + &[ + fixture_entry( + "rust-help", + "Guidance for writing idiomatic rust and using cargo", + "rust cargo", + ), + fixture_entry( + "node-help", + "Guidance for node projects using npm and javascript", + "node javascript npm", + ), + ], + ) + .unwrap(); + + let rust_ranked = rank_candidates(&conn, &["rust"]); + let node_ranked = rank_candidates(&conn, &["node", "javascript"]); + + assert_eq!(rust_ranked.first().unwrap().name, "rust-help"); + assert_eq!(node_ranked.first().unwrap().name, "node-help"); + assert_ne!( + rust_ranked.first().unwrap().name, + node_ranked.first().unwrap().name + ); + } + + #[test] + fn rank_candidates_empty_stack_yields_no_candidates() { + let mut conn = skill_registry::db::open_in_memory().unwrap(); + skill_registry::db::rebuild(&mut conn, &[fixture_entry("rust-help", "rust", "rust")]) + .unwrap(); + assert!(rank_candidates(&conn, &[]).is_empty()); + } } diff --git a/src/skill_proactive.rs b/src/skill_proactive.rs index 392b5ca9..f21dbd2a 100644 --- a/src/skill_proactive.rs +++ b/src/skill_proactive.rs @@ -46,11 +46,6 @@ pub fn load_settings() -> SkillAdvisorySettings { .unwrap_or_default() } -// TODO(task-6-followup): no CLI/MCP path calls this yet -- the snooze/dismiss -// write side (e.g. `skill snooze `) is unbuilt, only the read side -// (proactive_suggestions() checking skill_overrides) exists. Remove this -// allow once a caller lands. -#[allow(dead_code)] pub fn save_settings(settings: &SkillAdvisorySettings) { if let Some(parent) = SETTINGS_PATH.parent() { let _ = std::fs::create_dir_all(parent); @@ -60,6 +55,46 @@ pub fn save_settings(settings: &SkillAdvisorySettings) { } } +fn apply_snooze(settings: &mut SkillAdvisorySettings, name: &str, snooze_until: i64) { + settings + .skill_overrides + .entry(name.to_string()) + .or_insert(SkillOverride { + snooze_until: 0, + dismissed: false, + }) + .snooze_until = snooze_until; +} + +fn apply_dismiss(settings: &mut SkillAdvisorySettings, name: &str) { + settings + .skill_overrides + .entry(name.to_string()) + .or_insert(SkillOverride { + snooze_until: 0, + dismissed: false, + }) + .dismissed = true; +} + +/// Suppress proactive suggestions for `name` for `days` days (from now). +pub fn snooze(name: &str, days: i64) { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0); + let mut settings = load_settings(); + apply_snooze(&mut settings, name, now + days.max(0) * 86_400); + save_settings(&settings); +} + +/// Permanently suppress proactive suggestions for `name`. +pub fn dismiss(name: &str) { + let mut settings = load_settings(); + apply_dismiss(&mut settings, name); + save_settings(&settings); +} + /// Generate proactive skill suggestions: runs `session_context_queries()` → /// `classify()` → `find_skills()`, filters by settings, returns formatted /// text suitable for hook injection. Returns None when no suggestions pass @@ -134,3 +169,62 @@ pub fn proactive_suggestions() -> Option { suggestions.join("\n") )) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn apply_snooze_sets_until_for_new_and_existing_override() { + let mut settings = SkillAdvisorySettings::default(); + apply_snooze(&mut settings, "flare-code", 12345); + assert_eq!( + settings + .skill_overrides + .get("flare-code") + .unwrap() + .snooze_until, + 12345 + ); + assert!( + !settings + .skill_overrides + .get("flare-code") + .unwrap() + .dismissed + ); + + apply_snooze(&mut settings, "flare-code", 99999); + assert_eq!( + settings + .skill_overrides + .get("flare-code") + .unwrap() + .snooze_until, + 99999 + ); + } + + #[test] + fn apply_dismiss_sets_flag_without_disturbing_snooze() { + let mut settings = SkillAdvisorySettings::default(); + apply_snooze(&mut settings, "flare-code", 555); + apply_dismiss(&mut settings, "flare-code"); + let entry = settings.skill_overrides.get("flare-code").unwrap(); + assert!(entry.dismissed); + assert_eq!(entry.snooze_until, 555); + } + + #[test] + fn apply_dismiss_on_unknown_skill_creates_override() { + let mut settings = SkillAdvisorySettings::default(); + apply_dismiss(&mut settings, "never-seen"); + assert!( + settings + .skill_overrides + .get("never-seen") + .unwrap() + .dismissed + ); + } +} From cc956fb91af121f4a9f5c4d7fc72829706711070 Mon Sep 17 00:00:00 2001 From: Shivakumar Date: Tue, 28 Jul 2026 10:56:03 +0530 Subject: [PATCH 2/2] fix(skill): address CodeRabbit findings on provision/snooze - rank_candidates picked the highest bm25 score as "best" and sorted descending; bm25 is negative-is-better (search.rs sorts ASC on the same raw value), so this had it backwards. Flip to min-selection + ascending sort, add confidence_pct() for a bounded 0-100 display value instead of the broken score.min(1.0)*100.0. - run_provision --yes rebuilt the DB from a fresh flat-dir scan only, and rebuild() is full-replace -- silently dropping any other provisioned:*/imported:*/hub:* DB-only rows on every run. Merge existing DB rows (via list_all_name_source_pairs + load, same reconstruction run_export already uses) before rebuilding, mirroring HubAction::Pull's merge-before-rebuild shape. - Use crate::components::detected_skill_agents() instead of a hardcoded ["claude-code"] source list. - snooze() clamped days to >=0 but not the upper bound; saturating_add from a large --days no longer risks i64 overflow. - New adversarial rank_candidates test (two same-tag matches of different strength) -- the previous test had only one candidate per tag, so it couldn't have caught the sign inversion. --- src/cli/skill.rs | 97 +++++++++++++++++++++++++++++++++++++++--- src/skill_proactive.rs | 3 +- 2 files changed, 93 insertions(+), 7 deletions(-) diff --git a/src/cli/skill.rs b/src/cli/skill.rs index e72cd684..bd85a048 100644 --- a/src/cli/skill.rs +++ b/src/cli/skill.rs @@ -703,6 +703,15 @@ fn detect_stack(path: &Path) -> Vec<&'static str> { /// Best-scoring hit per skill name across every stack tag, ranked /// descending. Takes a raw `Connection` (not `Registry`) so it's testable /// against an in-memory DB seeded via `skill_registry::db::rebuild`. +/// SQLite FTS5's `bm25()` returns negative values where *lower* (more +/// negative) is a better match (`search.rs` sorts `ORDER BY score` ascending +/// on the same raw value). Map that to a bounded, monotonic 0..100 display +/// value: 0 for no match, asymptotically approaching 100 for a strong one. +fn confidence_pct(score: f64) -> f64 { + let relevance = (-score).max(0.0); + 100.0 * relevance / (1.0 + relevance) +} + fn rank_candidates( conn: &rusqlite::Connection, stack: &[&str], @@ -716,7 +725,8 @@ fn rank_candidates( for hit in hits { best.entry(hit.name.clone()) .and_modify(|existing| { - if hit.score > existing.score { + // Lower bm25 = better match. + if hit.score < existing.score { *existing = hit.clone(); } }) @@ -725,8 +735,8 @@ fn rank_candidates( } let mut ranked: Vec<_> = best.into_values().collect(); ranked.sort_by(|a, b| { - b.score - .partial_cmp(&a.score) + a.score + .partial_cmp(&b.score) .unwrap_or(std::cmp::Ordering::Equal) }); ranked @@ -763,7 +773,7 @@ fn run_provision(path: &str, yes: bool) -> Result<(), Box " {} — {} (confidence {:.0}%, ~{} tokens)", hit.name, hit.description, - hit.score.min(1.0) * 100.0, + confidence_pct(hit.score), hit.est_tokens ); } @@ -775,8 +785,11 @@ fn run_provision(path: &str, yes: bool) -> Result<(), Box let home = dirs::home_dir().unwrap_or_else(|| PathBuf::from(".")); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); - let sources = - skill_registry::sources::default_sources(&home, &cwd, &["claude-code".to_string()]); + let sources = skill_registry::sources::default_sources( + &home, + &cwd, + &crate::components::detected_skill_agents(), + ); let scan = skill_registry::sources::scan_sources(&sources); let mut all = scan.entries; let mut existing: HashSet<(String, String)> = all @@ -784,6 +797,39 @@ fn run_provision(path: &str, yes: bool) -> Result<(), Box .map(|e| (e.name.clone(), e.source.clone())) .collect(); + // `rebuild()` is documented full-replace ("the filesystem is the source + // of truth"), but DB-only rows (prior `provisioned:*`/`imported:*`/ + // `hub:*` entries) aren't rediscoverable by a flat-dir rescan. Carry them + // forward so provisioning one repo doesn't silently drop another's + // provisioned/imported skills — same merge shape `HubAction::Pull` uses + // for hub entries, generalized to every DB-only source. + if let Ok(pairs) = skill_registry::search::list_all_name_source_pairs(&conn) { + for (name, source) in pairs { + let key = (name.clone(), source.clone()); + if existing.contains(&key) { + continue; + } + let qualified = format!("{source}:{name}"); + if let Ok(skill) = skill_registry::load(&conn, &qualified, false) { + all.push(skill_registry::sources::SkillEntry { + name, + source, + path: PathBuf::new(), + description: skill.description, + body: skill.body.clone(), + neg_text: String::new(), + tags: String::new(), + est_tokens: skill.body.len() as i64 / 4, + mtime: 0, + bandit_alpha: 1.0, + bandit_beta: 1.0, + shadow_path: None, + }); + existing.insert(key); + } + } + } + let provisioned_source = format!("provisioned:{path}"); let mut installed = 0; for hit in &ranked { @@ -1060,6 +1106,45 @@ mod tests { ); } + #[test] + fn rank_candidates_prefers_stronger_bm25_match_within_a_tag() { + // bm25 is negative-is-better; both entries match "rust" via FTS, but + // rust-deep-dive repeats the term across name/description/tags so it + // must score more negative (better) than rust-mention, which only + // has one incidental occurrence. A max-score (instead of min-score) + // selection bug would invert this. + let mut conn = skill_registry::db::open_in_memory().unwrap(); + skill_registry::db::rebuild( + &mut conn, + &[ + fixture_entry( + "rust-deep-dive", + "rust rust rust idiomatic rust patterns and rust cargo workflows", + "rust rust cargo", + ), + fixture_entry( + "misc-notes", + "assorted project notes that mention rust once in passing", + "notes", + ), + ], + ) + .unwrap(); + + let ranked = rank_candidates(&conn, &["rust"]); + assert_eq!(ranked.first().unwrap().name, "rust-deep-dive"); + // The winning (best/lowest) score must sort before the rest. + assert!(ranked.windows(2).all(|w| w[0].score <= w[1].score)); + } + + #[test] + fn confidence_pct_is_bounded_and_monotonic() { + assert_eq!(confidence_pct(0.0), 0.0); + assert!(confidence_pct(-1.0) > 0.0 && confidence_pct(-1.0) < 100.0); + assert!(confidence_pct(-1000.0) > confidence_pct(-1.0)); + assert!(confidence_pct(-1000.0) < 100.0); + } + #[test] fn rank_candidates_empty_stack_yields_no_candidates() { let mut conn = skill_registry::db::open_in_memory().unwrap(); diff --git a/src/skill_proactive.rs b/src/skill_proactive.rs index f21dbd2a..cfe50414 100644 --- a/src/skill_proactive.rs +++ b/src/skill_proactive.rs @@ -84,7 +84,8 @@ pub fn snooze(name: &str, days: i64) { .map(|d| d.as_secs() as i64) .unwrap_or(0); let mut settings = load_settings(); - apply_snooze(&mut settings, name, now + days.max(0) * 86_400); + let snooze_until = now.saturating_add(days.max(0).saturating_mul(86_400)); + apply_snooze(&mut settings, name, snooze_until); save_settings(&settings); }