Skip to content

Fix two liveness false-negatives: MCP stdio subprocess + builtin cron ticker - #95947

Closed
magnuslundstedt wants to merge 2 commits into
NousResearch:mainfrom
magnuslundstedt:fix/liveness-false-negatives
Closed

magnuslundstedt wants to merge 2 commits into
NousResearch:mainfrom
magnuslundstedt:fix/liveness-false-negatives

Conversation

@magnuslundstedt

Copy link
Copy Markdown
Contributor

Two independent liveness checks report a live thing as dead; each surfaces as a spurious, user-facing failure. Both were found and fixed while running Hermes on macOS (launchd) and verified live.

1. tools/mcp_tool.py::_stdio_children_dead — inverted loop (commit 1)

Documented as "True when every stdio child we spawned has exited", but the loop returns True as soon as it finds a child that is still alive:

    if not psutil.pid_exists(pid):
        continue          # dead → check next
    return True           # ← a LIVE child wrongly reports "all dead"
    return False          # ← unreachable dead code

So the #81995 fast-fail path (_stdio_children_dead()TimeoutError) aborts every stdio MCP tool call with MCP stdio subprocess for 'X' has exited while the servers are perfectly alive — the processes (and their watchdog/sudo wrappers) are running; only this client-side liveness check is wrong. It triggers whenever _stdio_child_pids is populated, taking down all stdio MCP tools at once.

Fix: return False as soon as any tracked child is alive; True only when none are — the correct meaning of the docstring.

2. hermes_cli/cron.py::_builtin_gateway_liveness — flaky PID scan (commit 2)

It decides whether the builtin cron ticker can fire via find_gateway_pids() (a PID scan). That scan can transiently return empty even while the gateway is up (e.g. right after a restart), so the in-gateway cronjob tool emits a false Gateway is not running — jobs won't fire while jobs are firing on schedule.

Fix: prefer the gateway runtime lock — it is held for exactly the gateway's lifetime (a reliable liveness signal) and short-circuits to True inside the gateway process, so the in-gateway check can never false-alarm. Falls back to the PID scan only when the lock reads inactive (the external-CLI path).

Both changes are minimal and self-contained.

_stdio_children_dead() is documented as 'True when every stdio child we spawned has
exited', but the loop returns True as soon as it finds a child that is still ALIVE:

    if not psutil.pid_exists(pid):
        continue          # dead → check next
    return True           # ← a LIVE child wrongly reports 'all dead'
    return False          # ← unreachable

So the NousResearch#81995 fast-fail path (_stdio_children_dead() → raise TimeoutError) aborts every
stdio MCP tool call with 'MCP stdio subprocess for X has exited' while the servers are
perfectly alive. Return False as soon as any tracked child is alive; return True only
when none are — the correct meaning of the docstring.
_builtin_gateway_liveness() decides whether the builtin cron ticker can fire by
PID-scanning via find_gateway_pids(). That scan can transiently return empty even
while the gateway is up (e.g. just after a restart), so the in-gateway cronjob tool
emits a false 'Gateway is not running — jobs won't fire' while jobs are firing on time.

Prefer the gateway runtime lock: it is held for exactly the gateway's lifetime (a
reliable liveness signal), and inside the gateway process it short-circuits to True,
so the in-gateway check can never false-alarm. Fall back to the PID scan only when the
lock reads inactive (the external-CLI path).
@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management comp/tools Tool registry, model_tools, toolsets tool/mcp MCP client and OAuth P1 High — major feature broken, no workaround duplicate This issue or pull request already exists labels Aug 27, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #94339 for the MCP liveness repair: both reverse the same live-PID branch in _stdio_children_dead(). This PR also overlaps #94155's cron self-liveness work.

kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Aug 27, 2026
Sibling site of the salvaged NousResearch#95947 fix (same file): cron_status
declared 'Gateway is not running — cron jobs will NOT fire' from a bare
find_gateway_pids() miss even while the runtime lock proved the gateway
alive. Now the not-running verdict requires both the scan AND the lock
to read dead; when only the lock answers, the pid line falls back to
the recorded gateway pid (or is omitted).

Two regression tests pin the false-alarm suppression and the genuine
not-running warning.
@kshitijk4poor

Copy link
Copy Markdown
Contributor

Closing — both halves of this PR have now landed on main, with your cron commit merged via salvage:

Thanks for both finds — the live verification on launchd made the triage straightforward.

kshitijk4poor added a commit that referenced this pull request Aug 27, 2026
Follow-ups to the salvaged #95947 cron commit:

- Wrap the lock probe in its own try/except: a crashing probe is
  'unknown', not 'dead' — the pid scan still decides instead of the
  whole tri-state collapsing to None.
