Merge 22794a0f37 into a3259400a3

browse/src/cli.ts

@@ -372,12 +372,31 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
   throw new Error(`Server failed to start within ${MAX_START_WAIT / 1000}s`);
 }
 
+function errorCode(err: unknown): string {
+  if (err && typeof err === 'object' && 'code' in err) {
+    const code = (err as { code?: unknown }).code;
+    if (typeof code === 'string' && code.length > 0) return code;
+  }
+  return 'UNKNOWN';
+}
+
+function errorMessage(err: unknown): string {
+  if (err && typeof err === 'object' && 'message' in err) {
+    const message = (err as { message?: unknown }).message;
+    if (typeof message === 'string' && message.length > 0) return message;
+  }
+  return String(err);
+}
+
+function logServerLockError(action: string, lockPath: string, err: unknown): void {
+  console.error(`[browse] acquireServerLock: unexpected ${errorCode(err)} while ${action} ${lockPath}: ${errorMessage(err)}`);
+}
+
 /**
  * Acquire an exclusive lockfile to prevent concurrent ensureServer() races (TOCTOU).
  * Returns a cleanup function that releases the lock.
  */
-function acquireServerLock(): (() => void) | null {
-  const lockPath = `${config.stateFile}.lock`;
+export function acquireServerLock(lockPath: string = `${config.stateFile}.lock`): (() => void) | null {
   try {
     // 'wx' — create exclusively, fails if file already exists (atomic check-and-create)
     // Using string flag instead of numeric constants for Bun Windows compatibility
@@ -385,19 +404,36 @@ function acquireServerLock(): (() => void) | null {
     fs.writeSync(fd, `${process.pid}\n`);
     fs.closeSync(fd);
     return () => { safeUnlink(lockPath); };
-  } catch {
-    // Lock already held — check if the holder is still alive
-    try {
-      const holderPid = parseInt(fs.readFileSync(lockPath, 'utf8').trim(), 10);
-      if (holderPid && isProcessAlive(holderPid)) {
-        return null; // Another live process holds the lock
-      }
-      // Stale lock — remove and retry
-      fs.unlinkSync(lockPath);
-      return acquireServerLock();
-    } catch {
+  } catch (err) {
+    if (errorCode(err) !== 'EEXIST') {
+      logServerLockError('opening', lockPath, err);
       return null;
     }
+
+    // Lock already held — check if the holder is still alive
+    let holderPid: number;
+    try {
+      holderPid = parseInt(fs.readFileSync(lockPath, 'utf8').trim(), 10);
+    } catch (readErr) {
+      if (errorCode(readErr) === 'ENOENT') {
+        return acquireServerLock(lockPath);
+      }
+      logServerLockError('reading holder PID from', lockPath, readErr);
+      return null;
+    }
+
+    if (holderPid && isProcessAlive(holderPid)) {
+      return null; // Another live process holds the lock
+    }
+
+    // Stale lock — remove and retry
+    try {
+      fs.unlinkSync(lockPath);
+    } catch (unlinkErr) {
+      logServerLockError('removing stale', lockPath, unlinkErr);
+      return null;
+    }
+    return acquireServerLock(lockPath);
   }
 }
 

browse/test/cli-lock.test.ts

@@ -0,0 +1,79 @@
+import { describe, expect, test } from 'bun:test';
+import * as fs from 'node:fs';
+import * as os from 'node:os';
+import * as path from 'node:path';
+import { acquireServerLock } from '../src/cli';
+
+function withTempDir<T>(fn: (dir: string) => T): T {
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-lock-'));
+  try {
+    return fn(dir);
+  } finally {
+    fs.rmSync(dir, { recursive: true, force: true });
+  }
+}
+
+function captureErrors<T>(fn: () => T): { result: T; messages: string[] } {
+  const original = console.error;
+  const messages: string[] = [];
+  console.error = (...args: unknown[]) => {
+    messages.push(args.map(String).join(' '));
+  };
+  try {
+    return { result: fn(), messages };
+  } finally {
+    console.error = original;
+  }
+}
+
+describe('browse CLI server lock diagnostics (#1084)', () => {
+  test('logs non-EEXIST open failures instead of reporting phantom lock contention', () => {
+    withTempDir((dir) => {
+      const lockPath = path.join(dir, 'missing-parent', 'browse.json.lock');
+      const { result, messages } = captureErrors(() => acquireServerLock(lockPath));
+
+      expect(result).toBeNull();
+      expect(messages.join('\n')).toContain('unexpected ENOENT while opening');
+      expect(messages.join('\n')).toContain(lockPath);
+    });
+  });
+
+  test('returns null silently when a live process holds the lock', () => {
+    withTempDir((dir) => {
+      const lockPath = path.join(dir, 'browse.json.lock');
+      fs.writeFileSync(lockPath, `${process.pid}\n`);
+
+      const { result, messages } = captureErrors(() => acquireServerLock(lockPath));
+
+      expect(result).toBeNull();
+      expect(messages).toEqual([]);
+    });
+  });
+
+  test('logs holder PID read failures with code and lock path', () => {
+    withTempDir((dir) => {
+      const lockPath = path.join(dir, 'browse.json.lock');
+      fs.mkdirSync(lockPath);
+
+      const { result, messages } = captureErrors(() => acquireServerLock(lockPath));
+
+      expect(result).toBeNull();
+      expect(messages.join('\n')).toContain('unexpected EISDIR while reading holder PID from');
+      expect(messages.join('\n')).toContain(lockPath);
+    });
+  });
+
+  test('removes stale lock and reacquires it', () => {
+    withTempDir((dir) => {
+      const lockPath = path.join(dir, 'browse.json.lock');
+      fs.writeFileSync(lockPath, 'not-a-pid\n');
+
+      const release = acquireServerLock(lockPath);
+
+      expect(release).toBeFunction();
+      expect(fs.readFileSync(lockPath, 'utf-8').trim()).toBe(String(process.pid));
+      release?.();
+      expect(fs.existsSync(lockPath)).toBe(false);
+    });
+  });
+});