From a9f2fbb684b40cff0b23e2eee2fccbc5428df471 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:00:46 -0700 Subject: [PATCH] chore(workspaces): drop the dead workspaceCleanup:hasKillableLocalProcesses IPC (#18386) --- ...orkspace-cleanup-process-preflight.test.ts | 112 ----------------- src/main/ipc/workspace-cleanup.ts | 116 +----------------- .../window/attach-main-window-services.ts | 3 +- src/preload/api/workspace-cleanup-api.ts | 5 - src/preload/api/workspace-cleanup-bridge.ts | 2 - ...anupDialog.stale-while-revalidate.test.tsx | 3 +- ...orkspace-cleanup-removal-preflight.test.ts | 20 +-- .../workspace-cleanup-slice-test-harness.ts | 5 +- src/shared/workspace-cleanup.ts | 10 -- 9 files changed, 8 insertions(+), 268 deletions(-) delete mode 100644 src/main/ipc/workspace-cleanup-process-preflight.test.ts diff --git a/src/main/ipc/workspace-cleanup-process-preflight.test.ts b/src/main/ipc/workspace-cleanup-process-preflight.test.ts deleted file mode 100644 index 1c6b8e78c30..00000000000 --- a/src/main/ipc/workspace-cleanup-process-preflight.test.ts +++ /dev/null @@ -1,112 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { ipcMain } from 'electron' -import type { Store } from '../persistence' - -const { getSshPtyProviderMock } = vi.hoisted(() => ({ - getSshPtyProviderMock: vi.fn() -})) - -vi.mock('electron', () => ({ - ipcMain: { - handle: vi.fn(), - removeHandler: vi.fn() - } -})) - -vi.mock('./pty', () => ({ - getSshPtyProvider: getSshPtyProviderMock -})) - -vi.mock('../memory/pty-registry', () => ({ - listRegisteredPtys: vi.fn(() => []) -})) - -vi.mock('../workspace-cleanup-scan-snapshot', () => ({ - persistWorkspaceCleanupScanResult: vi.fn(async () => undefined), - readWorkspaceCleanupScanSnapshot: vi.fn(async () => null) -})) - -vi.mock('../workspace-cleanup-removal-snapshot-prune', () => ({ - beginWorkspaceCleanupRemovalSnapshotPruneBatch: vi.fn(), - finishWorkspaceCleanupRemovalSnapshotPruneBatch: vi.fn(async () => undefined), - recordWorkspaceCleanupRemovalSnapshotPrune: vi.fn() -})) - -import { registerWorkspaceCleanupHandlers } from './workspace-cleanup' - -function makeEmptyStore(): Store { - return { - getProfileStorageDirectory: () => '/profile-a', - getRepos: () => [], - getWorktreeMeta: () => ({}), - getAllWorktreeMeta: () => ({}), - getGitHubCache: () => ({ pr: {}, issue: {} }) - } as unknown as Store -} - -function getPreflightHandler(): ((...args: never[]) => unknown) | undefined { - return vi - .mocked(ipcMain.handle) - .mock.calls.find(([channel]) => channel === 'workspaceCleanup:hasKillableLocalProcesses')?.[1] -} - -describe('workspace cleanup process preflight', () => { - beforeEach(() => { - vi.mocked(ipcMain.handle).mockReset() - getSshPtyProviderMock.mockReset() - }) - - it('reports local processes that workspace deletion would kill', async () => { - const localProvider = { - listProcesses: vi.fn().mockResolvedValue([ - { - id: 'repo-1::/repo-feature@@session-1', - cwd: '/repo-feature', - title: 'zsh' - } - ]) - } - registerWorkspaceCleanupHandlers(makeEmptyStore(), { - runtime: { - hasTerminalsForWorktree: vi.fn().mockResolvedValue(false) - } as never, - getLocalPtyProvider: () => localProvider as never - }) - - await expect( - getPreflightHandler()?.({} as never, { worktreeId: 'repo-1::/repo-feature' } as never) - ).resolves.toEqual({ - hasKillableProcesses: true - }) - }) - - it('reports SSH processes inside the remote workspace path', async () => { - getSshPtyProviderMock.mockReturnValue({ - listProcesses: vi.fn().mockResolvedValue([ - { - id: 'remote-session-1', - cwd: '/remote/repo-feature/subdir', - title: 'codex' - } - ]) - }) - registerWorkspaceCleanupHandlers(makeEmptyStore(), { - runtime: { - hasTerminalsForWorktree: vi.fn().mockResolvedValue(false) - } as never - }) - - await expect( - getPreflightHandler()?.( - {} as never, - { - worktreeId: 'repo-ssh::/remote/repo-feature', - connectionId: 'ssh-1', - worktreePath: '/remote/repo-feature' - } as never - ) - ).resolves.toEqual({ - hasKillableProcesses: true - }) - }) -}) diff --git a/src/main/ipc/workspace-cleanup.ts b/src/main/ipc/workspace-cleanup.ts index 23bbf55602f..7c8b5a2d8f4 100644 --- a/src/main/ipc/workspace-cleanup.ts +++ b/src/main/ipc/workspace-cleanup.ts @@ -1,14 +1,8 @@ import { ipcMain } from 'electron' import type { Store } from '../persistence' -import type { IPtyProvider } from '../providers/types' -import type { OrcaRuntimeService } from '../runtime/orca-runtime' -import { listRegisteredPtys } from '../memory/pty-registry' -import { getSshPtyProvider } from './pty' import { WORKSPACE_CLEANUP_CLASSIFIER_VERSION, type WorkspaceCleanupDismissArgs, - type WorkspaceCleanupLocalProcessArgs, - type WorkspaceCleanupLocalProcessResult, type WorkspaceCleanupScanArgs, type WorkspaceCleanupScanResult, type WorkspaceCleanupSnapshotPruneBatchArgs, @@ -30,11 +24,6 @@ import { export { scanWorkspaceCleanup } -type WorkspaceCleanupHandlerDeps = { - runtime?: OrcaRuntimeService - getLocalPtyProvider?: () => IPtyProvider -} - // Why: module scope — handler re-registration on a new main window must not // orphan the previous window's controllers in a discarded map. const activeScans = new Map() @@ -47,17 +36,13 @@ function getBroadScanModeKey(senderId: number, args: WorkspaceCleanupScanArgs): return `${senderId}\0${args.includeAllWorkspaces === true}` } -export function registerWorkspaceCleanupHandlers( - store: Store, - deps: WorkspaceCleanupHandlerDeps = {} -): void { +export function registerWorkspaceCleanupHandlers(store: Store): void { const snapshotDirectory = store.getProfileStorageDirectory() ipcMain.removeHandler('workspaceCleanup:scan') ipcMain.removeHandler('workspaceCleanup:cancelScan') ipcMain.removeHandler('workspaceCleanup:getCachedScan') ipcMain.removeHandler('workspaceCleanup:dismiss') ipcMain.removeHandler('workspaceCleanup:clearDismissals') - ipcMain.removeHandler('workspaceCleanup:hasKillableLocalProcesses') ipcMain.removeHandler('workspaceCleanup:beginRemovalSnapshotPruneBatch') ipcMain.removeHandler('workspaceCleanup:recordRemovalSnapshotPrune') ipcMain.removeHandler('workspaceCleanup:finishRemovalSnapshotPruneBatch') @@ -161,16 +146,6 @@ export function registerWorkspaceCleanupHandlers( store.updateUI({ workspaceCleanup: { dismissals: {} } }) }) - ipcMain.handle( - 'workspaceCleanup:hasKillableLocalProcesses', - async ( - _event, - args: WorkspaceCleanupLocalProcessArgs - ): Promise => ({ - hasKillableProcesses: await hasKillableProcesses(args, deps) - }) - ) - ipcMain.handle( 'workspaceCleanup:beginRemovalSnapshotPruneBatch', (_event, args: WorkspaceCleanupSnapshotPruneBatchArgs) => { @@ -214,92 +189,3 @@ function getWorkspaceCleanupScanKey(senderId: number, scanId: unknown): string | ? `${senderId}\0${scanId}` : null } - -async function hasKillableProcesses( - args: WorkspaceCleanupLocalProcessArgs, - deps: WorkspaceCleanupHandlerDeps -): Promise { - const { worktreeId } = args - if (typeof worktreeId !== 'string' || worktreeId.length === 0) { - return false - } - - let livenessUnknown = false - if (deps.runtime) { - try { - if (await deps.runtime.hasTerminalsForWorktree(worktreeId)) { - return true - } - } catch { - livenessUnknown = true - } - } - - if (args.connectionId) { - return hasKillableSshProcesses(args.connectionId, args.worktreePath ?? '', livenessUnknown) - } - - const registryPtyIds = new Set( - listRegisteredPtys() - .filter((entry) => entry.worktreeId === worktreeId) - .map((entry) => entry.ptyId) - ) - - const provider = deps.getLocalPtyProvider?.() - if (!provider) { - return registryPtyIds.size > 0 ? true : null - } - - try { - const prefix = `${worktreeId}@@` - const sessions = await provider.listProcesses() - if ( - sessions.some((session) => session.id.startsWith(prefix) || registryPtyIds.has(session.id)) - ) { - return true - } - return livenessUnknown ? null : false - } catch { - return registryPtyIds.size > 0 ? true : null - } -} - -async function hasKillableSshProcesses( - connectionId: string, - worktreePath: string, - livenessUnknown: boolean -): Promise { - const provider = getSshPtyProvider(connectionId) - if (!provider) { - return null - } - - try { - const normalizedWorktreePath = normalizeRemotePath(worktreePath) - const sessions = await provider.listProcesses() - if ( - sessions.some((session) => { - if (session.id.startsWith(`${worktreePath}@@`)) { - return true - } - return ( - normalizedWorktreePath.length > 0 && - isPathWithin(normalizeRemotePath(session.cwd), normalizedWorktreePath) - ) - }) - ) { - return true - } - return livenessUnknown ? null : false - } catch { - return null - } -} - -function normalizeRemotePath(path: string): string { - return path.replace(/\\/g, '/').replace(/\/+$/, '') -} - -function isPathWithin(candidatePath: string, parentPath: string): boolean { - return candidatePath === parentPath || candidatePath.startsWith(`${parentPath}/`) -} diff --git a/src/main/window/attach-main-window-services.ts b/src/main/window/attach-main-window-services.ts index ff7d84bd706..46621d7f76c 100644 --- a/src/main/window/attach-main-window-services.ts +++ b/src/main/window/attach-main-window-services.ts @@ -13,7 +13,6 @@ import { setWorktreeCatalogRemoteClientNotifier } from '../ipc/watched-worktree- import { registerWorktreeHandlers } from '../ipc/worktrees' import { registerWorkspaceCleanupHandlers } from '../ipc/workspace-cleanup' import { - getLocalPtyProvider, registerPtyHandlers, type CodexHomePtySpawnedLifecycleArgs, type GetSelectedCodexHomePath, @@ -83,7 +82,7 @@ export function attachMainWindowServices( // Why: folder projects get no watch target, so an external `git init` needs its own // marker poll to upgrade them without a restart (#11477). startFolderRepoGitUpgradeWatch(store, mainWindow) - registerWorkspaceCleanupHandlers(store, { runtime, getLocalPtyProvider }) + registerWorkspaceCleanupHandlers(store) registerPtyHandlers( mainWindow, runtime, diff --git a/src/preload/api/workspace-cleanup-api.ts b/src/preload/api/workspace-cleanup-api.ts index 134dc2ee519..9f224efd88b 100644 --- a/src/preload/api/workspace-cleanup-api.ts +++ b/src/preload/api/workspace-cleanup-api.ts @@ -1,7 +1,5 @@ import type { WorkspaceCleanupDismissArgs, - WorkspaceCleanupLocalProcessArgs, - WorkspaceCleanupLocalProcessResult, WorkspaceCleanupScanArgs, WorkspaceCleanupScanProgress, WorkspaceCleanupScanResult, @@ -24,9 +22,6 @@ export type WorkspaceCleanupApi = { getCachedScan: () => Promise dismiss: (args: WorkspaceCleanupDismissArgs) => Promise clearDismissals: () => Promise - hasKillableLocalProcesses: ( - args: WorkspaceCleanupLocalProcessArgs - ) => Promise beginRemovalSnapshotPruneBatch?: (args: WorkspaceCleanupSnapshotPruneBatchArgs) => Promise recordRemovalSnapshotPrune?: (args: WorkspaceCleanupSnapshotPruneRecordArgs) => Promise finishRemovalSnapshotPruneBatch?: (args: WorkspaceCleanupSnapshotPruneBatchArgs) => Promise diff --git a/src/preload/api/workspace-cleanup-bridge.ts b/src/preload/api/workspace-cleanup-bridge.ts index e9e4ae227cb..04fc20800b7 100644 --- a/src/preload/api/workspace-cleanup-bridge.ts +++ b/src/preload/api/workspace-cleanup-bridge.ts @@ -25,8 +25,6 @@ export const workspaceCleanupApi = { getCachedScan: () => ipcRenderer.invoke('workspaceCleanup:getCachedScan'), dismiss: (args) => ipcRenderer.invoke('workspaceCleanup:dismiss', args), clearDismissals: () => ipcRenderer.invoke('workspaceCleanup:clearDismissals'), - hasKillableLocalProcesses: (args) => - ipcRenderer.invoke('workspaceCleanup:hasKillableLocalProcesses', args), beginRemovalSnapshotPruneBatch: (args) => ipcRenderer.invoke('workspaceCleanup:beginRemovalSnapshotPruneBatch', args), recordRemovalSnapshotPrune: (args) => diff --git a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsx b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsx index 262cc541526..be838766b58 100644 --- a/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsx +++ b/src/renderer/src/components/workspace-cleanup/WorkspaceCleanupDialog.stale-while-revalidate.test.tsx @@ -120,8 +120,7 @@ function installApi(cachedScan: WorkspaceCleanupScanResult | null): ScanRig { scan: rig.scan, getCachedScan: vi.fn().mockResolvedValue(cachedScan), dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ hasKillableProcesses: false }) + clearDismissals: vi.fn().mockResolvedValue(undefined) }, workspaceSpace: { getCachedAnalysis: vi.fn().mockResolvedValue(null), diff --git a/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts b/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts index d9e656c3242..ae4b606e6d5 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-removal-preflight.test.ts @@ -258,10 +258,7 @@ describe('workspace cleanup removal and protection', () => { }) ), dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ - hasKillableProcesses: false - }) + clearDismissals: vi.fn().mockResolvedValue(undefined) } } } @@ -291,10 +288,7 @@ describe('workspace cleanup removal and protection', () => { workspaceCleanup: { scan, dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ - hasKillableProcesses: false - }) + clearDismissals: vi.fn().mockResolvedValue(undefined) } } } @@ -333,10 +327,7 @@ describe('workspace cleanup removal and protection', () => { workspaceCleanup: { scan, dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ - hasKillableProcesses: false - }) + clearDismissals: vi.fn().mockResolvedValue(undefined) } } } @@ -368,10 +359,7 @@ describe('workspace cleanup removal and protection', () => { workspaceCleanup: { scan, dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ - hasKillableProcesses: true - }) + clearDismissals: vi.fn().mockResolvedValue(undefined) } } } diff --git a/src/renderer/src/store/slices/workspace-cleanup-slice-test-harness.ts b/src/renderer/src/store/slices/workspace-cleanup-slice-test-harness.ts index 07f3e660645..cfde8bf76bc 100644 --- a/src/renderer/src/store/slices/workspace-cleanup-slice-test-harness.ts +++ b/src/renderer/src/store/slices/workspace-cleanup-slice-test-harness.ts @@ -93,10 +93,7 @@ export function installWorkspaceCleanupApi( scan, getCachedScan, dismiss: vi.fn().mockResolvedValue(undefined), - clearDismissals: vi.fn().mockResolvedValue(undefined), - hasKillableLocalProcesses: vi.fn().mockResolvedValue({ - hasKillableProcesses: false - }) + clearDismissals: vi.fn().mockResolvedValue(undefined) } } } diff --git a/src/shared/workspace-cleanup.ts b/src/shared/workspace-cleanup.ts index a2bedc2d9ed..603b050c38a 100644 --- a/src/shared/workspace-cleanup.ts +++ b/src/shared/workspace-cleanup.ts @@ -100,12 +100,6 @@ export type WorkspaceCleanupScanArgs = { export const WORKSPACE_CLEANUP_TARGET_BATCH_LIMIT = 500 -export type WorkspaceCleanupLocalProcessArgs = { - worktreeId: string - connectionId?: string | null - worktreePath?: string -} - export type WorkspaceCleanupSnapshotPruneBatchArgs = { batchId: string } @@ -144,10 +138,6 @@ export type WorkspaceCleanupUnverifiedRemovalConsent = { attemptId: string } -export type WorkspaceCleanupLocalProcessResult = { - hasKillableProcesses: boolean | null -} - export type WorkspaceCleanupDismissArgs = { dismissals: WorkspaceCleanupDismissal[] /** Removed worktrees' persisted dismissals are dead weight; prune them. */