From 77bd82c302dba5ba10b0bc7621ce78ab89d33070 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:28:41 +0800 Subject: [PATCH] feat(cli): add source-safe skill upgrades Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- cli/CHANGELOG.md | 10 + cli/README.md | 35 ++- cli/src/commands/help.ts | 9 + cli/src/commands/publish.ts | 4 +- cli/src/commands/upgrade.ts | 78 +++++ cli/src/index.ts | 18 ++ cli/src/services/install-service.ts | 58 +++- cli/src/services/installed-skill-metadata.ts | 69 +++++ cli/src/services/upgrade-service.ts | 287 ++++++++++++++++++ cli/test/helpers/fake-registry.ts | 17 +- cli/test/integration/publish-command.test.ts | 21 +- cli/test/integration/upgrade-command.test.ts | 224 ++++++++++++++ .../unit/services/install-service.test.ts | 28 +- docs/skillhub/en/guide/cli.md | 49 ++- 14 files changed, 884 insertions(+), 23 deletions(-) create mode 100644 cli/src/commands/upgrade.ts create mode 100644 cli/src/services/installed-skill-metadata.ts create mode 100644 cli/src/services/upgrade-service.ts create mode 100644 cli/test/integration/upgrade-command.test.ts diff --git a/cli/CHANGELOG.md b/cli/CHANGELOG.md index 8a378be0..6c52d037 100644 --- a/cli/CHANGELOG.md +++ b/cli/CHANGELOG.md @@ -4,8 +4,18 @@ All notable CLI behavior changes are documented in this file. ## Unreleased +### Added + +- Add `skillhub upgrade ` for bounded, explicit upgrades of already-installed Skills, + including side-effect-free `--check`, structured `--json`, local-change protection, and target + filters. + ### Fixed +- Prevent `--force` from overwriting an unmanaged or different-source Skill at the same target path. + Source ownership is the full `registry + namespace + slug` identity. +- Exclude installer-owned `.skillhub/` state when publishing a local Skill directory. + - Resolve `namespace/slug`, `@namespace/slug`, and `namespace--slug` coordinates against their declared namespace instead of silently falling back to `global`. diff --git a/cli/README.md b/cli/README.md index 11014547..e5a915c4 100644 --- a/cli/README.md +++ b/cli/README.md @@ -171,7 +171,7 @@ skillhub install pdf-parser --agent codex --agent claude-code # Install to custom directory skillhub install pdf-parser --dir ~/.claude/skills -# Force overwrite existing installation +# Reinstall a SkillHub-managed installation from the same source skillhub install pdf-parser --force ``` @@ -228,17 +228,44 @@ For a custom path or an unsupported Agent directory, use `--dir` to specify the ```json { + "schemaVersion": 1, "registry": "https://skill.xfyun.cn", "namespace": "global", "slug": "pdf-parser", "version": "1.0.0", + "versionId": 123, "fingerprint": "sha256:...", + "files": { "SKILL.md": "sha256..." }, "source": "skillhub", "agent": "codex", "installedAt": "2026-04-28T06:00:00.000Z" } ``` +The CLI creates `.skillhub/metadata.json` after extracting a downloaded package. It is not part of +the published ZIP and is excluded when a managed directory is published again. + +## ⬆️ Upgrade Installed Skills + +`upgrade` only operates on explicitly selected, SkillHub-managed local installations. It never +installs a missing Skill and has no implicit upgrade-all mode. + +```bash +# Preview without changing files +skillhub upgrade @global/skillhub-registry --check + +# Upgrade one or a bounded list of installed Skills +skillhub upgrade @global/skillhub-registry +skillhub upgrade @team/code-review @team/java-guide + +# Machine-readable plan +skillhub upgrade @team/code-review --check --json +``` + +The source identity is `registry + namespace + slug`. `--force` may replace local changes only when +that full identity matches the installation metadata; it never overwrites an unmanaged directory or +a Skill installed from another source. + ## 🔄 Namespace Workspaces Use namespace synchronization when an Agent workspace should maintain all installable skills from one team space. @@ -402,6 +429,7 @@ Update mechanism: | `skillhub whoami [--registry ] [--token ] [--json]` | Validate current token and display user information | | `skillhub search [--registry ] [--token ] [--limit ] [--json]` | Search published skills | | `skillhub install [--scope ] [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--registry ] [--token ] [--json]` | Install a skill | +| `skillhub upgrade [--namespace ] [--agent ] [--dir ] [--registry ] [--check] [--force] [--json]` | Upgrade explicitly selected installed skills | | `skillhub list [--agent ] [--dir ] [--registry ] [--json]` | List installed skills | | `skillhub remove [--agent ] [--all] [--remote] [--hard] [--namespace ] [--registry ] [--token ] [--json]` | Remove a skill | | `skillhub doctor [--json]` | Scan project directory and rebuild local inventory | @@ -447,13 +475,16 @@ skillhub search test --registry https://skillhub.example.com ```bash # Use --force to overwrite -skillhub install pdf-parser --force +skillhub install pdf-parser --force # same SkillHub source only # Or remove first then install skillhub remove pdf-parser skillhub install pdf-parser ``` +`--force` does not bypass source ownership. Move or explicitly remove an unmanaged or different-source +directory before installing another Skill with the same visible slug. + ### Corrupted Inventory ```bash diff --git a/cli/src/commands/help.ts b/cli/src/commands/help.ts index 37857834..7b225964 100644 --- a/cli/src/commands/help.ts +++ b/cli/src/commands/help.ts @@ -43,6 +43,15 @@ export const commands = { 'skillhub install pdf-parser --scope project --agent codex' ] }, + upgrade: { + summary: 'Upgrade explicitly selected installed skills', + usage: 'skillhub upgrade [--namespace ] [--agent ] [--dir ] [--registry ] [--check] [--force] [--json]', + examples: [ + 'skillhub upgrade @global/skillhub-registry', + 'skillhub upgrade @team/code-review @team/java-guide --check --json', + 'skillhub upgrade code-review --namespace team --agent codex' + ] + }, sync: { summary: 'Synchronize and maintain namespace workspaces', usage: 'skillhub sync [options]', diff --git a/cli/src/commands/publish.ts b/cli/src/commands/publish.ts index 1c73202f..6f9c5dbf 100644 --- a/cli/src/commands/publish.ts +++ b/cli/src/commands/publish.ts @@ -49,7 +49,9 @@ export async function publishCommand(path: string, options: PublishCommandOption throw new CliError(`file must be a zip archive: ${path}`, EXIT.filesystem, { path }) } } else if (pathStat.isDirectory()) { - archiveBlob = await createZip(path) + archiveBlob = await createZip(path, { + exclude: relativePath => relativePath === '.skillhub' || relativePath.startsWith('.skillhub/') + }) archiveName = `${basename(path)}.zip` } else { throw new CliError(`path must be a file or directory: ${path}`, EXIT.filesystem, { path }) diff --git a/cli/src/commands/upgrade.ts b/cli/src/commands/upgrade.ts new file mode 100644 index 00000000..934d0934 --- /dev/null +++ b/cli/src/commands/upgrade.ts @@ -0,0 +1,78 @@ +import { CredentialsStore } from '../stores/credentials-store' +import { resolveToken } from '../services/registry-service' +import { CliError } from '../shared/errors' +import { EXIT } from '../shared/constants' +import { executeSkillUpgradePlan, planSkillUpgrades, type UpgradePlan } from '../services/upgrade-service' + +export interface UpgradeCommandOptions { + namespace?: string + agent?: string[] + dir?: string + registry?: string + token?: string + check?: boolean + force?: boolean + json?: boolean +} + +export async function upgradeCommand(coordinates: string[], options: UpgradeCommandOptions): Promise { + const credentials = new CredentialsStore() + const tokenForRegistry = async (registry: string): Promise => + resolveToken(options, process.env, await credentials.getToken(registry)) + + const plan = await planSkillUpgrades({ + coordinates, + namespace: options.namespace, + registry: options.registry, + agents: options.agent, + dir: options.dir, + force: Boolean(options.force), + tokenForRegistry + }) + const output = renderUpgradePlan(plan, Boolean(options.check), Boolean(options.json)) + + if (plan.blocked > 0) { + process.stdout.write(`${output}\n`) + throw new CliError('upgrade plan contains blocked skills', EXIT.validation, { + blocked: plan.items.filter(item => item.action === 'blocked').map(item => ({ + coordinate: item.coordinate, + reason: item.reason + })) + }) + } + if (!options.check) { + await executeSkillUpgradePlan(plan, { force: Boolean(options.force), tokenForRegistry }) + } + return output +} + +function renderUpgradePlan(plan: UpgradePlan, check: boolean, json: boolean): string { + if (json) { + return JSON.stringify({ + ok: plan.blocked === 0, + check, + summary: { upgrades: plan.upgrades, unchanged: plan.unchanged, blocked: plan.blocked }, + items: plan.items.map(item => ({ + coordinate: item.coordinate, + registry: item.registry, + currentVersion: item.currentVersion, + remoteVersion: item.remoteVersion, + action: item.action === 'upgrade' && !check ? 'upgraded' : item.action, + reason: item.reason, + changedFiles: item.changedFiles, + targets: item.targets + })) + }) + } + + const heading = check ? 'Upgrade plan' : 'Upgrade result' + return [ + `${heading}: ${plan.upgrades} upgrade, ${plan.unchanged} unchanged, ${plan.blocked} blocked`, + ...plan.items.map(item => { + const action = item.action === 'upgrade' && !check ? 'upgraded' : item.action + const versions = item.remoteVersion ? ` ${item.currentVersion} -> ${item.remoteVersion}` : '' + const reason = item.reason ? ` (${item.reason})` : '' + return `${action.padEnd(10)} ${item.coordinate}${versions}${reason}` + }) + ].join('\n') +} diff --git a/cli/src/index.ts b/cli/src/index.ts index d743d46a..4328bc5b 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -11,6 +11,7 @@ import { removeCommand, type RemoveCommandOptions } from './commands/remove' import { searchCommand } from './commands/search' import { syncDiffCommand, syncPullCommand, syncPushCommand, syncStatusCommand, type SyncCommonOptions, type SyncPullOptions, type SyncPushOptions } from './commands/sync' import { updateCommand } from './commands/update' +import { upgradeCommand, type UpgradeCommandOptions } from './commands/upgrade' import { versionCommand } from './commands/version' import { whoamiCommand } from './commands/whoami' import { EXIT } from './shared/constants' @@ -247,6 +248,23 @@ cli return runCommand(() => installCommand(slug, { ...options, agent: toArray(options.agent) }), Boolean(options.json)) }) +cli + .command('upgrade [...coordinates]', 'Upgrade explicitly selected installed skills') + .option('--namespace ', 'Filter a bare slug by namespace') + .option('--agent ', 'Filter installed targets by Agent (repeatable)') + .option('--dir ', 'Filter installed targets by directory') + .option('--registry ', 'Filter by installation source registry') + .option('--token ', 'API token override') + .option('--check', 'Show the exact plan without writing') + .option('--force', 'Replace local changes from the same source') + .option('--json', 'Output JSON') + .action((coordinates: string[], options: UpgradeCommandOptions & { agent?: string | string[] }) => { + return runCommand( + () => upgradeCommand(coordinates, { ...options, agent: toArray(options.agent) }), + Boolean(options.json) + ) + }) + cli .command('sync [path]', 'Synchronize and maintain a namespace workspace') .option('--namespace ', 'Namespace', { default: 'global' }) diff --git a/cli/src/services/install-service.ts b/cli/src/services/install-service.ts index 36ba1f4b..e53b0809 100644 --- a/cli/src/services/install-service.ts +++ b/cli/src/services/install-service.ts @@ -8,8 +8,14 @@ import { extractZip } from '../platform/archive' import { readBoundedResponseBody } from '../platform/download' import { canonicalizeExistingPath, pathExists } from '../platform/paths' import { snapshotSkillDirectory } from './skill-fingerprint' +import { + readInstalledSkillMetadata, + sameInstalledSkillSource, + type InstalledSkillIdentity +} from './installed-skill-metadata' import type { AgentCandidate } from '../agents/types' import type { ResolveResponse } from '../clients/skillhub-client' +import type { Inventory } from '../stores/inventory-store' export interface InstallOptions { registry: string @@ -25,15 +31,16 @@ export interface InstallOptions { async function preflightInstallTargets( targets: AgentCandidate[], - slug: string, - force: boolean + identity: InstalledSkillIdentity, + force: boolean, + inventory: Inventory ): Promise> { const seenSkillDirs = new Set() const preparedTargets: Array<{ target: AgentCandidate; skillDir: string }> = [] for (const target of targets) { const canonicalRootDir = await canonicalizeExistingPath(target.rootDir) - const canonicalSkillDir = join(canonicalRootDir, slug) + const canonicalSkillDir = join(canonicalRootDir, identity.slug) if (seenSkillDirs.has(canonicalSkillDir)) { throw new CliError(`multiple install targets resolve to ${canonicalSkillDir}`, EXIT.usage, { path: canonicalSkillDir, @@ -42,13 +49,40 @@ async function preflightInstallTargets( } seenSkillDirs.add(canonicalSkillDir) - const skillDir = join(target.rootDir, slug) - if (await pathExists(skillDir) && !force) { + const skillDir = join(target.rootDir, identity.slug) + const exists = await pathExists(skillDir) + if (exists && !force) { throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, { path: skillDir, - next: 'pass --force to overwrite' + next: 'pass --force to replace a same-source installation' }) } + if (exists) { + const metadataResult = await readInstalledSkillMetadata(skillDir) + const inventoryOwners = inventory.items.filter(item => + item.targets.some(existing => existing.installDir === skillDir)) + + if (metadataResult.status !== 'valid') { + throw new CliError(`cannot verify SkillHub ownership of ${skillDir}`, EXIT.filesystem, { + path: skillDir, + reason: metadataResult.status === 'missing' ? 'installation metadata is missing' : metadataResult.reason, + next: 'move or remove the existing directory before installing' + }) + } + if (!sameInstalledSkillSource(metadataResult.metadata, identity) || + inventoryOwners.some(owner => !sameInstalledSkillSource(owner, identity))) { + throw new CliError(`source conflict at ${skillDir}`, EXIT.filesystem, { + path: skillDir, + expected: identity, + actual: { + registry: metadataResult.metadata.registry, + namespace: metadataResult.metadata.namespace, + slug: metadataResult.metadata.slug + }, + next: 'choose another target directory or remove the conflicting skill explicitly' + }) + } + } preparedTargets.push({ target, skillDir }) } @@ -56,15 +90,19 @@ async function preflightInstallTargets( } export async function installSkill(options: InstallOptions): Promise<{ installed: Array<{ agent: string; dir: string }> }> { - const preparedTargets = await preflightInstallTargets(options.targets, options.slug, options.force) + const store = new InventoryStore(options.home) + const inventory = await store.read() + const preparedTargets = await preflightInstallTargets(options.targets, { + registry: options.registry, + namespace: options.namespace, + slug: options.slug + }, options.force, inventory) const client = new SkillHubClient(options.registry, options.token) const resolved = options.resolved ?? await client.resolve(options.namespace, options.slug, options.version) const response = await client.download(options.namespace, options.slug, resolved.version) const buffer = await readBoundedResponseBody(response) const installed: Array<{ agent: string; dir: string }> = [] - const store = new InventoryStore(options.home) - for (const { target, skillDir } of preparedTargets) { await mkdir(target.rootDir, { recursive: true }) const tempDir = await mkdtemp(join(target.rootDir, `.${options.slug}.install-`)) @@ -78,10 +116,12 @@ export async function installSkill(options: InstallOptions): Promise<{ installed const metaDir = join(tempDir, '.skillhub') await mkdir(metaDir, { recursive: true }) await writeFile(join(metaDir, 'metadata.json'), JSON.stringify({ + schemaVersion: 1, registry: options.registry, namespace: options.namespace, slug: options.slug, version: resolved.version, + versionId: resolved.versionId, fingerprint: resolved.fingerprint, files: snapshot.files, source: 'skillhub', diff --git a/cli/src/services/installed-skill-metadata.ts b/cli/src/services/installed-skill-metadata.ts new file mode 100644 index 00000000..618e4326 --- /dev/null +++ b/cli/src/services/installed-skill-metadata.ts @@ -0,0 +1,69 @@ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { pathExists } from '../platform/paths' + +export interface InstalledSkillIdentity { + registry: string + namespace: string + slug: string +} + +export interface InstalledSkillMetadata extends InstalledSkillIdentity { + schemaVersion?: number + version: string + versionId?: number + fingerprint?: string + files?: Record + source?: string + agent?: string + installedAt?: string +} + +export type InstalledMetadataReadResult = + | { status: 'missing' } + | { status: 'invalid'; reason: string } + | { status: 'valid'; metadata: InstalledSkillMetadata } + +export async function readInstalledSkillMetadata(skillDir: string): Promise { + const metadataPath = join(skillDir, '.skillhub', 'metadata.json') + if (!(await pathExists(metadataPath))) return { status: 'missing' } + + try { + const value = JSON.parse(await readFile(metadataPath, 'utf-8')) as unknown + if (!isRecord(value)) return { status: 'invalid', reason: 'metadata root must be an object' } + + for (const field of ['registry', 'namespace', 'slug', 'version'] as const) { + if (typeof value[field] !== 'string' || value[field].length === 0) { + return { status: 'invalid', reason: `metadata field "${field}" must be a non-empty string` } + } + } + if (value.files !== undefined && !isStringRecord(value.files)) { + return { status: 'invalid', reason: 'metadata field "files" must map paths to hashes' } + } + + return { status: 'valid', metadata: value as unknown as InstalledSkillMetadata } + } catch { + return { status: 'invalid', reason: 'metadata is not valid JSON' } + } +} + +export function sameInstalledSkillSource( + left: InstalledSkillIdentity, + right: InstalledSkillIdentity +): boolean { + return normalizeRegistry(left.registry) === normalizeRegistry(right.registry) && + left.namespace === right.namespace && + left.slug === right.slug +} + +function normalizeRegistry(registry: string): string { + return registry.replace(/\/+$/, '') +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function isStringRecord(value: unknown): value is Record { + return isRecord(value) && Object.values(value).every(entry => typeof entry === 'string') +} diff --git a/cli/src/services/upgrade-service.ts b/cli/src/services/upgrade-service.ts new file mode 100644 index 00000000..9f1a3a54 --- /dev/null +++ b/cli/src/services/upgrade-service.ts @@ -0,0 +1,287 @@ +import { relative, resolve } from 'node:path' +import { pathExists } from '../platform/paths' +import { SkillHubClient, type ResolveResponse } from '../clients/skillhub-client' +import { InventoryStore, type InventoryItem, type InventoryTarget } from '../stores/inventory-store' +import { CliError } from '../shared/errors' +import { EXIT } from '../shared/constants' +import { hasExplicitNamespace, parseSkillName, resolveSkillName } from '../shared/skill-name-parser' +import { diffSkillFiles, snapshotSkillDirectory } from './skill-fingerprint' +import { installSkill } from './install-service' +import { readInstalledSkillMetadata, sameInstalledSkillSource } from './installed-skill-metadata' + +const MAX_UPGRADE_SELECTION = 50 + +export interface UpgradeSelectionOptions { + coordinates: string[] + namespace?: string + registry?: string + agents?: string[] + dir?: string + force: boolean + home?: string + tokenForRegistry: (registry: string) => Promise +} + +export type UpgradePlanAction = 'upgrade' | 'unchanged' | 'blocked' + +export interface UpgradePlanItem { + coordinate: string + registry: string + currentVersion: string + remoteVersion?: string + action: UpgradePlanAction + reason?: string + changedFiles: string[] + targets: Array<{ agent: string; dir: string }> + resolved?: ResolveResponse + inventoryItem: InventoryItem + selectedTargets: InventoryTarget[] +} + +export interface UpgradePlan { + items: UpgradePlanItem[] + blocked: number + upgrades: number + unchanged: number +} + +export async function planSkillUpgrades(options: UpgradeSelectionOptions): Promise { + if (options.coordinates.length === 0) { + throw new CliError('provide at least one installed skill coordinate', EXIT.usage) + } + if (options.coordinates.length > MAX_UPGRADE_SELECTION) { + throw new CliError(`upgrade accepts at most ${MAX_UPGRADE_SELECTION} coordinates`, EXIT.usage) + } + + const store = new InventoryStore(options.home) + const inventory = await store.read() + const selected = selectInventoryItems(inventory.items, options) + const items: UpgradePlanItem[] = [] + + for (const selection of selected) { + const coordinate = `@${selection.item.namespace}/${selection.item.slug}` + const base = { + coordinate, + registry: selection.item.registry, + currentVersion: selection.item.version, + changedFiles: [] as string[], + targets: selection.targets.map(target => ({ agent: target.agent, dir: target.installDir })), + inventoryItem: selection.item, + selectedTargets: selection.targets + } + + let resolved: ResolveResponse + try { + const token = await options.tokenForRegistry(selection.item.registry) + resolved = await new SkillHubClient(selection.item.registry, token) + .resolve(selection.item.namespace, selection.item.slug) + } catch (error) { + items.push({ + ...base, + action: 'blocked', + reason: error instanceof Error ? error.message : 'remote version unavailable' + }) + continue + } + + if (resolved.namespace !== selection.item.namespace || resolved.slug !== selection.item.slug) { + items.push({ + ...base, + remoteVersion: resolved.version, + action: 'blocked', + reason: 'registry resolved a different skill identity' + }) + continue + } + + const inspection = await inspectTargets(selection.item, selection.targets) + const hardConflict = inspection.hardConflicts[0] + if (hardConflict) { + items.push({ + ...base, + remoteVersion: resolved.version, + action: 'blocked', + reason: hardConflict, + changedFiles: inspection.changedFiles, + resolved + }) + continue + } + if (inspection.changedFiles.length > 0 && !options.force) { + items.push({ + ...base, + remoteVersion: resolved.version, + action: 'blocked', + reason: 'local changes detected; pass --force to replace same-source files', + changedFiles: inspection.changedFiles, + resolved + }) + continue + } + if (inspection.baselineMissing && !options.force) { + items.push({ + ...base, + remoteVersion: resolved.version, + action: 'blocked', + reason: 'installed metadata has no file baseline; pass --force to migrate this same-source installation', + resolved + }) + continue + } + + const unchanged = selection.item.version === resolved.version && + selection.item.fingerprint === resolved.fingerprint && + inspection.metadataCurrent + items.push({ + ...base, + remoteVersion: resolved.version, + action: unchanged ? 'unchanged' : 'upgrade', + changedFiles: inspection.changedFiles, + resolved + }) + } + + return { + items, + blocked: items.filter(item => item.action === 'blocked').length, + upgrades: items.filter(item => item.action === 'upgrade').length, + unchanged: items.filter(item => item.action === 'unchanged').length + } +} + +export async function executeSkillUpgradePlan( + plan: UpgradePlan, + options: Pick +): Promise { + if (plan.blocked > 0) { + throw new CliError('upgrade plan contains blocked skills', EXIT.validation) + } + + for (const item of plan.items) { + if (item.action !== 'upgrade' || !item.resolved) continue + const token = await options.tokenForRegistry(item.registry) + await installSkill({ + registry: item.registry, + token, + namespace: item.inventoryItem.namespace, + slug: item.inventoryItem.slug, + resolved: item.resolved, + targets: item.selectedTargets.map(target => ({ + agent: target.agent, + rootDir: target.rootDir, + scope: 'project', + source: 'explicit' + })), + force: true, + home: options.home + }) + } +} + +function selectInventoryItems( + items: InventoryItem[], + options: Pick +): Array<{ item: InventoryItem; targets: InventoryTarget[] }> { + const selected = new Map() + + for (const coordinate of options.coordinates) { + const explicitNamespace = hasExplicitNamespace(coordinate) + const parsed = explicitNamespace + ? resolveSkillName(coordinate, options.namespace) + : parseSkillName(coordinate) + const namespace = explicitNamespace ? parsed.namespace : options.namespace + + const matches = items.flatMap(item => { + if (item.slug !== parsed.slug) return [] + if (namespace && item.namespace !== namespace) return [] + if (options.registry && normalizeRegistry(item.registry) !== normalizeRegistry(options.registry)) return [] + const targets = item.targets.filter(target => matchesTargetFilters(target, options.agents, options.dir)) + return targets.length > 0 ? [{ item, targets }] : [] + }) + + if (matches.length === 0) { + throw new CliError(`skill "${coordinate}" is not installed`, EXIT.usage, { + next: `use skillhub install ${coordinate}` + }) + } + if (matches.length > 1) { + throw new CliError(`installed skill "${coordinate}" is ambiguous`, EXIT.usage, { + matches: matches.map(match => `${match.item.registry} @${match.item.namespace}/${match.item.slug}`), + next: 'use a full coordinate and --registry to select one installation source' + }) + } + + const match = matches[0]! + const key = `${normalizeRegistry(match.item.registry)}\u0000${match.item.namespace}\u0000${match.item.slug}` + selected.set(key, match) + } + + if (selected.size > MAX_UPGRADE_SELECTION) { + throw new CliError(`upgrade resolves to at most ${MAX_UPGRADE_SELECTION} skills`, EXIT.usage) + } + return [...selected.values()] +} + +async function inspectTargets(item: InventoryItem, targets: InventoryTarget[]): Promise<{ + hardConflicts: string[] + changedFiles: string[] + baselineMissing: boolean + metadataCurrent: boolean +}> { + const hardConflicts: string[] = [] + const changedFiles = new Set() + let baselineMissing = false + let metadataCurrent = true + + for (const target of targets) { + if (!(await pathExists(target.installDir))) { + hardConflicts.push(`installed target is missing: ${target.installDir}`) + continue + } + const result = await readInstalledSkillMetadata(target.installDir) + if (result.status !== 'valid') { + hardConflicts.push(`metadata-invalid at ${target.installDir}: ${result.status === 'missing' ? 'missing' : result.reason}`) + continue + } + if (!sameInstalledSkillSource(result.metadata, item)) { + hardConflicts.push(`source-conflict at ${target.installDir}`) + continue + } + if (!result.metadata.files) { + baselineMissing = true + } else { + const snapshot = await snapshotSkillDirectory(target.installDir) + for (const path of diffSkillFiles(result.metadata.files, snapshot.files)) { + changedFiles.add(`${target.installDir}:${path}`) + } + } + if (result.metadata.version !== item.version || result.metadata.fingerprint !== item.fingerprint) { + metadataCurrent = false + } + } + + return { + hardConflicts, + changedFiles: [...changedFiles].sort(), + baselineMissing, + metadataCurrent + } +} + +function matchesTargetFilters(target: InventoryTarget, agents?: string[], dir?: string): boolean { + if (agents?.length && !agents.includes(target.agent)) return false + if (!dir) return true + const filterPath = resolve(dir) + const installPath = resolve(target.installDir) + const rootPath = resolve(target.rootDir) + return isSameOrWithin(filterPath, installPath) || isSameOrWithin(filterPath, rootPath) +} + +function isSameOrWithin(parent: string, candidate: string): boolean { + const rel = relative(parent, candidate) + return rel === '' || (!rel.startsWith('..') && !rel.startsWith('/') && !rel.startsWith('\\')) +} + +function normalizeRegistry(registry: string): string { + return registry.replace(/\/+$/, '') +} diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index 6a3b7bce..be3b9eec 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -1,3 +1,5 @@ +import { unzipSync } from 'fflate' + type FakeHandler = (req: Request) => Response | Promise export function createFakeRegistry(handlers: Record) { @@ -103,6 +105,7 @@ export interface CapturedPublish { /** Visibility string from the multipart form field. */ visibility: string rejectExistingVersion: boolean + archiveEntries: string[] } export interface CapturedValidate { @@ -203,7 +206,8 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { delete: CapturedDelete | null validate: CapturedValidate | null review: CapturedReview | null - } = { publish: null, resolve: null, delete: null, validate: null, review: null } + downloads: number + } = { publish: null, resolve: null, delete: null, validate: null, review: null, downloads: 0 } // If any endpoint is configured with 'network' failure mode, we need a real // TCP-level failure. Start a connection-dropping server and return its URL @@ -345,6 +349,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { if (!skill) { return Response.json({ code: 404, message: 'not found' }, { status: 404 }) } + state.downloads += 1 const bytes = skill.zipBytes ?? MINIMAL_ZIP return new Response(bytes as BodyInit, { status: 200, @@ -364,6 +369,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { if (!skill) { return Response.json({ code: 404, message: 'not found' }, { status: 404 }) } + state.downloads += 1 const bytes = skill.zipBytes ?? MINIMAL_ZIP return new Response(bytes as BodyInit, { status: 200, @@ -415,6 +421,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { if (fileField instanceof File) { fileName = fileField.name || fileName } + state.validate = { namespace, fileName, @@ -442,7 +449,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { const namespace = publishMatch[1]! // Parse multipart form data asynchronously — return a Promise. - return req.formData().then(form => { + return req.formData().then(async form => { const fileField = form.get('file') const visibility = (form.get('visibility') as string | null) ?? 'PUBLIC' @@ -451,13 +458,17 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { if (fileField instanceof File) { fileName = fileField.name || fileName } + const archiveEntries = fileField instanceof File + ? Object.keys(unzipSync(new Uint8Array(await fileField.arrayBuffer()))).sort() + : [] // Record for test assertions. state.publish = { namespace, fileName, visibility, - rejectExistingVersion: form.get('rejectExistingVersion') === 'true' + rejectExistingVersion: form.get('rejectExistingVersion') === 'true', + archiveEntries } return Response.json({ diff --git a/cli/test/integration/publish-command.test.ts b/cli/test/integration/publish-command.test.ts index 560e2f33..c6f9f25a 100644 --- a/cli/test/integration/publish-command.test.ts +++ b/cli/test/integration/publish-command.test.ts @@ -1,4 +1,4 @@ -import { mkdtemp, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, test } from 'bun:test' @@ -117,6 +117,25 @@ describe('publish command — P0', () => { expect(registry.received.publish!.visibility).toBe('PUBLIC') }) + test('directory publish excludes local SkillHub installation metadata', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '# Demo']) + await mkdir(join(dir, '.skillhub'), { recursive: true }) + await writeFile(join(dir, '.skillhub', 'metadata.json'), '{"registry":"private"}') + + const result = await runCli(['publish', dir, '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(0) + expect(registry.received.publish!.archiveEntries).toContain('SKILL.md') + expect(registry.received.publish!.archiveEntries.some(path => path.startsWith('.skillhub'))).toBe(false) + }) + test('zip file happy path: exit 0, fileName matches passed file', async () => { const env = await createTempHome() registry = await startFakeRegistry({ token: 'sk_ok' }) diff --git a/cli/test/integration/upgrade-command.test.ts b/cli/test/integration/upgrade-command.test.ts new file mode 100644 index 00000000..9f5e9e85 --- /dev/null +++ b/cli/test/integration/upgrade-command.test.ts @@ -0,0 +1,224 @@ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { strToU8, zipSync } from 'fflate' +import { createTempHome } from '../helpers/temp-env' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' + +let registries: Array>> = [] + +afterEach(() => { + for (const registry of registries) registry.stop() + registries = [] +}) + +function makeSkillZip(content: string): Uint8Array { + return zipSync({ 'SKILL.md': strToU8(content) }) +} + +describe('upgrade command', () => { + test('check is side-effect free and execute upgrades an installed skill', async () => { + const env = await createTempHome() + const skill = { + namespace: 'global', + slug: 'skillhub-registry', + version: '1.0.0', + versionId: 1, + fingerprint: 'fp-v1', + zipBytes: makeSkillZip('# v1') + } + const registry = await startFakeRegistry({ skills: [skill] }) + registries.push(registry) + const rootDir = join(env.cwd, 'skills') + await mkdir(rootDir, { recursive: true }) + + const installed = await runCli([ + 'install', '@global/skillhub-registry', '--dir', rootDir, '--registry', registry.url + ], { HOME: env.home, USERPROFILE: env.home }) + expect(installed.exitCode).toBe(0) + + skill.version = '1.1.0' + skill.versionId = 2 + skill.fingerprint = 'fp-v2' + skill.zipBytes = makeSkillZip('# v2') + + const checked = await runCli([ + 'upgrade', '@global/skillhub-registry', '--registry', registry.url, '--check', '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(checked.exitCode).toBe(0) + expect(JSON.parse(checked.stdout).items[0]).toMatchObject({ + coordinate: '@global/skillhub-registry', + currentVersion: '1.0.0', + remoteVersion: '1.1.0', + action: 'upgrade' + }) + expect(await readFile(join(rootDir, 'skillhub-registry', 'SKILL.md'), 'utf-8')).toBe('# v1') + expect(registry.received.downloads).toBe(1) + + const upgraded = await runCli([ + 'upgrade', '@global/skillhub-registry', '--registry', registry.url, '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(upgraded.exitCode).toBe(0) + expect(JSON.parse(upgraded.stdout).items[0].action).toBe('upgraded') + expect(await readFile(join(rootDir, 'skillhub-registry', 'SKILL.md'), 'utf-8')).toBe('# v2') + expect(registry.received.downloads).toBe(2) + + const metadata = JSON.parse(await readFile( + join(rootDir, 'skillhub-registry', '.skillhub', 'metadata.json'), + 'utf-8' + )) + expect(metadata).toMatchObject({ schemaVersion: 1, version: '1.1.0', versionId: 2, fingerprint: 'fp-v2' }) + }) + + test('local changes block by default and --force replaces only the same source', async () => { + const env = await createTempHome() + const skill = { + namespace: 'team', + slug: 'code-review', + version: '1.0.0', + fingerprint: 'fp-v1', + zipBytes: makeSkillZip('# v1') + } + const registry = await startFakeRegistry({ skills: [skill] }) + registries.push(registry) + const rootDir = join(env.cwd, 'skills') + await mkdir(rootDir, { recursive: true }) + await runCli(['install', '@team/code-review', '--dir', rootDir, '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + await writeFile(join(rootDir, 'code-review', 'SKILL.md'), '# locally edited') + skill.version = '1.1.0' + skill.fingerprint = 'fp-v2' + skill.zipBytes = makeSkillZip('# v2') + + const blocked = await runCli([ + 'upgrade', '@team/code-review', '--registry', registry.url, '--check', '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(blocked.exitCode).toBe(6) + expect(JSON.parse(blocked.stdout).items[0]).toMatchObject({ action: 'blocked' }) + expect(JSON.parse(blocked.stdout).items[0].reason).toContain('local changes') + expect(await readFile(join(rootDir, 'code-review', 'SKILL.md'), 'utf-8')).toBe('# locally edited') + + const forced = await runCli([ + 'upgrade', '@team/code-review', '--registry', registry.url, '--force', '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(forced.exitCode).toBe(0) + expect(await readFile(join(rootDir, 'code-review', 'SKILL.md'), 'utf-8')).toBe('# v2') + }) + + test('source conflict is a hard block even with --force', async () => { + const env = await createTempHome() + const skill = { + namespace: 'global', + slug: 'demo', + version: '1.0.0', + fingerprint: 'fp-v1', + zipBytes: makeSkillZip('# v1') + } + const registry = await startFakeRegistry({ skills: [skill] }) + registries.push(registry) + const rootDir = join(env.cwd, 'skills') + await mkdir(rootDir, { recursive: true }) + await runCli(['install', '@global/demo', '--dir', rootDir, '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + const metadataPath = join(rootDir, 'demo', '.skillhub', 'metadata.json') + const metadata = JSON.parse(await readFile(metadataPath, 'utf-8')) + metadata.namespace = 'another-team' + await writeFile(metadataPath, JSON.stringify(metadata)) + skill.version = '2.0.0' + skill.fingerprint = 'fp-v2' + + const result = await runCli([ + 'upgrade', '@global/demo', '--registry', registry.url, '--force', '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(result.exitCode).toBe(6) + expect(JSON.parse(result.stdout).items[0].reason).toContain('source-conflict') + expect(await readFile(join(rootDir, 'demo', 'SKILL.md'), 'utf-8')).toBe('# v1') + }) + + test('a bare slug must identify exactly one installed source', async () => { + const env = await createTempHome() + const skillA = { namespace: 'team-a', slug: 'demo', version: '1.0.0', fingerprint: 'a', zipBytes: makeSkillZip('# A') } + const skillB = { namespace: 'team-b', slug: 'demo', version: '1.0.0', fingerprint: 'b', zipBytes: makeSkillZip('# B') } + const registryA = await startFakeRegistry({ skills: [skillA] }) + const registryB = await startFakeRegistry({ skills: [skillB] }) + registries.push(registryA, registryB) + const rootA = join(env.cwd, 'a') + const rootB = join(env.cwd, 'b') + await mkdir(rootA, { recursive: true }) + await mkdir(rootB, { recursive: true }) + await runCli(['install', '@team-a/demo', '--dir', rootA, '--registry', registryA.url], { + HOME: env.home, + USERPROFILE: env.home + }) + await runCli(['install', '@team-b/demo', '--dir', rootB, '--registry', registryB.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + const ambiguous = await runCli(['upgrade', 'demo', '--check'], { + HOME: env.home, + USERPROFILE: env.home + }) + expect(ambiguous.exitCode).toBe(5) + expect(ambiguous.stderr).toContain('ambiguous') + + const selected = await runCli(['upgrade', 'demo', '--namespace', 'team-a', '--registry', registryA.url, '--check'], { + HOME: env.home, + USERPROFILE: env.home + }) + expect(selected.exitCode).toBe(0) + expect(selected.stdout).toContain('@team-a/demo') + }) + + test('one resolved archive is reused for every managed target', async () => { + const env = await createTempHome() + const skill = { + namespace: 'global', + slug: 'shared', + version: '1.0.0', + fingerprint: 'fp-v1', + zipBytes: makeSkillZip('# v1') + } + const registry = await startFakeRegistry({ skills: [skill] }) + registries.push(registry) + + const installed = await runCli([ + 'install', '@global/shared', '--agent', 'codex', '--agent', 'claude-code', '--registry', registry.url + ], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + expect(installed.exitCode).toBe(0) + expect(registry.received.downloads).toBe(1) + + skill.version = '1.1.0' + skill.fingerprint = 'fp-v2' + skill.zipBytes = makeSkillZip('# v2') + const upgraded = await runCli(['upgrade', '@global/shared', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }, { cwd: env.cwd }) + expect(upgraded.exitCode).toBe(0) + expect(registry.received.downloads).toBe(2) + expect(await readFile(join(env.cwd, '.codex', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') + expect(await readFile(join(env.cwd, '.claude', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') + }) + + test('never installs a missing skill and never offers an implicit upgrade-all', async () => { + const env = await createTempHome() + const missing = await runCli(['upgrade', '@global/missing', '--check'], { + HOME: env.home, + USERPROFILE: env.home + }) + expect(missing.exitCode).toBe(5) + expect(missing.stderr).toContain('use skillhub install') + + const empty = await runCli(['upgrade'], { HOME: env.home, USERPROFILE: env.home }) + expect(empty.exitCode).toBe(5) + expect(empty.stderr).toContain('at least one') + }) +}) diff --git a/cli/test/unit/services/install-service.test.ts b/cli/test/unit/services/install-service.test.ts index 95525d5f..afa4579d 100644 --- a/cli/test/unit/services/install-service.test.ts +++ b/cli/test/unit/services/install-service.test.ts @@ -51,6 +51,22 @@ function installFetchWithDownloadResponse(downloadResponse: Response): typeof fe return fakeFetch as unknown as typeof fetch } +async function writeManagedMetadata( + skillDir: string, + identity: { registry: string; namespace: string; slug: string } = { + registry: 'http://registry.test', + namespace: 'global', + slug: 'demo' + } +): Promise { + await mkdir(join(skillDir, '.skillhub'), { recursive: true }) + await writeFile(join(skillDir, '.skillhub', 'metadata.json'), JSON.stringify({ + ...identity, + version: '0.1.0', + source: 'skillhub' + })) +} + describe('installSkill', () => { afterEach(() => { globalThis.fetch = originalFetch @@ -135,6 +151,7 @@ describe('installSkill', () => { const skillDir = join(rootDir, 'demo') await mkdir(skillDir, { recursive: true }) await writeFile(join(skillDir, 'stale.txt'), 'old') + await writeManagedMetadata(skillDir) await installSkill({ registry: 'http://registry.test', @@ -149,12 +166,13 @@ describe('installSkill', () => { expect(await exists(join(skillDir, 'stale.txt'))).toBe(false) }) - test('force removes stale inventory records that point at the replaced install directory', async () => { + test('force rejects a different namespace at the same install directory', async () => { globalThis.fetch = installFetch({ 'SKILL.md': '# Team Demo' }) const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-')) const skillDir = join(rootDir, 'demo') await mkdir(skillDir, { recursive: true }) + await writeManagedMetadata(skillDir) const inventoryPath = join(home, '.skillhub', 'inventory.json') await mkdir(join(home, '.skillhub'), { recursive: true }) await writeFile(inventoryPath, JSON.stringify({ @@ -172,18 +190,18 @@ describe('installSkill', () => { }] })) - await installSkill({ + await expect(installSkill({ registry: 'http://registry.test', namespace: 'team', slug: 'demo', targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }], force: true, home - }) + })).rejects.toThrow('source conflict') const inventory = JSON.parse(await readFile(inventoryPath, 'utf-8')) expect(inventory.items).toHaveLength(1) - expect(inventory.items[0]).toMatchObject({ namespace: 'team', slug: 'demo' }) + expect(inventory.items[0]).toMatchObject({ namespace: 'global', slug: 'demo' }) expect(inventory.items[0].targets).toHaveLength(1) expect(inventory.items[0].targets[0].installDir).toBe(skillDir) }) @@ -195,6 +213,7 @@ describe('installSkill', () => { const skillDir = join(rootDir, 'demo') await mkdir(skillDir, { recursive: true }) await writeFile(join(skillDir, 'SKILL.md'), '# Old') + await writeManagedMetadata(skillDir) const inventoryPath = join(home, '.skillhub', 'inventory.json') await mkdir(join(home, '.skillhub'), { recursive: true }) await writeFile(inventoryPath, JSON.stringify({ @@ -234,6 +253,7 @@ describe('installSkill', () => { const skillDir = join(rootDir, 'demo') await mkdir(skillDir, { recursive: true }) await writeFile(join(skillDir, 'SKILL.md'), '# Old') + await writeManagedMetadata(skillDir) const invalidHome = join(rootDir, 'home-is-a-file') await writeFile(invalidHome, 'not a directory') diff --git a/docs/skillhub/en/guide/cli.md b/docs/skillhub/en/guide/cli.md index 4712cbca..8c21476f 100644 --- a/docs/skillhub/en/guide/cli.md +++ b/docs/skillhub/en/guide/cli.md @@ -157,7 +157,7 @@ skillhub install pdf-parser --agent codex --agent claude-code # Install to custom directory skillhub install pdf-parser --dir ~/.claude/skills -# Force overwrite existing installation +# Reinstall a SkillHub-managed installation from the same source skillhub install pdf-parser --force ``` @@ -214,15 +214,40 @@ For a custom path or an unsupported Agent directory, use `--dir` to specify the ```json { + "schemaVersion": 1, "registry": "https://skill.xfyun.cn", "namespace": "global", "slug": "pdf-parser", "version": "1.0.0", + "versionId": 123, + "fingerprint": "sha256:...", + "files": { "SKILL.md": "sha256..." }, "agent": "codex", "installedAt": "2026-04-28T06:00:00.000Z" } ``` +The CLI creates this file after extracting the registry package. It is not included in the downloaded +ZIP, and `.skillhub/` is excluded when an installed directory is published again. + +## Upgrade Installed Skills + +```bash +# Preview without changing files +skillhub upgrade @global/skillhub-registry --check + +# Upgrade one or a bounded list of installed Skills +skillhub upgrade @global/skillhub-registry +skillhub upgrade @team/code-review @team/java-guide + +# Deterministic machine-readable plan +skillhub upgrade @team/code-review --check --json +``` + +`upgrade` never installs a missing Skill and has no implicit upgrade-all mode. Local changes require +`--force`; it only replaces a managed installation whose full `registry + namespace + slug` source +matches. + ## Local Management ### List Installed Skills @@ -504,11 +529,26 @@ Options: - `--version ` — Version (default: latest) - `--agent ` — Agent profile (repeatable) - `--dir ` — Custom installation directory (mutually exclusive with `--scope` and `--agent`) -- `--force` — Overwrite existing installation +- `--force` — Replace an existing SkillHub-managed installation from the same source - `--registry ` — Registry URL - `--token ` — API token - `--json` — JSON output +### upgrade + +```bash +skillhub upgrade [options] +``` + +Options: +- `--namespace ` — Filter a bare slug by namespace +- `--agent ` — Filter installed targets by Agent (repeatable) +- `--dir ` — Filter installed targets by directory +- `--registry ` — Filter by installation source registry +- `--check` — Print the exact plan without modifying files +- `--force` — Replace local changes only for the same full source identity +- `--json` — JSON output + ### list ```bash @@ -604,7 +644,7 @@ skillhub search test --registry https://skillhub.example.com ### Installation Directory Conflict ```bash -# Use --force to overwrite +# Reinstall only when the existing directory has matching SkillHub source metadata skillhub install pdf-parser --force # Or remove first then install @@ -612,6 +652,9 @@ skillhub remove pdf-parser skillhub install pdf-parser ``` +`--force` never overwrites an unmanaged directory or a Skill installed from another registry, +namespace, or slug. Move or explicitly remove that directory first. + ### Corrupted Inventory ```bash