mirror of
https://github.com/stablyai/orca.git
synced 2026-10-29 16:17:51 +00:00
fix(daemon): stop discarding Linux terminals to solve a macOS problem
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<number, ProcessTableRow[]>()
|
||||
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'
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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 }
|
||||
|
||||
Reference in New Issue
Block a user