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
34 changes: 20 additions & 14 deletions hermes_cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,29 @@ def _cleanup_oneshot_runtime() -> None:
pass


def _oneshot_kwargs_from_args(args) -> dict:
"""Build the ``_run_and_exit_oneshot`` kwargs shared by both -z dispatch sites.

Kept as a single place so a field forwarded on one path is never silently
missing from the other -- ``--skills`` used to be dropped exactly this way
(see #75930): parsed at the top level, forwarded nowhere.
"""
return {
"model": getattr(args, "model", None),
"provider": getattr(args, "provider", None),
"toolsets": getattr(args, "toolsets", None),
"skills": getattr(args, "skills", None),
"usage_file": getattr(args, "usage_file", None),
}


def _run_and_exit_oneshot(
prompt: str,
*,
model: object = None,
provider: object = None,
toolsets: object = None,
skills: object = None,
usage_file: object = None,
) -> None:
try:
Expand All @@ -180,6 +197,7 @@ def _run_and_exit_oneshot(
model=model,
provider=provider,
toolsets=toolsets,
skills=skills,
usage_file=usage_file,
)
except KeyboardInterrupt:
Expand Down Expand Up @@ -10835,13 +10853,7 @@ def _try_termux_fast_cli_launch() -> bool:

if getattr(args, "oneshot", None):
_prepare_agent_startup(args)
_run_and_exit_oneshot(
args.oneshot,
model=getattr(args, "model", None),
provider=getattr(args, "provider", None),
toolsets=getattr(args, "toolsets", None),
usage_file=getattr(args, "usage_file", None),
)
_run_and_exit_oneshot(args.oneshot, **_oneshot_kwargs_from_args(args))

if (args.resume or args.continue_last) and args.command is None:
args.command = "chat"
Expand Down Expand Up @@ -12439,13 +12451,7 @@ def _add_session_filter_args(p, default_older_help):
# Handle top-level --oneshot / -z: single-shot mode, stdout = final
# response only, nothing else. Bypasses cli.py entirely.
if getattr(args, "oneshot", None):
_run_and_exit_oneshot(
args.oneshot,
model=getattr(args, "model", None),
provider=getattr(args, "provider", None),
toolsets=getattr(args, "toolsets", None),
usage_file=getattr(args, "usage_file", None),
)
_run_and_exit_oneshot(args.oneshot, **_oneshot_kwargs_from_args(args))

# Handle top-level --resume / --continue as shortcut to chat
if (args.resume or args.continue_last) and args.command is None:
Expand Down
35 changes: 34 additions & 1 deletion hermes_cli/oneshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

Toolsets = explicit --toolsets when provided, otherwise whatever the user has
configured for "cli" in `hermes tools`.
Rules / memory / AGENTS.md / preloaded skills = same as a normal chat turn.
Rules / memory / AGENTS.md = same as a normal chat turn.
Explicit --skills/-s preloading is honoured the same way --toolsets is.
Approvals = auto-bypassed (HERMES_YOLO_MODE=1 is set for the call).
Working directory = the user's CWD (AGENTS.md etc. resolve from there as usual).

