Skip to content

feat(cli): skills freshness — version check, manifest, global install + multi-agent mirror#1753

Merged
WaterrrForever merged 15 commits into
mainfrom
feat/skills-version-check
Jun 27, 2026
Merged

feat(cli): skills freshness — version check, manifest, global install + multi-agent mirror#1753
WaterrrForever merged 15 commits into
mainfrom
feat/skills-version-check

Conversation

@WaterrrForever

Copy link
Copy Markdown
Collaborator

Summary

Makes HyperFrames' bundled agent skills self-updating and verifiable. Adds a content-hash freshness manifest, a hyperframes skills check/update command group, automatic refresh on init, a single global install that mirrors to every other installed agent, and a one-line staleness nudge on common commands. Workflow docs are updated so newly scaffolded projects pull the latest skills by default.

What's included

Freshness manifest (source of truth)

  • skills-manifest.json at the repo root — a per-skill content fingerprint (hashes the whole skill bundle: SKILL.md + references/ + scripts/ + …), no version/timestamp so it only changes on real content change.
  • Generator packages/cli/scripts/gen-skills-manifest.ts (bun run gen:skills-manifest).
  • Kept in sync automatically: a lefthook pre-commit hook regenerates + re-stages it on any skills/** change, and a CI job (“Skills: manifest in sync”) fails the build if it drifts.

hyperframes skills command group

  • skills check — compares the installed skills against the latest manifest on GitHub; --json for a machine-readable verdict; exits non-zero when anything is outdated or missing.
  • skills update — pulls the full set to the latest, installing any not yet present.
  • bare skills — installs the full set.

init integration

  • init checks installed skills against GitHub and installs/refreshes only when stale or missing (no-op when current; never hard-fails on a network hiccup — falls back to installing after a short timeout).
  • Opt out with init --skip-skills.

Global install + multi-agent mirror

  • Installs the full set once globally (~/.claude/skills + the universal ~/.agents/skills) with --copy --full-depth--full-depth clones main HEAD so a fresh install reads as current instead of lagging the skills.sh registry blob.
  • skillsMirror then symlink-mirrors that canonical store into every other installed agent's global dir (Codex, Cursor, Gemini, Hermes, Goose, … — 71 agents), but only for agents actually present on the machine. Unix uses relative symlinks (one source of truth, auto-fresh on update); Windows copies.
  • Agent dir table is generated: packages/cli/scripts/sync-agent-dirs.tsagentDirs.generated.ts (honors XDG_CONFIG_HOME, CODEX_HOME, CLAUDE_CONFIG_DIR, …).

Staleness nudge

  • render / lint / validate print a one-line reminder when skills are stale — cached 24h, best-effort, never blocks or fails the command, suppressed under --json.

Workflow docs

  • Dropped --skip-skills from the workflow init commands (product-launch / faceless / pr-to-video / music-to-video / motion-graphics / embedded-captions) so each new project refreshes skills, with a one-line note in each and in hyperframes-cli / the entry hyperframes skill.

Also

  • CI path-filter + telemetry config for the new flow; CodeQL file-system race fix; Windows npx test de-flake; manifest review-feedback follow-ups.

How precedence works (why check is global-first)

Per Claude Code's documented order — personal (~/.claude/skills) overrides project (.claude/skills) — so a fresh global install silently supersedes a stale project copy. skills check reports on the global copy because that's the one the agent actually loads.

Test plan

  • bun run test (adds coverage for manifest hashing/diff, mirror fan-out incl. XDG/Codex bases, and the init check path).
  • bun packages/cli/scripts/gen-skills-manifest.ts --check passes (manifest in sync).
  • Verified init refreshes the global store and mirrors into other installed agents; skills check --json exit codes for current / outdated / missing.

🤖 Generated with Claude Code

WaterrrForever and others added 14 commits June 26, 2026 17:09
Give the HyperFrames skill bundle a content fingerprint so agents and
users can tell whether installed skills are the latest version, on any
platform that can run the CLI.

- skills-manifest.json (repo root): per-skill sha256 over the whole skill
  directory; minimal {source, skills}, no version/timestamp so it is fully
  deterministic. Generated by scripts/gen-skills-manifest.ts.
- `hyperframes skills check` [--json]: compares installed skills to the
  manifest; exits non-zero when something is outdated (agent/CI gate).
- `hyperframes skills update`: thin wrapper over `npx skills update`.
- Passive nudge on render/lint/validate when skills are stale (24h cache,
  same opt-out as the CLI self-update notice).
- "latest" resolved via `git ls-remote` + SHA-pinned raw URL to dodge
  GitHub raw-CDN lag, falling back to the main branch URL.
- CI job + lefthook hook keep skills-manifest.json in sync with skills/.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
skills.test.ts mocks node:child_process but only declared execFileSync
and spawn. Loading skills.js transitively loads skillsManifest.ts, which
runs promisify(execFile) at module load, so vitest threw on the missing
execFile named export. Add a bare stub — these tests never invoke it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make `hyperframes init` the single place skills are pulled in full, and
make "update" mean "get everything" rather than "refresh what's there".

- init now always installs/refreshes ALL skills (incl. ones not yet
  present) instead of prompting "Install AI coding skills?" — opt out
  with `init --skip-skills`. Both the interactive and non-interactive
  paths pass `--all --yes` so the complete set is fetched.
- `hyperframes skills update` switches from `npx skills update` (which
  only refreshes already-installed skills) to `skills add --all`, so it
  installs missing skills too — the same install step init runs.
- SKILL.md documents init-installs-all and the new update semantics.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The full skill set is now the goal (init and `skills update` both pull
all, including ones not installed), so a partial install is no longer
"a choice" — it's something to fix.

- diffSkills: updateAvailable is now true when anything is outdated OR
  missing (local-only still doesn't count). So `skills check` exits
  non-zero — and renders "Update:" instead of "up to date" — whenever a
  skill is missing, not just when one is stale.
- The passive render/lint/validate nudge follows suit: it now counts
  missing alongside outdated ("N skills out of date or missing"),
  tracked via a new skillsMissingCount cache field.
- SKILL.md documents the stricter check.

Note: platforms that intentionally vendor only a subset of skills (e.g.
a Codex snapshot) will now see check report non-zero.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`skills add owner/repo` can resolve through the skills.sh registry, which
lags behind the repo — so `update` could install a stale version while
`check` (which resolves latest directly from GitHub) keeps reporting
"outdated", an endless loop.

Switch the install source to the full GitHub URL
(https://github.com/heygen-com/hyperframes), which makes `skills add`
git-clone the repo directly at latest main, bypassing the registry. This
covers `hyperframes skills`, `hyperframes skills update`, and `init`'s
skill install — all of which go through SOURCES. Now install/update and
check agree on what "latest" means.

The init "install skills" hint now points at `npx hyperframes skills
update` so the manual path uses the same GitHub-direct fetch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`hyperframes init` now runs the skills version check first and only
(re)installs when something is outdated or missing — instead of
unconditionally re-pulling every time. Re-running init on an
already-current project is now a no-op ("skills are already up to date").

- New ensureSkillsCurrent() helper, shared by both the interactive and
  non-interactive init paths (no duplicated install logic).
- The check resolves "latest" straight from GitHub (same source the
  install uses); best-effort — if it can't reach GitHub it installs anyway.
- SKILL.md updated to describe the check-then-install behavior.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the PR review (points 1, 2, 4, 5):

1. Remove the `local-only` skill status. checkSkills only ever hashes
   manifest-listed skills, so a local-only status could never appear in
   the end-to-end output — and making it appear would wrongly flag
   unrelated skills (the `.../skills` dir is shared across sources).
   diffSkills now reports only on manifest skills; skills on disk that
   aren't in the manifest are ignored.
2. Drop the redundant per-directory sort in listFilesSorted — the single
   final out.sort() is what guarantees a deterministic hash (verified:
   manifest unchanged).
4. resolveLatestManifest local-path detection now uses path.isAbsolute,
   so Windows absolute paths (C:\...) are treated as local instead of
   falling through to a remote fetch.
5. fetchManifest validates the response shape (asSkillsManifest) instead
   of a blind `as` cast, so a CDN error page served as 200 fails with a
   clear error rather than a cryptic crash later in diffSkills.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR review (Magi blocker + James/Rames robustness):

- Blocker (Magi): `skills update` is the documented recovery path for
  `skills check || skills update`, but it delegated to installAllSkills()
  which swallowed missing-npx and failed `skills add` as "skipped",
  exiting 0 even when nothing changed. Add a strict mode that throws on
  failure; update sets a non-zero exit (init stays best-effort). New tests
  simulate a non-zero `skills add` (exit 1) and the success path.

- Robustness (James/Rames #2): the upstream `skills` CLI installs into
  ~72 agent conventions; a hard-coded list (4, or even 11) can't track
  that. Replace defaultSkillRoots with discoverSkillRoots — it scans cwd +
  $HOME for any `<host>/skills/<manifest-skill>/SKILL.md` (plus the XDG
  `.config/<host>/skills`), so detection is structural and future-proof,
  no closed list. agentFromDir infers the host from the path.

- Tests (Rames #3): temp-fixture detection tests for every convention ×
  {project, global}, scope priority, claude-code preference, the
  no-install case, the --dir override, and an unknown/new host (proving
  the no-closed-list property).

- Docs (Rames #4/#5): SKILL.md notes init's best-effort GitHub round-trip;
  findRepoManifest climbs 16 levels (was 8) for deep monorepos.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two CI fixes:

- CodeQL (high, js/file-system-race) at gen-skills-manifest.ts: the
  existsSync(outPath) precheck followed by writeFileSync(outPath) is a
  check-then-write race. Read the committed manifest directly in a
  try/catch instead (missing/unreadable ⇒ "no committed manifest"), so
  there's no precheck to race against. Behavior is unchanged.

- Windows Tests: npxCommand.test.ts's real `npx --version` smoke test
  cold-starts slower than vitest's 5s default on Windows runners and
  timed out. Give the test 60s headroom (and a 30s exec timeout). Kept
  as a real execution check — mocking would reduce it to a tautology.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The explanatory comment for the 60s timeout was scrambled across the
callback/timeout arguments, failing oxfmt --check (and thus preflight,
which in turn skipped preview-parity and failed the regression gate).
Move it above the it() call so it no longer sits between call arguments.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The previous install path sprayed a full ~6.7MB skill copy into each of the
~70 agent conventions `skills add --all` knows (a fresh init produced 40+
dirs / 341MB, incl. a stray dotless `agent/` from the Eve convention).

Install ONCE, globally, as one faithful copy, then symlink it everywhere:
  - `skills add <url> --skill '*' --global --agent claude-code universal
    --copy` lands real files in ~/.claude/skills (Claude Code reads this at
    global priority) and ~/.agents/skills (the shared universal store).
  - mirrorGlobalSkills() fans that store out to every OTHER installed agent's
    GLOBAL dir (~/.cursor/skills, goose -> ~/.config/goose/skills, ...) — but
    only for agents present on the machine (marker dir exists), so nothing is
    sprayed. Unix: per-skill relative symlink into the store (one source of
    truth, auto-fresh on update); Windows: copy (symlinks need admin /
    Developer Mode there — the same fallback upstream and gstack make).

Why global: skills are framework-general knowledge, not project content;
Claude Code (and most agents) prioritize the personal/global scope, so the
global copy is the one actually loaded — and it installs once instead of
multiplying per project.

The per-agent dir list is GENERATED from upstream's src/agents.ts at a pinned
tag (the `skills` package exports nothing importable), committed as
agentDirs.generated.ts and resolved env-faithfully at runtime
(XDG_CONFIG_HOME / CODEX_HOME / CLAUDE_CONFIG_DIR honored). Regenerate with
`bun run --cwd packages/cli gen:agent-dirs` when the pin moves. Covers all 70
agents that define a global dir (eve/promptscript define none); the bare
project-dir agents (openclaw, astrbot) are namespaced globally, so the
stray-`agent/` footgun is gone.

`skills check` now scans global ($HOME) before project (cwd) to match the
runtime load order — so it reports on the copy the agent will really use, not
a stale project copy a newer global install silently overrides.

Test plan:
- skills.test.ts: install spawns the global --copy args, never --all; update
  stays strict + exits non-zero on failure.
- skillsMirror.test.ts: Unix relative symlinks, Windows copy, XDG_CONFIG_HOME
  honored, install-owned stores skipped, marker-gating, idempotent refresh,
  generated-table shape.
- skillsManifest.test.ts: check is global-first.
- Full CLI suite green (981); oxlint / oxfmt / tsc clean; gen:agent-dirs
  --check clean (offline + network produce byte-identical output).
- Benchmark (isolated HOME, local CLI): claude+hermes and all 70 agents —
  ~/.claude + ~/.agents real (19 each), every installed agent's global dir =
  19 symlinks into the store, zero spray into unseeded agents, check
  global-first. (The 9 "outdated" check reports are the separate skills.sh
  registry lag, not this change.)
- .fallowrc.jsonc: exempt the codegen script's inherent parser complexity and
  the parallel-case duplication in skillsManifest.test.ts (same rationale the
  config already uses for SlideshowPanel.test.ts / hyperframes-player.test.ts).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s current

`skills add <url>` without --full-depth fetches from the skills.sh registry
blob ("Fetching skills"), which lags GitHub main by hours — so a freshly
installed/updated set read as ~9 skills "outdated" right after install, and
`skills update` couldn't fix it (it re-fetched the same stale blob → death
loop). --full-depth switches it to a real `git clone` of HEAD ("Cloning
repository"), the only path that yields the genuine latest.

- Add --full-depth to the global install args. Verified (isolated HOME): blob
  path → 10 current / 9 outdated; --full-depth → 19 current / 0 outdated.
- The clone is heavier than the blob fetch, so set GIT_LFS_SKIP_SMUDGE=1 (skills
  are text; the repo's LFS objects are unrelated binaries the install doesn't
  need) and raise the spawn timeout 120s → 300s.
- Correct the stale comment that claimed a full URL already bypasses skills.sh —
  it doesn't; only --full-depth does.

Benchmark (skills-bench, local CLI): B.death-loop and J1.init-detect-and-refresh
flip FAIL → PASS (install/update/init now 19/0); mirror smoke reports 19 current
/ 0 outdated. (spine still reflects the raw documented `skills add <slug>`
command — the upstream skills.sh path, not this CLI.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…efresh skills

The creation workflows scaffolded with `hyperframes init … --skip-skills`, which
skipped the skills currency check. Now that init installs globally, is a no-op
when already current, and pulls the genuine latest (via --full-depth), there's
no reason to skip it: removing --skip-skills means every new project runs the
check and refreshes the global skill set from GitHub when it's stale. Add a
one-line note to each workflow (embedded-captions, faceless-explainer,
motion-graphics, music-to-video, pr-to-video, product-launch-video) and the
hyperframes-cli + /hyperframes router explaining what init does.

skills-manifest.json regenerated by the pre-commit hook to match the edited
skill bundles.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve the skills-distribution conflicts. main already carried the
manifest/version-check + prune work (#1738/#1740/#1743) and the #1748
"scope installs via skillsTargets" approach; this branch reworks install
to the global symlink-mirror model, so:

- Keep main's prune/removed-detection (skillsManifest.ts, the rm + update
  prune path, the related tests) verbatim; re-apply only the global-first
  locateInstall on top.
- Replace #1748's skillsTargets-based scoped install with the global
  --copy + --full-depth install + mirrorGlobalSkills fan-out; delete the
  now-superseded skillsTargets.ts (+ test).
- Keep main's 0.7.13 version bump + add the gen:agent-dirs script.
- Regenerate skills-manifest.json against the merged skills/ tree.

Verified: typecheck, oxlint, oxfmt, fallow gate all clean; 1044 CLI tests
pass (main's prune suite + the new mirror/global suite together).

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills Freshness — Version Check, Manifest, Global Install + Multi-Agent Mirror

Reviewed: 3904420 (merge commit)

Thorough and well-structured PR. The shift from per-project scoped installs to a single global install + symlink mirror is a clean architectural improvement — one source of truth, one install, n symlinks. The generated agent-dir table, the content-hash manifest, and the global-first check order all hang together consistently. Test coverage is solid, CI is fully green (all 35 checks pass), and the deleted skillsTargets.ts was cleanly superseded.

I have a few observations, none blocking:


Nits

  1. Typo in mirror log message (skills.ts L155): "Linked skills into ${mirrored.length} other agent director(ies)." — "director(ies)" is not a word. Since mirrored.length > 0 is always true at this point, just use "directories". Or if you want singular/plural: ${mirrored.length} other agent ${mirrored.length === 1 ? "directory" : "directories"}.

  2. Marker-dir detection granularity for deep sub-paths: The marker check !existsSync(dirname(targetDir)) works well for single-segment agents like .cursor/skills (checks ~/.cursor), but for deeper paths like .gemini/antigravity/skills it checks ~/.gemini/antigravity instead of ~/.gemini. This means if Gemini creates ~/.gemini/ on install but only populates ~/.gemini/antigravity/ on first use, the mirror would skip that agent even though it's installed. Probably fine in practice — agents tend to create their full dir tree — but worth a note if you want the marker to be the top-level agent dir consistently. Not blocking.

Observations (informational, not action items)

  1. resolveBases env-var asymmetry: configHome validates isAbsolute(xdg) before using XDG_CONFIG_HOME (matching the XDG spec), but CODEX_HOME, VIBE_HOME, etc. are used as-is without the absolute-path check. Intentional difference (XDG is spec'd, the others aren't), but worth noting for symmetry. If an operator sets CODEX_HOME=./relative, it would produce an unexpected path. Low risk, just calling it out.

  2. sync-agent-dirs.ts pin management: The SKILLS_REF = "v1.5.13" pin is the right call for determinism. Might be worth adding a comment about when/how to discover that a bump is needed (e.g. a dependabot-style check, or a CI job that runs --check against latest). Currently bumping is purely manual discovery.

  3. Silent catch in installAllSkills (skills.ts L158): The catch {} after mirrorGlobalSkills() is intentionally best-effort, which is good — a mirror failure should not block the install. The empty catch body is fine given the "best-effort" contract is documented in the comment above it.

What I liked

  • The global-first check order matching the agent's actual load order is a smart alignment — it eliminates the confusing "check says outdated but the agent is using the fresh global copy" scenario.
  • The --full-depth fix for the skills.sh registry lag is a solid catch. The death-loop diagnosis (install → check → "still outdated" → install → ...) is well documented in the commit messages.
  • The generated agentDirs.generated.ts with a pinned upstream ref is the right balance between tracking upstream and runtime determinism.
  • Excellent commit history — each commit tells a clear story of the evolution, and the review-feedback commit shows good iteration.

Verdict: LGTM with one cosmetic nit (the "director(ies)" typo).

— Miga

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additive review on top of <https://github.com/heygen-com/hyperframes/pull/1753#pullrequestreview-4583368434|Miga's pass>. Miga's review covers the priority code surface + nits cleanly. My additive lane: the supersession question — does this PR cleanly replace #1748's targeting logic, or accidentally regress it? Plus a coverage check on #1738/#1744.

Supersession verdict — DELIBERATE clean replacement, not accidental revert ✓

The architectural shift is the key — #1753 doesn't replace #1748's resolveAgentTargets with an equivalent targeting function. It replaces the question itself:

  • #1748's question: "we're installing PER-PROJECT — which agents in this project should get the skills?"
  • #1753's answer: "install once GLOBALLY; the agent's documented load order makes global override project, so per-project targeting is no longer needed."

This sidesteps the targeting problem rather than re-solving it. The four cases #1748's resolveAgentTargets handled all map cleanly to the new model:

#1748 case #1753 coverage
1. Existing project skill folder honoring (.claude/skills, .hermes/skills exist → install only to those) Effectively preserved by override semantics. Global ~/.claude/skills shadows project .claude/skills per Claude Code's documented load order. The project folder is untouched but the agent reads from global.
2. CLAUDECODE env → install to claude-code only Effectively preserved + extended. GLOBAL_INSTALL_ARGS hardcodes --agent claude-code universal, so claude-code gets skills regardless of whether the env var is set. More complete than #1748.
3. PATH-probe (gstack: claude/hermes/droid/cursor/codex/opencode/gemini binaries) Replaced with marker-dir detection (existsSync(dirname(targetDir))). Different heuristic — Miga's nit #2 flags one edge case where deep sub-paths (.gemini/antigravity/skills) check the parent (.gemini/antigravity) instead of .gemini. For most agents the marker-dir test works; the Gemini-antigravity case is the only one I'd flag for follow-up.
4. Floor (claude-code + universal .agents) Built-in structural — hardcoded in GLOBAL_INSTALL_ARGS. Used to be a fallback heuristic; now it's an invariant.

Net: this is a deliberate architectural improvement, not a regression. The new model is stronger on cases 2 and 4 (no longer heuristic — structural), equivalent on case 1 (different mechanism, same observable behavior), and slightly different on case 3 (marker-dir vs PATH-binary detection).

Preservation of already-merged behavior

#1738 (freshness manifest) — preserved + extended. The manifest is regenerated via lefthook pre-commit + the CI Skills: manifest in sync gate (carried forward from #1745). skills-manifest.json is in the PR diff with the manifest re-pinned to the consolidated skill set. ✓

#1744 (--copy real-file install) — preserved + extended. --copy is still in GLOBAL_INSTALL_ARGS (line 80, packages/cli/src/commands/skills.ts), now paired with --full-depth to bypass the skills.sh registry blob lag (the documented "death-loop" diagnosis in the commit history). The --full-depth rationale is well-explained — verified as a real flag at vercel-labs/skills@v1.5.13 (grep "--full-depth" src/cli.ts--full-depth Search all subdirectories even when a root SKILL.md exists). ✓

Additive nit — customized-project-skills shadowing

Per the override semantics, if a user has customized their project's .claude/skills/ with project-specific skills not in the global set, the global install doesn't touch the project folder — but skills with the same name in both would be shadowed by global. No data loss (project skills remain on disk), but a user expecting their customized project skill to "win" would see global override it. The PR body's "Per Claude Code's documented order — personal overrides project — so a fresh global install silently supersedes a stale project copy" is the right framing; just worth noting that "stale project copy" could occasionally mean "intentionally customized project copy." Not a blocker — Claude Code's load order is the upstream contract this PR conforms to.

Concur with Miga's findings

  • Typo (director(ies)directories or pluralized) — cosmetic.
  • Marker-dir granularity on deep sub-paths (e.g. .gemini/antigravity/skills) — the Gemini-antigravity edge case is the one I'd flag for a tiny follow-up; reasonable to ship as-is.
  • resolveBases env-var asymmetry (XDG validated absolute; CODEX_HOME / VIBE_HOME / etc. used as-is) — informational, not blocking.
  • SKILLS_REF = "v1.5.13" pin — right call for determinism; bump discovery is manual today (worth a future automation).
  • Silent catch in installAllSkills mirror block — best-effort by design, correctly documented inline.

Verdict

COMMENT — external author, routing through <@U08E7PV788Z> for any stamp/merge call per protocol. Supersession is deliberate + clean; #1738/#1744 behavior preserved; CI fully green. No additional blockers from me on top of Miga's "LGTM with cosmetic nit" verdict.

— Jerrai

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Miga/Rames — I agree with the high-level architecture, but I found one blocker before stamping.

mirrorGlobalSkills currently mirrors every */SKILL.md directory present in the Claude global store, not just the HyperFrames skills this command just installed. Concretely, listSkillDirs(source) reads all of ~/.claude/skills, and then mirrorInto removes/replaces the same skill-name entry under every installed agent global dir.

