fix(terminal): retain shutdown retry ownership

2026-10-25 08:04:05 +00:00 · 2026-07-12 13:32:18 -07:00
parent 459de71dab
commit d87c8fa9af
12 changed files with 335 additions and 75 deletions
@@ -3600,21 +3600,24 @@
      "providers": ["ssh", "remote-runtime"],
      "coveredPlatforms": ["macos"],
      "coveredProviders": ["ssh"],
-      "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, and idempotently verify the fix. Relay tests simulate win32 shutdown, one-shot ownership, and retry after native failure. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.",
+      "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, stale-spawn fallback ownership, one-shot ownership, and retry after native failure. Renderer tests retain exact local/SSH kill identity across rejection and retry, while project removal keeps local or remote ownership state until provider shutdown succeeds. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.",
      "motivatingLinks": [
        "https://github.com/stablyai/orca/issues/8260",
        "https://github.com/stablyai/orca/pull/8279"
      ],
      "invariant": "A Windows relay PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca does not delete ownership for a native kill failure, and a later retry can still target the same managed PTY.",
-      "oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0 plus an artifact version derived from the patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In the relay, assert immediate, graceful, fallback, and handler-dispose paths issue at most one no-argument kill per request; injected native failures retain one managed PTY, and a successful retry clears it.",
+      "oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0, AttachConsole failure falls back to the shell pid, and the artifact version derives from all patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In the relay, assert immediate, graceful, stale-spawn fallback, and handler-dispose paths preserve ownership through native throw and issue at most one successful no-argument shutdown. In the renderer, reject the first kill, retain exact identity, and assert the next lifecycle/removal retry succeeds before ownership state is purged.",
      "commands": [
-        "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts"
+        "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts"
      ],
      "testFiles": [
        "config/scripts/build-relay-windows-node-pty-patch.test.mjs",
        "src/main/ssh/ssh-relay-native-deps-install.test.ts",
        "src/relay/node-pty-windows-pre-ready-shutdown.test.ts",
-        "src/relay/pty-handler.test.ts"
+        "src/relay/pty-handler.test.ts",
+        "src/renderer/src/components/terminal-pane/pty-transport.test.ts",
+        "src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts",
+        "src/renderer/src/store/slices/repos.test.ts"
      ],
      "assertionRefs": [
        {
@@ -3622,6 +3625,7 @@
          "assertions": [
            "both Windows artifacts contain the strict patcher and derive their version hash from its bytes",
            "vanilla npm node-pty defers pre-ready kill, while the shipped patch executes one native close immediately and clears deferred work",
+            "the shipped relay patch preserves the local ConPTY console-list shell-pid fallback",
            "patch application and dependency verification are idempotent"
          ]
        },
@@ -3643,7 +3647,25 @@
          "file": "src/relay/pty-handler.test.ts",
          "assertions": [
            "Windows immediate, graceful/fallback, and relay-dispose shutdown issue one no-argument kill",
-            "a native kill failure retains one managed PTY and a later successful retry clears it"
+            "stale spawn and explicit shutdown native failures retain one managed PTY and a later successful retry clears it"
+          ]
+        },
+        {
+          "file": "src/renderer/src/components/terminal-pane/pty-transport.test.ts",
+          "assertions": [
+            "a rejected teardown kill retains exact PTY identity and retries on the next lifecycle event"
+          ]
+        },
+        {
+          "file": "src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts",
+          "assertions": [
+            "project removal preserves PTY lookup ownership after provider rejection and purges only after retry succeeds"
+          ]
+        },
+        {
+          "file": "src/renderer/src/store/slices/repos.test.ts",
+          "assertions": [
+            "remote-runtime project removal retains terminal ownership after stop rejection and purges after retry succeeds"
          ]
        }
      ],
@@ -3652,10 +3674,10 @@
          "date": "2026-07-12",
          "runner": "local",
          "platform": "macos",
-          "command": "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts",
+          "command": "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts",
          "result": "passed",
          "durationSeconds": 2,
