fix: issue #288 (automated)

[github-actions]

Closes #288

[claude]

Claude finished @github-actions[bot]'s task in 3m 7s —— View job

---

Review Complete

• Clean up previous Claude comments
• Read changed files
• Review code changes
• Submit formal review

---

PASS — no blocking issues.

What's in this PR (issue #288): three related features added to the skills system:

1. Per-skill pncli-origin.json — self-contained provenance file written inside each installed skill directory alongside the directory-level .pncli-installed.json index.
2. marketplace purge-plugin — removes skills from a specific plugin (or all plugins in a marketplace), using provenance from both the index and per-skill origin file for backward compat.
3. skills purge-user — removes all user-level skills for the target agent, gated behind --force.

What I verified:

• Path traversal protection on both purge commands (skillDir.startsWith(resolvedTarget + path.sep)) ✓
• Token scrubbing on all git error surfaces ✓
• detectRepoBranch uses execFileSync (not shell), no injection risk ✓
• recordInstalledSkills write order is correct: per-skill origin first (try/catch), then index — a FS error on per-skill never aborts the index write ✓
• Token preservation on re-add (entry.token ?? all[idxByUrl].token) ✓
• All tests stub fs.* via vi.spyOn/afterEach — no live FS/network access ✓

One non-blocking observation: purge-user removes all directories in the user skills folder (not just marketplace-installed ones), which differs from purge-plugin's provenance-filtered behaviour. The --force gate and pre-flight skill list in the error message mitigate the risk, but the command description could call this out explicitly.

[claude]

PASS — no blocking issues found.

Summary

This PR adds three related features to the skills system:

1. Per-skill pncli-origin.json — a self-contained provenance file written inside each installed skill directory, supplementing the directory-level .pncli-installed.json index. Enables backward-compatible provenance lookup for skills without a full index entry.
2. marketplace purge-plugin — removes skills installed from a specific plugin or all plugins in a marketplace, using provenance from both the index and the per-skill origin file.
3. skills purge-user — nukes the entire user-level skills folder for the target agent, gated behind --force.

What I checked

• Security / path traversal: Both new purge commands guard against traversal with skillDir.startsWith(resolvedTarget + path.sep). ✓
• Token scrubbing: scrubToken() is applied wherever git error output might reach the user. ✓
• detectRepoBranch: Uses execFileSync('git', [...], ...) (not shell) — no injection risk. Silently returns undefined on failure, which is the right non-fatal behaviour. ✓
• recordInstalledSkills write order: per-skill origin files are written first, directory-level index last. The per-skill write is wrapped in try/catch so a filesystem error there never aborts the index write. ✓
• upsertMarketplace token preservation: entry.token ?? all[idxByUrl].token correctly retains a previously stored token when re-adding without --token. ✓
• Test isolation: All tests stub fs.* via vi.spyOn and restore mocks in afterEach. No live filesystem or network access. Matches the project's testing rule. ✓

Observations (non-blocking)

purge-user removes ALL user skills, not just marketplace-installed ones. This is a behavioural difference from purge-plugin, which filters by provenance. If a user has any hand-authored skills in their user skills folder, purge-user --force will silently delete them alongside marketplace-installed ones. The --force requirement and the pre-flight skill list in the error message mitigate this, but the description could be clearer.

loadJsonFile does not check existsSync — it calls readFileSync directly and catches on ENOENT. The tests that mock existsSync but not readFileSync for the meta path are still correct because the uncaught readFileSync throws, is caught by loadJsonFile, and returns null. Not a bug, but worth noting for future test authors.