fix(skills): skip mirror fan-out to agents that read the universal store - #3325
Conversation
miga-heygen
left a comment
There was a problem hiding this comment.
LGTM — clean, minimal fix.
The problem is well-defined: Pi discovers ~/.agents/skills natively, so the mirror's per-agent fan-out created a duplicate that Pi resolved by keeping the per-agent copy and skipping the universal one — the wrong winner.
The fix is a single continue in the fan-out loop gated on a UNIVERSAL_STORE_READERS set. Good design choices:
- Set lives in
skillsMirror.ts, not inagentDirs.generated.ts. The generated file is a plain(agent, base, sub)list synced from vercel-labs/skills — it can't carry per-agent capabilities, and a hand edit would be clobbered. Correct placement. - No stale-entry cleanup. Removing entries in a directory the mirror no longer manages risks deleting user-placed content. The replace-on-refresh precedent only applies to dirs still mirrored. Right call.
- Test is specific. Seeds both Pi and Cursor markers, asserts Pi is excluded and Cursor included, verifies no
~/.pi/agent/skills/hyperframesentry was created. Covers both the positive and negative cases.
No SSOT concerns — the skip check is one line, one source of truth for which agents are universal-store readers, and the comment explains the invariant it protects.
Review by Miga
|
@santhiprakash sign the commit pls |
Pi discovers both ~/.pi/agent/skills and the universal ~/.agents/skills globally, so the per-agent mirror copy collided with the universal copy and Pi skipped the universal entry on name conflict (heygen-com#3294). Skip agents that natively consume the universal store instead of fanning out to them. Fixes heygen-com#3294
6c9d5c6 to
4dc197d
Compare
|
Done — commits are now signed. You should see the Verified badge on the latest commit. |
|
Hi @miguel-heygen — quick check-in on this one. The head commit is signed now (verified badge on 4dc197d) per your earlier request, and the PR is approved, current with main, and green on checks. Happy to adjust anything else if something is still blocking the merge on your end — otherwise no rush. |
What
The global skills mirror no longer fans out into agent directories whose agent already discovers the universal
~/.agents/skillsstore globally. Currently that set is{ pi }.Why
Pi discovers skills from both
~/.pi/agent/skills/and~/.agents/skills/as global locations (see pi'spackages/coding-agent/docs/skills.md→ Locations). The global install already writes real files into the universal store, so mirroring an additional per-agent copy into~/.pi/agent/skillsmakes Pi discover every HyperFrames skill twice. Pi then reports a name collision, keeps the~/.pi/agent/skillsentry, and skips the universal entry:Fixes #3294 (the deterministic duplicate-discovery half; the broader target-control surface proposed there is left for maintainers to design).
How
UNIVERSAL_STORE_READERScapability set inpackages/cli/src/utils/skillsMirror.ts(currentlypi) and skip those agents in the fan-out loop, before the installed-marker check.agentDirs.generated.ts: that file is@generatedfrom vercel-labs/skills and a plain(agent, base, sub)list — it cannot carry per-agent capabilities, and any hand edit would be clobbered by the nextgen:agent-dirssync.~/.pi/agent/skillsentries from earlier runs. Removing entries in a directory we no longer manage would risk deleting user-placed content (the mirror's replace-on-refresh precedent only applies to dirs it still mirrors). A one-time stale-link cleanup can be a follow-up if maintainers want it.Test plan
skillsMirror.test.ts: with both Pi (~/.pi/agent) and cursor markers present,mirroredcontains cursor and not pi, and no~/.pi/agent/skills/hyperframesentry is created.skillsMirror,skillsManifest,skillsUpdateCheck,commands/skills— 116/116.oxlint/oxfmt --checkclean on changed files; repocheck-skill-mirrorinvariant still passes; lefthook pre-commit gates (format, fallow against synced main, typecheck after generating core runtime artifacts) pass.