Skip to content

fix(gateway): expire orphaned drain markers past a max-age so a leaked marker can't wedge the gateway - #85444

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/85433-drain-marker-max-age
Aug 13, 2026
Merged

fix(gateway): expire orphaned drain markers past a max-age so a leaked marker can't wedge the gateway#85444
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/85433-drain-marker-max-age

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

A .drain_request.json marker orphaned without a machine restart wedges the gateway in draining indefinitely — every inbound message on every platform is bounced with "⏳ This agent is draining for a maintenance action…" until someone manually deletes the marker. Observed in the field: a Hermes Cloud instance refused all Telegram turns for ~3 days (marker written for a maintenance action that completed without recreating the container; the writer never cancelled the drain).

The NS-570 epoch stamp (#53050) only clears markers that survive a restart — it bakes in the assumption that every drain-gated action ends in one. A same-epoch orphan passes the epoch check forever, and the marker's requested_at was write-only: no reader ever looked at it. Compare the neighbouring .restart_notify.json marker, which gateway/run.py guards with a 5-minute requested_at staleness check for exactly this reason.

Fixes #85433.

Changes

  • gateway/drain_control.py:
    • New _marker_is_expired(): a marker whose requested_at parses and is older than DRAIN_REQUEST_MAX_AGE_SECONDS (1h) reads as stale. Same leniency contract as the epoch check — a missing/unparseable timestamp still reads as drain-active (fail-safe toward quiescing), a future-dated timestamp (clock skew) is honoured, a tz-naive timestamp is treated as UTC.
    • New _marker_is_stale() composes epoch-mismatch OR expiry; drain_requested() and drain_notification_suppressed() both route through it (all external callers — the 1s gateway watcher, the shutdown broadcast gate, and the dashboard status endpoint — go through these two readers, so every path is covered).
    • Expiry logs a warning once per marker (keyed by requested_at), not once per 1s poll tick (~86k/day otherwise). A refreshed marker that expires again warns again.
    • Long drains keep a sanctioned keep-alive: write_drain_request() is idempotent and re-writing refreshes requested_at (docstring updated to say so).

Before / after

Before: marker written at 20:04 for a no-restart maintenance action, never cleared → gateway bounces every message with the draining text indefinitely (same epoch → honoured on every 1s tick).

After: the same orphan is honoured for at most 1h, then reads as absent; the watcher flips the gateway back to running on the next tick and logs one warning naming the marker's timestamp, age, and principal.

Validation

  • tests/gateway/test_external_drain_control.py: 7 new tests — expired marker reads as absent (drain state AND suppression flag), missing timestamp honoured, unparseable timestamp honoured, tz-naive timestamp expires, keep-alive re-write revives an expired drain, expiry warning fires once across repeated polls and re-fires for a refreshed marker.
  • Full file: 18 passed. Wider -k "drain or restart" slice over tests/gateway/: 168 passed, 3 skipped.
  • Mutation check: reverting gateway/drain_control.py to the pre-fix version makes the 4 new-behavior guard tests fail (the leniency tests correctly still pass — they assert behavior that predates the fix); restoring goes green.
  • ruff check clean on both files.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 13, 2026
kshitijk4poor pushed a commit that referenced this pull request Aug 13, 2026
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.

This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).

Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.

Salvaged from #65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).

Fixes the red slice on #85444, #85452 and every other open PR.

Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
…d marker can't wedge the gateway (NousResearch#85433)

The NS-570 epoch stamp clears a drain marker that survives a machine
restart — but it assumes every drain-gated action ends in a restart. When
a maintenance action completes WITHOUT recreating the container and the
writer never cancels the drain, the orphaned marker still carries the
current epoch, so the 1s drain watcher honours it forever and the gateway
bounces every inbound message with the 'draining for a maintenance
action' text (observed in the field: a Hermes Cloud instance refused all
Telegram turns for ~3 days).

The marker already records requested_at; now the readers check it. A
marker older than DRAIN_REQUEST_MAX_AGE_SECONDS (1h) reads as stale in
drain_requested() and drain_notification_suppressed(), with a loud
warning log. Leniency mirrors the epoch check: a missing or unparseable
timestamp still reads as drain-active (fail-safe toward quiescing), and
a legitimately long drain keeps a sanctioned keep-alive — re-calling
write_drain_request() refreshes requested_at.

Fixes NousResearch#85433
@kshitijk4poor
kshitijk4poor force-pushed the fix/85433-drain-marker-max-age branch from be2ae0d to 81009f8 Compare August 13, 2026 17:50
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 13, 2026 17:50
@kshitijk4poor
kshitijk4poor merged commit 8855766 into NousResearch:main Aug 13, 2026
43 checks passed
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.

This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).

Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.

Salvaged from NousResearch#65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).

Fixes the red slice on NousResearch#85444, NousResearch#85452 and every other open PR.

Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
bobaba76 pushed a commit to bobaba76/hermes-agent that referenced this pull request Aug 27, 2026
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.

This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).

Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.

Salvaged from NousResearch#65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).

Fixes the red slice on NousResearch#85444, NousResearch#85452 and every other open PR.

Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway: same-epoch orphaned .drain_request.json has no TTL — agent bounces every message indefinitely ("draining for a maintenance action")

2 participants