fix(runtime): address teardown review findings

src/main/daemon/terminal-host.test.ts

@@ -383,6 +383,29 @@ describe('TerminalHost', () => {
       expect(host.isKilled('session-1')).toBe(true)
     })
 
+    it('does not tombstone a session when graceful kill admission fails', async () => {
+      await host.createOrAttach({
+        sessionId: 'session-1',
+        cols: 80,
+        rows: 24,
+        streamClient: { onData: vi.fn(), onExit: vi.fn() }
+      })
+      lastSubprocess.kill = vi.fn(() => {
+        throw new Error('signal rejected')
+      })
+
+      expect(() => host.kill('session-1')).toThrow('signal rejected')
+      expect(host.isKilled('session-1')).toBe(false)
+      await expect(
+        host.createOrAttach({
+          sessionId: 'session-1',
+          cols: 80,
+          rows: 24,
+          streamClient: { onData: vi.fn(), onExit: vi.fn() }
+        })
+      ).resolves.toMatchObject({ isNew: false })
+    })
+
     it('force-kills immediately when requested', async () => {
       await host.createOrAttach({
         sessionId: 'session-1',

src/main/daemon/terminal-host.ts

@@ -208,11 +208,11 @@ export class TerminalHost {
 
   kill(sessionId: string, opts: { immediate?: boolean } = {}): Promise<void> {
     const session = this.getAliveSession(sessionId)
+    const killed = opts.immediate
+      ? session.forceKillAndWaitForExit()
+      : Promise.resolve(session.kill())
     this.recordTombstone(sessionId)
-    if (opts.immediate) {
-      return session.forceKillAndWaitForExit()
-    }
-    return Promise.resolve(session.kill())
+    return killed
   }
 
   // Why: dispose a dead session's headless emulator and drop it from the map so

src/main/ipc/watcher-removal-gate.test.ts

@@ -88,6 +88,16 @@ describe('watcher removal gate', () => {
     removal.release()
   })
 
+  it('rejects enclosing installs while a nested root is being removed', async () => {
+    const removal = acquireWatcherRemovalGate('/repo/nested')
+    await removal.ready
+
+    expect(() => beginWatcherInstall('/repo')).toThrow(WatcherRemovalInProgressError)
+    expect(() => beginTerminalInstall('/repo')).toThrow(TerminalRemovalInProgressError)
+
+    removal.release()
+  })
+
   it('does not fence a distinct POSIX root containing a literal backslash', async () => {
     const removal = acquireWatcherRemovalGate('/srv/team\\repo')
     await removal.ready

src/main/ipc/watcher-removal-gate.ts

@@ -58,9 +58,14 @@ function beginRemovalSensitiveInstall(
   createRemovalError: () => Error
 ): () => void {
   const normalizedRoot = normalizeRuntimePathForComparison(rootPath)
+  // Why: PTY admission can fence both worktree identity and cwd, which may be
+  // parent/child roots; neither side may overlap an active removal.
   if (
     matchingHostStates(connectionId).some(
-      (state) => state.removalCount > 0 && isPathInsideOrEqual(state.rootPath, normalizedRoot)
+      (state) =>
+        state.removalCount > 0 &&
+        (isPathInsideOrEqual(state.rootPath, normalizedRoot) ||
+          isPathInsideOrEqual(normalizedRoot, state.rootPath))
     )
   ) {
     throw createRemovalError()

src/main/providers/local-pty-provider.test.ts

@@ -1417,5 +1417,21 @@ describe('LocalPtyProvider', () => {
       expect(killSpy).toHaveBeenCalledTimes(1)
       expect(destroySpy).not.toHaveBeenCalled()
     })
+
+    it('settles an overlapping shutdown when app quit takes final ownership', async () => {
+      mockProc.kill.mockImplementation(() => undefined)
+      const { id } = await provider.spawn({ cols: 80, rows: 24 })
+      const shutdown = provider.shutdown(id, { immediate: true })
+      let settled = false
+      void shutdown.then(() => {
+        settled = true
+      })
+      await Promise.resolve()
+      expect(settled).toBe(false)
+
+      provider.killAll()
+
+      await expect(shutdown).resolves.toBeUndefined()
+    })
   })
 })

src/main/providers/local-pty-provider.ts

@@ -1244,6 +1244,9 @@ export class LocalPtyProvider implements IPtyProvider {
       // Why: app quit cannot retain NAPI callbacks into FreeEnvironment; the
       // process exit itself is the final physical handle boundary here.
       destroyPtyProcess(proc, { alreadyKilled: true })
+      // Why: app quit replaces node-pty's onExit callback as the final owner;
+      // overlapping shutdown waiters must join that same terminal boundary.
+      ptyPhysicalExits.get(id)?.markExited()
       clearPtyState(id)
     }
   }

src/main/runtime/orca-runtime.test.ts

@@ -28725,6 +28725,28 @@ describe('OrcaRuntimeService', () => {
     expect(deleteWorktreeHistoryDirMock).not.toHaveBeenCalled()
   })
 
+  it('stops worktree PTYs before removing linked paths', async () => {
+    const repo = { ...store.getRepos()[0], symlinkPaths: ['node_modules'] }
+    const runtimeStore = { ...store, getRepos: () => [repo], getRepo: () => repo }
+    const runtime = createWorktreeRemovalRuntime(runtimeStore)
+    const stopAndWait = vi.fn().mockResolvedValue(true)
+    runtime.setPtyController({
+      write: () => true,
+      kill: () => true,
+      stopAndWait,
+      getForegroundProcess: async () => null
+    })
+    syncSinglePty(runtime, 'pty-1')
+    vi.mocked(removeWorktree).mockResolvedValue({})
+    removeWorktreeLinkedPathsMock.mockImplementationOnce(async () => {
+      expect(stopAndWait).toHaveBeenCalledWith('pty-1')
+    })
+
+    await runtime.removeManagedWorktree(TEST_WORKTREE_ID)
+
+    expect(removeWorktreeLinkedPathsMock).toHaveBeenCalledWith(TEST_WORKTREE_PATH, ['node_modules'])
+  })
+
   it('waits for every watcher layer to settle before restoring after teardown failure', async () => {
     const runtime = createRuntime()
     let finishRuntimeClose: () => void = () => {}

src/main/runtime/orca-runtime.ts

@@ -17711,14 +17711,14 @@ export class OrcaRuntimeService {
       const removalGate = await this.acquireFileWatcherRemoval(canonicalWorktreePath)
       let removalCompleted = false
       try {
+        // Why: linked-path deletion is destructive too; PTYs must release every
+        // handle before Windows or WSL filesystem cleanup starts.
+        await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id)
+
         if (linkedPaths.length > 0) {
           await removeWorktreeLinkedPaths(canonicalWorktreePath, linkedPaths)
         }
 
-        // Why: the gate spans terminal shutdown through Git and any recursive
-        // fallback so a late headless spawn cannot recreate a native handle.
-        await this.stopPtysForDestructiveWorktreeRemoval(removalTarget.id)
-
         try {
           const removeOptions = {
             ...(!deleteBranch ? { deleteBranch } : {}),

src/main/runtime/worktree-teardown.test.ts

@@ -111,6 +111,26 @@ describe('killAllProcessesForWorktree', () => {
     expect(result.providerStopped).toBe(2)
   })
 
+  it('does not use cwd fallback against a different authoritative worktree id', async () => {
+    const localProvider = createProviderStub(async () => [
+      {
+        id: 'owned-by-sibling',
+        cwd: '/repo/app/nested',
+        title: 'shell',
+        worktreeId: 'repo-1::/repo/sibling'
+      }
+    ])
+    listRegisteredPtysMock.mockReturnValue([])
+
+    const result = await killAllProcessesForWorktree('repo-1::/repo/app', {
+      localProvider,
+      requirePhysicalStop: true
+    })
+
+    expect(localProvider.shutdown).not.toHaveBeenCalled()
+    expect(result.providerStopped).toBe(0)
+  })
+
   it('uses authoritative remote worktree ownership without sweeping the local registry', async () => {
     const remoteProvider = createProviderStub(async () => [
       { id: 'pty-remote', cwd: '/remote/w1', title: 'shell', worktreeId: 'w1' },
@@ -338,7 +358,7 @@ describe('killAllProcessesForWorktree', () => {
   })
 
   it('tolerates runtime.stopTerminalsForWorktree throwing (headless assertGraphReady reject)', async () => {
-    const stopTerminalsForWorktree = vi.fn().mockRejectedValue(new Error('graph not ready'))
+    const stopTerminalsForWorktree = vi.fn().mockRejectedValue(new Error('runtime_unavailable'))
     const runtime = {
       stopTerminalsForWorktree
     } as unknown as Parameters<typeof killAllProcessesForWorktree>[1]['runtime']
@@ -351,6 +371,23 @@ describe('killAllProcessesForWorktree', () => {
     expect(result.runtimeStopped).toBe(0)
   })
 
+  it('fails destructive teardown closed when the runtime sweep rejects', async () => {
+    const stopTerminalsForWorktree = vi.fn().mockRejectedValue(new Error('runtime sweep failed'))
+    const runtime = {
+      stopTerminalsForWorktree
+    } as unknown as Parameters<typeof killAllProcessesForWorktree>[1]['runtime']
+    const localProvider = createProviderStub(async () => [])
+    listRegisteredPtysMock.mockReturnValue([])
+
+    await expect(
+      killAllProcessesForWorktree('w1', {
+        runtime,
+        localProvider,
+        requirePhysicalStop: true
+      })
+    ).rejects.toThrow('runtime sweep failed')
+  })
+
   it('bounds the entire process sweep when a provider never settles', async () => {
     vi.useFakeTimers()
     try {

src/main/runtime/worktree-teardown.ts

@@ -76,13 +76,15 @@ export async function killAllProcessesForWorktree(
     return current
   }
 
+  // Why: headless CLI has no ready renderer graph; only that known sentinel
+  // may fall through to the provider and registry physical-owner sweeps.
   const runtimeSweep = deps.runtime
     ? settleBeforeDeadline(
         () => deps.runtime!.stopTerminalsForWorktree(worktreeId, { deadline, stopPty }),
         { stopped: 0 },
         deadline,
         deps.requirePhysicalStop ? deadlineError : undefined,
-        false
+        (error) => !(error instanceof Error && error.message === 'runtime_unavailable')
       )
     : Promise.resolve({ stopped: 0 })
   const providerSweep = settleBeforeDeadline(
@@ -139,7 +141,7 @@ async function settleBeforeDeadline<T>(
   fallback: T,
   deadline: number,
   failClosedError?: Error,
-  failClosedOnRunError = true
+  failClosedOnRunError: (error: unknown) => boolean = () => true
 ): Promise<T> {
   const remaining = deadline - Date.now()
   if (remaining <= 0) {
@@ -172,7 +174,7 @@ async function settleBeforeDeadline<T>(
     )
     timer.unref?.()
     void run().then(finish, (error: unknown) =>
-      failClosedError && failClosedOnRunError ? fail(error) : finish(fallback)
+      failClosedError && failClosedOnRunError(error) ? fail(error) : finish(fallback)
     )
   })
 }
@@ -202,6 +204,7 @@ async function sweepProviderByPrefix(
     // and authoritative worktree ownership must remain usable during teardown.
     const cwdOwned =
       worktreePath !== undefined &&
+      s.worktreeId === undefined &&
       typeof s.cwd === 'string' &&
       s.cwd.length > 0 &&
       isPathInsideOrEqual(worktreePath, s.cwd)

src/relay/relay-filesystem-watch-registry.test.ts

@@ -199,6 +199,16 @@ describe('RelayFilesystemWatchRegistry', () => {
     expect(rejectSubscribe).toBeDefined()
   })
 
+  it('propagates unexpected non-Error setup failures', async () => {
+    vi.spyOn(pool, 'subscribe').mockRejectedValueOnce('native setup failed')
+    const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true)
+    try {
+      await expect(registry.watch('/repo', context(1))).rejects.toBe('native setup failed')
+    } finally {
+      stderr.mockRestore()
+    }
+  })
+
   it('rejects acknowledged teardown while another client still owns the watch', async () => {
     await registry.watch('/repo', context(1))
     await registry.watch('/repo', context(2))

src/relay/relay-filesystem-watch-registry.ts

@@ -284,13 +284,12 @@ export class RelayFilesystemWatchRegistry {
       const expectedAbort =
         (error instanceof Error && error.name === 'AbortError') ||
         (isWatcherProcessFailure(error) && error.code === 'subscribe_aborted')
-      if (!expectedAbort && error instanceof Error) {
-        process.stderr.write(
-          `[relay] File watcher not available for ${state.rootPath}: ${error.message}\n`
-        )
-        throw error
+      if (expectedAbort) {
+        return
       }
-      return
+      const message = error instanceof Error ? error.message : String(error)
+      process.stderr.write(`[relay] File watcher not available for ${state.rootPath}: ${message}\n`)
+      throw error
     }
     if (context?.isStale()) {
       this.releaseWatchClient(state, clientId)