diff --git a/Cargo.lock b/Cargo.lock index f0c5555d..5c3918c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -131,6 +131,9 @@ dependencies = [ name = "agentflare-artifacts" version = "0.1.0" dependencies = [ + "agentflare-db-kit", + "agentflare-store", + "blake3", "flate2", "nanoid", "serde", diff --git a/crates/agentflare-artifacts/Cargo.toml b/crates/agentflare-artifacts/Cargo.toml index 3278f8d2..dd90ffe6 100644 --- a/crates/agentflare-artifacts/Cargo.toml +++ b/crates/agentflare-artifacts/Cargo.toml @@ -12,6 +12,9 @@ nanoid = "0.5" flate2 = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" +agentflare-store = { path = "../agentflare-store" } +db_kit = { package = "agentflare-db-kit", path = "../agentflare-db-kit" } +blake3 = "1.5" [dev-dependencies] tempfile = "3" diff --git a/crates/agentflare-artifacts/src/store.rs b/crates/agentflare-artifacts/src/store.rs index 02ebdc42..393856ea 100644 --- a/crates/agentflare-artifacts/src/store.rs +++ b/crates/agentflare-artifacts/src/store.rs @@ -2,6 +2,7 @@ use crate::types::{ Artifact, ArtifactSummary, ArtifactType, GitProvenance, PublishRequest, PublishResponse, VersionInfo, }; +use agentflare_store::{documents::DocUpsertOpts, Store}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fs; @@ -22,6 +23,9 @@ const VERSIONS_DIR: &str = "versions"; /// old snapshot bodies are gone. v1 is always kept as the origin anchor. const MAX_KEPT_VERSIONS: u32 = 50; +/// Doc-store project key for artifact documents. +const DOC_PROJECT: &str = "__artifacts__"; + #[derive(Clone, Debug, Serialize, Deserialize)] struct ArtifactMeta { pub id: String, @@ -54,6 +58,7 @@ struct ArtifactMeta { pub struct ArtifactStore { base_path: PathBuf, live_broadcast: Arc>>>>, + store: Option>, } impl ArtifactStore { @@ -61,12 +66,212 @@ impl ArtifactStore { let store = ArtifactStore { base_path: base_path.join(ARTIFACTS_DIR), live_broadcast: Arc::new(Mutex::new(HashMap::new())), + store: None, }; let _ = fs::create_dir_all(&store.base_path); store } + pub fn with_store(store: Store) -> Self { + ArtifactStore { + base_path: PathBuf::from("store:"), + live_broadcast: Arc::new(Mutex::new(HashMap::new())), + store: Some(Arc::new(store)), + } + } + + /// True when the store is backed by agentflare_store documents+blobs. + pub fn has_store(&self) -> bool { + self.store.is_some() + } + + fn store_conn_err(e: impl std::fmt::Display) -> std::io::Error { + std::io::Error::other(e.to_string()) + } + + fn doc_to_artifact(doc: &agentflare_store::documents::Document) -> std::io::Result { + let meta: ArtifactMeta = serde_json::from_str(&doc.metadata) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + Ok(Artifact { + id: doc.path.clone(), + name: doc.title.clone(), + artifact_type: meta.artifact_type, + content: doc.content.clone(), + session_id: doc.session_id.clone().unwrap_or_default(), + created_at: doc.created_at as u64, + updated_at: doc.updated_at as u64, + version: doc.version as u32, + description: meta.description, + favicon: meta.favicon, + sender: meta.sender, + recipient: meta.recipient, + thread_id: meta.thread_id, + reply_to: meta.reply_to, + git: meta.git, + }) + } + + fn doc_to_summary(doc: &agentflare_store::documents::Document) -> ArtifactSummary { + let meta: Option = serde_json::from_str(&doc.metadata).ok(); + let art_type = meta + .as_ref() + .map(|m| m.artifact_type.clone()) + .or_else(|| doc.tags.first().map(|s| ArtifactType::from(s.as_str()))) + .unwrap_or(ArtifactType::Text); + ArtifactSummary { + id: doc.path.clone(), + name: doc.title.clone(), + artifact_type: art_type, + session_id: doc.session_id.clone().unwrap_or_default(), + created_at: doc.created_at as u64, + updated_at: doc.updated_at as u64, + version: doc.version as u32, + description: meta.as_ref().and_then(|m| m.description.clone()), + favicon: meta.as_ref().and_then(|m| m.favicon.clone()), + sender: meta.as_ref().and_then(|m| m.sender.clone()), + recipient: meta.as_ref().and_then(|m| m.recipient.clone()), + thread_id: meta.as_ref().and_then(|m| m.thread_id.clone()), + reply_to: meta.as_ref().and_then(|m| m.reply_to.clone()), + } + } + + fn meta_to_metadata( + prev: Option<&ArtifactMeta>, + req: &PublishRequest, + new_version: u32, + now: i64, + ) -> String { + let keep = |new: &Option, old: fn(&ArtifactMeta) -> Option| { + new.clone().or_else(|| prev.and_then(old)) + }; + let mut history = prev.map(|m| m.history.clone()).unwrap_or_default(); + history.push(VersionInfo { + version: new_version, + label: req.label.clone(), + created_at: now as u64, + }); + let meta = ArtifactMeta { + id: String::new(), + name: req.name.clone(), + artifact_type: req.artifact_type.clone(), + session_id: req.session_id.clone(), + created_at: prev.map(|m| m.created_at).unwrap_or(now as u64), + updated_at: now as u64, + version: new_version, + description: keep(&req.description, |m| m.description.clone()), + favicon: keep(&req.favicon, |m| m.favicon.clone()), + history, + sender: keep(&req.sender, |m| m.sender.clone()), + recipient: keep(&req.recipient, |m| m.recipient.clone()), + thread_id: keep(&req.thread_id, |m| m.thread_id.clone()), + reply_to: keep(&req.reply_to, |m| m.reply_to.clone()), + git: req.git.clone().or_else(|| prev.and_then(|m| m.git.clone())), + }; + serde_json::to_string(&meta).unwrap_or_else(|_| "{}".into()) + } + pub fn publish(&self, req: &PublishRequest) -> std::io::Result { + if let Some(ref store) = self.store { + return self.publish_store(store, req); + } + self.publish_flat(req) + } + + fn publish_store( + &self, + store: &Store, + req: &PublishRequest, + ) -> std::io::Result { + let id = req.update_id.clone().unwrap_or_else(|| nanoid::nanoid!()); + let now = db_kit::ids::now(); + let path = &id; + + // Read existing doc to check CAS and dedup + let existing = store + .doc_get_by_path(DOC_PROJECT, path) + .map_err(Self::store_conn_err)?; + + if let (Some(base), Some(doc)) = (req.base_version, existing.as_ref()) { + if base as i32 != doc.version { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!( + "version conflict: base_version {base}, but current version is {}", + doc.version + ), + )); + } + } + + let unchanged = existing.as_ref().is_some_and(|doc| { + if !doc.content.is_empty() { + doc.content == req.content + } else { + let new_hash = blake3::hash(req.content.as_bytes()).to_hex().to_string(); + doc.blob_hash.as_deref() == Some(&new_hash) + } + }); + + if !unchanged { + // Store content as blob for size efficiency + let blob_hash = store + .blob_store(req.content.as_bytes()) + .map_err(Self::store_conn_err)?; + + let prev_meta: Option = existing + .as_ref() + .and_then(|doc| serde_json::from_str(&doc.metadata).ok()); + + let next_version = existing.as_ref().map(|d| d.version as u32 + 1).unwrap_or(1); + let metadata = Self::meta_to_metadata(prev_meta.as_ref(), req, next_version, now); + + store + .doc_upsert_with_opts( + DOC_PROJECT, + path, + "", + DocUpsertOpts { + title: Some(req.name.clone()), + doc_type: Some("artifact".into()), + blob_hash: Some(blob_hash), + mime: Some(req.artifact_type.mime_type().into()), + tags: Some(vec![req.artifact_type.to_string()]), + session_id: Some(req.session_id.clone()), + source: Some("artifact".into()), + metadata: Some(metadata), + size: Some(req.content.len() as i64), + ..Default::default() + }, + ) + .map_err(Self::store_conn_err)?; + } + + let doc = store + .doc_get_by_path(DOC_PROJECT, path) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "artifact not found after upsert", + ) + })?; + + let version = doc.version as u32; + let response = PublishResponse { + id: id.clone(), + url: format!("/{id}"), + session_id: req.session_id.clone(), + version, + }; + + if existing.is_some() && !unchanged { + self.broadcast(&id, "refresh"); + } + + Ok(response) + } + + fn publish_flat(&self, req: &PublishRequest) -> std::io::Result { let id = req .update_id .clone() @@ -92,8 +297,6 @@ impl ArtifactStore { } } - // Dedupe: an update whose content is byte-identical refreshes - // metadata in place — no new snapshot, no bump, no broadcast. let dir = self.artifact_dir(&id); let unchanged = prev.is_some() && fs::read_to_string(dir.join(CONTENT_FILE)) @@ -114,7 +317,6 @@ impl ArtifactStore { }); } - // Omitted optional fields on an update keep their old value. let keep = |new: &Option, old: fn(&ArtifactMeta) -> Option| { new.clone().or_else(|| prev.as_ref().and_then(old)) }; @@ -167,6 +369,15 @@ impl ArtifactStore { /// Unified diff between two version snapshots of an artifact. pub fn diff(&self, id: &str, from: u32, to: u32) -> std::io::Result { + if let Some(ref store) = self.store { + let old = self.get_version_store(store, id, from)?; + let new = self.get_version_store(store, id, to)?; + let diff = similar::TextDiff::from_lines(&old.content, &new.content); + return Ok(diff + .unified_diff() + .header(&format!("{id} v{from}"), &format!("{id} v{to}")) + .to_string()); + } let versions_dir = self.artifact_dir(id).join(VERSIONS_DIR); let old = read_version_file(&versions_dir.join(from.to_string()))?; let new = read_version_file(&versions_dir.join(to.to_string()))?; @@ -179,6 +390,20 @@ impl ArtifactStore { /// Version history, oldest first. pub fn versions(&self, id: &str) -> std::io::Result> { + if let Some(ref store) = self.store { + let doc = store + .doc_get_by_path(DOC_PROJECT, id) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("artifact {id} not found"), + ) + })?; + let meta: ArtifactMeta = serde_json::from_str(&doc.metadata) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + return Ok(meta.history); + } self.read_meta(id).map(|m| m.history).ok_or_else(|| { std::io::Error::new( std::io::ErrorKind::NotFound, @@ -187,8 +412,111 @@ impl ArtifactStore { }) } + fn get_version_store( + &self, + store: &Store, + id: &str, + version: u32, + ) -> std::io::Result { + let doc = store + .doc_get_by_path(DOC_PROJECT, id) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("artifact {id} not found"), + ) + })?; + let latest = Self::doc_to_artifact(&doc)?; + if doc.version as u32 == version { + let content = Self::blob_or_content(store, &doc)?; + return Ok(Artifact { content, ..latest }); + } + let hist = store.doc_history(&doc.id).map_err(Self::store_conn_err)?; + for h in hist { + if h.version as u32 == version { + let content = match &h.blob_hash { + Some(hash) => { + let bytes = store + .blob_get(hash) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::NotFound, "blob not found") + })?; + String::from_utf8(bytes) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))? + } + None => h.content, + }; + return Ok(Artifact { + content, + version, + ..latest + }); + } + } + Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("version {version} not found"), + )) + } + + fn blob_or_content( + store: &Store, + doc: &agentflare_store::documents::Document, + ) -> std::io::Result { + match &doc.blob_hash { + Some(hash) => { + let bytes = store + .blob_get(hash) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new(std::io::ErrorKind::NotFound, "blob not found") + })?; + String::from_utf8(bytes) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e)) + } + None => Ok(doc.content.clone()), + } + } + + fn get_store(&self, store: &Store, id: &str) -> std::io::Result { + let doc = store + .doc_get_by_path(DOC_PROJECT, id) + .map_err(Self::store_conn_err)? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("artifact {id} not found"), + ) + })?; + let content = Self::blob_or_content(store, &doc)?; + let meta: ArtifactMeta = serde_json::from_str(&doc.metadata) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + Ok(Artifact { + id: doc.path.clone(), + name: doc.title.clone(), + artifact_type: meta.artifact_type, + content, + session_id: doc.session_id.clone().unwrap_or_default(), + created_at: doc.created_at as u64, + updated_at: doc.updated_at as u64, + version: doc.version as u32, + description: meta.description, + favicon: meta.favicon, + sender: meta.sender, + recipient: meta.recipient, + thread_id: meta.thread_id, + reply_to: meta.reply_to, + git: meta.git, + }) + } + /// A specific version's snapshot; `get()` always serves the latest. pub fn get_version(&self, id: &str, version: u32) -> std::io::Result { + if let Some(ref store) = self.store { + return self.get_version_store(store, id, version); + } let mut artifact = self.get(id)?; let content_path = self .artifact_dir(id) @@ -200,6 +528,9 @@ impl ArtifactStore { } pub fn get(&self, id: &str) -> std::io::Result { + if let Some(ref store) = self.store { + return self.get_store(store, id); + } let dir = self.artifact_dir(id); if !dir.exists() { return Err(std::io::Error::new( @@ -229,6 +560,16 @@ impl ArtifactStore { } pub fn list(&self, session_id: Option<&str>) -> std::io::Result> { + if let Some(ref store) = self.store { + let docs = store.doc_list(DOC_PROJECT).map_err(Self::store_conn_err)?; + let mut artifacts: Vec = docs + .iter() + .filter(|d| session_id.is_none_or(|sid| d.session_id.as_deref() == Some(sid))) + .map(Self::doc_to_summary) + .collect(); + artifacts.sort_by_key(|a| std::cmp::Reverse(a.created_at)); + return Ok(artifacts); + } let dir = &self.base_path; if !dir.exists() { return Ok(vec![]); @@ -267,6 +608,15 @@ impl ArtifactStore { } pub fn delete(&self, id: &str) -> std::io::Result { + if let Some(ref store) = self.store { + let doc = store + .doc_get_by_path(DOC_PROJECT, id) + .map_err(Self::store_conn_err)?; + return match doc { + Some(d) => store.doc_delete(&d.id).map_err(Self::store_conn_err), + None => Ok(false), + }; + } let dir = self.artifact_dir(id); if !dir.exists() { return Ok(false); @@ -359,6 +709,14 @@ mod tests { (tmp, store) } + fn doc_store() -> (tempfile::TempDir, ArtifactStore) { + let tmp = tempfile::tempdir().unwrap(); + let db_path = tmp.path().join("store.db"); + let s = agentflare_store::Store::open_file(&db_path).unwrap(); + let store = ArtifactStore::with_store(s); + (tmp, store) + } + fn publish(store: &ArtifactStore, update_id: Option, content: &str) -> String { store .publish(&PublishRequest { @@ -463,4 +821,99 @@ mod tests { "legacy plaintext, no gzip header" ); } + + // ── store-backed (agentflare_store) tests ── + + #[test] + fn doc_store_publish_and_get() { + let (_tmp, store) = doc_store(); + assert!(store.has_store()); + let id = publish(&store, None, "hello from doc store"); + let artifact = store.get(&id).unwrap(); + assert_eq!(artifact.content, "hello from doc store"); + assert_eq!(artifact.version, 1); + } + + #[test] + fn doc_store_update_increments_version() { + let (_tmp, store) = doc_store(); + let id = publish(&store, None, "v1"); + assert_eq!(store.get(&id).unwrap().version, 1); + + let id2 = publish(&store, Some(id.clone()), "v2"); + assert_eq!(id, id2); + assert_eq!(store.get(&id).unwrap().content, "v2"); + assert_eq!(store.get(&id).unwrap().version, 2); + } + + #[test] + fn doc_store_versions_and_history() { + let (_tmp, store) = doc_store(); + let id = publish(&store, None, "v1"); + publish(&store, Some(id.clone()), "v2"); + publish(&store, Some(id.clone()), "v3"); + + let history = store.versions(&id).unwrap(); + assert_eq!(history.len(), 3); + assert_eq!(history[0].version, 1); + assert_eq!(history[1].version, 2); + assert_eq!(history[2].version, 3); + + assert_eq!(store.get_version(&id, 1).unwrap().content, "v1"); + assert_eq!(store.get_version(&id, 2).unwrap().content, "v2"); + assert_eq!(store.get_version(&id, 3).unwrap().content, "v3"); + } + + #[test] + fn doc_store_list_by_session() { + let (_tmp, store) = doc_store(); + let s1 = PublishRequest { + name: "s1-doc".into(), + artifact_type: ArtifactType::Text, + content: "a".into(), + session_id: "session-A".into(), + ..Default::default() + }; + let s2 = PublishRequest { + name: "s2-doc".into(), + artifact_type: ArtifactType::Text, + content: "b".into(), + session_id: "session-B".into(), + ..Default::default() + }; + store.publish(&s1).unwrap(); + store.publish(&s2).unwrap(); + + let all = store.list(None).unwrap(); + assert_eq!(all.len(), 2); + + let only_a = store.list(Some("session-A")).unwrap(); + assert_eq!(only_a.len(), 1); + assert_eq!(only_a[0].name, "s1-doc"); + + let only_b = store.list(Some("session-B")).unwrap(); + assert_eq!(only_b.len(), 1); + } + + #[test] + fn doc_store_delete() { + let (_tmp, store) = doc_store(); + let id = publish(&store, None, "to-delete"); + assert!(store.get(&id).is_ok()); + assert!(store.delete(&id).unwrap()); + assert!(store.get(&id).is_err()); + // second delete returns false + assert!(!store.delete(&id).unwrap()); + } + + #[test] + fn doc_store_diff_between_versions() { + let (_tmp, store) = doc_store(); + let id = publish(&store, None, "line one\nline two\n"); + publish(&store, Some(id.clone()), "line one\nline two modified\n"); + + let d = store.diff(&id, 1, 2).unwrap(); + assert!(d.contains("line two")); + assert!(d.contains("line two modified")); + } } diff --git a/crates/agentflare-store/src/documents.rs b/crates/agentflare-store/src/documents.rs index 44407a69..719d9abd 100644 --- a/crates/agentflare-store/src/documents.rs +++ b/crates/agentflare-store/src/documents.rs @@ -183,10 +183,15 @@ impl Store { let new_version = old_version + 1; // Snapshot current version to history, unless the caller opted - // out (cache-type documents) or content is byte-identical to - // what's already stored (a no-op re-upsert has nothing to - // snapshot). - if opts.track_history && old_content != content { + // out (cache-type documents) or nothing actually changed. Blob- + // backed callers (e.g. agentflare-artifacts) always pass an + // empty `content` string here -- the real payload lives in + // `blob_hash` -- so comparing inline `content` alone would never + // detect a change and history would never record; also check + // `blob_hash` so a no-op re-upsert still skips (both unchanged) + // while a genuine new blob still snapshots (content stays ""). + let unchanged = old_content == content && old_blob_hash == opts.blob_hash; + if opts.track_history && !unchanged { let history_id = db_kit::ids::new_id(); tx.execute( "INSERT INTO store_doc_history (id, doc_id, version, content, blob_hash, mime, title, metadata, size, created_at) diff --git a/src/artifacts.rs b/src/artifacts.rs index 4ea84deb..db60a562 100644 --- a/src/artifacts.rs +++ b/src/artifacts.rs @@ -2,13 +2,22 @@ use agentflare_artifacts::{ArtifactServer, ArtifactStore}; use std::sync::Arc; pub fn serve(host: &str, port: u16, dir: Option) { - let dir = dir.unwrap_or_else(|| crate::paths::home().join(".agentflare").join("artifacts")); - let store = Arc::new(ArtifactStore::new(dir.clone())); + let store = if let Some(d) = dir { + Arc::new(ArtifactStore::new(d)) + } else { + match crate::store::open() { + Ok(s) => Arc::new(ArtifactStore::with_store(s)), + Err(e) => { + eprintln!("[artifacts] fallback to flat-file store: {e}"); + let d = crate::paths::home().join(".agentflare").join("artifacts"); + Arc::new(ArtifactStore::new(d)) + } + } + }; let server = ArtifactServer::start_on(store, host, port).expect("failed to start artifact server"); let url = server.base_url(); crate::ui::info(&format!("agentflare artifacts server listening on {url}")); - crate::ui::info(&format!(" store: {}", dir.display())); if host != "127.0.0.1" && host != "localhost" { crate::ui::warning(&format!( "bound to {host} — anyone on your network can view these artifacts" diff --git a/src/cli/coaching.rs b/src/cli/coaching.rs index 18d7156d..38690ba2 100644 --- a/src/cli/coaching.rs +++ b/src/cli/coaching.rs @@ -1,3 +1,4 @@ +use crate::coaching::rule::RuleTier; use clap::{Args, Subcommand}; #[derive(Subcommand)] @@ -18,10 +19,24 @@ pub enum CoachingAction { /// to maintain. #[arg(long = "trigger-auto")] trigger_auto: bool, + /// Rule tier: override (default) or builtin. Builtin rules are + /// re-materialized by `agentflare init` so user edits are not lost. + #[arg(long)] + tier: Option, + /// Host to sync/coach this rule to (comma- or repeatable). Known values: + /// claude-code, opencode, cursor, codex, windsurf, vscode-copilot, cline. + #[arg(long, value_delimiter = ',')] + sync: Vec, }, Remove { id: String, }, + /// Regenerate every synced rule file for one host, or all of them. + Sync { + /// Agent host to sync rules for (default: all known hosts). + #[arg(long)] + agent: Option, + }, } #[derive(Args)] @@ -40,8 +55,19 @@ impl CoachingArgs { body, trigger_tool, trigger_auto, - } => crate::coaching::cli_apply(&id, &title, &body, trigger_tool, trigger_auto), + tier, + sync, + } => crate::coaching::cli_apply( + &id, + &title, + &body, + trigger_tool, + trigger_auto, + tier, + sync, + ), CoachingAction::Remove { id } => crate::coaching::cli_remove(&id), + CoachingAction::Sync { agent } => crate::coaching::cli_sync(agent.as_deref()), } } } diff --git a/src/cli/handoff.rs b/src/cli/handoff.rs index 0d066971..cb8d4f0b 100644 --- a/src/cli/handoff.rs +++ b/src/cli/handoff.rs @@ -89,10 +89,18 @@ impl HandoffArgs { .or_else(agent_detector::agent_name) .unwrap_or_else(|| "cli".into()); - let dir = self - .dir - .unwrap_or_else(|| crate::paths::home().join(".agentflare").join("artifacts")); - let store = agentflare_artifacts::ArtifactStore::new(dir); + let store: agentflare_artifacts::ArtifactStore = match self.dir.clone() { + Some(d) => agentflare_artifacts::ArtifactStore::new(d), + None => match crate::store::open() { + Ok(s) => agentflare_artifacts::ArtifactStore::with_store(s), + Err(e) => { + eprintln!("[handoff] fallback to flat-file store: {e}"); + agentflare_artifacts::ArtifactStore::new( + crate::paths::home().join(".agentflare").join("artifacts"), + ) + } + }, + }; let req = agentflare_artifacts::PublishRequest { name: self.name.or(stem).unwrap_or_else(|| "handoff".into()), artifact_type: agentflare_artifacts::ArtifactType::from( diff --git a/src/coaching/cli.rs b/src/coaching/cli.rs index 676089f9..cdcbfd57 100644 --- a/src/coaching/cli.rs +++ b/src/coaching/cli.rs @@ -1,8 +1,18 @@ -//! CLI-facing presentation for `agentflare coaching {list,apply,remove}`. +//! CLI-facing presentation for `agentflare coaching {list,apply,remove,sync}`. -use super::rule::RuleTrigger; +use super::rule::{RuleTier, RuleTrigger}; use super::store::{self, MAX_RULES}; +const ALL_HOSTS: &[&str] = &[ + "claude-code", + "opencode", + "cursor", + "codex", + "windsurf", + "vscode-copilot", + "cline", +]; + fn describe_trigger(trigger: Option<&RuleTrigger>) -> String { match trigger { None => "no trigger — always shown at SessionStart".to_string(), @@ -29,7 +39,24 @@ pub fn print_list() { } println!("agentflare coaching rules ({}/{MAX_RULES}):\n", rules.len()); for r in &rules { - println!(" {:<10} {} (applied {})", r.id, r.title, r.applied_at); + if !r.sync.is_empty() { + println!( + " {:<10} {} ({}, applied {}, synced: {})", + r.id, + r.title, + r.tier.as_str(), + r.applied_at, + r.sync.join(", ") + ); + } else { + println!( + " {:<10} {} ({}, applied {}, no sync)", + r.id, + r.title, + r.tier.as_str(), + r.applied_at + ); + } println!(" {}", r.body); println!(" {}", describe_trigger(r.trigger.as_ref())); } @@ -41,6 +68,8 @@ pub fn cli_apply( body: &str, trigger_tools: Vec, trigger_auto: bool, + tier: Option, + sync: Vec, ) { let trigger = if trigger_tools.is_empty() && !trigger_auto { None @@ -50,8 +79,18 @@ pub fn cli_apply( auto_match: trigger_auto, }) }; - match store::apply_rule(id, title, body, trigger) { - Ok(rule) => println!("Applied coaching rule '{}': {}", rule.id, rule.title), + let tier = tier.unwrap_or(RuleTier::Override); + let sync_hosts = sync.clone(); + match store::apply_rule(id, title, body, trigger, tier, sync) { + Ok(rule) => { + println!("Applied coaching rule '{}': {}", rule.id, rule.title); + for host in &sync_hosts { + match crate::components::sync_now(host) { + Ok(msg) => println!(" synced to {host}: {msg}"), + Err(e) => crate::ui::error(&format!(" failed to sync to {host}: {e}")), + } + } + } Err(e) => { crate::ui::error(&format!("agentflare coaching apply: {e}")); std::process::exit(1); @@ -61,10 +100,31 @@ pub fn cli_apply( pub fn cli_remove(id: &str) { match store::remove_rule(id) { - Ok(()) => crate::ui::success(&format!("Removed coaching rule '{id}'.")), + Ok(sync) => { + for host in &sync { + if let Err(e) = crate::components::unsync_host(id, host) { + crate::ui::error(&format!("failed to unsync rule '{id}' from {host}: {e}")); + std::process::exit(1); + } + } + crate::ui::success(&format!("Removed coaching rule '{id}'.")); + } Err(e) => { crate::ui::error(&format!("agentflare coaching remove: {e}")); std::process::exit(1); } } } + +pub fn cli_sync(agent: Option<&str>) { + let targets: Vec<&str> = match agent { + Some(a) => vec![a], + None => ALL_HOSTS.to_vec(), + }; + for host in targets { + match crate::components::sync_now(host) { + Ok(msg) => println!("{host}: {msg}"), + Err(e) => crate::ui::error(&format!("{host}: {e}")), + } + } +} diff --git a/src/coaching/mod.rs b/src/coaching/mod.rs index 959fd415..af492d20 100644 --- a/src/coaching/mod.rs +++ b/src/coaching/mod.rs @@ -15,8 +15,11 @@ mod cli; pub(crate) mod rule; mod store; -pub use cli::{cli_apply, cli_remove, print_list}; -pub use store::{rule_bodies_for_prompt, rule_bodies_for_tool, untriggered_rule_bodies}; +pub use cli::{cli_apply, cli_remove, cli_sync, print_list}; +pub use store::{ + rule_bodies_for_prompt, rule_bodies_for_tool, superseded_bodies, sync_targets_for_host, + untriggered_rule_bodies, +}; // Only reached from hook.rs's SessionStart test (#[cfg(test)]), to seed a // rule before asserting it appears in the printed message — a plain, diff --git a/src/coaching/rule.rs b/src/coaching/rule.rs index 2cf7ad56..fbfd951e 100644 --- a/src/coaching/rule.rs +++ b/src/coaching/rule.rs @@ -1,6 +1,31 @@ //! Coaching rule data model: the `CoachingRule` struct and the //! `coaching-.md` file format (parsing + serialization). +/// Whether a rule ships as an agentflare default (drift-protected across +/// version bumps) or is a user override (always wins, always overwrites). +#[derive(Debug, Clone, PartialEq, clap::ValueEnum)] +pub enum RuleTier { + Builtin, + Override, +} + +impl RuleTier { + pub fn as_str(&self) -> &'static str { + match self { + RuleTier::Builtin => "builtin", + RuleTier::Override => "override", + } + } + + pub fn parse(s: &str) -> Option { + match s.trim().to_lowercase().as_str() { + "builtin" => Some(RuleTier::Builtin), + "override" => Some(RuleTier::Override), + _ => None, + } + } +} + /// A coaching rule loaded from a `coaching-.md` file. #[derive(Debug)] pub struct CoachingRule { @@ -9,6 +34,8 @@ pub struct CoachingRule { pub body: String, pub applied_at: String, pub trigger: Option, + pub tier: RuleTier, + pub sync: Vec, } /// Declares when a rule should fire contextually instead of at every @@ -33,6 +60,16 @@ pub(super) fn is_valid_rule_id(id: &str) -> bool { && id.chars().all(|c| c.is_ascii_alphanumeric() || c == '-') } +const KNOWN_SYNC_HOSTS: &[&str] = &[ + "claude-code", + "opencode", + "cursor", + "codex", + "windsurf", + "vscode-copilot", + "cline", +]; + /// Validates fields that will be serialized into a rule file's header. /// A title containing a newline would break the one-line-per-header-field /// format; commas/semicolons or newlines in a tool name would corrupt the @@ -43,10 +80,19 @@ pub(super) fn is_valid_rule_id(id: &str) -> bool { pub(super) fn validate_rule_fields( title: &str, trigger: Option<&RuleTrigger>, + sync: &[String], ) -> Result<(), String> { if title.contains('\n') { return Err("rule title must not contain newlines".to_string()); } + for host in sync { + if !KNOWN_SYNC_HOSTS.contains(&host.as_str()) { + return Err(format!( + "invalid sync host '{host}': must be one of {}", + KNOWN_SYNC_HOSTS.join(", ") + )); + } + } let Some(trigger) = trigger else { return Ok(()); }; @@ -133,6 +179,8 @@ pub(super) fn parse_rule_file(path: &std::path::Path) -> Option { let mut title = String::new(); let mut applied_at = String::new(); let mut trigger = None; + let mut tier = RuleTier::Override; + let mut sync = Vec::new(); let mut in_header = false; let mut header_done = false; let mut body_lines = Vec::new(); @@ -159,6 +207,16 @@ pub(super) fn parse_rule_file(path: &std::path::Path) -> Option { "[agentflare] coaching: malformed or empty Trigger line, treating as untriggered: {rest:?}" ); } + } else if let Some(rest) = line.strip_prefix("# Tier:") { + if let Some(t) = RuleTier::parse(rest) { + tier = t; + } + } else if let Some(rest) = line.strip_prefix("# Sync:") { + sync = rest + .split(',') + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect(); } } else if !line.is_empty() { body_lines.push(line); @@ -175,6 +233,8 @@ pub(super) fn parse_rule_file(path: &std::path::Path) -> Option { body: body_lines.join(" "), applied_at, trigger, + tier, + sync, }) } @@ -184,6 +244,8 @@ pub(super) fn write_rule_file( title: &str, body: &str, trigger: Option<&RuleTrigger>, + tier: RuleTier, + sync: &[String], ) -> std::io::Result<()> { std::fs::create_dir_all(dir)?; let date = chrono::Local::now().date_naive(); @@ -191,8 +253,14 @@ pub(super) fn write_rule_file( Some(t) => format!("\n# Trigger: {}", format_trigger_line(t)), None => String::new(), }; + let sync_line = if sync.is_empty() { + String::new() + } else { + format!("\n# Sync: {}", sync.join(", ")) + }; let content = format!( - "---\n# Pattern: {id} \u{2014} {title}\n# Applied: {date}{trigger_line}\n---\n\n{body}\n" + "---\n# Pattern: {id} \u{2014} {title}\n# Applied: {date}{trigger_line}\n# Tier: {}{sync_line}\n---\n\n{body}\n", + tier.as_str() ); let final_path = dir.join(format!("coaching-{id}.md")); let tmp_path = dir.join(format!("coaching-{id}.md.tmp")); @@ -222,8 +290,8 @@ mod tests { #[test] fn validate_rule_fields_rejects_newline_in_title() { - assert!(validate_rule_fields("bad\ntitle", None).is_err()); - assert!(validate_rule_fields("fine title", None).is_ok()); + assert!(validate_rule_fields("bad\ntitle", None, &[]).is_err()); + assert!(validate_rule_fields("fine title", None, &[]).is_ok()); } #[test] @@ -232,7 +300,7 @@ mod tests { tools: vec![], auto_match: false, }; - assert!(validate_rule_fields("Title", Some(&empty)).is_err()); + assert!(validate_rule_fields("Title", Some(&empty), &[]).is_err()); } #[test] @@ -241,13 +309,19 @@ mod tests { tools: vec!["a,b".to_string()], auto_match: false, }; - assert!(validate_rule_fields("Title", Some(&bad)).is_err()); + assert!(validate_rule_fields("Title", Some(&bad), &[]).is_err()); let ok = RuleTrigger { tools: vec!["mcp__flare__review".to_string()], auto_match: false, }; - assert!(validate_rule_fields("Title", Some(&ok)).is_ok()); + assert!(validate_rule_fields("Title", Some(&ok), &[]).is_ok()); + } + + #[test] + fn validate_rule_fields_rejects_unknown_sync_host() { + assert!(validate_rule_fields("Title", None, &["unknown".to_string()]).is_err()); + assert!(validate_rule_fields("Title", None, &["claude-code".to_string()]).is_ok()); } #[test] @@ -337,11 +411,15 @@ mod tests { "Reviews ship with fixes", "Body text", Some(&trigger), + RuleTier::Override, + &[], ) .unwrap(); let rule = parse_rule_file(&dir.join("coaching-revfix.md")).unwrap(); assert_eq!(rule.trigger, Some(trigger)); + assert_eq!(rule.tier, RuleTier::Override); + assert!(rule.sync.is_empty()); std::fs::remove_dir_all(&dir).unwrap(); } @@ -349,7 +427,16 @@ mod tests { #[test] fn write_then_parse_roundtrips_no_trigger() { let dir = temp_dir_for_test(); - write_rule_file(&dir, "hygiene", "Title", "Body", None).unwrap(); + write_rule_file( + &dir, + "hygiene", + "Title", + "Body", + None, + RuleTier::Override, + &[], + ) + .unwrap(); let rule = parse_rule_file(&dir.join("coaching-hygiene.md")).unwrap(); assert_eq!(rule.trigger, None); @@ -357,10 +444,59 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + #[test] + fn write_then_parse_roundtrips_tier_and_sync() { + let dir = temp_dir_for_test(); + write_rule_file( + &dir, + "search17", + "Search", + "Body", + None, + RuleTier::Builtin, + &["claude-code".to_string(), "opencode".to_string()], + ) + .unwrap(); + + let rule = parse_rule_file(&dir.join("coaching-search17.md")).unwrap(); + assert_eq!(rule.tier, RuleTier::Builtin); + assert_eq!( + rule.sync, + vec!["claude-code".to_string(), "opencode".to_string()] + ); + + std::fs::remove_dir_all(&dir).unwrap(); + } + + #[test] + fn parse_old_file_without_tier_sync_defaults_to_override() { + let dir = temp_dir_for_test(); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("coaching-old.md"), + "---\n# Pattern: old \u{2014} Old\n# Applied: 2026-01-01\n---\n\nBody\n", + ) + .unwrap(); + let rule = parse_rule_file(&dir.join("coaching-old.md")).unwrap(); + assert_eq!(rule.tier, RuleTier::Override); + assert!(rule.sync.is_empty()); + + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn parse_rule_file_skips_file_with_invalid_id_in_filename() { let dir = temp_dir_for_test(); - write_rule_file(&dir, "hygiene", "Title", "Body", None).unwrap(); + write_rule_file( + &dir, + "hygiene", + "Title", + "Body", + None, + RuleTier::Override, + &[], + ) + .unwrap(); std::fs::create_dir_all(&dir).unwrap(); std::fs::write( dir.join("coaching-not a valid id.md"), diff --git a/src/coaching/store.rs b/src/coaching/store.rs index ae0a0707..31924d2f 100644 --- a/src/coaching/store.rs +++ b/src/coaching/store.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; -use super::rule::{self, CoachingRule}; +use super::rule::{self, CoachingRule, RuleTier}; pub(super) const MAX_RULES: usize = 8; @@ -86,13 +86,15 @@ pub fn apply_rule( title: &str, body: &str, trigger: Option, + tier: RuleTier, + sync: Vec, ) -> Result { if !rule::is_valid_rule_id(id) { return Err(format!( "invalid rule id '{id}': must be 1-10 chars, start with a letter, and contain only letters, digits, or hyphens" )); } - rule::validate_rule_fields(title, trigger.as_ref())?; + rule::validate_rule_fields(title, trigger.as_ref(), &sync)?; let _lock = RulesLock::acquire(&rules_dir()) .map_err(|e| format!("failed to acquire rules lock: {e}"))?; @@ -105,8 +107,25 @@ pub fn apply_rule( )); } - rule::write_rule_file(&rules_dir(), id, title, body, trigger.as_ref()) - .map_err(|e| format!("failed to write rule file: {e}"))?; + if is_overwrite + && tier == rule::RuleTier::Builtin + && let Some(prev) = existing.iter().find(|r| r.id == id) + && prev.body != body + { + snapshot_previous_body(id, &prev.body) + .map_err(|e| format!("failed to snapshot previous rule body: {e}"))?; + } + + rule::write_rule_file( + &rules_dir(), + id, + title, + body, + trigger.as_ref(), + tier.clone(), + &sync, + ) + .map_err(|e| format!("failed to write rule file: {e}"))?; list_rules() .into_iter() @@ -114,8 +133,9 @@ pub fn apply_rule( .ok_or_else(|| "rule written but could not be re-read".to_string()) } -/// Remove a coaching rule file by id. -pub(super) fn remove_rule(id: &str) -> Result<(), String> { +/// Remove a coaching rule file by id. Returns the rule's sync hosts so the +/// caller can clean up generated files for those hosts (see components::unsync_host). +pub(super) fn remove_rule(id: &str) -> Result, String> { if !rule::is_valid_rule_id(id) { return Err(format!("invalid rule id '{id}'")); } @@ -125,7 +145,55 @@ pub(super) fn remove_rule(id: &str) -> Result<(), String> { if !path.exists() { return Err(format!("rule not found: {id}")); } - std::fs::remove_file(&path).map_err(|e| format!("failed to remove rule file: {e}")) + let sync = list_rules() + .into_iter() + .find(|r| r.id == id) + .map(|r| r.sync) + .unwrap_or_default(); + std::fs::remove_file(&path).map_err(|e| format!("failed to remove rule file: {e}"))?; + Ok(sync) +} + +/// (id, body) pairs for every rule in the store that is tagged to sync +/// to `host`, in rule-id order. Callers (components::rule_targets) build +/// the actual per-host file path themselves. +pub fn sync_targets_for_host(host: &str) -> Vec<(String, String)> { + list_rules() + .into_iter() + .filter(|r| r.sync.iter().any(|h| h == host)) + .map(|r| (r.id, r.body)) + .collect() +} + +/// Appends `previous_body` to `~/.agentflare/rules/superseded-.json` +/// (creating it if absent) — the coaching-store equivalent of +/// `rule_text::superseded()`'s hardcoded lists, built automatically +/// instead of hand-maintained, so `init::is_stale_rule` can tell "still +/// has a body we shipped before" from "user edited this" for builtin-tier +/// coaching rules the same way it already does for `rule_text.rs` consts. +fn snapshot_previous_body(id: &str, previous_body: &str) -> std::io::Result<()> { + let path = rules_dir().join(format!("superseded-{id}.json")); + let mut bodies: Vec = std::fs::read_to_string(&path) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()) + .unwrap_or_default(); + if !bodies.iter().any(|b| b == previous_body) { + bodies.push(previous_body.to_string()); + } + std::fs::create_dir_all(rules_dir())?; + std::fs::write(&path, serde_json::to_string(&bodies).unwrap_or_default()) +} + +/// Known old bodies for a coaching-sourced builtin rule, by id — the +/// coaching-store analogue of `rule_text::superseded(filename)`, built +/// automatically instead of hand-maintained, so `init::is_stale_rule` can +/// tell "still has a body we shipped before" from "user edited this". +pub fn superseded_bodies(id: &str) -> Vec { + let path = rules_dir().join(format!("superseded-{id}.json")); + std::fs::read_to_string(&path) + .ok() + .and_then(|s| serde_json::from_str(&s).ok()) + .unwrap_or_default() } /// Bodies of rules with no Trigger declared, in id order, what @@ -202,6 +270,8 @@ mod tests { "Close sessions promptly", "Wrap up each phase before starting the next.", None, + RuleTier::Override, + vec![], ) .unwrap(); assert_eq!(applied.id, "hygiene"); @@ -217,7 +287,8 @@ mod tests { #[test] fn apply_rule_rejects_invalid_id() { with_temp_home(|| { - let err = apply_rule("1bad", "Title", "Body", None).unwrap_err(); + let err = + apply_rule("1bad", "Title", "Body", None, RuleTier::Override, vec![]).unwrap_err(); assert!(err.contains("invalid rule id")); assert!(list_rules().is_empty()); }); @@ -226,7 +297,15 @@ mod tests { #[test] fn apply_rule_rejects_newline_in_title() { with_temp_home(|| { - let err = apply_rule("hygiene", "bad\ntitle", "Body", None).unwrap_err(); + let err = apply_rule( + "hygiene", + "bad\ntitle", + "Body", + None, + RuleTier::Override, + vec![], + ) + .unwrap_err(); assert!(err.contains("newline")); assert!(list_rules().is_empty()); }); @@ -239,7 +318,15 @@ mod tests { tools: vec![], auto_match: false, }; - let err = apply_rule("hygiene", "Title", "Body", Some(empty)).unwrap_err(); + let err = apply_rule( + "hygiene", + "Title", + "Body", + Some(empty), + RuleTier::Override, + vec![], + ) + .unwrap_err(); assert!(err.contains("empty trigger")); assert!(list_rules().is_empty()); }); @@ -249,9 +336,10 @@ mod tests { fn apply_rule_enforces_max_rules_for_new_ids() { with_temp_home(|| { for i in 0..MAX_RULES { - apply_rule(&format!("r{i}"), "T", "B", None).unwrap(); + apply_rule(&format!("r{i}"), "T", "B", None, RuleTier::Override, vec![]).unwrap(); } - let err = apply_rule("one-more", "T", "B", None).unwrap_err(); + let err = + apply_rule("one-more", "T", "B", None, RuleTier::Override, vec![]).unwrap_err(); assert!(err.contains("maximum")); assert_eq!(list_rules().len(), MAX_RULES); }); @@ -261,9 +349,17 @@ mod tests { fn apply_rule_allows_overwriting_existing_id_at_capacity() { with_temp_home(|| { for i in 0..MAX_RULES { - apply_rule(&format!("r{i}"), "T", "B", None).unwrap(); + apply_rule(&format!("r{i}"), "T", "B", None, RuleTier::Override, vec![]).unwrap(); } - let updated = apply_rule("r0", "New Title", "New Body", None).unwrap(); + let updated = apply_rule( + "r0", + "New Title", + "New Body", + None, + RuleTier::Override, + vec![], + ) + .unwrap(); assert_eq!(updated.title, "New Title"); assert_eq!(list_rules().len(), MAX_RULES); }); @@ -272,7 +368,7 @@ mod tests { #[test] fn remove_rule_deletes_existing_file() { with_temp_home(|| { - apply_rule("hygiene", "T", "B", None).unwrap(); + apply_rule("hygiene", "T", "B", None, RuleTier::Override, vec![]).unwrap(); remove_rule("hygiene").unwrap(); assert!(list_rules().is_empty()); }); @@ -291,7 +387,7 @@ mod tests { with_temp_home(|| { std::fs::create_dir_all(rules_dir()).unwrap(); std::fs::write(rules_dir().join("coaching-broken.md"), "").unwrap(); - apply_rule("good", "T", "B", None).unwrap(); + apply_rule("good", "T", "B", None, RuleTier::Override, vec![]).unwrap(); let rules = list_rules(); assert_eq!( @@ -306,8 +402,24 @@ mod tests { #[test] fn untriggered_rule_bodies_returns_all_untriggered_bodies_in_id_order() { with_temp_home(|| { - apply_rule("b-rule", "Title B", "Body B", None).unwrap(); - apply_rule("a-rule", "Title A", "Body A", None).unwrap(); + apply_rule( + "b-rule", + "Title B", + "Body B", + None, + RuleTier::Override, + vec![], + ) + .unwrap(); + apply_rule( + "a-rule", + "Title A", + "Body A", + None, + RuleTier::Override, + vec![], + ) + .unwrap(); assert_eq!( untriggered_rule_bodies(), vec!["Body A".to_string(), "Body B".to_string()] @@ -318,7 +430,15 @@ mod tests { #[test] fn apply_rule_body_with_dashes_line_is_not_truncated() { with_temp_home(|| { - let applied = apply_rule("dashes", "Title", "before\n---\nafter", None).unwrap(); + let applied = apply_rule( + "dashes", + "Title", + "before\n---\nafter", + None, + RuleTier::Override, + vec![], + ) + .unwrap(); assert!( applied.body.contains("before"), "body should contain text before the --- line: {}", @@ -340,7 +460,15 @@ mod tests { #[test] fn apply_rule_title_with_em_dash_is_not_truncated() { with_temp_home(|| { - let applied = apply_rule("emdash", "Foo \u{2014} Bar", "Body", None).unwrap(); + let applied = apply_rule( + "emdash", + "Foo \u{2014} Bar", + "Body", + None, + RuleTier::Override, + vec![], + ) + .unwrap(); assert_eq!(applied.title, "Foo \u{2014} Bar"); let rules = list_rules(); @@ -356,7 +484,15 @@ mod tests { tools: vec!["mcp__flare__review".to_string()], auto_match: true, }; - let applied = apply_rule("revfix", "Title", "Body", Some(trigger.clone())).unwrap(); + let applied = apply_rule( + "revfix", + "Title", + "Body", + Some(trigger.clone()), + RuleTier::Override, + vec![], + ) + .unwrap(); assert_eq!(applied.trigger, Some(trigger.clone())); let rules = list_rules(); @@ -368,7 +504,8 @@ mod tests { #[test] fn apply_rule_without_trigger_is_untriggered() { with_temp_home(|| { - let applied = apply_rule("hygiene", "Title", "Body", None).unwrap(); + let applied = + apply_rule("hygiene", "Title", "Body", None, RuleTier::Override, vec![]).unwrap(); assert_eq!(applied.trigger, None); }); } @@ -376,7 +513,7 @@ mod tests { #[test] fn untriggered_rule_bodies_excludes_triggered_rules() { with_temp_home(|| { - apply_rule("plain", "T", "Plain body", None).unwrap(); + apply_rule("plain", "T", "Plain body", None, RuleTier::Override, vec![]).unwrap(); apply_rule( "scoped", "T", @@ -385,6 +522,8 @@ mod tests { tools: vec!["mcp__flare__review".to_string()], auto_match: false, }), + RuleTier::Override, + vec![], ) .unwrap(); @@ -403,6 +542,8 @@ mod tests { tools: vec!["mcp__flare__review".to_string()], auto_match: false, }), + RuleTier::Override, + vec![], ) .unwrap(); @@ -426,6 +567,8 @@ mod tests { tools: vec![], auto_match: true, }), + RuleTier::Override, + vec![], ) .unwrap(); @@ -448,6 +591,8 @@ mod tests { tools: vec![], auto_match: true, }), + RuleTier::Override, + vec![], ) .unwrap(); @@ -458,7 +603,7 @@ mod tests { #[test] fn rule_bodies_for_prompt_ignores_non_auto_match_rules() { with_temp_home(|| { - apply_rule("plain", "T", "Plain body", None).unwrap(); + apply_rule("plain", "T", "Plain body", None, RuleTier::Override, vec![]).unwrap(); apply_rule( "tool-only", "T", @@ -467,6 +612,8 @@ mod tests { tools: vec!["mcp__flare__review".to_string()], auto_match: false, }), + RuleTier::Override, + vec![], ) .unwrap(); @@ -480,4 +627,83 @@ mod tests { assert!(rule_bodies_for_prompt("anything").is_empty()); }); } + + #[test] + fn apply_rule_snapshots_previous_body_when_overwriting_a_builtin_rule() { + with_temp_home(|| { + apply_rule( + "search17", + "T", + "Old body", + None, + rule::RuleTier::Builtin, + vec!["claude-code".to_string()], + ) + .unwrap(); + apply_rule( + "search17", + "T", + "New body", + None, + rule::RuleTier::Builtin, + vec!["claude-code".to_string()], + ) + .unwrap(); + + let path = rules_dir().join("superseded-search17.json"); + let bodies: Vec = + serde_json::from_str(&std::fs::read_to_string(path).unwrap()).unwrap(); + assert_eq!(bodies, vec!["Old body".to_string()]); + }); + } + + #[test] + fn apply_rule_does_not_snapshot_when_overwriting_an_override_tier_rule() { + with_temp_home(|| { + apply_rule( + "hygiene", + "T", + "Old body", + None, + rule::RuleTier::Override, + vec![], + ) + .unwrap(); + apply_rule( + "hygiene", + "T", + "New body", + None, + rule::RuleTier::Override, + vec![], + ) + .unwrap(); + assert!(!rules_dir().join("superseded-hygiene.json").exists()); + }); + } + + #[test] + fn apply_rule_does_not_snapshot_when_body_is_unchanged() { + with_temp_home(|| { + apply_rule( + "search17", + "T", + "Same body", + None, + rule::RuleTier::Builtin, + vec![], + ) + .unwrap(); + apply_rule( + "search17", + "T", + "Same body", + None, + rule::RuleTier::Builtin, + vec![], + ) + .unwrap(); + assert!(!rules_dir().join("superseded-search17.json").exists()); + }); + } } diff --git a/src/components.rs b/src/components.rs index 1d7f10c0..0175b641 100644 --- a/src/components.rs +++ b/src/components.rs @@ -176,58 +176,135 @@ fn write_if_absent(path: &PathBuf, content: &str) -> bool { /// no dedicated rules convention (per research), so it gets none. pub(crate) fn rule_targets(host: &str) -> Vec<(PathBuf, String)> { let joined = || rule_text::all().join("\n\n"); + let coaching: Vec<(String, String)> = crate::coaching::sync_targets_for_host(host); + let joined_extra = || { + coaching + .iter() + .map(|(_, body)| body.clone()) + .collect::>() + .join("\n\n") + }; + let append_joined = |base: String| { + if coaching.is_empty() { + base + } else { + format!("{base}\n\n{}", joined_extra()) + } + }; match host { "claude-code" => { let dir = claude_rules_dir(); - vec![ + let mut v = vec![ (dir.join("exa.md"), rule_text::EXA.to_string()), (dir.join("git.md"), rule_text::GIT.to_string()), (dir.join("lean-ctx.md"), rule_text::LEANCTX.to_string()), (dir.join("flare-docs.md"), rule_text::FLARE_DOCS.to_string()), - ] + ]; + v.extend( + coaching + .iter() + .map(|(id, body)| (dir.join(format!("{id}.md")), body.clone())), + ); + v } "cursor" => { - let content = format!("---\nalwaysApply: true\n---\n\n{}", joined()); + let content = format!("---\nalwaysApply: true\n---\n\n{}", append_joined(joined())); vec![( cwd().join(".cursor").join("rules").join("agentflare.mdc"), content, )] } "codex" => { - let content = format!("# Rules (agentflare)\n\n{}\n", joined()); + let content = format!("# Rules (agentflare)\n\n{}\n", append_joined(joined())); vec![(cwd().join("AGENTS.md"), content)] } "windsurf" => { vec![( cwd().join(".windsurf").join("rules").join("agentflare.md"), - joined() + "\n", + append_joined(joined()) + "\n", )] } "vscode-copilot" => { vec![( cwd().join(".github").join("copilot-instructions.md"), - joined() + "\n", + append_joined(joined()) + "\n", )] } "cline" => { vec![( cwd().join(".clinerules").join("agentflare.md"), - joined() + "\n", + append_joined(joined()) + "\n", )] } "opencode" => { let dir = opencode_rules_dir(); - vec![ + let mut v = vec![ (dir.join("exa.md"), rule_text::EXA.to_string()), (dir.join("git.md"), rule_text::GIT.to_string()), (dir.join("lean-ctx.md"), rule_text::LEANCTX.to_string()), (dir.join("flare-docs.md"), rule_text::FLARE_DOCS.to_string()), - ] + ]; + v.extend( + coaching + .iter() + .map(|(id, body)| (dir.join(format!("{id}.md")), body.clone())), + ); + v } _ => vec![], // "continue" — no dedicated rules convention found } } +/// Immediately materializes every `rule_targets(host)` entry to disk: +/// writes it if absent, refreshes it if `init::is_stale_rule` says its +/// current content is a known-old wording, otherwise leaves it alone +/// (a hand-edit, or already current). For `host == "opencode"`, also +/// re-runs the instructions-array registration. +pub(crate) fn sync_now(host: &str) -> Result { + let mut written = 0usize; + let mut refreshed = 0usize; + for (path, content) in rule_targets(host) { + if !path.exists() { + if let Some(parent) = path.parent() { + let _ = fs::create_dir_all(parent); + } + fs::write(&path, format!("{content}\n")).map_err(|e| e.to_string())?; + written += 1; + } else if crate::init::is_stale_rule(&path, &content) { + fs::write(&path, format!("{content}\n")).map_err(|e| e.to_string())?; + refreshed += 1; + } + } + if host == "opencode" { + crate::init::wire_opencode_instructions(); + } + Ok(format!("{written} written, {refreshed} refreshed")) +} + +/// Remove a coaching rule's generated file from `host`'s rules directory. +/// Returns Ok(()) if the file didn't exist or was successfully deleted. +pub(crate) fn unsync_host(rule_id: &str, host: &str) -> Result<(), String> { + use crate::paths::{claude_rules_dir, opencode_rules_dir}; + let dir = match host { + "claude-code" => claude_rules_dir(), + "opencode" => opencode_rules_dir(), + "cursor" => cwd().join(".cursor").join("rules"), + "codex" => cwd().join("."), + "windsurf" => cwd().join(".windsurf").join("rules"), + "vscode-copilot" => cwd().join(".github"), + "cline" => cwd().join(".clinerules"), + _ => return Ok(()), + }; + let path = dir.join(format!("{rule_id}.md")); + if path.exists() { + std::fs::remove_file(&path).map_err(|e| format!("failed to remove {path:?}: {e}"))?; + } + if host == "opencode" { + crate::init::wire_opencode_instructions(); + } + Ok(()) +} + /// Agent IDs detected on this machine, for `skill_registry::Registry::open_default`'s /// `detected_agents` param. skill-registry itself has no `agent-registry` dependency /// (deliberately decoupled — skill discovery only needs agent IDs, not the version- @@ -795,6 +872,76 @@ mod tests { } } + #[test] + fn rule_targets_includes_coaching_sourced_rule_for_claude_code_and_opencode() { + crate::paths::test_support::with_temp_home(|| { + crate::coaching::apply_rule( + "search17", + "T", + "Coaching body", + None, + crate::coaching::rule::RuleTier::Builtin, + vec!["claude-code".to_string(), "opencode".to_string()], + ) + .unwrap(); + + let cc = rule_targets("claude-code"); + assert!( + cc.iter() + .any(|(p, c)| p.to_string_lossy().ends_with("search17.md") + && c == "Coaching body") + ); + + let oc = rule_targets("opencode"); + assert!( + oc.iter() + .any(|(p, c)| p.to_string_lossy().ends_with("search17.md") + && c == "Coaching body") + ); + }); + } + + #[test] + fn rule_targets_appends_coaching_sourced_body_into_joined_hosts() { + crate::paths::test_support::with_temp_home(|| { + crate::coaching::apply_rule( + "search17", + "T", + "Coaching body", + None, + crate::coaching::rule::RuleTier::Builtin, + vec!["cursor".to_string()], + ) + .unwrap(); + + let targets = rule_targets("cursor"); + assert_eq!(targets.len(), 1, "cursor stays a single joined file"); + assert!(targets[0].1.contains("Coaching body")); + assert!( + targets[0].1.contains(rule_text::FLARE_DOCS), + "existing builtin content must still be present" + ); + }); + } + + #[test] + fn rule_targets_omits_coaching_rule_not_synced_to_this_host() { + crate::paths::test_support::with_temp_home(|| { + crate::coaching::apply_rule( + "search17", + "T", + "Coaching body", + None, + crate::coaching::rule::RuleTier::Builtin, + vec!["opencode".to_string()], + ) + .unwrap(); + + let cc = rule_targets("claude-code"); + assert!(!cc.iter().any(|(_, c)| c == "Coaching body")); + }); + } + #[test] fn agentflare_mcp_check_reflects_codex_config_toml_substring() { crate::paths::test_support::with_temp_home(|| { diff --git a/src/dashboard/artifacts.rs b/src/dashboard/artifacts.rs new file mode 100644 index 00000000..b0133eeb --- /dev/null +++ b/src/dashboard/artifacts.rs @@ -0,0 +1,107 @@ +use agentflare_artifacts::ArtifactStore; +use axum::{ + Router, + extract::{Path, State}, + http::{StatusCode, header}, + response::{IntoResponse, Response, Sse}, + routing::get, +}; +use serde::Deserialize; +use std::sync::Arc; +use tokio_stream::StreamExt; +use tokio_stream::wrappers::UnboundedReceiverStream; + +#[derive(Clone)] +struct ArtifactState { + store: Arc, + base_url: String, +} + +fn open_store() -> ArtifactState { + let store = match crate::store::open() { + Ok(s) => ArtifactStore::with_store(s), + Err(e) => { + eprintln!("[dashboard/artifacts] failed to open store: {e}"); + ArtifactStore::new(crate::paths::home().join(".agentflare").join("artifacts")) + } + }; + ArtifactState { + store: Arc::new(store), + base_url: "/artifacts".to_string(), + } +} + +async fn index(State(state): State) -> Response { + let html = tokio::task::spawn_blocking(move || { + agentflare_artifacts::render_index(&state.store, &state.base_url) + }) + .await + .unwrap_or_else(|_| "error".into()); + ([(header::CONTENT_TYPE, "text/html; charset=utf-8")], html).into_response() +} + +#[derive(Deserialize)] +struct VersionPath { + id: String, + version: u32, +} + +async fn artifact_page(State(state): State, Path(id): Path) -> Response { + let Ok(artifact) = state.store.get(&id) else { + return (StatusCode::NOT_FOUND, "artifact not found").into_response(); + }; + let html = agentflare_artifacts::render_artifact_page(&artifact, true, &state.base_url); + ([(header::CONTENT_TYPE, "text/html; charset=utf-8")], html).into_response() +} + +async fn artifact_version_page( + State(state): State, + Path(VersionPath { id, version }): Path, +) -> Response { + let Ok(artifact) = state.store.get_version(&id, version) else { + return (StatusCode::NOT_FOUND, "artifact version not found").into_response(); + }; + let html = agentflare_artifacts::render_artifact_page(&artifact, false, &state.base_url); + ([(header::CONTENT_TYPE, "text/html; charset=utf-8")], html).into_response() +} + +async fn versions_json(State(state): State, Path(id): Path) -> Response { + match state.store.versions(&id) { + Ok(history) => ( + [(header::CONTENT_TYPE, "application/json")], + serde_json::to_string_pretty(&history).unwrap_or_else(|_| "[]".into()), + ) + .into_response(), + Err(_) => (StatusCode::NOT_FOUND, "artifact not found").into_response(), + } +} + +async fn artifact_live(State(state): State, Path(id): Path) -> Response { + if !agentflare_artifacts::valid_id(&id) { + return (StatusCode::NOT_FOUND, "invalid id").into_response(); + } + let rx = state.store.subscribe(&id); + let (tx, async_rx) = tokio::sync::mpsc::unbounded_channel::(); + tokio::task::spawn_blocking(move || { + while let Ok(event) = rx.recv() { + if tx.send(event).is_err() { + break; + } + } + }); + let stream = UnboundedReceiverStream::new(async_rx).map(|event| { + Ok::<_, std::convert::Infallible>(axum::response::sse::Event::default().data(event)) + }); + Sse::new(stream).into_response() +} + +pub fn router() -> Router { + let state = open_store(); + Router::new() + .route("/", get(index)) + .route("/{id}", get(artifact_page)) + .route("/{id}/v/{version}", get(artifact_version_page)) + .route("/{id}/versions", get(versions_json)) + .route("/{id}/live", get(artifact_live)) + .with_state(state) +} diff --git a/src/dashboard/mod.rs b/src/dashboard/mod.rs index 71034098..5d7b8fae 100644 --- a/src/dashboard/mod.rs +++ b/src/dashboard/mod.rs @@ -1,3 +1,4 @@ +mod artifacts; mod data; mod server; diff --git a/src/dashboard/server.rs b/src/dashboard/server.rs index 975a2b4d..b747d11e 100644 --- a/src/dashboard/server.rs +++ b/src/dashboard/server.rs @@ -227,6 +227,7 @@ pub fn router() -> Router { .route("/api/webhooks", get(webhooks_handler)) .route("/api/cost", get(cost_handler)) .route("/events", get(events_handler)) + .nest("/artifacts", super::artifacts::router()) .merge(flare_proxy::router()) .fallback(static_handler) } diff --git a/src/hook.rs b/src/hook.rs index c601ae84..7cfe2b0c 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -693,6 +693,7 @@ second line #[test] fn session_start_includes_untriggered_coaching_rule_bodies() { + use crate::coaching::rule::RuleTier; use crate::paths::test_support::with_temp_home; with_temp_home(|| { crate::coaching::apply_rule( @@ -700,6 +701,8 @@ second line "Close sessions promptly", "Wrap up each phase before starting the next.", None, + RuleTier::Override, + vec![], ) .unwrap(); @@ -748,6 +751,7 @@ second line #[test] fn pre_tool_use_surfaces_tool_triggered_coaching_rule() { + use crate::coaching::rule::RuleTier; use crate::paths::test_support::with_temp_home; with_temp_home(|| { crate::coaching::apply_rule( @@ -758,6 +762,8 @@ second line vec!["mcp__flare__review".to_string()], false, )), + RuleTier::Override, + vec![], ) .unwrap(); @@ -769,6 +775,7 @@ second line #[test] fn prompt_submit_surfaces_auto_match_coaching_rule() { + use crate::coaching::rule::RuleTier; use crate::paths::test_support::with_temp_home; with_temp_home(|| { crate::coaching::apply_rule( @@ -776,6 +783,8 @@ second line "Reviews ship with fixes", "Every review finding needs a diff.", Some(crate::coaching::test_support::trigger(vec![], true)), + RuleTier::Override, + vec![], ) .unwrap(); diff --git a/src/hook_redirect.rs b/src/hook_redirect.rs index b5c42bde..57eb652a 100644 --- a/src/hook_redirect.rs +++ b/src/hook_redirect.rs @@ -58,6 +58,60 @@ fn is_spec_like_path(path: &str) -> bool { normalized.contains("/specs/") && normalized.ends_with(".md") } +/// Blocks shell commands that delete agentflare's own SQLite data files +/// (`~/.agentflare/*.db*`, or the `.agentflare` dir wholesale). Landed after +/// a 2026-07-25 incident: opencode ran a delete mid-migration and silently +/// wiped `store.db`'s metadata for 168 artifacts — recovered by hand from a +/// pre-migration flat-file backup that happened to still exist, but nothing +/// would have caught it if that backup hadn't been there. `rm`/`del`/ +/// `Remove-Item`/`unlink`/`rmdir` are the verbs every shell tool (Bash, +/// PowerShell, opencode's `bash`) actually uses; deletion of these files is +/// something only the user should ever do by hand. +/// +/// Checks each `;`/`&&`/`||`/`|`/newline-separated statement's *first word* +/// against the destructive-verb list, rather than substring-matching the +/// whole command blob — a raw `.contains("rm ")` also fires on a `git commit` +/// whose heredoc message happens to describe this very guard in prose (this +/// function's own commit message is a real example: "opencode ran an rm +/// mid-migration ... store.db ... ~/.agentflare/*.db*" — none of that is an +/// executed command, but a whole-string substring check can't tell). +fn destructive_data_file_reason(command: &str) -> Option { + for statement in command + .split([';', '\n']) + .flat_map(|s| s.split("&&")) + .flat_map(|s| s.split("||")) + .flat_map(|s| s.split('|')) + { + let trimmed = statement.trim().to_lowercase().replace('\\', "/"); + let Some(first_word) = trimmed.split_whitespace().next() else { + continue; + }; + let is_destructive_verb = matches!( + first_word, + "rm" | "del" | "erase" | "remove-item" | "unlink" | "rmdir" + ); + if !is_destructive_verb { + continue; + } + let targets_agentflare_dir = + trimmed.contains(".agentflare/") || trimmed.ends_with(".agentflare"); + if !targets_agentflare_dir { + continue; + } + // Either a specific *.db*/-wal/-shm file, or a recursive/whole-dir + // delete of .agentflare itself (which would take the db files with it). + let targets_db_or_whole_dir = trimmed.contains(".db") + || trimmed.ends_with(".agentflare") + || trimmed.contains(" -r ") + || trimmed.contains(" -rf") + || trimmed.contains("-recurse"); + if targets_db_or_whole_dir { + return Some("deleting agentflare's local data files (~/.agentflare/*.db, *.db-wal, *.db-shm, or the .agentflare directory itself) is blocked — they hold tracked items, artifacts, and secrets with no automatic backup. If a file genuinely needs to be removed, ask the user to run the command themselves.".to_string()); + } + } + None +} + /// Resolve the current branch of the repo containing `start_path`, or cwd if /// `start_path` is None. `None` outside a git repo. fn current_branch(start_path: Option<&Path>) -> Option { @@ -135,6 +189,19 @@ fn classify( ) }) } + "Bash" | "bash" | "PowerShell" | "powershell" | "shell" => { + // "command" is Claude Code's and (by convention) opencode's bash + // tool field; "cmd"/"script" are cheap insurance against a + // harness using a different name rather than a hard dependency + // on guessing right. + let input = tool_input?; + let command = input + .get("command") + .or_else(|| input.get("cmd")) + .or_else(|| input.get("script")) + .and_then(Value::as_str)?; + destructive_data_file_reason(command) + } _ => None, } } @@ -256,6 +323,60 @@ mod tests { assert!(classify("Bash", None, NOT_A_REPO).is_none()); } + #[test] + fn classify_blocks_rm_of_agentflare_db_via_bash() { + let input = json!({ "command": "rm ~/.agentflare/store.db" }); + let reason = classify("Bash", Some(&input), NOT_A_REPO).unwrap(); + assert!(reason.contains("blocked"), "{reason}"); + } + + #[test] + fn classify_blocks_del_of_agentflare_db_via_opencode_bash() { + let input = json!({ "command": "del C:\\Users\\shiva\\.agentflare\\backend.db" }); + assert!(classify("bash", Some(&input), NOT_A_REPO).is_some()); + } + + #[test] + fn classify_blocks_remove_item_via_powershell() { + let input = + json!({ "command": "Remove-Item $env:USERPROFILE\\.agentflare\\agentflare.db" }); + assert!(classify("PowerShell", Some(&input), NOT_A_REPO).is_some()); + } + + #[test] + fn classify_blocks_recursive_delete_of_whole_agentflare_dir() { + let input = json!({ "command": "rm -rf ~/.agentflare" }); + assert!(classify("Bash", Some(&input), NOT_A_REPO).is_some()); + } + + #[test] + fn classify_allows_unrelated_rm_commands() { + let input = json!({ "command": "rm /tmp/scratch.txt" }); + assert!(classify("Bash", Some(&input), NOT_A_REPO).is_none()); + } + + #[test] + fn classify_allows_non_destructive_agentflare_db_commands() { + let input = json!({ "command": "sqlite3 ~/.agentflare/store.db '.tables'" }); + assert!(classify("Bash", Some(&input), NOT_A_REPO).is_none()); + } + + #[test] + fn classify_allows_commit_message_prose_mentioning_rm_and_db_paths() { + // Regression: this exact scenario blocked the real commit that landed + // this guard -- a `git commit` heredoc whose message describes the + // incident in prose ("opencode ran an rm ... store.db ... + // ~/.agentflare/*.db*") is not an executed rm, and must not match. + let input = json!({ "command": "git commit -m \"$(cat <<'EOF'\nhook_redirect: block agent shell commands from deleting agentflare db files\n\nopencode ran an rm mid-migration and silently wiped store.db's metadata,\ntargeting ~/.agentflare/*.db* or the .agentflare dir itself.\nEOF\n)\"" }); + assert!(classify("Bash", Some(&input), NOT_A_REPO).is_none()); + } + + #[test] + fn classify_blocks_rm_as_second_statement_after_separator() { + let input = json!({ "command": "cd /tmp && rm ~/.agentflare/store.db" }); + assert!(classify("Bash", Some(&input), NOT_A_REPO).is_some()); + } + #[test] fn classify_write_with_no_path_falls_through() { assert!(classify("Write", None, ON_FEATURE_BRANCH).is_none()); diff --git a/src/init.rs b/src/init.rs index 0cb4472e..c17c1847 100644 --- a/src/init.rs +++ b/src/init.rs @@ -29,17 +29,29 @@ pub(crate) fn is_stale_rule(path: &PathBuf, current: &str) -> bool { let Some(filename) = path.file_name().and_then(|f| f.to_str()) else { return false; }; - let superseded = rule_text::superseded(filename); - if superseded.is_empty() { - return false; - } let Ok(existing) = fs::read_to_string(path) else { return false; }; - existing.trim_end() != current.trim_end() - && superseded - .iter() - .any(|old| existing.trim_end() == old.trim_end()) + let trimmed = existing.trim_end(); + if trimmed == current.trim_end() { + return false; + } + + // Check compiled-in superseded bodies first (exa.md, git.md, etc.). + let superseded = rule_text::superseded(filename); + if superseded.contains(&trimmed) { + return true; + } + + // For coaching-sourced rules (.md), also check auto-tracked bodies. + if let Some(id) = filename.strip_suffix(".md") { + let coaching = crate::coaching::superseded_bodies(id); + if coaching.iter().any(|old| trimmed == old.as_str()) { + return true; + } + } + + false } pub(crate) fn prompt_yes(message: &str, agent: &str, yes: bool) -> bool { @@ -412,9 +424,20 @@ fn wire_codex_hooks() { } fn wire_opencode() { + wire_opencode_instructions(); +} + +/// Re-register opencode.jsonc instructions from `rule_targets`, adding any +/// coaching-sourced rule files and removing entries for rules no longer synced. +/// Shared between `wire_opencode` (called by `init`) and `sync_now`/`unsync_host` +/// (called by `coaching apply/remove`). +pub(crate) fn wire_opencode_instructions() { let path = opencode_config_path(); let rules_dir = opencode_rules_dir(); - let rule_files: &[&str] = &["exa.md", "git.md", "lean-ctx.md"]; + let rule_paths: Vec = crate::components::rule_targets("opencode") + .into_iter() + .map(|(p, _)| p) + .collect(); // opencode deep-merges opencode.json with opencode.jsonc, so a rule // entry the user (or a hand-written opencode.json) already registered @@ -457,21 +480,47 @@ fn wire_opencode() { arr.retain(|v| v.as_str() != Some(legacy_engram_path.as_str())); let removed_legacy = arr.len() != before_cleanup; + let expected_filenames: Vec = rule_paths + .iter() + .filter_map(|p| p.file_name().map(|f| f.to_string_lossy().to_string())) + .collect(); + + // Drop entries under our rules_dir for coaching-sourced rules that are + // no longer synced to opencode (e.g. after `coaching remove` or a + // `--sync` list edit) — otherwise unsync_host's file deletion leaves a + // dangling path registered here forever. + let rules_dir_str = rules_dir.to_string_lossy().replace('\\', "/"); + let before_prune = arr.len(); + arr.retain(|v| { + let Some(s) = v.as_str() else { return true }; + let normalized = s.replace('\\', "/"); + if !normalized.starts_with(&rules_dir_str) { + return true; + } + expected_filenames + .iter() + .any(|f| normalized.ends_with(f.as_str())) + }); + let pruned = before_prune - arr.len(); + let mut added = 0; - for &file in rule_files { + for file in &expected_filenames { let rule_path = rules_dir.join(file); let path_str = rule_path.to_string_lossy().replace('\\', "/"); - let has_it = arr + let has_it = arr.iter().any(|v| { + v.as_str() + .map(|s| s.contains(file.as_str())) + .unwrap_or(false) + }) || sibling_instructions .iter() - .any(|v| v.as_str().map(|s| s.contains(file)).unwrap_or(false)) - || sibling_instructions.iter().any(|s| s.contains(file)); + .any(|s| s.contains(file.as_str())); if !has_it && rule_path.exists() { arr.push(json!(path_str)); added += 1; } } - if added > 0 || removed_legacy { + if added > 0 || removed_legacy || pruned > 0 { if let Some(parent) = path.parent() { let _ = fs::create_dir_all(parent); } @@ -479,6 +528,9 @@ fn wire_opencode() { Ok(_) if removed_legacy => ui::success(&format!( "opencode.jsonc instructions wired ({added} rule(s), removed stale engram.md)" )), + Ok(_) if pruned > 0 => ui::success(&format!( + "opencode.jsonc instructions wired ({added} rule(s), pruned {pruned} stale rule(s))" + )), Ok(_) => ui::success(&format!( "opencode.jsonc instructions wired ({added} rule(s))" )), @@ -691,6 +743,35 @@ mod tests { }); } + #[test] + fn is_stale_rule_true_for_coaching_builtin_with_known_old_body() { + with_temp_home(|| { + let dir = home().join(".claude").join("rules"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("search17.md"); + fs::write(&path, "Old body\n").unwrap(); + let state_rules = crate::state::state_dir().join("rules"); + fs::create_dir_all(&state_rules).unwrap(); + fs::write( + state_rules.join("superseded-search17.json"), + r#"["Old body"]"#, + ) + .unwrap(); + assert!(is_stale_rule(&path, "New body")); + }); + } + + #[test] + fn is_stale_rule_false_for_coaching_builtin_with_unrecognized_content() { + with_temp_home(|| { + let dir = home().join(".claude").join("rules"); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("search17.md"); + fs::write(&path, "Hand-edited content\n").unwrap(); + assert!(!is_stale_rule(&path, "New body")); + }); + } + #[test] fn confirm_rule_refresh_updates_stale_file_when_yes() { with_temp_home(|| { @@ -1080,4 +1161,41 @@ mod tests { assert!(content.contains("lean-ctx.md")); }); } + + #[test] + fn wire_opencode_prunes_stale_coaching_rule_entry() { + with_temp_home(|| { + let config_path = home() + .join(".config") + .join("opencode") + .join("opencode.jsonc"); + let rules_dir = home().join(".config").join("opencode").join("rules"); + fs::create_dir_all(&rules_dir).unwrap(); + + // Simulates a coaching rule that was synced to opencode and then + // removed: unsync_host already deleted rules/search17.md, but + // without pruning this would leave the array entry dangling. + let stale_rule_path = rules_dir + .join("search17.md") + .to_string_lossy() + .replace('\\', "/"); + fs::create_dir_all(config_path.parent().unwrap()).unwrap(); + fs::write( + &config_path, + serde_json::to_string(&json!({ + "instructions": [stale_rule_path.clone()] + })) + .unwrap(), + ) + .unwrap(); + + wire_opencode_instructions(); + + let content = fs::read_to_string(&config_path).unwrap(); + assert!( + !content.contains(&stale_rule_path), + "dangling coaching rule entry should be pruned: {content}" + ); + }); + } } diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 5949bad2..7bed25ff 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -416,8 +416,16 @@ impl AgentflareMcp { (store, ArtifactBackend::Owned(server)) } None => { - let dir = crate::paths::home().join(".agentflare").join("artifacts"); - let store = std::sync::Arc::new(agentflare_artifacts::ArtifactStore::new(dir)); + let store = match crate::store::open() { + Ok(s) => { + std::sync::Arc::new(agentflare_artifacts::ArtifactStore::with_store(s)) + } + Err(e) => { + eprintln!("[artifacts] fallback to flat-file store: {e}"); + let dir = crate::paths::home().join(".agentflare").join("artifacts"); + std::sync::Arc::new(agentflare_artifacts::ArtifactStore::new(dir)) + } + }; let backend = Self::shared_backend(&store)?; (store, backend) }