mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-15 04:18:20 +00:00
fix(cli): use proven cross-process target locks
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
XiaoSeS
parent
73806c5e6e
commit
181e0ba076
6 changed files with 76 additions and 86 deletions
14
cli/bun.lock
14
cli/bun.lock
|
|
@ -8,12 +8,14 @@
|
|||
"cac": "^6.7.14",
|
||||
"fflate": "^0.8.2",
|
||||
"prompts": "^2.4.2",
|
||||
"proper-lockfile": "4.1.2",
|
||||
"semver": "^7.6.3",
|
||||
"zod": "^3.24.1",
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/bun": "^1.3.13",
|
||||
"@types/prompts": "^2.4.9",
|
||||
"@types/proper-lockfile": "4.1.4",
|
||||
"@types/semver": "^7.5.8",
|
||||
"@typescript-eslint/eslint-plugin": "^7.18.0",
|
||||
"@typescript-eslint/parser": "^7.18.0",
|
||||
|
|
@ -49,6 +51,10 @@
|
|||
|
||||
"@types/prompts": ["@types/prompts@2.4.9", "https://registry.npmmirror.com/@types/prompts/-/prompts-2.4.9.tgz", { "dependencies": { "@types/node": "*", "kleur": "^3.0.3" } }, "sha512-qTxFi6Buiu8+50/+3DGIWLHM6QuWsEKugJnnP6iv2Mc4ncxE4A/OJkjuVOA+5X0X1S/nq5VJRa8Lu+nwcvbrKA=="],
|
||||
|
||||
"@types/proper-lockfile": ["@types/proper-lockfile@4.1.4", "https://registry.npmmirror.com/@types/proper-lockfile/-/proper-lockfile-4.1.4.tgz", { "dependencies": { "@types/retry": "*" } }, "sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ=="],
|
||||
|
||||
"@types/retry": ["@types/retry@0.12.5", "https://registry.npmmirror.com/@types/retry/-/retry-0.12.5.tgz", {}, "sha512-3xSjTp3v03X/lSQLkczaN9UIEwJMoMCA1+Nb5HfbJEQWogdeQIyVtTvxPXDQjZ5zws8rFQfVfRdz03ARihPJgw=="],
|
||||
|
||||
"@types/semver": ["@types/semver@7.7.1", "https://registry.npmmirror.com/@types/semver/-/semver-7.7.1.tgz", {}, "sha512-FmgJfu+MOcQ370SD0ev7EI8TlCAfKYU+B4m5T3yXc1CiRN94g/SZPtsCkk506aUDtlMnFZvasDwHHUcZUEaYuA=="],
|
||||
|
||||
"@typescript-eslint/eslint-plugin": ["@typescript-eslint/eslint-plugin@7.18.0", "https://registry.npmmirror.com/@typescript-eslint/eslint-plugin/-/eslint-plugin-7.18.0.tgz", { "dependencies": { "@eslint-community/regexpp": "^4.10.0", "@typescript-eslint/scope-manager": "7.18.0", "@typescript-eslint/type-utils": "7.18.0", "@typescript-eslint/utils": "7.18.0", "@typescript-eslint/visitor-keys": "7.18.0", "graphemer": "^1.4.0", "ignore": "^5.3.1", "natural-compare": "^1.4.0", "ts-api-utils": "^1.3.0" }, "peerDependencies": { "@typescript-eslint/parser": "^7.0.0", "eslint": "^8.56.0" } }, "sha512-94EQTWZ40mzBc42ATNIBimBEDltSJ9RQHCC8vc/PDbxi4k8dVwUAv4o98dk50M1zB+JGFxp43FP7f8+FP8R6Sw=="],
|
||||
|
|
@ -163,6 +169,8 @@
|
|||
|
||||
"globby": ["globby@11.1.0", "https://registry.npmmirror.com/globby/-/globby-11.1.0.tgz", { "dependencies": { "array-union": "^2.1.0", "dir-glob": "^3.0.1", "fast-glob": "^3.2.9", "ignore": "^5.2.0", "merge2": "^1.4.1", "slash": "^3.0.0" } }, "sha512-jhIXaOzy1sb8IyocaruWSn1TjmnBVs8Ayhcy83rmxNJ8q2uWKCAj3CnJY+KpGSXCueAPc0i05kVvVKtP1t9S3g=="],
|
||||
|
||||
"graceful-fs": ["graceful-fs@4.2.11", "https://registry.npmmirror.com/graceful-fs/-/graceful-fs-4.2.11.tgz", {}, "sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ=="],
|
||||
|
||||
"graphemer": ["graphemer@1.4.0", "https://registry.npmmirror.com/graphemer/-/graphemer-1.4.0.tgz", {}, "sha512-EtKwoO6kxCL9WO5xipiHTZlSzBm7WLT627TqC/uVRd0HKmq8NXyebnNYxDoBi7wt8eTWrUrKXCOVaFq9x1kgag=="],
|
||||
|
||||
"has-flag": ["has-flag@4.0.0", "https://registry.npmmirror.com/has-flag/-/has-flag-4.0.0.tgz", {}, "sha512-EykJT/Q1KjTWctppgIAgfSO0tKVuZUjhgMr17kqTumMl6Afv3EISleU7qZUzoXDFTAHTDC4NOoG/ZxU3EvlMPQ=="],
|
||||
|
|
@ -239,12 +247,16 @@
|
|||
|
||||
"prompts": ["prompts@2.4.2", "https://registry.npmmirror.com/prompts/-/prompts-2.4.2.tgz", { "dependencies": { "kleur": "^3.0.3", "sisteransi": "^1.0.5" } }, "sha512-NxNv/kLguCA7p3jE8oL2aEBsrJWgAakBpgmgK6lpPWV+WuOmY6r2/zbAVnP+T8bQlA0nzHXSJSJW0Hq7ylaD2Q=="],
|
||||
|
||||
"proper-lockfile": ["proper-lockfile@4.1.2", "https://registry.npmmirror.com/proper-lockfile/-/proper-lockfile-4.1.2.tgz", { "dependencies": { "graceful-fs": "^4.2.4", "retry": "^0.12.0", "signal-exit": "^3.0.2" } }, "sha512-TjNPblN4BwAWMXU8s9AEz4JmQxnD1NNL7bNOY/AKUzyamc379FWASUhc/K1pL2noVb+XmZKLL68cjzLsiOAMaA=="],
|
||||
|
||||
"punycode": ["punycode@2.3.1", "https://registry.npmmirror.com/punycode/-/punycode-2.3.1.tgz", {}, "sha512-vYt7UD1U9Wg6138shLtLOvdAu+8DsC/ilFtEVHcH+wydcSpNE20AfSOduf6MkRFahL5FY7X1oU7nKVZFtfq8Fg=="],
|
||||
|
||||
"queue-microtask": ["queue-microtask@1.2.3", "https://registry.npmmirror.com/queue-microtask/-/queue-microtask-1.2.3.tgz", {}, "sha512-NuaNSa6flKT5JaSYQzJok04JzTL1CA6aGhv5rfLW3PgqA+M2ChpZQnAC8h8i4ZFkBS8X5RqkDBHA7r4hej3K9A=="],
|
||||
|
||||
"resolve-from": ["resolve-from@4.0.0", "https://registry.npmmirror.com/resolve-from/-/resolve-from-4.0.0.tgz", {}, "sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g=="],
|
||||
|
||||
"retry": ["retry@0.12.0", "https://registry.npmmirror.com/retry/-/retry-0.12.0.tgz", {}, "sha512-9LkiTwjUh6rT555DtE9rTX+BKByPfrMzEAtnlEtdEwr3Nkffwiihqe2bWADg+OQRjt9gl6ICdmB/ZFDCGAtSow=="],
|
||||
|
||||
"reusify": ["reusify@1.1.0", "https://registry.npmmirror.com/reusify/-/reusify-1.1.0.tgz", {}, "sha512-g6QUff04oZpHs0eG5p83rFLhHeV00ug/Yf9nZM6fLeUrPguBTkTQOdpAWWspMh55TZfVQDPaN3NQJfbVRAxdIw=="],
|
||||
|
||||
"rimraf": ["rimraf@3.0.2", "https://registry.npmmirror.com/rimraf/-/rimraf-3.0.2.tgz", { "dependencies": { "glob": "^7.1.3" }, "bin": { "rimraf": "bin.js" } }, "sha512-JZkJMZkAGFFPP2YqXZXPbMlMBgsxzE8ILs4lMIX/2o0L9UBw9O/Y3o6wFw/i9YLapcUJWwqbi3kdxIPdC62TIA=="],
|
||||
|
|
@ -257,6 +269,8 @@
|
|||
|
||||
"shebang-regex": ["shebang-regex@3.0.0", "https://registry.npmmirror.com/shebang-regex/-/shebang-regex-3.0.0.tgz", {}, "sha512-7++dFhtcx3353uBaq8DDR4NuxBetBzC7ZQOhmTQInHEd6bSrXdiEyzCvG07Z44UYdLShWUyXt5M/yhz8ekcb1A=="],
|
||||
|
||||
"signal-exit": ["signal-exit@3.0.7", "https://registry.npmmirror.com/signal-exit/-/signal-exit-3.0.7.tgz", {}, "sha512-wnD2ZE+l+SPC/uoS0vXeE9L1+0wuaMqKlfz9AMUo38JsyLSBWSFcHR1Rri62LZc12vLr1gb3jl7iwQhgwpAbGQ=="],
|
||||
|
||||
"sisteransi": ["sisteransi@1.0.5", "https://registry.npmmirror.com/sisteransi/-/sisteransi-1.0.5.tgz", {}, "sha512-bLGGlR1QxBcynn2d5YmDX4MGjlZvy2MRBDRNHLJ8VI6l6+9FUiyTFNJ0IveOSP0bcXgVDPRcfGqA0pjaqUpfVg=="],
|
||||
|
||||
"slash": ["slash@3.0.0", "https://registry.npmmirror.com/slash/-/slash-3.0.0.tgz", {}, "sha512-g9Q1haeby36OSStwb4ntCGGGaKsaVSjQ68fBxoQcutl5fS1vuY18H3wSt3jFyFtrkx+Kz0V1G85A4MyAdDMi2Q=="],
|
||||
|
|
|
|||
|
|
@ -48,12 +48,14 @@
|
|||
"cac": "^6.7.14",
|
||||
"fflate": "^0.8.2",
|
||||
"prompts": "^2.4.2",
|
||||
"proper-lockfile": "4.1.2",
|
||||
"semver": "^7.6.3",
|
||||
"zod": "^3.24.1"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/bun": "^1.3.13",
|
||||
"@types/prompts": "^2.4.9",
|
||||
"@types/proper-lockfile": "4.1.4",
|
||||
"@types/semver": "^7.5.8",
|
||||
"@typescript-eslint/eslint-plugin": "^7.18.0",
|
||||
"@typescript-eslint/parser": "^7.18.0",
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { createHash, randomUUID } from 'node:crypto'
|
||||
import { open, readFile, rename, rm } from 'node:fs/promises'
|
||||
import { createHash } from 'node:crypto'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { lock } from 'proper-lockfile'
|
||||
import { ensureDir } from '../platform/paths'
|
||||
import { CliError } from '../shared/errors'
|
||||
import { EXIT } from '../shared/constants'
|
||||
|
|
@ -9,61 +9,19 @@ import { EXIT } from '../shared/constants'
|
|||
/** Serializes every local lifecycle mutation for one Skill target directory. */
|
||||
export async function acquireSkillTargetLock(rootDir: string, slug: string): Promise<() => Promise<void>> {
|
||||
const lockPath = await skillTargetLockPath(rootDir, slug)
|
||||
const ownershipToken = randomUUID()
|
||||
|
||||
const createLock = async () => {
|
||||
const handle = await open(lockPath, 'wx')
|
||||
try {
|
||||
await handle.writeFile(JSON.stringify({
|
||||
pid: process.pid,
|
||||
ownershipToken,
|
||||
createdAt: new Date().toISOString()
|
||||
}))
|
||||
return handle
|
||||
} catch (error) {
|
||||
await handle.close().catch(() => {})
|
||||
await removeOwnedLock(lockPath, ownershipToken)
|
||||
throw error
|
||||
try {
|
||||
return await lock(lockPath, {
|
||||
lockfilePath: lockPath,
|
||||
realpath: false,
|
||||
stale: 10_000,
|
||||
update: 3_000,
|
||||
retries: 0
|
||||
})
|
||||
} catch (error) {
|
||||
if (error instanceof Error && 'code' in error && error.code === 'ELOCKED') {
|
||||
throw targetBusyError(rootDir, slug)
|
||||
}
|
||||
}
|
||||
|
||||
let handle: Awaited<ReturnType<typeof open>> | null = null
|
||||
for (let attempt = 0; attempt < 3 && !handle; attempt++) {
|
||||
try {
|
||||
handle = await createLock()
|
||||
break
|
||||
} catch (error) {
|
||||
if (!(error instanceof Error && 'code' in error && error.code === 'EEXIST')) throw error
|
||||
|
||||
let ownerIsRunning = true
|
||||
try {
|
||||
const lock = JSON.parse(await readFile(lockPath, 'utf-8')) as { pid?: unknown }
|
||||
if (typeof lock.pid !== 'number') throw new Error('invalid Skill target lock')
|
||||
process.kill(lock.pid, 0)
|
||||
} catch (lockError) {
|
||||
if (lockError instanceof Error && 'code' in lockError && lockError.code === 'ESRCH') {
|
||||
ownerIsRunning = false
|
||||
}
|
||||
}
|
||||
|
||||
if (ownerIsRunning) throw targetBusyError(rootDir, slug)
|
||||
|
||||
const quarantinedPath = `${lockPath}.stale-${ownershipToken}`
|
||||
try {
|
||||
await rename(lockPath, quarantinedPath)
|
||||
} catch (reclaimError) {
|
||||
if (reclaimError instanceof Error && 'code' in reclaimError && reclaimError.code === 'ENOENT') continue
|
||||
throw reclaimError
|
||||
}
|
||||
await rm(quarantinedPath, { force: true }).catch(() => {})
|
||||
}
|
||||
}
|
||||
if (!handle) throw targetBusyError(rootDir, slug)
|
||||
const ownedHandle = handle
|
||||
|
||||
return async () => {
|
||||
await ownedHandle.close().catch(() => {})
|
||||
await removeOwnedLock(lockPath, ownershipToken)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -82,13 +40,3 @@ function targetBusyError(rootDir: string, slug: string): CliError {
|
|||
next: 'wait for the other SkillHub CLI process to finish and retry'
|
||||
})
|
||||
}
|
||||
|
||||
async function removeOwnedLock(lockPath: string, ownershipToken: string): Promise<void> {
|
||||
try {
|
||||
const lock = JSON.parse(await readFile(lockPath, 'utf-8')) as { ownershipToken?: unknown }
|
||||
if (lock.ownershipToken !== ownershipToken) return
|
||||
await rm(lockPath, { force: true })
|
||||
} catch {
|
||||
// Missing or replaced locks are no longer owned by this caller.
|
||||
}
|
||||
}
|
||||
|
|
|
|||
19
cli/test/helpers/target-lock-worker.ts
Normal file
19
cli/test/helpers/target-lock-worker.ts
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
import { acquireSkillTargetLock } from '../../src/services/skill-target-lock'
|
||||
import { CliError } from '../../src/shared/errors'
|
||||
|
||||
const [rootDir, slug, holdMilliseconds] = process.argv.slice(2)
|
||||
if (!rootDir || !slug || !holdMilliseconds) process.exit(5)
|
||||
|
||||
try {
|
||||
const release = await acquireSkillTargetLock(rootDir, slug)
|
||||
process.stdout.write('acquired\n')
|
||||
await new Promise(resolve => setTimeout(resolve, Number(holdMilliseconds)))
|
||||
await release()
|
||||
process.exit(0)
|
||||
} catch (error) {
|
||||
if (error instanceof CliError) {
|
||||
process.stderr.write(`${error.message}\n`)
|
||||
process.exit(error.exitCode)
|
||||
}
|
||||
throw error
|
||||
}
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
import { access, mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from 'node:fs/promises'
|
||||
import { access, mkdir, mkdtemp, readFile, readdir, rm, symlink, utimes, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, test } from 'bun:test'
|
||||
|
|
@ -407,7 +407,7 @@ describe('installSkill', () => {
|
|||
const home = await mkdtemp(join(tmpdir(), 'skillhub-install-home-'))
|
||||
const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-install-root-'))
|
||||
const lockPath = await skillTargetLockPath(rootDir, 'demo')
|
||||
await writeFile(lockPath, JSON.stringify({ pid: process.pid }))
|
||||
await mkdir(lockPath)
|
||||
|
||||
await expect(installSkill({
|
||||
registry: 'http://registry.test', namespace: 'global', slug: 'demo',
|
||||
|
|
@ -416,7 +416,10 @@ describe('installSkill', () => {
|
|||
})).rejects.toThrow('install target is busy')
|
||||
expect(await exists(join(rootDir, 'demo'))).toBe(false)
|
||||
|
||||
await writeFile(lockPath, JSON.stringify({ pid: 999999 }))
|
||||
await rm(lockPath, { recursive: true })
|
||||
await mkdir(lockPath)
|
||||
const staleTime = new Date(Date.now() - 60_000)
|
||||
await utimes(lockPath, staleTime, staleTime)
|
||||
await installSkill({
|
||||
registry: 'http://registry.test', namespace: 'global', slug: 'demo',
|
||||
targets: [{ agent: 'codex', rootDir, scope: 'project', source: 'explicit' }],
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
import { access, writeFile } from 'node:fs/promises'
|
||||
import { mkdtemp } from 'node:fs/promises'
|
||||
import { access, mkdir, mkdtemp, utimes } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
import { acquireSkillTargetLock, skillTargetLockPath } from '../../../src/services/skill-target-lock'
|
||||
import { skillTargetLockPath } from '../../../src/services/skill-target-lock'
|
||||
|
||||
async function exists(path: string): Promise<boolean> {
|
||||
try {
|
||||
|
|
@ -15,24 +15,28 @@ async function exists(path: string): Promise<boolean> {
|
|||
}
|
||||
|
||||
describe('skill target lifecycle lock', () => {
|
||||
test('simultaneous stale recovery admits exactly one owner', async () => {
|
||||
test('simultaneous stale recovery admits exactly one owner across processes', async () => {
|
||||
const rootDir = await mkdtemp(join(tmpdir(), 'skillhub-target-lock-root-'))
|
||||
const lockPath = await skillTargetLockPath(rootDir, 'demo')
|
||||
await writeFile(lockPath, JSON.stringify({ pid: 2_147_483_647, createdAt: '2026-09-01T00:00:00Z' }))
|
||||
await mkdir(lockPath)
|
||||
const staleTime = new Date(Date.now() - 60_000)
|
||||
await utimes(lockPath, staleTime, staleTime)
|
||||
const worker = fileURLToPath(new URL('../../helpers/target-lock-worker.ts', import.meta.url))
|
||||
const bunPath = (await Bun.which('bun')) ?? process.execPath
|
||||
|
||||
const outcomes = await Promise.allSettled([
|
||||
acquireSkillTargetLock(rootDir, 'demo'),
|
||||
acquireSkillTargetLock(rootDir, 'demo')
|
||||
])
|
||||
const winners = outcomes.filter(
|
||||
(outcome): outcome is PromiseFulfilledResult<() => Promise<void>> => outcome.status === 'fulfilled'
|
||||
)
|
||||
const losers = outcomes.filter(outcome => outcome.status === 'rejected')
|
||||
const processes = [0, 1].map(() => Bun.spawn({
|
||||
cmd: [bunPath, worker, rootDir, 'demo', '1000'],
|
||||
stdout: 'pipe',
|
||||
stderr: 'pipe'
|
||||
}))
|
||||
const results = await Promise.all(processes.map(async process => ({
|
||||
exitCode: await process.exited,
|
||||
stdout: (await new Response(process.stdout).text()).trim(),
|
||||
stderr: (await new Response(process.stderr).text()).trim()
|
||||
})))
|
||||
|
||||
expect(winners).toHaveLength(1)
|
||||
expect(losers).toHaveLength(1)
|
||||
expect(await exists(lockPath)).toBe(true)
|
||||
await winners[0]!.value()
|
||||
expect(results.map(result => result.exitCode).sort()).toEqual([0, 4])
|
||||
expect(results.filter(result => result.stdout === 'acquired')).toHaveLength(1)
|
||||
expect(await exists(lockPath)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue