From 03fe971d9ffdb4de893daf2926e003e894815de9 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 13:14:02 -0700 Subject: [PATCH] Clean up rate limit window listeners --- src/main/rate-limits/service.test.ts | 59 ++++++++++++++++++++++++++++ src/main/rate-limits/service.ts | 23 +++++++---- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/main/rate-limits/service.test.ts b/src/main/rate-limits/service.test.ts index 2c4bfd5d078..489e61efc2f 100644 --- a/src/main/rate-limits/service.test.ts +++ b/src/main/rate-limits/service.test.ts @@ -3,6 +3,7 @@ stale-data handling, account-switch generation, and OpenCode config-change semantics covered in service.ts, which already carries the same pragma. Keeping them in one file makes the ordering contract reviewable as a unit. */ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { EventEmitter } from 'events' import type { ProviderRateLimits } from '../../shared/rate-limit-types' import { RateLimitService } from './service' import { fetchClaudeRateLimits, fetchManagedAccountUsage } from './claude-fetcher' @@ -78,6 +79,34 @@ function serviceInternals(service: RateLimitService): { fetchAll: () => Promise< return service as unknown as { fetchAll: () => Promise } } +type RateLimitWindow = Parameters[0] + +class FakeRateLimitWindow extends EventEmitter { + webContents = { + send: vi.fn() + } + + isDestroyed(): boolean { + return false + } + + isVisible(): boolean { + return true + } + + isMinimized(): boolean { + return false + } + + isFocused(): boolean { + return true + } +} + +function asRateLimitWindow(window: FakeRateLimitWindow): RateLimitWindow { + return window as unknown as RateLimitWindow +} + describe('RateLimitService', () => { beforeEach(() => { vi.clearAllMocks() @@ -111,6 +140,36 @@ describe('RateLimitService', () => { expect(fetchCodexRateLimits).toHaveBeenCalledTimes(2) }) + it('removes all window listeners when replacing the attached window', () => { + const service = new RateLimitService() + const firstWindow = new FakeRateLimitWindow() + const secondWindow = new FakeRateLimitWindow() + + service.attach(asRateLimitWindow(firstWindow)) + expect(firstWindow.listenerCount('focus')).toBe(1) + expect(firstWindow.listenerCount('show')).toBe(1) + expect(firstWindow.listenerCount('restore')).toBe(1) + expect(firstWindow.listenerCount('closed')).toBe(1) + + service.attach(asRateLimitWindow(secondWindow)) + + expect(firstWindow.listenerCount('focus')).toBe(0) + expect(firstWindow.listenerCount('show')).toBe(0) + expect(firstWindow.listenerCount('restore')).toBe(0) + expect(firstWindow.listenerCount('closed')).toBe(0) + expect(secondWindow.listenerCount('focus')).toBe(1) + expect(secondWindow.listenerCount('show')).toBe(1) + expect(secondWindow.listenerCount('restore')).toBe(1) + expect(secondWindow.listenerCount('closed')).toBe(1) + + service.stop() + + expect(secondWindow.listenerCount('focus')).toBe(0) + expect(secondWindow.listenerCount('show')).toBe(0) + expect(secondWindow.listenerCount('restore')).toBe(0) + expect(secondWindow.listenerCount('closed')).toBe(0) + }) + it('keeps recent stale data across repeated failures', async () => { const service = new RateLimitService() const internal = serviceInternals(service) diff --git a/src/main/rate-limits/service.ts b/src/main/rate-limits/service.ts index eb55cef180f..4c04510fcd8 100644 --- a/src/main/rate-limits/service.ts +++ b/src/main/rate-limits/service.ts @@ -153,21 +153,28 @@ export class RateLimitService { const refreshOnResume = (): void => { void this.refreshIfWindowActive() } - mainWindow.on('focus', refreshOnResume) - mainWindow.on('show', refreshOnResume) - mainWindow.on('restore', refreshOnResume) - this.detachWindowListeners = () => { + // Why: attach() can replace windows; the previous closed listener also + // captures this service and must be removed with the focus listeners. + const detachWindowListeners = (): void => { mainWindow.removeListener('focus', refreshOnResume) mainWindow.removeListener('show', refreshOnResume) mainWindow.removeListener('restore', refreshOnResume) + mainWindow.removeListener('closed', onClosed) } - mainWindow.on('closed', () => { - this.detachWindowListeners?.() - this.detachWindowListeners = null + const onClosed = (): void => { + detachWindowListeners() + if (this.detachWindowListeners === detachWindowListeners) { + this.detachWindowListeners = null + } if (this.mainWindow === mainWindow) { this.mainWindow = null } - }) + } + mainWindow.on('focus', refreshOnResume) + mainWindow.on('show', refreshOnResume) + mainWindow.on('restore', refreshOnResume) + mainWindow.on('closed', onClosed) + this.detachWindowListeners = detachWindowListeners } start(): void {