mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
feat(skills+terminal): make bundled skill scripts runnable out of the box (#13384)
* feat(skills): inject absolute skill dir and expand ${HERMES_SKILL_DIR} templates
When a skill loads, the activation message now exposes the absolute
skill directory and substitutes ${HERMES_SKILL_DIR} /
${HERMES_SESSION_ID} tokens in the SKILL.md body, so skills with
bundled scripts can instruct the agent to run them by absolute path
without an extra skill_view round-trip.
Also adds opt-in inline-shell expansion: !`cmd` snippets in SKILL.md
are pre-executed (with the skill directory as CWD) and their stdout is
inlined into the message before the agent reads it. Off by default —
enable via skills.inline_shell in config.yaml — because any snippet
runs on the host without approval.
Changes:
- agent/skill_commands.py: template substitution, inline-shell
expansion, absolute skill-dir header, supporting-files list now
shows both relative and absolute forms.
- hermes_cli/config.py: new skills.template_vars,
skills.inline_shell, skills.inline_shell_timeout knobs.
- tests/agent/test_skill_commands.py: coverage for header, both
template tokens (present and missing session id), template_vars
disable, inline-shell default-off, enabled, CWD, and timeout.
- website/docs/developer-guide/creating-skills.md: documents the
template tokens, the absolute-path header, and the opt-in inline
shell with its security caveat.
Validation: tests/agent/ 1591 passed (includes 9 new tests).
E2E: loaded a real skill in an isolated HERMES_HOME; confirmed
${HERMES_SKILL_DIR} resolves to the absolute path, ${HERMES_SESSION_ID}
resolves to the passed task_id, !`date` runs when opt-in is set, and
stays literal when it isn't.
* feat(terminal): source ~/.bashrc (and user-listed init files) into session snapshot
bash login shells don't source ~/.bashrc, so tools that install themselves
there — nvm, asdf, pyenv, cargo, custom PATH exports — stay invisible to
the environment snapshot Hermes builds once per session. Under systemd
or any context with a minimal parent env, that surfaces as
'node: command not found' in the terminal tool even though the binary
is reachable from every interactive shell on the machine.
Changes:
- tools/environments/local.py: before the login-shell snapshot bootstrap
runs, prepend guarded 'source <file>' lines for each resolved init
file. Missing files are skipped, each source is wrapped with a
'[ -r ... ] && . ... || true' guard so a broken rc can't abort the
bootstrap.
- hermes_cli/config.py: new terminal.shell_init_files (explicit list,
supports ~ and ${VAR}) and terminal.auto_source_bashrc (default on)
knobs. When shell_init_files is set it takes precedence; when it's
empty and auto_source_bashrc is on, ~/.bashrc gets auto-sourced.
- tests/tools/test_local_shell_init.py: 10 tests covering the resolver
(auto-bashrc, missing file, explicit override, ~/${VAR} expansion,
opt-out) and the prelude builder (quoting, guarded sourcing), plus
a real-LocalEnvironment snapshot test that confirms exports in the
init file land in subsequent commands' environment.
- website/docs/reference/faq.md: documents the fix in Troubleshooting,
including the zsh-user pattern of sourcing ~/.zshrc or nvm.sh
directly via shell_init_files.
Validation: 10/10 new tests pass; tests/tools/test_local_*.py 40/40
pass; tests/agent/ 1591/1591 pass; tests/hermes_cli/test_config.py
50/50 pass. E2E in an isolated HERMES_HOME: confirmed that a fake
~/.bashrc setting a marker var and PATH addition shows up in a real
LocalEnvironment().execute() call, that auto_source_bashrc=false
suppresses it, that an explicit shell_init_files entry wins over the
auto default, and that a missing bashrc is silently skipped.
This commit is contained in:
@@ -405,3 +405,191 @@ class TestPlanSkillHelpers:
|
||||
assert "Add a /plan command" in msg
|
||||
assert ".hermes/plans/plan.md" in msg
|
||||
assert "Runtime note:" in msg
|
||||
|
||||
|
||||
class TestSkillDirectoryHeader:
|
||||
"""The activation message must expose the absolute skill directory and
|
||||
explain how to resolve relative paths, so skills with bundled scripts
|
||||
don't force the agent into a second ``skill_view()`` round-trip."""
|
||||
|
||||
def test_header_contains_absolute_skill_dir(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
skill_dir = _make_skill(tmp_path, "abs-dir-skill")
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/abs-dir-skill", "go")
|
||||
|
||||
assert msg is not None
|
||||
assert f"[Skill directory: {skill_dir}]" in msg
|
||||
assert "Resolve any relative paths" in msg
|
||||
|
||||
def test_supporting_files_shown_with_absolute_paths(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
skill_dir = _make_skill(tmp_path, "scripted-skill")
|
||||
(skill_dir / "scripts").mkdir()
|
||||
(skill_dir / "scripts" / "run.js").write_text("console.log('hi')")
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/scripted-skill")
|
||||
|
||||
assert msg is not None
|
||||
# The supporting-files block must emit both the relative form (so the
|
||||
# agent can call skill_view on it) and the absolute form (so it can
|
||||
# run the script directly via terminal).
|
||||
assert "scripts/run.js" in msg
|
||||
assert str(skill_dir / "scripts" / "run.js") in msg
|
||||
assert f"node {skill_dir}/scripts/foo.js" in msg
|
||||
|
||||
|
||||
class TestTemplateVarSubstitution:
|
||||
"""``${HERMES_SKILL_DIR}`` and ``${HERMES_SESSION_ID}`` in SKILL.md body
|
||||
are replaced before the agent sees the content."""
|
||||
|
||||
def test_substitutes_skill_dir(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
skill_dir = _make_skill(
|
||||
tmp_path,
|
||||
"templated",
|
||||
body="Run: node ${HERMES_SKILL_DIR}/scripts/foo.js",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/templated")
|
||||
|
||||
assert msg is not None
|
||||
assert f"node {skill_dir}/scripts/foo.js" in msg
|
||||
# The literal template token must not leak through.
|
||||
assert "${HERMES_SKILL_DIR}" not in msg.split("[Skill directory:")[0]
|
||||
|
||||
def test_substitutes_session_id_when_available(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"sess-templated",
|
||||
body="Session: ${HERMES_SESSION_ID}",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message(
|
||||
"/sess-templated", task_id="abc-123"
|
||||
)
|
||||
|
||||
assert msg is not None
|
||||
assert "Session: abc-123" in msg
|
||||
|
||||
def test_leaves_session_id_token_when_missing(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"sess-missing",
|
||||
body="Session: ${HERMES_SESSION_ID}",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/sess-missing", task_id=None)
|
||||
|
||||
assert msg is not None
|
||||
# No session — token left intact so the author can spot it.
|
||||
assert "Session: ${HERMES_SESSION_ID}" in msg
|
||||
|
||||
def test_disable_template_vars_via_config(self, tmp_path):
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch(
|
||||
"agent.skill_commands._load_skills_config",
|
||||
return_value={"template_vars": False},
|
||||
),
|
||||
):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"no-sub",
|
||||
body="Run: node ${HERMES_SKILL_DIR}/scripts/foo.js",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/no-sub")
|
||||
|
||||
assert msg is not None
|
||||
# Template token must survive when substitution is disabled.
|
||||
assert "${HERMES_SKILL_DIR}/scripts/foo.js" in msg
|
||||
|
||||
|
||||
class TestInlineShellExpansion:
|
||||
"""Inline ``!`cmd`` snippets in SKILL.md run before the agent sees the
|
||||
content — but only when the user has opted in via config."""
|
||||
|
||||
def test_inline_shell_is_off_by_default(self, tmp_path):
|
||||
with patch("tools.skills_tool.SKILLS_DIR", tmp_path):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"dyn-default-off",
|
||||
body="Today is !`echo INLINE_RAN`.",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/dyn-default-off")
|
||||
|
||||
assert msg is not None
|
||||
# Default config has inline_shell=False — snippet must stay literal.
|
||||
assert "!`echo INLINE_RAN`" in msg
|
||||
assert "Today is INLINE_RAN." not in msg
|
||||
|
||||
def test_inline_shell_runs_when_enabled(self, tmp_path):
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch(
|
||||
"agent.skill_commands._load_skills_config",
|
||||
return_value={"template_vars": True, "inline_shell": True,
|
||||
"inline_shell_timeout": 5},
|
||||
),
|
||||
):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"dyn-on",
|
||||
body="Marker: !`echo INLINE_RAN`.",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/dyn-on")
|
||||
|
||||
assert msg is not None
|
||||
assert "Marker: INLINE_RAN." in msg
|
||||
assert "!`echo INLINE_RAN`" not in msg
|
||||
|
||||
def test_inline_shell_runs_in_skill_directory(self, tmp_path):
|
||||
"""Inline snippets get the skill dir as CWD so relative paths work."""
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch(
|
||||
"agent.skill_commands._load_skills_config",
|
||||
return_value={"template_vars": True, "inline_shell": True,
|
||||
"inline_shell_timeout": 5},
|
||||
),
|
||||
):
|
||||
skill_dir = _make_skill(
|
||||
tmp_path,
|
||||
"dyn-cwd",
|
||||
body="Here: !`pwd`",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/dyn-cwd")
|
||||
|
||||
assert msg is not None
|
||||
assert f"Here: {skill_dir}" in msg
|
||||
|
||||
def test_inline_shell_timeout_does_not_break_message(self, tmp_path):
|
||||
with (
|
||||
patch("tools.skills_tool.SKILLS_DIR", tmp_path),
|
||||
patch(
|
||||
"agent.skill_commands._load_skills_config",
|
||||
return_value={"template_vars": True, "inline_shell": True,
|
||||
"inline_shell_timeout": 1},
|
||||
),
|
||||
):
|
||||
_make_skill(
|
||||
tmp_path,
|
||||
"dyn-slow",
|
||||
body="Slow: !`sleep 5 && printf DYN_MARKER`",
|
||||
)
|
||||
scan_skill_commands()
|
||||
msg = build_skill_invocation_message("/dyn-slow")
|
||||
|
||||
assert msg is not None
|
||||
# Timeout is surfaced as a marker instead of propagating as an error,
|
||||
# and the rest of the skill message still renders.
|
||||
assert "inline-shell timeout" in msg
|
||||
# The command's intended stdout never made it through — only the
|
||||
# timeout marker (which echoes the command text) survives.
|
||||
assert "DYN_MARKER" not in msg.replace("sleep 5 && printf DYN_MARKER", "")
|
||||
|
||||
162
tests/tools/test_local_shell_init.py
Normal file
162
tests/tools/test_local_shell_init.py
Normal file
@@ -0,0 +1,162 @@
|
||||
"""Tests for terminal.shell_init_files / terminal.auto_source_bashrc.
|
||||
|
||||
A bash ``-l -c`` invocation does NOT source ``~/.bashrc``, so tools that
|
||||
register themselves there (nvm, asdf, pyenv) stay invisible to the
|
||||
environment snapshot built by ``LocalEnvironment.init_session``. These
|
||||
tests verify the config-driven prelude that fixes that.
|
||||
"""
|
||||
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.environments.local import (
|
||||
LocalEnvironment,
|
||||
_prepend_shell_init,
|
||||
_read_terminal_shell_init_config,
|
||||
_resolve_shell_init_files,
|
||||
)
|
||||
|
||||
|
||||
class TestResolveShellInitFiles:
|
||||
def test_auto_sources_bashrc_when_present(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export MARKER=seen\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
# Default config: auto_source_bashrc on, no explicit list.
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == [str(bashrc)]
|
||||
|
||||
def test_skips_bashrc_when_missing(self, tmp_path, monkeypatch):
|
||||
# No bashrc written.
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
def test_auto_source_bashrc_off_suppresses_default(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export MARKER=seen\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], False),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
def test_explicit_list_wins_over_auto(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export FROM_BASHRC=1\n')
|
||||
custom = tmp_path / "custom.sh"
|
||||
custom.write_text('export FROM_CUSTOM=1\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
# auto_source_bashrc stays True but the explicit list takes precedence.
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(custom)], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == [str(custom)]
|
||||
assert str(bashrc) not in resolved
|
||||
|
||||
def test_expands_home_and_env_vars(self, tmp_path, monkeypatch):
|
||||
target = tmp_path / "rc" / "custom.sh"
|
||||
target.parent.mkdir()
|
||||
target.write_text('export A=1\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
monkeypatch.setenv("CUSTOM_RC_DIR", str(tmp_path / "rc"))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=(["~/rc/custom.sh"], False),
|
||||
):
|
||||
resolved_home = _resolve_shell_init_files()
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=(["${CUSTOM_RC_DIR}/custom.sh"], False),
|
||||
):
|
||||
resolved_var = _resolve_shell_init_files()
|
||||
|
||||
assert resolved_home == [str(target)]
|
||||
assert resolved_var == [str(target)]
|
||||
|
||||
def test_missing_explicit_files_are_skipped_silently(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(tmp_path / "does-not-exist.sh")], False),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
|
||||
class TestPrependShellInit:
|
||||
def test_empty_list_returns_command_unchanged(self):
|
||||
assert _prepend_shell_init("echo hi", []) == "echo hi"
|
||||
|
||||
def test_prepends_guarded_source_lines(self):
|
||||
wrapped = _prepend_shell_init("echo hi", ["/tmp/a.sh", "/tmp/b.sh"])
|
||||
assert "echo hi" in wrapped
|
||||
# Each file is sourced through a guarded [ -r … ] && . '…' || true
|
||||
# pattern so a missing/broken rc can't abort the bootstrap.
|
||||
assert "/tmp/a.sh" in wrapped
|
||||
assert "/tmp/b.sh" in wrapped
|
||||
assert "|| true" in wrapped
|
||||
assert "set +e" in wrapped
|
||||
|
||||
def test_escapes_single_quotes(self):
|
||||
wrapped = _prepend_shell_init("echo hi", ["/tmp/o'malley.sh"])
|
||||
# The path must survive as the shell receives it; embedded single
|
||||
# quote is escaped as '\'' rather than breaking the outer quoting.
|
||||
assert "o'\\''malley" in wrapped
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
os.environ.get("CI") == "true" and not os.path.isfile("/bin/bash"),
|
||||
reason="Requires bash; CI sandbox may strip it.",
|
||||
)
|
||||
class TestSnapshotEndToEnd:
|
||||
"""Spin up a real LocalEnvironment and confirm the snapshot sources
|
||||
extra init files."""
|
||||
|
||||
def test_snapshot_picks_up_init_file_exports(self, tmp_path, monkeypatch):
|
||||
init_file = tmp_path / "custom-init.sh"
|
||||
init_file.write_text(
|
||||
'export HERMES_SHELL_INIT_PROBE="probe-ok"\n'
|
||||
'export PATH="/opt/shell-init-probe/bin:$PATH"\n'
|
||||
)
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(init_file)], False),
|
||||
):
|
||||
env = LocalEnvironment(cwd=str(tmp_path), timeout=15)
|
||||
try:
|
||||
result = env.execute(
|
||||
'echo "PROBE=$HERMES_SHELL_INIT_PROBE"; echo "PATH=$PATH"'
|
||||
)
|
||||
finally:
|
||||
env.cleanup()
|
||||
|
||||
output = result.get("output", "")
|
||||
assert "PROBE=probe-ok" in output
|
||||
assert "/opt/shell-init-probe/bin" in output
|
||||
Reference in New Issue
Block a user