fix(node): debounce health failures and recovery

[dr-hoseyn]

A single failed probe currently changes node health and can trigger a reconnect, while a single successful probe or automatic reconnect immediately reports recovery. This adds consecutive failure/recovery thresholds so transient errors do not produce false downtime or premature recovery notifications.

• Add NODE_HEALTH_FAIL_THRESHOLD=3 and NODE_HEALTH_RECOVER_THRESHOLD=2, validated as positive integers. Setting both to 1 restores single-probe transitions.
• Keep bounded, per-node streaks and reset them on opposite observations, node replacement, status/leadership changes, invalidation, and cleanup. Only the elected job leader changes shared status or initiates health-driven repairs; every worker continues maintaining its local attachment.
• Preserve immediate repair of an explicitly missing backend without immediately reporting downtime. Never restart an in-flight core operation. Automatic repairs leave status and notifications to subsequent successful health probes.
• Guard database transitions against concurrent status changes, preserve version metadata on health failures, avoid rewriting identical errors, and surface previously swallowed per-node job exceptions.

Validation: 134 passing tests on Python 3.14 with node-bridge 0.9.2: 58 existing node/leader/database tests, 33 local threshold/configuration/race checks, 2 real-NATS integration tests (including four Python processes), and 41 node API tests. Ruff and git diff --check pass. The PR contains only four production Python files; local validation files are excluded.

Existing test limitation: test_health_poll_recovers_claim_that_expires_after_startup times out at line 291 while awaiting the bridge's idle sync worker, before invoking the health checker. The same failure was reproduced with the unchanged origin/dev health checker and node-bridge 0.9.2.

Closes #620.

Summary by CodeRabbit

• Bug Fixes
  • Node health status now changes only after configurable consecutive failed or successful checks, reducing reactions to temporary connection issues.
  • Status updates are skipped when the node’s status has changed since it was checked, helping prevent stale health results from overwriting newer status.
  • Nodes with an uninitialized backend can be repaired before the failure threshold is reached.
  • Health-check leadership and recovery behavior are more consistent across instances, reducing conflicting updates.
• Documentation
  • Added configuration guidance for health-check thresholds and their effects.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2bb7219d-5844-4a97-bafe-b99b9cc5e687

---

📥 Commits

Reviewing files that changed from the base of the PR and between 8894045 and ee1bdcd.

---

📒 Files selected for processing (2)

• .env.example
• tests/test_node_health_debounce.py

---

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

---

Walkthrough

Node health checks now use configurable consecutive failure and recovery thresholds. The job tracks per-node result streaks and assigns shared status transitions and core restarts to the health-check owner. Status writes can require the database status to match the observed status.

Changes

Node Health Check Flow

Layer / File(s)	Summary
Thresholds and health-result streaks config.py, .env.example, app/jobs/node_checker.py, tests/test_node_health_debounce.py	Adds configurable failure and recovery thresholds. The job tracks consecutive results, resets streaks when relevant node or ownership state changes, and separates probing from local health-state updates. Tests cover threshold behavior and streak resets.
Guarded node status updates app/db/crud/node.py, app/operation/node.py	Adds an optional expected-status condition to node status writes. Status operations report whether an update applied and skip stale or redundant updates.
Leader-owned repair and health-check lifecycle app/operation/node.py, app/jobs/node_checker.py, tests/test_node_health_debounce.py	Routes health-check recovery through deferred local connection handling. The owning worker handles shared status transitions and repairs. The job clears stale streaks and resets them on leadership loss or shutdown. Tests cover follower behavior and dead-core repair.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant process_node_health_check
  participant verify_node_backend_health
  participant NodeOperation
  participant update_node_status
  process_node_health_check->>verify_node_backend_health: Probe node health
  verify_node_backend_health-->>process_node_health_check: Return health result
  process_node_health_check->>process_node_health_check: Apply configured streak threshold
  process_node_health_check->>NodeOperation: Request status update with observed status
  NodeOperation->>update_node_status: Apply guarded database update

Loading

Suggested reviewers: x0sina

Merge Risk: ⚪ Minimal · up to ee1bd

No actionable merge-blocking issue was established in the reviewed changes. Normal checks can proceed.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 88940

The new thresholds and worker-ownership rules affect outage reporting and recovery across workers. They add safeguards, but an in-flight check can still finish after its worker loses leadership. That exposure also existed before this change, and the remaining deployment and security coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A registered node’s backend probe results influence that node’s stored status, lifecycle state, repairs, and notifications. The inspected health path keys its streak and status transition by node ID; broader downstream uses of status were not established.

Security Findings and Attack Paths

• inferred — No introduced attacker-reachable bypass was established from the inspected callers. Health-driven deferred repair is called by the leader-gated health path; external reachability of the operation method was not independently mapped.

Trust Boundaries and Controls

• observed — The health path checks job ownership before shared transitions, and its database updates require the observed status still to match. Neither the status predicate nor the lifecycle epoch check is a leader-lease fence.

Resilience and Maintainability Implications

• inferred — A leadership change during an awaited operation can leave a former leader completing shared work. Head reduces this pre-existing multi-worker exposure through ownership checks, but streak cleanup alone cannot stop work already in flight.

Hardening Proposals

• proposed — For a strict leader-only guarantee, fence shared writes and publications against a leadership generation or lease, and stop or reconcile in-flight work on leadership loss; another check after a committed write alone cannot retract it.

---

---

---

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 5 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: debouncing node health failure and recovery transitions.
Linked Issues check		The PR meets #620. JobSettings adds positive-integer settings with defaults 3 and 2. process_node_health_check applies consecutive failure and recovery thresholds and resets streaks on opposit…
Out of Scope Changes check		The changes remain within #620. Conditional status updates, leader-only shared transitions, local attachment repair, and health-state safeguards support correct debounced health behavior. The database…

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 5 files. (1 skipped: 1 unsupported.)

---

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{A rabbit counts each probe in rows,
Three thumps before the red light glows.
Two bright hops bring the green back near,
The leader tends the nodes with care.
Streaks clear when duties end,
And quiet checks resume again.}

_{Comment @coderabbitai help to get the list of available commands.}

[dr-hoseyn]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[T3ST3ST3R0N]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.