diff --git a/cli/src/commands/suite.ts b/cli/src/commands/suite.ts index 38367ff8..2d9723c4 100644 --- a/cli/src/commands/suite.ts +++ b/cli/src/commands/suite.ts @@ -83,8 +83,10 @@ export async function suiteCommand( ].join('\n') } - if (hasInstallOnlyOptions(options)) { - throw new CliError('--scope, --agent, --dir, --force, and --version are only valid with suite install', EXIT.usage) + if (hasInstallOnlyOptions(options) || (options.force === true && action !== 'upgrade')) { + throw new CliError( + '--scope, --agent, --dir, and --version are only valid with suite install; --force is valid with install or upgrade', + EXIT.usage) } if (action === 'check') { @@ -116,7 +118,7 @@ export async function suiteCommand( const plan = await planSuiteUpgrade(common) return renderUpgradePlan(plan, Boolean(options.json)) } - const { upgrade, result } = await upgradeSuite(common) + const { upgrade, result } = await upgradeSuite({ ...common, force: Boolean(options.force) }) if (options.json) return JSON.stringify({ ok: true, upgrade, result }) if (!result) return `Suite @${namespace}/${slug}@${upgrade.current.version} is current` return [ @@ -127,7 +129,7 @@ export async function suiteCommand( function hasInstallOnlyOptions(options: SuiteCommandOptions): boolean { return options.scope !== undefined || options.agent !== undefined || options.dir !== undefined || - options.force !== undefined || options.version !== undefined + options.version !== undefined } function renderUpgradePlan(plan: Awaited>, json: boolean): string { diff --git a/cli/src/index.ts b/cli/src/index.ts index 39d3080e..0129ced2 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -255,7 +255,7 @@ cli .option('--scope ', 'Install scope: user or project') .option('--agent ', 'Agent profile (repeatable)') .option('--dir ', 'Install directory') - .option('--force', 'Replace same-source member versions or local changes') + .option('--force', 'Replace local changes during install or upgrade') .option('--check', 'Show an upgrade plan without writing') .option('--registry ', 'Registry URL') .option('--token ', 'API token') diff --git a/cli/src/services/suite-service.ts b/cli/src/services/suite-service.ts index 19e09e8d..cf63b83d 100644 --- a/cli/src/services/suite-service.ts +++ b/cli/src/services/suite-service.ts @@ -146,13 +146,14 @@ async function installSuiteWithPlan( client: SkillHubClient, renameOperation: typeof rename, plan: SuiteInstallPlan, - expectedCurrentSuite?: InventorySuite + expectedCurrentSuite?: InventorySuite, + allowVersionReplacement = options.force ): Promise { const releaseSuiteLock = await acquireSuiteOperationLock( options.home, options.registry, plan.namespace, plan.slug) try { return await installSuiteTransaction( - options, client, renameOperation, plan, expectedCurrentSuite) + options, client, renameOperation, plan, expectedCurrentSuite, allowVersionReplacement) } finally { await releaseSuiteLock().catch(() => {}) } @@ -163,7 +164,8 @@ async function installSuiteTransaction( client: SkillHubClient, renameOperation: typeof rename, plan: SuiteInstallPlan, - expectedCurrentSuite?: InventorySuite + expectedCurrentSuite?: InventorySuite, + allowVersionReplacement = options.force ): Promise { const store = new InventoryStore(options.home) const before = await store.read() @@ -179,7 +181,8 @@ async function installSuiteTransaction( const retired = await prepareRetiredTargets(before, previousSuite, plan, stageToken) try { - await preflightExistingTargets(before, options.registry, plan, options.targets, options.force) + await preflightExistingTargets( + before, options.registry, plan, options.targets, options.force, allowVersionReplacement) for (const member of plan.members) { const stagingTargets = options.targets.map((target, index) => ({ @@ -243,7 +246,8 @@ async function installSuiteTransaction( const lockedPreviousSuite = installedSuites(lockedInventory).find(candidate => candidate.registry === options.registry && candidate.namespace === plan.namespace && candidate.slug === plan.slug) assertSuiteSnapshotUnchanged(previousSuite, lockedPreviousSuite) - await preflightExistingTargets(lockedInventory, options.registry, plan, options.targets, options.force) + await preflightExistingTargets( + lockedInventory, options.registry, plan, options.targets, options.force, allowVersionReplacement) for (const item of prepared) { item.reuse = await isReusable(lockedInventory, options.registry, item.member, item.installDir) item.replace = await pathExists(item.installDir) && !item.reuse @@ -542,6 +546,7 @@ export async function upgradeSuite(options: { token?: string | undefined namespace: string slug: string + force?: boolean | undefined home?: string | undefined client?: SkillHubClient | undefined }): Promise<{ upgrade: SuiteUpgradePlan; result?: SuiteInstallResult }> { @@ -561,8 +566,8 @@ export async function upgradeSuite(options: { ...options, version: upgrade.remote.version, targets: upgrade.targets, - force: true - }, client, rename, installPlan, upgrade.current) + force: Boolean(options.force) + }, client, rename, installPlan, upgrade.current, true) return { upgrade, result } } @@ -624,7 +629,8 @@ async function preflightExistingTargets( registry: string, plan: SuiteInstallPlan, targets: AgentCandidate[], - force: boolean + force: boolean, + allowVersionReplacement: boolean ): Promise { for (const member of plan.members) { const selectedDirs = new Set(targets.map(target => join(resolve(target.rootDir), member.slug))) @@ -673,7 +679,7 @@ async function preflightExistingTargets( next: 'install the Suite into another target or upgrade the sharing Suite first' }) } - if (owner && owner.version !== member.version && !force) { + if (owner && owner.version !== member.version && !allowVersionReplacement) { throw new CliError(`different Skill version already installed at ${installDir}`, EXIT.validation, { currentVersion: owner.version, requestedVersion: member.version, diff --git a/cli/test/unit/services/suite-service.test.ts b/cli/test/unit/services/suite-service.test.ts index 2a0b5353..6a962c01 100644 --- a/cli/test/unit/services/suite-service.test.ts +++ b/cli/test/unit/services/suite-service.test.ts @@ -908,6 +908,59 @@ describe('Suite local lifecycle', () => { expect(inventory.items.map((item: { slug: string }) => item.slug).sort()).toEqual(['alpha', 'gamma']) }) + test('does not overwrite a locally modified member during Suite upgrade by default', async () => { + const home = await mkdtemp(join(tmpdir(), 'skillhub-suite-home-')) + const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-suite-root-')) + const first = makePlan() + await installSuite({ + registry, + namespace: 'global', + slug: 'starter-pack', + targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }], + force: false, + home, + client: clientFor(first.plan, first.downloads) + }) + await writeFile(join(rootDir, 'alpha', 'SKILL.md'), '# Locally modified Alpha') + + const alphaV2 = archive('# Alpha v2') + const second: SuiteInstallPlan = { + ...first.plan, + operationId: 'operation-2', + version: '2.0.0', + fingerprint: 'sha256:suite-v2', + members: [ + { ...first.plan.members[0]!, skillVersionId: 12, version: '2.0.0', fingerprint: alphaV2.fingerprint }, + first.plan.members[1]! + ] + } + + await expect(upgradeSuite({ + registry, + namespace: 'global', + slug: 'starter-pack', + home, + client: clientFor(second, { ...first.downloads, '/downloads/alpha': alphaV2.bytes }) + })).rejects.toThrow('local changes') + + expect(await readFile(join(rootDir, 'alpha', 'SKILL.md'), 'utf8')).toBe('# Locally modified Alpha') + const unchangedInventory = JSON.parse(await readFile(join(home, '.skillhub', 'inventory.json'), 'utf8')) + expect(unchangedInventory.suites[0]).toMatchObject({ version: '1.0.0', fingerprint: 'sha256:suite' }) + + await upgradeSuite({ + registry, + namespace: 'global', + slug: 'starter-pack', + force: true, + home, + client: clientFor(second, { ...first.downloads, '/downloads/alpha': alphaV2.bytes }) + }) + + expect(await readFile(join(rootDir, 'alpha', 'SKILL.md'), 'utf8')).toBe('# Alpha v2') + const upgradedInventory = JSON.parse(await readFile(join(home, '.skillhub', 'inventory.json'), 'utf8')) + expect(upgradedInventory.suites[0]).toMatchObject({ version: '2.0.0', fingerprint: 'sha256:suite-v2' }) + }) + test('does not reinstall a Suite removed after upgrade planning', async () => { const home = await mkdtemp(join(tmpdir(), 'skillhub-suite-home-')) const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-suite-root-')) diff --git a/openspec/changes/add-skill-suites/specs/skill-suites/spec.md b/openspec/changes/add-skill-suites/specs/skill-suites/spec.md index e153d0f3..ea03ea8e 100644 --- a/openspec/changes/add-skill-suites/specs/skill-suites/spec.md +++ b/openspec/changes/add-skill-suites/specs/skill-suites/spec.md @@ -279,10 +279,11 @@ CLI SHALL 在修改目标目录前完成全部成员和全部 Agent 目标的解 - **AND** 另一个操作明确报告繁忙,不得基于旧 inventory 提交 #### Scenario: Existing Member has local changes -- **WHEN** Suite 安装将复用或替换一个已登记但 fingerprint 已变化的 Member 目录 +- **WHEN** Suite 安装或升级将复用或替换一个已登记但 fingerprint 已变化的 Member 目录 - **AND** 用户未明确传入 `--force` -- **THEN** CLI 在写入任何目标或 inventory 前拒绝安装 +- **THEN** CLI 在写入任何目标或 inventory 前拒绝该操作 - **AND** 保留本地文件和现有 inventory +- **AND** 只有用户显式传入 `--force` 时才允许覆盖本地修改 ### Requirement: Suite installation SHALL preserve Agent Skills compatibility