fix(cli): reject upgrade targets removed after planning

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS 2026-09-01 17:30:25 +08:00
parent 5e6dd21f5b
commit cbb3c009ec
3 changed files with 65 additions and 5 deletions

View file

@ -29,6 +29,7 @@ export interface InstallOptions {
resolved?: ResolveResponse | undefined
expectedTargetFiles?: Record<string, Record<string, string>> | undefined
allowTargetDrift?: boolean | undefined
requireExistingTargets?: boolean | undefined
}
interface StagedInstall {
@ -137,15 +138,23 @@ export async function installSkill(options: InstallOptions): Promise<{ installed
}, resolved)
for (const item of staged) {
if (await pathExists(item.skillDir)) {
const targetExists = await pathExists(item.skillDir)
if (!targetExists && options.requireExistingTargets) {
throw new CliError(`installed target disappeared before upgrade commit: ${item.skillDir}`, EXIT.validation, {
path: item.skillDir,
next: 'reinstall the Skill explicitly before upgrading it'
})
}
if (targetExists) {
if (!options.force) {
throw new CliError(`skill already installed at ${item.skillDir}`, EXIT.filesystem, {
path: item.skillDir,
next: 'pass --force to overwrite'
})
}
item.backupDir = `${item.skillDir}.skillhub-backup-${process.pid}-${Date.now()}`
await rename(item.skillDir, item.backupDir)
const backupDir = `${item.skillDir}.skillhub-backup-${process.pid}-${Date.now()}`
await rename(item.skillDir, backupDir)
item.backupDir = backupDir
await assertReplaceableInstallation(item.backupDir, item.skillDir, {
registry: options.registry,
namespace: options.namespace,

View file

@ -231,7 +231,8 @@ export async function executeSkillUpgradePlan(
force: true,
home: options.home,
expectedTargetFiles: item.expectedTargetFiles,
allowTargetDrift: item.allowTargetDrift
allowTargetDrift: item.allowTargetDrift,
requireExistingTargets: true
})
}
}

View file

@ -1,4 +1,4 @@
import { mkdir, readFile, writeFile } from 'node:fs/promises'
import { access, mkdir, readFile, rm, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import { afterEach, describe, expect, test } from 'bun:test'
import { strToU8, zipSync } from 'fflate'
@ -18,6 +18,15 @@ function makeSkillZip(content: string): Uint8Array {
return zipSync({ 'SKILL.md': strToU8(content) })
}
async function exists(path: string): Promise<boolean> {
try {
await access(path)
return true
} catch {
return false
}
}
describe('upgrade command', () => {
test('check is side-effect free and execute upgrades an installed skill', async () => {
const env = await createTempHome()
@ -166,6 +175,47 @@ describe('upgrade command', () => {
expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: 'fp-v1' })
})
test('a target removed after planning is not recreated by upgrade', async () => {
const env = await createTempHome()
const skill = {
namespace: 'global',
slug: 'removed-late',
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 })
await runCli(['install', '@global/removed-late', '--dir', rootDir, '--registry', registry.url], {
HOME: env.home,
USERPROFILE: env.home
}, { cwd: env.cwd })
skill.version = '1.1.0'
skill.versionId = 2
skill.fingerprint = 'fp-v2'
skill.zipBytes = makeSkillZip('# v2')
const tokenForRegistry = async () => undefined
const plan = await planSkillUpgrades({
coordinates: ['@global/removed-late'],
registry: registry.url,
force: false,
home: env.home,
tokenForRegistry
})
const skillDir = join(rootDir, 'removed-late')
await rm(skillDir, { recursive: true })
await expect(executeSkillUpgradePlan(plan, { home: env.home, tokenForRegistry }))
.rejects.toThrow('installed target disappeared before upgrade commit')
expect(await exists(skillDir)).toBe(false)
const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8'))
expect(inventory.items[0]).toMatchObject({ version: '1.0.0', fingerprint: 'fp-v1' })
})
test('new installs persist absolute targets and legacy relative targets are blocked safely', async () => {
const env = await createTempHome()
const skill = {