mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
fix(daemon): survive preserving a daemon too wedged to be adopted
Adversarial review found the veto's own success path could not complete against the daemon it exists to protect. Preserving routed through holdDaemonAdoptionLease(), which opens a hello — the exact operation a wedged daemon cannot answer — so it threw, aborted initDaemonPtyProvider, and left no spawner. restartDaemon() throws without one, so the user lost the documented Manage Sessions -> Restart remedy on top of having no daemon: strictly worse than the data loss being fixed. A still-listening endpoint means wedged, not gone, so keep a lease-free handle instead. The lease only cancels the adoption watchdog, which never fires on a daemon that owns sessions. Degraded mode likewise tolerates a lease and a session discovery it cannot complete. Three more from the same review: - The veto keyed on reason === 'failed_health_check', but a daemon that answered listSessions with 0 lands in that same branch and must stay replaceable. Key on liveSessionCount === null, which is what the option actually documents. - Zombies are not evidence of live work. A wedged daemon cannot reap, so its exited agents linger as <defunct> and would read as "still running" — a false positive correlated with the wedge itself. Enumerate through the process table's stat column and exclude them; sample twice so a resolver probe or health-check shell cannot masquerade as an agent. - Restore the "did anything answer?" log guard alongside the confirmed kill, so a daemon that self-retires before the kill is still announced. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -2926,9 +2926,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(killStaleDaemonMock).toHaveBeenCalledWith(
|
||||
FAKE_RUNTIME_DIR,
|
||||
'/fake/socket',
|
||||
'/fake/token',
|
||||
PROTOCOL_VERSION,
|
||||
{ preserveWhenOwningLivePtys: true }
|
||||
'/fake/token'
|
||||
)
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
})
|
||||
@@ -3197,6 +3195,53 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps a daemon handle when the preserved daemon is too wedged to be adopted', async () => {
|
||||
// Why: adoption needs a hello, which is exactly what a daemon wedged enough to be
|
||||
// preserved cannot answer. Throwing here would abort initDaemonPtyProvider, leaving no
|
||||
// spawner — and restartDaemon() throws without one, so the user loses the documented
|
||||
// Manage Sessions → Restart remedy on top of having no daemon.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const answeringDefault = function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {}),
|
||||
request: vi.fn(async () => ({ sessions: [] })),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
// Permanently wedged: every client, including the adoption client, fails its hello.
|
||||
daemonClientMock.mockImplementation(function MockWedgedDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
killStaleDaemonMock.mockResolvedValueOnce({ killed: false, liveOwnerSurvived: true })
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void>; mode?: string }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
|
||||
// The endpoint still listens, so the daemon is wedged rather than gone.
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
netConnectMock.mockImplementation(stubAliveSocketConnect)
|
||||
|
||||
try {
|
||||
const handle = await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(handle.mode).toBe('degraded-new-pty-fallback')
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
} finally {
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
})
|
||||
|
||||
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.
|
||||
expect(WEDGED_DAEMON_GRACE_RETRIES).toBeGreaterThanOrEqual(11)
|
||||
@@ -3292,9 +3337,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(killStaleDaemonMock).toHaveBeenCalledWith(
|
||||
FAKE_RUNTIME_DIR,
|
||||
'/fake/socket',
|
||||
'/fake/token',
|
||||
PROTOCOL_VERSION,
|
||||
{ preserveWhenOwningLivePtys: true }
|
||||
'/fake/token'
|
||||
)
|
||||
expect(forkMock).toHaveBeenCalled()
|
||||
// 'rejected' gets no grace window (probed once): count = initial adoption + rejected probe + fresh daemon lease.
|
||||
@@ -3427,9 +3470,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
expect(killStaleDaemonMock).toHaveBeenCalledWith(
|
||||
FAKE_RUNTIME_DIR,
|
||||
'/fake/socket',
|
||||
'/fake/token',
|
||||
PROTOCOL_VERSION,
|
||||
{ preserveWhenOwningLivePtys: true }
|
||||
'/fake/token'
|
||||
)
|
||||
expect(forkMock).toHaveBeenCalledWith(
|
||||
FAKE_DAEMON_ENTRY_PATH,
|
||||
|
||||
@@ -464,6 +464,7 @@ function createOutOfProcessLauncher(
|
||||
| undefined
|
||||
let confirmedReplacement = false
|
||||
let healthCheckReplacementVerdict: string | null = null
|
||||
let healthCheckReplacementAnnounced = false
|
||||
let adoptionClient: DaemonClient | null = new DaemonClient({
|
||||
socketPath,
|
||||
tokenPath
|
||||
@@ -603,10 +604,13 @@ function createOutOfProcessLauncher(
|
||||
)
|
||||
return preserveDaemon()
|
||||
}
|
||||
// Why: a cold start reaches this same fall-through with nothing to replace, but the
|
||||
// old "did anything answer?" guard also silenced the one case that destroys user
|
||||
// work — an unverifiable count from a daemon that never answered. Announce on the
|
||||
// post-kill truth instead: that is silent on a cold start because nothing was killed.
|
||||
// Why: a cold start reaches this same fall-through with nothing to replace, so stay
|
||||
// quiet unless something actually answered — a count, a socket that survived a grace
|
||||
// retry, or a refused hello. The confirmed-kill case is added below because the one
|
||||
// replacement that destroys user work, an unverifiable count from a daemon that never
|
||||
// answered, satisfies none of those and used to be killed in total silence.
|
||||
healthCheckReplacementAnnounced =
|
||||
liveSessionCount !== null || graceRetry > 0 || health === 'rejected'
|
||||
healthCheckReplacementVerdict = `health=${health}, liveSessions=${liveSessionCount ?? 'unverifiable'}, graceRetries=${graceRetry}`
|
||||
// Why: telemetry gates on confirmedReplacement below — the post-kill truth — so a
|
||||
// cold start that killed nothing never reports a replacement.
|
||||
@@ -619,11 +623,11 @@ function createOutOfProcessLauncher(
|
||||
// Why: a raw socket can outlive a broken daemon; kill by PID before respawn so the new daemon doesn't race the stale one.
|
||||
adoptionClient?.disconnect()
|
||||
adoptionClient = null
|
||||
// Why: only the unverifiable-state branch needs the live-PTY veto. The branches above
|
||||
// positively confirmed zero live sessions, and vetoing an explicit user restart would
|
||||
// make a wedged daemon unkillable.
|
||||
// Why: the veto answers "we could not verify", not "we failed the health check" — a
|
||||
// daemon that answered listSessions with 0 lands in this same branch and must stay
|
||||
// replaceable, as must an explicit user restart.
|
||||
const killOutcome =
|
||||
pendingReplacement?.reason === 'failed_health_check'
|
||||
pendingReplacement?.liveSessionCount === null
|
||||
? await killStaleDaemon(runtimeDir, socketPath, tokenPath, PROTOCOL_VERSION, {
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
@@ -639,13 +643,28 @@ function createOutOfProcessLauncher(
|
||||
try {
|
||||
return await preserveDaemon('degraded-new-pty-fallback')
|
||||
} catch {
|
||||
// Why: adoption needs a hello, which is exactly what a daemon wedged enough to be
|
||||
// preserved cannot answer. A still-listening endpoint means it is wedged, not gone,
|
||||
// so keep a lease-free handle: the lease only cancels the adoption watchdog, which
|
||||
// cannot fire on a daemon that still owns sessions, and throwing here would cost the
|
||||
// app its daemon handle — taking Manage Sessions → Restart down with it.
|
||||
if (await probeSocket(socketPath)) {
|
||||
return createPreservedDaemonHandle(
|
||||
runtimeDir,
|
||||
PROTOCOL_VERSION,
|
||||
'degraded-new-pty-fallback'
|
||||
)
|
||||
}
|
||||
// It died between the probe and the adoption; the endpoint is genuinely free now.
|
||||
throw new DaemonEndpointOwnershipError(
|
||||
'Daemon replacement aborted: the existing daemon could not be confirmed stopped'
|
||||
)
|
||||
}
|
||||
}
|
||||
if (killOutcome.killed && healthCheckReplacementVerdict) {
|
||||
if (
|
||||
healthCheckReplacementVerdict &&
|
||||
(healthCheckReplacementAnnounced || killOutcome.killed)
|
||||
) {
|
||||
console.warn(
|
||||
`[daemon] Replacing daemon that failed the health check (${healthCheckReplacementVerdict})`
|
||||
)
|
||||
@@ -996,7 +1015,13 @@ export async function initDaemonPtyProvider(
|
||||
let routedAdapter: DaemonProvider = newAdapter
|
||||
try {
|
||||
// Why: the launcher's temporary pair closes only after this permanent pair is established, leaving no adoption gap.
|
||||
await newAdapter.establishLifecycleLease()
|
||||
// Why tolerated in degraded mode: it exists for a daemon we deliberately kept without
|
||||
// being able to talk to it, so a lease it cannot grant must not abort init — that would
|
||||
// leave the app with no spawner at all, and restartDaemon() throws without one. The lease
|
||||
// only cancels the adoption watchdog, which never fires on a daemon that owns sessions.
|
||||
await (launchMode === 'degraded-new-pty-fallback'
|
||||
? newAdapter.establishLifecycleLease().catch(() => {})
|
||||
: newAdapter.establishLifecycleLease())
|
||||
releaseDaemonAdoptionLease(newSpawner.getHandle())
|
||||
|
||||
legacyAdapters = await createLegacyDaemonAdapters(runtimeDir)
|
||||
@@ -1017,7 +1042,9 @@ export async function initDaemonPtyProvider(
|
||||
: newAdapter
|
||||
if (routedAdapter instanceof DegradedDaemonPtyProvider) {
|
||||
// Why: preserved daemon can't create fresh terminals; discover its live session ids so only they route to it (fresh panes fall back locally).
|
||||
await routedAdapter.discoverDaemonSessions()
|
||||
// Best-effort for the same reason as the lease above: a wedged daemon answers nothing,
|
||||
// and routing everything locally is the correct degradation — not a reason to have no daemon.
|
||||
await routedAdapter.discoverDaemonSessions().catch(() => {})
|
||||
} else if (routedAdapter instanceof DaemonPtyRouter) {
|
||||
await routedAdapter.discoverLegacySessions()
|
||||
}
|
||||
|
||||
@@ -1,90 +1,124 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { inspectDaemonPtyOwnership } from './daemon-live-pty-evidence'
|
||||
import type { DescendantSnapshot, ProcessTableRow } from '../pty-descendant-termination'
|
||||
import type { ProcessTableRow } from '../../shared/process-table-snapshot'
|
||||
import type { WindowsProcessCandidate } from '../providers/windows-foreground-process-rows'
|
||||
|
||||
const DAEMON_PID = 4242
|
||||
|
||||
function posixRow(pid: number): ProcessTableRow {
|
||||
return { pid, ppid: DAEMON_PID, pgid: pid, startedAt: 'Mon Jul 13 12:54:47 2026' }
|
||||
function row(pid: number, ppid: number, overrides: Partial<ProcessTableRow> = {}): ProcessTableRow {
|
||||
return { pid, ppid, stat: 'Ss', command: '/bin/bash', ...overrides }
|
||||
}
|
||||
|
||||
function posixSnapshot(overrides: Partial<DescendantSnapshot> = {}): DescendantSnapshot {
|
||||
return { rootPgid: DAEMON_PID, descendants: [], capturedAtMs: 1_700_000_000_000, ...overrides }
|
||||
}
|
||||
const daemonRow = row(DAEMON_PID, 1, { command: 'daemon-entry.js' })
|
||||
|
||||
function windowsCandidate(pid: number): WindowsProcessCandidate {
|
||||
return { pid, ppid: DAEMON_PID, name: 'bash.exe', command: 'bash', executablePath: '', depth: 1 }
|
||||
}
|
||||
|
||||
function posixTable(rows: ProcessTableRow[]): () => Promise<ProcessTableRow[]> {
|
||||
return async () => rows
|
||||
}
|
||||
|
||||
describe('inspectDaemonPtyOwnership on POSIX', () => {
|
||||
it.each(['darwin', 'linux'] as const)(
|
||||
'reports live PTY ownership from descendants on %s',
|
||||
async (platform) => {
|
||||
const capturePosixDescendants = vi.fn(
|
||||
async (_pid: number, _deps?: { platform?: NodeJS.Platform; timeoutMs?: number }) =>
|
||||
posixSnapshot({ descendants: [posixRow(101), posixRow(102)] })
|
||||
)
|
||||
it.each(['darwin', 'linux'] as const)('reports live PTY ownership on %s', async (platform) => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform,
|
||||
readPosixProcessTable: posixTable([daemonRow, row(101, DAEMON_PID)])
|
||||
})
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
})
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform, capturePosixDescendants })
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
// The walk must run on the caller's platform, not the host's, and with a budget
|
||||
// generous enough to survive the load that wedged the daemon in the first place.
|
||||
expect(capturePosixDescendants.mock.calls[0]?.[1]).toMatchObject({ platform })
|
||||
expect(capturePosixDescendants.mock.calls[0]?.[1]?.timeoutMs ?? 0).toBeGreaterThanOrEqual(
|
||||
5_000
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
it('reports no live PTYs only when the root itself was observed', async () => {
|
||||
it('counts a grandchild, since macOS wraps every shell in login(1)', async () => {
|
||||
// daemon -> login(1) -> shell: a direct-children test would miss the agent entirely.
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
capturePosixDescendants: async () => posixSnapshot({ rootPgid: DAEMON_PID })
|
||||
readPosixProcessTable: posixTable([
|
||||
daemonRow,
|
||||
row(101, DAEMON_PID, { command: '/usr/bin/login -flpq nwparker' }),
|
||||
row(202, 101, { command: 'claude' })
|
||||
])
|
||||
})
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
})
|
||||
|
||||
it('reports no live PTYs for an observed root with no descendants', async () => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
readPosixProcessTable: posixTable([daemonRow, row(999, 1)])
|
||||
})
|
||||
).resolves.toBe('no-live-ptys')
|
||||
})
|
||||
|
||||
it('reports unknown when the walk never saw the root, despite zero descendants', async () => {
|
||||
// Why: an unobserved root yields the same empty list as a childless one —
|
||||
it('does not count zombies, which a wedged daemon cannot reap', async () => {
|
||||
// Why this matters: the daemon is wedged precisely because its event loop is blocked,
|
||||
// so every already-exited agent lingers as <defunct>. Counting them would read
|
||||
// "all agents finished" as "agents still running" — correlated with the wedge itself.
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
readPosixProcessTable: posixTable([
|
||||
daemonRow,
|
||||
row(101, DAEMON_PID, { stat: 'Z+', command: '<defunct>' }),
|
||||
row(102, DAEMON_PID, { stat: 'Z', command: '<defunct>' })
|
||||
])
|
||||
})
|
||||
).resolves.toBe('no-live-ptys')
|
||||
})
|
||||
|
||||
it('still counts a live descendant hidden behind a zombie parent', async () => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
readPosixProcessTable: posixTable([
|
||||
daemonRow,
|
||||
row(101, DAEMON_PID, { stat: 'Z', command: '<defunct>' }),
|
||||
row(202, 101, { command: 'codex' })
|
||||
])
|
||||
})
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
})
|
||||
|
||||
it('reports unknown when the table never contained the daemon', async () => {
|
||||
// Why: an unobserved root yields the same empty result as a childless one —
|
||||
// reading that as "empty" authorizes killing a daemon full of live agents.
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
capturePosixDescendants: async () => posixSnapshot({ rootPgid: null })
|
||||
readPosixProcessTable: posixTable([row(999, 1)])
|
||||
})
|
||||
).resolves.toBe('unknown')
|
||||
})
|
||||
|
||||
it('reports unknown when the process table could not be read', async () => {
|
||||
it('reports unknown when the process table cannot be read', async () => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
capturePosixDescendants: async () => null
|
||||
})
|
||||
).resolves.toBe('unknown')
|
||||
})
|
||||
|
||||
it('reports unknown when the capture throws', async () => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
capturePosixDescendants: async () => {
|
||||
readPosixProcessTable: async () => {
|
||||
throw new Error('ps timed out')
|
||||
}
|
||||
})
|
||||
).resolves.toBe('unknown')
|
||||
})
|
||||
|
||||
it('tolerates a ppid cycle without hanging', async () => {
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
readPosixProcessTable: posixTable([daemonRow, row(101, 102), row(102, 101)])
|
||||
})
|
||||
).resolves.toBe('no-live-ptys')
|
||||
})
|
||||
|
||||
it('never consults the Windows enumerator on POSIX', async () => {
|
||||
const queryWindowsDescendants = vi.fn(async () => [windowsCandidate(101)])
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'darwin',
|
||||
capturePosixDescendants: async () => posixSnapshot(),
|
||||
readPosixProcessTable: posixTable([daemonRow]),
|
||||
queryWindowsDescendants
|
||||
})
|
||||
).resolves.toBe('no-live-ptys')
|
||||
@@ -132,59 +166,75 @@ describe('inspectDaemonPtyOwnership on win32', () => {
|
||||
).resolves.toBe('unknown')
|
||||
})
|
||||
|
||||
it('never consults the POSIX walk on win32', async () => {
|
||||
const capturePosixDescendants = vi.fn(async () => posixSnapshot())
|
||||
it('never consults the POSIX table on win32', async () => {
|
||||
const readPosixProcessTable = vi.fn(async () => [daemonRow])
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, {
|
||||
platform: 'win32',
|
||||
capturePosixDescendants,
|
||||
readPosixProcessTable,
|
||||
queryWindowsDescendants: async () => null
|
||||
})
|
||||
).resolves.toBe('unknown')
|
||||
expect(capturePosixDescendants).not.toHaveBeenCalled()
|
||||
expect(readPosixProcessTable).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('inspectDaemonPtyOwnership retries', () => {
|
||||
it('retries a blind probe, because the load that wedges the daemon also blinds ps', async () => {
|
||||
const capturePosixDescendants = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(null)
|
||||
.mockResolvedValueOnce(posixSnapshot({ descendants: [posixRow(101)] }))
|
||||
describe('inspectDaemonPtyOwnership sampling', () => {
|
||||
it('discards a transient child that is gone by the second sample', async () => {
|
||||
// A resolver probe or PTY-spawn health check is a descendant for milliseconds; it must
|
||||
// not read as an agent and strand the daemon in degraded mode.
|
||||
const readPosixProcessTable = vi
|
||||
.fn<() => Promise<ProcessTableRow[]>>()
|
||||
.mockResolvedValueOnce([daemonRow, row(101, DAEMON_PID, { command: 'scutil --dns' })])
|
||||
.mockResolvedValueOnce([daemonRow])
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', capturePosixDescendants })
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
expect(capturePosixDescendants).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('stops retrying once the answer is conclusive', async () => {
|
||||
const capturePosixDescendants = vi.fn(async () => posixSnapshot({ rootPgid: DAEMON_PID }))
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', capturePosixDescendants })
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', readPosixProcessTable })
|
||||
).resolves.toBe('no-live-ptys')
|
||||
expect(capturePosixDescendants).toHaveBeenCalledTimes(1)
|
||||
expect(readPosixProcessTable).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('gives up as unknown rather than guessing when every probe stays blind', async () => {
|
||||
const capturePosixDescendants = vi.fn(async () => null)
|
||||
it('retries a blind read, because the load that wedges the daemon also blinds ps', async () => {
|
||||
const readPosixProcessTable = vi
|
||||
.fn<() => Promise<ProcessTableRow[]>>()
|
||||
.mockRejectedValueOnce(new Error('ps timed out'))
|
||||
.mockResolvedValueOnce([daemonRow, row(101, DAEMON_PID)])
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', capturePosixDescendants })
|
||||
).resolves.toBe('unknown')
|
||||
expect(capturePosixDescendants.mock.calls.length).toBeGreaterThan(1)
|
||||
})
|
||||
|
||||
it('retries a throwing probe too', async () => {
|
||||
const capturePosixDescendants = vi
|
||||
.fn()
|
||||
.mockRejectedValueOnce(new Error('ps died'))
|
||||
.mockResolvedValueOnce(posixSnapshot({ descendants: [posixRow(101)] }))
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', capturePosixDescendants })
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', readPosixProcessTable })
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
})
|
||||
|
||||
it('preserves on a sighting the second read could not contradict', async () => {
|
||||
// Why: a blind read is not evidence against a live one. Killing agents is unrecoverable.
|
||||
const readPosixProcessTable = vi
|
||||
.fn<() => Promise<ProcessTableRow[]>>()
|
||||
.mockResolvedValueOnce([daemonRow, row(101, DAEMON_PID)])
|
||||
.mockRejectedValueOnce(new Error('ps timed out'))
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', readPosixProcessTable })
|
||||
).resolves.toBe('owns-live-ptys')
|
||||
})
|
||||
|
||||
it('answers no-live-ptys on the first conclusive read without re-sampling', async () => {
|
||||
const readPosixProcessTable = vi.fn(async () => [daemonRow])
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', readPosixProcessTable })
|
||||
).resolves.toBe('no-live-ptys')
|
||||
expect(readPosixProcessTable).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('gives up as unknown rather than guessing when every read stays blind', async () => {
|
||||
const readPosixProcessTable = vi.fn(async (): Promise<ProcessTableRow[]> => {
|
||||
throw new Error('ps timed out')
|
||||
})
|
||||
|
||||
await expect(
|
||||
inspectDaemonPtyOwnership(DAEMON_PID, { platform: 'darwin', readPosixProcessTable })
|
||||
).resolves.toBe('unknown')
|
||||
expect(readPosixProcessTable.mock.calls.length).toBeGreaterThan(1)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
import { captureDescendantSnapshot } from '../pty-descendant-termination'
|
||||
import {
|
||||
getFreshProcessTableSnapshot,
|
||||
type ProcessTableRow
|
||||
} from '../../shared/process-table-snapshot'
|
||||
import { queryWindowsProcessDescendants } from '../providers/windows-foreground-process-rows'
|
||||
|
||||
/**
|
||||
@@ -16,19 +19,58 @@ export type DaemonPtyOwnership = 'owns-live-ptys' | 'no-live-ptys' | 'unknown'
|
||||
|
||||
export type DaemonPtyOwnershipDeps = {
|
||||
platform?: NodeJS.Platform
|
||||
capturePosixDescendants?: typeof captureDescendantSnapshot
|
||||
readPosixProcessTable?: () => Promise<ProcessTableRow[]>
|
||||
queryWindowsDescendants?: typeof queryWindowsProcessDescendants
|
||||
}
|
||||
|
||||
/**
|
||||
* Why generous, and why retried: the load that wedges the daemon is the same load
|
||||
* that makes `ps` miss its default 1s budget, so the evidence would go blind exactly
|
||||
* when it matters most. This runs only on the replace path, after ~60s of grace has
|
||||
* already been spent — seconds here are free next to killing a live agent.
|
||||
* Why generous, and why sampled twice: the load that wedges the daemon is the same
|
||||
* load that makes `ps` miss a tight budget, so the evidence would go blind exactly
|
||||
* when it matters most. A positive is confirmed by a second sample so a short-lived
|
||||
* child — a resolver probe, a health-check shell — cannot masquerade as an agent.
|
||||
* This runs only on the replace path, after ~60s of grace is already spent.
|
||||
*/
|
||||
const PTY_OWNERSHIP_PROBE_TIMEOUT_MS = 5_000
|
||||
const PTY_OWNERSHIP_PROBE_ATTEMPTS = 2
|
||||
|
||||
// A wedged daemon cannot reap, so its exited PTYs linger as zombies. Counting them
|
||||
// would read "every agent already exited" as "agents still running" — and that
|
||||
// false positive is systematically correlated with the wedge we are diagnosing.
|
||||
function isZombie(row: ProcessTableRow): boolean {
|
||||
return row.stat.startsWith('Z')
|
||||
}
|
||||
|
||||
function collectLiveDescendants(rows: ProcessTableRow[], rootPid: number): ProcessTableRow[] {
|
||||
const childrenByPpid = new Map<number, ProcessTableRow[]>()
|
||||
for (const row of rows) {
|
||||
if (row.pid === rootPid) {
|
||||
continue
|
||||
}
|
||||
const siblings = childrenByPpid.get(row.ppid)
|
||||
if (siblings) {
|
||||
siblings.push(row)
|
||||
} else {
|
||||
childrenByPpid.set(row.ppid, [row])
|
||||
}
|
||||
}
|
||||
const visited = new Set<number>([rootPid])
|
||||
const queue: number[] = [rootPid]
|
||||
const live: ProcessTableRow[] = []
|
||||
while (queue.length > 0) {
|
||||
const pid = queue.shift() as number
|
||||
for (const child of childrenByPpid.get(pid) ?? []) {
|
||||
if (visited.has(child.pid)) {
|
||||
continue
|
||||
}
|
||||
visited.add(child.pid)
|
||||
queue.push(child.pid)
|
||||
if (!isZombie(child)) {
|
||||
live.push(child)
|
||||
}
|
||||
}
|
||||
}
|
||||
return live
|
||||
}
|
||||
|
||||
async function probeOnce(
|
||||
daemonPid: number,
|
||||
deps: DaemonPtyOwnershipDeps,
|
||||
@@ -45,39 +87,42 @@ async function probeOnce(
|
||||
}
|
||||
return descendants.length > 0 ? 'owns-live-ptys' : 'no-live-ptys'
|
||||
}
|
||||
const snapshot = await (deps.capturePosixDescendants ?? captureDescendantSnapshot)(daemonPid, {
|
||||
platform,
|
||||
timeoutMs: PTY_OWNERSHIP_PROBE_TIMEOUT_MS
|
||||
})
|
||||
// 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 (!snapshot || snapshot.rootPgid === null) {
|
||||
const rows = await (deps.readPosixProcessTable ?? getFreshProcessTableSnapshot)()
|
||||
// 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)) {
|
||||
return 'unknown'
|
||||
}
|
||||
return snapshot.descendants.length > 0 ? 'owns-live-ptys' : 'no-live-ptys'
|
||||
return collectLiveDescendants(rows, daemonPid).length > 0 ? 'owns-live-ptys' : 'no-live-ptys'
|
||||
}
|
||||
|
||||
/**
|
||||
* PTY children are the daemon's only long-lived descendants, so a non-empty
|
||||
* descendant set is positive proof that killing it would destroy running work.
|
||||
* Descendants rather than direct children: macOS wraps every shell in login(1)
|
||||
* for TCC attribution, so the agent is a grandchild at best.
|
||||
* PTY children are the daemon's only long-lived descendants, so a descendant that
|
||||
* survives two samples is positive proof that killing it would destroy running work.
|
||||
* Descendants rather than direct children: macOS wraps every shell in login(1) for
|
||||
* TCC attribution, so the agent is a grandchild at best.
|
||||
*/
|
||||
export async function inspectDaemonPtyOwnership(
|
||||
daemonPid: number,
|
||||
deps: DaemonPtyOwnershipDeps = {}
|
||||
): Promise<DaemonPtyOwnership> {
|
||||
const platform = deps.platform ?? process.platform
|
||||
let ownership: DaemonPtyOwnership = 'unknown'
|
||||
let sawLivePtys = false
|
||||
for (let attempt = 0; attempt < PTY_OWNERSHIP_PROBE_ATTEMPTS; attempt++) {
|
||||
let sample: DaemonPtyOwnership
|
||||
try {
|
||||
ownership = await probeOnce(daemonPid, deps, platform)
|
||||
sample = await probeOnce(daemonPid, deps, platform)
|
||||
} catch {
|
||||
ownership = 'unknown'
|
||||
sample = 'unknown'
|
||||
}
|
||||
if (ownership !== 'unknown') {
|
||||
return ownership
|
||||
// An observed root with no live descendants is conclusive, and it settles the transient
|
||||
// case too: a resolver probe or health-check shell seen in one sample is gone by the next.
|
||||
if (sample === 'no-live-ptys') {
|
||||
return sample
|
||||
}
|
||||
sawLivePtys ||= sample === 'owns-live-ptys'
|
||||
}
|
||||
return ownership
|
||||
// Why a single sighting is enough to preserve: a blind second read is not evidence against
|
||||
// the first. Killing live agents is unrecoverable; preserving costs one degraded launch.
|
||||
return sawLivePtys ? 'owns-live-ptys' : 'unknown'
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user