Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,26 @@ def run(self):

def _build_cli_from_args(model, toolsets, provider, reasoning, api_key, base_url, max_turns, run_budget, verbose, compact, resume, checkpoints, pass_session_id, ignore_rules, skills):
"""Resolve the toolset list (explicit / coding posture / platform default), construct HermesCLI, and start the background skills preload."""
from hermes_cli.oneshot import _normalize_toolsets
from toolsets import ZERO_TOOLSET

# A flag that was PASSED but names nothing is not "unspecified". `hermes chat -q ... -t ""`
# must not silently receive the coding posture or the platform default, which is the same
# footgun -z had: the caller asked for a restricted session and got a fully equipped one.
# The parser already distinguishes absent (None) from empty (""), so the resolver must too.
explicit = toolsets is not None
normalized = _normalize_toolsets(toolsets) if explicit else None
if explicit and not normalized:
raise ValueError(
"--toolsets was passed an empty value, which means \"unspecified\" — "
f"pass a toolset name, or '{ZERO_TOOLSET}' for an explicitly tool-less session."
)
if normalized and ZERO_TOOLSET in normalized and len(normalized) > 1:
raise ValueError(
f"--toolsets: '{ZERO_TOOLSET}' means zero tools and cannot be combined with "
f"other toolsets: {', '.join(n for n in normalized if n != ZERO_TOOLSET)}"
)

toolsets_list = None
if isinstance(toolsets, str) and toolsets:
toolsets_list = [t.strip() for t in toolsets.split(",")]
Expand Down
3 changes: 2 additions & 1 deletion hermes_cli/_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ def _add_top_level_flags(parser: argparse.ArgumentParser) -> None:
"config.yaml for this run only; the persistent level lives there "
"(or per-model under agent.reasoning_overrides)."))
add("-t", "--toolsets", default=None,
help="Comma-separated toolsets to enable for this invocation. Applies to -z/--oneshot and --tui.")
help=("Comma-separated toolsets to enable for this invocation. Pass 'none' for an "
"explicitly tool-less session. Applies to -z/--oneshot and --tui."))
add("--resume", "-r", metavar="SESSION", default=None, help=(
"Resume a previous session by ID or title, or pass 'latest' for "
"the most recent session (workspace-scoped, like -c with no name)"))
Expand Down
20 changes: 20 additions & 0 deletions hermes_cli/oneshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,30 @@ def _configured_mcp_servers() -> tuple[set[str], set[str]]:


def _validate_explicit_toolsets(toolsets: object = None) -> tuple[list[str] | None, str | None]:
from toolsets import ZERO_TOOLSET

# A flag that was PASSED but names nothing is not "unspecified". Folding it into the
# config default is how `-t ""` — the natural way to ask for a tool-less probe — silently
# ran with the full configured toolset. Fail closed instead, and name the way to actually
# ask for zero tools (#126122).
if toolsets is not None and not _normalize_toolsets(toolsets):
return None, (
"hermes -z: --toolsets was passed an empty value, which means \"unspecified\" — "
f"pass a toolset name, or '{ZERO_TOOLSET}' for an explicitly tool-less session.\n"
)

normalized = _normalize_toolsets(toolsets)
if normalized is None:
return None, None

if ZERO_TOOLSET in normalized:
if len(normalized) > 1:
return None, (
f"hermes -z: '{ZERO_TOOLSET}' means zero tools and cannot be combined with "
f"other toolsets: {', '.join(n for n in normalized if n != ZERO_TOOLSET)}\n"
)
return [ZERO_TOOLSET], None

