fix(cli): reject stale suite upgrades

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

cli/src/services/suite-service.ts

@@ -138,10 +138,21 @@ export async function installSuite(options: SuiteInstallOptions): Promise<SuiteI
   )
   assertNoTargetCollisions(plan)
 
+  return installSuiteWithPlan(options, client, renameOperation, plan)
+}
+
+async function installSuiteWithPlan(
+  options: SuiteInstallOptions,
+  client: SkillHubClient,
+  renameOperation: typeof rename,
+  plan: SuiteInstallPlan,
+  expectedCurrentSuite?: InventorySuite
+): Promise<SuiteInstallResult> {
   const releaseSuiteLock = await acquireSuiteOperationLock(
     options.home, options.registry, plan.namespace, plan.slug)
   try {
-    return await installSuiteTransaction(options, client, renameOperation, plan)
+    return await installSuiteTransaction(
+      options, client, renameOperation, plan, expectedCurrentSuite)
   } finally {
     await releaseSuiteLock().catch(() => {})
   }
@@ -151,12 +162,16 @@ async function installSuiteTransaction(
   options: SuiteInstallOptions,
   client: SkillHubClient,
   renameOperation: typeof rename,
-  plan: SuiteInstallPlan
+  plan: SuiteInstallPlan,
+  expectedCurrentSuite?: InventorySuite
 ): Promise<SuiteInstallResult> {
   const store = new InventoryStore(options.home)
   const before = await store.read()
   const previousSuite = installedSuites(before).find(candidate =>
     candidate.registry === options.registry && candidate.namespace === plan.namespace && candidate.slug === plan.slug)
+  if (expectedCurrentSuite) {
+    assertSuiteSnapshotUnchanged(expectedCurrentSuite, previousSuite)
+  }
   const source = suiteSource(plan.namespace, plan.slug, plan.version)
   const stageHome = await mkdtemp(join(tmpdir(), 'skillhub-suite-inventory-'))
   const stageToken = `${process.pid}-${Date.now()}`
@@ -529,16 +544,24 @@ export async function upgradeSuite(options: {
   home?: string | undefined
   client?: SkillHubClient | undefined
 }): Promise<{ upgrade: SuiteUpgradePlan; result?: SuiteInstallResult }> {
-  const upgrade = await planSuiteUpgrade(options)
+  const client = options.client ?? new SkillHubClient(options.registry, options.token)
+  const upgrade = await planSuiteUpgrade({ ...options, client })
   if (upgrade.current.version === upgrade.remote.version && upgrade.changes.length === 0) {
     return { upgrade }
   }
-  const result = await installSuite({
+  const installPlan = await client.suiteInstallPlan(
+    options.namespace,
+    options.slug,
+    upgrade.remote.version,
+    randomUUID()
+  )
+  assertNoTargetCollisions(installPlan)
+  const result = await installSuiteWithPlan({
     ...options,
     version: upgrade.remote.version,
     targets: upgrade.targets,
     force: true
-  })
+  }, client, rename, installPlan, upgrade.current)
   return { upgrade, result }
 }
 

cli/test/unit/services/suite-service.test.ts

@@ -827,6 +827,44 @@ describe('Suite local lifecycle', () => {
     expect(inventory.items.map((item: { slug: string }) => item.slug).sort()).toEqual(['alpha', 'gamma'])
   })
 
+  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-'))
+    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)
+    })
+
+    const nextPlan = { ...first.plan, operationId: 'operation-2', version: '2.0.0' }
+    const client = clientFor(nextPlan, first.downloads)
+    const fetchDetail = client.suiteDetail.bind(client)
+    let signalPlanRead: (() => void) | undefined
+    let releasePlan: (() => void) | undefined
+    const planRead = new Promise<void>((resolvePromise) => { signalPlanRead = resolvePromise })
+    const holdPlan = new Promise<void>((resolvePromise) => { releasePlan = resolvePromise })
+    client.suiteDetail = async (...args) => {
+      signalPlanRead?.()
+      await holdPlan
+      return fetchDetail(...args)
+    }
+
+    const upgrading = upgradeSuite({ registry, namespace: 'global', slug: 'starter-pack', home, client })
+    await planRead
+    await removeSuite({ registry, namespace: 'global', slug: 'starter-pack', home })
+    releasePlan?.()
+
+    await expect(upgrading).rejects.toThrow('installed Suite changed while waiting for target locks')
+    expect(await readdir(rootDir)).toEqual([])
+    const inventory = JSON.parse(await readFile(join(home, '.skillhub', 'inventory.json'), 'utf8'))
+    expect(inventory).toMatchObject({ items: [], suites: [] })
+  })
+
   test('preserves a member modified after removal starts but before locked validation', async () => {
     const home = await mkdtemp(join(tmpdir(), 'skillhub-suite-home-'))
     const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-suite-root-'))

server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftServiceTest.java

@@ -60,21 +60,28 @@ class SkillSuiteDraftServiceTest {
 
         SkillSuiteMemberSelection member = new SkillSuiteMemberSelection(
                 30L, 40L, "global", "writer", "1.0.0", "sha256:abc");
+        SkillSuiteMemberSelection supportingMember = new SkillSuiteMemberSelection(
+                31L, 41L, "global", "editor", "1.0.0", "sha256:def");
         SkillSuiteDraftService.CreatedDraft result = service.create(
                 new CreateSkillSuiteDraftCommand(
                         1L, "writers", "Writers", "Writing tools", "## Start here", "1.0.0",
-                        SkillVisibility.PRIVATE, null, 40L, List.of(member)),
+                        SkillVisibility.PRIVATE, null, 40L, List.of(member, supportingMember)),
                 new SkillSuiteActionContext(
                         "author", Map.of(1L, NamespaceRole.MEMBER), Set.of(),
                         "request-1", "127.0.0.1", "test"));
 
-        assertThat(result.members()).singleElement().satisfies(saved -> {
+        assertThat(result.members()).filteredOn(SkillSuiteVersionMember::isEntry)
+                .singleElement().satisfies(saved -> {
             assertThat(saved.getSuiteVersionId()).isEqualTo(20L);
             assertThat(saved.getSkillVersionId()).isEqualTo(40L);
             assertThat(saved.getPosition()).isZero();
             assertThat(saved.getFingerprintSnapshot()).isEqualTo("sha256:abc");
-            assertThat(saved.isEntry()).isTrue();
         });
+        assertThat(result.members()).filteredOn(memberSnapshot -> !memberSnapshot.isEntry())
+                .singleElement().satisfies(saved -> {
+                    assertThat(saved.getSkillVersionId()).isEqualTo(41L);
+                    assertThat(saved.getPosition()).isEqualTo(1);
+                });
         assertThat(result.version().getOverview()).isEqualTo("## Start here");
         verify(publicationValidator).validate(result.suite(), result.version());
     }