From 4034e98dfb75e8612ca2591c10fcceef898c2576 Mon Sep 17 00:00:00 2001 From: ZhenYi <434836402@qq.com> Date: Mon, 18 May 2026 20:43:27 +0800 Subject: [PATCH] refactor(service): multi-root skill scanner and chat/join_request updates Skill scanner now walks .claude/skills and .codex/skills directories separately, adds relative_path/system fields to DiscoveredSkill, and supports root-level SKILL.md. Update chat context and join request handling. --- libs/service/chat/context.rs | 4 +- libs/service/chat/mod.rs | 2 +- libs/service/project/join_request.rs | 10 +- libs/service/skill/scanner.rs | 360 ++++++++++++++++++++++----- 4 files changed, 308 insertions(+), 68 deletions(-) diff --git a/libs/service/chat/context.rs b/libs/service/chat/context.rs index 06d3691..5861ebc 100644 --- a/libs/service/chat/context.rs +++ b/libs/service/chat/context.rs @@ -17,9 +17,7 @@ fn metadata_object( fn slash_context_object( metadata: Option<&serde_json::Value>, ) -> Option<&serde_json::Map> { - metadata_object(metadata)? - .get("slash_context")? - .as_object() + metadata_object(metadata)?.get("slash_context")?.as_object() } fn stringify_text(value: &serde_json::Value) -> Option { diff --git a/libs/service/chat/mod.rs b/libs/service/chat/mod.rs index fda1108..37a0695 100644 --- a/libs/service/chat/mod.rs +++ b/libs/service/chat/mod.rs @@ -1,6 +1,6 @@ pub mod access; -pub mod conversation; pub mod context; +pub mod conversation; pub mod fork; pub mod message; pub mod share; diff --git a/libs/service/project/join_request.rs b/libs/service/project/join_request.rs index 3e99803..a29ecb4 100644 --- a/libs/service/project/join_request.rs +++ b/libs/service/project/join_request.rs @@ -202,12 +202,10 @@ impl AppService { if let Some(ref s) = settings { if s.require_questions { - let required_questions: Vec = - serde_json::from_value(s.questions.clone()).map_err(|_| { - AppError::InternalServerError( - "Invalid join question configuration".to_string(), - ) - })?; + let required_questions: Vec = serde_json::from_value(s.questions.clone()) + .map_err(|_| { + AppError::InternalServerError("Invalid join question configuration".to_string()) + })?; Self::validate_join_request_answers(&required_questions, &request.answers)?; } } diff --git a/libs/service/skill/scanner.rs b/libs/service/skill/scanner.rs index dd6f094..d8dc472 100644 --- a/libs/service/skill/scanner.rs +++ b/libs/service/skill/scanner.rs @@ -15,6 +15,9 @@ use sha1::Digest; use std::path::Path; use uuid::Uuid; +const SKILL_ROOTS: &[(&str, &str)] = &[(".claude/skills", "claude"), (".codex/skills", "codex")]; +const ROOT_SKILL_SYSTEM: &str = "root"; + fn should_descend_dir(name: &str) -> bool { name != ".git" } @@ -32,6 +35,10 @@ pub struct DiscoveredSkill { pub content: String, /// Parsed frontmatter as JSON. pub metadata: serde_json::Value, + /// Relative path of the SKILL.md file in the repository. + pub relative_path: Option, + /// Skill system/source directory, e.g. "claude" or "codex". + pub system: Option, /// Git commit SHA where this skill was found (git hook path only). pub commit_sha: Option, /// Git blob SHA-1 of the SKILL.md file. @@ -78,6 +85,8 @@ fn parse_skill_file(slug: &str, raw: &str) -> DiscoveredSkill { description, content: content.trim().to_string(), metadata, + relative_path: None, + system: None, commit_sha: None, blob_hash: None, } @@ -98,16 +107,34 @@ fn extract_frontmatter(raw: &str) -> (Option<&str>, &str) { } } -/// Recursively scan `repo_path` for `SKILL.md` files (filesystem walk, non-bare repos). -/// The skill slug is `{short_repo_id}/{parent_dir_name}` to ensure uniqueness across repos. +/// Recursively scan supported skill roots for `SKILL.md` files (filesystem walk, non-bare repos). +/// The skill slug is `{short_repo_id}/{system}/{relative_skill_dir}`. pub fn scan_repo_for_skills( repo_path: &Path, repo_id: Uuid, ) -> Result, AppError> { let repo_id_prefix = &repo_id.to_string()[..8]; let mut discovered = Vec::new(); - let mut stack = vec![repo_path.to_path_buf()]; + for (root, system) in SKILL_ROOTS { + let root_path = repo_path.join(root); + if root_path.exists() { + scan_skill_root_fs(&root_path, repo_id_prefix, system, root, &mut discovered); + } + } + scan_root_skill_pack_fs(repo_path, repo_id_prefix, &mut discovered); + + Ok(discovered) +} + +fn scan_skill_root_fs( + root_path: &Path, + repo_id_prefix: &str, + system: &str, + root: &str, + discovered: &mut Vec, +) { + let mut stack = vec![root_path.to_path_buf()]; while let Some(dir) = stack.pop() { let entries = match std::fs::read_dir(&dir) { Ok(e) => e, @@ -117,30 +144,83 @@ pub fn scan_repo_for_skills( let path = entry.path(); if path.is_dir() { stack.push(path); - } else if path - .file_name() - .and_then(|n| n.to_str()) - .map(|s| s.to_lowercase()) - == Some("skill.md".to_string()) - { - if let Some(dir_name) = path - .parent() - .and_then(|p| p.file_name()) - .and_then(|n| n.to_str()) - { - let slug = format!("{}/{}", repo_id_prefix, dir_name); - if let Ok(raw) = std::fs::read(&path) { - let blob_hash = git_blob_hash(&raw); - let mut skill = parse_skill_file(&slug, &String::from_utf8_lossy(&raw)); - skill.blob_hash = Some(blob_hash); - discovered.push(skill); - } - } + continue; + } + if !is_skill_file_name(&path) { + continue; + } + + let Some(parent) = path.parent() else { + continue; + }; + let relative_skill_dir = parent + .strip_prefix(root_path) + .ok() + .and_then(path_to_slug) + .filter(|s| !s.is_empty()); + let Some(relative_skill_dir) = relative_skill_dir else { + continue; + }; + let slug = format!("{}/{}/{}", repo_id_prefix, system, relative_skill_dir); + if let Ok(raw) = std::fs::read(&path) { + let blob_hash = git_blob_hash(&raw); + let mut skill = parse_skill_file(&slug, &String::from_utf8_lossy(&raw)); + skill.blob_hash = Some(blob_hash); + skill.system = Some(system.to_string()); + skill.relative_path = Some(format!("{}/{}/SKILL.md", root, relative_skill_dir)); + skill.metadata = + enrich_metadata(skill.metadata, system, skill.relative_path.as_deref()); + discovered.push(skill); } } } +} - Ok(discovered) +fn scan_root_skill_pack_fs( + repo_path: &Path, + repo_id_prefix: &str, + discovered: &mut Vec, +) { + let entries = match std::fs::read_dir(repo_path) { + Ok(entries) => entries, + Err(_) => return, + }; + + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + let Some(dir_name) = path.file_name().and_then(|n| n.to_str()) else { + continue; + }; + if dir_name == ".git" || dir_name == ".claude" || dir_name == ".codex" { + continue; + } + + let skill_file = path.join("SKILL.md"); + if !skill_file.exists() { + continue; + } + let relative_skill_dir = slugify_segment(dir_name); + if relative_skill_dir.is_empty() { + continue; + } + let slug = format!("{}/{}", repo_id_prefix, relative_skill_dir); + if let Ok(raw) = std::fs::read(&skill_file) { + let blob_hash = git_blob_hash(&raw); + let mut skill = parse_skill_file(&slug, &String::from_utf8_lossy(&raw)); + skill.blob_hash = Some(blob_hash); + skill.system = Some(ROOT_SKILL_SYSTEM.to_string()); + skill.relative_path = Some(format!("{}/SKILL.md", relative_skill_dir)); + skill.metadata = enrich_metadata( + skill.metadata, + ROOT_SKILL_SYSTEM, + skill.relative_path.as_deref(), + ); + discovered.push(skill); + } + } } /// Scan git tree objects for `SKILL.md` files (works for bare repos). @@ -158,7 +238,6 @@ pub fn scan_repo_tree_for_skills( .map_err(|e| AppError::InternalServerError(format!("no tree: {e}")))?; let mut discovered = Vec::new(); - // Stack: (tree, path_prefix relative to root) let mut stack: Vec<(git2::Tree<'_>, String)> = vec![(tree, String::new())]; while let Some((current_tree, prefix)) = stack.pop() { @@ -183,20 +262,27 @@ pub fn scan_repo_tree_for_skills( } } } - Some(git2::ObjectType::Blob) if name.to_lowercase() == "skill.md" => { - // Derive skill name from parent directory - let dir_name = std::path::Path::new(&entry_path) - .parent() - .and_then(|p| p.file_name()) - .and_then(|n| n.to_str()); - let Some(dir_name) = dir_name else { continue }; + Some(git2::ObjectType::Blob) if name.eq_ignore_ascii_case("SKILL.md") => { + let Some((system, relative_skill_dir, legacy_slug)) = + skill_location_from_path(&entry_path) + else { + continue; + }; - let slug = format!("{}/{}", repo_id_prefix, dir_name); + let slug = if legacy_slug { + format!("{}/{}", repo_id_prefix, relative_skill_dir) + } else { + format!("{}/{}/{}", repo_id_prefix, system, relative_skill_dir) + }; if let Ok(blob) = entry.to_object(git_repo).and_then(|o| o.peel_to_blob()) { let raw = blob.content(); let blob_hash = git_blob_hash(raw); let mut skill = parse_skill_file(&slug, &String::from_utf8_lossy(raw)); skill.blob_hash = Some(blob_hash); + skill.system = Some(system.to_string()); + skill.relative_path = Some(entry_path.clone()); + skill.metadata = + enrich_metadata(skill.metadata, system, skill.relative_path.as_deref()); discovered.push(skill); } } @@ -208,9 +294,93 @@ pub fn scan_repo_tree_for_skills( Ok(discovered) } +fn is_skill_file_name(path: &Path) -> bool { + path.file_name() + .and_then(|n| n.to_str()) + .is_some_and(|name| name.eq_ignore_ascii_case("SKILL.md")) +} + +fn path_to_slug(path: &Path) -> Option { + let parts: Vec = path + .components() + .filter_map(|c| c.as_os_str().to_str()) + .map(slugify_segment) + .filter(|s| !s.is_empty()) + .collect(); + (!parts.is_empty()).then(|| parts.join("/")) +} + +fn slugify_segment(input: &str) -> String { + let mut out = String::with_capacity(input.len()); + let mut last_dash = false; + for ch in input.chars() { + let ch = ch.to_ascii_lowercase(); + if ch.is_ascii_alphanumeric() || ch == '_' || ch == '-' { + out.push(ch); + last_dash = false; + } else if !last_dash { + out.push('-'); + last_dash = true; + } + } + out.trim_matches('-').to_string() +} + +fn skill_location_from_path(path: &str) -> Option<(&'static str, String, bool)> { + let normalized = path.replace('\\', "/"); + for (root, system) in SKILL_ROOTS { + let prefix = format!("{}/", root); + let suffix = "/SKILL.md"; + if normalized.starts_with(&prefix) && normalized.ends_with(suffix) { + let relative = &normalized[prefix.len()..normalized.len() - suffix.len()]; + let slug = relative + .split('/') + .map(slugify_segment) + .filter(|s| !s.is_empty()) + .collect::>() + .join("/"); + if !slug.is_empty() { + return Some((*system, slug, false)); + } + } + } + + let suffix = "/SKILL.md"; + if normalized.ends_with(suffix) && !normalized.starts_with('.') { + let relative = &normalized[..normalized.len() - suffix.len()]; + if !relative.contains('/') { + let slug = slugify_segment(relative); + if !slug.is_empty() { + return Some((ROOT_SKILL_SYSTEM, slug, true)); + } + } + } + + None +} + +fn enrich_metadata( + mut metadata: serde_json::Value, + system: &str, + relative_path: Option<&str>, +) -> serde_json::Value { + if !metadata.is_object() { + metadata = serde_json::json!({}); + } + if let Some(obj) = metadata.as_object_mut() { + obj.entry("system") + .or_insert_with(|| serde_json::Value::String(system.to_string())); + if let Some(relative_path) = relative_path { + obj.entry("path") + .or_insert_with(|| serde_json::Value::String(relative_path.to_string())); + } + } + metadata +} + #[cfg(test)] mod tests { - use super::scan_repo_tree_for_skills; + use super::{scan_repo_for_skills, scan_repo_tree_for_skills}; use git2::{Repository, Signature}; use std::fs; use tempfile::tempdir; @@ -221,7 +391,11 @@ mod tests { let temp = tempdir().expect("tempdir"); let repo = Repository::init(temp.path()).expect("init repo"); - let skill_dir = temp.path().join(".claude").join("skills").join("demo-skill"); + let skill_dir = temp + .path() + .join(".claude") + .join("skills") + .join("demo-skill"); fs::create_dir_all(&skill_dir).expect("create skill dir"); fs::write( skill_dir.join("SKILL.md"), @@ -244,7 +418,86 @@ mod tests { assert_eq!(discovered.len(), 1); assert_eq!(discovered[0].name, "Demo Skill"); - assert_eq!(discovered[0].slug, "00000000/demo-skill"); + assert_eq!(discovered[0].slug, "00000000/claude/demo-skill"); + assert_eq!( + discovered[0].metadata["system"], + serde_json::Value::String("claude".into()) + ); + } + + #[test] + fn tree_scan_keeps_claude_and_codex_skills_separate() { + let temp = tempdir().expect("tempdir"); + let repo = Repository::init(temp.path()).expect("init repo"); + + for root in [".claude", ".codex"] { + let skill_dir = temp.path().join(root).join("skills").join("review"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write( + skill_dir.join("SKILL.md"), + format!("---\nname: {} Review\n---\ncontent", root), + ) + .expect("write skill"); + } + + let mut index = repo.index().expect("index"); + index + .add_path(std::path::Path::new(".claude/skills/review/SKILL.md")) + .expect("add claude skill"); + index + .add_path(std::path::Path::new(".codex/skills/review/SKILL.md")) + .expect("add codex skill"); + index.write().expect("write index"); + let tree_id = index.write_tree().expect("write tree"); + let tree = repo.find_tree(tree_id).expect("find tree"); + let sig = Signature::now("tester", "tester@example.com").expect("signature"); + repo.commit(Some("HEAD"), &sig, &sig, "add skills", &tree, &[]) + .expect("commit"); + + let mut slugs = scan_repo_tree_for_skills(&repo, Uuid::nil()) + .expect("scan tree") + .into_iter() + .map(|s| s.slug) + .collect::>(); + slugs.sort(); + + assert_eq!( + slugs, + vec!["00000000/claude/review", "00000000/codex/review"] + ); + } + + #[test] + fn scans_root_level_skill_pack_layout() { + let temp = tempdir().expect("tempdir"); + let repo = Repository::init(temp.path()).expect("init repo"); + + let skill_dir = temp.path().join("code-review"); + fs::create_dir_all(&skill_dir).expect("create skill dir"); + fs::write(skill_dir.join("SKILL.md"), "# Code Review\n\ncontent").expect("write skill"); + + let mut index = repo.index().expect("index"); + index + .add_path(std::path::Path::new("code-review/SKILL.md")) + .expect("add skill"); + index.write().expect("write index"); + let tree_id = index.write_tree().expect("write tree"); + let tree = repo.find_tree(tree_id).expect("find tree"); + let sig = Signature::now("tester", "tester@example.com").expect("signature"); + repo.commit(Some("HEAD"), &sig, &sig, "add skill", &tree, &[]) + .expect("commit"); + + let fs_discovered = scan_repo_for_skills(temp.path(), Uuid::nil()).expect("scan fs"); + let tree_discovered = scan_repo_tree_for_skills(&repo, Uuid::nil()).expect("scan tree"); + + assert_eq!(fs_discovered.len(), 1); + assert_eq!(tree_discovered.len(), 1); + assert_eq!(fs_discovered[0].slug, "00000000/code-review"); + assert_eq!(tree_discovered[0].slug, "00000000/code-review"); + assert_eq!( + tree_discovered[0].metadata["system"], + serde_json::Value::String("root".into()) + ); } } @@ -297,7 +550,7 @@ pub async fn scan_and_sync_skills( sync_discovered_skills(db, project_uuid, repo.id, discovered).await } -/// Sync discovered skills with deduplication by {repo_id}+{blob_hash}. +/// Sync discovered skills by stable slug. async fn sync_discovered_skills( db: &db::database::AppDatabase, project_uuid: Uuid, @@ -317,26 +570,21 @@ async fn sync_discovered_skills( let mut created = 0i64; let mut updated = 0i64; - // Deduplicate by {repo_id}+{blob_hash}, keep latest by commit_sha + // Deduplicate by slug. The slug includes repo prefix + skill system + relative skill path. let mut deduped: std::collections::HashMap = std::collections::HashMap::new(); for skill in discovered { - let key = format!( - "{}:{}", - repo_id, - skill.blob_hash.as_ref().unwrap_or(&skill.slug) - ); - match deduped.get(&key) { + match deduped.get(&skill.slug) { Some(existing) => { // Keep the one with the later commit_sha if skill.commit_sha.as_ref().unwrap_or(&String::new()) > existing.commit_sha.as_ref().unwrap_or(&String::new()) { - deduped.insert(key, skill); + deduped.insert(skill.slug.clone(), skill); } } None => { - deduped.insert(key, skill); + deduped.insert(skill.slug.clone(), skill); } } } @@ -349,17 +597,8 @@ async fn sync_discovered_skills( .all(db) .await?; - let existing_by_hash: std::collections::HashMap<_, _> = existing - .into_iter() - .map(|s| { - let key = format!( - "{}:{}", - s.repo_id.unwrap_or_default(), - s.blob_hash.clone().unwrap_or_default() - ); - (key, s) - }) - .collect(); + let existing_by_slug: std::collections::HashMap<_, _> = + existing.into_iter().map(|s| (s.slug.clone(), s)).collect(); let mut seen_keys = std::collections::HashSet::new(); @@ -368,12 +607,17 @@ async fn sync_discovered_skills( seen_keys.insert(key.clone()); let json_meta = serde_json::to_value(&skill.metadata).unwrap_or_default(); - if let Some(existing_skill) = existing_by_hash.get(&key) { + if let Some(existing_skill) = existing_by_slug.get(&key) { if existing_skill.content != skill.content || existing_skill.metadata != json_meta || existing_skill.commit_sha != skill.commit_sha + || existing_skill.blob_hash != skill.blob_hash + || existing_skill.name != skill.name + || existing_skill.description != skill.description { let mut active: SkillActiveModel = existing_skill.clone().into(); + active.name = Set(skill.name); + active.description = Set(skill.description); active.content = Set(skill.content); active.metadata = Set(json_meta); active.commit_sha = Set(skill.commit_sha); @@ -407,7 +651,7 @@ async fn sync_discovered_skills( // Remove skills that no longer exist in the repo let mut removed = 0i64; - for (key, old_skill) in existing_by_hash { + for (key, old_skill) in existing_by_slug { if !seen_keys.contains(&key) { SkillEntity::delete_by_id(old_skill.id).exec(db).await?; removed += 1;