mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(pty): stop detached OMP tools on immediate terminal close (#20642)
* test(omp): add opt-in owned PTY closure probe * fix(pty): sweep detached tools on immediate unrecognized shell close * test(omp): create close probe evidence root in fresh worktrees * test(pty): account for asynchronous immediate descendant cleanup * test(pty): reject inconclusive descendant cleanup probes
This commit is contained in:
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { readFileSync, mkdtempSync, rmSync } from 'node:fs'
|
||||
import { connect, type Socket } from 'node:net'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
existsSync,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { spawn } from 'node:child_process'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { connect, type Socket } from 'node:net'
|
||||
import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { connect, type Socket } from 'node:net'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Re-anchoring after a cold restore: aliveness probing, sticky restore cache, persistence. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Cold-restore seed transfer and the payload shapes handed back to the renderer. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { hostname } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Dead-endpoint write handling and daemon respawn after the daemon dies. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { existsSync, rmSync } from 'node:fs'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Periodic/final history checkpointing: scheduling, work caps, cooldown and shutdown writes. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* History recovery / quarantine / reconcile regressions for DaemonPtyAdapter. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Inventory after the terminal host dies: worktree removal must not hard-fail. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { DaemonPtyAdapter } from './daemon-pty-adapter'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* DaemonPtyAdapter behaviour that varies with the negotiated daemon protocol version. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { rmSync } from 'node:fs'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Adopting daemon sessions that already exist: reattach, attach-only, inventory, tombstones, startup reconcile. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/* Core IPtyProvider surface of DaemonPtyAdapter: spawn, io, sizing, teardown. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { rmSync } from 'node:fs'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { existsSync, mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Socket } from 'node:net'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { chmodSync, linkSync, mkdtempSync, readFileSync, rmSync, unlinkSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { connect, type Server, type Socket } from 'node:net'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/**
|
||||
* OOM regression: a daemon owning 100+ terminals retained ~5000 rows of grid per session with no
|
||||
* bound, grew to ~1.9 GB, and was killed under system memory pressure — losing every session it
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import type { Socket } from 'node:net'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
/**
|
||||
* Attachment-leak regression: an attachment that outlives its transport leaves the session looking
|
||||
* viewed forever — producer pause/resume and any attachment-gated behavior then act on a client that
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
// Regression coverage for issue #6814 (terminal lockup after upgrade).
|
||||
//
|
||||
// Drives the real DaemonServer + checkDaemonHealth client over a real unix
|
||||
|
||||
@@ -0,0 +1,8 @@
|
||||
import { vi } from 'vitest'
|
||||
|
||||
// Mock subprocess PIDs must never reach the host process table or signal real descendants.
|
||||
vi.mock('../pty-descendant-termination', () => ({
|
||||
killWithDescendantSweep: async (_pid: number, killRoot: () => void): Promise<void> => {
|
||||
killRoot()
|
||||
}
|
||||
}))
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
import { HeadlessEmulator } from './headless-emulator'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { connect, createServer, type Server, type Socket } from 'node:net'
|
||||
import { tmpdir } from 'node:os'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
import { TerminalHost, type TerminalHostOptions } from './terminal-host'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
|
||||
@@ -130,13 +130,21 @@ describe('TerminalHost dead-session reaping (leak regression)', () => {
|
||||
})
|
||||
lastSubprocess.forceKill = vi.fn()
|
||||
|
||||
let releaseSweep = (): void => {}
|
||||
killWithDescendantSweepMock.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise<void>((resolve) => {
|
||||
releaseSweep = resolve
|
||||
})
|
||||
)
|
||||
const killed = host.kill('session-1', { immediate: true })
|
||||
|
||||
// Immediate teardown skips the graceful kill and force-kills the child directly. On POSIX
|
||||
// that reaches the child pgroup, so no Windows taskkill /T /F descendant sweep is needed.
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledTimes(1)
|
||||
expect(lastSubprocess.kill).not.toHaveBeenCalled()
|
||||
expect(lastSubprocess.forceKill).toHaveBeenCalled()
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
expect(lastSubprocess.forceKill).not.toHaveBeenCalled()
|
||||
expect(emulatorDispose).not.toHaveBeenCalled()
|
||||
releaseSweep()
|
||||
await vi.waitFor(() => expect(lastSubprocess.forceKill).toHaveBeenCalledTimes(1))
|
||||
expect(emulatorDispose).not.toHaveBeenCalled()
|
||||
expect(host.listSessions()).toHaveLength(1)
|
||||
lastSubprocess._onExitCb?.(137)
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { TerminalHost } from './terminal-host'
|
||||
import type { SubprocessHandle } from './session-subprocess-handle'
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import './mock-descendant-sweep'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as WslModule from '../wsl'
|
||||
|
||||
|
||||
@@ -61,50 +61,60 @@ describe('TerminalSessionTeardown plain-shell teardown', () => {
|
||||
expect(() => killRoot()).not.toThrow()
|
||||
})
|
||||
|
||||
it('win32 immediate kill claims termination before awaiting the sweep', async () => {
|
||||
// Why: createOrAttach rejects a doomed plain shell only via isTerminating, so the claim
|
||||
// must land before the taskkill await or an attach can bind a pane to a dying session.
|
||||
setPlatform('win32')
|
||||
const session = createPlainShellSession()
|
||||
const beginTermination = session.beginTermination as unknown as ReturnType<typeof vi.fn>
|
||||
let claimedBeforeSweep = false
|
||||
killWithDescendantSweepMock.mockImplementation(async () => {
|
||||
claimedBeforeSweep = beginTermination.mock.calls.length === 1
|
||||
})
|
||||
const teardown = new TerminalSessionTeardown(new Map([['s1', session]]))
|
||||
it.each(['win32', 'linux', 'darwin'] as const)(
|
||||
'%s immediate kill claims termination before awaiting the sweep',
|
||||
async (platform) => {
|
||||
// Why: createOrAttach rejects a doomed plain shell only via isTerminating, so the claim
|
||||
// must land before the taskkill await or an attach can bind a pane to a dying session.
|
||||
setPlatform(platform)
|
||||
const session = createPlainShellSession()
|
||||
const beginTermination = session.beginTermination as unknown as ReturnType<typeof vi.fn>
|
||||
let claimedBeforeSweep = false
|
||||
killWithDescendantSweepMock.mockImplementation(async () => {
|
||||
claimedBeforeSweep = beginTermination.mock.calls.length === 1
|
||||
})
|
||||
const teardown = new TerminalSessionTeardown(new Map([['s1', session]]))
|
||||
|
||||
await teardown.killSession('s1', session, true)
|
||||
await teardown.killSession('s1', session, true)
|
||||
|
||||
expect(claimedBeforeSweep).toBe(true)
|
||||
})
|
||||
expect(claimedBeforeSweep).toBe(true)
|
||||
}
|
||||
)
|
||||
|
||||
it('win32 sweep ownsRoot guard requires the live session to still own the id', async () => {
|
||||
setPlatform('win32')
|
||||
const session = createPlainShellSession()
|
||||
const sessions = new Map([['s1', session]])
|
||||
const teardown = new TerminalSessionTeardown(sessions)
|
||||
it.each(['win32', 'linux', 'darwin'] as const)(
|
||||
'%s sweep ownsRoot guard requires the live session to still own the id',
|
||||
async (platform) => {
|
||||
setPlatform(platform)
|
||||
const session = createPlainShellSession()
|
||||
const sessions = new Map([['s1', session]])
|
||||
const teardown = new TerminalSessionTeardown(sessions)
|
||||
|
||||
await teardown.killSession('s1', session, true)
|
||||
const ownsRoot = (killWithDescendantSweepMock.mock.calls[0][2] as { ownsRoot: () => boolean })
|
||||
.ownsRoot
|
||||
expect(ownsRoot()).toBe(true)
|
||||
await teardown.killSession('s1', session, true)
|
||||
const ownsRoot = (killWithDescendantSweepMock.mock.calls[0][2] as { ownsRoot: () => boolean })
|
||||
.ownsRoot
|
||||
expect(ownsRoot()).toBe(true)
|
||||
|
||||
// A natural exit or reap must stop us from taskkilling a recycled PID.
|
||||
;(session as unknown as { isAlive: boolean }).isAlive = false
|
||||
expect(ownsRoot()).toBe(false)
|
||||
sessions.delete('s1')
|
||||
;(session as unknown as { isAlive: boolean }).isAlive = true
|
||||
expect(ownsRoot()).toBe(false)
|
||||
})
|
||||
// A natural exit or reap must stop us from taskkilling a recycled PID.
|
||||
;(session as unknown as { isAlive: boolean }).isAlive = false
|
||||
expect(ownsRoot()).toBe(false)
|
||||
sessions.delete('s1')
|
||||
;(session as unknown as { isAlive: boolean }).isAlive = true
|
||||
expect(ownsRoot()).toBe(false)
|
||||
}
|
||||
)
|
||||
|
||||
it('non-win32 immediate kill skips the tree kill (pgroup force-kill suffices)', async () => {
|
||||
it('POSIX immediate close sweeps detached OMP tools before killing their parent', async () => {
|
||||
setPlatform('linux')
|
||||
const session = createPlainShellSession()
|
||||
const teardown = new TerminalSessionTeardown(new Map([['s1', session]]))
|
||||
|
||||
await teardown.killSession('s1', session, true)
|
||||
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledWith(
|
||||
session.pid,
|
||||
expect.any(Function),
|
||||
expect.objectContaining({ ownsRoot: expect.any(Function) })
|
||||
)
|
||||
expect(session.forceKillAndWaitForExit).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
|
||||
@@ -80,27 +80,13 @@ export class TerminalSessionTeardown {
|
||||
return operation
|
||||
}
|
||||
|
||||
/**
|
||||
* Immediate teardown of a non-agent shell. On Windows, closing the ConPTY does not
|
||||
* reap orphaned children (node-pty `useConptyDll` skips the console-process reap), so a
|
||||
* live `pnpm i`/`node` survives shell exit, keeps the ConPTY console non-empty, and holds
|
||||
* the worktree cwd — failing destructive worktree removal with "Failed to physically stop
|
||||
* every PTY". Tree-kill only when the OS identity probe returns `own`; `unknown`/`foreign`/
|
||||
* `absent` skip taskkill and rely on root close alone. Mirrors the agent path
|
||||
* (#10004/#10100). POSIX shells already reach their child pgroup on forceKill, so they
|
||||
* stay on the plain force-kill path.
|
||||
*/
|
||||
/** Immediate close must reach detached tools even when startup did not identify an agent. */
|
||||
private async forceKillPlainShellSession(sessionId: string, session: Session): Promise<void> {
|
||||
if (process.platform === 'win32') {
|
||||
// Why: forceKillAndWaitForExit claims termination synchronously; awaiting the sweep
|
||||
// ahead of it would leave attach open on a doomed session for the taskkill's duration.
|
||||
session.beginTermination()
|
||||
await killWithDescendantSweep(session.pid, () => {}, {
|
||||
// Why: the descendant tree is only ours while this Session still owns the live root PID.
|
||||
ownsRoot: () => this.sessions.get(sessionId) === session && session.isAlive,
|
||||
terminateOwnedTree: () => session.terminateOwnedTree()
|
||||
})
|
||||
}
|
||||
session.beginTermination()
|
||||
await killWithDescendantSweep(session.pid, () => {}, {
|
||||
ownsRoot: () => this.sessions.get(sessionId) === session && session.isAlive,
|
||||
terminateOwnedTree: () => session.terminateOwnedTree()
|
||||
})
|
||||
await session.forceKillAndWaitForExit()
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import {
|
||||
existsSyncMock,
|
||||
loginPreflightExecFileMock,
|
||||
spawnMock,
|
||||
openCodeClearPtyMock,
|
||||
piClearPtyMock
|
||||
@@ -162,9 +163,25 @@ describe('registerPtyHandlers', () => {
|
||||
rows: 24
|
||||
})) as { id: string }
|
||||
|
||||
let finishSnapshot: (() => void) | undefined
|
||||
loginPreflightExecFileMock.mockImplementationOnce(
|
||||
(
|
||||
_file: string,
|
||||
_args: string[],
|
||||
_options: unknown,
|
||||
callback: (error: Error | null, stdout: string) => void
|
||||
) => {
|
||||
finishSnapshot = () => callback(null, '')
|
||||
}
|
||||
)
|
||||
const killPromise = handlers.get('pty:kill')!(null, { id: spawnResult.id }) as Promise<void>
|
||||
|
||||
expect(killSpy).toHaveBeenCalledTimes(1)
|
||||
await vi.waitFor(() => expect(finishSnapshot).toBeTypeOf('function'))
|
||||
expect(killSpy).not.toHaveBeenCalled()
|
||||
expect(onDataDisposable.dispose).not.toHaveBeenCalled()
|
||||
expect(onExitDisposable.dispose).not.toHaveBeenCalled()
|
||||
finishSnapshot?.()
|
||||
await vi.waitFor(() => expect(killSpy).toHaveBeenCalledTimes(1))
|
||||
expect(onDataDisposable.dispose).not.toHaveBeenCalled()
|
||||
expect(onExitDisposable.dispose).not.toHaveBeenCalled()
|
||||
|
||||
|
||||
@@ -513,13 +513,16 @@ describe('LocalPtyProvider', () => {
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('non-win32 immediate shutdown of a plain shell skips the tree kill', async () => {
|
||||
// beforeEach pins platform to linux; POSIX force-kill already reaches the child pgroup.
|
||||
it('POSIX immediate shutdown sweeps detached OMP tools without startup recognition', async () => {
|
||||
const { id } = await provider.spawn({ cols: 80, rows: 24 })
|
||||
|
||||
await provider.shutdown(id, { immediate: true })
|
||||
|
||||
expect(killWithDescendantSweepMock).not.toHaveBeenCalled()
|
||||
expect(killWithDescendantSweepMock).toHaveBeenCalledWith(
|
||||
mockProc.pid,
|
||||
expect.any(Function),
|
||||
expect.objectContaining({ ownsRoot: expect.any(Function) })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -175,19 +175,8 @@ async function shutdownTrackedPty(
|
||||
operation.rootSignalled = true
|
||||
requestTrackedPtyShutdown(id, proc, operation.immediate)
|
||||
}
|
||||
if (ptyAgentSessionIds.has(id)) {
|
||||
// Why: POSIX needs a pre-kill descendant snapshot; Windows tree-kills only when the
|
||||
// identity probe returns `own` so agent/MCP orphans cannot hold the worktree cwd
|
||||
// (#10004). `unknown`/`foreign`/`absent` skip taskkill and rely on root close alone.
|
||||
await killWithDescendantSweep(proc.pid, signalRoot, {
|
||||
ownsRoot: () => ptyProcesses.get(id) === proc,
|
||||
terminateOwnedTree: () => terminatePtyJob(proc)
|
||||
})
|
||||
} else if (process.platform === 'win32' && operation.immediate) {
|
||||
// Why: a plain shell's ConPTY teardown doesn't reap orphaned children (useConptyDll
|
||||
// skips the console reap), so a live `pnpm i`/`node` keeps the ConPTY console alive and
|
||||
// holds the worktree cwd. Tree kill runs only when the OS identity probe returns `own`;
|
||||
// otherwise root close alone, and detached children may block physical stop (#10004).
|
||||
if (ptyAgentSessionIds.has(id) || operation.immediate) {
|
||||
// Typed agents also detach tool process groups; immediate close must snapshot before root exit.
|
||||
await killWithDescendantSweep(proc.pid, signalRoot, {
|
||||
ownsRoot: () => ptyProcesses.get(id) === proc,
|
||||
terminateOwnedTree: () => terminatePtyJob(proc)
|
||||
|
||||
@@ -16,7 +16,8 @@ import { describe, expect, it } from 'vitest'
|
||||
*/
|
||||
const SRC_DIR = join(__dirname, '..')
|
||||
const CALL = 'killWithDescendantSweep('
|
||||
const EXPECTED_MINIMUM_SITES = 5
|
||||
// Local immediate and recognized-agent shutdown share one guarded call site.
|
||||
const EXPECTED_MINIMUM_SITES = 4
|
||||
|
||||
function collectTypeScriptFiles(dir: string): string[] {
|
||||
const found: string[] = []
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import '../daemon/mock-descendant-sweep'
|
||||
import { mkdtempSync, rmSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
# OMP owned-PTY close probe (#9530)
|
||||
|
||||
This opt-in probe launches an actual installed OMP binary in disposable local PTYs
|
||||
and calls Orca's production `shutdownLocalPty` and `killAllLocalPtys` functions,
|
||||
or daemon `Session`, native subprocess handle, and `TerminalSessionTeardown`.
|
||||
It sets the same agent-session ownership flag that `activateLocalPtySession` sets
|
||||
for `launchAgent` / recognized startup commands, then repeats without that flag
|
||||
to represent OMP typed into a shell. This isolates termination policy; it does not
|
||||
exercise Agent button delivery or terminal-tab/handle routing.
|
||||
|
||||
```sh
|
||||
ORCA_BACKGROUND_LAUNCH=1 ORCA_OMP_PROBE_BINARY=/absolute/path/to/omp \
|
||||
node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts \
|
||||
tests/tools/omp-close-lifecycle.test.mjs
|
||||
```
|
||||
|
||||
The probe defaults to zsh on macOS and bash on other POSIX hosts. Set
|
||||
`ORCA_OMP_PROBE_SHELL` to the absolute path of either shell to override. Windows
|
||||
is skipped. It requires the existing node-pty native dependency for the current
|
||||
Node runtime. The normal unit suite skips the test unless a binary is supplied.
|
||||
|
||||
Each case waits five seconds for OMP startup, captures the owned process tree,
|
||||
requests explicit close or local quit cleanup, and verifies those exact process
|
||||
IDs are absent using host `ps` after a six-second observation window. It records
|
||||
raw terminal output and before/after process rows in `.bench-fixtures/omp-close-*`.
|
||||
The fixture contains no prompt or model request. It disables the first-run setup
|
||||
wizard, startup splash and update checks in a temporary config; OMP's normal tools
|
||||
and extensions remain enabled. HOME, ZDOTDIR, XDG_CONFIG_HOME and OMP's agent home
|
||||
are disposable. Cleanup signals only owned identities with matching process start
|
||||
time and group, then removes the temporary home.
|
||||
|
||||
## Observed on 2026-09-14
|
||||
|
||||
At Orca base `93c370246388`, macOS arm64, installed `omp/18.1.18`:
|
||||
|
||||
- Explicit local close with the agent flag: shell and foreground OMP exited.
|
||||
- Explicit local close without the flag: shell and foreground OMP exited.
|
||||
- Local quit cleanup with or without the flag: shell and foreground OMP exited.
|
||||
- Explicit close used the existing five-second force deadline for the shell.
|
||||
Quit removes native exit tracking immediately, so the probe uses independent
|
||||
host process evidence; an empty provider map is not its exit oracle.
|
||||
|
||||
The same four outcomes were observed in an initial first-run setup-splash pass.
|
||||
The normal-idle transcript displayed the OMP prompt and reported no LSP servers.
|
||||
No stale foreground OMP was reproduced in these local termination-policy cases.
|
||||
|
||||
## Detached external tool reproduction and correction
|
||||
|
||||
Set `ORCA_OMP_PROBE_EXTERNAL_TOOL=1` to run `! /bin/sleep 120` in OMP before
|
||||
explicit immediate close. Add `ORCA_OMP_PROBE_BACKEND=daemon` to exercise the daemon
|
||||
backend. Each mode tests both recognized and typed launches; these modes do not
|
||||
run the local-quit cases. The probe makes no model requests. Both OMP/PI profiles
|
||||
are cleared, and XDG data/cache/state roots are isolated alongside configuration.
|
||||
|
||||
On macOS, installed Orca `1.4.202-hourly.202609132311` and OMP `18.1.18`, an actual
|
||||
non-focus CLI-created terminal reproduced the detached-child leak: shell PID
|
||||
71632 and OMP PID 71667 exited after CLI close, but sleep PID 72125 (PGID 72125)
|
||||
remained after the grace window, reparented to PID 1. The owned survivor was
|
||||
cleaned using its captured PID/start-time/group identity. This is a detached-tool
|
||||
leak, not a reproduction of the reported foreground OMP surviving for days.
|
||||
|
||||
With the correction, all four actual OMP/external-sleep cases (recognized/typed,
|
||||
local/daemon) left none of the captured shell, OMP or sleep PIDs present. This
|
||||
runs production backend code with real PTYs; it does not run a rebuilt installed
|
||||
app through the CLI. Reports/transcripts remain local under `.bench-fixtures/`.
|
||||
|
||||
### Termination contract
|
||||
|
||||
Immediate close now uses the existing descendant sweep for all local-provider
|
||||
and daemon shells, including agents typed after startup. This also terminates
|
||||
still-parented, intentionally detached jobs that previously survived POSIX close.
|
||||
The sweep captures descendants before root exit, checks current root ownership,
|
||||
and retains the existing identity-guarded delayed escalation. It adds a bounded
|
||||
process-table capture (one-second timeout) and, when descendants exist, the
|
||||
existing single two-second delayed recheck; there is no recurring polling.
|
||||
Daemon termination is claimed before awaiting capture, preventing reattachment.
|
||||
Physical root exit still gates session reaping. Snapshot failure falls back to
|
||||
root termination; children already reparented before capture are not covered.
|
||||
|
||||
The execution host runs this policy. Paired runtimes using these backends receive
|
||||
the fix when their host updates; no wire fields or client-side remote PID signals
|
||||
are added. Direct SSH relay PTYs use separate `src/relay/pty-handler.ts` termination
|
||||
and are not fixed or runtime-validated by this change. Graceful plain-shell
|
||||
shutdown, disconnect and daemon/remote keep-alive policy are unchanged. The code
|
||||
uses no repository metadata and applies to folder workspaces as well as worktrees.
|
||||
Windows retains its existing guarded job/tree termination; this probe skips it.
|
||||
|
||||
## Limits and next evidence
|
||||
|
||||
Do not close #9530 from this probe. The original report did not identify Orca/OMP
|
||||
versions or the exact close action. A tab can disappear without this termination
|
||||
entry point running, which this probe does not cover. It also does not exercise
|
||||
full app quit lifecycle, background/floating/mobile handle resolution, a busy
|
||||
model turn, initialized eval workers or LSPs, Windows/WSL/Linux execution, or live SSH ownership. Daemon/remote keep-alive is intentional and remains unchanged.
|
||||
|
||||
A failing reproduction needs the original surface/close action, provider mode,
|
||||
owning runtime, and process identities before and after. Signal-resistant fixture
|
||||
processes alone do not establish that current OMP has the reported leak.
|
||||
@@ -0,0 +1,224 @@
|
||||
import { it, expect } from 'vitest'
|
||||
import * as pty from 'node-pty'
|
||||
import { Session } from '../../src/main/daemon/session.ts'
|
||||
import { TerminalSessionTeardown } from '../../src/main/daemon/terminal-session-teardown.ts'
|
||||
import { createDaemonPtySubprocessHandle } from '../../src/main/daemon/pty-subprocess/subprocess-handle.ts'
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { runProcess } from '../../src/shared/child-process/run-process.ts'
|
||||
import {
|
||||
captureDescendantSnapshot,
|
||||
readProcessTable
|
||||
} from '../../src/main/pty-descendant-termination.ts'
|
||||
import {
|
||||
createPtyPhysicalExit,
|
||||
shutdownLocalPty,
|
||||
killAllLocalPtys
|
||||
} from '../../src/main/providers/local-pty-termination.ts'
|
||||
import {
|
||||
ptyProcesses,
|
||||
ptyAgentSessionIds,
|
||||
ptyPhysicalExits,
|
||||
ptyExitDisposables,
|
||||
clearPtyState
|
||||
} from '../../src/main/providers/local-pty-provider-state.ts'
|
||||
|
||||
const binary = process.env.ORCA_OMP_PROBE_BINARY
|
||||
const externalTool = process.env.ORCA_OMP_PROBE_EXTERNAL_TOOL === '1'
|
||||
const daemonBackend = process.env.ORCA_OMP_PROBE_BACKEND === 'daemon'
|
||||
const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms))
|
||||
const quote = (value) => `'${value.replaceAll("'", "'\\''")}'`
|
||||
const ownedPidRows = async (pids) => {
|
||||
const result = await runProcess({
|
||||
program: 'ps',
|
||||
args: ['-p', pids.join(','), '-o', 'pid=,ppid=,pgid=,stat=,comm='],
|
||||
maxOutputBytes: 16000
|
||||
})
|
||||
expect(result.timedOut).toBe(false)
|
||||
expect(result.signal).toBeNull()
|
||||
expect(result.stderr.trim()).toBe('')
|
||||
expect([0, 1]).toContain(result.code)
|
||||
if (result.code === 1) {
|
||||
expect(result.stdout.trim()).toBe('')
|
||||
}
|
||||
return result.stdout.trim()
|
||||
}
|
||||
it.skipIf(!binary || process.platform === 'win32')(
|
||||
'observes actual OMP under production owned-PTY closure policy',
|
||||
async () => {
|
||||
const fixtures = join(process.cwd(), '.bench-fixtures')
|
||||
mkdirSync(fixtures, { recursive: true })
|
||||
const output = mkdtempSync(join(fixtures, 'omp-close-'))
|
||||
const report = []
|
||||
for (const launch of ['recognized', 'typed']) {
|
||||
for (const close of externalTool || daemonBackend ? ['explicit'] : ['explicit', 'quit']) {
|
||||
expect(ptyProcesses.size).toBe(0)
|
||||
const home = mkdtempSync(join(tmpdir(), 'orca-omp-close-home-'))
|
||||
const agentHome = join(home, 'agent')
|
||||
mkdirSync(agentHome)
|
||||
const config = join(home, 'probe.yml')
|
||||
writeFileSync(
|
||||
config,
|
||||
'startup:\n setupWizard: false\n showSplash: false\n checkUpdate: false\n'
|
||||
)
|
||||
const id = `${launch}-${close}`
|
||||
let transcript = ''
|
||||
let nativeExit = null
|
||||
const shell =
|
||||
process.env.ORCA_OMP_PROBE_SHELL ??
|
||||
(process.platform === 'darwin' ? '/bin/zsh' : '/bin/bash')
|
||||
const shellArgs = shell.endsWith('zsh') ? ['-f', '-i'] : ['--noprofile', '--norc', '-i']
|
||||
const proc = pty.spawn(shell, shellArgs, {
|
||||
name: 'xterm-256color',
|
||||
cols: 120,
|
||||
rows: 35,
|
||||
cwd: home,
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
ZDOTDIR: home,
|
||||
XDG_CONFIG_HOME: join(home, 'config'),
|
||||
XDG_DATA_HOME: join(home, 'data'),
|
||||
XDG_CACHE_HOME: join(home, 'cache'),
|
||||
XDG_STATE_HOME: join(home, 'state'),
|
||||
OMP_CODING_AGENT_DIR: agentHome,
|
||||
PI_CODING_AGENT_DIR: agentHome,
|
||||
OMP_PROFILE: '',
|
||||
PI_PROFILE: '',
|
||||
PI_CONFIG_DIR: '.omp',
|
||||
PI_CONFIG_FILES: '',
|
||||
ORCA_BACKGROUND_LAUNCH: '1'
|
||||
}
|
||||
})
|
||||
const daemonSession = daemonBackend
|
||||
? new Session({
|
||||
sessionId: id,
|
||||
cols: 120,
|
||||
rows: 35,
|
||||
shellReadySupported: false,
|
||||
...(launch === 'recognized' ? { launchAgent: 'omp' } : {}),
|
||||
subprocess: createDaemonPtySubprocessHandle({
|
||||
process: proc,
|
||||
shellPath: shell,
|
||||
spawnCwd: home,
|
||||
env: process.env,
|
||||
startupCommandDeliveredInShellArgs: false,
|
||||
reportsChildExitStatus: true,
|
||||
sessionId: id,
|
||||
startupAgentRecognition: null
|
||||
})
|
||||
})
|
||||
: null
|
||||
proc.onData((data) => {
|
||||
transcript = (transcript + data).slice(-131072)
|
||||
})
|
||||
if (!daemonSession) {
|
||||
ptyProcesses.set(id, proc)
|
||||
createPtyPhysicalExit(id)
|
||||
if (launch === 'recognized') {
|
||||
ptyAgentSessionIds.add(id)
|
||||
}
|
||||
}
|
||||
ptyExitDisposables.set(
|
||||
id,
|
||||
proc.onExit((event) => {
|
||||
nativeExit = event
|
||||
ptyPhysicalExits.get(id)?.markExited()
|
||||
clearPtyState(id)
|
||||
rmSync(home, { recursive: true, force: true })
|
||||
})
|
||||
)
|
||||
let snapshot
|
||||
try {
|
||||
proc.write(`${quote(binary)} --no-session --config ${quote(config)}\r`)
|
||||
await delay(5000)
|
||||
snapshot = await captureDescendantSnapshot(proc.pid)
|
||||
expect(snapshot?.descendants.length).toBeGreaterThan(0)
|
||||
if (externalTool) {
|
||||
proc.write('! /bin/sleep 120\r')
|
||||
for (let attempt = 0; attempt < 25; attempt++) {
|
||||
await delay(200)
|
||||
snapshot = await captureDescendantSnapshot(proc.pid)
|
||||
if (snapshot?.descendants.length > 1) {
|
||||
break
|
||||
}
|
||||
}
|
||||
expect(snapshot?.descendants.length).toBeGreaterThan(1)
|
||||
}
|
||||
const pids = [proc.pid, ...snapshot.descendants.map((row) => row.pid)]
|
||||
const before = await ownedPidRows(pids)
|
||||
expect(before).toContain('omp')
|
||||
if (externalTool) {
|
||||
expect(before).toContain('sleep')
|
||||
}
|
||||
const started = Date.now()
|
||||
let closeError = null
|
||||
try {
|
||||
if (daemonSession) {
|
||||
await new TerminalSessionTeardown(new Map([[id, daemonSession]])).killSession(
|
||||
id,
|
||||
daemonSession,
|
||||
true
|
||||
)
|
||||
} else if (close === 'explicit') {
|
||||
await shutdownLocalPty(id, { immediate: externalTool })
|
||||
} else {
|
||||
killAllLocalPtys()
|
||||
}
|
||||
} catch (error) {
|
||||
closeError = String(error)
|
||||
}
|
||||
await delay(6000)
|
||||
const after = await ownedPidRows(pids)
|
||||
report.push({
|
||||
launch,
|
||||
close,
|
||||
externalTool,
|
||||
backend: daemonBackend ? 'daemon' : 'local',
|
||||
before,
|
||||
after,
|
||||
nativeExit,
|
||||
tracked: daemonSession ? daemonSession.isAlive : ptyProcesses.has(id),
|
||||
elapsedMs: Date.now() - started,
|
||||
closeError,
|
||||
home
|
||||
})
|
||||
writeFileSync(join(output, `${id}.txt`), transcript)
|
||||
writeFileSync(join(output, 'report.json'), JSON.stringify(report, null, 2))
|
||||
expect(closeError).toBeNull()
|
||||
expect(after).toBe('')
|
||||
} finally {
|
||||
if (snapshot) {
|
||||
const current = await readProcessTable()
|
||||
const owned = [
|
||||
...snapshot.descendants,
|
||||
...(snapshot.root ? [{ ...snapshot.root, pgid: snapshot.rootPgid }] : [])
|
||||
]
|
||||
for (const row of current.rows) {
|
||||
if (
|
||||
owned.some(
|
||||
(known) =>
|
||||
known.pid === row.pid &&
|
||||
known.startedAt === row.startedAt &&
|
||||
known.pgid === row.pgid
|
||||
)
|
||||
) {
|
||||
try {
|
||||
process.kill(row.pid, 'SIGKILL')
|
||||
} catch {}
|
||||
}
|
||||
}
|
||||
}
|
||||
daemonSession?.dispose()
|
||||
clearPtyState(id)
|
||||
rmSync(home, { recursive: true, force: true })
|
||||
}
|
||||
}
|
||||
}
|
||||
writeFileSync(join(output, 'report.json'), JSON.stringify(report, null, 2))
|
||||
console.log(output)
|
||||
},
|
||||
90000
|
||||
)
|
||||
Reference in New Issue
Block a user