fix(kanban): gate task.skills on resolvability to prevent worker crash loops

[noestelar]

Problem

The dispatcher already gates the built-in --skills kanban-worker injection on resolvability under the worker's HERMES_HOME (see _kanban_worker_skill_available at hermes_cli/kanban_db.py:5202). The reason is documented in that function's docstring: preloading a missing skill is fatal at CLI startup (ValueError: Unknown skill(s): <name>), aborting the worker before the agent loop runs.

But per-task task.skills entries (hermes_cli/kanban_db.py:5387-5389 before this patch) were passed through unconditionally:

if task.skills:
    for sk in task.skills:
        if sk and sk != "kanban-worker":
            cmd.extend(["--skills", sk])

So a task whose skill name only exists in the global skills root — not the profile-scoped one the worker actually loads — crash-loops the worker until the dispatcher's watchdog eventually auto-blocks it.

Observed impact

A task with skills=['mlflow-eval-datasets-for-llm-pipelines'] running under a profile-scoped HERMES_HOME respawned 1032 times before being blocked manually, because the skill lives in ~/.hermes/skills/mlops/ but not in ~/.hermes/profiles/<name>/skills/mlops/. The CLI fails inside _apply_skills_overrides before any tool can call block, so the lifecycle's normal error path never runs.

Watchdog auto-block fires far too late (hundreds of respawns) when the failure happens before the agent loop.

Fix

• Generalize _kanban_worker_skill_available() into _skill_available_for_home(skill_name, hermes_home). The legacy name is kept as a back-compat shim that delegates, so external callers (if any) aren't broken.
• Apply the same gate to every task.skills entry in _default_spawn. Skipped skills emit a single-line stderr warning naming the task id and the missing skill so operators triaging logs see why the worker did not pick up the requested skill. The task still proceeds — running without the supplementary skill context is strictly better than crash-looping until the watchdog gives up.

Contract change

This is a deliberate fail-fast → silent-skip-with-warning shift for task.skills. Trade-off:

• Before: dispatcher trusts the caller, missing skill crashes the worker loudly (in the kanban log) but invisibly to the operator (until the auto-block triggers).
• After: dispatcher validates against the actual filesystem the worker will load, drops the unresolvable flag, warns on stderr, and lets the worker run. Operators see the warning in the kanban log alongside the worker's normal output.

If a stricter posture is preferred — e.g. task.skills is treated as a hard contract and should fail the task immediately — I'm happy to flip this to kanban_db.block_task(...) instead of a warning. The current behavior matches what _kanban_worker_skill_available already does for the built-in skill.

Tests

• Existing tests (test_default_spawn_appends_per_task_skills, test_default_spawn_dedupes_kanban_worker_from_task_skills) updated to stub the new helper so synthetic skill names (translation, github-code-review) still resolve.
• New regression test test_default_spawn_drops_unresolvable_task_skills exercises the mixed-resolvability path: one resolvable skill + one missing skill, asserts only the resolvable one reaches argv, and asserts the stderr warning contains both the task id and the missing skill name.

56 passed, 269 deselected in 0.95s

(spawn or skill filter across tests/hermes_cli/test_kanban_core_functionality.py + tests/hermes_cli/test_kanban_db.py.)

Files

• hermes_cli/kanban_db.py — generalize helper + gate task.skills
• tests/hermes_cli/test_kanban_core_functionality.py — stub new helper in existing tests + add regression test

[hehehe0803]

Thanks for tackling this crash-loop class. I agree with the core direction: the dispatcher should not pass --skills <name> to a profile worker unless the skill is resolvable in the worker's effective skill environment.

One semantics question: should unresolved task.skills be silently dropped, or should the task be blocked before spawn?

I think Kanban should treat current task.skills as required by default and block before spawning when a requested skill cannot resolve under the assignee profile. Rationale:

