skillhub/cli/test/integration/concurrency.test.ts

/**
 * 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<ReturnType<typeof startFakeRegistry>> | 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()
  })
})