Skip to content

fix(kanban): has_spawnable_ready excludes respawn-guarded tasks - #44899

Open
Zazzles2908 wants to merge 1 commit into
NousResearch:mainfrom
Zazzles2908:upstream-pr/has-spawnable-exclude-respawn-guarded
Open

Zazzles2908 wants to merge 1 commit into
NousResearch:mainfrom
Zazzles2908:upstream-pr/has-spawnable-exclude-respawn-guarded

Conversation

@Zazzles2908

Copy link
Copy Markdown

Summary

The dispatcher "stuck" detection uses has_spawnable_ready() to decide whether 0 spawned is a real stuck condition (genuinely spawnable work waiting) or a correctly-idle condition. But has_spawnable_ready() previously returned True for tasks that the dispatch path would correctly skip via check_respawn_guard — so the gateway fires a false "kanban dispatcher stuck" warning every 6 ticks (60s) when:

  • The ready queue has tasks (non-empty)
  • Those tasks are all respawn-guarded (rate_limit_cooldown, blocker_auth, recent_success, active_pr)
  • dispatch_once correctly defers them
  • But the health check thinks they're spawnable work

This is a 42-line addition to hermes_cli/kanban_db.py:5968-6022 that calls check_respawn_guard for each candidate and returns True only if at least one task is not guarded.

Root cause

has_spawnable_ready() and dispatch_once() use different criteria for "spawnable":

Function Criteria
has_spawnable_ready() (telemetry) ready AND assignee IS NOT NULL AND claim_lock IS NULL AND profile_exists(assignee)
dispatch_once() (actual dispatch) The above AND NOT respawn-guarded

The mismatch causes the false warning.

Repro

Before fix:

# Setup: single ready+ops task with a recent completed run
conn.execute("INSERT INTO tasks VALUES ('t2', NULL, NULL, 'ready', NULL, 'ops', NULL, 0, ?)", (now,))
conn.execute("INSERT INTO task_runs VALUES (NULL, 't2', ?, ?, 'completed', NULL, NULL, NULL)", (now-60, now))

has_spawnable_ready(conn)
# → True (WRONG: check_respawn_guard would return 'recent_success', so dispatch_once defers)
# Gateway logs: "kanban dispatcher: tick failed on board default"

After fix:

# Same task
has_spawnable_ready(conn)
# → False (correctly filters out the guarded task)
# No "stuck" warning. Dispatcher correctly idle.

Tested 6 scenarios:

  1. Unguarded ready task → True ✓
  2. Only guarded task (recent_success) → False ✓
  3. Only guarded task (blocker_auth) → False ✓
  4. Mixed (1 un-guarded + 1 guarded) → True (at least one spawnable) ✓
  5. No ready tasks → False ✓
  6. Mixed (1 un-guarded + 1 blocker_auth) → True ✓

Fix

 def has_spawnable_ready(conn: sqlite3.Connection) -> bool:
     rows = conn.execute(
-        "SELECT DISTINCT assignee FROM tasks "
+        "SELECT id, assignee FROM tasks "
         "WHERE status = 'ready' AND assignee IS NOT NULL "
         "    AND claim_lock IS NULL"
     ).fetchall()
     if not rows:
         return False
     try:
         from hermes_cli.profiles import profile_exists
     except Exception:
         return True
+    check_respawn_guard = globals().get("check_respawn_guard")
     for row in rows:
-        if profile_exists(row["assignee"]):
-            return True
+        if not profile_exists(row["assignee"]):
+            continue
+        if check_respawn_guard is not None:
+            try:
+                if check_respawn_guard(conn, row["id"]) is not None:
+                    continue
+            except Exception:
+                pass
+        return True
     return False

has_spawnable_review() also gets the id column added to the SELECT (consistency) and uses continue instead of early-return, but does NOT filter on guard — dispatch_once doesn't call check_respawn_guard on review tasks, so the review path stays consistent with the dispatch path.

Why fail-open?

If check_respawn_guard raises or is unavailable, we return True. This is the same posture as the existing profile_exists fallback (line 5999) and the existing dispatch_once path. We never want has_spawnable_ready to silently return False when a guard check transiently fails — that would suppress legitimate "stuck" warnings and hide real dispatch bugs.

Production evidence

The WSL2 consolidation hook at ~/.hermes/hooks/wsl2-kanban-monkeypatches/handler.py:241-367 implements this exact filtering logic. That hook has been live in the user's hermes-agent deployment since 2026-06-12 with 5 active workers and 0 false "stuck" warnings. Filing this PR retires the hook dependency for this specific fix.

Risk

  • Low. Adds a guard check to an existing per-candidate loop. Behavior change is conservative — the only change is that previously-True becomes False when all candidates are guarded.
  • No new code paths, no new dependencies.
  • Fail-open posture preserves legacy "warning still fires in degraded environments" behavior.
  • Compatible with the existing dispatch_once invariant (which already skips guarded tasks via check_respawn_guard).

