Skip to content

fix(kanban): trust fresh heartbeat over pid miss - #50118

Closed
GoliathInteractive wants to merge 1 commit into
NousResearch:mainfrom
GoliathInteractive:goliath/kanban-liveness-guard
Closed

GoliathInteractive wants to merge 1 commit into
NousResearch:mainfrom
GoliathInteractive:goliath/kanban-liveness-guard

Conversation

@GoliathInteractive

Copy link
Copy Markdown

Summary

  • Treat fresh current-run Kanban heartbeats as authoritative over host-local PID misses in crash detection
  • Adds bounded heartbeat freshness knob for PID namespace / container visibility mismatch
  • Adds regression tests for fresh-heartbeat PID miss and stale duplicate-run completion safety

Validation

  • /home/hermes/.hermes/hermes-agent/venv/bin/python -m pytest tests/hermes_cli/test_kanban_db.py -q -k 'crashed_workers or stale_run_completion'
  • /home/hermes/.hermes/hermes-agent/venv/bin/python -m pytest tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py tests/hermes_cli/test_kanban_cli.py tests/tools/test_kanban_tools.py tests/gateway/test_kanban_watchers_mixin.py -q

Goliath evidence: resolves P0-A Kanban liveness / duplicate-spawn guard where a worker with fresh heartbeats was marked crashed due host/container PID mismatch.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the PID-miss/heartbeat race. The premise remains present on current main: detect_crashed_workers() proceeds from a false _pid_alive() result at hermes_cli/kanban_db.py:6626 without consulting run-owned heartbeat state.

Problems

  • hermes_cli/kanban_db.py:216 adds HERMES_KANBAN_CRASH_HEARTBEAT_FRESH_SECONDS as a user-facing behavioral knob. The repository policy at AGENTS.md:102-107 requires this category of setting to be configured through config.yaml, not a new non-secret HERMES_* variable.
  • tests/hermes_cli/test_kanban_db.py:888 duplicates existing stale-run completion coverage in tests/hermes_cli/test_kanban_core_functionality.py:1516 and the worker-facing coverage in tests/tools/test_kanban_tools.py:1645.

Suggested changes

  • Keep the fixed freshness window, or plumb any configurable value through kanban.* configuration and the dispatcher.
  • Retain the fresh-heartbeat/PID-miss regression test and remove the duplicate completion test.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
making progress. Set ``HERMES_KANBAN_CRASH_HEARTBEAT_FRESH_SECONDS=0`` in
tests or unusual deployments to restore PID-only crash detection.
"""
raw = os.environ.get("HERMES_KANBAN_CRASH_HEARTBEAT_FRESH_SECONDS", "").strip()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a new non-secret, user-facing HERMES_* behavioral setting. Repository policy requires such knobs to be exposed through config.yaml; please keep the fixed bound or wire it through kanban.* configuration instead.

assert task.status == "ready"


def test_stale_run_completion_cannot_overwrite_new_current_run(kanban_home):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current main already covers this invariant in tests/hermes_cli/test_kanban_core_functionality.py:1516 and covers the worker environment path in tests/tools/test_kanban_tools.py:1645; the focused PID-miss regression test above is the new coverage needed here.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@GoliathInteractive GoliathInteractive closed this by deleting the head repository Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants