diff --git a/cli/src/services/suite-service.ts b/cli/src/services/suite-service.ts index a54ab6d5..a6dc8efd 100644 --- a/cli/src/services/suite-service.ts +++ b/cli/src/services/suite-service.ts @@ -138,10 +138,21 @@ export async function installSuite(options: SuiteInstallOptions): Promise { 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 { 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 } } diff --git a/cli/test/unit/services/suite-service.test.ts b/cli/test/unit/services/suite-service.test.ts index e2892d55..4d3aca66 100644 --- a/cli/test/unit/services/suite-service.test.ts +++ b/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((resolvePromise) => { signalPlanRead = resolvePromise }) + const holdPlan = new Promise((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-')) diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftServiceTest.java index 7f6bd6de..0769b31c 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/suite/SkillSuiteDraftServiceTest.java +++ b/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()); }