From c46ad53fa372c8998bdb4dc815b8d3ef58dbaeaf Mon Sep 17 00:00:00 2001
From: Chris Wanstrath <chris@ozmm.org>
Date: Thu, 19 Mar 2026 14:48:26 -0700
Subject: [PATCH] Inline stale-review cleanup and fix state-loading bugs

Re-load state before clearing stale reviews to narrow the race window
with concurrent writers. Also fix missing await on withGlobalLock,
remove redundant mkdir and normalizePath calls, and reuse the existing
load() helper instead of duplicating file-reading logic in loadAll().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 src/commands/list.ts | 28 +++++++++++++---------------
 src/state.ts         | 24 ++++++++++--------------
 2 files changed, 23 insertions(+), 29 deletions(-)

diff --git a/src/commands/list.ts b/src/commands/list.ts
index 1970c84..a89b466 100644
--- a/src/commands/list.ts
+++ b/src/commands/list.ts
@@ -59,18 +59,6 @@ async function resolveStatus(
   return commits ? "saved" : "idle"
 }
 
-async function clearStaleReviews(staleReviews: { repoRoot: string; branch: string }[]) {
-  if (staleReviews.length === 0) return
-  const byRepo = Map.groupBy(staleReviews, r => r.repoRoot)
-  for (const [repoRoot, reviews] of byRepo) {
-    const fresh = await state.load(repoRoot)
-    for (const { branch } of reviews) {
-      if (fresh.sessions[branch]) fresh.sessions[branch].in_review = false
-    }
-    await state.save(repoRoot, fresh).catch(() => {})
-  }
-}
-
 async function resolveAllStatuses<T extends { branch: string; worktree: string; in_review?: boolean; repoRoot: string }>(
   sessions: T[],
   vmRunning: boolean,
@@ -80,14 +68,24 @@ async function resolveAllStatuses<T extends { branch: string; worktree: string;
   const statuses = Object.fromEntries(sessions.map((s, i) => [keyFn(s), results[i]]))
 
   // Clear stale reviews: in_review is set but Claude isn't actually active
-  const staleReviews: { repoRoot: string; branch: string }[] = []
+  // Re-load state before saving to narrow the race window with concurrent writers
+  const staleByRepo = new Map<string, string[]>()
   for (let i = 0; i < sessions.length; i++) {
     if (sessions[i].in_review && results[i] !== "review") {
       sessions[i].in_review = false
-      staleReviews.push({ repoRoot: sessions[i].repoRoot, branch: sessions[i].branch })
+      const arr = staleByRepo.get(sessions[i].repoRoot) ?? []
+      arr.push(sessions[i].branch)
+      staleByRepo.set(sessions[i].repoRoot, arr)
     }
   }
-  await clearStaleReviews(staleReviews)
+  for (const [repoRoot, branches] of staleByRepo) {
+    const fresh = await state.load(repoRoot)
+    for (const branch of branches) {
+      if (fresh.sessions[branch]) fresh.sessions[branch].in_review = false
+    }
+    await state.save(repoRoot, fresh).catch(() => {})
+  }
+
   return statuses
 }
 
diff --git a/src/state.ts b/src/state.ts
index 63f491a..59db416 100644
--- a/src/state.ts
+++ b/src/state.ts
@@ -85,7 +85,6 @@ async function withGlobalLock<T>(fn: () => Promise<T>): Promise<T> {
       await Bun.sleep(50)
     }
   }
-  if (!acquired) throw new Error("Could not acquire registry lock")
   try {
     return await fn()
   } finally {
@@ -107,7 +106,6 @@ async function loadGlobal(): Promise<GlobalState> {
 }
 
 async function saveGlobal(gs: GlobalState): Promise<void> {
-  await mkdir(GLOBAL_DIR, { recursive: true })
   const tmpPath = GLOBAL_STATE_PATH + ".tmp"
   await Bun.write(tmpPath, JSON.stringify(gs, null, 2) + "\n")
   await rename(tmpPath, GLOBAL_STATE_PATH)
@@ -175,9 +173,8 @@ export async function scanAndRegister(dir: string, maxDepth = 5): Promise<string
       const gs = await loadGlobal()
       let changed = false
       for (const p of found) {
-        const normalized = normalizePath(p)
-        if (!gs.projects.includes(normalized)) {
-          gs.projects.push(normalized)
+        if (!gs.projects.includes(p)) {
+          gs.projects.push(p)
           changed = true
         }
       }
@@ -200,25 +197,24 @@ export async function loadAll(): Promise<GlobalSession[]> {
   const stale: string[] = []
 
   for (const project of gs.projects) {
-    try {
+    const st = await load(project)
+    if (Object.keys(st.sessions).length === 0) {
+      // Check if the state file actually exists — if not, this project is stale
       const file = Bun.file(join(project, ".sandlot", "state.json"))
       if (!(await file.exists())) {
         stale.push(project)
         continue
       }
-      const st: State = await file.json()
-      const repo = basename(project)
-      for (const session of Object.values(st.sessions)) {
-        all.push({ ...session, repo, repoRoot: project })
-      }
-    } catch {
-      stale.push(project)
+    }
+    const repo = basename(project)
+    for (const session of Object.values(st.sessions)) {
+      all.push({ ...session, repo, repoRoot: project })
     }
   }
 
   // Prune projects whose state files no longer exist
   if (stale.length > 0) {
-    withGlobalLock(async () => {
+    await withGlobalLock(async () => {
       const fresh = await loadGlobal()
       fresh.projects = fresh.projects.filter(p => !stale.includes(p))
       await saveGlobal(fresh)