• A task author usually adds skills=[...] because the worker probably needs that procedure/context to deliver the task correctly.
• Dropping the skill lets the worker run, but it can produce low-quality or misleading output while appearing successful.
• Blocking before spawn makes the failure deterministic and actionable: the operator can install/sync/allow the skill, then unblock the card.
• It distinguishes infrastructure/profile drift from a domain-task failure.

A future extension could add an explicit optional skill channel, for example optional_skills, where unresolved skills warn/drop and continue. But for the existing task.skills field, fail-closed seems safer.

Suggested behavior:

1. Resolve the assignee profile's effective skill roots before spawn.
2. Validate every task-level required skill against that environment.
3. If any are missing, do not spawn the worker.
4. Block the task with a normal blocked event and a message like:
   missing required skill(s) for profile '<profile>': <skill>. Install/sync/allow the skill and unblock this task.
5. Keep existing spawn-failure handling unchanged for non-skill startup errors.

Tests I would expect:

• missing required task skill blocks before worker spawn;
• available skill by directory path/name passes;
• available skill by SKILL.md frontmatter name passes, if Hermes supports that resolution style elsewhere;
• normal spawn failure behavior still releases the claim;
• optional/missing skill behavior can be added separately if/when an optional field exists.

Separately, I opened a related feature request for task-scoped read-only skill overlays plus a lightweight orchestrator skill catalog: #33245. That feels bigger than this PR and probably should not be mixed into the minimal crash-loop fix.

[hehehe0803]

I did a related-issue/PR scan to place this PR in the existing Kanban/skills work. The short version: this PR is the right place for the minimal crash-loop fix, while broader task-scoped skill availability probably belongs in #33245.

Relevant nearby work:

• fix(cli): unknown skills should warn instead of crash to prevent Kanban worker auto-blocking #27136 / fix(cli): unknown skills should warn instead of crash #27201 — CLI-level unknown-skill mitigation. Warn/continue is useful in some flows, but for Kanban task.skills it can silently degrade task quality if those skills were intended as required procedure.
• fix: surface kanban completion evidence and validate skills #28752 — validates per-task skill identifiers and improves evidence surfacing.
• fix(kanban): preserve task-specific skills during review dispatch #28711 — preserves task-specific skills during review dispatch; important so review handoffs do not erase explicit skill requirements.
• fix(kanban): pass skills from decomposer to child tasks #28875 — lets decomposer-generated child tasks carry skills; makes this preflight path more important because decomposers may create skill-bearing tasks automatically.
• fix(kanban): skip invalid task skills and add recovery commands #23154 / fix(kanban): harden runtime recovery and worker startup #23827 / [Feature]: Harden Kanban task validity, dispatch preflight, and worker ownership #23209 — prior Kanban task validity/preflight/recovery hardening; this PR fits that lineage.
• Bug: kanban_swarm.py references missing skill avoid-ai-writing, synthesizer crashes in retry loop (v0.14.0) #29415 / fix(kanban): remove missing avoid-ai-writing skill from swarm synthesizer #29485 — missing hard-coded swarm skill caused Unknown skill(s) crash loops; concrete example of the failure class.
• Add Claude CLI runtime and Kanban auth preflight #31461 — adjacent pattern: preflight runtime prerequisites before claiming/spawning workers.
• sync_skills writes into profile homes that delegate to default via external_dirs, causing skill name collisions that crash worker agents #28126 / fix: sync_skills skips bundled skills already in external_dirs #28187 — profile external_dirs/sync_skills collisions; important caution against solving task skills by blindly copying/shadowing skills into profile homes.

For the broader feature side:

