Skip to content

Commit 65ce554

Browse files
fix(skills): attribute symlinked skills to real source, not enclosing repo
A skill installed into the shared ~/.agents/skills and symlinked into an agent home (e.g. ~/.claude/skills) was mis-attributed when that home sits inside a dotfiles git repo: detect_source walked the symlink's textual parents, hit ~/.claude/.git, and recorded the dotfiles repo as the source. The git-source backfill then stamped install_type='git' + pack, forking one on-disk skill into two Extension rows (marketplace vs dotfiles). - scanner: canonicalize the skill path before detect_source so a symlinked skill is attributed to its real content's source; plain skills resolve to the same tree and are unchanged. - store: self-heal existing DBs by clearing the bogus git install_meta (and the pack derived from it) for skill rows whose on-disk entry is a symlink and whose freshly-scanned source is non-git; real git installs are plain files, never symlinks, so they are untouched. Run before backfill_packs so cleared rows do not re-acquire a pack. Fixes #87
1 parent bf2dfc6 commit 65ce554

2 files changed

Lines changed: 291 additions & 1 deletion

File tree

crates/hk-core/src/scanner.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,16 @@ pub fn scan_skill_dir(dir: &Path, agent_name: &str) -> Vec<Extension> {
174174
(name, String::new(), vec![])
175175
});
176176

177-
let source = detect_source(&path, true);
177+
// A skill reached through a symlink (e.g. `~/.claude/skills/tdd` ->
178+
// `~/.agents/skills/tdd`) must be attributed to the source of its real
179+
// content, not to a `.git` the link merely sits under: keeping an agent
180+
// home inside a dotfiles repo would otherwise stamp every linked skill
181+
// with that backup remote, masking its true (e.g. marketplace) origin
182+
// and forking one skill into two group rows. Resolve symlinks before
183+
// walking up for `.git`. Plain (non-symlinked) skills canonicalize to
184+
// the same real tree, so their detection is unchanged.
185+
let resolved = std::fs::canonicalize(&path).unwrap_or_else(|_| path.clone());
186+
let source = detect_source(&resolved, true);
178187
let pack = source.url.as_deref().and_then(extract_pack_from_url);
179188
extensions.push(Extension {
180189
id: stable_id(&name, "skill", agent_name),
@@ -2031,6 +2040,40 @@ mod tests {
20312040
assert_eq!(extensions[0].kind, ExtensionKind::Skill);
20322041
}
20332042

2043+
#[cfg(unix)]
2044+
#[test]
2045+
fn test_symlinked_skill_attributed_to_real_source_not_enclosing_repo() {
2046+
// Regression: `~/.claude` kept inside a dotfiles git repo, with a skill
2047+
// symlinked in from the canonical `~/.agents/skills`. Walking textual
2048+
// parents would hit `.claude/.git` and mislabel the skill as a git
2049+
// install of the dotfiles repo. Resolving the symlink first attributes
2050+
// it to the real content's (here sourceless) location.
2051+
use std::os::unix::fs::symlink;
2052+
let dir = TempDir::new().unwrap();
2053+
std::fs::create_dir_all(dir.path().join(".claude").join(".git")).unwrap();
2054+
let claude_skills = dir.path().join(".claude").join("skills");
2055+
std::fs::create_dir_all(&claude_skills).unwrap();
2056+
let real = dir.path().join(".agents").join("skills").join("tdd");
2057+
std::fs::create_dir_all(&real).unwrap();
2058+
std::fs::write(
2059+
real.join("SKILL.md"),
2060+
"---\nname: tdd\ndescription: Test-driven development\n---\n",
2061+
)
2062+
.unwrap();
2063+
symlink(&real, claude_skills.join("tdd")).unwrap();
2064+
2065+
let exts = scan_skill_dir(&claude_skills, "claude");
2066+
assert_eq!(exts.len(), 1);
2067+
assert_eq!(exts[0].name, "tdd");
2068+
assert_ne!(
2069+
exts[0].source.origin,
2070+
SourceOrigin::Git,
2071+
"symlinked skill must not inherit the enclosing dotfiles repo"
2072+
);
2073+
assert!(exts[0].source.url.is_none());
2074+
assert!(exts[0].pack.is_none());
2075+
}
2076+
20342077
#[test]
20352078
fn test_scan_mcp_from_adapter() {
20362079
let dir = TempDir::new().unwrap();

crates/hk-core/src/store.rs

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ fn parse_dt(s: String) -> chrono::DateTime<chrono::Utc> {
6060
})
6161
}
6262

63+
/// The on-disk entry to stat for symlink detection: the containing directory
64+
/// for a `<dir>/SKILL.md` skill, or the standalone `.md` file itself. The
65+
/// scanner always records a dir-skill's `source_path` as `<dir>/SKILL.md`, even
66+
/// when the on-disk file is `SKILL.md.disabled`, so only `SKILL.md` is matched.
67+
fn skill_entry_path(source_path: &str) -> &Path {
68+
let p = Path::new(source_path);
69+
match p.file_name().and_then(|f| f.to_str()) {
70+
Some("SKILL.md") => p.parent().unwrap_or(p),
71+
_ => p,
72+
}
73+
}
74+
6375
/// Upsert SQL for scanner-derived extensions (18 columns, no install meta).
6476
/// Used by `sync_extensions` and `sync_extensions_for_agent`.
6577
const UPSERT_EXTENSION_SQL: &str =
@@ -1048,6 +1060,11 @@ impl Store {
10481060
AND cli_meta_json IS NOT NULL",
10491061
)?;
10501062

1063+
// Undo bogus `git` install_meta the backfill above stamped onto skills
1064+
// reached via a symlink inside an agent-home dotfiles repo. Run before
1065+
// pack backfill so the cleared rows don't re-acquire a pack.
1066+
Self::heal_symlinked_git_install_meta(&tx, extensions)?;
1067+
10511068
// Backfill pack from install_url or source_json URL for deployed extensions
10521069
// that lost their git context after being copied to agent directories
10531070
Self::backfill_packs(&tx)?;
@@ -1152,12 +1169,51 @@ impl Store {
11521169
AND cli_meta_json IS NOT NULL",
11531170
)?;
11541171

1172+
Self::heal_symlinked_git_install_meta(&tx, extensions)?;
1173+
11551174
Self::backfill_packs(&tx)?;
11561175

11571176
tx.commit()?;
11581177
Ok(())
11591178
}
11601179

