fix(cli): install the published skill set, not every SKILL.md in the repo

[WaterrrForever]

What

Bare hyperframes skills (the full install) now installs the skills the canonical manifest publishes, instead of every SKILL.md the upstream installer can find in the clone. Docs updated to match.

Why

The command passed --skill '*' to skills add, which scans .claude/skills and .agents/skills too. Since the repo-native skills landed there, a full install put 26 skills on disk against the 20 the manifest publishes:

npx skills add heygen-com/hyperframes --list --full-depth   # Found 26 skills

The extra six (captions-overlay, changelog-video, cut-the-curve, motion-doctrine, oversized-cursor, seam-craft) are repo-internal, and motion-doctrine tells its reader to load it first, so a fresh install started with rules the published set never asked for. README / CLAUDE.md say "20 skills".

How

• updateSkills gains all: true; the bare command resolves the canonical manifest and installs its names, the same path skills update already uses. The * wildcard survives only as the offline fallback, when the published set is unknowable.
• README, CLAUDE.md, AGENTS.md, docs/guides/skills.mdx, docs/prompting/overview.mdx: the full set is npx hyperframes skills; skills add --all is documented as also pulling the repo-internal skills. The old "non-interactive without --skill installs all 20" claim is dropped: upstream only installs all under --all / --yes, otherwise it opens the picker.

Test plan

• Unit tests updated: the three full-install platform cases in skills.test.ts expect per-name --skill flags (39/39 pass)
• Manual: --list / --list --full-depth against the published repo both show the six repo-internal skills; skills-manifest.json has 20
• Documentation updated

[mintlify]

Preview deployment for your docs. Learn more about Mintlify Previews.

Project	Status	Preview	Updated
hyperframes	🟢 Ready	View Preview	Sep 4, 2026, 2:24 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

[jerrai-bot-heygen]

Requesting changes at 6372dd948b508f01b8d95c147708440585d787c5.

The canonical-manifest path correctly selects explicit published names while online, but the full-install offline fallback still invokes skills add --skill '*'. That is the same wildcard discovery behavior that installs the six internal changelog-video skills, so an offline/rate-limited hyperframes skills silently installs 26 rather than the documented 20.

Please use a pinned canonical full-set fallback, or fail explicitly when it cannot be resolved, and add a regression for the offline all:true path.

Review by Jerrai

[jerrai-bot-heygen]

The bare command still reaches the upstream wildcard whenever the canonical manifest cannot be fetched. That wildcard is precisely the behavior this PR removes: it discovers repo-internal skills and installs 26 rather than the published 20. An offline/rate-limited invocation therefore silently breaks the documented full-install contract. Please use a pinned canonical full-set fallback (and a regression that makes all:true offline install the 20 manifest names, not *), or fail the full install explicitly when that set cannot be resolved.

[jrusso1020]

Reviewed at head 6372dd948b508f01b8d95c147708440585d787c5. First review on this PR; no prior reviews to be additive to at the time of reading.

Strengths

• The core fix is the minimal correct one: skills.ts:801 swaps installSkills("*") for updateSkills({ all: true }), so bare hyperframes skills resolves targets through the canonical manifest (checkSkills({ canonical: true }), skills.ts:339) instead of depending on the upstream installer's directory scan. It reuses an existing path rather than adding a mechanism.
• opts.all === true || is placed first in the targets filter (skills.ts:379-384), so it short-circuits without disturbing the requested / core / refreshInstalled precedence below it.
• The docs change is complete rather than partial: the --all recommendation is replaced in all five surfaces that carried it (AGENTS.md, CLAUDE.md, README.md, docs/guides/skills.mdx, docs/prompting/overview.mdx).

Body claims, verified at this head

• "26 skills found vs 20 published" - confirmed. skills/ holds exactly 20 SKILL.md files; .claude/skills/ and .agents/skills/ hold the same six names. Those two directories are byte-identical copies (every pair resolves to a single distinct blob SHA), so it is 12 files for 6 unique names, and 20 + 6 = 26.
• "the same path skills update already uses" - confirmed for resolution. Both call sites go through updateSkills: bare skills at skills.ts:801, skills update at skills.ts:723. See the note below for the one place the wiring differs.

On whether dropping the six breaks anything (the question I was asked to answer)

It does not, and the check is cheap to repeat. I cross-referenced all 20 published SKILL.md files against the six internal names. Exactly one reference crosses the boundary, and it is explicitly conditional:

• skills/general-video/SKILL.md:115 - "where the doctrine chain (/motion-doctrine) is installed, translate them into the project ledger before stamping seams."

That degrades by construction, so a published-set install has no unmet prerequisite. The six are a self-contained cluster pointing inward only: motion-doctrine routes to cut-the-curve / oversized-cursor / seam-craft / captions-overlay (.claude/skills/motion-doctrine/SKILL.md:17-22), and cut-the-curve names motion-doctrine as read-first (.claude/skills/cut-the-curve/SKILL.md:11). Nothing in the published 20 hard-depends on any of them, so the manifest is the right list here.

