Skip to content

fix: preflight kanban task skills before spawning - #33747

Open
arterial2539-1 wants to merge 1 commit into
NousResearch:mainfrom
arterial2539-1:fix/kanban-skill-preflight
Open

fix: preflight kanban task skills before spawning#33747
arterial2539-1 wants to merge 1 commit into
NousResearch:mainfrom
arterial2539-1:fix/kanban-skill-preflight

Conversation

@arterial2539-1

Copy link
Copy Markdown

Summary

  • Preflight per-task skills against the assignee profile before Kanban dispatch claims/spawns a worker.
  • Block tasks immediately with an actionable reason when a required skill is missing or ambiguous, preventing Unknown skill(s) crash/retry loops.
  • Apply the same preflight to review-lane injected skills and allow block_task() to block review tasks.
  • Add regression coverage for missing and available profile skills.

Test Plan

  • pytest -q tests/hermes_cli/test_kanban_db.py::test_dispatch_blocks_ready_task_with_missing_profile_skill tests/hermes_cli/test_kanban_db.py::test_dispatch_spawns_when_profile_skill_exists tests/hermes_cli/test_kanban_db.py::test_dispatch_promotes_ready_and_spawns tests/hermes_cli/test_kanban_db.py::test_dispatch_spawn_failure_releases_claim tests/hermes_cli/test_kanban_core_functionality.py::test_spawn_failure_auto_blocks_after_limit tests/hermes_cli/test_kanban_core_functionality.py::test_spawned_event_emitted_with_pid
  • python -m compileall -q hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels May 28, 2026
@frankyh75

Copy link
Copy Markdown

Real-world reproduction: 1192 crashes, failure_limit bypass, system reboot

This PR addresses exactly what we hit in production. Sharing the case as additional context.

Setup: macOS (M1 Ultra), hermes-agent 0.14.0 (upgraded to 0.15.1 today), kanban.failure_limit: 2

What happened: A task was created with a misspelled skill name:

skills: foundry-mcp-bridge, foundry-vtt-mcp-macos   # ← typo, should be foundry-vtt-macos

The worker started, immediately raised ValueError: Unknown skill(s): foundry-vtt-mcp-macos, and exited with code 1 — before its first heartbeat.

Consequence: The dispatcher respawned the task every ~60 seconds for 2 days straight1192 consecutive crashes — without ever triggering the failure_limit: 2 auto-block.

Why failure_limit didn't fire: The crash happens before the worker writes its first heartbeat/lock entry. Without a lock entry, the dispatcher's crash detection (detect_crashed_workers) doesn't have a stable consecutive_failures counter to increment across ticks. Each dispatch cycle sees a "fresh" task claim with no prior failure record, so the counter never reaches 2.

Downstream impact:

  • System load peaked at 17.26 (20-core CPU) the following morning
  • The persistent background load contributed to a gateway crash loop (all 4 Discord bots reconnecting every 12s → 1000+ Discord reconnect attempts)
  • System rebooted

Workaround applied: Directly patched the skill name in the kanban SQLite DB, then blocked the task.

Suggestion beyond this PR: Even with preflight validation at creation time, it would be valuable to also handle the race condition where a skill is deleted/renamed after a task is created. A sticky auto-block after even 1 pre-heartbeat crash (classified separately from post-heartbeat crashes) would prevent this class of infinite loop entirely.

Thanks for the fix — this is a significant stability issue in long-running gateway deployments.

@hehehe0803

Copy link
Copy Markdown
Contributor

Related-work note to avoid parallel duplicate Kanban skill-preflight branches:

This PR overlaps the fail-closed preflight part of the existing forced-skill work:

Recommendation: reconcile this branch with #30025/#33640 rather than keep multiple independent preflight implementations open. If this branch has cleaner tests or simpler preflight structure, it may be a good base; #33640's unique contribution is the explicit allowlisted sync-before-block step and should be carried over only if maintainers want that operational behavior.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for targeting a real worker-startup failure. Current main still forwards task.skills directly into --skills at hermes_cli/kanban_db.py:8036-8039, so the preflight goal remains valid.

Problems

  • hermes_cli/kanban_db.py:5969-5975 implements only a subset of the runtime lookup. tools/skills_tool.py:1164-1170 also resolves a requested skill through its SKILL.md frontmatter name; this preflight would instead block that valid task before spawning.
  • hermes_cli/kanban_db.py:5983-5984 permits any unresolved colon-qualified name. The runtime validates plugin namespaces and registry lookup in tools/skills_tool.py:1002-1045, so an unavailable qualified skill can still reach startup and fail.

Suggested changes

  • Share or extract the runtime candidate-resolution logic, then cover frontmatter-name, external-dir collision, and unavailable qualified-plugin cases.
  • Salvage the logic into current dispatch_once paths: the dispatcher moved to hermes_cli/kanban_db.py:7240+ after 84e1d31e5, and the review path remains separately force-loaded at hermes_cli/kanban_db.py:7611-7616.

Automated hermes-sweeper review.

Comment thread hermes_cli/kanban_db.py
elif categorized_path.with_suffix(".md").is_file():
_record(categorized_path.with_suffix(".md"))

for found_skill_md in iter_skill_index_files(search_dir, "SKILL.md"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This scan is not equivalent to skill_view: it accepts a matching SKILL.md frontmatter name even when the directory basename differs (tools/skills_tool.py:1164-1170). Please reuse/extract that candidate resolution or parse frontmatter here; otherwise a valid forced skill is falsely blocked.

Comment thread hermes_cli/kanban_db.py
return None
if len(candidates) > 1:
return "ambiguous"
if ":" in name:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

An unresolved qualified name is not necessarily available. The runtime validates plugin namespaces and registry lookup before local fallback (tools/skills_tool.py:1002-1045); allowing every namespace:skill here can still send an unavailable skill to the child and preserve the startup crash loop.

@teknium1 teknium1 added 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 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants