Merge remote-tracking branch 'shareai/main' into fix/pr510-subprocess-output

# Conflicts:
#	s07_skill_loading/code.py

s07_skill_loading/code.py

@@ -1,38 +1,36 @@
 #!/usr/bin/env python3
 """
-s07: Skill Loading — two-level on-demand knowledge injection.
+s07_skill_loading.py - Skill Loading
 
-  Layer 1 (cheap, always present):
-    SYSTEM prompt includes skill names + one-line descriptions (~100 tokens/skill)
-    "Skills available: agent-builder, code-review, mcp-builder, pdf"
+The system prompt contains a catalog of skill names and descriptions.
+The model loads the full SKILL.md only when it calls load_skill.
 
-  Layer 2 (expensive, on demand):
-    Agent calls load_skill("code-review") → full SKILL.md content
-    injected via tool_result (~2000 tokens/skill)
+    skills/                    Startup
+    +------------------+       +------------------+
+    | code-review/     | ----> | SkillLoader      |
+    |   SKILL.md       |       | name + summary   |
+    | pdf/             |       +--------+---------+
+    |   SKILL.md       |                |
+    +------------------+                v
+                                 system prompt catalog
 
-  skills/
-    agent-builder/SKILL.md
-    code-review/SKILL.md
-    mcp-builder/SKILL.md
-    pdf/SKILL.md
-
-Changes from s06:
-  + build_system() — scan skills/ dir at startup, inject catalog into SYSTEM
-  + load_skill(name) — return full SKILL.md content via tool_result
-  + SKILLS_DIR config
-  Loop unchanged: load_skill auto-dispatches via TOOL_HANDLERS.
-
-Run: python s07_skill_loading/code.py
-Needs: pip install anthropic python-dotenv pyyaml + ANTHROPIC_API_KEY in .env
+    LLM -- load_skill(name) --> full SKILL.md
+     ^                              |
+     +--------- tool_result --------+
 """
 
-import ast, json, os, subprocess
+import os
+import subprocess
 from pathlib import Path
+
 import yaml
 
 try:
     import readline
     readline.parse_and_bind('set bind-tty-special-chars off')
+    readline.parse_and_bind('set input-meta on')
+    readline.parse_and_bind('set output-meta on')
+    readline.parse_and_bind('set convert-meta off')
 except ImportError:
     pass
 
@@ -47,77 +45,98 @@ WORKDIR = Path.cwd()
 SKILLS_DIR = WORKDIR / "skills"
 client = Anthropic(base_url=os.getenv("ANTHROPIC_BASE_URL"))
 MODEL = os.environ["MODEL_ID"]
-CURRENT_TODOS: list[dict] = []
 
-# s07: Skill catalog scan (used by build_system below)
-def _parse_frontmatter(text: str) -> tuple[dict, str]:
-    """Parse YAML frontmatter from SKILL.md. Returns (meta, body)."""
-    if not text.startswith("---"):
-        return {}, text
-    parts = text.split("---", 2)
-    if len(parts) < 3:
-        return {}, text
-    try:
-        meta = yaml.safe_load(parts[1]) or {}
-    except yaml.YAMLError:
-        meta = {}
-    return meta, parts[2].strip()
 
-# Build skill registry at startup (used for safe lookup in load_skill)
-SKILL_REGISTRY: dict[str, dict] = {}
+# -- Skill catalog --
 
-def _scan_skills():
-    """Scan skills/ dir, populate SKILL_REGISTRY with name/description/content."""
-    if not SKILLS_DIR.exists():
-        return
-    for d in sorted(SKILLS_DIR.iterdir()):
-        if not d.is_dir():
-            continue
-        manifest = d / "SKILL.md"
-        if manifest.exists():
-            raw = manifest.read_text()
-            meta, body = _parse_frontmatter(raw)
-            name = meta.get("name", d.name)
-            desc = meta.get("description", raw.split("\n")[0].lstrip("#").strip())
-            SKILL_REGISTRY[name] = {"name": name, "description": desc, "content": raw}
+class SkillLoader:
+    def __init__(self, skills_dir: Path):
+        self.skills_dir = skills_dir
+        self.skills: dict[str, dict[str, str]] = {}
+        self.scan()
 
