Skip to content

[factory] An unbounded readiness-reconcile sweep stops the loop permanently and silently #296

Description

@khaliqgant

Summary

#reconcileReadyIssues() has no bounded deadline. A single runOnce() that never settles stops the readiness reconcile loop permanently and silently — no error, no failure count, no state change, and no recovery path. Observed live in production for 84+ minutes and still ongoing at time of filing.

Observed, on the deployed instance

Container booted 11:09:49Z. From the authenticated /evidence projection at 12:30Z and after:

readinessReconcile.state              : healthy
readinessReconcile.consecutiveFailures: 0
readinessReconcile.lastError          : (none)
readinessReconcile.lastCompletedAtMs  : 1787224535802   -> 11:15:35Z
readinessReconcile.lastStartedAtMs    : 1787224595805   -> 11:16:35Z
readinessReconcile.lastDurationMs     : 686
heartbeat.updatedAt                   : advancing normally every ~30s

lastCompleted is 60.003 s earlier than lastStarted. The pass begun at 11:16:35Z has neither completed nor failed. Against #readinessReconcileIntervalMs = 60_000, that is ~84 missed passes and counting. No issue can be discovered, claimed, or dispatched for the entire duration.

Why it is permanent

#scheduleReadinessReconcile re-arms the timer only from sweep.finally(...):

void sweep.finally(() => {
  if (this.#readinessReconcileInFlight === sweep) this.#readinessReconcileInFlight = undefined
  if (this.#started && !this.#stopping) this.#scheduleReadinessReconcile()
})

A pending promise never reaches finally, so the loop is never rescheduled. There is no watchdog and no timeout anywhere on the path, so the only exit is a process restart.

Why it is invisible

The subsystem's reported state is written only on settle — the success path sets lastCompletedAtMs and clears lastError; the failure path sets lastFailureAtMs, increments consecutiveFailures, and is what drives degraded. A hang takes neither path. The values from the last successful pass therefore persist unchanged for as long as the process is stuck, so the subsystem reports healthy forever, and /healthz reports "ok": true alongside it.

This is strictly worse than the failure mode fixed in #292/#293. That one failed loudly — 7 then 8 consecutive failures with an error string that named the cause. This one is silent by construction, and nothing in the health surface can express it.

Probable trigger, and why the trigger is not the defect

The relay/relaycast control plane was degraded in the window the pass began: D1 returning D1 DB is overloaded [code: 7429], with reads failing about as often as writes. A network call without a timeout, issued during that window, blocks indefinitely.

The backend has since recovered — control-plane reads now measure ~1.3 s, against ~6.9 s during the incident — and the pass is still hung. That is the point. A transient dependency failure became a permanent local stall purely because nothing bounded the wait. With a deadline, that pass would have failed, incremented the counter, re-armed the loop, and the next pass would have succeeded on its own once D1 recovered.

Proposed fix

  1. Bound the sweep. Give #reconcileReadyIssues() a deadline — a sensible default is a small multiple of #readinessReconcileIntervalMs. On expiry, reject. That routes it into the existing failure path, which already increments consecutiveFailures, records lastError, marks degraded past the threshold, and — critically — re-arms the loop. The recovery machinery already exists; it is simply unreachable from a hang.
  2. Report in-flight duration. state should be derived, not last-write-wins. A pass in flight for longer than some multiple of the interval is not healthy; stalled is the missing value. See [factory] A deployed Factory has no operator-reachable diagnostics: the field naming an outage is unretrievable by design #295 for the observability half of this.
  3. Audit runOnce() for unbounded awaits. The deadline in (1) is the backstop, but any individual provider/roster/mount call without its own timeout can produce this, and the fatal-vs-per-item classifier added in fix(orchestrator): skip per-item dispatch failures instead of aborting the run-once pass (#292) #293 only helps once an error actually exists.

Tests

  • must-fire: a runOnce() that never settles causes the sweep to reject on the deadline, consecutiveFailures to increment, and the next pass to be scheduled. This must fail before the change — today it hangs forever.
  • must-not-fire: a legitimately slow-but-completing pass under the deadline still succeeds and does not get killed. fix(factory): replay existing Slack triage answers #36 measured a real 3,665,173 ms cold-mirror reconcile, so the deadline must not be set below realistic worst-case hydration, or this fix converts a slow start into a crash loop.
  • A pass in flight beyond the threshold reports a non-healthy state.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions