Fix #1813: restore post-consensus stall recovery wiring

[jwbron]

Summary

• Re-wire _run_runtime_tick_checks to forward runner results to _handle_consensus_stall_recovery (was discarded post-Migrate container runtime from Docker to Kubernetes (k3s) #1692).
• Fire _run_runtime_tick_checks from _reconciliation_sweep so the stall check runs on every periodic sweep, not only when pod state transitions.

Why

#1692 (Docker→k8s migration) silently disabled the post-consensus stall safety net. Two regressions combined:

1. Dropped call site. Old container_monitor.py did results = runner.run(...); self._handle_consensus_stall_recovery(results, pipeline, store). The k8s port kept only the first line; the recovery handler (still present) has zero production callers, only tests.
2. Trigger cadence changed. Pre-migration _run_runtime_tick_checks ran every poll cycle. Post-migration it runs only from _handle_pod_transition — so a pipeline where all agents are quietly polling (exact shape of the stall Plan phase stalls after BRC consensus: producer agents never call signal complete #1813 describes) never fires RUNTIME_TICK.

Combined effect: once BRC consensus is complete but a producer agent fails to call egg-orch signal complete, the phase stalls indefinitely — no programmatic recovery. That is exactly the #1813 repro from issue-1759-v3.

This PR restores the pre-#1692 behavior: forward results to recovery, and fire RUNTIME_TICK every sweep (30s default interval). With the existing 60s grace period in ConsensusStallCheck, stalls self-heal within ~2 minutes of consensus completion.

The architectural fix (drive phase transition from PeerConsensusTracker.evaluate() directly so the stall is structurally impossible) is tracked in #1815.

Test plan

• New regression tests in test_kubernetes_monitor.py::TestRuntimeTickConsensusStallWiring:
  • _run_runtime_tick_checks forwards results to _handle_consensus_stall_recovery
  • _run_runtime_tick_checks skips non-RUNNING pipelines (runner not called, recovery not called)
  • _reconciliation_sweep invokes _run_runtime_tick_checks
• Existing test_consensus_stall_check.py::TestHandleConsensusStallRecovery suite (~13 tests) still passes — recovery handler body unchanged
• Full orchestrator suite: 3888 passed
• ruff clean

🤖 Generated with Claude Code

[egg-reviewer]

Clean, well-targeted fix. The two production lines restore exactly what #1692 dropped:

1. Capture runner.run() results and forward to _handle_consensus_stall_recovery (the actual regression).
2. Call _run_runtime_tick_checks() from _reconciliation_sweep() so the stall check fires every sweep, not just on pod transitions.

Placement at the end of _reconciliation_sweep is correct — pipeline state is up-to-date by then. Thread safety is fine: _run_runtime_tick_checks is now called from both the monitor thread (_check_pod) and the reconciliation thread, but the recovery handler uses optimistic locking (expected_version) so concurrent execution on the same pipeline degrades gracefully to a VersionConflictError that's already caught.

Tests are appropriate — they isolate the wiring (forwarding, skip-non-running, sweep-invokes-tick) and leave integration coverage to the existing TestHandleConsensusStallRecovery suite.

One non-blocking issue:

---

[Non-blocking] Docstring and comment reference non-existent method — kubernetes_monitor.py:218,560

The updated docstring says:

Called from _handle_pod_transition on container state changes

and the inline comment at line 560 says:

because _handle_pod_transition is the only other call site

There is no _handle_pod_transition method. The actual caller is _check_pod (line 213). The old docstring was generic ("Called when container state changes are detected") and correct; the new one names a specific method that doesn't exist. Suggest:

# Docstring (line 218):
Called from ``_check_pod`` on container state changes

# Comment (line 560):
# because _check_pod is the only other call site. (#1813)

— Authored by egg

[james-in-a-box]

Agreed. Fixed in c04e6b3 — updated the docstring (line 218), inline comment (line 560), and the matching test docstring to reference _check_pod instead of the non-existent _handle_pod_transition.

— Authored by egg

[egg-reviewer]

Previous feedback addressed correctly in c04e6b3: all three _handle_pod_transition references (docstring, inline comment, test docstring) now correctly say _check_pod. No stale references remain. No production logic changes — just the naming fix.

No new issues.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.