- Regression tests (shape adapted from #94155 by @liuhao1024): lock
  held + empty pid scan -> alive (the reported false alarm); lock
  inactive -> pid-scan fallback both ways; crashing lock probe still
  falls back.
- patch_liveness now pins the lock probe inactive by default so the
  pre-existing pid-scan tests stay deterministic on machines where a
  real gateway holds the real lock.
- contributors mapping for magnus.lundstedt@infidyne.com.

Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
kshitijk4poor added a commit that referenced this pull request Aug 27, 2026
Sibling site of the salvaged #95947 fix (same file): cron_status
declared 'Gateway is not running — cron jobs will NOT fire' from a bare
find_gateway_pids() miss even while the runtime lock proved the gateway
alive. Now the not-running verdict requires both the scan AND the lock
to read dead; when only the lock answers, the pid line falls back to
the recorded gateway pid (or is omitted).

Two regression tests pin the false-alarm suppression and the genuine
not-running warning.
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
Follow-ups to the salvaged NousResearch#95947 cron commit:

- Wrap the lock probe in its own try/except: a crashing probe is
  'unknown', not 'dead' — the pid scan still decides instead of the
  whole tri-state collapsing to None.
- Regression tests (shape adapted from NousResearch#94155 by @liuhao1024): lock
  held + empty pid scan -> alive (the reported false alarm); lock
  inactive -> pid-scan fallback both ways; crashing lock probe still
  falls back.
- patch_liveness now pins the lock probe inactive by default so the
  pre-existing pid-scan tests stay deterministic on machines where a
  real gateway holds the real lock.
- contributors mapping for magnus.lundstedt@infidyne.com.

Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
Sibling site of the salvaged NousResearch#95947 fix (same file): cron_status
declared 'Gateway is not running — cron jobs will NOT fire' from a bare
find_gateway_pids() miss even while the runtime lock proved the gateway
alive. Now the not-running verdict requires both the scan AND the lock
to read dead; when only the lock answers, the pid line falls back to
the recorded gateway pid (or is omitted).

Two regression tests pin the false-alarm suppression and the genuine
not-running warning.
Izzy-Gottz added a commit to Izzy-Gottz/hermes-agent that referenced this pull request Aug 31, 2026
`hermes cron status` and the `cronjob` model tool both decided liveness
from two proxies for the ticker's process: a `find_gateway_pids` scan
matching on argv, and the gateway runtime lock. A gateway supervised by
something other than `hermes gateway install` — an embedding app that
spawns it as a grandchild of its own wrapper — is invisible to both, so
a perfectly healthy scheduler reported:

    ✗ Gateway is not running — cron jobs will NOT fire

while its heartbeat was 57 seconds old and jobs were firing on time.
This is the third site of the class fixed for the lock in NousResearch#95947, and it
was patched there by adding another indirect probe.

The ticker writes `cron/ticker_heartbeat` itself, from inside its own
loop, on every iteration. That is direct evidence of the only thing the
question is actually about, so consult it: `_ticker_heartbeat_proves_alive`
votes only "alive", and only on a fresh stamp — missing (None) or stale
is not evidence either way and leaves the existing signals to decide.
The pre-existing STALLED report for an observed-but-silent gateway is
therefore untouched.

`cron status` also stops overclaiming: when the heartbeat is the sole
witness it says the ticker is running, not the gateway, because the
process was never actually seen.

The stale threshold moves to `_ticker_stale_after_seconds` so the
liveness probe and the stall report cannot drift apart.

8 tests alongside the NousResearch#95947 ones: fresh heartbeat proves liveness with
no pid and no lock; stale and missing do not; a positive pid scan is
never overridden; status reports firing on the heartbeat alone, names
only the ticker when that is all it saw, still warns when stale, and
still reports STALLED for an observed gateway with a dead ticker.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QU3jn4k6Uuf2DR6HytuZhi
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Follow-ups to the salvaged NousResearch#95947 cron commit:

- Wrap the lock probe in its own try/except: a crashing probe is
  'unknown', not 'dead' — the pid scan still decides instead of the
  whole tri-state collapsing to None.
- Regression tests (shape adapted from NousResearch#94155 by @liuhao1024): lock
  held + empty pid scan -> alive (the reported false alarm); lock
  inactive -> pid-scan fallback both ways; crashing lock probe still
  falls back.
- patch_liveness now pins the lock probe inactive by default so the
  pre-existing pid-scan tests stay deterministic on machines where a
  real gateway holds the real lock.
- contributors mapping for magnus.lundstedt@infidyne.com.

Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
Sibling site of the salvaged NousResearch#95947 fix (same file): cron_status
declared 'Gateway is not running — cron jobs will NOT fire' from a bare
find_gateway_pids() miss even while the runtime lock proved the gateway
alive. Now the not-running verdict requires both the scan AND the lock
to read dead; when only the lock answers, the pid line falls back to
the recorded gateway pid (or is omitted).

Two regression tests pin the false-alarm suppression and the genuine
not-running warning.
zapabob pushed a commit to zapabob/hermes-agent-windows that referenced this pull request Sep 5, 2026
Follow-ups to the salvaged NousResearch#95947 cron commit:

- Wrap the lock probe in its own try/except: a crashing probe is
  'unknown', not 'dead' — the pid scan still decides instead of the
  whole tri-state collapsing to None.
- Regression tests (shape adapted from NousResearch#94155 by @liuhao1024): lock
  held + empty pid scan -> alive (the reported false alarm); lock
  inactive -> pid-scan fallback both ways; crashing lock probe still
  falls back.
- patch_liveness now pins the lock probe inactive by default so the
  pre-existing pid-scan tests stay deterministic on machines where a
  real gateway holds the real lock.
- contributors mapping for magnus.lundstedt@infidyne.com.

Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
zapabob pushed a commit to zapabob/hermes-agent-windows that referenced this pull request Sep 5, 2026
Sibling site of the salvaged NousResearch#95947 fix (same file): cron_status
declared 'Gateway is not running — cron jobs will NOT fire' from a bare
find_gateway_pids() miss even while the runtime lock proved the gateway
alive. Now the not-running verdict requires both the scan AND the lock
to read dead; when only the lock answers, the pid line falls back to
the recorded gateway pid (or is omitted).

Two regression tests pin the false-alarm suppression and the genuine
not-running warning.
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 comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P1 High — major feature broken, no workaround tool/mcp MCP client and OAuth type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants