mirror of
https://github.com/stablyai/orca.git
synced 2026-09-29 08:03:20 +00:00
Merge remote-tracking branch 'origin/main' into brennanb2025/pr18697-merge
This commit is contained in:
@@ -807,6 +807,7 @@ jobs:
|
||||
src/main/agent-hooks/windows-hook-payload-delivery.test.ts
|
||||
src/main/windows/windows-pty-job.win32.test.ts
|
||||
src/main/windows/windows-host-job.win32.test.ts
|
||||
src/main/windows-live-tree-kill.win32.test.ts
|
||||
src/main/wsl/wsl-runner.test.ts
|
||||
src/main/wsl/wsl-guest-environment.test.ts
|
||||
src/main/wsl/wsl-invocation-boundary.test.ts
|
||||
|
||||
@@ -219,6 +219,7 @@ const WINDOWS_PACKAGE_TESTS = [
|
||||
'src/main/agent-hooks/windows-hook-payload-delivery.test.ts',
|
||||
'src/main/windows/windows-pty-job.win32.test.ts',
|
||||
'src/main/windows/windows-host-job.win32.test.ts',
|
||||
'src/main/windows-live-tree-kill.win32.test.ts',
|
||||
'src/main/wsl/wsl-runner.test.ts',
|
||||
'src/main/wsl/wsl-guest-environment.test.ts',
|
||||
'src/main/wsl/wsl-invocation-boundary.test.ts',
|
||||
|
||||
@@ -127,10 +127,6 @@ __orca_osc133_precmd() {
|
||||
unset __orca_in_command
|
||||
fi
|
||||
printf "\033]133;A\007"
|
||||
# Why: emit the shell-ready marker here (not a trailing PROMPT_COMMAND entry)
|
||||
# so a framework that must be last in PROMPT_COMMAND — bash-preexec — is not
|
||||
# displaced by one of Orca's own hooks.
|
||||
[[ -n "$__orca_ready_marker" ]] && printf "\033]777;orca-shell-ready\007"
|
||||
return "$exit_code"
|
||||
}
|
||||
__orca_osc133_preexec() {
|
||||
@@ -188,6 +184,11 @@ __orca_osc133_epilogue() {
|
||||
unset __orca_in_prompt_command
|
||||
__orca_adopt_outer_debug_trap
|
||||
trap '__orca_osc133_preexec' DEBUG
|
||||
# Readline renders PS1 after entering raw mode; prompt hooks still run in cooked mode.
|
||||
if [[ -n "$__orca_ready_marker" ]]; then
|
||||
PS1="${PS1-}"'\[\e]777;orca-shell-ready\a\]'
|
||||
__orca_ready_marker=""
|
||||
fi
|
||||
}
|
||||
__orca_normalize_prompt_command_part() {
|
||||
local __orca_value="$1" __orca_output_name="$2" __orca_character __orca_chunk
|
||||
|
||||
@@ -4,6 +4,7 @@ import type { AutomationPrecheck, AutomationPrecheckResult } from '../../shared/
|
||||
import { MAX_AUTOMATION_PRECHECK_OUTPUT_CHARS } from '../../shared/automation-precheck'
|
||||
import { getSshConnectionManager } from '../ipc/ssh'
|
||||
import { shellEscape } from '../ssh/ssh-connection-utils'
|
||||
import { admitSelfInitiatedTreeKill } from '../own-chromium-tree-kill-guard'
|
||||
|
||||
type AutomationPrecheckExecutionTarget =
|
||||
| {
|
||||
@@ -73,7 +74,10 @@ function failedPrecheckResult(
|
||||
})
|
||||
}
|
||||
|
||||
function killLocalPrecheckProcessTree(child: ChildProcess): ReturnType<typeof setTimeout> | null {
|
||||
/** Exported for the refusal-fallback test; the timeout path is otherwise unreachable. */
|
||||
export function killLocalPrecheckProcessTree(
|
||||
child: ChildProcess
|
||||
): ReturnType<typeof setTimeout> | null {
|
||||
const pid = child.pid
|
||||
if (!pid) {
|
||||
child.kill()
|
||||
@@ -81,6 +85,18 @@ function killLocalPrecheckProcessTree(child: ChildProcess): ReturnType<typeof se
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
if (
|
||||
!admitSelfInitiatedTreeKill({
|
||||
pid,
|
||||
site: 'automation-precheck-timeout',
|
||||
scope: 'win-taskkill-tree'
|
||||
})
|
||||
) {
|
||||
// Refusal blocks the tree walk, not the termination: killing the root by
|
||||
// handle cannot reach a recycled pid, and a timed-out precheck must stop.
|
||||
child.kill()
|
||||
return null
|
||||
}
|
||||
try {
|
||||
// Why: shell prechecks can launch child processes; taskkill walks the
|
||||
// Windows process tree so timeout means the command is actually stopped.
|
||||
|
||||
@@ -2,6 +2,7 @@ import { spawn, type ChildProcess, type ChildProcessWithoutNullStreams } from 'n
|
||||
import { waitForProcessExitUntil } from './codex-process-exit-deadline'
|
||||
import { stderrIndicatesMissingAppServer } from './codex-app-server-capability-signal'
|
||||
import { withCliRuntimeOnPath } from '../../shared/node-cli-command-resolution'
|
||||
import { admitProcessTreeKill } from '../../shared/child-process/process-tree-kill-gate'
|
||||
|
||||
// Why: `codex app-server` is Orca's sanctioned RPC surface into Codex-owned
|
||||
// state (hook trust hashes, the sqlite thread index). This module owns the
|
||||
@@ -74,6 +75,18 @@ export function killCodexAppServerProcessTree(
|
||||
const platform = options.platform ?? process.platform
|
||||
const spawnImpl = options.spawnImpl ?? spawn
|
||||
if (platform === 'win32' && child.pid) {
|
||||
if (
|
||||
!admitProcessTreeKill({
|
||||
pid: child.pid,
|
||||
site: 'codex-app-server-session-deadline',
|
||||
scope: 'win-taskkill-tree'
|
||||
})
|
||||
) {
|
||||
// Refusal blocks the tree walk, not the termination: the root kill is
|
||||
// handle-addressed, so it cannot reach the recycled pid we refused.
|
||||
child.kill('SIGKILL')
|
||||
return
|
||||
}
|
||||
try {
|
||||
// Why: npm-installed Codex runs behind cmd.exe; killing only that wrapper
|
||||
// leaves the app-server child alive after a timeout or failed shutdown.
|
||||
|
||||
@@ -57,6 +57,8 @@ async function terminateWindowsAddedProcesses(
|
||||
const added = current.filter((row) => baseline.get(row.pid) !== windowsIdentity(row))
|
||||
const addedPids = new Set(added.map((row) => row.pid))
|
||||
const roots = added.filter((row) => !addedPids.has(row.ppid))
|
||||
// Added roots come from a table walk, not a spawn, so a refused tree walk has
|
||||
// no handle to fall back to: the row stays in `remaining` and this reports false.
|
||||
await Promise.all(
|
||||
roots.map((row) => terminateWindowsProcessTree(row.pid, { site: 'codex-turn-added-roots' }))
|
||||
)
|
||||
|
||||
@@ -17,17 +17,17 @@ import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-go
|
||||
import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation'
|
||||
import {
|
||||
findSelfInitiatedTreeKills,
|
||||
installProcessTreeKillBreadcrumbObserver,
|
||||
recordRefusedOwnChromiumTreeKill,
|
||||
recordSelfInitiatedTreeKill,
|
||||
resetSelfInitiatedTreeKillLogForTest,
|
||||
selfInitiatedTreeKillDetails
|
||||
} from './self-initiated-tree-kill-log'
|
||||
import {
|
||||
notifyProcessTreeKill,
|
||||
setProcessTreeKillObserver
|
||||
} from '../../shared/child-process/process-tree-kill-observer'
|
||||
admitProcessTreeKill,
|
||||
setProcessTreeKillGate
|
||||
} from '../../shared/child-process/process-tree-kill-gate'
|
||||
import { terminateWindowsProcessTree } from '../windows-process-tree-kill'
|
||||
import { installMainProcessTreeKillGate } from '../own-chromium-tree-kill-guard'
|
||||
import { _resetTracerForTests, setActiveSink } from '../observability/tracer'
|
||||
|
||||
/** The field shape: renderer, `reason=killed exitCode=1`, win32 (#G2). */
|
||||
@@ -252,12 +252,90 @@ describe('self-initiated tree kill breadcrumb', () => {
|
||||
expect(String(details.selfInitiatedKills)).toContain('more)')
|
||||
})
|
||||
|
||||
it('records a kill issued through the shared runProcess choke point', () => {
|
||||
installProcessTreeKillBreadcrumbObserver()
|
||||
it('keeps the pid-addressed kill when a window-close burst overruns the ring', () => {
|
||||
// Review probe: one taskkill, then 32 routine Job Object teardowns. Under
|
||||
// plain FIFO the discriminating entry is evicted and the persisted detail
|
||||
// becomes byte-identical to the external-kill arm.
|
||||
const goneAt = 5_000_000
|
||||
recordSelfInitiatedTreeKill({
|
||||
pid: 4242,
|
||||
site: 'pty-descendant-sweep',
|
||||
scope: 'win-taskkill-tree',
|
||||
at: goneAt - 4_000
|
||||
})
|
||||
for (let index = 0; index < 32; index += 1) {
|
||||
recordSelfInitiatedTreeKill({
|
||||
pid: 6000 + index,
|
||||
site: 'windows-pty-job-teardown',
|
||||
scope: 'win-pty-job',
|
||||
at: goneAt - 100
|
||||
})
|
||||
}
|
||||
|
||||
notifyProcessTreeKill({ pid: 3131, site: 'run-process-tree', scope: 'posix-process-group' })
|
||||
const details = selfInitiatedTreeKillDetails(goneAt)
|
||||
|
||||
expect(details.selfInitiatedTreeKillCount).toBe(1)
|
||||
expect(details.selfInitiatedGroupKillCount).toBe(31)
|
||||
expect(String(details.selfInitiatedKills)).toMatch(
|
||||
/^win-taskkill-tree\/pty-descendant-sweep\/pid4242 -4000ms/
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps the newest teardown when a session has saturated the ring with pid kills', () => {
|
||||
// Review probe, the mirror of the case above: 32 session-old taskkills (six
|
||||
// routine families feed them) then the Job Object teardown 50ms before the
|
||||
// death. A scope-preference eviction with no floor splices the entry it just
|
||||
// pushed, and `{}` is byte-identical to the external-kill arm.
|
||||
const goneAt = 5_000_000
|
||||
for (let index = 0; index < 32; index += 1) {
|
||||
recordSelfInitiatedTreeKill({
|
||||
pid: 6000 + index,
|
||||
site: 'pty-descendant-sweep',
|
||||
scope: 'win-taskkill-tree',
|
||||
at: goneAt - 600_000 + index * 1_000
|
||||
})
|
||||
}
|
||||
recordSelfInitiatedTreeKill({
|
||||
pid: 7777,
|
||||
site: 'windows-pty-job-teardown',
|
||||
scope: 'win-pty-job',
|
||||
at: goneAt - 50
|
||||
})
|
||||
|
||||
const details = selfInitiatedTreeKillDetails(goneAt)
|
||||
|
||||
expect(details.selfInitiatedGroupKillCount).toBe(1)
|
||||
expect(String(details.selfInitiatedKills)).toContain(
|
||||
'win-pty-job/windows-pty-job-teardown/pid7777 -50ms'
|
||||
)
|
||||
})
|
||||
|
||||
it('evicts the oldest pid kill, not the newest, once every candidate is pid-addressed', () => {
|
||||
const goneAt = 5_000_000
|
||||
for (let index = 0; index < 33; index += 1) {
|
||||
recordSelfInitiatedTreeKill({
|
||||
pid: 6000 + index,
|
||||
site: 'git-command-tree-kill',
|
||||
scope: 'win-taskkill-tree',
|
||||
at: goneAt - 1_000
|
||||
})
|
||||
}
|
||||
|
||||
const pids = findSelfInitiatedTreeKills(goneAt).map((kill) => kill.pid)
|
||||
|
||||
expect(pids).toHaveLength(32)
|
||||
expect(pids).toContain(6032)
|
||||
expect(pids).not.toContain(6000)
|
||||
})
|
||||
|
||||
it('records a kill issued through the shared runProcess choke point', () => {
|
||||
installMainProcessTreeKillGate()
|
||||
|
||||
expect(
|
||||
admitProcessTreeKill({ pid: 3131, site: 'run-process-tree', scope: 'posix-process-group' })
|
||||
).toBe(true)
|
||||
|
||||
expect(findSelfInitiatedTreeKills(Date.now()).map((kill) => kill.pid)).toEqual([3131])
|
||||
setProcessTreeKillObserver(null)
|
||||
setProcessTreeKillGate(null)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
import type { CrashReportDetailValue } from '../../shared/crash-reporting'
|
||||
import {
|
||||
setProcessTreeKillObserver,
|
||||
type ProcessTreeKillScope
|
||||
} from '../../shared/child-process/process-tree-kill-observer'
|
||||
import type { ProcessTreeKillScope } from '../../shared/child-process/process-tree-kill-gate'
|
||||
import { recordCoalescedDurableCrashBreadcrumb } from './durable-crash-breadcrumb'
|
||||
|
||||
/**
|
||||
@@ -19,24 +16,38 @@ import { recordCoalescedDurableCrashBreadcrumb } from './durable-crash-breadcrum
|
||||
* The ring is per-process and its only reader is `process-gone-recorder`, which
|
||||
* exists in Electron main. So a count reported on a `render-process-gone` covers
|
||||
* kills issued *from Electron main*, and nothing else:
|
||||
* - Main only: the three `taskkill /T /F` families that gate on
|
||||
* `admitSelfInitiatedTreeKill` (`terminateWindowsProcessTree` and the codex /
|
||||
* claude account-login teardowns) and the codex app-server POSIX group
|
||||
* - Main only: the families that import the gate directly —
|
||||
* `terminateWindowsProcessTree`, the codex and claude account-login
|
||||
* teardowns, the git command-runner abort, the notebook-cell and
|
||||
* automation-precheck timeouts — plus the codex app-server POSIX group
|
||||
* teardowns.
|
||||
* - Main *and* other hosts: `signalProcessTree` (the `runProcess` choke point,
|
||||
* reached from the CLI, relay and daemon too — a fourth pid-addressed
|
||||
* `taskkill` family, gated on the child not being reaped rather than on the
|
||||
* Chromium set it cannot read), the POSIX PTY process-group sweep and the
|
||||
* Windows PTY Job Object (relay `pty-handler`, daemon
|
||||
* `subprocess-handle`). When those run outside main they record into that
|
||||
* process's own ring, which nothing reads — no observer is installed there,
|
||||
* and the tracer sink is a no-op.
|
||||
* - Never instrumented: the direct `process.kill(-pid)` calls in the browser
|
||||
* routes, notebooks, automation prechecks and ephemeral-VM recipes.
|
||||
* - Main *and* other hosts, through the `process-tree-kill-gate` seam main
|
||||
* installs the same guard into: `signalProcessTree` (the `runProcess` choke
|
||||
* point, reached from the CLI, relay and daemon too), the codex app-server
|
||||
* deadline kill (compiled into the CLI as well) and the ephemeral-VM recipe
|
||||
* kill. Also host-spanning but recording directly: the POSIX PTY
|
||||
* process-group sweep and the Windows PTY Job Object (relay `pty-handler`,
|
||||
* daemon `subprocess-handle`). When any of these run outside main they record
|
||||
* into that process's own ring, which nothing reads — no gate is installed
|
||||
* there, and the tracer sink is a no-op.
|
||||
* - Never instrumented, and none of them a pid-addressed kill issued from main:
|
||||
* the POSIX `process.kill(-pid, …)` group arms of the notebook, precheck,
|
||||
* browser-route and ephemeral-VM kills, plus the macOS keyboard-input-source
|
||||
* probe's group kill in `ipc/app.ts`; the relay's own
|
||||
* `subprocess-tree-termination` taskkill and the CLI's login-interruption
|
||||
* taskkill (neither runs in main); and the browser-route Electron probes,
|
||||
* which are reached only from `*.electron.test.ts`.
|
||||
*
|
||||
* `main-process-tree-kill-gate.test.ts` is the ratchet that keeps that list
|
||||
* closed: it counts `/pid` call sites against gate admissions per file, so a new
|
||||
* pid-addressed kill fails it whether it lands in a new file or inside a family
|
||||
* that already asks the gate. It does not see a `/pid` argument built from a
|
||||
* variable.
|
||||
*
|
||||
* A daemon or relay kill missing from the count is a diagnostics gap, not a
|
||||
* missed suspect: those hosts cannot reach a Chromium pid in the first place
|
||||
* (see `orca-chromium-process-pids.ts`). Absence is evidence, not proof.
|
||||
* (see `orca-chromium-process-pids.ts`), and a group or Job-Object kill can
|
||||
* only contain what Orca put in it. Absence is evidence, not proof.
|
||||
*/
|
||||
|
||||
/** Which mechanism issued the kill; each has a different blast radius. */
|
||||
@@ -81,6 +92,25 @@ function isPidAddressedTreeKill(scope: SelfInitiatedTreeKillScope): boolean {
|
||||
return scope === 'win-taskkill-tree'
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop one entry, newest-first-preserving.
|
||||
*
|
||||
* Two rules, in order. The entry just recorded is never a candidate: it is the
|
||||
* one closest to any death that follows, and evicting it leaves a detail
|
||||
* byte-identical to the external-kill arm. Among the rest, routine group/job
|
||||
* teardown goes before a pid-addressed kill — a window-close burst is 30+ group
|
||||
* kills and plain FIFO would drop the one entry that can explain the death —
|
||||
* falling back to plain FIFO once every candidate is pid-addressed, which is
|
||||
* what an ordinary session saturates the ring with.
|
||||
*/
|
||||
function evictOneSelfInitiatedTreeKill(): void {
|
||||
const lastCandidate = selfInitiatedKills.length - 1
|
||||
const oldestGroupKill = selfInitiatedKills.findIndex(
|
||||
(kill, index) => index < lastCandidate && !isPidAddressedTreeKill(kill.scope)
|
||||
)
|
||||
selfInitiatedKills.splice(Math.max(oldestGroupKill, 0), 1)
|
||||
}
|
||||
|
||||
export function recordSelfInitiatedTreeKill({
|
||||
pid,
|
||||
site,
|
||||
@@ -96,13 +126,15 @@ export function recordSelfInitiatedTreeKill({
|
||||
return
|
||||
}
|
||||
selfInitiatedKills.push({ pid, site, scope, at })
|
||||
if (selfInitiatedKills.length > MAX_TRACKED_SELF_KILLS) {
|
||||
selfInitiatedKills = selfInitiatedKills.slice(-MAX_TRACKED_SELF_KILLS)
|
||||
while (selfInitiatedKills.length > MAX_TRACKED_SELF_KILLS) {
|
||||
evictOneSelfInitiatedTreeKill()
|
||||
}
|
||||
// Durable so it survives into the diagnostic bundle even when the kill takes
|
||||
// the reporting renderer with it; coalesced because the crash detail above is
|
||||
// the primary record and a teardown burst must not cost 30 ring slots plus a
|
||||
// forced disk flush each. The newest pid still rides the emitted crumb.
|
||||
// forced disk flush each. The retained ring crumb carries the newest pid, but
|
||||
// the span trail emits only the first of a coalesced burst — read
|
||||
// `selfInitiatedKills` for the rest.
|
||||
recordCoalescedDurableCrashBreadcrumb({
|
||||
name: 'self_tree_kill',
|
||||
data: { pid, site, scope },
|
||||
@@ -133,11 +165,6 @@ export function recordRefusedOwnChromiumTreeKill(target: {
|
||||
})
|
||||
}
|
||||
|
||||
/** Routes the `runProcess` choke point's kills here; shared code cannot import us. */
|
||||
export function installProcessTreeKillBreadcrumbObserver(): void {
|
||||
setProcessTreeKillObserver((kill) => recordSelfInitiatedTreeKill(kill))
|
||||
}
|
||||
|
||||
export function findSelfInitiatedTreeKills(at: number): SelfInitiatedTreeKill[] {
|
||||
return selfInitiatedKills.filter((kill) => {
|
||||
const offsetMs = kill.at - at
|
||||
|
||||
@@ -0,0 +1,123 @@
|
||||
import { appendFileSync, existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createPtySubprocess } from './pty-subprocess'
|
||||
import { Session } from './session'
|
||||
|
||||
const SHELLS = process.platform === 'win32' ? [] : ['/bin/bash', '/bin/zsh'].filter(existsSync)
|
||||
const COMMAND = "printf 'AGENT_%s\\n' STARTED"
|
||||
|
||||
async function launch(
|
||||
shell: string,
|
||||
slow: boolean,
|
||||
legacy = false
|
||||
): Promise<{ output: string; ms: number }> {
|
||||
const root = mkdtempSync(join(tmpdir(), 'orca-startup-latency-'))
|
||||
const bash = shell.endsWith('bash')
|
||||
const pause = slow ? 'sleep 0.6\n' : ''
|
||||
const prompt = slow ? "PS1='$(sleep 0.3)prompt> '\n" : "PS1='prompt> '\n"
|
||||
writeFileSync(
|
||||
join(root, bash ? '.bash_profile' : '.zshrc'),
|
||||
`${pause}${bash ? '' : 'setopt PROMPT_SUBST\n'}${prompt}`
|
||||
)
|
||||
vi.stubEnv('HOME', root)
|
||||
vi.stubEnv('ZDOTDIR', root)
|
||||
vi.stubEnv('ORCA_ORIG_ZDOTDIR', root)
|
||||
let session: Session | undefined
|
||||
let timer: ReturnType<typeof setTimeout> | undefined
|
||||
let legacyTimer: ReturnType<typeof setTimeout> | undefined
|
||||
const readinessEvents: string[] = []
|
||||
const started = performance.now()
|
||||
try {
|
||||
const subprocess = await createPtySubprocess({
|
||||
sessionId: 'startup-latency',
|
||||
cols: 120,
|
||||
rows: 30,
|
||||
cwd: root,
|
||||
shellOverride: shell,
|
||||
command: COMMAND,
|
||||
env: { HOME: root, SHELL: shell, TERM: 'xterm-256color' }
|
||||
})
|
||||
session = new Session({
|
||||
sessionId: 'startup-latency',
|
||||
cols: 120,
|
||||
rows: 30,
|
||||
subprocess,
|
||||
shellReadySupported: !legacy,
|
||||
reportReadinessEvent: (event) => readinessEvents.push(event)
|
||||
})
|
||||
const active = session
|
||||
return await new Promise((resolve, reject) => {
|
||||
let output = ''
|
||||
timer = setTimeout(
|
||||
() => reject(new Error(`Startup timed out: ${JSON.stringify(output)}`)),
|
||||
5000
|
||||
)
|
||||
active.attachClient({
|
||||
onExit: () => {},
|
||||
onData: (data) => {
|
||||
output += data
|
||||
if (output.includes('AGENT_STARTED')) {
|
||||
resolve({ output, ms: performance.now() - started })
|
||||
}
|
||||
}
|
||||
})
|
||||
if (legacy) {
|
||||
legacyTimer = setTimeout(() => active.write(`${COMMAND}\n`), 300)
|
||||
} else {
|
||||
active.write(`${COMMAND}\n`)
|
||||
}
|
||||
})
|
||||
} finally {
|
||||
clearTimeout(timer)
|
||||
clearTimeout(legacyTimer)
|
||||
if (session) {
|
||||
await session.forceKillAndWaitForExit(3000)
|
||||
session.dispose()
|
||||
}
|
||||
vi.unstubAllEnvs()
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
expect(readinessEvents).toEqual([])
|
||||
}
|
||||
}
|
||||
|
||||
describe('agent startup at the rendered shell prompt', () => {
|
||||
afterEach(() => vi.unstubAllEnvs())
|
||||
it.each(SHELLS)(
|
||||
'%s displays the command once after slow startup and prompt expansion',
|
||||
async (shell) => {
|
||||
const before = await launch(shell, true, true)
|
||||
expect(before.output.split(COMMAND)).toHaveLength(3)
|
||||
const result = await launch(shell, true)
|
||||
expect(result.output).not.toContain('orca-shell-ready')
|
||||
expect(result.output.split(COMMAND)).toHaveLength(2)
|
||||
expect(result.output.indexOf('prompt> ')).toBeLessThan(result.output.indexOf(COMMAND))
|
||||
}
|
||||
)
|
||||
|
||||
it.skipIf(!process.env.ORCA_STARTUP_BENCH || SHELLS.length === 0)(
|
||||
'compares legacy input timing with prompt delivery',
|
||||
async () => {
|
||||
for (const shell of SHELLS) {
|
||||
for (const slow of [false, true]) {
|
||||
const legacy: number[] = []
|
||||
const current: number[] = []
|
||||
for (let i = 0; i < 5; i++) {
|
||||
legacy.push((await launch(shell, slow, true)).ms)
|
||||
const result = await launch(shell, slow)
|
||||
expect(result.output).not.toContain('orca-shell-ready')
|
||||
expect(result.output.split(COMMAND)).toHaveLength(2)
|
||||
current.push(result.ms)
|
||||
}
|
||||
const result = JSON.stringify({ shell, slow, legacy, current })
|
||||
if (process.env.ORCA_STARTUP_BENCH_OUTPUT) {
|
||||
appendFileSync(process.env.ORCA_STARTUP_BENCH_OUTPUT, `${result}\n`)
|
||||
}
|
||||
console.log(result)
|
||||
}
|
||||
}
|
||||
},
|
||||
60_000
|
||||
)
|
||||
})
|
||||
@@ -2,7 +2,6 @@ import { getPosixOmpShellWrapper } from '../pty/omp-shell-wrapper'
|
||||
import { getPosixCodexShellLaunchPreflight } from '../pty/codex-shell-launch-preflight'
|
||||
import { BASH_PROMPT_COMMAND_COMPOSITION_BLOCK } from '../bash-prompt-command-composition'
|
||||
import { BASH_FEATURE_CHANNEL_BLOCK, SHELL_STARTUP_IDENTITY_MARKER_BLOCK } from '../shell-templates'
|
||||
import { SHELL_READY_MARKER } from './daemon-shell-ready-marker'
|
||||
|
||||
export function getDaemonBashShellReadyRcfileContent(): string {
|
||||
return `# Orca daemon bash shell-ready wrapper
|
||||
@@ -56,10 +55,6 @@ __orca_osc133_precmd() {
|
||||
unset __orca_in_command
|
||||
fi
|
||||
printf "\\033]133;A\\007"
|
||||
# Why: emit the shell-ready marker here (not a trailing PROMPT_COMMAND entry)
|
||||
# so a framework that must be last in PROMPT_COMMAND — bash-preexec — is not
|
||||
# displaced by one of Orca's own hooks.
|
||||
[[ -n "$__orca_ready_marker" ]] && printf "${SHELL_READY_MARKER}"
|
||||
return "$exit_code"
|
||||
}
|
||||
__orca_osc133_preexec() {
|
||||
@@ -117,6 +112,11 @@ __orca_osc133_epilogue() {
|
||||
unset __orca_in_prompt_command
|
||||
__orca_adopt_outer_debug_trap
|
||||
trap '__orca_osc133_preexec' DEBUG
|
||||
# Readline renders PS1 after entering raw mode; prompt hooks still run in cooked mode.
|
||||
if [[ -n "$__orca_ready_marker" ]]; then
|
||||
PS1="\${PS1-}"'\\[\\e]777;orca-shell-ready\\a\\]'
|
||||
__orca_ready_marker=""
|
||||
fi
|
||||
}
|
||||
${BASH_PROMPT_COMMAND_COMPOSITION_BLOCK}
|
||||
__orca_prepend_prompt_command "__orca_osc133_precmd"
|
||||
|
||||
@@ -344,35 +344,32 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
itOnPosix('keeps plain Codex startup on the short daemon shell-ready timeout', async () => {
|
||||
await adapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: 'codex',
|
||||
env: { SHELL: '/bin/zsh' }
|
||||
})
|
||||
|
||||
itOnPosix('preserves the existing fast-start timing for fish', async () => {
|
||||
await adapter.spawn({ cols: 80, rows: 24, command: 'codex', env: { SHELL: '/usr/bin/fish' } })
|
||||
await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0)
|
||||
expect(lastSubprocess.write).toHaveBeenCalledWith('codex\n')
|
||||
expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith('codex\n')
|
||||
expect(lastSpawnOpts).not.toEqual(
|
||||
expect.objectContaining({ startupCommandDelivery: 'shell-ready' })
|
||||
)
|
||||
})
|
||||
|
||||
itOnPosix('waits for shell-ready for delivery-hinted Codex startup', async () => {
|
||||
await adapter.spawn({
|
||||
cols: 80,
|
||||
rows: 24,
|
||||
command: "codex 'linked issue context'",
|
||||
startupCommandDelivery: 'shell-ready',
|
||||
env: { SHELL: '/bin/zsh' }
|
||||
})
|
||||
itOnPosix.each([
|
||||
{ command: 'codex' },
|
||||
{ command: 'codex', startupCommandDelivery: 'fast' as const },
|
||||
{ command: "codex 'linked issue context'", startupCommandDelivery: 'shell-ready' as const }
|
||||
])('waits past 300ms and submits once after readiness: %j', async (startup) => {
|
||||
await adapter.spawn({ cols: 80, rows: 24, ...startup, env: { SHELL: '/bin/zsh' } })
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 350))
|
||||
expect(lastSubprocess.write).not.toHaveBeenCalled()
|
||||
|
||||
expect(lastSpawnOpts).toEqual(
|
||||
expect.objectContaining({ startupCommandDelivery: 'shell-ready' })
|
||||
)
|
||||
lastSubprocess._simulateData('\x1b]777;orca-shell-ready\x07')
|
||||
lastSubprocess._simulateData('\r\nuser@host $ ')
|
||||
|
||||
await waitFor(() => vi.mocked(lastSubprocess.write).mock.calls.length > 0)
|
||||
expect(lastSubprocess.write).toHaveBeenCalledWith("codex 'linked issue context'\n")
|
||||
expect(lastSubprocess.write).toHaveBeenCalledExactlyOnceWith(`${startup.command}\n`)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition'
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../shared/codex-startup-delivery'
|
||||
import { CODEX_SHELL_READY_TIMEOUT_MS } from './session-shell-ready-barrier'
|
||||
import type {
|
||||
HistoryRecoveryContext,
|
||||
PendingDaemonSpawnOperation
|
||||
@@ -11,8 +12,11 @@ import { DaemonPtySpawnResult } from './daemon-pty-spawn-result'
|
||||
import type { DaemonPtySpawnContext } from './daemon-pty-spawn-request'
|
||||
import type { ColdRestoreInfo } from './history-reader'
|
||||
import { mintPtySessionId } from './pty-session-id'
|
||||
import { CODEX_SHELL_READY_TIMEOUT_MS } from './session-shell-ready-barrier'
|
||||
import { supportsPtyStartupBarrier } from './shell-ready'
|
||||
import {
|
||||
supportsPtyStartupBarrier,
|
||||
shellReadyMarkerComesFromLineEditor,
|
||||
resolvePtyShellPath
|
||||
} from './shell-ready'
|
||||
import { getRecoveredHistorySeedSegments } from './terminal-history-seed-segments'
|
||||
import { AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION, type CreateOrAttachResult } from './types'
|
||||
import { normalizeWslColdRestoreCwd } from './wsl-cold-restore-cwd'
|
||||
@@ -213,21 +217,23 @@ export abstract class DaemonPtySessionSpawn extends DaemonPtySpawnResult {
|
||||
let effectiveRows = restoreInfo?.rows ?? opts.rows
|
||||
|
||||
const shellReadySupported = opts.command ? supportsPtyStartupBarrier(opts.env ?? {}) : false
|
||||
const isCodexStartupCommand =
|
||||
recognizeAgentProcessFromCommandLine(opts.command)?.agent === 'codex'
|
||||
const shouldWaitForShellReady =
|
||||
isCodexStartupCommand &&
|
||||
shouldUseShellReadyStartupDelivery({
|
||||
const immediateMarker =
|
||||
process.platform !== 'win32' &&
|
||||
shellReadyMarkerComesFromLineEditor(opts.shellOverride || resolvePtyShellPath(opts.env ?? {}))
|
||||
const shellReadyTimeoutMs =
|
||||
shellReadySupported &&
|
||||
!immediateMarker &&
|
||||
recognizeAgentProcessFromCommandLine(opts.command)?.agent === 'codex' &&
|
||||
!shouldUseShellReadyStartupDelivery({
|
||||
command: opts.command,
|
||||
startupCommandDelivery: opts.startupCommandDelivery
|
||||
})
|
||||
const shellReadyTimeoutMs =
|
||||
shellReadySupported && isCodexStartupCommand && !shouldWaitForShellReady
|
||||
? CODEX_SHELL_READY_TIMEOUT_MS
|
||||
: undefined
|
||||
|
||||
const context: DaemonPtySpawnContext = {
|
||||
opts,
|
||||
// Older daemons also need the existing hint to enable their ready marker.
|
||||
opts:
|
||||
opts.command && immediateMarker ? { ...opts, startupCommandDelivery: 'shell-ready' } : opts,
|
||||
operation,
|
||||
historyRecovery,
|
||||
requestedSessionId,
|
||||
|
||||
@@ -20,6 +20,14 @@ describe('PostReadyFlushGate', () => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('flushes synchronously when the marker comes from the line editor', () => {
|
||||
gate = new PostReadyFlushGate(onFlush, true)
|
||||
gate.arm()
|
||||
expect(onFlush).toHaveBeenCalledTimes(1)
|
||||
expect(gate.isPending).toBe(false)
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
})
|
||||
|
||||
it('does not flush immediately when armed', () => {
|
||||
gate.arm()
|
||||
expect(onFlush).not.toHaveBeenCalled()
|
||||
|
||||
@@ -1,23 +1,5 @@
|
||||
/**
|
||||
* Defers a flush callback until after the shell has drawn its prompt and
|
||||
* switched the PTY into raw mode.
|
||||
*
|
||||
* Why: the OSC 777 shell-ready marker fires from zsh's precmd_functions /
|
||||
* bash's PROMPT_COMMAND — before the shell draws its prompt and before
|
||||
* zle/readline flips the PTY into raw mode. Flushing queued input then lets
|
||||
* the kernel (ECHO still on) echo the command once, and the line editor
|
||||
* redraws it under the prompt — producing a visible duplicate (e.g. "claude"
|
||||
* appears twice on agent launch).
|
||||
*
|
||||
* Strategy: after arm() is called, wait for prompt bytes plus a short delay
|
||||
* for the tcsetattr() that enables raw mode. If the marker-completing scan
|
||||
* already saw post-marker bytes, use that same short path immediately.
|
||||
* A conservative wall-clock fallback covers ambiguous marker-only cases.
|
||||
*
|
||||
* Mirrors the gate in local-pty-shell-ready.ts::writeStartupCommandWhenShellReady,
|
||||
* which solves the same race on the non-daemon path.
|
||||
*/
|
||||
|
||||
// Bash's prompt and zsh's line-init marker are ready for input immediately.
|
||||
// Other shells retain the existing settling delay.
|
||||
export const POST_READY_FLUSH_DELAY_MS = 30
|
||||
export const POST_READY_FLUSH_FALLBACK_MS = 200
|
||||
|
||||
@@ -26,7 +8,10 @@ export class PostReadyFlushGate {
|
||||
private postDataTimer: ReturnType<typeof setTimeout> | null = null
|
||||
private fallbackTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
constructor(private readonly onFlush: () => void) {}
|
||||
constructor(
|
||||
private readonly onFlush: () => void,
|
||||
private readonly markerIsLineEditorReady = false
|
||||
) {}
|
||||
|
||||
/** True between arm() and the actual flush firing. Callers should treat
|
||||
* input as still-queued during this window to preserve ordering. */
|
||||
@@ -38,6 +23,10 @@ export class PostReadyFlushGate {
|
||||
* wall-clock fallback unless the marker scan already observed post-marker
|
||||
* bytes, in which case the short post-data settle path is enough. */
|
||||
arm(postMarkerBytesObserved = false): void {
|
||||
if (this.markerIsLineEditorReady) {
|
||||
this.onFlush()
|
||||
return
|
||||
}
|
||||
this.awaitingPromptDraw = true
|
||||
if (postMarkerBytesObserved) {
|
||||
this.notifyData()
|
||||
|
||||
@@ -261,7 +261,7 @@ describe('createPtySubprocess', () => {
|
||||
expect(lastCall[2].env.ORCA_SHELL_FEATURES).not.toContain('ready')
|
||||
})
|
||||
|
||||
it('keeps plain Codex startup commands on the no-marker wrapper', async () => {
|
||||
it('enables readiness and shell identity for plain Codex startup', async () => {
|
||||
const proc = mockPtyProcess()
|
||||
spawnMock.mockReturnValue(proc)
|
||||
const platform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
@@ -285,7 +285,8 @@ describe('createPtySubprocess', () => {
|
||||
const lastCall = spawnMock.mock.calls.at(-1)!
|
||||
expect(lastCall[1]).toEqual(['-l'])
|
||||
expect(lastCall[2].env.ZDOTDIR).toMatch(ZSH_SHELL_READY_DIR)
|
||||
expect(lastCall[2].env.ORCA_SHELL_FEATURES).not.toContain('ready')
|
||||
expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('ready')
|
||||
expect(lastCall[2].env.ORCA_SHELL_FEATURES).toContain('identity')
|
||||
})
|
||||
|
||||
it('uses shell-ready wrapper for delivery-hinted Codex startup commands', async () => {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../../shared/codex-startup-delivery'
|
||||
import { win32 as pathWin32 } from 'node:path'
|
||||
import { isWindowsGitBashShellPath, resolveWindowsGitBashShellPath } from '../../git-bash'
|
||||
import { isPwshAvailable } from '../../pwsh'
|
||||
@@ -32,10 +33,13 @@ import {
|
||||
recognizeAgentProcessFromCommandLine,
|
||||
type RecognizedAgentProcess
|
||||
} from '../../../shared/agent-process-recognition'
|
||||
import { shouldUseShellReadyStartupDelivery } from '../../../shared/codex-startup-delivery'
|
||||
import { ORCA_HERMES_STARTUP_QUERY_ENV } from '../../../shared/hermes-startup-query'
|
||||
import { WINDOWS_GIT_BASH_SHELL } from '../../../shared/windows-terminal-shell'
|
||||
import { getShellLaunchConfig, resolvePtyShellPath } from '../shell-ready'
|
||||
import {
|
||||
getShellLaunchConfig,
|
||||
resolvePtyShellPath,
|
||||
shellReadyMarkerComesFromLineEditor
|
||||
} from '../shell-ready'
|
||||
import { resolveWslSessionContext } from '../wsl-session-context'
|
||||
import { finalizeDaemonPtyEnvironment, rescrubDaemonPtyEnvironment } from './spawn-environment'
|
||||
import type { PtySubprocessOptions } from '../pty-subprocess'
|
||||
@@ -60,7 +64,6 @@ export function createPtyShellLaunchPlan(
|
||||
let startupCommandDeliveredInShellArgs = false
|
||||
let windowsFallbackAttempts: WindowsShellSpawnAttempt[] = []
|
||||
const startupAgentRecognition = recognizeAgentProcessFromCommandLine(opts.command)
|
||||
const isCodexStartupCommand = startupAgentRecognition?.agent === 'codex'
|
||||
const requestedCwd = opts.cwd || resolveSafePtyDefaultCwd()
|
||||
if (opts.command && startupAgentRecognition) {
|
||||
assertSafeAgentStartupCwd(requestedCwd, opts.command)
|
||||
@@ -192,9 +195,10 @@ export function createPtyShellLaunchPlan(
|
||||
}
|
||||
const waitsForShellReady =
|
||||
Boolean(opts.command) &&
|
||||
(!isCodexStartupCommand ||
|
||||
(startupAgentRecognition?.agent !== 'codex' ||
|
||||
shellReadyMarkerComesFromLineEditor(shellPath) ||
|
||||
shouldUseShellReadyStartupDelivery({
|
||||
command: opts.command as string,
|
||||
command: opts.command,
|
||||
startupCommandDelivery: opts.startupCommandDelivery
|
||||
}))
|
||||
delete env.ORCA_SHELL_FEATURES
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { shellReadyMarkerComesFromLineEditor } from './shell-ready'
|
||||
import {
|
||||
installDeviceAttributesResponder,
|
||||
STARTUP_DA1_RESPONSE
|
||||
@@ -19,7 +20,6 @@ import { basename } from 'node:path'
|
||||
import type { ShellReadyState } from './types'
|
||||
|
||||
const SHELL_READY_TIMEOUT_MS = 15_000
|
||||
// Why: Codex skips marker-gated command delivery; this only bounds older daemon/local paths that still report shell-ready for Codex.
|
||||
export const CODEX_SHELL_READY_TIMEOUT_MS = 300
|
||||
|
||||
export type SessionShellReadyBarrierDeps = {
|
||||
@@ -69,7 +69,10 @@ export class SessionShellReadyBarrier {
|
||||
this._state = 'unsupported'
|
||||
}
|
||||
|
||||
this.postReadyFlushGate = new PostReadyFlushGate(() => this.flushPreReadyQueue())
|
||||
this.postReadyFlushGate = new PostReadyFlushGate(
|
||||
() => this.flushPreReadyQueue(),
|
||||
shellReadyMarkerComesFromLineEditor(deps.subprocess.shellPath ?? '')
|
||||
)
|
||||
}
|
||||
|
||||
get state(): ShellReadyState {
|
||||
|
||||
@@ -101,6 +101,11 @@ export function resolvePtyShellPath(env: Record<string, string>): string {
|
||||
return env.SHELL || process.env.SHELL || '/bin/zsh'
|
||||
}
|
||||
|
||||
export function shellReadyMarkerComesFromLineEditor(shellPath: string): boolean {
|
||||
const shellName = pathWin32.basename(basename(shellPath)).toLowerCase()
|
||||
return shellName === 'bash' || shellName === 'zsh'
|
||||
}
|
||||
|
||||
export function shellPathSupportsPtyStartupBarrier(shellPath: string): boolean {
|
||||
const shellName = pathWin32.basename(basename(shellPath)).toLowerCase()
|
||||
// Why fish: markerless, its startup command is written before fish's reader owns
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { spawn, type ChildProcess } from 'node:child_process'
|
||||
import { admitSelfInitiatedTreeKill } from '../../own-chromium-tree-kill-guard'
|
||||
|
||||
const WINDOWS_TREE_KILL_WAIT_MS = 2_000
|
||||
|
||||
@@ -8,6 +9,14 @@ export function killSpawnedCommandTree(child: ChildProcess): Promise<void> {
|
||||
child.kill()
|
||||
return Promise.resolve()
|
||||
}
|
||||
if (
|
||||
!admitSelfInitiatedTreeKill({ pid, site: 'git-command-tree-kill', scope: 'win-taskkill-tree' })
|
||||
) {
|
||||
// Refusal blocks the pid-addressed tree walk, never the termination: the
|
||||
// handle-addressed root kill cannot reach a recycled pid.
|
||||
child.kill()
|
||||
return Promise.resolve()
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
let killer: ChildProcess
|
||||
try {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { dirname } from 'node:path'
|
||||
import { ipcMain } from 'electron'
|
||||
import type { Store } from '../persistence'
|
||||
import { resolveAuthorizedPath } from './filesystem-auth'
|
||||
import { admitSelfInitiatedTreeKill } from '../own-chromium-tree-kill-guard'
|
||||
|
||||
export type NotebookRunResult = {
|
||||
stdout: string
|
||||
@@ -53,7 +54,8 @@ function appendBounded(capture: BoundedCapture, chunk: Buffer): void {
|
||||
capture.truncated = true
|
||||
}
|
||||
|
||||
function terminateNotebookProcessTree(
|
||||
/** Exported for the refusal-fallback test; the timeout path is otherwise unreachable. */
|
||||
export function terminateNotebookProcessTree(
|
||||
child: ChildProcessWithoutNullStreams
|
||||
): ReturnType<typeof setTimeout> | null {
|
||||
if (!child.pid) {
|
||||
@@ -62,6 +64,18 @@ function terminateNotebookProcessTree(
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
if (
|
||||
!admitSelfInitiatedTreeKill({
|
||||
pid: child.pid,
|
||||
site: 'notebook-cell-timeout',
|
||||
scope: 'win-taskkill-tree'
|
||||
})
|
||||
) {
|
||||
// Refusal blocks the tree walk, not the termination: killing the root by
|
||||
// handle cannot reach a recycled pid, and a timed-out cell must still stop.
|
||||
child.kill()
|
||||
return null
|
||||
}
|
||||
try {
|
||||
// Why: a timed-out cell can spawn descendants. taskkill /T is the
|
||||
// Windows equivalent of terminating the whole process group.
|
||||
|
||||
@@ -0,0 +1,184 @@
|
||||
import { readFileSync, readdirSync, statSync } from 'node:fs'
|
||||
import { join, relative, resolve } from 'node:path'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
/**
|
||||
* The ratchet behind the guard's claim to be a choke point.
|
||||
*
|
||||
* `admitSelfInitiatedTreeKill` is only "one decision" for as long as every
|
||||
* pid-addressed `taskkill /pid <pid> /t /f` in Electron main asks it. Each such
|
||||
* kill can land on a recycled pid that is now one of Orca's own Chromium
|
||||
* processes (#10680), and an ungated one is also invisible to
|
||||
* `selfInitiatedTreeKillCount`, which makes a zero read as exculpatory when it
|
||||
* is not. A new family fails here rather than in the field.
|
||||
*
|
||||
* Exactly what is enforced, so no comment elsewhere claims more: per file, the
|
||||
* number of gate admissions must be at least the number of `/pid` call sites.
|
||||
* Counting sites rather than files is the point — a file-granular scan would let
|
||||
* a second, ungated taskkill land inside a family that already mentions the gate,
|
||||
* which is the shape the six highest-risk files now have. What it still cannot
|
||||
* see: a site that pairs an ungated kill with a second admission of an already
|
||||
* gated one in the same file, and a kill whose `/pid` argument is itself built
|
||||
* from a variable.
|
||||
*/
|
||||
const REPOSITORY_ROOT = resolve(__dirname, '..', '..')
|
||||
const MAIN_DIRECTORY = 'src/main/'
|
||||
const SCANNED_EXTENSIONS = ['.ts', '.tsx']
|
||||
const IGNORED_DIRECTORIES = new Set([
|
||||
'node_modules',
|
||||
'dist',
|
||||
'out',
|
||||
'build',
|
||||
'.git',
|
||||
'__fixtures__'
|
||||
])
|
||||
|
||||
/**
|
||||
* One match per call site. Keyed on the `/pid` argument rather than the program
|
||||
* name because `/pid <n>` is what makes the kill pid-addressed — it walks
|
||||
* whatever tree owns that pid *now* — and because the literal survives a
|
||||
* `taskkill` spawned through a constant or a variable, which a quoted-program
|
||||
* pattern misses entirely.
|
||||
*/
|
||||
const PID_ADDRESSED_KILL_SITE = /['"]\/pid['"]/gi
|
||||
|
||||
/**
|
||||
* A call, not an import or a comment: `admitSelfInitiatedTreeKill` in main, and
|
||||
* `admitProcessTreeKill` for the `src/shared` seam main installs the same gate
|
||||
* into, which shared code cannot import directly.
|
||||
*/
|
||||
const GATE_ADMISSION = /\badmit(?:SelfInitiatedTreeKill|ProcessTreeKill)\s*\(/g
|
||||
|
||||
function countMatches(source: string, pattern: RegExp): number {
|
||||
return source.match(pattern)?.length ?? 0
|
||||
}
|
||||
|
||||
/** Sites left over once each admission in the file has claimed one. */
|
||||
function ungatedKillSiteCount(source: string): number {
|
||||
return Math.max(
|
||||
countMatches(source, PID_ADDRESSED_KILL_SITE) - countMatches(source, GATE_ADMISSION),
|
||||
0
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Only ever shrinks. Each entry states why the gate cannot reach it — never
|
||||
* "not got to yet", which is what a new ungated family would also look like.
|
||||
*/
|
||||
const UNGATED_TASKKILL_ALLOWLIST = new Map<string, string>([
|
||||
[
|
||||
'src/main/browser/browser-route-egress-electron-launch.ts',
|
||||
'Electron probe reached only from *.electron.test.ts; kills the probe Electron it spawned'
|
||||
],
|
||||
[
|
||||
'src/main/browser/browser-route-persisted-worker-electron-process.ts',
|
||||
'Electron probe reached only from *.electron.test.ts; kills the probe Electron it spawned'
|
||||
],
|
||||
[
|
||||
'src/cli/handlers/interactive-login-interruption.ts',
|
||||
'CLI host: no Chromium pid on the machine to reach, and no reader for the ring'
|
||||
],
|
||||
[
|
||||
'src/relay/subprocess-tree-termination.ts',
|
||||
'Relay host: same, and the relay cannot import the main-process gate'
|
||||
]
|
||||
])
|
||||
|
||||
function isTestFile(path: string): boolean {
|
||||
return /\.(?:test|spec)\.tsx?$/.test(path) || /(?:test-harness|test-fixture|fixture)/.test(path)
|
||||
}
|
||||
|
||||
function scanSourceFiles(directory: string, found: string[] = []): string[] {
|
||||
for (const entry of readdirSync(directory)) {
|
||||
if (IGNORED_DIRECTORIES.has(entry)) {
|
||||
continue
|
||||
}
|
||||
const path = join(directory, entry)
|
||||
if (statSync(path).isDirectory()) {
|
||||
scanSourceFiles(path, found)
|
||||
continue
|
||||
}
|
||||
if (SCANNED_EXTENSIONS.some((extension) => entry.endsWith(extension)) && !isTestFile(path)) {
|
||||
found.push(path)
|
||||
}
|
||||
}
|
||||
return found
|
||||
}
|
||||
|
||||
// Only the Node-side hosts: a renderer or preload cannot spawn a process at all.
|
||||
const SCANNED_HOSTS = ['src/main', 'src/shared', 'src/cli', 'src/relay']
|
||||
|
||||
/** Scanned once at import: 10k files is seconds, and every case below reuses it. */
|
||||
const PID_ADDRESSED_KILL_FILES = SCANNED_HOSTS.flatMap((host) =>
|
||||
scanSourceFiles(join(REPOSITORY_ROOT, host))
|
||||
.map((path) => ({
|
||||
path: relative(REPOSITORY_ROOT, path).split('\\').join('/'),
|
||||
source: readFileSync(path, 'utf8')
|
||||
}))
|
||||
.filter((file) => countMatches(file.source, PID_ADDRESSED_KILL_SITE) > 0)
|
||||
)
|
||||
|
||||
function pidAddressedKillFiles(): { path: string; source: string }[] {
|
||||
return PID_ADDRESSED_KILL_FILES
|
||||
}
|
||||
|
||||
describe('main-process tree-kill gate', () => {
|
||||
it('finds the taskkill families it is meant to police', () => {
|
||||
// Falsifiable: a scanner that matched nothing would pass every case below.
|
||||
expect(pidAddressedKillFiles().map((file) => file.path)).toContain(
|
||||
'src/main/windows-process-tree-kill.ts'
|
||||
)
|
||||
})
|
||||
|
||||
it('routes every pid-addressed taskkill in Electron main through the gate', () => {
|
||||
const ungated = pidAddressedKillFiles()
|
||||
.filter((file) => file.path.startsWith(MAIN_DIRECTORY))
|
||||
.filter((file) => ungatedKillSiteCount(file.source) > 0)
|
||||
.map((file) => file.path)
|
||||
.filter((path) => !UNGATED_TASKKILL_ALLOWLIST.has(path))
|
||||
|
||||
expect(ungated).toEqual([])
|
||||
})
|
||||
|
||||
it('leaves no pid-addressed taskkill outside main unaccounted for', () => {
|
||||
const unaccounted = pidAddressedKillFiles()
|
||||
.filter((file) => !file.path.startsWith(MAIN_DIRECTORY))
|
||||
.filter((file) => ungatedKillSiteCount(file.source) > 0)
|
||||
.map((file) => file.path)
|
||||
.filter((path) => !UNGATED_TASKKILL_ALLOWLIST.has(path))
|
||||
|
||||
expect(unaccounted).toEqual([])
|
||||
})
|
||||
|
||||
it('counts call sites, not files: a second ungated kill in a gated file is caught', () => {
|
||||
// The failure a file-granular scan let through: one gate mention exempting
|
||||
// every taskkill in the file.
|
||||
const gated = `
|
||||
import { admitSelfInitiatedTreeKill } from './own-chromium-tree-kill-guard'
|
||||
if (admitSelfInitiatedTreeKill({ pid, site: 's', scope: 'win-taskkill-tree' })) {
|
||||
spawn('taskkill', ['/pid', String(pid), '/t', '/f'])
|
||||
}
|
||||
`
|
||||
|
||||
expect(ungatedKillSiteCount(gated)).toBe(0)
|
||||
expect(
|
||||
ungatedKillSiteCount(`${gated}\nspawn('taskkill', ['/pid', String(other), '/t', '/f'])`)
|
||||
).toBe(1)
|
||||
})
|
||||
|
||||
it('sees a kill whose program name comes from a constant', () => {
|
||||
// A quoted-program pattern misses this shape; the `/pid` argument does not.
|
||||
expect(
|
||||
ungatedKillSiteCount(`
|
||||
const KILLER = 'taskkill'
|
||||
spawn(KILLER, ['/pid', String(pid), '/t', '/f'])
|
||||
`)
|
||||
).toBe(1)
|
||||
})
|
||||
|
||||
it('keeps the allowlist honest: every entry still spawns a taskkill', () => {
|
||||
const spawning = new Set(pidAddressedKillFiles().map((file) => file.path))
|
||||
|
||||
expect([...UNGATED_TASKKILL_ALLOWLIST.keys()].filter((path) => !spawning.has(path))).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -12,6 +12,12 @@ import { recordCoalescedDurableCrashBreadcrumb } from './crash-reporting/durable
|
||||
* Empty on a Node host and empty on failure: that is "no refusal proven", never
|
||||
* "safe to kill" — callers must keep every other guard they already have.
|
||||
*
|
||||
* The other direction is real too, and bounded by design: `getAppMetrics()` can
|
||||
* still list a renderer Electron has not finished reaping, so on Windows a pid
|
||||
* already recycled onto an unrelated child of ours reads as `own` and its tree
|
||||
* walk is refused. That is why a refusal only blocks the pid-addressed walk and
|
||||
* every gated site still kills its own root through the child handle.
|
||||
*
|
||||
* Why failure stays open rather than refusing everything: a refusal is not free.
|
||||
* `terminateWindowsProcessTree` resolves without killing, and
|
||||
* `killSourceControlAgentProcess` returns that straight to a caller that then
|
||||
|
||||
@@ -13,7 +13,12 @@ import {
|
||||
import { readOrcaChromiumProcessPids } from './orca-chromium-process-pids'
|
||||
import { classifyWindowsTreeKillTarget } from './windows-pty-root-identity'
|
||||
import { terminateWindowsProcessTree } from './windows-process-tree-kill'
|
||||
import { admitSelfInitiatedTreeKill } from './own-chromium-tree-kill-guard'
|
||||
import {
|
||||
admitSelfInitiatedTreeKill,
|
||||
installMainProcessTreeKillGate
|
||||
} from './own-chromium-tree-kill-guard'
|
||||
import { killCodexAppServerProcessTree } from './codex/codex-app-server-session'
|
||||
import { setProcessTreeKillGate } from '../shared/child-process/process-tree-kill-gate'
|
||||
import { resetSelfInitiatedTreeKillLogForTest } from './crash-reporting/self-initiated-tree-kill-log'
|
||||
import {
|
||||
clearCrashBreadcrumbsForTest,
|
||||
@@ -57,6 +62,7 @@ beforeEach(() => {
|
||||
setActiveSink({ push: () => {}, flush: () => {}, close: () => {} })
|
||||
clearCrashBreadcrumbsForTest()
|
||||
resetSelfInitiatedTreeKillLogForTest()
|
||||
installMainProcessTreeKillGate()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -66,6 +72,7 @@ afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
_resetTracerForTests()
|
||||
clearCrashBreadcrumbsForTest()
|
||||
setProcessTreeKillGate(null)
|
||||
})
|
||||
|
||||
describe('refusing to tree-kill our own Chromium processes', () => {
|
||||
@@ -143,6 +150,38 @@ describe('refusing to tree-kill our own Chromium processes', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses the codex app-server deadline kill against one of our own pids', () => {
|
||||
const spawnImpl = vi.fn(() => ({ on: vi.fn(), unref: vi.fn() }))
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
killCodexAppServerProcessTree(child as never, {
|
||||
platform: 'win32',
|
||||
spawnImpl: spawnImpl as never
|
||||
})
|
||||
|
||||
// The deadline timer fires on `child.pid` alone; a reaped-then-recycled pid
|
||||
// is the stale-pid mechanism this gate exists to stop.
|
||||
expect(spawnImpl).not.toHaveBeenCalled()
|
||||
expect(getCrashBreadcrumbSnapshot()).toEqual([
|
||||
expect.objectContaining({ name: 'self_tree_kill_refused_own_chromium' })
|
||||
])
|
||||
})
|
||||
|
||||
it('still lets the codex app-server deadline kill reach a foreign pid', () => {
|
||||
const killer = { on: vi.fn(), unref: vi.fn() }
|
||||
const spawnImpl = vi.fn(() => killer)
|
||||
|
||||
killCodexAppServerProcessTree({ pid: 7777, kill: vi.fn() } as never, {
|
||||
platform: 'win32',
|
||||
spawnImpl: spawnImpl as never
|
||||
})
|
||||
|
||||
expect(spawnImpl).toHaveBeenCalledWith('taskkill', ['/pid', '7777', '/t', '/f'], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Fail-open is the deliberate choice — see `orca-chromium-process-pids.ts` for
|
||||
* why refusing everything is worse — so the crumb is the only thing that keeps
|
||||
|
||||
@@ -4,16 +4,23 @@ import {
|
||||
type SelfInitiatedTreeKillScope
|
||||
} from './crash-reporting/self-initiated-tree-kill-log'
|
||||
import { readOrcaChromiumProcessPids } from './orca-chromium-process-pids'
|
||||
import { setProcessTreeKillGate } from '../shared/child-process/process-tree-kill-gate'
|
||||
|
||||
/**
|
||||
* Gate every main-process tree-kill through one decision: refuse the pid when
|
||||
* Electron is currently accounting for it, otherwise put it on the record.
|
||||
* Gate every main-process tree-kill through one decision: refuse a pid-addressed
|
||||
* walk when Electron is currently accounting for the pid, otherwise put the
|
||||
* kill on the record.
|
||||
*
|
||||
* Why a shared gate rather than a check inside `terminateWindowsProcessTree`:
|
||||
* the codex and claude account-login teardowns run their own `taskkill /T /F`
|
||||
* with different lifetimes (one sync, one with its own timeout ladder), so a
|
||||
* guard that only lived in the tree-kill helper would cover one of three
|
||||
* families. Returns false when the caller must not kill.
|
||||
* five other families in main run their own `taskkill /T /F` with different
|
||||
* lifetimes (sync, fire-and-forget, timeout ladder), and three more live in
|
||||
* `src/shared` and reach this through `process-tree-kill-gate`, so a guard that
|
||||
* only lived in the tree-kill helper would cover one of nine.
|
||||
* `main-process-tree-kill-gate.test.ts` holds that set closed by counting `/pid`
|
||||
* call sites against gate admissions per file, not by file. Returns false
|
||||
* when the caller must not walk that pid's tree; the caller still kills its own
|
||||
* root through the child handle (`refused-tree-kill-root-termination.test.ts`),
|
||||
* so a refusal is never a process leak.
|
||||
*
|
||||
* Electron main only, by construction. `terminateWindowsProcessTree` also runs
|
||||
* in the standalone daemon (the `pty-descendant-sweep` site), where
|
||||
@@ -30,11 +37,26 @@ export function admitSelfInitiatedTreeKill(target: {
|
||||
}): boolean {
|
||||
// Why: no PTY root, codex root or git child is ever one of our own Chromium
|
||||
// processes, so a pid that is means the caller is about to kill a renderer,
|
||||
// the GPU or the browser itself (#10680).
|
||||
if (readOrcaChromiumProcessPids().has(target.pid)) {
|
||||
recordRefusedOwnChromiumTreeKill(target)
|
||||
return false
|
||||
// the GPU or the browser itself (#10680). Only the pid-addressed scope can
|
||||
// land there: a POSIX group holds only what Orca put in it, so that arm is
|
||||
// recorded and admitted like every other group kill in main, and a stale
|
||||
// `getAppMetrics()` entry cannot orphan a macOS/Linux tree.
|
||||
const isOwnChromiumPid =
|
||||
target.scope === 'win-taskkill-tree' && readOrcaChromiumProcessPids().has(target.pid)
|
||||
try {
|
||||
if (isOwnChromiumPid) {
|
||||
recordRefusedOwnChromiumTreeKill(target)
|
||||
} else {
|
||||
recordSelfInitiatedTreeKill(target)
|
||||
}
|
||||
} catch {
|
||||
// Recording must never turn a successful termination into a failed one, and
|
||||
// never flip the decision: it is taken above, before anything can throw.
|
||||
}
|
||||
recordSelfInitiatedTreeKill(target)
|
||||
return true
|
||||
return !isOwnChromiumPid
|
||||
}
|
||||
|
||||
/** Hands the gate to the shared choke points, which cannot import main. */
|
||||
export function installMainProcessTreeKillGate(): void {
|
||||
setProcessTreeKillGate((kill) => admitSelfInitiatedTreeKill(kill))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,220 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { spawnMock, execFileMock, queryWindowsProcessDescendantsMock } = vi.hoisted(() => ({
|
||||
spawnMock: vi.fn(),
|
||||
execFileMock: vi.fn(),
|
||||
queryWindowsProcessDescendantsMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('node:child_process', async (importOriginal) => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
spawn: spawnMock,
|
||||
execFile: execFileMock
|
||||
}))
|
||||
vi.mock('electron', () => ({ ipcMain: { handle: vi.fn(), on: vi.fn() } }))
|
||||
vi.mock('./providers/windows-foreground-process-rows', () => ({
|
||||
queryWindowsProcessDescendants: queryWindowsProcessDescendantsMock
|
||||
}))
|
||||
|
||||
import {
|
||||
getAppEnvironment,
|
||||
hasAppEnvironment,
|
||||
setAppEnvironment,
|
||||
type AppEnvironment
|
||||
} from '../shared/app-environment'
|
||||
import { installMainProcessTreeKillGate } from './own-chromium-tree-kill-guard'
|
||||
import { setProcessTreeKillGate } from '../shared/child-process/process-tree-kill-gate'
|
||||
import { resetSelfInitiatedTreeKillLogForTest } from './crash-reporting/self-initiated-tree-kill-log'
|
||||
import {
|
||||
clearCrashBreadcrumbsForTest,
|
||||
getCrashBreadcrumbSnapshot
|
||||
} from './crash-reporting/crash-breadcrumb-store'
|
||||
import { _resetTracerForTests, setActiveSink } from './observability/tracer'
|
||||
import { terminateNotebookProcessTree } from './ipc/notebook'
|
||||
import { killLocalPrecheckProcessTree } from './automations/precheck-runner'
|
||||
import { killRecipeProcess } from '../shared/ephemeral-vm-recipe-process'
|
||||
import { killSpawnedCommandTree } from './git/command-runner/spawned-command-tree-kill'
|
||||
import { killCodexAppServerProcessTree } from './codex/codex-app-server-session'
|
||||
import { signalProcessTree } from '../shared/child-process/process-tree-termination'
|
||||
import { killSourceControlAgentProcess } from './text-generation/source-control-local-process'
|
||||
import { terminateCodexTurnProcesses } from './codex/codex-structured-turn-processes'
|
||||
|
||||
/** A pid Electron reports as one of ours: every gate below must refuse it. */
|
||||
const RENDERER_PID = 1001
|
||||
|
||||
function appEnvironment(): AppEnvironment {
|
||||
return {
|
||||
getPath: () => process.cwd(),
|
||||
getAppPath: () => process.cwd(),
|
||||
getVersion: () => '0.0.0-test',
|
||||
isPackaged: () => false,
|
||||
onWillQuit: () => {},
|
||||
exit: () => {},
|
||||
getAppMetrics: (() => [
|
||||
{ pid: RENDERER_PID, type: 'Tab' }
|
||||
]) as unknown as AppEnvironment['getAppMetrics']
|
||||
}
|
||||
}
|
||||
|
||||
let previousEnvironment: AppEnvironment | null = null
|
||||
let previousPlatform: PropertyDescriptor | undefined
|
||||
|
||||
function setPlatform(platform: NodeJS.Platform): void {
|
||||
Object.defineProperty(process, 'platform', { value: platform, configurable: true })
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
previousEnvironment = hasAppEnvironment() ? getAppEnvironment() : null
|
||||
previousPlatform = Object.getOwnPropertyDescriptor(process, 'platform')
|
||||
setAppEnvironment(appEnvironment())
|
||||
setActiveSink(null)
|
||||
clearCrashBreadcrumbsForTest()
|
||||
resetSelfInitiatedTreeKillLogForTest()
|
||||
installMainProcessTreeKillGate()
|
||||
spawnMock.mockReset()
|
||||
execFileMock.mockReset()
|
||||
queryWindowsProcessDescendantsMock.mockReset()
|
||||
spawnMock.mockReturnValue({ on: vi.fn(), once: vi.fn(), unref: vi.fn(), kill: vi.fn() })
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
setProcessTreeKillGate(null)
|
||||
if (previousPlatform) {
|
||||
Object.defineProperty(process, 'platform', previousPlatform)
|
||||
}
|
||||
if (previousEnvironment) {
|
||||
setAppEnvironment(previousEnvironment)
|
||||
}
|
||||
_resetTracerForTests()
|
||||
})
|
||||
|
||||
/**
|
||||
* A refusal must never become a process leak. The gate only blocks the
|
||||
* pid-addressed tree walk; the root kill is addressed by the child handle, so it
|
||||
* cannot reach the recycled pid we refused, and skipping it would report a
|
||||
* timed-out command as stopped while its tree keeps running.
|
||||
*/
|
||||
describe('a refused tree-kill still terminates the root it owns', () => {
|
||||
it('kills the git command root when the tree walk is refused', async () => {
|
||||
setPlatform('win32')
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
await killSpawnedCommandTree(child as never)
|
||||
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('kills the notebook cell root when the tree walk is refused', () => {
|
||||
setPlatform('win32')
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
expect(terminateNotebookProcessTree(child as never)).toBeNull()
|
||||
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('kills the automation precheck root when the tree walk is refused', () => {
|
||||
setPlatform('win32')
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
expect(killLocalPrecheckProcessTree(child as never)).toBeNull()
|
||||
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('kills the ephemeral-VM recipe root when the tree walk is refused', () => {
|
||||
setPlatform('win32')
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
killRecipeProcess(child as never, true)
|
||||
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('kills the codex app-server root when the deadline tree walk is refused', () => {
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
killCodexAppServerProcessTree(child as never, {
|
||||
platform: 'win32',
|
||||
spawnImpl: spawnMock as never
|
||||
})
|
||||
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('kills the commit-message agent root when the tree walk is refused', async () => {
|
||||
setPlatform('win32')
|
||||
const child = { pid: RENDERER_PID, kill: vi.fn() }
|
||||
|
||||
await killSourceControlAgentProcess(child as never)
|
||||
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('kills the runProcess root when the Windows arm of the shared choke point is refused', async () => {
|
||||
setPlatform('win32')
|
||||
const windowsChild = { pid: RENDERER_PID, kill: vi.fn(), exitCode: null, signalCode: null }
|
||||
|
||||
await expect(signalProcessTree(windowsChild as never, 'SIGKILL')).resolves.toBe(false)
|
||||
expect(spawnMock).not.toHaveBeenCalled()
|
||||
expect(windowsChild.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
})
|
||||
|
||||
it('still signals the POSIX process group: a group only holds what Orca put in it', async () => {
|
||||
// Same contract as main and as the other three POSIX group arms in main
|
||||
// (claude-login, codex teardown, PTY sweep): record, never refuse. A stale
|
||||
// `getAppMetrics()` entry must not orphan a macOS/Linux tree.
|
||||
setPlatform('linux')
|
||||
const posixChild = { pid: RENDERER_PID, kill: vi.fn(), exitCode: null, signalCode: null }
|
||||
const processKill = vi.spyOn(process, 'kill').mockImplementation(() => true)
|
||||
|
||||
await expect(signalProcessTree(posixChild as never, 'SIGKILL')).resolves.toBe(true)
|
||||
expect(processKill).toHaveBeenCalledWith(-RENDERER_PID, 'SIGKILL')
|
||||
expect(posixChild.kill).not.toHaveBeenCalled()
|
||||
expect(getCrashBreadcrumbSnapshot()).toEqual([
|
||||
expect.objectContaining({
|
||||
name: 'self_tree_kill',
|
||||
data: expect.objectContaining({ pid: RENDERER_PID, scope: 'posix-process-group' })
|
||||
})
|
||||
])
|
||||
processKill.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* The one gated site with nothing to fall back to: the roots it kills are found
|
||||
* by a process-table walk, not spawned here, so there is no child handle. A
|
||||
* refusal must then be visible — the refusal crumb is written and the turn is
|
||||
* reported as not cancelled — rather than resolving as if the tree had gone.
|
||||
*/
|
||||
describe('a refused tree-kill with no handle to fall back to', () => {
|
||||
it('reports the codex turn as not cancelled and records the refused added root', async () => {
|
||||
const appServerPid = 500
|
||||
const addedRoot = {
|
||||
pid: RENDERER_PID,
|
||||
ppid: appServerPid,
|
||||
name: 'node.exe',
|
||||
command: 'node',
|
||||
depth: 1
|
||||
}
|
||||
queryWindowsProcessDescendantsMock.mockResolvedValue([addedRoot])
|
||||
|
||||
await expect(
|
||||
terminateCodexTurnProcesses(appServerPid, { platform: 'win32', identities: new Map() })
|
||||
).resolves.toBe(false)
|
||||
|
||||
expect(execFileMock).not.toHaveBeenCalled()
|
||||
expect(getCrashBreadcrumbSnapshot()).toEqual([
|
||||
expect.objectContaining({
|
||||
name: 'self_tree_kill_refused_own_chromium',
|
||||
data: expect.objectContaining({ pid: RENDERER_PID, site: 'codex-turn-added-roots' })
|
||||
})
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -3,6 +3,8 @@ import { OrcaRuntimeWithStopStructuredSessionProcess } from './orca-runtime-stop
|
||||
import type { AgentSessionOwnerBinding } from '../../shared/agent-session-host-authority'
|
||||
import { agentSessionOwnerBindingsEqual } from '../../shared/claimed-agent-pty-owner-snapshot'
|
||||
import { resolvePinnedCodexRolloutProof } from '../codex/codex-tui-rollout-proof'
|
||||
import { supportsCodexStructuredLocation } from '../codex/codex-structured-location-support'
|
||||
import { supportsClaudeStructuredLocation } from '../claude/claude-structured-location-support'
|
||||
import { getStructuredAgentSessionHost } from '../native-chat/agent-session-wire/structured-agent-session-registry'
|
||||
import { resolveStructuredAgentSessionCreateSupport } from '../native-chat/structured-agent-session-create-support'
|
||||
import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
|
||||
@@ -51,13 +53,13 @@ export class OrcaRuntimeWithResolveRecoveredStructuredTuiTranscript extends Orca
|
||||
agent: 'claude' | 'codex'
|
||||
): Promise<{ supported: boolean; reason?: 'agent' | 'remote' | 'wsl' }> {
|
||||
const location = await this.resolveStructuredAgentSessionLocation(worktreeSelector)
|
||||
await this.ensureStructuredAgentSessionHost()
|
||||
// The verdict lives in a typechecked module; this file is @ts-nocheck.
|
||||
return resolveStructuredAgentSessionCreateSupport({
|
||||
agent,
|
||||
location,
|
||||
adapterSupportsCreate:
|
||||
getStructuredAgentSessionHost()?.supportsCreate(location, agent) === true,
|
||||
agent === 'claude'
|
||||
? supportsClaudeStructuredLocation(location)
|
||||
: supportsCodexStructuredLocation(location),
|
||||
getSettings: () => this.requireStore().getSettings()
|
||||
})
|
||||
}
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { OrcaRuntimeService } from './orca-runtime'
|
||||
import {
|
||||
getStructuredAgentSessionHost,
|
||||
setStructuredAgentSessionHost
|
||||
} from '../native-chat/agent-session-wire/structured-agent-session-registry'
|
||||
import { agentSessionPtyWriteGate } from './agent-session-pty-write-gate'
|
||||
|
||||
type InstallEffects = {
|
||||
storeOpened: boolean
|
||||
writeGateAttached: boolean
|
||||
reaperStarted: boolean
|
||||
}
|
||||
|
||||
/** Stands in for `install()` by performing the three effects it performs, so a probe that
|
||||
* reinstalls the host is caught by what the install *does*, not by a call count alone. */
|
||||
function stubStructuredHostInstall(runtime: OrcaRuntimeService): {
|
||||
effects: InstallEffects
|
||||
ensure: ReturnType<typeof vi.fn>
|
||||
} {
|
||||
const effects: InstallEffects = {
|
||||
storeOpened: false,
|
||||
writeGateAttached: false,
|
||||
reaperStarted: false
|
||||
}
|
||||
// `supportsCreate` answers as the real Codex adapter would, so a probe that reinstalls the host
|
||||
// still returns the right answer and fails on the install effects alone.
|
||||
const host = {
|
||||
reconcileRestartLeases: vi.fn(async () => {}),
|
||||
supportsCreate: (location: { executionHostId: string; wslDistro: string | null }) =>
|
||||
location.executionHostId === 'local' && location.wslDistro === null
|
||||
}
|
||||
const ensure = vi.fn(async () => {
|
||||
effects.storeOpened = true
|
||||
effects.reaperStarted = true
|
||||
agentSessionPtyWriteGate.attachRecordLookup(() => null)
|
||||
effects.writeGateAttached = true
|
||||
setStructuredAgentSessionHost(host as never)
|
||||
})
|
||||
vi.spyOn(runtime, 'ensureStructuredAgentSessionHost').mockImplementation(ensure)
|
||||
return { effects, ensure }
|
||||
}
|
||||
|
||||
type TestLocation = {
|
||||
executionHostId: string
|
||||
wslDistro: string | null
|
||||
workspaceKind?: 'folder' | 'git-worktree'
|
||||
}
|
||||
|
||||
type SupportResult = {
|
||||
supported: boolean
|
||||
reason?: 'agent' | 'remote' | 'wsl'
|
||||
}
|
||||
|
||||
function createRuntime(location: TestLocation): OrcaRuntimeService {
|
||||
const runtime = new OrcaRuntimeService({ getSettings: () => ({}) } as never)
|
||||
const internal = runtime as unknown as {
|
||||
resolveStructuredAgentSessionLocation: () => Promise<unknown>
|
||||
}
|
||||
internal.resolveStructuredAgentSessionLocation = vi.fn(async () => ({
|
||||
executionHostId: location.executionHostId,
|
||||
wslDistro: location.wslDistro,
|
||||
workspaceId: 'workspace-1',
|
||||
workspaceKind: location.workspaceKind ?? 'git-worktree'
|
||||
}))
|
||||
return runtime
|
||||
}
|
||||
|
||||
async function expectSupportWithoutInstall(input: {
|
||||
agent: 'claude' | 'codex'
|
||||
location: TestLocation
|
||||
expected: SupportResult
|
||||
repetitions?: number
|
||||
}): Promise<void> {
|
||||
const runtime = createRuntime(input.location)
|
||||
const { effects, ensure } = stubStructuredHostInstall(runtime)
|
||||
|
||||
const answers: SupportResult[] = []
|
||||
for (let index = 0; index < (input.repetitions ?? 1); index += 1) {
|
||||
answers.push(
|
||||
await runtime.getStructuredAgentSessionCreateSupport('id:workspace-1', input.agent)
|
||||
)
|
||||
}
|
||||
|
||||
expect(answers).toEqual(Array(input.repetitions ?? 1).fill(input.expected))
|
||||
expect(ensure).not.toHaveBeenCalled()
|
||||
expect(effects).toEqual({
|
||||
storeOpened: false,
|
||||
writeGateAttached: false,
|
||||
reaperStarted: false
|
||||
})
|
||||
expect(getStructuredAgentSessionHost()).toBeNull()
|
||||
}
|
||||
|
||||
describe('structured agent-session create-support probe', () => {
|
||||
afterEach(() => {
|
||||
setStructuredAgentSessionHost(null)
|
||||
agentSessionPtyWriteGate.detachRecordLookup()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it.each(['codex', 'claude'] as const)(
|
||||
'answers %s support repeatedly without installing the host',
|
||||
async (agent) => {
|
||||
await expectSupportWithoutInstall({
|
||||
agent,
|
||||
location: { executionHostId: 'local', wslDistro: null },
|
||||
expected: { supported: true },
|
||||
repetitions: 3
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it.each(['codex', 'claude'] as const)(
|
||||
'still reports an unsupported remote %s location without installing the host',
|
||||
async (agent) => {
|
||||
await expectSupportWithoutInstall({
|
||||
agent,
|
||||
location: { executionHostId: 'ssh-host-1', wslDistro: null },
|
||||
expected: { supported: false, reason: 'remote' }
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it.each(['codex', 'claude'] as const)(
|
||||
'still reports an unsupported WSL %s location without installing the host',
|
||||
async (agent) => {
|
||||
await expectSupportWithoutInstall({
|
||||
agent,
|
||||
location: { executionHostId: 'local', wslDistro: 'Ubuntu' },
|
||||
expected: { supported: false, reason: 'wsl' }
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it.each(['codex', 'claude'] as const)(
|
||||
'supports a local folder workspace for %s without installing the host',
|
||||
async (agent) => {
|
||||
await expectSupportWithoutInstall({
|
||||
agent,
|
||||
location: {
|
||||
executionHostId: 'local',
|
||||
wslDistro: null,
|
||||
workspaceKind: 'folder'
|
||||
},
|
||||
expected: { supported: true }
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
it('still installs and reconciles on startup when a store is already persisted', async () => {
|
||||
const runtime = createRuntime({ executionHostId: 'local', wslDistro: null })
|
||||
const { effects, ensure } = stubStructuredHostInstall(runtime)
|
||||
const internal = runtime as unknown as {
|
||||
hasPersistedStructuredAgentSessionStore: () => boolean
|
||||
refreshMobileSessionPtyRecords: () => Promise<void>
|
||||
}
|
||||
internal.hasPersistedStructuredAgentSessionStore = () => true
|
||||
internal.refreshMobileSessionPtyRecords = vi.fn(async () => {})
|
||||
|
||||
await runtime.prepareStructuredAgentSessionStartupRestoration()
|
||||
|
||||
expect(ensure).toHaveBeenCalledTimes(1)
|
||||
expect(effects).toEqual({
|
||||
storeOpened: true,
|
||||
writeGateAttached: true,
|
||||
reaperStarted: true
|
||||
})
|
||||
expect(
|
||||
(getStructuredAgentSessionHost() as unknown as { reconcileRestartLeases: () => void })
|
||||
.reconcileRestartLeases
|
||||
).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
@@ -73,6 +73,7 @@ const CONTRACT_GLOBALS = new Set([
|
||||
'OPENCODE_CONFIG_DIR',
|
||||
'PATH',
|
||||
'PROMPT_COMMAND',
|
||||
'PS1', // Bash appends its non-printing Readline readiness marker.
|
||||
'CURSOR',
|
||||
'ZDOTDIR',
|
||||
'precmd_functions',
|
||||
|
||||
@@ -48,7 +48,7 @@ import {
|
||||
} from './single-instance-lock'
|
||||
import { setAppEnvironment } from '../../shared/app-environment'
|
||||
import { ElectronAppEnvironment } from '../host/electron-app-environment'
|
||||
import { installProcessTreeKillBreadcrumbObserver } from '../crash-reporting/self-initiated-tree-kill-log'
|
||||
import { installMainProcessTreeKillGate } from '../own-chromium-tree-kill-guard'
|
||||
import { setSecretStore } from '../../shared/secret-store'
|
||||
import { ElectronSecretStore } from '../host/electron-secret-store'
|
||||
import { setPtyHostBindings } from '../ipc/pty-host-bindings'
|
||||
@@ -163,8 +163,8 @@ export function runMainProcessPreflight(options: MainProcessPreflightOptions): b
|
||||
})
|
||||
}
|
||||
// Why before any spawn: `signalProcessTree` is shared with the CLI and relay, so
|
||||
// it can only reach the main-process breadcrumb store through a registered observer.
|
||||
installProcessTreeKillBreadcrumbObserver()
|
||||
// it can only reach the main-process guard and breadcrumb store once this is registered.
|
||||
installMainProcessTreeKillGate()
|
||||
const isDev = is.dev
|
||||
configureDevUserDataPath(isDev)
|
||||
configureOrcaUserDataPathEnv()
|
||||
|
||||
@@ -101,7 +101,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
|
||||
cancelGenerateCommitMessageLocal('/repo')
|
||||
|
||||
expectChildTerminated(children[0]!)
|
||||
await expectChildTerminated(children[0]!)
|
||||
expect(children[1]?.kill).not.toHaveBeenCalled()
|
||||
|
||||
children[0]?.listeners.get('close')?.(null)
|
||||
@@ -192,7 +192,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
cancelGeneratePullRequestFieldsLocal('/repo')
|
||||
|
||||
expect(children[0]?.kill).not.toHaveBeenCalled()
|
||||
expectChildTerminated(children[1]!)
|
||||
await expectChildTerminated(children[1]!)
|
||||
|
||||
const commitStdout = children[0]?.listeners.get('stdout:data')
|
||||
commitStdout?.(Buffer.from('Update README\n'))
|
||||
@@ -250,7 +250,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
cancelGeneratePullRequestFieldsLocal('/repo')
|
||||
listeners.get('close')?.(null)
|
||||
|
||||
expectChildTerminated(child)
|
||||
await expectChildTerminated(child)
|
||||
await expect(pullRequest).resolves.toEqual({
|
||||
success: false,
|
||||
error: 'Generation canceled.',
|
||||
@@ -306,7 +306,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
)
|
||||
|
||||
cancelGenerateCommitMessageLocal('/repo')
|
||||
expectChildTerminated(child)
|
||||
await expectChildTerminated(child)
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
@@ -344,7 +344,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
error: 'Generation canceled.',
|
||||
canceled: true
|
||||
})
|
||||
expectChildTerminated(firstChild)
|
||||
await expectChildTerminated(firstChild)
|
||||
|
||||
const second = generateCommitMessageFromContext(context, params, {
|
||||
kind: 'local',
|
||||
@@ -377,7 +377,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(1))
|
||||
cancelGenerateCommitMessageLocal('/descendant-repo')
|
||||
await expect(first).resolves.toMatchObject({ canceled: true })
|
||||
expectChildTerminated(firstChild)
|
||||
await expectChildTerminated(firstChild)
|
||||
|
||||
// SIGKILL reaches the codex process but not a grandchild that inherited its
|
||||
// stdout, so 'exit' arrives and 'close' never does.
|
||||
|
||||
@@ -71,7 +71,7 @@ describe('generateCommitMessageFromContext', () => {
|
||||
error:
|
||||
'agent CLI command produced too much output. Check the agent CLI configuration and try again.'
|
||||
})
|
||||
expectChildTerminated(child)
|
||||
await expectChildTerminated(child)
|
||||
})
|
||||
|
||||
it('passes prepared provider environment to local agent subprocesses', async () => {
|
||||
|
||||
@@ -325,7 +325,7 @@ describe('discoverCommitMessageModelsLocal', () => {
|
||||
await vi.advanceTimersByTimeAsync(60_000)
|
||||
|
||||
await assertion
|
||||
expectChildTerminated(child)
|
||||
await expectChildTerminated(child)
|
||||
expect(child.stdout.listenerCount('data')).toBe(0)
|
||||
expect(child.stderr.listenerCount('data')).toBe(0)
|
||||
expect(child.listenerCount('error')).toBe(0)
|
||||
@@ -352,7 +352,7 @@ describe('discoverCommitMessageModelsLocal', () => {
|
||||
success: false,
|
||||
error: 'Codex model discovery timed out after 60s.'
|
||||
})
|
||||
expectChildTerminated(firstChild)
|
||||
await expectChildTerminated(firstChild)
|
||||
expect(spawnMock).toHaveBeenCalledTimes(1)
|
||||
|
||||
firstChild.emit('close', null)
|
||||
@@ -413,7 +413,7 @@ describe('discoverCommitMessageModelsLocal', () => {
|
||||
success: false,
|
||||
error: 'Cursor returned too much model data.'
|
||||
})
|
||||
expectChildTerminated(child)
|
||||
await expectChildTerminated(child)
|
||||
expect(child.stdout.listenerCount('data')).toBe(0)
|
||||
expect(child.stderr.listenerCount('data')).toBe(0)
|
||||
expect(child.listenerCount('error')).toBe(0)
|
||||
|
||||
@@ -33,15 +33,17 @@ export function withPlatform<T>(platform: NodeJS.Platform, fn: () => T): T {
|
||||
// expectChildTerminated(child) with no extra argument.
|
||||
export function createChildTerminationExpectation(
|
||||
terminateWindowsProcessTreeMock: ReturnType<typeof vi.fn>
|
||||
): (child: { pid: number; kill: ReturnType<typeof vi.fn> }) => void {
|
||||
return (child) => {
|
||||
): (child: { pid: number; kill: ReturnType<typeof vi.fn> }) => Promise<void> {
|
||||
return async (child) => {
|
||||
if (process.platform === 'win32') {
|
||||
expect(terminateWindowsProcessTreeMock).toHaveBeenCalledWith(child.pid, {
|
||||
site: 'source-control-text-generation'
|
||||
})
|
||||
expect(child.kill).not.toHaveBeenCalled()
|
||||
return
|
||||
}
|
||||
expect(child.kill).toHaveBeenCalledWith('SIGKILL')
|
||||
// Every platform kills the root by its own handle. On win32 that is not a
|
||||
// duplicate of the tree walk: it is what keeps a refused walk from resolving
|
||||
// having killed nothing while the caller releases the managed-home lock. It
|
||||
// runs after the walk there, so it can be a tick behind the caller.
|
||||
await vi.waitFor(() => expect(child.kill).toHaveBeenCalledWith('SIGKILL'))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,22 +22,25 @@ import type {
|
||||
TextGenerationOperation
|
||||
} from './source-control-text-generation-types'
|
||||
|
||||
export function killSourceControlAgentProcess(
|
||||
export async function killSourceControlAgentProcess(
|
||||
child: SpawnedSourceControlAgentProcess
|
||||
): Promise<void> {
|
||||
const pid = child.pid
|
||||
if (!pid) {
|
||||
return Promise.resolve()
|
||||
return
|
||||
}
|
||||
if (process.platform === 'win32') {
|
||||
return terminateWindowsProcessTree(pid, { site: 'source-control-text-generation' })
|
||||
// taskkill owns the tree, but the own-Chromium gate can refuse the
|
||||
// pid-addressed walk; the handle-addressed root kill below cannot reach the
|
||||
// recycled pid it refused, and callers release the managed-home lock on this
|
||||
// promise, so it must not resolve having killed nothing.
|
||||
await terminateWindowsProcessTree(pid, { site: 'source-control-text-generation' })
|
||||
}
|
||||
try {
|
||||
child.kill('SIGKILL')
|
||||
} catch {
|
||||
// The process may exit between the PID check and kill.
|
||||
}
|
||||
return Promise.resolve()
|
||||
}
|
||||
|
||||
export function runLocalSourceControlPlan(input: {
|
||||
|
||||
@@ -0,0 +1,197 @@
|
||||
import { existsSync, mkdtempSync, readFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { spawn, ChildProcess } from 'node:child_process'
|
||||
import { subscribe, unsubscribe } from 'node:diagnostics_channel'
|
||||
import { afterAll, afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { setAppEnvironment, type AppEnvironment } from '../shared/app-environment'
|
||||
import { setProcessTreeKillGate } from '../shared/child-process/process-tree-kill-gate'
|
||||
import { signalProcessTree } from '../shared/child-process/process-tree-termination'
|
||||
import { removeTreeSync } from '../shared/windows-transient-lock-removal'
|
||||
import {
|
||||
findSelfInitiatedTreeKills,
|
||||
resetSelfInitiatedTreeKillLogForTest
|
||||
} from './crash-reporting/self-initiated-tree-kill-log'
|
||||
import { installMainProcessTreeKillGate } from './own-chromium-tree-kill-guard'
|
||||
import { terminateWindowsProcessTree } from './windows-process-tree-kill'
|
||||
|
||||
/**
|
||||
* The unit tests pin the gate's decision against a mocked `taskkill`; this pins
|
||||
* what that decision does to real Windows processes.
|
||||
*
|
||||
* Both are needed. Every claim the gate makes is about a mechanism the mocks
|
||||
* cannot show: that `taskkill /T /F` actually reaps a detached grandchild, that
|
||||
* a refusal actually leaves that tree standing, and that the handle-addressed
|
||||
* root kill the refusal path falls back to actually reaps the root while
|
||||
* orphaning its descendants — the asymmetry the PR discloses rather than fixes.
|
||||
*
|
||||
* Runs only on win32; skipped elsewhere.
|
||||
*/
|
||||
const describeOnWindows = process.platform === 'win32' ? describe : describe.skip
|
||||
|
||||
/** Read live by the guard on every kill, so a case can flip it mid-test. */
|
||||
let orcaChromiumPids: number[] = []
|
||||
|
||||
function appEnvironment(): AppEnvironment {
|
||||
return {
|
||||
getPath: () => process.cwd(),
|
||||
getAppPath: () => process.cwd(),
|
||||
getVersion: () => '0.0.0-live',
|
||||
isPackaged: () => false,
|
||||
onWillQuit: () => {},
|
||||
exit: () => {},
|
||||
getAppMetrics: (() =>
|
||||
orcaChromiumPids.map((pid) => ({
|
||||
pid,
|
||||
type: 'Tab'
|
||||
}))) as unknown as AppEnvironment['getAppMetrics']
|
||||
}
|
||||
}
|
||||
|
||||
function isAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
const sleep = (ms: number): Promise<void> => new Promise((resolve) => setTimeout(resolve, ms))
|
||||
|
||||
async function waitFor(predicate: () => boolean, timeoutMs = 10_000): Promise<boolean> {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
while (Date.now() < deadline && !predicate()) {
|
||||
await sleep(100)
|
||||
}
|
||||
return predicate()
|
||||
}
|
||||
|
||||
let markerDirectory = ''
|
||||
let markerSequence = 0
|
||||
const spawnedRoots: ChildProcess[] = []
|
||||
const spawnedLeaves: number[] = []
|
||||
const observedSpawns: ChildProcess[] = []
|
||||
|
||||
function observeSpawn(message: unknown): void {
|
||||
if (
|
||||
typeof message === 'object' &&
|
||||
message !== null &&
|
||||
'process' in message &&
|
||||
message.process instanceof ChildProcess
|
||||
) {
|
||||
observedSpawns.push(message.process)
|
||||
}
|
||||
}
|
||||
|
||||
/** A real root with a real grandchild; the grandchild reports its pid on disk. */
|
||||
async function spawnLiveTree(): Promise<{
|
||||
child: ChildProcess
|
||||
rootPid: number
|
||||
leafPid: number
|
||||
}> {
|
||||
const marker = join(markerDirectory, `leaf-${markerSequence++}.pid`)
|
||||
const leafSource = `require('node:fs').writeFileSync(${JSON.stringify(marker)}, String(process.pid)); setTimeout(() => {}, 600000)`
|
||||
// Non-detached Windows children can die with the root's libuv Job Object.
|
||||
const rootSource = `require('node:child_process').spawn(process.execPath, ['-e', ${JSON.stringify(leafSource)}], { stdio: 'ignore', detached: true, windowsHide: true }); setTimeout(() => {}, 600000)`
|
||||
const child = spawn(process.execPath, ['-e', rootSource], {
|
||||
stdio: 'ignore',
|
||||
windowsHide: true
|
||||
})
|
||||
spawnedRoots.push(child)
|
||||
const rootPid = child.pid as number
|
||||
expect(rootPid).toBeGreaterThan(0)
|
||||
expect(await waitFor(() => existsSync(marker))).toBe(true)
|
||||
const leafPid = Number(readFileSync(marker, 'utf8'))
|
||||
spawnedLeaves.push(leafPid)
|
||||
expect(await waitFor(() => isAlive(leafPid))).toBe(true)
|
||||
return { child, rootPid, leafPid }
|
||||
}
|
||||
|
||||
describeOnWindows('own-Chromium gate against real Windows process trees', () => {
|
||||
beforeEach(() => {
|
||||
markerDirectory ||= mkdtempSync(join(tmpdir(), 'orca-live-tree-kill-'))
|
||||
resetSelfInitiatedTreeKillLogForTest()
|
||||
orcaChromiumPids = []
|
||||
setAppEnvironment(appEnvironment())
|
||||
installMainProcessTreeKillGate()
|
||||
observedSpawns.length = 0
|
||||
subscribe('child_process', observeSpawn)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
unsubscribe('child_process', observeSpawn)
|
||||
orcaChromiumPids = []
|
||||
for (const leafPid of spawnedLeaves.splice(0)) {
|
||||
await terminateWindowsProcessTree(leafPid, { site: 'live-tree-kill-cleanup' })
|
||||
}
|
||||
for (const root of spawnedRoots.splice(0)) {
|
||||
root.kill('SIGKILL')
|
||||
}
|
||||
setProcessTreeKillGate(null)
|
||||
})
|
||||
|
||||
afterAll(() => {
|
||||
if (markerDirectory) {
|
||||
removeTreeSync(markerDirectory)
|
||||
}
|
||||
})
|
||||
|
||||
it('admitted: taskkill reaps the root and its detached grandchild, and the kill is recorded', async () => {
|
||||
const { rootPid, leafPid } = await spawnLiveTree()
|
||||
|
||||
await terminateWindowsProcessTree(rootPid, { site: 'live-tree-kill-admit' })
|
||||
|
||||
expect(await waitFor(() => !isAlive(rootPid))).toBe(true)
|
||||
expect(await waitFor(() => !isAlive(leafPid))).toBe(true)
|
||||
expect(
|
||||
findSelfInitiatedTreeKills(Date.now()).some(
|
||||
(kill) => kill.pid === rootPid && kill.site === 'live-tree-kill-admit'
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('refused: the tree survives, nothing is recorded, and the handle kill still reaps the root', async () => {
|
||||
const { child, rootPid, leafPid } = await spawnLiveTree()
|
||||
orcaChromiumPids = [rootPid]
|
||||
|
||||
await terminateWindowsProcessTree(rootPid, { site: 'live-tree-kill-refuse' })
|
||||
|
||||
await sleep(1_000)
|
||||
expect(isAlive(rootPid)).toBe(true)
|
||||
expect(isAlive(leafPid)).toBe(true)
|
||||
expect(findSelfInitiatedTreeKills(Date.now())).toEqual([])
|
||||
|
||||
// The fallback every gated site runs after a refusal.
|
||||
child.kill('SIGKILL')
|
||||
expect(await waitFor(() => !isAlive(rootPid))).toBe(true)
|
||||
// Let root-owned job cleanup finish before asserting independent survival.
|
||||
await sleep(250)
|
||||
// Disclosed asymmetry: a refusal orphans descendants rather than reaping them.
|
||||
expect(isAlive(leafPid)).toBe(true)
|
||||
})
|
||||
|
||||
it('signalProcessTree refused: the root goes by handle and the barrier reports unverified', async () => {
|
||||
const { child, rootPid, leafPid } = await spawnLiveTree()
|
||||
orcaChromiumPids = [rootPid]
|
||||
observedSpawns.length = 0
|
||||
|
||||
await expect(signalProcessTree(child, 'SIGKILL')).resolves.toBe(false)
|
||||
|
||||
expect(observedSpawns).toHaveLength(0)
|
||||
expect(await waitFor(() => !isAlive(rootPid))).toBe(true)
|
||||
await sleep(250)
|
||||
expect(isAlive(leafPid)).toBe(true)
|
||||
})
|
||||
|
||||
it('signalProcessTree admitted: the whole tree goes and the barrier reports verified', async () => {
|
||||
const { child, rootPid, leafPid } = await spawnLiveTree()
|
||||
observedSpawns.length = 0
|
||||
|
||||
await expect(signalProcessTree(child, 'SIGKILL')).resolves.toBe(true)
|
||||
|
||||
expect(observedSpawns.map((child) => child.spawnfile)).toEqual(['taskkill'])
|
||||
expect(await waitFor(() => !isAlive(rootPid))).toBe(true)
|
||||
expect(await waitFor(() => !isAlive(leafPid))).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -11,8 +11,9 @@ export const WINDOWS_PROCESS_TREE_KILL_TIMEOUT_MS = 5_000
|
||||
* Best-effort: missing/already-dead roots still resolve so callers can finish
|
||||
* their own handle cleanup via killRoot.
|
||||
*
|
||||
* Nearly every main-process taskkill runs through here; the two account-login
|
||||
* teardowns keep their own spawn but share the same gate, so the refusal and the
|
||||
* Most main-process taskkills run through here; the families that keep their own
|
||||
* spawn (account-login teardowns, codex app-server deadline, git-command abort,
|
||||
* notebook and precheck timeouts) share the same gate, so the refusal and the
|
||||
* breadcrumb live in `admitSelfInitiatedTreeKill` rather than in this function.
|
||||
*/
|
||||
export function terminateWindowsProcessTree(
|
||||
|
||||
@@ -22,7 +22,7 @@ const IOS_CHANNEL_COPY: Record<IosChannel, InstallCopy> = {
|
||||
|
||||
const ANDROID_COPY: InstallCopy = {
|
||||
ctaLabel: 'Download APK',
|
||||
url: 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.46/app-release.apk'
|
||||
url: 'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.47/app-release.apk'
|
||||
}
|
||||
|
||||
export function getInstallCopy(platform: Platform, iosChannel: IosChannel): InstallCopy {
|
||||
|
||||
@@ -13,7 +13,7 @@ export { getMobileSettingsPaneSearchEntries }
|
||||
|
||||
const ORCA_IOS_APP_STORE_URL = 'https://apps.apple.com/app/orca-ide/id6766130217'
|
||||
const ORCA_ANDROID_APK_URL =
|
||||
'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.46/app-release.apk'
|
||||
'https://github.com/stablyai/orca/releases/download/mobile-android-v0.0.47/app-release.apk'
|
||||
|
||||
export function MobileSettingsPane(): React.JSX.Element {
|
||||
const showMobileButton = useAppStore((s) => s.settings?.showMobileButton !== false)
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
/**
|
||||
* Seam that lets the main process decide, and record, the tree-kills issued
|
||||
* from code it does not own.
|
||||
*
|
||||
* Why a seam and not a direct call: `signalProcessTree` is the choke point every
|
||||
* `runProcess` termination funnels through, and the codex app-server and
|
||||
* ephemeral-VM kills are shared with the CLI — all of them live outside
|
||||
* `src/main` and cannot import the own-Chromium guard or the crash breadcrumb
|
||||
* store. Main registers the guard at startup; everywhere else this admits every
|
||||
* kill and records nothing.
|
||||
*/
|
||||
|
||||
/** Blast radius, not mechanism: `win-taskkill-tree` is addressed by pid and walks
|
||||
* whatever tree that pid has *now*, so it can land on a recycled pid that is
|
||||
* since one of Orca's own Chromium processes. A process group can only contain
|
||||
* processes Orca itself put there. */
|
||||
export type ProcessTreeKillScope = 'win-taskkill-tree' | 'posix-process-group'
|
||||
|
||||
export type ProcessTreeKill = {
|
||||
pid: number
|
||||
site: string
|
||||
scope: ProcessTreeKillScope
|
||||
}
|
||||
|
||||
/** False means the caller must not walk that pid's tree — main is accounting for
|
||||
* it. Killing the root through its own child handle stays correct and required:
|
||||
* a handle cannot land on the recycled pid the refusal is about. */
|
||||
type ProcessTreeKillGate = (kill: ProcessTreeKill) => boolean
|
||||
|
||||
let gate: ProcessTreeKillGate | null = null
|
||||
|
||||
export function setProcessTreeKillGate(next: ProcessTreeKillGate | null): void {
|
||||
gate = next
|
||||
}
|
||||
|
||||
export function admitProcessTreeKill(kill: ProcessTreeKill): boolean {
|
||||
try {
|
||||
return gate?.(kill) ?? true
|
||||
} catch {
|
||||
// Diagnostics must never turn a successful termination into a failed one.
|
||||
return true
|
||||
}
|
||||
}
|
||||
@@ -1,37 +0,0 @@
|
||||
/**
|
||||
* Seam that lets the main process record the tree-kills issued from here.
|
||||
*
|
||||
* Why a seam and not a direct call: `signalProcessTree` is the choke point every
|
||||
* `runProcess` termination funnels through, but it lives in `src/shared` and so
|
||||
* runs in the CLI and relay too — it cannot import the main-process crash
|
||||
* breadcrumb store. Main registers the recorder at startup; everywhere else this
|
||||
* stays a no-op.
|
||||
*/
|
||||
|
||||
/** Blast radius, not mechanism: `win-taskkill-tree` is addressed by pid and walks
|
||||
* whatever tree that pid has *now*, so it can land on a recycled pid that is
|
||||
* since one of Orca's own Chromium processes. A process group can only contain
|
||||
* processes Orca itself put there. */
|
||||
export type ProcessTreeKillScope = 'win-taskkill-tree' | 'posix-process-group'
|
||||
|
||||
export type ProcessTreeKill = {
|
||||
pid: number
|
||||
site: string
|
||||
scope: ProcessTreeKillScope
|
||||
}
|
||||
|
||||
type ProcessTreeKillObserver = (kill: ProcessTreeKill) => void
|
||||
|
||||
let observer: ProcessTreeKillObserver | null = null
|
||||
|
||||
export function setProcessTreeKillObserver(next: ProcessTreeKillObserver | null): void {
|
||||
observer = next
|
||||
}
|
||||
|
||||
export function notifyProcessTreeKill(kill: ProcessTreeKill): void {
|
||||
try {
|
||||
observer?.(kill)
|
||||
} catch {
|
||||
// Diagnostics must never turn a successful termination into a failed one.
|
||||
}
|
||||
}
|
||||
@@ -7,7 +7,7 @@ const { spawnMock } = vi.hoisted(() => ({ spawnMock: vi.fn() }))
|
||||
vi.mock('node:child_process', () => ({ spawn: spawnMock }))
|
||||
|
||||
import { forceTerminateProcessTree, signalProcessTree } from './process-tree-termination'
|
||||
import { setProcessTreeKillObserver, type ProcessTreeKill } from './process-tree-kill-observer'
|
||||
import { setProcessTreeKillGate, type ProcessTreeKill } from './process-tree-kill-gate'
|
||||
|
||||
function mockProcess(pid: number): ChildProcess {
|
||||
const child = new EventEmitter() as EventEmitter & {
|
||||
@@ -103,11 +103,14 @@ describe('process-tree-kill breadcrumb seam', () => {
|
||||
|
||||
beforeEach(() => {
|
||||
observed.length = 0
|
||||
setProcessTreeKillObserver((kill) => observed.push(kill))
|
||||
setProcessTreeKillGate((kill) => {
|
||||
observed.push(kill)
|
||||
return true
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
setProcessTreeKillObserver(null)
|
||||
setProcessTreeKillGate(null)
|
||||
spawnMock.mockReset()
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { spawn as nodeSpawn, type ChildProcess } from 'node:child_process'
|
||||
import { notifyProcessTreeKill } from './process-tree-kill-observer'
|
||||
import { admitProcessTreeKill } from './process-tree-kill-gate'
|
||||
|
||||
const PROBE_INTERVAL_MS = 25
|
||||
const SUBPROCESS_TIMEOUT_MS = 2_000
|
||||
@@ -13,8 +13,17 @@ const MAX_PS_OUTPUT_BYTES = 8 * 1024 * 1024
|
||||
* leader would hand it to whatever group it inherited instead.
|
||||
*
|
||||
* Runs in every host — Electron main, the daemon, the relay, the CLI — so the
|
||||
* main-process own-Chromium guard cannot reach here; the exit check below is
|
||||
* what keeps the Windows branch off a pid that is no longer ours.
|
||||
* own-Chromium guard arrives through `process-tree-kill-gate`, which main
|
||||
* installs and every other host leaves admitting. The exit check below is what
|
||||
* keeps the Windows branch off a pid that is no longer ours on those hosts.
|
||||
*
|
||||
* Both arms ask before walking the tree, so a refused pid never gets a
|
||||
* pid-addressed kill; that also means the recorded crumb says "about to kill",
|
||||
* not "killed". The root is still killed through its handle, which cannot reach
|
||||
* the recycled pid the refusal was about, so a refusal is never a leak. Main's
|
||||
* gate only ever refuses the `win-taskkill-tree` scope — a POSIX group holds
|
||||
* only what Orca put in it — so the POSIX refusal arm is the seam's contract,
|
||||
* not something any installed gate exercises today.
|
||||
*/
|
||||
export function signalProcessTree(child: ChildProcess, signal?: NodeJS.Signals): Promise<boolean> {
|
||||
if (!child.pid) {
|
||||
@@ -39,13 +48,20 @@ export function signalProcessTree(child: ChildProcess, signal?: NodeJS.Signals):
|
||||
}
|
||||
return taskkillTree(child, child.pid, signal)
|
||||
}
|
||||
try {
|
||||
process.kill(-child.pid, signal)
|
||||
notifyProcessTreeKill({
|
||||
if (
|
||||
!admitProcessTreeKill({
|
||||
pid: child.pid,
|
||||
site: 'run-process-tree',
|
||||
scope: 'posix-process-group'
|
||||
})
|
||||
) {
|
||||
// Same shape as the reaped-pid skip above: refuse the group, still kill the
|
||||
// root by handle, and report unverified.
|
||||
killRoot(child, signal)
|
||||
return Promise.resolve(false)
|
||||
}
|
||||
try {
|
||||
process.kill(-child.pid, signal)
|
||||
return Promise.resolve(true)
|
||||
} catch {
|
||||
return Promise.resolve(!processGroupExists(child.pid))
|
||||
@@ -73,6 +89,13 @@ function taskkillTree(
|
||||
rootPid: number,
|
||||
signal?: NodeJS.Signals
|
||||
): Promise<boolean> {
|
||||
// Asked before the spawn, not after: a refusal has to prevent the taskkill.
|
||||
if (
|
||||
!admitProcessTreeKill({ pid: rootPid, site: 'run-process-tree', scope: 'win-taskkill-tree' })
|
||||
) {
|
||||
killRoot(child, signal)
|
||||
return Promise.resolve(false)
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
let killer: ChildProcess
|
||||
try {
|
||||
@@ -86,7 +109,6 @@ function taskkillTree(
|
||||
resolve(false)
|
||||
return
|
||||
}
|
||||
notifyProcessTreeKill({ pid: rootPid, site: 'run-process-tree', scope: 'win-taskkill-tree' })
|
||||
let settled = false
|
||||
const finish = (fallback: boolean): void => {
|
||||
if (settled) {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'
|
||||
import type { EphemeralVmRecipeContext } from './ephemeral-vm-recipe-runner'
|
||||
import { admitProcessTreeKill } from './child-process/process-tree-kill-gate'
|
||||
|
||||
const DEFAULT_MAX_CAPTURE_BYTES = 1024 * 1024
|
||||
const CANCEL_FORCE_KILL_DELAY_MS = 5_000
|
||||
@@ -132,13 +133,26 @@ export async function runRecipeCommand(args: {
|
||||
})
|
||||
}
|
||||
|
||||
function killRecipeProcess(child: ChildProcessWithoutNullStreams, force = false): void {
|
||||
/** Exported for the refusal-fallback test; the abort path is otherwise unreachable. */
|
||||
export function killRecipeProcess(child: ChildProcessWithoutNullStreams, force = false): void {
|
||||
const signal = force ? 'SIGKILL' : 'SIGTERM'
|
||||
if (process.platform === 'win32') {
|
||||
// Recipes run through `cmd.exe /c` (shell: true), so child.kill() would only
|
||||
// terminate the wrapper and orphan the actual recipe subprocess (e.g. a cloud
|
||||
// CLI mid-provision). taskkill /T walks and kills the whole tree.
|
||||
if (child.pid) {
|
||||
if (
|
||||
!admitProcessTreeKill({
|
||||
pid: child.pid,
|
||||
site: 'ephemeral-vm-recipe',
|
||||
scope: 'win-taskkill-tree'
|
||||
})
|
||||
) {
|
||||
// Refusal blocks the tree walk, not the termination: the root kill is
|
||||
// handle-addressed, so it cannot reach the recycled pid we refused.
|
||||
child.kill(signal)
|
||||
return
|
||||
}
|
||||
const killer = spawn('taskkill', ['/pid', String(child.pid), '/t', '/f'], {
|
||||
windowsHide: true,
|
||||
stdio: 'ignore'
|
||||
|
||||
Reference in New Issue
Block a user