xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(worktrees): fence SSH worktree deletion PTY teardown to the owning host (#12388) 

Destructive worktree removal swept PTYs by worktree id alone. Worktree ids are
`repoId::path` and the store keeps one per host, so deleting an SSH worktree
could stop a same-id local (or other-connection) workspace's terminals — or fail
outright with `selector_ambiguous` when two hosts owned the id.

Every destructive teardown now names its owner (resolvedWorktreeId plus the
connection/runtime environment), matching the already-hardened forget-local path:

- IPC `worktrees:remove` (git + folder workspaces)
- runtime `removeManagedWorktree` (CLI/mobile `worktree.rm`, git + folder)
- missing-worktree terminal reconciliation, including its no-provider fallback

The #11960 allowUnverifiedStop force-delete gate is untouched.
This commit is contained in:
Brennan Benson 2026-08-03 19:18:37 -07:00 committed by GitHub
parent e39cdb897d
commit 9e5bd5fb84
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 265 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

95
src/main/ipc/worktrees.test.ts
View File

@ -8350,6 +8350,7 @@ describe('registerWorktreeHandlers', () => {
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(worktreeId, {
runtime: runtimeStub,
resolvedWorktreeId: worktreeId,
localProvider: ptyProvider,
onPtyStopped: clearProviderPtyStateMock
})
@ -8364,6 +8365,36 @@ describe('registerWorktreeHandlers', () => {
})
})
// Folder projects can be SSH-backed, and folder workspace ids are `repoId::path::workspace:<uuid>`
// — reusable across hosts — so the sweep must name the owning connection.
it('fences an SSH folder workspace PTY sweep to the owning connection', async () => {
const sshPtyProvider = { id: 'ssh-pty-provider' } as never
const worktreeId = 'repo-folder::/remote/folder::workspace:child-1'
store.getRepo.mockReturnValue({
id: 'repo-folder',
path: '/remote/folder',
displayName: 'folder',
badgeColor: '#000',
addedAt: 0,
kind: 'folder',
connectionId: 'conn-1'
})
getSshPtyProviderMock.mockReturnValue(sshPtyProvider)
await handlers['worktrees:remove'](null, { worktreeId })
expect(getSshPtyProviderMock).toHaveBeenCalledWith('conn-1')
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(worktreeId, {
runtime: runtimeStub,
resolvedWorktreeId: worktreeId,
resolvedConnectionId: 'conn-1',
localProvider: sshPtyProvider,
onPtyStopped: clearProviderPtyStateMock,
includeProviderInventory: true,
includeLocalRegistry: false
})
})
it('runs the archive hook on remove when skipArchive is not set', async () => {
mockKnownFeatureWorktree()
removeWorktreeMock.mockResolvedValue(undefined)
@ -9950,6 +9981,70 @@ describe('registerWorktreeHandlers', () => {
expect(callOrder).toEqual(['preflight', 'kill', 'git'])
})
// Regression: `repoId::path` ids repeat across hosts, so an SSH delete used to reach the
// runtime's same-id local (or other-connection) terminals and stop them.
it('fences an SSH worktree delete PTY sweep to the owning connection', async () => {
const repo = {
id: 'repo-ssh',
path: '/remote/repo',
displayName: 'ssh',
badgeColor: '#000',
addedAt: 0,
connectionId: 'conn-1',
worktreeBaseRef: null
}
const sshPtyProvider = { id: 'ssh-pty-provider' } as never
store.getRepos.mockReturnValue([repo])
store.getRepo.mockReturnValue(repo)
getSshGitProviderMock.mockReturnValue({
listWorktrees: vi.fn().mockResolvedValue([
{ path: '/remote/repo', head: 'main', branch: 'main', isBare: false, isMainWorktree: true },
{
path: '/remote/feature-wt',
head: 'feature',
branch: 'feature',
isBare: false,
isMainWorktree: false
}
]),
removeWorktree: vi.fn().mockResolvedValue({}),
worktreeIsClean: vi.fn().mockResolvedValue({ clean: true })
})
getSshPtyProviderMock.mockReturnValue(sshPtyProvider)
getEffectiveHooksFromConfigMock.mockReturnValue(null)
await handlers['worktrees:remove'](null, { worktreeId: 'repo-ssh::/remote/feature-wt' })
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith('repo-ssh::/remote/feature-wt', {
runtime: runtimeStub,
resolvedWorktreeId: 'repo-ssh::/remote/feature-wt',
resolvedConnectionId: 'conn-1',
localProvider: sshPtyProvider,
onPtyStopped: clearProviderPtyStateMock,
requirePhysicalStop: true,
includeLocalRegistry: false
})
})
// The local counterpart still identifies itself by exact id so a selector that resolves
// two hosts can no longer decide which workspace loses its terminals.
it('pins a local worktree delete PTY sweep to the exact worktree id', async () => {
mockKnownFeatureWorktree()
getEffectiveHooksMock.mockReturnValue(null)
removeWorktreeMock.mockResolvedValue({})
await handlers['worktrees:remove'](null, { worktreeId: 'repo-1::/workspace/feature-wt' })
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(
'repo-1::/workspace/feature-wt',
expect.objectContaining({ resolvedWorktreeId: 'repo-1::/workspace/feature-wt' })
)
expect(killAllProcessesForWorktreeMock).toHaveBeenCalledWith(
'repo-1::/workspace/feature-wt',
expect.not.objectContaining({ resolvedConnectionId: expect.anything() })
)
})
// Why (#11960): the PTY gate previously had no escape hatch at all, so a
// workspace with an unprovable PTY was unremovable forever.
it('forwards an explicit Force Delete to the PTY gate', async () => {

29
src/main/ipc/worktrees.ts
View File

@ -169,6 +169,11 @@ async function stopPtysForDestructiveWorktreeRemoval(
}
const teardownResult = await killAllProcessesForWorktree(worktreeId, {
runtime,
// Why: `repoId::path` ids repeat across hosts, so an unfenced sweep stops a same-id
// workspace's terminals on another connection — and the selector lookup this replaces
// throws `selector_ambiguous` the moment two hosts own the id.
resolvedWorktreeId: worktreeId,
...(connectionId ? { resolvedConnectionId: connectionId } : {}),
localProvider: provider,
onPtyStopped: clearProviderPtyState,
requirePhysicalStop: true,
@ -2290,10 +2295,30 @@ export function registerWorktreeHandlers(
}
// Why: folder workspaces share one root, so there's no Git remove step to close shells; sweep PTYs before dropping metadata.
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)
)
const sshPtyProvider =
ownerHost?.kind === 'ssh' ? getSshPtyProvider(ownerHost.targetId) : undefined
const externalHost = ownerHost?.kind === 'ssh' || ownerHost?.kind === 'runtime'
await killAllProcessesForWorktree(args.worktreeId, {
runtime,
localProvider: getLocalPtyProvider(),
onPtyStopped: clearProviderPtyState
resolvedWorktreeId: args.worktreeId,
...(ownerHost?.kind === 'ssh' ? { resolvedConnectionId: ownerHost.targetId } : {}),
...(ownerHost?.kind === 'runtime'
? { resolvedRuntimeEnvironmentId: ownerHost.environmentId }
: {}),
localProvider: sshPtyProvider ?? getLocalPtyProvider(),
onPtyStopped: clearProviderPtyState,
...(externalHost
? {
includeProviderInventory:
ownerHost?.kind === 'ssh' && Boolean(sshPtyProvider),
includeLocalRegistry: false
}
: {})
}).catch((err) => {
console.warn(`[worktree-teardown] failed for ${args.worktreeId}:`, err)
})

18
src/main/runtime/missing-worktree-terminal-reconciliation.test.ts
View File

@ -64,14 +64,23 @@ describe('stopMissingWorktreeTerminals', () => {
const localProvider = createProvider([`${deletedId}@@local-session`])
const sshProvider = createProvider([`${deletedId}@@ssh-session`])
const getSshProvider = vi.fn(() => sshProvider)
const runtime = createRuntime()
await stopMissingWorktreeTerminals({ ...localRepo, connectionId: 'ssh-1' }, [deletedId], [], {
runtime: createRuntime(),
runtime,
getLocalProvider: () => localProvider,
getSshProvider
})
expect(getSshProvider).toHaveBeenCalledWith('ssh-1')
// The runtime graph holds both hosts' terminals under one id, so the sweep must fence to this one.
expect(runtime.stopTerminalsForWorktree).toHaveBeenCalledWith(
deletedId,
expect.objectContaining({
resolvedWorktreeId: deletedId,
resolvedConnectionId: 'ssh-1'
})
)
expect(sshProvider.shutdown).toHaveBeenCalledWith(
`${deletedId}@@ssh-session`,
expect.objectContaining({ immediate: true })
@ -95,7 +104,12 @@ describe('stopMissingWorktreeTerminals', () => {
)
expect(result).toEqual({ stoppedWorktreeIds: [deletedId] })
expect(runtime.stopTerminalsForWorktree).toHaveBeenCalledWith(deletedId)
// The graph fallback still names the owning connection: this repo's inventory must not
// stop a same-id workspace's terminals on another host.
expect(runtime.stopTerminalsForWorktree).toHaveBeenCalledWith(deletedId, {
resolvedWorktreeId: deletedId,
resolvedConnectionId: 'ssh-1'
})
})
// Why: an agent cleaning up workspaces deletes many at once. Enumerating the

15
src/main/runtime/missing-worktree-terminal-reconciliation.ts
View File

@ -46,6 +46,18 @@ function withSharedProcessSnapshot(provider: IPtyProvider): IPtyProvider {
})
}
// Why: `repoId::path` ids repeat across hosts, so a sweep driven by one repo's inventory
// must name its owner or it stops a same-id workspace's terminals on another host.
function hostFence(
repo: Repo,
worktreeId: string
): { resolvedWorktreeId: string; resolvedConnectionId?: string } {
return {
resolvedWorktreeId: worktreeId,
...(repo.connectionId ? { resolvedConnectionId: repo.connectionId } : {})
}
}
type MissingWorktreeTerminalReconciliationDeps = {
runtime: OrcaRuntimeService
getLocalProvider: () => IPtyProvider | null
@ -83,7 +95,7 @@ export async function stopMissingWorktreeTerminals(
MISSING_WORKTREE_TEARDOWN_CONCURRENCY,
async (worktreeId) => {
try {
await deps.runtime.stopTerminalsForWorktree(worktreeId)
await deps.runtime.stopTerminalsForWorktree(worktreeId, hostFence(repo, worktreeId))
return worktreeId
} catch {
return null
@ -102,6 +114,7 @@ export async function stopMissingWorktreeTerminals(
try {
await killAllProcessesForWorktree(worktreeId, {
runtime: deps.runtime,
...hostFence(repo, worktreeId),
localProvider: provider,
onPtyStopped: deps.onPtyStopped,
// Why: the shared process snapshot is only valid while nothing needs a

85
src/main/runtime/orca-runtime.test.ts
View File

@ -5923,6 +5923,66 @@ describe('OrcaRuntimeService', () => {
expect(deleteWorktreeHistoryDirMock).toHaveBeenCalledWith(`${TEST_REPO_ID}::/remote/feature`)
})
// Regression: `repoId::path` ids repeat across hosts, so the SSH delete's runtime sweep used to
// stop the same-id local workspace's terminals too.
it('leaves a same-id local terminal running when the SSH copy is removed', async () => {
const remoteRepo = {
id: TEST_REPO_ID,
path: '/remote/repo',
displayName: 'repo',
badgeColor: 'blue',
addedAt: 1,
connectionId: 'ssh-1'
}
const remoteStore = { ...store, getRepos: () => [remoteRepo], getRepo: () => remoteRepo }
const gitProvider = {
listWorktrees: vi.fn().mockResolvedValue([
{
path: '/remote/repo',
head: 'main',
branch: 'refs/heads/main',
isBare: false,
isMainWorktree: true
},
{
path: '/remote/feature',
head: 'abc',
branch: 'feature/foo',
isBare: false,
isMainWorktree: false
}
]),
removeWorktree: vi.fn().mockResolvedValue(undefined)
}
registerSshGitProvider('ssh-1', gitProvider as never)
const ptyProvider = {
listProcesses: vi.fn().mockResolvedValue([]),
shutdown: vi.fn().mockResolvedValue(undefined)
}
const runtime = new OrcaRuntimeService(remoteStore as never, undefined, {
getSshProvider: () => ptyProvider as never
})
const stopAndWait = vi.fn(async () => true)
runtime.setPtyController({
write: () => true,
kill: vi.fn(() => true),
stopAndWait,
getForegroundProcess: async () => null
})
syncSinglePty(runtime, null)
runtime.registerPty('pty-remote', `${TEST_REPO_ID}::/remote/feature`, 'ssh-1')
runtime.registerPty('pty-local-same-id', `${TEST_REPO_ID}::/remote/feature`, null)
try {
await runtime.removeManagedWorktree('path:/remote/feature', true, false)
} finally {
unregisterSshGitProvider('ssh-1')
}
expect(stopAndWait).toHaveBeenCalledWith('pty-remote', expect.anything())
expect(stopAndWait).not.toHaveBeenCalledWith('pty-local-same-id', expect.anything())
})
it('rejects SSH-backed runtime removal of the main worktree before provider deletion', async () => {
const remoteStore = {
...store,
@ -34495,6 +34555,31 @@ describe('OrcaRuntimeService', () => {
expect(kill).not.toHaveBeenCalled()
})
it('stops only the owning connection when one worktree id lives on two hosts', async () => {
const runtime = new OrcaRuntimeService(store)
const kill = vi.fn(() => true)
runtime.setPtyController({
write: () => true,
kill,
stopAndWait: vi.fn(async () => true),
getForegroundProcess: async () => null
})
syncSinglePty(runtime, null)
// The store keeps one `repoId::path` per host, so deleting the SSH copy must leave the
// local copy's terminals running — the fence the destructive removal paths now supply.
runtime.registerPty('pty-ssh', TEST_WORKTREE_ID, 'ssh-1')
runtime.registerPty('pty-local', TEST_WORKTREE_ID, null)
await expect(
runtime.stopTerminalsForWorktree(TEST_WORKTREE_ID, {
resolvedWorktreeId: TEST_WORKTREE_ID,
resolvedConnectionId: 'ssh-1'
})
).resolves.toEqual({ stopped: 1 })
expect(kill).toHaveBeenCalledWith('pty-ssh')
expect(kill).not.toHaveBeenCalledWith('pty-local')
})
it('awaits physical PTY stop when destructive teardown supplies shared dedupe', async () => {
const runtime = new OrcaRuntimeService(store)
const physicalStop = makeDeferred()

32
src/main/runtime/orca-runtime.ts
View File

@ -3387,6 +3387,10 @@ export class OrcaRuntimeService {
}
const teardownResult = await killAllProcessesForWorktree(worktreeId, {
runtime: this,
// Why: `repoId::path` ids repeat across hosts, so an unfenced sweep stops a same-id
// workspace's terminals on another connection (mirrors the IPC removal path).
resolvedWorktreeId: worktreeId,
...(connectionId ? { resolvedConnectionId: connectionId } : {}),
localProvider: provider,
onPtyStopped: this.onPtyStopped ?? undefined,
requirePhysicalStop: true,
@ -23438,14 +23442,34 @@ export class OrcaRuntimeService {
'Cannot delete the project root workspace. Remove the folder project instead.'
)
}
const localProvider = this.getLocalProvider()
if (localProvider) {
// 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'
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,
localProvider,
onPtyStopped: this.onPtyStopped ?? undefined
// 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 }
: {}),
localProvider: folderPtyProvider,
onPtyStopped: this.onPtyStopped ?? undefined,
...(externalFolderHost
? {
includeProviderInventory:
folderHost?.kind === 'ssh' && Boolean(folderSshPtyProvider),
includeLocalRegistry: false
}
: {})
}).catch((err) => {
console.warn(`[worktree-teardown] failed for ${removalTarget.id}:`, err)
})