ext/hyperframes
2
0
Fork 0
mirror of https://github.com/heygen-com/hyperframes.git synced 2026-10-30 09:16:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(cli): init never touches installed skills and no command removes one (#6168)

Browse Source 
* fix(cli): init and skills update only touch HyperFrames skills; --skip-skills works

* fix(cli): init never touches installed skills and no command removes one

* fix(cli): shorten the init skills comment

* fix(cli): shorten the init skills comment

* fix(cli): the stale-skills notice reaches agents that run without a terminal

* test(cli): a run without a terminal now checks skills in the background

* fix(cli): the stale-skills notice also prints on --json runs
This commit is contained in:
Miguel Ángel
2026-10-24 04:08:29 +00:00
committed by GitHub
parent 886e8696df
commit 0d104e6fca
17 changed files with 236 additions and 1029 deletions
Download Patch File Download Diff File Expand all files Collapse all files
README.md
+1 -1
View File
@@ -71,7 +71,7 @@ HyperFrames ships 7 skills. Make any video by asking for it; the `/hyperframes`
For standalone installation, default to the **core set** (`/hyperframes`, `/hyperframes-cli`, `/hyperframes-registry`, `/media-use`). `npx hyperframes skills update` installs exactly that from anywhere; the interactive picker (`npx skills add heygen-com/hyperframes`) lists it as the "Core Skills" group, nothing pre-selected. The picker is interactive-only — a non-interactive or agent run without `--skill` installs all 7. Use `npx skills add heygen-com/hyperframes --all` to install the complete published set deliberately (skips the picker), or `npx skills add heygen-com/hyperframes --skill <name>` for just one (bare name, no leading `/`).
Standalone installs stay lean after that: `npx hyperframes init` keeps the **core set** fresh (plus whatever is already installed) and never expands a partial install; `/figma`, `/slideshow` and `/remotion-to-hyperframes` install **on demand** with `npx hyperframes skills update <name>`. Nothing re-pulls the full set behind your back.
Standalone installs stay lean after that: `npx hyperframes init` installs the **core set** only when it is missing and never touches a skill you already have (`npx hyperframes skills update` refreshes them, and never removes one); `/figma`, `/slideshow` and `/remotion-to-hyperframes` install **on demand** with `npx hyperframes skills update <name>`. Nothing re-pulls the full set behind your back.
### Plugin packages
docs/packages/cli.mdx
+6 -5
View File
@@ -113,7 +113,7 @@ npx hyperframes init my-video --tailwind
| `--keep-original` | Copy the `--video` file as is instead of converting a non-web codec to H.264 MP4 |
| `--model` | Whisper model for that transcription (for example `small.en`, `medium.en`, `large`) |
| `--language` | Language code for transcription (`en`, `es`, `ja`, …). Filters out speech in other languages. |
| `--skip-skills` | Currently ignored. Set `HYPERFRAMES_SKIP_SKILLS=1` to opt out in CI or tests. |
| `--skip-skills` | Leave AI coding skills alone: `init` does not install the core set. `HYPERFRAMES_SKIP_SKILLS=1` does the same.
Pass `--video` or `--audio` and the CLI transcribes the audio with Whisper and
patches the captions into the composition for you. `--skip-transcribe` turns
@@ -1862,9 +1862,10 @@ npx hyperframes skills update slideshow
Bare `skills` installs everything. `skills update` keeps a deliberate partial
installation partial: it refreshes the core set plus whatever is already
installed, and never expands beyond that unless you name a skill. Naming one
adds it. `check` and `update` both accept `--json`, plus `--dir` and `--source`
to point removed-detection at a different skills directory or source — those two
scope the prune, not the install.
adds it. `check` and `update` both accept `--json`, and `check` also accepts
`--dir` and `--source`. `update` never removes a skill, and it leaves a skill the
install record attributes to another publisher alone, even when the name matches
one of ours.
Set `HYPERFRAMES_SKIP_SKILLS=1` when something else manages the skills (CI, tests,
or an install pinned to one commit). `skills update` and bare `skills` then exit 0
@@ -1872,7 +1873,7 @@ with a one-line notice and leave the installed skills unchanged.
The CLI installs from the current HyperFrames GitHub source and links the global
bundles into whichever compatible agents it finds. After scaffolding, `init`
also refreshes the core set plus any HyperFrames skills you already have.
installs the core set if it is missing. It never refreshes or removes a skill you already have; run `skills update` for that.
#### `fatal: active post-checkout hook found during git clone`
packages/cli/src/cli.lifecycle.test.ts
+26
View File
@@ -321,6 +321,32 @@ describe("CLI lifecycle", () => {
});
});
describe("stale-skills notice under --json", () => {
it("prints on a --json run while the self-update notices stay off", async () => {
const printSkillsUpdateNotice = vi.fn();
const printUpdateNotice = vi.fn();
mockInitCommand(vi.fn());
mockTelemetry();
vi.doMock("./utils/autoUpdate.js", () => ({
reportCompletedUpdate: vi.fn(),
scheduleBackgroundInstall: vi.fn(),
}));
vi.doMock("./utils/updateCheck.js", () => ({
cachedUpdateCheck: () => ({ current: "1.0.0", latest: "1.0.0", updateAvailable: false }),
printUpdateNotice,
printStalePinNotice: vi.fn(),
}));
vi.doMock("./utils/skillsUpdateCheck.js", () => ({ printSkillsUpdateNotice }));
vi.doMock("./utils/backgroundChecks.js", () => ({ launchBackgroundChecks: vi.fn() }));
process.argv = ["node", "cli.ts", "init", "--json"];
await import("./cli.js");
await vi.waitFor(() => expect(printSkillsUpdateNotice).toHaveBeenCalledTimes(1));
expect(printUpdateNotice).not.toHaveBeenCalled();
});
});
const MOCKED_MODULES = [
"./commands/init.js",
"./telemetry/events.js",
packages/cli/src/cli.ts
+19 -18
View File
@@ -266,31 +266,32 @@ if (
// cached check while reporting fresh results of its own).
if (
!isHelp &&
!hasJsonFlag &&
command !== "upgrade" &&
command !== "events" &&
command !== "telemetry" &&
command !== "skills" &&
command !== "usage"
) {
// Report any completed auto-install from the previous run first, before
// kicking off the next check — so the user sees "updated to vX" once and
// we don't over-print.
import("./utils/autoUpdate.js").then((mod) => mod.reportCompletedUpdate()).catch(() => {});
if (!hasJsonFlag) {
// Report any completed auto-install from the previous run first, before
// kicking off the next check — so the user sees "updated to vX" once and
// we don't over-print.
import("./utils/autoUpdate.js").then((mod) => mod.reportCompletedUpdate()).catch(() => {});
import("./utils/updateCheck.js")
.then(async (mod) => {
_printUpdateNotice = mod.printUpdateNotice;
_printStalePinNotice = mod.printStalePinNotice;
const result = mod.cachedUpdateCheck();
if (result.updateAvailable) {
const auto = await import("./utils/autoUpdate.js").catch(() => null);
auto?.scheduleBackgroundInstall(result.latest, result.current);
}
})
.catch(() => {});
import("./utils/updateCheck.js")
.then(async (mod) => {
_printUpdateNotice = mod.printUpdateNotice;
_printStalePinNotice = mod.printStalePinNotice;
const result = mod.cachedUpdateCheck();
if (result.updateAvailable) {
const auto = await import("./utils/autoUpdate.js").catch(() => null);
auto?.scheduleBackgroundInstall(result.latest, result.current);
}
})
.catch(() => {});
}
// Skills freshness nudge — same gating as the CLI self-update notice.
// Skills freshness nudge: stderr only, so it also runs under --json, which agents pass.
import("./utils/skillsUpdateCheck.js")
.then((mod) => {
_printSkillsUpdateNotice = mod.printSkillsUpdateNotice;
@@ -342,8 +343,8 @@ async function finalizeCli(result: CommandResult): Promise<void> {
if (!hasJsonFlag) {
_printUpdateNotice?.();
_printStalePinNotice?.();
_printSkillsUpdateNotice?.();
}
_printSkillsUpdateNotice?.();
if (_appHistoryNotice) process.stderr.write(`◇ ${_appHistoryNotice}\n`);
process.exitCode = exitCode;
}
packages/cli/src/commands/init.test.ts
+26 -4
View File
@@ -30,13 +30,20 @@ const tailwindScript =
// TypeScript-aware runtime. vitest runs under node, so `process.execPath`
// would be node and couldn't load the entry. This repo hard-depends on bun
// (package.json scripts), so assuming it's on PATH is safe.
function runInit(args: string[]): { status: number; stdout: string; stderr: string } {
function runInit(
args: string[],
opts: { skillsEnvOptOut?: boolean } = {},
): { status: number; stdout: string; stderr: string } {
const env = { ...process.env, HYPERFRAMES_SKIP_SKILLS: "1" } as Record<
string,
string | undefined
>;
// Default keeps tests offline and fast; a test passes false to rely on the flag alone.
if (opts.skillsEnvOptOut === false) delete env.HYPERFRAMES_SKIP_SKILLS;
const res = spawnSync("bun", ["run", cliEntry, "init", ...args], {
encoding: "utf-8",
timeout: 30_000,
// The `--skip-skills` flag is neutered (see init.ts); the GitHub skills check
// is opted out only via this env var, so tests stay offline and fast.
env: { ...process.env, HYPERFRAMES_SKIP_SKILLS: "1" },
env,
});
return {
status: res.status ?? -1,
@@ -139,6 +146,21 @@ describe("hyperframes init flag rename", () => {
}
});
it("--skip-skills alone leaves skills alone, without the env opt-out", () => {
const dir = mkdtempSync(join(tmpdir(), "hf-init-test-"));
const target = join(dir, "proj");
try {
const res = runInit([target, "--example", "blank", "--non-interactive", "--skip-skills"], {
skillsEnvOptOut: false,
});
expect(res.status).toBe(0);
expect(res.stdout).not.toContain("Checking AI coding skills");
expect(res.stdout).not.toContain("temporarily ignored");
} finally {
rmSync(dir, { recursive: true, force: true });
}
});
it("--tailwind enables Tailwind utilities in scaffolded HTML", () => {
const dir = mkdtempSync(join(tmpdir(), "hf-init-test-"));
const target = join(dir, "proj");
packages/cli/src/commands/init.ts
+7 -39
View File
@@ -669,19 +669,8 @@ async function scaffoldProject(
}
}
/**
* Keep the AI coding skills present and current — TARGETED, not the full
* set. Guarantees the core set (the `/hyperframes` entry router + shared
* domain skills) and refreshes any skill already installed; the end-user
* workflow skills are NOT pulled here — they install on demand when their
* workflow is triggered (`hyperframes skills update <name>`, which the router
* runs before entering a workflow). Re-running `init` on an up-to-date machine
* is a no-op, and `init` never expands a deliberate partial install.
* Best-effort: offline, it degrades to a presence check and never breaks init.
* The install itself lands once GLOBALLY (~/.claude/skills + ~/.agents/skills)
* and mirrors into every other installed agent, so it is project-independent —
* the check is global-first to match.
*/
// Installs the core set when missing, globally; an installed skill is never refreshed or removed
// here (that is `hyperframes skills update`). Offline it only checks presence.
async function keepSkillsCurrent(destDir: string): Promise<void> {
const { updateSkills } = await import("./skills.js");
@@ -689,9 +678,9 @@ async function keepSkillsCurrent(destDir: string): Promise<void> {
console.log(c.bold("Checking AI coding skills against GitHub..."));
// Wrap defensively (non-strict already swallows most failures): a
// skills-install failure can never break `init` itself — it warns and
// proceeds, since --skip-skills no longer escapes this path.
// proceeds.
try {
const result = await updateSkills({ refreshInstalled: true, cwd: destDir });
const result = await updateSkills({ onlyMissing: true, cwd: destDir });
if (result.presenceOnly) {
// Freshness never got checked (GitHub unreachable) — don't claim
// "up to date"; the engine already reported what it could verify or
@@ -829,17 +818,7 @@ export default defineCommand({
const audioFlag = args.audio;
const skipTranscribe = args["skip-transcribe"] === true;
const keepOriginal = args["keep-original"] === true;
// Temporary measure while the skills.sh registry sync lags GitHub main: the
// `--skip-skills` FLAG is neutered so an agent (or user) that passes it can
// NOT dodge the GitHub skills freshness check. The "don't pass --skip-skills"
// guidance lives in SKILL.md, which ships through the same laggy skills.sh
// channel and can't be relied on to reach the agent — so the guarantee has to
// live in the CLI, the one channel that updates promptly (`npx
// hyperframes@latest`). CI and unit tests still opt out via the
// HYPERFRAMES_SKIP_SKILLS=1 env var, which the agent/user CLI path never sets.
// Revert to `args["skip-skills"] === true` once skills.sh catches up.
const skipSkills = process.env.HYPERFRAMES_SKIP_SKILLS === "1";
const skipSkillsFlagIgnored = args["skip-skills"] === true && !skipSkills;
const skipSkills = args["skip-skills"] === true || process.env.HYPERFRAMES_SKIP_SKILLS === "1";
const tailwind = args.tailwind === true;
const nonInteractive = args["non-interactive"] === true || args.agent === true;
const modelFlag = args.model;
@@ -850,15 +829,6 @@ export default defineCommand({
);
const interactive = !nonInteractive && process.stdout.isTTY === true;
if (skipSkillsFlagIgnored) {
console.log(
c.dim(
"Note: --skip-skills is temporarily ignored — init always checks AI skills " +
"against GitHub while the skills.sh registry catches up.",
),
);
}
let resolutionPreset: CanvasResolution | undefined;
if (args.resolution !== undefined) {
resolutionPreset = normalizeResolutionFlag(args.resolution);
@@ -1227,10 +1197,8 @@ export default defineCommand({
const files = readdirSync(destDir);
clack.note(files.map((f) => c.accent(f)).join("\n"), c.success(`Created ${name}/`));
// Check skills against GitHub and refresh only what's stale — the core set
// plus anything already installed; workflow skills install on demand. The
// --skip-skills flag is temporarily neutered (see above); CI/tests opt out
// via HYPERFRAMES_SKIP_SKILLS=1.
// Install the core set if it is missing; installed skills are left alone.
// --skip-skills (or HYPERFRAMES_SKIP_SKILLS=1) leaves skills alone.
if (!skipSkills) {
await keepSkillsCurrent(destDir);
}
packages/cli/src/commands/skills.test.ts
+22 -235
View File
@@ -107,11 +107,6 @@ vi.mock("../utils/skillsManifest.js", async (importOriginal) => {
checkSkills: vi.fn(async () => DEFAULT_CHECK),
hyperframesSkillNames: vi.fn(() => ["hyperframes"]),
presentSkills: vi.fn((names: readonly string[]) => [...names]),
// Default: nothing left to prune after `runSkillsRemove`. The real
// (unmocked) fs-level behavior is covered in skillsManifest.test.ts;
// here we only assert the wiring — what update passes in, and that it
// isn't reached when there's nothing removed.
pruneOrphanedLockEntries: vi.fn(() => []),
};
});
@@ -192,19 +187,11 @@ describe("hyperframes skills", () => {
vi.resetModules();
// vi.resetModules re-imports skills.js but the manifest mock's vi.fn
// instances persist — restore their default behavior for each test.
// pruneOrphanedLockEntries is reset explicitly too: relying on afterEach's
// vi.restoreAllMocks() to clear vi.fn() call state is vitest-3-specific
// (vitest 4 restores spies only), and call-count assertions like the
// twice-in-a-row convergence test would then see counts accumulated from
// earlier tests.
const { checkSkills, presentSkills, pruneOrphanedLockEntries } =
await import("../utils/skillsManifest.js");
const { checkSkills, presentSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills).mockReset();
vi.mocked(checkSkills).mockImplementation(async () => DEFAULT_CHECK as never);
vi.mocked(presentSkills).mockReset();
vi.mocked(presentSkills).mockImplementation((names: readonly string[]) => [...names]);
vi.mocked(pruneOrphanedLockEntries).mockReset();
vi.mocked(pruneOrphanedLockEntries).mockImplementation(() => []);
await resetCommandResult();
});
@@ -373,83 +360,33 @@ describe("hyperframes skills", () => {
// `skills add` never deletes, so update must separately prune skills the
// manifest dropped (renames/removals) for `check || update` to fully reconcile.
it("skills update prunes skills removed upstream, in the attributed scope", async () => {
it("skills update never removes a skill, even one check once called retired", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
// First call feeds the targeted install; the second is the prune detection.
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: [{ name: "graphic-overlays", status: "removed" }],
} as never);
vi.mocked(checkSkills).mockImplementation(
async () =>
({
...DEFAULT_CHECK,
skills: [...DEFAULT_CHECK.skills, { name: "graphic-overlays", status: "removed" }],
}) as never,
);
await runSkillsUpdate();
// install first, then a `skills remove` for the dropped skill
expect(state.spawnCalls[0]?.args).toContain("add");
const removeCall = state.spawnCalls.find((s) => s.args.includes("remove"));
expect(removeCall, "expected a `skills remove` spawn").toBeDefined();
expect(removeCall!.args).toContain("graphic-overlays");
expect(removeCall!.args).toContain("--yes");
expect(removeCall!.args).toContain("-g"); // attributed from the global lock → remove globally
expect(await commandExitCode()).toBe(0);
});
it("skills update removes every retired skill check reports, in one call", async () => {
setPlatform("linux");
const { checkSkills, RETIRED_SKILLS } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: RETIRED_SKILLS.map((name) => ({ name, status: "removed" })),
} as never);
await runSkillsUpdate();
const removeCalls = state.spawnCalls.filter((s) => s.args.includes("remove"));
expect(removeCalls).toHaveLength(1);
for (const name of RETIRED_SKILLS) expect(removeCalls[0]!.args).toContain(name);
expect(removeCalls[0]!.args).toContain("-g");
expect(await commandExitCode()).toBe(0);
});
// The scope the skill was attributed from drives the remove scope: a
// project-scoped removal must NOT pass -g (which would target a different,
// possibly user-owned, global skill of the same name).
it("skills update prunes in project scope without -g", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "project",
skills: [{ name: "graphic-overlays", status: "removed" }],
} as never);
await runSkillsUpdate();
const removeCall = state.spawnCalls.find((s) => s.args.includes("remove"));
expect(removeCall, "expected a `skills remove` spawn").toBeDefined();
expect(removeCall!.args).toContain("graphic-overlays");
expect(removeCall!.args).not.toContain("-g");
});
it("skills update does not prune when nothing was removed upstream", async () => {
setPlatform("linux");
await runSkillsUpdate();
expect(state.spawnCalls.some((s) => s.args.includes("remove"))).toBe(false);
expect(await commandExitCode()).toBe(0);
});
it("updateSkills with onlyMissing installs what is missing and leaves outdated skills alone", async () => {
setPlatform("linux");
const { updateSkills } = await import("./skills.js");
const result = await updateSkills({ onlyMissing: true });
expect(result.installed).toEqual(["hyperframes-registry"]);
expect(skillFlagValues(state.spawnCalls[0]?.args ?? [])).toEqual(["hyperframes-registry"]);
});
// Retired-skill regression (variant 1): the update engine's OWN targeted-
// install check must resolve the canonical (published) manifest, never a
// stale local `skills-manifest.json` a checkout might still have lying
// around — see resolveLatestManifest's in-repo shortcut. Without this, a
// skill retired upstream but still listed locally gets forced into
// `targets` (isCoreSkill pattern-matches `hyperframes-*`), `skills add`
// silently declines to install something that doesn't exist canonically,
// and the old code strict-threw on a "failure" that was never real.
it("checks freshness against the canonical manifest, never a possibly-stale local one", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
@@ -475,151 +412,6 @@ describe("hyperframes skills", () => {
// "Removing 6 skill(s) no longer published: captions-overlay, changelog-video,
// cut-the-curve, motion-doctrine, oversized-cursor, seam-craft" and deleted
// all six — every one of them currently published.
it("resolves the prune's manifest canonically, so a local manifest can never drive deletion", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
await runSkillsUpdate();
// The prune's check is the LAST call; assert on every call so a future
// caller can't reintroduce a non-canonical deletion path.
const calls = vi.mocked(checkSkills).mock.calls;
expect(calls.length).toBeGreaterThan(1);
for (const [arg] of calls) {
expect(arg).toEqual(expect.objectContaining({ canonical: true }));
}
});
// Retired-skill regression (variant 2): `skills remove` is a silent no-op
// for a lock entry with no on-disk bundle (upstream scans disk, not the
// lock, to decide what's "installed" — see pruneOrphanedLockEntries's
// doc comment). `skills update` must self-heal that lock entry itself so
// `check || update` actually converges instead of re-flagging it forever.
it("self-heals an orphaned lock entry after `skills remove` no-ops on it", async () => {
setPlatform("linux");
const { checkSkills, pruneOrphanedLockEntries } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: [{ name: "hyperframes-captions", status: "removed" }],
} as never);
vi.mocked(pruneOrphanedLockEntries).mockReturnValueOnce(["hyperframes-captions"]);
await runSkillsUpdate();
expect(pruneOrphanedLockEntries).toHaveBeenCalledWith(["hyperframes-captions"], "global");
expect(await commandExitCode()).toBe(0);
});
// The idempotent-second-run contract at the command level: once nothing is
// left attributed as removed (the fs-level idempotency of the prune itself
// is covered directly in skillsManifest.test.ts), a second `skills update`
// must be a clean no-op — no `skills remove` spawn, no prune call finding
// anything, still exit 0.
it("running update twice in a row converges — the second run prunes nothing", async () => {
setPlatform("linux");
const { checkSkills, pruneOrphanedLockEntries } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: [{ name: "hyperframes-captions", status: "removed" }],
} as never);
vi.mocked(pruneOrphanedLockEntries).mockReturnValueOnce(["hyperframes-captions"]);
await runSkillsUpdate();
expect(await commandExitCode()).toBe(0);
expect(state.spawnCalls.some((s) => s.args.includes("remove"))).toBe(true);
// Second run: nothing attributed as removed anymore (the lock entry was
// pruned above), so there's nothing left to reconcile.
state.spawnCalls = [];
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({ scope: "global", skills: [] } as never);
await runSkillsUpdate();
expect(await commandExitCode()).toBe(0);
expect(state.spawnCalls.some((s) => s.args.includes("remove"))).toBe(false);
// Nothing to prune this time — pruneOrphanedLockEntries isn't even reached.
expect(pruneOrphanedLockEntries).toHaveBeenCalledTimes(1);
});
// `update`'s prune runs the same removed-detection as `check`, so its
// --source/--dir must reach the internal checkSkills() — otherwise the prune
// reconciles against defaults even when the user pointed elsewhere.
it("skills update plumbs --source/--dir to its prune detection (parity with check)", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
await runSkillsUpdate({ source: "owner/repo", dir: "/custom/skills" });
// The last checkSkills call is the prune's — the update engine's own check
// (first call) doesn't take --source/--dir, matching where the install
// actually lands. `canonical` rides along on every call (GH #3111); an
// explicit --source still wins over it inside resolveLatestManifest.
expect(checkSkills).toHaveBeenLastCalledWith({
source: "owner/repo",
dir: "/custom/skills",
canonical: true,
});
});
// Skill names come from lock-file JSON keys; a flag-like / shell-special name
// must never reach the spawn (esp. the Windows cmd.exe path).
it("skills update never passes a non-slug skill name to remove", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: [
{ name: "graphic-overlays", status: "removed" },
{ name: "--config=evil.js", status: "removed" },
],
} as never);
await runSkillsUpdate();
const removeCall = state.spawnCalls.find((s) => s.args.includes("remove"));
expect(removeCall, "expected a `skills remove` spawn for the valid name").toBeDefined();
expect(removeCall!.args).toContain("graphic-overlays");
expect(removeCall!.args).not.toContain("--config=evil.js");
});
// The early-return guard in runSkillsRemove: when EVERY candidate name is
// rejected as non-slug, no `skills remove` is spawned at all (the prior test
// only covers a mix of valid + invalid). A spawn here would run `skills remove
// --yes` with no names — which the upstream CLI treats as "remove nothing" at
// best, or prompts interactively at worst — so we must not reach it.
it("skills update spawns no remove when every removed name is rejected", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills)
.mockResolvedValueOnce(DEFAULT_CHECK as never)
.mockResolvedValueOnce({
scope: "global",
skills: [
{ name: "--config=evil.js", status: "removed" },
{ name: "../escape", status: "removed" },
],
} as never);
await runSkillsUpdate();
expect(state.spawnCalls.some((s) => s.args.includes("remove"))).toBe(false);
// The install still ran and the update still succeeded — a cleanup no-op
// doesn't fail the update.
expect(state.spawnCalls[0]?.args).toContain("add");
expect(await commandExitCode()).toBe(0);
});
// When git is missing the upstream `skills add` would clone-abort with a noisy
// `spawn git ENOENT` block. Detect it first and never spawn the install, so a
// best-effort caller (init) skips cleanly and a fresh boot without git still
// scaffolds the project.
it("bare `skills` skips the install (no spawn) when git is unavailable", async () => {
setPlatform("linux");
state.gitMissing = true;
@@ -686,7 +478,7 @@ describe("hyperframes skills", () => {
expect(String(logSpy.mock.calls.at(-1)?.[0] ?? "")).toMatch(/managed externally/);
});
it("bare `skills update` neither installs nor prunes", async () => {
it("bare `skills update` does not install", async () => {
setPlatform("linux");
const { checkSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills).mockImplementation(
@@ -811,16 +603,11 @@ describe("hyperframes skills update <names>", () => {
state.spawnExitCode = 0;
state.gitMissing = false;
vi.resetModules();
// Same explicit resets as the describe above (incl. the vitest-4-proofing
// note on pruneOrphanedLockEntries).
const { checkSkills, presentSkills, pruneOrphanedLockEntries } =
await import("../utils/skillsManifest.js");
const { checkSkills, presentSkills } = await import("../utils/skillsManifest.js");
vi.mocked(checkSkills).mockReset();
vi.mocked(checkSkills).mockImplementation(async () => DEFAULT_CHECK as never);
vi.mocked(presentSkills).mockReset();
vi.mocked(presentSkills).mockImplementation((names: readonly string[]) => [...names]);
vi.mocked(pruneOrphanedLockEntries).mockReset();
vi.mocked(pruneOrphanedLockEntries).mockImplementation(() => []);
await resetCommandResult();
});
packages/cli/src/commands/skills.ts
+9 -121
View File
@@ -12,10 +12,8 @@ import {
hyperframesSkillNames,
isCoreSkill,
presentSkills,
pruneOrphanedLockEntries,
RETIRED_SKILLS,
retiredSkillNotice,
SKILLS_CLI_LOCK_PATHS_VERIFIED_AT,
type SkillDiff,
type SkillsCheckResult,
} from "../utils/skillsManifest.js";
@@ -148,20 +146,6 @@ function runSkillsAdd(
// on the Windows `cmd.exe` spawn path, where arg escaping is fragile.
const PLAIN_SKILL_NAME = /^[a-z0-9][a-z0-9._-]*$/i;
function runSkillsRemove(names: string[], opts: { global: boolean }): Promise<void> {
const safe = names.filter((n) => PLAIN_SKILL_NAME.test(n));
const rejected = names.filter((n) => !PLAIN_SKILL_NAME.test(n));
if (rejected.length) {
clack.log.warn(c.warn(`Skipping unexpected skill name(s): ${rejected.join(", ")}`));
}
if (!safe.length) return Promise.resolve();
// `skills remove --yes` deletes the bundle dir, every agent symlink, and the
// lock entry non-interactively. `-g` targets the global install; without it,
// the project (cwd) install — we pass whichever scope detection attributed
// these names from, so we never reach into a scope we didn't inspect.
return spawnNpx(["skills", "remove", ...safe, ...(opts.global ? ["-g"] : []), "--yes"]);
}
// Use the full GitHub URL (not the `owner/repo` slug) as the clone source. The
// freshness comes from --full-depth (see GLOBAL_INSTALL_ARGS_TAIL), which clones the
// repo at latest `main`; the URL just names what to clone. Our freshness check
@@ -169,7 +153,7 @@ function runSkillsRemove(names: string[], opts: { global: boolean }): Promise<vo
const SOURCES = [{ name: "HyperFrames", url: "https://github.com/heygen-com/hyperframes" }];
// Fan HyperFrames' own skills out to every other installed agent. Scope by the
// lock's source attribution (the same definition prune uses) — NOT by listing
// lock's source attribution (the same attribution the foreign check uses) — NOT by listing
// ~/.claude/skills, which is shared with the user's other Claude skills (gstack,
// personal, company). No-op when nothing is attributed or the global store is
// absent, so it's safe to run unconditionally after any install. Best-effort: a
@@ -331,6 +315,8 @@ export async function updateSkills(
opts: {
requested?: readonly string[];
refreshInstalled?: boolean;
/** Install only what is missing; never touch a skill that is already installed. */
onlyMissing?: boolean;
/** Every skill the manifest publishes (bare `hyperframes skills`). */
all?: boolean;
strict?: boolean;
@@ -383,9 +369,7 @@ export async function updateSkills(
return { ...(await updateSkillsOffline(requested, { strict, cwd: opts.cwd })), retired };
}
// "removed" entries are lock-attributed leftovers, not manifest skills —
// they are `skills update`'s prune concern, never an update target.
const manifestSkills = check.skills.filter((s) => s.status !== "removed");
const manifestSkills = check.skills;
const manifestNames = new Set(manifestSkills.map((s) => s.name));
const unknown = requested.filter((name) => !manifestNames.has(name));
@@ -404,7 +388,9 @@ export async function updateSkills(
isCoreSkill(s.name) ||
(opts.refreshInstalled === true && s.status !== "missing"),
);
const toInstall = targets.filter((s) => s.status === "missing" || s.status === "outdated");
const toInstall = targets.filter(
(s) => s.status === "missing" || (s.status === "outdated" && opts.onlyMissing !== true),
);
const result: UpdateSkillsResult = {
targets: targets.map((s) => s.name),
installed: toInstall.map((s) => s.name),
@@ -543,7 +529,6 @@ function renderCheck(result: SkillsCheckResult): void {
if (summary.outdated) parts.push(c.warn(`↑ ${summary.outdated} outdated`));
if (summary.coreMissing) parts.push(c.warn(`◦ ${summary.coreMissing} core not installed`));
if (onDemandMissing) parts.push(c.dim(`◦ ${onDemandMissing} available on demand`));
if (summary.removed) parts.push(c.warn(`✗ ${summary.removed} removed upstream`));
console.log(` ${parts.join(" ")}`);
printSkillSection(result, "outdated", "Outdated:", "↑", c.warn);
@@ -563,27 +548,6 @@ function renderCheck(result: SkillsCheckResult): void {
c.dim,
(s) => !isCoreSkill(s.name),
);
printSkillSection(
result,
"removed",
"Removed upstream (renamed or dropped — no longer published):",
"✗",
c.warn,
);
// Removed-detection cross-references the upstream skills lock. If that lock is
// absent where we expect it (e.g. upstream moved its path), removed-detection
// silently reports zero — so warn rather than imply a clean "up to date".
if (result.lockMissing) {
console.log();
console.log(
` ${c.warn(`! Skills lock not found — can't check for skills removed upstream.`)}`,
);
console.log(
` ${c.dim(` (lock paths verified against ${SKILLS_CLI_LOCK_PATHS_VERIFIED_AT})`)}`,
);
}
console.log();
if (result.updateAvailable) {
console.log(` ${c.accent("Update: npx hyperframes skills update")}`);
@@ -719,29 +683,12 @@ const updateCommand = defineCommand({
meta: {
name: "update",
description:
"Update the core set plus every installed HyperFrames skill to the latest, and remove any no longer published. Pass skill names to also install those (how on-demand skills install) — without names it never expands a partial install",
"Update the core set plus every installed HyperFrames skill to the latest. Pass skill names to also install those (how on-demand skills install) — without names it never expands a partial install, and it never removes a skill",
},
// Mirror `check`'s flags: the prune step runs the same removed-detection, so it
// must respect the same overrides. Without these, `update`'s internal
// checkSkills() fell back to defaults — pruning the auto-detected install
// against the default manifest even when the user pointed `check` elsewhere.
args: {
json: { type: "boolean", description: "Output as JSON", default: false },
dir: {
type: "string",
description:
"Skills dir for removed-detection only — scopes the prune, not the install (default: auto-detect)",
},
source: {
type: "string",
description:
"Where 'latest' comes from for removed-detection (local path, owner/repo, or URL) — does not change the install source",
},
},
async run({ args }) {
const dir = args.dir;
const source = args.source;
// Positional skill names (e.g. `hyperframes skills update slideshow`) are
// the ONLY way update expands an install: each named skill is guaranteed
// present and current. A retired name installs nothing; the run says where
@@ -753,7 +700,7 @@ const updateCommand = defineCommand({
return;
}
// Exit 0 without installing, pruning or touching the nudge cache: the pinned
// Exit 0 without installing or touching the nudge cache: the pinned
// install is the one the caller wants, so the skill's step keeps working.
if (skillsManagedExternally()) {
reportSkillsManagedExternally(requested, args.json === true);
@@ -769,12 +716,6 @@ const updateCommand = defineCommand({
// about. Explicit full set: `hyperframes skills` or `npx skills add
// heygen-com/hyperframes --all`.
//
// Note: the upstream `skills add` CLI has no `--dir` flag (it installs into
// 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 refreshes the published skills.
//
// strict: this is the documented recovery path for the agent/CI contract
// `hyperframes skills check || hyperframes skills update`. If the install fails (no npx, `skills add` exits
// non-zero, a named skill still absent afterwards) it must exit non-zero
@@ -787,59 +728,6 @@ const updateCommand = defineCommand({
setCommandExitCode(1);
return;
}
// `skills add` never deletes, so a skill renamed or dropped upstream
// (e.g. a retired workflow skill) 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.
//
// Safety: `removed` only ever contains skills the lock records as installed
// from our source (see detectRemoved) — never a user's own or another
// source's skills. We remove in the exact scope detection attributed from,
// so we never reach into a scope we didn't inspect. Best-effort: cleanup
// failure doesn't fail the update — the install the CI contract gates on
// already succeeded.
try {
// `canonical: true` for the same reason the install's target selection
// uses it (see updateSkills) — and more urgently, because this branch
// DELETES. Without it, resolveLatestManifest takes the findRepoManifest
// shortcut: any `skills-manifest.json` within 16 parent dirs of cwd
// becomes "latest". HyperFrames' own repo manifest declares
// `source: heygen-com/hyperframes`, so a checkout (or any project
// carrying a copy) matches attribution and every published skill absent
// from that local file is deleted globally as "no longer published".
// An explicit `--source` still wins — canonical only decides what
// "latest" means when no source was given. GH #3111.
const { skills, scope } = await checkSkills({ dir, source, canonical: true });
const removed = skills.filter((s) => s.status === "removed").map((s) => s.name);
if (removed.length) {
console.log();
console.log(
c.dim(`Removing ${removed.length} skill(s) no longer published: ${removed.join(", ")}`),
);
await runSkillsRemove(removed, { global: scope === "global" });
// Self-heal: `skills remove` only clears a lock entry for a name it
// found an on-disk bundle for (see pruneOrphanedLockEntries). A skill
// retired before it ever shipped a bundle to this machine has none, so
// the call above is a silent no-op for it — the lock entry lingers and
// would be re-flagged "removed" on every future run. Prune whatever is
// still attributed after the call so `check || update` converges
// instead of looping forever. Best-effort and scoped to exactly the
// lock the remove above targeted (same `scope`); a write failure here
// must not fail the update — the install already succeeded.
const scopeForPrune = scope ?? "global";
const stillOrphaned = pruneOrphanedLockEntries(removed, scopeForPrune);
if (stillOrphaned.length) {
console.log(
c.dim(
`Reconciled ${stillOrphaned.length} orphaned lock entr${stillOrphaned.length === 1 ? "y" : "ies"} with no on-disk bundle: ${stillOrphaned.join(", ")}`,
),
);
}
}
} catch (err) {
clack.log.warn(c.warn(`Skipped removed-skill cleanup: ${(err as Error).message}`));
}
},
});
packages/cli/src/telemetry/config.ts
-2
View File
@@ -448,7 +448,6 @@ export interface HyperframesConfig {
/** How many skills were missing (not installed) at the last check. */
skillsMissingCount?: number;
/** How many installed skills were flagged removed-upstream at the last check. */
skillsRemovedCount?: number;
/**
* True once the DE parallel-router experiment ("HF_DE_PARALLEL_ROUTER")
* has actually FAILED (its self-verify/generic-failure safety net fired —
@@ -660,7 +659,6 @@ function passthroughFields(parsed: Partial<HyperframesConfig>): Partial<Hyperfra
skillsUpdateAvailable: parsed.skillsUpdateAvailable,
skillsOutdatedCount: parsed.skillsOutdatedCount,
skillsMissingCount: parsed.skillsMissingCount,
skillsRemovedCount: parsed.skillsRemovedCount,
localEmbeddingEnabled: consentFrom(parsed.localEmbeddingEnabled),
};
}
packages/cli/src/utils/backgroundChecks.test.ts
+1 -1
View File
@@ -119,7 +119,7 @@ describe("launchBackgroundChecks", () => {
[
"no terminal",
() => Object.defineProperty(process.stderr, "isTTY", { value: false, configurable: true }),
["update"],
["update", "skills"],
],
])("with %s, hands the child only what the parent used to check", (_, arrange, checks) => {
arrange();
packages/cli/src/utils/skillsManifest.test.ts
+25 -371
View File
@@ -1,13 +1,11 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import {
chmodSync,
existsSync,
mkdtempSync,
mkdirSync,
readdirSync,
readFileSync,
rmSync,
statSync,
writeFileSync,
} from "node:fs";
import { execFile } from "node:child_process";
@@ -23,8 +21,6 @@ import {
isCoreSkill,
MANIFEST_FILE,
presentSkills,
pruneOrphanedLockEntries,
RETIRED_SKILLS,
retiredSkillNotice,
skillsAttributedToSource,
type SkillsManifest,
@@ -490,7 +486,14 @@ describe("skillsAttributedToSource", () => {
});
});
describe("checkSkills removed-upstream detection", () => {
describe("retiredSkillNotice", () => {
it("says where a retired skill's knowledge lives", () => {
expect(retiredSkillNotice("hyperframes-audio")).toContain("media-use");
expect(retiredSkillNotice("motion-graphics")).toContain("/hyperframes skill");
});
});
describe("checkSkills foreign skills", () => {
// The global lock path honours XDG_STATE_HOME; clear it so the fixture under
// <home>/.agents/.skill-lock.json is the one that's read.
let xdg: string | undefined;
@@ -548,309 +551,47 @@ describe("checkSkills removed-upstream detection", () => {
);
}
it("flags a lock-attributed skill the manifest dropped, ignoring other sources", async () => {
it("never compares or refreshes a skill another publisher installed under one of our names", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeGlobalLock(home, {
alpha: { source: "test" }, // still ours and in the manifest
gamma: { source: "test" }, // ours, dropped from manifest → removed
delta: { source: "someone/else" }, // a different source → never ours
});
const res = await checkSkills(opts);
const byName = Object.fromEntries(res.skills.map((s) => [s.name, s.status]));
expect(byName.gamma).toBe("removed");
expect(byName.delta).toBeUndefined();
expect(res.summary.removed).toBe(1);
});
// GH #3111 — the data-loss regression. `skills add --skill '*'` installs every
// skill in the repo, including the repo-native ones under `.claude/skills/`
// and `.agents/skills/`, and attributes them all to our source. The published
// manifest is generated from `<repoRoot>/skills` ONLY (gen-skills-manifest.ts),
// so it never lists them — and reading that silence as "no longer published"
// deleted them from every agent directory on the machine, immediately after
// the same command installed them.
//
// Reproduced end-to-end pre-fix against the real CLI: `skills add` installed
// 25 skills, then `skills update` printed "Removing 6 skill(s) no longer
// published: captions-overlay, changelog-video, cut-the-curve, motion-doctrine,
// oversized-cursor, seam-craft" and deleted all six. Their lock entries carried
// `.agents/skills/<name>/SKILL.md`; the survivors carried `skills/<name>/…`.
it("never prunes a skill installed outside the manifest's coverage root", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeGlobalLock(home, {
alpha: { source: "test" }, // published, in the manifest → untouched
gamma: {
source: "test", // ours, absent from the manifest…
skillPath: ".agents/skills/gamma/SKILL.md", // …but the manifest never covered it
},
});
writeGlobalLock(home, { alpha: { source: "someone/else" } });
const res = await checkSkills(opts);
const byName = Object.fromEntries(res.skills.map((s) => [s.name, s.status]));
expect(byName.gamma).not.toBe("removed");
expect(res.summary.removed).toBe(0);
expect(res.skills.find((s) => s.name === "alpha")).toBeUndefined();
expect(res.foreign).toEqual(["alpha"]);
expect(res.updateAvailable).toBe(false);
});
// The other half of the contract: the coverage filter must not blunt the
// retired-skill convergence #2176 added. A skill installed FROM `skills/` and
// since dropped from the manifest is still a real removal.
it("still prunes a manifest-covered skill that was genuinely dropped upstream", async () => {
it("never compares or refreshes a skill another publisher installed under one of our names", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeGlobalLock(home, {
alpha: { source: "test" },
gamma: { source: "test", skillPath: "skills/gamma/SKILL.md" },
});
writeGlobalLock(home, { alpha: { source: "someone/else" } });
const res = await checkSkills(opts);
expect(Object.fromEntries(res.skills.map((s) => [s.name, s.status])).gamma).toBe("removed");
expect(res.summary.removed).toBe(1);
expect(res.skills.find((s) => s.name === "alpha")).toBeUndefined();
expect(res.foreign).toEqual(["alpha"]);
expect(res.updateAvailable).toBe(false);
});
// Fail safe on unknown provenance: an entry written by an older upstream that
// recorded no skillPath cannot be shown to be manifest-covered, and this is a
// DELETE path — so it is left alone rather than guessed at.
it("leaves an entry with no skillPath alone rather than guessing", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeFileSync(
join(home, ".agents/.skill-lock.json"),
JSON.stringify({
version: 3,
skills: { alpha: { source: "test" }, gamma: { source: "test" } }, // no skillPath at all
}),
);
it("treats a skill with no lock entry as ours: provenance unknown, not foreign", async () => {
const { opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
const res = await checkSkills(opts);
expect(res.summary.removed).toBe(0);
expect(res.foreign).toEqual([]);
expect(res.skills.find((s) => s.name === "alpha")?.status).toBe("outdated");
});
// Retired skills (folded into guide folders of a remaining skill) are ours by
// name, so they are removed even where the generic rule fails safe: a lock
// entry with no skillPath, or a bundle on disk with no lock entry at all.
describe("retired skills", () => {
const retired = "hyperframes-core";
it("is one of RETIRED_SKILLS and absent from the skills/ tree", () => {
expect(RETIRED_SKILLS).toContain(retired);
const skillsRoot = join(dirname(fileURLToPath(import.meta.url)), "../../../../skills");
for (const name of RETIRED_SKILLS) expect(existsSync(join(skillsRoot, name))).toBe(false);
});
it("flags a retired skill whose lock entry has no skillPath", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeFileSync(
join(home, ".agents/.skill-lock.json"),
JSON.stringify({
version: 3,
skills: { alpha: { source: "test" }, [retired]: { source: "test" } },
}),
);
const res = await checkSkills(opts);
expect(res.skills.filter((s) => s.status === "removed").map((s) => s.name)).toEqual([
retired,
]);
});
it("flags a retired bundle on disk that no lock attributes", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
installSkills(join(home, ".agents/skills"), [retired]);
const res = await checkSkills(opts);
expect(res.skills.filter((s) => s.status === "removed").map((s) => s.name)).toEqual([
retired,
]);
expect(res.updateAvailable).toBe(true);
});
it("lists the eight retired workflow skills", () => {
for (const name of [
"embedded-captions",
"faceless-explainer",
"general-video",
"motion-graphics",
"music-to-video",
"pr-to-video",
"product-launch-video",
"talking-head-recut",
])
expect(RETIRED_SKILLS).toContain(name);
});
it("flags a retired workflow the lock attributes to us, even without a skillPath", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
installSkills(join(home, ".agents/skills"), ["general-video"]);
writeGlobalLock(home, { alpha: { source: "test" }, "general-video": { source: "test" } });
const res = await checkSkills(opts);
expect(res.skills.filter((s) => s.status === "removed").map((s) => s.name)).toEqual([
"general-video",
]);
});
it("leaves a same-named workflow bundle alone when no lock attributes it to us", async () => {
// Workflow names are generic: another source's `motion-graphics` must survive.
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
installSkills(join(home, ".agents/skills"), ["motion-graphics"]);
const res = await checkSkills(opts);
expect(res.summary.removed).toBe(0);
});
it("explains where a retired skill's knowledge lives", () => {
expect(retiredSkillNotice("pr-to-video")).toMatch(/\/hyperframes skill and its references/);
expect(retiredSkillNotice("hyperframes-audio")).toMatch(/\/media-use skill/);
});
it("leaves a retired name alone while the manifest still publishes it", async () => {
const { home, opts } = setup({
source: "test",
skills: { alpha: { hash: "x", files: 1 }, [retired]: { hash: "y", files: 1 } },
});
installSkills(join(home, ".agents/skills"), [retired]);
writeGlobalLock(home, { alpha: { source: "test" }, [retired]: { source: "test" } });
const res = await checkSkills(opts);
expect(res.summary.removed).toBe(0);
});
});
it("a removed skill alone makes an update available (no outdated/missing)", async () => {
// Manifest lists only alpha, with its REAL hash → "current" (not outdated).
const { home, opts } = setup({ source: "test", skills: {} });
const manifest: SkillsManifest = {
source: "test",
skills: { alpha: hashSkillBundle(join(home, ".agents/skills/alpha")) },
};
writeFileSync(opts.source, JSON.stringify(manifest));
it("never reports a skill the manifest dropped as an update", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeGlobalLock(home, { alpha: { source: "test" }, gamma: { source: "test" } });
const res = await checkSkills(opts);
expect(res.summary).toEqual({
current: 1,
outdated: 0,
missing: 0,
coreMissing: 0,
removed: 1,
});
expect(res.updateAvailable).toBe(true);
});
it("reports no removed skills when there is no lock to attribute from", async () => {
const { opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
// gamma is an orphan on disk, but with no lock we can't attribute it to us.
const res = await checkSkills(opts);
expect(res.summary.removed).toBe(0);
expect(res.skills.some((s) => s.status === "removed")).toBe(false);
// No install was located via auto-detect (skills live under <home>/.agents/skills
// which IS discovered) — so lockMissing reflects the genuinely-absent lock.
expect(res.lockMissing).toBe(true);
});
// A `--dir` pointing at a global-scoped install (under $HOME) must resolve the
// GLOBAL lock (<home>/.agents/.skill-lock.json), not the project lock
// (<cwd>/skills-lock.json). Before this fix, locateInstall hardcoded scope
// "project" for every --dir, so the global lock was never read and
// removed-detection silently found nothing for --dir installs.
it("--dir under $HOME resolves the global lock so removed-detection works", async () => {
const { home, opts } = setup({ source: "test", skills: { alpha: { hash: "x", files: 1 } } });
writeGlobalLock(home, {
alpha: { source: "test" }, // ours and still in the manifest
gamma: { source: "test" }, // ours, dropped from manifest → removed
});
const dir = join(home, ".agents/skills");
const res = await checkSkills({ source: opts.source, dir, cwd: opts.cwd, home });
expect(res.scope).toBe("global");
const byName = Object.fromEntries(res.skills.map((s) => [s.name, s.status]));
expect(byName.gamma).toBe("removed");
expect(res.summary.removed).toBe(1);
expect(res.lockMissing).toBe(false);
});
// Regression (Magi REQUEST_CHANGES at 88daa820): the common project-local case
// is *also* under $HOME — a project checkout at `<home>/work/proj` with skills
// at `<cwd>/.claude/skills`. A HOME-first heuristic misclassified this as
// global, so checkSkills read the global lock and `skills update` would prune
// with `-g` against a project install. CWD-containment must win: `--dir` under
// `cwd` resolves to PROJECT even when `cwd` itself is nested under $HOME.
it("--dir under a cwd that is itself nested under $HOME stays project-scoped", async () => {
const home = join(root, "home");
const project = join(home, "work", "proj"); // cwd nested INSIDE home
const skillsDir = join(project, ".claude/skills");
installSkills(skillsDir, ["alpha", "gamma"]);
const manifestPath = join(root, "m3.json");
writeFileSync(
manifestPath,
JSON.stringify({ source: "test", skills: { alpha: { hash: "x", files: 1 } } }),
);
// Project lock at <cwd>/skills-lock.json — only read if scope is "project".
// No global lock exists under $HOME, so a wrong-scope ("global") read would
// find no lock → zero removed (gamma would NOT be flagged). The project lock
// being read (gamma → removed) is the proof CWD-containment won.
writeFileSync(
join(project, "skills-lock.json"),
JSON.stringify({
version: 1,
skills: {
alpha: { source: "test", skillPath: "skills/alpha/SKILL.md" },
gamma: { source: "test", skillPath: "skills/gamma/SKILL.md" },
},
}),
);
const res = await checkSkills({ source: manifestPath, dir: skillsDir, cwd: project, home });
expect(res.scope).toBe("project");
const byName = Object.fromEntries(res.skills.map((s) => [s.name, s.status]));
expect(byName.gamma).toBe("removed");
expect(res.summary.removed).toBe(1);
});
// The complementary case: a `--dir` under the project tree (not $HOME) stays
// project-scoped and reads <cwd>/skills-lock.json.
it("--dir outside $HOME stays project-scoped and reads the project lock", async () => {
const project = join(root, "proj2");
const skillsDir = join(project, ".claude/skills");
installSkills(skillsDir, ["alpha", "gamma"]);
const manifestPath = join(root, "m2.json");
writeFileSync(
manifestPath,
JSON.stringify({ source: "test", skills: { alpha: { hash: "x", files: 1 } } }),
);
// Project lock lives at <cwd>/skills-lock.json — point cwd at the project.
writeFileSync(
join(project, "skills-lock.json"),
JSON.stringify({
version: 1,
skills: {
alpha: { source: "test", skillPath: "skills/alpha/SKILL.md" },
gamma: { source: "test", skillPath: "skills/gamma/SKILL.md" },
},
}),
);
const home = join(root, "home2");
mkdirSync(home, { recursive: true });
const res = await checkSkills({ source: manifestPath, dir: skillsDir, cwd: project, home });
expect(res.scope).toBe("project");
const byName = Object.fromEntries(res.skills.map((s) => [s.name, s.status]));
expect(byName.gamma).toBe("removed");
expect(res.summary.removed).toBe(1);
expect(res.skills.map((s) => s.name)).toEqual(["alpha"]);
});
});
// Regression coverage for "variant 1" of the retired-skill bug: `updateSkills`
// (see commands/skills.ts) resolves its own targeted-install check with
// `canonical: true` specifically so it never trusts a stale local
// `skills-manifest.json` — this is the mechanism that makes that safe.
describe("checkSkills canonical bypass of the in-repo manifest shortcut", () => {
afterEach(() => {
vi.unstubAllGlobals();
@@ -936,90 +677,3 @@ describe("checkSkills canonical bypass of the in-repo manifest shortcut", () =>
expect(res.skills.map((s) => s.name)).toEqual(["from-explicit-source"]);
});
});
describe("pruneOrphanedLockEntries", () => {
function writeLock(path: string, skills: Record<string, { source: string }>): void {
writeFileSync(path, JSON.stringify({ version: 1, skills, dismissed: [] }));
}
it("removes only the given names, leaving other entries and lock fields intact", () => {
const home = join(root, "home");
mkdirSync(join(home, ".agents"), { recursive: true });
const lockPath = join(home, ".agents", ".skill-lock.json");
writeLock(lockPath, {
a: { source: "heygen-com/hyperframes" },
b: { source: "heygen-com/hyperframes" },
c: { source: "heygen-com/hyperframes" },
});
const pruned = pruneOrphanedLockEntries(["a", "b"], "global", { home });
expect(pruned.sort()).toEqual(["a", "b"]);
const rewritten = JSON.parse(readFileSync(lockPath, "utf8"));
expect(Object.keys(rewritten.skills)).toEqual(["c"]);
expect(rewritten.version).toBe(1); // other lock fields survive the rewrite
});
it("is idempotent — a second call with the same names finds nothing left and no-ops", () => {
const home = join(root, "home");
mkdirSync(join(home, ".agents"), { recursive: true });
const lockPath = join(home, ".agents", ".skill-lock.json");
writeLock(lockPath, { a: { source: "heygen-com/hyperframes" } });
const first = pruneOrphanedLockEntries(["a"], "global", { home });
expect(first).toEqual(["a"]);
const before = readFileSync(lockPath, "utf8");
const second = pruneOrphanedLockEntries(["a"], "global", { home });
expect(second).toEqual([]);
// No entries left to touch → the file is never rewritten a second time.
expect(readFileSync(lockPath, "utf8")).toBe(before);
});
it("writes atomically with no trailing newline, no leftover temp file, and preserves the file mode", () => {
const home = join(root, "home-atomic");
mkdirSync(join(home, ".agents"), { recursive: true });
const lockPath = join(home, ".agents", ".skill-lock.json");
writeLock(lockPath, {
a: { source: "heygen-com/hyperframes" },
b: { source: "heygen-com/hyperframes" },
});
chmodSync(lockPath, 0o640);
const pruned = pruneOrphanedLockEntries(["a"], "global", { home });
expect(pruned).toEqual(["a"]);
const raw = readFileSync(lockPath, "utf8");
expect(raw.endsWith("\n")).toBe(false);
expect(JSON.parse(raw).skills).toEqual({ b: { source: "heygen-com/hyperframes" } });
// No `.tmp` sibling left behind by the temp-file + rename.
expect(readdirSync(join(home, ".agents"))).toEqual([".skill-lock.json"]);
// Original permissions survive the rewrite (POSIX only — Windows's fs
// layer reports 0o666 regardless of the mode we set, so the bits aren't
// meaningful there).
if (process.platform !== "win32") {
expect(statSync(lockPath).mode & 0o777).toBe(0o640);
}
});
it("no-ops without throwing when the lock file doesn't exist", () => {
const home = join(root, "home-without-lock");
mkdirSync(home, { recursive: true });
expect(pruneOrphanedLockEntries(["a"], "global", { home })).toEqual([]);
});
it("resolves the project lock at <cwd>/skills-lock.json for scope: project", () => {
const project = join(root, "project");
mkdirSync(project, { recursive: true });
writeLock(join(project, "skills-lock.json"), {
a: { source: "heygen-com/hyperframes" },
b: { source: "heygen-com/hyperframes" },
});
const pruned = pruneOrphanedLockEntries(["a"], "project", { cwd: project });
expect(pruned).toEqual(["a"]);
const rewritten = JSON.parse(readFileSync(join(project, "skills-lock.json"), "utf8"));
expect(Object.keys(rewritten.skills)).toEqual(["b"]);
});
});
packages/cli/src/utils/skillsManifest.ts
+42 -170
View File
@@ -17,14 +17,7 @@
import { execFile } from "node:child_process";
import { createHash } from "node:crypto";
import {
existsSync,
readdirSync,
readFileSync,
renameSync,
statSync,
writeFileSync,
} from "node:fs";
import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
import { homedir } from "node:os";
import { isAbsolute, join, relative, resolve, sep } from "node:path";
import { promisify } from "node:util";
@@ -65,9 +58,7 @@ export interface SkillsManifest {
skills: Record<string, SkillEntry>;
}
// "removed" = installed from our source but no longer in the manifest (renamed
// or dropped upstream). Attributed via the skills lock — see detectRemoved.
export type SkillStatus = "current" | "outdated" | "missing" | "removed";
export type SkillStatus = "current" | "outdated" | "missing";
export interface SkillDiff {
name: string;
@@ -97,16 +88,10 @@ export interface SkillsCheckResult {
outdated: number;
missing: number;
coreMissing: number;
removed: number;
};
skills: SkillDiff[];
/**
* True when an install was located but the upstream skills lock was absent at
* the expected path, so removed-detection couldn't run (it silently reports
* zero removed). Lets the CLI warn instead of misreporting "up to date" — a
* guard against the lock path silently no-op'ing if upstream moves it.
*/
lockMissing: boolean;
/** Manifest-named skills the lock attributes to another publisher; left untouched. */
foreign: string[];
}
const DEFAULT_REPO_SLUG = "heygen-com/hyperframes";
@@ -342,7 +327,7 @@ function discoverSkillRoots(base: string, scope: "project" | "global"): SkillRoo
/**
* Decide whether an explicit `--dir` is a project- or global-scoped install, so
* removed-detection reads the *right* lock. The upstream `skills` CLI keeps two
* lock attribution reads the *right* lock. The upstream `skills` CLI keeps two
* locks: a project lock at `<cwd>/skills-lock.json` and a global lock under
* `$HOME` (see lockPathForScope).
*
@@ -372,7 +357,7 @@ function scopeForDir(dir: string, home: string, cwd: string): "project" | "globa
/**
* Find the first skill root that actually contains HyperFrames skills. A
* `--dir` override (if given) is treated as a `.../skills` directory directly;
* its scope is inferred (see scopeForDir) so removed-detection reads the right
* its scope is inferred (see scopeForDir) so lock attribution reads the right
* lock. Otherwise scan global ($HOME) then project (cwd), auto-discovering hosts.
*
* Global is checked FIRST to match how agents actually load skills: Claude Code
@@ -478,27 +463,15 @@ export function diffSkills(
};
}
// ── Removed-upstream (orphaned) skills ────────────────────────────────────────
// ── Install record (lock) ────────────────────────────────────────────────────
//
// `skills add` / `init` / `hyperframes skills update` only ever add or refresh —
// none of them delete a skill the manifest no longer publishes (e.g. the
// eight retired workflow skills), so a stale bundle lingers forever and
// the manifest-only diff above can't see it. We surface these by cross-checking
// the vercel-labs/skills lock: a skill the lock attributes to OUR manifest
// `source` that the manifest no longer lists is "removed". Attribution is the
// whole point — `.../skills` is shared across sources, so we never infer "ours"
// from a directory name alone (that would flag every other source's skills too).
// The vercel-labs/skills lock records which source each installed skill came
// from. `.../skills` is shared across sources, so a skill is never called ours
// from its directory name alone.
interface LockEntry {
source?: string;
sourceUrl?: string;
/**
* Path of the skill's SKILL.md within the source repo, as upstream records it
* (`skills/slideshow/SKILL.md`, `.agents/skills/seam-craft/SKILL.md`). The
* only field distinguishing skills the published manifest covers from ones
* installed out of the same repo's other skill roots — see manifestCoversSkill.
*/
skillPath?: string;
}
/** The slice of the vercel-labs/skills lock file we read. */
@@ -524,19 +497,20 @@ export function skillsAttributedToSource(lock: SkillLock | null, source: string)
.map(([name]) => name);
}
// Removed-detection reads the vercel-labs/skills lock, whose on-disk paths live
// in *their* repo, not ours — so if upstream moves the lock, our cross-reference
// silently finds nothing and `detectRemoved` no-ops without a peep. Pin the
// upstream version these paths were verified against so a future bump is a
// deliberate, reviewable edit (re-check src/skill-lock.ts `getSkillLockPath`
// and src/local-lock.ts `getLocalLockPath` when bumping):
/**
* The lock records another publisher as the skill's source. A skill with no
* lock entry is not "from another source": provenance is unknown, not foreign.
*/
function isFromOtherSource(entry: LockEntry | undefined, source: string): boolean {
const recorded = [repoSlug(entry?.source), repoSlug(entry?.sourceUrl)].filter(Boolean);
return recorded.length > 0 && !recorded.includes(repoSlug(source));
}
// Lock locations, verified against vercel-labs/skills@v1.5.13 (src/skill-lock.ts,
// src/local-lock.ts); re-check them when bumping:
// - global: $XDG_STATE_HOME/skills/.skill-lock.json else ~/.agents/.skill-lock.json
// - project: <cwd>/skills-lock.json
// https://github.com/vercel-labs/skills/blob/v1.5.13/src/skill-lock.ts (global)
// https://github.com/vercel-labs/skills/blob/v1.5.13/src/local-lock.ts (project)
export const SKILLS_CLI_LOCK_PATHS_VERIFIED_AT = "vercel-labs/skills@v1.5.13";
/** Locate the vercel-labs/skills lock for a scope (paths pinned to the version above). */
/** Locate the vercel-labs/skills lock for a scope. */
function lockPathForScope(
scope: "project" | "global",
opts: { cwd?: string; home?: string },
@@ -572,116 +546,6 @@ export function hyperframesSkillNames(opts: {
return skillsAttributedToSource(readSkillLock(lockPath), DEFAULT_REPO_SLUG);
}
interface RemovedResult {
removed: SkillDiff[];
/** The lock was absent at the expected path — removed-detection silently no-ops. */
lockMissing: boolean;
}
/**
* The repo directory the published manifest is generated from — see
* `packages/cli/scripts/gen-skills-manifest.ts`, which walks `<repoRoot>/skills`
* and nothing else. Anything installed from a DIFFERENT root of the same repo
* (`.claude/skills/`, `.agents/skills/` — the repo-native contributor skills) is
* outside the manifest's coverage, so the manifest says nothing about it.
*/
const MANIFEST_COVERAGE_ROOT = "skills/";
/**
* Is the manifest authoritative about whether this skill still exists upstream?
*
* Only for skills installed from the directory the manifest is generated from.
* The lock records where in the repo each skill came from (`skillPath`, e.g.
* `skills/slideshow/SKILL.md` vs `.agents/skills/seam-craft/SKILL.md`), and
* both carry the same `source`, so source attribution alone cannot tell them
* apart. An entry with no `skillPath` (older lock format) is treated as NOT
* covered — for a delete, unknown provenance must fail safe. GH #3111.
*/
function manifestCoversSkill(entry: LockEntry | undefined): boolean {
const path = entry?.skillPath;
return typeof path === "string" && path.startsWith(MANIFEST_COVERAGE_ROOT);
}
/**
* Skills the lock attributes to our source that the manifest no longer ships.
*
* "Absent from the manifest" only means "removed upstream" for skills the
* manifest actually covers. `skills add --skill '*'` installs every skill in the
* repo — including the repo-native ones under `.claude/skills/` and
* `.agents/skills/`, which the published manifest deliberately omits — and
* attributes them all to our source. Without the coverage filter, every install
* is immediately followed by a prune that deletes those skills as "no longer
* published", so `check || update` never converges: `add` reinstalls them and
* the next `update` deletes them again, forever. GH #3111.
*/
function detectRemoved(
root: SkillRoot,
latest: SkillsManifest,
opts: { cwd?: string; home?: string },
): RemovedResult {
const lock = readSkillLock(lockPathForScope(root.scope, opts));
const attributed = skillsAttributedToSource(lock, latest.source);
const dropped = attributed.filter((name) => manifestCoversSkill(lock?.skills?.[name]));
// A retired name the lock attributes to us needs no skillPath (older locks
// lack one); a retired domain skill needs no lock entry at all when its
// bundle is on disk, because its prefix makes it ours by name.
const retired = RETIRED_SKILLS.filter(
(name) =>
attributed.includes(name) ||
(RETIRED_DOMAIN_SKILLS.includes(name) && existsSync(join(root.dir, name, "SKILL.md"))),
);
const removed = [...new Set([...dropped, ...retired])]
.filter((name) => !(name in latest.skills))
.sort()
.map((name) => ({ name, status: "removed" as const }));
return { removed, lockMissing: lock === null };
}
/**
* Remove `names` from the vercel-labs/skills lock at `scope`, writing the file
* back if anything changed. Self-heals the half of removed-upstream detection
* that `skills remove` can't: upstream's `remove` command scans ON-DISK skill
* directories to decide what's installed (see vercel-labs/skills'
* `removeCommand`), so a lock entry for a skill retired before it ever shipped
* a bundle to this machine has no on-disk dir to match. That makes `skills
* remove <name> -g --yes` a silent no-op — it prints "No matching skills found
* for: …" and exits 0 WITHOUT touching the lock. Left alone, `detectRemoved`
* re-flags the same lock entry as "removed" on every future run, forever.
*
* Reuses the pinned lock path (see SKILLS_CLI_LOCK_PATHS_VERIFIED_AT above —
* re-check that comment before bumping the upstream version this is pinned
* against) so this writes to exactly where the upstream CLI itself reads and
* writes the lock.
*
* Idempotent by construction: only entries still present in the lock are ever
* touched, so calling this again with the same names — after the upstream
* `skills remove` no-op reported above has already run once — finds nothing
* left and returns `[]`.
*/
export function pruneOrphanedLockEntries(
names: readonly string[],
scope: "project" | "global",
opts: { cwd?: string; home?: string } = {},
): string[] {
const path = lockPathForScope(scope, opts);
const lock = readSkillLock(path);
if (!lock?.skills) return [];
const pruned = names.filter((name) => name in lock.skills!);
if (pruned.length === 0) return [];
for (const name of pruned) delete lock.skills[name];
// Atomic write (temp file + rename, same pattern as telemetry/autoUpdate.ts
// and utils/download.ts) so a crash mid-write can never leave a truncated
// lock behind. `path` is guaranteed to exist here (readSkillLock already
// returned a non-null lock), so preserving its mode on the temp file before
// the rename is safe. No trailing newline: matches the upstream
// vercel-labs/skills lock's on-disk shape, so a prune stays a minimal diff.
const mode = statSync(path).mode & 0o777;
const tmp = `${path}.tmp`;
writeFileSync(tmp, JSON.stringify(lock, null, 2), { mode });
renameSync(tmp, path);
return pruned;
}
// ── Resolving the "latest" manifest ──────────────────────────────────────────
/** Walk up from `cwd` to find a repo checkout that ships the manifest. */
@@ -828,21 +692,29 @@ export async function checkSkills(
const latest = await resolveLatestManifest(opts.source, opts.cwd, { canonical: opts.canonical });
const skillNames = Object.keys(latest.skills);
const root = locateInstall(skillNames, { dir: opts.dir, cwd: opts.cwd, home: opts.home });
const installed = root ? hashInstalled(root, skillNames) : {};
const diff = diffSkills(installed, latest);
const removedResult = root
? detectRemoved(root, latest, { cwd: opts.cwd, home: opts.home })
: { removed: [], lockMissing: false };
const { removed, lockMissing } = removedResult;
// A directory sharing a name with one of ours can belong to another publisher.
// The lock says so; such a skill is never compared, refreshed or overwritten.
const lock = root ? readSkillLock(lockPathForScope(root.scope, opts)) : null;
const foreign = root
? skillNames.filter(
(name) =>
existsSync(join(root.dir, name, "SKILL.md")) &&
isFromOtherSource(lock?.skills?.[name], latest.source),
)
: [];
const ours = skillNames.filter((name) => !foreign.includes(name));
const installed = root ? hashInstalled(root, ours) : {};
const diff = diffSkills(installed, {
...latest,
skills: Object.fromEntries(ours.map((name) => [name, latest.skills[name]!])),
});
return {
location: root?.dir ?? null,
agent: root?.agent ?? null,
scope: root?.scope ?? null,
// Removed skills also mean the install isn't reconciled with the manifest —
// `skills update` now prunes them, so they count toward "update available".
updateAvailable: diff.updateAvailable || removed.length > 0,
summary: { ...diff.summary, removed: removed.length },
skills: [...diff.skills, ...removed],
lockMissing,
updateAvailable: diff.updateAvailable,
summary: diff.summary,
skills: diff.skills,
foreign,
};
}
packages/cli/src/utils/skillsUpdateCheck.test.ts
+29 -39
View File
@@ -1,7 +1,4 @@
// Nudge-count regression coverage: `refreshSkillsCache` must persist
// `summary.removed` (renamed/dropped skills), and the printed nudge total must
// include it — otherwise the background nudge undercounts what a plain
// `skills update` would actually reconcile (the "misleading 2 vs 3" bug).
// Nudge-count coverage: the printed total is outdated plus missing, as cached by refreshSkillsCache.
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
@@ -19,9 +16,10 @@ vi.mock("../telemetry/config.js", () => ({
},
}));
const gate = vi.hoisted(() => ({ suppressed: false, answers: true }));
const gate = vi.hoisted(() => ({ disabled: false, noTty: false, answers: true }));
vi.mock("./updateCheck.js", () => ({
updateNoticesSuppressed: () => gate.suppressed,
updateCheckDisabled: () => gate.disabled,
updateNoticesSuppressed: () => gate.disabled || gate.noTty,
}));
vi.mock("./hostAnswers.js", () => ({ hostAnswers: async () => gate.answers }));
@@ -36,7 +34,8 @@ describe("skillsUpdateCheck", () => {
config = {};
cachedRead = null;
mockCheckSkills.mockReset();
gate.suppressed = false;
gate.disabled = false;
gate.noTty = false;
gate.answers = true;
vi.stubEnv("HYPERFRAMES_SKIP_SKILLS", "");
});
@@ -47,7 +46,7 @@ describe("skillsUpdateCheck", () => {
it.each([
["HYPERFRAMES_SKIP_SKILLS is set", () => vi.stubEnv("HYPERFRAMES_SKIP_SKILLS", "1")],
["the run shows no notices", () => (gate.suppressed = true)],
["the update check is disabled (CI, dev, opt-out)", () => (gate.disabled = true)],
])("no background check is due when %s", async (_, arrange) => {
const { skillsCheckDue } = await import("./skillsUpdateCheck.js");
expect(skillsCheckDue()).toBe(true);
@@ -55,6 +54,12 @@ describe("skillsUpdateCheck", () => {
expect(skillsCheckDue()).toBe(false);
});
it("a run without a TTY (an agent) still has its background check due", async () => {
gate.noTty = true;
const { skillsCheckDue } = await import("./skillsUpdateCheck.js");
expect(skillsCheckDue()).toBe(true);
});
it("does not check skills when DNS does not answer", async () => {
gate.answers = false;
const { refreshSkillsCache } = await import("./skillsUpdateCheck.js");
@@ -69,18 +74,18 @@ describe("skillsUpdateCheck", () => {
mockCheckSkills.mockResolvedValue({
location: "/home/user/.claude/skills",
updateAvailable: false,
summary: { current: 1, outdated: 0, missing: 0, coreMissing: 0, removed: 0 },
summary: { current: 1, outdated: 0, missing: 0, coreMissing: 0 },
});
const { refreshSkillsCache } = await import("./skillsUpdateCheck.js");
await refreshSkillsCache();
expect(config).toMatchObject({ commandCount: 2, skillsOutdatedCount: 0 });
});
it("refreshSkillsCache persists the removed count alongside outdated/missing", async () => {
it("refreshSkillsCache persists the outdated and core-missing counts", async () => {
mockCheckSkills.mockResolvedValue({
location: "/home/user/.claude/skills",
updateAvailable: true,
summary: { current: 1, outdated: 2, missing: 3, coreMissing: 1, removed: 3 },
summary: { current: 1, outdated: 2, missing: 3, coreMissing: 1 },
});
const { refreshSkillsCache } = await import("./skillsUpdateCheck.js");
@@ -90,7 +95,6 @@ describe("skillsUpdateCheck", () => {
skillsUpdateAvailable: true,
skillsOutdatedCount: 2,
skillsMissingCount: 1,
skillsRemovedCount: 3,
});
// Must resolve against the canonical upstream manifest, not a possibly
// stale in-repo skills-manifest.json, so this nudge agrees with what
@@ -102,13 +106,11 @@ describe("skillsUpdateCheck", () => {
mockCheckSkills.mockResolvedValue({
location: null,
updateAvailable: false,
summary: { current: 0, outdated: 0, missing: 0, coreMissing: 0, removed: 0 },
summary: { current: 0, outdated: 0, missing: 0, coreMissing: 0 },
});
const { refreshSkillsCache } = await import("./skillsUpdateCheck.js");
await refreshSkillsCache();
expect(config["skillsRemovedCount"]).toBeUndefined();
});
/** Drive printSkillsUpdateNotice from the given cache shape; returns what it wrote (if anything). */
@@ -130,36 +132,25 @@ describe("skillsUpdateCheck", () => {
expect(await noticeTextFor({ skillsOutdatedCount: 2, skillsMissingCount: 1 })).toBeNull();
});
it("the cached nudge total counts removed skills, not just outdated/missing", async () => {
// Cache pre-populated as if a prior refreshSkillsCache had run — only
// outdated + missing, no removed (the pre-fix shape).
const text = await noticeTextFor({
skillsOutdatedCount: 1,
skillsMissingCount: 1,
skillsRemovedCount: 2,
});
// 1 outdated + 1 missing + 2 removed = 4, not the pre-fix "2".
expect(text).toContain("4 HyperFrames skills out of date or missing");
it("prints the stale-skills notice without a TTY, so an agent hears it", async () => {
gate.noTty = true;
const text = await noticeTextFor({ skillsOutdatedCount: 2, skillsMissingCount: 1 });
expect(text).toContain("3 HyperFrames skills out of date or missing");
});
it("prints nothing when outdated, missing, and removed are all zero", async () => {
it("prints nothing when the update check is disabled (CI, dev, opt-out)", async () => {
gate.disabled = true;
expect(await noticeTextFor({ skillsOutdatedCount: 2, skillsMissingCount: 1 })).toBeNull();
});
it("prints nothing when outdated and missing are both zero", async () => {
const text = await noticeTextFor({
skillsOutdatedCount: 0,
skillsMissingCount: 0,
skillsRemovedCount: 0,
});
expect(text).toBeNull();
});
it("a removed-only count (no outdated/missing) still triggers the nudge", async () => {
const text = await noticeTextFor({
skillsOutdatedCount: 0,
skillsMissingCount: 0,
skillsRemovedCount: 1,
});
expect(text).toContain("1 HyperFrames skill out of date or missing");
});
// Regression: the stale-24h-cache bug. A successful `skills update`/install
// never wrote the cache, and the skills commands are excluded from the nudge
// pipeline entirely — so the pre-install "20 out of date or missing" verdict
@@ -172,7 +163,6 @@ describe("skillsUpdateCheck", () => {
skillsUpdateAvailable: true,
skillsOutdatedCount: 12,
skillsMissingCount: 8,
skillsRemovedCount: 0,
};
it("a fresh cache short-circuits the background check with the stale verdict (the bug's precondition)", async () => {
@@ -187,14 +177,14 @@ describe("skillsUpdateCheck", () => {
mockCheckSkills.mockResolvedValue({
location: "/home/user/.claude/skills",
updateAvailable: false,
summary: { current: 20, outdated: 0, missing: 0, coreMissing: 0, removed: 0 },
summary: { current: 20, outdated: 0, missing: 0, coreMissing: 0 },
});
const { skillsCheckDue, refreshSkillsCache, invalidateSkillsCache } =
await import("./skillsUpdateCheck.js");
invalidateSkillsCache();
// All five cached fields are gone — timestamp AND counts.
// All four cached fields are gone — timestamp AND counts.
expect(config["lastSkillsCheck"]).toBeUndefined();
expect(config["skillsUpdateAvailable"]).toBeUndefined();
expect(config["skillsOutdatedCount"]).toBeUndefined();
packages/cli/src/utils/skillsUpdateCheck.ts
+12 -16
View File
@@ -9,7 +9,7 @@
import { readConfig, readConfigFresh, writeConfig } from "../telemetry/config.js";
import { hostAnswers } from "./hostAnswers.js";
import { checkSkills } from "./skillsManifest.js";
import { updateNoticesSuppressed } from "./updateCheck.js";
import { updateCheckDisabled } from "./updateCheck.js";
const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; // 24 hours
@@ -17,8 +17,6 @@ interface SkillsUpdateMeta {
updateAvailable: boolean;
outdated: number;
missing: number;
/** Installed skills flagged removed-upstream (renamed/dropped) at the last check. */
removed: number;
}
/** Synchronous read from cache — never fetches. */
@@ -28,7 +26,6 @@ function getSkillsUpdateMeta(): SkillsUpdateMeta {
updateAvailable: config.skillsUpdateAvailable ?? false,
outdated: config.skillsOutdatedCount ?? 0,
missing: config.skillsMissingCount ?? 0,
removed: config.skillsRemovedCount ?? 0,
};
}
@@ -54,11 +51,6 @@ export async function refreshSkillsCache(): Promise<void> {
// Core-missing only: skills that install on demand (workflows not yet
// triggered on this machine) are not "missing" worth nagging about.
config.skillsMissingCount = result.summary.coreMissing;
// Removed-upstream skills are just as reconcilable as outdated/missing
// ones (a plain `skills update` prunes them) — omitting them here is what
// made the nudge undercount (e.g. reporting "2 skills out of date or
// missing" while a 3rd, renamed/dropped skill sat unmentioned).
config.skillsRemovedCount = result.summary.removed;
writeConfig(config);
}
}
@@ -90,33 +82,37 @@ export function invalidateSkillsCache(): void {
delete config.skillsUpdateAvailable;
delete config.skillsOutdatedCount;
delete config.skillsMissingCount;
delete config.skillsRemovedCount;
writeConfig(config);
} catch {
// best-effort — never break the command that just reconciled skills
}
}
/** Not tied to a TTY: agents run the CLI without one and are the ones that need to hear it. */
function skillsNoticeOff(): boolean {
return process.env["HYPERFRAMES_SKIP_SKILLS"] === "1" || updateCheckDisabled();
}
/** Whether a background skills check is due: this run shows notices and the 24 h cache is stale. */
export function skillsCheckDue(): boolean {
if (process.env["HYPERFRAMES_SKIP_SKILLS"] === "1" || updateNoticesSuppressed()) return false;
if (skillsNoticeOff()) return false;
return !cacheFresh(readConfig().lastSkillsCheck, Date.now());
}
/** The stale-skills nudge text, or null when nothing is outdated, missing, or removed. */
/** The stale-skills nudge text, or null when nothing is outdated or missing. */
function skillsNoticeText(meta: SkillsUpdateMeta): string | null {
const total = meta.outdated + meta.missing + meta.removed;
const total = meta.outdated + meta.missing;
if (total < 1) return null;
const noun = total === 1 ? "skill" : "skills";
return `\n ${total} HyperFrames ${noun} out of date or missing.\n Run: npx hyperframes skills update\n\n`;
}
/**
* Print a one-line nudge to stderr if installed skills are stale. Same gating
* as the CLI self-update notice (CI, non-TTY, dev, HYPERFRAMES_NO_UPDATE_CHECK).
* Print a one-line nudge to stderr if installed skills are stale. Off in CI, dev,
* HYPERFRAMES_NO_UPDATE_CHECK and HYPERFRAMES_SKIP_SKILLS; on without a TTY.
*/
export function printSkillsUpdateNotice(): void {
if (process.env["HYPERFRAMES_SKIP_SKILLS"] === "1" || updateNoticesSuppressed()) return;
if (skillsNoticeOff()) return;
const text = skillsNoticeText(getSkillsUpdateMeta());
if (text) process.stderr.write(text);
}
skills-manifest.json
+2 -2
View File
@@ -6,11 +6,11 @@
"files": 2
},
"hyperframes": {
"hash": "b222ac5809fd2633",
"hash": "8fc4b05db245cbbb",
"files": 77
},
"hyperframes-cli": {
"hash": "c7d7fb8c6106d8dd",
"hash": "d82c156c1b2069d1",
"files": 13
},
"hyperframes-registry": {
skills/hyperframes-cli/references/init-and-scaffold.md
+1 -1
View File
@@ -24,7 +24,7 @@ Other useful flags:
- `--resolution` — preset: `landscape` (1920×1080), `portrait` (1080×1920), `landscape-4k`, `portrait-4k`, `square` (1080×1080), `square-4k`. Aliases: `1080p`, `4k`, `uhd`, `1080p-square`, `4k-square`.
- `--skill=<slug>` — record the owning skill (e.g. `hyperframes`) in `hyperframes.json`, so every later render of this project — re-renders, `npm run render`, `--batch` — is attributed to it on anonymous telemetry without re-passing the flag. You rarely pass it by hand.
- `--skip-skills` — **temporarily ignored**: `init` always checks AI coding skills against GitHub while the skills.sh registry catches up. To opt out (CI/tests), set the `HYPERFRAMES_SKIP_SKILLS=1` env var instead.
- `--skip-skills` — leave AI coding skills alone: `init` does not install the core set. `HYPERFRAMES_SKIP_SKILLS=1` does the same.
- `--skip-transcribe` — don't auto-transcribe `--audio` / `--video` with Whisper.
- `--model`, `--language` — Whisper model / language for the auto-transcription.
skills/hyperframes/references/skill-lifecycle.md
+8 -4
View File
@@ -9,11 +9,15 @@ For a plugin installation, follow [plugin execution rules](plugin-installation.m
## What `init` does
`npx hyperframes init` checks GitHub and refreshes the core set plus other skills already installed. A current install is a no-op. Offline or rate-limited checks degrade gracefully and do not fail project scaffolding.
`npx hyperframes init` installs the core set when it is missing and leaves every skill that is already installed as it is: it never refreshes, replaces or removes one. Updating is `npx hyperframes skills update`, run by you. Offline or rate-limited checks degrade gracefully and do not fail project scaffolding.
The `--skip-skills` CLI flag is temporarily ignored. CI and tests may opt out with `HYPERFRAMES_SKIP_SKILLS=1`.
`--skip-skills` makes `init` leave skills alone. `HYPERFRAMES_SKIP_SKILLS=1` does the same for CI and tests.
`HYPERFRAMES_SKIP_SKILLS=1` also makes `skills update` (bare or named) and bare `skills` exit 0 without installing or pruning, so an install pinned to one commit stays pinned.
`HYPERFRAMES_SKIP_SKILLS=1` also makes `skills update` (bare or named) and bare `skills` exit 0 without installing, so an install pinned to one commit stays pinned.
## Stale-skills notice
After `render`, `lint` and `validate`, the CLI prints `N HyperFrames skills out of date or missing` to stderr when the cached check finds stale skills, so an agent run without a terminal sees it too. It is off in CI, in dev, with `HYPERFRAMES_NO_UPDATE_CHECK=1` and with `HYPERFRAMES_SKIP_SKILLS=1`, and runs with `--json` too, since it only writes stderr and leaves `--json` stdout untouched.
## Diagnose and update
@@ -26,7 +30,7 @@ npx hyperframes skills
```
- `skills check` exits non-zero when an installed skill is stale or the core set is incomplete. Skills available on demand but not installed are not failures.
- Bare `skills update` refreshes the core set and everything already installed, and removes skills that are no longer published.
- Bare `skills update` refreshes the core set and the HyperFrames skills already installed. It never removes a skill, and it leaves a skill the install record attributes to another publisher alone, even when the name matches one of ours.
- Named `skills update <name...>` also installs those named skills.
- Bare `skills` installs the full published set explicitly.