From 007a693b9b7d96e56b5c6fa5e5f4b1b358da65d5 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Tue, 1 Sep 2026 16:22:16 +0800 Subject: [PATCH] test(cli): prove upgrade safety invariants Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- cli/test/helpers/fake-registry.ts | 4 +- cli/test/integration/upgrade-command.test.ts | 32 +++++++++++++--- .../unit/services/install-service.test.ts | 38 +++++++++++++++++++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index be3b9eec..8833d39f 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -206,8 +206,9 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { delete: CapturedDelete | null validate: CapturedValidate | null review: CapturedReview | null + resolves: number downloads: number - } = { publish: null, resolve: null, delete: null, validate: null, review: null, downloads: 0 } + } = { publish: null, resolve: null, delete: null, validate: null, review: null, resolves: 0, downloads: 0 } // If any endpoint is configured with 'network' failure mode, we need a real // TCP-level failure. Start a connection-dropping server and return its URL @@ -312,6 +313,7 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { // Resolve: GET /api/cli/v1/skills/:namespace/:slug/resolve const resolveMatch = path.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/([^/]+)\/resolve$/) if (resolveMatch && req.method === 'GET') { + state.resolves++ if (options.failures?.resolve) return failureResponse(options.failures.resolve) const namespace = resolveMatch[1]! const slug = resolveMatch[2]! diff --git a/cli/test/integration/upgrade-command.test.ts b/cli/test/integration/upgrade-command.test.ts index 8694c3a0..f1bc146b 100644 --- a/cli/test/integration/upgrade-command.test.ts +++ b/cli/test/integration/upgrade-command.test.ts @@ -43,6 +43,10 @@ describe('upgrade command', () => { skill.fingerprint = 'fp-v2' skill.zipBytes = makeSkillZip('# v2') + const inventoryPath = join(env.home, '.skillhub', 'inventory.json') + const metadataPath = join(rootDir, 'skillhub-registry', '.skillhub', 'metadata.json') + const inventoryBeforeCheck = await readFile(inventoryPath, 'utf-8') + const metadataBeforeCheck = await readFile(metadataPath, 'utf-8') const checked = await runCli([ 'upgrade', '@global/skillhub-registry', '--registry', registry.url, '--dir', rootDir, '--check', '--json' ], { HOME: env.home, USERPROFILE: env.home }) @@ -55,6 +59,17 @@ describe('upgrade command', () => { }) expect(await readFile(join(rootDir, 'skillhub-registry', 'SKILL.md'), 'utf-8')).toBe('# v1') expect(registry.received.downloads).toBe(1) + expect(await readFile(inventoryPath, 'utf-8')).toBe(inventoryBeforeCheck) + expect(await readFile(metadataPath, 'utf-8')).toBe(metadataBeforeCheck) + + const checkedAgain = await runCli([ + 'upgrade', '@global/skillhub-registry', '--registry', registry.url, '--dir', rootDir, '--check', '--json' + ], { HOME: env.home, USERPROFILE: env.home }) + expect(checkedAgain.exitCode).toBe(0) + expect(checkedAgain.stdout).toBe(checked.stdout) + expect(await readFile(inventoryPath, 'utf-8')).toBe(inventoryBeforeCheck) + expect(await readFile(metadataPath, 'utf-8')).toBe(metadataBeforeCheck) + expect(registry.received.downloads).toBe(1) const upgraded = await runCli([ 'upgrade', '@global/skillhub-registry', '--registry', registry.url, '--json' @@ -64,13 +79,10 @@ describe('upgrade command', () => { expect(await readFile(join(rootDir, 'skillhub-registry', 'SKILL.md'), 'utf-8')).toBe('# v2') expect(registry.received.downloads).toBe(2) - const metadata = JSON.parse(await readFile( - join(rootDir, 'skillhub-registry', '.skillhub', 'metadata.json'), - 'utf-8' - )) + const metadata = JSON.parse(await readFile(metadataPath, 'utf-8')) expect(metadata).toMatchObject({ schemaVersion: 1, version: '1.1.0', versionId: 2, fingerprint: 'fp-v2' }) expect(Object.keys(metadata.files)).toContain('SKILL.md') - const inventory = JSON.parse(await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8')) + const inventory = JSON.parse(await readFile(inventoryPath, 'utf-8')) expect(inventory.items[0]).toMatchObject({ version: '1.1.0', fingerprint: 'fp-v2' }) }) @@ -179,6 +191,14 @@ describe('upgrade command', () => { expect(selected.exitCode).toBe(0) expect(selected.stdout).toContain('@team-a/demo') + const fullCoordinate = await runCli(['upgrade', '@team-a/demo', '--check'], { + HOME: env.home, + USERPROFILE: env.home + }) + expect(fullCoordinate.exitCode).toBe(0) + expect(fullCoordinate.stdout).toContain('@team-a/demo') + expect(fullCoordinate.stdout).not.toContain('@team-b/demo') + const noNamespaceMatch = await runCli(['upgrade', 'demo', '--namespace', 'missing', '--check'], { HOME: env.home, USERPROFILE: env.home @@ -244,6 +264,7 @@ describe('upgrade command', () => { 'install', '@global/shared', '--agent', 'codex', '--agent', 'claude-code', '--registry', registry.url ], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) expect(installed.exitCode).toBe(0) + expect(registry.received.resolves).toBe(1) expect(registry.received.downloads).toBe(1) skill.version = '1.1.0' @@ -262,6 +283,7 @@ describe('upgrade command', () => { USERPROFILE: env.home }, { cwd: env.cwd }) expect(upgraded.exitCode).toBe(0) + expect(registry.received.resolves).toBe(2) expect(registry.received.downloads).toBe(2) expect(await readFile(join(env.home, '.codex', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') expect(await readFile(join(env.home, '.claude', 'skills', 'shared', 'SKILL.md'), 'utf-8')).toBe('# v2') diff --git a/cli/test/unit/services/install-service.test.ts b/cli/test/unit/services/install-service.test.ts index c97975fb..949e9875 100644 --- a/cli/test/unit/services/install-service.test.ts +++ b/cli/test/unit/services/install-service.test.ts @@ -228,6 +228,43 @@ describe('installSkill', () => { })).rejects.toThrow('cannot verify SkillHub ownership') }) + test('force rejects a directory without installation metadata', async () => { + globalThis.fetch = installFetch({ 'SKILL.md': '# New' }) + const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) + const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-')) + const skillDir = join(rootDir, 'demo') + await mkdir(skillDir, { recursive: true }) + await writeFile(join(skillDir, 'local.txt'), 'keep') + + await expect(installSkill({ + registry: 'http://registry.test', namespace: 'global', slug: 'demo', + targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }], + force: true, home + })).rejects.toThrow('cannot verify SkillHub ownership') + expect(await readFile(join(skillDir, 'local.txt'), 'utf-8')).toBe('keep') + expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false) + }) + + test('force rejects a different slug in installation metadata', async () => { + globalThis.fetch = installFetch({ 'SKILL.md': '# New' }) + const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) + const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-')) + const skillDir = join(rootDir, 'demo') + await mkdir(skillDir, { recursive: true }) + await writeFile(join(skillDir, 'SKILL.md'), '# Other') + await writeManagedMetadata(skillDir, { + registry: 'http://registry.test', namespace: 'global', slug: 'other' + }) + + await expect(installSkill({ + registry: 'http://registry.test', namespace: 'global', slug: 'demo', + targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }], + force: true, home + })).rejects.toThrow('source conflict') + expect(await readFile(join(skillDir, 'SKILL.md'), 'utf-8')).toBe('# Other') + expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false) + }) + test('revalidates ownership after download before replacing the target', async () => { const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-')) const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-')) @@ -281,6 +318,7 @@ describe('installSkill', () => { expect(metadata.registry).toBe('http://other-registry.test') expect((await readdir(rootDir)).some(name => name.includes('skillhub-backup'))).toBe(false) expect(await exists(join(rootDir, '.demo.skillhub-install.lock'))).toBe(false) + expect(await exists(join(home, '.skillhub', 'inventory.json'))).toBe(false) }) test('rolls back every target when a later target changes source before commit', async () => {