From e876895185f24fec33b99ab871133588ed17ead0 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Fri, 3 Jul 2026 11:55:21 -0300 Subject: [PATCH 1/2] fix(state): wire PID lock into mutating commands (#148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Improved acquireLock error message to be clearer: 'another sharekit process is running (PID ) — retry after it finishes, or delete ~/.sharekit/.lock if stale' - Wired lock acquisition/release into: install, update, rollback, uninstall - Lock is acquired at function entry (after initial checks) and released in finally block (guarantees release on error) - Read-only commands (preview, inspect, list, search, scan, init) remain unlocked per design - Added comprehensive lock tests: live-PID detection, stale-lock cleanup, successful release, directory creation - All gates pass: typecheck, test, format:check --- src/commands.ts | 293 +++++++++++++++++++++++++--------------------- src/sharekit.ts | 9 +- src/state.ts | 17 ++- test/lock.test.ts | 123 +++++++++++++++++++ 4 files changed, 303 insertions(+), 139 deletions(-) create mode 100644 test/lock.test.ts diff --git a/src/commands.ts b/src/commands.ts index 8433a21..0fb9b79 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -7,7 +7,14 @@ import kleur from 'kleur'; import { parseUserRef, fetchProfile, readManifest } from './fetch.js'; import { plan, printPlan, isExecutable, applyProfile } from './plan.js'; import { restoreBackup, listBackups, restoreBackupToStamp, parseApplied } from './backup.js'; -import { recordInstall, readInstalled, list, isImmutableRef } from './state.js'; +import { + recordInstall, + readInstalled, + list, + isImmutableRef, + acquireLock, + releaseLock, +} from './state.js'; import { scanForSecrets, printAndGateFindings, type Finding } from './scanner.js'; import { walk, tildify, Dirs, DEFAULT_DIRS, cp, rootsFor } from './paths.js'; @@ -158,6 +165,8 @@ export async function update( opts?: InstallOpts, dirs: Dirs = DEFAULT_DIRS ): Promise { + const lockPath = path.join(dirs.state, '.lock'); + acquireLock(lockPath); try { const includeHooks = opts?.includeHooks ?? false; const yes = opts?.yes ?? false; @@ -248,10 +257,13 @@ export async function update( ); console.log(kleur.dim(` Undo: sharekit rollback ${user}\n`)); } finally { + releaseLock(lockPath); } } export async function install(user: string, opts?: InstallOpts): Promise { + const lockPath = path.join(DEFAULT_DIRS.state, '.lock'); + acquireLock(lockPath); try { const includeHooks = opts?.includeHooks ?? false; const { user: userName, ref: userRef } = parseUserRef(user); @@ -316,6 +328,7 @@ export async function install(user: string, opts?: InstallOpts): Promise { } console.log(); } finally { + releaseLock(lockPath); } } @@ -366,68 +379,74 @@ export async function inspect(user: string): Promise { export async function rollback(user: string, opts?: InstallOpts): Promise { const HOME = os.homedir(); const STATE = path.join(HOME, '.sharekit'); + const lockPath = path.join(STATE, '.lock'); - const yes = opts?.yes ?? false; - const dryRun = opts?.dryRun ?? false; + acquireLock(lockPath); + try { + const yes = opts?.yes ?? false; + const dryRun = opts?.dryRun ?? false; - const root = path.join(STATE, 'backups'); - const last = fs.existsSync(root) - ? fs - .readdirSync(root) - .filter((e) => e.startsWith(user + '-')) - .sort() - .pop() - : undefined; - if (!last) return void console.log(kleur.yellow(`No backup for ${user}.`)); + const root = path.join(STATE, 'backups'); + const last = fs.existsSync(root) + ? fs + .readdirSync(root) + .filter((e) => e.startsWith(user + '-')) + .sort() + .pop() + : undefined; + if (!last) return void console.log(kleur.yellow(`No backup for ${user}.`)); + + const dir = path.join(root, last); + + // Read and parse applied.json with safe error handling + let applied: { dest: string; status: string }[]; + const appliedPath = path.join(dir, 'applied.json'); + try { + const rawData = fs.readFileSync(appliedPath, 'utf8'); + applied = parseApplied(rawData); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Backup data is corrupt or unreadable: ${msg}`); + } - const dir = path.join(root, last); + let versionStr = ''; + const metadataPath = path.join(dir, 'metadata.json'); + if (fs.existsSync(metadataPath)) { + try { + const metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf8')); + if (metadata.sourceVersion) versionStr = ` (v${metadata.sourceVersion})`; + } catch { + // If metadata can't be read, just continue without version info + } + } - // Read and parse applied.json with safe error handling - let applied: { dest: string; status: string }[]; - const appliedPath = path.join(dir, 'applied.json'); - try { - const rawData = fs.readFileSync(appliedPath, 'utf8'); - applied = parseApplied(rawData); - } catch (error) { - const msg = error instanceof Error ? error.message : String(error); - throw new Error(`Backup data is corrupt or unreadable: ${msg}`); - } + console.log(kleur.bold(`\n Rollback ${user}${versionStr} (${applied.length} file(s))\n`)); + if (!(await confirm('Restore?', yes))) return void console.log(kleur.dim('\n Aborted.\n')); - let versionStr = ''; - const metadataPath = path.join(dir, 'metadata.json'); - if (fs.existsSync(metadataPath)) { - try { - const metadata = JSON.parse(fs.readFileSync(metadataPath, 'utf8')); - if (metadata.sourceVersion) versionStr = ` (v${metadata.sourceVersion})`; - } catch { - // If metadata can't be read, just continue without version info + if (dryRun) { + console.log(kleur.cyan(`\n (dry-run — no files restored)`)); + console.log(kleur.green(`\n ✓ Would restore ${applied.length} file(s).`)); + console.log(); + return; } - } - console.log(kleur.bold(`\n Rollback ${user}${versionStr} (${applied.length} file(s))\n`)); - if (!(await confirm('Restore?', yes))) return void console.log(kleur.dim('\n Aborted.\n')); + const metadata = restoreBackup(user); + const summary = `${metadata.filesRestored} file(s) restored${ + metadata.filesRemoved > 0 ? `, ${metadata.filesRemoved} removed` : '' + }`; + // Handle null sourceCommit (offline cache case) + let versionSuffix = ''; + if (metadata.sourceCommit === null) { + versionSuffix = ' — from offline cache, exact version unknown'; + } else if (metadata.sourceVersion) { + versionSuffix = ` (reverted to v${metadata.sourceVersion})`; + } - if (dryRun) { - console.log(kleur.cyan(`\n (dry-run — no files restored)`)); - console.log(kleur.green(`\n ✓ Would restore ${applied.length} file(s).`)); + console.log(kleur.green(`\n ✓ ${summary}${versionSuffix}`)); console.log(); - return; - } - - const metadata = restoreBackup(user); - const summary = `${metadata.filesRestored} file(s) restored${ - metadata.filesRemoved > 0 ? `, ${metadata.filesRemoved} removed` : '' - }`; - // Handle null sourceCommit (offline cache case) - let versionSuffix = ''; - if (metadata.sourceCommit === null) { - versionSuffix = ' — from offline cache, exact version unknown'; - } else if (metadata.sourceVersion) { - versionSuffix = ` (reverted to v${metadata.sourceVersion})`; + } finally { + releaseLock(lockPath); } - - console.log(kleur.green(`\n ✓ ${summary}${versionSuffix}`)); - console.log(); } export async function uninstall( @@ -435,103 +454,111 @@ export async function uninstall( dirs: Dirs = DEFAULT_DIRS, force = false ): Promise { - const installed = readInstalled(dirs); - const record = installed[user]; + const lockPath = path.join(dirs.state, '.lock'); + acquireLock(lockPath); + try { + const installed = readInstalled(dirs); + const record = installed[user]; - if (!record) { - throw new Error(`${user} is not installed.`); - } + if (!record) { + throw new Error(`${user} is not installed.`); + } - // Find the latest backup for this user - const root = path.join(dirs.state, 'backups'); - const last = fs.existsSync(root) - ? fs - .readdirSync(root) - .filter((e) => e.startsWith(user + '-')) - .sort() - .pop() - : undefined; - - if (!last) { - throw new Error(`No backup found for ${user}. Cannot uninstall without restore information.`); - } + // Find the latest backup for this user + const root = path.join(dirs.state, 'backups'); + const last = fs.existsSync(root) + ? fs + .readdirSync(root) + .filter((e) => e.startsWith(user + '-')) + .sort() + .pop() + : undefined; + + if (!last) { + throw new Error(`No backup found for ${user}. Cannot uninstall without restore information.`); + } - const backupDir = path.join(root, last); + const backupDir = path.join(root, last); - // Read and parse applied.json with safe error handling - let applied: { dest: string; status: string }[]; - const appliedPath = path.join(backupDir, 'applied.json'); - try { - const rawData = fs.readFileSync(appliedPath, 'utf8'); - applied = parseApplied(rawData); - } catch (error) { - const msg = error instanceof Error ? error.message : String(error); - throw new Error(`Backup data is corrupt or unreadable: ${msg}`); - } + // Read and parse applied.json with safe error handling + let applied: { dest: string; status: string }[]; + const appliedPath = path.join(backupDir, 'applied.json'); + try { + const rawData = fs.readFileSync(appliedPath, 'utf8'); + applied = parseApplied(rawData); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + throw new Error(`Backup data is corrupt or unreadable: ${msg}`); + } - // Print what will be removed/restored - const toRemove = applied.filter((a) => a.status === 'new'); - const toRestore = applied.filter((a) => a.status === 'changed'); + // Print what will be removed/restored + const toRemove = applied.filter((a) => a.status === 'new'); + const toRestore = applied.filter((a) => a.status === 'changed'); - console.log(); - console.log(kleur.bold(` Uninstall ${user}${record.version ? ` (v${record.version})` : ''}\n`)); + console.log(); + console.log( + kleur.bold(` Uninstall ${user}${record.version ? ` (v${record.version})` : ''}\n`) + ); - if (toRemove.length > 0) { - console.log(kleur.red(` - remove (${toRemove.length})`)); - for (const a of toRemove) { - console.log(kleur.red(` ${tildify(a.dest)}`)); + if (toRemove.length > 0) { + console.log(kleur.red(` - remove (${toRemove.length})`)); + for (const a of toRemove) { + console.log(kleur.red(` ${tildify(a.dest)}`)); + } } - } - if (toRestore.length > 0) { - console.log(kleur.yellow(`\n ~ restore (${toRestore.length})`)); - for (const a of toRestore) { - console.log(kleur.yellow(` ${tildify(a.dest)}`)); + if (toRestore.length > 0) { + console.log(kleur.yellow(`\n ~ restore (${toRestore.length})`)); + for (const a of toRestore) { + console.log(kleur.yellow(` ${tildify(a.dest)}`)); + } } - } - console.log(); - if (!force && !(await confirm(`Remove ${user}?`))) { - return void console.log(kleur.dim('\n Aborted.\n')); - } + console.log(); + if (!force && !(await confirm(`Remove ${user}?`))) { + return void console.log(kleur.dim('\n Aborted.\n')); + } - // Resolve home directory once for bounds checking - const resolvedHome = path.resolve(dirs.home); + // Resolve home directory once for bounds checking + const resolvedHome = path.resolve(dirs.home); - // Execute the uninstall: reverse all changes - for (const a of applied) { - // Bounds-check: ensure dest is within dirs.home - const resolvedDest = path.resolve(a.dest); - if (!resolvedDest.startsWith(resolvedHome + path.sep)) { - console.warn(`Skipping out-of-bounds entry: ${a.dest}`); - continue; - } + // Execute the uninstall: reverse all changes + for (const a of applied) { + // Bounds-check: ensure dest is within dirs.home + const resolvedDest = path.resolve(a.dest); + if (!resolvedDest.startsWith(resolvedHome + path.sep)) { + console.warn(`Skipping out-of-bounds entry: ${a.dest}`); + continue; + } - if (a.status === 'new') { - // File was added by the profile — remove it - fs.rmSync(resolvedDest, { force: true }); - } else if (a.status === 'changed') { - // File was changed — restore from backup - const src = path.join(backupDir, path.relative(resolvedHome, resolvedDest)); - if (fs.existsSync(src)) { - fs.mkdirSync(path.dirname(resolvedDest), { recursive: true }); - cp(src, resolvedDest); + if (a.status === 'new') { + // File was added by the profile — remove it + fs.rmSync(resolvedDest, { force: true }); + } else if (a.status === 'changed') { + // File was changed — restore from backup + const src = path.join(backupDir, path.relative(resolvedHome, resolvedDest)); + if (fs.existsSync(src)) { + fs.mkdirSync(path.dirname(resolvedDest), { recursive: true }); + cp(src, resolvedDest); + } } } - } - // Remove user from installed.json atomically (#122) - delete installed[user]; - const stateFile = path.join(dirs.state, 'installed.json'); - const tmp = stateFile + '.tmp'; - fs.writeFileSync(tmp, JSON.stringify(installed, null, 2)); - fs.renameSync(tmp, stateFile); - - const summary = `${toRemove.length} file(s) removed${ - toRestore.length > 0 ? `, ${toRestore.length} restored` : '' - }`; - console.log(kleur.green(`\n ✓ Uninstalled ${user}. ${summary}`)); - console.log(); + // Remove user from installed.json atomically (#122) + delete installed[user]; + const stateFile = path.join(dirs.state, 'installed.json'); + const tmp = stateFile + '.tmp'; + fs.writeFileSync(tmp, JSON.stringify(installed, null, 2)); + fs.renameSync(tmp, stateFile); + + const summary = `${toRemove.length} file(s) removed${ + toRestore.length > 0 ? `, ${toRestore.length} restored` : '' + }`; + console.log(kleur.green(`\n ✓ Uninstalled ${user}. ${summary}`)); + console.log(); + } finally { + releaseLock(lockPath); + } } export async function scan(dir?: string, force = false): Promise { diff --git a/src/sharekit.ts b/src/sharekit.ts index bf69cde..c81c3ac 100644 --- a/src/sharekit.ts +++ b/src/sharekit.ts @@ -39,7 +39,14 @@ export { export type { BackupInfo, RestoreMetadata } from './backup.js'; // From state.ts -export { recordInstall, readInstalled, list, isImmutableRef } from './state.js'; +export { + recordInstall, + readInstalled, + list, + isImmutableRef, + acquireLock, + releaseLock, +} from './state.js'; export type { InstallRecord } from './state.js'; // From commands.ts diff --git a/src/state.ts b/src/state.ts index c757a2a..cda82d0 100644 --- a/src/state.ts +++ b/src/state.ts @@ -104,16 +104,23 @@ export function acquireLock(lockPath: string = LOCK_FILE): void { const existingPid = fs.readFileSync(lockPath, 'utf8').trim(); try { fs.statSync(`/proc/${existingPid}`); - console.error( - `Another sharekit operation is running (PID ${existingPid}). Try again in a moment.` + throw new Error( + `another sharekit process is running (PID ${existingPid}) — retry after it finishes, or delete ${lockPath} if stale` ); - process.exit(1); - } catch { + } catch (e) { + // If /proc/ doesn't exist, the process is dead — clean up stale lock + if (e instanceof Error && e.message.includes('another sharekit process')) { + throw e; // Re-throw the "process still running" error + } try { fs.unlinkSync(lockPath); } catch {} } - } catch { + } catch (e) { + // If reading the lock file itself fails, try to clean it up + if (e instanceof Error && e.message.includes('another sharekit process')) { + throw e; // Re-throw the "process still running" error + } try { fs.unlinkSync(lockPath); } catch {} diff --git a/test/lock.test.ts b/test/lock.test.ts new file mode 100644 index 0000000..ecd856b --- /dev/null +++ b/test/lock.test.ts @@ -0,0 +1,123 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { acquireLock, releaseLock } from '../src/state.js'; + +// Test 1: acquireLock against a live-PID lock file errors with clear message +// Skip on systems without /proc (e.g., macOS) +test('acquireLock throws when another process holds the lock', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, '.lock'); + + // Write the current process's PID to the lock file + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, process.pid.toString()); + + // Try to acquire the same lock + let errorThrown = false; + let errorMessage = ''; + try { + acquireLock(lockPath); + } catch (e) { + errorThrown = true; + errorMessage = e instanceof Error ? e.message : String(e); + } + + // On systems with /proc (Linux), acquireLock should throw + // On systems without /proc (macOS), it will clean up the stale lock and succeed + if (fs.existsSync('/proc')) { + assert.ok(errorThrown, 'acquireLock should throw when lock is held by live process'); + assert.ok( + errorMessage.includes('another sharekit process is running'), + 'error should mention another process is running' + ); + assert.ok(errorMessage.includes(process.pid.toString()), 'error should include the PID'); + assert.ok(errorMessage.includes('retry'), 'error should suggest retrying'); + assert.ok(errorMessage.includes('delete'), 'error should suggest deleting the lock file'); + } else { + // On macOS (no /proc), the lock will be cleaned up as if stale and acquisition should succeed + assert.ok(!errorThrown, 'acquireLock should succeed on systems without /proc'); + assert.ok(fs.existsSync(lockPath), 'lock file should exist after successful acquisition'); + } + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +// Test 2: stale lock (dead PID) is cleaned and acquisition succeeds +test('acquireLock cleans up stale lock and acquires successfully', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, '.lock'); + + // Write a dead PID (very unlikely to exist) to the lock file + const deadPid = '999999999'; + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, deadPid); + + // Try to acquire the lock — should succeed (cleaning up stale lock) + acquireLock(lockPath); + + // Verify that the lock file now contains the current process PID + assert.ok(fs.existsSync(lockPath), 'lock file should exist'); + const content = fs.readFileSync(lockPath, 'utf8').trim(); + assert.equal(content, process.pid.toString(), 'lock file should contain current PID'); + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +// Test 3: release lock removes the lock file +test('releaseLock removes the lock file', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, '.lock'); + + // Create a lock file + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, process.pid.toString()); + assert.ok(fs.existsSync(lockPath), 'lock file should exist before release'); + + // Release the lock + releaseLock(lockPath); + + // Verify that the lock file is gone + assert.ok(!fs.existsSync(lockPath), 'lock file should be removed after release'); + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +// Test 4: releaseLock silently succeeds even if lock file doesn't exist +test('releaseLock succeeds even if lock file does not exist', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, '.lock'); + + // Don't create the lock file, just try to release + assert.doesNotThrow(() => { + releaseLock(lockPath); + }, 'releaseLock should not throw if lock file does not exist'); + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +}); + +// Test 5: acquireLock creates lock directory if it doesn't exist +test('acquireLock creates the lock directory if needed', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, 'subdir', '.lock'); + + // Don't create the directory beforehand + assert.ok(!fs.existsSync(path.dirname(lockPath)), 'lock directory should not exist yet'); + + // Acquire the lock + acquireLock(lockPath); + + // Verify that the directory and file were created + assert.ok(fs.existsSync(lockPath), 'lock file should be created'); + const content = fs.readFileSync(lockPath, 'utf8').trim(); + assert.equal(content, process.pid.toString(), 'lock file should contain current PID'); + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +}); From e4c1e8d42d901b144d8d85520c736c7a0e3a2c55 Mon Sep 17 00:00:00 2001 From: Lucas Santana Date: Fri, 3 Jul 2026 11:58:56 -0300 Subject: [PATCH 2/2] fix(state): use portable process.kill(pid, 0) for PID liveness check Replace /proc/ check (Linux-only, breaks on macOS) with process.kill(pid, 0) which works cross-platform: - EPERM: process is alive but not owned by us - ESRCH: process is dead - Other errors: treat as dead Also handle non-numeric PIDs as stale (corrupted lock files). Update test suite: - Live-PID test now unconditionally throws on all platforms - Added test for non-numeric/corrupted lock content - All tests pass on both macOS and Linux --- src/state.ts | 37 +++++++++++++++++++++++---------- test/lock.test.ts | 52 +++++++++++++++++++++++++++++------------------ 2 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/state.ts b/src/state.ts index cda82d0..c361fd0 100644 --- a/src/state.ts +++ b/src/state.ts @@ -97,30 +97,45 @@ export function isImmutableRef(ref: string): boolean { const LOCK_FILE = path.join(DEFAULT_DIRS.state, '.lock'); +/** + * Portable cross-platform PID liveness check using process.kill(pid, 0). + * Signal 0 doesn't send a signal; it only checks if the process exists and is accessible. + * - EPERM: process exists but we don't own it (return true — alive) + * - ESRCH: process doesn't exist (return false — dead) + * - Other errors: treat as dead + */ +function isPidAlive(pidStr: string): boolean { + const pid = Number(pidStr); + if (!Number.isInteger(pid) || pid <= 0) return false; + try { + process.kill(pid, 0); + return true; + } catch (e) { + const code = (e as NodeJS.ErrnoException).code; + return code === 'EPERM'; // EPERM = alive but not owned by us; ESRCH = dead + } +} + export function acquireLock(lockPath: string = LOCK_FILE): void { const pid = process.pid.toString(); if (fs.existsSync(lockPath)) { try { const existingPid = fs.readFileSync(lockPath, 'utf8').trim(); - try { - fs.statSync(`/proc/${existingPid}`); + if (isPidAlive(existingPid)) { throw new Error( `another sharekit process is running (PID ${existingPid}) — retry after it finishes, or delete ${lockPath} if stale` ); - } catch (e) { - // If /proc/ doesn't exist, the process is dead — clean up stale lock - if (e instanceof Error && e.message.includes('another sharekit process')) { - throw e; // Re-throw the "process still running" error - } - try { - fs.unlinkSync(lockPath); - } catch {} } + // Process is dead — clean up stale lock + try { + fs.unlinkSync(lockPath); + } catch {} } catch (e) { - // If reading the lock file itself fails, try to clean it up + // If checking liveness or reading the lock file fails, check if it's our error if (e instanceof Error && e.message.includes('another sharekit process')) { throw e; // Re-throw the "process still running" error } + // Otherwise try to clean up the lock file try { fs.unlinkSync(lockPath); } catch {} diff --git a/test/lock.test.ts b/test/lock.test.ts index ecd856b..ae52488 100644 --- a/test/lock.test.ts +++ b/test/lock.test.ts @@ -5,17 +5,16 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { acquireLock, releaseLock } from '../src/state.js'; -// Test 1: acquireLock against a live-PID lock file errors with clear message -// Skip on systems without /proc (e.g., macOS) +// Test 1: acquireLock throws when lock is held by a live process (portable across macOS/Linux) test('acquireLock throws when another process holds the lock', () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); const lockPath = path.join(tmp, '.lock'); - // Write the current process's PID to the lock file + // Write the current process's PID to the lock file (this process is definitely alive) fs.mkdirSync(path.dirname(lockPath), { recursive: true }); fs.writeFileSync(lockPath, process.pid.toString()); - // Try to acquire the same lock + // Try to acquire the same lock — should fail let errorThrown = false; let errorMessage = ''; try { @@ -25,22 +24,14 @@ test('acquireLock throws when another process holds the lock', () => { errorMessage = e instanceof Error ? e.message : String(e); } - // On systems with /proc (Linux), acquireLock should throw - // On systems without /proc (macOS), it will clean up the stale lock and succeed - if (fs.existsSync('/proc')) { - assert.ok(errorThrown, 'acquireLock should throw when lock is held by live process'); - assert.ok( - errorMessage.includes('another sharekit process is running'), - 'error should mention another process is running' - ); - assert.ok(errorMessage.includes(process.pid.toString()), 'error should include the PID'); - assert.ok(errorMessage.includes('retry'), 'error should suggest retrying'); - assert.ok(errorMessage.includes('delete'), 'error should suggest deleting the lock file'); - } else { - // On macOS (no /proc), the lock will be cleaned up as if stale and acquisition should succeed - assert.ok(!errorThrown, 'acquireLock should succeed on systems without /proc'); - assert.ok(fs.existsSync(lockPath), 'lock file should exist after successful acquisition'); - } + assert.ok(errorThrown, 'acquireLock should throw when lock is held by live process'); + assert.ok( + errorMessage.includes('another sharekit process is running'), + 'error should mention another process is running' + ); + assert.ok(errorMessage.includes(process.pid.toString()), 'error should include the PID'); + assert.ok(errorMessage.includes('retry'), 'error should suggest retrying'); + assert.ok(errorMessage.includes('delete'), 'error should suggest deleting the lock file'); // Cleanup fs.rmSync(tmp, { recursive: true, force: true }); @@ -121,3 +112,24 @@ test('acquireLock creates the lock directory if needed', () => { // Cleanup fs.rmSync(tmp, { recursive: true, force: true }); }); + +// Test 6: acquireLock treats non-numeric PID as stale and cleans up +test('acquireLock treats non-numeric lock content as stale', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'sk-lock-')); + const lockPath = path.join(tmp, '.lock'); + + // Write non-numeric content (corrupted lock file) + fs.mkdirSync(path.dirname(lockPath), { recursive: true }); + fs.writeFileSync(lockPath, 'corrupted-pid-data'); + + // Try to acquire the lock — should succeed (cleaning up corrupted lock) + acquireLock(lockPath); + + // Verify that the lock file now contains the current process PID + assert.ok(fs.existsSync(lockPath), 'lock file should exist'); + const content = fs.readFileSync(lockPath, 'utf8').trim(); + assert.equal(content, process.pid.toString(), 'lock file should contain current PID'); + + // Cleanup + fs.rmSync(tmp, { recursive: true, force: true }); +});