From 6e40aba6f6c5cb905b84bd4015cb66620321c71e Mon Sep 17 00:00:00 2001 From: Mike Wassmer Date: Tue, 24 Feb 2026 21:25:55 +0100 Subject: [PATCH] fix: match installed skills by source repo, not just name (#190) --- interface/src/api/client.ts | 1 + interface/src/routes/AgentSkills.tsx | 14 ++- src/api/skills.rs | 3 + src/skills.rs | 8 ++ src/skills/installer.rs | 132 ++++++++++++++++++++++++++- 5 files changed, 153 insertions(+), 5 deletions(-) diff --git a/interface/src/api/client.ts b/interface/src/api/client.ts index 2683cc1e3..7cb33a66c 100644 --- a/interface/src/api/client.ts +++ b/interface/src/api/client.ts @@ -813,6 +813,7 @@ export interface SkillInfo { file_path: string; base_dir: string; source: "instance" | "workspace"; + source_repo?: string; } export interface SkillsListResponse { diff --git a/interface/src/routes/AgentSkills.tsx b/interface/src/routes/AgentSkills.tsx index bd386ba1e..53ad106e8 100644 --- a/interface/src/routes/AgentSkills.tsx +++ b/interface/src/routes/AgentSkills.tsx @@ -266,8 +266,13 @@ export function AgentSkills({ agentId }: AgentSkillsProps) { }); const installedSkills = skillsData?.skills ?? []; - const installedSkillNames = new Map( - installedSkills.map((skill) => [skill.name.toLowerCase(), skill.name]), + const installedKeys = new Map( + installedSkills.map((s) => [ + s.source_repo + ? `${s.source_repo}/${s.name}`.toLowerCase() + : s.name.toLowerCase(), + s.name, + ]), ); // Flatten browse pages or use search results @@ -462,7 +467,10 @@ export function AgentSkills({ agentId }: AgentSkillsProps) {
{registrySkills.map((skill) => { const spec = installSpec(skill); - const installedName = installedSkillNames.get(skill.name.toLowerCase()); + const compositeKey = `${skill.source}/${skill.name}`.toLowerCase(); + const installedName = + installedKeys.get(compositeKey) ?? + installedKeys.get(skill.name.toLowerCase()); const isInstalled = Boolean(installedName); return ( , } #[derive(Serialize)] @@ -145,6 +147,7 @@ pub(super) async fn list_skills( crate::skills::SkillSource::Instance => "instance".to_string(), crate::skills::SkillSource::Workspace => "workspace".to_string(), }, + source_repo: s.source_repo, }) .collect(); diff --git a/src/skills.rs b/src/skills.rs index da6cfe550..33136a429 100644 --- a/src/skills.rs +++ b/src/skills.rs @@ -35,6 +35,8 @@ pub struct Skill { pub content: String, /// Where this skill was loaded from. pub source: SkillSource, + /// GitHub `owner/repo` that this skill was installed from, if any. + pub source_repo: Option, } /// Where a skill was loaded from, used for precedence tracking. @@ -213,6 +215,7 @@ impl SkillSet { file_path: s.file_path.clone(), base_dir: s.base_dir.clone(), source: s.source.clone(), + source_repo: s.source_repo.clone(), }) .collect() } @@ -226,6 +229,7 @@ pub struct SkillInfo { pub file_path: PathBuf, pub base_dir: PathBuf, pub source: SkillSource, + pub source_repo: Option, } /// Load all skills from a directory. @@ -293,6 +297,7 @@ async fn load_skill( }); let description = frontmatter.get("description").cloned().unwrap_or_default(); + let source_repo = frontmatter.get("source_repo").cloned(); // Resolve {baseDir} template variable in the body let base_dir_str = base_dir.to_string_lossy(); @@ -305,6 +310,7 @@ async fn load_skill( base_dir: base_dir.to_path_buf(), content, source, + source_repo, }) } @@ -465,6 +471,7 @@ mod tests { base_dir: PathBuf::from("/skills/weather"), content: "# Weather\n\nUse curl.".into(), source: SkillSource::Instance, + source_repo: None, }, ); @@ -487,6 +494,7 @@ mod tests { base_dir: PathBuf::from("/skills/weather"), content: "# Weather\n\nUse curl.".into(), source: SkillSource::Instance, + source_repo: None, }, ); diff --git a/src/skills/installer.rs b/src/skills/installer.rs index bbb4d785a..1f8c82e41 100644 --- a/src/skills/installer.rs +++ b/src/skills/installer.rs @@ -63,7 +63,11 @@ pub async fn install_from_github(spec: &str, target_dir: &Path) -> Result Result, + source_repo: Option<&str>, ) -> Result> { let file = std::fs::File::open(zip_path).context("failed to open zip file")?; @@ -176,6 +181,32 @@ async fn extract_and_install( // Copy skill directory copy_dir_recursive(&skill_dir, &target_skill_dir).await?; + // Write source_repo into SKILL.md frontmatter so we can track provenance + if let Some(repo) = source_repo { + let skill_md = target_skill_dir.join("SKILL.md"); + if skill_md.exists() { + match fs::read_to_string(&skill_md).await { + Ok(content) => { + let patched = inject_source_repo(&content, repo); + if let Err(error) = fs::write(&skill_md, patched).await { + tracing::warn!( + skill = %skill_name, + %error, + "failed to write source_repo to SKILL.md" + ); + } + } + Err(error) => { + tracing::warn!( + skill = %skill_name, + %error, + "failed to read SKILL.md for source_repo injection" + ); + } + } + } + } + installed.push(skill_name.to_string()); tracing::debug!( @@ -188,6 +219,45 @@ async fn extract_and_install( Ok(installed) } +/// Inject or update `source_repo` in SKILL.md frontmatter. +fn inject_source_repo(content: &str, repo: &str) -> String { + let trimmed = content.trim_start(); + let line = format!("source_repo: {repo}"); + + if !trimmed.starts_with("---") { + // No frontmatter — add one + return format!("---\n{line}\n---\n{content}"); + } + + let after_opening = &trimmed[3..]; + let Some((end_pos, delimiter_len)) = after_opening + .find("\n---\n") + .map(|pos| (pos, 5)) + .or_else(|| after_opening.find("\n---").map(|pos| (pos, 4))) + else { + // Malformed frontmatter, prepend + return format!("---\n{line}\n---\n{content}"); + }; + + let fm_block = &after_opening[..end_pos]; + let body = &after_opening[end_pos + delimiter_len..]; + + // Remove any existing source_repo line + let filtered: Vec<&str> = fm_block + .lines() + .filter(|l| { + !l.trim_start() + .starts_with("source_repo:") + }) + .collect(); + + let mut new_fm = filtered.join("\n"); + new_fm.push('\n'); + new_fm.push_str(&line); + + format!("---{new_fm}\n---\n{body}") +} + /// Parse a GitHub spec: `owner/repo` or `owner/repo/skill-name` fn parse_github_spec(spec: &str) -> Result<(String, String, Option)> { let parts: Vec<&str> = spec.split('/').collect(); @@ -308,4 +378,62 @@ mod tests { assert!(parse_github_spec("invalid").is_err()); assert!(parse_github_spec("too/many/slashes/here").is_err()); } + + #[test] + fn test_inject_source_repo_into_existing_frontmatter() { + let content = "---\nname: weather\ndescription: Get weather\n---\n\n# Weather\n"; + let result = inject_source_repo(content, "anthropics/skills"); + assert!(result.contains("source_repo: anthropics/skills")); + assert!(result.contains("name: weather")); + assert!(result.contains("# Weather")); + // source_repo should be inside the frontmatter delimiters + let after_first = result.splitn(2, "---").nth(1).unwrap(); + let fm = after_first.splitn(2, "\n---").next().unwrap(); + assert!(fm.contains("source_repo: anthropics/skills")); + } + + #[test] + fn test_inject_source_repo_no_frontmatter() { + let content = "# Just markdown\n\nNo frontmatter here."; + let result = inject_source_repo(content, "owner/repo"); + assert!(result.starts_with("---\nsource_repo: owner/repo\n---\n")); + assert!(result.contains("# Just markdown")); + } + + #[test] + fn test_inject_source_repo_updates_existing() { + let content = "---\nname: weather\nsource_repo: old/repo\ndescription: foo\n---\n\nBody\n"; + let result = inject_source_repo(content, "new/repo"); + assert!(result.contains("source_repo: new/repo")); + assert!(!result.contains("old/repo")); + // Should only have one source_repo line + assert_eq!(result.matches("source_repo:").count(), 1); + } + + #[test] + fn test_inject_source_repo_malformed_frontmatter() { + let content = "---\nname: broken\nno closing delimiter"; + let result = inject_source_repo(content, "owner/repo"); + // Falls back to prepending new frontmatter + assert!(result.starts_with("---\nsource_repo: owner/repo\n---\n")); + assert!(result.contains("no closing delimiter")); + } + + #[test] + fn test_inject_source_repo_preserves_delimiter_body_newline() { + let content = "---\nname: weather\n---\n# Weather\n"; + let patched = inject_source_repo(content, "anthropics/skills"); + assert!(patched.contains("\n---\n# Weather\n")); + } + + #[test] + fn test_inject_source_repo_roundtrip_with_parse() { + use crate::skills::parse_frontmatter; + let content = "---\nname: weather\ndescription: Get weather\n---\n\n# Weather\n"; + let patched = inject_source_repo(content, "anthropics/skills"); + let (fm, body) = parse_frontmatter(&patched).unwrap(); + assert_eq!(fm.get("source_repo").unwrap(), &"anthropics/skills".to_string()); + assert_eq!(fm.get("name").unwrap(), &"weather".to_string()); + assert!(body.contains("# Weather")); + } }