Skip to content

test(toolsets): include kanban in expected post-#17805 toolset assertions#18122

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
briandevans:fix/tests-include-kanban-toolset
May 1, 2026
Merged

test(toolsets): include kanban in expected post-#17805 toolset assertions#18122
teknium1 merged 1 commit into
NousResearch:mainfrom
briandevans:fix/tests-include-kanban-toolset

Conversation

@briandevans
Copy link
Copy Markdown
Contributor

Summary

  • The kanban PR (feat(kanban): durable multi-profile collaboration board #17805, c868425) added the kanban toolset (in toolsets.py) and tools/kanban_tools.py but did not update three pre-existing test assertions that bake the full toolset/tool inventory.
  • CI on main itself is red on these tests right now (last push, 8fed969, fails the same four). This commit restores green so unrelated PRs stop inheriting these failures.

The bug

Four tests fail on clean origin/main:

Test Expected Actual
tests/tools/test_registry.py::TestBuiltinDiscovery::test_matches_previous_manual_builtin_tool_set manual builtin list excluding kanban includes new tools.kanban_tools
tests/hermes_cli/test_tools_config.py::test_get_platform_tools_preserves_explicit_empty_selection set() {'kanban'}
tests/test_tui_gateway_server.py::test_load_enabled_toolsets_rejects_disabled_mcp_env ['memory'] ['kanban', 'memory']
tests/test_tui_gateway_server.py::test_load_enabled_toolsets_falls_back_when_tui_env_invalid ['memory'] ['kanban', 'memory']

Root cause for the three resolver-driven failures: _get_platform_tools runs a recovery loop that adds non-configurable platform toolsets whose tools live inside hermes-cli's universe but are absent from CONFIGURABLE_TOOLSETS (so they can't appear in the TUI checklist and would otherwise be silently dropped on save). kanban is exactly such a toolset — the kanban tools are in _HERMES_CORE_TOOLS but kanban is not in CONFIGURABLE_TOOLSETS, so the loop now picks it up. That is the intended behavior of the recovery loop; the tests just predate it.

The fix

Test-only:

  • test_matches_previous_manual_builtin_tool_set: add tools.kanban_tools to the expected manual list.
  • test_load_enabled_toolsets_* (both): expect ['kanban', 'memory'] and add a one-line comment pointing future readers at _get_platform_tools so the auto-recovery isn't surprising.
  • test_get_platform_tools_preserves_explicit_empty_selection: replace assert enabled == set() with assert enabled.isdisjoint(CONFIGURABLE_TOOLSETS) and a comment explaining the actual contract — no toolset the user could have unchecked in the TUI gets re-enabled. This keeps the test honest as more non-configurable platform toolsets land (kanban is unlikely to be the last).

No production change.

Test plan

  • Reproduced all four failures on clean origin/main (180a703) with uv run --with pytest --with pytest-xdist --with fastapi --with starlette --with httpx --with pytest-asyncio — same install shape CI uses for [all,dev].
  • All four pass with this commit applied.
  • Adjacent suites green: tests/hermes_cli/test_tools_config.py, tests/tools/test_registry.py, tests/tools/test_kanban_tools.py, tests/hermes_cli/test_kanban_cli.py, tests/test_tui_gateway_server.py — 238/239 pass; the 1 unrelated failure (test_browser_manage_connect_default_local_reports_launch_hint) reproduces on clean main and is about Chrome executable detection on the test runner.
  • Regression guard: assert enabled == set() failed ({'kanban'}); reframed assertion passes; pre-feat(kanban): durable multi-profile collaboration board #17805 behavior (no kanban) would still pass under the new assertion since kanban was never in CONFIGURABLE_TOOLSETS.

Related

…olset assertions

The kanban PR (NousResearch#17805, c868425) added the `kanban` toolset and
`tools/kanban_tools.py`, but didn't update three pre-existing test
assertions that bake the full toolset/tool inventory:

* `tests/tools/test_registry.py::test_matches_previous_manual_builtin_tool_set`
  hard-codes the manual list of builtin tool modules. `tools.kanban_tools`
  was missing.
* `tests/test_tui_gateway_server.py::test_load_enabled_toolsets_rejects_disabled_mcp_env`
  and `test_load_enabled_toolsets_falls_back_when_tui_env_invalid` both
  expect `["memory"]` from `_load_enabled_toolsets()`. With kanban now
  auto-recovered by `_get_platform_tools` (its tools live in hermes-cli's
  universe but are not in CONFIGURABLE_TOOLSETS), the resolver returns
  `["kanban", "memory"]`.
* `tests/hermes_cli/test_tools_config.py::test_get_platform_tools_preserves_explicit_empty_selection`
  asserts `set()` for an explicit empty list. The recovery loop now also
  surfaces `kanban`. Reframed to assert the contract the test name
  describes — no CONFIGURABLE toolset gets re-enabled when the user
  explicitly saved an empty list — which stays correct as more
  non-configurable platform toolsets are added.

Verified the failures reproduce on clean origin/main (180a703) with
`.[all,dev]`-equivalent extras (fastapi, starlette, httpx, pytest-asyncio)
and that all four pass with this commit applied. CI on main itself is
currently red on these tests; this restores green for everyone's PRs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 30, 2026 23:16
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates test expectations to account for the kanban toolset introduced in #17805, restoring CI by aligning “full toolset inventory” assertions with current toolset resolution behavior.

Changes:

  • Add tools.kanban_tools to the expected builtin tool module discovery list.
  • Update TUI gateway server toolset-loading tests to expect the auto-recovered kanban toolset alongside memory.
  • Reframe the “explicit empty selection” tools-config test to assert that no configurable toolsets reappear (while allowing non-configurable auto-recovered toolsets like kanban).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/tools/test_registry.py Extends the manual builtin module expectation to include tools.kanban_tools.
tests/test_tui_gateway_server.py Updates fallback assertions to include sorted ["kanban", "memory"] to reflect _get_platform_tools recovery behavior.
tests/hermes_cli/test_tools_config.py Adjusts the explicit-empty-selection contract to focus on configurable toolsets, allowing non-configurable recovered toolsets.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P2 Medium — degraded but workaround exists comp/tools Tool registry, model_tools, toolsets labels Apr 30, 2026
@teknium1 teknium1 merged commit 97d6f25 into NousResearch:main May 1, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P2 Medium — degraded but workaround exists type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants