diff --git a/.gitignore b/.gitignore index f148045..38ade80 100644 --- a/.gitignore +++ b/.gitignore @@ -4,5 +4,6 @@ __pycache__/ logs/ runs/ tasks/ +skill-candidates/ config/telegram_state.json config/imessage_state.json diff --git a/hermes.py b/hermes.py index a88233e..78cd9d7 100644 --- a/hermes.py +++ b/hermes.py @@ -6,6 +6,7 @@ import subprocess import logging +import re import time import threading import yaml @@ -45,6 +46,33 @@ def load_config(): return yaml.safe_load(f) +def _ollama_generate(prompt: str, timeout: int = 120, model: str = "llama3.1:8b") -> str: + """POST a prompt to the local Ollama server; return the response text ("" on any + failure). Shared by the resume-handoff and post-task-review writers — both distill + text locally so they cost zero Claude tokens.""" + try: + import json as _json + import urllib.request as _urllib + + payload = _json.dumps({"model": model, "prompt": prompt, "stream": False}).encode() + req = _urllib.Request( + "http://localhost:11434/api/generate", + data=payload, + headers={"Content-Type": "application/json"}, + method="POST", + ) + with _urllib.urlopen(req, timeout=timeout) as resp: + return _json.loads(resp.read().decode()).get("response", "").strip() + except TimeoutError: + log.error(f"Ollama generate timed out after {timeout}s") + return "" + except (OSError, ValueError) as exc: + # OSError covers connect-refused AND HTTPError (e.g. model not pulled); + # ValueError covers a non-JSON body. Honor the "" -on-any-failure contract. + log.error(f"Ollama generate failed: {exc}") + return "" + + def write_handoff_via_ollama( task: str, session_name: str, @@ -86,24 +114,12 @@ def write_handoff_via_ollama( Be specific and factual. Only include what is supported by the partial output above.""" - try: - import json as _json - import urllib.request as _urllib - - payload = _json.dumps({"model": "llama3.1:8b", "prompt": prompt, "stream": False}).encode() - req = _urllib.Request( - "http://localhost:11434/api/generate", - data=payload, - headers={"Content-Type": "application/json"}, - method="POST", - ) - with _urllib.urlopen(req, timeout=120) as resp: - response_text = _json.loads(resp.read().decode()).get("response", "").strip() - - if not response_text: - log.error("Ollama handoff generation returned empty response") - return "" + response_text = _ollama_generate(prompt, timeout=120) + if not response_text: + log.error("Ollama handoff generation returned empty response") + return "" + try: handoff_dir = Path.home() / "Documents" / "RaphBrain" / "Projects" / "handoffs" handoff_dir.mkdir(parents=True, exist_ok=True) handoff_path = handoff_dir / f"{session_name}-handoff.md" @@ -116,11 +132,8 @@ def write_handoff_via_ollama( ) log.info(f"Handoff written: {handoff_path}") return str(handoff_path) - except TimeoutError: - log.error("Ollama handoff timed out after 120s") - return "" - except OSError: - log.error("Ollama not found — is it installed?") + except OSError as exc: + log.error(f"Failed to write handoff file: {exc}") return "" @@ -292,6 +305,202 @@ def _append_pipeline_to_daily_note( log.warning(f"[daily-note] Failed to write to daily note: {exc}") +RAPHBRAIN_PROJECTS_DIR = Path.home() / "Documents" / "RaphBrain" / "Projects" +HERMES_RUN_LOG_HEADER = "## Hermes Run Log" +# Auto-drafted skills land here for review — NEVER written straight into +# ~/.claude/skills, which is globally active in every Claude Code session. Jaiden +# promotes a candidate by hand once he's read it. +SKILL_CANDIDATES_DIR = Path.home() / "hermes" / "skill-candidates" + + +_PROGRESS_LOCK = threading.Lock() +_RUN_LOG_HEADER_RE = re.compile(r"(?m)^" + re.escape(HERMES_RUN_LOG_HEADER) + r"$") + + +def _prepend_under_runlog(project: str, entry: str) -> None: + """Insert `entry` newest-first under the '## Hermes Run Log' header in the + project's Progress.md, creating the file + section if missing. Shared by the + deterministic run-note and the post-task learnings writer. + + Held under a lock so the daemon's two writer threads (a slow post-task-review + thread and the next run's deterministic write) can't clobber each other's + read-modify-write. Cross-PROCESS writers (a `claude -p` sub-agent, Obsidian sync) + are out of scope — different sections, infrequent — and not synchronized here.""" + # Registry is the source of truth for the vault folder — capitalize() misroutes + # hyphenated projects (finance-tracker → a Finance-tracker orphan nothing reads). + proj = resolve_project(project) + dir_name = (proj.raphbrain_dir if proj else None) or project.capitalize() + progress_path = RAPHBRAIN_PROJECTS_DIR / dir_name / "Progress.md" + with _PROGRESS_LOCK: + if progress_path.exists(): + content = progress_path.read_text(encoding="utf-8") + m = _RUN_LOG_HEADER_RE.search(content) # line-anchored — not any prose match + if m: + end = m.end() + if content[end : end + 1] == "\n": + content = content[: end + 1] + entry + "\n" + content[end + 1 :] + else: + # Header is the file's last line with no trailing newline — insert + # one so it stays on its own line. Without this the entry glues on + # ("## Hermes Run Log### …") and the next run can't match the header, + # appending a SECOND section and breaking the single-section rule. + content = content[:end] + "\n" + entry + "\n" + content[end:] + else: + content = content.rstrip() + f"\n\n{HERMES_RUN_LOG_HEADER}\n{entry}" + progress_path.write_text(content, encoding="utf-8") + else: + progress_path.parent.mkdir(parents=True, exist_ok=True) + progress_path.write_text( + f"# {dir_name} — Progress\n\n{HERMES_RUN_LOG_HEADER}\n{entry}", encoding="utf-8" + ) + + +def _append_pipeline_to_progress_note( + project: str, + branch: str, + final_status: str, + duration: str, + pr_url: str = "", + failed_step: str = "", + reason: str = "", + done_count: int = 0, + step_count: int = 0, +) -> None: + """Deterministically record a run's outcome in the project's Progress.md. + + Runs in Python on the completion path, so 'done' GUARANTEES a note exists — it + never depends on a sub-agent remembering to write one. Entries go under a + dedicated '## Hermes Run Log' section so human-curated sections are untouched.""" + try: + stamp = datetime.now().strftime("%Y-%m-%d %H:%M") + emoji = {"done": "✅", "failed": "❌", "aborted": "🛑"}.get(final_status, "🏁") + lines = [f"### {stamp} — {emoji} {project}/{branch or '(no branch)'} — {final_status}"] + lines.append( + f"- {done_count}/{step_count} steps · {duration}" if step_count else f"- {duration}" + ) + if pr_url: + lines.append(f"- PR: {pr_url}") + if final_status == "failed" and failed_step: + lines.append(f"- Failed at `{failed_step}`" + (f" — {reason}" if reason else "")) + _prepend_under_runlog(project, "\n".join(lines) + "\n") + log.info(f"[progress-note] Recorded {final_status} run for {project} in Progress.md") + except Exception as exc: + log.warning(f"[progress-note] Failed to write Progress.md: {exc}") + + +_POST_TASK_REVIEW_PROMPT = """You are reviewing a COMPLETED automation run to capture reusable knowledge for next time. Be factual — use ONLY what the notes below support. + +ORIGINAL TASK: {task} +PROJECT: {project} BRANCH: {branch} + +WHAT THE AGENTS PRODUCED (their handoff notes): +{context} + +Respond with EXACTLY these two sections and nothing else: + +## Learnings +2-4 short bullets: what worked, and any gotcha worth remembering. Specific and factual. + +## Skill Candidate +DEFAULT TO NO_SKILL. Propose a skill ONLY if this run followed a genuinely reusable, repeatable procedure you would run again on a DIFFERENT project. One-off, project-specific, or "just did the task" work is NOT a skill — when in doubt, write exactly: NO_SKILL +If (and only if) it is truly reusable, output ONLY a skill in this exact format: +--- +name: +description: +--- + +""" + + +def _split_review(text: str) -> tuple[str, str]: + """Split an Ollama review into (learnings, skill_candidate). skill is "" unless the + section actually parses as a frontmatter skill — a paraphrased "No skill needed" + or plain prose must NOT stage a candidate (the model rarely emits the literal + NO_SKILL token, so the gate is 'looks like a skill', not 'absence of NO_SKILL').""" + skill = "" + pre = text + if "## Skill Candidate" in text: + pre, _, post = text.partition("## Skill Candidate") + post = post.strip() + if post.startswith("---") and "NO_SKILL" not in post.upper(): + skill = post + learnings = pre.split("## Learnings", 1)[1].strip() if "## Learnings" in pre else pre.strip() + return learnings, skill + + +def _skill_name(skill_md: str) -> str: + """Pull the kebab `name:` from a skill candidate's frontmatter, sanitized to a safe + bare filename ("" if none). Sanitizing is load-bearing security, not cosmetics: the + name comes from an 8B model fed agent notes, and it becomes a path — an unsanitized + `../../.claude/skills/x` or absolute path would escape the staging dir into the + globally-active skills folder, defeating the whole review gate.""" + for line in skill_md.splitlines(): + if line.strip().startswith("name:"): + raw = line.split("name:", 1)[1].strip().strip("\"'") + return re.sub(r"[^a-z0-9-]+", "-", raw.lower()).strip("-")[:60] + return "" + + +def spawn_post_task_review(engine, run_id: str, project: str, branch: str, hermes_config) -> None: + """Nous-style 'learning': after a DONE run, distill it (local Ollama, free, + non-blocking) into a Learnings entry in Progress.md + an optional skill CANDIDATE + staged in ~/hermes/skill-candidates for review. Never self-installs a skill — + an 8B model's draft is a suggestion, not a globally-active skill. + ponytail: local model + human promote-gate; good enough for a draft.""" + + def _worker(): + try: + run = engine.get_run(run_id) + task = run.get("task_raw", "") or run.get("task", "") + notes = [] + for step in run.get("pipeline", []): + op = step.get("output_path") + if not op: + continue + note_path = Path.home() / "Documents" / "RaphBrain" / op + if note_path.exists(): + notes.append( + f"### {step.get('role')}\n{note_path.read_text(encoding='utf-8')[:1500]}" + ) + context = ("\n\n".join(notes))[:6000] or "(no step notes captured)" + out = _ollama_generate( + _POST_TASK_REVIEW_PROMPT.format( + task=task[:400], project=project, branch=branch, context=context + ), + timeout=180, + ) + if not out: + return + learnings, skill = _split_review(out) + if learnings: + stamp = datetime.now().strftime("%Y-%m-%d %H:%M") + _prepend_under_runlog(project, f"#### {stamp} — 🧠 Learnings\n{learnings}\n") + candidate_path = "" + # Stage ONLY a candidate that parses as a real skill with a safe name — no + # timestamp fallback. A nameless/prose blob is not a skill, so it never + # stages a garbage file or pings Jaiden on a plain successful run. + name = _skill_name(skill) if skill else "" + if skill and skill.startswith("---") and name: + SKILL_CANDIDATES_DIR.mkdir(parents=True, exist_ok=True) + candidate = SKILL_CANDIDATES_DIR / f"{name}.md" + # Extra belt: the resolved path must stay inside the staging dir. + if candidate.resolve().parent == SKILL_CANDIDATES_DIR.resolve(): + candidate.write_text(skill + "\n", encoding="utf-8") + candidate_path = str(candidate) + _send_reply( + hermes_config, + f"🧠 {project}: drafted a skill candidate — review to promote:\n{candidate_path}", + ) + log.info( + f"[post-task-review] {project}: learnings={'y' if learnings else 'n'} " + f"skill_candidate={'y' if candidate_path else 'n'}" + ) + except Exception as exc: + log.warning(f"[post-task-review] failed for {project}: {exc}") + + threading.Thread(target=_worker, daemon=True, name=f"hermes-review-{run_id}").start() + + def _format_duration(started_iso: str, ended_iso: str) -> str: """Return a human-readable duration like '2m 14s'.""" try: @@ -607,6 +816,26 @@ def _finalize_step_failure(reason: str) -> None: _append_pipeline_to_daily_note( project, branch, final_status, total_duration, failed_step, failed_reason ) + + # Guarantee a Progress.md entry for every terminal run — deterministic Python, + # not dependent on a sub-agent choosing to write one. + _append_pipeline_to_progress_note( + project, + branch, + final_status, + total_duration, + pr_url or "", + failed_step, + failed_reason, + done_count, + step_count, + ) + + # Nous-style "learning": on success, distill the run into a Progress.md + # Learnings note + an optional skill CANDIDATE (local Ollama, free, staged + # for review — never auto-installed). Non-blocking. + if final_status == "done": + spawn_post_task_review(engine, run_id, project, branch, hermes_config) except Exception as exc: log.error(f"[orchestrate] Failed to send completion notification: {exc}") diff --git a/tests/test_progress_note.py b/tests/test_progress_note.py new file mode 100644 index 0000000..2ea673a --- /dev/null +++ b/tests/test_progress_note.py @@ -0,0 +1,196 @@ +""" +test_progress_note.py — the deterministic Progress.md writer + post-task-review +parsing/sanitizing. The write path is what makes 'done' GUARANTEE a note exists, so +it gets a real check; RAPHBRAIN_PROJECTS_DIR is patched to a tmp so the real vault is +untouched, and resolve_project is patched so folder resolution is deterministic. +""" + +import sys +import tempfile +import threading +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path.home() / "hermes")) +import hermes # noqa: E402 + + +class _FakeProj: + def __init__(self, raphbrain_dir): + self.raphbrain_dir = raphbrain_dir + self.repo = "~/Projects/x" + + +class _FakeEngine: + def __init__(self, run): + self._run = run + + def get_run(self, _run_id): + return self._run + + +class TestProgressNote(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self._dir = Path(self._tmp.name) / "Projects" + self._p = patch.object(hermes, "RAPHBRAIN_PROJECTS_DIR", self._dir) + self._p.start() + # Default: no registry hit → falls back to project.capitalize() (deterministic, + # no dependency on the real ~/Projects layout). H2 test overrides this. + self._rp = patch.object(hermes, "resolve_project", return_value=None) + self._rp.start() + + def tearDown(self): + self._rp.stop() + self._p.stop() + self._tmp.cleanup() + + def _progress(self, proj="Arbiter"): + return self._dir / proj / "Progress.md" + + def test_creates_file_and_section_when_missing(self): + # done ⇒ a note MUST exist even if Progress.md didn't. + hermes._append_pipeline_to_progress_note( + "arbiter", + "feature/x", + "done", + "12m", + pr_url="http://pr/1", + done_count=4, + step_count=4, + ) + body = self._progress().read_text() + self.assertIn(hermes.HERMES_RUN_LOG_HEADER, body) + self.assertIn("✅ arbiter/feature/x — done", body) + self.assertIn("4/4 steps · 12m", body) + self.assertIn("PR: http://pr/1", body) + + def test_failed_entry_records_step_and_reason(self): + hermes._append_pipeline_to_progress_note( + "arbiter", + "", + "failed", + "3m", + failed_step="deployer", + reason="no PR marker", + done_count=2, + step_count=4, + ) + body = self._progress().read_text() + self.assertIn("❌ arbiter/(no branch) — failed", body) + self.assertIn("Failed at `deployer` — no PR marker", body) + + def test_newest_first_and_preserves_human_sections(self): + p = self._progress() + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text( + "# Arbiter — Progress\n\n## Status\nhand-curated\n\n" + f"{hermes.HERMES_RUN_LOG_HEADER}\n### old entry\n" + ) + hermes._append_pipeline_to_progress_note( + "arbiter", "feature/new", "done", "1m", done_count=1, step_count=1 + ) + body = p.read_text() + self.assertIn("hand-curated", body) # human section survived + self.assertLess(body.index("feature/new"), body.index("### old entry")) # newest first + + def test_header_without_trailing_newline_stays_one_section(self): + # a hand-edited file whose header is the last line (no trailing \n): the entry + # must NOT glue onto the header, and a SECOND run must not append a 2nd section. + p = self._progress() + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(f"# Arbiter — Progress\n\n{hermes.HERMES_RUN_LOG_HEADER}") + hermes._append_pipeline_to_progress_note( + "arbiter", "one", "done", "1m", step_count=1, done_count=1 + ) + hermes._append_pipeline_to_progress_note( + "arbiter", "two", "done", "1m", step_count=1, done_count=1 + ) + body = p.read_text() + self.assertEqual(body.count(hermes.HERMES_RUN_LOG_HEADER), 1) # still one section + self.assertIn( + f"{hermes.HERMES_RUN_LOG_HEADER}\n", body + ) # header on its own line, not glued + self.assertNotIn(f"{hermes.HERMES_RUN_LOG_HEADER}###", body) + self.assertLess(body.index("two"), body.index("one")) # newest-first preserved + + def test_note_uses_registry_folder_not_capitalize(self): + # hyphenated project must land in the registry folder, not a capitalize() orphan. + with patch.object(hermes, "resolve_project", return_value=_FakeProj("FinanceTracker")): + hermes._append_pipeline_to_progress_note( + "finance-tracker", "b", "done", "1m", done_count=1, step_count=1 + ) + self.assertTrue((self._dir / "FinanceTracker" / "Progress.md").exists()) + self.assertFalse((self._dir / "Finance-tracker" / "Progress.md").exists()) + + # --- post-task review parsing / sanitizing --- + + def test_split_review_literal_no_skill(self): + learnings, skill = hermes._split_review( + "## Learnings\n- did a thing\n\n## Skill Candidate\nNO_SKILL" + ) + self.assertIn("did a thing", learnings) + self.assertEqual(skill, "") + + def test_split_review_paraphrased_no_skill_stages_nothing(self): + # the 8B model rarely emits the literal token — prose must not become a skill. + for prose in ("No skill needed here.", "We could maybe reuse the deploy step."): + _, skill = hermes._split_review(f"## Learnings\n- x\n\n## Skill Candidate\n{prose}") + self.assertEqual(skill, "", prose) + + def test_split_review_extracts_real_frontmatter_skill(self): + text = ( + "## Learnings\n- reusable flow\n\n## Skill Candidate\n" + "---\nname: deploy-vercel-site\ndescription: ship a static site\n---\n1. build\n2. deploy" + ) + learnings, skill = hermes._split_review(text) + self.assertIn("reusable flow", learnings) + self.assertTrue(skill.startswith("---")) + self.assertEqual(hermes._skill_name(skill), "deploy-vercel-site") + + def test_skill_name_sanitizes_path_traversal(self): + # the name is model output used as a path — it must never escape the staging dir. + self.assertEqual( + hermes._skill_name("name: ../../.claude/skills/evil"), "claude-skills-evil" + ) + self.assertEqual(hermes._skill_name("name: /etc/passwd"), "etc-passwd") + self.assertEqual(hermes._skill_name('name: "My Cool Skill!"'), "my-cool-skill") + self.assertEqual(hermes._skill_name("no frontmatter"), "") + + # --- end-to-end staging path (security-relevant: model output → file on disk) --- + + def _run_review(self, review_out, staging): + with ( + patch.object(hermes, "SKILL_CANDIDATES_DIR", staging), + patch.object(hermes, "_ollama_generate", return_value=review_out), + patch.object(hermes, "_send_reply") as reply, + ): + engine = _FakeEngine({"task_raw": "do a thing", "pipeline": []}) + hermes.spawn_post_task_review(engine, "rid", "arbiter", "feat/x", {}) + for t in list(threading.enumerate()): + if t.name == "hermes-review-rid": + t.join(timeout=5) + return reply + + def test_review_stages_real_skill_and_pings(self): + staging = self._dir.parent / "skill-candidates" + reply = self._run_review( + "## Learnings\n- did stuff\n\n## Skill Candidate\n" + "---\nname: my-skill\ndescription: does a thing\n---\n1. step\n", + staging, + ) + self.assertEqual([f.name for f in staging.glob("*.md")], ["my-skill.md"]) + reply.assert_called_once() + + def test_review_no_skill_stages_nothing_and_no_ping(self): + staging = self._dir.parent / "skill-candidates" + reply = self._run_review( + "## Learnings\n- did stuff\n\n## Skill Candidate\nNO_SKILL", staging + ) + self.assertFalse(staging.exists() and list(staging.glob("*.md"))) + reply.assert_not_called() + + +if __name__ == "__main__": + unittest.main()