From f036d0cc8a5f41815bc91d4b73b0676193edb512 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Apr 2026 11:53:00 +1000 Subject: [PATCH 1/2] fix(staged): prevent double review row when adopting already-running autoreview Check session status before attempting resume in tryAdoptAutoReview. If the session is already running, register it on the frontend instead of calling resumeSession (which rejects already-running sessions). Move setReviewAuto after fallible operations so the review stays hidden if adoption fails, preventing the revealed-autoreview + new-session double row. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../BranchCardSessionManager.svelte.ts | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index fa4ffdbc2..2c29737bd 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -186,25 +186,34 @@ export default class BranchCardSessionManager { const review = await commands.findFreshAutoReview(branch.id); if (!review) return false; - await commands.setReviewAuto(review.id, false); - if (this.autoReviewSessionId) { + // We're tracking the session locally — register it before revealing sessionRegistry.register(this.autoReviewSessionId, branch.projectId, 'review', branch.id); projectStateStore.addRunningSession(branch.projectId, this.autoReviewSessionId); + } else if (!review.completedAt && review.sessionId) { + // The autoreview has a session we're not tracking. Check its status + // to decide whether to resume or just register it. + const session = await commands.getSession(review.sessionId); + if (session && session.status === 'running') { + // Session is already running (e.g. agent connected but frontend + // lost track) — just register it, no resume needed. + sessionRegistry.register(review.sessionId, branch.projectId, 'review', branch.id); + projectStateStore.addRunningSession(branch.projectId, review.sessionId); + } else { + // Session exists but isn't running — resume it + await commands.resumeSession( + review.sessionId, + 'Continue reviewing the code changes on this branch.', + undefined, + branch.id + ); + sessionRegistry.register(review.sessionId, branch.projectId, 'review', branch.id); + projectStateStore.addRunningSession(branch.projectId, review.sessionId); + } } - // If the autoreview was interrupted before completing, resume it - const needsResume = !review.completedAt && review.sessionId && !this.autoReviewSessionId; - if (needsResume) { - await commands.resumeSession( - review.sessionId!, - 'Continue reviewing the code changes on this branch.', - undefined, - branch.id - ); - sessionRegistry.register(review.sessionId!, branch.projectId, 'review', branch.id); - projectStateStore.addRunningSession(branch.projectId, review.sessionId!); - } + // Only reveal the review after all fallible operations succeed + await commands.setReviewAuto(review.id, false); this.autoReviewSessionId = null; this.autoReviewId = null; From a50ebc47c123d2106c0cc55fef97493147855ca9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 16 Apr 2026 13:00:42 +1000 Subject: [PATCH 2/2] refactor(staged): extract registerRunningSession helper to reduce duplication Extract a private registerRunningSession() method that encapsulates the paired sessionRegistry.register + projectStateStore.addRunningSession calls. This pattern appeared in 4 places (3 in tryAdoptAutoReview, 1 in startBranchSessionWithPendingItem) and was flagged in code review as easy to get out of sync if either call changed. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../BranchCardSessionManager.svelte.ts | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 2c29737bd..aed767613 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -166,6 +166,17 @@ export default class BranchCardSessionManager { } } + /** Register a session on the frontend and mark it as running. */ + private registerRunningSession( + sessionId: string, + projectId: string, + mode: BranchSessionType, + branchId: string + ) { + sessionRegistry.register(sessionId, projectId, mode, branchId); + projectStateStore.addRunningSession(projectId, sessionId); + } + cancelAutoReview() { if (this.autoReviewSessionId) { commands.cancelSession(this.autoReviewSessionId).catch(() => {}); @@ -188,8 +199,12 @@ export default class BranchCardSessionManager { if (this.autoReviewSessionId) { // We're tracking the session locally — register it before revealing - sessionRegistry.register(this.autoReviewSessionId, branch.projectId, 'review', branch.id); - projectStateStore.addRunningSession(branch.projectId, this.autoReviewSessionId); + this.registerRunningSession( + this.autoReviewSessionId, + branch.projectId, + 'review', + branch.id + ); } else if (!review.completedAt && review.sessionId) { // The autoreview has a session we're not tracking. Check its status // to decide whether to resume or just register it. @@ -197,8 +212,7 @@ export default class BranchCardSessionManager { if (session && session.status === 'running') { // Session is already running (e.g. agent connected but frontend // lost track) — just register it, no resume needed. - sessionRegistry.register(review.sessionId, branch.projectId, 'review', branch.id); - projectStateStore.addRunningSession(branch.projectId, review.sessionId); + this.registerRunningSession(review.sessionId, branch.projectId, 'review', branch.id); } else { // Session exists but isn't running — resume it await commands.resumeSession( @@ -207,8 +221,7 @@ export default class BranchCardSessionManager { undefined, branch.id ); - sessionRegistry.register(review.sessionId, branch.projectId, 'review', branch.id); - projectStateStore.addRunningSession(branch.projectId, review.sessionId); + this.registerRunningSession(review.sessionId, branch.projectId, 'review', branch.id); } } @@ -263,8 +276,7 @@ export default class BranchCardSessionManager { throw new Error('Failed to start session: no session ID returned'); } - sessionRegistry.register(result.sessionId, branch.projectId, mode, branch.id); - projectStateStore.addRunningSession(branch.projectId, result.sessionId); + this.registerRunningSession(result.sessionId, branch.projectId, mode, branch.id); this.pendingSessionItems = this.pendingSessionItems.map((item) => item.key === pendingKey