try:
from toolsets import validate_toolset
except Exception as exc:
Expand Down
7 changes: 6 additions & 1 deletion model_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,16 @@ def _select_tool_names(enabled_toolsets: Optional[list[str]], disabled_toolsets:
"""Tool names requested by the toolset selection (before check_fn filtering)."""
tools: set = set()
if enabled_toolsets is not None:
from toolsets import ZERO_TOOLSET

enabled = list(enabled_toolsets)
# Dispatcher-spawned kanban workers always get the lifecycle handoff
# tools, even when the assignee profile restricts its chat toolsets.
# The exception is an explicit zero-tool request: the caller asked for a session that
# cannot act at all, and quietly adding kanban_* would contradict that (#126122).
if (os.environ.get("HERMES_KANBAN_TASK") and not _is_delegated_child_context()
and _is_dispatcher_owned_worker() and "kanban" not in enabled):
and _is_dispatcher_owned_worker() and "kanban" not in enabled
and ZERO_TOOLSET not in enabled):
enabled.append("kanban")
_apply_toolset_selection(tools, enabled, quiet_mode, disable=False)
else:
Expand Down
156 changes: 156 additions & 0 deletions tests/test_toolsets_zero_selection.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
"""``--toolsets`` must be able to say "zero tools", and must never silently mean the opposite.

``hermes -z '<prompt>' -t ''`` is the idiom people reach for when they want a tool-less
liveness probe or classifier. It did the opposite: ``_normalize_toolsets`` folds a falsy value
to ``None``, and ``None`` means "use the user's configured toolsets" — so the probe ran with
terminal, file writes and everything else, with nothing in the output saying so.

Two contracts, both about the relationship between what was asked for and what is sent:

* an EXPLICITLY EMPTY selection is an error, not a silent fallback to the config default;
* ``none`` is a real toolset meaning zero tools, and it survives the kanban worker's
automatic toolset injection (which exists so a worker can always hand its card back).

Absent-vs-empty is a real distinction at the parser (``default=None``), so it is preserved
here: ``None`` still means "not specified".

The same footgun existed on the interactive/one-query entry point (``hermes chat -q``), where
an empty value fell through to the coding posture or the platform default instead. Both call
sites now share one rule.

Regression for #126122.
"""

import pytest

import cli
from hermes_cli.oneshot import _validate_explicit_toolsets
from model_tools import _select_tool_names
from toolsets import get_toolset, resolve_toolset, validate_toolset


class TestNoneIsARealToolset:
"""``none`` must exist and mean exactly zero tools."""

def test_none_is_registered_and_validates(self):
assert validate_toolset("none"), "the sentinel must pass the same validation as any name"
assert get_toolset("none") is not None

def test_none_resolves_to_zero_tools(self):
# resolve_toolset is documented to return a sorted List[str], not a set.
assert resolve_toolset("none") == [], (
"the whole point of the sentinel is an empty tool list"
)


class TestExplicitEmptyIsRejected:
"""``-t ''`` asked for nothing; silently granting the configured toolsets is the bug."""

def test_empty_string_is_an_error_not_a_config_fallback(self):
toolsets, error = _validate_explicit_toolsets("")
assert error is not None, (
f"an explicitly empty --toolsets silently resolved to {toolsets!r} "
"(None means 'use the configured toolsets')"
)
assert toolsets is None

@pytest.mark.parametrize("value", [" ", ",", " , "])
def test_whitespace_and_bare_separators_are_also_errors(self, value):
_, error = _validate_explicit_toolsets(value)
assert error is not None, f"{value!r} carries no toolset name and must not fall back"

def test_absent_flag_still_means_config_default(self):
"""The absent case is unchanged — this is the behavior everything else relies on."""
assert _validate_explicit_toolsets(None) == (None, None)


class TestNoneSentinelThroughTheOneShotPath:
"""``-t none`` must survive validation as a real, empty selection."""

def test_none_alone_is_accepted(self):
toolsets, error = _validate_explicit_toolsets("none")
assert error is None
assert toolsets == ["none"]

def test_none_combined_with_another_toolset_is_rejected(self):
"""``none`` plus anything else is contradictory — ``none,web`` cannot mean zero tools."""
_, error = _validate_explicit_toolsets("none,web")
assert error is not None, "combining the zero-tool sentinel with a real toolset must fail"