Test plan

  • Synthetic test: 6 scenarios (unguarded, recent_success guard, blocker_auth guard, mixed, empty, mixed with blocker_auth) — all 6 pass
  • Run existing tests/hermes_cli/test_kanban_db.py tests for has_spawnable_ready and has_spawnable_review
  • Verify no existing test relies on the (now-changed) unguarded-but-guarded behavior

Bead

  • prod-audit-wr2yw (parent epic for upstream PRs in this batch)

Note: this PR is the third of a 3-PR series that retires the WSL2 consolidation hook dependency:

Each PR is independently mergable. They fix 3 separate bugs, not a chain.

The dispatcher 'stuck' detection uses has_spawnable_ready() to
decide whether '0 spawned' is a real stuck condition (genuinely
spawnable work waiting) or a correctly-idle condition (only
control-plane lanes waiting). But has_spawnable_ready() previously
returned True for tasks that the dispatch path would correctly
skip via check_respawn_guard — so the gateway fires a false
'kanban dispatcher stuck' warning every 6 ticks (60s) when:

  - the ready queue has tasks (non-empty)
  - those tasks are all respawn-guarded (rate_limit_cooldown,
    blocker_auth, recent_success, active_pr)
  - dispatch_once correctly defers them
  - but the health check thinks they're spawnable work

Root cause: has_spawnable_ready() and dispatch_once() use
DIFFERENT criteria for 'spawnable'. has_spawnable_ready() checks
ready+assigned+unclaimed+valid-profile. dispatch_once() checks
the same AND NOT respawn-guarded. The mismatch causes the
false warning.

Fix: call check_respawn_guard for each candidate task and return
True only if at least one is NOT guarded. Same posture as the
patch 4 fix to check_respawn_guard itself (result IS NOT NULL
early-return). Fail-open: if check_respawn_guard raises, assume
spawnable — same behavior as the legacy code path.

has_spawnable_review(): mirror the function signature for
compatibility but deliberately do NOT filter on guard (the
dispatch path doesn't call check_respawn_guard on review tasks
— that's a different layer of the system).

Repro before fix:
  Single task in 'ready' with completed task_runs row (recent_success)
  has_spawnable_ready() returns True
  dispatch_once() defers the task
  Gateway logs: 'kanban dispatcher: tick failed on board default'

Repro after fix:
  Same task, has_spawnable_ready() returns False
  No 'stuck' warning. Dispatcher correctly idle.

Production evidence: WSL2 hook
~/.hermes/hooks/wsl2-kanban-monkeypatches/handler.py:241-367
implements this exact filtering. Live dispatch health: 5 active
workers, 0 failed ticks since 2026-06-12.

Closes: bead prod-audit-wr2yw
Tested: 6-scenario test (unguarded, recent_success guard,
blocker_auth guard, mixed, empty, mixed with blocker_auth) —
all 6 pass
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management labels Jun 12, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Good telemetry fix. has_spawnable_ready now also checks check_respawn_guard so the gateway health telemetry does not false-positive a "kanban dispatcher stuck" warning every 6 ticks for respawn-guarded tasks.

Looks Good

  • Uses globals().get("check_respawn_guard") for safe fallback in partial-install environments
  • The check is applied to each individual task row rather than batch-filtered, allowing the loop to short-circuit
  • Clear documentation distinguishing ready-task guard checking from review-task (which does not use check_respawn_guard in the dispatch path)
  • Documented note explaining why review tasks intentionally skip the guard filter

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for tracing the dispatcher-health mismatch. The current-head premise is confirmed: dispatch_once() skips a task when check_respawn_guard() returns a reason (hermes_cli/kanban_db.py:7473-7485), while has_spawnable_ready() still returns true solely from profile existence (hermes_cli/kanban_db.py:7131-7146). The gateway uses that helper for its stuck-warning predicate (gateway/kanban_watchers.py:1101-1104, 1253-1268).

Problems

  • The PR changes this predicate without adding a regression test. Existing helper coverage at tests/hermes_cli/test_kanban_db.py:1680-1709 does not cover a valid-profile task guarded by recent_success, active_pr, or blocker_auth.

Suggested changes

  • Add helper-level cases for all-guarded → False, guarded plus unguarded → True, and a guard exception preserving the documented fail-open True behavior.

Automated hermes-sweeper review.

@KostaGorod

Copy link
Copy Markdown

Additional current-lane and diagnostics work is tracked in #126263 . Current upstream dispatch checks lane-specific guards for review too, so the older review-lane assumption needs adaptation. The follow-up will preserve guard policy and mixed-queue stall detection and credit this work.

@KostaGorod

Copy link
Copy Markdown

Additional complementary work is tracked in #126263 and draft PR #126268 (#126268). This follow-up aligns the split upstream ready/review spawnability probe with lane-specific guards and adds the per-task historical guard explanation; it does not replace this PR's own scope.

This branch has not been deployed

No deployments
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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants