mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 16:03:47 +00:00
refactor(daemon): decide occupancy before the kill, not inside it
Three review rounds each found a new failure state in the previous shape, which was the design telling us something. The safety rule — never destroy running work — was replicated across the launcher's branches instead of being decided once, and the last round added it to one more branch behind a boolean. Policy had been put inside a mechanism: killStaleDaemon grew an input flag to disable its new veto, an output back-channel to report it, and a caller-side re-derivation of the classification the flag had lost. One structural error, one symptom per layer it crossed. Name the question instead. resolveDaemonOccupancy answers occupied | empty | unknown, asking the daemon first (authoritative both ways) and falling back to the process table only to RAISE the answer to occupied. OS evidence can prove work exists; it can never prove absence, so it never licenses a kill. The launcher now decides before it destroys anything, so killStaleDaemon goes back to being only "make this pid go away" — no options, nothing for a future caller to forget to disable, and Manage Sessions -> Restart cannot be vetoed because there is no veto left to hit. Holding is a real outcome now. A daemon that owns live terminals but cannot answer a handshake gets mode 'held': no adoption attempt, no lease, no fork beside it. That deletes the lease-free-handle fallback, the try/catch around preserve, and the tolerated-lease branch in init that existed only because the correct outcome had no representation. Two defects this removes outright: - The endpoint check used a local boolean probe that returns false on timeout, so under load — and unconditionally on Windows named pipes, where a busy server answers ERROR_PIPE_BUSY — the guard disabled itself in exactly the conditions it was written for. Use the canonical three-valued probe, whose own docs say absence of proof is not proof of death. - A daemon that answered and refused the handshake was killed with its agents. It is now held like any other occupied daemon. Also folds the replacement verdict onto pendingReplacement, retiring a pair of mutable launcher locals whose only job was moving one warning past the kill. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -22,16 +22,8 @@ import {
|
||||
startTimesWithinTolerance
|
||||
} from './daemon-health'
|
||||
import type { SubprocessHandle } from './session'
|
||||
import type { DaemonPtyOwnership } from './daemon-live-pty-evidence'
|
||||
|
||||
// Why: the veto's production default is otherwise never exercised — every other test injects it.
|
||||
const { inspectDaemonPtyOwnershipMock } = vi.hoisted(() => ({
|
||||
inspectDaemonPtyOwnershipMock: vi.fn(async (_pid: number) => 'unknown' as DaemonPtyOwnership)
|
||||
}))
|
||||
|
||||
vi.mock('./daemon-live-pty-evidence', () => ({
|
||||
inspectDaemonPtyOwnership: inspectDaemonPtyOwnershipMock
|
||||
}))
|
||||
|
||||
function createMockSubprocess(): SubprocessHandle {
|
||||
return {
|
||||
@@ -597,151 +589,3 @@ describe('killStaleDaemon ownership decisions', () => {
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
describe.skipIf(process.platform === 'win32')('killStaleDaemon live PTY ownership veto', () => {
|
||||
let dir: string
|
||||
let socketPath: string
|
||||
let tokenPath: string
|
||||
let stubPid: number
|
||||
let stubExited: Promise<void>
|
||||
|
||||
beforeEach(async () => {
|
||||
dir = mkdtempSync(join(tmpdir(), 'daemon-health-pty-veto-test-'))
|
||||
socketPath = daemonTestSocketPath(dir)
|
||||
tokenPath = join(dir, 'daemon.token')
|
||||
// A real process is required: identity 'match' is decided from the live command line.
|
||||
const child = spawn(
|
||||
process.execPath,
|
||||
[
|
||||
'-e',
|
||||
"console.log('ready'); setInterval(() => {}, 1000)",
|
||||
'daemon-entry',
|
||||
socketPath,
|
||||
tokenPath
|
||||
],
|
||||
{ stdio: ['ignore', 'pipe', 'ignore'] }
|
||||
)
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
child.once('error', reject)
|
||||
child.stdout?.once('data', () => resolve())
|
||||
})
|
||||
stubPid = child.pid as number
|
||||
stubExited = new Promise<void>((resolve) => child.once('exit', () => resolve()))
|
||||
writeFileSync(
|
||||
getDaemonPidPath(dir),
|
||||
serializeDaemonPidFile({ pid: stubPid, startedAtMs: null, launchNonce: 'daemon-a' }),
|
||||
{ mode: 0o600 }
|
||||
)
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
try {
|
||||
process.kill(stubPid, 'SIGKILL')
|
||||
} catch {
|
||||
// Already gone.
|
||||
}
|
||||
await stubExited
|
||||
rmSync(dir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
function terminationSignals(killSpy: { mock: { calls: unknown[][] } }): unknown[][] {
|
||||
return killSpy.mock.calls.filter(([, sig]) => sig === 'SIGTERM' || sig === 'SIGKILL')
|
||||
}
|
||||
|
||||
it('sends no signal at all when the daemon still owns live PTYs', async () => {
|
||||
// Why: a wedged daemon cannot list its own sessions, so descendants are the
|
||||
// only evidence left that replacing it would destroy running agents.
|
||||
const inspectPtyOwnership = vi.fn(async () => 'owns-live-ptys' as const)
|
||||
const killSpy = vi.spyOn(process, 'kill')
|
||||
|
||||
try {
|
||||
await expect(
|
||||
killStaleDaemon(dir, socketPath, tokenPath, undefined, {
|
||||
probeEndpoint: async () => 'missing',
|
||||
inspectPtyOwnership,
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
).resolves.toEqual({ killed: false, liveOwnerSurvived: true })
|
||||
expect(inspectPtyOwnership).toHaveBeenCalledWith(stubPid)
|
||||
expect(terminationSignals(killSpy)).toEqual([])
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
expect(() => process.kill(stubPid, 0)).not.toThrow()
|
||||
expect(readFileSync(getDaemonPidPath(dir), 'utf8')).toContain('daemon-a')
|
||||
})
|
||||
|
||||
it('still SIGTERMs a wedged daemon that owns no live PTYs', async () => {
|
||||
// #8689: nothing to lose, so an unresponsive daemon must stay replaceable.
|
||||
const killSpy = vi.spyOn(process, 'kill')
|
||||
|
||||
try {
|
||||
await expect(
|
||||
killStaleDaemon(dir, socketPath, tokenPath, undefined, {
|
||||
probeEndpoint: async () => 'missing',
|
||||
inspectPtyOwnership: async () => 'no-live-ptys',
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
).resolves.toEqual({ killed: true, liveOwnerSurvived: false })
|
||||
expect(killSpy).toHaveBeenCalledWith(stubPid, 'SIGTERM')
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('still SIGTERMs when PTY ownership cannot be determined', async () => {
|
||||
// Residual by design: only positive evidence of live PTYs vetoes the kill.
|
||||
const killSpy = vi.spyOn(process, 'kill')
|
||||
|
||||
try {
|
||||
await expect(
|
||||
killStaleDaemon(dir, socketPath, tokenPath, undefined, {
|
||||
probeEndpoint: async () => 'missing',
|
||||
inspectPtyOwnership: async () => 'unknown',
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
).resolves.toEqual({ killed: true, liveOwnerSurvived: false })
|
||||
expect(killSpy).toHaveBeenCalledWith(stubPid, 'SIGTERM')
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('routes the opt-in through the real ownership inspector when no hook is injected', async () => {
|
||||
// Why: production passes no hook, so a default that never sees live PTYs would ship a
|
||||
// veto that silently never fires — indistinguishable from having no veto at all.
|
||||
inspectDaemonPtyOwnershipMock.mockResolvedValueOnce('owns-live-ptys')
|
||||
const killSpy = vi.spyOn(process, 'kill')
|
||||
|
||||
try {
|
||||
await expect(
|
||||
killStaleDaemon(dir, socketPath, tokenPath, undefined, {
|
||||
probeEndpoint: async () => 'missing',
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
).resolves.toEqual({ killed: false, liveOwnerSurvived: true })
|
||||
expect(inspectDaemonPtyOwnershipMock).toHaveBeenCalledWith(stubPid)
|
||||
expect(terminationSignals(killSpy)).toEqual([])
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('never consults PTY ownership without the opt-in, so restarts stay killable', async () => {
|
||||
const inspectPtyOwnership = vi.fn(async () => 'owns-live-ptys' as const)
|
||||
const killSpy = vi.spyOn(process, 'kill')
|
||||
|
||||
try {
|
||||
await expect(
|
||||
killStaleDaemon(dir, socketPath, tokenPath, undefined, {
|
||||
probeEndpoint: async () => 'missing',
|
||||
inspectPtyOwnership
|
||||
})
|
||||
).resolves.toEqual({ killed: true, liveOwnerSurvived: false })
|
||||
expect(inspectPtyOwnership).not.toHaveBeenCalled()
|
||||
expect(killSpy).toHaveBeenCalledWith(stubPid, 'SIGTERM')
|
||||
} finally {
|
||||
killSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -10,7 +10,6 @@ import {
|
||||
} from '../../shared/process-output-field-scanner'
|
||||
import { isStartupDiagnosticsEnabled, logStartupDiagnostic } from '../startup/startup-diagnostics'
|
||||
import { encodeNdjson } from './ndjson'
|
||||
import { inspectDaemonPtyOwnership, type DaemonPtyOwnership } from './daemon-live-pty-evidence'
|
||||
import {
|
||||
getDaemonPidPath,
|
||||
unlinkDaemonPidFileWhen,
|
||||
@@ -613,7 +612,7 @@ export async function getDaemonLaunchIdentity(
|
||||
return commandLine.includes(expectedEntryPath) ? 'match' : 'mismatch'
|
||||
}
|
||||
|
||||
async function readVerifiedDaemonPid(
|
||||
export async function readVerifiedDaemonPid(
|
||||
runtimeDir: string,
|
||||
socketPath: string,
|
||||
tokenPath: string,
|
||||
@@ -732,18 +731,6 @@ async function waitForProcessExit(pid: number, timeoutMs: number): Promise<boole
|
||||
*/
|
||||
export type StaleDaemonKillTestHooks = {
|
||||
probeEndpoint?: (socketPath: string) => Promise<SocketProbeOutcome>
|
||||
inspectPtyOwnership?: (daemonPid: number) => Promise<DaemonPtyOwnership>
|
||||
}
|
||||
|
||||
export type StaleDaemonKillOptions = {
|
||||
/**
|
||||
* Refuse to signal a daemon whose process still owns live PTY children.
|
||||
*
|
||||
* Only for callers replacing on *unverifiable* state. Callers that positively
|
||||
* confirmed zero live sessions — or that are carrying out an explicit user
|
||||
* restart — must leave this off, or the daemon becomes unkillable.
|
||||
*/
|
||||
preserveWhenOwningLivePtys?: boolean
|
||||
}
|
||||
|
||||
export type StaleDaemonKillOutcome = {
|
||||
@@ -762,12 +749,9 @@ export async function killStaleDaemon(
|
||||
socketPath: string,
|
||||
tokenPath: string,
|
||||
protocolVersion = PROTOCOL_VERSION,
|
||||
// Why: one trailing bag rather than two optional params — a caller that skipped the
|
||||
// hooks slot silently landed its options in it, disabling the veto with no type error.
|
||||
options?: StaleDaemonKillTestHooks & StaleDaemonKillOptions
|
||||
testHooks?: StaleDaemonKillTestHooks
|
||||
): Promise<StaleDaemonKillOutcome> {
|
||||
const probeEndpoint = options?.probeEndpoint ?? probeSocketConnect
|
||||
const inspectPtyOwnership = options?.inspectPtyOwnership ?? inspectDaemonPtyOwnership
|
||||
const probeEndpoint = testHooks?.probeEndpoint ?? probeSocketConnect
|
||||
const pidPath = getDaemonPidPath(runtimeDir, protocolVersion)
|
||||
let killedDaemon = false
|
||||
let liveOwnerSurvived = false
|
||||
@@ -802,18 +786,6 @@ export async function killStaleDaemon(
|
||||
}
|
||||
if (parsedPid && identity === 'match') {
|
||||
const { pid, startedAtMs } = parsedPid
|
||||
// Why: identity is already 'match', so these descendants provably belong to this
|
||||
// daemon incarnation — not to a recycled pid. A wedged daemon cannot list its own
|
||||
// sessions, and treating that silence as "empty" is what killed live agents.
|
||||
if (options?.preserveWhenOwningLivePtys) {
|
||||
const ownership = await inspectPtyOwnership(pid)
|
||||
if (ownership === 'owns-live-ptys') {
|
||||
console.warn(
|
||||
'[daemon] Preserving daemon that could not report its sessions: its process still owns live terminal processes'
|
||||
)
|
||||
return { killed: false, liveOwnerSurvived: true }
|
||||
}
|
||||
}
|
||||
try {
|
||||
process.kill(pid, 'SIGTERM')
|
||||
} catch (error) {
|
||||
|
||||
@@ -28,6 +28,8 @@ const {
|
||||
getDaemonLaunchIdentityMock,
|
||||
isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemonMock,
|
||||
readVerifiedDaemonPidMock,
|
||||
inspectDaemonPtyOwnershipMock,
|
||||
getProcessStartedAtMsMock,
|
||||
parseDaemonPidFileMock,
|
||||
replaceDaemonPidFileMock,
|
||||
@@ -91,6 +93,10 @@ const {
|
||||
const getMacDaemonTccAttributionHealthMock = vi.fn(async () => 'unknown')
|
||||
const getDaemonLaunchIdentityMock = vi.fn(() => 'match')
|
||||
const isDaemonStaleForCurrentBundleMock = vi.fn(() => false)
|
||||
const inspectDaemonPtyOwnershipMock = vi.fn(
|
||||
async (): Promise<'owns-live-ptys' | 'no-live-ptys' | 'unknown'> => 'unknown'
|
||||
)
|
||||
const readVerifiedDaemonPidMock = vi.fn(async (): Promise<{ pid: number } | null> => null)
|
||||
const killStaleDaemonMock = vi.fn(async () => ({
|
||||
killed: true,
|
||||
liveOwnerSurvived: false
|
||||
@@ -196,6 +202,8 @@ const {
|
||||
getDaemonLaunchIdentityMock,
|
||||
isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemonMock,
|
||||
readVerifiedDaemonPidMock,
|
||||
inspectDaemonPtyOwnershipMock,
|
||||
getProcessStartedAtMsMock,
|
||||
parseDaemonPidFileMock,
|
||||
replaceDaemonPidFileMock,
|
||||
@@ -265,6 +273,17 @@ vi.mock('electron', () => ({
|
||||
}
|
||||
}))
|
||||
|
||||
// Map the existing boolean socket double onto the canonical three-valued probe:
|
||||
// present ⇒ something is serving, absent ⇒ positively dead.
|
||||
vi.mock('./daemon-live-pty-evidence', () => ({
|
||||
inspectDaemonPtyOwnership: inspectDaemonPtyOwnershipMock
|
||||
}))
|
||||
|
||||
vi.mock('./daemon-endpoint-probe', async (importOriginal) => ({
|
||||
...(await importOriginal<Record<string, unknown>>()),
|
||||
probeSocketConnect: async (p: string) => (probeSocketExistsMock(p) ? 'connected' : 'missing')
|
||||
}))
|
||||
|
||||
vi.mock('fs', () => ({
|
||||
mkdirSync: vi.fn(),
|
||||
existsSync: (p: string) => probeSocketExistsMock(p) || p.includes('.pid'),
|
||||
@@ -289,6 +308,7 @@ vi.mock('./daemon-health', () => ({
|
||||
healthCheckDaemon: healthCheckDaemonMock,
|
||||
isDaemonStaleForCurrentBundle: isDaemonStaleForCurrentBundleMock,
|
||||
killStaleDaemon: killStaleDaemonMock,
|
||||
readVerifiedDaemonPid: readVerifiedDaemonPidMock,
|
||||
getProcessStartedAtMs: getProcessStartedAtMsMock,
|
||||
parseDaemonPidFile: parseDaemonPidFileMock
|
||||
}))
|
||||
@@ -3156,9 +3176,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()
|
||||
// The launcher probes the full grace budget: 1 initial probe + WEDGED_DAEMON_GRACE_RETRIES retries.
|
||||
@@ -3178,11 +3196,9 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('adopts a wedged daemon that still owns live PTYs instead of killing its agents', async () => {
|
||||
// Why: the regression that motivated the veto. A daemon too wedged to answer listSessions
|
||||
// still hosts every running agent; killing it destroys work that no restore can recover.
|
||||
// killStaleDaemon owns the veto, so the launcher's contract is to opt in here and to route
|
||||
// the refusal into degraded adoption rather than forking a second daemon beside it.
|
||||
it('holds a wedged daemon that still owns live terminals instead of killing its agents', async () => {
|
||||
// Why hold and not adopt: a daemon that cannot answer listSessions cannot answer a hello,
|
||||
// so there is nothing to adopt. The kill must never be attempted — its agents are alive.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
@@ -3193,22 +3209,19 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
let daemonClientConstructionCount = 0
|
||||
daemonClientMock.mockImplementation(function MockWedgedDaemonClient() {
|
||||
daemonClientConstructionCount++
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
if (daemonClientConstructionCount <= 2 + WEDGED_DAEMON_GRACE_RETRIES) {
|
||||
throw new Error('Hello response timed out')
|
||||
}
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
getDaemonIdentity: vi.fn(readLaunchedDaemonIdentity),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
// The real killStaleDaemon refuses and reports the owner alive; this is that verdict.
|
||||
killStaleDaemonMock.mockResolvedValueOnce({ killed: false, liveOwnerSurvived: true })
|
||||
// The daemon is identity-verified and its process still owns live terminals.
|
||||
readVerifiedDaemonPidMock.mockResolvedValue({ pid: 4242 })
|
||||
inspectDaemonPtyOwnershipMock.mockResolvedValue('owns-live-ptys')
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
@@ -3218,27 +3231,16 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
netConnectMock.mockImplementation(stubAliveSocketConnect)
|
||||
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
try {
|
||||
const handle = await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
expect(killStaleDaemonMock).toHaveBeenCalledWith(
|
||||
FAKE_RUNTIME_DIR,
|
||||
'/fake/socket',
|
||||
'/fake/token',
|
||||
PROTOCOL_VERSION,
|
||||
{ preserveWhenOwningLivePtys: true }
|
||||
)
|
||||
// No replacement daemon beside the one still hosting the agents.
|
||||
expect(handle.mode).toBe('held')
|
||||
expect(killStaleDaemonMock).not.toHaveBeenCalled()
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
expect(handle.mode).toBe('degraded-new-pty-fallback')
|
||||
// A refusal is not a replacement, so the replace verdict must not be announced.
|
||||
expect(warnSpy).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining('Replacing daemon that failed the health check')
|
||||
)
|
||||
} finally {
|
||||
warnSpy.mockRestore()
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
readVerifiedDaemonPidMock.mockResolvedValue(null)
|
||||
inspectDaemonPtyOwnershipMock.mockResolvedValue('unknown')
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -15,9 +15,12 @@ import {
|
||||
unlinkOwnedDaemonPidFile,
|
||||
type DaemonLauncher,
|
||||
type DaemonPidFile,
|
||||
type DaemonLaunchMode,
|
||||
type DaemonProcessHandle
|
||||
} from './daemon-spawner'
|
||||
import { DAEMON_EXIT_ENDPOINT_OCCUPIED } from './daemon-endpoint-ownership'
|
||||
import { endpointIsProvenDead, probeSocketConnect } from './daemon-endpoint-probe'
|
||||
import { resolveDaemonOccupancy } from './daemon-occupancy'
|
||||
import { DaemonPtyAdapter, type DaemonRespawnReason } from './daemon-pty-adapter'
|
||||
import { DaemonPtyRouter } from './daemon-pty-router'
|
||||
import { DaemonClient } from './client'
|
||||
@@ -34,6 +37,7 @@ import {
|
||||
checkDaemonHealth,
|
||||
isDaemonStaleForCurrentBundle,
|
||||
killStaleDaemon,
|
||||
readVerifiedDaemonPid,
|
||||
parseDaemonPidFile,
|
||||
type MacDaemonTccAttributionHealth
|
||||
} from './daemon-health'
|
||||
@@ -193,7 +197,7 @@ async function getAliveDaemonSessionCount(
|
||||
function createPreservedDaemonHandle(
|
||||
runtimeDir: string,
|
||||
protocolVersion = PROTOCOL_VERSION,
|
||||
mode?: 'degraded-new-pty-fallback'
|
||||
mode?: DaemonLaunchMode
|
||||
): DaemonProcessHandle {
|
||||
const handle: DaemonProcessHandle = {
|
||||
shutdown: async () => {
|
||||
@@ -460,11 +464,12 @@ function createOutOfProcessLauncher(
|
||||
| {
|
||||
reason: Parameters<typeof trackDaemonReplaced>[0]
|
||||
liveSessionCount: number | null
|
||||
/** Rendered once the outcome is known; only the health-check branch announces. */
|
||||
verdict?: string
|
||||
announce?: boolean
|
||||
}
|
||||
| undefined
|
||||
let confirmedReplacement = false
|
||||
let healthCheckReplacementVerdict: string | null = null
|
||||
let healthCheckReplacementAnnounced = false
|
||||
let adoptionClient: DaemonClient | null = new DaemonClient({
|
||||
socketPath,
|
||||
tokenPath
|
||||
@@ -477,9 +482,16 @@ function createOutOfProcessLauncher(
|
||||
adoptionClient.disconnect()
|
||||
adoptionClient = null
|
||||
}
|
||||
const preserveDaemon = async (
|
||||
mode?: 'degraded-new-pty-fallback'
|
||||
): Promise<DaemonProcessHandle> => {
|
||||
/**
|
||||
* Keep the incumbent without talking to it. No adoption, so no lease: the lease only
|
||||
* cancels an adoption watchdog, which cannot fire on a daemon that owns sessions.
|
||||
*/
|
||||
const holdIncumbentDaemon = (): DaemonProcessHandle => {
|
||||
adoptionClient?.disconnect()
|
||||
adoptionClient = null
|
||||
return createPreservedDaemonHandle(runtimeDir, PROTOCOL_VERSION, 'held')
|
||||
}
|
||||
const preserveDaemon = async (mode?: DaemonLaunchMode): Promise<DaemonProcessHandle> => {
|
||||
const connectedClient = adoptionClient ?? undefined
|
||||
adoptionClient = null
|
||||
return holdDaemonAdoptionLease(
|
||||
@@ -579,65 +591,69 @@ function createOutOfProcessLauncher(
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Why: a busy machine can time out the health check on a live daemon; re-verify with a session list before killing its sessions.
|
||||
let liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
// 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)
|
||||
let occupancy = await resolveDaemonOccupancy({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
recordedPid: verifiedPid?.pid ?? null
|
||||
})
|
||||
// Why: a wedged-but-connectable daemon (Windows update relaunch) may still own live sessions, so grace-retry before replacing; a permanent wedge (#8689) exhausts the grace, and 'rejected' skips it (handshake refused = never adoptable).
|
||||
let graceRetry = 0
|
||||
while (
|
||||
liveSessionCount === null &&
|
||||
occupancy.state === 'unknown' &&
|
||||
health !== 'rejected' &&
|
||||
graceRetry < WEDGED_DAEMON_GRACE_RETRIES &&
|
||||
(await probeSocket(socketPath))
|
||||
!endpointIsProvenDead(await probeSocketConnect(socketPath))
|
||||
) {
|
||||
liveSessionCount = await getAliveDaemonSessionCount(socketPath, tokenPath)
|
||||
occupancy = await resolveDaemonOccupancy({
|
||||
socketPath,
|
||||
tokenPath,
|
||||
recordedPid: verifiedPid?.pid ?? null
|
||||
})
|
||||
graceRetry++
|
||||
}
|
||||
if (liveSessionCount !== null && liveSessionCount > 0) {
|
||||
if (occupancy.state === 'occupied') {
|
||||
// Why hold rather than adopt: a count we could only get from the process table means
|
||||
// the daemon never answered us, so it cannot answer a hello either and adoption would
|
||||
// fail. Holding keeps its terminals alive and routes fresh ones locally until the
|
||||
// wedge clears or the user restarts.
|
||||
if (occupancy.liveSessions === null) {
|
||||
console.warn(
|
||||
'[daemon] DEGRADED MODE: holding a daemon that cannot report its sessions but still owns live terminal processes. Killing it would end them; fresh terminals run on the local provider WITHOUT daemon persistence until it recovers or you restart it (Manage Sessions → Restart).'
|
||||
)
|
||||
return holdIncumbentDaemon()
|
||||
}
|
||||
const owned = `${occupancy.liveSessions} live session${occupancy.liveSessions === 1 ? '' : 's'}`
|
||||
if (health === 'pty-spawn-unhealthy') {
|
||||
console.warn(
|
||||
`[daemon] DEGRADED MODE: preserving daemon that failed the PTY spawn health check because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}. Existing sessions keep working; fresh terminals run on the local provider WITHOUT daemon persistence until you restart the daemon (Manage Sessions → Restart).`
|
||||
`[daemon] DEGRADED MODE: preserving daemon that failed the PTY spawn health check because it owns ${owned}. Existing sessions keep working; fresh terminals run on the local provider WITHOUT daemon persistence until you restart the daemon (Manage Sessions → Restart).`
|
||||
)
|
||||
return preserveDaemon('degraded-new-pty-fallback')
|
||||
}
|
||||
console.warn(
|
||||
`[daemon] Preserving daemon that failed the health check because it owns ${liveSessionCount} live session${liveSessionCount === 1 ? '' : 's'}`
|
||||
`[daemon] Preserving daemon that failed the health check because it owns ${owned}`
|
||||
)
|
||||
return preserveDaemon()
|
||||
}
|
||||
// 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}`
|
||||
// quiet unless something actually answered — a verified count, a socket that survived
|
||||
// a grace retry, or a refused hello.
|
||||
// Why: telemetry gates on confirmedReplacement below — the post-kill truth — so a
|
||||
// cold start that killed nothing never reports a replacement.
|
||||
pendingReplacement = {
|
||||
reason: 'failed_health_check',
|
||||
liveSessionCount
|
||||
liveSessionCount: occupancy.liveSessions,
|
||||
verdict: `health=${health}, occupancy=${occupancy.state}, graceRetries=${graceRetry}`,
|
||||
announce: occupancy.state === 'empty' || graceRetry > 0 || health === 'rejected'
|
||||
}
|
||||
}
|
||||
|
||||
// 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 these three conditions: the veto answers "we could not verify" (a daemon that
|
||||
// answered listSessions with 0 must stay replaceable, as must an explicit user
|
||||
// restart); a 'rejected' daemon answered and refused the handshake, so it can never be
|
||||
// adopted and preserving it would loop forever; and an endpoint nothing can reach
|
||||
// serves no one, so its PTYs are already unreachable and holding it costs the app a
|
||||
// daemon it could otherwise replace.
|
||||
const canPreserveLivePtys =
|
||||
pendingReplacement?.liveSessionCount === null &&
|
||||
health !== 'rejected' &&
|
||||
(await probeSocket(socketPath))
|
||||
const killOutcome = canPreserveLivePtys
|
||||
? await killStaleDaemon(runtimeDir, socketPath, tokenPath, PROTOCOL_VERSION, {
|
||||
preserveWhenOwningLivePtys: true
|
||||
})
|
||||
: await killStaleDaemon(runtimeDir, socketPath, tokenPath)
|
||||
const killOutcome = await killStaleDaemon(runtimeDir, socketPath, tokenPath)
|
||||
if (killOutcome.liveOwnerSurvived) {
|
||||
// Why: forking beside a daemon we could not prove dead is precisely how the endpoint
|
||||
// owner and the session host diverge. But refusing outright would leave the user with
|
||||
@@ -654,7 +670,7 @@ function createOutOfProcessLauncher(
|
||||
// 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)) {
|
||||
if (!endpointIsProvenDead(await probeSocketConnect(socketPath))) {
|
||||
return createPreservedDaemonHandle(
|
||||
runtimeDir,
|
||||
PROTOCOL_VERSION,
|
||||
@@ -667,12 +683,9 @@ function createOutOfProcessLauncher(
|
||||
)
|
||||
}
|
||||
}
|
||||
if (
|
||||
healthCheckReplacementVerdict &&
|
||||
(healthCheckReplacementAnnounced || killOutcome.killed)
|
||||
) {
|
||||
if (pendingReplacement?.verdict && (pendingReplacement.announce || killOutcome.killed)) {
|
||||
console.warn(
|
||||
`[daemon] Replacing daemon that failed the health check (${healthCheckReplacementVerdict})`
|
||||
`[daemon] Replacing daemon that failed the health check (${pendingReplacement.verdict})`
|
||||
)
|
||||
}
|
||||
confirmedReplacement = killOutcome.killed || confirmedReplacement
|
||||
@@ -1021,27 +1034,17 @@ 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.
|
||||
// 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.
|
||||
let permanentLeaseHeld = true
|
||||
if (launchMode === 'degraded-new-pty-fallback') {
|
||||
permanentLeaseHeld = await newAdapter.establishLifecycleLease().then(
|
||||
() => true,
|
||||
() => false
|
||||
)
|
||||
} else {
|
||||
// Why skipped when held: we deliberately never talked to that daemon, so there is no
|
||||
// handshake to complete and no temporary lease to hand over. Attempting one would throw
|
||||
// and abort init, leaving the app with no spawner — and restartDaemon() throws without one.
|
||||
if (launchMode !== 'held') {
|
||||
await newAdapter.establishLifecycleLease()
|
||||
}
|
||||
// Why conditional: releasing the launcher's temporary pair with no permanent pair
|
||||
// established is exactly the adoption gap the ordering above exists to prevent.
|
||||
if (permanentLeaseHeld) {
|
||||
releaseDaemonAdoptionLease(newSpawner.getHandle())
|
||||
}
|
||||
|
||||
legacyAdapters = await createLegacyDaemonAdapters(runtimeDir)
|
||||
routedAdapter =
|
||||
launchMode === 'degraded-new-pty-fallback'
|
||||
launchMode === 'degraded-new-pty-fallback' || launchMode === 'held'
|
||||
? new DegradedDaemonPtyProvider({
|
||||
current: newAdapter,
|
||||
legacy: legacyAdapters,
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { resolveDaemonOccupancy, type DaemonOccupancyDeps } from './daemon-occupancy'
|
||||
import type { inspectDaemonPtyOwnership } from './daemon-live-pty-evidence'
|
||||
|
||||
const SOCKET_PATH = '/tmp/orca-daemon.sock'
|
||||
const TOKEN_PATH = '/tmp/orca-daemon.token'
|
||||
const DAEMON_PID = 4242
|
||||
|
||||
type Ownership = Awaited<ReturnType<typeof inspectDaemonPtyOwnership>>
|
||||
|
||||
function ipcAnswers(count: number | null) {
|
||||
return vi.fn<NonNullable<DaemonOccupancyDeps['listSessions']>>(async () => count)
|
||||
}
|
||||
|
||||
function ownershipIs(ownership: Ownership) {
|
||||
return vi.fn<typeof inspectDaemonPtyOwnership>(async () => ownership)
|
||||
}
|
||||
|
||||
function resolve(
|
||||
deps: DaemonOccupancyDeps,
|
||||
recordedPid: number | null = DAEMON_PID
|
||||
): Promise<Awaited<ReturnType<typeof resolveDaemonOccupancy>>> {
|
||||
return resolveDaemonOccupancy({
|
||||
socketPath: SOCKET_PATH,
|
||||
tokenPath: TOKEN_PATH,
|
||||
recordedPid,
|
||||
deps
|
||||
})
|
||||
}
|
||||
|
||||
describe('resolveDaemonOccupancy with a daemon that answered', () => {
|
||||
it('reports occupied with the counted sessions, without consulting the process table', async () => {
|
||||
const listSessions = ipcAnswers(3)
|
||||
const inspectPtyOwnership = ownershipIs('no-live-ptys')
|
||||
|
||||
await expect(resolve({ listSessions, inspectPtyOwnership })).resolves.toEqual({
|
||||
state: 'occupied',
|
||||
liveSessions: 3
|
||||
})
|
||||
expect(listSessions).toHaveBeenCalledWith(SOCKET_PATH, TOKEN_PATH)
|
||||
// The daemon's own reply is authoritative; process-table evidence could only muddy it.
|
||||
expect(inspectPtyOwnership).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reports empty on a count of zero, without consulting the process table', async () => {
|
||||
// The one state that licenses a kill, and only the daemon itself can establish it.
|
||||
const listSessions = ipcAnswers(0)
|
||||
const inspectPtyOwnership = ownershipIs('owns-live-ptys')
|
||||
|
||||
await expect(resolve({ listSessions, inspectPtyOwnership })).resolves.toEqual({
|
||||
state: 'empty',
|
||||
liveSessions: 0
|
||||
})
|
||||
expect(inspectPtyOwnership).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('reports occupied for a single session', async () => {
|
||||
await expect(
|
||||
resolve({ listSessions: ipcAnswers(1), inspectPtyOwnership: ownershipIs('unknown') })
|
||||
).resolves.toEqual({ state: 'occupied', liveSessions: 1 })
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveDaemonOccupancy when the daemon could not answer', () => {
|
||||
it('raises to occupied on process-table evidence, keyed to the recorded pid', async () => {
|
||||
const inspectPtyOwnership = ownershipIs('owns-live-ptys')
|
||||
|
||||
await expect(resolve({ listSessions: ipcAnswers(null), inspectPtyOwnership })).resolves.toEqual(
|
||||
{
|
||||
state: 'occupied',
|
||||
liveSessions: null
|
||||
}
|
||||
)
|
||||
expect(inspectPtyOwnership).toHaveBeenCalledWith(DAEMON_PID)
|
||||
})
|
||||
|
||||
it('stays unknown — never empty — when the process table shows no live PTYs', async () => {
|
||||
// The asymmetry the module exists for: the table may only ever *raise* the answer.
|
||||
// A daemon too wedged to list its sessions is exactly as likely to be hosting them,
|
||||
// and ps can miss PTYs it never observed. Reading this as 'empty' would license
|
||||
// killing live agents unrecoverably; 'unknown' is the residual, not permission.
|
||||
const inspectPtyOwnership = ownershipIs('no-live-ptys')
|
||||
|
||||
await expect(resolve({ listSessions: ipcAnswers(null), inspectPtyOwnership })).resolves.toEqual(
|
||||
{
|
||||
state: 'unknown',
|
||||
liveSessions: null
|
||||
}
|
||||
)
|
||||
expect(inspectPtyOwnership).toHaveBeenCalledWith(DAEMON_PID)
|
||||
})
|
||||
|
||||
it('stays unknown when the process table could not be read', async () => {
|
||||
await expect(
|
||||
resolve({ listSessions: ipcAnswers(null), inspectPtyOwnership: ownershipIs('unknown') })
|
||||
).resolves.toEqual({ state: 'unknown', liveSessions: null })
|
||||
})
|
||||
|
||||
it('stays unknown without inspecting an unverified pid', async () => {
|
||||
// A pid we could not tie back to this daemon may have been recycled; its children
|
||||
// would be some other process's, and counting them is evidence about the wrong tree.
|
||||
const inspectPtyOwnership = ownershipIs('owns-live-ptys')
|
||||
|
||||
await expect(
|
||||
resolve({ listSessions: ipcAnswers(null), inspectPtyOwnership }, null)
|
||||
).resolves.toEqual({ state: 'unknown', liveSessions: null })
|
||||
expect(inspectPtyOwnership).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveDaemonOccupancy when an injected dep throws', () => {
|
||||
it('propagates an inspector rejection rather than degrading to unknown', async () => {
|
||||
// Pins current behavior, and it is a gap: the shipped inspector swallows its own
|
||||
// failures, so this only bites an injected or future-refactored one. A throw here
|
||||
// escapes into the launch path instead of landing on the safe 'unknown' residual.
|
||||
const inspectPtyOwnership = vi.fn<typeof inspectDaemonPtyOwnership>(async () => {
|
||||
throw new Error('process table read exploded')
|
||||
})
|
||||
|
||||
await expect(resolve({ listSessions: ipcAnswers(null), inspectPtyOwnership })).rejects.toThrow(
|
||||
'process table read exploded'
|
||||
)
|
||||
})
|
||||
|
||||
it('propagates a failing listSessions dep, which the real one never does', async () => {
|
||||
// countLiveSessionsOverIpc catches internally and returns null; an injected dep is
|
||||
// not held to that, and the module adds no guard of its own.
|
||||
await expect(
|
||||
resolve({
|
||||
listSessions: async () => {
|
||||
throw new Error('socket vanished')
|
||||
},
|
||||
inspectPtyOwnership: ownershipIs('owns-live-ptys')
|
||||
})
|
||||
).rejects.toThrow('socket vanished')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,69 @@
|
||||
import { DaemonClient } from './client'
|
||||
import { inspectDaemonPtyOwnership } from './daemon-live-pty-evidence'
|
||||
import { PROTOCOL_VERSION, type ListSessionsResult } from './types'
|
||||
|
||||
/**
|
||||
* How much work a daemon is hosting, and how sure we are.
|
||||
*
|
||||
* The distinction that matters: only the daemon itself can prove it is *empty*.
|
||||
* The OS process table can prove work exists, but a table that shows nothing may
|
||||
* simply have failed to observe it — so it may only ever add protection, never
|
||||
* license a kill. 'unknown' is the residual, and it is not permission.
|
||||
*/
|
||||
export type DaemonOccupancy =
|
||||
| { state: 'occupied'; liveSessions: number | null }
|
||||
| { state: 'empty'; liveSessions: 0 }
|
||||
| { state: 'unknown'; liveSessions: null }
|
||||
|
||||
export type DaemonOccupancyDeps = {
|
||||
listSessions?: (socketPath: string, tokenPath: string) => Promise<number | null>
|
||||
inspectPtyOwnership?: typeof inspectDaemonPtyOwnership
|
||||
}
|
||||
|
||||
/** Live session count over the daemon's own socket; null when it could not answer. */
|
||||
async function countLiveSessionsOverIpc(
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
): Promise<number | null> {
|
||||
const client = new DaemonClient({ socketPath, tokenPath, protocolVersion: PROTOCOL_VERSION })
|
||||
try {
|
||||
await client.ensureConnected()
|
||||
const result = await client.request<ListSessionsResult>('listSessions', undefined)
|
||||
return result.sessions.filter((session) => session.isAlive).length
|
||||
} catch {
|
||||
return null
|
||||
} finally {
|
||||
client.disconnect()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ask the daemon first — a reply is authoritative both ways. Only when it cannot
|
||||
* answer do we fall back to the process table, and then only to *raise* the answer
|
||||
* to 'occupied'. A blind or empty-looking table stays 'unknown', because a daemon
|
||||
* too wedged to list its sessions is exactly as likely to be hosting them.
|
||||
*
|
||||
* `recordedPid` must already be identity-verified, or the evidence could describe
|
||||
* a recycled pid's children rather than this daemon's terminals.
|
||||
*/
|
||||
export async function resolveDaemonOccupancy(args: {
|
||||
socketPath: string
|
||||
tokenPath: string
|
||||
recordedPid: number | null
|
||||
deps?: DaemonOccupancyDeps
|
||||
}): Promise<DaemonOccupancy> {
|
||||
const { socketPath, tokenPath, recordedPid, deps = {} } = args
|
||||
const counted = await (deps.listSessions ?? countLiveSessionsOverIpc)(socketPath, tokenPath)
|
||||
if (counted !== null) {
|
||||
return counted > 0
|
||||
? { state: 'occupied', liveSessions: counted }
|
||||
: { state: 'empty', liveSessions: 0 }
|
||||
}
|
||||
if (recordedPid === null) {
|
||||
return { state: 'unknown', liveSessions: null }
|
||||
}
|
||||
const ownership = await (deps.inspectPtyOwnership ?? inspectDaemonPtyOwnership)(recordedPid)
|
||||
return ownership === 'owns-live-ptys'
|
||||
? { state: 'occupied', liveSessions: null }
|
||||
: { state: 'unknown', liveSessions: null }
|
||||
}
|
||||
@@ -28,8 +28,15 @@ export type DaemonPidFile = {
|
||||
spawnerExecPath?: string
|
||||
}
|
||||
|
||||
/**
|
||||
* 'degraded-new-pty-fallback' — adopted, but it cannot spawn fresh PTYs.
|
||||
* 'held' — deliberately kept without adopting it: it owns live terminals but cannot
|
||||
* answer a handshake, so there is no lease to take and none must be attempted.
|
||||
*/
|
||||
export type DaemonLaunchMode = 'degraded-new-pty-fallback' | 'held'
|
||||
|
||||
export type DaemonProcessHandle = {
|
||||
mode?: 'degraded-new-pty-fallback'
|
||||
mode?: DaemonLaunchMode
|
||||
releaseAdoptionLease?(): void
|
||||
shutdown(): Promise<void>
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user