-          "summary": "97 tests passed, including the built Windows relay artifact red/green harness, strict remote install ordering/failure/upgrade-repair handling, three installed-dependency pre-ready assertions, and relay shutdown/retry call-count coverage."
+          "summary": "193 tests passed, including built Windows relay red/green and ConPTY fallback parity, strict remote install ordering/failure/upgrade-repair handling, installed-dependency pre-ready assertions, stale-spawn fallback ownership, renderer retained retry identity, and local/remote project removal fail-closed ownership coverage."
        }
      ],
      "runtimeBudget": {
@@ -3672,7 +3694,7 @@
      },
      "performanceBudget": {
        "required": true,
-        "evidence": "Shutdown removes indefinite deferred retention and preserves at most one native kill and one existing graceful timer per PTY request; it adds no polling, listener, startup await, or steady-state work."
+        "evidence": "Shutdown removes indefinite deferred retention and preserves at most one in-flight native kill plus one existing graceful timer per PTY request. Failed renderer kills retain a bounded exact-id entry and retry only on a later PTY lifecycle event; no polling, new permanent listener, startup await, or steady-state work is added."
      },
      "promotionCriteria": [
        "Run the deterministic command across the required CI platforms without flakes.",
@@ -4,6 +4,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync
 import { createRequire } from 'node:module'
 import { tmpdir } from 'node:os'
 import { dirname, join, resolve } from 'node:path'
+import { runInNewContext } from 'node:vm'
 import { describe, expect, it } from 'vitest'

 const projectRoot = resolve(import.meta.dirname, '../..')
@@ -37,6 +38,14 @@ WindowsTerminal.prototype._deferNoArgs = function (deferredFn) {
 };
 exports.WindowsTerminal = WindowsTerminal;
 `
+const vanillaConptyConsoleListAgent = `'use strict';
+var utils_1 = require("./utils");
+var getConsoleProcessList = utils_1.loadNativeModule('conpty_console_list').module.getConsoleProcessList;
+var shellPid = parseInt(process.argv[2], 10);
+var consoleProcessList = getConsoleProcessList(shellPid);
+process.send({ consoleProcessList: consoleProcessList });
+process.exit(0);
+`

 function loadHarness(modulePath) {
  delete require.cache[require.resolve(modulePath)]
@@ -92,9 +101,17 @@ describe('packaged Windows SSH relay node-pty patch', () => {
    try {
      const modulePath = join(tempRoot, 'node_modules', 'node-pty', 'lib', 'windowsTerminal.js')
      const watcherPath = join(tempRoot, 'node_modules', '@parcel', 'watcher', 'index.js')
+      const consoleListPath = join(
+        tempRoot,
+        'node_modules',
+        'node-pty',
+        'lib',
+        'conpty_console_list_agent.js'
+      )
      mkdirSync(dirname(modulePath), { recursive: true })
      mkdirSync(dirname(watcherPath), { recursive: true })
      writeFileSync(modulePath, vanillaWindowsTerminal, 'utf8')
+      writeFileSync(consoleListPath, vanillaConptyConsoleListAgent, 'utf8')
      writeFileSync(watcherPath, 'module.exports = {}\n', 'utf8')

      expect(loadHarness(modulePath)).toEqual({
@@ -102,7 +119,6 @@ describe('packaged Windows SSH relay node-pty patch', () => {
        nativeKillCount: 0,
        deferredCount: 1
      })
-
      const patch = spawnSync(
        process.execPath,
        [join(projectRoot, 'out', 'relay', 'win32-x64', patchName), '--apply', tempRoot],
@@ -115,6 +131,24 @@ describe('packaged Windows SSH relay node-pty patch', () => {
        nativeKillCount: 1,
        deferredCount: 0
      })
+      const sent = []
+      runInNewContext(readFileSync(consoleListPath, 'utf8'), {
+        require: () => ({
+          loadNativeModule: () => ({
+            module: {
+              getConsoleProcessList: () => {
+                throw new Error('AttachConsole failed')
+              }
+            }
+          })
+        }),
+        process: {
+          argv: ['node', consoleListPath, '42'],
+          exit: () => {},
+          send: (message) => sent.push(message)
+        }
+      })
+      expect(sent).toEqual([{ consoleProcessList: [42] }])

      const secondPatch = spawnSync(
        process.execPath,
@@ -30,6 +30,15 @@ const PATCHED_KILL = `    WindowsTerminal.prototype.kill = function (signal) {
        this._close();
        this._agent.kill();
    };`
+const VANILLA_CONPTY_CONSOLE_LIST = `var consoleProcessList = getConsoleProcessList(shellPid);`
+const PATCHED_CONPTY_CONSOLE_LIST = `var consoleProcessList;
+try {
+    consoleProcessList = getConsoleProcessList(shellPid);
+}
+catch (_a) {
+    // Orca: AttachConsole can fail after the shell exits; preserve the root pid fallback.
+    consoleProcessList = [shellPid];
+}`

 function replaceExactlyOnce(source, before, after, label) {
  const first = source.indexOf(before)
@@ -51,20 +60,45 @@ function patchWindowsTerminal(source) {
  return replaceExactlyOnce(withDestroy, VANILLA_KILL, PATCHED_KILL, 'kill()')
 }

-function applyPatch(rootDir = process.cwd()) {
-  const target = join(rootDir, 'node_modules', 'node-pty', 'lib', 'windowsTerminal.js')
-  const source = readFileSync(target, 'utf8')
-  const patched = patchWindowsTerminal(source)
-  if (patched !== source) {
-    const pendingTarget = `${target}.orca-patch-pending`
-    try {
-      writeFileSync(pendingTarget, patched, 'utf8')
-      renameSync(pendingTarget, target)
-    } finally {
-      rmSync(pendingTarget, { force: true })
-    }
+function patchConptyConsoleListAgent(source) {
+  if (source.includes(PATCHED_CONPTY_CONSOLE_LIST)) {
+    return source
  }
-  return target
+  return replaceExactlyOnce(
+    source,
+    VANILLA_CONPTY_CONSOLE_LIST,
+    PATCHED_CONPTY_CONSOLE_LIST,
+    'ConPTY console-list fallback'
+  )
+}
+
+function writeAtomicallyIfChanged(target, source, patched) {
+  if (patched === source) {
+    return
+  }
+  const pendingTarget = `${target}.orca-patch-pending`
+  try {
+    writeFileSync(pendingTarget, patched, 'utf8')
+    renameSync(pendingTarget, target)
+  } finally {
+    rmSync(pendingTarget, { force: true })
+  }
+}
+
+function applyPatch(rootDir = process.cwd()) {
+  const moduleRoot = join(rootDir, 'node_modules', 'node-pty', 'lib')
+  const terminalTarget = join(moduleRoot, 'windowsTerminal.js')
+  const terminalSource = readFileSync(terminalTarget, 'utf8')
+  writeAtomicallyIfChanged(terminalTarget, terminalSource, patchWindowsTerminal(terminalSource))
+
+  const consoleListTarget = join(moduleRoot, 'conpty_console_list_agent.js')
+  const consoleListSource = readFileSync(consoleListTarget, 'utf8')
+  writeAtomicallyIfChanged(
+    consoleListTarget,
+    consoleListSource,
+    patchConptyConsoleListAgent(consoleListSource)
+  )
+  return terminalTarget
 }

 function checkNativeDeps(rootDir = process.cwd()) {
@@ -73,6 +107,16 @@ function checkNativeDeps(rootDir = process.cwd()) {
  if (!source.includes(PATCHED_DESTROY) || !source.includes(PATCHED_KILL)) {
    throw new Error('node-pty Windows pre-ready shutdown patch is missing')
  }
+  const consoleListTarget = join(
+    rootDir,
+    'node_modules',
+    'node-pty',
+    'lib',
+    'conpty_console_list_agent.js'
+  )
+  if (!readFileSync(consoleListTarget, 'utf8').includes(PATCHED_CONPTY_CONSOLE_LIST)) {
+    throw new Error('node-pty ConPTY console-list fallback patch is missing')
+  }
  require.resolve('@parcel/watcher', { paths: [rootDir] })
  return target
 }
@@ -91,4 +135,10 @@ if (require.main === module) {
  }
 }

-module.exports = { PATCH_MARKER, applyPatch, checkNativeDeps, patchWindowsTerminal }
+module.exports = {
+  PATCH_MARKER,
+  applyPatch,
+  checkNativeDeps,
+  patchConptyConsoleListAgent,
+  patchWindowsTerminal
+}
@@ -694,6 +694,24 @@ describe('PtyHandler', () => {
    expect(killSpy).toHaveBeenCalledWith('SIGKILL')
  })

+  it('keeps stale-spawn fallback ownership when the first native kill throws', async () => {
+    const firstError = new Error('native ConPTY kill failed')
+    const killSpy = vi.fn().mockImplementationOnce(() => {
+      throw firstError
+    })
+    const term = { ...mockPtyInstance, kill: killSpy, onData: vi.fn(), onExit: vi.fn() }
+    mockPtySpawn.mockReturnValue(term)
+
+    await expect(dispatcher.callRequest('pty.spawn', {}, { isStale: () => true })).rejects.toThrow(
+      firstError
+    )
+
+    expect(handler.activePtyCount).toBe(1)
+    vi.advanceTimersByTime(5000)
+    expect(killSpy).toHaveBeenNthCalledWith(2, 'SIGKILL')
+    expect(handler.activePtyCount).toBe(0)
+  })
+
  it('does not submit provider-delivered commands for stale spawn responses', async () => {
    const killSpy = vi.fn()
    const term = { ...mockPtyInstance, kill: killSpy, onData: vi.fn(), onExit: vi.fn() }
@@ -728,7 +728,6 @@ export class PtyHandler {
      // response is discarded and no renderer can own this PTY. Shut it down
      // immediately so it does not linger as an unreachable remote shell.
      this.releaseStartupCommand(managed)
-      killPtyForShutdown(managed, 'SIGTERM')
      managed.killTimer = setTimeout(() => {
        const still = this.ptys.get(id)
        if (still && !still.disposed) {
@@ -742,6 +741,10 @@ export class PtyHandler {
          this.ptys.delete(id)
        }
      }, 5000)
+      // Why: arm fallback ownership before native shutdown. node-pty can throw
+      // synchronously, especially during ConPTY startup, and the orphan still
+      // needs a later force-close owner even when this request rejects.
+      killPtyForShutdown(managed, 'SIGTERM')
    } else if (managed.startupCommand) {
      this.scheduleStartupCommandDelivery(
        managed,
@@ -28,6 +28,10 @@ import {
 } from './terminal-delivery-watchdog'
 import { recordTerminalFreezeBreadcrumb } from './terminal-freeze-breadcrumbs'
 import { installTerminalFreezeReport } from './terminal-freeze-report'
+import {
+  releaseRetainedPtyKillOwnership,
+  retryRetainedPtyKills
+} from '@/lib/pty-kill-retry-ownership'

 // ── Singleton PTY event dispatcher ───────────────────────────────────
 // One global IPC listener per channel, routes events to transports by
@@ -134,6 +138,9 @@ export function reattachPtyDispatcherPushListeners(): void {
 }

 export function ensurePtyDispatcher(): void {
+  // Why: a prior provider disconnect can reject shutdown while retaining the
+  // exact PTY id. A later terminal lifecycle event is the safe retry trigger.
+  retryRetainedPtyKills()
  if (ptyDispatcherAttached) {
    return
  }
@@ -231,6 +238,7 @@ function attachPtySecondaryPushListeners(unsubscribes: (() => void)[]): void {
      // cumulative totals too so a reused id restarts at zero on both sides.
      clearProcessedPtyCharTotal(payload.id)
      clearReceivedPtyCharTotal(payload.id)
+      releaseRetainedPtyKillOwnership(payload.id)
      const handler = ptyExitHandlers.get(payload.id)
      if (handler) {
        clearPreHandlerPtyState(payload.id)
@@ -1355,7 +1355,7 @@ describe('createIpcPtyTransport', () => {
    })
    const spawnMock = vi.fn().mockReturnValue(spawnPromise)
    const killError = new Error('remote connection dropped')
-    const killMock = vi.fn().mockRejectedValue(killError)
+    const killMock = vi.fn().mockRejectedValueOnce(killError).mockResolvedValue(undefined)
    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
    const onPtySpawn = vi.fn()

@@ -1404,6 +1404,12 @@ describe('createIpcPtyTransport', () => {
      )
      expect(onPtySpawn).not.toHaveBeenCalled()
      expect(transport.getPtyId()).toBeNull()
+
+      // A later transport lifecycle event retries the retained exact id.
+      spawnMock.mockResolvedValueOnce({ id: 'pty-next' })
+      await createIpcPtyTransport().connect({ url: '', callbacks: {} })
+      await flushPtySideEffects()
+      expect(killMock).toHaveBeenNthCalledWith(2, 'pty-late')
    } finally {
      warn.mockRestore()
    }
@@ -1412,7 +1418,7 @@ describe('createIpcPtyTransport', () => {
  it('settles and diagnoses a rejected PTY kill after transport teardown', async () => {
    const { createIpcPtyTransport } = await import('./pty-transport')
    const killError = new Error('remote connection dropped')
-    const kill = vi.fn().mockRejectedValue(killError)
+    const kill = vi.fn().mockRejectedValueOnce(killError).mockResolvedValue(undefined)
    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
    window.api.pty.kill = kill
    const transport = createIpcPtyTransport()
@@ -1423,6 +1429,9 @@ describe('createIpcPtyTransport', () => {
      await flushPtySideEffects()

      expect(warn).toHaveBeenCalledWith('[pty] Failed to stop disconnected PTY', killError)
+      await createIpcPtyTransport().connect({ url: '', callbacks: {} })
+      await flushPtySideEffects()
+      expect(kill).toHaveBeenNthCalledWith(2, 'pty-1')
    } finally {
      warn.mockRestore()
    }
@@ -37,6 +37,7 @@ import {
 } from '../../../../shared/agent-status-osc'
 import { extractIpcErrorMessage } from '@/lib/ipc-error'
 import { isTuiAgent } from '../../../../shared/tui-agent-config'
+import { killPtyRetainingRetryOwnership } from '@/lib/pty-kill-retry-ownership'

 // Re-export public API so existing consumers keep working.
 export {
@@ -732,9 +733,10 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra

        // If destroyed while spawn was in flight, kill the new pty and bail
        if (destroyed) {
-          void window.api.pty.kill(spawnResult.id).catch((error) => {
-            console.warn('[pty] Failed to stop PTY spawned after transport teardown', error)
-          })
+          void killPtyRetainingRetryOwnership(
+            spawnResult.id,
+            '[pty] Failed to stop PTY spawned after transport teardown'
+          ).catch(() => {})
          return
        }

@@ -920,9 +922,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra
      inputWriteQueue.clear()
      if (ptyId) {
        const id = ptyId
-        void window.api.pty.kill(id).catch((error) => {
-          console.warn('[pty] Failed to stop disconnected PTY', error)
-        })
+        void killPtyRetainingRetryOwnership(id, '[pty] Failed to stop disconnected PTY').catch(
+          () => {}
+        )
        connected = false
        ptyId = null
        unregisterPtyHandlers(id)
@@ -0,0 +1,47 @@
+type RetainedPtyKill = {
+  diagnostic: string
+  inFlight: Promise<void> | null
+}
+
+const retainedPtyKills = new Map<string, RetainedPtyKill>()
+
+/**
+ * Keep exact PTY identity until the owning provider accepts shutdown. A rejected
+ * local/SSH IPC call can otherwise leave a live process with no renderer owner.
+ */
+export function killPtyRetainingRetryOwnership(id: string, diagnostic: string): Promise<void> {
+  const retained = retainedPtyKills.get(id) ?? { diagnostic, inFlight: null }
+  retained.diagnostic = diagnostic
+  retainedPtyKills.set(id, retained)
+  if (retained.inFlight) {
+    return retained.inFlight
+  }
+
+  const attempt = Promise.resolve()
+    .then(() => window.api.pty.kill(id))
+    .then(() => {
+      retainedPtyKills.delete(id)
+    })
+    .catch((error: unknown) => {
+      console.warn(retained.diagnostic, error)
+      throw error
+    })
+    .finally(() => {
+      if (retained.inFlight === attempt) {
+        retained.inFlight = null
+      }
+    })
+  retained.inFlight = attempt
+  return attempt
+}
+
+/** Retry on the next PTY lifecycle event; no polling or permanent timer is added. */
+export function retryRetainedPtyKills(): void {
+  for (const [id, retained] of retainedPtyKills) {
+    void killPtyRetainingRetryOwnership(id, retained.diagnostic).catch(() => {})
+  }
+}
+
+export function releaseRetainedPtyKillOwnership(id: string): void {
+  retainedPtyKills.delete(id)
+}
@@ -85,7 +85,7 @@ describe('removeProject purges per-worktree state (leak regression)', () => {
    expect(s.everActivatedWorktreeIds.has(W2)).toBe(true)
  })

-  it('purges project state after a rejected best-effort PTY kill', async () => {
+  it('retains project PTY ownership after kill rejection and purges after retry', async () => {
    const store = createTestStore()
    seedTwoProjects(store)
    store.setState({
@@ -99,8 +99,18 @@ describe('removeProject purges per-worktree state (leak regression)', () => {
    try {
      await store.getState().removeProject(repo1.id)

-      expect(store.getState().unifiedTabsByWorktree[W1]).toBeUndefined()
+      expect(store.getState().unifiedTabsByWorktree[W1]).toBeDefined()
+      expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-1'])
+      expect(reposRemove).not.toHaveBeenCalled()
      expect(warn).toHaveBeenCalledWith('[pty] Failed to stop PTY while removing project', error)
+
+      ptyKill.mockResolvedValue(undefined)
+      await store.getState().removeProject(repo1.id)
+
+      expect(ptyKill).toHaveBeenCalledTimes(2)
+      expect(reposRemove).toHaveBeenCalledTimes(1)
+      expect(store.getState().unifiedTabsByWorktree[W1]).toBeUndefined()
+      expect(store.getState().ptyIdsByTabId['tab-1']).toBeUndefined()
    } finally {
      warn.mockRestore()
    }
@@ -748,6 +748,53 @@ describe('repo slice runtime routing', () => {
    expect(ptyKill).not.toHaveBeenCalledWith('remote:term-1')
  })

+  it('retains remote terminal ownership until runtime stop retry succeeds', async () => {
+    let stopAttempts = 0
+    runtimeEnvironmentCall.mockImplementation(async (request: { method: string }) => {
+      if (request.method === 'terminal.stop' && stopAttempts++ === 0) {
+        throw new Error('runtime disconnected')
+      }
+      return {
+        id: 'rpc-remote-retry',
+        ok: true,
+        result: { ok: true },
+        _meta: { runtimeId: 'runtime-remote' }
+      }
+    })
+    const store = createTestStore()
+    const worktreeId = `${remoteRepo.id}::/remote/wt`
+    store.setState({
+      settings: { activeRuntimeEnvironmentId: 'env-1' } as never,
+      repos: [remoteRepo],
+      worktreesByRepo: {
+        [remoteRepo.id]: [makeWorktree({ id: worktreeId, repoId: remoteRepo.id })]
+      },
+      tabsByWorktree: {
+        [worktreeId]: [{ id: 'tab-1', worktreeId } as never]
+      },
+      ptyIdsByTabId: {
+        'tab-1': ['remote:term-1']
+      }
+    })
+
+    await store.getState().removeProject(remoteRepo.id)
+
+    expect(store.getState().repos).toEqual([remoteRepo])
+    expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['remote:term-1'])
+    expect(
+      runtimeEnvironmentCall.mock.calls.filter(([request]) => request.method === 'repo.rm')
+    ).toHaveLength(0)
+
+    await store.getState().removeProject(remoteRepo.id)
+
+    expect(stopAttempts).toBe(2)
+    expect(
+      runtimeEnvironmentCall.mock.calls.filter(([request]) => request.method === 'repo.rm')
+    ).toHaveLength(1)
+    expect(store.getState().repos).toEqual([])
+    expect(store.getState().ptyIdsByTabId['tab-1']).toBeUndefined()
+  })
+
  it('cleans up hidden detected worktree state when removing a repo', async () => {
    const store = createTestStore()
    const hiddenWorktree = makeWorktree({
@@ -75,6 +75,7 @@ import { markOnboardingProjectAdded } from '@/lib/onboarding-project-checklist'
 import { filterSetupScriptPromptDismissalsToValidRepos } from '@/lib/setup-script-prompt'
 import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills'
 import { translate } from '@/i18n/i18n'
+import { killPtyRetainingRetryOwnership } from '@/lib/pty-kill-retry-ownership'
 import {
  getRepoExecutionHostId,
  isRuntimeOwnedSshTargetId,
@@ -2754,16 +2755,6 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
        return
      }
      const ownerHostId = getRepoExecutionHostId(ownerRepo)
-      // Why: an SSH-mode per-workspace-env's workspace is the repo's main worktree, so deleting it
-      // routes here (project removal) rather than through removeWorktree. Tear down the backing
-      // ephemeral runtime (Docker/VM + hidden SSH target) first so it doesn't leak when the project
-      // is removed. Matches on the repo's runtime-owned connectionId and its known worktree ids.
-      if (isRuntimeOwnedSshTargetId(ownerRepo.connectionId)) {
-        await cleanupEphemeralVmRuntimesForDeleted({
-          workspaceIds: getKnownRepoWorktreeIds(get(), projectId, ownerHostId),
-          runtimeOwnedSshTargetIds: [ownerRepo.connectionId as string]
-        })
-      }
      // Why: derive the runtime target from the owner's own settings, passing the
      // explicit options.hostId so a duplicate repo id across hosts resolves to the
      // intended row. settingsForRepoOwner clears the focused runtime for SSH/local
@@ -2777,6 +2768,56 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
      const idExistsOnOtherHost = get().repos.some(
        (repo) => repo.id === projectId && getRepoExecutionHostId(repo) !== ownerHostId
      )
+      // Kill PTYs for all worktrees belonging to this repo
+      const worktreeIds = getKnownRepoWorktreeIds(get(), projectId, ownerHostId)
+      const killedTabIds = new Set<string>()
+      const killedPtyIds = new Set<string>()
+      if (target.kind === 'environment') {
+        await Promise.all(
+          worktreeIds.map((worktreeId) =>
+            callRuntimeRpc(
+              target,
+              'terminal.stop',
+              { worktree: toRuntimeWorktreeSelector(worktreeId) },
+              { timeoutMs: 15_000 }
+            )
+          )
+        )
+      }
+      const localPtyKills: Promise<void>[] = []
+      for (const wId of worktreeIds) {
+        const tabs = get().tabsByWorktree[wId] ?? []
+        for (const tab of tabs) {
+          killedTabIds.add(tab.id)
+          for (const ptyId of get().ptyIdsByTabId[tab.id] ?? []) {
+            killedPtyIds.add(ptyId)
+            if (!ptyId.startsWith('remote:')) {
+              localPtyKills.push(
+                killPtyRetainingRetryOwnership(
+                  ptyId,
+                  '[pty] Failed to stop PTY while removing project'
+                )
+              )
+            }
+          }
+        }
+      }
+      // Why: purge deletes the last renderer lookup path for these exact ids.
+      // Fail closed so a provider rejection keeps state available for retry.
+      await Promise.all(localPtyKills)
+
+      // Why: an SSH-mode per-workspace-env's workspace is the repo's main worktree.
+      // Stop its PTYs before tearing down the backing VM/hidden SSH target so a
+      // provider failure retains a reachable process and exact retry identity.
+      if (isRuntimeOwnedSshTargetId(ownerRepo.connectionId)) {
+        await cleanupEphemeralVmRuntimesForDeleted({
+          workspaceIds: worktreeIds,
+          runtimeOwnedSshTargetIds: [ownerRepo.connectionId as string]
+        })
+      }
+
+      // Why: provider shutdown must succeed before deleting the repo remotely.
+      // Otherwise a retry can be blocked by a non-idempotent repo.rm response.
      await (target.kind === 'local'
        ? idExistsOnOtherHost
          ? window.api.repos.removeForHost({ repoId: projectId, hostId: ownerHostId })
@@ -2791,37 +2832,6 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (set,
      const { clearRepoSlugCacheEntry } = await import('../../lib/repo-slug-index')
      clearRepoSlugCacheEntry(projectId)

-      // Kill PTYs for all worktrees belonging to this repo
-      const worktreeIds = getKnownRepoWorktreeIds(get(), projectId, ownerHostId)
-      const killedTabIds = new Set<string>()
-      const killedPtyIds = new Set<string>()
-      if (target.kind === 'environment') {
-        await Promise.allSettled(
-          worktreeIds.map((worktreeId) =>
-            callRuntimeRpc(
-              target,
-              'terminal.stop',
-              { worktree: toRuntimeWorktreeSelector(worktreeId) },
-              { timeoutMs: 15_000 }
-            )
-          )
-        )
-      }
-      for (const wId of worktreeIds) {
-        const tabs = get().tabsByWorktree[wId] ?? []
-        for (const tab of tabs) {
-          killedTabIds.add(tab.id)
-          for (const ptyId of get().ptyIdsByTabId[tab.id] ?? []) {
-            killedPtyIds.add(ptyId)
-            if (!ptyId.startsWith('remote:')) {
-              void Promise.resolve(window.api.pty.kill(ptyId)).catch((error) => {
-                console.warn('[pty] Failed to stop PTY while removing project', error)
-              })
-            }
-          }
-        }
-      }
-
      // Why: route project removal through the canonical per-worktree purge so all
      // ~30 worktree-scoped maps are evicted. removeProject previously hand-deleted
      // only a handful (tabs/layouts/ptys), leaking the rest (unified tabs, groups,