Replace patchSession with setSession to fix concurrent state writes

The fire-and-forget patchSession calls in list could race with each
other, each reading stale state before writing. Collecting stale
branches and doing a single load-modify-save eliminates the race.
Also emits valid JSON (`[]`) when listing with --json and no sessions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Chris Wanstrath 2026-03-19 11:17:41 -07:00
parent 9336deed9c
commit d10adac712
3 changed files with 32 additions and 25 deletions

View File

@ -2,7 +2,7 @@ import { homedir } from "os"
import * as git from "../git.ts" import * as git from "../git.ts"
import * as vm from "../vm.ts" import * as vm from "../vm.ts"
import * as state from "../state.ts" import * as state from "../state.ts"
import { reset, dim, bold, green, yellow, cyan, magenta, red } from "../fmt.ts" import { reset, dim, green, yellow, cyan, magenta, red } from "../fmt.ts"
export async function action(opts: { json?: boolean }) { export async function action(opts: { json?: boolean }) {
const root = await git.repoRoot() const root = await git.repoRoot()
@ -30,22 +30,26 @@ export async function action(opts: { json?: boolean }) {
} catch {} } catch {}
} }
if (sessions.length === 0 && !opts.json) { if (sessions.length === 0) {
console.log("◆ No active sessions.") if (opts.json) {
if ((await vm.status()) !== "running") { console.log("[]")
console.log(`\n${red}VM is not running.${reset}`) } else {
console.log("◆ No active sessions.")
if ((await vm.status()) !== "running") {
console.log(`\n${red}VM is not running.${reset}`)
}
} }
return return
} }
// Determine status for each session in parallel // Determine status for each session in parallel
const staleReviewBranches: string[] = []
const statusEntries = await Promise.all( const statusEntries = await Promise.all(
sessions.map(async (s): Promise<[string, string]> => { sessions.map(async (s): Promise<[string, string]> => {
const active = await vm.isClaudeActive(s.worktree, s.branch) const active = await vm.isClaudeActive(s.worktree, s.branch)
if (active && s.in_review) return [s.branch, "review"] if (active && s.in_review) return [s.branch, "review"]
if (!active && s.in_review) { if (!active && s.in_review) {
// Self-heal stale in_review flag (fire-and-forget) staleReviewBranches.push(s.branch)
state.patchSession(root, s.branch, { in_review: false }).catch(() => {})
s.in_review = false s.in_review = false
} }
if (active) return [s.branch, "active"] if (active) return [s.branch, "active"]
@ -57,6 +61,15 @@ export async function action(opts: { json?: boolean }) {
) )
const statuses = Object.fromEntries(statusEntries) const statuses = Object.fromEntries(statusEntries)
// Batch-clear stale in_review flags in a single write
if (staleReviewBranches.length > 0) {
const freshState = await state.load(root)
for (const branch of staleReviewBranches) {
if (freshState.sessions[branch]) freshState.sessions[branch].in_review = false
}
await state.save(root, freshState).catch(() => {})
}
if (opts.json) { if (opts.json) {
const withStatus = sessions.map(s => ({ ...s, status: statuses[s.branch] })) const withStatus = sessions.map(s => ({ ...s, status: statuses[s.branch] }))
console.log(JSON.stringify(withStatus, null, 2)) console.log(JSON.stringify(withStatus, null, 2))

View File

@ -68,13 +68,13 @@ Your thoughts, in brief.
` `
if (extra) prompt += "\n\n" + extra if (extra) prompt += "\n\n" + extra
await state.patchSession(root, session.branch, { in_review: true }) session.in_review = true
await state.setSession(root, session).catch(() => {})
try { try {
if (opts.print) { if (opts.print) {
spin.text = "Running review…" spin.text = "Running review…"
const result = await vm.claude(session.worktree, { print: prompt }) const result = await vm.claude(session.worktree, { print: prompt })
spin.stop()
if (result.output) process.stdout.write(result.output + "\n") if (result.output) process.stdout.write(result.output + "\n")
} else { } else {
spin.succeed("Session ready") spin.succeed("Session ready")
@ -82,7 +82,8 @@ Your thoughts, in brief.
} }
} finally { } finally {
spin.stop() spin.stop()
await state.patchSession(root, session.branch, { in_review: false }).catch(() => {}) session.in_review = false
await state.setSession(root, session).catch(() => {})
if (!opts.print) await saveChanges(session.worktree, session.branch).catch(() => {}) if (!opts.print) await saveChanges(session.worktree, session.branch).catch(() => {})
} }
} }

View File

@ -37,27 +37,20 @@ export async function save(repoRoot: string, state: State): Promise<void> {
} }
export async function getSession(repoRoot: string, branch: string): Promise<Session | undefined> { export async function getSession(repoRoot: string, branch: string): Promise<Session | undefined> {
const st = await load(repoRoot) const state = await load(repoRoot)
return st.sessions[branch] return state.sessions[branch]
} }
export async function setSession(repoRoot: string, session: Session): Promise<void> { export async function setSession(repoRoot: string, session: Session): Promise<void> {
const st = await load(repoRoot) const state = await load(repoRoot)
st.sessions[session.branch] = session state.sessions[session.branch] = session
await save(repoRoot, st) await save(repoRoot, state)
}
export async function patchSession(repoRoot: string, branch: string, patch: Partial<Session>): Promise<void> {
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> { export async function removeSession(repoRoot: string, branch: string): Promise<void> {
const st = await load(repoRoot) const state = await load(repoRoot)
delete st.sessions[branch] delete state.sessions[branch]
await save(repoRoot, st) await save(repoRoot, state)
} }
export interface GlobalSession extends Session { export interface GlobalSession extends Session {