Skip to content

fix(toolsets): make zero tools expressible, fail closed on an empty -t - #138999

Closed
suhas-svg wants to merge 2 commits into
NousResearch:mainfrom
suhas-svg:fix/zero-tools-sentinel
Closed

suhas-svg wants to merge 2 commits into
NousResearch:mainfrom
suhas-svg:fix/zero-tools-sentinel

Conversation

@suhas-svg

Copy link
Copy Markdown

What does this PR do?

Refresh of #127881 (closed stale) onto current main.

hermes -z '<prompt>' -t '' is the idiom people reach for when they want a tool-less liveness probe or classifier. It did the opposite: the empty value was indistinguishable from "flag not passed", so the one-shot fell back to the user's configured toolsets and ran with terminal, file writes and everything else — with nothing in the output saying so.

This makes zero tools expressible, and makes the ambiguous empty value fail closed instead of silently granting full access.

hermes -z '<prompt>' -t ''
  _normalize_toolsets("")  →  None
  None  →  use_config_toolsets = True   →  ALL configured tools
 TOOLSETS = {
+    ZERO_TOOLSET: _ts("No tools at all — an explicitly tool-less session ..."),

 _validate_explicit_toolsets(toolsets):
-    normalized = _normalize_toolsets(toolsets)
-    if normalized is None:
-        return None, None          # "" and None both land here
+    if toolsets is not None and not _normalize_toolsets(toolsets):
+        return None, "... pass a toolset name, or 'none' ..."
+    normalized = _normalize_toolsets(toolsets)
+    if normalized is None:
+        return None, None          # only the ABSENT flag reaches this now
+    if ZERO_TOOLSET in normalized:
+        if len(normalized) > 1:
+            return None, "'none' means zero tools and cannot be combined with ..."
+        return [ZERO_TOOLSET], None

 _select_tool_names(enabled_toolsets, ...):
     # dispatcher-owned workers always get the kanban lifecycle tools
-    if (... and "kanban" not in enabled):
+    if (... and "kanban" not in enabled and ZERO_TOOLSET not in enabled):
         enabled.append("kanban")

none is registered in the registry rather than special-cased downstream, so it passes the same validate_toolset() / resolve_toolset() path as every other name and needs no branch at each call site.

The second commit applies the same empty-vs-absent rule to hermes chat -q (cli.py::_build_cli_from_args), where an empty -t silently received the coding posture or the platform default — the same footgun, surfaced on the other entry point.

Evidence

Same probe, base main vs this branch:

$ python -c "from hermes_cli.oneshot import _validate_explicit_toolsets, _normalize_toolsets; ..."
base main:  validate('') -> (None, None)                      use_config_toolsets = True
branch:     validate('') -> (None, 'hermes -z: --toolsets was passed an empty value, ...')
scripts/run_tests.sh tests/test_toolsets_zero_selection.py tests/hermes_cli/test_toolset_validation.py tests/hermes_cli/test_oneshot_exit_contract.py
→ 31 passed, 0 failed

Two unrelated files (test_cli_profile_toolsets.py, test_kanban_worker_spawn_toolsets.py) fail 6 tests identically on base main on this host — a home-I/O guard artifact of my local test environment, not this change.

Related Issue

Fixes #126122

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)

Changes Made

  • toolsets.py — ZERO_TOOLSET = "none" and a TOOLSETS entry for it, so an explicitly tool-less selection is a real, resolvable toolset like any other.
  • hermes_cli/oneshot.py — _validate_explicit_toolsets() distinguishes the three cases the parser already distinguishes (default=None): absent → config default (unchanged); passed but empty → error, no provider request; none → ["none"]. none combined with any other name is rejected.
  • cli.py — _build_cli_from_args() applies the same rule to hermes chat -q.
  • model_tools.py — the dispatcher-owned kanban worker injection skips an explicit zero-tool request.
  • tests/test_toolsets_zero_selection.py — new; the three cases plus the kanban exception and the -q path.

How to Test

  1. hermes -z 'say ok' -t '' → exits 2 with the "empty value" diagnostic instead of running with every configured tool.
  2. hermes -z 'say ok' -t none → runs with zero tools; hermes -z 'say ok' -t none,web → rejected.
  3. scripts/run_tests.sh tests/test_toolsets_zero_selection.py

NousResearch#126122)

`hermes -z '<prompt>' -t ''` is how people ask for a tool-less liveness
probe or classifier. It did the opposite: _normalize_toolsets folds the
empty string to None, None means "use the configured toolsets", and the
one-shot ran with terminal, file writes and everything else. Nothing in
the output said so.

Two changes, on the -z/--oneshot path:

* Register `none` as a real toolset. It resolves through the same
  validate_toolset()/resolve_toolset() path as every other name, so no
  call site needs a special case for it.

* Distinguish an absent flag from a passed-but-empty one, which the
  parser already distinguishes (default=None). Absent still means the
  config default. Empty now errors before any provider request, and the
  message names `none` as the way to actually ask for zero tools.
  `none` combined with another name is rejected - `none,web` cannot mean
  zero tools.

_select_tool_names() no longer injects the kanban toolset into an
explicit `none` selection. That injection exists so a dispatcher-owned
worker can always hand its card back, which is right for a normal
selection and wrong when the caller asked for a session that cannot act.

_normalize_toolsets() itself is unchanged: it is shared with --skills
and the TUI launcher, where an empty value legitimately means "none
requested". The empty-vs-absent rule belongs at the toolsets call site.

_followup: hermes chat -q and the TUI launcher still treat -t "" as
unspecified; same rule, different call site, not required for the -z
case this fixes.
`cli.py::_build_cli_from_args` folded a passed-but-empty `-t ""` into the
same branch as an absent flag (`elif not toolsets:`), so `hermes chat -q
'<prompt>' -t ''` silently received the coding posture or the platform
default toolsets — the identical footgun the -z path had.

Reuse the splitter rather than a third parsing rule: the same
`_normalize_toolsets` that `hermes_cli/main_tui_launch.py` already imports
returns None for an empty value, so "was this flag passed and did it name
anything" is one comparison. Absent (None) still resolves the platform
default; empty raises before HermesCLI is constructed; `none` alone passes
through to the agent as the sentinel; `none` combined with another name is
rejected, since `none,web` cannot mean zero tools.

Four tests, two of which are negative controls: absent must still resolve a
real toolset list, and `none` alone must arrive as `["none"]` at the call
site rather than being rejected by an over-eager guard.

Verified red on base cli.py — the two error cases fail without this change
and both controls pass.

_followup: the TUI launcher still normalizes an empty -t to []; that is a
subprocess argv hand-off where empty and absent are the same string by the
time it arrives, so it needs its own decision rather than this rule.
@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets labels Oct 15, 2026
@suhas-svg

Copy link
Copy Markdown
Author

Closing — not pursuing this batch.

@suhas-svg suhas-svg closed this Oct 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI/-z: no way to request zero tools — -t "" silently uses configured toolsets; add a none sentinel

2 participants