From 9336deed9ce29a682034d0fb405c056cff38c815 Mon Sep 17 00:00:00 2001
From: Chris Wanstrath <chris@ozmm.org>
Date: Thu, 19 Mar 2026 11:11:16 -0700
Subject: [PATCH] Remove state file locking and simplify review cleanup

The mkdir-based lock was unreliable (stale lock recovery was racy) and
added latency. The atomic rename in save() already prevents corruption,
and concurrent writes to different keys are rare enough to not warrant
the complexity. Also inlines stale review self-healing into the map
callback and collapses the review try/catch/finally into just finally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/commands/list.ts   | 14 ++++------
 src/commands/review.ts |  8 ++----
 src/state.ts           | 62 +++++++++---------------------------------
 3 files changed, 20 insertions(+), 64 deletions(-)

diff --git a/src/commands/list.ts b/src/commands/list.ts
index e9c986e..14d615c 100644
--- a/src/commands/list.ts
+++ b/src/commands/list.ts
@@ -39,13 +39,15 @@ export async function action(opts: { json?: boolean }) {
   }
 
   // Determine status for each session in parallel
-  const staleReviewSessions: state.Session[] = []
   const statusEntries = await Promise.all(
     sessions.map(async (s): Promise<[string, string]> => {
       const active = await vm.isClaudeActive(s.worktree, s.branch)
       if (active && s.in_review) return [s.branch, "review"]
-      // Collect stale in_review flags for batch self-heal below
-      if (!active && s.in_review) staleReviewSessions.push(s)
+      if (!active && s.in_review) {
+        // Self-heal stale in_review flag (fire-and-forget)
+        state.patchSession(root, s.branch, { in_review: false }).catch(() => {})
+        s.in_review = false
+      }
       if (active) return [s.branch, "active"]
       const dirty = await git.isDirty(s.worktree)
       if (dirty) return [s.branch, "dirty"]
@@ -55,12 +57,6 @@ export async function action(opts: { json?: boolean }) {
   )
   const statuses = Object.fromEntries(statusEntries)
 
-  // Self-heal stale in_review flags in parallel and update in-memory objects
-  await Promise.all(staleReviewSessions.map(async (s) => {
-    await state.patchSession(root, s.branch, { in_review: false }).catch(() => {})
-    s.in_review = false
-  }))
-
   if (opts.json) {
     const withStatus = sessions.map(s => ({ ...s, status: statuses[s.branch] }))
     console.log(JSON.stringify(withStatus, null, 2))
diff --git a/src/commands/review.ts b/src/commands/review.ts
index 66321e7..5ef95b4 100644
--- a/src/commands/review.ts
+++ b/src/commands/review.ts
@@ -80,13 +80,9 @@ Your thoughts, in brief.
       spin.succeed("Session ready")
       await vm.claude(session.worktree, { prompt })
     }
-  } catch (e) {
-    spin.stop()
-    throw e
   } finally {
-    // Clear review flag before saveChanges to minimize the race window
+    spin.stop()
     await state.patchSession(root, session.branch, { in_review: false }).catch(() => {})
-    // Save worktree changes only in interactive mode
-    if (!opts.print) await saveChanges(session.worktree, session.branch)
+    if (!opts.print) await saveChanges(session.worktree, session.branch).catch(() => {})
   }
 }
diff --git a/src/state.ts b/src/state.ts
index a9b2764..4708d26 100644
--- a/src/state.ts
+++ b/src/state.ts
@@ -1,5 +1,5 @@
 import { join, dirname } from "path"
-import { readdir, rename, mkdir, rmdir } from "fs/promises"
+import { readdir, rename } from "fs/promises"
 import { homedir } from "os"
 
 export interface Session {
@@ -36,64 +36,28 @@ export async function save(repoRoot: string, state: State): Promise<void> {
   await rename(tmpPath, path)
 }
 
-const LOCK_TIMEOUT = 5000
-const LOCK_RETRY_MS = 50
-
-async function withStateLock<T>(repoRoot: string, fn: () => Promise<T>): Promise<T> {
-  const lockPath = statePath(repoRoot) + ".lock"
-  const start = Date.now()
-
-  while (true) {
-    try {
-      await mkdir(lockPath)
-      break
-    } catch (e: any) {
-      if (e.code !== "EEXIST") throw e
-      if (Date.now() - start > LOCK_TIMEOUT) {
-        // Stale lock — force acquire
-        await rmdir(lockPath).catch(() => {})
-        await mkdir(lockPath)
-        break
-      }
-      await Bun.sleep(LOCK_RETRY_MS)
-    }
-  }
-
-  try {
-    return await fn()
-  } finally {
-    await rmdir(lockPath).catch(() => {})
-  }
-}
-
 export async function getSession(repoRoot: string, branch: string): Promise<Session | undefined> {
-  const state = await load(repoRoot)
-  return state.sessions[branch]
+  const st = await load(repoRoot)
+  return st.sessions[branch]
 }
 
 export async function setSession(repoRoot: string, session: Session): Promise<void> {
-  await withStateLock(repoRoot, async () => {
-    const state = await load(repoRoot)
-    state.sessions[session.branch] = session
-    await save(repoRoot, state)
-  })
+  const st = await load(repoRoot)
+  st.sessions[session.branch] = session
+  await save(repoRoot, st)
 }
 
 export async function patchSession(repoRoot: string, branch: string, patch: Partial<Session>): Promise<void> {
-  await withStateLock(repoRoot, async () => {
-    const state = await load(repoRoot)
-    if (!state.sessions[branch]) throw new Error(`session not found: ${branch}`)
-    Object.assign(state.sessions[branch], patch)
-    await save(repoRoot, state)
-  })
+  const st = await load(repoRoot)
+  if (!st.sessions[branch]) throw new Error(`session not found: ${branch}`)
+  Object.assign(st.sessions[branch], patch)
+  await save(repoRoot, st)
 }
 
 export async function removeSession(repoRoot: string, branch: string): Promise<void> {
-  await withStateLock(repoRoot, async () => {
-    const state = await load(repoRoot)
-    delete state.sessions[branch]
-    await save(repoRoot, state)
-  })
+  const st = await load(repoRoot)
+  delete st.sessions[branch]
+  await save(repoRoot, st)
 }
 
 export interface GlobalSession extends Session {