From 3d7f773bb42498d66ea4c0be1fe7d2e0d00278d7 Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Thu, 10 Sep 2026 21:55:13 +0000 Subject: [PATCH] fix(kanban): honor explicit platform tool opt-ins across configuration surfaces --- hermes_cli/tools_config.py | 12 +- model_tools.py | 7 +- .../test_kanban_worker_spawn_toolsets.py | 9 +- tests/hermes_cli/test_tools_config.py | 38 ++--- tests/tools/test_kanban_toolset_opt_in.py | 138 ++++++++++++++++++ tools/kanban_tools.py | 27 +++- tools/kanban_toolset_context.py | 28 ++++ tui_gateway/server.py | 4 +- website/docs/user-guide/features/kanban.md | 20 +++ 9 files changed, 244 insertions(+), 39 deletions(-) create mode 100644 tests/tools/test_kanban_toolset_opt_in.py create mode 100644 tools/kanban_toolset_context.py diff --git a/hermes_cli/tools_config.py b/hermes_cli/tools_config.py index 4241a8443f..9294610b40 100644 --- a/hermes_cli/tools_config.py +++ b/hermes_cli/tools_config.py @@ -64,6 +64,7 @@ CONFIGURABLE_TOOLSETS = [ ("stt", "🎙️ Speech-to-Text", "voice transcription (gateway voice messages + voice mode)"), ("skills", "📚 Skills", "list, view, manage"), ("todo", "📋 Task Planning", "todo_list"), + ("kanban", "📌 Kanban", "opt-in task board tools for this platform"), ("memory", "💾 Memory", "persistent memory across sessions"), ("context_engine", "🧩 Context Engine", "runtime tools from the active context engine"), ("session_search", "🔎 Session Search", "search past conversations"), @@ -92,7 +93,7 @@ def gui_toolset_label(label: str) -> str: # OFF by default for new installs (still in _HERMES_CORE_TOOLS; the checklist won't pre-select them). x_search # auto-enables when xAI creds exist (mirrors HASS_TOKEN → homeassistant); its check_fn still gates the schema. -_DEFAULT_OFF_TOOLSETS = {"homeassistant", "spotify", "discord", "discord_admin", "video", "video_gen", "x_search", "a2a"} +_DEFAULT_OFF_TOOLSETS = {"homeassistant", "spotify", "discord", "discord_admin", "video", "video_gen", "x_search", "a2a", "kanban"} # Config-only capabilities: provider setup in `hermes tools` (TOOL_CATEGORIES) but not model toolsets — zero # schemas, own switch (``stt.enabled``), never in ``platform_toolsets`` or the per-platform checklist. @@ -166,7 +167,7 @@ def _get_plugin_toolset_keys() -> set: def _checklist_toolset_keys(platform: str) -> Set[str]: """Toolset keys the ``hermes tools`` checklist offers for ``platform`` (mirrors ``_prompt_toolset_checklist``); - read-time-resolved toolsets (``kanban``, recovered composites, MCP names) are NOT here.""" + read-time-resolved toolsets (recovered composites, MCP names) are NOT here.""" return { ts_key for ts_key, _, _ in _get_effective_configurable_toolsets() if _toolset_allowed_for_platform(ts_key, platform) and ts_key not in _CONFIG_ONLY_TOOLSETS} @@ -589,6 +590,11 @@ def _get_platform_tools(config: dict, platform: str, *, include_default_mcp_serv explicit_passthrough = {ts for ts in toolset_names if ts not in explicit_known_keys and ts not in platform_default_keys} enabled_toolsets |= _merge_mcp_servers(config, toolset_names, explicit_passthrough, include_default_mcp_servers) + # Legacy profile opt-in is a fallback only. A saved platform list (even + # empty) is authoritative, so a later disable cannot silently re-enable it. + if not explicitly_configured and "kanban" in (config.get("toolsets") or []): + enabled_toolsets.add("kanban") + # agent.disabled_toolsets is a global suppression list (#86661) and runs LAST so it overrides everything # above. It may arrive as a JSON-array string ("['memory']") from `hermes config set` or a JSON-mode editor. disabled_toolsets = (config.get("agent") or {}).get("disabled_toolsets") @@ -944,7 +950,7 @@ def _configure_list(to_configure: List[str], config: dict, *, selected: bool = T def _checklist_diff(new_enabled: Set[str], prev: Set[str], platform: str) -> tuple[Set[str], Set[str]]: - """``(added, removed)`` scoped to the checklist universe, so read-time toolsets (``kanban``) the user never + """``(added, removed)`` scoped to the checklist universe, so read-time toolsets (MCP names) the user never saw a checkbox for don't print as spurious removals.""" universe = _checklist_toolset_keys(platform) return (new_enabled - prev) & universe, (prev - new_enabled) & universe diff --git a/model_tools.py b/model_tools.py index bec779925a..4193fb2210 100644 --- a/model_tools.py +++ b/model_tools.py @@ -491,8 +491,11 @@ def _compute_tool_definitions(enabled_toolsets: Optional[List[str]] = None, disa quiet_mode: bool = False, skip_tool_search_assembly: bool = False) -> List[Dict[str, Any]]: """Uncached implementation of :func:`get_tool_definitions`.""" tools_to_include = _select_tool_names(enabled_toolsets, disabled_toolsets, quiet_mode) - # Registry returns only tools whose check_fn passes. - filtered_tools = _apply_dynamic_schemas(registry.get_definitions(tools_to_include, quiet=quiet_mode)) + # Selection is per schema, not per process/profile. Kanban's local checks + # are uncached; the outer definitions cache already keys on this selection. + from tools.kanban_toolset_context import scoped_kanban_toolset_selection + with scoped_kanban_toolset_selection(enabled_toolsets): + filtered_tools = _apply_dynamic_schemas(registry.get_definitions(tools_to_include, quiet=quiet_mode)) global _last_resolved_tool_names _last_resolved_tool_names = [t["function"]["name"] for t in filtered_tools] diff --git a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py index c221d63f96..948f2b9f63 100644 --- a/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py +++ b/tests/hermes_cli/test_kanban_worker_spawn_toolsets.py @@ -162,5 +162,12 @@ toolsets: assert resolved is not None assert "terminal" in resolved assert "web" in resolved - assert "kanban" in resolved # recovered worker lifecycle surface + # Opt-in is no longer inferred for ordinary chats. The dispatcher-owned + # worker gets lifecycle tools at schema assembly, independently of the + # assignee's saved chat selection. + monkeypatch.setenv("HERMES_KANBAN_TASK", "t_spawn_tools") + from model_tools import get_tool_definitions + names = {t["function"]["name"] for t in get_tool_definitions(resolved, quiet_mode=True, skip_tool_search_assembly=True)} + assert "kanban_complete" in names + assert "kanban_list" not in names assert resolved != ["kanban"] diff --git a/tests/hermes_cli/test_tools_config.py b/tests/hermes_cli/test_tools_config.py index 791219ee2b..82839a63a1 100644 --- a/tests/hermes_cli/test_tools_config.py +++ b/tests/hermes_cli/test_tools_config.py @@ -848,38 +848,18 @@ def test_get_effective_configurable_toolsets_dedupes_bundled_plugins(): -# ── Checklist diff scope: non-configurable toolsets (kanban) must not be -# reported as added/removed by `hermes tools` ────────────────────────── - - - - -def test_kanban_not_reported_as_removed_in_diff(): - """Reproduces the false-signal bug: `hermes tools` printed ``- kanban`` - when saving a platform that resolves kanban as enabled, even though the - checklist never offered kanban as a toggle. - - The printed diff must be scoped to ``_checklist_toolset_keys`` so a tool - the user could not deselect is never reported as removed. The persisted - config still keeps kanban (verified separately by _save_platform_tools). - """ +# Kanban now participates in the checklist: an explicit deselection must be +# both visible in the diff and durable in the platform selection. +def test_kanban_checklist_reports_and_persists_explicit_removal(): config = {"platform_toolsets": {"telegram": ["kanban", "web", "terminal"]}} current = _get_platform_tools(config, "telegram", include_default_mcp_servers=False) - assert "kanban" in current # resolved as enabled at read time - - # The checklist can only return configurable keys it was shown; kanban - # is never one of them. universe = _checklist_toolset_keys("telegram") - new_enabled = {t for t in current if t != "kanban"} - - # Unscoped (old, buggy) diff would surface kanban. - assert (current - new_enabled) == {"kanban"} - # Scoped (fixed) diff drops it. - assert ((current - new_enabled) & universe) == set() - - - - + new_enabled = current - {"kanban"} + assert ((current - new_enabled) & universe) == {"kanban"} + with patch("hermes_cli.tools_config.save_config"): + _save_platform_tools(config, "telegram", new_enabled) + assert "kanban" not in _get_platform_tools(config, "telegram", include_default_mcp_servers=False) + assert {"web", "terminal"} <= set(config["platform_toolsets"]["telegram"]) def test_vision_picker_custom_endpoint(tmp_path, monkeypatch): diff --git a/tests/tools/test_kanban_toolset_opt_in.py b/tests/tools/test_kanban_toolset_opt_in.py new file mode 100644 index 0000000000..727ca82fc8 --- /dev/null +++ b/tests/tools/test_kanban_toolset_opt_in.py @@ -0,0 +1,138 @@ +"""Saved tool opt-ins must reach the real schema without leaking across platforms.""" +from __future__ import annotations + +import json +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path + +import pytest + + +def _names(selection, disabled=None): + from model_tools import get_tool_definitions + + return { + row["function"]["name"] + for row in get_tool_definitions( + selection, disabled_toolsets=disabled, quiet_mode=True, + skip_tool_search_assembly=True, + ) + if row["function"]["name"].startswith("kanban_") + } + + +@pytest.mark.parametrize("surface", ["cli", "http", "rpc"]) +def test_saved_opt_in_roundtrip_reaches_schema_and_board(surface, tmp_path, monkeypatch): + """Exercise the real config writer, availability gate, skills gate and handler.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) + monkeypatch.delenv("HERMES_KANBAN_BOARD", raising=False) + from hermes_cli.config import load_config, save_config + from hermes_cli.tools_config import _apply_toolset_change, _get_platform_tools + from tools.registry import registry + + save_config({"platform_toolsets": {"cli": ["file"], "telegram": ["file"]}}) + + def selected(platform="cli"): + return sorted(_get_platform_tools(load_config(), platform, include_default_mcp_servers=False)) + + before = _names(selected()) # Warm both cache layers before enabling. + assert not before + client = None + if surface == "http": + from fastapi import FastAPI + from fastapi.testclient import TestClient + from hermes_cli.web_routers.tools import router + + app = FastAPI() + app.include_router(router) + client = TestClient(app) + + def toggle(enabled): + action = "enable" if enabled else "disable" + if surface == "http": + response = client.put("/api/tools/toolsets/kanban", json={"enabled": enabled}) + assert response.status_code == 200, response.text + assert response.json()["enabled"] is enabled + elif surface == "rpc": + from tui_gateway.server import _methods + + response = _methods["tools.configure"]("kanban-opt-in", {"action": action, "names": ["kanban"]}) + assert "error" not in response, response + assert "kanban" in response["result"]["changed"] + else: + _apply_toolset_change(load_config(), "cli", ["kanban"], action) + + try: + toggle(True) + enabled_names = _names(selected()) + assert {"kanban_list", "kanban_create", "kanban_complete"} <= enabled_names + assert not _names(selected("telegram")), "CLI opt-in leaked to Telegram" + assert "file" in selected() + # A second profile in the same process must not borrow this grant or + # poison the first profile's cached schema on return. + from hermes_constants import set_hermes_home_override, reset_hermes_home_override + other_home = tmp_path / "profiles" / "observer" + token = set_hermes_home_override(other_home) + try: + save_config({"platform_toolsets": {"cli": ["file"]}}) + assert not _names(selected()) + finally: + reset_hermes_home_override(token) + assert _names(selected()) == enabled_names + from agent.skill_utils import _detect_kanban + assert _detect_kanban(), "Saved opt-in still hides the Kanban playbook" + result = json.loads(registry.dispatch("kanban_create", {"title": "opt-in roundtrip", "assignee": "default"})) + assert result.get("ok"), result + from hermes_cli.kanban_db_connect import connect_closing + from hermes_cli.kanban_db import get_task + with connect_closing() as conn: + assert get_task(conn, result["task_id"]).title == "opt-in roundtrip" + toggle(False) + assert not _names(selected()) + assert not _detect_kanban() + assert "file" in selected() + assert enabled_names, "Changing config must not mutate an already-built schema" + finally: + if client is not None: + client.close() + + +@pytest.mark.parametrize("legacy", [False, True]) +def test_selection_is_scoped_and_preserves_worker_and_deny_boundaries(legacy, tmp_path, monkeypatch): + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.delenv("HERMES_KANBAN_TASK", raising=False) + monkeypatch.delenv("HERMES_KANBAN_BOARD", raising=False) + from hermes_cli.config import load_config, save_config + from hermes_cli.tools_config import _get_platform_tools + from agent.delegation_context import delegated_child_context + + save_config({"toolsets": ["kanban"] if legacy else [], "platform_toolsets": {"telegram": ["kanban"]}}) + # The same profile concurrently builds an explicitly opted-in schema and + # an all/default schema. A platform grant must not become a cached global grant. + with ThreadPoolExecutor(max_workers=2) as pool: + named, broad = list(pool.map(_names, [["kanban"], ["hermes-cli"]])) + assert "kanban_create" in named + assert bool(broad) is legacy + assert bool(_names(None)) is legacy + assert bool(_names(["all"])) is legacy + assert not _names(["kanban"], ["kanban"]) + assert not _names([]) + assert bool(_names(sorted(_get_platform_tools(load_config(), "cli")))) is legacy + + # Explicitly empty saved configuration wins over a legacy fallback, including + # the actual Desktop/TUI loader (which must not turn [] back into None/all). + cfg = load_config() + cfg["platform_toolsets"]["cli"] = [] + save_config(cfg) + monkeypatch.delenv("HERMES_TUI_TOOLSETS", raising=False) + from tui_gateway.server import _load_enabled_toolsets + assert not _names(_load_enabled_toolsets("desktop")) + + monkeypatch.setenv("HERMES_KANBAN_TASK", "t_worker") + worker = _names(["file"]) + assert "kanban_complete" in worker + assert "kanban_list" not in worker + with delegated_child_context(): + assert not _names(["kanban"]) + assert "kanban_complete" in _names(["file"]) diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 57128b4191..f616a4d53a 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -17,7 +17,7 @@ from typing import Any, Callable, Optional from agent.redact import redact_sensitive_text from hermes_cli.goals import judge_goal -from tools.registry import registry, tool_error +from tools.registry import no_cache_check_fn, registry, tool_error from hermes_cli.config import cfg_get, load_config from tools.kanban_tools_schemas import ( KANBAN_ATTACH_SCHEMA, @@ -35,9 +35,28 @@ KANBAN_LIST_MAX_LIMIT = 200 # --- Gating --- def _profile_has_kanban_toolset() -> bool: - # load_config() is mtime-cached and check_fn results are TTL-cached (~30s). + from tools.kanban_toolset_context import kanban_toolset_requested + + requested = kanban_toolset_requested() + if requested: + return True try: - return "kanban" in load_config().get("toolsets", []) + config = load_config() + # Preserve the legacy profile-wide opt-in for callers using bundles. + if "kanban" in (config.get("toolsets") or []): + return True + if requested is not None: + # Never borrow another platform's opt-in during schema assembly. + return False + # Offer-time skill discovery has no platform selection. A saved opt-in + # makes the playbook relevant; actual schemas still use the scope above. + from hermes_cli.tools_config import _get_platform_tools + + platforms = config.get("platform_toolsets") or {} + return any( + "kanban" in _get_platform_tools(config, platform, include_default_mcp_servers=False) + for platform, names in platforms.items() if isinstance(names, list) + ) except Exception: return False @@ -71,11 +90,13 @@ def _visible(*, to_env_worker: bool) -> bool: return _profile_has_kanban_toolset() +@no_cache_check_fn def _check_kanban_mode() -> bool: """Lifecycle tools: dispatcher workers + profiles with the ``kanban`` toolset.""" return _visible(to_env_worker=True) +@no_cache_check_fn def _check_kanban_orchestrator_mode() -> bool: """Board-routing tools (kanban_list, kanban_unblock): hidden from task workers.""" return _visible(to_env_worker=False) diff --git a/tools/kanban_toolset_context.py b/tools/kanban_toolset_context.py new file mode 100644 index 0000000000..da93274491 --- /dev/null +++ b/tools/kanban_toolset_context.py @@ -0,0 +1,28 @@ +"""Explicit Kanban selection during one model-schema build. + +The registry's ordinary availability cache is profile-wide, while a gateway +can build schemas for several platforms in the same profile concurrently. +Carry only the explicit selection through a ContextVar, never process env. +""" +from __future__ import annotations + +from contextlib import contextmanager +from contextvars import ContextVar +from typing import Iterable, Iterator, Optional + +_requested: ContextVar[Optional[bool]] = ContextVar("kanban_toolset_requested", default=None) + + +def kanban_toolset_requested() -> Optional[bool]: + """None outside schema assembly; otherwise whether Kanban was named explicitly.""" + return _requested.get() + + +@contextmanager +def scoped_kanban_toolset_selection(toolsets: Optional[Iterable[str]]) -> Iterator[None]: + """An all/default selection is not an explicit workflow opt-in.""" + token = _requested.set("kanban" in (toolsets or ())) + try: + yield + finally: + _requested.reset(token) diff --git a/tui_gateway/server.py b/tui_gateway/server.py index cbfe41572d..4ba8a6738d 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1869,7 +1869,9 @@ def _load_enabled_toolsets(platform: str | None = None) -> list[str] | None: enabled = _get_platform_tools(cfg, "cli", include_default_mcp_servers=True) if fallback_notice is not None: _tui_notice(fallback_notice) - return sorted(enabled | _gui_surface_toolsets(session_platform)) if enabled else None + # An explicitly empty selection is not "all". Keep only client-surface + # affordances instead of resurrecting legacy opt-ins via the None path. + return sorted(enabled | _gui_surface_toolsets(session_platform)) except Exception: if fallback_notice is not None: _tui_notice("[tui] no valid HERMES_TUI_TOOLSETS entries and configured CLI toolsets could not be loaded; enabling all toolsets") diff --git a/website/docs/user-guide/features/kanban.md b/website/docs/user-guide/features/kanban.md index 01012f9ff8..8b06513e1d 100644 --- a/website/docs/user-guide/features/kanban.md +++ b/website/docs/user-guide/features/kanban.md @@ -341,6 +341,26 @@ parent, missing input, unmet capability) before unblocking, or raise `BLOCK_RECURRENCE_LIMIT` if the loop is expected. ::: +## Enabling tools for a chat profile + +The Desktop Kanban plugin displays the board; it does not grant the chat agent +permission to manage tasks. Enable the `kanban` toolset for the profile and +platform that should orchestrate work: + +```bash +hermes -p planner tools enable kanban +``` + +This saves `platform_toolsets.cli`, used by CLI, TUI and Desktop chats. Other +messaging platforms have independent selections. Start a new chat after changing +this setting; existing conversations retain their tool schemas and prompt cache. +Explicitly empty selections and `agent.disabled_toolsets` remain authoritative. +Legacy top-level `toolsets: [kanban]` remains a fallback when no platform selection +was saved; `all` alone is not a Kanban opt-in. + +Dispatcher-owned workers receive their task lifecycle tools automatically. +`delegate_task` children do not gain permission to mutate the board. + ## How workers interact with the board **Workers do not shell out to `hermes kanban`.** When the dispatcher spawns a worker it sets `HERMES_KANBAN_TASK=t_abcd` in the child's env, and that env var flips on a dedicated **kanban toolset** in the model's schema. The same toolset is also available to orchestrator profiles that enable `kanban` in their toolsets config. These tools read and mutate the board directly via the Python `kanban_db` layer, same as the CLI does. A running worker calls these like any other tool; it never sees or needs the `hermes kanban` CLI.