That means a user with unrelated Claude Code skills already installed, e.g. gstack or personal/company skills in ~/.claude/skills, can run npx hyperframes skills update and have those unrelated skills symlinked/copied into Cursor/Codex/Goose/etc. Worse, if another agent already has a same-named skill from a different source, linkOrCopy removes it before linking to the Claude copy.

This PR should keep the mirror scoped to the HyperFrames skill set/source only. The clean fix is to pass the manifest-listed HyperFrames skill names into mirrorGlobalSkills (or derive them from skills-manifest.json / the install source lock) and add a regression test seeding ~/.claude/skills/gstack/SKILL.md plus a target agent marker, asserting gstack is not mirrored or replaced.

The cosmetic director(ies) typo Miga flagged is still non-blocking. CI is green; the blocker is the cross-source mirror scope.

…e store

mirrorGlobalSkills listed every */SKILL.md in ~/.claude/skills and fanned them
out — but that store is shared, so a user's gstack / personal / company Claude
skills would get symlinked (and, since linkOrCopy removes the target first,
could overwrite a same-named skill) into Cursor / Codex / Goose / etc.

Scope the mirror to HyperFrames' own skills via the upstream lock's source
attribution — the same definition the prune already uses
(skillsAttributedToSource) — never a directory listing. New
hyperframesSkillNames() reads the global lock and returns only skills attributed
to heygen-com/hyperframes; the mirror intersects that allow-list with what's in
the store. Empty (no lock / nothing attributed) → mirror nothing, never
everything.

