fix(control-plane): resolve agent node health status flapping - #169
Merged
Conversation
Three independent health systems (HealthMonitor, StatusManager, PresenceManager) were fighting each other, causing nodes to flicker between online/stale/offline. Root causes fixed: - Single HTTP failure instantly marked nodes inactive (now requires 3 consecutive failures) - Heartbeats silently dropped for 10s after health check marked node inactive (removed) - 30s recovery debounce blocked legitimate recovery (reduced to configurable 5s) - 8s heartbeat DB cache caused phantom staleness (reduced to 2s) - 30s reconciliation threshold too aggressive with cache delay (increased to 60s) Changes: - health_monitor.go: Add consecutive failure tracking, recovery debounce, sync.Once for Stop() - status_manager.go: Remove heartbeat-dropping logic, configurable stale threshold - config.go: Add NodeHealthConfig with env var overrides - nodes.go: Reduce heartbeat cache from 8s to 2s - server.go: Wire config into health monitor and status manager - NodesPage.tsx: Add 30s background refresh for fresh timestamps Tests: 10 new tests (5 unit + 3 integration + 2 status manager) all passing. Integration tests wire all 3 services concurrently to validate no-flapping behavior. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…d stale MCP data Code review follow-up for #167. Addresses race conditions, missing MCP health refresh, and test reliability issues found during review. Key fixes: - Eliminate stale pointer race: checkAgentHealth now takes nodeID string instead of *ActiveAgent, re-fetching canonical state after HTTP call - Fix MCP health going stale: active agents now refresh MCP data on every health check, not only on status transitions - Initialize LastTransition on registration so debounce has a valid baseline - Cap consecutive failure counter to prevent unbounded growth - Add lifecycle guard to NodesPage polling to prevent React state updates after unmount - Fix RecoverFromDatabase tests that raced against async goroutine - Extract health score magic numbers into named constants - Document zero-value-means-default semantics on NodeHealthConfig Tests: 30/30 health monitor + 3/3 integration tests pass Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Author
|
github actions down, waiting to rerun CI |
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
thank you. i will try it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #167 — Nodes flickering between online/stale/offline in the UI despite being consistently online.
Root cause: Three independent health systems (HealthMonitor, StatusManager, PresenceManager) independently changing node status and fighting each other.
Changes
Backend (control-plane)
sync.Oncefor idempotentStop().NodeHealthConfigstruct with 5 configurable parameters and env var overrides (AGENTFIELD_HEALTH_CHECK_INTERVAL, etc.)Frontend (web UI)
last_heartbeattimestamps fresh (SSE events don't carry heartbeat timestamps)Test Plan
10 new tests added (all passing):
Integration tests (wire all 3 services concurrently, matching production):
TestIntegration_NoFlapping_HeartbeatsDuringTransientFailures— THE critical test. Runs heartbeats while introducing intermittent health check failures. Asserts node NEVER goes inactive.TestIntegration_ProperInactiveWhenHeartbeatsStop— Genuine outage correctly detectedTestIntegration_RecoveryAfterGenuineOutage— Full lifecycle: healthy → outage → recoveryUnit tests:
Status manager tests:
🤖 Generated with Claude Code