From 699332c16dc0da59c5841bc7fa15d54bddb79e2f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 4 Aug 2026 00:34:39 -0700 Subject: [PATCH] fix(status-bar): invalidate the CLI session count on kill and restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pty:management:killOne` / `killAll` / `restart` tear sessions down via `adapter.shutdown()` and broadcast nothing — unlike `pty:kill`, which ends in `sendPtyExitToRenderer`. The status-bar count is an event-sourced cache, so killing sessions from Manage Sessions or "Kill all terminals" left the `>_ N` chip frozen until the popover was opened, which itself triggers a refresh. > [!NOTE] > The dual-source split described in the issue text was already fixed by merged #9387. This closes a *different* remaining invalidation gap that produces the same reported symptom. Broadcast the teardown so the chip updates without needing the popover opened. Fixes #8372 Co-authored-by: Orca --- config/reliability-gates.jsonc | 5 ++- .../settings/ManageSessionsSection.tsx | 3 ++ .../shared/kill-all-terminal-surfaces.test.ts | 20 ++++++++++ .../shared/kill-all-terminal-surfaces.ts | 10 +++++ .../daemon-session-inventory-invalidation.ts | 18 +++++++++ .../use-resource-session-inventory.test.tsx | 38 ++++++++++++++++++- .../use-resource-session-inventory.ts | 12 ++++++ 7 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 src/renderer/src/components/status-bar/daemon-session-inventory-invalidation.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index ad6fdd1550c..c924467d9a4 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -4321,7 +4321,7 @@ "providers": ["local", "daemon", "ssh", "wsl", "remote-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": [], - "coverageNotes": "Platform-neutral unit coverage proves the Resource Manager closed badge performs one readiness seed, coalesces unknown spawn signals, skips known-session reattach signals, and installs no interval. Broader terminal interaction coverage remains on the pending reliability stack.", + "coverageNotes": "Platform-neutral unit coverage proves the Resource Manager closed badge performs one readiness seed, coalesces unknown spawn signals, skips known-session reattach signals, installs no interval, and re-reads only once per explicit daemon-management kill/restart (which emits no pty:exit). Broader terminal interaction coverage remains on the pending reliability stack.", "motivatingLinks": [ "https://github.com/stablyai/orca/pull/7002", "https://github.com/stablyai/orca/pull/6858", @@ -4350,7 +4350,8 @@ "spawn signals during a slow inventory read never overlap provider-wide scans and cause at most one required follow-up", "unknown sessions that exit before reconciliation cancel their queued inventory read", "unmount during a slow inventory read cannot schedule follow-up work", - "exit and out-of-order refresh races cannot resurrect stale sessions" + "exit and out-of-order refresh races cannot resurrect stale sessions", + "an explicit daemon-management invalidation performs exactly one inventory read, and none before readiness or after unmount" ] }, { diff --git a/src/renderer/src/components/settings/ManageSessionsSection.tsx b/src/renderer/src/components/settings/ManageSessionsSection.tsx index de48b535e3d..d0f39bbeec6 100644 --- a/src/renderer/src/components/settings/ManageSessionsSection.tsx +++ b/src/renderer/src/components/settings/ManageSessionsSection.tsx @@ -9,6 +9,7 @@ import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' import { useDaemonActions, DaemonActionDialog } from '../shared/useDaemonActions' import { ManageSessionKillDialog } from './ManageSessionKillDialog' import { ManageSessionsTable } from './ManageSessionsTable' +import { notifyDaemonSessionInventoryInvalidated } from '../status-bar/daemon-session-inventory-invalidation' import { translate } from '@/i18n/i18n' type ConfirmKind = 'killOne' @@ -122,6 +123,7 @@ export function ManageSessionsSection(): React.JSX.Element { void refresh() }, onRestartSettled: () => { + notifyDaemonSessionInventoryInvalidated() void refresh() } }) @@ -150,6 +152,7 @@ export function ManageSessionsSection(): React.JSX.Element { ) } mutationInFlight.current = false + notifyDaemonSessionInventoryInvalidated() await refresh() } catch (err) { toast.error( diff --git a/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts b/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts index 2ec53edce39..35b77f6fcb2 100644 --- a/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts +++ b/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts @@ -339,6 +339,26 @@ describe('runKillAllTerminalSurfaces', () => { expect(killPty).not.toHaveBeenCalled() }) + it('invalidates cached session inventories once the daemon sweep settles either way', async () => { + const notifyInventoryInvalidated = vi.fn() + + await runKillAllTerminalSurfaces([], { + getState: () => state(), + killDaemonSessions: vi.fn().mockResolvedValue({ killedCount: 3, remainingCount: 0 }), + notifyInventoryInvalidated, + reportSummary: vi.fn() + }) + expect(notifyInventoryInvalidated).toHaveBeenCalledTimes(1) + + await runKillAllTerminalSurfaces([], { + getState: () => state(), + killDaemonSessions: vi.fn().mockRejectedValue(new Error('daemon gone')), + notifyInventoryInvalidated, + reportSummary: vi.fn() + }) + expect(notifyInventoryInvalidated).toHaveBeenCalledTimes(2) + }) + it('uses one daemon management call and never inventories sessions afterward', async () => { const killAll = vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }) const listSessions = vi.fn() diff --git a/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts b/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts index 8d280ba3f73..311b69c3592 100644 --- a/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts +++ b/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts @@ -9,6 +9,7 @@ import { type TerminalTabRetirementState } from '@/store/slices/terminal-tab-retirement' import { reserveTerminalRetirementTeardowns } from '@/store/slices/terminal-retirement-teardown-reservation' +import { notifyDaemonSessionInventoryInvalidated } from '../status-bar/daemon-session-inventory-invalidation' const CLOSE_BATCH_SIZE = 2 @@ -42,6 +43,7 @@ export type KillAllTerminalSurfacesSummary = { type KillAllTerminalSurfaceDependencies = { getState: () => KillAllTerminalSurfaceState killDaemonSessions: () => Promise + notifyInventoryInvalidated: () => void closeSurface: ( tabId: string, options: { @@ -127,6 +129,7 @@ function createDefaultDependencies(): KillAllTerminalSurfaceDependencies { return { getState: useAppStore.getState, killDaemonSessions: () => window.api.pty.management.killAll(), + notifyInventoryInvalidated: notifyDaemonSessionInventoryInvalidated, closeSurface: closeTerminalTab, killPty: (ptyId) => window.api.pty.kill(ptyId), now: () => globalThis.performance?.now() ?? Date.now(), @@ -157,6 +160,13 @@ export async function runKillAllTerminalSurfaces( } catch { daemon = { status: 'rejected' } } + try { + // Why: the management sweep ends sessions without per-PTY pty:exit events, + // so cached inventories (status-bar badge) must be told to re-read. + deps.notifyInventoryInvalidated() + } catch { + // A stale badge must not abort the destructive action already in progress. + } const cleanupState = deps.getState() const remainingTargetIds = new Set(targetIds) diff --git a/src/renderer/src/components/status-bar/daemon-session-inventory-invalidation.ts b/src/renderer/src/components/status-bar/daemon-session-inventory-invalidation.ts new file mode 100644 index 00000000000..b63ff78ed2f --- /dev/null +++ b/src/renderer/src/components/status-bar/daemon-session-inventory-invalidation.ts @@ -0,0 +1,18 @@ +// Why: daemon-session kills routed through pty:management:* emit no pty:exit, +// so the status-bar inventory needs an explicit signal to stop showing a stale count. +const listeners = new Set<() => void>() + +export function subscribeDaemonSessionInventoryInvalidated(listener: () => void): () => void { + listeners.add(listener) + return () => { + listeners.delete(listener) + } +} + +export function notifyDaemonSessionInventoryInvalidated(): void { + // Copy so a listener unsubscribing during dispatch cannot skip its neighbours. + const snapshot = [...listeners] + for (const listener of snapshot) { + listener() + } +} diff --git a/src/renderer/src/components/status-bar/use-resource-session-inventory.test.tsx b/src/renderer/src/components/status-bar/use-resource-session-inventory.test.tsx index 936f83c3178..5112e613672 100644 --- a/src/renderer/src/components/status-bar/use-resource-session-inventory.test.tsx +++ b/src/renderer/src/components/status-bar/use-resource-session-inventory.test.tsx @@ -1,7 +1,8 @@ // @vitest-environment happy-dom -import { act, renderHook, waitFor } from '@testing-library/react' +import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { DaemonSession } from './resource-usage-merge-types' +import { notifyDaemonSessionInventoryInvalidated } from './daemon-session-inventory-invalidation' import { useResourceSessionInventory } from './use-resource-session-inventory' function session(id: string): DaemonSession { @@ -45,6 +46,8 @@ describe('useResourceSessionInventory', () => { }) afterEach(() => { + // Unmount leftover hooks so their module-level invalidation subscriptions cannot bleed into the next test. + cleanup() delete (window as unknown as { api?: unknown }).api }) @@ -254,6 +257,39 @@ describe('useResourceSessionInventory', () => { expect(result.current.sessionInventory.count).toBe(2) }) + it('re-reads the inventory once when a management kill invalidates it without a pty exit', async () => { + listSessions + .mockResolvedValueOnce([session('one'), session('two')]) + .mockResolvedValue([session('one')]) + const { result } = renderHook(() => useResourceSessionInventory(true)) + await waitFor(() => expect(result.current.sessionInventory.count).toBe(2)) + + await act(async () => { + notifyDaemonSessionInventoryInvalidated() + }) + + await waitFor(() => expect(result.current.sessionInventory.count).toBe(1)) + expect(listSessions).toHaveBeenCalledTimes(2) + }) + + it('ignores inventory invalidation before session restore is ready and after unmount', async () => { + listSessions.mockResolvedValue([session('one')]) + const { unmount } = renderHook(({ ready }) => useResourceSessionInventory(ready), { + initialProps: { ready: false } + }) + + await act(async () => { + notifyDaemonSessionInventoryInvalidated() + }) + expect(listSessions).not.toHaveBeenCalled() + + unmount() + await act(async () => { + notifyDaemonSessionInventoryInvalidated() + }) + expect(listSessions).not.toHaveBeenCalled() + }) + it('unsubscribes from lifecycle events on unmount', () => { listSessions.mockResolvedValue([]) const { unmount } = renderHook(() => useResourceSessionInventory(true)) diff --git a/src/renderer/src/components/status-bar/use-resource-session-inventory.ts b/src/renderer/src/components/status-bar/use-resource-session-inventory.ts index dc0b5febc1f..dd28e4b0398 100644 --- a/src/renderer/src/components/status-bar/use-resource-session-inventory.ts +++ b/src/renderer/src/components/status-bar/use-resource-session-inventory.ts @@ -1,5 +1,6 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { useMountedRef } from '@/hooks/useMountedRef' +import { subscribeDaemonSessionInventoryInvalidated } from './daemon-session-inventory-invalidation' import { EMPTY_DAEMON_SESSION_INVENTORY, inventoryFromSessions, @@ -123,6 +124,17 @@ export function useResourceSessionInventory(ready: boolean): ResourceSessionInve void refreshSessions() }, [ready, refreshSessions]) + useEffect(() => { + if (!ready) { + return + } + // Why: management kills/restarts destroy daemon sessions without a pty:exit, + // so the closed badge would keep the pre-kill count until the popover opens. + return subscribeDaemonSessionInventoryInvalidated(() => { + void refreshSessions() + }) + }, [ready, refreshSessions]) + useEffect(() => { if (!ready) { return