fix(worktree): verify exited PTYs before blocking deletion (#10106)

This commit is contained in:
Neil 2026-07-22 22:55:35 -07:00 committed by GitHub
parent c445f26541
commit c3620f0954
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 77 additions and 5 deletions

View File

@ -375,6 +375,53 @@ describe('killAllProcessesForWorktree', () => {
expect(localProvider.shutdown).toHaveBeenCalledTimes(1)
})
it('accepts a failed Windows stop when a fresh inventory proves the PTY exited', async () => {
const worktreeId = 'repo-1::C:/Users/User/orca/workspaces/repo/feature'
const ptyId = `${worktreeId}@@windows-pty`
const stopTerminalsForWorktree = vi.fn(
async (
_worktreeId: string,
options: {
stopPty: (
ptyId: string,
stop: () => boolean
) => Promise<{ stopped: boolean; owner: boolean }>
}
) => ({
stopped: (await options.stopPty(ptyId, () => false)).owner ? 1 : 0
})
)
const runtime = {
stopTerminalsForWorktree
} as unknown as Parameters<typeof killAllProcessesForWorktree>[1]['runtime']
let inventoryCount = 0
const localProvider = createProviderStub(async () => {
inventoryCount += 1
return inventoryCount === 1
? [{ id: ptyId, cwd: 'C:/Users/User/orca/workspaces/repo/feature', title: 'shell' }]
: []
})
;(localProvider.shutdown as unknown as ReturnType<typeof vi.fn>).mockRejectedValue(
new Error(`Session not found: ${ptyId}`)
)
listRegisteredPtysMock.mockReturnValue([
{ ptyId, worktreeId, sessionId: null, paneKey: null, pid: 100 }
])
await expect(
killAllProcessesForWorktree(worktreeId, {
runtime,
localProvider,
requirePhysicalStop: true
})
).resolves.toEqual({
runtimeStopped: 0,
providerStopped: 0,
registryStopped: 0
})
expect(localProvider.listProcesses).toHaveBeenCalledTimes(2)
})
it('keeps duplicate sweeps behind the runtime physical-stop promise', async () => {
let releasePhysicalStop: () => void = () => undefined
const physicalStop = new Promise<boolean>((resolve) => {

View File

@ -26,9 +26,8 @@ export type WorktreeTeardownResult = {
export const WORKTREE_PROCESS_SWEEP_TIMEOUT_MS = 10_000
// Why: margin so a bounded daemon RPC rejects BEFORE the sweep deadline and its
// rejection can propagate — otherwise the outer deadline wins with a confusing
// "Timed out waiting for physical PTY teardown" instead of the accurate stop failure.
// Why: reserve time after bounded stop RPCs to recheck whether a reported
// failure actually left a live PTY before the outer sweep deadline.
export const WORKTREE_TEARDOWN_RPC_MARGIN_MS = 500
// Absolute deadline (epoch ms) threaded into provider RPCs on the destructive
@ -150,15 +149,41 @@ export async function killAllProcessesForWorktree(
result.providerStopped = providerStopped
result.registryStopped = registryStopped
if (deps.requirePhysicalStop) {
const stops = await Promise.all(stopAttempts.values())
if (stops.some((stopped) => !stopped)) {
const stopResults = await Promise.all(
[...stopAttempts].map(async ([ptyId, stopped]) => [ptyId, await stopped] as const)
)
const failedPtyIds = stopResults.filter(([, stopped]) => !stopped).map(([ptyId]) => ptyId)
const failedPtysExited =
failedPtyIds.length === 0 ||
(await verifyFailedPtysExited(failedPtyIds, deps.localProvider, deadline))
if (!failedPtysExited) {
throw new Error(`Failed to physically stop every PTY for worktree: ${worktreeId}`)
}
for (const ptyId of failedPtyIds) {
clearStoppedPtyState(ptyId, deps.onPtyStopped)
}
}
return result
}
async function verifyFailedPtysExited(
failedPtyIds: readonly string[],
provider: IPtyProvider,
deadline: number
): Promise<boolean> {
const sessions = await settleBeforeDeadline(
() => provider.listProcesses({ deadlineMs: deadline }),
null,
deadline
).catch(() => null)
if (!sessions) {
return false
}
const livePtyIds = new Set(sessions.map((session) => session.id))
return failedPtyIds.every((ptyId) => !livePtyIds.has(ptyId))
}
async function settleBeforeDeadline<T>(
run: () => Promise<T>,
fallback: T,