class TestNoneSurvivesTheKanbanInjection:
"""A worker asked for zero tools gets zero tools.

The dispatcher appends ``kanban`` so a worker can always hand its card back, which is
right for a normal selection and wrong for an explicit zero-tool request: the caller
asked for a session that cannot act at all.
"""

def test_dispatcher_worker_gets_no_kanban_tools_when_none_requested(self, monkeypatch):
monkeypatch.setenv("HERMES_KANBAN_TASK", "card-123")
monkeypatch.setattr("model_tools._is_delegated_child_context", lambda: False)
monkeypatch.setattr("model_tools._is_dispatcher_owned_worker", lambda: True)

assert _select_tool_names(["none"], None, quiet_mode=True) == set(), (
"the kanban toolset was injected into an explicit zero-tool selection"
)

def test_dispatcher_worker_still_gets_kanban_for_a_normal_selection(self, monkeypatch):
"""The negative case: the injection must survive for every other selection."""
monkeypatch.setenv("HERMES_KANBAN_TASK", "card-123")
monkeypatch.setattr("model_tools._is_delegated_child_context", lambda: False)
monkeypatch.setattr("model_tools._is_dispatcher_owned_worker", lambda: True)

tools = _select_tool_names(["web"], None, quiet_mode=True)
assert tools, "a normal toolset selection must still resolve tools"
assert any(t.startswith("kanban") for t in tools), (
"a dispatcher-owned worker must keep the kanban lifecycle tools for a normal selection"
)


def _build_cli(toolsets):
"""Run cli._build_cli_from_args with a stub CLI and return the toolsets it was handed.

The guard under test raises BEFORE HermesCLI is constructed, so the two error cases cost
nothing. The accept cases need construction, hence the stub: the assertion is on the
``toolsets=`` the real call site would pass to the agent.
"""
captured = {}

class _StubCLI:
def __init__(self, **kwargs):
captured.update(kwargs)
self.ignore_rules = kwargs.get("ignore_rules", False)

original = cli.HermesCLI
cli.HermesCLI = _StubCLI
try:
cli._build_cli_from_args(
None, toolsets, None, None, None, None, None, None,
False, False, None, False, False, False, None,
)
finally:
cli.HermesCLI = original
return captured.get("toolsets")


class TestChatOneQueryPath:
"""``hermes chat -q`` resolves the same flag and must apply the same rule."""

def test_empty_value_is_rejected_before_anything_is_built(self):
with pytest.raises(ValueError, match="empty value"):
_build_cli("")

def test_none_combined_with_another_toolset_is_rejected(self):
with pytest.raises(ValueError, match="cannot be combined"):
_build_cli("none,web")

def test_none_alone_reaches_the_agent_as_the_sentinel(self):
assert _build_cli("none") == ["none"], (
"an explicit zero-tool request must survive the resolver untouched"
)

def test_absent_flag_still_resolves_the_platform_default(self):
"""The negative control: absent is NOT empty, and must keep working."""
resolved = _build_cli(None)
assert resolved, "an absent --toolsets must still resolve a real toolset list"
8 changes: 8 additions & 0 deletions toolsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,16 @@ def _core_without(*excluded, kanban=True):
# config: another surface lacking them made no configuration choice.
CLIENT_SURFACE_TOOLSETS = frozenset({"project", "desktop_ui", "catalog"})

# The toolset name that means "explicitly zero tools". Registered in TOOLSETS so it resolves
# through the same validate_toolset()/resolve_toolset() path as every other name, letting a
# caller SAY "no tools" instead of passing an empty value that reads as "unspecified" and
# silently receives the configured toolsets (#126122).
ZERO_TOOLSET = "none"

# Core toolset definitions: individual tools or references to other toolsets.
TOOLSETS = {
ZERO_TOOLSET: _ts(
"No tools at all — an explicitly tool-less session (scripted one-shots, liveness probes)"),
# Basic toolsets - individual tool categories
"web": _ts("Web research and content extraction tools", ["web_search", "web_extract"]),
"search": _ts("Web search only (no content extraction/scraping)", ["web_search"]),
Expand Down