diff --git a/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs b/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs index cb6f4446e20..a135c8adc30 100644 --- a/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs +++ b/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs @@ -50,6 +50,7 @@ import { checkLauncherHoldsOccupiedDaemon } from './daemon-replacement-launcher- import { findTaggedPid, isMarkerAlive, + verifiedSessionLeaderPid, isProcessAlive, processArgs, processState, @@ -429,7 +430,14 @@ function teardown(staged) { if (!staged) { return } - const daemonPid = staged.daemon?.child.pid + // Why the exit check: phase 1 kills this daemon on purpose, and once Node has reaped the + // child its pid is free for the OS to reuse. Signalling the remembered number after that is + // signalling a stranger. + const daemonChild = staged.daemon?.child + const daemonPid = + daemonChild && daemonChild.exitCode === null && daemonChild.signalCode === null + ? daemonChild.pid + : undefined if (daemonPid) { for (const signal of staged.stopped ? ['SIGCONT', 'SIGKILL'] : ['SIGKILL']) { try { @@ -451,7 +459,13 @@ function teardown(staged) { if (!isMarkerAlive(marker)) { continue } - for (const pid of [marker.pid, marker.sessionPid]) { + // The leader is re-read from the live marker rather than remembered: the marker proves its + // own identity by tag, but nothing proved the leader's, and it is the one pid here that + // could have been recycled while its child stayed alive under a new parent. + for (const pid of [marker.pid, verifiedSessionLeaderPid(marker)]) { + if (!pid) { + continue + } try { process.kill(pid, 'SIGKILL') } catch { diff --git a/config/scripts/daemon-replacement-process-inspection.mjs b/config/scripts/daemon-replacement-process-inspection.mjs index 5b0000f90fe..fed5c49f7ab 100644 --- a/config/scripts/daemon-replacement-process-inspection.mjs +++ b/config/scripts/daemon-replacement-process-inspection.mjs @@ -6,7 +6,15 @@ */ import { execFileSync } from 'node:child_process' -const REAL_USER_DAEMON_MARKER = 'Library/Application Support/orca/daemon' +// Electron's userData path differs per platform, and hardcoding the macOS one meant a real +// daemon could never be recognised on Linux — so the guard that this run harmed nothing was +// inert on exactly the platform where it would go unnoticed. +const REAL_USER_DAEMON_MARKERS = { + darwin: ['Library/Application Support/orca/daemon'], + linux: ['.config/orca/daemon'], + win32: ['AppData/Roaming/orca/daemon', 'AppData\\Roaming\\orca\\daemon'] +} +const REAL_USER_DAEMON_MARKER_LIST = REAL_USER_DAEMON_MARKERS[process.platform] ?? ['orca/daemon'] export function processArgs(pid) { try { @@ -66,6 +74,31 @@ export function isMarkerAlive(marker) { return processArgs(marker.pid)?.includes(marker.tag) === true } +/** + * The session leader of a still-live marker, read now rather than remembered. + * + * Why not trust the pid captured at staging: teardown runs a minute later, and phase 1 has + * deliberately killed things in between. A remembered leader pid may by then belong to whatever + * the OS recycled it onto, and SIGKILLing that is precisely the mistake this script exists to + * demonstrate. Returns null unless the live marker still claims this leader. + */ +export function verifiedSessionLeaderPid(marker) { + if (!isMarkerAlive(marker)) { + return null + } + try { + const ppid = Number( + execFileSync('ps', ['-p', String(marker.pid), '-o', 'ppid='], { + encoding: 'utf8', + timeout: 5_000 + }).trim() + ) + return Number.isInteger(ppid) && ppid === marker.sessionPid ? ppid : null + } catch { + return null + } +} + // Pre-existing daemons (the user's real one above all) must be untouched by this run. export function snapshotForeignDaemons() { const daemons = [] @@ -77,7 +110,10 @@ export function snapshotForeignDaemons() { } const pid = Number(line.trim().split(/\s+/, 1)[0]) if (Number.isInteger(pid) && pid > 0) { - daemons.push({ pid, isRealUserDaemon: line.includes(REAL_USER_DAEMON_MARKER) }) + daemons.push({ + pid, + isRealUserDaemon: REAL_USER_DAEMON_MARKER_LIST.some((marker) => line.includes(marker)) + }) } } } catch {