fix(cli): protect suite upgrades from local changes

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>

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<ReturnType<typeof planSuiteUpgrade>>, json: boolean): string {

cli/src/index.ts

@@ -255,7 +255,7 @@ cli
   .option('--scope <scope>', 'Install scope: user or project')
   .option('--agent <profile>', 'Agent profile (repeatable)')
   .option('--dir <path>', '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 <url>', 'Registry URL')
   .option('--token <token>', 'API token')

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<SuiteInstallResult> {
   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<SuiteInstallResult> {
   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<void> {
   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,

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-'))

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