• Kanban: task-scoped read-only skill overlays and orchestrator skill catalog #33245 — task-scoped read-only skill overlays + orchestrator skill catalog.
• Feature: policy-driven local skill overlays and profile sync preflight #29714 / Skills overlay architecture: separate bundled (upstream) from custom (user) layers #16852 — broader skill overlay/profile sync architecture.
• feat: support project-local skills and stack-based recommendations #14662 / Add project-local skill discovery #17328 / fix(skills): skill_manage create respects skills.external_dirs #22236 — multiple/project/external skill roots and management.
• fix: reduce gateway context burn from skills catalog injection #12015 / fix(anthropic): default skills catalog to names-only to reduce prompt token usage #28929 / Proposal: Move Hermes toward a more progressive-loading architecture (minimal startup context, upgrade-on-demand toolsets) #16493 / feat: lazy skill loading with per-session deduplication #5938 — context-bloat/progressive-loading/lazy-skill-loading prior art.
• feat(capabilities): add capabilities_list tool for orchestrator profile discovery #25247 — capabilities_list for orchestrator profile discovery; likely a good architectural neighbor for skill/capability catalog metadata.

My recommendation after the scan:

1. Keep this PR narrow: dispatcher preflights current task.skills and avoids worker startup crash loops.
2. Treat current task.skills as required/fail-closed, or at least make missing-required behavior explicit. A separate future optional_skills channel could warn/drop and continue.
3. Do not solve this by profile mutation/copying skills into profile homes; sync_skills writes into profile homes that delegate to default via external_dirs, causing skill name collisions that crash worker agents #28126/fix: sync_skills skips bundled skills already in external_dirs #28187 show why that can create shadow/collision failures.
4. Keep task-scoped overlay/capability catalog work in Kanban: task-scoped read-only skill overlays and orchestrator skill catalog #33245 so this PR stays reviewable.

So the design split I’d suggest is:

• Here: required skill preflight and deterministic block/skip semantics before spawn.
• Kanban: task-scoped read-only skill overlays and orchestrator skill catalog #33245: read-only task-scoped skill roots, orchestrator catalog/capability metadata, audit trail, required/optional distinction, duplicate/disabled policy.

[hehehe0803]

Follow-up from live Kanban dogfood today: the fail-closed preflight semantics are still the right default, but there is one missing middle step for installs that have an explicit profile-skill sync policy.

Observed workflow:

1. A task had task.skills=["github-code-review"] and assignee code-reviewer.
2. Dispatcher preflight correctly detected the skill was missing in the worker profile and blocked before spawn. Good: no crash loop.
3. But in this install, github-code-review is a shared/reviewer skill that should be auto-created by the allowlisted profile sync policy. The intended behavior was:
   • detect missing forced skill;
   • if profile + skill are allowlisted and create_missing_skill_dirs: true, run profile skill sync once;
   • re-check skill resolvability under the worker HERMES_HOME;
   • spawn worker if resolved;
   • block only if still missing, not allowlisted, or sync fails.

So I think the safest contract is slightly more nuanced than just “block missing skill” or “drop missing skill”:

required task.skills missing
→ attempt explicit allowlisted sync/overlay resolution if configured
→ re-check effective worker skill roots
→ if resolved: spawn normally
→ if unresolved: block before spawn with actionable reason

This preserves the fail-closed behavior I argued for earlier, but avoids human intervention for known-good shared skills where the local policy already says the profile may receive them. It also avoids the dangerous version of this fix: blindly copying arbitrary missing skills into every profile. The sync step must be policy-gated.

I tested a local patch with these semantics:

• _default_spawn() checks missing forced skills.
• _maybe_sync_missing_forced_skills(profile, hermes_home, missing) runs one policy-gated sync only when:
  • the profile is in sync_profiles;
  • every missing skill is in sync_skills;
  • create_missing_skill_dirs is true;
  • policy/script exist and exit successfully.
• Then it re-runs _missing_forced_skills(...).
• If anything is still missing, it raises MissingForcedSkillsError and the task blocks before spawn.

Regression coverage I added locally:

• allowlisted missing forced skill auto-syncs and becomes resolvable;
• non-allowlisted missing forced skill fails closed and remains missing;
• existing “missing forced skill blocks before worker spawn” behavior still passes.

Verification:

python -m pytest -o 'addopts=' \
  tests/hermes_cli/test_kanban_db.py::test_missing_forced_skill_auto_syncs_when_policy_allows \
  tests/hermes_cli/test_kanban_db.py::test_missing_forced_skill_auto_sync_fails_closed_when_not_allowlisted \
  tests/hermes_cli/test_kanban_db.py::test_dispatch_blocks_missing_forced_skill_before_worker_spawn -q
