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>
This commit is contained in:
Neil
2026-05-14 01:50:25 -07:00
committed by GitHub
co-authored by hermes
parent f0705af998
commit 11bd7dfe37
@@ -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>
)