Skip to content

fix(tools): don't drop a toolset from platform inference when a tool is registered into it (salvage #56480) - #722

Merged
hashbender merged 1 commit into
mainfrom
mirror/pr-56887
Jul 2, 2026
Merged

hashbender merged 1 commit into
mainfrom
mirror/pr-56887

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Summary

Fixes the class where an inference-based platform (api_server, and plugin platforms with no explicit platform_toolsets entry) silently loses an entire toolset because _get_platform_tools() reverse-maps the static composite against the registry-merged toolset view.

resolve_toolset("terminal") grows to {terminal, process, read_terminal, close_terminal} once tools are registered, but the hermes-api-server composite enumerates only the static names {terminal, process}. The old all-tools set.issubset() test then failed and dropped the whole terminal toolset from api_server — no log, no error, model just loses terminal.

The fix compares each candidate toolset's static (pre-registry-merge) membership during reverse-mapping via a new include_registry flag on get_toolset()/resolve_toolset(). Registry-added tools stop gating inference but remain exposed (subject to their own check_fn) once the toolset is enabled — matching how explicitly-configured platforms already behave. This fixes the bug class at the reverse-map layer instead of data-patching one composite at a time.

Changes

  • toolsets.py: include_registry: bool = True kwarg on get_toolset()/resolve_toolset(), threaded through recursion, the all/* alias, includes, and the hermes-<plugin> fallback. include_registry=False returns the static TOOLSETS view.
  • hermes_cli/tools_config.py: the three reverse-map candidate comparisons in _get_platform_tools() resolve the candidate toolset with include_registry=False. Universes stay registry-merged so a platform never under-counts what it provides.
  • tests/: behavior-contract tests for the flag + regression tests (default api_server keeps terminal; registering a tool into a toolset doesn't drop it from inference; default-off/restricted toolsets stay off).
  • Follow-up (maintainer): docstring precision on get_toolset's None return; AUTHOR_MAP entry for the salvaged contributor commits.

Validation

Before After
default api_server terminal in final tool defs ❌ dropped ✅ present
static view excludes registry-only read_terminal
default-off (homeassistant/discord) stay off
targeted suite (toolsets + api_server + tools_config) ✅ 155 passed
mutation check (revert fix → regression tests fail)

Credit

Salvage of NousResearch#56480 by @designnotdrum (root-cause fix) — commits cherry-picked with authorship preserved. Supersedes symptom-patch PRs NousResearch#56735 (@HexLab98) and NousResearch#49626 (@liuhao1024), which data-patched the composites; this fixes the class at the reverse-map layer so no composite needs per-tool maintenance.

Fixes NousResearch#56732
Fixes NousResearch#49622


Mirror-of: NousResearch#56887
NousResearch#56887

@hashbender
hashbender merged commit 906914e into main Jul 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant