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 }