diff --git a/src/main/ipc/ai-vault-scan-coalescing.test.ts b/src/main/ipc/ai-vault-scan-coalescing.test.ts index 8a8ad0a3665..0efbea4d1ee 100644 --- a/src/main/ipc/ai-vault-scan-coalescing.test.ts +++ b/src/main/ipc/ai-vault-scan-coalescing.test.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AiVaultListResult } from '../../shared/ai-vault-types' import type { IFilesystemProvider } from '../providers/types' @@ -121,37 +122,44 @@ describe('Agent Session History scan coalescing', () => { await expect(second).resolves.toMatchObject({ sessions: [], issues: [] }) }) - it('keeps a shared multi-window scan alive when one window cancels', async () => { - let resolveRelay: ((result: AiVaultListResult) => void) | undefined - mocks.requestActiveSshAiVaultSessionList.mockImplementation( - () => - new Promise((resolve) => { - resolveRelay = resolve - }) - ) - registerAiVaultHandlers() - const list = ipcHandler('aiVault:listSessions') - const cancel = ipcHandler('aiVault:cancelListSessions') - const firstEvent = { sender: { id: 1 } } - const secondEvent = { sender: { id: 2 } } - const first = list(firstEvent, { - executionHostScope: 'ssh:dev-box', - requestToken: 'scan' - }) as Promise - const second = list(secondEvent, { - executionHostScope: 'ssh:dev-box', - requestToken: 'scan' - }) as Promise - await vi.waitFor(() => expect(resolveRelay).toBeDefined()) + it.each(['cancel', 'destroyed', 'render-process-gone', 'did-navigate'])( + 'keeps a shared multi-window scan alive when one window emits %s', + async (eventName) => { + let resolveRelay: ((result: AiVaultListResult) => void) | undefined + mocks.requestActiveSshAiVaultSessionList.mockImplementation( + () => + new Promise((resolve) => { + resolveRelay = resolve + }) + ) + registerAiVaultHandlers() + const list = ipcHandler('aiVault:listSessions') + const cancel = ipcHandler('aiVault:cancelListSessions') + const firstEvent = { sender: Object.assign(new EventEmitter(), { id: 1 }) } + const secondEvent = { sender: Object.assign(new EventEmitter(), { id: 2 }) } + const first = list(firstEvent, { + executionHostScope: 'ssh:dev-box', + requestToken: 'scan' + }) as Promise + const second = list(secondEvent, { + executionHostScope: 'ssh:dev-box', + requestToken: 'scan' + }) as Promise + await vi.waitFor(() => expect(resolveRelay).toBeDefined()) - cancel(firstEvent, { requestToken: 'scan' }) + if (eventName === 'cancel') { + cancel(firstEvent, { requestToken: 'scan' }) + } else { + firstEvent.sender.emit(eventName) + } - // Electron logs every rejected handler, so a cancelled scan resolves instead. - await expect(first).resolves.toMatchObject({ cancelled: true, sessions: [], issues: [] }) - expect(mocks.requestActiveSshAiVaultSessionList).toHaveBeenCalledTimes(1) - resolveRelay?.(EMPTY_RESULT) - await expect(second).resolves.toEqual(EMPTY_RESULT) - }) + // Electron logs every rejected handler, so a cancelled scan resolves instead. + await expect(first).resolves.toMatchObject({ cancelled: true, sessions: [], issues: [] }) + expect(mocks.requestActiveSshAiVaultSessionList).toHaveBeenCalledTimes(1) + resolveRelay?.(EMPTY_RESULT) + await expect(second).resolves.toEqual(EMPTY_RESULT) + } + ) it('reports a real scan failure as a host issue rather than cancellation', async () => { mocks.requestActiveSshAiVaultSessionList.mockRejectedValue(new Error('relay socket closed')) @@ -162,7 +170,7 @@ describe('Agent Session History scan coalescing', () => { // SSH host legs convert unexpected throws into scan issues so an `all` // multi-host list still returns the other hosts' sessions. const result = await list( - { sender: { id: 1 } }, + { sender: Object.assign(new EventEmitter(), { id: 1 }) }, { executionHostScope: 'ssh:dev-box', requestToken: 'scan' } ) expect(result).toMatchObject({ @@ -180,7 +188,7 @@ describe('Agent Session History scan coalescing', () => { // The local leg degrades like the SSH legs above: a rejection reaches the // renderer as a raw string painted over the list instead of an issue row. const result = await list( - { sender: { id: 1 } }, + { sender: Object.assign(new EventEmitter(), { id: 1 }) }, { executionHostScope: 'local', requestToken: 'scan' } ) expect(result).toMatchObject({ @@ -190,6 +198,68 @@ describe('Agent Session History scan coalescing', () => { expect(result).not.toHaveProperty('cancelled') }) + it('aborts local and SSH all-host legs only after the last renderer leaves', async () => { + const scanSignals: AbortSignal[] = [] + const waitForAbort = (signal: AbortSignal): Promise => { + scanSignals.push(signal) + return new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(EMPTY_RESULT), { once: true }) + }) + } + mocks.scanAiVaultSessionsInWorker.mockImplementation((_args, signal: AbortSignal) => + waitForAbort(signal) + ) + mocks.requestActiveSshAiVaultSessionList.mockImplementation( + (_targetId, _params, options: { signal: AbortSignal }) => waitForAbort(options.signal) + ) + let resolveRuntime: ((result: AiVaultListResult) => void) | undefined + mocks.scanRuntimeAiVaultSessions.mockImplementation( + () => + new Promise((resolve) => { + resolveRuntime = resolve + }) + ) + registerRuntimeHost() + const list = ipcHandler('aiVault:listSessions') + const firstEvent = { sender: Object.assign(new EventEmitter(), { id: 1 }) } + const secondEvent = { sender: Object.assign(new EventEmitter(), { id: 2 }) } + const first = list(firstEvent, { executionHostScope: 'all', requestToken: 'scan' }) + const second = list(secondEvent, { executionHostScope: 'all', requestToken: 'scan' }) + await vi.waitFor(() => expect(scanSignals).toHaveLength(2)) + await vi.waitFor(() => expect(resolveRuntime).toBeDefined()) + + firstEvent.sender.emit('did-navigate') + await expect(first).resolves.toMatchObject({ cancelled: true }) + expect(scanSignals.every((signal) => !signal.aborted)).toBe(true) + secondEvent.sender.emit('render-process-gone') + await expect(second).resolves.toMatchObject({ cancelled: true }) + expect(scanSignals.every((signal) => signal.aborted)).toBe(true) + for (const event of [firstEvent, secondEvent]) { + expect(event.sender.eventNames()).toEqual([]) + } + // Runtime RPC cannot be canceled; its late result remains safely observed. + resolveRuntime?.(EMPTY_RESULT) + }) + + it('does not start an abandoned scan after ownership initialization completes', async () => { + let finishOwnership: (() => void) | undefined + registerAiVaultHandlers({ + ensureStructuredSessionOwnership: () => + new Promise((resolve) => { + finishOwnership = resolve + }) + }) + const event = { sender: Object.assign(new EventEmitter(), { id: 1 }) } + const pending = ipcHandler('aiVault:listSessions')(event, { requestToken: 'scan' }) + + event.sender.emit('did-navigate') + finishOwnership?.() + + await expect(pending).resolves.toMatchObject({ cancelled: true }) + expect(mocks.scanAiVaultSessionsInWorker).not.toHaveBeenCalled() + expect(event.sender.eventNames()).toEqual([]) + }) + it('re-joins a preempted same-scope caller onto the forced refresh', async () => { const signals: AbortSignal[] = [] let resolveForced: ((result: AiVaultListResult) => void) | undefined diff --git a/src/main/ipc/ai-vault.test.ts b/src/main/ipc/ai-vault.test.ts index 17f282e54be..b9f5661e19c 100644 --- a/src/main/ipc/ai-vault.test.ts +++ b/src/main/ipc/ai-vault.test.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import { homedir } from 'node:os' import { join, sep } from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -488,20 +489,17 @@ describe('listAiVaultSessions host routing', () => { }) ) registerAiVaultHandlers() - const event = { sender: { id: 7 } } + const event = { sender: Object.assign(new EventEmitter(), { id: 7 }) } const pending = getIpcHandler('aiVault:listSessions')(event, { executionHostScope: 'ssh:dev-box', requestToken: 'scan-1' }) await vi.waitFor(() => expect(relaySignal).toBeDefined()) - await getIpcHandler('aiVault:cancelListSessions')(event, { - requestToken: 'scan-1' - }) + await getIpcHandler('aiVault:cancelListSessions')(event, { requestToken: 'scan-1' }) expect(relaySignal?.aborted).toBe(true) - // Resolved, not rejected: Electron logs every rejected handler, and a - // superseded scan is normal control flow rather than a failure. + // Superseded scans resolve because Electron logs every rejected handler. await expect(pending).resolves.toMatchObject({ cancelled: true, sessions: [] }) }) }) diff --git a/src/main/ipc/filesystem-git-status-staging.test.ts b/src/main/ipc/filesystem-git-status-staging.test.ts index 5cc1b4e9540..b86a75b2314 100644 --- a/src/main/ipc/filesystem-git-status-staging.test.ts +++ b/src/main/ipc/filesystem-git-status-staging.test.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import path from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { @@ -305,42 +306,51 @@ describe('registerFilesystemHandlers', () => { }) }) - it('aborts tokenized local status without crossing renderer boundaries', async () => { - registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) - const statusSignals: AbortSignal[] = [] - getStatusMock.mockImplementation( - (_worktreePath: string, options: { signal?: AbortSignal }) => - new Promise((_resolve, reject) => { - if (options.signal) { - statusSignals.push(options.signal) - options.signal.addEventListener('abort', () => reject(new Error('aborted')), { - once: true - }) - } - }) - ) - registerFilesystemHandlers(store as never) + it.each(['cancel', 'did-navigate', 'render-process-gone', 'destroyed'])( + 'aborts tokenized local status on %s without crossing renderer boundaries', + async (eventName) => { + registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) + const statusSignals: AbortSignal[] = [] + getStatusMock.mockImplementation( + (_worktreePath: string, options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + if (options.signal) { + statusSignals.push(options.signal) + options.signal.addEventListener('abort', () => reject(new Error('aborted')), { + once: true + }) + } + }) + ) + registerFilesystemHandlers(store as never) - const firstEvent = { sender: { id: 7 } } - const secondEvent = { sender: { id: 8 } } - const firstRequest = handlers.get('git:status')!(firstEvent, { - worktreePath: WORKTREE_FEATURE_PATH, - requestToken: 'status-1' - }) as Promise - const secondRequest = handlers.get('git:status')!(secondEvent, { - worktreePath: WORKTREE_FEATURE_PATH, - requestToken: 'status-1' - }) as Promise - await vi.waitFor(() => expect(statusSignals).toHaveLength(2)) - await handlers.get('git:cancelStatus')!(firstEvent, { requestToken: 'status-1' }) + const firstEvent = { sender: Object.assign(new EventEmitter(), { id: 7 }) } + const secondEvent = { sender: Object.assign(new EventEmitter(), { id: 8 }) } + const firstRequest = handlers.get('git:status')!(firstEvent, { + worktreePath: WORKTREE_FEATURE_PATH, + requestToken: 'status-1' + }) as Promise + const secondRequest = handlers.get('git:status')!(secondEvent, { + worktreePath: WORKTREE_FEATURE_PATH, + requestToken: 'status-1' + }) as Promise + await vi.waitFor(() => expect(statusSignals).toHaveLength(2)) + if (eventName === 'cancel') { + await handlers.get('git:cancelStatus')!(firstEvent, { requestToken: 'status-1' }) + } else { + firstEvent.sender.emit(eventName) + } - expect(statusSignals[0]?.aborted).toBe(true) - expect(statusSignals[1]?.aborted).toBe(false) - await expect(firstRequest).rejects.toThrow('aborted') + expect(statusSignals[0]?.aborted).toBe(true) + expect(statusSignals[1]?.aborted).toBe(false) + await expect(firstRequest).rejects.toThrow('aborted') - await handlers.get('git:cancelStatus')!(secondEvent, { requestToken: 'status-1' }) - await expect(secondRequest).rejects.toThrow('aborted') - }) + await handlers.get('git:cancelStatus')!(secondEvent, { requestToken: 'status-1' }) + await expect(secondRequest).rejects.toThrow('aborted') + expect(firstEvent.sender.eventNames()).toEqual([]) + expect(secondEvent.sender.eventNames()).toEqual([]) + } + ) it('checks ignored paths through local and SSH git providers', async () => { registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index fe1568dbf72..2d452fe4ee6 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import path from 'node:path' import { beforeEach, describe, expect, it, vi } from 'vitest' import { @@ -608,38 +609,50 @@ describe('registerFilesystemHandlers', () => { // Why #7721: without a cancel path, every workspace switch left the previous // workspace's full-tree SSH scan running, stacking scans on the relay until // interactive fs.readDir/fs.stat starved past their 30s timeout. - it('fs:cancelListFiles aborts an in-flight SSH listing by request token (#7721)', async () => { - let capturedSignal: AbortSignal | undefined - const listFilesMock = vi.fn( - (_rootPath: string, options: { signal?: AbortSignal }) => - new Promise((_resolve, reject) => { - capturedSignal = options.signal - options.signal?.addEventListener('abort', () => reject(new Error('listing cancelled')), { - once: true + it.each(['cancel', 'did-navigate', 'render-process-gone', 'destroyed'])( + 'aborts an in-flight SSH file listing on %s (#7721)', + async (eventName) => { + let capturedSignal: AbortSignal | undefined + const listFilesMock = vi.fn( + (_rootPath: string, options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + capturedSignal = options.signal + options.signal?.addEventListener( + 'abort', + () => reject(new Error('listing cancelled')), + { + once: true + } + ) }) - }) - ) - getSshFilesystemProviderMock.mockReturnValue({ listFiles: listFilesMock }) + ) + getSshFilesystemProviderMock.mockReturnValue({ listFiles: listFilesMock }) - registerFilesystemHandlers(store as never) + registerFilesystemHandlers(store as never) - // Why: cancellation keys are scoped to the issuing webContents, so the - // cancel must come from the same sender as the listing request. - const senderEvent = { sender: { id: 7 } } - const pending = handlers.get('fs:listFiles')!(senderEvent, { - rootPath: '/home/user/repo', - connectionId: 'conn-1', - requestToken: 'token-1' - }) as Promise + // Why: cancellation keys are scoped to the issuing webContents, so the + // cancel must come from the same sender as the listing request. + const senderEvent = { sender: Object.assign(new EventEmitter(), { id: 7 }) } + const pending = handlers.get('fs:listFiles')!(senderEvent, { + rootPath: '/home/user/repo', + connectionId: 'conn-1', + requestToken: 'token-1' + }) as Promise - expect(capturedSignal?.aborted).toBe(false) - await handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'token-1' }) - expect(capturedSignal?.aborted).toBe(true) - await expect(pending).rejects.toThrow('listing cancelled') + expect(capturedSignal?.aborted).toBe(false) + if (eventName === 'cancel') { + await handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'token-1' }) + } else { + senderEvent.sender.emit(eventName) + } + expect(capturedSignal?.aborted).toBe(true) + await expect(pending).rejects.toThrow('listing cancelled') + expect(senderEvent.sender.eventNames()).toEqual([]) - // Unknown or already-settled tokens are a no-op, not an error. - expect(() => - handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'unknown' }) - ).not.toThrow() - }) + // Unknown or already-settled tokens are a no-op, not an error. + expect(() => + handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'unknown' }) + ).not.toThrow() + } + ) }) diff --git a/src/main/ipc/sender-scoped-request-cancellation.test.ts b/src/main/ipc/sender-scoped-request-cancellation.test.ts new file mode 100644 index 00000000000..e8bfc6b8d6b --- /dev/null +++ b/src/main/ipc/sender-scoped-request-cancellation.test.ts @@ -0,0 +1,148 @@ +import { EventEmitter } from 'node:events' +import type { IpcMainInvokeEvent } from 'electron' +import { describe, expect, it } from 'vitest' +import { createSenderScopedRequestCancellations } from './sender-scoped-request-cancellation' + +function requestEvent(id = 1) { + const sender = Object.assign(new EventEmitter(), { id }) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The registry only reads sender.id and its EventEmitter lifetime methods. + const event = { sender } as unknown as IpcMainInvokeEvent + return { sender, event } +} + +const lifetimeEvents = ['destroyed', 'render-process-gone', 'did-navigate'] as const + +describe('sender-scoped request lifetime', () => { + it.each(lifetimeEvents)( + 'aborts every owned request on %s and preserves another sender', + (name) => { + const registry = createSenderScopedRequestCancellations() + const first = requestEvent(1) + const second = requestEvent(2) + const firstRequests = ['one', 'two'].map((token) => registry.begin(first.event, token)) + const secondRequest = registry.begin(second.event, 'one') + + first.sender.emit(name) + + expect(firstRequests.map((request) => request?.signal.aborted)).toEqual([true, true]) + expect(secondRequest?.signal.aborted).toBe(false) + for (const eventName of lifetimeEvents) { + expect(first.sender.listenerCount(eventName)).toBe(0) + } + registry.finish(second.event, 'one', secondRequest) + } + ) + + it('keeps only one set of lifecycle listeners for concurrent requests', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const controllers = Array.from({ length: 100 }, (_, index) => registry.begin(event, `${index}`)) + + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(1) + } + controllers.forEach((controller, index) => registry.finish(event, `${index}`, controller)) + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(0) + } + expect(controllers.some((controller) => controller?.signal.aborted)).toBe(false) + }) + + it('does not accumulate live requests across repeated document replacements', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const controllers: AbortController[] = [] + for (let generation = 0; generation < 16; generation++) { + if (generation > 0) { + sender.emit('did-navigate') + } + for (let request = 0; request < 100; request++) { + const controller = registry.begin(event, `${generation}:${request}`) + if (controller) { + controllers.push(controller) + } + } + } + + expect(controllers.filter((controller) => !controller.signal.aborted)).toHaveLength(100) + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(1) + } + sender.emit('destroyed') + expect(controllers.every((controller) => controller.signal.aborted)).toBe(true) + }) + + it('preserves requests across same-document or prevented navigation', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const controller = registry.begin(event, 'one') + + sender.emit('did-start-navigation') + sender.emit('will-navigate', { defaultPrevented: true }) + sender.emit('did-navigate-in-page') + + expect(controller?.signal.aborted).toBe(false) + registry.finish(event, 'one', controller) + }) + + it('keeps the replacement when an old request finishes late', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const old = registry.begin(event, 'one') + const replacement = registry.begin(event, 'one') + expect(old?.signal.aborted).toBe(true) + + registry.finish(event, 'one', old) + registry.cancel(event, 'one') + + expect(replacement?.signal.aborted).toBe(true) + registry.finish(event, 'one', replacement) + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(0) + } + }) + + it('does not let a prior document finish remove the current document owner', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const old = registry.begin(event, 'one') + sender.emit('did-navigate') + const current = registry.begin(event, 'one') + + registry.finish(event, 'one', old) + expect(current?.signal.aborted).toBe(false) + sender.emit('render-process-gone') + + expect(current?.signal.aborted).toBe(true) + }) + + it('allows synchronous finish callbacks while aborting the owner', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + const first = registry.begin(event, 'one') + const second = registry.begin(event, 'two') + first?.signal.addEventListener('abort', () => registry.finish(event, 'one', first)) + second?.signal.addEventListener('abort', () => registry.finish(event, 'two', second)) + + sender.emit('destroyed') + + expect(first?.signal.aborted).toBe(true) + expect(second?.signal.aborted).toBe(true) + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(0) + } + }) + + it('preserves the no-token opt-out and ignores unknown cancellation', () => { + const registry = createSenderScopedRequestCancellations() + const { event, sender } = requestEvent() + + expect(registry.begin(event, undefined)).toBeNull() + expect(registry.begin(event, '')).toBeNull() + registry.finish(event, undefined, null) + registry.cancel(event, 'unknown') + for (const name of lifetimeEvents) { + expect(sender.listenerCount(name)).toBe(0) + } + }) +}) diff --git a/src/main/ipc/sender-scoped-request-cancellation.ts b/src/main/ipc/sender-scoped-request-cancellation.ts index 9700be9c3da..229bc33c7fd 100644 --- a/src/main/ipc/sender-scoped-request-cancellation.ts +++ b/src/main/ipc/sender-scoped-request-cancellation.ts @@ -1,4 +1,5 @@ import type { IpcMainInvokeEvent } from 'electron' +import { abortWhenRendererGone } from './renderer-lifetime-abort' export type SenderScopedRequestCancellations = { /** Registers a cancellable request; aborts any previous request that reused the token. */ @@ -13,38 +14,67 @@ export type SenderScopedRequestCancellations = { cancel: (event: IpcMainInvokeEvent, requestToken: string) => void } -/** - * Registry for renderer-cancellable IPC requests. Keys are scoped to the - * issuing webContents so one window's token can never cancel another window's - * request, and reusing a token aborts the previous request before the new one - * registers. - */ +type SenderRequests = { + controllers: Map + lifetime: ReturnType +} + +/** Requests belong to the issuing document; one window cannot cancel another's work. */ export function createSenderScopedRequestCancellations(): SenderScopedRequestCancellations { - const controllers = new Map() - const keyFor = (event: IpcMainInvokeEvent, requestToken: string): string => - `${event.sender.id}\0${requestToken}` + const senders = new Map() + const release = (senderId: number, requests: SenderRequests): void => { + if (senders.get(senderId) === requests) { + senders.delete(senderId) + } + requests.lifetime.dispose() + } + const requestsFor = (event: IpcMainInvokeEvent): SenderRequests => { + const senderId = event.sender.id + let requests = senders.get(senderId) + if (!requests) { + const lifetime = abortWhenRendererGone(event.sender) + const owned: SenderRequests = { controllers: new Map(), lifetime } + senders.set(senderId, owned) + lifetime.signal.addEventListener( + 'abort', + () => { + // Detach before abort callbacks can finish old requests or register new ones. + release(senderId, owned) + for (const controller of owned.controllers.values()) { + controller.abort() + } + owned.controllers.clear() + }, + { once: true } + ) + requests = owned + } + return requests + } return { begin: (event, requestToken) => { if (!requestToken) { return null } - const key = keyFor(event, requestToken) - controllers.get(key)?.abort() + senders.get(event.sender.id)?.controllers.get(requestToken)?.abort() const controller = new AbortController() - controllers.set(key, controller) + requestsFor(event).controllers.set(requestToken, controller) return controller }, finish: (event, requestToken, controller) => { if (!requestToken || !controller) { return } - const key = keyFor(event, requestToken) - if (controllers.get(key) === controller) { - controllers.delete(key) + const requests = senders.get(event.sender.id) + if (requests?.controllers.get(requestToken) === controller) { + requests.controllers.delete(requestToken) + if (requests.controllers.size === 0) { + release(event.sender.id, requests) + } } }, cancel: (event, requestToken) => { - controllers.get(keyFor(event, requestToken))?.abort() + senders.get(event.sender.id)?.controllers.get(requestToken)?.abort() } } } diff --git a/src/main/ipc/worktrees-ssh-provider-authority.test.ts b/src/main/ipc/worktrees-ssh-provider-authority.test.ts index 8c2d6b0edd7..8d14755ab18 100644 --- a/src/main/ipc/worktrees-ssh-provider-authority.test.ts +++ b/src/main/ipc/worktrees-ssh-provider-authority.test.ts @@ -264,51 +264,59 @@ describe('registerWorktreeHandlers', () => { expect(store.removeWorktreeLineage).not.toHaveBeenCalled() }) - it('cancels an SSH provider request by sender-scoped provider request ID', async () => { - let providerSignal: AbortSignal | undefined - const provider = { - listWorktrees: vi.fn( - (_repoPath: string, options?: { signal?: AbortSignal }) => - new Promise((_resolve, reject) => { - providerSignal = options?.signal - providerSignal?.addEventListener( - 'abort', - () => reject(new DOMException('Canceled', 'AbortError')), - { once: true } - ) - }) - ) - } - const sshRepo = { - id: 'repo-1', - path: '/remote/repo', - displayName: 'repo', - badgeColor: '#000', - addedAt: 0, - connectionId: 'target-a' - } - store.getRepos.mockReturnValue([sshRepo]) - getSshGitProviderMock.mockReturnValue(provider) + it.each(['cancel', 'did-navigate', 'render-process-gone', 'destroyed'])( + 'cancels an SSH provider request on %s', + async (eventName) => { + let providerSignal: AbortSignal | undefined + const provider = { + listWorktrees: vi.fn( + (_repoPath: string, options?: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + providerSignal = options?.signal + providerSignal?.addEventListener( + 'abort', + () => reject(new DOMException('Canceled', 'AbortError')), + { once: true } + ) + }) + ) + } + const sshRepo = { + id: 'repo-1', + path: '/remote/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 0, + connectionId: 'target-a' + } + store.getRepos.mockReturnValue([sshRepo]) + getSshGitProviderMock.mockReturnValue(provider) - const pending = handlers['worktrees:listDetected'](ipcEvent, { - providerRequestId: 'request-1' as ProviderRequestId, - repoId: sshRepo.id, - executionHostId: toSshExecutionHostId('target-a'), - expectedAuthority: getSshProviderAuthority('target-a') - }) - await Promise.resolve() - handlers['worktrees:cancelListDetected'](ipcEvent, { - providerRequestId: 'request-1' as ProviderRequestId - }) + const pending = handlers['worktrees:listDetected'](ipcEvent, { + providerRequestId: 'request-1' as ProviderRequestId, + repoId: sshRepo.id, + executionHostId: toSshExecutionHostId('target-a'), + expectedAuthority: getSshProviderAuthority('target-a') + }) + await Promise.resolve() + if (eventName === 'cancel') { + handlers['worktrees:cancelListDetected'](ipcEvent, { + providerRequestId: 'request-1' as ProviderRequestId + }) + } else { + ipcEvent.sender.emit(eventName) + } - expect(providerSignal?.aborted).toBe(true) - await expect(pending).resolves.toMatchObject({ - status: 'canceled', - providerRequestId: 'request-1' - }) - expect(store.setWorktreeMeta).not.toHaveBeenCalled() - expect(store.removeWorktreeLineage).not.toHaveBeenCalled() - }) + expect(providerSignal?.aborted).toBe(true) + await expect(pending).resolves.toMatchObject({ + status: 'canceled', + providerRequestId: 'request-1' + }) + expect(store.setWorktreeMeta).not.toHaveBeenCalled() + expect(store.removeWorktreeLineage).not.toHaveBeenCalled() + expect(ipcEvent.sender.eventNames()).toEqual([]) + } + ) it('settles a noncooperative SSH provider at the main-owned deadline and cleans up', async () => { vi.useFakeTimers() diff --git a/src/main/ipc/worktrees-test-ipc-surface.ts b/src/main/ipc/worktrees-test-ipc-surface.ts index 7df4f6cda0a..985107b067e 100644 --- a/src/main/ipc/worktrees-test-ipc-surface.ts +++ b/src/main/ipc/worktrees-test-ipc-surface.ts @@ -1,3 +1,4 @@ +import { EventEmitter } from 'node:events' import { type Mock, vi } from 'vitest' import type { WorktreeMeta } from '../../shared/worktree/meta-types' @@ -50,7 +51,7 @@ export const mainWindow: TestMainWindow = { send: vi.fn() } } -export const ipcEvent = { sender: { id: 1 } } +export const ipcEvent = { sender: Object.assign(new EventEmitter(), { id: 1 }) } export const store: TestStore = { getProfileStorageDirectory: vi.fn(() => '/profile-a'), getRepos: vi.fn(),