-_scan_skills()
+    @staticmethod
+    def parse_frontmatter(text: str) -> tuple[dict, str]:
+        lines = text.splitlines(keepends=True)
+        if not lines or lines[0].rstrip("\r\n") != "---":
+            return {}, text
 
-def list_skills() -> str:
-    """List all skills (name + one-line description)."""
-    if not SKILL_REGISTRY:
-        return "(no skills found)"
-    return "\n".join(f"- **{s['name']}**: {s['description']}" for s in SKILL_REGISTRY.values())
+        closing_index = next(
+            (index for index, line in enumerate(lines[1:], start=1)
+             if line.rstrip("\r\n") == "---"),
+            None,
+        )
+        if closing_index is None:
+            return {}, text
 
-# s07: SYSTEM includes skill catalog (cheap — just names + descriptions)
-def build_system() -> str:
-    """Build SYSTEM prompt with skill catalog injected at startup."""
-    catalog = list_skills()
+        frontmatter = "".join(lines[1:closing_index])
+        body = "".join(lines[closing_index + 1:]).strip()
+        try:
+            metadata = yaml.safe_load(frontmatter) or {}
+        except yaml.YAMLError:
+            metadata = {}
+        if not isinstance(metadata, dict):
+            metadata = {}
+        return metadata, body
+
+    def scan(self):
+        self.skills.clear()
+        if not self.skills_dir.exists():
+            return
+
+        skills_root = self.skills_dir.resolve()
+        for manifest in sorted(self.skills_dir.glob("*/SKILL.md")):
+            if (not manifest.is_file()
+                    or not manifest.resolve().is_relative_to(skills_root)):
+                continue
+            content = manifest.read_text(encoding="utf-8")
+            metadata, body = self.parse_frontmatter(content)
+            raw_name = metadata.get("name")
+            name = raw_name.strip() if isinstance(raw_name, str) else ""
+            name = name or manifest.parent.name
+            raw_description = metadata.get("description")
+            description = (raw_description.strip()
+                           if isinstance(raw_description, str) else "")
+            description = description or body.split("\n", 1)[0]
+            description = " ".join(str(description).lstrip("# ").split())
+            self.skills[name] = {
+                "name": name,
+                "description": description,
+                "content": content,
+            }
+
+    def catalog(self) -> str:
+        if not self.skills:
+            return "(no skills found)"
+        return "\n".join(
+            f"- {skill['name']}: {skill['description']}"
+            for skill in self.skills.values()
+        )
+
+    def load(self, name: str) -> str:
+        skill = self.skills.get(name)
+        if skill:
+            return skill["content"]
+        available = ", ".join(self.skills) or "none"
+        return f"Error: Unknown skill '{name}'. Available: {available}"
+
+
+SKILL_LOADER = SkillLoader(SKILLS_DIR)
+
+
+def build_system_prompt() -> str:
     return (
-        f"You are a coding agent at {WORKDIR}. "
-        f"Skills available:\n{catalog}\n"
-        "Use load_skill to get full details when needed."
+        f"You are a coding agent at {WORKDIR}. Use tools to solve tasks. "
+        "Act, don't explain.\n\n"
+        f"Skills available:\n{SKILL_LOADER.catalog()}\n\n"
+        "Use load_skill to read the full instructions when a skill applies."
     )
 
-SYSTEM = build_system()
 
-# s07: subagent gets its own system prompt — no skill loading, no task
-SUB_SYSTEM = (
-    f"You are a coding agent at {WORKDIR}. "
-    "Complete the task you were given, then return a concise summary. "
-    "Do not delegate further."
-)
+SYSTEM = build_system_prompt()
 
 
-# ═══════════════════════════════════════════════════════════
-#  FROM s02-s06 (unchanged): Tool Implementations
-# ═══════════════════════════════════════════════════════════
-
-def safe_path(p: str) -> Path:
-    path = (WORKDIR / p).resolve()
-    if not path.is_relative_to(WORKDIR):
-        raise ValueError(f"Path escapes workspace: {p}")
-    return path
+# -- Tools --
 
 # s07: Fix Windows subprocess encoding crash (fallback UTF-8 -> GBK with error replacement)
 def safe_decode(data: bytes) -> str:
@@ -140,155 +159,54 @@ def run_bash(command: str) -> str:
     except subprocess.TimeoutExpired:
         return "Error: Timeout (120s)"
 
+
 def run_read(path: str, limit: int | None = None) -> str:
     try:
-        lines = safe_path(path).read_text().splitlines()
+        lines = (WORKDIR / path).resolve().read_text(encoding="utf-8").splitlines()
         if limit and limit < len(lines):
             lines = lines[:limit] + [f"... ({len(lines) - limit} more lines)"]
         return "\n".join(lines)
     except Exception as e:
         return f"Error: {e}"
 
+
 def run_write(path: str, content: str) -> str:
     try:
-        file_path = safe_path(path)
+        file_path = (WORKDIR / path).resolve()
         file_path.parent.mkdir(parents=True, exist_ok=True)
-        file_path.write_text(content)
+        file_path.write_text(content, encoding="utf-8")
         return f"Wrote {len(content)} bytes to {path}"
     except Exception as e:
         return f"Error: {e}"
 
+
 def run_edit(path: str, old_text: str, new_text: str) -> str:
     try:
-        file_path = safe_path(path)
-        text = file_path.read_text()
+        file_path = (WORKDIR / path).resolve()
+        text = file_path.read_text(encoding="utf-8")
         if old_text not in text:
             return f"Error: text not found in {path}"
-        file_path.write_text(text.replace(old_text, new_text, 1))
+        file_path.write_text(text.replace(old_text, new_text, 1), encoding="utf-8")
         return f"Edited {path}"
     except Exception as e:
         return f"Error: {e}"
 
+
 def run_glob(pattern: str) -> str:
-    import glob as g
+    import glob
     try:
-        results = []
-        for match in g.glob(pattern, root_dir=WORKDIR):
-            if (WORKDIR / match).resolve().is_relative_to(WORKDIR):
-                results.append(match)
-        return "\n".join(results) if results else "(no matches)"
+        matches = sorted({
+            match for match in glob.glob(
+                pattern, root_dir=WORKDIR, recursive=True)
+            if (WORKDIR / match).resolve().is_relative_to(WORKDIR)
+        })
+        shown = matches[:200]
+        if len(matches) > 200:
+            shown.append("... (more matches omitted; narrow the pattern)")
+        return "\n".join(shown) if shown else "(no matches)"
     except Exception as e:
         return f"Error: {e}"
 
