mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 16:03:20 +00:00
fix(status-bar): invalidate the CLI session count on kill and restart
`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 <help@stably.ai>
This commit is contained in:
@@ -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"
|
||||
]
|
||||
},
|
||||
{
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<DaemonKillAllResult>
|
||||
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)
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
@@ -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))
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user