1180+
/// Clear `git`-typed install_meta (and the pack derived from it) that the
1181+
/// git-source backfill wrongly stamped onto a symlinked skill. Such a skill
1182+
/// is reached through a link sitting inside an agent-home dotfiles repo
1183+
/// (e.g. `~/.claude/skills/X` -> `~/.agents/skills/X`); its real content
1184+
/// lives elsewhere with its own (e.g. marketplace) source. Now that the
1185+
/// scanner resolves symlinks before walking up for `.git`, these rows scan
1186+
/// as non-git, so a leftover `git` install_meta is stale pollution that
1187+
/// forks the skill into a bogus dotfiles-repo group. Real git installs are
1188+
/// plain files (never symlinks), so they are never matched.
1189+
fn heal_symlinked_git_install_meta(
1190+
conn: &rusqlite::Connection,
1191+
extensions: &[Extension],
1192+
) -> Result<(), HkError> {
1193+
for ext in extensions {
1194+
if ext.kind != ExtensionKind::Skill || ext.source.origin == SourceOrigin::Git {
1195+
continue;
1196+
}
1197+
let Some(source_path) = ext.source_path.as_deref() else {
1198+
continue;
1199+
};
1200+
let is_symlink = std::fs::symlink_metadata(skill_entry_path(source_path))
1201+
.map(|m| m.file_type().is_symlink())
1202+
.unwrap_or(false);
1203+
if is_symlink {
1204+
conn.execute(
1205+
"UPDATE extensions
1206+
SET install_type = NULL, install_url = NULL, install_url_resolved = NULL,
1207+
install_branch = NULL, install_subpath = NULL, install_revision = NULL,
1208+
remote_revision = NULL, checked_at = NULL, check_error = NULL, pack = NULL
1209+
WHERE id = ?1 AND install_type = 'git'",
1210+
params![ext.id],
1211+
)?;
1212+
}
1213+
}
1214+
Ok(())
1215+
}
1216+
11611217
/// Backfill `pack` from install_url, source_json URL, or child extensions.
11621218
/// Deployed skills lose their git context after being copied to agent directories,
11631219
/// but install_url retains the repo URL. CLI parent extensions inherit pack from children.
@@ -2489,6 +2545,197 @@ mod tests {
24892545
assert_eq!(im.remote_revision.as_deref(), Some("def456"));
24902546
}
24912547