One residual worth knowing, pre-existing and not a finding against this PR: .claude/skills/motion-doctrine/SKILL.md:22 routes to text-beat-economics and brand-faithful, and neither exists as a SKILL.md anywhere in the tree at this head. The internal cluster is therefore not fully resolvable on its own. Untouched by this change, but it is context for anyone later deciding whether these six should be published rather than excluded.

important - the offline branch reinstates the exact wildcard this PR removes, silently

skills.ts:356-360:

if (opts.all) {
  // Offline the published set is unknowable; fall back to the upstream wildcard.
  await installSkills("*", { cwd: opts.cwd, strict });

The docstring added 20 lines above, in this same diff, says the all path installs "every skill the manifest publishes - never the upstream * wildcard, which also sweeps up the repo-internal skills under .claude/skills" (skills.ts:304-307). The branch below falsifies that "never".

Failure scenario: an unauthenticated run hits the GitHub API rate limit - the catch at skills.ts:341-353 is itself labelled "offline / rate-limited" - and hyperframes skills installs 26 skills again, with nothing printed to say the published set could not be resolved. The user sees a successful full install and gets the pre-fix result.

I am not calling this a blocker: the base always passed *, so this is a strict improvement with a residual, and offline the published set genuinely is unknowable, which makes the fallback defensible on its own terms. Two things still make it worth landing before merge:

1. The sibling degradation immediately above already warns (clack.log.warn, skills.ts:346-352) for the malformed-manifest case, which is strictly less surprising than this one. The idiom is right there in the same function.
2. The docstring is what the next reader copies. An absolute "never" that is false in one branch is the kind of thing that gets cited later as if it were the contract.

Suggested: a one-line clack.log.warn in that branch saying the published set could not be resolved and every repo skill was installed, and softening "never" to name the offline exception.

nit - the docstring (skills.ts:306) and the new README.md line both say the wildcard sweeps up the six "under .claude/skills". They are equally under .agents/skills, byte-identical. The count is right; the location is half of it. Only matters for whoever later tries to fix this at the directory rather than at the flag.

Note, not a finding - the one wiring difference between the two call sites: skills update passes strict: true (skills.ts:723), bare skills passes none (skills.ts:801), so on the new path verifyInstalled warns instead of throwing (skills.ts:419-426). I checked this and it is correct rather than an oversight - updateSkillsOffline's docstring names strict as "the documented check || update CI contract" that "must fail loudly" (skills.ts:431-447), while bare skills is an interactive command and non-strict matches the base behaviour. Recording it so the next reviewer does not have to re-derive it.

Verdict: APPROVE
Reasoning: The mechanism is right, the manifest is the correct list (verified: no published skill hard-depends on the six), and the docs are updated consistently. The offline residual is a strict improvement over the base rather than a regression, so it does not block, but the silent fallback plus the absolute "never" in the docstring are both worth a two-line fix.

• Rames Jusso

[jrusso1020]

Correction to my own review above, on two counts. Read at head 6372dd948b508f01b8d95c147708440585d787c5.

1. My APPROVE should not be read as clearing @jerrai-bot-heygen's request for changes. Their review landed at 14:36:19Z and mine at 14:36:24Z, five seconds apart. I had queried this PR's review list shortly before writing and it came back empty, so my review says "no prior reviews to be additive to" - accurate when I read it, wrong by the time it posted. The pre-write check I run to catch exactly this did detect their review and failed to stop the submission; that is a bug in my tooling, not a judgement that their block was wrong. For the record, GitHub agrees with them and not with me: reviewDecision is CHANGES_REQUESTED and mergeStateStatus is BLOCKED, so nothing about my approval changed this PR's state.

2. On the merits, their severity is right and mine was wrong. We independently found the same defect - the offline/rate-limited all branch calling installSkills("*") at skills.ts:356-360. I graded it "important, not a blocker" on the premise that offline the published set is genuinely unknowable, which makes a wildcard fallback defensible. That premise is false, and the disproof is in the same file:

• FALLBACK_CORE_SKILLS (packages/cli/src/utils/skillsManifest.ts:150-160) is a pinned list of nine explicit skill names.
• It exists precisely for the manifest-unreachable path, and updateSkillsOffline uses it that way (skills.ts:459, docstring at skills.ts:431).

So this codebase already has an established answer to "the manifest cannot be resolved," and that answer is a pinned list of names. The branch this PR adds is the one degradation path in the function that reaches for the wildcard instead. That makes @jerrai-bot-heygen's requested remedy - a pinned full-set fallback, or an explicit failure - the existing local convention rather than a new mechanism, which is the argument I should have made and did not.

Everything else in my review above stands, in particular the part I was asked to check: excluding the six internal skills does not break a documented path from the published set, because the only cross-boundary reference is conditional (skills/general-video/SKILL.md:115). The manifest is the right list. The offline fallback is the part that should change.

Verdict: COMMENT - deferring to the standing CHANGES_REQUESTED, which I now agree with.
Reasoning: Same finding as the prior reviewer, wrong severity from me, corrected here. The pinned-fallback precedent in skillsManifest.ts:150-160 is what settles it.

• Rames Jusso

[WaterrrForever]

Addressed in HEAD (second commit on the branch):

• Offline all: true no longer reaches the upstream * wildcard. When the canonical manifest cannot be fetched, the full install warns ("Can't resolve the published skill set … installing the pinned core set only") and goes through the existing updateSkillsOffline path, so it installs FALLBACK_CORE_SKILLS and nothing discovered by directory scan. Regression test: bare \skills` offline never falls back to the wildcardasserts no*` in the spawn args, exactly the pinned core names, and the warning. 40/40.
• Docstring no longer says "never" without the exception; it names the offline degrade and both internal dirs.
• README nit: .claude/skills / .agents/skills.

I went with the pinned core set rather than a pinned full list: a hard-coded 20-name list would drift on every skill add/remove, while the core list already exists and is what the rest of the offline path guarantees.

[jerrai-bot-heygen]

Re-approved at 027bccc94b174837faa36ad710f99b814b611004.

The manifest-unavailable full-install branch no longer invokes the upstream wildcard. It emits an explicit degraded-network warning and routes through the established explicit FALLBACK_CORE_SKILLS mechanism, so no directory-discovered internal skill can be installed. The new bare-command regression forces that failure seam, asserts the exact fallback names and warning, and proves * is absent from the upstream invocation.

The required regression aggregate is red only on the unrelated sub-comp-t0 visual suite; the changed CLI-skill path has no connection to that render case. Build, lint, typecheck, unit/integration, CLI smoke, and platform checks are green. Code-quality approval only; no merge or enqueue.

Review by Jerrai

[jrusso1020]

Reviewed at head 027bccc94b174837faa36ad710f99b814b611004.

To be explicit about what this is: not a re-issue of my earlier approve. That review was wrong twice over - it posted past @jerrai-bot-heygen's live CHANGES_REQUESTED, and it graded the defect "important, not blocker" on the premise that the published set is unknowable offline, which FALLBACK_CORE_SKILLS in the same package already falsified. I withdrew that grade in a follow-up comment. This approval rests on reading this head, and their blocker call was the correct one.

Additive to @jerrai-bot-heygen's approve at this same head, which covers the fallback flow and the new regression. Two things I checked that their review does not state:

Deleting the early return changed the return shape, and it regresses nothing. The old all-offline branch returned { targets: [], installed: [], current: [], unknown: [], presenceOnly: true }; it now falls through to updateSkillsOffline, which returns populated values with presenceOnly: true (skills.ts:482-488). So the flag survives, but installed is no longer always empty - and reportUpdate branches on installed.length > 0 at :638 before it reaches presenceOnly at :644, so a populated installed would suppress the "Freshness unknown" line. That turns out to be inert on this path: :808 discards the return value entirely, and reportUpdate only runs for the update subcommand at :731, which never passes all. Both call sites confirm the reachability - :730 is {requested, refreshInstalled, strict: true} and :808 is {all: true}, so all + strict cannot co-occur and the :460 strict throw cannot fire here. That is also what makes the test's exit-0 assertion correct rather than lucky.

The wildcard is now unreachable, but still permitted. Verified no caller passes "*" any more: installSkills is reached only at :402 with result.installed and :479 with absent, both string[]. So the docstring's "not used on any path" is accurate as written. The consequence is that the "*" arm of SkillSelection (:122) and its two branches (:127, :239) are now dead, so the capability to reinstate the 26-skill sweep still lives in the type even though nothing exercises it.

nit - narrowing SkillSelection to readonly string[] and dropping those two branches would make this fix structural rather than behavioral. It stays a nit because of your own test: expect(skillFlagValues(args)).not.toContain("*") fails if anyone reintroduces the wildcard on this path, so the regression that actually happened is guarded. A future caller on a different path is not, which is the entire remaining gap.

The regression drives the real seam rather than stubbing past it: checkSkills rejects, so the :343 catch runs, and an "offline" message correctly misses the Malformed skills manifest branch at :348, which would otherwise print a misleading upstream/CDN warning. Asserting the exact FALLBACK_CORE_SKILLS contents rather than a count is the right shape, since a count still passes if the list drifts.

Agreed on the pinned core set over a literal 20-name list, and the drift argument is the reason: a hard-coded list needs editing on every skill add or remove, and it fails silently when someone forgets. The README correction to .claude/skills / .agents/skills matches what is on disk - those two directories hold the same six names as byte-identical copies, which is where 26 comes from against 20 published.

Verdict: APPROVE
Reasoning: The offline all path no longer touches the upstream wildcard, degrades to the pinned set with a warning that names the remedy, and the return-shape change from removing the early return regresses no consumer. The one item left is structural hardening, not behavior.

• Rames Jusso