mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-04 07:19:52 +00:00
fix(cli): scope skill installs to relevant agents, not all ~70 via --all (#1748)
`hyperframes skills`, `skills update`, and `init` all shelled out to
`skills add ... --all`, which the upstream CLI expands to
`--skill '*' --agent '*' -y` — every skill into every one of the ~70 agent
conventions it knows about. A user who only runs Claude Code got skill
folders for Cursor, Codex, and 50+ tools they never touch.
Replace `--all` with a resolved target set (new resolveAgentTargets):
1. If the project already has agent skill folders (`.claude/skills`,
`.hermes/skills`, …), install ONLY to those — an existing folder is the
strongest signal of intent, so honour it exactly.
2. Otherwise (blank project):
a. Running under Claude Code (CLAUDECODE) → just claude-code.
b. Else probe PATH for installed agent CLIs (claude, hermes, droid,
cursor, codex, opencode, gemini) — the gstack approach.
c. Else fall back to claude-code + the shared `.agents` universal dir,
which Cursor, Codex, OpenCode, Gemini, Copilot and ~14 others read
from in project scope. Never `--agent '*'`.
Installs stay `--skill '*'` (all skills) + `--copy` (faithful, detectable by
`skills check`); only the agent fan-out is scoped. The dir<->key map is
explicit because dir names differ from upstream keys (`.factory`/droid,
`.hermes`/hermes-agent) and many agents share `.agents` (-> the single
`universal` key). Keys verified against vercel-labs/skills@v1.5.13.
Verified end-to-end in an isolated project + HOME: the new args land exactly
`.claude/skills` (19) + `.agents/skills` (19) and nothing else — 2 folders,
not 50+. Pure resolver covered by skillsTargets.test.ts; command-arg shape and
the "never --all" regression pinned in skills.test.ts.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
05af482f22
commit
70a03f788b
@@ -584,12 +584,6 @@ async function scaffoldProject(
|
||||
async function ensureSkillsCurrent(destDir: string): Promise<void> {
|
||||
const { installAllSkills } = await import("./skills.js");
|
||||
const { checkSkills } = await import("../utils/skillsManifest.js");
|
||||
// --all pulls every skill (incl. ones not yet installed); --yes keeps it
|
||||
// non-interactive. When Claude Code is driving, target its native dir so
|
||||
// skills land in .claude/skills/.
|
||||
const extraArgs = process.env["CLAUDECODE"]
|
||||
? ["--all", "--agent", "claude-code", "--yes"]
|
||||
: ["--all", "--yes"];
|
||||
|
||||
console.log();
|
||||
console.log(c.bold("Checking AI coding skills against GitHub..."));
|
||||
@@ -602,7 +596,12 @@ async function ensureSkillsCurrent(destDir: string): Promise<void> {
|
||||
}
|
||||
|
||||
if (needsInstall) {
|
||||
await installAllSkills({ cwd: destDir, extraArgs });
|
||||
// installAllSkills resolves the agent target set from destDir + the
|
||||
// environment (Claude Code → claude-code; otherwise installed CLIs, else a
|
||||
// Claude-Code + `.agents` floor). A freshly-scaffolded project has no agent
|
||||
// folders yet, so this lands skills where the running agent will read them
|
||||
// rather than spraying to every agent convention.
|
||||
await installAllSkills({ cwd: destDir });
|
||||
} else {
|
||||
console.log(c.success("AI coding skills are already up to date."));
|
||||
}
|
||||
|
||||
@@ -56,6 +56,17 @@ vi.mock("../utils/skillsManifest.js", () => ({
|
||||
checkSkills: vi.fn(async () => ({ skills: [] })),
|
||||
}));
|
||||
|
||||
// Agent-target resolution probes the real cwd / PATH / env, which would make
|
||||
// the spawned-args assertions environment-dependent. Pin it to a fixed result
|
||||
// so these tests verify how the command BUILDS the spawn, not what's installed
|
||||
// on the test host. The resolver's own decision tree is covered in
|
||||
// skillsTargets.test.ts. buildSkillsAddArgs is reproduced (it's trivial) so the
|
||||
// arg shape under test stays real.
|
||||
vi.mock("../utils/skillsTargets.js", () => ({
|
||||
resolveAgentTargets: vi.fn(() => ({ agents: ["claude-code", "universal"], reason: "test" })),
|
||||
buildSkillsAddArgs: (agents: string[]) => ["--skill", "*", "--agent", ...agents, "--yes"],
|
||||
}));
|
||||
|
||||
function setPlatform(platform: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, "platform", {
|
||||
value: platform,
|
||||
@@ -109,13 +120,35 @@ describe("hyperframes skills", () => {
|
||||
"linux",
|
||||
"npx",
|
||||
["--version"],
|
||||
["skills", "add", "https://github.com/heygen-com/hyperframes", "--all", "--copy"],
|
||||
[
|
||||
"skills",
|
||||
"add",
|
||||
"https://github.com/heygen-com/hyperframes",
|
||||
"--skill",
|
||||
"*",
|
||||
"--agent",
|
||||
"claude-code",
|
||||
"universal",
|
||||
"--yes",
|
||||
"--copy",
|
||||
],
|
||||
],
|
||||
[
|
||||
"darwin",
|
||||
"npx",
|
||||
["--version"],
|
||||
["skills", "add", "https://github.com/heygen-com/hyperframes", "--all", "--copy"],
|
||||
[
|
||||
"skills",
|
||||
"add",
|
||||
"https://github.com/heygen-com/hyperframes",
|
||||
"--skill",
|
||||
"*",
|
||||
"--agent",
|
||||
"claude-code",
|
||||
"universal",
|
||||
"--yes",
|
||||
"--copy",
|
||||
],
|
||||
],
|
||||
[
|
||||
"win32",
|
||||
@@ -129,7 +162,12 @@ describe("hyperframes skills", () => {
|
||||
"skills",
|
||||
"add",
|
||||
"https://github.com/heygen-com/hyperframes",
|
||||
"--all",
|
||||
"--skill",
|
||||
"*",
|
||||
"--agent",
|
||||
"claude-code",
|
||||
"universal",
|
||||
"--yes",
|
||||
"--copy",
|
||||
],
|
||||
],
|
||||
@@ -162,9 +200,16 @@ describe("hyperframes skills", () => {
|
||||
setPlatform("linux");
|
||||
await runSkillsUpdate();
|
||||
expect(process.exitCode).toBe(0);
|
||||
const args = state.spawnCalls[0]?.args ?? [];
|
||||
// pulls the full set straight from GitHub
|
||||
expect(state.spawnCalls[0]?.args).toContain("https://github.com/heygen-com/hyperframes");
|
||||
expect(state.spawnCalls[0]?.args).toContain("--all");
|
||||
expect(args).toContain("https://github.com/heygen-com/hyperframes");
|
||||
// every skill, but to a scoped agent set — never the `--all` (= `--agent '*'`) spray
|
||||
expect(args).toContain("--skill");
|
||||
expect(args).toContain("--agent");
|
||||
expect(args).not.toContain("--all");
|
||||
// `--agent` must be followed by a concrete key, never the `'*'` wildcard
|
||||
const agentValue = args[args.indexOf("--agent") + 1];
|
||||
expect(agentValue).not.toBe("*");
|
||||
});
|
||||
|
||||
// `skills add --all` never deletes, so update must separately prune skills the
|
||||
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
type SkillDiff,
|
||||
type SkillsCheckResult,
|
||||
} from "../utils/skillsManifest.js";
|
||||
import { buildSkillsAddArgs, resolveAgentTargets } from "../utils/skillsTargets.js";
|
||||
import type { Example } from "./_examples.js";
|
||||
|
||||
export const examples: Example[] = [
|
||||
@@ -57,6 +58,25 @@ function runSkillsAdd(
|
||||
source: string,
|
||||
opts: { cwd?: string; extraArgs?: string[] } = {},
|
||||
): Promise<void> {
|
||||
// Targeting: an explicit `extraArgs` wins (callers/tests that know exactly
|
||||
// what they want); otherwise resolve which agents to install to. We must NOT
|
||||
// use the upstream `--all` (= `--skill '*' --agent '*' -y`), which sprays the
|
||||
// skills into every one of ~70 agent conventions on the machine. Instead we
|
||||
// install every skill (`--skill '*'`) to a scoped agent set: the project's
|
||||
// existing skill folders, else the agent running us / installed agent CLIs,
|
||||
// else a Claude-Code + `.agents` floor. See resolveAgentTargets.
|
||||
let extraArgs = opts.extraArgs;
|
||||
if (!extraArgs) {
|
||||
const targets = resolveAgentTargets({
|
||||
cwd: opts.cwd ?? process.cwd(),
|
||||
env: process.env,
|
||||
pathStr: process.env["PATH"] ?? "",
|
||||
platform: process.platform,
|
||||
});
|
||||
console.log(c.dim(`Installing to: ${targets.agents.join(", ")} — ${targets.reason}`));
|
||||
extraArgs = buildSkillsAddArgs(targets.agents);
|
||||
}
|
||||
|
||||
// `--copy` writes real files into each target agent's skills dir, instead of
|
||||
// the upstream default (a canonical `.agents/skills` store + per-agent
|
||||
// symlinks). That default re-serialises each SKILL.md's frontmatter, so an
|
||||
@@ -64,7 +84,7 @@ function runSkillsAdd(
|
||||
// check` then reports a freshly-installed set as outdated, and the symlinked
|
||||
// layout doesn't reliably land where the agent actually reads. Real copies
|
||||
// keep the install faithful to the manifest and detectable by `skills check`.
|
||||
return spawnNpx(["skills", "add", source, ...(opts.extraArgs ?? ["--all"]), "--copy"], opts);
|
||||
return spawnNpx(["skills", "add", source, ...extraArgs, "--copy"], opts);
|
||||
}
|
||||
|
||||
// Skill names are kebab-case directory names. Refuse anything that isn't one
|
||||
@@ -244,12 +264,15 @@ const updateCommand = defineCommand({
|
||||
const dir = args.dir;
|
||||
const source = args.source;
|
||||
|
||||
// `skills add --all` re-fetches every skill to the latest AND installs ones
|
||||
// not yet present — so "update" pulls the full set, not just what is already
|
||||
// The install re-fetches every skill to the latest AND installs ones not yet
|
||||
// present — so "update" pulls the full set, not just what is already
|
||||
// installed. This is where `init` and the stale-skills nudge both lead.
|
||||
// runSkillsAdd resolves the agent target set itself (existing project
|
||||
// folders → installed CLIs → a Claude-Code + `.agents` floor); we no longer
|
||||
// spray to every agent via `--all`.
|
||||
//
|
||||
// Note: the upstream `skills add` CLI has no `--dir` flag (it installs into
|
||||
// detected agent dirs), so `--dir` here scopes only the *prune* detection
|
||||
// the resolved agent dirs), so `--dir` here scopes only the *prune* detection
|
||||
// below, not the install. `--source` likewise drives where the prune's
|
||||
// "latest" manifest comes from; the install always targets the canonical
|
||||
// HyperFrames repo so `update` reliably pulls the published set.
|
||||
@@ -259,14 +282,14 @@ const updateCommand = defineCommand({
|
||||
// fails (no npx, `skills add` exits non-zero) it must exit non-zero too —
|
||||
// otherwise the `||` chain passes while nothing actually changed.
|
||||
try {
|
||||
await installAllSkills({ extraArgs: ["--all", "--yes"], strict: true });
|
||||
await installAllSkills({ strict: true });
|
||||
} catch (err) {
|
||||
clack.log.error(c.error(`Update failed: ${(err as Error).message}`));
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
|
||||
// `skills add --all` never deletes, so a skill renamed or dropped upstream
|
||||
// `skills add` never deletes, so a skill renamed or dropped upstream
|
||||
// (e.g. graphic-overlays → talking-head-recut) would linger forever. Prune
|
||||
// skills the lock attributes to our source that the manifest no longer
|
||||
// ships, so `check || update` fully reconciles the install to the manifest.
|
||||
|
||||
Reference in New Issue
Block a user