Repository navigation
Conversation
|
Thanks for isolating the current gateway false-positive and for adding real Problems
Suggested changes
Automated hermes-sweeper review. |
1a7c2bf to
5419346
Compare
|
@teknium1 — your second point is correct and I am not going to defend it. The fix: match on the |
Rebase telemetry into kanban_db_dispatch and retain current watcher plumbing. Only rate_limit_cooldown respawn guards suppress stalls; auth and other non-capacity guards remain actionable. Record early workspace and launch failures in both dispatch lanes. Verified with scripts/run_tests.sh: 79 passed, 1 Windows-only skip across watcher, database, workspace and review lifecycle suites. Restoring the old watcher predicate fails the cooldown warning regression (1 failed, 1 passed).
5419346 to
91ef796
Compare
|
Rebased onto current |
hermes_cli/kanban_db_dispatch.py: upstream classified a RestartSafeScopeUnavailable launch failure as infrastructure (NousResearch#114720) in the same except block where the PR records result.spawn_failed. Kept both: upstream's classification first, then the PR's per-tick record, so a host that cannot place workers still counts as a fault for the stall detector. Upstream's new "infrastructure_cooldown" respawn-guard reason falls into the PR's fail-closed else-branch (fault, not benign), which is the intended reading. tests/gateway/test_kanban_watchers_mixin.py: deleted upstream in a test purge (61fa0f4); the PR had added the singleton-lock and stall-detector tests to it. Kept the file with only the PR-added tests; the pruned method-list test and its import stay gone. Tests: test_kanban_watchers_mixin.py + test_kanban_stall_warning.py 16 passed; test_kanban_db.py -k "spawn_failure or spawn_failed or failure_telemetry or infrastructure or respawn" 16 passed. ruff clean.
fix(kanban): stop the false "check profile health" stall warning when the dispatcher is throttled
Symptom
The gateway kanban dispatcher logs, every 5 minutes:
…for the entire duration of a provider rate-limit window, or of any large
fan-out where every eligible profile is at its concurrency cap. Nothing is
broken: the dispatcher resumes spawning normally the moment quota frees, and
there are zero
spawn_failedevents in the whole window. Users are told togo audit their venv, PATH and credentials for a problem that does not exist.
Root cause
gateway/kanban_watchers.py, in_kanban_dispatcher_watcher:ready_pending and not any_spawnedis not only the "stuck" condition — it isalso the healthy steady state whenever the dispatcher declines to spawn
for a benign, self-clearing reason:
DispatchResultbucketkanban.max_in_progress_per_profileskipped_per_profile_cappedreadywithout counting a failure)rate_limitedskipped_lockedrespawn_guardedDispatchResultalready buckets each of those separately from real faults —precisely so "busy/throttled" can be told apart from "genuinely stuck". Their
field docstrings say so verbatim (e.g.
skipped_per_profile_capped: "NOT anoperator-actionable failure … Separate bucket so telemetry / dashboards can
show 'this profile is busy' vs 'task is genuinely stuck'"). The detector
simply never consulted them.
Second defect found while fixing the first: cross-board fault masking
The detector aggregates across all boards in one tick. The only "genuine
fault" signal available was
DispatchResult.auto_blocked, which is populatedonly when a board's spawn-failure circuit breaker trips — after
failure_limitconsecutive failures.So an early, pre-breaker spawn failure on board A (broken venv / PATH /
credentials; failures
1 .. failure_limit-1) was invisible as a fault. Abenign decline on board B in the same tick then reset the streak, masking A's
real stall until A's breaker finally tripped several ticks later. On a
multi-board gateway that alignment is routine, not theoretical — and it delays
the one warning a user actually needs.
Fix
hermes_cli/kanban_db.py— addDispatchResult.spawn_failed, populatedat every
_record_spawn_failurecall site (all four: workspace-resolutionand worker-launch failures, in both the ready and the review dispatch
columns), whether or not that failure tripped the breaker.
auto_blockedstays the breaker-trip subset;
spawn_failedis the superset includingearly failures. Purely additive: no existing field or call signature changes.
gateway/kanban_watchers.py— extract the bad-tick decision into a pure,unit-testable
_stall_streak_is_bad(ready_pending, any_spawned, results)that counts a zero-spawn tick only when it is not explained by a benign
decline, or when a genuine fault (
spawn_failed/auto_blocked) ispresent anywhere in the tick. The true positive the warning exists to catch
still fires — and now fires on failure Terminal tool #1 instead of waiting for the
breaker.
The warning text now states that no benign decline explains the streak, so a
real fire is unambiguous rather than one more cry-wolf line.
Tests
tests/gateway/test_kanban_watchers_mixin.py— 11 behavior-contract testscovering both directions (each of the four benign declines stays quiet; a bare
unexplained zero-spawn, an
auto_blockedfault alongside a benign siblingboard, and an early
spawn_failedalongside a benign sibling board all count),plus an invariant test asserting every bucket name the detector reads actually
exists on
DispatchResult— the detector reads them viagetattr(res, name, None), so a rename would otherwise degrade it silently to"never fires".
The fake-result helper constructs a real
DispatchResult, not a stand-in,so the tests are pinned to the production dataclass.
tests/hermes_cli/test_kanban_db.py— 2 E2E tests through the realdispatch_oncewith a failingspawn_fn: failure #1 of 5 lands inspawn_failedand not inauto_blocked(the previously-invisible window),and with
failure_limit=1the task lands in both, pinningauto_blocked ⊆ spawn_failed.RED proof (production logic reverted to the pre-fix rule, tests unchanged):
6 of the 13 new tests fail — the four benign-decline cases and both E2E
spawn_failedcases. SeeTEST-EVIDENCE.md.GREEN:
tests/gateway/test_kanban_watchers_mixin.py+tests/hermes_cli/test_kanban_db.py= 247 passed (234 pre-existing onclean
main, +13 new). Zero pre-existing tests modified.