Skip to content

fix(kanban): sync allowlisted forced skills before blocking - #33640

Open
hehehe0803 wants to merge 4 commits into
NousResearch:mainfrom
hehehe0803:fix/kanban-skill-gating
Open

fix(kanban): sync allowlisted forced skills before blocking#33640
hehehe0803 wants to merge 4 commits into
NousResearch:mainfrom
hehehe0803:fix/kanban-skill-gating

Conversation

@hehehe0803

Copy link
Copy Markdown
Contributor

Summary

  • Extends Kanban forced-skill preflight from “detect missing skill and block” to “try one explicit allowlisted profile-skill sync, then block if still missing.”
  • Keeps failure closed by default: no policy file, no sync script, non-allowlisted profile, non-allowlisted skill, or failed sync all leave the task blocked before worker spawn.
  • Preserves worker crash-loop protection by raising a deterministic MissingForcedSkillsError before Popen rather than letting hermes -p ... --skills missing fail at CLI startup.
  • Improves skill resolvability checks to cover directory names, relative paths, frontmatter name, plugin-qualified skills, env overlay roots, and profile skills.external_dirs.
  • Adds conservative catalog-based skill suggestions for Kanban PM/orchestrator task-skill selection.

Why

Kanban tasks can force-load skills through task.skills. If a profile-scoped worker lacks one of those skills, the worker dies before the agent loop can explain/block the task. PR #30025 fixed the first half by gating forced skills on resolvability. This branch adds the operational follow-up: if a local install has an explicit allowlisted profile-skill sync policy, attempt sync once before blocking.

This prevents repeated “missing forced skill” document/task crashes when the fix is safe and mechanical, while still avoiding broad implicit mutation of profile skill directories.

Related

Test plan

  • python -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py
    • Result: 166 passed, 0 failed

@hehehe0803

Copy link
Copy Markdown
Contributor Author

Follow-up context for reviewers:

This branch builds on the forced-skill crash-loop mitigation in #30025.

What changed here:

  • fix(kanban): gate task.skills on resolvability to prevent worker crash loops #30025: gate missing task.skills so a worker does not crash at CLI startup.
  • This PR: before blocking, attempt exactly one local policy-driven profile-skill sync when the target profile and missing skills are explicitly allowlisted.
  • If policy/script is absent, the profile/skill is not allowlisted, or sync fails, it still fails closed and blocks before spawn.

Related Kanban hardening context:

Local verification:

  • python -m py_compile hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py → 166 passed.

@hehehe0803

Copy link
Copy Markdown
Contributor Author

Pushed one follow-up commit after initial CI: chore(release): map noestelar contributor email, because this branch includes #30025 commits and the attribution check requires noe@nousresearch.com in AUTHOR_MAP.

@alt-glitch alt-glitch added type/feature New feature or request comp/cli CLI entry point, hermes_cli/, setup wizard tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels May 28, 2026
@hehehe0803

Copy link
Copy Markdown
Contributor Author

Related-work / non-duplication scan before this PR is treated as the active forced-skill path:

I checked the nearby Kanban/skills PRs and issues so we do not create yet another overlapping branch by accident.

Closest PRs:

Closest issues / RFCs:

Conclusion: #33640 is not redundant if scoped narrowly to "allowlisted sync-before-block for forced Kanban task skills" and kept separate from #33245's broader read-only overlay/catalog design. If #33747 is preferred as the base for preflight, this PR's unique part should be reconciled as the sync-before-block extension rather than maintained as a parallel duplicate.

Noé Rivera and others added 4 commits June 2, 2026 15:11
…h loops

The dispatcher already gates the built-in --skills kanban-worker injection
on resolvability under the worker's HERMES_HOME — a missing skill is fatal
at CLI startup (ValueError: Unknown skill(s): <name>), aborting the worker
before the agent loop runs. Per-task skills from task.skills were passed
through unconditionally, 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 watchdog auto-blocks it.

Observed in the wild: a task with skills=['mlflow-eval-datasets-for-llm-pipelines']
on 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/work/skills/mlops/.

Changes:
- 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.
- Apply the same gate to task.skills entries in _default_spawn. Skipped
  skills emit a stderr warning naming the task id so operators see why
  the worker did not pick up the requested skill; the task still proceeds
  rather than crash-looping.
- Existing tests stub the new helper to keep synthetic skill names
  resolvable; a new regression test exercises the mixed-resolvability
  path and asserts the warning surfaces the task id.

56/56 spawn+skill tests in test_kanban_core_functionality.py and
test_kanban_db.py pass.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the forced-skill worker failure; the underlying current-main issue is real: hermes_cli/kanban_db.py:8036-8039 forwards task skills directly, and an all-missing set raises during CLI preload (cli.py:15926-15947).

Problems

  • The new HERMES_SKILLS_EXTERNAL_DIRS preflight branch does not match the worker's resolver. Current external roots come only from skills.external_dirs in the active profile config (agent/skill_utils.py:420-500), so a preflight-approved env-only skill can still fail at child startup.
  • The sync path adds non-secret HERMES_PROFILE_SKILL_SYNC_* configuration and runs a local script. This conflicts with the project config policy (AGENTS.md:102-107); the test-created script does not validate a maintained runtime path.

Suggested changes

  • Share a target-profile-aware resolver with the CLI preload path and add a subprocess test proving preflight-approved skills actually load.
  • Put any supported sync policy in config.yaml and use a maintained sync interface.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows 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 sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/skills Skills system (list, view, manage) type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants