diff --git a/cli/test/integration/auth-resolution.test.ts b/cli/test/integration/auth-resolution.test.ts new file mode 100644 index 000000000..1251445fd --- /dev/null +++ b/cli/test/integration/auth-resolution.test.ts @@ -0,0 +1,159 @@ +/** + * End-to-end integration coverage for token / registry priority resolution. + * + * The unit test in test/unit/services/registry-service.test.ts pins the + * resolution function in isolation. These tests verify the same priorities + * are wired through the actual CLI subprocess: --flag > SKILLHUB_* env > + * stored config / credentials > built-in default. + * + * Why this matters: a regression in the wiring (e.g. command forgets to + * forward `process.env`) would silently downgrade users to the wrong + * registry / token without surfacing in unit tests. + */ +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined +let registryB: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined + registryB?.stop(); registryB = undefined +}) + +async function seedCredentials(home: string, registryUrl: string, token: string): Promise { + await mkdir(join(home, '.skillhub'), { recursive: true }) + await writeFile( + join(home, '.skillhub', 'credentials.json'), + JSON.stringify({ tokens: { [registryUrl]: token } }) + ) +} + +async function seedConfig(home: string, registryUrl: string): Promise { + await mkdir(join(home, '.skillhub'), { recursive: true }) + await writeFile( + join(home, '.skillhub', 'config.json'), + JSON.stringify({ registry: registryUrl }) + ) +} + +// --------------------------------------------------------------------------- +// Token priority: --token > SKILLHUB_TOKEN > stored +// --------------------------------------------------------------------------- + +describe('auth resolution — token priority', () => { + test('--token flag wins over SKILLHUB_TOKEN env', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_flag', + user: { handle: 'flag-user', displayName: 'Flag' } + }) + + const result = await runCli( + ['whoami', '--registry', registry.url, '--token', 'sk_from_flag'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_TOKEN: 'sk_wrong_from_env' } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('flag-user') + }) + + test('SKILLHUB_TOKEN env wins over stored token', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_env', + user: { handle: 'env-user', displayName: 'Env' } + }) + await seedCredentials(env.home, registry.url, 'sk_wrong_from_storage') + + const result = await runCli( + ['whoami', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_TOKEN: 'sk_from_env' } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('env-user') + }) + + test('stored token used when neither --token nor env is set', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_storage', + user: { handle: 'storage-user', displayName: 'Storage' } + }) + await seedCredentials(env.home, registry.url, 'sk_from_storage') + + const result = await runCli( + ['whoami', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('storage-user') + }) +}) + +// --------------------------------------------------------------------------- +// Registry priority: --registry > SKILLHUB_REGISTRY > config.json +// --------------------------------------------------------------------------- + +describe('auth resolution — registry priority', () => { + test('--registry flag wins over SKILLHUB_REGISTRY env', async () => { + const env = await createTempHome() + // Each registry only authenticates its own token. The wrong registry + // would 401, so a successful whoami proves the right one was used. + registry = await startFakeRegistry({ + token: 'sk_a', + user: { handle: 'a-user', displayName: 'A' } + }) + registryB = await startFakeRegistry({ + token: 'sk_b', + user: { handle: 'b-user', displayName: 'B' } + }) + + const result = await runCli( + ['whoami', '--registry', registry.url, '--token', 'sk_a'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_REGISTRY: registryB.url } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('a-user') + }) + + test('SKILLHUB_REGISTRY env wins over config.registry', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_env', + user: { handle: 'env-reg', displayName: 'EnvReg' } + }) + registryB = await startFakeRegistry({ + token: 'sk_config', + user: { handle: 'config-reg', displayName: 'ConfigReg' } + }) + await seedConfig(env.home, registryB.url) + + const result = await runCli( + ['whoami', '--token', 'sk_env'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_REGISTRY: registry.url } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('env-reg') + }) + + test('config.registry used when no --registry / env present', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_config', + user: { handle: 'config-only-user', displayName: 'CfgOnly' } + }) + await seedConfig(env.home, registry.url) + await seedCredentials(env.home, registry.url, 'sk_config') + + const result = await runCli( + ['whoami'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('config-only-user') + }) +}) diff --git a/cli/test/integration/concurrency.test.ts b/cli/test/integration/concurrency.test.ts new file mode 100644 index 000000000..8490ed235 --- /dev/null +++ b/cli/test/integration/concurrency.test.ts @@ -0,0 +1,164 @@ +/** + * Concurrency tests for inventory.json bookkeeping. + * + * inventory-store.ts uses an OS-level lock file with retry + stale-lock + * detection. These tests exercise that path through real CLI subprocesses + * (Bun.spawn) running in parallel — the same way users hit it when scripts + * fan out installs. + * + * The unit test in test/unit/stores/inventory-store.test.ts pins the + * single-process lock recovery; here we cover the cross-process case. + */ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# c') }) +} + +describe('cross-process concurrency on inventory.json', () => { + // KNOWN BUG (documented here, not yet fixed): + // inventory-store.upsertTarget() reads inventory, modifies in memory, + // then writeAtomic() acquires the lock only over the write half. Two + // concurrent installs each read the (empty) inventory, each adds their + // own item, and the second writer overwrites the first — a classic + // lost-update. + // + // When the fix lands (lock spans read+write, or upsertTarget acquires + // the lock first and re-reads), tighten the inventory assertion to + // `expect(slugs).toEqual(['first', 'second'])`. + test('two parallel installs of distinct slugs: filesystem is correct, inventory has at least one (lost-update bug pinned)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'first', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'second', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const dirA = join(env.cwd, 'A') + const dirB = join(env.cwd, 'B') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + const [r1, r2] = await Promise.all([ + runCli( + ['install', 'first', '--dir', dirA, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ), + runCli( + ['install', 'second', '--dir', dirB, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + ]) + + // Both subprocess installs report success — neither errored at the + // protocol level even though the inventory bookkeeping race ate one of + // their inventory writes. + expect(r1.exitCode).toBe(0) + expect(r2.exitCode).toBe(0) + + // Filesystem is correct: both bundles extracted independently. + expect(await Bun.file(join(dirA, 'first', 'SKILL.md')).exists()).toBe(true) + expect(await Bun.file(join(dirB, 'second', 'SKILL.md')).exists()).toBe(true) + + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + const slugs = inv.items.map(i => i.slug).sort() + // Today: at least one slug always lands; under the lost-update race + // both may NOT be there. When the lock widens to cover read+write, + // upgrade this to `toEqual(['first', 'second'])`. + expect(slugs.length).toBeGreaterThanOrEqual(1) + const lastSlug = slugs[slugs.length - 1]! + expect(['first', 'second']).toContain(lastSlug) + }) + + test('two parallel installs of the same slug to the same dir: exactly one wins, one conflicts', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'race', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'race-dir') + await mkdir(installDir, { recursive: true }) + + const [r1, r2] = await Promise.all([ + runCli( + ['install', 'race', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ), + runCli( + ['install', 'race', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + ]) + + // Two valid outcomes: (a) both succeed because the loser's existence + // check ran BEFORE the winner extracted, OR (b) one succeeds and the + // other reports already-installed (EXIT.filesystem). + // Either way, inventory must end up coherent (single item, single + // target — no duplicates). + const codes = [r1.exitCode, r2.exitCode].sort((a, b) => a - b) + expect(codes[0]).toBe(0) // at least one succeeded + const otherCode = codes[1]! + expect([0, 4]).toContain(otherCode) // other either succeeded or got conflict + + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + const item = inv.items.find(i => i.slug === 'race') + expect(item).toBeDefined() + expect(item!.targets).toHaveLength(1) // no duplicate targets + }) + + test('install proceeds after a stale lock file from a dead process', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'after-stale', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Plant a stale lock file: PID 1 (init, never the same as our test + // child, and won't match the spawned subprocess's PID), with a very + // old timestamp so the store treats it as stale. + const skillhubDir = join(env.home, '.skillhub') + await mkdir(skillhubDir, { recursive: true }) + const lockPath = join(skillhubDir, 'inventory.json.lock') + const ancientTimestamp = Date.now() - 600_000 // 10 minutes ago — past the 30s stale threshold + await writeFile(lockPath, JSON.stringify({ pid: 1, timestamp: ancientTimestamp })) + + const installDir = join(env.cwd, 'stale') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'after-stale', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse( + await readFile(join(skillhubDir, 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inv.items.find(i => i.slug === 'after-stale')).toBeDefined() + }) +}) diff --git a/cli/test/integration/cross-command.test.ts b/cli/test/integration/cross-command.test.ts new file mode 100644 index 000000000..0eed997b1 --- /dev/null +++ b/cli/test/integration/cross-command.test.ts @@ -0,0 +1,509 @@ +/** + * Cross-command flow tests. + * + * Per-command tests verify each subcommand in isolation. These cases pin + * behaviors that only emerge when commands chain — e.g. "logout then install + * fails with auth" or "install + fs-delete + list reports status=missing". + * Bugs in the boundaries between commands (shared inventory, credentials, + * config) tend to slip through single-command suites. + */ +import { mkdir, rm, writeFile, readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined +let registryB: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined + registryB?.stop(); registryB = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# x-cross') }) +} + +// --------------------------------------------------------------------------- +// 1. Auth lifecycle: login → whoami → logout → whoami +// --------------------------------------------------------------------------- + +describe('cross-command — auth lifecycle', () => { + test('login → whoami(success) → logout → whoami(not logged in)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'cycle-user', displayName: 'Cycle' } + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + const w1 = await runCli(['whoami', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(w1.exitCode).toBe(0) + expect(w1.stdout).toContain('cycle-user') + + await runCli(['logout', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + const w2 = await runCli(['whoami', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(w2.exitCode).toBe(2) + expect(w2.stderr.toLowerCase()).toContain('not logged in') + }) + + test('logout-then-install against an auth-required registry fails with EXIT.auth', async () => { + const env = await createTempHome() + // Inject auth failure on resolve so this fake server behaves like a + // production registry that requires a bearer token even on resolve. + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + failures: { resolve: 'auth' } + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'after-logout') + await mkdir(installDir, { recursive: true }) + + // No --token here — credentials were just cleared by logout. + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(2) // EXIT.auth + expect(result.stderr.toLowerCase()).toMatch(/auth|401|unauthorized/) + }) +}) + +// --------------------------------------------------------------------------- +// 2. Full local lifecycle: install → list → remove → list +// --------------------------------------------------------------------------- + +describe('cross-command — local lifecycle', () => { + test('install → list → remove --all → list shows empty', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'lifecycle') + await mkdir(installDir, { recursive: true }) + + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list1 = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(JSON.parse(list1.stdout).items).toHaveLength(1) + + await runCli( + ['remove', 'pdf-parser', '--all', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list2 = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(JSON.parse(list2.stdout).items).toHaveLength(0) + }) + + test('install x2 same slug + same dir without --force conflicts; --force succeeds; second install replaces first', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'reinstall-here') + await mkdir(installDir, { recursive: true }) + + const r1 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r1.exitCode).toBe(0) + + const r2 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(4) // EXIT.filesystem (already installed) + + const r3 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r3.exitCode).toBe(0) + + // Inventory has exactly one target, not two duplicates. + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + const item = inv.items.find(i => i.slug === 'pdf-parser') + expect(item?.targets).toHaveLength(1) + }) + + test('install A then install B (different slugs, same parent dir) → list shows both', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'a-skill', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'b-skill', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'two-skills') + await mkdir(installDir, { recursive: true }) + + await runCli( + ['install', 'a-skill', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + await runCli( + ['install', 'b-skill', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + const items = JSON.parse(list.stdout).items as Array<{ slug: string }> + expect(items.map(i => i.slug).sort()).toEqual(['a-skill', 'b-skill']) + }) + + test('remove --all → install same slug again succeeds (no stale inventory state)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'reuse') + await mkdir(installDir, { recursive: true }) + + await runCli(['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['remove', 'pdf-parser', '--all', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + + // Re-install at the same dir without --force should now succeed, since + // the previous install was removed. + const reinstall = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(reinstall.exitCode).toBe(0) + }) +}) + +// --------------------------------------------------------------------------- +// 3. Filesystem drift between install dir and inventory +// --------------------------------------------------------------------------- + +describe('cross-command — filesystem drift', () => { + test('install → fs-delete the install dir → list reports status=missing', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'drift') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // External clobber: delete the install dir behind the CLI's back. + await rm(join(installDir, 'pdf-parser'), { recursive: true, force: true }) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items[0]?.slug).toBe('pdf-parser') + expect(items[0]?.status).toBe('missing') + }) + + // After commit a14d89d8 ("refactor(cli): improve doctor command + // semantics and transparency") doctor switched from REPLACE to MERGE + // semantics: it never removes inventory entries, even when the install + // dir on disk is gone. Stale entries are surfaced via `list --json`'s + // status="missing" instead. This test pins that contract. + test('install → fs-delete the install dir → doctor preserves the entry; list reports status=missing', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Install into an agent-shaped dir under cwd so doctor will scan it. + const codexSkills = join(env.cwd, '.codex', 'skills') + await mkdir(codexSkills, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', codexSkills, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Wipe the install but leave the dir tree shape — metadata gone. + await rm(join(codexSkills, 'pdf-parser'), { recursive: true, force: true }) + + const doctor = await runCli(['doctor', '--json'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + expect(doctor.exitCode).toBe(0) + + // Inventory still has the entry — doctor preserved it because the + // installDir was NOT in the (now-empty) scan result. + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inv.items.find(i => i.slug === 'pdf-parser')).toBeDefined() + + // The user-facing surface for "this is gone on disk" is `list` — it + // reports status="missing" by stat'ing the installDir at read time. + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items.find(i => i.slug === 'pdf-parser')?.status).toBe('missing') + }) +}) + +// --------------------------------------------------------------------------- +// 4. doctor idempotence +// --------------------------------------------------------------------------- + +describe('cross-command — doctor idempotence', () => { + test('two consecutive doctor runs produce identical inventory (idempotent)', async () => { + const env = await createTempHome() + + // Seed one valid metadata file. + const metaDir = join(env.cwd, '.codex', 'skills', 'pdf-parser', '.skillhub') + await mkdir(metaDir, { recursive: true }) + await writeFile(join(metaDir, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-20T12:00:00Z' + })) + + await runCli(['doctor'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + const after1 = await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + + await runCli(['doctor'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + const after2 = await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + + expect(after2).toBe(after1) + }) +}) + +// --------------------------------------------------------------------------- +// 5. publish does not change local inventory +// --------------------------------------------------------------------------- + +describe('cross-command — publish vs local inventory', () => { + test('publish does NOT add the published skill to local inventory', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok', user: { handle: 'u', displayName: 'U' } }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Build a tiny skill dir to publish. + const dir = join(env.cwd, 'src-skill') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'SKILL.md'), '---\nname: pub-only\ndescription: x\n---\n# pub-only') + + const pub = await runCli(['publish', dir, '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(pub.exitCode).toBe(0) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + expect(JSON.parse(list.stdout).items).toHaveLength(0) + }) +}) + +// --------------------------------------------------------------------------- +// 6. Cross-registry isolation in queries +// --------------------------------------------------------------------------- + +describe('cross-command — cross-registry isolation', () => { + test('list scoped to registry A does not show items installed from registry B', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_a', + user: { handle: 'a', displayName: 'A' }, + skills: [{ namespace: 'global', slug: 'a-only', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + registryB = await startFakeRegistry({ + token: 'sk_b', + user: { handle: 'b', displayName: 'B' }, + skills: [{ namespace: 'global', slug: 'b-only', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', registryB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + const dirA = join(env.cwd, 'A') + const dirB = join(env.cwd, 'B') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + await runCli(['install', 'a-only', '--dir', dirA, '--registry', registry.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['install', 'b-only', '--dir', dirB, '--registry', registryB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + const listA = await runCli(['list', '--registry', registry.url, '--json'], { HOME: env.home, USERPROFILE: env.home }) + const slugsA = (JSON.parse(listA.stdout).items as Array<{ slug: string }>).map(i => i.slug) + expect(slugsA).toEqual(['a-only']) + + const listB = await runCli(['list', '--registry', registryB.url, '--json'], { HOME: env.home, USERPROFILE: env.home }) + const slugsB = (JSON.parse(listB.stdout).items as Array<{ slug: string }>).map(i => i.slug) + expect(slugsB).toEqual(['b-only']) + }) +}) + +// --------------------------------------------------------------------------- +// 7. Auto-detect + list filter integration (project-level) +// --------------------------------------------------------------------------- + +describe('cross-command — auto-detect + list', () => { + test('install auto-detects project-level .codex; subsequent list --agent codex shows it', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Pre-create .codex/skills so auto-detect picks codex/project-level. + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + + const inst = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(inst.exitCode).toBe(0) + + const list = await runCli( + ['list', '--agent', 'codex', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; agent: string }> + expect(items.some(i => i.slug === 'pdf-parser' && i.agent === 'codex')).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// 8. Inventory metadata corruption resilience after install +// --------------------------------------------------------------------------- + +describe('cross-command — metadata.json drift', () => { + test('install → manually corrupt metadata.json → list reports the row but with sane handling', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'meta-drift') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Corrupt the installed metadata. inventory.json (the authoritative + // source for `list`) is untouched, so `list` should still work. + await writeFile( + join(installDir, 'pdf-parser', '.skillhub', 'metadata.json'), + '{ truncated' + ) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items[0]?.slug).toBe('pdf-parser') + // Status remains "ok" because list uses inventory.json, not metadata.json. + expect(items[0]?.status).toBe('ok') + }) +}) + +// --------------------------------------------------------------------------- +// 9. Registry priority chain end-to-end +// --------------------------------------------------------------------------- + +describe('cross-command — registry priority end-to-end', () => { + test('search uses --registry over SKILLHUB_REGISTRY env over default', async () => { + registry = await startFakeRegistry({ + searchItems: [{ namespace: 'global', slug: 'wins', latestVersion: '1.0.0', summary: 'right one' }] + }) + registryB = await startFakeRegistry({ + searchItems: [{ namespace: 'global', slug: 'loses', latestVersion: '1.0.0', summary: 'wrong one' }] + }) + + const result = await runCli( + ['search', '', '--registry', registry.url, '--json'], + { SKILLHUB_REGISTRY: registryB.url } + ) + expect(result.exitCode).toBe(0) + const items = JSON.parse(result.stdout).items as Array<{ slug: string }> + expect(items.map(i => i.slug)).toEqual(['wins']) + }) +}) + +// --------------------------------------------------------------------------- +// 10. Help / Version ergonomics across commands +// --------------------------------------------------------------------------- + +describe('cross-command — help reaches every documented command', () => { + test('every command listed in help responds to --help with non-empty body', async () => { + const helpResult = await runCli(['help']) + expect(helpResult.exitCode).toBe(0) + + const commandNames = [ + 'help', 'version', 'login', 'logout', 'whoami', + 'search', 'install', 'list', 'remove', 'doctor', + 'publish', 'update' + ] + for (const cmd of commandNames) { + expect(helpResult.stdout).toContain(cmd) + const sub = await runCli([cmd, '--help']) + // --help exits 0 for cac-style CLIs; we don't insist on that, just + // that some informative output makes it to stdout. + expect(sub.stdout.length).toBeGreaterThan(0) + } + }) +}) diff --git a/cli/test/integration/doctor-command.test.ts b/cli/test/integration/doctor-command.test.ts index f2afecb19..0d833722d 100644 --- a/cli/test/integration/doctor-command.test.ts +++ b/cli/test/integration/doctor-command.test.ts @@ -143,6 +143,139 @@ describe('doctor command', () => { expect(json.inventoryPath).toContain('inventory.json') }) + // ------------------------------------------------------------------------- + // P1: same registry+namespace+slug appearing in two agent dirs with + // different versions surfaces in `conflicts` and is excluded from items. + // ------------------------------------------------------------------------- + test('doctor reports conflicts when two agent dirs disagree on version', async () => { + const { home, cwd } = await createTempHome() + + // Two installs of the same global/pdf-parser with mismatched versions. + await seedSkill(cwd, { + agentDir: '.codex', + slug: 'pdf-parser', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-20T12:00:00Z' + } + }) + await seedSkill(cwd, { + agentDir: '.claude', + slug: 'pdf-parser', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '2.0.0', + agent: 'claude-code', + installedAt: '2026-04-21T09:00:00Z' + } + }) + + const result = await runCli(['doctor', '--json'], { + HOME: home, + USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + ok: boolean + itemsScanned: number + targetsScanned: number + conflicts: Array<{ key: string; versions: string[] }> + } + expect(json.ok).toBe(true) + // Conflicting group is dropped from items, recorded as a conflict. + expect(json.itemsScanned).toBe(0) + expect(json.targetsScanned).toBe(0) + expect(json.conflicts).toHaveLength(1) + expect(json.conflicts[0]?.key).toBe('https://skill.xfyun.cn|global|pdf-parser') + expect(json.conflicts[0]?.versions.sort()).toEqual(['1.0.0', '2.0.0']) + + // The persisted inventory must mirror the JSON output: no items. + const inventory = JSON.parse( + await readFile(join(home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: unknown[] } + expect(inventory.items).toHaveLength(0) + }) + + // ------------------------------------------------------------------------- + // P1: malformed metadata (unparseable JSON, or missing required fields) + // is reported in `skipped` and does not produce inventory entries. Two + // distinct failure modes are seeded to exercise both branches in + // scanMetadata: JSON.parse throw and the post-parse field check. + // ------------------------------------------------------------------------- + test('doctor reports skipped entries for malformed and incomplete metadata', async () => { + const { home, cwd } = await createTempHome() + + // (1) Bad JSON: triggers the catch around JSON.parse → "no .skillhub/metadata.json" + // because the catch block is shared with the readFile failure path. + const badJsonDir = join(cwd, '.codex', 'skills', 'broken-json', '.skillhub') + await mkdir(badJsonDir, { recursive: true }) + await writeFile(join(badJsonDir, 'metadata.json'), '{ this is not json') + + // (2) Incomplete fields: parses fine but is missing `version`. + const incompleteDir = join(cwd, '.claude', 'skills', 'incomplete', '.skillhub') + await mkdir(incompleteDir, { recursive: true }) + await writeFile( + join(incompleteDir, 'metadata.json'), + JSON.stringify({ + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'incomplete', + // version intentionally missing + agent: 'claude-code', + installedAt: '2026-04-22T10:00:00Z' + }) + ) + + // (3) A valid sibling so we can prove skipped entries don't poison the + // surrounding scan — the valid skill should still land in inventory. + await seedSkill(cwd, { + agentDir: '.codex', + slug: 'good-skill', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'good-skill', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-22T10:00:00Z' + } + }) + + const result = await runCli(['doctor', '--json'], { + HOME: home, + USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + ok: boolean + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.ok).toBe(true) + + // Both broken entries should be in skipped, the good one in items. + const broken = json.skipped.find(s => s.path.endsWith('broken-json')) + expect(broken).toBeDefined() + const incomplete = json.skipped.find(s => s.path.endsWith('incomplete')) + expect(incomplete).toBeDefined() + expect(incomplete?.reason).toContain('incomplete') + + expect(json.itemsScanned).toBe(1) // only good-skill + const inventory = JSON.parse( + await readFile(join(home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inventory.items).toHaveLength(1) + expect(inventory.items[0]?.slug).toBe('good-skill') + }) + test('doctor backs up existing inventory.json and reports backupPath', async () => { const { home, cwd } = await createTempHome() @@ -227,4 +360,104 @@ describe('doctor command', () => { expect.arrayContaining(['external-skill', 'local-skill']) ) }) + + // ------------------------------------------------------------------------- + // P1 — Symlink safety: doctor must skip (not follow) symlinked agent / + // skill / .skillhub directories. This protects against malicious or + // accidental symlinks that would otherwise let metadata be slurped from + // arbitrary filesystem locations. + // ------------------------------------------------------------------------- + test('doctor skips an agent dir that is a symlink', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real target with a valid metadata file off in /tmp. + const realRoot = join(cwd, '__real__', '.codex', 'skills', 'pdf-parser', '.skillhub') + await mkdirP(realRoot, { recursive: true }) + await writeFile(join(realRoot, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + // Symlink ./.codex -> __real__/.codex inside cwd. Doctor scans cwd. + await symlink(join(cwd, '__real__', '.codex'), join(cwd, '.codex')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + // The symlinked agent dir must NOT contribute an inventory item. + expect(json.itemsScanned).toBe(0) + expect(json.skipped.some(s => s.path.endsWith('.codex') && s.reason.includes('regular directory'))).toBe(true) + }) + + test('doctor skips a slug dir that is a symlink (real agent dir, symlinked slug)', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real metadata under cwd/__real__/pdf-parser/.skillhub/ + const realSlug = join(cwd, '__real__', 'pdf-parser') + const realSkillhub = join(realSlug, '.skillhub') + await mkdirP(realSkillhub, { recursive: true }) + await writeFile(join(realSkillhub, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + // .codex/skills exists as a real dir, but pdf-parser inside it is a + // symlink to the real metadata location. + const skillsDir = join(cwd, '.codex', 'skills') + await mkdirP(skillsDir, { recursive: true }) + await symlink(realSlug, join(skillsDir, 'pdf-parser')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.itemsScanned).toBe(0) + const symlinked = json.skipped.find(s => s.path.endsWith('pdf-parser')) + expect(symlinked).toBeDefined() + expect(symlinked?.reason).toContain('regular directory') + }) + + test('doctor skips a .skillhub dir that is a symlink', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real metadata reachable through a symlinked .skillhub directory. + const realSkillhub = join(cwd, '__real_meta__') + await mkdirP(realSkillhub, { recursive: true }) + await writeFile(join(realSkillhub, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + const slugDir = join(cwd, '.codex', 'skills', 'pdf-parser') + await mkdirP(slugDir, { recursive: true }) + await symlink(realSkillhub, join(slugDir, '.skillhub')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.itemsScanned).toBe(0) + const skipped = json.skipped.find(s => s.path.endsWith('pdf-parser')) + expect(skipped).toBeDefined() + expect(skipped?.reason.toLowerCase()).toMatch(/skillhub|regular directory/) + }) }) diff --git a/cli/test/integration/install-command.test.ts b/cli/test/integration/install-command.test.ts index e25cdbd7c..854d8c5b7 100644 --- a/cli/test/integration/install-command.test.ts +++ b/cli/test/integration/install-command.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile } from 'node:fs/promises' +import { mkdir, readFile, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { afterEach, describe, expect, test } from 'bun:test' import { zipSync, strToU8 } from 'fflate' @@ -272,3 +272,540 @@ describe('install command — P1', () => { // test/unit/agents/resolver.test.ts. // ------------------------------------------------------------------------- }) + +// --------------------------------------------------------------------------- +// P0/P1 — Conflict & --force handling +// --------------------------------------------------------------------------- + +describe('install command — conflict and --force', () => { + test('re-installing without --force into an existing dir errors with EXIT.filesystem', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-conflict') + await mkdir(installDir, { recursive: true }) + + const first = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(first.exitCode).toBe(0) + + const second = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(second.exitCode).toBe(4) // EXIT.filesystem + expect(second.stderr).toContain('already installed') + expect(second.stderr).toContain('--force') + }) + + test('--force overwrites stale files left in the install dir', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-force') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Tamper with SKILL.md to prove the second install replaces it. + const skillFile = join(installDir, 'pdf-parser', 'SKILL.md') + await writeFile(skillFile, '# tampered content') + expect(await readFile(skillFile, 'utf-8')).toBe('# tampered content') + + const forced = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(forced.exitCode).toBe(0) + expect(await readFile(skillFile, 'utf-8')).toBe('# test skill') + }) +}) + +// --------------------------------------------------------------------------- +// P1 — Server-side error mapping during install +// --------------------------------------------------------------------------- + +describe('install command — server errors', () => { + test('resolve 404 surfaces an error and aborts install (no metadata.json written)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + failures: { resolve: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-resolve-404') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'no-such-slug', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr).toMatch(/404|not found/i) + + // No metadata file should have been created at the install destination. + const metaPath = join(installDir, 'no-such-slug', '.skillhub', 'metadata.json') + expect(await Bun.file(metaPath).exists()).toBe(false) + }) + + // Regression test for the production bug observed on 2026-05-06: server + // marks `bundle_ready=true` in DB but the bundle file is missing on disk. + // /resolve returns 200 with a downloadUrl, then /download returns 404. The + // CLI must surface a non-zero exit and a meaningful stderr — not silently + // succeed with an empty install dir. + test('download 404 (resolve OK) is reported as a download failure', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + // resolve succeeds (skill is present in fixture list) but the download + // endpoint is forced to 404 to simulate a missing bundle on storage. + skills: [{ namespace: 'global', slug: 'orphan-bundle', version: '1.0.0', zipBytes: makeSkillZip() }], + failures: { download: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-bundle-missing') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'orphan-bundle', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr.toLowerCase()).toMatch(/download|404|not found/) + }) + + // ------------------------------------------------------------------------- + // P1 — Path safety: install only writes inside // + // ------------------------------------------------------------------------- + + test('install only writes inside // — sibling files in are untouched', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'shared-dir') + await mkdir(installDir, { recursive: true }) + // Place an unrelated file as a sibling of the future / subdir. + const sibling = join(installDir, 'IMPORTANT.txt') + await writeFile(sibling, 'this file must survive install') + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + // Sibling file must still exist with original content. + expect(await readFile(sibling, 'utf-8')).toBe('this file must survive install') + // / subdir created. + expect(await Bun.file(join(installDir, 'pdf-parser', 'SKILL.md')).exists()).toBe(true) + }) + + test('--force re-install does not touch sibling files in ', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'shared-force') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // After first install, drop a sibling file; --force should not delete it. + const sibling = join(installDir, 'sibling-after-install.bin') + await writeFile(sibling, 'sentinel') + + const r2 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(0) + expect(await readFile(sibling, 'utf-8')).toBe('sentinel') + }) + + test('install --dir creates the subdir even when is empty', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'empty-dir') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(await Bun.file(join(installDir, 'pdf-parser', 'SKILL.md')).exists()).toBe(true) + expect(await Bun.file(join(installDir, 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + }) + + test('install --dir pointing at a regular file (not a directory) fails before download', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Create a file at the location --dir would otherwise treat as a directory. + const filePath = join(env.cwd, 'not-a-dir') + await writeFile(filePath, 'i am a file, not a dir') + + const result = await runCli( + ['install', 'pdf-parser', '--dir', filePath, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + // Original file must still be unchanged (the install should not have + // scribbled on it before bailing). + expect(await readFile(filePath, 'utf-8')).toBe('i am a file, not a dir') + }) + + test('--json emits a parseable error envelope when install fails', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + failures: { resolve: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-json-error') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'no-such-slug', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + // JSON error envelope is printed to stdout (or stderr, depending on the + // command); we accept either to keep the test resilient to that choice. + const candidate = result.stdout || result.stderr + const json = JSON.parse(candidate) as { + ok: boolean + message: string + exitCode: number + } + expect(json.ok).toBe(false) + expect(typeof json.message).toBe('string') + expect(json.exitCode).toBe(result.exitCode) + }) +}) + +// --------------------------------------------------------------------------- +// P1 — Multi-agent and auto-detect targeting +// --------------------------------------------------------------------------- + +describe('install command — multi-agent & auto-detect', () => { + test('multi --agent installs the same skill into every specified user-level dir', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + [ + 'install', 'pdf-parser', + '--agent', 'codex', + '--agent', 'claude-code', + '--registry', registry.url, + '--token', 'sk_ok', + '--json' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ agent: string }> } + const agents = parsed.installed.map(t => t.agent).sort() + expect(agents).toEqual(['claude-code', 'codex']) + + // Both metadata files exist on disk under user-level /./skills. + expect(await Bun.file(join(env.home, '.codex', 'skills', 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + expect(await Bun.file(join(env.home, '.claude', 'skills', 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + }) + + test('duplicate --agent dedupes to one target', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + [ + 'install', 'pdf-parser', + '--agent', 'codex', + '--agent', 'codex', + '--registry', registry.url, + '--token', 'sk_ok', + '--json' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ agent: string }> } + expect(parsed.installed).toHaveLength(1) + expect(parsed.installed[0]?.agent).toBe('codex') + }) + + test('--agent unknown-id surfaces a usage error with hint', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + ['install', 'pdf-parser', '--agent', 'totally-not-a-real-agent', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(5) // EXIT.usage + expect(result.stderr.toLowerCase()).toMatch(/unknown agent|--dir/) + }) + + test('auto-detect: cwd with only .codex/skills present installs project-level there', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Pre-create the codex skills dir so auto-detect picks project scope. + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(0) + + const parsed = JSON.parse(result.stdout) as { installed: Array<{ dir: string; agent: string }> } + expect(parsed.installed[0]?.agent).toBe('codex') + // On macOS env.cwd may resolve through /private/var/... symlinks; assert + // against the structural part of the path instead of an exact prefix. + // Use a regex that accepts both Unix (/) and Windows (\) path separators. + expect(parsed.installed[0]?.dir).toMatch(/[/\\]\.codex[/\\]skills[/\\]pdf-parser/) + expect(parsed.installed[0]?.dir).not.toContain(env.home) // not user-level + }) + + test('auto-detect: multiple agent dirs in cwd and non-interactive mode fails with hint', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + await mkdir(join(env.cwd, '.claude', 'skills'), { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(5) // EXIT.usage + expect(result.stderr.toLowerCase()).toMatch(/multiple install targets|--agent|--dir/) + }) + + test('auto-detect: cwd with no agent dirs falls back to .agents/skills', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ dir: string; agent: string }> } + expect(parsed.installed[0]?.agent).toBe('generic') + expect(parsed.installed[0]?.dir).toContain('.agents') + }) + + // ------------------------------------------------------------------------- + // P1 — Bundle integrity: download body that's not a valid zip + // ------------------------------------------------------------------------- + test('download body that is not a valid zip surfaces an extraction error', async () => { + const env = await createTempHome() + // Stand up a custom server that returns valid resolve JSON but plain + // text on download. + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + const baseUrl = `${url.protocol}//${url.host}` + const resolveMatch = url.pathname.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/([^/]+)\/resolve$/) + if (resolveMatch && req.method === 'GET') { + return Response.json({ + code: 0, + data: { + namespace: resolveMatch[1], + slug: resolveMatch[2], + version: '1.0.0', + versionId: 1, + fingerprint: 'deadbeef', + downloadUrl: `${baseUrl}/api/cli/v1/skills/${resolveMatch[1]}/${resolveMatch[2]}/versions/1.0.0/download` + } + }) + } + if (url.pathname.includes('/download')) { + // NOT a zip — plain text. + return new Response('this is plain text, not a zip', { + status: 200, headers: { 'Content-Type': 'application/zip' } + }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'bad-bundle') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + // No metadata should have been written. + expect(await Bun.file(join(installDir, 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(false) + } finally { + server.stop() + } + }) + + // ------------------------------------------------------------------------- + // P2 — Slug edge cases (Unicode, very long) + // ------------------------------------------------------------------------- + test('slug with non-ASCII characters round-trips through resolve URL (encoded)', async () => { + const env = await createTempHome() + let resolveUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.includes('/resolve')) { + resolveUrl = req.url + // Return 404 — we only care that the URL was constructed correctly. + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'unicode-slug') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', '中文-技能', '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + // Server returns 404 — install fails. Just confirm CLI didn't crash + // before hitting the server. + expect(result.exitCode).not.toBe(0) + // The slug must appear URL-percent-encoded in the resolve URL. + expect(resolveUrl).toMatch(/%E4%B8%AD%E6%96%87/) + } finally { + server.stop() + } + }) + + test('slug 200+ characters is forwarded as-is to /resolve (server is authoritative)', async () => { + const env = await createTempHome() + let resolveUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.includes('/resolve')) { + resolveUrl = req.url + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'long-slug') + await mkdir(installDir, { recursive: true }) + + const longSlug = 'a'.repeat(220) + const result = await runCli( + ['install', longSlug, '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + expect(resolveUrl).toContain(longSlug) + } finally { + server.stop() + } + }) +}) diff --git a/cli/test/integration/inventory-resilience.test.ts b/cli/test/integration/inventory-resilience.test.ts new file mode 100644 index 000000000..ac08fb7cd --- /dev/null +++ b/cli/test/integration/inventory-resilience.test.ts @@ -0,0 +1,107 @@ +/** + * inventory.json resilience. + * + * inventory.json is the local manifest of installed skills. These tests pin + * how the CLI behaves when that file is corrupt or written by overlapping + * operations: + * - list against a corrupt inventory should fail loudly (not silently) + * - install against a corrupt inventory should still complete the + * filesystem extraction even if inventory bookkeeping fails — partial + * state surfaces a clear error + * - sequential installs of distinct skills do not corrupt the manifest + * + * The unit test in test/unit/stores/inventory-store.test.ts asserts the + * lock-file recovery path. These cover the user-facing CLI surface. + */ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# test') }) +} + +describe('inventory resilience', () => { + test('list exits non-zero when inventory.json is malformed (documents current generic-error UX)', async () => { + const env = await createTempHome() + await mkdir(join(env.home, '.skillhub'), { recursive: true }) + await writeFile(join(env.home, '.skillhub', 'inventory.json'), '{ this is not JSON') + + const result = await runCli(['list'], { HOME: env.home, USERPROFILE: env.home }) + + // Contract: CLI must not crash silently or print a stack trace. It + // exits non-zero and emits a short message. + expect(result.exitCode).not.toBe(0) + expect(result.stderr.length).toBeGreaterThan(0) + expect(result.stderr.length).toBeLessThan(2000) + // Documented gap: today's message is the generic "unexpected failure" + // and does not mention `inventory` or `JSON`. When the CLI surfaces a + // more specific message in the future, tighten this assertion. + expect(result.stderr).toContain('Error') + }) + + test('list --json on a corrupt inventory emits a parseable error envelope (not a stack trace)', async () => { + const env = await createTempHome() + await mkdir(join(env.home, '.skillhub'), { recursive: true }) + await writeFile(join(env.home, '.skillhub', 'inventory.json'), '{"items":') + + const result = await runCli(['list', '--json'], { HOME: env.home, USERPROFILE: env.home }) + + expect(result.exitCode).not.toBe(0) + const candidate = result.stdout || result.stderr + expect(candidate.length).toBeLessThan(2000) + // Contract: --json error path is machine-parseable, regardless of the + // (currently generic) human message. + const json = JSON.parse(candidate) as { ok: boolean; message: string; exitCode: number } + expect(json.ok).toBe(false) + expect(typeof json.message).toBe('string') + expect(json.exitCode).toBe(result.exitCode) + }) + + test('two sequential installs of distinct slugs leave a coherent inventory', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'one', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'two', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + + const baseDir = join(env.cwd, 'pool') + await mkdir(baseDir, { recursive: true }) + + const r1 = await runCli( + ['install', 'one', '--dir', baseDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r1.exitCode).toBe(0) + + const r2 = await runCli( + ['install', 'two', '--dir', baseDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(0) + + const inventory = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + + const slugs = inventory.items.map(i => i.slug).sort() + expect(slugs).toEqual(['one', 'two']) + for (const item of inventory.items) { + expect(item.targets.length).toBeGreaterThan(0) + } + }) +}) diff --git a/cli/test/integration/list-command.test.ts b/cli/test/integration/list-command.test.ts index 2c2f95d42..da15ed42b 100644 --- a/cli/test/integration/list-command.test.ts +++ b/cli/test/integration/list-command.test.ts @@ -351,4 +351,112 @@ describe('list command', () => { expect(json.items).toHaveLength(1) expect(json.items[0].status).toBe('missing') }) + + // ------------------------------------------------------------------------- + // P1: Combined filters — --agent + --registry should narrow precisely + // ------------------------------------------------------------------------- + test('--agent codex --registry A shows only codex targets from registry A', async () => { + const { home } = await createTempHome() + + const codexA = join(home, 'a', 'codex', 'pdf') + const claudeA = join(home, 'a', 'claude', 'pdf') + const codexB = join(home, 'b', 'codex', 'pdf') + for (const d of [codexA, claudeA, codexB]) await mkdir(d, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'a', 'codex'), installDir: codexA, installedAt: INSTALLED_AT }, + { agent: 'claude-code', rootDir: join(home, 'a', 'claude'), installDir: claudeA, installedAt: INSTALLED_AT } + ] + }, + { + registry: FAKE_REGISTRY_B, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'b', 'codex'), installDir: codexB, installedAt: INSTALLED_AT } + ] + } + ]) + + const result = await runCli( + ['list', '--agent', 'codex', '--registry', FAKE_REGISTRY_A, '--json'], + { HOME: home, USERPROFILE: home } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ agent: string; installDir: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.agent).toBe('codex') + expect(json.items[0]?.installDir).toBe(codexA) + }) + + // ------------------------------------------------------------------------- + // P1: --agent + --dir should compose AND, not OR + // ------------------------------------------------------------------------- + test('--agent + --dir composes as AND: only items matching both surface', async () => { + const { home } = await createTempHome() + + const codexHere = join(home, 'here', 'codex', 'pdf') + const codexElse = join(home, 'else', 'codex', 'pdf') + await mkdir(codexHere, { recursive: true }) + await mkdir(codexElse, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'here', 'codex'), installDir: codexHere, installedAt: INSTALLED_AT } + ] + }, + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf-elsewhere', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'else', 'codex'), installDir: codexElse, installedAt: INSTALLED_AT } + ] + } + ]) + + const result = await runCli( + ['list', '--registry', FAKE_REGISTRY_A, '--agent', 'codex', '--dir', join(home, 'here'), '--json'], + { HOME: home, USERPROFILE: home } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ slug: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.slug).toBe('pdf') + }) + + // ------------------------------------------------------------------------- + // P1: SKILLHUB_REGISTRY env scopes list to the env-specified registry + // (registry priority --registry > env > config > default also applies to + // list, not just to network-touching commands). + // ------------------------------------------------------------------------- + test('SKILLHUB_REGISTRY env scopes list to that registry, hiding the other', async () => { + const { home } = await createTempHome() + const dirA = join(home, 'a', 'codex', 'one') + const dirB = join(home, 'b', 'codex', 'two') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'one', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: join(home, 'a', 'codex'), installDir: dirA, installedAt: INSTALLED_AT }] + }, + { + registry: FAKE_REGISTRY_B, namespace: 'global', slug: 'two', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: join(home, 'b', 'codex'), installDir: dirB, installedAt: INSTALLED_AT }] + } + ]) + + // No --registry flag — scope comes from SKILLHUB_REGISTRY env. + const result = await runCli( + ['list', '--json'], + { HOME: home, USERPROFILE: home, SKILLHUB_REGISTRY: FAKE_REGISTRY_B } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ slug: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.slug).toBe('two') + }) }) diff --git a/cli/test/integration/multi-registry.test.ts b/cli/test/integration/multi-registry.test.ts new file mode 100644 index 000000000..eecd563f4 --- /dev/null +++ b/cli/test/integration/multi-registry.test.ts @@ -0,0 +1,110 @@ +/** + * Multi-registry credential isolation. + * + * credentials.json keys tokens by registry URL. Operations on one registry + * must not leak into another. These tests cover: + * - Logging into A then B preserves both tokens. + * - Logging out of A leaves B's token intact. + * - whoami after logout reflects per-registry session state. + * - Re-login to A overwrites only A's slot. + */ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let regA: Awaited> | undefined +let regB: Awaited> | undefined + +afterEach(() => { + regA?.stop(); regA = undefined + regB?.stop(); regB = undefined +}) + +async function readCreds(home: string): Promise<{ tokens: Record }> { + return JSON.parse(await readFile(join(home, '.skillhub', 'credentials.json'), 'utf-8')) +} + +describe('multi-registry credential isolation', () => { + test('login to A then B leaves both tokens in credentials.json', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b', displayName: 'B' } }) + + await runCli( + ['login', '--registry', regA.url, '--token', 'sk_a'], + { HOME: env.home, USERPROFILE: env.home } + ) + await runCli( + ['login', '--registry', regB.url, '--token', 'sk_b'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a') + expect(creds.tokens[regB.url]).toBe('sk_b') + }) + + test('logout from A removes A token while B token survives', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', regA.url], { HOME: env.home, USERPROFILE: env.home }) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBeUndefined() + expect(creds.tokens[regB.url]).toBe('sk_b') + }) + + test('whoami after logout-A: A reports not-logged-in, B still authenticates', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a-user', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b-user', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', regA.url], { HOME: env.home, USERPROFILE: env.home }) + + const whoamiA = await runCli( + ['whoami', '--registry', regA.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(whoamiA.exitCode).toBe(2) // EXIT.auth + expect(whoamiA.stderr.toLowerCase()).toContain('not logged in') + + const whoamiB = await runCli( + ['whoami', '--registry', regB.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(whoamiB.exitCode).toBe(0) + expect(whoamiB.stdout).toContain('b-user') + }) + + test('re-login to A overwrites only A entry; B token unchanged', async () => { + const env = await createTempHome() + // Don't pin a token on either registry so any value passes whoami; we + // only care about credentials.json bookkeeping here. + regA = await startFakeRegistry({ user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ user: { handle: 'b', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a_old'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + { + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a_old') + expect(creds.tokens[regB.url]).toBe('sk_b') + } + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a_new'], { HOME: env.home, USERPROFILE: env.home }) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a_new') + expect(creds.tokens[regB.url]).toBe('sk_b') + }) +}) diff --git a/cli/test/integration/publish-command.test.ts b/cli/test/integration/publish-command.test.ts index 5f382fdf6..560e2f33e 100644 --- a/cli/test/integration/publish-command.test.ts +++ b/cli/test/integration/publish-command.test.ts @@ -237,3 +237,295 @@ describe('publish command — P1', () => { expect(result.stderr).toContain('registry') }) }) + +// --------------------------------------------------------------------------- +// P1 — content shape: directory layout and edge files +// --------------------------------------------------------------------------- + +import { mkdir } from 'node:fs/promises' +import { unzipSync, strFromU8 } from 'fflate' + +describe('publish command — content shape', () => { + /** + * Spin up a publish endpoint that captures the raw zip body and lets us + * inspect entries server-side. Returns the captured bytes alongside a + * stop() handle so tests can assert what the CLI actually packaged. + */ + async function startCapturingPublishServer() { + let capturedBytes: Uint8Array | null = null + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + const form = await req.formData() + const file = form.get('file') + if (file instanceof File) { + capturedBytes = new Uint8Array(await file.arrayBuffer()) + } + return Response.json({ + code: 0, + data: { namespace: 'global', slug: 'captured', version: '1.0.0', visibility: 'PUBLIC' } + }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + return { + url: `http://localhost:${server.port}`, + stop: () => server.stop(), + getCaptured: () => capturedBytes + } + } + + test('publishing a directory with subdirs packages every file at its relative path', async () => { + const env = await createTempHome() + const server = await startCapturingPublishServer() + try { + await login(env, server.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-nested-')) + await writeFile(join(dir, 'SKILL.md'), '# nested') + await mkdir(join(dir, 'references'), { recursive: true }) + await writeFile(join(dir, 'references', 'a.md'), 'aa') + await mkdir(join(dir, 'scripts'), { recursive: true }) + await writeFile(join(dir, 'scripts', 'run.sh'), '#!/bin/sh\necho ok\n') + + const result = await runCli(['publish', dir, '--registry', server.url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(0) + + const captured = server.getCaptured() + expect(captured).not.toBeNull() + const rawEntries = unzipSync(captured!) + // Normalize all entry keys to use forward slashes for cross-platform compatibility + const entries = Object.fromEntries( + Object.entries(rawEntries).map(([key, value]) => [key.replace(/\\/g, '/'), value]) + ) + // Filter out directory marker entries (zip records empty entries for + // dirs with a trailing slash); we only care about file entries. + const files = Object.keys(entries).filter(k => !k.endsWith('/')).sort() + expect(files).toEqual([ + 'SKILL.md', + 'references/a.md', + 'scripts/run.sh' + ]) + expect(strFromU8(entries['SKILL.md']!)).toBe('# nested') + expect(strFromU8(entries['references/a.md']!)).toBe('aa') + } finally { + server.stop() + } + }) + + test('publishing a directory with hidden dotfiles packages them as-is', async () => { + const env = await createTempHome() + const server = await startCapturingPublishServer() + try { + await login(env, server.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-hidden-')) + await writeFile(join(dir, 'SKILL.md'), '# h') + await writeFile(join(dir, '.DS_Store'), 'macos junk') + await writeFile(join(dir, '.editorconfig'), 'root = true\n') + + const result = await runCli(['publish', dir, '--registry', server.url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(0) + + const captured = server.getCaptured() + expect(captured).not.toBeNull() + const rawEntries = unzipSync(captured!) + // Normalize all entry keys to use forward slashes for cross-platform compatibility + const entries = Object.fromEntries( + Object.entries(rawEntries).map(([key, value]) => [key.replace(/\\/g, '/'), value]) + ) + // Pin current behavior so future filtering changes are intentional. + expect(Object.keys(entries).sort()).toEqual(['.DS_Store', '.editorconfig', 'SKILL.md']) + } finally { + server.stop() + } + }) + + test('publishing an empty directory still issues a request and reports the server outcome', async () => { + const env = await createTempHome() + // Fake registry accepts publish unconditionally; CLI is not authoritative + // on SKILL.md presence (server is). We assert only that the CLI does not + // crash client-side and exits with whatever the server returned. + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-empty-')) + + const result = await runCli(['publish', dir, '--registry', registry.url], { + HOME: env.home, USERPROFILE: env.home + }) + // Today's contract: empty dir → empty zip uploaded → server returns 200. + // If the server adds client-side or server-side validation later this + // assertion will need to flip; that's intentional and traceable. + expect(result.exitCode).toBe(0) + }) + + test('server 422 with a JSON validation body surfaces a non-zero exit and stderr', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json( + { code: 422, message: 'validation.token.name.size', errors: ['name exceeds 64 chars'] }, + { status: 422 } + ) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).not.toBe(0) + // The server's HTTP status should propagate visibly so a CI log + // shows what happened. + expect(result.stderr).toMatch(/422|registry|validation/i) + } finally { + server.stop() + } + }) + + // 502/503 are special-cased to EXIT.network because they indicate + // infrastructure-level unavailability (gateway/proxy failure). + test('server 503 Service Unavailable maps to EXIT.network with status in stderr', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 503, message: 'service unavailable' }, { status: 503 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(3) // EXIT.network + expect(result.stderr).toMatch(/503|registry/i) + } finally { + server.stop() + } + }) + + test('server 401 mid-session (token revoked) maps to EXIT.auth', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + // Whoami succeeds (login step). Publish then returns 401 as if the + // server revoked the token between the login + publish calls. + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(2) // EXIT.auth + expect(result.stderr.toLowerCase()).toMatch(/auth|401|unauthorized/) + } finally { + server.stop() + } + }) + + test('publish response missing required fields is handled without crash', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + // 200 OK but body shape doesn't match the expected schema. + return Response.json({ code: 0, data: { unexpected: true } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url, '--json'], { + HOME: env.home, USERPROFILE: env.home + }) + // Either parses with placeholder values or fails — the contract we + // want is "no crash". Pin: exit 0 means current behavior accepts + // partial responses; flip if/when stricter validation lands. + expect([0, 1, 2, 3]).toContain(result.exitCode) + // Either way, output is bounded — no stack trace dump. + expect((result.stdout + result.stderr).length).toBeLessThan(2000) + } finally { + server.stop() + } + }) + + test('server 413 Payload Too Large maps to a network-class non-zero exit', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 413, message: 'payload too large' }, { status: 413 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).not.toBe(0) + expect(result.stderr).toMatch(/413|registry/i) + } finally { + server.stop() + } + }) +}) diff --git a/cli/test/integration/remove-command.test.ts b/cli/test/integration/remove-command.test.ts index 3704d8628..35148d773 100644 --- a/cli/test/integration/remove-command.test.ts +++ b/cli/test/integration/remove-command.test.ts @@ -320,4 +320,111 @@ describe('remove command — local remove (P1)', () => { const agents = parsed.removed.map((r: { agent: string }) => r.agent).sort() expect(agents).toEqual(['claude-code', 'cursor']) }) + + // ------------------------------------------------------------------------- + // P1: --remote --hard against a slug that doesn't exist on the server + // ------------------------------------------------------------------------- + test('--remote --hard for a nonexistent slug surfaces server 404 as non-zero exit', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' } + // No skills configured → DELETE returns 404. + }) + + const result = await runCli( + [ + 'remove', 'never-published', + '--remote', '--hard', + '--namespace', 'global', + '--registry', registry.url, + '--token', 'sk_ok' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr.toLowerCase()).toMatch(/404|not found|registry returned 4/) + }) + + // ------------------------------------------------------------------------- + // P1: --agent on a multi-target inventory leaves OTHER agents' targets + // intact in the inventory file (not just in the JSON envelope). + // ------------------------------------------------------------------------- + test('--agent removes one target while leaving others in inventory.json', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + + const rootDir = `${env.home}/agents` + const codexDir = `${rootDir}/codex/skills/keep-others` + const claudeDir = `${rootDir}/claude-code/skills/keep-others` + await createInstallDir(codexDir) + await createInstallDir(claudeDir) + + await seedInventory(env.home, [ + { + registry: registry.url, + namespace: 'global', + slug: 'keep-others', + version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: `${rootDir}/codex`, installDir: codexDir, installedAt: '2026-04-20T00:00:00Z' }, + { agent: 'claude-code', rootDir: `${rootDir}/claude-code`, installDir: claudeDir, installedAt: '2026-04-20T00:00:00Z' } + ] + } + ]) + + const result = await runCli( + ['remove', 'keep-others', '--agent', 'codex', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as { + items: Array<{ slug: string; targets: Array<{ agent: string }> }> + } + const survived = inv.items.find(i => i.slug === 'keep-others') + expect(survived).toBeDefined() + expect(survived!.targets.map(t => t.agent)).toEqual(['claude-code']) + }) + + // ------------------------------------------------------------------------- + // P1: --agent + --namespace together filter precisely so a same-slug skill + // in a different namespace is not collateral damage. + // ------------------------------------------------------------------------- + test('--agent + --namespace filters precisely; same slug under different namespace is untouched', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + + const rootDir = `${env.home}/agents` + const aDir = `${rootDir}/codex/skills/dup-slug-A` + const bDir = `${rootDir}/codex/skills/dup-slug-B` + await createInstallDir(aDir) + await createInstallDir(bDir) + + await seedInventory(env.home, [ + { + registry: registry.url, namespace: 'team-a', slug: 'dup-slug-A', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: aDir, installedAt: '2026-04-20T00:00:00Z' }] + }, + { + registry: registry.url, namespace: 'team-b', slug: 'dup-slug-B', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: bDir, installedAt: '2026-04-20T00:00:00Z' }] + } + ]) + + // Remove dup-slug-A only — dup-slug-B should survive even though both + // share the codex agent. + const result = await runCli( + ['remove', 'dup-slug-A', '--agent', 'codex', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as { + items: Array<{ slug: string }> + } + const slugs = inv.items.map(i => i.slug).sort() + expect(slugs).toEqual(['dup-slug-B']) + }) }) diff --git a/cli/test/integration/search-command.test.ts b/cli/test/integration/search-command.test.ts index 2f0647838..1902142c0 100644 --- a/cli/test/integration/search-command.test.ts +++ b/cli/test/integration/search-command.test.ts @@ -103,4 +103,235 @@ describe('search command', () => { expect(result.exitCode).toBe(3) // EXIT.network expect(result.stderr).toMatch(/registry unreachable|registry returned 5\d\d/) }) + + // ------------------------------------------------------------------------- + // P2: query containing non-ASCII characters must be URL-encoded in the + // outgoing request. We capture the raw URL via a custom Bun.serve and + // assert the q parameter is the percent-encoded UTF-8 form of "中文测试". + // ------------------------------------------------------------------------- + test('non-ASCII query is URL-encoded as UTF-8 percent escapes', async () => { + let capturedUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedUrl = req.url + return Response.json({ code: 0, data: { items: [], total: 0, limit: 20 } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + const registryUrl = `http://localhost:${server.port}` + try { + const result = await runCli(['search', '中文测试', '--registry', registryUrl]) + expect(result.exitCode).toBe(0) + // UTF-8 of 中文测试 = E4 B8 AD E6 96 87 E6 B5 8B E8 AF 95 + expect(capturedUrl).toContain('q=%E4%B8%AD%E6%96%87%E6%B5%8B%E8%AF%95') + } finally { + server.stop() + } + }) + + // ------------------------------------------------------------------------- + // P2: queries containing special characters (script tags, ampersands, + // equals signs) are percent-encoded so they don't break the query string. + // ------------------------------------------------------------------------- + test('special-character query is encoded so the URL stays parseable', async () => { + let capturedUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedUrl = req.url + return Response.json({ code: 0, data: { items: [], total: 0, limit: 20 } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + const registryUrl = `http://localhost:${server.port}` + try { + const result = await runCli(['search', '