From e5f1de471725893396340e66c7a4e87faf12f24f Mon Sep 17 00:00:00 2001 From: Chris Wanstrath Date: Thu, 19 Mar 2026 23:48:34 -0700 Subject: [PATCH] Remove ListSession type and harden lock acquisition Consolidate on state.GlobalSession to eliminate redundant interface, extract clearStaleReviews for clarity, and fix withGlobalLock to explicitly throw when the lock cannot be acquired instead of silently proceeding. --- src/commands/list.ts | 42 +++++++++++++++++++++++------------------- src/state.ts | 4 ++++ 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/commands/list.ts b/src/commands/list.ts index a72ebf3..e8f8206 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -6,25 +6,20 @@ import * as vm from "../vm.ts" import * as state from "../state.ts" import { die, reset, dim, green, yellow, cyan, magenta, red } from "../fmt.ts" -interface ListSession extends state.Session { - repoRoot: string - repo?: string -} - // ── Shared rendering ───────────────────────────────────────────────── const styleDefs: [string, string, string][] = [ ["idle", dim, "◯"], ["active", cyan, "◎"], ["dirty", yellow, "◐"], - ["saved", green, "●"], ["review", magenta, "⊛"], + ["saved", green, "●"], ["review", magenta, "⦿"], ] const styles = Object.fromEntries( styleDefs.map(([k, c, ch]) => [k, { icon: `${c}${ch}${reset}`, color: c }]) ) function renderSessions( - sessions: ListSession[], + sessions: state.GlobalSession[], statuses: Record, - keyFn: (s: ListSession) => string, + keyFn: (s: state.GlobalSession) => string, ) { const branchWidth = Math.max(6, ...sessions.map(s => s.branch.length)) const cols = process.stdout.columns || 80 @@ -43,7 +38,7 @@ function renderSessions( } function renderLegend() { - console.log(`\n${dim}◯ idle${reset} · ${cyan}◎ active${reset} · ${yellow}◐ unsaved${reset} · ${green}● saved${reset} · ${magenta}⊛ review${reset}`) + console.log(`\n${dim}◯ idle${reset} · ${cyan}◎ active${reset} · ${yellow}◐ unsaved${reset} · ${green}● saved${reset} · ${magenta}⦿ review${reset}`) } // ── Shared logic ───────────────────────────────────────────────────── @@ -75,9 +70,15 @@ async function resolveAllStatuses> { const results = await Promise.all(sessions.map(s => resolveStatus(s, vmRunning))) const statuses = Object.fromEntries(sessions.map((s, i) => [keyFn(s), results[i]])) + await clearStaleReviews(sessions, results) + return statuses +} - // Clear stale reviews: in_review is set but Claude isn't actually active - // Re-load state before saving to narrow the race window with concurrent writers +/** Clear in_review flags for sessions where Claude is no longer active. */ +async function clearStaleReviews( + sessions: T[], + results: string[], +) { const staleByRepo = new Map() for (let i = 0; i < sessions.length; i++) { if (sessions[i].in_review && results[i] !== "review") { @@ -94,8 +95,6 @@ async function resolveAllStatuses {}) } - - return statuses } async function backfillPrompts(sessions: { worktree: string; prompt?: string }[], vmRunning: boolean) { @@ -130,13 +129,13 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string if (opts.remove) return actionRemove(opts.remove) // Load sessions with repoRoot attached - let sessions: ListSession[] + let sessions: state.GlobalSession[] if (opts.all) { sessions = await state.loadAll() } else { const root = await git.repoRoot() const st = await state.load(root) - sessions = Object.values(st.sessions).map(s => ({ ...s, repoRoot: root })) + sessions = Object.values(st.sessions).map(s => ({ ...s, repo: basename(root), repoRoot: root })) } const vmRunning = (await vm.status()) === "running" @@ -149,7 +148,7 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string await backfillPrompts(sessions, vmRunning) // Key by repoRoot/branch to avoid collisions across repos with the same basename - const keyFn = (s: ListSession) => `${s.repoRoot}/${s.branch}` + const keyFn = (s: state.GlobalSession) => `${s.repoRoot}/${s.branch}` const statuses = await resolveAllStatuses(sessions, vmRunning, keyFn) if (opts.json) { @@ -159,7 +158,7 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string } if (opts.all) { - const byRepo = Map.groupBy(sessions as state.GlobalSession[], s => s.repoRoot) + const byRepo = Map.groupBy(sessions, s => s.repoRoot) for (const [repoRoot, repoSessions] of byRepo) { console.log(`\n${dim}── ${reset}${basename(repoRoot)}${dim} ──${reset}`) renderSessions(repoSessions, statuses, keyFn) @@ -173,7 +172,12 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string } async function actionAdd(dir: string) { - const registered = await state.scanAndRegister(dir) + let registered: string[] + try { + registered = await state.scanAndRegister(dir) + } catch (e) { + die(e instanceof Error ? e.message : `Failed to scan ${dir}`) + } if (registered.length === 0) { console.log(`No sandlot projects found under ${dir}`) } else { @@ -185,7 +189,7 @@ async function actionAdd(dir: string) { } async function actionRemove(dir: string) { - let removed: boolean + let removed = false try { removed = await state.unregisterProject(dir) } catch { diff --git a/src/state.ts b/src/state.ts index 103ace7..36b1c22 100644 --- a/src/state.ts +++ b/src/state.ts @@ -66,9 +66,11 @@ const GLOBAL_STATE_PATH = join(GLOBAL_DIR, "registry.json") async function withGlobalLock(fn: () => Promise): Promise { const lockPath = join(GLOBAL_DIR, "registry.lock") await mkdir(GLOBAL_DIR, { recursive: true }) + let acquired = false for (let i = 0; i < 20; i++) { try { await mkdir(lockPath) + acquired = true break } catch { // If the lock is older than 5 minutes, assume it's stale (crashed process) @@ -76,6 +78,7 @@ async function withGlobalLock(fn: () => Promise): Promise { const info = await stat(lockPath) if (Date.now() - info.mtimeMs > 300_000) { await rmdir(lockPath).catch(() => {}) + await Bun.sleep(50) continue } } catch {} @@ -83,6 +86,7 @@ async function withGlobalLock(fn: () => Promise): Promise { await Bun.sleep(50) } } + if (!acquired) throw new Error("Could not acquire registry lock") try { return await fn() } finally {