From ab3ede496de97cd1576386e7afabcbc7a63aeca9 Mon Sep 17 00:00:00 2001 From: Deepak Jain Date: Sat, 9 May 2026 05:35:41 -0700 Subject: [PATCH] feat(cli): add per-skill pinning Add per-skill CLI pinning and harden install semantics so pinned skills cannot be overwritten by forced installs.\n\nTests:\n- bunx vitest run packages/clawhub/src/cli/commands/skills.test.ts packages/clawhub/src/skills.test.ts packages/schema/src/schemas.test.ts\n- bun run --cwd packages/clawhub verify:build\n- bun run ci:static\n- bunx tsc -p packages/schema/tsconfig.json --noEmit\n- bunx tsc -p packages/clawhub/tsconfig.json --noEmit\n\nCo-authored-by: deepujain --- CHANGELOG.md | 1 + docs/cli.md | 16 ++ packages/clawhub/README.md | 2 + packages/clawhub/src/cli.ts | 19 +++ .../clawhub/src/cli/commands/skills.test.ts | 150 ++++++++++++++++++ packages/clawhub/src/cli/commands/skills.ts | 109 +++++++++++-- packages/clawhub/src/schema/schemas.ts | 2 + packages/clawhub/src/skills.test.ts | 11 +- packages/schema/src/schemas.test.ts | 14 +- packages/schema/src/schemas.ts | 2 + 10 files changed, 311 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00d315ec..57209839 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changes +- CLI: add per-skill pinning so installed skills can be frozen against direct updates, bulk updates, and force reinstalls (#1806) (thanks @deepujain). - Web: polish browse/listing surfaces across skills, plugins, and search, including plugin card view parity, clearer search controls, visible safety filtering, and more consistent card metadata treatment (#2084) (thanks @vyctorbrzezowski). - Web: rename the skills and plugins browse alternate view from Cards to Grid while keeping legacy `view=cards` URLs compatible (#2090) (thanks @vyctorbrzezowski). - Web: allow skill owners and publisher admins to edit a skill summary from the detail page (#1411) (thanks @SylvanXiao). diff --git a/docs/cli.md b/docs/cli.md index fe5b28d1..2c5b0190 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -126,6 +126,7 @@ Stores your API token + cached registry URL. - Resolves latest version via `/api/v1/skills/`. - Downloads zip via `/api/v1/download`. - Extracts into `//`. +- Refuses to overwrite pinned skills; run `clawhub unpin ` first. - Writes: - `/.clawhub/lock.json` (legacy `.clawdhub`) - `/.clawhub/origin.json` (legacy `.clawdhub`) @@ -139,6 +140,18 @@ Stores your API token + cached registry URL. ### `list` - Reads `/.clawhub/lock.json` (legacy `.clawdhub`). +- Shows `pinned` next to skills frozen with `clawhub pin`, including the optional reason. + +### `pin ` + +- Marks an installed skill as pinned in the lockfile. +- `--reason ` records why the skill is frozen. +- Pinned skills are skipped by `update --all` and rejected by direct `update `. +- Pinned skills also reject `install --force` so the local bytes cannot be replaced accidentally. + +### `unpin ` + +- Removes the lockfile pin from an installed skill so future updates can modify it. ### `update [slug]` / `update --all` @@ -147,6 +160,9 @@ Stores your API token + cached registry URL. - If fingerprint does not match: - refuses by default - overwrites with `--force` (or prompt, if interactive) +- Pinned skills are never updated by `--force`. +- `update ` fails fast for pinned slugs and tells you to run `clawhub unpin ` first. +- `update --all` skips pinned slugs and prints a summary of what stayed frozen. ### `skill publish ` diff --git a/packages/clawhub/README.md b/packages/clawhub/README.md index 4d1708f8..ab203322 100644 --- a/packages/clawhub/README.md +++ b/packages/clawhub/README.md @@ -39,8 +39,10 @@ Notes: ```bash clawhub search "postgres backups" clawhub install my-skill-pack +clawhub pin bear-notes --reason "scanner-flagged while awaiting moderation" clawhub update --all clawhub update --all --no-input --force +clawhub unpin bear-notes clawhub skill publish ./my-skill-pack --slug my-skill-pack --name "My Skill Pack" --version 1.2.0 --changelog "Fixes + docs" clawhub skill publish ./org-skill --owner openclaw --version 1.2.0 --changelog "Org publish" clawhub package explore --family skill diff --git a/packages/clawhub/src/cli.ts b/packages/clawhub/src/cli.ts index 4cd6e140..1e6ddaf1 100644 --- a/packages/clawhub/src/cli.ts +++ b/packages/clawhub/src/cli.ts @@ -35,8 +35,10 @@ import { cmdExplore, cmdInstall, cmdList, + cmdPin, cmdSearch, cmdUninstall, + cmdUnpin, cmdUpdate, } from "./cli/commands/skills.js"; import { cmdStarSkill } from "./cli/commands/star.js"; @@ -243,6 +245,23 @@ registerCommand(program, ["list"]) await cmdList(opts); }); +registerCommand(program, ["pin"]) + .description("Pin an installed skill so update commands skip it") + .argument("", "Skill slug") + .option("--reason ", "Optional pin reason") + .action(async (slug, options) => { + const opts = await resolveGlobalOpts(); + await cmdPin(opts, slug, options); + }); + +registerCommand(program, ["unpin"]) + .description("Remove a skill pin so updates can change it again") + .argument("", "Skill slug") + .action(async (slug) => { + const opts = await resolveGlobalOpts(); + await cmdUnpin(opts, slug); + }); + registerCommand(program, ["explore"]) .description("Browse latest updated skills from the registry") .option( diff --git a/packages/clawhub/src/cli/commands/skills.test.ts b/packages/clawhub/src/cli/commands/skills.test.ts index 022ec025..29fa086d 100644 --- a/packages/clawhub/src/cli/commands/skills.test.ts +++ b/packages/clawhub/src/cli/commands/skills.test.ts @@ -68,13 +68,16 @@ const { clampLimit, cmdExplore, cmdInstall, + cmdList, cmdListSkillAppeals, cmdListSkillReports, + cmdPin, cmdReportSkill, cmdResolveSkillAppeal, cmdSearch, cmdTriageSkillReport, cmdUninstall, + cmdUnpin, cmdUpdate, formatExploreLine, } = await import("./skills.js"); @@ -440,6 +443,59 @@ describe("skill moderation commands", () => { }); describe("cmdUpdate", () => { + it("fails when directly updating a pinned skill", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: "0.1.0", installedAt: 123, pinned: true, pinReason: "hold" }, + }, + }); + + await expect(cmdUpdate(makeOpts(), "demo", { force: true }, false)).rejects.toThrow( + /is pinned/i, + ); + + expect(mockApiRequest).not.toHaveBeenCalled(); + expect(mockDownloadZip).not.toHaveBeenCalled(); + }); + + it("skips pinned skills during update --all and reports them in the summary", async () => { + mockApiRequest.mockResolvedValue({ + latestVersion: { version: "2.0.0" }, + moderation: null, + }); + mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: "0.1.0", installedAt: 123, pinned: true, pinReason: "hold" }, + other: { version: "1.0.0", installedAt: 456 }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + vi.mocked(readSkillOrigin).mockResolvedValue(null); + vi.mocked(writeSkillOrigin).mockResolvedValue(); + vi.mocked(extractZipToDir).mockResolvedValue(); + vi.mocked(listTextFiles).mockResolvedValue([]); + vi.mocked(hashSkillFiles).mockReturnValue({ fingerprint: "hash", files: [] }); + vi.mocked(stat).mockRejectedValue(new Error("missing")); + vi.mocked(rm).mockResolvedValue(); + + await cmdUpdate(makeOpts(), undefined, { all: true }, false); + + expect(mockApiRequest).toHaveBeenCalledTimes(1); + const [, args] = mockApiRequest.mock.calls[0] ?? []; + expect(args?.path).toBe(`${ApiRoutes.skills}/${encodeURIComponent("other")}`); + expect(writeLockfile).toHaveBeenCalledWith("/work", { + version: 1, + skills: { + demo: { version: "0.1.0", installedAt: 123, pinned: true, pinReason: "hold" }, + other: { version: "2.0.0", installedAt: expect.any(Number) }, + }, + }); + expect(mockLog).toHaveBeenCalledWith("Skipped 1 pinned skill: demo"); + }); + it("uses path-based skill lookup when no local fingerprint is available", async () => { mockApiRequest.mockResolvedValue({ latestVersion: { version: "1.0.0" } }); mockDownloadZip.mockResolvedValue(new Uint8Array([1, 2, 3])); @@ -464,6 +520,85 @@ describe("cmdUpdate", () => { }); }); +describe("pin commands", () => { + it("pins an installed skill and preserves its version metadata", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { demo: { version: "1.0.0", installedAt: 123 } }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + + await cmdPin(makeOpts(), "demo", { reason: "scanner hold" }); + + expect(writeLockfile).toHaveBeenCalledWith("/work", { + version: 1, + skills: { + demo: { + version: "1.0.0", + installedAt: 123, + pinned: true, + pinReason: "scanner hold", + }, + }, + }); + expect(mockLog).toHaveBeenCalledWith("Pinned demo: scanner hold"); + }); + + it("reports when an installed skill is already pinned without changes", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: "1.0.0", installedAt: 123, pinned: true, pinReason: "scanner hold" }, + }, + }); + + await cmdPin(makeOpts(), "demo"); + + expect(writeLockfile).not.toHaveBeenCalled(); + expect(mockLog).toHaveBeenCalledWith('Skill "demo" is already pinned: scanner hold'); + }); + + it("unpinned skills clear pin metadata and keep the installed version", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: "1.0.0", installedAt: 123, pinned: true, pinReason: "scanner hold" }, + }, + }); + vi.mocked(writeLockfile).mockResolvedValue(); + + await cmdUnpin(makeOpts(), "demo"); + + expect(writeLockfile).toHaveBeenCalledWith("/work", { + version: 1, + skills: { + demo: { + version: "1.0.0", + installedAt: 123, + }, + }, + }); + expect(mockLog).toHaveBeenCalledWith("Unpinned demo"); + }); +}); + +describe("cmdList", () => { + it("shows pinned state in list output", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { + demo: { version: "1.0.0", installedAt: 123, pinned: true, pinReason: "scanner hold" }, + other: { version: "2.0.0", installedAt: 456 }, + }, + }); + + await cmdList(makeOpts()); + + expect(mockLog).toHaveBeenCalledWith("demo 1.0.0 pinned (scanner hold)"); + expect(mockLog).toHaveBeenCalledWith("other 2.0.0"); + }); +}); + describe("cmdInstall", () => { it("passes optional auth token to API + download requests", async () => { mockGetOptionalAuthToken.mockResolvedValue("tkn"); @@ -497,6 +632,21 @@ describe("cmdInstall", () => { expect(zipArgs?.token).toBe("tkn"); }); + it("blocks force reinstall when a skill is pinned", async () => { + vi.mocked(readLockfile).mockResolvedValue({ + version: 1, + skills: { demo: { version: "0.9.0", installedAt: 123, pinned: true, pinReason: "hold" } }, + }); + vi.mocked(stat).mockRejectedValue(new Error("missing")); + + await expect(cmdInstall(makeOpts(), "demo", undefined, true)).rejects.toThrow(/is pinned/i); + + expect(mockApiRequest).not.toHaveBeenCalled(); + expect(mockDownloadZip).not.toHaveBeenCalled(); + expect(rm).not.toHaveBeenCalled(); + expect(writeLockfile).not.toHaveBeenCalled(); + }); + it("does not rm local directory when skill is malware-blocked (--force)", async () => { vi.mocked(stat).mockResolvedValue({} as unknown as Awaited>); // target exists mockApiRequest.mockResolvedValue({ diff --git a/packages/clawhub/src/cli/commands/skills.ts b/packages/clawhub/src/cli/commands/skills.ts index 8d23fae1..b5eca623 100644 --- a/packages/clawhub/src/cli/commands/skills.ts +++ b/packages/clawhub/src/cli/commands/skills.ts @@ -100,6 +100,27 @@ function isSafeSkillSlug(slug: string) { return Boolean(slug) && !slug.includes("/") && !slug.includes("\\") && !slug.includes(".."); } +function isPinnedSkillEntry(entry?: { pinned?: boolean | null }) { + return entry?.pinned === true; +} + +function withPinnedMetadata( + version: string | null, + installedAt: number, + existing?: { pinned?: boolean; pinReason?: string }, +) { + return { + version, + installedAt, + ...(existing?.pinned ? { pinned: true } : {}), + ...(existing?.pinned && existing.pinReason ? { pinReason: existing.pinReason } : {}), + }; +} + +function formatPinnedDetails(entry?: { pinReason?: string }) { + return entry?.pinReason ? ` (${entry.pinReason})` : ""; +} + export async function cmdSearch(opts: GlobalOpts, query: string, limit?: number) { if (!query) fail("Query required"); @@ -148,6 +169,12 @@ export async function cmdInstall( if (exists) fail(`Already installed: ${target} (use --force)`); } + const lock = await readLockfile(opts.workdir); + const existingEntry = lock.skills[trimmed]; + if (isPinnedSkillEntry(existingEntry)) { + fail(`skill "${trimmed}" is pinned; run \`clawhub unpin ${trimmed}\` first`); + } + const spinner = createSpinner(`Resolving ${trimmed}`); try { // Fetch skill metadata including moderation status @@ -212,11 +239,7 @@ export async function cmdInstall( installedAt: Date.now(), }); - const lock = await readLockfile(opts.workdir); - lock.skills[trimmed] = { - version: resolvedVersion, - installedAt: Date.now(), - }; + lock.skills[trimmed] = withPinnedMetadata(resolvedVersion, Date.now(), existingEntry); await writeLockfile(opts.workdir, lock); spinner.succeed(`OK. Installed ${trimmed} -> ${target}`); } catch (error) { @@ -237,14 +260,30 @@ export async function cmdUpdate( if (slug && all) fail("Use either or --all"); if (options.version && !slug) fail("--version requires a single "); if (options.version && !semver.valid(options.version)) fail("--version must be valid semver"); + const lock = await readLockfile(opts.workdir); + if (slug && isPinnedSkillEntry(lock.skills[slug])) { + fail(`skill "${slug}" is pinned; run \`clawhub unpin ${slug}\` first`); + } const allowPrompt = isInteractive() && inputAllowed; const token = await getOptionalAuthToken(); const registry = await getRegistry(opts, { cache: true }); - const lock = await readLockfile(opts.workdir); - const slugs = slug ? [slug] : Object.keys(lock.skills).filter(isSafeSkillSlug); + const requestedSlugs = slug ? [slug] : Object.keys(lock.skills).filter(isSafeSkillSlug); + const skippedPinned = slug + ? [] + : requestedSlugs.filter((entry) => isPinnedSkillEntry(lock.skills[entry])); + const slugs = slug + ? requestedSlugs + : requestedSlugs.filter((entry) => !isPinnedSkillEntry(lock.skills[entry])); if (slugs.length === 0) { + if (skippedPinned.length > 0) { + const suffix = skippedPinned.length === 1 ? "" : "s"; + console.log( + `Skipped ${skippedPinned.length} pinned skill${suffix}: ${skippedPinned.join(", ")}`, + ); + return; + } console.log("No installed skills."); return; } @@ -308,10 +347,11 @@ export async function cmdUpdate( const matched = resolveResult.match?.version ?? null; if (matched && lock.skills[entry]?.version !== matched) { - lock.skills[entry] = { - version: matched, - installedAt: lock.skills[entry]?.installedAt ?? Date.now(), - }; + lock.skills[entry] = withPinnedMetadata( + matched, + lock.skills[entry]?.installedAt ?? Date.now(), + lock.skills[entry], + ); } if (!latest) { @@ -364,7 +404,7 @@ export async function cmdUpdate( installedAt: existingOrigin?.installedAt ?? Date.now(), }); - lock.skills[entry] = { version: targetVersion, installedAt: Date.now() }; + lock.skills[entry] = withPinnedMetadata(targetVersion, Date.now(), lock.skills[entry]); spinner.succeed(`${entry}: updated -> ${targetVersion}`); } catch (error) { spinner.fail(formatError(error)); @@ -373,6 +413,12 @@ export async function cmdUpdate( } await writeLockfile(opts.workdir, lock); + if (skippedPinned.length > 0) { + const suffix = skippedPinned.length === 1 ? "" : "s"; + console.log( + `Skipped ${skippedPinned.length} pinned skill${suffix}: ${skippedPinned.join(", ")}`, + ); + } } export async function cmdList(opts: GlobalOpts) { @@ -384,7 +430,8 @@ export async function cmdList(opts: GlobalOpts) { return; } for (const [slug, entry] of entries) { - console.log(`${slug} ${entry.version ?? "latest"}`); + const pinned = isPinnedSkillEntry(entry) ? ` pinned${formatPinnedDetails(entry)}` : ""; + console.log(`${slug} ${entry.version ?? "latest"}${pinned}`); } if (manualSkills.length > 0) { if (entries.length > 0) console.log(); @@ -395,6 +442,42 @@ export async function cmdList(opts: GlobalOpts) { } } +export async function cmdPin(opts: GlobalOpts, slug: string, options: { reason?: string } = {}) { + const trimmed = normalizeSkillSlugOrFail(slug); + const lock = await readLockfile(opts.workdir); + const existing = lock.skills[trimmed]; + if (!existing) fail(`Not installed: ${trimmed}`); + + const reason = options.reason?.trim() || existing.pinReason; + if (isPinnedSkillEntry(existing) && reason === existing.pinReason) { + console.log(`Skill "${trimmed}" is already pinned${reason ? `: ${reason}` : ""}`); + return; + } + + lock.skills[trimmed] = { + ...existing, + pinned: true, + ...(reason ? { pinReason: reason } : {}), + }; + await writeLockfile(opts.workdir, lock); + console.log(`Pinned ${trimmed}${reason ? `: ${reason}` : ""}`); +} + +export async function cmdUnpin(opts: GlobalOpts, slug: string) { + const trimmed = normalizeSkillSlugOrFail(slug); + const lock = await readLockfile(opts.workdir); + const existing = lock.skills[trimmed]; + if (!existing) fail(`Not installed: ${trimmed}`); + if (!isPinnedSkillEntry(existing)) fail(`Skill "${trimmed}" is not pinned`); + + lock.skills[trimmed] = { + version: existing.version, + installedAt: existing.installedAt, + }; + await writeLockfile(opts.workdir, lock); + console.log(`Unpinned ${trimmed}`); +} + export async function cmdUninstall( opts: GlobalOpts, slug: string, diff --git a/packages/clawhub/src/schema/schemas.ts b/packages/clawhub/src/schema/schemas.ts index 56d4b02d..bb465640 100644 --- a/packages/clawhub/src/schema/schemas.ts +++ b/packages/clawhub/src/schema/schemas.ts @@ -23,6 +23,8 @@ export const LockfileSchema = type({ "[string]": { version: "string|null", installedAt: "number", + pinned: "boolean?", + pinReason: "string?", }, }, }); diff --git a/packages/clawhub/src/skills.test.ts b/packages/clawhub/src/skills.test.ts index b4abaa7d..52f90a47 100644 --- a/packages/clawhub/src/skills.test.ts +++ b/packages/clawhub/src/skills.test.ts @@ -39,10 +39,19 @@ describe("skills", () => { const workdir = await mkdtemp(join(tmpdir(), "clawhub-work-")); await writeLockfile(workdir, { version: 1, - skills: { demo: { version: "1.0.0", installedAt: 1 } }, + skills: { + demo: { + version: "1.0.0", + installedAt: 1, + pinned: true, + pinReason: "awaiting moderation review", + }, + }, }); const read = await readLockfile(workdir); expect(read.skills.demo?.version).toBe("1.0.0"); + expect(read.skills.demo?.pinned).toBe(true); + expect(read.skills.demo?.pinReason).toBe("awaiting moderation review"); }); it("returns empty lockfile on invalid json", async () => { diff --git a/packages/schema/src/schemas.test.ts b/packages/schema/src/schemas.test.ts index 7a084443..fa161725 100644 --- a/packages/schema/src/schemas.test.ts +++ b/packages/schema/src/schemas.test.ts @@ -16,10 +16,22 @@ describe("clawhub-schema", () => { it("parses lockfile records", () => { const lock = parseArk( LockfileSchema, - { version: 1, skills: { demo: { version: "1.0.0", installedAt: 123 } } }, + { + version: 1, + skills: { + demo: { + version: "1.0.0", + installedAt: 123, + pinned: true, + pinReason: "scanner-flagged", + }, + }, + }, "Lockfile", ); expect(lock.skills.demo?.version).toBe("1.0.0"); + expect(lock.skills.demo?.pinned).toBe(true); + expect(lock.skills.demo?.pinReason).toBe("scanner-flagged"); }); it("allows publish payload without tags", () => { diff --git a/packages/schema/src/schemas.ts b/packages/schema/src/schemas.ts index 47601d33..499a2a21 100644 --- a/packages/schema/src/schemas.ts +++ b/packages/schema/src/schemas.ts @@ -24,6 +24,8 @@ export const LockfileSchema = type({ "[string]": { version: "string|null", installedAt: "number", + pinned: "boolean?", + pinReason: "string?", }, }, });