Expand Down Expand Up @@ -172,6 +173,7 @@ def run_oneshot(
model: Optional[str] = None,
provider: Optional[str] = None,
toolsets: object = None,
skills: object = None,
usage_file: Optional[str] = None,
) -> int:
"""Execute a single prompt and print only the final content block.
Expand All @@ -183,6 +185,9 @@ def run_oneshot(
provider: Optional provider override. Falls back to config.yaml's
model.provider, then "auto".
toolsets: Optional comma-separated string or iterable of toolsets.
skills: Optional comma-separated string or iterable of skill
identifiers to preload for this invocation (same identifiers
accepted by `hermes --skills`).
usage_file: Optional path; when set, a JSON usage report (estimated
cost, token counts, model, api_calls) is written there after the
run — even when the run fails — so pipelines can account for
Expand Down Expand Up @@ -216,6 +221,31 @@ def run_oneshot(
return 2
use_config_toolsets = _normalize_toolsets(toolsets) is None

# --skills/-s preloading. Built up-front (before the stdout/stderr
# redirect below) so an unknown-skill error actually reaches the
# caller instead of vanishing into devnull. Mirrors cli.py's
# interactive --skills handling (agent/skill_commands.py), which this
# path previously bypassed entirely: --skills was parsed at the top
# level but never forwarded past _run_and_exit_oneshot()/run_oneshot(),
# so -z --skills <name> silently ran with no skill content injected.
ephemeral_system_prompt: Optional[str] = None
if skills:
from cli import _parse_skills_argument
from agent.skill_commands import build_preloaded_skills_prompt

parsed_skills = _parse_skills_argument(skills)
if parsed_skills:
skills_prompt, loaded_skills, missing_skills = build_preloaded_skills_prompt(
parsed_skills
)
if missing_skills and not loaded_skills:
sys.stderr.write(
"hermes -z: Unknown skill(s): " + ", ".join(missing_skills) + "\n"
)
return 2
if skills_prompt:
ephemeral_system_prompt = skills_prompt

# Auto-approve any shell / tool approvals. Non-interactive by
# definition — a prompt would hang forever.
os.environ["HERMES_YOLO_MODE"] = "1"
Expand Down Expand Up @@ -248,6 +278,7 @@ def run_oneshot(
provider=provider,
toolsets=explicit_toolsets,
use_config_toolsets=use_config_toolsets,
ephemeral_system_prompt=ephemeral_system_prompt,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please add a hermetic regression test around this handoff: stub the preload builder and _run_agent, invoke run_oneshot(..., skills=...), and assert the generated content reaches _run_agent as ephemeral_system_prompt. Existing tests call _run_agent directly, so they cannot catch a dropped --skills value.

)
except BaseException as exc: # noqa: BLE001
# Capture anything that escapes the agent (including OSError
Expand Down Expand Up @@ -316,6 +347,7 @@ def _run_agent(
provider: Optional[str] = None,
toolsets: object = None,
use_config_toolsets: bool = True,
ephemeral_system_prompt: Optional[str] = None,
) -> tuple[str, dict]:
"""Build an AIAgent exactly like a normal CLI chat turn would, then
run a single conversation. Returns ``(final_response, run_result)``."""
Expand Down Expand Up @@ -432,6 +464,7 @@ def _run_agent(
# - dangerous-command approval → bypassed via HERMES_YOLO_MODE=1
# - skill secret capture → returns gracefully when no callback set
clarify_callback=_oneshot_clarify_callback,
ephemeral_system_prompt=ephemeral_system_prompt,
)

# Belt-and-braces: make sure AIAgent doesn't invoke any streaming
Expand Down
204 changes: 204 additions & 0 deletions tests/hermes_cli/test_oneshot_skills.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
"""Regression tests for --skills/-s forwarding through -z/--oneshot (#75930).

Before this fix, ``--skills`` was parsed at the top level but never forwarded
past ``_run_and_exit_oneshot()``/``run_oneshot()`` -- every ``hermes -z ...
--skills <name>`` call ran with zero skill content injected, exiting 0 with
no visible error. These tests pin the forwarding chain at both ends: the
shared dispatch-kwargs helper in ``hermes_cli/main.py``, and the
skills-to-``ephemeral_system_prompt`` translation in
``hermes_cli/oneshot.py::run_oneshot``.
"""

import sys
import types
from types import SimpleNamespace
from unittest.mock import patch

import pytest

from hermes_cli.main import _oneshot_kwargs_from_args, _run_and_exit_oneshot
from hermes_cli.oneshot import run_oneshot


def _parse_skills_argument(skills):
"""Stand-in mirroring cli.py's real ``_parse_skills_argument`` (comma-split,
strip, dedupe, drop-empties).

``run_oneshot`` does a *local* ``from cli import _parse_skills_argument``,
which would otherwise pull the entire ~17k-line ``cli.py`` (and its
``prompt_toolkit`` TUI dependency) into these unit tests just to reach one
small pure string-parsing helper. Patched in via the ``_parse_skills``
fixture below instead.
"""
if not skills:
return []
raw_values = [skills] if isinstance(skills, str) else list(skills)
parsed, seen = [], set()
for raw in raw_values:
for part in str(raw).split(","):
normalized = part.strip()
if not normalized or normalized in seen:
continue
seen.add(normalized)
parsed.append(normalized)
return parsed


class TestOneshotKwargsFromArgs:
"""The kwargs both -z dispatch sites in main.py build from parsed args."""

def test_includes_skills(self):
args = SimpleNamespace(
model="m", provider="p", toolsets="t", skills="my-skill", usage_file=None
)
kwargs = _oneshot_kwargs_from_args(args)
assert kwargs["skills"] == "my-skill"

def test_missing_skills_attr_defaults_to_none(self):
# A future argparse refactor that drops the attribute must not crash
# the dispatcher -- it should degrade to "no skills requested".
args = SimpleNamespace(model=None, provider=None, toolsets=None, usage_file=None)
kwargs = _oneshot_kwargs_from_args(args)
assert kwargs["skills"] is None

def test_all_fields_present(self):
# Locks the exact kwarg set both call sites rely on -- a field added
# here now only needs to be added once.
args = SimpleNamespace(
model="m", provider="p", toolsets="t", skills="s", usage_file="u"
)
assert _oneshot_kwargs_from_args(args) == {
"model": "m",
"provider": "p",
"toolsets": "t",
"skills": "s",
"usage_file": "u",
}


class TestRunOneshotSkillsForwarding:
"""hermes_cli.oneshot.run_oneshot's --skills handling."""

@pytest.fixture(autouse=True)
def _stub_cli_parse_skills_argument(self, monkeypatch):
# run_oneshot() does a LOCAL `from cli import _parse_skills_argument`.
# unittest.mock.patch("cli._parse_skills_argument", ...) would still
# have to import the real `cli` module first to find the attribute --
# which drags in prompt_toolkit and everything else `cli.py` needs at
# module-import time. Inject a lightweight stand-in module into
# sys.modules instead, so the local import never touches the real one.
fake_cli = types.ModuleType("cli")
fake_cli._parse_skills_argument = _parse_skills_argument
monkeypatch.setitem(sys.modules, "cli", fake_cli)