# 3 passed

scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py
# 179 passed

This may be too local-policy-specific to land directly in this PR as-is, but it is important for the design discussion: “missing required skill” should not necessarily mean “block immediately” if the system has an explicit, allowlisted way to make that exact skill available to that exact profile before spawn. It should mean “resolve/sync through approved mechanisms, then block if still unavailable.”

[hehehe0803]

Related follow-up opened from hehehe0803:fix/kanban-skill-gating: #33640.

It keeps this PR’s core safety direction (do not pass unknown --skills into workers), then adds a local-policy extension: if a missing forced skill is explicitly allowlisted for profile-skill sync, the dispatcher attempts one sync before blocking. If not allowlisted or sync fails, it still blocks before spawn.

This is meant as an operational refinement for profile-scoped Hermes/Kanban installs, not a broad implicit mutation path.

[hehehe0803]

Follow-up from dogfooding: the preflight block prevents worker startup crashes, but repeated local Kanban runs showed the next root fix should be sync-before-block for allowlisted forced skills.

I could not push directly to this fork branch (GitHub returned 403 for noestelar/hermes-agent), so I published a clean one-commit branch here:

• Branch: https://github.com/hehehe0803/hermes-agent/tree/fix/kanban-skill-gating-autosync-pr30025
• Commit: 60163d6

What it adds:

• Detect missing task-level forced skills for the target worker profile.
• If the local Hermes OS profile-skill sync policy/script exists, run one allowlisted attempt before blocking.
• Re-check skill resolution after sync.
• Spawn immediately if sync resolves the skill; fail closed with if not allowlisted, sync unavailable, sync fails, or the skill remains missing.
• Regression coverage for allowed sync, denied sync, external skill dirs, and frontmatter name resolution.

Verification:

• ▶ running per-file parallel test suite via run_tests_parallel.py
  (TZ=UTC LANG=C.UTF-8 PYTHONHASHSEED=0; clean env)
  Discovered 1 test files (166 tests) under ['tests/hermes_cli/test_kanban_db.py']; running with -j 40
  [100.0% | 166/166 | ✓166 | ✗ 0] ✓ tests/hermes_cli/test_kanban_db.py (166✓, 7.0s)

=== Summary: 1 files, 166 tests passed, 0 failed (100% complete) in 7.0s (40 workers) ===

• Result: 166/166 passed.

This keeps the original safety guarantee from this PR while avoiding the manual “sync profile skill, unblock, redispatch” loop for known shared skills like / .

[teknium1]

Thanks for isolating the startup crash path. The premise remains present on current main: _default_spawn() still passes every task.skills entry to --skills (hermes_cli/kanban_db.py:8036-8039), while CLI startup raises if none load (cli.py:15931-15947).

Problems

• The new probe at hermes_cli/kanban_db.py:5231 only scans <HERMES_HOME>/skills/<name>/SKILL.md. That is narrower than the worker resolver: tools/skills_tool.py:1066-1087 includes configured skills.external_dirs, tools/skills_tool.py:1155-1170 accepts frontmatter name: aliases, and tools/skills_tool.py:1182-1204 rejects ambiguous candidates. The proposed filter would therefore drop valid task skills before the CLI can load them.
• The added tests mock the probe, so they do not exercise those resolver cases.

Suggested changes

• Rework the guard to match the worker's effective resolver and add real filesystem tests for external directories, frontmatter aliases, and ambiguity, alongside the missing-skill regression.

Automated hermes-sweeper review.

[teknium1]

This probe is narrower than the worker resolver: skill_view searches configured skills.external_dirs, accepts frontmatter name: aliases, and rejects ambiguous matches (tools/skills_tool.py:1066-1087, 1155-1204). A valid external or aliased task skill would be dropped here; please use equivalent resolution semantics.