diff --git a/config/scripts/daemon-replacement-launcher-hold-source-assertions.mjs b/config/scripts/daemon-replacement-launcher-hold-source-assertions.mjs new file mode 100644 index 00000000000..e265bc3a764 --- /dev/null +++ b/config/scripts/daemon-replacement-launcher-hold-source-assertions.mjs @@ -0,0 +1,154 @@ +#!/usr/bin/env node +/** + * Static source assertions for the launcher's hold decision. + * + * Split from the repro script it serves: those phases prove behaviour with real processes, + * while these read `daemon-init.ts` to pin the one property real processes cannot reach — + * that the decision is taken, and returns, before anything is killed. daemon-init.ts imports + * electron, so it cannot be executed outside the app. + */ +import { readFileSync } from 'node:fs' +import { join, resolve } from 'node:path' + +const repoRoot = resolve(import.meta.dirname, '..', '..') + +export function stripComments(source) { + // Blanked rather than deleted so offsets and line numbers stay true to the real file. + const blank = (text) => text.replace(/[^\n]/g, ' ') + return source + .replace(/\/\*[\s\S]*?\*\//g, blank) + .replace(/(^|[^:])(\/\/[^\n]*)/g, (_match, prefix, comment) => prefix + blank(comment)) +} + +/** The balanced `{...}` block starting at `braceIndex`, or null if it never closes. */ +function extractBlock(source, braceIndex) { + let depth = 0 + for (let i = braceIndex; i < source.length; i++) { + if (source[i] === '{') { + depth++ + } else if (source[i] === '}') { + depth-- + if (depth === 0) { + return { text: source.slice(braceIndex, i + 1), start: braceIndex, end: i + 1 } + } + } + } + return null +} + +function lineOf(source, index) { + return source.slice(0, index).split('\n').length +} + +function normalize(text) { + return text.replace(/\s+/g, ' ') +} + +/** + * PHASE 3 — the launcher must hold rather than kill. daemon-init.ts imports electron, so it + * cannot be executed here; this reads the source instead, whitespace-tolerantly, and asserts + * the structural properties phase 2's inputs depend on. + */ +export function checkLauncherHoldsOccupiedDaemon({ log, assert }) { + const relativePath = 'src/main/daemon/daemon-init.ts' + const source = stripComments(readFileSync(join(repoRoot, relativePath), 'utf8')) + + // 1. holdIncumbentDaemon() returns a preserved handle in 'held' mode — it does not adopt, + // which a daemon too wedged to answer listSessions could never complete anyway. + const holdDecl = source.match(/const\s+holdIncumbentDaemon\s*=\s*\([^)]*\)[^{]*\{/) + assert(holdDecl !== null, `${relativePath} does not declare holdIncumbentDaemon()`) + const holdBody = extractBlock(source, holdDecl.index + holdDecl[0].length - 1) + assert(holdBody !== null, `could not parse the holdIncumbentDaemon() body in ${relativePath}`) + assert( + /createPreservedDaemonHandle\([^)]*'held'\s*\)/.test(normalize(holdBody.text)), + `holdIncumbentDaemon() does not return createPreservedDaemonHandle(..., 'held'): ${normalize(holdBody.text)}` + ) + log( + `phase 3: ${relativePath}:${lineOf(source, holdDecl.index)} holdIncumbentDaemon() = ${normalize(holdBody.text)}` + ) + + // 2. Process-table evidence is only ever raised from an identity-verified pid — otherwise + // it could describe a recycled pid's children rather than this daemon's terminals. + const verifiedPidCall = source.search(/readVerifiedDaemonPid\s*\(/) + const evidenceCall = source.match(/raiseOccupancyWithProcessEvidence\s*\(([^)]*)\)/) + assert(verifiedPidCall !== -1, `${relativePath} never calls readVerifiedDaemonPid()`) + assert(evidenceCall !== null, `${relativePath} never raises occupancy with process evidence`) + assert( + verifiedPidCall < evidenceCall.index, + `${relativePath} raises occupancy with process evidence before verifying the recorded pid` + ) + // Whatever identifier carries the pid, its declaration must come from the verified read. + const evidencePidName = evidenceCall[1] + .split(',')[1] + ?.trim() + .replace(/[^\w$]/g, '') + assert( + Boolean(evidencePidName), + `could not read the pid argument of raiseOccupancyWithProcessEvidence: ${normalize(evidenceCall[1])}` + ) + const evidencePidDecl = new RegExp( + `const\\s+${evidencePidName}\\b[\\s\\S]{0,400}?readVerifiedDaemonPid\\s*\\(` + ) + assert( + evidencePidDecl.test(source), + `${relativePath} passes '${evidencePidName}' to raiseOccupancyWithProcessEvidence without deriving it from readVerifiedDaemonPid — the evidence could then describe a recycled pid's children` + ) + + // 3. The 'occupied' branch holds and never kills. + const occupiedGuard = source.match(/if\s*\(\s*occupancy\.state\s*===\s*'occupied'\s*\)\s*\{/) + assert(occupiedGuard !== null, `${relativePath} has no 'occupancy.state === occupied' guard`) + const occupiedBlock = extractBlock(source, occupiedGuard.index + occupiedGuard[0].length - 1) + assert(occupiedBlock !== null, `could not parse the occupied branch in ${relativePath}`) + const occupiedLine = lineOf(source, occupiedGuard.index) + assert( + !occupiedBlock.text.includes('killStaleDaemon'), + `${relativePath}:${occupiedLine} calls killStaleDaemon inside the occupied branch` + ) + + // Holding requires BOTH: no hello ever completed, and only the process table could answer. + // A daemon that did complete a hello is adoptable, so it must not be routed to a mode that + // never adopts. + const unverifiableGuard = occupiedBlock.text.match( + /if\s*\(\s*health\s*===\s*'rejected'\s*\|\|\s*occupancy\.liveSessions\s*===\s*null\s*\)\s*\{/ + ) + assert( + unverifiableGuard !== null, + `${relativePath}:${occupiedLine} does not gate the hold on a daemon that cannot be adopted (rejected, or an unverifiable session count)` + ) + const unverifiableBlock = extractBlock( + occupiedBlock.text, + unverifiableGuard.index + unverifiableGuard[0].length - 1 + ) + assert(unverifiableBlock !== null, 'could not parse the liveSessions === null branch') + assert( + normalize(unverifiableBlock.text).includes('return holdIncumbentDaemon()'), + `${relativePath}:${occupiedLine} does not return holdIncumbentDaemon() when the session count came from the process table` + ) + log( + `phase 3: ${relativePath}:${occupiedLine} occupancy.state === 'occupied' + cannot-be-adopted -> return holdIncumbentDaemon(); the branch contains no kill` + ) + + // 4. Ordering: every kill on this path is downstream of the occupied branch, so a hold + // returns before any of them can run. + const killCalls = [...source.matchAll(/killStaleDaemon\s*\(/g)].map((match) => match.index) + assert(killCalls.length > 0, `${relativePath} never calls killStaleDaemon()`) + const killsBeforeTheDecision = killCalls.filter( + (index) => index > evidenceCall.index && index < occupiedBlock.end + ) + assert( + killsBeforeTheDecision.length === 0, + `${relativePath} kills at line(s) ${killsBeforeTheDecision.map((i) => lineOf(source, i)).join(', ')}, between resolving occupancy and the hold` + ) + const fallThroughKill = killCalls.find((index) => index > occupiedBlock.end) + assert( + fallThroughKill !== undefined, + `${relativePath} has no killStaleDaemon() after the occupied branch — the replacement path is gone` + ) + const killLines = killCalls.map((index) => lineOf(source, index)).join(', ') + log( + `phase 3: every killStaleDaemon() call site in the file is at line(s) ${killLines} — all downstream of the occupied branch, which returns at line ${lineOf(source, occupiedBlock.start)}` + ) + log( + 'phase 3 RESULT: statically, the failed-health-check path resolves occupancy from a verified pid and returns a held handle before any kill. This proves the source ordering and branch contents; it does NOT execute daemon-init.ts (it imports electron), so the runtime proof stops at the inputs phase 2 produced with real processes.' + ) +} 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 8dcc0697c25..495865be268 100644 --- a/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs +++ b/config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs @@ -40,19 +40,28 @@ * * Usage: node config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs */ -import { execFileSync, fork } from 'node:child_process' +import { fork } from 'node:child_process' import { randomUUID } from 'node:crypto' import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { pathToFileURL } from 'node:url' +import { checkLauncherHoldsOccupiedDaemon } from './daemon-replacement-launcher-hold-source-assertions.mjs' +import { + findTaggedPid, + isMarkerAlive, + isProcessAlive, + processArgs, + processState, + snapshotForeignDaemons, + waitFor +} from './daemon-replacement-process-inspection.mjs' const repoRoot = resolve(import.meta.dirname, '..', '..') const entryPath = join(repoRoot, 'out', 'main', 'daemon-entry.js') const READY_TIMEOUT_MS = 30_000 const MARKER_SPAWN_TIMEOUT_MS = 30_000 const SESSION_COUNT = 2 -const REAL_USER_DAEMON_MARKER = 'Library/Application Support/orca/daemon' const startedAt = Date.now() const timeline = [] @@ -126,97 +135,6 @@ async function countLiveSessionsOverIpc(DaemonClient, socketPath, tokenPath) { } } -function processArgs(pid) { - try { - return execFileSync('ps', ['-p', String(pid), '-o', 'args='], { - encoding: 'utf8', - timeout: 5_000 - }).trim() - } catch { - return null - } -} - -function processState(pid) { - try { - return execFileSync('ps', ['-p', String(pid), '-o', 'stat='], { - encoding: 'utf8', - timeout: 5_000 - }).trim() - } catch { - return null - } -} - -function isProcessAlive(pid) { - try { - process.kill(pid, 0) - return true - } catch (error) { - return error?.code !== 'ESRCH' - } -} - -// Why scan by tag rather than trust the session pid: macOS wraps the PTY in -// /usr/bin/login for TCC attribution, so the agent process is a descendant of -// the session leader — exactly as a real `claude`/`codex` launch would be. -function findTaggedPid(tag) { - try { - const output = execFileSync('ps', ['-eo', 'pid=,args='], { - encoding: 'utf8', - timeout: 5_000 - }) - for (const line of output.split('\n')) { - if (line.includes(tag)) { - const pid = Number(line.trim().split(/\s+/, 1)[0]) - if (Number.isInteger(pid) && pid > 0) { - return pid - } - } - } - } catch { - // ps failed; treat as not found. - } - return null -} - -function isMarkerAlive(marker) { - return processArgs(marker.pid)?.includes(marker.tag) === true -} - -// Pre-existing daemons (the user's real one above all) must be untouched by this run. -function snapshotForeignDaemons() { - const daemons = [] - try { - const output = execFileSync('ps', ['-eo', 'pid=,args='], { encoding: 'utf8', timeout: 5_000 }) - for (const line of output.split('\n')) { - if (!line.includes('daemon-entry.js')) { - continue - } - 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) }) - } - } - } catch { - // ps failed; the exit check will report an empty snapshot. - } - return daemons -} - -async function waitFor(predicate, description, timeoutMs) { - const deadline = Date.now() + timeoutMs - for (;;) { - if (await predicate()) { - return - } - if (Date.now() >= deadline) { - throw new Error(`timed out waiting for ${description}`) - } - await new Promise((r) => setTimeout(r, 200)) - } -} - function forkDaemon({ runtimeDir, socketPath, tokenPath, pidPath, launchNonce, logFile }) { // Argv and spawn options mirror daemon-init.ts createOutOfProcessLauncher(). const child = fork( @@ -515,135 +433,6 @@ async function runOccupancyDecisionPhase(primitives, scratch, graceRetries, regi return staged } -function stripComments(source) { - // Blanked rather than deleted so offsets and line numbers stay true to the real file. - const blank = (text) => text.replace(/[^\n]/g, ' ') - return source - .replace(/\/\*[\s\S]*?\*\//g, blank) - .replace(/(^|[^:])(\/\/[^\n]*)/g, (_match, prefix, comment) => prefix + blank(comment)) -} - -/** The balanced `{...}` block starting at `braceIndex`, or null if it never closes. */ -function extractBlock(source, braceIndex) { - let depth = 0 - for (let i = braceIndex; i < source.length; i++) { - if (source[i] === '{') { - depth++ - } else if (source[i] === '}') { - depth-- - if (depth === 0) { - return { text: source.slice(braceIndex, i + 1), start: braceIndex, end: i + 1 } - } - } - } - return null -} - -function lineOf(source, index) { - return source.slice(0, index).split('\n').length -} - -function normalize(text) { - return text.replace(/\s+/g, ' ') -} - -/** - * PHASE 3 — the launcher must hold rather than kill. daemon-init.ts imports electron, so it - * cannot be executed here; this reads the source instead, whitespace-tolerantly, and asserts - * the structural properties phase 2's inputs depend on. - */ -function checkLauncherHoldsOccupiedDaemon() { - const relativePath = 'src/main/daemon/daemon-init.ts' - const source = stripComments(readFileSync(join(repoRoot, relativePath), 'utf8')) - - // 1. holdIncumbentDaemon() returns a preserved handle in 'held' mode — it does not adopt, - // which a daemon too wedged to answer listSessions could never complete anyway. - const holdDecl = source.match(/const\s+holdIncumbentDaemon\s*=\s*\([^)]*\)[^{]*\{/) - assert(holdDecl !== null, `${relativePath} does not declare holdIncumbentDaemon()`) - const holdBody = extractBlock(source, holdDecl.index + holdDecl[0].length - 1) - assert(holdBody !== null, `could not parse the holdIncumbentDaemon() body in ${relativePath}`) - assert( - /createPreservedDaemonHandle\([^)]*'held'\s*\)/.test(normalize(holdBody.text)), - `holdIncumbentDaemon() does not return createPreservedDaemonHandle(..., 'held'): ${normalize(holdBody.text)}` - ) - log( - `phase 3: ${relativePath}:${lineOf(source, holdDecl.index)} holdIncumbentDaemon() = ${normalize(holdBody.text)}` - ) - - // 2. Process-table evidence is only ever raised from an identity-verified pid — otherwise - // it could describe a recycled pid's children rather than this daemon's terminals. - const verifiedPidCall = source.search(/readVerifiedDaemonPid\s*\(/) - const evidenceCall = source.match(/raiseOccupancyWithProcessEvidence\s*\(([^)]*)\)/) - assert(verifiedPidCall !== -1, `${relativePath} never calls readVerifiedDaemonPid()`) - assert(evidenceCall !== null, `${relativePath} never raises occupancy with process evidence`) - assert( - verifiedPidCall < evidenceCall.index, - `${relativePath} raises occupancy with process evidence before verifying the recorded pid` - ) - assert( - /verifiedPid\b/.test(evidenceCall[1]), - `${relativePath} passes a pid to raiseOccupancyWithProcessEvidence that is not the verified one: ${normalize(evidenceCall[1])}` - ) - - // 3. The 'occupied' branch holds and never kills. - const occupiedGuard = source.match(/if\s*\(\s*occupancy\.state\s*===\s*'occupied'\s*\)\s*\{/) - assert(occupiedGuard !== null, `${relativePath} has no 'occupancy.state === occupied' guard`) - const occupiedBlock = extractBlock(source, occupiedGuard.index + occupiedGuard[0].length - 1) - assert(occupiedBlock !== null, `could not parse the occupied branch in ${relativePath}`) - const occupiedLine = lineOf(source, occupiedGuard.index) - assert( - !occupiedBlock.text.includes('killStaleDaemon'), - `${relativePath}:${occupiedLine} calls killStaleDaemon inside the occupied branch` - ) - - // Holding requires BOTH: no hello ever completed, and only the process table could answer. - // A daemon that did complete a hello is adoptable, so it must not be routed to a mode that - // never adopts. - const unverifiableGuard = occupiedBlock.text.match( - /if\s*\(\s*health\s*===\s*'rejected'\s*\|\|\s*occupancy\.liveSessions\s*===\s*null\s*\)\s*\{/ - ) - assert( - unverifiableGuard !== null, - `${relativePath}:${occupiedLine} does not gate the hold on a daemon that cannot be adopted (rejected, or an unverifiable session count)` - ) - const unverifiableBlock = extractBlock( - occupiedBlock.text, - unverifiableGuard.index + unverifiableGuard[0].length - 1 - ) - assert(unverifiableBlock !== null, 'could not parse the liveSessions === null branch') - assert( - normalize(unverifiableBlock.text).includes('return holdIncumbentDaemon()'), - `${relativePath}:${occupiedLine} does not return holdIncumbentDaemon() when the session count came from the process table` - ) - log( - `phase 3: ${relativePath}:${occupiedLine} occupancy.state === 'occupied' + cannot-be-adopted -> return holdIncumbentDaemon(); the branch contains no kill` - ) - - // 4. Ordering: every kill on this path is downstream of the occupied branch, so a hold - // returns before any of them can run. - const killCalls = [...source.matchAll(/killStaleDaemon\s*\(/g)].map((match) => match.index) - assert(killCalls.length > 0, `${relativePath} never calls killStaleDaemon()`) - const killsBeforeTheDecision = killCalls.filter( - (index) => index > evidenceCall.index && index < occupiedBlock.end - ) - assert( - killsBeforeTheDecision.length === 0, - `${relativePath} kills at line(s) ${killsBeforeTheDecision.map((i) => lineOf(source, i)).join(', ')}, between resolving occupancy and the hold` - ) - const fallThroughKill = killCalls.find((index) => index > occupiedBlock.end) - assert( - fallThroughKill !== undefined, - `${relativePath} has no killStaleDaemon() after the occupied branch — the replacement path is gone` - ) - const killLines = killCalls.map((index) => lineOf(source, index)).join(', ') - log( - `phase 3: every killStaleDaemon() call site in the file is at line(s) ${killLines} — all downstream of the occupied branch, which returns at line ${lineOf(source, occupiedBlock.start)}` - ) - log( - 'phase 3 RESULT: statically, the failed-health-check path resolves occupancy from a verified pid and returns a held handle before any kill. This proves the source ordering and branch contents; it does NOT execute daemon-init.ts (it imports electron), so the runtime proof stops at the inputs phase 2 produced with real processes.' - ) -} - function teardown(staged) { if (!staged) { return @@ -707,7 +496,7 @@ async function main() { await runOccupancyDecisionPhase(primitives, scratch, graceRetries, staged) log('=== PHASE 3: does the launcher actually hold on that verdict? ===') - checkLauncherHoldsOccupiedDaemon() + checkLauncherHoldsOccupiedDaemon({ log, assert }) verdict = 'PASS' } finally { diff --git a/config/scripts/daemon-replacement-process-inspection.mjs b/config/scripts/daemon-replacement-process-inspection.mjs new file mode 100644 index 00000000000..5b0000f90fe --- /dev/null +++ b/config/scripts/daemon-replacement-process-inspection.mjs @@ -0,0 +1,100 @@ +#!/usr/bin/env node +/** + * Process-table helpers for the daemon PTY preservation repro: what is alive, what a pid is + * running, and which daemons were already here before the run. Split out so the repro script + * itself stays about the sequence it proves rather than the plumbing it proves it with. + */ +import { execFileSync } from 'node:child_process' + +const REAL_USER_DAEMON_MARKER = 'Library/Application Support/orca/daemon' + +export function processArgs(pid) { + try { + return execFileSync('ps', ['-p', String(pid), '-o', 'args='], { + encoding: 'utf8', + timeout: 5_000 + }).trim() + } catch { + return null + } +} + +export function processState(pid) { + try { + return execFileSync('ps', ['-p', String(pid), '-o', 'stat='], { + encoding: 'utf8', + timeout: 5_000 + }).trim() + } catch { + return null + } +} + +export function isProcessAlive(pid) { + try { + process.kill(pid, 0) + return true + } catch (error) { + return error?.code !== 'ESRCH' + } +} + +// Why scan by tag rather than trust the session pid: macOS wraps the PTY in +// /usr/bin/login for TCC attribution, so the agent process is a descendant of +// the session leader — exactly as a real `claude`/`codex` launch would be. +export function findTaggedPid(tag) { + try { + const output = execFileSync('ps', ['-eo', 'pid=,args='], { + encoding: 'utf8', + timeout: 5_000 + }) + for (const line of output.split('\n')) { + if (line.includes(tag)) { + const pid = Number(line.trim().split(/\s+/, 1)[0]) + if (Number.isInteger(pid) && pid > 0) { + return pid + } + } + } + } catch { + // ps failed; treat as not found. + } + return null +} + +export function isMarkerAlive(marker) { + return processArgs(marker.pid)?.includes(marker.tag) === true +} + +// Pre-existing daemons (the user's real one above all) must be untouched by this run. +export function snapshotForeignDaemons() { + const daemons = [] + try { + const output = execFileSync('ps', ['-eo', 'pid=,args='], { encoding: 'utf8', timeout: 5_000 }) + for (const line of output.split('\n')) { + if (!line.includes('daemon-entry.js')) { + continue + } + 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) }) + } + } + } catch { + // ps failed; the exit check will report an empty snapshot. + } + return daemons +} + +export async function waitFor(predicate, description, timeoutMs) { + const deadline = Date.now() + timeoutMs + for (;;) { + if (await predicate()) { + return + } + if (Date.now() >= deadline) { + throw new Error(`timed out waiting for ${description}`) + } + await new Promise((r) => setTimeout(r, 200)) + } +} diff --git a/src/main/daemon/daemon-init.test.ts b/src/main/daemon/daemon-init.test.ts index 730ad7aa67f..6ea2a095577 100644 --- a/src/main/daemon/daemon-init.test.ts +++ b/src/main/daemon/daemon-init.test.ts @@ -3444,7 +3444,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => { }) it('grace budget is generous enough to ride out a ~60s transient wedge', () => { - // Why: each probe waits the client's 5s hello timeout, so 1 + 11 probes ≈ 60s of drain grace; don't cut without telemetry. + // Why still pinned: the wall-clock ceiling binds first, so this is an upper bound on + // retries rather than the real budget — but lowering it silently shortens the drain + // window for a daemon whose process table cannot be read, which has no other protection. expect(WEDGED_DAEMON_GRACE_RETRIES).toBeGreaterThanOrEqual(11) }) diff --git a/src/main/daemon/daemon-init.ts b/src/main/daemon/daemon-init.ts index 1c540adcbb9..ba583c63dbc 100644 --- a/src/main/daemon/daemon-init.ts +++ b/src/main/daemon/daemon-init.ts @@ -598,7 +598,6 @@ function createOutOfProcessLauncher( } else { // Why: a busy machine can time out the health check on a live daemon; re-verify what // it is hosting before killing its sessions. - const verifiedPid = await readVerifiedDaemonPid(runtimeDir, socketPath, tokenPath) // Why recordedPid is withheld here: the loop below is waiting for *IPC* to recover, // and the process table cannot change its answer within a grace window. Scanning it // every pass would multiply the launch budget for an answer we already have. @@ -618,7 +617,13 @@ function createOutOfProcessLauncher( } // One scan, once IPC has had its full chance: this is the evidence that separates a // wedged daemon still hosting terminals from one with nothing left to lose (#8689). - occupancy = await raiseOccupancyWithProcessEvidence(occupancy, verifiedPid?.pid ?? null) + // Why re-verify: the grace window is long enough for the daemon to die and its pid to + // be recycled, and the evidence would then describe a stranger's children. + const evidencePid = + occupancy.state === 'unknown' + ? ((await readVerifiedDaemonPid(runtimeDir, socketPath, tokenPath))?.pid ?? null) + : null + occupancy = await raiseOccupancyWithProcessEvidence(occupancy, evidencePid) if (occupancy.state === 'occupied') { const owned = occupancy.liveSessions === null @@ -966,6 +971,9 @@ function createOutOfProcessLauncher( try { return await preserveDaemon('degraded-new-pty-fallback') } catch { + // Why not hold here, unlike the failed-health path: that one declined to kill + // because it had proof of live work. This one arrives with occupancy unknown or + // empty, so holding would swallow a real launch failure to protect nothing. // It stopped answering between the probe and the adoption; report the launch failure. } } diff --git a/src/main/daemon/daemon-live-pty-evidence.ts b/src/main/daemon/daemon-live-pty-evidence.ts index 5eaf5f63404..941ebbd6413 100644 --- a/src/main/daemon/daemon-live-pty-evidence.ts +++ b/src/main/daemon/daemon-live-pty-evidence.ts @@ -25,6 +25,7 @@ export type DaemonPtyOwnershipDeps = { readPosixProcessTable?: () => Promise queryWindowsDescendants?: typeof queryWindowsProcessDescendants windowsDeadlineMs?: number + posixDeadlineMs?: number } /** @@ -42,6 +43,12 @@ const PTY_OWNERSHIP_PROBE_ATTEMPTS = 2 */ const WINDOWS_OWNERSHIP_PROBE_DEADLINE_MS = 6_000 +/** + * POSIX needs its own ceiling for the same reason: the shared reader's `ps` timeout does not + * cover queueing behind an in-flight scan, and this runs on a launch that fails open. + */ +const POSIX_OWNERSHIP_PROBE_DEADLINE_MS = 4_000 + function withDeadline(work: Promise, deadlineMs: number, onDeadline: T): Promise { return new Promise((resolve) => { const timer = setTimeout(() => resolve(onDeadline), deadlineMs) @@ -156,10 +163,15 @@ async function probeOnce( ) return hosted.length > 0 ? 'owns-live-ptys' : 'no-live-ptys' } - const rows = await (deps.readPosixProcessTable ?? getFreshProcessTableSnapshot)() + const rows = await withDeadline( + (deps.readPosixProcessTable ?? getFreshProcessTableSnapshot)(), + deps.posixDeadlineMs ?? POSIX_OWNERSHIP_PROBE_DEADLINE_MS, + null + ) // Why: a walk that never saw the root reports zero descendants for a process it - // never examined. Only a root we actually observed can prove emptiness. - if (!rows.some((row) => row.pid === daemonPid)) { + // never examined. Only a root we actually observed can prove emptiness — and a read + // that blew its deadline saw nothing at all. + if (rows === null || !rows.some((row) => row.pid === daemonPid)) { return 'unknown' } return collectLivePtyDescendants(rows, daemonPid).length > 0 ? 'owns-live-ptys' : 'no-live-ptys' diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 3198281df9b..561234f64ca 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -1027,7 +1027,9 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra inputWriteQueue.clear() if (ptyId) { const id = ptyId - window.api.pty.kill(id) + // Why swallow: an unreachable terminal host rejects rather than pretending the + // session closed, and teardown must still run — but nothing here can act on it. + void Promise.resolve(window.api.pty.kill(id)).catch(() => {}) connected = false ptyId = null unregisterPtyHandlers(id) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 39b9edd20e1..afd3db641e3 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -3443,7 +3443,9 @@ export const createRepoSlice: StateCreator = (set, killedTabIds.add(tab.id) for (const ptyId of get().ptyIdsByTabId[tab.id] ?? []) { if (!ptyId.startsWith('remote:')) { - window.api.pty.kill(ptyId) + // Why swallow: an unreachable terminal host rejects instead of reporting a + // close it did not perform; removal proceeds either way. + void Promise.resolve(window.api.pty.kill(ptyId)).catch(() => {}) } } }