def _run_agent_ok(self, _prompt, **_kwargs):
return "final answer", {"final_response": "final answer"}

def test_skills_preload_becomes_ephemeral_system_prompt(self):
with patch(
"agent.skill_commands.build_preloaded_skills_prompt",
return_value=("SKILL BODY TEXT", ["my-skill"], []),
) as build_mock, patch(
"hermes_cli.oneshot._run_agent",
side_effect=self._run_agent_ok,
) as run_agent_mock:
rc = run_oneshot("hi", skills="my-skill")

assert rc == 0
build_mock.assert_called_once()
run_agent_mock.assert_called_once()
assert build_mock.call_args.args[0] == ["my-skill"]
assert run_agent_mock.call_args.kwargs["ephemeral_system_prompt"] == "SKILL BODY TEXT"

def test_no_skills_requested_means_no_ephemeral_system_prompt(self):
with patch(
"hermes_cli.oneshot._run_agent", side_effect=self._run_agent_ok
) as run_agent_mock:
rc = run_oneshot("hi")

assert rc == 0
run_agent_mock.assert_called_once()
assert run_agent_mock.call_args.kwargs["ephemeral_system_prompt"] is None

def test_whitespace_only_skills_string_is_treated_as_no_skills(self):
# _parse_skills_argument strips/drops empty parts -- " , , " must
# not reach build_preloaded_skills_prompt at all.
with patch(
"agent.skill_commands.build_preloaded_skills_prompt"
) as build_mock, patch(
"hermes_cli.oneshot._run_agent", side_effect=self._run_agent_ok
) as run_agent_mock:
rc = run_oneshot("hi", skills=" , , ")

assert rc == 0
build_mock.assert_not_called()
run_agent_mock.assert_called_once()
assert run_agent_mock.call_args.kwargs["ephemeral_system_prompt"] is None

def test_all_requested_skills_missing_exits_2_without_running_agent(self, capsys):
with patch(
"agent.skill_commands.build_preloaded_skills_prompt",
return_value=("", [], ["unknown-skill"]),
), patch("hermes_cli.oneshot._run_agent") as run_agent_mock:
rc = run_oneshot("hi", skills="unknown-skill")

assert rc == 2
run_agent_mock.assert_not_called()
assert "Unknown skill(s): unknown-skill" in capsys.readouterr().err

def test_partially_missing_skills_still_runs_with_the_loaded_ones(self):
with patch(
"agent.skill_commands.build_preloaded_skills_prompt",
return_value=("LOADED SKILL BODY", ["good-skill"], ["bad-skill"]),
), patch(
"hermes_cli.oneshot._run_agent", side_effect=self._run_agent_ok
) as run_agent_mock:
rc = run_oneshot("hi", skills="good-skill,bad-skill")

assert rc == 0
assert run_agent_mock.call_args.kwargs["ephemeral_system_prompt"] == "LOADED SKILL BODY"

def test_skills_that_resolve_to_no_content_leave_prompt_none(self):
# build_preloaded_skills_prompt can return an empty prompt string
# even with a loaded skill (e.g. an empty SKILL.md body) -- must not
# pass an empty string as ephemeral_system_prompt.
with patch(
"agent.skill_commands.build_preloaded_skills_prompt",
return_value=("", ["empty-skill"], []),
), patch(
"hermes_cli.oneshot._run_agent", side_effect=self._run_agent_ok
) as run_agent_mock:
rc = run_oneshot("hi", skills="empty-skill")

assert rc == 0
assert run_agent_mock.call_args.kwargs["ephemeral_system_prompt"] is None


class TestRunAndExitOneshotForwardsSkills:
"""Closes the remaining gap: _run_and_exit_oneshot -> run_oneshot.

Without this, a regression that drops ``skills`` between
``_run_and_exit_oneshot`` and ``run_oneshot`` could pass every test
above (they call ``run_oneshot`` directly) while the real -z path stays
broken -- exactly the shape of the original bug.
"""

def test_skills_reaches_run_oneshot(self):
# _exit_after_oneshot does a hard os._exit(); must be patched out or
# the test process itself would terminate.
with patch("hermes_cli.oneshot.run_oneshot", return_value=0) as run_oneshot_mock, \
patch("hermes_cli.main._exit_after_oneshot") as exit_mock, \
patch("hermes_cli.main._cleanup_oneshot_runtime"):
_run_and_exit_oneshot("hi", skills="my-skill")

run_oneshot_mock.assert_called_once()
assert run_oneshot_mock.call_args.kwargs["skills"] == "my-skill"
exit_mock.assert_called_once_with(0)

def test_no_skills_forwards_none(self):
with patch("hermes_cli.oneshot.run_oneshot", return_value=0) as run_oneshot_mock, \
patch("hermes_cli.main._exit_after_oneshot"), \
patch("hermes_cli.main._cleanup_oneshot_runtime"):
_run_and_exit_oneshot("hi")

assert run_oneshot_mock.call_args.kwargs["skills"] is None