From 11bd7dfe37bf0609fdce4d11f7d090af8019a344 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Thu, 14 May 2026 01:50:25 -0700
Subject: [PATCH] fix: pr-bug-scan findings from #1415 (re-applied on current
 main) (#1516)

Re-applies the four ResourceUsageStatusSegment fixes from #1464 on top of
current main. Original PR was branched off the #1415 merge commit and
would have silently reverted #1451 (focus right pane after clicking an
agent), which reworks navigateToTab to thread paneKey through and call
activateTabAndFocusPane.

Findings:
- Orphan kill race re-adds the killed row (await kill before refresh).
- Duplicate sessions interval inside open-gated effect (removed).
- Kill-confirm Dialog mounted inside PopoverContent (hoisted to sibling).
- daemonUnreachable required at least one snapshot failure (also treat
  null snapshot + sessions error as unreachable).

Co-authored-by: hermes <bot@hermes>
---
 .../status-bar/ResourceUsageStatusSegment.tsx | 138 ++++++++++--------
 1 file changed, 77 insertions(+), 61 deletions(-)

diff --git a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx
index 635429b091b..2908994204b 100644
--- a/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx
+++ b/src/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsx
@@ -701,15 +701,15 @@ export function ResourceUsageStatusSegment({
     }
     void fetchSnapshot()
     void refreshSessions()
+    // Why: sessions already have an always-on poll in the effect below; only
+    // the memory snapshot is gated on the popover being open. Stacking a
+    // second sessions interval here doubled IPC traffic while the popover
+    // was open.
     const memTimer = window.setInterval(() => {
       void fetchSnapshot()
     }, POLL_MS)
-    const sessTimer = window.setInterval(() => {
-      void refreshSessions()
-    }, SESSIONS_POLL_MS)
     return () => {
       window.clearInterval(memTimer)
-      window.clearInterval(sessTimer)
     }
   }, [open, fetchSnapshot, refreshSessions])
 
@@ -824,7 +824,12 @@ export function ResourceUsageStatusSegment({
     }
   }, [snapshot])
 
-  const daemonUnreachable = sessionsError && memorySnapshotError !== null
+  // Why: memorySnapshotError is null both for "last fetch succeeded" and
+  // "never fetched". When the segment is mounted but the popover hasn't
+  // been opened, fetchMemorySnapshot has never run, so a sessions IPC
+  // failure on the always-on poll would otherwise be silent. Treat the
+  // absence of any snapshot plus a sessions error as unreachable too.
+  const daemonUnreachable = sessionsError && (memorySnapshotError !== null || snapshot === null)
   // Why: a partial failure where the sessions IPC fails but the snapshot
   // IPC still works was silently invisible after the merge — the old
   // SessionsTabPanel surfaced it as "Terminal sessions unavailable". Show
@@ -911,10 +916,17 @@ export function ResourceUsageStatusSegment({
       // bulk "Kill orphan terminals" button. Bound sessions still confirm.
       if (!session.bound) {
         setSessions((prev) => prev.filter((s) => s.id !== session.sessionId))
-        void window.api.pty.kill(session.sessionId).catch(() => {
-          /* already dead */
-        })
-        void refreshSessions()
+        // Why: await the kill before refreshing — otherwise the optimistic
+        // removal races a refresh that re-reads the daemon list before the
+        // kill lands and re-adds the row that was just removed.
+        void (async () => {
+          try {
+            await window.api.pty.kill(session.sessionId)
+          } catch {
+            /* already dead */
+          }
+          await refreshSessions()
+        })()
         return
       }
       setKillConfirm(session)
@@ -1268,62 +1280,66 @@ export function ResourceUsageStatusSegment({
             </button>
           </div>
         )}
-
-        <Dialog
-          open={killConfirm !== null}
-          onOpenChange={(next) => {
-            if (next) {
-              return
-            }
+      </PopoverContent>
+      {/* Why: Radix Dialog must not be a descendant of PopoverContent — when
+          the popover unmounts (e.g. clicking outside, focus moving to the
+          confirm dialog), the Dialog unmounts mid-interaction and the kill
+          confirm flow disappears. Hoist it to a sibling so its lifetime is
+          independent of the popover. */}
+      <Dialog
+        open={killConfirm !== null}
+        onOpenChange={(next) => {
+          if (next) {
+            return
+          }
+          if (killing) {
+            return
+          }
+          setKillConfirm(null)
+        }}
+      >
+        <DialogContent
+          className="max-w-md"
+          showCloseButton={!killing}
+          onPointerDownOutside={(e) => {
             if (killing) {
-              return
+              e.preventDefault()
+            }
+          }}
+          onEscapeKeyDown={(e) => {
+            if (killing) {
+              e.preventDefault()
             }
-            setKillConfirm(null)
           }}
         >
-          <DialogContent
-            className="max-w-md"
-            showCloseButton={!killing}
-            onPointerDownOutside={(e) => {
-              if (killing) {
-                e.preventDefault()
-              }
-            }}
-            onEscapeKeyDown={(e) => {
-              if (killing) {
-                e.preventDefault()
-              }
-            }}
-          >
-            <DialogHeader>
-              <DialogTitle className="text-sm">
-                Kill{' '}
-                <span className="font-medium text-foreground">
-                  {killConfirm?.label ?? 'this session'}
-                </span>
-                ?
-              </DialogTitle>
-              <DialogDescription className="text-xs">
-                Force-quits this terminal. Any unsaved work in the pane is lost. This can&apos;t be
-                undone.
-              </DialogDescription>
-            </DialogHeader>
-            <DialogFooter>
-              <Button variant="outline" onClick={() => setKillConfirm(null)} disabled={killing}>
-                Cancel
-              </Button>
-              <Button
-                variant="destructive"
-                onClick={() => void runKillConfirmed()}
-                disabled={killing}
-              >
-                {killing ? <LoaderCircle className="size-4 animate-spin" /> : null}
-                {killing ? 'Killing…' : 'Kill session'}
-              </Button>
-            </DialogFooter>
-          </DialogContent>
-        </Dialog>
-      </PopoverContent>
+          <DialogHeader>
+            <DialogTitle className="text-sm">
+              Kill{' '}
+              <span className="font-medium text-foreground">
+                {killConfirm?.label ?? 'this session'}
+              </span>
+              ?
+            </DialogTitle>
+            <DialogDescription className="text-xs">
+              Force-quits this terminal. Any unsaved work in the pane is lost. This can&apos;t be
+              undone.
+            </DialogDescription>
+          </DialogHeader>
+          <DialogFooter>
+            <Button variant="outline" onClick={() => setKillConfirm(null)} disabled={killing}>
+              Cancel
+            </Button>
+            <Button
+              variant="destructive"
+              onClick={() => void runKillConfirmed()}
+              disabled={killing}
+            >
+              {killing ? <LoaderCircle className="size-4 animate-spin" /> : null}
+              {killing ? 'Killing…' : 'Kill session'}
+            </Button>
+          </DialogFooter>
+        </DialogContent>
+      </Dialog>
       <DaemonActionDialog api={daemonActions} />
     </Popover>
   )