From 3d91e9a0037bd904fedafd60561a63fa431e558e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 19 Oct 2026 08:00:37 -0700 Subject: [PATCH] fix(projects): removing a project on this computer stops its chats, pages and every terminal on the host (#30728) * fix(projects): removing a project on this computer stops its sessions on the host, like repo.rm * fix(runtime): a spent removal budget still asks every project terminal to stop; paired residue rows are no twins --- src/main/ipc/repos.ts | 2 +- .../project-host-setup-wsl-move-fence.test.ts | 2 +- src/main/ipc/repos/repo-catalog-handlers.ts | 25 +++++++- .../ipc/repos/repo-ipc-wsl-move-fence.test.ts | 2 +- .../repo-remove-for-host-teardown.test.ts | 58 +++++++++++++++++++ ...roject-removal-structured-sessions.test.ts | 31 ++++++++++ .../runtime-repository-settings-controller.ts | 5 +- .../terminal/stop/project-terminal-stop.ts | 18 ++++-- .../stop/worktree-terminal-stop-options.ts | 3 + .../terminal/stop/worktree-terminal-stop.ts | 23 ++++++-- src/renderer/src/store/repos/repo-removal.ts | 1 + 11 files changed, 154 insertions(+), 16 deletions(-) create mode 100644 src/main/ipc/repos/repo-remove-for-host-teardown.test.ts diff --git a/src/main/ipc/repos.ts b/src/main/ipc/repos.ts index 1c2b5d8473a..ef7520e47b2 100644 --- a/src/main/ipc/repos.ts +++ b/src/main/ipc/repos.ts @@ -63,7 +63,7 @@ export function registerRepoHandlers( ipcMain.removeHandler('sparsePresets:save') ipcMain.removeHandler('sparsePresets:remove') - registerRepoCatalogHandlers(mainWindow, store) + registerRepoCatalogHandlers(mainWindow, store, runtime) registerProjectHostSetupHandlers(mainWindow, store) registerRepoCreationHandlers(mainWindow, store) registerProjectGroupHandlers(mainWindow, store) diff --git a/src/main/ipc/repos/project-host-setup-wsl-move-fence.test.ts b/src/main/ipc/repos/project-host-setup-wsl-move-fence.test.ts index c4e566178b3..62658d5c57f 100644 --- a/src/main/ipc/repos/project-host-setup-wsl-move-fence.test.ts +++ b/src/main/ipc/repos/project-host-setup-wsl-move-fence.test.ts @@ -146,7 +146,7 @@ describe('projects:update once the WSL host owns a repo', () => { it('lets project-wide settings change again', async () => { store.addRepo(repo('repo-1')) seedWslMoveCutover(testState.dir, cutover('move-1', 'destination-committed')) - registerRepoCatalogHandlers(new BrowserWindow(), store) + registerRepoCatalogHandlers(new BrowserWindow(), store, { removeProject: vi.fn() }) const project = store.getProjects().find((entry) => entry.sourceRepoIds.includes('repo-1')) const updates = { localWindowsRuntimePreference: { kind: 'windows-host' } } diff --git a/src/main/ipc/repos/repo-catalog-handlers.ts b/src/main/ipc/repos/repo-catalog-handlers.ts index 071c8d9dfd9..0db9c5b89fd 100644 --- a/src/main/ipc/repos/repo-catalog-handlers.ts +++ b/src/main/ipc/repos/repo-catalog-handlers.ts @@ -2,11 +2,17 @@ import type { BrowserWindow } from 'electron' import { ipcMain } from 'electron' import type { Store } from '../../persistence' import type { Project, ProjectUpdateArgs } from '../../../shared/project-types' -import { normalizeExecutionHostId } from '../../../shared/execution-host' +import { + getRepoExecutionHostId, + normalizeExecutionHostId, + parseExecutionHostId +} from '../../../shared/execution-host' +import type { OrcaRuntimeService } from '../../runtime/orca-runtime' import { enrichRepoGitUsernames } from '../../repo-git-username-enrichment' import { enrichMissingRepoGitRemoteIdentities } from '../../repo-git-remote-identity-enrichment' import { invalidateAuthorizedRootsCache } from '../registered-worktree-roots-cache' import { notifyReposChanged } from './repos-changed-notification' +import { scheduleCurrentWorktreeBaseDirectoryWatcherSync } from '../worktree-base-directory-watcher' import { ProjectUpdateIpcArgs, parseProjectGroupIpcArgs } from './repo-ipc-arg-schemas' import { visibleRepos } from '../../ssh/orcad-retained-source' import { @@ -15,7 +21,11 @@ import { isWslMoveSourceHost } from '../../persistence/moving-wsl-projects/wsl-move-scope-guard' -export function registerRepoCatalogHandlers(mainWindow: BrowserWindow, store: Store): void { +export function registerRepoCatalogHandlers( + mainWindow: BrowserWindow, + store: Store, + runtime: Pick +): void { // Why one shared reference: enrichment dedupes coalesced callers by callback identity, so a fresh // closure per list call would stack up (and re-broadcast) for the length of a slow sweep. const broadcastReposChanged = (): void => notifyReposChanged(mainWindow) @@ -93,6 +103,17 @@ export function registerRepoCatalogHandlers(mainWindow: BrowserWindow, store: St if (isWslMoveSourceHost(hostId)) { assertWslMoveProjectWritable(store, { repoId: args.repoId }) } + const row = store + .getRepos() + .find((repo) => repo.id === args.repoId && getRepoExecutionHostId(repo) === hostId) + // Why the runtime's removal: this process runs the project's local and SSH sessions, and only + // the host's own sweep stops the ones no renderer lists. A paired server's row is residue. + if (row && parseExecutionHostId(hostId)?.kind !== 'runtime') { + // It notifies the renderer and paired clients itself. + await runtime.removeProject(row) + scheduleCurrentWorktreeBaseDirectoryWatcherSync() + return + } store.removeProjectForHost(args.repoId, hostId) invalidateAuthorizedRootsCache() notifyReposChanged(mainWindow) diff --git a/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts b/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts index 1b8ec667fa5..fdc80d51e10 100644 --- a/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts +++ b/src/main/ipc/repos/repo-ipc-wsl-move-fence.test.ts @@ -112,7 +112,7 @@ describe('projects:update under a WSL move', () => { store.addRepo(repo('repo-1')) store.addRepo(repo('repo-2')) seedWslMoveCutover(testState.dir, cutover('move-1', 'source-fenced')) - registerRepoCatalogHandlers(new BrowserWindow(), store) + registerRepoCatalogHandlers(new BrowserWindow(), store, { removeProject: vi.fn() }) const projectFor = (repoId: string) => store.getProjects().find((project) => project.sourceRepoIds.includes(repoId)) const updates = { localWindowsRuntimePreference: { kind: 'windows-host' } } diff --git a/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts b/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts new file mode 100644 index 00000000000..55f05f054b2 --- /dev/null +++ b/src/main/ipc/repos/repo-remove-for-host-teardown.test.ts @@ -0,0 +1,58 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../shared/repo-types' + +const { handlers } = vi.hoisted(() => ({ + handlers: new Map unknown>() +})) + +vi.mock('electron', () => ({ + ipcMain: { + handle: (channel: string, handler: (event: unknown, args: unknown) => unknown) => + handlers.set(channel, handler) + } +})) + +vi.mock('../worktree-base-directory-watcher', () => ({ + scheduleCurrentWorktreeBaseDirectoryWatcherSync: vi.fn() +})) + +const { registerRepoCatalogHandlers } = await import('./repo-catalog-handlers') + +const repo = (fields: Partial): Repo => ({ + id: 'repo-1', + path: '/work/repo', + displayName: 'repo', + badgeColor: '#000', + addedAt: 1, + ...fields +}) + +function register(repos: Repo[]) { + const store = { getRepos: () => repos, removeProjectForHost: vi.fn() } + const runtime = { removeProject: vi.fn(async () => ({ removed: true as const })) } + const mainWindow = { isDestroyed: () => false, webContents: { send: vi.fn() } } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: removeForHost reads only getRepos and removeProjectForHost. + registerRepoCatalogHandlers(mainWindow as never, store as never, runtime) + return { store, runtime, remove: handlers.get('repos:removeForHost')! } +} + +describe('repos:removeForHost', () => { + beforeEach(() => handlers.clear()) + + it("removes this computer's project through the runtime, which stops its sessions first", async () => { + const local = repo({}) + const ssh = repo({ executionHostId: 'ssh:box', connectionId: 'box' }) + const { store, runtime, remove } = register([local, ssh]) + await remove(null, { repoId: 'repo-1', hostId: 'ssh:box' }) + expect(runtime.removeProject).toHaveBeenCalledWith(ssh) + expect(store.removeProjectForHost).not.toHaveBeenCalled() + }) + + it("only drops a paired server's residue row, which runs nothing here", async () => { + const paired = repo({ executionHostId: 'runtime:env-1' }) + const { store, runtime, remove } = register([paired]) + await remove(null, { repoId: 'repo-1', hostId: 'runtime:env-1' }) + expect(runtime.removeProject).not.toHaveBeenCalled() + expect(store.removeProjectForHost).toHaveBeenCalledWith('repo-1', 'runtime:env-1') + }) +}) diff --git a/src/main/runtime/project-removal-structured-sessions.test.ts b/src/main/runtime/project-removal-structured-sessions.test.ts index 1783409cfe4..be2f6219e58 100644 --- a/src/main/runtime/project-removal-structured-sessions.test.ts +++ b/src/main/runtime/project-removal-structured-sessions.test.ts @@ -152,4 +152,35 @@ describe('project removal and native chat sessions', () => { expect(closed).toEqual(['live-chat']) }) + + it("closes a local project's chats though a paired server's residue row shares its id", async () => { + const localRow = store.getRepo(TEST_REPO_ID)! + const residue: Repo = { ...localRow, executionHostId: 'runtime:env-1' } + const { closed } = installHost([record('local-chat', TEST_WORKTREE_ID)]) + const runtime = createRuntime([localRow, residue]) + + await runtime.removeProject(localRow) + + expect(closed).toEqual(['local-chat']) + }) + + it('still asks every terminal to stop once the removal budget is spent', async () => { + installHost([]) + const runtime = createRuntime([store.getRepo(TEST_REPO_ID)!]) + runtime.registerPty('pty-1', TEST_WORKTREE_ID, null) + runtime.registerPty('pty-2', TEST_WORKTREE_ID, null) + const kill = vi.fn((_ptyId: string) => true) + runtime.setPtyController({ + write: () => true, + kill, + stopAndWait: vi.fn(async () => true), + getForegroundProcess: async () => null + }) + + await runtime.stopTerminalsForProject(store.getRepo(TEST_REPO_ID)!, { + deadline: Date.now() - 1 + }) + + expect(kill.mock.calls.map(([ptyId]) => ptyId).sort()).toEqual(['pty-1', 'pty-2']) + }) }) diff --git a/src/main/runtime/runtime-repository-settings-controller.ts b/src/main/runtime/runtime-repository-settings-controller.ts index bf78bbb74a2..014f9482dc7 100644 --- a/src/main/runtime/runtime-repository-settings-controller.ts +++ b/src/main/runtime/runtime-repository-settings-controller.ts @@ -123,12 +123,13 @@ export class RuntimeRepositorySettingsController { return updated } - async remove(repoSelector: string): Promise<{ removed: true }> { + /** `repo.rm` names a selector; this desktop's own removal names the exact host row. */ + async remove(target: string | Repo): Promise<{ removed: true }> { const store = this.deps.getStore() if (!store?.removeProjectForHost) { throw new Error('runtime_unavailable') } - const repo = await this.deps.resolveRepo(repoSelector) + const repo = typeof target === 'string' ? await this.deps.resolveRepo(target) : target const hostId = getRepoExecutionHostId(repo) assertWslMoveRepoWritable(store, repo) // Why here: the host owns these shells; a client only stops the workspaces it has loaded, diff --git a/src/main/runtime/terminal/stop/project-terminal-stop.ts b/src/main/runtime/terminal/stop/project-terminal-stop.ts index 4924631aef1..1ac942109c4 100644 --- a/src/main/runtime/terminal/stop/project-terminal-stop.ts +++ b/src/main/runtime/terminal/stop/project-terminal-stop.ts @@ -2,7 +2,8 @@ import { OrcaRuntimeWithStopExactTerminalsForWorktree } from './worktree-termina import { getRepoExecutionHostId, getRepoSshConnectionId, - normalizeHostPart + normalizeHostPart, + parseExecutionHostId } from '../../../../shared/execution-host' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' import type { Repo } from '../../../../shared/repo-types' @@ -119,7 +120,7 @@ export abstract class OrcaRuntimeWithStopTerminalsForProject extends OrcaRuntime /** Browser pages and failed-launch records of a removed project's workspaces on this host. */ private releaseProjectHostSessions(repoId: string, knownWorktreeIds: readonly string[]): void { // Why: page records carry no host, so a surviving same-id row's pages are indistinguishable. - if ((this.store?.getRepos?.() ?? []).some((row) => row.id === repoId)) { + if (this.rowsRunHere().some((row) => row.id === repoId)) { return } const pageOwners = [ @@ -174,9 +175,9 @@ export abstract class OrcaRuntimeWithStopTerminalsForProject extends OrcaRuntime projectWorktreeIds.add(id) } for (const id of projectWorktreeIds) { - const opts = { resolvedWorktreeId: id, ...fence, ...options } + const opts = { resolvedWorktreeId: id, ...fence, ...options, requestStopsPastDeadline: true } // Why exclusive: it waits out a spawn that holds the shared lease, so its PTY is registered - // and stopped. Bounded by the deadline so repo.rm still answers; past it the stop is skipped. + // and stopped. Bounded by the deadline so repo.rm still answers; past it PTYs are only asked. // A timed-out waiter is dequeued by the lock, so it cannot hold later spawns back. const release = await this.acquireWorktreeTerminalMutation( id, @@ -240,12 +241,19 @@ export abstract class OrcaRuntimeWithStopTerminalsForProject extends OrcaRuntime return survivors.some((row) => getRepoSshConnectionId(row) === connectionId) } + /** A paired server's row is residue here: its sessions run on that server, never in this process. */ + private rowsRunHere(): Repo[] { + return (this.store?.getRepos?.() ?? []).filter( + (row) => parseExecutionHostId(getRepoExecutionHostId(row))?.kind !== 'runtime' + ) + } + /** A same-id sibling row with the same fence owns sessions this host cannot tell apart. */ private projectSiblingSharesFence( repo: Pick ): boolean { const connectionId = getRepoSshConnectionId(repo) - return (this.store?.getRepos?.() ?? []).some( + return this.rowsRunHere().some( (row) => row.id === repo.id && getRepoExecutionHostId(row) !== getRepoExecutionHostId(repo) && diff --git a/src/main/runtime/terminal/stop/worktree-terminal-stop-options.ts b/src/main/runtime/terminal/stop/worktree-terminal-stop-options.ts index 602703295ec..06278ac45c8 100644 --- a/src/main/runtime/terminal/stop/worktree-terminal-stop-options.ts +++ b/src/main/runtime/terminal/stop/worktree-terminal-stop-options.ts @@ -1,5 +1,8 @@ export type WorktreeTerminalStopOptions = { deadline?: number + /** Past the deadline, still send each remaining PTY a stop without waiting on it. Only for a + * caller giving the PTYs up for good: the stop is recorded and may land after it returns. */ + requestStopsPastDeadline?: boolean stopPty?: ( ptyId: string, stop: () => Promise diff --git a/src/main/runtime/terminal/stop/worktree-terminal-stop.ts b/src/main/runtime/terminal/stop/worktree-terminal-stop.ts index a9bc8732671..ca993e042cc 100644 --- a/src/main/runtime/terminal/stop/worktree-terminal-stop.ts +++ b/src/main/runtime/terminal/stop/worktree-terminal-stop.ts @@ -180,7 +180,9 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim ? { id: options.resolvedWorktreeId } : await this.resolveWorktreeSelector(worktreeSelector) this.assertStableReadyGraph(graphEpoch) - if (options.deadline !== undefined && Date.now() >= options.deadline) { + const pastDeadline = (): boolean => + options.deadline !== undefined && Date.now() >= options.deadline + if (pastDeadline() && !options.requestStopsPastDeadline) { return { stopped: 0 } } // Preserve folder-instance suffixes while normalizing cross-platform path spelling. @@ -194,11 +196,16 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim let stopped = 0 for (const ptyId of ptyIds) { - if (options.deadline !== undefined && Date.now() >= options.deadline) { - break + if (pastDeadline()) { + if (!options.requestStopsPastDeadline) { + break + } + // Unverified, so never counted: the budget bounds the wait, not who is asked to stop. + this.requestPtyStopWithoutWaiting(ptyId) + continue } const stop = async (): Promise => { - if (options.deadline !== undefined && Date.now() >= options.deadline) { + if (pastDeadline()) { return false } try { @@ -230,6 +237,14 @@ export abstract class OrcaRuntimeWithStopTerminalsForWorktree extends OrcaRuntim return { stopped } } + private requestPtyStopWithoutWaiting(ptyId: string): void { + try { + this.ptyController?.kill(ptyId) + } catch (error) { + console.warn(`[runtime] failed to request a stop for terminal ${ptyId}`, error) + } + } + async sleepTerminalsForWorktree( worktreeSelector: string ): Promise { diff --git a/src/renderer/src/store/repos/repo-removal.ts b/src/renderer/src/store/repos/repo-removal.ts index b28c266e4ac..a05f0d17c25 100644 --- a/src/renderer/src/store/repos/repo-removal.ts +++ b/src/renderer/src/store/repos/repo-removal.ts @@ -200,6 +200,7 @@ export function createRepoRemovalActions( .flatMap((worktree) => (worktree.projectId ? [worktree.projectId] : [])) ] : [] + // Backstop for the host's sweep, which a spent budget or reloading graph can cut short. for (const wId of exclusiveWorktreeIds) { const tabs = get().tabsByWorktree[wId] ?? [] for (const tab of tabs) {