diff --git a/src/utils/lock.ts b/src/utils/lock.ts index 8caf7f2..bc1ec9c 100644 --- a/src/utils/lock.ts +++ b/src/utils/lock.ts @@ -91,7 +91,13 @@ function writeLockOwnerSync(lockPath: string): void { } async function removeStaleLock(lockPath: string): Promise { - const owner = await readLockOwnerDuringAcquisitionGrace(lockPath) + let owner: { pid: number } | undefined + try { + owner = await readLockOwnerDuringAcquisitionGrace(lockPath) + } catch { + // Unreadable or ambiguous owner metadata — never delete (fail closed vs mutual exclusion loss). + return false + } if (owner && isProcessAlive(owner.pid)) return false @@ -117,12 +123,14 @@ async function readLockOwnerDuringAcquisitionGrace(lockPath: string): Promise<{ async function readLockOwner(lockPath: string): Promise<{ pid: number } | undefined> { try { - const owner = JSON.parse(await readFile(join(lockPath, 'owner.json'), 'utf8')) as { pid?: unknown } + const raw = await readFile(join(lockPath, 'owner.json'), 'utf8') + const owner = JSON.parse(raw) as { pid?: unknown } return typeof owner.pid === 'number' && Number.isInteger(owner.pid) && owner.pid > 0 ? { pid: owner.pid } : undefined - } catch { - return undefined + } catch (error) { + if (isMissingPathError(error)) return undefined + throw error } } @@ -142,6 +150,12 @@ function isFileExistsError(error: unknown): boolean { ) } +function isMissingPathError(error: unknown): boolean { + const code = + typeof error === 'object' && error !== null && 'code' in error ? (error as { code?: unknown }).code : undefined + return code === 'ENOENT' || code === 'ENOTDIR' +} + export async function withResourceLock(options: ResourceLockOptions, run: () => Promise): Promise { const release = await acquireResourceLock(options) diff --git a/test/utils/lock.test.ts b/test/utils/lock.test.ts index a75630f..09a4503 100644 --- a/test/utils/lock.test.ts +++ b/test/utils/lock.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -70,6 +70,51 @@ describe('resource locks', () => { await release() }) + it('does not remove the lock directory when owner.json is unreadable', async () => { + if (process.platform === 'win32') return + + const lockPath = getResourceLockPath(['unreadable-owner']) + mkdirSync(lockPath, { recursive: true }) + const ownerPath = join(lockPath, 'owner.json') + writeFileSync(ownerPath, `${JSON.stringify({ pid: process.pid })}\n`, 'utf8') + chmodSync(ownerPath, 0o000) + + try { + await expect( + acquireResourceLock({ + resource: 'agent lifecycle', + scope: ['unreadable-owner'], + }), + ).rejects.toMatchObject({ + name: 'ResourceLockError', + }) + expect(existsSync(lockPath)).toBe(true) + } finally { + try { + chmodSync(ownerPath, 0o644) + } catch { + // ignore cleanup chmod failures + } + } + }) + + it('does not remove the lock directory when owner.json is not valid JSON', async () => { + const lockPath = getResourceLockPath(['invalid-json-owner']) + mkdirSync(lockPath, { recursive: true }) + writeFileSync(join(lockPath, 'owner.json'), '{broken\n', 'utf8') + + await expect( + acquireResourceLock({ + resource: 'agent lifecycle', + scope: ['invalid-json-owner'], + }), + ).rejects.toMatchObject({ + name: 'ResourceLockError', + }) + + expect(existsSync(lockPath)).toBe(true) + }) + it('does not delete the lock directory when owner.json appears during the acquisition grace window', async () => { const lockPath = getResourceLockPath(['grace-window']) mkdirSync(lockPath, { recursive: true })