feat(skill-registry): index npm-package skills (node_modules/*/skills)

[ChaoXu1997]

Problem

extensions/skill-registry.ts scans only loose skill directories (~/.pi/agent/skills, <cwd>/.claude/skills, ~/.trae/skills, …) and misses npm-package-installed skills — the ones living at ~/.pi/agent/npm/node_modules/*/skills/.

These include gentle-pi's own skills (gentle-ai-*, comment-writer, cognitive-doc-design, release, work-unit-commits), plus context-mode, pi-lens, pi-mcp-adapter, pi-subagents, pi-intercom — roughly 26 skills that pi loads into the agent's <available_skills> but the registry never indexes.

Impact

Subagent delegation consumes .atl/skill-registry.md to discover and pass skill paths. Because npm-package skills aren't indexed, a delegated subagent can't see them — e.g. a task delegated to a subagent that needs a gentle-ai / context-mode / pi-lens skill gets skill_resolution: none, even though the skill is installed and available to the parent agent.

On my machine the registry went from 53 → 79 skills after this patch (26 package skills correctly indexed under a new package scope).

Fix

1. discoverNpmPackageSkillDirs() — scans ~/.pi/agent/npm/node_modules/*/skills/ and returns the existing per-package skill dirs.
2. Wire it into regenerateRegistry (scan list) and startSkillRegistryWatcher (watch dirs), so adding/removing a package skill refreshes the registry.
3. scopeForPath gains a "package" scope for node_modules paths, so the registry distinguishes project / user / package sources.

+async function discoverNpmPackageSkillDirs(): Promise<string[]> {
+	const home = homedir();
+	const nodeModules = join(home, ".pi/agent/npm/node_modules");
+	if (!(await pathExists(nodeModules))) return [];
+	… // readdir node_modules, collect <pkg>/skills dirs that exist
+}

 const existingDirs = await uniqueExistingDirs([
 	...projectSkillDirs(cwd),
 	...userSkillDirs(),
+	...(await discoverNpmPackageSkillDirs()),
 ]);

 function scopeForPath(cwd, path) {
+	const cleanPath = comparablePath(path);
+	if (cleanPath.includes(`${sep}node_modules${sep}`)) return "package";
 	… // existing project / user logic
 }

Related (separate, flagged for follow-up — NOT in this PR)

While wiring this I noticed shouldSkipDuplicateExtensionLoad checks <cwd>/extensions/skill-registry.ts for the project-local override, but pi's discoverAndLoadExtensions (@earendil-works/pi-coding-agent/dist/core/extensions/loader.js) actually loads project extensions from <cwd>/.pi/extensions/ (CONFIG_DIR_NAME/extensions). So the self-skip never fires for a real project override → duplicate execution (or, if the override is placed at <cwd>/extensions/, the package self-skips but pi never loads it → registry disappears). That path mismatch is a separate bug; this PR leaves it untouched.

Testing

• Patch transpiles clean (pi's tsx loader).
• Applied locally: registry regenerated with 26 package skills indexed under package scope; project + user skills unchanged.

Summary by CodeRabbit

• New Features
  • Added automatic discovery of skills installed through npm packages.
  • Included discovered skills in registry updates and filesystem monitoring.
  • Correctly categorizes npm-installed skills as package-provided.

[coderabbitai]

Warning

Review limit reached

@ChaoXu1997, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 77ad459b-0337-4618-8dd8-2559afe7eb6a

📥 Commits

Reviewing files that changed from the base of the PR and between c1020cd and 0928faa.

📒 Files selected for processing (1)

• extensions/skill-registry.ts

📝 Walkthrough

Walkthrough

The skill registry now discovers skill directories in npm packages, assigns them package scope, includes them in registry generation and watches, and refreshes discovery when packages are added or removed.

Changes

NPM skill registry integration

Layer / File(s)	Summary
Package skill discovery and scope classification extensions/skill-registry.ts	The registry discovers skill directories in unscoped and scoped npm packages. Paths under node_modules receive package scope.
Registry regeneration and watcher integration extensions/skill-registry.ts	Registry regeneration and filesystem watcher setup include discovered npm package skill directories. The npm node_modules root is watched for package additions and removals.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SkillRegistry
  participant NpmModules
  participant FilesystemWatcher
  SkillRegistry->>NpmModules: Discover package skill directories
  NpmModules-->>SkillRegistry: Return existing directories
  SkillRegistry->>SkillRegistry: Regenerate registry
  SkillRegistry->>FilesystemWatcher: Watch package skill directories
  FilesystemWatcher-->>SkillRegistry: Trigger rediscovery on package changes

Loading

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: indexing skills installed in npm package node_modules directories.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@extensions/skill-registry.ts`:
- Around line 83-92: Update discoverNpmPackageSkillDirs() to inspect package
directories nested inside scoped entries under node_modules, checking each
<scope>/<package>/skills path alongside direct package paths. Preserve the
existing behavior for unscoped packages, avoid treating scope directories
themselves as packages, and add coverage for both unscoped and scoped package
layouts.
- Around line 511-515: Update startSkillRegistryWatcher around the npm directory
discovery and watcher setup to also watch the npm package root, not only the
initially discovered skill directories. When a new package-level directory
appears under that root, re-run discoverNpmPackageSkillDirs and register the
resulting directories so regenerateRegistry is invoked and the registry reflects
newly installed packages.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4a2dbe9d-1042-422d-b3a1-e7bc1adb27db

📥 Commits

Reviewing files that changed from the base of the PR and between 19b0ed7 and 2134082.

📒 Files selected for processing (1)

• extensions/skill-registry.ts

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

extensions/skill-registry.ts (1)

533-533: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Reconcile npm package skill-directory watchers on registry refresh.

dirs is built once before the watcher loop. A mid-session npm package discovery refreshes the registry through regenerateRegistry, but it does not add the package’s skills directory to activeWatchers. Later changes to that package’s SKILL.md files are not observed. When an npm root event changes discovered skill directories, reopen watchers for newly present skill directories and close watchers for directories that are no longer discovered.

Also address the skipped root watcher case when .pi/agent/npm/node_modules does not exist at startup (lines 559-571).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@extensions/skill-registry.ts` at line 533, Update the watcher refresh flow
around regenerateRegistry and discoverNpmPackageSkillDirs so npm discovery
changes reconcile activeWatchers: open watchers for newly discovered skill
directories and close watchers for directories no longer present. Also ensure
the npm root watcher is created or retried when .pi/agent/npm/node_modules is
absent at startup, rather than permanently skipping npm package updates.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@extensions/skill-registry.ts`:
- Around line 559-571: Update the npm watch setup around npmRoot and
discoverNpmPackageSkillDirs so changes under scoped packages in
node_modules/@scope trigger refresh as well as direct package changes. Add
non-recursive watchers for each existing scope directory, register them in
activeWatchers, and preserve the existing best-effort error handling and refresh
callback.

---

Outside diff comments:
In `@extensions/skill-registry.ts`:
- Line 533: Update the watcher refresh flow around regenerateRegistry and
discoverNpmPackageSkillDirs so npm discovery changes reconcile activeWatchers:
open watchers for newly discovered skill directories and close watchers for
directories no longer present. Also ensure the npm root watcher is created or
retried when .pi/agent/npm/node_modules is absent at startup, rather than
permanently skipping npm package updates.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c931e485-811d-4018-b43f-e639eec10cfd

📥 Commits

Reviewing files that changed from the base of the PR and between 2134082 and c1020cd.

📒 Files selected for processing (1)

• extensions/skill-registry.ts