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.
This commit is contained in:
Chris Wanstrath 2026-03-19 23:48:34 -07:00
parent 3d07643547
commit e5f1de4717
2 changed files with 27 additions and 19 deletions

View File

@ -6,25 +6,20 @@ import * as vm from "../vm.ts"
import * as state from "../state.ts" import * as state from "../state.ts"
import { die, reset, dim, green, yellow, cyan, magenta, red } from "../fmt.ts" import { die, reset, dim, green, yellow, cyan, magenta, red } from "../fmt.ts"
interface ListSession extends state.Session {
repoRoot: string
repo?: string
}
// ── Shared rendering ───────────────────────────────────────────────── // ── Shared rendering ─────────────────────────────────────────────────
const styleDefs: [string, string, string][] = [ const styleDefs: [string, string, string][] = [
["idle", dim, "◯"], ["active", cyan, "◎"], ["dirty", yellow, "◐"], ["idle", dim, "◯"], ["active", cyan, "◎"], ["dirty", yellow, "◐"],
["saved", green, "●"], ["review", magenta, ""], ["saved", green, "●"], ["review", magenta, "⦿"],
] ]
const styles = Object.fromEntries( const styles = Object.fromEntries(
styleDefs.map(([k, c, ch]) => [k, { icon: `${c}${ch}${reset}`, color: c }]) styleDefs.map(([k, c, ch]) => [k, { icon: `${c}${ch}${reset}`, color: c }])
) )
function renderSessions( function renderSessions(
sessions: ListSession[], sessions: state.GlobalSession[],
statuses: Record<string, string>, statuses: Record<string, string>,
keyFn: (s: ListSession) => string, keyFn: (s: state.GlobalSession) => string,
) { ) {
const branchWidth = Math.max(6, ...sessions.map(s => s.branch.length)) const branchWidth = Math.max(6, ...sessions.map(s => s.branch.length))
const cols = process.stdout.columns || 80 const cols = process.stdout.columns || 80
@ -43,7 +38,7 @@ function renderSessions(
} }
function renderLegend() { 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 ───────────────────────────────────────────────────── // ── Shared logic ─────────────────────────────────────────────────────
@ -75,9 +70,15 @@ async function resolveAllStatuses<T extends { branch: string; worktree: string;
): Promise<Record<string, string>> { ): Promise<Record<string, string>> {
const results = await Promise.all(sessions.map(s => resolveStatus(s, vmRunning))) const results = await Promise.all(sessions.map(s => resolveStatus(s, vmRunning)))
const statuses = Object.fromEntries(sessions.map((s, i) => [keyFn(s), results[i]])) 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 /** Clear in_review flags for sessions where Claude is no longer active. */
// Re-load state before saving to narrow the race window with concurrent writers async function clearStaleReviews<T extends { branch: string; in_review?: boolean; repoRoot: string }>(
sessions: T[],
results: string[],
) {
const staleByRepo = new Map<string, string[]>() const staleByRepo = new Map<string, string[]>()
for (let i = 0; i < sessions.length; i++) { for (let i = 0; i < sessions.length; i++) {
if (sessions[i].in_review && results[i] !== "review") { if (sessions[i].in_review && results[i] !== "review") {
@ -94,8 +95,6 @@ async function resolveAllStatuses<T extends { branch: string; worktree: string;
} }
await state.save(repoRoot, fresh).catch(() => {}) await state.save(repoRoot, fresh).catch(() => {})
} }
return statuses
} }
async function backfillPrompts(sessions: { worktree: string; prompt?: string }[], vmRunning: boolean) { 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) if (opts.remove) return actionRemove(opts.remove)
// Load sessions with repoRoot attached // Load sessions with repoRoot attached
let sessions: ListSession[] let sessions: state.GlobalSession[]
if (opts.all) { if (opts.all) {
sessions = await state.loadAll() sessions = await state.loadAll()
} else { } else {
const root = await git.repoRoot() const root = await git.repoRoot()
const st = await state.load(root) 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" 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) await backfillPrompts(sessions, vmRunning)
// Key by repoRoot/branch to avoid collisions across repos with the same basename // 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) const statuses = await resolveAllStatuses(sessions, vmRunning, keyFn)
if (opts.json) { if (opts.json) {
@ -159,7 +158,7 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string
} }
if (opts.all) { 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) { for (const [repoRoot, repoSessions] of byRepo) {
console.log(`\n${dim}── ${reset}${basename(repoRoot)}${dim} ──${reset}`) console.log(`\n${dim}── ${reset}${basename(repoRoot)}${dim} ──${reset}`)
renderSessions(repoSessions, statuses, keyFn) renderSessions(repoSessions, statuses, keyFn)
@ -173,7 +172,12 @@ export async function action(opts: { json?: boolean; all?: boolean; add?: string
} }
async function actionAdd(dir: 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) { if (registered.length === 0) {
console.log(`No sandlot projects found under ${dir}`) console.log(`No sandlot projects found under ${dir}`)
} else { } else {
@ -185,7 +189,7 @@ async function actionAdd(dir: string) {
} }
async function actionRemove(dir: string) { async function actionRemove(dir: string) {
let removed: boolean let removed = false
try { try {
removed = await state.unregisterProject(dir) removed = await state.unregisterProject(dir)
} catch { } catch {

View File

@ -66,9 +66,11 @@ const GLOBAL_STATE_PATH = join(GLOBAL_DIR, "registry.json")
async function withGlobalLock<T>(fn: () => Promise<T>): Promise<T> { async function withGlobalLock<T>(fn: () => Promise<T>): Promise<T> {
const lockPath = join(GLOBAL_DIR, "registry.lock") const lockPath = join(GLOBAL_DIR, "registry.lock")
await mkdir(GLOBAL_DIR, { recursive: true }) await mkdir(GLOBAL_DIR, { recursive: true })
let acquired = false
for (let i = 0; i < 20; i++) { for (let i = 0; i < 20; i++) {
try { try {
await mkdir(lockPath) await mkdir(lockPath)
acquired = true
break break
} catch { } catch {
// If the lock is older than 5 minutes, assume it's stale (crashed process) // If the lock is older than 5 minutes, assume it's stale (crashed process)
@ -76,6 +78,7 @@ async function withGlobalLock<T>(fn: () => Promise<T>): Promise<T> {
const info = await stat(lockPath) const info = await stat(lockPath)
if (Date.now() - info.mtimeMs > 300_000) { if (Date.now() - info.mtimeMs > 300_000) {
await rmdir(lockPath).catch(() => {}) await rmdir(lockPath).catch(() => {})
await Bun.sleep(50)
continue continue
} }
} catch {} } catch {}
@ -83,6 +86,7 @@ async function withGlobalLock<T>(fn: () => Promise<T>): Promise<T> {
await Bun.sleep(50) await Bun.sleep(50)
} }
} }
if (!acquired) throw new Error("Could not acquire registry lock")
try { try {
return await fn() return await fn()
} finally { } finally {