Merge pull request #9 from John-Lin/wip/shell-without-skills

feat: decouple shell mode from shell skills

Author: John-Lin

README.md

@@ -9,7 +9,7 @@ See also: [agentic-telegram-bot](https://github.com/John-Lin/agentic-telegram-bo
 - Channel @mention and DM support
 - Thread-aware conversations (follow-ups stay in the same thread)
 - Connects to any MCP server via `servers_config.json`
-- Local shell skills via `ShellTool` (opt-in via `SHELL_SKILLS_ENABLED`)
+- Optional local shell via `ShellTool`, controlled by `SHELL_ENABLED` and `SHELL_SKILLS_DIR`
 - Supports OpenAI and OpenAI-compatible endpoints (including Azure OpenAI v1 API)
 - Per-conversation history with automatic truncation (last 10 turns)
 
@@ -43,8 +43,9 @@ export SLACK_APP_TOKEN=""
 export OPENAI_API_KEY=""
 export OPENAI_MODEL="gpt-5.4"
 
-# Shell skills (disabled by default)
-# export SHELL_SKILLS_ENABLED=1
+# Local shell (disabled by default)
+# export SHELL_ENABLED=1
+# export SHELL_SKILLS_DIR="./skills"  # optional; mount skills alongside the shell
 ```
 
 If you are using Azure OpenAI (v1 API) or another OpenAI-compatible endpoint:
@@ -61,6 +62,32 @@ Optional HTTP proxy for outbound requests:
 export HTTP_PROXY=""
 ```
 
+Optional verbose OpenAI Agents SDK logging:
+
+```
+export AGENT_VERBOSE_LOG=1
+```
+
+`AGENT_VERBOSE_LOG` is defined by this project: when set to a truthy value, the app
+calls the SDK's `enable_verbose_stdout_logging()` helper during startup. Values like
+`0`, `false`, `no`, `off` (case-insensitive) are treated as disabled.
+
+The following are environment variables read directly by the OpenAI Agents SDK (not
+this project). By default, the SDK does not log model or tool payloads. To include
+them temporarily for debugging:
+
+```
+export OPENAI_AGENTS_DONT_LOG_MODEL_DATA=0
+export OPENAI_AGENTS_DONT_LOG_TOOL_DATA=0
+```
+
+These payload logs may contain sensitive data. Re-enable the defaults after debugging:
+
+```
+export OPENAI_AGENTS_DONT_LOG_MODEL_DATA=1
+export OPENAI_AGENTS_DONT_LOG_TOOL_DATA=1
+```
+
 ## Agent Instructions
 
 Create an `instructions.md` file in the project root with the agent system prompt:
@@ -122,23 +149,34 @@ For local MCP servers, use `uv --directory`:
 uv run bot
 ```
 
-## Shell Skills (Optional)
+## Local Shell (Optional)
 
-The bot can execute local shell commands via skills defined in a `skills/` directory. Each subdirectory containing a `SKILL.md` file is registered as a skill.
+The bot can expose a local `ShellTool`. This is **disabled by default**. Enable it with:
 
-When using the Docker image, mount `skills/` at runtime (the image build excludes this directory by default):
-
-```bash
--v /path/to/skills:/app/skills:ro
 ```
+export SHELL_ENABLED=1
+```
+
+With just `SHELL_ENABLED=1`, the agent gets bare local shell access with no pre-defined skills.
+
+### Shell Skills (Optional)
 
-This feature is **disabled by default**. To enable it, set:
+You can optionally mount a skills directory alongside the shell. Each immediate subdirectory containing a `SKILL.md` file is registered as a skill and exposed to the agent as a hint (skills are advisory metadata — they do **not** sandbox command execution).
 
 ```
-SHELL_SKILLS_ENABLED=1
+export SHELL_ENABLED=1
+export SHELL_SKILLS_DIR="./skills"
+```
+
+`SHELL_SKILLS_DIR` is ignored unless `SHELL_ENABLED` is set. If the directory is missing or contains no valid skills, the bot falls back to a bare shell and logs a warning.
+
+When using the Docker image, mount your skills directory at runtime (the image build excludes it by default):
+
+```bash
+-v /path/to/skills:/app/skills:ro
 ```
 
-Skills are auto-discovered at startup. The `SKILL.md` file should have YAML frontmatter with `name` and `description` fields:
+The `SKILL.md` file should have YAML frontmatter with `name` and `description` fields:
 
 ```markdown
 ---
@@ -160,7 +198,8 @@ docker run -d \
   -e SLACK_APP_TOKEN="" \
   -e OPENAI_API_KEY="" \
   -e OPENAI_MODEL="gpt-5.4" \
-  -e SHELL_SKILLS_ENABLED=1 \
+  -e SHELL_ENABLED=1 \
+  -e SHELL_SKILLS_DIR=/app/skills \
   -v /path/to/instructions.md:/app/instructions.md \
   -v /path/to/skills:/app/skills:ro \
   agentic-slackbot
@@ -175,7 +214,8 @@ docker run -d \
   -e SLACK_APP_TOKEN="" \
   -e OPENAI_API_KEY="" \
   -e OPENAI_MODEL="gpt-5.4" \
-  -e SHELL_SKILLS_ENABLED=1 \
+  -e SHELL_ENABLED=1 \
+  -e SHELL_SKILLS_DIR=/app/skills \
   -v /path/to/instructions.md:/app/instructions.md \
   -v /path/to/skills:/app/skills:ro \
   -v /path/to/servers_config.json:/app/servers_config.json \

bot/agent.py

@@ -20,12 +20,13 @@
 from agents.tracing import set_tracing_disabled
 from openai import AsyncOpenAI
 
+from .config import env_flag
+
 INSTRUCTIONS_FILE = Path("instructions.md")
 
 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)
 
@@ -80,17 +81,17 @@ def _parse_skill_description(content: str) -> str:
     return ""
 
 
-def _load_shell_skills() -> list[ShellToolLocalSkill]:
-    """Discover local shell skills under SKILLS_DIR.
+def _load_shell_skills(skills_dir: Path) -> 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.
+    Each immediate subdirectory containing a ``SKILL.md`` file 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():
+    if not skills_dir.is_dir():
         return []
     skills: list[ShellToolLocalSkill] = []
-    for skill_dir in sorted(SKILLS_DIR.iterdir()):
+    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
@@ -109,6 +110,37 @@ def _load_shell_skills() -> list[ShellToolLocalSkill]:
     return skills
 
 
+def _get_shell_environment() -> ShellToolLocalEnvironment | None:
+    """Return the configured local shell environment, if enabled.
+
+    Controlled by two independent environment variables:
+
+    * ``SHELL_ENABLED`` — when truthy, the ``ShellTool`` is attached to the agent.
+    * ``SHELL_SKILLS_DIR`` — optional path; when set, skills discovered under it
+      are mounted alongside the shell. Ignored if ``SHELL_ENABLED`` is not set.
+    """
+    skills_dir_env = os.getenv("SHELL_SKILLS_DIR")
+    if not env_flag("SHELL_ENABLED"):
+        if skills_dir_env:
+            logging.warning(
+                "SHELL_SKILLS_DIR=%r is set but SHELL_ENABLED is not; ignoring skills dir.",
+                skills_dir_env,
+            )
+        return None
+
+    environment: ShellToolLocalEnvironment = {"type": "local"}
+    if skills_dir_env:
+        skills = _load_shell_skills(Path(skills_dir_env))
+        if skills:
+            environment["skills"] = skills
+        else:
+            logging.warning(
+                "SHELL_SKILLS_DIR=%r yielded no skills; attaching bare local shell.",
+                skills_dir_env,
+            )
+    return environment
+
+
 async def _shell_executor(request: ShellCommandRequest) -> str:
     """Run each shell command from the request and return combined output.
 
@@ -215,11 +247,9 @@ 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))
+        environment = _get_shell_environment()
+        if environment is not None:
+            tools.append(ShellTool(executor=_shell_executor, environment=environment))
 
         instructions = _load_instructions()
         return cls(name, instructions=instructions, mcp_servers=mcp_servers, tools=tools)

bot/app.py

@@ -1,16 +1,25 @@
 import asyncio
 import logging
 
+from agents import enable_verbose_stdout_logging
+
 from .agent import OpenAIAgent
 from .config import Configuration
+from .config import env_flag
 from .slack import SlackMCPBot
 
 
-async def main() -> None:
+def _configure_logging() -> None:
     logging.basicConfig(
         format="%(asctime)s - %(name)s - %(levelname)s - %(message)s",
         level=logging.INFO,
     )
+    if env_flag("AGENT_VERBOSE_LOG"):
+        enable_verbose_stdout_logging()
+
+
+async def main() -> None:
+    _configure_logging()
 
     config = Configuration()
     server_config = config.load_config("servers_config.json")

bot/config.py

@@ -8,6 +8,21 @@
 
 logger = logging.getLogger(__name__)
 
+_FALSY_ENV_VALUES = frozenset({"", "0", "false", "no", "off"})
+
+
+def env_flag(name: str) -> bool:
+    """Return True if env var ``name`` is set to a truthy value.
+
+    Common falsy spellings (empty, "0", "false", "no", "off") are treated as
+    disabled so that ``FOO=0`` behaves as users intuitively expect rather than
+    as Python's default "non-empty string is truthy" rule.
+    """
+    raw = os.getenv(name)
+    if raw is None:
+        return False
+    return raw.strip().lower() not in _FALSY_ENV_VALUES
+
 
 class Configuration:
     """Manages configuration and environment variables for the MCP Slackbot."""