From f26702abd261ecf1055e02873ee0e044651ef6bb Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:13:48 -0700 Subject: [PATCH] fix(worktrees): stop runtime-stamped folder PTYs and trust the resolved repo host on folder deletion Two folder-workspace gaps in the #12388 fence audit, with red-before proof for each: removeManagedWorktree treated a runtime stamp as an external desktop-mirror namespace and skipped the selected runtime server's local PTYs, and desktop folder deletion trusted collision-prone global worktree metadata over the already-resolved repo host. Authored by the post-merge audit reviewer; committed by the coordinator for the gated follow-up PR. --- src/main/ipc/worktrees.test.ts | 33 +++++++++++++++++++++++++++ src/main/ipc/worktrees.ts | 6 ++--- src/main/runtime/orca-runtime.test.ts | 4 +++- src/main/runtime/orca-runtime.ts | 24 ++++++++----------- 4 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/main/ipc/worktrees.test.ts b/src/main/ipc/worktrees.test.ts index 8d576b965ee..c83eece63b6 100644 --- a/src/main/ipc/worktrees.test.ts +++ b/src/main/ipc/worktrees.test.ts @@ -8380,6 +8380,8 @@ describe('registerWorktreeHandlers', () => { connectionId: 'conn-1' }) getSshPtyProviderMock.mockReturnValue(sshPtyProvider) + // One global meta key can describe the same-id local copy; the resolved repo still owns this delete. + store.getWorktreeMeta.mockReturnValue(makeWorktreeMeta({ hostId: 'local' })) await handlers['worktrees:remove'](null, { worktreeId }) @@ -8395,6 +8397,37 @@ describe('registerWorktreeHandlers', () => { }) }) + it('fences a mirrored runtime folder workspace sweep to its environment', async () => { + const runtimePtyProvider = {} as never + const worktreeId = 'repo-folder::/runtime/folder::workspace:child-1' + store.getRepo.mockReturnValue({ + id: 'repo-folder', + path: '/runtime/folder', + displayName: 'folder', + badgeColor: '#000', + addedAt: 0, + kind: 'folder', + executionHostId: 'runtime:env-1' + }) + getLocalPtyProviderMock.mockReturnValue(runtimePtyProvider) + + await handlers['worktrees:remove'](null, { + worktreeId, + hostId: 'runtime:env-1' + }) + + expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(worktreeId, { + runtime: runtimeStub, + resolvedWorktreeId: worktreeId, + resolvedRuntimeEnvironmentId: 'env-1', + localProvider: runtimePtyProvider, + onPtyStopped: clearProviderPtyStateMock, + includeProviderInventory: false, + includeLocalRegistry: false + }) + expect(getSshPtyProviderMock).not.toHaveBeenCalled() + }) + it('runs the archive hook on remove when skipArchive is not set', async () => { mockKnownFeatureWorktree() removeWorktreeMock.mockResolvedValue(undefined) diff --git a/src/main/ipc/worktrees.ts b/src/main/ipc/worktrees.ts index dde33ca7628..c8dceec6bed 100644 --- a/src/main/ipc/worktrees.ts +++ b/src/main/ipc/worktrees.ts @@ -2297,9 +2297,9 @@ export function registerWorktreeHandlers( await withWorktreeRemoveStageSpan('pty_sweep', 'folder', async () => { // Folder projects can be SSH-backed, so fence the sweep to the owning host exactly // like the git paths — the local inventory must never reach a remote workspace's id. - const ownerHost = parseExecutionHostId( - resolveWorktreeRemovalOwnerHostId(store, args.worktreeId, repo, args.hostId) - ) + // The resolved repo is authoritative here: path-derived metadata is shared by + // same-id host copies and can describe a different owner's workspace. + const ownerHost = parseExecutionHostId(removalHostId) const sshPtyProvider = ownerHost?.kind === 'ssh' ? getSshPtyProvider(ownerHost.targetId) : undefined const externalHost = ownerHost?.kind === 'ssh' || ownerHost?.kind === 'runtime' diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 3aa249a0d99..722acf67937 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -3943,7 +3943,9 @@ describe('OrcaRuntimeService', () => { displayName: 'Folder', badgeColor: 'blue', addedAt: 1, - kind: 'folder' as const + kind: 'folder' as const, + // removeManagedWorktree executes inside this selected runtime, where PTYs are local ids. + executionHostId: 'runtime:env-1' as const } const rootWorktreeId = 'folder-repo::/workspace/folder' const rootPriorWorktreeIds = ['folder-repo::/workspace/old-folder'] diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 088f52d5b26..849cfdc9c72 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -23511,31 +23511,25 @@ export class OrcaRuntimeService { 'Cannot delete the project root workspace. Remove the folder project instead.' ) } - // Folder projects can be SSH-backed, so resolve the owner before sweeping. - const folderHost = parseExecutionHostId( - store.getWorktreeMeta(removalTarget.id)?.hostId ?? getRepoExecutionHostId(repo) - ) - const folderSshPtyProvider = - folderHost?.kind === 'ssh' ? this.getSshProviderFn?.(folderHost.targetId) : undefined - const externalFolderHost = folderHost?.kind === 'ssh' || folderHost?.kind === 'runtime' + // This service runs inside the selected runtime, so runtime-stamped repos use its + // local PTY namespace; only a direct SSH connection is external from here. + const folderConnectionId = repo.connectionId?.trim() || null + const folderSshPtyProvider = folderConnectionId + ? this.getSshProviderFn?.(folderConnectionId) + : undefined const folderPtyProvider = folderSshPtyProvider ?? this.getLocalProvider() if (folderPtyProvider) { // Why: folder workspace deletion has no Git removal phase where PTYs // would otherwise be swept; tear them down before hiding the workspace. await killAllProcessesForWorktree(removalTarget.id, { runtime: this, - // External host inventories must never sweep a same-id local workspace. resolvedWorktreeId: removalTarget.id, - ...(folderHost?.kind === 'ssh' ? { resolvedConnectionId: folderHost.targetId } : {}), - ...(folderHost?.kind === 'runtime' - ? { resolvedRuntimeEnvironmentId: folderHost.environmentId } - : {}), + ...(folderConnectionId ? { resolvedConnectionId: folderConnectionId } : {}), localProvider: folderPtyProvider, onPtyStopped: this.onPtyStopped ?? undefined, - ...(externalFolderHost + ...(folderConnectionId ? { - includeProviderInventory: - folderHost?.kind === 'ssh' && Boolean(folderSshPtyProvider), + includeProviderInventory: Boolean(folderSshPtyProvider), includeLocalRegistry: false } : {})