2548+
#[cfg(unix)]
2549+
#[test]
2550+
fn test_sync_heals_symlinked_git_install_meta() {
2551+
// Regression: the git-source backfill stamped `install_type=git` (+ a
2552+
// pack) onto a skill symlinked in from `~/.agents/skills` while
2553+
// `~/.claude` sat inside a dotfiles repo, forking it into a bogus
2554+
// dotfiles-repo group. Such symlinked, non-git skills must be healed;
2555+
// real file-backed git installs must be left intact.
2556+
use std::os::unix::fs::symlink;
2557+
let (store, dir) = test_store();
2558+
2559+
// Symlinked skill: real content elsewhere, link under .claude/skills.
2560+
let real = dir.path().join(".agents").join("skills").join("tdd");
2561+
std::fs::create_dir_all(&real).unwrap();
2562+
std::fs::write(real.join("SKILL.md"), "---\nname: tdd\n---\n").unwrap();
2563+
let claude_skills = dir.path().join(".claude").join("skills");
2564+
std::fs::create_dir_all(&claude_skills).unwrap();
2565+
let link = claude_skills.join("tdd");
2566+
symlink(&real, &link).unwrap();
2567+
2568+
let mut linked = sample_extension();
2569+
linked.id = "linked-tdd".into();
2570+
linked.name = "tdd".into();
2571+
linked.source.origin = SourceOrigin::Agent;
2572+
linked.source.url = None;
2573+
linked.source_path = Some(link.join("SKILL.md").to_string_lossy().into());
2574+
store.insert_extension(&linked).unwrap();
2575+
store
2576+
.set_install_meta(
2577+
"linked-tdd",
2578+
&InstallMeta {
2579+
install_type: "git".into(),
2580+
url: Some("https://github.com/octo/dotfiles".into()),
2581+
url_resolved: None,
2582+
branch: None,
2583+
subpath: None,
2584+
revision: Some("03dc45c".into()),
2585+
remote_revision: None,
2586+
checked_at: None,
2587+
check_error: None,
2588+
},
2589+
)
2590+
.unwrap();
2591+
store.update_pack("linked-tdd", Some("octo/dotfiles")).unwrap();
2592+
2593+
// Real (non-symlink) git install: must survive untouched.
2594+
let realdir = dir.path().join(".codex").join("skills").join("foo");
2595+
std::fs::create_dir_all(&realdir).unwrap();
2596+
std::fs::write(realdir.join("SKILL.md"), "---\nname: foo\n---\n").unwrap();
2597+
let mut plain = sample_extension();
2598+
plain.id = "plain-foo".into();
2599+
plain.name = "foo".into();
2600+
plain.source.origin = SourceOrigin::Agent;
2601+
plain.source_path = Some(realdir.join("SKILL.md").to_string_lossy().into());
2602+
store.insert_extension(&plain).unwrap();
2603+
store
2604+
.set_install_meta(
2605+
"plain-foo",
2606+
&InstallMeta {
2607+
install_type: "git".into(),
2608+
url: Some("https://github.com/real/repo".into()),
2609+
url_resolved: None,
2610+
branch: None,
2611+
subpath: None,
2612+
revision: None,
2613+
remote_revision: None,
2614+
checked_at: None,
2615+
check_error: None,
2616+
},
2617+
)
2618+
.unwrap();
2619+
2620+
// Disabled symlinked skill: on disk the file is `SKILL.md.disabled`, but
2621+
// the scanner records source_path as `<dir>/SKILL.md` (see scanner
2622+
// scan_skill_dir), and the entry dir is still a symlink, so it must heal
2623+
// too.
2624+
let real_dis = dir.path().join(".agents").join("skills").join("ddd");
2625+
std::fs::create_dir_all(&real_dis).unwrap();
2626+
std::fs::write(real_dis.join("SKILL.md.disabled"), "---\nname: ddd\n---\n").unwrap();
2627+
let link_dis = claude_skills.join("ddd");
2628+
symlink(&real_dis, &link_dis).unwrap();
2629+
let mut disabled = sample_extension();
2630+
disabled.id = "linked-ddd".into();
2631+
disabled.name = "ddd".into();
2632+
disabled.enabled = false;
2633+
disabled.source.origin = SourceOrigin::Agent;
2634+
disabled.source.url = None;
2635+
disabled.source_path = Some(link_dis.join("SKILL.md").to_string_lossy().into());
2636+
store.insert_extension(&disabled).unwrap();
2637+
store
2638+
.set_install_meta(
2639+
"linked-ddd",
2640+
&InstallMeta {
2641+
install_type: "git".into(),
2642+
url: Some("https://github.com/octo/dotfiles".into()),
2643+
url_resolved: None,
2644+
branch: None,
2645+
subpath: None,
2646+
revision: None,
2647+
remote_revision: None,
2648+
checked_at: None,
2649+
check_error: None,
2650+
},
2651+
)
2652+
.unwrap();
2653+
store.update_pack("linked-ddd", Some("octo/dotfiles")).unwrap();
2654+
2655+
// Symlinked skill whose *real content* is itself a git repo: the scanner
2656+
// reports origin=Git, so heal must skip it (symlink alone is not enough)
2657+
// and preserve its install_meta + pack.
2658+
let real_git = dir.path().join(".agents").join("skills").join("ggg");
2659+
std::fs::create_dir_all(&real_git).unwrap();
2660+
std::fs::write(real_git.join("SKILL.md"), "---\nname: ggg\n---\n").unwrap();
2661+
let link_git = claude_skills.join("ggg");
2662+
symlink(&real_git, &link_git).unwrap();
2663+
let mut gitlink = sample_extension();
2664+
gitlink.id = "linked-ggg".into();
2665+
gitlink.name = "ggg".into();
2666+
gitlink.source.origin = SourceOrigin::Git;
2667+
gitlink.source.url = Some("https://github.com/team/shared".into());
2668+
gitlink.source_path = Some(link_git.join("SKILL.md").to_string_lossy().into());
2669+
store.insert_extension(&gitlink).unwrap();
2670+
store
2671+
.set_install_meta(
2672+
"linked-ggg",
2673+
&InstallMeta {
2674+
install_type: "git".into(),
2675+
url: Some("https://github.com/team/shared".into()),
2676+
url_resolved: None,
2677+
branch: None,
2678+
subpath: None,
2679+
revision: None,
2680+
remote_revision: None,
2681+
checked_at: None,
2682+
check_error: None,
2683+
},
2684+
)
2685+
.unwrap();
2686+
store.update_pack("linked-ggg", Some("team/shared")).unwrap();
2687+
2688+
// Re-sync as the scanner now reports them (no install_meta; the
2689+
// git-backed symlink keeps origin=Git).
2690+
let mut s_linked = linked.clone();
2691+
s_linked.install_meta = None;
2692+
s_linked.pack = None;
2693+
let mut s_plain = plain.clone();
2694+
s_plain.install_meta = None;
2695+
let mut s_disabled = disabled.clone();
2696+
s_disabled.install_meta = None;
2697+
s_disabled.pack = None;
2698+
let mut s_gitlink = gitlink.clone();
2699+
s_gitlink.install_meta = None;
2700+
store
2701+
.sync_extensions(&[s_linked, s_plain, s_disabled, s_gitlink])
2702+
.unwrap();
2703+
2704+
let healed = store.get_extension("linked-tdd").unwrap().unwrap();
2705+
assert!(
2706+
healed.install_meta.is_none(),
2707+
"symlinked skill's bogus git install_meta should be cleared"
2708+
);
2709+
assert!(
2710+
healed.pack.is_none(),
2711+
"symlinked skill's dotfiles-repo pack should be cleared"
2712+
);
2713+
2714+
let kept = store.get_extension("plain-foo").unwrap().unwrap();
2715+
assert_eq!(
2716+
kept.install_meta.expect("real git install preserved").install_type,
2717+
"git"
2718+
);
2719+
2720+
let healed_dis = store.get_extension("linked-ddd").unwrap().unwrap();
2721+
assert!(
2722+
healed_dis.install_meta.is_none(),
2723+
"disabled symlinked skill (SKILL.md.disabled) must heal too"
2724+
);
2725+
assert!(healed_dis.pack.is_none(), "disabled symlinked skill's pack must clear");
2726+
2727+
let kept_git = store.get_extension("linked-ggg").unwrap().unwrap();
2728+
assert_eq!(
2729+
kept_git.install_meta.expect("git-backed symlink preserved").install_type,
2730+
"git"
2731+
);
2732+
assert_eq!(
2733+
kept_git.pack.as_deref(),
2734+
Some("team/shared"),
2735+
"git-backed symlink's pack must survive (heal skips origin=Git)"
2736+
);
2737+
}
2738+
24922739
#[test]
24932740
fn test_stale_row_should_prune_decision() {
24942741
let cli = ExtensionKind::Cli.as_str();

0 commit comments

Comments
 (0)