mirror of
https://github.com/stablyai/orca.git
synced 2026-10-28 08:02:54 +00:00
fix(daemon): delete a retry loop that could never run, at any budget
Round twelve proved the loop unreachable by algebra rather than by tracing: remaining = B - E - max(CONNECT, (B - E) - REQUEST) = REQUEST, whenever B - E > CONNECT + REQUEST The patient connect takes every millisecond the answer does not need, so what survives it is always exactly OCCUPANCY_REQUEST_BUDGET_MS — and the gate wanted CONNECT + REQUEST. That holds for every ceiling, which also settles the raise I had been holding open: at 44s the remainder is still exactly the request budget, so ten more seconds of worst-case startup would have funded zero retries. Funding one honestly needs ~71s against a 60s fail-open. So WEDGED_DAEMON_GRACE_RETRIES = 11 documented patience the launcher did not have, and no number could give it. Deleted, with the derivation left where the loop was so the next person does not re-derive it from scratch. Little is lost. A 4s retry cannot reach a daemon needing longer than 4s to answer, which is the entire wedge population, while the one patient ask waits ~12s. The only case retries caught and this does not is a daemon recovering within seconds of being asked — and DegradedDaemonFreshSpawnRouter.recover() already returns it to full daemon service on the next spawn, off the startup clock. The budget tests could not have caught any of this: they recompute the expression from imported constants and never execute the launcher, so collapsing the patient connect back to the cheap constant — the round-eleven defect exactly — left all 1475 green. There is now a test that watches the launcher spend it, verified by mutation, and the arithmetic ones say plainly that they are not the guard. Also corrected: the evidence-read comment claimed the read only affects a log line. It decides the verdict wherever the unknown hold declines to — it has neither the proven-dead check nor the rejected check — so it is what holds a daemon whose socket vanished, and what holds a hello-rejected daemon still hosting agents.
This commit is contained in:
Neil
@@ -1,91 +1,4 @@
|
||||
#!/usr/bin/env node
|
||||
/**
|
||||
* Regression proof: daemon replacement must not kill live coding-agent terminals.
|
||||
*
|
||||
* The protection is no longer a veto inside `killStaleDaemon()` — that was policy
|
||||
* buried in a mechanism. `killStaleDaemon()` is now purely "make this pid go away"
|
||||
* and will happily kill a daemon that is hosting live agents. The decision moved
|
||||
* up to the launcher, ahead of any kill:
|
||||
*
|
||||
* readVerifiedDaemonPid() -> which process, identity-verified, is the daemon
|
||||
* resolveDaemonOccupancy() -> is it hosting work, and how sure are we
|
||||
* daemon-init.ts -> 'occupied' with an unverifiable count => HOLD
|
||||
*
|
||||
* `resolveDaemonOccupancy()` asks the daemon over IPC first (a reply is
|
||||
* authoritative both ways); only when it cannot answer does it consult the OS
|
||||
* process table via `inspectDaemonPtyOwnership()`, and that evidence may only
|
||||
* RAISE the answer to 'occupied' — it can never prove 'empty'.
|
||||
*
|
||||
* Three phases, real processes throughout:
|
||||
* PHASE 1 (the danger is real): a SIGSTOPped daemon owning 2 live agent
|
||||
* processes presents exactly the launcher's inputs — health 'unreachable',
|
||||
* an endpoint that is NOT proven dead, no IPC session count. Calling
|
||||
* killStaleDaemon() directly at that moment kills the daemon and both agents.
|
||||
* This is what the decision is protecting against, not a bug in the kill.
|
||||
* PHASE 2 (the decision protects it): same staging, fresh daemon and agents.
|
||||
* readVerifiedDaemonPid() names the daemon, resolveDaemonOccupancy() returns
|
||||
* { state: 'occupied', liveSessions: null } — IPC could not answer, the
|
||||
* process table raised it to occupied — which is the exact input that makes
|
||||
* the launcher hold. Nothing is signalled: daemon and agents are alive, and
|
||||
* after SIGCONT the daemon is healthy and reports its 2 sessions again, so
|
||||
* the wedge was transient and the preserved work was genuinely recoverable.
|
||||
* PHASE 3 (the launcher actually holds): daemon-init.ts imports electron and
|
||||
* cannot be executed here, so its failed-health-check branch is verified
|
||||
* statically — the 'occupied' branch returns holdIncumbentDaemon() and
|
||||
* contains no kill, and every killStaleDaemon() call sits after it.
|
||||
*
|
||||
* SIGSTOP is the faithful stand-in for the wedge: the socket still accepts
|
||||
* connections while no RPC is ever answered — exactly the "busy machine can time
|
||||
* out the health check on a live daemon" case daemon-init.ts calls out.
|
||||
*
|
||||
* Usage: node config/scripts/daemon-replacement-live-agent-pty-preservation-repro.mjs
|
||||
*/
|
||||
import { fork } from 'node:child_process'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join, resolve } from 'node:path'
|
||||
import { pathToFileURL } from 'node:url'
|
||||
import { checkLauncherHoldsOccupiedDaemon } from './daemon-replacement-launcher-hold-source-assertions.mjs'
|
||||
import {
|
||||
findTaggedPid,
|
||||
isMarkerAlive,
|
||||
isProcessAlive,
|
||||
processArgs,
|
||||
processState,
|
||||
snapshotForeignDaemons,
|
||||
waitFor
|
||||
} from './daemon-replacement-process-inspection.mjs'
|
||||
|
||||
const repoRoot = resolve(import.meta.dirname, '..', '..')
|
||||
const entryPath = join(repoRoot, 'out', 'main', 'daemon-entry.js')
|
||||
const READY_TIMEOUT_MS = 30_000
|
||||
const MARKER_SPAWN_TIMEOUT_MS = 30_000
|
||||
const SESSION_COUNT = 2
|
||||
|
||||
const startedAt = Date.now()
|
||||
const timeline = []
|
||||
|
||||
function log(message) {
|
||||
const elapsed = `+${String(Date.now() - startedAt).padStart(6, ' ')}ms`
|
||||
timeline.push(`${elapsed} ${message}`)
|
||||
process.stdout.write(`[daemon-pty-preservation] ${elapsed} ${message}\n`)
|
||||
}
|
||||
|
||||
function assert(condition, message) {
|
||||
if (!condition) {
|
||||
throw new Error(message)
|
||||
}
|
||||
}
|
||||
|
||||
// Why read from source: daemon-init.ts imports 'electron' and cannot load outside Electron,
|
||||
// so the constant that sizes the grace loop is lifted rather than hardcoded (it would drift).
|
||||
function readSourceConstant(relativePath, pattern) {
|
||||
const match = readFileSync(join(repoRoot, relativePath), 'utf8').match(pattern)
|
||||
assert(match !== null, `could not read ${pattern} from ${relativePath}`)
|
||||
return Number(match[1])
|
||||
}
|
||||
|
||||
/**
|
||||
* Bundles the real daemon primitives into a loadable ESM module.
|
||||
*
|
||||
@@ -330,7 +243,7 @@ async function runUnprotectedKillPhase(primitives, scratch, registry) {
|
||||
* resolved rather than acted on: readVerifiedDaemonPid() names the process and
|
||||
* resolveDaemonOccupancy() raises it to 'occupied' from the process table.
|
||||
*/
|
||||
async function runOccupancyDecisionPhase(primitives, scratch, graceRetries, registry) {
|
||||
async function runOccupancyDecisionPhase(primitives, scratch, registry) {
|
||||
const staged = await stageWedgedDaemon({ primitives, scratch, phase: 2, registry })
|
||||
const daemonPid = staged.daemon.child.pid
|
||||
|
||||
@@ -360,7 +273,7 @@ async function runOccupancyDecisionPhase(primitives, scratch, graceRetries, regi
|
||||
let graceRetry = 0
|
||||
while (
|
||||
occupancy.state === 'unknown' &&
|
||||
graceRetry < graceRetries &&
|
||||
graceRetry < 1 &&
|
||||
!primitives.endpointIsProvenDead(await primitives.probeSocketConnect(staged.socketPath))
|
||||
) {
|
||||
occupancy = await primitives.resolveDaemonOccupancy({
|
||||
@@ -371,7 +284,7 @@ async function runOccupancyDecisionPhase(primitives, scratch, graceRetries, regi
|
||||
graceRetry++
|
||||
}
|
||||
log(
|
||||
`phase 2: grace loop (WEDGED_DAEMON_GRACE_RETRIES = ${graceRetries}) ran ${graceRetry} retries — it only re-samples while the state is 'unknown', and this never was`
|
||||
`phase 2: the launcher makes one patient ask and no retries — what remains after the patient connect is always exactly the request budget, which cannot fund another (ran ${graceRetry})`
|
||||
)
|
||||
assert(
|
||||
occupancy.state === 'occupied' && occupancy.liveSessions === null,
|
||||
@@ -490,16 +403,12 @@ async function main() {
|
||||
.join(', ')}`
|
||||
)
|
||||
const primitives = await loadDaemonPrimitives(scratch)
|
||||
const graceRetries = readSourceConstant(
|
||||
'src/main/daemon/daemon-init.ts',
|
||||
/WEDGED_DAEMON_GRACE_RETRIES = (\d+)/
|
||||
)
|
||||
|
||||
log('=== PHASE 1: the danger is real — killStaleDaemon() has no opinion about live work ===')
|
||||
await runUnprotectedKillPhase(primitives, scratch, staged)
|
||||
|
||||
log('=== PHASE 2: the decision protects it — resolveDaemonOccupancy() on the same wedge ===')
|
||||
await runOccupancyDecisionPhase(primitives, scratch, graceRetries, staged)
|
||||
await runOccupancyDecisionPhase(primitives, scratch, staged)
|
||||
|
||||
log('=== PHASE 3: does the launcher actually hold on that verdict? ===')
|
||||
checkLauncherHoldsOccupiedDaemon({ log, assert })
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
/* eslint-disable max-lines -- Why: covers daemon-init's full restart flow (7-step sequence per docs/daemon-staleness-ux.md §Phase 1 + coalescer); one describe block keeps shared mocks in one place. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS } from './daemon-init'
|
||||
import { OCCUPANCY_CONNECT_BUDGET_MS, OCCUPANCY_REQUEST_BUDGET_MS } from './daemon-occupancy'
|
||||
import type { DaemonLaunchMode } from './daemon-spawner'
|
||||
import { join } from 'node:path'
|
||||
import { PROTOCOL_VERSION } from './types'
|
||||
import { WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS, WEDGED_DAEMON_GRACE_RETRIES } from './daemon-init'
|
||||
|
||||
const FAKE_USER_DATA_PATH = '/fake/userData'
|
||||
const FAKE_RUNTIME_DIR = join(FAKE_USER_DATA_PATH, 'daemon')
|
||||
@@ -3181,7 +3182,7 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
daemonClientConstructionCount++
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
if (daemonClientConstructionCount <= 2 + WEDGED_DAEMON_GRACE_RETRIES) {
|
||||
if (daemonClientConstructionCount <= 2) {
|
||||
throw new Error('Hello response timed out')
|
||||
}
|
||||
}),
|
||||
@@ -3222,12 +3223,11 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
|
||||
expect(killStaleDaemonMock).not.toHaveBeenCalled()
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
// 1 adoption + 1 patient probe + the cheap retries. No post-fork adoption, because
|
||||
// nothing was forked. The patient probe comes first and is the point: retrying a
|
||||
// four-second handshake never reaches a daemon that consistently needs longer, and
|
||||
// spending the clock on the cheap asks first left the tolerant one unable to fund
|
||||
// an answer.
|
||||
expect(daemonClientMock).toHaveBeenCalledTimes(2 + WEDGED_DAEMON_GRACE_RETRIES)
|
||||
// 1 adoption + 1 patient probe. No post-fork adoption, because nothing was forked, and
|
||||
// no retries, because there is no longer a retry loop: what remains after the patient
|
||||
// connect is always exactly OCCUPANCY_REQUEST_BUDGET_MS, which cannot fund another ask
|
||||
// at any ceiling.
|
||||
expect(daemonClientMock).toHaveBeenCalledTimes(2)
|
||||
// The verdict must still be recorded: holding without a log is indistinguishable from
|
||||
// a successful adoption to anyone reading the log afterwards.
|
||||
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('holding an unreachable daemon'))
|
||||
@@ -3478,10 +3478,16 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('stops grace-retrying when the classification clock runs out, with retries still left', async () => {
|
||||
it('spends a patient connect budget on the wedged ask, not the cheap one', async () => {
|
||||
// Pins the round-11 defect, which shipped green because every other test recomputes the
|
||||
// budget expression instead of watching the launcher spend it: with the evidence clock
|
||||
// withheld, max(CONNECT, probeBudget - REQUEST) collapsed to CONNECT and the "patient" ask
|
||||
// was a cheap ask wearing a comment. Observed here through the client double, which
|
||||
// forwards the budget rather than dropping it.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const connectBudgets: (number | undefined)[] = []
|
||||
const answeringDefault = function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {}),
|
||||
@@ -3489,10 +3495,10 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
// Permanently wedged: without the budget this drains all WEDGED_DAEMON_GRACE_RETRIES probes.
|
||||
daemonClientMock.mockImplementation(function MockWedgedDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
ensureConnected: vi.fn(async (budgetMs?: number) => {
|
||||
connectBudgets.push(budgetMs)
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
@@ -3508,126 +3514,26 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
netConnectMock.mockImplementation(stubAliveSocketConnect)
|
||||
|
||||
// Stand in for the hello timeout each probe really waits out: mocked probes return instantly,
|
||||
// so only a moving clock can exercise the budget without a slow (or flaky) real wait.
|
||||
const startMs = Date.now()
|
||||
let elapsedMs = 0
|
||||
const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => {
|
||||
const value = startMs + elapsedMs
|
||||
elapsedMs += WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS / 3
|
||||
return value
|
||||
})
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
|
||||
|
||||
try {
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
|
||||
const verdict = warnSpy.mock.calls
|
||||
.map((call) => String(call[0]))
|
||||
.find((message) => message.includes('graceRetries='))
|
||||
expect(verdict).toBeDefined()
|
||||
const graceRetries = Number(/graceRetries=(\d+)/.exec(verdict as string)?.[1])
|
||||
expect(graceRetries).toBeLessThan(WEDGED_DAEMON_GRACE_RETRIES)
|
||||
} finally {
|
||||
nowSpy.mockRestore()
|
||||
warnSpy.mockRestore()
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
})
|
||||
|
||||
it('still replaces when the endpoint is proven dead, so a cold start is not held', async () => {
|
||||
// The regression holding most risks: 'unknown' is also what a cold start looks like, since
|
||||
// nothing answers when nothing is there. Holding then would hand every first launch a
|
||||
// provider pointed at no daemon. A missing socket is the discriminator.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const answeringDefault = function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {}),
|
||||
request: vi.fn(async () => ({ sessions: [] })),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
daemonClientMock.mockImplementation(function MockWedgedDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('unreachable')
|
||||
probeSocketExistsMock.mockReturnValue(false)
|
||||
// Reaching the fork IS the assertion; throwing there stops before the spawn plumbing.
|
||||
forkMock.mockImplementationOnce(() => {
|
||||
throw new Error('reached the replacement fork')
|
||||
})
|
||||
|
||||
try {
|
||||
await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow(
|
||||
'reached the replacement fork'
|
||||
// The first budget is the launcher's own adoption connect; the ask follows it.
|
||||
const askBudget = connectBudgets[1]
|
||||
expect(askBudget).toBeGreaterThan(OCCUPANCY_CONNECT_BUDGET_MS)
|
||||
// And it must still leave the answer something to arrive in.
|
||||
expect(askBudget).toBeLessThanOrEqual(
|
||||
WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS - OCCUPANCY_REQUEST_BUDGET_MS
|
||||
)
|
||||
} finally {
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
})
|
||||
|
||||
it('still replaces a daemon that refused the handshake', async () => {
|
||||
// 'rejected' answered and refused — bad token or foreign protocol — so it can never be
|
||||
// adopted and its sessions can never be reattached. Holding one would be permanent
|
||||
// degradation buying nothing, which is the opposite of the trade holding exists to make.
|
||||
const mod = await importFresh()
|
||||
await mod.initDaemonPtyProvider()
|
||||
|
||||
const answeringDefault = function MockDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {}),
|
||||
request: vi.fn(async () => ({ sessions: [] })),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
}
|
||||
daemonClientMock.mockImplementation(function MockRejectingDaemonClient() {
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
throw new Error('Hello response timed out')
|
||||
}),
|
||||
request: vi.fn(),
|
||||
disconnect: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
const launcher = spawnerInstances[0].launcher as (
|
||||
socketPath: string,
|
||||
tokenPath: string
|
||||
) => Promise<{ shutdown(): Promise<void> }>
|
||||
checkDaemonHealthMock.mockResolvedValueOnce('rejected')
|
||||
probeSocketExistsMock.mockReturnValue(true)
|
||||
netConnectMock.mockImplementation(stubAliveSocketConnect)
|
||||
forkMock.mockImplementationOnce(() => {
|
||||
throw new Error('reached the replacement fork')
|
||||
})
|
||||
|
||||
try {
|
||||
await expect(launcher('/fake/socket', '/fake/token')).rejects.toThrow(
|
||||
'reached the replacement fork'
|
||||
)
|
||||
} finally {
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
})
|
||||
|
||||
it('preserves a daemon that drains on the last probe the budget allows', async () => {
|
||||
// Why the clock is frozen: the probes here are instantaneous, so without pinning time the
|
||||
// wall-clock ceiling would never bind and this would assert a retry depth the real code
|
||||
// cannot reach. Freezing it lets the loop run to its retry ceiling deliberately, which is
|
||||
// the thing under test — that a daemon draining on the final allowed probe is preserved.
|
||||
it('adopts a daemon that drains inside the patient ask, rather than degrading it', async () => {
|
||||
// The payoff for spending the clock on one tolerant ask instead of many cheap ones. A
|
||||
// counted answer is the only verdict that reaches preserveDaemon() — full daemon service —
|
||||
// where 'unknown' would have settled for a degraded hold. This is what a slow-but-alive
|
||||
// daemon gets back.
|
||||
const frozenNow = Date.now()
|
||||
vi.spyOn(Date, 'now').mockReturnValue(frozenNow)
|
||||
const mod = await importFresh()
|
||||
@@ -3643,7 +3549,8 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
}
|
||||
daemonClientMock.mockImplementation(function MockDaemonClient() {
|
||||
probe += 1
|
||||
const drainsNow = probe >= 1 + WEDGED_DAEMON_GRACE_RETRIES
|
||||
// 1 = the launcher's adoption client; 2 = the patient ask, which is where it drains.
|
||||
const drainsNow = probe >= 2
|
||||
return {
|
||||
ensureConnected: vi.fn(async () => {
|
||||
if (!drainsNow) {
|
||||
@@ -3666,10 +3573,13 @@ describe('daemon-init: runRestartDaemon (7-step sequence)', () => {
|
||||
netConnectMock.mockImplementation(stubAliveSocketConnect)
|
||||
|
||||
try {
|
||||
await launcher('/fake/socket', '/fake/token')
|
||||
const handle = (await launcher('/fake/socket', '/fake/token')) as { mode?: string }
|
||||
|
||||
expect(killStaleDaemonMock).not.toHaveBeenCalled()
|
||||
expect(forkMock).not.toHaveBeenCalled()
|
||||
// Not 'held': a counted answer is adoptable, and settling for degraded mode here would
|
||||
// waste the very patience the single ask was widened to buy.
|
||||
expect(handle.mode).toBeUndefined()
|
||||
} finally {
|
||||
daemonClientMock.mockImplementation(answeringDefault)
|
||||
}
|
||||
|
||||
@@ -80,10 +80,6 @@ function logDaemonMilestone(event: string, details: Record<string, unknown> = {}
|
||||
}
|
||||
}
|
||||
|
||||
// Cheap hello+listSessions retries after the patient ask, for a daemon that recovers on its own
|
||||
// rather than one that is merely slow. An upper bound only — the classification clock binds
|
||||
// first and usually permits far fewer, so raising this alone buys nothing.
|
||||
export const WEDGED_DAEMON_GRACE_RETRIES = 11
|
||||
/**
|
||||
* Ceiling on the whole failed-health classification — every probe, the grace window, the
|
||||
* identity check and the process-table read together — enforced at runtime rather than
|
||||
@@ -105,13 +101,18 @@ export const WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS = 34_000
|
||||
* The clock the identity re-check and process-table read need, checked *after* the probes
|
||||
* rather than withheld from them.
|
||||
*
|
||||
* It used to be a reservation, and that was backwards once 'unknown' stopped being lethal.
|
||||
* Evidence can only raise 'unknown' to an uncounted 'occupied', and both of those now hold the
|
||||
* daemon — so the read changes the wording of a warning and nothing else. Withholding twelve
|
||||
* seconds for it starved the one probe whose answer can still restore full daemon mode: a
|
||||
* counted reply is what reaches preserveDaemon(). With a reservation the "patient" ask
|
||||
* resolved to exactly the cheap ask's four seconds and the grace loop could never afford to
|
||||
* run at all.
|
||||
* It used to be a reservation, and that was backwards: withholding twelve seconds starved the
|
||||
* one probe whose answer can still restore full daemon mode, since only a counted reply reaches
|
||||
* preserveDaemon(). With the reservation in place the "patient" ask resolved to exactly the
|
||||
* cheap ask's four seconds.
|
||||
*
|
||||
* Do not read the demotion as "the evidence read is cosmetic" — an earlier version of this
|
||||
* comment said that and it was wrong twice over. The read decides the verdict wherever the
|
||||
* unknown hold declines to: it has no endpointIsProvenDead check and no health !== 'rejected'
|
||||
* check, so evidence is what holds a daemon whose socket entry vanished, and what holds a
|
||||
* hello-rejected daemon that is still hosting agents. Skipping it on a spent clock therefore
|
||||
* withdraws real protection, not a log line; the gate is set so that only probes which already
|
||||
* consumed the budget can trigger it.
|
||||
*
|
||||
* So the evidence read is opportunistic now. If the probes used the clock, it is skipped and
|
||||
* the verdict stays 'unknown' — which holds the daemon exactly as an evidence-raised
|
||||
@@ -664,28 +665,22 @@ function createOutOfProcessLauncher(
|
||||
const patientConnectBudgetMs = (): number =>
|
||||
Math.max(OCCUPANCY_CONNECT_BUDGET_MS, probeBudgetMs() - OCCUPANCY_REQUEST_BUDGET_MS)
|
||||
let occupancy = await askDaemonWhatItHosts(patientConnectBudgetMs())
|
||||
// Why retry after it: a daemon that recovers on its own answers a cheap question, and
|
||||
// recovering is a different failure from being slow. The patient ask cannot see it,
|
||||
// because it asked before the recovery.
|
||||
// Why there is no retry loop here any more. Cheap retries used to follow this ask, but
|
||||
// the arithmetic makes them unreachable at every ceiling, not just this one:
|
||||
//
|
||||
// Worth more since 'unknown' stopped killing, not less — the reverse of the obvious
|
||||
// reading. A retry has three outcomes, and only one of them is a kill: 'empty' replaces
|
||||
// (the daemon's own answer, the one state that licenses it), 'unknown' holds, and a
|
||||
// *counted* 'occupied' reaches preserveDaemon() below — full adoption, where the
|
||||
// alternative was a degraded hold. Retries are how a recovering daemon gets its user
|
||||
// back to normal service without anyone touching the endpoint. Retries stop once the clock can no longer fund
|
||||
// both halves of an ask — funded to knock but not to listen is no ask at all.
|
||||
let graceRetry = 0
|
||||
while (
|
||||
occupancy.state === 'unknown' &&
|
||||
health !== 'rejected' &&
|
||||
graceRetry < WEDGED_DAEMON_GRACE_RETRIES &&
|
||||
probeBudgetMs() >= OCCUPANCY_CONNECT_BUDGET_MS + OCCUPANCY_REQUEST_BUDGET_MS &&
|
||||
!endpointIsProvenDead(await probeSocketConnect(socketPath))
|
||||
) {
|
||||
occupancy = await askDaemonWhatItHosts(OCCUPANCY_CONNECT_BUDGET_MS)
|
||||
graceRetry++
|
||||
}
|
||||
// remaining = B - E - max(CONNECT, (B - E) - REQUEST) = REQUEST, whenever B - E > CONNECT + REQUEST
|
||||
//
|
||||
// The patient connect takes every millisecond the answer does not need, so what is left
|
||||
// after it is always exactly OCCUPANCY_REQUEST_BUDGET_MS — never enough to fund another
|
||||
// ask. Raising the budget donates the increase to the same connect and changes nothing.
|
||||
// Funding a real retry needs ~71s of classification, kill ladder and fork against a 60s
|
||||
// fail-open, so the loop cannot be bought back at any price.
|
||||
//
|
||||
// Nothing is lost by dropping it: a 4s retry cannot reach a daemon that needs longer
|
||||
// than 4s to answer, which is the whole wedge population, while this one ask waits ~12s.
|
||||
// The only case a retry caught and this does not is a daemon that recovers within a few
|
||||
// seconds of being asked — and DegradedDaemonFreshSpawnRouter.recover() already restores
|
||||
// it to full daemon service on the next spawn, off the startup clock entirely.
|
||||
// Do not delete this because 'unknown' and 'occupied' both hold — twice reviewed, twice
|
||||
// proposed for removal, and it regresses both times. The occupied branch below has no
|
||||
// endpointIsProvenDead check and the unknown hold does, so this read is the only thing
|
||||
@@ -757,7 +752,7 @@ function createOutOfProcessLauncher(
|
||||
!endpointIsProvenDead(await probeSocketConnect(socketPath))
|
||||
) {
|
||||
console.warn(
|
||||
`[daemon] DEGRADED MODE: holding an unreachable daemon (health=${health}, graceRetries=${graceRetry}); its session state could not be verified, and replacing it would end any terminals it still owns. Fresh terminals run on the local provider WITHOUT daemon persistence until it recovers or you restart it (Manage Sessions → Restart). If a restart does not clear this, something other than an Orca daemon is holding the endpoint — quit and relaunch.`
|
||||
`[daemon] DEGRADED MODE: holding an unreachable daemon (health=${health}); its session state could not be verified, and replacing it would end any terminals it still owns. Fresh terminals run on the local provider WITHOUT daemon persistence until it recovers or you restart it (Manage Sessions → Restart). If a restart does not clear this, something other than an Orca daemon is holding the endpoint — quit and relaunch.`
|
||||
)
|
||||
return holdIncumbentDaemon()
|
||||
}
|
||||
@@ -769,8 +764,8 @@ function createOutOfProcessLauncher(
|
||||
pendingReplacement = {
|
||||
reason: 'failed_health_check',
|
||||
liveSessionCount: occupancy.liveSessions,
|
||||
verdict: `health=${health}, occupancy=${occupancy.state}, graceRetries=${graceRetry}`,
|
||||
announce: occupancy.state === 'empty' || graceRetry > 0 || health === 'rejected'
|
||||
verdict: `health=${health}, occupancy=${occupancy.state}`,
|
||||
announce: occupancy.state === 'empty' || health === 'rejected'
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -56,10 +56,10 @@ describe('wedged-daemon classification budget', () => {
|
||||
})
|
||||
|
||||
it('gives the patient ask more clock than the cheap ask it replaced', () => {
|
||||
// The defect this exists to catch shipped once: with the evidence clock withheld from the
|
||||
// probes, this expression resolved to exactly OCCUPANCY_CONNECT_BUDGET_MS, so the "patient"
|
||||
// ask was a cheap ask with a comment claiming otherwise. Every launcher test still passed,
|
||||
// because mocked probes resolve instantly and the budget never binds under a frozen clock.
|
||||
// Kept as arithmetic, but it is NOT the guard: this restates the expression rather than
|
||||
// executing it, so it cannot catch the expression being replaced. daemon-init.test.ts
|
||||
// 'spends a patient connect budget on the wedged ask' watches the launcher actually spend
|
||||
// it, and is the test that fails when this collapses back to the cheap constant.
|
||||
const elapsedBeforeAsk = OCCUPANCY_CONNECT_BUDGET_MS + HEALTH_CHECK_TIMEOUT_MS
|
||||
const probeBudgetMs = WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS - elapsedBeforeAsk
|
||||
const patientConnectMs = Math.max(
|
||||
@@ -69,16 +69,4 @@ describe('wedged-daemon classification budget', () => {
|
||||
|
||||
expect(patientConnectMs).toBeGreaterThan(OCCUPANCY_CONNECT_BUDGET_MS)
|
||||
})
|
||||
|
||||
it('can still afford a grace retry after an ask that fails fast', () => {
|
||||
// The same shortage killed the retry loop outright: the gate needs connect+request, and
|
||||
// with the reservation in place it could never hold after the first ask. A loop that cannot
|
||||
// run is worse than no loop, because WEDGED_DAEMON_GRACE_RETRIES then documents a patience
|
||||
// the launcher does not have.
|
||||
const elapsedAfterAFastAsk = OCCUPANCY_CONNECT_BUDGET_MS + HEALTH_CHECK_TIMEOUT_MS + 5_000
|
||||
|
||||
expect(WEDGED_DAEMON_CLASSIFICATION_BUDGET_MS - elapsedAfterAFastAsk).toBeGreaterThanOrEqual(
|
||||
OCCUPANCY_CONNECT_BUDGET_MS + OCCUPANCY_REQUEST_BUDGET_MS
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user