Also fixes the cosmetic "director(ies)" log typo (now singular/plural-aware) and
extracts the fan-out into mirrorToInstalledAgents() to keep installAllSkills
under the complexity gate.

Regression: skillsMirror.test.ts asserts a foreign gstack skill in the store is
neither mirrored out nor allowed to replace another agent's same-named skill;
the skills-bench harness seeds ~/.claude/skills/gstack and asserts it never
leaks to any agent. 1045 CLI tests + lint/types/fallow green.

Addresses Magi's request-changes on #1753.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 verification at 402fbfae — clean fix for the store-sharing scoping issue. The change is exactly the right shape:

What R1 had wrong + what R2 fixes

R1's mirrorGlobalSkills called listSkillDirs(source) against ~/.claude/skillsthe entire shared store. So a user with gstack / personal / company / other-source skills installed alongside HyperFrames in their Claude store would have had ALL of them fanned out to every other installed agent's global dir. Not a HyperFrames-scoped mirror; a "your entire Claude library spread to Cursor/Codex/Goose/etc."

The fix:

  • mirrorGlobalSkills now requires skills: readonly string[] as a mandatory parameter (no more directory-scan).
  • New hyperframesSkillNames({ scope }) reuses the same source-attribution mechanism the prune already uses (skillsAttributedToSource(readSkillLock(...), DEFAULT_REPO_SLUG)) — so the allow-list is derived from the install lock's authoritative attribution, not heuristic.
  • The mirror callsite in installAllSkills extracts the lock-attributed names via hyperframesSkillNames({ scope: "global" }) and passes them through — no scan ever happens.
  • Empty when lock absent → mirror-none default (safe).

