From 70b22a4dcdb9740541e1286859f9f85638a734ca Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:46:37 -0700 Subject: [PATCH] fix(daemon): stop discarding Linux terminals to solve a macOS problem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stranded-login(1) exclusion ran on every POSIX host. Orca only wraps terminals in login(1) for TCC attribution on macOS, so off darwin the pattern can only match a user's own login — and one still prompting for credentials has no child yet, which is precisely the shape the exclusion throws away. A Linux daemon hosting that terminal read as childless, and a childless daemon is one nothing protects from the endpoint-dead path. Now scoped to darwin, where the problem it solves lives. And a count that is not a count is no longer read as emptiness. `counted > 0` maps NaN, -1 and 1.5 to 'empty', which is the single verdict that licenses a kill; the listSessions dep is injectable, so reaching it never required asking a daemon anything. Non-integers and negatives now resolve to 'unknown'. Both mutation-verified. Noting for whoever runs the next mutation pass: the first attempt at the login mutation silently failed to apply because the pattern had been reflowed by the formatter, and a mutation that does not apply looks exactly like a test suite that caught it. Assert the pattern matched. --- .../daemon/daemon-live-pty-evidence.test.ts | 33 +++++++++++++++++++ src/main/daemon/daemon-live-pty-evidence.ts | 28 +++++++++++++--- src/main/daemon/daemon-occupancy.test.ts | 13 ++++++++ src/main/daemon/daemon-occupancy.ts | 6 ++++ 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/main/daemon/daemon-live-pty-evidence.test.ts b/src/main/daemon/daemon-live-pty-evidence.test.ts index d1d12f9bee2..aad786f2cfd 100644 --- a/src/main/daemon/daemon-live-pty-evidence.test.ts +++ b/src/main/daemon/daemon-live-pty-evidence.test.ts @@ -270,6 +270,39 @@ describe('inspectDaemonPtyOwnership on win32', () => { }) }) +describe('inspectDaemonPtyOwnership login(1) handling', () => { + it('counts a childless login(1) as live work off darwin', async () => { + // Orca only wraps terminals in login(1) on macOS, so elsewhere this pattern is the user's + // own login — and one still prompting for credentials has no child yet. Excluding it there + // discards real work to solve a macOS problem. + const rows = [ + daemonRow, + row(5000, DAEMON_PID, { stat: 'Ss', command: '/usr/bin/login nwparker' }) + ] + + await expect( + inspectDaemonPtyOwnership(DAEMON_PID, { + platform: 'linux', + readPosixProcessTable: async () => rows + }) + ).resolves.toBe('owns-live-ptys') + }) + + it('still excludes a childless login(1) on darwin (#13764)', async () => { + const rows = [ + daemonRow, + row(5000, DAEMON_PID, { stat: 'Ss', command: '/usr/bin/login -pf nwparker /bin/zsh' }) + ] + + await expect( + inspectDaemonPtyOwnership(DAEMON_PID, { + platform: 'darwin', + readPosixProcessTable: async () => rows + }) + ).resolves.toBe('no-live-ptys') + }) +}) + describe('inspectDaemonPtyOwnership sampling', () => { it('will not confirm emptiness from the cached table the first read already used', async () => { // Two agreeing samples are only worth more than one if they are two observations. When the diff --git a/src/main/daemon/daemon-live-pty-evidence.ts b/src/main/daemon/daemon-live-pty-evidence.ts index e77e47c2710..7e6c7d7ab8d 100644 --- a/src/main/daemon/daemon-live-pty-evidence.ts +++ b/src/main/daemon/daemon-live-pty-evidence.ts @@ -107,11 +107,27 @@ function isLivePtySessionLeader(row: ProcessTableRow): boolean { * would hold a daemon whose sessions have all ended, on hosts where they accumulate by the * hundred. A wrapper still doing its job always has the shell it exec'd beneath it. */ -function isStrandedLoginWrapper(row: ProcessTableRow, hasChildren: boolean): boolean { - return !hasChildren && row.command.trim().startsWith(MACOS_LOGIN_WRAPPER_PREFIX) +function isStrandedLoginWrapper( + row: ProcessTableRow, + hasChildren: boolean, + platform: NodeJS.Platform +): boolean { + // Why darwin only: Orca wraps terminals in login(1) for TCC attribution on macOS and nowhere + // else, so off darwin this pattern can only ever be a user's own login(1) — and one that is + // still prompting for credentials has no child yet, which is exactly the shape excluded here. + // Applied POSIX-wide it discarded real work to solve a macOS problem. + return ( + platform === 'darwin' && + !hasChildren && + row.command.trim().startsWith(MACOS_LOGIN_WRAPPER_PREFIX) + ) } -function collectLivePtyDescendants(rows: ProcessTableRow[], rootPid: number): ProcessTableRow[] { +function collectLivePtyDescendants( + rows: ProcessTableRow[], + rootPid: number, + platform: NodeJS.Platform +): ProcessTableRow[] { const childrenByPpid = new Map() for (const row of rows) { if (row.pid === rootPid) { @@ -137,7 +153,7 @@ function collectLivePtyDescendants(rows: ProcessTableRow[], rootPid: number): Pr queue.push(child.pid) if ( isLivePtySessionLeader(child) && - !isStrandedLoginWrapper(child, childrenByPpid.has(child.pid)) + !isStrandedLoginWrapper(child, childrenByPpid.has(child.pid), platform) ) { live.push(child) } @@ -187,7 +203,9 @@ async function probeOnce( if (rows === null || !rows.some((row) => row.pid === daemonPid)) { return 'unknown' } - return collectLivePtyDescendants(rows, daemonPid).length > 0 ? 'owns-live-ptys' : 'no-live-ptys' + return collectLivePtyDescendants(rows, daemonPid, deps.platform ?? process.platform).length > 0 + ? 'owns-live-ptys' + : 'no-live-ptys' } /** diff --git a/src/main/daemon/daemon-occupancy.test.ts b/src/main/daemon/daemon-occupancy.test.ts index fbfe9135e56..fc3599178eb 100644 --- a/src/main/daemon/daemon-occupancy.test.ts +++ b/src/main/daemon/daemon-occupancy.test.ts @@ -254,3 +254,16 @@ describe('resolveDaemonOccupancy when an injected dep throws', () => { ).resolves.toEqual({ state: 'unknown', liveSessions: null }) }) }) + +describe('resolveDaemonOccupancy with a nonsense count', () => { + it.each([Number.NaN, -1, 1.5])('refuses to read %p as emptiness', async (counted) => { + // 'empty' is the only verdict that licenses a kill, and `counted > 0` reads every one of + // these as empty. The dep is injectable, so it is reachable without asking the daemon. + await expect( + resolve({ + listSessions: async () => counted, + inspectPtyOwnership: ownershipIs('unknown') + }) + ).resolves.toEqual({ state: 'unknown', liveSessions: null }) + }) +}) diff --git a/src/main/daemon/daemon-occupancy.ts b/src/main/daemon/daemon-occupancy.ts index eb175d97217..31d4c67b40e 100644 --- a/src/main/daemon/daemon-occupancy.ts +++ b/src/main/daemon/daemon-occupancy.ts @@ -106,6 +106,12 @@ export async function resolveDaemonOccupancy(args: { connectBudgetMs ) if (counted !== null) { + // Why validate a number we just asked for: 'empty' is the one verdict that licenses a + // kill, and `counted > 0` quietly reads NaN, -1 and every other non-count as emptiness. + // The dep is injectable, so that is reachable without the daemon ever being asked. + if (!Number.isInteger(counted) || counted < 0) { + return unknown + } return counted > 0 ? { state: 'occupied', liveSessions: counted } : { state: 'empty', liveSessions: 0 }