From 69ba73b3c389c8c1a99ea54c99311dbcd14d5ab0 Mon Sep 17 00:00:00 2001 From: Chris Wanstrath Date: Thu, 19 Mar 2026 10:56:50 -0700 Subject: [PATCH] Use patchSession to avoid race conditions in review flag updates The load-modify-save pattern could overwrite concurrent state changes. patchSession does an atomic read-patch-write, and the list command now re-checks activity before clearing stale flags to avoid racing with a review that just started. Co-Authored-By: Claude Opus 4.6 --- src/commands/list.ts | 14 +++++--------- src/commands/review.ts | 14 ++++---------- src/state.ts | 8 ++++++++ 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/commands/list.ts b/src/commands/list.ts index a08eb9b..2c6586c 100644 --- a/src/commands/list.ts +++ b/src/commands/list.ts @@ -59,15 +59,11 @@ export async function action(opts: { json?: boolean }) { ) const statuses = Object.fromEntries(statusEntries) - // Batch self-heal stale in_review flags — reload fresh state to avoid overwriting concurrent changes - if (staleReviewSessions.length > 0) { - for (const s of staleReviewSessions) s.in_review = false - const fresh = await state.load(root).catch(() => null) - if (fresh) { - for (const s of staleReviewSessions) { - if (fresh.sessions[s.branch]) fresh.sessions[s.branch].in_review = false - } - await state.save(root, fresh).catch(() => {}) + // Self-heal stale in_review flags — re-check activity to avoid racing with a concurrent review start + for (const s of staleReviewSessions) { + const stillActive = await vm.isClaudeActive(s.worktree, s.branch) + if (!stillActive) { + await state.patchSession(root, s.branch, { in_review: false }).catch(() => {}) } } diff --git a/src/commands/review.ts b/src/commands/review.ts index 5e0b4f5..4798d1a 100644 --- a/src/commands/review.ts +++ b/src/commands/review.ts @@ -68,14 +68,12 @@ Your thoughts, in brief. ` if (extra) prompt += "\n\n" + extra - session.in_review = true - await state.setSession(root, session) + await state.patchSession(root, session.branch, { in_review: true }) try { if (opts.print) { spin.text = "Running review…" const result = await vm.claude(session.worktree, { print: prompt }) - spin.stop() if (result.output) process.stdout.write(result.output + "\n") } else { spin.succeed("Session ready") @@ -83,13 +81,9 @@ Your thoughts, in brief. } } finally { spin.stop() + // Clear review flag before saveChanges to minimize the race window + 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).catch(() => {}) - // Patch only in_review on fresh state to avoid overwriting concurrent changes - const current = await state.load(root).catch(() => null) - if (current?.sessions[session.branch]) { - current.sessions[session.branch].in_review = false - await state.save(root, current).catch(() => {}) - } + if (!opts.print) await saveChanges(session.worktree, session.branch) } } diff --git a/src/state.ts b/src/state.ts index f212ec5..f30e827 100644 --- a/src/state.ts +++ b/src/state.ts @@ -47,6 +47,14 @@ export async function setSession(repoRoot: string, session: Session): Promise): Promise { + const state = await load(repoRoot) + if (state.sessions[branch]) { + Object.assign(state.sessions[branch], patch) + await save(repoRoot, state) + } +} + export async function removeSession(repoRoot: string, branch: string): Promise { const state = await load(repoRoot) delete state.sessions[branch]