mirror of
https://github.com/stablyai/orca.git
synced 2026-10-19 08:03:13 +00:00
Gate Windows tree-kill on PTY identity verification
Only taskkill when the identity probe returns `own`; skip tree-kill for `unknown`/`foreign`/`absent` to avoid terminating unrelated processes. Clarifies the fail-closed behavior across agent and plain shell teardown paths, matching the POSIX descendant-snapshot discipline.
This commit is contained in:
@@ -49,9 +49,10 @@ export class TerminalSessionTeardown {
|
||||
* 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". taskkill /T /F the tree first so physical exit becomes verifiable. Mirrors
|
||||
* the agent path (#10004/#10100). POSIX shells already reach their child pgroup on
|
||||
* forceKill, so they stay on the plain force-kill path.
|
||||
* 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.
|
||||
*/
|
||||
private async forceKillPlainShellSession(sessionId: string, session: Session): Promise<void> {
|
||||
if (process.platform === 'win32') {
|
||||
|
||||
@@ -1123,16 +1123,17 @@ export class LocalPtyProvider implements IPtyProvider {
|
||||
this.requestTrackedPtyShutdown(id, proc, operation.immediate)
|
||||
}
|
||||
if (ptyAgentSessionIds.has(id)) {
|
||||
// Why: POSIX needs a pre-kill descendant snapshot; Windows uses taskkill /T so
|
||||
// agent/MCP orphans cannot hold the worktree cwd after shell stop (#10004).
|
||||
// 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
|
||||
})
|
||||
} 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, failing destructive removal. taskkill /T /F clears the tree so
|
||||
// physical stop is verifiable. POSIX shells reach their child pgroup on forceKill (#10004).
|
||||
// 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).
|
||||
await killWithDescendantSweep(proc.pid, signalRoot, {
|
||||
ownsRoot: () => ptyProcesses.get(id) === proc
|
||||
})
|
||||
|
||||
@@ -204,7 +204,7 @@ type SnapshotDeps = {
|
||||
* signalled: once the root dies, surviving descendants reparent to pid 1 and
|
||||
* can no longer be found by a ppid walk. Resolves null (never rejects) on
|
||||
* Windows, ps failure, or timeout — callers then degrade to shell-only kill
|
||||
* on POSIX, or Windows `taskkill /T` via killWithDescendantSweep.
|
||||
* on POSIX, or identity-gated Windows `taskkill /T` via killWithDescendantSweep.
|
||||
*/
|
||||
export async function captureDescendantSnapshot(
|
||||
rootPid: number,
|
||||
@@ -237,9 +237,9 @@ type KillSweepDeps = SnapshotDeps &
|
||||
/**
|
||||
* Standard agent-session kill sequencing.
|
||||
* - POSIX: snapshot the descendant tree, signal members, then killRoot.
|
||||
* - Windows: taskkill /T /F walks the ConPTY tree (shell → agent → MCP) so
|
||||
* worktree teardown is not blocked by orphans holding the cwd handle, but only
|
||||
* after the OS confirms the root PID is still ours and not a recycled stranger.
|
||||
* - Windows: taskkill /T /F walks the ConPTY tree only when the identity probe
|
||||
* returns `own` (and ownsRoot still holds). `unknown`/`foreign`/`absent` skip
|
||||
* tree kill; killRoot always runs. Detached children may survive probe failure.
|
||||
* Callers must not signal the root before this runs on POSIX — a dead root's
|
||||
* descendants reparent to pid 1 and become unfindable. Snapshot failure
|
||||
* degrades to killRoot alone on POSIX.
|
||||
|
||||
@@ -36,8 +36,11 @@ describe('classifyWindowsTreeKillTarget', () => {
|
||||
expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('own')
|
||||
})
|
||||
|
||||
it('documents that another Orca descendant remains indistinguishable from the PTY root', () => {
|
||||
const rows = [link(ORCA_PID, 900), link(6100, ORCA_PID), link(4242, 6100)]
|
||||
it('documents that a recycled PID under another Orca pane still classifies as own', () => {
|
||||
// Dead PTY root 4242 recycled as a tool under a different pane's agent tree.
|
||||
// Ancestry still reaches us, so taskkill is allowed — wrong process, own tree.
|
||||
// Closing this needs spawn-time CreationDate / Job Object (#10680).
|
||||
const rows = [link(ORCA_PID, 900), link(7000, ORCA_PID), link(7100, 7000), link(4242, 7100)]
|
||||
expect(classifyWindowsTreeKillTarget(4242, rows, ORCA_PID)).toBe('own')
|
||||
})
|
||||
|
||||
|
||||
@@ -28,11 +28,12 @@ export type WindowsProcessLinkReader = () => Promise<readonly ProcessLink[] | nu
|
||||
* so it resolves `foreign` and must never reach `taskkill /T /F`. Ambiguous
|
||||
* tables resolve `unknown` because ambiguity is not evidence of ownership.
|
||||
*
|
||||
* Known limit: a recycle that lands on one of our OWN descendants — another
|
||||
* pane's shell, an agent CLI, a `git.exe` we spawned — still reads `own`. That
|
||||
* is not remote during teardown, when Orca is itself the process allocating
|
||||
* pids. Closing it needs real identity (a `Win32_Process.CreationDate` baseline,
|
||||
* the analogue of the POSIX `lstart` check, or an inherited handle/Job Object).
|
||||
* Known limit (#10680): a recycle that lands on one of our OWN descendants —
|
||||
* another pane's shell, an agent CLI, a `git.exe` we spawned — still reads
|
||||
* `own`. That is not remote during teardown, when Orca is itself the process
|
||||
* allocating pids. Closing it needs real identity (a `Win32_Process.CreationDate`
|
||||
* baseline, the analogue of the POSIX `lstart` check, or an inherited handle /
|
||||
* Job Object).
|
||||
*/
|
||||
export function classifyWindowsTreeKillTarget(
|
||||
rootPid: number,
|
||||
|
||||
Reference in New Issue
Block a user