From a995951d7592b8b8bb48a8442d03c0b09e109694 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 15:27:31 +1100 Subject: [PATCH 1/3] feat(pnpm): recognize package-managed Skills --- crates/skilld-command/src/doctor.rs | 22 +++++--- crates/skilld-command/src/doctor_actions.rs | 5 ++ crates/skilld-command/src/lib.rs | 1 + crates/skilld-command/src/outdated.rs | 8 ++- crates/skilld-command/src/pnpm.rs | 36 +++++++++++++ crates/skilld-command/tests/doctor.rs | 58 +++++++++++++++++++++ crates/skilld-command/tests/outdated.rs | 37 +++++++++++++ crates/skilld-native/src/doctor_ui.rs | 3 ++ skills/generate-package-skill/SKILL.md | 4 ++ skills/skilld/SKILL.md | 2 + 10 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 crates/skilld-command/src/pnpm.rs diff --git a/crates/skilld-command/src/doctor.rs b/crates/skilld-command/src/doctor.rs index c257c552..ae31c300 100644 --- a/crates/skilld-command/src/doctor.rs +++ b/crates/skilld-command/src/doctor.rs @@ -204,6 +204,7 @@ impl SkillLocation { pub enum DoctorOwner { SkillsSh { record: ForeignRecord }, Skilld { store: PathBuf }, + Pnpm { package: String, version: String }, Plugin, Source, Unknown, @@ -215,6 +216,7 @@ impl DoctorOwner { match self { Self::SkillsSh { .. } => "skills.sh", Self::Skilld { .. } => "skilld", + Self::Pnpm { .. } => "pnpm", Self::Plugin => "Plugin", Self::Source => "Source", Self::Unknown => "Unknown", @@ -773,12 +775,6 @@ pub fn scan_with_progress( .unwrap_or_default() .to_string_lossy() .into_owned(); - if SkillName::parse(name.clone()).is_err() { - report.problems.push(ScanProblem { - path: canonical.clone(), - message: "Invalid Skill directory name.".into(), - }); - } let owner = owner( &canonical, &name, @@ -788,6 +784,12 @@ pub fn scan_with_progress( &mut locks, &mut report.problems, ); + if !matches!(owner, DoctorOwner::Pnpm { .. }) && SkillName::parse(name.clone()).is_err() { + report.problems.push(ScanProblem { + path: canonical.clone(), + message: "Invalid Skill directory name.".into(), + }); + } let fingerprint = if canonical.join(".git").exists() && matches!(owner, DoctorOwner::Source) { None @@ -861,6 +863,14 @@ fn owner( locks: &mut BTreeMap>, problems: &mut Vec, ) -> DoctorOwner { + for path in paths.iter().filter(|path| path.agent.is_some()) { + if let Some(package) = crate::pnpm::linked_package(&path.path, canonical) { + return DoctorOwner::Pnpm { + package: package.name, + version: package.version, + }; + } + } if canonical.components().any(|p| p.as_os_str() == "plugins") || canonical.starts_with(context.home.join(".codex/skills/.system")) { diff --git a/crates/skilld-command/src/doctor_actions.rs b/crates/skilld-command/src/doctor_actions.rs index 0b35c0b1..29f5a00d 100644 --- a/crates/skilld-command/src/doctor_actions.rs +++ b/crates/skilld-command/src/doctor_actions.rs @@ -236,6 +236,11 @@ impl LocalHost { .get(index) .ok_or_else(|| failure("Select a Skill first."))?; let record = match &selected.owner { + DoctorOwner::Pnpm { .. } => { + return Err(failure( + "pnpm owns this Skill. Update or remove its package with pnpm.", + )); + } DoctorOwner::SkillsSh { record } => Some(record.clone()), DoctorOwner::Unknown if action == DoctorAction::Remove => None, _ => { diff --git a/crates/skilld-command/src/lib.rs b/crates/skilld-command/src/lib.rs index 91b8da70..29248e3e 100644 --- a/crates/skilld-command/src/lib.rs +++ b/crates/skilld-command/src/lib.rs @@ -8,6 +8,7 @@ pub mod doctor_metadata; pub use dependencies::{ExternalReference, external_references}; mod local_store; mod outdated; +mod pnpm; pub use outdated::{NoOutdatedProgress, OutdatedProgress, ancestor_roots}; mod output; mod provenance; diff --git a/crates/skilld-command/src/outdated.rs b/crates/skilld-command/src/outdated.rs index c1dd1ed5..fae72b0f 100644 --- a/crates/skilld-command/src/outdated.rs +++ b/crates/skilld-command/src/outdated.rs @@ -87,12 +87,18 @@ pub(crate) fn scan_unmanaged( let Some(name) = entry.file_name().to_str().map(ToOwned::to_owned) else { continue; }; - if SkillName::parse(name.clone()).is_err() || !entry.path().join("SKILL.md").is_file() { + if !entry.path().join("SKILL.md").is_file() { continue; } let Some(canonical) = fs::canonicalize(entry.path()).ok() else { continue; }; + // pnpm updates these through dependency versions. Never suggest replacing its links. + if crate::pnpm::linked_package(&entry.path(), &canonical).is_some() + || SkillName::parse(name.clone()).is_err() + { + continue; + } if stores.iter().any(|root| canonical.starts_with(root)) { continue; } diff --git a/crates/skilld-command/src/pnpm.rs b/crates/skilld-command/src/pnpm.rs new file mode 100644 index 00000000..ecd23166 --- /dev/null +++ b/crates/skilld-command/src/pnpm.rs @@ -0,0 +1,36 @@ +use std::fs; +use std::path::Path; + +use serde::Deserialize; + +#[derive(Deserialize)] +pub(crate) struct PnpmPackage { + pub name: String, + pub version: String, +} + +/// A pnpm prefix alone never establishes ownership. Check the link and package metadata. +pub(crate) fn linked_package(link: &Path, canonical: &Path) -> Option { + if !fs::symlink_metadata(link).ok()?.file_type().is_symlink() { + return None; + } + let skills = canonical.parent()?; + if skills.file_name()? != "skills" + || !canonical + .components() + .any(|part| part.as_os_str() == "node_modules") + { + return None; + } + let manifest = skills.parent()?.join("package.json"); + if fs::metadata(&manifest).ok()?.len() > 64 * 1024 { + return None; + } + let package: PnpmPackage = serde_json::from_slice(&fs::read(manifest).ok()?).ok()?; + if package.name.is_empty() || package.version.is_empty() { + return None; + } + let skill = canonical.file_name()?.to_str()?; + let expected = format!("pnpm-{}-{skill}", package.name.replace('/', "+")); + (link.file_name()?.to_str()? == expected).then_some(package) +} diff --git a/crates/skilld-command/tests/doctor.rs b/crates/skilld-command/tests/doctor.rs index 37808c4d..f54223d1 100644 --- a/crates/skilld-command/tests/doctor.rs +++ b/crates/skilld-command/tests/doctor.rs @@ -25,6 +25,64 @@ fn context(home: &Path) -> ScanContext { } } +#[cfg(unix)] +#[test] +fn pnpm_links_belong_to_the_package_manager() { + let home = tempfile::tempdir().unwrap(); + skill( + home.path(), + "app/node_modules/@acme/kit/skills/auth", + "Bundled", + ); + fs::write( + home.path().join("app/node_modules/@acme/kit/package.json"), + r#"{"name":"@acme/kit","version":"1.2.3"}"#, + ) + .unwrap(); + fs::create_dir_all(home.path().join("app/.agents/skills")).unwrap(); + std::os::unix::fs::symlink( + home.path().join("app/node_modules/@acme/kit/skills/auth"), + home.path().join("app/.agents/skills/pnpm-@acme+kit-auth"), + ) + .unwrap(); + let report = scan(&DoctorOptions::for_root(home.path()), &context(home.path())).unwrap(); + assert_eq!(report.skills.len(), 1); + assert_eq!(report.skills[0].owner.label(), "pnpm"); + assert!(report.problems.is_empty(), "{:?}", report.problems); + let owner = serde_json::to_value(&report.skills[0].owner).unwrap(); + assert_eq!(owner["package"], "@acme/kit"); + assert_eq!(owner["version"], "1.2.3"); + let host = skilld_command::LocalHost::new(home.path().join("app"), home.path().join("data")); + for action in [ + skilld_command::doctor_actions::DoctorAction::Remove, + skilld_command::doctor_actions::DoctorAction::Migrate, + ] { + let error = host + .doctor_plan(&report, 0, action) + .err() + .expect("pnpm ownership blocks changes"); + assert!(error.message.contains("pnpm owns this Skill")); + } + assert!( + home.path() + .join("app/.agents/skills/pnpm-@acme+kit-auth") + .is_symlink() + ); +} + +#[cfg(unix)] +#[test] +fn pnpm_prefix_does_not_claim_an_ordinary_skill() { + let home = tempfile::tempdir().unwrap(); + skill( + home.path(), + "app/.agents/skills/pnpm-kit-auth", + "User-owned", + ); + let report = scan(&DoctorOptions::for_root(home.path()), &context(home.path())).unwrap(); + assert_eq!(report.skills[0].owner.label(), "Unknown"); +} + #[test] fn sweep_keeps_hidden_claude_skills_and_prunes_dependency_and_session_trees() { let home = tempfile::tempdir().unwrap(); diff --git a/crates/skilld-command/tests/outdated.rs b/crates/skilld-command/tests/outdated.rs index 8564b0ee..b6a841f0 100644 --- a/crates/skilld-command/tests/outdated.rs +++ b/crates/skilld-command/tests/outdated.rs @@ -297,6 +297,43 @@ fn unmanaged_skill(home: &Path, agent_dir: &str, name: &str) { .unwrap(); } +#[cfg(unix)] +#[test] +fn outdated_does_not_replace_pnpm_owned_links() { + let temporary = tempfile::tempdir().unwrap(); + let project = temporary.path().join("project"); + let bundled = project.join("node_modules/kit/skills/auth"); + fs::create_dir_all(&bundled).unwrap(); + fs::write( + bundled.join("SKILL.md"), + "---\nname: auth\ndescription: Auth.\n---\n", + ) + .unwrap(); + fs::write( + project.join("node_modules/kit/package.json"), + r#"{"name":"kit","version":"1.2.3"}"#, + ) + .unwrap(); + fs::create_dir_all(project.join(".agents/skills")).unwrap(); + let link = project.join(".agents/skills/pnpm-kit-auth"); + std::os::unix::fs::symlink(&bundled, &link).unwrap(); + let provider = Arc::new(Provider::new("---\nname: example\n---\n")); + let host = LocalHost::new(project, temporary.path().join("data")) + .with_remote_provider(provider.clone()); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let result = run( + ["skilld", "outdated", "--all"], + &host, + &mut stdout, + &mut stderr, + ); + assert_eq!(result.exit_code, 0); + assert_eq!(provider.search_calls.load(Ordering::Relaxed), 0); + assert!(!String::from_utf8(stdout).unwrap().contains("Delete")); + assert!(link.is_symlink()); +} + #[test] fn outdated_reports_current_and_stale_project_skills() { let temporary = tempfile::tempdir().unwrap(); diff --git a/crates/skilld-native/src/doctor_ui.rs b/crates/skilld-native/src/doctor_ui.rs index 42347341..0b13f8d6 100644 --- a/crates/skilld-native/src/doctor_ui.rs +++ b/crates/skilld-native/src/doctor_ui.rs @@ -714,6 +714,9 @@ Linked paths include links in parent folders." } DoctorOwner::Unknown => "\nNo installation record found.\nAction: d review removal.", DoctorOwner::Skilld { .. } => "\nManaged by skilld. Use skilld update or remove.", + DoctorOwner::Pnpm { .. } => { + "\nManaged by pnpm. Update or remove its package with pnpm." + } DoctorOwner::Plugin => "\nManaged by a plugin. Change it through its plugin manager.", DoctorOwner::Source => "\nSource directory. No cleanup action applies here.", DoctorOwner::Unavailable { .. } => { diff --git a/skills/generate-package-skill/SKILL.md b/skills/generate-package-skill/SKILL.md index e9674b8c..fee17ccd 100644 --- a/skills/generate-package-skill/SKILL.md +++ b/skills/generate-package-skill/SKILL.md @@ -144,6 +144,10 @@ Report to the user: Give each item expected, observed, a minimal repro with its output, and a source link at the release tag. File it only if asked. - The source path or documentation URL behind each version-specific rule. - If the Skill sits inside the published package directory, add its directory to `files` in `package.json`. After one build, list packed files with `npm pack --dry-run --ignore-scripts`; `pnpm pack` rejects that flag. +- For npm packages, use `skills//SKILL.md` beside the package's `package.json`. Include linked files in the tarball. + [pnpm 12.11 and newer](https://pnpm.io/agent-skills) link Skills from approved direct dependencies. + Document `pnpm approve` for developers who want these links. Approval covers every Skill and later version of the package. + Never approve a package for the user without their request. pnpm owns its links, dependency updates, and removal. - Edit the README beside the Skill's `package.json`. If it already links the skilld.dev page, keep that link. Else add the badge below after the others. Replace a `skilld add` tip that names this package in place with the tip below. Else add the tip after the install command. Replace `OWNER`, `REPOSITORY`, and `PACKAGE`. Count every `SKILL.md` in the Repository, hidden Agent folders included; the skilld.dev indexer skips test and fixture folders. diff --git a/skills/skilld/SKILL.md b/skills/skilld/SKILL.md index 69673f7d..3e89a962 100644 --- a/skills/skilld/SKILL.md +++ b/skills/skilld/SKILL.md @@ -431,6 +431,8 @@ Use `--exclude GLOB` to prune more paths. Use `--include-excluded` or `--include-worktrees` only when those files matter to the task. Read owner labels, duplicate paths, source matches, and scan problems separately. +The `pnpm` owner names Skills linked from npm dependencies. Update these through pnpm, not skilld. +Doctor never migrates or removes pnpm-owned links. `outdated --all` excludes them from replacement suggestions. Exit code `1` with success data means some files could not be checked. Use `--check-sources` for up to 20 source candidates through skilld.dev. A name match alone never establishes provenance. From 17bd5e8bed78008ce975cfa2479f901ffa9eb671 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 15:34:16 +1100 Subject: [PATCH 2/3] fix(pnpm): bundle the public skilld Skill in npm --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 7953e522..b601e736 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,8 @@ }, "files": [ "bin", - "loader" + "loader", + "skills/skilld" ], "engines": { "node": ">=22.0.0" From 6b6fa7c773b226a4cf7273ced1a5544050bba9d9 Mon Sep 17 00:00:00 2001 From: Harlan Wilton Date: Sat, 10 Oct 2026 15:46:47 +1100 Subject: [PATCH 3/3] fix(pnpm): show package Skills in the doctor browser --- crates/skilld-native/src/doctor_ui.rs | 4 ++++ crates/skilld-native/tests/doctor_ui.rs | 29 +++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/skilld-native/src/doctor_ui.rs b/crates/skilld-native/src/doctor_ui.rs index 0b13f8d6..fa5719b0 100644 --- a/crates/skilld-native/src/doctor_ui.rs +++ b/crates/skilld-native/src/doctor_ui.rs @@ -209,6 +209,7 @@ impl Model { ("Unavailable", "Resolve ownership problems"), ("Duplicates", "Review duplicate copies"), ("skilld", "Browse skilld installs"), + ("pnpm", "Browse pnpm-linked Skills"), ("Plugin", "Browse plugin Skills"), ("Source", "Browse source directories"), ] @@ -623,6 +624,9 @@ Linked paths include links in parent folders." Browse::Locations { owner: "skilld" } => { "Managed by skilld. Use skilld update or remove for these Skills." } + Browse::Locations { owner: "pnpm" } => { + "Managed by pnpm. Update or remove the package with pnpm." + } Browse::Locations { owner: "Plugin" } => { "Managed by plugins. Use the plugin manager to make changes." } diff --git a/crates/skilld-native/tests/doctor_ui.rs b/crates/skilld-native/tests/doctor_ui.rs index 2c8d4540..bd2506a2 100644 --- a/crates/skilld-native/tests/doctor_ui.rs +++ b/crates/skilld-native/tests/doctor_ui.rs @@ -407,3 +407,32 @@ fn linked_parent_paths_are_in_the_symlink_view_and_explain_the_parent_link() { let directories = render_snapshot(&model, 180, 60); assert!(!directories.contains("project/.claude")); } + +#[cfg(unix)] +#[test] +fn pnpm_skills_are_visible_and_explain_package_ownership() { + let root = tempfile::tempdir().unwrap(); + let package = root.path().join("node_modules/example"); + let skill = package.join("skills/read-files"); + let target = root.path().join(".agents/skills"); + fs::create_dir_all(&skill).unwrap(); + fs::create_dir_all(&target).unwrap(); + fs::write( + package.join("package.json"), + r#"{"name":"example","version":"1.0.0"}"#, + ) + .unwrap(); + fs::write(skill.join("SKILL.md"), "# Read files\nRead the project.\n").unwrap(); + std::os::unix::fs::symlink(&skill, target.join("pnpm-example-read-files")).unwrap(); + let mut model = Model { + report: Some(scan_fixture(root.path())), + work: WorkState::Ready, + ..Model::default() + }; + assert!(render_snapshot(&model, 100, 26).contains("Browse pnpm-linked Skills")); + model.open_group(); + model.open_group(); + let snapshot = render_snapshot(&model, 180, 60); + assert!(snapshot.contains("pnpm-example-read-files")); + assert!(snapshot.contains("Managed by pnpm")); +}