diff --git a/README.md b/README.md index 26ed9ca..378d3e5 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ See also: [agentic-slackbot](https://github.com/John-Lin/agentic-slackbot) — a - Supports OpenAI, Azure OpenAI endpoints - Per-conversation history with automatic truncation - Group reply chain — after `@mention`, anyone can continue by replying +- Local shell skills — let the agent run shell scripts from `skills/` (opt-in via `SHELL_SKILLS_ENABLED`) ## Install Dependencies @@ -41,6 +42,9 @@ export TELEGRAM_BOT_TOKEN="" # OpenAI API export OPENAI_API_KEY="" export OPENAI_MODEL="gpt-5.4" + +# Shell skills (disabled by default) +# export SHELL_SKILLS_ENABLED=1 ``` ## Agent Instructions @@ -180,6 +184,7 @@ docker run -d \ -e TELEGRAM_BOT_TOKEN="" \ -e OPENAI_API_KEY="" \ -e OPENAI_MODEL="gpt-5.4" \ + -v /path/to/instructions.md:/app/instructions.md \ -v /path/to/access.json:/app/access.json \ agentic-telegram-bot ``` @@ -198,17 +203,3 @@ docker run -d \ -v /path/to/access.json:/app/access.json \ agentic-telegram-bot ``` - -If you do not use MCP servers, you still need to mount `instructions.md`: - -```bash -docker run -d \ - --name telegent \ - -e BOT_USERNAME="@your_bot_username" \ - -e TELEGRAM_BOT_TOKEN="" \ - -e OPENAI_API_KEY="" \ - -e OPENAI_MODEL="gpt-5.4" \ - -v /path/to/instructions.md:/app/instructions.md \ - -v /path/to/access.json:/app/access.json \ - agentic-telegram-bot -``` diff --git a/bot/agents.py b/bot/agents.py index eb39823..2894267 100644 --- a/bot/agents.py +++ b/bot/agents.py @@ -8,6 +8,10 @@ from agents import Agent from agents import Runner +from agents import ShellCommandRequest +from agents import ShellTool +from agents import ShellToolLocalEnvironment +from agents import ShellToolLocalSkill from agents import TResponseInputItem from agents.mcp import MCPServerStdio from agents.mcp import MCPServerStreamableHttp @@ -20,6 +24,8 @@ MAX_TURNS = 10 MCP_SESSION_TIMEOUT_SECONDS = 30.0 +SHELL_TIMEOUT = 30.0 +SKILLS_DIR = Path(__file__).resolve().parent.parent / "skills" set_tracing_disabled(True) @@ -58,20 +64,101 @@ def _get_model() -> OpenAIResponsesModel | OpenAIChatCompletionsModel: return OpenAIResponsesModel(model=model_name, openai_client=client) +def _parse_skill_description(content: str) -> str: + """Return the description field from a SKILL.md YAML frontmatter, or "".""" + if not content.startswith("---"): + return "" + end = content.find("\n---", 3) + if end == -1: + return "" + for line in content[3:end].splitlines(): + if line.startswith("description:"): + value = line[len("description:") :].strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"): + value = value[1:-1] + return value + return "" + + +def _load_shell_skills() -> list[ShellToolLocalSkill]: + """Discover local shell skills under SKILLS_DIR. + + Each immediate subdirectory of SKILLS_DIR containing a SKILL.md is mounted + as a ShellToolLocalSkill. The skill name is the directory name; the + description is read from the SKILL.md YAML frontmatter. + """ + if not SKILLS_DIR.is_dir(): + return [] + skills: list[ShellToolLocalSkill] = [] + for skill_dir in sorted(SKILLS_DIR.iterdir()): + skill_md = skill_dir / "SKILL.md" + if not skill_dir.is_dir() or not skill_md.is_file(): + continue + try: + content = skill_md.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + logging.warning("Skipping unreadable skill file: %s", skill_md, exc_info=True) + continue + skills.append( + ShellToolLocalSkill( + name=skill_dir.name, + description=_parse_skill_description(content), + path=str(skill_dir), + ) + ) + return skills + + +async def _shell_executor(request: ShellCommandRequest) -> str: + """Run each shell command from the request and return combined output. + + Honours action.timeout_ms when set, otherwise falls back to SHELL_TIMEOUT. + stderr is merged into stdout for simplicity. + """ + action = request.data.action + timeout = (action.timeout_ms / 1000.0) if action.timeout_ms is not None else SHELL_TIMEOUT + + outputs: list[str] = [] + for command in action.commands: + try: + proc = await asyncio.create_subprocess_shell( + command, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.STDOUT, + ) + except OSError as e: + outputs.append(f"Failed to run command: {command}: {e}") + break + try: + stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=timeout) + output = stdout.decode("utf-8", errors="replace") + if proc.returncode: + output += f"\n[exit code: {proc.returncode}]" + outputs.append(output) + except TimeoutError: + proc.kill() + await proc.communicate() + outputs.append(f"Command timed out after {timeout}s: {command}") + break + return "\n".join(outputs) + + class OpenAIAgent: - """A wrapper for OpenAI Agent with MCP server support.""" + """A wrapper for OpenAI Agent with MCP server and local shell skill support.""" def __init__( self, name: str, instructions: str, mcp_servers: list | None = None, + tools: list | None = None, ) -> None: self.agent = Agent( name=name, instructions=instructions, model=_get_model(), mcp_servers=(mcp_servers if mcp_servers is not None else []), + tools=(tools if tools is not None else []), ) self.name = name self._conversations: dict[int, list[TResponseInputItem]] = {} @@ -127,8 +214,15 @@ def from_dict(cls, name: str, config: dict[str, Any]) -> OpenAIAgent: }, ) ) + tools: list[Any] = [] + if os.getenv("SHELL_SKILLS_ENABLED"): + skills = _load_shell_skills() + if skills: + environment = ShellToolLocalEnvironment(type="local", skills=skills) + tools.append(ShellTool(executor=_shell_executor, environment=environment)) + instructions = _load_instructions() - return cls(name, instructions=instructions, mcp_servers=mcp_servers) + return cls(name, instructions=instructions, mcp_servers=mcp_servers, tools=tools) async def connect(self) -> None: for mcp_server in self.agent.mcp_servers: diff --git a/skills/.gitignore b/skills/.gitignore new file mode 100644 index 0000000..1425c6f --- /dev/null +++ b/skills/.gitignore @@ -0,0 +1,4 @@ +# Users drop their own shell skills into this directory; the bot auto-loads +# any subdirectory containing a SKILL.md. Skill content is not tracked. +* +!.gitignore diff --git a/tests/test_agents.py b/tests/test_agents.py index 9e8dead..1bf6636 100644 --- a/tests/test_agents.py +++ b/tests/test_agents.py @@ -1,10 +1,12 @@ from __future__ import annotations +from pathlib import Path from unittest.mock import MagicMock from unittest.mock import create_autospec from unittest.mock import patch import pytest +from agents import ShellTool from agents.mcp import MCPServerStdio from agents.mcp import MCPServerStreamableHttp from agents.models.interface import Model @@ -14,6 +16,8 @@ from bot.agents import MAX_TURNS from bot.agents import OpenAIAgent from bot.agents import _get_model +from bot.agents import _parse_skill_description +from bot.agents import _shell_executor @pytest.fixture @@ -23,9 +27,12 @@ def _stub_instructions(monkeypatch): @pytest.fixture(autouse=True) -def _mock_model(monkeypatch): - """Prevent tests from constructing a real OpenAI client.""" +def _mock_model(monkeypatch, tmp_path_factory): + """Prevent tests from constructing a real OpenAI client and isolate skills.""" monkeypatch.setattr("bot.agents._get_model", lambda: create_autospec(Model)) + # Point SKILLS_DIR at an empty directory so tests do not auto-load the + # real ./skills/ on disk. Tests that need skills can override this. + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path_factory.mktemp("empty_skills")) class TestGetModel: @@ -231,3 +238,247 @@ def test_no_truncation_when_under_limit(self): msgs = agent.get_messages(chat_id=100) user_msgs = [m for m in msgs if m["role"] == "user"] assert len(user_msgs) == 3 + + +@pytest.mark.usefixtures("_stub_instructions") +class TestLoadShellSkills: + def _make_skill_dir(self, tmp_path): + """Create a valid skill directory and return its parent.""" + skill_dir = tmp_path / "my-skill" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("---\nname: my-skill\ndescription: A test skill\n---\n") + return tmp_path + + def test_disabled_by_default(self, tmp_path, monkeypatch): + """Skills exist but SHELL_SKILLS_ENABLED is not set — no ShellTool.""" + monkeypatch.delenv("SHELL_SKILLS_ENABLED", raising=False) + monkeypatch.setattr("bot.agents.SKILLS_DIR", self._make_skill_dir(tmp_path)) + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tools = [t for t in agent.agent.tools if isinstance(t, ShellTool)] + assert len(shell_tools) == 0 + + def test_enabled_with_env_var(self, tmp_path, monkeypatch): + """SHELL_SKILLS_ENABLED=1 and skills exist — ShellTool is added.""" + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + monkeypatch.setattr("bot.agents.SKILLS_DIR", self._make_skill_dir(tmp_path)) + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tools = [t for t in agent.agent.tools if isinstance(t, ShellTool)] + assert len(shell_tools) == 1 + + def test_no_shell_tool_when_skills_dir_missing(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path / "nonexistent") + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tools = [t for t in agent.agent.tools if isinstance(t, ShellTool)] + assert len(shell_tools) == 0 + + def test_shell_tool_added_when_skill_found(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + skill_dir = tmp_path / "my-skill" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("---\nname: my-skill\ndescription: A test skill\n---\n") + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tool = next(t for t in agent.agent.tools if isinstance(t, ShellTool)) + skill = shell_tool.environment["skills"][0] + assert skill["name"] == "my-skill" + assert skill["description"] == "A test skill" + assert skill["path"] == str(skill_dir) + + def test_multiple_skills_all_mounted(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + for name in ["skill-a", "skill-b"]: + d = tmp_path / name + d.mkdir() + (d / "SKILL.md").write_text(f"---\nname: {name}\ndescription: desc {name}\n---\n") + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tool = next(t for t in agent.agent.tools if isinstance(t, ShellTool)) + assert len(shell_tool.environment["skills"]) == 2 + + def test_directory_without_skill_md_is_skipped(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + (tmp_path / "not-a-skill").mkdir() + good = tmp_path / "real-skill" + good.mkdir() + (good / "SKILL.md").write_text("---\nname: real-skill\ndescription: d\n---\n") + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tool = next(t for t in agent.agent.tools if isinstance(t, ShellTool)) + skills = shell_tool.environment["skills"] + assert len(skills) == 1 + assert skills[0]["name"] == "real-skill" + + def test_mcp_servers_and_shell_skills_coexist(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + skill_dir = tmp_path / "s" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text("---\nname: s\ndescription: d\n---\n") + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + + config = {"mcpServers": {"my-mcp": {"command": "uvx", "args": ["something"]}}} + agent = OpenAIAgent.from_dict("test", config) + assert len(agent.agent.mcp_servers) == 1 + shell_tools = [t for t in agent.agent.tools if isinstance(t, ShellTool)] + assert len(shell_tools) == 1 + + def test_unreadable_utf8_skill_file_is_skipped(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + bad = tmp_path / "bad-skill" + bad.mkdir() + (bad / "SKILL.md").write_bytes(b"\xff\xfe\x00\x00") + + good = tmp_path / "good-skill" + good.mkdir() + (good / "SKILL.md").write_text("---\nname: good-skill\ndescription: good\n---\n") + + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tool = next(t for t in agent.agent.tools if isinstance(t, ShellTool)) + skills = shell_tool.environment["skills"] + assert len(skills) == 1 + assert skills[0]["name"] == "good-skill" + + def test_oserror_reading_skill_file_is_skipped(self, tmp_path, monkeypatch): + monkeypatch.setenv("SHELL_SKILLS_ENABLED", "1") + bad = tmp_path / "bad-skill" + bad.mkdir() + bad_file = bad / "SKILL.md" + bad_file.write_text("---\nname: bad\ndescription: bad\n---\n") + + good = tmp_path / "good-skill" + good.mkdir() + (good / "SKILL.md").write_text("---\nname: good-skill\ndescription: good\n---\n") + + original_read_text = Path.read_text + + def _read_text(self: Path, *args, **kwargs): + if self == bad_file: + raise OSError("permission denied") + return original_read_text(self, *args, **kwargs) + + monkeypatch.setattr("bot.agents.SKILLS_DIR", tmp_path) + monkeypatch.setattr(Path, "read_text", _read_text) + + agent = OpenAIAgent.from_dict("test", {"mcpServers": {}}) + shell_tool = next(t for t in agent.agent.tools if isinstance(t, ShellTool)) + skills = shell_tool.environment["skills"] + assert len(skills) == 1 + assert skills[0]["name"] == "good-skill" + + +class TestParseSkillDescription: + def test_unquoted_description(self): + content = "---\nname: my-skill\ndescription: A test skill\n---\nBody text." + assert _parse_skill_description(content) == "A test skill" + + def test_double_quoted_description(self): + content = '---\nname: my-skill\ndescription: "A quoted skill"\n---\n' + assert _parse_skill_description(content) == "A quoted skill" + + def test_single_quoted_description(self): + content = "---\nname: my-skill\ndescription: 'Single quoted'\n---\n" + assert _parse_skill_description(content) == "Single quoted" + + def test_no_frontmatter(self): + content = "Just a plain markdown file." + assert _parse_skill_description(content) == "" + + def test_no_description_field(self): + content = "---\nname: my-skill\n---\nBody." + assert _parse_skill_description(content) == "" + + def test_unclosed_frontmatter(self): + content = "---\nname: my-skill\ndescription: never closed" + assert _parse_skill_description(content) == "" + + def test_empty_description(self): + content = "---\nname: my-skill\ndescription:\n---\n" + assert _parse_skill_description(content) == "" + + def test_description_with_colon_in_value(self): + content = "---\ndescription: Run this: do stuff\n---\n" + assert _parse_skill_description(content) == "Run this: do stuff" + + +def _make_shell_request(commands: list[str], timeout_ms: int | None = None): + """Build a minimal object matching the ShellCommandRequest interface.""" + from types import SimpleNamespace + + action = SimpleNamespace(commands=commands, timeout_ms=timeout_ms) + data = SimpleNamespace(action=action) + return SimpleNamespace(data=data) + + +class TestShellExecutor: + @pytest.mark.anyio + async def test_single_command_returns_stdout(self): + request = _make_shell_request(["echo hello"]) + result = await _shell_executor(request) + assert result.strip() == "hello" + + @pytest.mark.anyio + async def test_multiple_commands_combined(self): + request = _make_shell_request(["echo first", "echo second"]) + result = await _shell_executor(request) + assert "first" in result + assert "second" in result + assert result.index("first") < result.index("second") + + @pytest.mark.anyio + async def test_stderr_merged_into_stdout(self): + request = _make_shell_request(["echo err >&2"]) + result = await _shell_executor(request) + assert result.strip() == "err" + + @pytest.mark.anyio + async def test_command_timeout_kills_process(self): + request = _make_shell_request(["sleep 30"], timeout_ms=100) + result = await _shell_executor(request) + assert "timed out" in result.lower() + + @pytest.mark.anyio + async def test_nonzero_exit_code_appends_exit_code(self): + request = _make_shell_request(["echo failing && exit 1"]) + result = await _shell_executor(request) + assert "failing" in result + assert "[exit code: 1]" in result + + @pytest.mark.anyio + async def test_zero_exit_code_no_suffix(self): + request = _make_shell_request(["echo ok"]) + result = await _shell_executor(request) + assert "exit code" not in result + + @pytest.mark.anyio + async def test_timeout_ms_none_uses_default(self): + """When timeout_ms is None, SHELL_TIMEOUT is used (command completes fine).""" + request = _make_shell_request(["echo ok"], timeout_ms=None) + result = await _shell_executor(request) + assert result.strip() == "ok" + + @pytest.mark.anyio + async def test_timeout_stops_remaining_commands(self): + """After a timeout, subsequent commands are not executed.""" + request = _make_shell_request(["sleep 30", "echo should-not-run"], timeout_ms=100) + result = await _shell_executor(request) + assert "timed out" in result.lower() + assert "should-not-run" not in result + + @pytest.mark.anyio + async def test_subprocess_oserror_returns_error_message(self, monkeypatch): + """When create_subprocess_shell raises OSError, return error text instead of crashing.""" + import asyncio as _asyncio + + async def _failing_shell(*args, **kwargs): + raise OSError("fork failed") + + monkeypatch.setattr(_asyncio, "create_subprocess_shell", _failing_shell) + + request = _make_shell_request(["echo hello"]) + result = await _shell_executor(request) + assert "fork failed" in result