Test coverage on the fix

The new test pins exactly the failure mode that needed locking in:

"only mirrors the allow-listed skills, never other sources' (gstack)"

The test seeds ~/.claude/skills/{hyperframes, gstack} (HyperFrames + a foreign skill in the same shared store), then pre-creates a ~/.cursor/skills/gstack (cursor's own gstack from another source). Three invariants verified after mirrorGlobalSkills({ skills: ["hyperframes"], ... }):

  1. HyperFrames was linked into ~/.cursor/skills/hyperframes
  2. gstack stayed in ~/.claude/skills (foreign, didn't get pulled into the operation) ✓
  3. Cursor's pre-existing ~/.cursor/skills/gstack was NOT replaced with a symlink and NOT removed ✓

That third invariant is the critical one — proves the mirror doesn't clobber any same-named skill in another agent's dir that came from a different source. The rmSync(targetSkill, ...) in linkOrCopy is now scoped to the allow-list, so untouched skills are truly untouched.

Also picked up — typo + log shape

"Linked skills into N other agent director(ies).""Linked skills into N other agent ${n === 1 ? "directory" : "directories"}." per Miga's R1 nit #1. Cosmetic but cleaned.

Verdict

R2 cleanly addresses the scoping defect. Architecture is correct (mandatory allow-list, lock-derived attribution, no directory-scan). Test coverage is tight. Still COMMENT (external author, stamp routing through <@U08E7PV788Z>).

— Jerrai

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review at 402fbfa (one new commit since 3904420: fix(cli): scope agent mirror to HyperFrames' own skills, not the whole store).

This commit directly addresses Miguel's blocker — the cross-source mirror scope — and looks solid. The approach:

  1. New hyperframesSkillNames() in skillsManifest.ts — reads the skill lock and returns only the names attributed to the HyperFrames source (same source-attribution the prune path uses). Clean single-responsibility function, good JSDoc explaining why store-listing is wrong here.

  2. mirrorGlobalSkills() now takes a skills: readonly string[] allow-list and filters listSkillDirs() through a Set before mirroring. The type is readonly string[] which correctly signals the function won't mutate the caller's array. All call sites updated.

  3. mirrorToInstalledAgents() helper in skills.ts encapsulates the resolve-names → mirror → log flow, replaces the inline try/catch that previously mirrored everything. The director(ies) cosmetic nit is fixed too — now uses a proper ternary for singular/plural.

  4. Regression test — seeds gstack in the store + a pre-existing gstack in Cursor's skills dir, mirrors with skills: ["hyperframes"] only, and asserts: (a) hyperframes got linked, (b) gstack was NOT mirrored from the store, (c) Cursor's pre-existing gstack was neither replaced nor removed. This is exactly the scenario Miguel described. Well-structured, reads clearly.

  5. All existing mirror tests updated to pass the skills parameter. Mock for hyperframesSkillNames added to the skills command tests.

No concerns. The scoping is correct, the test covers the exact cross-source contamination scenario, and the implementation reuses the existing lock-attribution machinery rather than inventing a parallel concept. CI still running (several checks pending, key ones like SDK/skills/npx-shim already green).

LGTM — addresses the blocker cleanly.

— Miga

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 on 402fbfae: blocker resolved.

The mirror is now scoped by lock-attributed HyperFrames skill names via hyperframesSkillNames({ scope: "global" }), and mirrorGlobalSkills requires an explicit allow-list instead of scanning the whole Claude global store. That closes the cross-source contamination/data-replacement issue I requested changes on. The regression test covers the exact failure class: a foreign gstack in ~/.claude/skills is not mirrored, and an existing Cursor-side gstack is not replaced.

CI is green, including Test: skills, Smoke: global install, CodeQL, and Windows render/tests.

Verdict: APPROVE
Reasoning: The prior blocker is fixed with source-attributed scoping plus a targeted regression; no remaining blockers from my pass.

— Magi

@WaterrrForever WaterrrForever merged commit bf961d1 into main Jun 27, 2026
41 checks passed
@WaterrrForever WaterrrForever deleted the feat/skills-version-check branch June 27, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants