mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(renderer): dispose global listeners during HMR (#20908)
* fix(renderer): dispose combined diff cache listener on HMR * fix(renderer): dispose contextual tour key guard on HMR * fix(renderer): dispose activity pagehide listener on HMR * fix(renderer): dispose keyboard layout hooks on HMR * fix(renderer): dispose input quiet listeners on HMR * fix(renderer): dispose desync sentinel listener on HMR * fix: address memory PR review regressions and withdraw false positives --------- Co-authored-by: m4air <m4air@Mac.localdomain> Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
This commit is contained in:
@@ -0,0 +1,177 @@
|
||||
// @vitest-environment happy-dom
|
||||
import { afterEach, beforeEach, expect, it, vi } from 'vitest'
|
||||
import type { RetainedAgentEntry } from '@/store/slices/agent-status'
|
||||
import type { ActivityEvent, AgentPaneThread } from './activity-thread-types'
|
||||
import { makeTab, makeWorktree } from './ActivityPrototypePage-test-fixtures'
|
||||
|
||||
const mockStore = vi.hoisted(() => {
|
||||
const activityClearedAtByPaneKey: Record<string, number> = {}
|
||||
const agentStatusByPaneKey: Record<string, RetainedAgentEntry['entry']> = {}
|
||||
const retainedAgentsByPaneKey: Record<string, RetainedAgentEntry> = {}
|
||||
const retentionSuppressedPaneKeys: Record<string, true> = {}
|
||||
const state = {
|
||||
activityClearedAtByPaneKey,
|
||||
agentStatusByPaneKey,
|
||||
retainedAgentsByPaneKey,
|
||||
retentionSuppressedPaneKeys,
|
||||
applyActivityClearedAt: vi.fn((patch: Record<string, number | null>) => {
|
||||
const next = { ...state.activityClearedAtByPaneKey }
|
||||
for (const [key, value] of Object.entries(patch)) {
|
||||
if (value === null) {
|
||||
delete next[key]
|
||||
} else {
|
||||
next[key] = value
|
||||
}
|
||||
}
|
||||
state.activityClearedAtByPaneKey = next
|
||||
}),
|
||||
dismissRetainedAgents: vi.fn((paneKeys: readonly string[]) => {
|
||||
const next = { ...state.retainedAgentsByPaneKey }
|
||||
for (const key of paneKeys) {
|
||||
if (state.agentStatusByPaneKey[key]) {
|
||||
state.retentionSuppressedPaneKeys[key] = true
|
||||
}
|
||||
delete next[key]
|
||||
}
|
||||
state.retainedAgentsByPaneKey = next
|
||||
}),
|
||||
clearRetentionSuppressedPaneKeys: vi.fn((paneKeys: string[]) => {
|
||||
for (const key of paneKeys) {
|
||||
delete state.retentionSuppressedPaneKeys[key]
|
||||
}
|
||||
}),
|
||||
retainAgents: vi.fn((entries: RetainedAgentEntry[]) => {
|
||||
const next = { ...state.retainedAgentsByPaneKey }
|
||||
for (const retained of entries) {
|
||||
next[retained.entry.paneKey] = retained
|
||||
}
|
||||
state.retainedAgentsByPaneKey = next
|
||||
})
|
||||
}
|
||||
return state
|
||||
})
|
||||
|
||||
const toastSpy = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/store', () => ({
|
||||
useAppStore: { getState: () => mockStore }
|
||||
}))
|
||||
vi.mock('sonner', () => ({ toast: toastSpy }))
|
||||
|
||||
import {
|
||||
CLEAR_COMPLETED_EVICTION_FALLBACK_MS,
|
||||
disposePendingClearCompletedEvictionListener,
|
||||
clearCompletedActivity,
|
||||
flushPendingClearCompletedEvictions
|
||||
} from './activity-clear-completed'
|
||||
|
||||
function makeThread(paneKey: string, overrides: Partial<AgentPaneThread> = {}): AgentPaneThread {
|
||||
return {
|
||||
paneKey,
|
||||
tab: makeTab(),
|
||||
worktree: makeWorktree(),
|
||||
repo: null,
|
||||
currentAgentState: null,
|
||||
currentAgentEntry: null,
|
||||
latestEvent: null,
|
||||
latestTimestamp: 5_000,
|
||||
agentType: 'claude',
|
||||
unread: false,
|
||||
paneTitle: `Agent ${paneKey}`,
|
||||
responsePreview: '',
|
||||
events: [],
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
function doneEvent(interrupted: boolean): ActivityEvent {
|
||||
return {
|
||||
id: 'evt',
|
||||
state: 'done',
|
||||
timestamp: 5_000,
|
||||
worktree: makeWorktree(),
|
||||
repo: null,
|
||||
entry: { ...makeRetained('t-done:1').entry, interrupted },
|
||||
tab: makeTab(),
|
||||
agentType: 'claude',
|
||||
agentAlive: false,
|
||||
unread: false
|
||||
}
|
||||
}
|
||||
|
||||
const doneThread = makeThread('t-done:1', { latestEvent: doneEvent(false) })
|
||||
|
||||
function makeRetained(paneKey: string): RetainedAgentEntry {
|
||||
return {
|
||||
entry: {
|
||||
state: 'done',
|
||||
prompt: 'retained run',
|
||||
updatedAt: 5_000,
|
||||
stateStartedAt: 5_000,
|
||||
paneKey,
|
||||
stateHistory: [],
|
||||
agentType: 'claude'
|
||||
},
|
||||
worktreeId: 'wt-1',
|
||||
tab: makeTab(),
|
||||
agentType: 'claude',
|
||||
startedAt: 5_000
|
||||
}
|
||||
}
|
||||
|
||||
const drop = vi.fn()
|
||||
const domWindow = window
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
Object.assign(domWindow, { api: { agentStatus: { dropPersistedBatch: drop } } })
|
||||
mockStore.retainedAgentsByPaneKey = { 't-done:1': makeRetained('t-done:1') }
|
||||
mockStore.activityClearedAtByPaneKey = {}
|
||||
mockStore.retentionSuppressedPaneKeys = {}
|
||||
})
|
||||
afterEach(() => {
|
||||
flushPendingClearCompletedEvictions()
|
||||
vi.useRealTimers()
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('preserves the pending pagehide flush through HMR and removes the listener when drained', () => {
|
||||
const remove = vi.spyOn(domWindow, 'removeEventListener')
|
||||
try {
|
||||
clearCompletedActivity([doneThread])
|
||||
disposePendingClearCompletedEvictionListener()
|
||||
expect(remove).not.toHaveBeenCalledWith('pagehide', flushPendingClearCompletedEvictions)
|
||||
domWindow.dispatchEvent(new Event('pagehide'))
|
||||
expect(drop).toHaveBeenCalledOnce()
|
||||
expect(remove).toHaveBeenCalledWith('pagehide', flushPendingClearCompletedEvictions)
|
||||
} finally {
|
||||
remove.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves Undo after HMR without dropping the restored persisted row', () => {
|
||||
clearCompletedActivity([doneThread])
|
||||
disposePendingClearCompletedEvictionListener()
|
||||
const options = toastSpy.mock.calls.at(-1)?.[1]
|
||||
options.action.onClick()
|
||||
domWindow.dispatchEvent(new Event('pagehide'))
|
||||
vi.advanceTimersByTime(CLEAR_COMPLETED_EVICTION_FALLBACK_MS)
|
||||
expect(drop).not.toHaveBeenCalled()
|
||||
expect(mockStore.retainedAgentsByPaneKey['t-done:1']).toBeDefined()
|
||||
})
|
||||
|
||||
it('releases the retired listener when its fallback settles and supports a late stale handler', () => {
|
||||
const remove = vi.spyOn(domWindow, 'removeEventListener')
|
||||
try {
|
||||
clearCompletedActivity([doneThread])
|
||||
disposePendingClearCompletedEvictionListener()
|
||||
vi.advanceTimersByTime(CLEAR_COMPLETED_EVICTION_FALLBACK_MS)
|
||||
expect(drop).toHaveBeenCalledOnce()
|
||||
expect(remove).toHaveBeenCalledWith('pagehide', flushPendingClearCompletedEvictions)
|
||||
mockStore.retainedAgentsByPaneKey = { 't-done:1': makeRetained('t-done:1') }
|
||||
clearCompletedActivity([doneThread])
|
||||
domWindow.dispatchEvent(new Event('pagehide'))
|
||||
expect(drop).toHaveBeenCalledTimes(2)
|
||||
} finally {
|
||||
remove.mockRestore()
|
||||
}
|
||||
})
|
||||
@@ -68,16 +68,34 @@ export function planClearCompletedActivity(
|
||||
// Deferred evictions whose undo toast is still open; flushed on pagehide because the toast's
|
||||
// close callbacks never fire on quit/reload, which would let cleared rows replay next launch.
|
||||
const pendingDiskEvictions = new Set<() => void>()
|
||||
let evictionListenerRetired = false
|
||||
export function flushPendingClearCompletedEvictions(): void {
|
||||
// Set iteration tolerates the self-delete each evict() performs.
|
||||
for (const evict of pendingDiskEvictions) {
|
||||
evict()
|
||||
}
|
||||
}
|
||||
export function disposePendingClearCompletedEvictionListener(): void {
|
||||
evictionListenerRetired = true
|
||||
releaseRetiredEvictionListener()
|
||||
}
|
||||
|
||||
function releaseRetiredEvictionListener(): void {
|
||||
if (evictionListenerRetired && pendingDiskEvictions.size === 0 && typeof window !== 'undefined') {
|
||||
window.removeEventListener('pagehide', flushPendingClearCompletedEvictions)
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
window.addEventListener('pagehide', flushPendingClearCompletedEvictions)
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// pagehide hook so dev sessions do not retain stale eviction closures.
|
||||
import.meta.hot.dispose(disposePendingClearCompletedEvictionListener)
|
||||
}
|
||||
|
||||
// Why a fallback: sonner only fires onDismiss/onAutoClose for the toast's own close paths; a
|
||||
// `toast.dismiss()` from another caller leaves the eviction pending until pagehide.
|
||||
export const CLEAR_COMPLETED_EVICTION_FALLBACK_MS = 60_000
|
||||
@@ -143,6 +161,7 @@ export function clearCompletedActivity(threads: readonly AgentPaneThread[]): boo
|
||||
let fallbackTimer: ReturnType<typeof setTimeout> | null = null
|
||||
const dropRetainedFromDiskCache = (): void => {
|
||||
pendingDiskEvictions.delete(dropRetainedFromDiskCache)
|
||||
releaseRetiredEvictionListener()
|
||||
if (fallbackTimer !== null) {
|
||||
clearTimeout(fallbackTimer)
|
||||
fallbackTimer = null
|
||||
@@ -154,6 +173,9 @@ export function clearCompletedActivity(threads: readonly AgentPaneThread[]): boo
|
||||
evictPersistedStatuses(plan.cacheIdentities)
|
||||
}
|
||||
pendingDiskEvictions.add(dropRetainedFromDiskCache)
|
||||
if (evictionListenerRetired && typeof window !== 'undefined') {
|
||||
window.addEventListener('pagehide', flushPendingClearCompletedEvictions)
|
||||
}
|
||||
fallbackTimer = setTimeout(dropRetainedFromDiskCache, CLEAR_COMPLETED_EVICTION_FALLBACK_MS)
|
||||
toast(
|
||||
plan.clearedThreadCount === 1
|
||||
@@ -169,6 +191,7 @@ export function clearCompletedActivity(threads: readonly AgentPaneThread[]): boo
|
||||
onClick: () => {
|
||||
undone = true
|
||||
pendingDiskEvictions.delete(dropRetainedFromDiskCache)
|
||||
releaseRetiredEvictionListener()
|
||||
if (fallbackTimer !== null) {
|
||||
clearTimeout(fallbackTimer)
|
||||
fallbackTimer = null
|
||||
|
||||
@@ -60,6 +60,20 @@ type ContextualTourOverlaySurfaceProps = {
|
||||
onOverlayKeyDownCapture: (event: KeyboardEvent<HTMLDivElement>) => void
|
||||
}
|
||||
|
||||
function disposeContextualTourGlobalKeyGuard(): void {
|
||||
if (typeof window === 'undefined') {
|
||||
return
|
||||
}
|
||||
const guardedWindow = window as Window & {
|
||||
__orcaContextualTourGlobalKeyGuardInstalled?: boolean
|
||||
}
|
||||
if (!guardedWindow.__orcaContextualTourGlobalKeyGuardInstalled) {
|
||||
return
|
||||
}
|
||||
window.removeEventListener('keydown', handleContextualTourGlobalKeyDown, true)
|
||||
delete guardedWindow.__orcaContextualTourGlobalKeyGuardInstalled
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
const guardedWindow = window as Window & {
|
||||
__orcaContextualTourGlobalKeyGuardInstalled?: boolean
|
||||
@@ -70,6 +84,12 @@ if (typeof window !== 'undefined') {
|
||||
}
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// global key guard so dev sessions do not retain stale module closures.
|
||||
import.meta.hot.dispose(disposeContextualTourGlobalKeyGuard)
|
||||
}
|
||||
|
||||
const PANEL_BASE_CLASSES =
|
||||
'orca-contextual-tour-panel rounded-lg border border-border text-popover-foreground backdrop-blur-[2px]'
|
||||
|
||||
|
||||
+27
-7
@@ -34,12 +34,32 @@ function invalidateCombinedDiffCachesForRelativePath(relativePath: string): void
|
||||
}
|
||||
}
|
||||
|
||||
function handleCombinedDiffExternalFileChange(event: Event): void {
|
||||
const detail = (event as CustomEvent<EditorPathMutationTarget>).detail
|
||||
if (detail?.relativePath) {
|
||||
// Why: inactive combined-diff tabs are unmounted, so only a module-level cache bust stops a remount replaying stale bodies.
|
||||
invalidateCombinedDiffCachesForRelativePath(detail.relativePath)
|
||||
}
|
||||
}
|
||||
|
||||
export function disposeCombinedDiffViewMemory(): void {
|
||||
if (typeof window !== 'undefined') {
|
||||
window.removeEventListener(
|
||||
ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT,
|
||||
handleCombinedDiffExternalFileChange
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if (typeof window !== 'undefined') {
|
||||
window.addEventListener(ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT, (event) => {
|
||||
const detail = (event as CustomEvent<EditorPathMutationTarget>).detail
|
||||
if (detail?.relativePath) {
|
||||
// Why: inactive combined-diff tabs are unmounted, so only a module-level cache bust stops a remount replaying stale bodies.
|
||||
invalidateCombinedDiffCachesForRelativePath(detail.relativePath)
|
||||
}
|
||||
})
|
||||
window.addEventListener(
|
||||
ORCA_EDITOR_EXTERNAL_FILE_CHANGE_EVENT,
|
||||
handleCombinedDiffExternalFileChange
|
||||
)
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// global cache-bust listener so dev sessions do not accumulate handlers.
|
||||
import.meta.hot.dispose(disposeCombinedDiffViewMemory)
|
||||
}
|
||||
|
||||
@@ -118,3 +118,12 @@ function removeClickListener(): void {
|
||||
clickListener = null
|
||||
}
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// opt-in gesture hook so dev sessions do not retain stale pane closures.
|
||||
import.meta.hot.dispose(() => {
|
||||
removeClickListener()
|
||||
stopRenderDesyncSampleBurst()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ const INPUT_QUIET_EVENTS: readonly (keyof WindowEventMap)[] = [
|
||||
]
|
||||
|
||||
let listenersInstalled = false
|
||||
let listenersWindow: Window | null = null
|
||||
let lastInputAt = Number.NEGATIVE_INFINITY
|
||||
|
||||
function now(): number {
|
||||
@@ -49,6 +50,25 @@ function ensureInputQuietListeners(targetWindow: Window): void {
|
||||
for (const eventName of INPUT_QUIET_EVENTS) {
|
||||
targetWindow.addEventListener(eventName, recordInput, options)
|
||||
}
|
||||
listenersWindow = targetWindow
|
||||
}
|
||||
|
||||
function disposeInputQuietListeners(): void {
|
||||
if (!listenersWindow) {
|
||||
return
|
||||
}
|
||||
const options: AddEventListenerOptions = { capture: true }
|
||||
for (const eventName of INPUT_QUIET_EVENTS) {
|
||||
listenersWindow.removeEventListener(eventName, recordInput, options)
|
||||
}
|
||||
listenersWindow = null
|
||||
listenersInstalled = false
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// global input hooks so dev sessions do not retain stale scheduler closures.
|
||||
import.meta.hot.dispose(disposeInputQuietListeners)
|
||||
}
|
||||
|
||||
function scheduleIdleCallback(targetWindow: Window, callback: () => void, timeout: number): number {
|
||||
|
||||
@@ -108,6 +108,22 @@ function refreshOnFocus(): void {
|
||||
void refreshLayoutMap()
|
||||
}
|
||||
|
||||
function disposeLayoutCharacterPrefetch(): void {
|
||||
if (attachedWindow) {
|
||||
attachedWindow.removeEventListener('focus', refreshOnFocus)
|
||||
attachedWindow = null
|
||||
}
|
||||
unsubscribeLayoutChange?.()
|
||||
unsubscribeLayoutChange = null
|
||||
focusListenerAttached = false
|
||||
}
|
||||
|
||||
if (import.meta !== undefined && import.meta.hot) {
|
||||
// Vite can replace this module without a full renderer reload. Remove the
|
||||
// global focus/layout hooks so dev sessions do not accumulate listeners.
|
||||
import.meta.hot.dispose(disposeLayoutCharacterPrefetch)
|
||||
}
|
||||
|
||||
/** A layout map entry is usable as a kitty base key only if it is a single
|
||||
* printable codepoint (dead keys report names like 'Dead'; some entries are
|
||||
* empty). Exposed for tests. */
|
||||
|
||||
Reference in New Issue
Block a user