From 36d295628ebd9fa835c04f2cc09a75cfff96f409 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Wed, 28 Oct 2026 02:56:45 -0700 Subject: [PATCH] fix: cancel cleanup scans with retired renderer documents (#34896) Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- ...orkspace-cleanup-document-lifetime.test.ts | 174 ++++++++++++++++++ src/main/ipc/workspace-cleanup.ts | 10 +- 2 files changed, 179 insertions(+), 5 deletions(-) create mode 100644 src/main/ipc/workspace-cleanup-document-lifetime.test.ts diff --git a/src/main/ipc/workspace-cleanup-document-lifetime.test.ts b/src/main/ipc/workspace-cleanup-document-lifetime.test.ts new file mode 100644 index 00000000000..aee7d26a0a6 --- /dev/null +++ b/src/main/ipc/workspace-cleanup-document-lifetime.test.ts @@ -0,0 +1,174 @@ +import { EventEmitter } from 'node:events' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { GitStatusResult } from '../../shared/git-status-types' +import type { GitWorktreeInfo } from '../../shared/worktree/types' +import type * as RepoWorktrees from '../repo-worktrees' + +const boundary = vi.hoisted(() => ({ + handlers: new Map unknown>(), + list: vi.fn(), + status: vi.fn(), + persist: vi.fn() +})) +vi.mock('electron', () => ({ + ipcMain: { + handle: (name: string, fn: (event: unknown, args: unknown) => unknown) => + boundary.handlers.set(name, fn), + removeHandler: (name: string) => boundary.handlers.delete(name) + } +})) +vi.mock('node:fs/promises', () => ({ + lstat: vi.fn(async () => ({ mtimeMs: 0 })), + readFile: vi.fn(async () => { + throw new Error('no gitdir pointer') + }) +})) +vi.mock('../repo-worktrees', async (importOriginal) => ({ + ...(await importOriginal()), + listRepoWorktrees: boundary.list +})) +vi.mock('../git/status', () => ({ getStatus: boundary.status })) +vi.mock('../git/runner', () => ({ + gitExecFileAsync: vi.fn(async () => ({ stdout: '0\n', stderr: '' })) +})) +vi.mock('../project-runtime-git-options', () => ({ getLocalProjectWorktreeGitOptions: () => ({}) })) +vi.mock('../workspace-cleanup-scan-snapshot', () => ({ + persistWorkspaceCleanupScanResult: boundary.persist, + readWorkspaceCleanupScanSnapshot: vi.fn() +})) +vi.mock('../workspace-cleanup-removal-snapshot-prune', () => ({ + beginWorkspaceCleanupRemovalSnapshotPruneBatch: vi.fn(), + finishWorkspaceCleanupRemovalSnapshotPruneBatch: vi.fn(), + recordWorkspaceCleanupRemovalSnapshotPrune: vi.fn() +})) + +import { registerWorkspaceCleanupHandlers } from './workspace-cleanup' + +const NOW = Date.UTC(2025, 0, 1) +const STATUS: GitStatusResult = { + entries: [], + conflictOperation: 'unknown', + upstreamStatus: { hasUpstream: true, ahead: 0, behind: 0 } +} +class Sender extends EventEmitter { + id = 9 + send = vi.fn() + isDestroyed(): boolean { + return false + } +} +function rows(size: number, prefix = 'workspace'): GitWorktreeInfo[] { + return Array.from({ length: size }, (_, index) => ({ + path: `/${prefix}-${index}`, + head: 'abc', + branch: `refs/heads/branch-${index}`, + isBare: false, + isMainWorktree: false + })) +} +function setup() { + const store = { + getRepos: () => [ + { + id: 'repo', + path: '/repo', + displayName: 'Repo', + badgeColor: '#000', + addedAt: NOW, + symlinkPaths: [] + } + ], + getWorktreeMeta: () => ({ lastActivityAt: NOW - 40 * 24 * 60 * 60 * 1000 }), + getAllWorktreeMeta: () => ({}), + getGitHubCache: () => ({ pr: {}, issue: {} }), + getProfileStorageDirectory: () => 'unused' + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The actual scanner reads only these store methods; mergeWorktree supplies missing metadata defaults. + registerWorkspaceCleanupHandlers(store as never) + const handler = boundary.handlers.get('workspaceCleanup:scan') + if (!handler) { + throw new Error('Missing scan handler') + } + const sender = new Sender() + return { + sender, + scan: (scanId: string) => + Promise.resolve(handler({ sender }, { scanId, includeAllWorkspaces: true })) + } +} + +describe('workspace cleanup document ownership', () => { + beforeEach(() => { + vi.spyOn(Date, 'now').mockReturnValue(NOW) + boundary.handlers.clear() + boundary.list.mockReset().mockResolvedValue(rows(10)) + boundary.status.mockReset().mockResolvedValue(STATUS) + boundary.persist.mockReset().mockResolvedValue(undefined) + }) + afterEach(() => vi.restoreAllMocks()) + it.each( + [10, 100].flatMap((size) => + ['destroyed', 'render-process-gone', 'did-navigate'].map((event) => ({ size, event })) + ) + )( + 'cancels $size rows after $event without dispatching beyond active reads', + async ({ size, event }) => { + boundary.list.mockResolvedValue(rows(size)) + const pending = Promise.withResolvers() + boundary.status.mockReturnValue(pending.promise) + const { sender, scan } = setup() + const outcome = scan('retired').then( + () => 'complete', + () => 'cancelled' + ) + await vi.waitFor(() => expect(boundary.status).toHaveBeenCalledTimes(3)) + sender.send.mockClear() + sender.emit(event) + pending.resolve(STATUS) + expect(await outcome).toBe('cancelled') + expect(boundary.status).toHaveBeenCalledTimes(3) + expect(sender.send).not.toHaveBeenCalled() + expect(boundary.persist).not.toHaveBeenCalled() + expect(sender.eventNames()).toEqual([]) + } + ) + it('preserves same-document navigation and persists a completed scan', async () => { + const pending = Promise.withResolvers() + boundary.status.mockReturnValue(pending.promise) + const { sender, scan } = setup() + const result = scan('live') + await vi.waitFor(() => expect(boundary.status).toHaveBeenCalledTimes(3)) + sender.emit('did-navigate-in-page', {}, '#section', true) + pending.resolve(STATUS) + await result + expect(boundary.status).toHaveBeenCalledTimes(10) + expect(boundary.persist).toHaveBeenCalledTimes(1) + expect(sender.eventNames()).toEqual([]) + }) + it('lets a successor finish while late retired reads settle', async () => { + const pending = Promise.withResolvers() + boundary.list + .mockResolvedValueOnce(rows(10, 'retired')) + .mockResolvedValue(rows(10, 'replacement')) + boundary.status.mockImplementation((path: string) => + path.startsWith('/retired') ? pending.promise : Promise.resolve(STATUS) + ) + const { sender, scan } = setup() + const old = scan('retired').then( + () => 'complete', + () => 'cancelled' + ) + await vi.waitFor(() => expect(boundary.status).toHaveBeenCalledTimes(3)) + sender.emit('render-process-gone') + sender.send.mockClear() + await scan('replacement') + pending.resolve(STATUS) + expect(await old).toBe('cancelled') + expect(boundary.status).toHaveBeenCalledTimes(13) + expect(boundary.persist).toHaveBeenCalledTimes(1) + expect(sender.send.mock.calls.every(([, progress]) => progress.scanId === 'replacement')).toBe( + true + ) + expect(sender.eventNames()).toEqual([]) + }) +}) diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index d52cba21944..562aaefe849 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -11,7 +11,7 @@ import { import { parseExecutionHostId } from '../../shared/execution-host' import { getWorkspaceCleanupHostIdentity } from '../../shared/workspace-cleanup-host-identity' import { scanWorkspaceCleanup } from './workspace-cleanup-scan' -import { addFanoutListener } from '../event-listener-fanout' +import { abortWhenRendererGone } from './renderer-lifetime-abort' import { hasTargetedWorkspaceCleanupScan } from './workspace-cleanup-scan-targets' import { persistWorkspaceCleanupScanResult, @@ -63,14 +63,14 @@ export function registerWorkspaceCleanupHandlers(store: Store): void { broadScanControllersBySenderMode.set(broadScanKey, controller) } // Window retirement must cancel scan work, not merely mute its progress. - const onSenderDestroyed = (): void => controller.abort() - const releaseDestroyed = addFanoutListener(sender, 'destroyed', onSenderDestroyed) + const lifetime = abortWhenRendererGone(sender) + lifetime.signal.addEventListener('abort', () => controller.abort(), { once: true }) try { const result = await scanWorkspaceCleanup(store, scanArgs, { signal: controller.signal, onProgress: scanArgs.scanId ? (progress) => { - if (!sender.isDestroyed()) { + if (!controller.signal.aborted && !sender.isDestroyed()) { sender.send('workspaceCleanup:scanProgress', progress) } } @@ -82,7 +82,7 @@ export function registerWorkspaceCleanupHandlers(store: Store): void { } return result } finally { - releaseDestroyed() + lifetime.dispose() if (scanKey && activeScans.get(scanKey) === controller) { activeScans.delete(scanKey) }