From 8360cb21145341e7183b7a093eba88ff6e039809 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 1 Jul 2026 19:51:08 -0400 Subject: [PATCH 1/8] feat(kanban): standalone card-drop + comment receiver (goal cards, dedupe, fail-closed auth) The single inbound HTTP write surface connecting off-box producers (Render CPE website / diligence inbox, Modal cron producers) to the local Kanban board. Standalone stdlib-only launchd daemon on loopback :8646 (not api_server.py, to avoid widening the multiplex gateway port); shells out to `hermes kanban create/comment`. - POST /kanban/card-drop {assignee,title,body,dedupe_key?,priority?,goal?, goal_max_turns?} -> {id}; goal:true -> --goal; dedupe_key -> --idempotency-key. - POST /kanban/comment {card_id,text} -> {id,commented}. - X-Cron-Secret auth, FAIL CLOSED when secret unset (inverts research-agent's fail-open _check_cron_auth; public hostname makes the gate the boundary). - launchd plist + launcher (sources secret from ~/.hermes/.env) + README. - 20 tests pass; live e2e proved create(goal)/dedupe/comment/403 on a real board. Patch note: ~/.hermes/plans/hermes-patches/kanban-card-drop-receiver.md --- scripts/kanban_receiver/README.md | 90 +++++ .../ai.hermes.kanban-receiver.plist | 40 +++ scripts/kanban_receiver/kanban_receiver.py | 317 ++++++++++++++++++ scripts/kanban_receiver/run_receiver.sh | 36 ++ .../kanban_receiver/test_kanban_receiver.py | 272 +++++++++++++++ 5 files changed, 755 insertions(+) create mode 100644 scripts/kanban_receiver/README.md create mode 100644 scripts/kanban_receiver/ai.hermes.kanban-receiver.plist create mode 100755 scripts/kanban_receiver/kanban_receiver.py create mode 100755 scripts/kanban_receiver/run_receiver.sh create mode 100644 scripts/kanban_receiver/test_kanban_receiver.py diff --git a/scripts/kanban_receiver/README.md b/scripts/kanban_receiver/README.md new file mode 100644 index 000000000000..c96e1f5fa37b --- /dev/null +++ b/scripts/kanban_receiver/README.md @@ -0,0 +1,90 @@ +# Kanban card-drop receiver + +The single inbound HTTP write surface connecting off-box producers (the Render +CPE website / diligence inbox, Modal cron producers) to this machine's local +Kanban board (`$HERMES_HOME/kanban.db`). Remote tiers can't write the local +SQLite board directly, so they POST here. + +See the plans: +- `~/.hermes/plans/cpe-chat-route-to-research-lead.md` (Card A) +- `~/.hermes/plans/diligence-inbox-production.md` (section A+) + +## Why standalone (not gateway/platforms/api_server.py) + +`api_server.py` is a large OpenAI-compatible platform adapter whose routes are +baked into one hardcoded registration block with no auth'd custom-route hook. +Extending it risks the always-on multiplex gateway port (secondary profiles must +not bind ports → crash-loop risk). This receiver is a tiny **stdlib-only** +launchd daemon on its own loopback port (`8646`), far smaller blast radius, and +boots cleanly under a bare launchd context. It shells out to the installed +`hermes kanban` CLI — the same validated create/comment path a human uses. + +## Endpoints + +| Method | Path | Auth | Body | +|--------|-----------------------|------|------| +| GET | `/health` | none | — | +| POST | `/kanban/card-drop` | yes | `{assignee, title, body, dedupe_key?, priority?, goal?, goal_max_turns?}` → `{id}` | +| POST | `/kanban/comment` | yes | `{card_id, text}` → `{id, commented: true}` | + +- `goal: true` (+ optional `goal_max_turns: N`) passes `--goal --goal-max-turns N` + so the card runs as a Ralph-style goal loop. +- `dedupe_key` maps to `--idempotency-key`: a repeat drop with the same key + collapses onto the existing card and returns its id (no duplicate). +- `assignee` must be one of the known lanes (`ALLOWED_ASSIGNEES` in the source). + +## Auth: fail CLOSED + +`X-Cron-Secret` header, compared with `hmac.compare_digest` against +`KANBAN_RECEIVER_SECRET` (preferred) or `CRON_SECRET`. + +**When no secret is configured, the service refuses ALL writes with 403.** This +is the deliberate inversion of research-agent's `_check_cron_auth` (which +fails *open* for local testing). Because the receiver is reachable on the public +hostname `kanban.getbloom.app`, the secret gate IS the security boundary — a +fail-open default would let anyone who finds the URL spawn lanes. `/health` is +unauthenticated (liveness only). + +## Config (env) + +| Var | Default | Meaning | +|-----|---------|---------| +| `KANBAN_RECEIVER_SECRET` / `CRON_SECRET` | *(unset → 403 all writes)* | shared secret | +| `KANBAN_RECEIVER_PORT` | `8646` | listen port | +| `KANBAN_RECEIVER_HOST` | `127.0.0.1` | bind addr (loopback only; never 0.0.0.0) | +| `HERMES_HOME` | `~/.hermes` | which board `hermes kanban` targets | +| `HERMES_BIN` | *(PATH lookup)* | explicit `hermes` binary override | + +## Public reachability + +Reuse the existing `hermes-webhooks` cloudflared tunnel (already serves +`webhooks.getbloom.app` + `proxy.getbloom.app`). Add ONE ingress rule to +`~/.cloudflared/config.yml`: + +```yaml + - hostname: kanban.getbloom.app + service: http://localhost:8646 +``` + +then `cloudflared tunnel route dns hermes-webhooks kanban.getbloom.app` and +restart the tunnel (`kill `; KeepAlive respawns and re-reads +config). Do NOT create a new tunnel. Render/Modal env: +`KANBAN_CARD_DROP_URL=https://kanban.getbloom.app/kanban/card-drop`. + +## Install the launchd service + +```bash +cp scripts/kanban_receiver/ai.hermes.kanban-receiver.plist ~/Library/LaunchAgents/ +sed -i '' "s|__HOME__|$HOME|g" ~/Library/LaunchAgents/ai.hermes.kanban-receiver.plist +launchctl load ~/Library/LaunchAgents/ai.hermes.kanban-receiver.plist +# verify +curl -s http://127.0.0.1:8646/health +``` + +Logs: `/tmp/hermes/kanban-receiver-stdout.log` + `-stderr.log`. + +## Test + +```bash +python3 -m pytest scripts/kanban_receiver/test_kanban_receiver.py -q +``` diff --git a/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist b/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist new file mode 100644 index 000000000000..cc0f5ccf00bd --- /dev/null +++ b/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist @@ -0,0 +1,40 @@ + + + + + + Label + ai.hermes.kanban-receiver + ProgramArguments + + /bin/bash + __HOME__/projects/hermes-agent/scripts/kanban_receiver/run_receiver.sh + + EnvironmentVariables + + HERMES_HOME + __HOME__/.hermes + KANBAN_RECEIVER_REPO + __HOME__/projects/hermes-agent + + RunAtLoad + + KeepAlive + + StandardOutPath + /tmp/hermes/kanban-receiver-stdout.log + StandardErrorPath + /tmp/hermes/kanban-receiver-stderr.log + + diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py new file mode 100755 index 000000000000..a22e060846a8 --- /dev/null +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -0,0 +1,317 @@ +#!/usr/bin/env python3 +"""Standalone HTTP receiver that drops cards onto the local Kanban board. + +This is the single inbound write surface connecting off-box producers (the +Render CPE website, the diligence inbox, Modal cron producers) to the Hermes +Kanban board living at ``$HERMES_HOME/kanban.db`` on this machine. Remote tiers +cannot write the local SQLite board directly, so they POST here instead. + +Design decisions (see ~/.hermes/plans/cpe-chat-route-to-research-lead.md Card A +and ~/.hermes/plans/diligence-inbox-production.md section A+): + +* **Standalone, not api_server.py.** The gateway's ``api_server`` is a large + OpenAI-compatible platform adapter whose routes are baked into one hardcoded + block with no auth'd custom-route registration hook. Widening its surface + risks the multiplex gateway's always-on port (secondary profiles must not + bind ports -> crash-loop risk). This service is a tiny, stdlib-only launchd + daemon on its own port with a far smaller blast radius. + +* **stdlib only.** No aiohttp / third-party imports, so it boots cleanly under a + bare launchd context regardless of which venv the gateway uses. It shells out + to the installed ``hermes kanban`` CLI (the same validated create/comment path + a human or the dispatcher uses), inheriting ``HERMES_HOME``. + +* **Fail closed.** ``CRON_SECRET`` (or ``KANBAN_RECEIVER_SECRET``) must be set; + when it is unset the service refuses ALL writes with 403. Because the public + hostname (kanban.getbloom.app) makes the secret gate the security boundary, + an unset secret must never silently accept unauthenticated writes. This is the + fail-CLOSED inversion of research-agent's ``_check_cron_auth`` (which + fails-open for local testing); on a public surface fail-open is unacceptable. + +Endpoints: + GET /health -> {"ok": true, "ts": ...} (no auth; liveness) + POST /kanban/card-drop -> create a card, returns {"id": ...} + POST /kanban/comment -> append a comment to an existing card + +Auth: ``X-Cron-Secret`` header, compared with ``hmac.compare_digest``. +""" + +from __future__ import annotations + +import hmac +import json +import logging +import os +import shutil +import subprocess +import sys +import time +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from typing import Any, Optional + +LOG = logging.getLogger("kanban_receiver") + +DEFAULT_PORT = 8646 +MAX_BODY_BYTES = 64 * 1024 # generous for a card body, cheap DoS guard +CLI_TIMEOUT_SECONDS = 30 + +# Only these profiles may be targeted by an off-box drop. Keeps a leaked secret +# from spawning arbitrary lanes; extend deliberately. "none" creates an +# unassigned card (triaged by a human/orchestrator later). +ALLOWED_ASSIGNEES = { + "none", + "orchestrator", + "equity-analyst", + "memo-evaluator", + "researcher", + "dev", + "pm", + "designer", + "qa", + "infra-ops", + "pr-babysitter", + "code-reviewer", + "content-creator", + "ads-optimizer", +} + + +def _secret() -> Optional[str]: + """The configured shared secret, or None when unset. + + Accepts KANBAN_RECEIVER_SECRET first (service-specific) then CRON_SECRET + (the same secret the Modal / research-agent callers already send).""" + for var in ("KANBAN_RECEIVER_SECRET", "CRON_SECRET"): + val = os.environ.get(var) + if val and val.strip(): + return val.strip() + return None + + +def _hermes_bin() -> str: + """Resolve the ``hermes`` CLI entrypoint. + + Prefer an explicit override, then PATH, then the interpreter running this + service (``python -m hermes_cli.main``) as a last resort so the daemon works + even when launchd's PATH is minimal.""" + override = os.environ.get("HERMES_BIN") + if override: + return override + found = shutil.which("hermes") + if found: + return found + return "" # signals: fall back to python -m + + +def _run_hermes_kanban(args: list[str]) -> subprocess.CompletedProcess: + """Invoke ``hermes kanban `` inheriting the environment (HERMES_HOME).""" + hermes = _hermes_bin() + if hermes: + cmd = [hermes, "kanban", *args] + else: + cmd = [sys.executable, "-m", "hermes_cli.main", "kanban", *args] + LOG.info("exec: %s", " ".join(cmd[:2] + ["kanban"] + [a for a in args if not a.startswith("--body")])) + return subprocess.run( + cmd, + text=True, + capture_output=True, + timeout=CLI_TIMEOUT_SECONDS, + env=os.environ.copy(), + ) + + +def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: + """Handle a /kanban/card-drop payload -> hermes kanban create. + + Contract: {assignee, title, body, dedupe_key?, priority?, goal?, + goal_max_turns?}. Returns (status_code, json_body).""" + assignee = (payload.get("assignee") or "").strip() + title = (payload.get("title") or "").strip() + body = payload.get("body") or "" + + if not assignee or not title: + return 400, {"error": "assignee and title are required"} + if assignee not in ALLOWED_ASSIGNEES: + return 400, {"error": f"assignee '{assignee}' not permitted"} + + # `title` is a positional arg on `hermes kanban create`; `--body`/`--assignee` + # are flags. Build all flags first, then append `-- ` LAST so a title + # beginning with a dash can't be parsed as an option. + args = ["create", "--assignee", assignee, "--body", body, "--json"] + + dedupe_key = payload.get("dedupe_key") + if dedupe_key and str(dedupe_key).strip(): + args += ["--idempotency-key", str(dedupe_key).strip()] + + priority = payload.get("priority") + if priority is not None: + try: + args += ["--priority", str(int(priority))] + except (TypeError, ValueError): + return 400, {"error": "priority must be an integer"} + + if payload.get("goal"): + args.append("--goal") + gmt = payload.get("goal_max_turns") + if gmt is not None: + try: + args += ["--goal-max-turns", str(int(gmt))] + except (TypeError, ValueError): + return 400, {"error": "goal_max_turns must be an integer"} + + # Author the card as the drop's assignee-agnostic origin so the audit trail + # shows it arrived over the wire, not from a local human. + args += ["--created-by", "card-drop"] + + # Positional title last, guarded by `--`. + args += ["--", title] + + try: + proc = _run_hermes_kanban(args) + except subprocess.TimeoutExpired: + return 504, {"error": "kanban create timed out"} + + if proc.returncode != 0: + LOG.error("kanban create failed rc=%s stderr=%s", proc.returncode, proc.stderr[-500:]) + return 502, {"error": "kanban create failed", "detail": proc.stderr.strip()[-300:]} + + try: + created = json.loads(proc.stdout) + except json.JSONDecodeError: + LOG.error("kanban create returned non-JSON: %s", proc.stdout[-300:]) + return 502, {"error": "kanban create returned unparseable output"} + + card_id = created.get("id") + if not card_id: + return 502, {"error": "kanban create returned no id"} + return 200, {"id": card_id} + + +def comment_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: + """Handle a /kanban/comment payload -> hermes kanban comment. + + Contract: {card_id, text}. Returns (status_code, json_body).""" + card_id = (payload.get("card_id") or "").strip() + text = payload.get("text") or "" + if not card_id or not str(text).strip(): + return 400, {"error": "card_id and text are required"} + + # `hermes kanban comment <task_id> <text...>` — both positional. Put the + # optional `--author` first, then `--` and the two positionals, so a text + # (or id) starting with a dash can't be misparsed as a flag. + args = ["comment", "--author", "card-drop", "--", card_id, str(text)] + try: + proc = _run_hermes_kanban(args) + except subprocess.TimeoutExpired: + return 504, {"error": "kanban comment timed out"} + + if proc.returncode != 0: + LOG.error("kanban comment failed rc=%s stderr=%s", proc.returncode, proc.stderr[-500:]) + detail = proc.stderr.strip()[-300:] + # Distinguish an unknown card (client error) from a real server fault. + if "not found" in detail.lower() or "no such" in detail.lower(): + return 404, {"error": "card not found", "card_id": card_id} + return 502, {"error": "kanban comment failed", "detail": detail} + + return 200, {"id": card_id, "commented": True} + + +class Handler(BaseHTTPRequestHandler): + server_version = "kanban-receiver/1.0" + + # --- helpers ----------------------------------------------------------- + def _send_json(self, status: int, obj: dict[str, Any]) -> None: + data = json.dumps(obj).encode("utf-8") + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(data))) + self.end_headers() + self.wfile.write(data) + + def _authorized(self) -> bool: + """Fail CLOSED: unset secret -> refuse; set secret -> constant-time match.""" + secret = _secret() + if not secret: + LOG.warning("write refused: no secret configured (fail-closed)") + return False + provided = self.headers.get("X-Cron-Secret", "") + return hmac.compare_digest(provided, secret) + + def _read_json_body(self) -> Optional[dict[str, Any]]: + try: + length = int(self.headers.get("Content-Length", "0")) + except (TypeError, ValueError): + return None + if length <= 0 or length > MAX_BODY_BYTES: + return None + raw = self.rfile.read(length) + try: + obj = json.loads(raw) + except json.JSONDecodeError: + return None + return obj if isinstance(obj, dict) else None + + def log_message(self, format: str, *args: Any) -> None: # noqa: A002,A003 + LOG.info("%s - %s", self.address_string(), format % args) + + # --- routes ------------------------------------------------------------ + def do_GET(self) -> None: # noqa: N802 + if self.path.rstrip("/") in ("/health", "/kanban/health"): + self._send_json(200, {"ok": True, "ts": int(time.time())}) + return + self._send_json(404, {"error": "not found"}) + + def do_POST(self) -> None: # noqa: N802 + route = self.path.rstrip("/") + if route not in ("/kanban/card-drop", "/kanban/comment"): + self._send_json(404, {"error": "not found"}) + return + + # Auth first; a bad/missing secret never reaches the board. + if not self._authorized(): + self._send_json(403, {"error": "forbidden"}) + return + + payload = self._read_json_body() + if payload is None: + self._send_json(400, {"error": "invalid or oversized JSON body"}) + return + + if route == "/kanban/card-drop": + status, obj = create_card(payload) + else: + status, obj = comment_card(payload) + self._send_json(status, obj) + + +def main() -> int: + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)s %(name)s %(message)s", + stream=sys.stderr, + ) + port = int(os.environ.get("KANBAN_RECEIVER_PORT", DEFAULT_PORT)) + # Bind loopback only: the public path is the cloudflared tunnel, which + # forwards to localhost. Never bind 0.0.0.0 (would expose the port on the + # LAN, bypassing the tunnel's edge). + host = os.environ.get("KANBAN_RECEIVER_HOST", "127.0.0.1") + + if not _secret(): + LOG.warning( + "starting WITHOUT a secret configured -- ALL writes will be refused " + "(403) until KANBAN_RECEIVER_SECRET or CRON_SECRET is set." + ) + + httpd = ThreadingHTTPServer((host, port), Handler) + LOG.info("kanban receiver listening on http://%s:%d", host, port) + try: + httpd.serve_forever() + except KeyboardInterrupt: + LOG.info("shutting down") + finally: + httpd.server_close() + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/kanban_receiver/run_receiver.sh b/scripts/kanban_receiver/run_receiver.sh new file mode 100755 index 000000000000..808a17fc0b35 --- /dev/null +++ b/scripts/kanban_receiver/run_receiver.sh @@ -0,0 +1,36 @@ +#!/bin/bash +# Launcher for the Kanban card-drop receiver launchd service. +# +# Sources the receiver secret from ~/.hermes/.env (never hardcode it in the +# plist, which lands in a world-readable LaunchAgents dir), then execs the +# stdlib-only receiver. Keeps HERMES_HOME so `hermes kanban` targets the real +# board. +set -euo pipefail + +HERMES_HOME="${HERMES_HOME:-$HOME/.hermes}" +export HERMES_HOME + +REPO_ROOT="${KANBAN_RECEIVER_REPO:-$HOME/projects/hermes-agent}" + +# Pull KANBAN_RECEIVER_SECRET / CRON_SECRET (and optional PORT) from .env +# without exporting the entire secret file into the process env. Only the keys +# the receiver reads are lifted. +ENV_FILE="$HERMES_HOME/.env" +if [[ -f "$ENV_FILE" ]]; then + for key in KANBAN_RECEIVER_SECRET CRON_SECRET KANBAN_RECEIVER_PORT HERMES_BIN; do + line="$(grep -E "^${key}=" "$ENV_FILE" | tail -1 || true)" + if [[ -n "$line" ]]; then + val="${line#*=}" + # strip surrounding single/double quotes if present + val="${val%\"}"; val="${val#\"}" + val="${val%\'}"; val="${val#\'}" + export "${key}=${val}" + fi + done +fi + +# Prefer the installed `hermes` on PATH; fall back to the repo module. +export PATH="/usr/local/bin:/opt/homebrew/bin:$PATH" + +cd "$REPO_ROOT" +exec /usr/bin/env python3 scripts/kanban_receiver/kanban_receiver.py diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py new file mode 100644 index 000000000000..888594ca5c1f --- /dev/null +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -0,0 +1,272 @@ +"""Tests for the standalone Kanban card-drop receiver. + +Two layers: + * Unit: create_card / comment_card / auth logic with the CLI shell-out + monkeypatched (no board, no subprocess). + * Integration: boot the real ThreadingHTTPServer and drive it with urllib, + still monkeypatching the CLI so we assert wire behavior (auth 403, dedupe + pass-through, goal flag) without mutating a real board. +""" + +from __future__ import annotations + +import importlib.util +import json +import os +import subprocess +import threading +import urllib.error +import urllib.request +from pathlib import Path + +import pytest + +# Load the receiver module by path (it lives outside the package tree). +_MOD_PATH = Path(__file__).with_name("kanban_receiver.py") +_spec = importlib.util.spec_from_file_location("kanban_receiver", _MOD_PATH) +assert _spec and _spec.loader +kr = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(kr) + + +class _FakeProc: + def __init__(self, returncode=0, stdout="", stderr=""): + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + + +@pytest.fixture(autouse=True) +def _clear_secret(monkeypatch): + monkeypatch.delenv("KANBAN_RECEIVER_SECRET", raising=False) + monkeypatch.delenv("CRON_SECRET", raising=False) + + +# -------------------------------------------------------------------------- +# create_card +# -------------------------------------------------------------------------- + +def test_create_card_happy_path(monkeypatch): + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout=json.dumps({"id": "t_abc123", "status": "ready"})) + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + status, body = kr.create_card( + {"assignee": "equity-analyst", "title": "AVGO deep dive", "body": "the ask"} + ) + assert status == 200 + assert body == {"id": "t_abc123"} + a = captured["args"] + assert a[0] == "create" + assert "--assignee" in a and "equity-analyst" in a + assert "--json" in a + + +def test_create_card_requires_assignee_and_title(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) + status, body = kr.create_card({"title": "no assignee"}) + assert status == 400 + status, body = kr.create_card({"assignee": "dev"}) + assert status == 400 + + +def test_create_card_rejects_unknown_assignee(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) + status, body = kr.create_card({"assignee": "root", "title": "x"}) + assert status == 400 + assert "not permitted" in body["error"] + + +def test_create_card_passes_dedupe_key(monkeypatch): + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id": "t_1"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + kr.create_card({"assignee": "dev", "title": "x", "dedupe_key": "chat:run42"}) + a = captured["args"] + assert "--idempotency-key" in a + assert a[a.index("--idempotency-key") + 1] == "chat:run42" + + +def test_create_card_goal_flag(monkeypatch): + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id": "t_1"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + kr.create_card( + {"assignee": "equity-analyst", "title": "x", "goal": True, "goal_max_turns": 15} + ) + a = captured["args"] + assert "--goal" in a + assert "--goal-max-turns" in a + assert a[a.index("--goal-max-turns") + 1] == "15" + + +def test_create_card_bad_priority(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) + status, body = kr.create_card({"assignee": "dev", "title": "x", "priority": "high"}) + assert status == 400 + + +def test_create_card_cli_failure(monkeypatch): + monkeypatch.setattr( + kr, "_run_hermes_kanban", lambda args: _FakeProc(returncode=1, stderr="boom") + ) + status, body = kr.create_card({"assignee": "dev", "title": "x"}) + assert status == 502 + + +def test_create_card_timeout(monkeypatch): + def raise_timeout(args): + raise subprocess.TimeoutExpired(cmd="hermes", timeout=30) + + monkeypatch.setattr(kr, "_run_hermes_kanban", raise_timeout) + status, body = kr.create_card({"assignee": "dev", "title": "x"}) + assert status == 504 + + +# -------------------------------------------------------------------------- +# comment_card +# -------------------------------------------------------------------------- + +def test_comment_happy_path(monkeypatch): + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout="ok") + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + status, body = kr.comment_card({"card_id": "t_abc", "text": "a follow-up"}) + assert status == 200 + assert body["id"] == "t_abc" + a = captured["args"] + assert a[0] == "comment" + assert "t_abc" in a and "a follow-up" in a + # positionals come after the `--` guard + assert a[a.index("--") + 1] == "t_abc" + + +def test_comment_requires_fields(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) + assert kr.comment_card({"text": "hi"})[0] == 400 + assert kr.comment_card({"card_id": "t_1"})[0] == 400 + + +def test_comment_unknown_card_is_404(monkeypatch): + monkeypatch.setattr( + kr, "_run_hermes_kanban", + lambda args: _FakeProc(returncode=1, stderr="task not found: t_x"), + ) + status, body = kr.comment_card({"card_id": "t_x", "text": "hi"}) + assert status == 404 + + +# -------------------------------------------------------------------------- +# secret resolution +# -------------------------------------------------------------------------- + +def test_secret_unset_is_none(): + assert kr._secret() is None + + +def test_secret_prefers_receiver_var(monkeypatch): + monkeypatch.setenv("CRON_SECRET", "cron") + monkeypatch.setenv("KANBAN_RECEIVER_SECRET", "recv") + assert kr._secret() == "recv" + + +def test_secret_falls_back_to_cron(monkeypatch): + monkeypatch.setenv("CRON_SECRET", "cron") + assert kr._secret() == "cron" + + +# -------------------------------------------------------------------------- +# Integration over real HTTP (CLI still mocked) +# -------------------------------------------------------------------------- + +@pytest.fixture +def server(monkeypatch): + """Boot the receiver on an ephemeral port; yield its base URL.""" + from http.server import ThreadingHTTPServer + + httpd = ThreadingHTTPServer(("127.0.0.1", 0), kr.Handler) + port = httpd.server_address[1] + t = threading.Thread(target=httpd.serve_forever, daemon=True) + t.start() + try: + yield f"http://127.0.0.1:{port}" + finally: + httpd.shutdown() + httpd.server_close() + + +def _post(url, payload, headers=None): + data = json.dumps(payload).encode() + req = urllib.request.Request(url, data=data, method="POST") + req.add_header("Content-Type", "application/json") + for k, v in (headers or {}).items(): + req.add_header(k, v) + try: + with urllib.request.urlopen(req, timeout=5) as resp: + return resp.status, json.loads(resp.read()) + except urllib.error.HTTPError as e: + return e.code, json.loads(e.read()) + + +def test_http_health_no_auth(server): + with urllib.request.urlopen(f"{server}/health", timeout=5) as resp: + assert resp.status == 200 + assert json.loads(resp.read())["ok"] is True + + +def test_http_fail_closed_when_secret_unset(server): + # No secret in env -> every write is 403. + status, body = _post(f"{server}/kanban/card-drop", + {"assignee": "dev", "title": "x"}) + assert status == 403 + + +def test_http_403_wrong_secret(server, monkeypatch): + monkeypatch.setenv("KANBAN_RECEIVER_SECRET", "right") + status, body = _post(f"{server}/kanban/card-drop", + {"assignee": "dev", "title": "x"}, + headers={"X-Cron-Secret": "wrong"}) + assert status == 403 + + +def test_http_card_drop_with_secret(server, monkeypatch): + monkeypatch.setenv("KANBAN_RECEIVER_SECRET", "right") + monkeypatch.setattr( + kr, "_run_hermes_kanban", lambda args: _FakeProc(stdout='{"id": "t_live1"}') + ) + status, body = _post(f"{server}/kanban/card-drop", + {"assignee": "equity-analyst", "title": "AVGO"}, + headers={"X-Cron-Secret": "right"}) + assert status == 200 + assert body["id"] == "t_live1" + + +def test_http_comment_with_secret(server, monkeypatch): + monkeypatch.setenv("CRON_SECRET", "s") + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc(stdout="ok")) + status, body = _post(f"{server}/kanban/comment", + {"card_id": "t_live1", "text": "hi"}, + headers={"X-Cron-Secret": "s"}) + assert status == 200 + assert body["commented"] is True + + +def test_http_unknown_route_404(server, monkeypatch): + monkeypatch.setenv("CRON_SECRET", "s") + status, body = _post(f"{server}/nope", {}, headers={"X-Cron-Secret": "s"}) + assert status == 404 From f1c4671d5474efbba30f59c7496bf2aacba05600 Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Wed, 1 Jul 2026 19:55:24 -0400 Subject: [PATCH 2/8] fix(kanban-receiver): drop non-positive goal_max_turns to CLI default A zero/negative goal_max_turns forwarded a stall value to the worker. Clamp: >=1 passes through, non-positive drops the flag (CLI applies its own default), malformed still 400s. Adds coverage for both paths. Addresses gemini-code-assist review on #84. --- scripts/kanban_receiver/kanban_receiver.py | 7 ++++- .../kanban_receiver/test_kanban_receiver.py | 28 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index a22e060846a8..7d305e0eac11 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -155,9 +155,14 @@ def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: gmt = payload.get("goal_max_turns") if gmt is not None: try: - args += ["--goal-max-turns", str(int(gmt))] + val = int(gmt) except (TypeError, ValueError): return 400, {"error": "goal_max_turns must be an integer"} + # Guard against zero/negative-turn loops: a non-positive limit is + # meaningless, so drop the flag and let the CLI apply its own + # default rather than forwarding a value that stalls the worker. + if val >= 1: + args += ["--goal-max-turns", str(val)] # Author the card as the drop's assignee-agnostic origin so the audit trail # shows it arrived over the wire, not from a local human. diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index 888594ca5c1f..80506b83de37 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -111,6 +111,34 @@ def fake_run(args): assert a[a.index("--goal-max-turns") + 1] == "15" +def test_create_card_goal_nonpositive_turns_dropped(monkeypatch): + """A zero/negative goal_max_turns is meaningless: drop the flag and let + the CLI apply its own default rather than forwarding a stall value.""" + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id": "t_1"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + for bad in (0, -5): + captured.clear() + status, _ = kr.create_card( + {"assignee": "equity-analyst", "title": "x", "goal": True, "goal_max_turns": bad} + ) + assert status == 200 + assert "--goal" in captured["args"] + assert "--goal-max-turns" not in captured["args"] + + +def test_create_card_goal_malformed_turns_is_400(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) + status, _ = kr.create_card( + {"assignee": "dev", "title": "x", "goal": True, "goal_max_turns": "lots"} + ) + assert status == 400 + + def test_create_card_bad_priority(monkeypatch): monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) status, body = kr.create_card({"assignee": "dev", "title": "x", "priority": "high"}) From 6ca32f0c013e6703c1a64b2408d7f49fc56447af Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Wed, 1 Jul 2026 19:57:43 -0400 Subject: [PATCH 3/8] fix(kanban-receiver): redact body from exec log + omit "none" assignee Codex P2s: - The exec-log filter dropped only the --body flag token, leaking the following body value (diligence/inbox content) into the launchd stderr log. _redact_args() now strips the flag AND its value. - assignee \"none\" was forwarded as --assignee none, which hermes kanban create stores as a literal lane (create does not canonicalize the sentinel like assign/reassign do), stranding the card in ready. Omit the flag so the card lands genuinely unassigned. Adds coverage for both. --- scripts/kanban_receiver/kanban_receiver.py | 37 ++++++++++++++++--- .../kanban_receiver/test_kanban_receiver.py | 29 +++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index 7d305e0eac11..601131654e5d 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -103,14 +103,31 @@ def _hermes_bin() -> str: return "" # signals: fall back to python -m +_REDACT_FLAGS = {"--body"} + + +def _redact_args(args: list[str]) -> list[str]: + """Return args safe for logging: drop sensitive flag *values* (e.g. the + card ``--body``, which carries diligence/inbox content) so they never land + in the launchd stderr log. Both the flag and its following value go.""" + out: list[str] = [] + skip = False + for a in args: + if skip: + skip = False + continue + if a in _REDACT_FLAGS: + skip = True + continue + out.append(a) + return out + + def _run_hermes_kanban(args: list[str]) -> subprocess.CompletedProcess: """Invoke ``hermes kanban <args>`` inheriting the environment (HERMES_HOME).""" hermes = _hermes_bin() - if hermes: - cmd = [hermes, "kanban", *args] - else: - cmd = [sys.executable, "-m", "hermes_cli.main", "kanban", *args] - LOG.info("exec: %s", " ".join(cmd[:2] + ["kanban"] + [a for a in args if not a.startswith("--body")])) + cmd = [hermes, "kanban", *args] if hermes else [sys.executable, "-m", "hermes_cli.main", "kanban", *args] + LOG.info("exec: %s", " ".join(cmd[:2] + ["kanban"] + _redact_args(args))) return subprocess.run( cmd, text=True, @@ -137,7 +154,15 @@ def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: # `title` is a positional arg on `hermes kanban create`; `--body`/`--assignee` # are flags. Build all flags first, then append `-- <title>` LAST so a title # beginning with a dash can't be parsed as an option. - args = ["create", "--assignee", assignee, "--body", body, "--json"] + args = ["create", "--body", body, "--json"] + + # The "none" sentinel means "unassigned / triage later". `hermes kanban + # create` does NOT canonicalize it the way `assign`/`reassign` do, so + # forwarding `--assignee none` would store the literal lane "none" and + # strand the card in `ready` (no dispatcher serves it). Omit the flag + # instead so the card lands genuinely unassigned. + if assignee != "none": + args += ["--assignee", assignee] dedupe_key = payload.get("dedupe_key") if dedupe_key and str(dedupe_key).strip(): diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index 80506b83de37..948d1c263f12 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -145,6 +145,35 @@ def test_create_card_bad_priority(monkeypatch): assert status == 400 +def test_create_card_none_assignee_omits_flag(monkeypatch): + """The `none` sentinel must NOT be forwarded as `--assignee none` (which + would strand the card in a nonexistent lane); the flag is omitted so the + card lands genuinely unassigned.""" + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id": "t_1"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + status, _ = kr.create_card({"assignee": "none", "title": "triage me"}) + assert status == 200 + assert "--assignee" not in captured["args"] + assert "none" not in captured["args"] + + +def test_redact_args_drops_body_value(): + """The card body carries diligence/inbox content and must never reach the + exec log — both the `--body` flag AND its following value are stripped.""" + args = ["create", "--assignee", "dev", "--body", "SECRET diligence text", "--json", "--", "title"] + redacted = kr._redact_args(args) + assert "--body" not in redacted + assert "SECRET diligence text" not in redacted + # non-sensitive args survive + assert "--assignee" in redacted and "dev" in redacted + assert "title" in redacted + + def test_create_card_cli_failure(monkeypatch): monkeypatch.setattr( kr, "_run_hermes_kanban", lambda args: _FakeProc(returncode=1, stderr="boom") From 50b6747efc1021f9c47f04f032ba9786598470b2 Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Wed, 1 Jul 2026 20:02:25 -0400 Subject: [PATCH 4/8] fix(kanban-receiver): 400 on non-string JSON field values (review item) Addresses claude review item #2 on PR #84: title/assignee/body/card_id/text that arrive as int/object/list previously hit an uncaught AttributeError on .strip() and reset the connection. Now rejected up front with a clean 400. +3 tests (26 total). Items #1 (log redaction) and #3 (omit 'none') already landed in f1c4671d/6ca32f0c. --- scripts/kanban_receiver/kanban_receiver.py | 35 +++++++++++++++---- .../kanban_receiver/test_kanban_receiver.py | 35 ++++++++++++++++--- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index 601131654e5d..e292ab8aedb2 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -137,14 +137,32 @@ def _run_hermes_kanban(args: list[str]) -> subprocess.CompletedProcess: ) +def _opt_str(value: Any) -> Optional[str]: + """Coerce a JSON field to a stripped str, or None if absent/wrong type. + + A client that sends ``title: 5`` or ``body: {...}`` must get a clean 400, + not an uncaught AttributeError that resets the connection. Only real strings + (and None/missing) are accepted; numbers/objects/lists are rejected.""" + if value is None: + return None + if not isinstance(value, str): + return None + return value.strip() + + def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: """Handle a /kanban/card-drop payload -> hermes kanban create. Contract: {assignee, title, body, dedupe_key?, priority?, goal?, goal_max_turns?}. Returns (status_code, json_body).""" - assignee = (payload.get("assignee") or "").strip() - title = (payload.get("title") or "").strip() - body = payload.get("body") or "" + # Reject non-string title/assignee/body up front so a malformed JSON value + # (int, object, list) yields a clean 400 rather than an uncaught exception. + for field in ("assignee", "title", "body"): + if field in payload and payload[field] is not None and not isinstance(payload[field], str): + return 400, {"error": f"{field} must be a string"} + assignee = _opt_str(payload.get("assignee")) or "" + title = _opt_str(payload.get("title")) or "" + body = _opt_str(payload.get("body")) or "" if not assignee or not title: return 400, {"error": "assignee and title are required"} @@ -221,15 +239,18 @@ def comment_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: """Handle a /kanban/comment payload -> hermes kanban comment. Contract: {card_id, text}. Returns (status_code, json_body).""" - card_id = (payload.get("card_id") or "").strip() - text = payload.get("text") or "" - if not card_id or not str(text).strip(): + for field in ("card_id", "text"): + if field in payload and payload[field] is not None and not isinstance(payload[field], str): + return 400, {"error": f"{field} must be a string"} + card_id = _opt_str(payload.get("card_id")) or "" + text = _opt_str(payload.get("text")) or "" + if not card_id or not text: return 400, {"error": "card_id and text are required"} # `hermes kanban comment <task_id> <text...>` — both positional. Put the # optional `--author` first, then `--` and the two positionals, so a text # (or id) starting with a dash can't be misparsed as a flag. - args = ["comment", "--author", "card-drop", "--", card_id, str(text)] + args = ["comment", "--author", "card-drop", "--", card_id, text] try: proc = _run_hermes_kanban(args) except subprocess.TimeoutExpired: diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index 948d1c263f12..ce82e3816f51 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -131,14 +131,41 @@ def fake_run(args): assert "--goal-max-turns" not in captured["args"] -def test_create_card_goal_malformed_turns_is_400(monkeypatch): - monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) - status, _ = kr.create_card( - {"assignee": "dev", "title": "x", "goal": True, "goal_max_turns": "lots"} +def test_create_card_nonpositive_goal_max_turns(monkeypatch): + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id":"t"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + # Non-positive -> flag dropped, CLI default applies (200, no --goal-max-turns). + status, body = kr.create_card( + {"assignee": "dev", "title": "x", "goal": True, "goal_max_turns": 0} + ) + assert status == 200 + assert "--goal" in captured["args"] + assert "--goal-max-turns" not in captured["args"] + # Non-integer -> clean 400. + status, body = kr.create_card( + {"assignee": "dev", "title": "x", "goal": True, "goal_max_turns": "nope"} ) assert status == 400 +def test_create_card_rejects_non_string_fields(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc(stdout='{"id":"t"}')) + assert kr.create_card({"assignee": 5, "title": "x"})[0] == 400 + assert kr.create_card({"assignee": "dev", "title": 5})[0] == 400 + assert kr.create_card({"assignee": "dev", "title": "x", "body": {"a": 1}})[0] == 400 + + +def test_comment_rejects_non_string_fields(monkeypatch): + monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc(stdout="ok")) + assert kr.comment_card({"card_id": 7, "text": "hi"})[0] == 400 + assert kr.comment_card({"card_id": "t_1", "text": {"x": 1}})[0] == 400 + + def test_create_card_bad_priority(monkeypatch): monkeypatch.setattr(kr, "_run_hermes_kanban", lambda args: _FakeProc()) status, body = kr.create_card({"assignee": "dev", "title": "x", "priority": "high"}) From 6960406b04a46e3bc1da14ad1a99b107e772b6c4 Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Wed, 1 Jul 2026 20:06:11 -0400 Subject: [PATCH 5/8] fix(kanban-receiver): 404 on unknown-task comment + redact comment text Codex P2s: - comment_card mapped unknown cards to 502: the CLI emits "kanban: unknown task <id>" (from add_comment ValueError), which the 404 match ("not found"/"no such") missed. Match "unknown task" too; the fabricated test string is replaced with the real CLI output. - _redact_args only stripped --body, so the comment text positional (comment <id> <text>) leaked user content into the exec log. Redact the trailing comment positional while keeping the card id for debuggability. Adds coverage for both plus a 502 real-fault case. --- scripts/kanban_receiver/kanban_receiver.py | 30 ++++++++++++++++--- .../kanban_receiver/test_kanban_receiver.py | 23 +++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index e292ab8aedb2..d61b682e38b2 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -107,11 +107,19 @@ def _hermes_bin() -> str: def _redact_args(args: list[str]) -> list[str]: - """Return args safe for logging: drop sensitive flag *values* (e.g. the - card ``--body``, which carries diligence/inbox content) so they never land - in the launchd stderr log. Both the flag and its following value go.""" + """Return args safe for logging: drop user-supplied content (diligence / + inbox text) so it never lands in the launchd stderr log. + + Two sources of sensitive content: + - flag values after ``--body`` (card body on ``create``); + - the trailing positional ``text`` on ``comment`` (``comment <id> <text>``), + which arrives after the ``--`` separator alongside the safe card id. + """ out: list[str] = [] skip = False + is_comment = bool(args) and args[0] == "comment" + seen_ddash = False + positional_after_ddash = 0 for a in args: if skip: skip = False @@ -119,6 +127,16 @@ def _redact_args(args: list[str]) -> list[str]: if a in _REDACT_FLAGS: skip = True continue + if a == "--": + seen_ddash = True + out.append(a) + continue + # For `comment <id> <text...>`, keep the first positional (the card id) + # but redact everything after it (the user-supplied comment text). + if is_comment and seen_ddash: + positional_after_ddash += 1 + if positional_after_ddash >= 2: + continue out.append(a) return out @@ -260,7 +278,11 @@ def comment_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: LOG.error("kanban comment failed rc=%s stderr=%s", proc.returncode, proc.stderr[-500:]) detail = proc.stderr.strip()[-300:] # Distinguish an unknown card (client error) from a real server fault. - if "not found" in detail.lower() or "no such" in detail.lower(): + # `hermes kanban comment` on a missing id surfaces the CLI's + # `kanban: unknown task <id>` (from add_comment's ValueError), so match + # that alongside the generic phrasings. + low = detail.lower() + if "unknown task" in low or "no such task" in low or "not found" in low or "no such" in low: return 404, {"error": "card not found", "card_id": card_id} return 502, {"error": "kanban comment failed", "detail": detail} diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index ce82e3816f51..ff2fbd2e5d09 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -247,14 +247,35 @@ def test_comment_requires_fields(monkeypatch): def test_comment_unknown_card_is_404(monkeypatch): + # The real CLI surfaces `kanban: unknown task <id>` (from add_comment's + # ValueError), not "not found" — the receiver must map that to 404. monkeypatch.setattr( kr, "_run_hermes_kanban", - lambda args: _FakeProc(returncode=1, stderr="task not found: t_x"), + lambda args: _FakeProc(returncode=1, stderr="kanban: unknown task t_x"), ) status, body = kr.comment_card({"card_id": "t_x", "text": "hi"}) assert status == 404 +def test_comment_real_server_fault_is_502(monkeypatch): + monkeypatch.setattr( + kr, "_run_hermes_kanban", + lambda args: _FakeProc(returncode=1, stderr="kanban: could not initialize database"), + ) + status, _ = kr.comment_card({"card_id": "t_x", "text": "hi"}) + assert status == 502 + + +def test_redact_args_drops_comment_text(): + """The comment `text` positional carries user content and must not reach + the exec log; the card id (first positional) stays for debuggability.""" + args = ["comment", "--author", "card-drop", "--", "t_abc", "SECRET inbox note"] + redacted = kr._redact_args(args) + assert "SECRET inbox note" not in redacted + assert "t_abc" in redacted # card id is safe to log + assert "comment" in redacted + + # -------------------------------------------------------------------------- # secret resolution # -------------------------------------------------------------------------- From 9e4cd0f4d3f8d4a9507e2e3e36636917c98b49a8 Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Wed, 1 Jul 2026 20:12:59 -0400 Subject: [PATCH 6/8] fix(kanban-receiver): redact create title positional from exec log (review nit) Addresses the final claude review item on PR #84: the create positional title (inbox/diligence-derived, same sensitivity class as the already-redacted body) reached /tmp stderr verbatim. _redact_args now redacts every trailing positional after -- except the comment card id, and keeps flag names with a <redacted> value marker. 28 tests pass. --- scripts/kanban_receiver/kanban_receiver.py | 25 +++++++++++++------ .../kanban_receiver/test_kanban_receiver.py | 15 ++++++----- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index d61b682e38b2..2de54c4bfd0f 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -110,10 +110,16 @@ def _redact_args(args: list[str]) -> list[str]: """Return args safe for logging: drop user-supplied content (diligence / inbox text) so it never lands in the launchd stderr log. - Two sources of sensitive content: + Sensitive content sources: - flag values after ``--body`` (card body on ``create``); + - the trailing positional ``title`` on ``create`` (``-- <title>``), which is + inbox/diligence-derived (same sensitivity class as the body); - the trailing positional ``text`` on ``comment`` (``comment <id> <text>``), - which arrives after the ``--`` separator alongside the safe card id. + which arrives after ``--`` alongside the safe card id. + + Rule: everything after ``--`` is a positional. For ``comment`` the FIRST + such positional (the card id) is safe to log; every other trailing + positional is redacted to ``<redacted>``. """ out: list[str] = [] skip = False @@ -126,17 +132,22 @@ def _redact_args(args: list[str]) -> list[str]: continue if a in _REDACT_FLAGS: skip = True + out.append(a) # keep the flag name; its value is dropped + out.append("<redacted>") continue if a == "--": seen_ddash = True out.append(a) continue - # For `comment <id> <text...>`, keep the first positional (the card id) - # but redact everything after it (the user-supplied comment text). - if is_comment and seen_ddash: + if seen_ddash: positional_after_ddash += 1 - if positional_after_ddash >= 2: - continue + # Keep only the card id (first positional on `comment`); redact + # every other trailing positional (comment text, create title). + if is_comment and positional_after_ddash == 1: + out.append(a) + else: + out.append("<redacted>") + continue out.append(a) return out diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index ff2fbd2e5d09..fd0115b268f3 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -189,16 +189,19 @@ def fake_run(args): assert "none" not in captured["args"] -def test_redact_args_drops_body_value(): - """The card body carries diligence/inbox content and must never reach the - exec log — both the `--body` flag AND its following value are stripped.""" - args = ["create", "--assignee", "dev", "--body", "SECRET diligence text", "--json", "--", "title"] +def test_redact_args_drops_body_and_title(): + """The card body AND the title positional carry diligence/inbox content and + must never reach the exec log; non-sensitive flags survive.""" + args = ["create", "--assignee", "dev", "--body", "SECRET diligence text", "--json", "--", "SECRET title"] redacted = kr._redact_args(args) - assert "--body" not in redacted assert "SECRET diligence text" not in redacted + assert "SECRET title" not in redacted + # the flag NAME is kept (structure visible) but the value is redacted + assert "--body" in redacted + assert "<redacted>" in redacted # non-sensitive args survive assert "--assignee" in redacted and "dev" in redacted - assert "title" in redacted + assert "--json" in redacted def test_create_card_cli_failure(monkeypatch): From 98453f290313bd6ba670a5c7444d2aae4b981ecb Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Fri, 3 Jul 2026 02:25:51 -0400 Subject: [PATCH 7/8] fix(kanban-receiver): prefer repo venv before system Python launchd from a source checkout where hermes is installed only in the repo venv (not /usr/local/bin or /opt/homebrew/bin) execd system python3, so _hermes_bin()'s `which hermes` failed and fell back to `sys.executable -m hermes_cli.main` under the wrong interpreter, breaking authenticated card writes. Resolve $REPO_ROOT/.venv (or venv) first: prepend its bin to PATH and exec its python3 so both the CLI resolution and dependency imports use the venv. (codex P2) --- scripts/kanban_receiver/run_receiver.sh | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/scripts/kanban_receiver/run_receiver.sh b/scripts/kanban_receiver/run_receiver.sh index 808a17fc0b35..a941147e5556 100755 --- a/scripts/kanban_receiver/run_receiver.sh +++ b/scripts/kanban_receiver/run_receiver.sh @@ -29,8 +29,22 @@ if [[ -f "$ENV_FILE" ]]; then done fi -# Prefer the installed `hermes` on PATH; fall back to the repo module. +# Prefer the repo venv (a source checkout may install `hermes` only there), +# then the installed `hermes` on PATH; fall back to the repo module. +# Prepending the venv bin lets _hermes_bin()'s `which hermes` resolve it and +# runs kanban_receiver.py under the venv interpreter so CLI imports succeed. export PATH="/usr/local/bin:/opt/homebrew/bin:$PATH" +PY="" +for venv in "$REPO_ROOT/.venv" "$REPO_ROOT/venv"; do + if [[ -x "$venv/bin/python3" ]]; then + export PATH="$venv/bin:$PATH" + PY="$venv/bin/python3" + break + fi +done cd "$REPO_ROOT" +if [[ -n "$PY" ]]; then + exec "$PY" scripts/kanban_receiver/kanban_receiver.py +fi exec /usr/bin/env python3 scripts/kanban_receiver/kanban_receiver.py From 7140991f4dc267cf932d605e08d1bc5b412dc06c Mon Sep 17 00:00:00 2001 From: exiao <exiao3@gmail.com> Date: Fri, 3 Jul 2026 02:48:53 -0400 Subject: [PATCH 8/8] fix(kanban-receiver): validate goal as boolean; log to ~/.hermes/logs Address two Codex P2s on PR #84: - kanban_receiver.py: a producer serializing `goal` as the string "false"/"0" still passed the bare truthiness check, dispatching an ordinary card as a multi-turn goal loop and burning the goal budget. Add `_is_goal()` that accepts a JSON boolean and treats falsey string tokens ("", false, 0, no, off) as False. Regression test proves red-before/green-after. - plist: StandardOut/ErrorPath pointed at /tmp/hermes, whose parent dir is not guaranteed to exist on a clean account, so `launchctl load` could fail before the receiver starts. Point at __HOME__/.hermes/logs (always present via the __HOME__ substitution). README log path updated to match. --- scripts/kanban_receiver/README.md | 2 +- .../ai.hermes.kanban-receiver.plist | 4 +-- scripts/kanban_receiver/kanban_receiver.py | 19 ++++++++++++- .../kanban_receiver/test_kanban_receiver.py | 27 +++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/scripts/kanban_receiver/README.md b/scripts/kanban_receiver/README.md index c96e1f5fa37b..98b6c7374b9f 100644 --- a/scripts/kanban_receiver/README.md +++ b/scripts/kanban_receiver/README.md @@ -81,7 +81,7 @@ launchctl load ~/Library/LaunchAgents/ai.hermes.kanban-receiver.plist curl -s http://127.0.0.1:8646/health ``` -Logs: `/tmp/hermes/kanban-receiver-stdout.log` + `-stderr.log`. +Logs: `~/.hermes/logs/kanban-receiver-stdout.log` + `-stderr.log`. ## Test diff --git a/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist b/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist index cc0f5ccf00bd..e8ff27e7bbf4 100644 --- a/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist +++ b/scripts/kanban_receiver/ai.hermes.kanban-receiver.plist @@ -33,8 +33,8 @@ <key>KeepAlive</key> <true/> <key>StandardOutPath</key> - <string>/tmp/hermes/kanban-receiver-stdout.log</string> + <string>__HOME__/.hermes/logs/kanban-receiver-stdout.log</string> <key>StandardErrorPath</key> - <string>/tmp/hermes/kanban-receiver-stderr.log</string> + <string>__HOME__/.hermes/logs/kanban-receiver-stderr.log</string> </dict> </plist> diff --git a/scripts/kanban_receiver/kanban_receiver.py b/scripts/kanban_receiver/kanban_receiver.py index 2de54c4bfd0f..c47051c6c0fc 100755 --- a/scripts/kanban_receiver/kanban_receiver.py +++ b/scripts/kanban_receiver/kanban_receiver.py @@ -179,6 +179,23 @@ def _opt_str(value: Any) -> Optional[str]: return value.strip() +def _is_goal(value: Any) -> bool: + """Decide whether ``goal`` enables goal mode, accepting only real booleans. + + A producer that serializes ``goal`` as a string like ``"false"`` or ``"0"`` + is still truthy under a bare ``if payload.get("goal")`` check, which would + dispatch an ordinary card as a multi-turn goal loop and burn the goal + budget. Accept a JSON boolean directly; for string forms, treat the usual + falsey tokens ("", "false", "0", "no", "off") as False and everything else + True. Non-bool/non-str values (numbers, objects) fall back to truthiness. + """ + if isinstance(value, bool): + return value + if isinstance(value, str): + return value.strip().lower() not in ("", "false", "0", "no", "off") + return bool(value) + + def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: """Handle a /kanban/card-drop payload -> hermes kanban create. @@ -222,7 +239,7 @@ def create_card(payload: dict[str, Any]) -> tuple[int, dict[str, Any]]: except (TypeError, ValueError): return 400, {"error": "priority must be an integer"} - if payload.get("goal"): + if _is_goal(payload.get("goal")): args.append("--goal") gmt = payload.get("goal_max_turns") if gmt is not None: diff --git a/scripts/kanban_receiver/test_kanban_receiver.py b/scripts/kanban_receiver/test_kanban_receiver.py index fd0115b268f3..5ef392c2155a 100644 --- a/scripts/kanban_receiver/test_kanban_receiver.py +++ b/scripts/kanban_receiver/test_kanban_receiver.py @@ -111,6 +111,33 @@ def fake_run(args): assert a[a.index("--goal-max-turns") + 1] == "15" +def test_create_card_goal_false_string_does_not_enable_goal(monkeypatch): + """A producer serializing goal as the STRING "false"/"0"/"no" must NOT + enable goal mode -- otherwise an ordinary card is dispatched as a multi-turn + goal loop and burns the goal budget. Only a JSON boolean true (or a genuine + truthy string) appends --goal.""" + captured = {} + + def fake_run(args): + captured["args"] = args + return _FakeProc(stdout='{"id": "t_1"}') + + monkeypatch.setattr(kr, "_run_hermes_kanban", fake_run) + for falsey in ("false", "False", "0", "no", "off", ""): + captured.clear() + status, _ = kr.create_card( + {"assignee": "dev", "title": "x", "goal": falsey} + ) + assert status == 200 + assert "--goal" not in captured["args"], f"goal={falsey!r} should not enable goal mode" + + # A real boolean true and a genuine truthy string still enable it. + for truthy in (True, "true", "1", "yes"): + captured.clear() + kr.create_card({"assignee": "dev", "title": "x", "goal": truthy}) + assert "--goal" in captured["args"], f"goal={truthy!r} should enable goal mode" + + def test_create_card_goal_nonpositive_turns_dropped(monkeypatch): """A zero/negative goal_max_turns is meaningless: drop the flag and let the CLI apply its own default rather than forwarding a stall value."""