Skip to content

fix(skills): filter platform-disabled skills from prompt (#46201) - #46220

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/46201-disabled-skills-prompt
Closed

fix(skills): filter platform-disabled skills from prompt (#46201)#46220
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/46201-disabled-skills-prompt

Conversation

@Tranquil-Flow

Copy link
Copy Markdown
Contributor

Summary

Fixes #46201.

Disabled skills were still leaking into <available_skills> for active platforms because prompt construction did not pass the resolved platform into get_disabled_skill_names(). A second related bug meant platform-specific disables replaced the global skills.disabled set instead of merging with it.

This PR:

  • threads the resolved platform hint into build_skills_system_prompt() disabled-skill filtering
  • merges global and platform-specific disabled skill sets in get_disabled_skill_names()
  • adds prompt-builder regression coverage for platform-disabled skills and global+platform merge behavior
  • updates skill-config tests to assert the intended merged semantics

Verification

Ran focused regression suite with the shared Hermes Python 3.11 venv:

/Users/evinova-self/.hermes/hermes-agent/venv/bin/python3 -m pytest \
  tests/hermes_cli/test_skills_config.py \
  tests/agent/test_prompt_builder.py \
  -v -o "addopts=" --tb=short

Result: 164 passed, 1 skipped in 0.29s.

Competitor analysis

An existing PR, #46203, addresses the same issue and has green CI, but it includes unrelated desktop/shared TypeScript config changes, line-ending churn in tests/hermes_cli/test_skills_config.py, and weakens an existing prompt-builder test by removing an assertion. This PR keeps the fix focused to the Python prompt/skills paths and targeted tests only.

Auto-published by Moonsong via Path B automated pipeline.

@liuhao1024

Copy link
Copy Markdown
Contributor

Reviewed the diff — LGTM. Clean fix with good test coverage.

Details:

  • The core change in get_disabled_skill_names() correctly unions global + platform disabled sets instead of replacing one with the other. This fixes the regression where platform_disabled.telegram: [xurl] would silently un-hide globally disabled skills on Telegram sessions.
  • prompt_builder.py passes _platform_hint through so the disabled check actually sees the active platform.
  • Test coverage: test_excludes_platform_disabled_skills and test_global_disabled_merged_with_platform_disabled directly validate the regression path from [Bug]: Disabled skills still appear in <available_skills> prompt and trigger false loads #46201.
  • Existing assertions in test_skills_config.py updated to reflect merged behavior.

Note: #46203 (iborazzi) applies the same core fix but without the dedicated test coverage and with an unrelated tsconfig.json change. This PR is the stronger candidate.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #46236 (merged) — that PR fixes #46201 by unifying all disabled-skill resolution sites. Competes with #46203.

@Tranquil-Flow
Tranquil-Flow force-pushed the fix/46201-disabled-skills-prompt branch from 3116a3c to 70f9ff0 Compare June 16, 2026 16:00
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. This is an automated hermes-sweeper review; the reported behavior is already implemented on current main through merged PR #46236, which the member comment identified.

  • agent/prompt_builder.py:1480 now passes the resolved platform to get_disabled_skill_names().
  • agent/skill_utils.py:385-392 unions global and per-platform disabled skill sets.
  • Commit 7bbe7024c207a6dad982967780995ad18ac7e7be contains the matching core implementation and is an ancestor of current main.
  • Merged follow-up ce19fdb7ce27d94462a650ade8a2b94882590a38 additionally fixes the corresponding configuration UI and skill_view resolution sites, making fix(skills): platform-disabled skills still appear in <available_skills> + unify all resolution sites (#46201) #46236 the more complete fix.
  • The implementation shipped in v2026.6.19.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added the sweeper:implemented-on-main Sweeper: behavior already present on current main label Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:implemented-on-main Sweeper: behavior already present on current main tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Disabled skills still appear in <available_skills> prompt and trigger false loads

4 participants