-def _normalize_todos(todos):
-    if isinstance(todos, str):
-        try:
-            todos = json.loads(todos)
-        except json.JSONDecodeError:
-            try:
-                todos = ast.literal_eval(todos)
-            except (SyntaxError, ValueError):
-                return None, "Error: todos must be a list or JSON array string"
-    if not isinstance(todos, list):
-        return None, "Error: todos must be a list"
-    for i, t in enumerate(todos):
-        if not isinstance(t, dict):
-            return None, f"Error: todos[{i}] must be an object"
-        if "content" not in t or "status" not in t:
-            return None, f"Error: todos[{i}] missing 'content' or 'status'"
-        if t["status"] not in ("pending", "in_progress", "completed"):
-            return None, f"Error: todos[{i}] has invalid status '{t['status']}'"
-    return todos, None
-
-def run_todo_write(todos: list) -> str:
-    global CURRENT_TODOS
-    todos, error = _normalize_todos(todos)
-    if error:
-        return error
-    CURRENT_TODOS = todos
-    lines = ["\n\033[33m## Current Tasks\033[0m"]
-    for t in CURRENT_TODOS:
-        icon = {"pending": " ", "in_progress": "\033[36m▸\033[0m", "completed": "\033[32m✓\033[0m"}[t["status"]]
-        lines.append(f"  [{icon}] {t['content']}")
-    print("\n".join(lines))
-    return f"Updated {len(CURRENT_TODOS)} tasks"
-
-def extract_text(content) -> str:
-    if not isinstance(content, list):
-        return str(content)
-    return "\n".join(getattr(b, "text", "") for b in content if getattr(b, "type", None) == "text")
-
-
-# ═══════════════════════════════════════════════════════════
-#  FROM s06 (unchanged): Subagent
-# ═══════════════════════════════════════════════════════════
-
-SUB_TOOLS = [
-    {"name": "bash", "description": "Run a shell command.",
-     "input_schema": {"type": "object", "properties": {"command": {"type": "string"}}, "required": ["command"]}},
-    {"name": "read_file", "description": "Read file contents.",
-     "input_schema": {"type": "object", "properties": {"path": {"type": "string"}}, "required": ["path"]}},
-    {"name": "write_file", "description": "Write content to a file.",
-     "input_schema": {"type": "object", "properties": {"path": {"type": "string"}, "content": {"type": "string"}}, "required": ["path", "content"]}},
-    {"name": "edit_file", "description": "Replace exact text in a file once.",
-     "input_schema": {"type": "object", "properties": {"path": {"type": "string"}, "old_text": {"type": "string"}, "new_text": {"type": "string"}}, "required": ["path", "old_text", "new_text"]}},
-    {"name": "glob", "description": "Find files matching a glob pattern.",
-     "input_schema": {"type": "object", "properties": {"pattern": {"type": "string"}}, "required": ["pattern"]}},
-]
-SUB_HANDLERS = {"bash": run_bash, "read_file": run_read, "write_file": run_write,
-                "edit_file": run_edit, "glob": run_glob}
-
-def spawn_subagent(description: str) -> str:
-    print(f"\n\033[35m[Subagent spawned]\033[0m")
-    messages = [{"role": "user", "content": description}]
-    for _ in range(30):
-        response = client.messages.create(model=MODEL, system=SUB_SYSTEM,
-            messages=messages, tools=SUB_TOOLS, max_tokens=8000)
-        messages.append({"role": "assistant", "content": response.content})
-        if response.stop_reason != "tool_use":
-            break
-        results = []
-        for block in response.content:
-            if block.type == "tool_use":
-                blocked = trigger_hooks("PreToolUse", block)
-                if blocked:
-                    results.append({"type": "tool_result", "tool_use_id": block.id,
-                                    "content": str(blocked)})
-                    continue
-                handler = SUB_HANDLERS.get(block.name)
-                output = handler(**block.input) if handler else f"Unknown: {block.name}"
-                trigger_hooks("PostToolUse", block, output)
-                print(f"  \033[90m[sub] {block.name}: {str(output)[:100]}\033[0m")
-                results.append({"type": "tool_result", "tool_use_id": block.id, "content": output})
-        messages.append({"role": "user", "content": results})
-    result = extract_text(messages[-1]["content"])
-    if not result:
-        for msg in reversed(messages):
-            if msg["role"] == "assistant":
-                result = extract_text(msg["content"])
-                if result:
-                    break
-        if not result:
-            result = "Subagent stopped after 30 turns without final answer."
-    print(f"\033[35m[Subagent done]\033[0m")
-    return result
-
-
-# ═══════════════════════════════════════════════════════════
-#  NEW in s07: load_skill — runtime full content loading
-# ═══════════════════════════════════════════════════════════
-
-def load_skill(name: str) -> str:
-    """Load full skill content. Lookup via registry — no path traversal."""
-    skill = SKILL_REGISTRY.get(name)
-    if not skill:
-        return f"Skill not found: {name}"
-    return skill["content"]
-
-
-# ═══════════════════════════════════════════════════════════
-#  Tool Registry — all tools from s02-s07
-# ═══════════════════════════════════════════════════════════
 
 TOOLS = [
     {"name": "bash", "description": "Run a shell command.",
@@ -299,33 +217,31 @@ TOOLS = [
      "input_schema": {"type": "object", "properties": {"path": {"type": "string"}, "content": {"type": "string"}}, "required": ["path", "content"]}},
     {"name": "edit_file", "description": "Replace exact text in a file once.",
      "input_schema": {"type": "object", "properties": {"path": {"type": "string"}, "old_text": {"type": "string"}, "new_text": {"type": "string"}}, "required": ["path", "old_text", "new_text"]}},
-    {"name": "glob", "description": "Find files matching a glob pattern.",
+    {"name": "glob", "description": "Find files matching a glob pattern; ** matches recursively.",
      "input_schema": {"type": "object", "properties": {"pattern": {"type": "string"}}, "required": ["pattern"]}},
-    {"name": "todo_write", "description": "Create and manage a task list for your current coding session.",
-     "input_schema": {"type": "object", "properties": {"todos": {"type": "array", "items": {"type": "object", "properties": {"content": {"type": "string"}, "status": {"type": "string", "enum": ["pending", "in_progress", "completed"]}}, "required": ["content", "status"]}}}, "required": ["todos"]}},
-    {"name": "task", "description": "Launch a subagent to handle a complex subtask. Returns only the final conclusion.",
-     "input_schema": {"type": "object", "properties": {"description": {"type": "string"}}, "required": ["description"]}},
-    # s07: skill tool (catalog is already in SYSTEM prompt, this loads full content)
-    {"name": "load_skill", "description": "Load the full content of a skill by name.",
+    {"name": "load_skill", "description": "Load the full SKILL.md content by skill name.",
      "input_schema": {"type": "object", "properties": {"name": {"type": "string"}}, "required": ["name"]}},
 ]
 
 TOOL_HANDLERS = {
-    "bash": run_bash, "read_file": run_read, "write_file": run_write,
-    "edit_file": run_edit, "glob": run_glob, "todo_write": run_todo_write,
-    "task": spawn_subagent, "load_skill": load_skill,
+    "bash": run_bash,
+    "read_file": run_read,
+    "write_file": run_write,
+    "edit_file": run_edit,
+    "glob": run_glob,
+    "load_skill": SKILL_LOADER.load,
 }
 
 
-# ═══════════════════════════════════════════════════════════
-#  FROM s04 (unchanged): Hook System
-# ═══════════════════════════════════════════════════════════
+# -- Hooks --
 
 HOOKS = {"UserPromptSubmit": [], "PreToolUse": [], "PostToolUse": [], "Stop": []}
 
+
 def register_hook(event: str, callback):
     HOOKS[event].append(callback)
 
+
 def trigger_hooks(event: str, *args):
     for callback in HOOKS[event]:
         result = callback(*args)
@@ -333,96 +249,137 @@ def trigger_hooks(event: str, *args):
             return result
     return None
 
+
 DENY_LIST = ["rm -rf /", "sudo", "shutdown", "reboot", "mkfs", "dd if="]
+DESTRUCTIVE = ["rm ", "> /etc/", "chmod 777"]
+
 
 def permission_hook(block):
+    """PreToolUse: block denied operations and ask about risky ones."""
     if block.name == "bash":
-        for p in DENY_LIST:
-            if p in block.input.get("command", ""):
-                print(f"\n\033[31m⛔ Blocked: '{p}'\033[0m")
-                return "Permission denied"
+        command = block.input.get("command", "")
+        for pattern in DENY_LIST:
+            if pattern in command:
+                print(f"\n\033[31m[blocked] '{pattern}'\033[0m")
+                return "Permission denied by deny list"
+        for keyword in DESTRUCTIVE:
+            if keyword in command:
+                print("\n\033[33m[permission] Potentially destructive command\033[0m")
+                print(f"   Tool: {block.name}({block.input})")
+                choice = input("   Allow? [y/N] ").strip().lower()
+                if choice not in ("y", "yes"):
+                    return "Permission denied by user"
+
+    if block.name in ("read_file", "write_file", "edit_file"):
+        path = block.input.get("path", "")
+        if not (WORKDIR / path).resolve().is_relative_to(WORKDIR):
+            print("\n\033[33m[permission] Access outside workspace\033[0m")
+            print(f"   Tool: {block.name}({block.input})")
+            choice = input("   Allow? [y/N] ").strip().lower()
+            if choice not in ("y", "yes"):
+                return "Permission denied by user"
     return None
 
+
 def log_hook(block):
-    print(f"\033[90m[HOOK] {block.name}\033[0m")
+    """PreToolUse: log every tool call."""
+    args_preview = str(list(block.input.values())[:2])[:60]
+    print(f"\033[90m[HOOK] {block.name}({args_preview})\033[0m")
     return None
 
+
+def large_output_hook(block, output):
+    """PostToolUse: warn on large output."""
+    if len(str(output)) > 100000:
+        print(f"\033[33m[HOOK] Large output from {block.name}: {len(str(output))} chars\033[0m")
+    return None
+
+
 def context_inject_hook(query: str):
+    """UserPromptSubmit: log the working directory."""
     print(f"\033[90m[HOOK] UserPromptSubmit: working in {WORKDIR}\033[0m")
     return None
 
+
 def summary_hook(messages: list):
-    tool_count = sum(1 for m in messages
-                     for b in (m.get("content") if isinstance(m.get("content"), list) else [])
-                     if isinstance(b, dict) and b.get("type") == "tool_result")
+    """Stop: print the number of tool results in this message list."""
+    tool_count = sum(
+        1
+        for message in messages
+        for block in (
+            message.get("content")
+            if isinstance(message.get("content"), list)
+            else []
+        )
+        if isinstance(block, dict) and block.get("type") == "tool_result"
+    )
     print(f"\033[90m[HOOK] Stop: session used {tool_count} tool calls\033[0m")
     return None
 
+
 register_hook("UserPromptSubmit", context_inject_hook)
 register_hook("PreToolUse", permission_hook)
 register_hook("PreToolUse", log_hook)
+register_hook("PostToolUse", large_output_hook)
 register_hook("Stop", summary_hook)
 
 
-# ═══════════════════════════════════════════════════════════
-#  agent_loop — same as s05-s06 + nag reminder
-# ═══════════════════════════════════════════════════════════
+def execute_tool(block) -> str:
+    blocked = trigger_hooks("PreToolUse", block)
+    if blocked:
+        return str(blocked)
+
+    handler = TOOL_HANDLERS.get(block.name)
+    try:
+        output = handler(**block.input) if handler else f"Unknown: {block.name}"
+    except Exception as e:
+        output = f"Error: {e}"
+
+    trigger_hooks("PostToolUse", block, output)
+    return str(output)
+
 
 def agent_loop(messages: list):
-    rounds_since_todo = 0
     while True:
-        if rounds_since_todo >= 3 and messages:
-            messages.append({"role": "user",
-                             "content": "<reminder>Update your todos.</reminder>"})
-            rounds_since_todo = 0
-            
         response = client.messages.create(
-            model=MODEL, system=SYSTEM, messages=messages,
-            tools=TOOLS, max_tokens=8000,
+            model=MODEL,
+            system=SYSTEM,
+            messages=messages,
+            tools=TOOLS,
+            max_tokens=8000,
         )
         messages.append({"role": "assistant", "content": response.content})
 
-        if response.stop_reason != "tool_use":
+        tool_calls = [
+            block for block in response.content if block.type == "tool_use"
+        ]
+        if not tool_calls:
             force = trigger_hooks("Stop", messages)
             if force:
                 messages.append({"role": "user", "content": force})
                 continue
             return
 
-        rounds_since_todo += 1
         results = []
-        for block in response.content:
-            if block.type != "tool_use":
-                continue
-
-            blocked = trigger_hooks("PreToolUse", block)
-            if blocked:
-                results.append({"type": "tool_result", "tool_use_id": block.id,
-                                "content": str(blocked)})
-                continue
-
-            handler = TOOL_HANDLERS.get(block.name)
-            output = handler(**block.input) if handler else f"Unknown: {block.name}"
-
-            trigger_hooks("PostToolUse", block, output)
-
-            if block.name == "todo_write":
-                rounds_since_todo = 0
-
-            results.append({"type": "tool_result", "tool_use_id": block.id,
-                            "content": output})
-
+        for block in tool_calls:
+            output = execute_tool(block)
+            results.append({
+                "type": "tool_result",
+                "tool_use_id": block.id,
+                "content": output,
+            })
         messages.append({"role": "user", "content": results})
 
 
 if __name__ == "__main__":
-    print("s07: Skill Loading — catalog in SYSTEM, content on demand")
-    print("Type a question, press Enter. Type q to quit.\n")
+    print("s07: Skill Loading - catalog first, full content on demand")
+    print("Enter a question, press Enter to send. Type q to quit.\n")
 
     history = []
     while True:
         try:
-            query = input("\033[36ms07 >> \033[0m")
+            # \001/\002 tell Readline the ANSI escapes have zero display width.
+            query = input("\001\033[36m\002s07 >> \001\033[0m\002")
         except (EOFError, KeyboardInterrupt):
             break
         if query.strip().lower() in ("q", "exit", ""):