diff --git a/CLAUDE.md b/CLAUDE.md index e512ae4bfe..e3efc97911 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ npx skills add heygen-com/hyperframes --skill # a named match installs r > **Warning:** Never run `npx skills add ... --all` from inside a HyperFrames checkout: it selects the OpenClaw target, whose `skills/` directory is the repo's own `skills/`, and removes each existing `skills/` destination before linking. A fresh clone restores tracked source, but uncommitted work under `skills/` does not recover. Run it from an empty directory or from your own project root. This happens regardless of `metadata.internal`. -`skills add` resolves the skills.sh registry blob, which can lag `main` by hours, so a freshly added skill may be a little behind. `npx hyperframes skills update` installs from the current `main`; prefer it when freshness matters. +`skills add` resolves the skills.sh registry blob, which can lag `main` by hours, so a freshly added skill may be a little behind. `npx hyperframes skills update` installs the skills from the release tag that matches the CLI version, so they match the CLI that runs them. Make any video by asking for it; the `/hyperframes` skill carries the render contract and the references (format, runtimes, motion, look, captions, film, overlays, storyboards). The skills carry knowledge, not process. The README skills section mirrors this list; keep them in sync (see "Skill catalog maintenance" below). diff --git a/README.md b/README.md index da0e96f5c6..ee53a061ad 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,7 @@ npx skills add heygen-com/hyperframes > The picker opens with nothing pre-selected — the **Core Skills** group is all you need; `/slideshow`, `/remotion-to-hyperframes`, `/reverse-any-video`, `/style-sampling` and `/figma` install on demand. Agents and non-interactive runs should use `npx hyperframes skills update` instead — it installs exactly the core set, whereas `skills add --all` installs all 9 published skills. The six repo-internal skills under `.claude/skills` / `.agents/skills` are excluded by default. For the full published set use `npx hyperframes skills`. > -> `skills add` resolves the skills.sh registry blob, which can lag `main` by hours. `npx hyperframes skills update` installs from the current `main`, so reach for it when you need the newest copy of a skill. +> `skills add` resolves the skills.sh registry blob, which can lag `main` by hours. `npx hyperframes skills update` installs the skills from the release tag that matches your CLI version, so they match the CLI that runs them. Run project-local installs from your video project's directory. From a HyperFrames checkout, use `npx hyperframes skills update` for a global install instead: a diff --git a/docs/packages/cli.mdx b/docs/packages/cli.mdx index 5823b2e95f..294644d933 100644 --- a/docs/packages/cli.mdx +++ b/docs/packages/cli.mdx @@ -107,10 +107,12 @@ their final line, on stderr. `2` means the command line itself is wrong: an unknown command or flag, or a missing or invalid value. -**Version notices.** An owned install (npm, bun, pnpm, brew) updates itself +**Version notices.** An owned install (npm, bun, pnpm) updates itself quietly in the background and prints one line on the next run: -`hyperframes auto-updated to v0.1.5`. It never crosses a major version. Anywhere -it cannot install — `npx`, an unknown installer — you get a notice instead: +`hyperframes auto-updated to v0.1.5`. It never crosses a major version (before +1.0, a minor version: 0.8 to 0.9 is a notice, not an install). Anywhere +it cannot install — Homebrew (run `brew upgrade hyperframes` yourself), `npx`, an +unknown installer — you get a notice instead: ``` Update available: 0.1.4 → 0.1.5 @@ -2193,7 +2195,9 @@ Set `HYPERFRAMES_SKIP_SKILLS=1` when something else manages the skills (CI, test or an install pinned to one commit). `skills update` and bare `skills` then exit 0 with a one-line notice and leave the installed skills unchanged. -The CLI installs from the current HyperFrames GitHub source and links the global +The CLI installs the skills from the HyperFrames release tag that matches its own +version (`v`), and `check` compares against that same tag, so the skills +match the CLI that runs them; a development build uses `main`. It links the global bundles into whichever compatible agents it finds. After scaffolding, `init` installs the core set if it is missing. It never refreshes or removes a skill you already have; run `skills update` for that. diff --git a/packages/cli/src/commands/skills.test.ts b/packages/cli/src/commands/skills.test.ts index f42f444fd0..cd612f0627 100644 --- a/packages/cli/src/commands/skills.test.ts +++ b/packages/cli/src/commands/skills.test.ts @@ -210,7 +210,7 @@ describe("hyperframes skills", () => { const first = state.spawnCalls[0]; expect(first).toBeDefined(); expect(first!.command).toBe("npx"); - expect(first!.args).toContain("skills"); + expect(first!.args).toContain("skills@1.5.13"); expect(first!.args).toContain("add"); expect(first!.env?.GIT_CLONE_PROTECTION_ACTIVE).toBe("0"); // --full-depth clones the repo; skip LFS so we don't drag in unrelated blobs. @@ -223,7 +223,7 @@ describe("hyperframes skills", () => { "npx", ["--version"], [ - "skills", + "skills@1.5.13", "add", "https://github.com/heygen-com/hyperframes", "--skill", @@ -240,7 +240,7 @@ describe("hyperframes skills", () => { "npx", ["--version"], [ - "skills", + "skills@1.5.13", "add", "https://github.com/heygen-com/hyperframes", "--skill", @@ -261,7 +261,7 @@ describe("hyperframes skills", () => { "/s", "/c", "npx.cmd", - "skills", + "skills@1.5.13", "add", "https://github.com/heygen-com/hyperframes", "--skill", @@ -322,6 +322,25 @@ describe("hyperframes skills", () => { expect(agentValue).not.toBe("*"); }); + // The install uses the ref the check compared against (the CLI's release + // tag), resolved once, through the pinned skills CLI. + it("skills update installs at the ref its check read, through the pinned skills CLI", async () => { + setPlatform("linux"); + const { checkSkills } = await import("../utils/skillsManifest.js"); + vi.mocked(checkSkills).mockResolvedValueOnce({ ...DEFAULT_CHECK, ref: "v0.9.3" } as never); + + await runSkillsUpdate(); + + expect(await commandExitCode()).toBe(0); + const args = state.spawnCalls[0]?.args ?? []; + expect(args.slice(0, 3)).toEqual([ + "skills@1.5.13", + "add", + "https://github.com/heygen-com/hyperframes#v0.9.3", + ]); + expect(args).toContain("--full-depth"); + }); + it("skills update refreshes an outdated installed workflow (but never expands)", async () => { setPlatform("linux"); const { checkSkills } = await import("../utils/skillsManifest.js"); diff --git a/packages/cli/src/commands/skills.ts b/packages/cli/src/commands/skills.ts index a6b2eebf72..1e638e9516 100644 --- a/packages/cli/src/commands/skills.ts +++ b/packages/cli/src/commands/skills.ts @@ -5,6 +5,7 @@ import * as clack from "../ui/prompts.js"; import { c } from "../ui/colors.js"; import { diag } from "../ui/diagnostics.js"; import { buildNpxCommand } from "../utils/npxCommand.js"; +import { VERSION } from "../version.js"; import { withMeta } from "../utils/updateCheck.js"; import { checkSkills, @@ -14,6 +15,7 @@ import { presentSkills, RETIRED_SKILLS, retiredSkillNotice, + cliReleaseTag, type SkillDiff, type SkillsCheckResult, } from "../utils/skillsManifest.js"; @@ -105,7 +107,7 @@ function spawnNpx(args: string[], opts: { cwd?: string } = {}): Promise { // Why --copy: real files (not the upstream symlink default, which re-serialises // each SKILL.md's frontmatter) so an installed bundle byte-matches the published // manifest and `skills check` reads it as current. Why --full-depth: it forces a -// full `git clone` of HEAD; without it even a full-URL `skills add` fetches the +// full `git clone` of the requested ref; without it even a full-URL `skills add` fetches the // skills.sh registry blob, which lags GitHub main by hours, so a fresh install // would read as several skills "outdated" (verified: blob → ~9 outdated; // --full-depth → all current). @@ -129,13 +131,16 @@ function skillSelectionArgs(selection: SkillSelection): string[] { return names.flatMap((name) => ["--skill", name]); } +// Pinned: a bare `npx skills` runs whatever upstream published last. +const SKILLS_CLI = "skills@1.5.13"; + function runSkillsAdd( source: string, selection: SkillSelection, opts: { cwd?: string } = {}, ): Promise { return spawnNpx( - ["skills", "add", source, ...skillSelectionArgs(selection), ...GLOBAL_INSTALL_ARGS_TAIL], + [SKILLS_CLI, "add", source, ...skillSelectionArgs(selection), ...GLOBAL_INSTALL_ARGS_TAIL], opts, ); } @@ -147,10 +152,9 @@ 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; -// 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 -// resolves "latest" straight from GitHub too, so install and check agree. +// Use the full GitHub URL (not the `owner/repo` slug) as the clone source. +// --full-depth (see GLOBAL_INSTALL_ARGS_TAIL) clones it at `#`: the ref the +// check read its manifest at (this CLI's release tag), so check and install agree. const SOURCES = [{ name: "HyperFrames", url: "https://github.com/heygen-com/hyperframes" }]; // Fan HyperFrames' own skills out to every other installed agent. Scope by the @@ -242,13 +246,14 @@ function sanitizeSelection(selection: SkillSelection): SkillSelection | null { async function installSkills( selection: SkillSelection, - opts: { cwd?: string; strict?: boolean } = {}, + opts: { cwd?: string; strict?: boolean; ref: string }, ): Promise { const safeSelection = sanitizeSelection(selection); if (safeSelection === null) return; if (!skillsToolingReady(opts.strict ?? false)) return; + const { ref } = opts; // stderr (via diag): installSkills runs on the `skills update --json` path before // its JSON envelope is written to stdout — progress here must not corrupt it. for (const source of SOURCES) { @@ -256,7 +261,7 @@ async function installSkills( diag.notice(c.bold(`Installing ${source.name} skills...`)); diag.notice(); try { - await runSkillsAdd(source.url, safeSelection, opts); + await runSkillsAdd(ref === "main" ? source.url : `${source.url}#${ref}`, safeSelection, opts); } catch (err) { if (opts.strict) throw err instanceof Error ? err : new Error(String(err)); // warn, not notice: this is a non-fatal skip after a caught install error. @@ -341,7 +346,7 @@ export async function updateSkills( // that no longer exists, and verifyInstalled would strict-throw on a // "failure" that was never real. Resolving canonically means a retired // skill simply never appears as a target in the first place. - check = await checkSkills({ cwd: opts.cwd, canonical: true }); + check = await checkSkills({ cwd: opts.cwd, canonical: true, cliVersion: VERSION }); } catch (err) { // A *malformed* canonical manifest (the server was reached, but served a // bad shape) is otherwise indistinguishable from being offline — both fall @@ -402,7 +407,7 @@ export async function updateSkills( }; if (toInstall.length > 0) { - await installSkills(result.installed, { cwd: opts.cwd, strict }); + await installSkills(result.installed, { cwd: opts.cwd, strict, ref: check.ref ?? "main" }); verifyInstalled(result.installed, { strict, cwd: opts.cwd }); } // The install (or the fresh canonical check confirming everything current) @@ -479,7 +484,11 @@ async function updateSkillsOffline( ); if (absent.length > 0) { - await installSkills(absent, { cwd: opts.cwd, strict: opts.strict }); + await installSkills(absent, { + cwd: opts.cwd, + strict: opts.strict, + ref: cliReleaseTag(VERSION) ?? "main", + }); verifyInstalled(absent, opts); } return { @@ -572,6 +581,7 @@ const checkCommand = defineCommand({ const result = await checkSkills({ dir: args.dir, source: args.source, + cliVersion: VERSION, }); if (args.json) console.log(JSON.stringify(withMeta(result), null, 2)); diff --git a/packages/cli/src/utils/autoUpdate.invocation.test.ts b/packages/cli/src/utils/autoUpdate.invocation.test.ts new file mode 100644 index 0000000000..2c3e508cbf --- /dev/null +++ b/packages/cli/src/utils/autoUpdate.invocation.test.ts @@ -0,0 +1,108 @@ +import { afterEach, expect, it, vi } from "vitest"; + +// Drives scheduleBackgroundInstall through the real installer detection, argv helper and installer +// script; only the entry path, the settings file and spawn itself are stubbed. +const BREW_ENTRY = + "/opt/homebrew/Cellar/hyperframes/0.8.231/libexec/lib/node_modules/hyperframes/dist/cli.js"; +const NPM_ENTRY = "/usr/local/lib/node_modules/hyperframes/dist/cli.js"; + +const originalArgv = [...process.argv]; +const originalEnv = { ...process.env }; +const originalTTY = process.stderr.isTTY; +const originalWrite = process.stderr.write.bind(process.stderr); + +afterEach(() => { + process.argv = [...originalArgv]; + process.env = { ...originalEnv }; + Object.defineProperty(process.stderr, "isTTY", { value: originalTTY, configurable: true }); + process.stderr.write = originalWrite; + vi.resetModules(); + vi.doUnmock("../telemetry/config.js"); + vi.doUnmock("../version.js"); + vi.doUnmock("./env.js"); + vi.doUnmock("./attendedTerminal.js"); + vi.doUnmock("node:child_process"); + vi.doUnmock("node:fs"); +}); + +/** A 0.8.231 install at `entry` whose cached latest is the allowed patch 0.8.232. */ +async function patchAvailableAt(entry: string) { + vi.resetModules(); + const config: Record = { latestVersion: "0.8.232" }; + vi.doMock("../telemetry/config.js", () => ({ + readConfig: () => ({ ...config }), + readConfigFresh: () => ({ ...config }), + writeConfig: (next: Record) => { + for (const key of Object.keys(config)) delete config[key]; + Object.assign(config, next); + return true; + }, + })); + vi.doMock("../version.js", async (importOriginal) => ({ + ...(await importOriginal()), + VERSION: "0.8.231", + })); + vi.doMock("./env.js", () => ({ isDevMode: () => false })); + vi.doMock("./attendedTerminal.js", async (importOriginal) => ({ + ...(await importOriginal()), + isUnattended: () => false, + })); + const spawn = vi.fn((_bin: string, _args: readonly string[]) => ({ pid: 42, unref: () => {} })); + vi.doMock("node:child_process", async (importOriginal) => ({ + ...(await importOriginal()), + spawn, + })); + vi.doMock("node:fs", async (importOriginal) => ({ + ...(await importOriginal()), + mkdirSync: () => {}, + openSync: () => 99, + appendFileSync: () => {}, + })); + + process.argv = [process.argv[0] ?? "node", entry]; + for (const name of ["CI", "HYPERFRAMES_NO_AUTO_INSTALL", "HYPERFRAMES_NO_UPDATE_CHECK"]) { + delete process.env[name]; + } + Object.defineProperty(process.stderr, "isTTY", { value: true, configurable: true }); + const stderr: string[] = []; + process.stderr.write = ((chunk: unknown) => { + stderr.push(String(chunk)); + return true; + }) as typeof process.stderr.write; + + const { scheduleBackgroundInstall } = await import("./autoUpdate.js"); + const { printUpdateNotice } = await import("./updateCheck.js"); + const scheduled = scheduleBackgroundInstall("0.8.232", "0.8.231"); + printUpdateNotice(); + return { scheduled, spawn, config, notice: stderr.join("") }; +} + +/** The bin and argv the spawned installer script would execFile. */ +function installerArgv(spawn: { mock: { calls: [string, readonly string[]][] } }) { + const script = spawn.mock.calls[0]?.[1][1] ?? ""; + const bin = /const BIN = (.*);/.exec(script)?.[1]; + const args = /const ARGS = (.*);/.exec(script)?.[1]; + return { bin: bin && JSON.parse(bin), args: args && JSON.parse(args) }; +} + +it("never launches an upgrade on Homebrew, whose upgrade cannot be held to the patch", async () => { + const { scheduled, spawn, config, notice } = await patchAvailableAt(BREW_ENTRY); + + expect(scheduled).toBe(false); + expect(spawn).not.toHaveBeenCalled(); + expect(config["pendingUpdate"]).toBeUndefined(); + expect(notice).toContain("Update available: 0.8.231 → 0.8.232"); + expect(notice).toContain("Run: brew upgrade hyperframes"); +}); + +it("still installs an allowed patch on npm at that exact version", async () => { + const { scheduled, spawn, notice } = await patchAvailableAt(NPM_ENTRY); + + expect(scheduled).toBe(true); + expect(spawn).toHaveBeenCalledOnce(); + expect(installerArgv(spawn)).toEqual({ + bin: "npm", + args: ["install", "-g", "hyperframes@0.8.232"], + }); + expect(notice).toBe(""); +}); diff --git a/packages/cli/src/utils/autoUpdate.test.ts b/packages/cli/src/utils/autoUpdate.test.ts index 144e08e5bc..3204f99f40 100644 --- a/packages/cli/src/utils/autoUpdate.test.ts +++ b/packages/cli/src/utils/autoUpdate.test.ts @@ -150,6 +150,42 @@ describe("scheduleBackgroundInstall", () => { expect(spawnSpy).not.toHaveBeenCalled(); }); + // Before 1.0 a minor bump may break (semver), so 0.8.x -> 0.9.x is a "major" + // jump: notify only. Comparing the first number alone auto-installed it. + it("does NOT schedule across a 0.x minor jump", async () => { + const { spawnSpy, config } = setupMocks({ + installer: { kind: "npm", command: "npm install -g hyperframes@0.9.3" }, + }); + const { scheduleBackgroundInstall } = await import("./autoUpdate.js"); + + expect(scheduleBackgroundInstall("0.9.3", "0.8.231")).toBe(false); + expect(spawnSpy).not.toHaveBeenCalled(); + expect(config.pendingUpdate).toBeUndefined(); + }); + + it.each([ + ["0.8.232", "0.8.231"], + ["1.3.0", "1.2.5"], + ])("still schedules %s over %s (same breaking line)", async (latest, current) => { + const { spawnSpy } = setupMocks({ + installer: { kind: "npm", command: `npm install -g hyperframes@${latest}` }, + }); + const { scheduleBackgroundInstall } = await import("./autoUpdate.js"); + + expect(scheduleBackgroundInstall(latest, current)).toBe(true); + expect(spawnSpy).toHaveBeenCalledOnce(); + }); + + it("does NOT schedule across a major jump from 1.0 on", async () => { + const { spawnSpy } = setupMocks({ + installer: { kind: "npm", command: "npm install -g hyperframes@2.0.0" }, + }); + const { scheduleBackgroundInstall } = await import("./autoUpdate.js"); + + expect(scheduleBackgroundInstall("2.0.0", "1.9.4")).toBe(false); + expect(spawnSpy).not.toHaveBeenCalled(); + }); + it("skips in dev mode", async () => { const { spawnSpy } = setupMocks({ installer: { kind: "npm", command: "npm install -g hyperframes@0.4.4" }, diff --git a/packages/cli/src/utils/autoUpdate.ts b/packages/cli/src/utils/autoUpdate.ts index b399d009f0..c030b69ca2 100644 --- a/packages/cli/src/utils/autoUpdate.ts +++ b/packages/cli/src/utils/autoUpdate.ts @@ -12,9 +12,9 @@ * marker. The user is now on the new version. * * Guardrails: - * - Never auto-update across major versions. The user opts in explicitly - * via `hyperframes upgrade`. - * - Skip on CI, unattended runs (agents, no TTY), dev mode, unknown installer, ephemeral exec (npx), + * - Never auto-update across major versions (or minor versions while in + * 0.x). The user opts in explicitly via `hyperframes upgrade`. + * - Skip on CI, unattended runs (agents, no TTY), dev mode, Homebrew, unknown installer, ephemeral exec (npx), * or when `HYPERFRAMES_NO_AUTO_INSTALL` / `HYPERFRAMES_NO_UPDATE_CHECK` * is set. * - If a previous install is still in flight (less than 10 min old), don't @@ -53,10 +53,11 @@ function isAutoInstallDisabled(): boolean { ); } -/** Parse a semver-ish string's major number; returns NaN for pre-releases etc. */ -function majorOf(version: string): number { - const match = /^(\d+)\./.exec(version); - return match?.[1] ? Number.parseInt(match[1], 10) : Number.NaN; +/** `0.` before 1.0 (a 0.x minor may break), `` after; null when unparsable. */ +function breakingLine(version: string): string | null { + const match = /^(\d+)\.(\d+)\./.exec(version); + if (!match) return null; + return match[1] === "0" ? `0.${match[2]}` : String(match[1]); } /** @@ -242,18 +243,33 @@ function isSilentUpgrade(latestVersion: string, currentVersion: string): boolean } if (cmp <= 0) return false; - // Major-version jumps carry breaking-change risk. Don't silent-install; - // the existing `printUpdateNotice` banner nudges the user to run - // `hyperframes upgrade` explicitly. - const latestMajor = majorOf(latestVersion); - const currentMajor = majorOf(currentVersion); - if (Number.isFinite(latestMajor) && Number.isFinite(currentMajor) && latestMajor > currentMajor) { + // Major-version jumps (minor jumps while in 0.x) carry breaking-change risk. + // Don't silent-install; the existing `printUpdateNotice` banner nudges the + // user to run `hyperframes upgrade` explicitly. + const latestLine = breakingLine(latestVersion); + const currentLine = breakingLine(currentVersion); + if (latestLine !== null && currentLine !== null && latestLine !== currentLine) { log(`[skip] major-bump ${currentVersion} -> ${latestVersion}`); return false; } return true; } +/** The install to run unattended, or null when this installer cannot be held to `version`. */ +function backgroundInstaller( + version: string, +): { installCommand: string; invocation: InstallInvocation } | null { + const installer = detectInstaller(); + if (installer.kind === "skip" || installer.kind === "brew") { + const brew = `brew upgrade installs the tap's formula, not ${version}; notice only`; + log(`[skip] ${installer.kind === "brew" ? brew : installer.reason}`); + return null; + } + const installCommand = installer.installCommand(version); + const invocation = installInvocation(installer.kind, version); + return installCommand && invocation ? { installCommand, invocation } : null; +} + /** * If a new version is available and policy allows, kick off a detached * installer. Returns whether an install was spawned (for tests). @@ -262,14 +278,9 @@ export function scheduleBackgroundInstall(latestVersion: string, currentVersion: if (isAutoInstallDisabled()) return false; if (!isSilentUpgrade(latestVersion, currentVersion)) return false; - const installer = detectInstaller(); - if (installer.kind === "skip") { - log(`[skip] ${installer.reason}`); - return false; - } - const installCommand = installer.installCommand(latestVersion); - const invocation = installInvocation(installer.kind, latestVersion); - if (!installCommand || !invocation) return false; + const installer = backgroundInstaller(latestVersion); + if (!installer) return false; + const { installCommand, invocation } = installer; const config = readConfig(); diff --git a/packages/cli/src/utils/skillsManifest.releaseTag.test.ts b/packages/cli/src/utils/skillsManifest.releaseTag.test.ts new file mode 100644 index 0000000000..66da3a5462 --- /dev/null +++ b/packages/cli/src/utils/skillsManifest.releaseTag.test.ts @@ -0,0 +1,139 @@ +// A release build of the CLI reads its skills at its own release tag. + +import { execFile } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + buildManifest, + checkSkills, + cliReleaseTag, + MANIFEST_FILE, + type SkillsManifest, +} from "./skillsManifest.js"; + +const git = vi.hoisted(() => ({ fails: false, mainSha: "a".repeat(40) })); + +// `git ls-remote` knows main's HEAD, or fails like a missing git / blocked proxy. +vi.mock("node:child_process", () => ({ + execFile: vi.fn( + ( + _cmd: string, + _args: readonly string[], + _opts: unknown, + callback: (err: Error | null, out?: { stdout: string; stderr: string }) => void, + ) => + git.fails + ? callback(new Error("spawn git ENOENT")) + : callback(null, { stdout: `${git.mainSha}\trefs/heads/main\n`, stderr: "" }), + ), +})); + +const RAW = "https://raw.githubusercontent.com/heygen-com/hyperframes"; +const TAG_URL = `${RAW}/v0.9.2/${MANIFEST_FILE}`; + +let root: string; +let home: string; +let installed: SkillsManifest; +let onMain: SkillsManifest; + +/** Serve `byUrl` from the stubbed fetch; a function value answers that URL itself. */ +function serve(byUrl: Record Promise)>): void { + vi.stubGlobal( + "fetch", + vi.fn(async (url: string) => { + const answer = byUrl[url] ?? onMain; + if (typeof answer === "function") return answer(); + if (typeof answer === "number") return { ok: false, status: answer } as Response; + return { ok: true, status: 200, json: async () => answer } as unknown as Response; + }), + ); +} + +const fetchedUrls = (): string[] => vi.mocked(fetch).mock.calls.map(([url]) => String(url)); + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), "skills-release-tag-")); + home = join(root, "home"); + // The skills installed at v0.9.2; main has moved on since. + const skillsDir = join(home, ".claude/skills"); + mkdirSync(join(skillsDir, "hyperframes"), { recursive: true }); + writeFileSync(join(skillsDir, "hyperframes", "SKILL.md"), "# hyperframes at v0.9.2"); + installed = buildManifest(skillsDir, { source: "heygen-com/hyperframes" }); + onMain = { + source: "heygen-com/hyperframes", + skills: { hyperframes: { hash: "f".repeat(16), files: 1 } }, + }; + git.fails = false; + vi.mocked(execFile).mockClear(); +}); +afterEach(() => { + vi.unstubAllGlobals(); + rmSync(root, { recursive: true, force: true }); +}); + +const check = () => checkSkills({ cwd: root, home, canonical: true, cliVersion: "0.9.2" }); + +describe("cliReleaseTag", () => { + it.each([ + ["0.9.3", "v0.9.3"], + ["0.0.0-dev", null], + ["0.8.190-hdr.1", null], + ])("maps %s to %s", (version, tag) => { + expect(cliReleaseTag(version)).toBe(tag); + }); +}); + +describe("checkSkills on a release build", () => { + it("reads the manifest at the CLI's release tag without asking git first", async () => { + serve({ [TAG_URL]: installed }); + const res = await check(); + expect(fetchedUrls()).toEqual([TAG_URL]); + expect(vi.mocked(execFile)).not.toHaveBeenCalled(); + expect(res.ref).toBe("v0.9.2"); + expect(res.summary.outdated).toBe(0); + }); + + // The background nudge runs this check; comparing against main here would + // cache "1 skill outdated" for 24h while `skills update` finds nothing to do. + it("does not report skills outdated against main when the git lookup fails", async () => { + git.fails = true; + serve({ [TAG_URL]: installed }); + const res = await check(); + expect(res.summary.outdated).toBe(0); + expect(res.ref).toBe("v0.9.2"); + }); + + it("surfaces a timed-out tag fetch instead of comparing against main", async () => { + serve({ + [TAG_URL]: () => Promise.reject(new DOMException("This operation was aborted", "AbortError")), + }); + await expect(check()).rejects.toThrow(/aborted/); + expect(fetchedUrls()).toEqual([TAG_URL]); + }); + + it("falls back to main only when the tag's manifest is a 404", async () => { + serve({ [TAG_URL]: 404 }); + const res = await check(); + expect(fetchedUrls()).toEqual([TAG_URL, `${RAW}/${git.mainSha}/${MANIFEST_FILE}`]); + expect(res.ref).toBe("main"); + expect(res.summary.outdated).toBe(1); + }); + + it.each([500, 403])( + "surfaces an HTTP %i for the tag instead of comparing against main", + async (status) => { + serve({ [TAG_URL]: status }); + await expect(check()).rejects.toThrow(`HTTP ${status}`); + expect(fetchedUrls()).toEqual([TAG_URL]); + }, + ); + + it("compares a dev build against main", async () => { + serve({}); + const res = await checkSkills({ cwd: root, home, canonical: true, cliVersion: "0.0.0-dev" }); + expect(res.ref).toBe("main"); + expect(fetchedUrls()).toEqual([`${RAW}/${git.mainSha}/${MANIFEST_FILE}`]); + }); +}); diff --git a/packages/cli/src/utils/skillsManifest.ts b/packages/cli/src/utils/skillsManifest.ts index 71203fc9e2..835a6eb660 100644 --- a/packages/cli/src/utils/skillsManifest.ts +++ b/packages/cli/src/utils/skillsManifest.ts @@ -92,6 +92,8 @@ export interface SkillsCheckResult { skills: SkillDiff[]; /** Manifest-named skills the lock attributes to another publisher; left untouched. */ foreign: string[]; + /** Repo ref the manifest was read at; an install for this result must use the same one. */ + ref: string | null; } const DEFAULT_REPO_SLUG = "heygen-com/hyperframes"; @@ -576,12 +578,21 @@ function asSkillsManifest(data: unknown, sourceLabel: string): SkillsManifest { return m as SkillsManifest; } +/** A manifest fetch the server answered with an error status (404: no such ref). */ +class ManifestHttpError extends Error { + readonly status: number; + constructor(status: number, url: string) { + super(`HTTP ${status} fetching ${url}`); + this.status = status; + } +} + async function fetchManifest(url: string): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); try { const res = await fetch(url, { signal: controller.signal, headers: { Connection: "close" } }); - if (!res.ok) throw new Error(`HTTP ${res.status} fetching ${url}`); + if (!res.ok) throw new ManifestHttpError(res.status, url); return asSkillsManifest(await res.json(), url); } finally { clearTimeout(timeout); @@ -628,6 +639,11 @@ async function remoteHeadSha(repoSlug: string): Promise { } } +/** The release tag (`v`) of a published CLI build; null for a dev or pre-release build. */ +export function cliReleaseTag(version: string): string | null { + return /^\d+\.\d+\.\d+$/.test(version) ? `v${version}` : null; +} + /** Read a manifest from a local path — a manifest file or a repo root. */ function resolveLocalManifest(source: string): SkillsManifest { const direct = source.endsWith(".json") ? source : join(source, MANIFEST_FILE); @@ -638,15 +654,27 @@ function resolveLocalManifest(source: string): SkillsManifest { throw new Error(`No skills manifest found at: ${source}`); } -/** - * Fetch the manifest from GitHub. A full URL is fetched directly; an - * `owner/repo` slug (or the default repo) is SHA-pinned via `git ls-remote` to - * dodge raw-CDN lag, falling back to the branch URL when git is unavailable. - */ -async function fetchRemoteManifest(source?: string): Promise { - if (source?.startsWith("http")) return fetchManifest(source); +/** A manifest and the repo ref it was read at (null for a local file or a URL). */ +interface ResolvedManifest { + manifest: SkillsManifest; + ref: string | null; +} - const repoSlug = source ?? DEFAULT_REPO_SLUG; +/** The manifest at a release tag; null only when the repo has no such tag (404). */ +async function fetchTagManifest(repoSlug: string, tag: string): Promise { + try { + return await fetchManifest( + `https://raw.githubusercontent.com/${repoSlug}/${tag}/${MANIFEST_FILE}`, + ); + } catch (err) { + // A timeout or proxy error must not quietly compare against main. + if (err instanceof ManifestHttpError && err.status === 404) return null; + throw err; + } +} + +/** main's manifest, SHA-pinned via `git ls-remote` to dodge raw-CDN lag, else the branch URL. */ +async function fetchMainManifest(repoSlug: string): Promise { const sha = await remoteHeadSha(repoSlug); if (sha) { try { @@ -660,6 +688,24 @@ async function fetchRemoteManifest(source?: string): Promise { return fetchManifest(`https://raw.githubusercontent.com/${repoSlug}/main/${MANIFEST_FILE}`); } +/** + * Fetch the manifest from GitHub. A full URL is fetched directly; an + * `owner/repo` slug is read at the CLI's release tag (publish pushes it before + * npm), and at `main` only for a dev build or a tag the repo does not have. + */ +async function fetchRemoteManifest( + source?: string, + cliVersion?: string, +): Promise { + if (source?.startsWith("http")) return { manifest: await fetchManifest(source), ref: null }; + + const repoSlug = source ?? DEFAULT_REPO_SLUG; + const tag = cliReleaseTag(cliVersion ?? ""); + const atTag = tag ? await fetchTagManifest(repoSlug, tag) : null; + if (tag && atTag) return { manifest: atTag, ref: tag }; + return { manifest: await fetchMainManifest(repoSlug), ref: "main" }; +} + /** * Resolve the latest manifest. `source` may be: * - undefined → in-repo manifest if present (dev / CI), else fetch from GitHub @@ -678,18 +724,23 @@ async function fetchRemoteManifest(source?: string): Promise { async function resolveLatestManifest( source?: string, cwd = process.cwd(), - opts: { canonical?: boolean } = {}, -): Promise { + opts: { canonical?: boolean; cliVersion?: string } = {}, +): Promise { // A local path is a relative one (./ ../) or an absolute one — isAbsolute // covers POSIX `/…` and Windows `C:\…` / `\…` on their respective platforms. if (source && (source.startsWith(".") || isAbsolute(source))) { - return resolveLocalManifest(source); + return { manifest: resolveLocalManifest(source), ref: null }; } if (!source && !opts.canonical) { const repoManifest = findRepoManifest(cwd); - if (repoManifest) return JSON.parse(readFileSync(repoManifest, "utf8")) as SkillsManifest; + if (repoManifest) { + return { + manifest: JSON.parse(readFileSync(repoManifest, "utf8")) as SkillsManifest, + ref: null, + }; + } } - return fetchRemoteManifest(source); + return fetchRemoteManifest(source, opts.cliVersion); } /** @@ -704,9 +755,14 @@ export async function checkSkills( home?: string; /** See resolveLatestManifest — bypass the in-repo manifest shortcut. */ canonical?: boolean; + /** Read the remote manifest at this CLI version's release tag (see fetchRemoteManifest). */ + cliVersion?: string; } = {}, ): Promise { - const latest = await resolveLatestManifest(opts.source, opts.cwd, { canonical: opts.canonical }); + const { manifest: latest, ref } = await resolveLatestManifest(opts.source, opts.cwd, { + canonical: opts.canonical, + cliVersion: opts.cliVersion, + }); const skillNames = Object.keys(latest.skills); const root = locateInstall(skillNames, { dir: opts.dir, cwd: opts.cwd, home: opts.home }); // A directory sharing a name with one of ours can belong to another publisher. @@ -733,5 +789,6 @@ export async function checkSkills( summary: diff.summary, skills: diff.skills, foreign, + ref, }; } diff --git a/packages/cli/src/utils/skillsUpdateCheck.test.ts b/packages/cli/src/utils/skillsUpdateCheck.test.ts index 57ecaddeea..8a93c12e89 100644 --- a/packages/cli/src/utils/skillsUpdateCheck.test.ts +++ b/packages/cli/src/utils/skillsUpdateCheck.test.ts @@ -114,7 +114,7 @@ describe("skillsUpdateCheck", () => { // Must resolve against the canonical upstream manifest, not a possibly // stale in-repo skills-manifest.json, so this nudge agrees with what // `updateSkills` would actually reconcile. - expect(mockCheckSkills).toHaveBeenCalledWith({ canonical: true }); + expect(mockCheckSkills).toHaveBeenCalledWith({ canonical: true, cliVersion: "0.0.0-dev" }); }); it("does not persist anything when no install was located (nothing meaningful to cache)", async () => { @@ -228,7 +228,7 @@ describe("skillsUpdateCheck", () => { expect(skillsCheckDue()).toBe(true); await refreshSkillsCache(); - expect(mockCheckSkills).toHaveBeenCalledWith({ canonical: true }); + expect(mockCheckSkills).toHaveBeenCalledWith({ canonical: true, cliVersion: "0.0.0-dev" }); expect(config).toMatchObject({ skillsUpdateAvailable: false, skillsOutdatedCount: 0 }); }); diff --git a/packages/cli/src/utils/skillsUpdateCheck.ts b/packages/cli/src/utils/skillsUpdateCheck.ts index ced9be119b..e25bb18e49 100644 --- a/packages/cli/src/utils/skillsUpdateCheck.ts +++ b/packages/cli/src/utils/skillsUpdateCheck.ts @@ -13,6 +13,7 @@ import { isUnattended } from "./attendedTerminal.js"; import { hostAnswers } from "./hostAnswers.js"; import { checkSkills } from "./skillsManifest.js"; import { updateCheckDisabled } from "./updateCheck.js"; +import { VERSION } from "../version.js"; const CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000; // 24 hours @@ -43,7 +44,7 @@ export async function refreshSkillsCache(): Promise { // `canonical: true` so this nudge's counts agree with `updateSkills`'s // source of truth — otherwise a stale in-repo skills-manifest.json (e.g. // inside a hyperframes checkout) can produce a false-positive count here. - const result = await checkSkills({ canonical: true }); + const result = await checkSkills({ canonical: true, cliVersion: VERSION }); // Only record a meaningful check when skills were actually found. if (result.location) { // Fresh read: this runs in a background child, beside the command's own config writes.