From 15e80366187418e4e5251d2a1f48c0885c940c73 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 17:58:28 -0400 Subject: [PATCH 01/16] feat(kanban): add Modal memo evaluator lane --- hermes_cli/config.py | 4 + hermes_cli/kanban_db.py | 16 +- hermes_cli/kanban_modal.py | 254 ++++++++++++++++++ hermes_cli/kanban_modal_worker.py | 113 ++++++++ tests/hermes_cli/test_kanban_modal.py | 134 +++++++++ .../features/kanban-worker-lanes.md | 14 + 6 files changed, 534 insertions(+), 1 deletion(-) create mode 100644 hermes_cli/kanban_modal.py create mode 100644 hermes_cli/kanban_modal_worker.py create mode 100644 tests/hermes_cli/test_kanban_modal.py diff --git a/hermes_cli/config.py b/hermes_cli/config.py index df7b20707f15..3024fb593d9b 100644 --- a/hermes_cli/config.py +++ b/hermes_cli/config.py @@ -1543,6 +1543,10 @@ def _ensure_hermes_home_managed(home: Path): # behaviour — e.g. for a profile that prefers explicit # ``kanban_notify-subscribe`` calls per task. "auto_subscribe_on_create": True, + # Worker execution backend per lane. Only ``memo-evaluator`` supports + # ``modal`` in this phase; every other lane remains local even if a + # stray config value names Modal. + "worker_backends": {"memo-evaluator": "local"}, }, # Anthropic prompt caching (Claude via OpenRouter or native Anthropic API). diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index d765e52bd5d4..398b199301fd 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -9739,7 +9739,7 @@ def _dispatch_once_locked( if claimed.workspace_kind == "worktree": set_branch_name(conn, claimed.id, resolved_branch_name or (claimed.branch_name or "").strip() or f"wt/{claimed.id}") _maybe_emit_scratch_tip(conn, claimed.id, claimed.workspace_kind) - _spawn = spawn_fn if spawn_fn is not None else _default_spawn + _spawn = spawn_fn if spawn_fn is not None else _configured_worker_spawn try: # Back-compat: older spawn_fn signatures accept only # (task, workspace). Test stubs in the suite rely on that. @@ -10504,6 +10504,20 @@ def _default_spawn( return proc.pid +def _configured_worker_spawn( + task: Task, + workspace: str, + *, + board: Optional[str] = None, +) -> Optional[int]: + """Route the narrowly opted-in Modal lane; retain local spawn otherwise.""" + from hermes_cli.kanban_modal import resolve_worker_backend, spawn_modal_worker + + if resolve_worker_backend(task.assignee, _load_kanban_cfg()) == "modal": + return spawn_modal_worker(task, workspace, board=board) + return _default_spawn(task, workspace, board=board) + + # --------------------------------------------------------------------------- # Long-lived dispatcher daemon # --------------------------------------------------------------------------- diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py new file mode 100644 index 000000000000..6f47c02320ca --- /dev/null +++ b/hermes_cli/kanban_modal.py @@ -0,0 +1,254 @@ +"""Local Modal shim for the Kanban memo-evaluator lane. + +The dispatcher runs the shim locally so the board database remains the sole +lifecycle authority. The shim may send a bounded task brief to Modal, but it +never exposes a Kanban DB path or lifecycle credentials to the remote worker. +""" +from __future__ import annotations + +import argparse +import json +import logging +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import Any + +_MODAL_LANE = "memo-evaluator" +_MAX_MODAL_BRIEF_CHARS = 64_000 +_log = logging.getLogger(__name__) + + +def resolve_worker_backend(assignee: str | None, kanban_config: dict[str, Any]) -> str: + """Return the configured backend, restricting Modal to memo-evaluator. + + Every lane is local by default. Treating a non-memo lane's ``modal`` value + as local prevents an accidental configuration edit from widening this + phase-one integration before its remote workspace contract exists. + """ + if (assignee or "").strip().lower() != _MODAL_LANE: + return "local" + raw_backends = kanban_config.get("worker_backends", {}) + configured = raw_backends.get(_MODAL_LANE, "local") if isinstance(raw_backends, dict) else "local" + return "modal" if str(configured).strip().lower() == "modal" else "local" + + +def _audit_metadata(result: dict[str, Any]) -> dict[str, str]: + call_id = result.get("modal_call_id") + log_url = result.get("modal_log_url") + if not isinstance(call_id, str) or not call_id.strip(): + raise ValueError("Modal completion is missing modal_call_id") + if not isinstance(log_url, str) or not log_url.strip(): + raise ValueError("Modal completion is missing modal_log_url") + return { + "modal_call_id": call_id.strip(), + "modal_log_url": log_url.strip(), + } + + +def apply_modal_result( + conn: Any, + task_id: str, + result: dict[str, Any], + *, + expected_run_id: int | None, +) -> bool: + """Apply a remote result through the local Kanban lifecycle APIs only.""" + from hermes_cli import kanban_db as kb + + outcome = result.get("outcome") + if outcome == "complete": + audit = _audit_metadata(result) + summary = result.get("summary") + if not isinstance(summary, str) or not summary.strip(): + raise ValueError("Modal completion is missing summary") + remote_metadata = result.get("metadata") + metadata = dict(remote_metadata) if isinstance(remote_metadata, dict) else {} + metadata.update(audit) + kb.add_comment( + conn, + task_id, + "modal-shim", + f"Modal audit: call {audit['modal_call_id']} — {audit['modal_log_url']}", + ) + return kb.complete_task( + conn, + task_id, + summary=summary.strip(), + metadata=metadata, + expected_run_id=expected_run_id, + ) + + if outcome == "block": + reason = result.get("reason") + if not isinstance(reason, str) or not reason.strip(): + raise ValueError("Modal block is missing reason") + kind = result.get("kind") + if kind is not None and not isinstance(kind, str): + raise ValueError("Modal block kind must be a string") + return kb.block_task( + conn, + task_id, + reason=reason.strip(), + kind=kind, + expected_run_id=expected_run_id, + ) + + raise ValueError("Modal result outcome must be 'complete' or 'block'") + + +def _modal_runner_path() -> str: + runner = Path(__file__).with_name("kanban_modal_worker.py") + if not runner.is_file(): + raise RuntimeError(f"Modal worker script is missing: {runner}") + return str(runner) + + +def spawn_modal_worker(task: Any, workspace: str, *, board: str | None = None) -> int: + """Launch a local shim that owns Modal invocation and local DB writes.""" + import subprocess + from hermes_cli import kanban_db as kb + + env = dict(os.environ) + env["HERMES_KANBAN_TASK"] = task.id + env["HERMES_KANBAN_WORKSPACE"] = workspace + env["HERMES_KANBAN_DB"] = str(kb.kanban_db_path(board=board)) + env["HERMES_KANBAN_WORKSPACES_ROOT"] = str(kb.workspaces_root(board=board)) + env["HERMES_KANBAN_BOARD"] = kb._normalize_board_slug(board) or kb.get_current_board() + if task.current_run_id is not None: + env["HERMES_KANBAN_RUN_ID"] = str(task.current_run_id) + + log_dir = kb.worker_logs_dir(board=board) + log_dir.mkdir(parents=True, exist_ok=True) + log_path = log_dir / f"{task.id}.log" + rotate_bytes, backup_count = kb.worker_log_rotation_config() + kb._rotate_worker_log(log_path, rotate_bytes, backup_count) + log_f = open(log_path, "ab") + try: + proc = subprocess.Popen( # noqa: S603 -- fixed interpreter/module argv + [sys.executable, "-m", "hermes_cli.kanban_modal", "--task-id", task.id], + cwd=workspace if os.path.isdir(workspace) else None, + stdin=subprocess.DEVNULL, + stdout=log_f, + stderr=subprocess.STDOUT, + env=env, + start_new_session=True, + creationflags=subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0, + ) + except Exception: + log_f.close() + raise + return proc.pid + + +def _build_modal_request(task_id: str, workspace: str) -> tuple[dict[str, Any], int | None]: + """Build the remote-safe payload without leaking local board credentials.""" + from hermes_cli import kanban_db as kb + + with kb.connect_closing() as conn: + task = kb.get_task(conn, task_id) + if task is None or task.status != "running": + raise ValueError(f"Kanban task {task_id} is not running") + if (task.assignee or "").strip().lower() != _MODAL_LANE: + raise ValueError(f"Kanban task {task_id} is not assigned to {_MODAL_LANE}") + brief = kb.build_worker_context(conn, task.id) + if len(brief) > _MAX_MODAL_BRIEF_CHARS: + raise ValueError( + f"Kanban task brief is too large for the Modal invocation " + f"({_MAX_MODAL_BRIEF_CHARS} character limit)" + ) + return { + "task_id": task.id, + "brief": brief, + "workspace": workspace, + }, task.current_run_id + + +def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[str, Any]: + """Synchronously invoke the Modal app and parse its structured response.""" + modal_bin = shutil.which("modal") + if modal_bin is None: + raise RuntimeError("Modal CLI is not installed or not on PATH") + fd, result_name = tempfile.mkstemp(prefix="hermes-kanban-modal-", suffix=".json") + os.close(fd) + result_path = Path(result_name) + try: + completed = subprocess.run( # noqa: S603 -- fixed CLI plus serialized request + [ + modal_bin, + "run", + "--write-result", + str(result_path), + _modal_runner_path(), + "--request-json", + json.dumps(request, separators=(",", ":")), + ], + check=False, + capture_output=True, + text=True, + timeout=timeout, + ) + if completed.returncode != 0: + raise RuntimeError(f"Modal run exited with status {completed.returncode}") + try: + parsed = json.loads(result_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + raise RuntimeError("Modal run did not return a structured result") from exc + if not isinstance(parsed, dict): + raise RuntimeError("Modal run returned a non-object result") + return parsed + finally: + result_path.unlink(missing_ok=True) + + +def run_modal_shim(task_id: str, workspace: str) -> bool: + """Run Modal then map its response to a local Kanban completion or block.""" + from hermes_cli import kanban_db as kb + + expected_run_id: int | None = None + try: + request, expected_run_id = _build_modal_request(task_id, workspace) + timeout = None + with kb.connect_closing() as conn: + task = kb.get_task(conn, task_id) + if task is not None and task.max_runtime_seconds: + timeout = int(task.max_runtime_seconds) + result = _run_modal(request, timeout=timeout) + with kb.connect_closing() as conn: + return apply_modal_result( + conn, task_id, result, expected_run_id=expected_run_id + ) + except Exception as exc: + _log.error("modal Kanban shim failed for %s: %s", task_id, exc) + with kb.connect_closing() as conn: + if expected_run_id is None: + task = kb.get_task(conn, task_id) + expected_run_id = task.current_run_id if task else None + kb.add_comment( + conn, + task_id, + "modal-shim", + "Modal worker invocation failed; see the worker log for details.", + ) + return kb.block_task( + conn, + task_id, + reason="Modal worker invocation failed; see the worker log.", + kind="transient", + expected_run_id=expected_run_id, + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Run a Kanban memo evaluator via Modal") + parser.add_argument("--task-id", required=True) + args = parser.parse_args(argv) + workspace = os.environ.get("HERMES_KANBAN_WORKSPACE", "") + return 0 if run_modal_shim(args.task_id, workspace) else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py new file mode 100644 index 000000000000..585fa8fd2f58 --- /dev/null +++ b/hermes_cli/kanban_modal_worker.py @@ -0,0 +1,113 @@ +"""Modal entrypoint for the isolated Kanban ``memo-evaluator`` lane. + +Run only through ``hermes_cli.kanban_modal``. The local shim owns every Kanban +DB lifecycle transition; this app receives a serialized brief and returns a +structured result only. +""" +from __future__ import annotations + +import json +import os +import subprocess +from pathlib import Path +from typing import Any + +import modal + +APP_NAME = "hermes-kanban-memo-evaluator" +PROFILE_SOURCE = Path( + os.environ.get( + "HERMES_MODAL_MEMO_EVALUATOR_PROFILE", + "~/.hermes/profiles/memo-evaluator", + ) +).expanduser() +SOUL_SOURCE = PROFILE_SOURCE / "SOUL.md" +SKILLS_SOURCE = PROFILE_SOURCE / "skills" +PROXY_BASE_URL = "https://proxy.getbloom.app" +PROXY_SECRET = modal.Secret.from_name( + "bloom-llm-proxy", required_keys=["OPENAI_API_KEY"] +) + +if not SOUL_SOURCE.is_file() or not SKILLS_SOURCE.is_dir(): + raise RuntimeError( + "Set HERMES_MODAL_MEMO_EVALUATOR_PROFILE to a profile containing SOUL.md and skills/." + ) + +image = ( + modal.Image.debian_slim(python_version="3.13") + .pip_install("hermes-agent>=0.18.2,<0.19") + .add_local_file(SOUL_SOURCE, "/opt/memo-evaluator/SOUL.md") + .add_local_dir(SKILLS_SOURCE, "/opt/memo-evaluator/skills") +) +app = modal.App(APP_NAME) + + +def _block(reason: str, *, kind: str = "transient") -> dict[str, Any]: + return {"outcome": "block", "reason": reason, "kind": kind} + + +def _parse_worker_result(text: str) -> dict[str, Any]: + result = json.loads(text.strip()) + if not isinstance(result, dict): + raise ValueError("worker result must be a JSON object") + outcome = result.get("outcome") + if outcome == "complete" and isinstance(result.get("summary"), str): + return result + if outcome == "block" and isinstance(result.get("reason"), str): + return result + raise ValueError("worker result must declare a complete or block outcome") + + +@app.function( + image=image, + secrets=[PROXY_SECRET], + timeout=3600, + env={ + "HERMES_HOME": "/opt/memo-evaluator", + "HERMES_INFERENCE_PROVIDER": "custom", + "OPENAI_BASE_URL": PROXY_BASE_URL, + }, +) +def evaluate_memo(request_json: str) -> str: + """Evaluate a brief remotely without a Kanban DB or local workspace mount.""" + try: + request = json.loads(request_json) + brief = request["brief"] + if not isinstance(brief, str) or not brief.strip(): + return json.dumps(_block("Modal request is missing a task brief.", kind="capability")) + except (KeyError, TypeError, json.JSONDecodeError): + return json.dumps(_block("Modal request is malformed.", kind="capability")) + + prompt = """You are the memo-evaluator lane. Evaluate the supplied Kanban task. +You are running remotely with no Kanban database or local workspace mount. Do not +claim, complete, block, or edit a Kanban card yourself. Return exactly one JSON +object and no markdown: either +{"outcome":"complete","summary":"...","metadata":{...}} +or {"outcome":"block","reason":"...","kind":"needs_input|capability|transient"}. + +Task brief follows: +""" + brief + completed = subprocess.run( + ["hermes", "--cli", "chat", "-q", prompt], + check=False, + capture_output=True, + text=True, + ) + if completed.returncode != 0: + return json.dumps(_block("Remote memo evaluator failed; inspect the Modal call logs.")) + try: + return json.dumps(_parse_worker_result(completed.stdout)) + except (ValueError, json.JSONDecodeError): + return json.dumps( + _block("Remote memo evaluator returned an invalid structured result.", kind="transient") + ) + + +@app.local_entrypoint() +def main(request_json: str) -> str: + """Return the remote result plus the FunctionCall audit handle to the shim.""" + call = evaluate_memo.spawn(request_json) + result = json.loads(call.get()) + result["modal_call_id"] = call.object_id + result["modal_log_url"] = call.get_dashboard_url() + return json.dumps(result) diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py new file mode 100644 index 000000000000..58c0fec6440e --- /dev/null +++ b/tests/hermes_cli/test_kanban_modal.py @@ -0,0 +1,134 @@ +from __future__ import annotations + +import os +from types import SimpleNamespace + +import pytest + + +def test_memo_evaluator_is_the_only_lane_that_can_use_modal(): + from hermes_cli import kanban_modal + + assert kanban_modal.resolve_worker_backend("memo-evaluator", {}) == "local" + assert kanban_modal.resolve_worker_backend( + "memo-evaluator", {"worker_backends": {"memo-evaluator": "modal"}} + ) == "modal" + assert kanban_modal.resolve_worker_backend( + "dev", {"worker_backends": {"dev": "modal"}} + ) == "local" + + +def test_modal_completion_is_written_locally_with_audit_metadata(): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + + assert kanban_modal.apply_modal_result( + conn, + task_id, + { + "outcome": "complete", + "summary": "Memo passed the rubric.", + "metadata": {"score": 8}, + "modal_call_id": "fc-123", + "modal_log_url": "https://modal.com/apps/example/logs/fc-123", + }, + expected_run_id=task.current_run_id, + ) + + completed = kb.get_task(conn, task_id) + assert completed is not None and completed.status == "done" + run = kb.list_runs(conn, task_id)[-1] + assert run.metadata == { + "score": 8, + "modal_call_id": "fc-123", + "modal_log_url": "https://modal.com/apps/example/logs/fc-123", + } + assert "fc-123" in kb.list_comments(conn, task_id)[-1].body + + +def test_configured_spawn_routes_only_memo_evaluator_to_modal(monkeypatch): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + calls = [] + monkeypatch.setattr( + kanban_modal, + "spawn_modal_worker", + lambda task, workspace, *, board=None: calls.append((task.assignee, workspace, board)) or 71, + ) + monkeypatch.setattr(kb, "_load_kanban_cfg", lambda: {"worker_backends": {"memo-evaluator": "modal"}}) + monkeypatch.setattr(kb, "_default_spawn", lambda *_args, **_kwargs: 72) + + memo_task = SimpleNamespace(assignee="memo-evaluator") + dev_task = SimpleNamespace(assignee="dev") + assert kb._configured_worker_spawn(memo_task, "/tmp/memo", board="test-board") == 71 + assert kb._configured_worker_spawn(dev_task, "/tmp/dev", board="test-board") == 72 + assert calls == [("memo-evaluator", "/tmp/memo", "test-board")] + + +def test_modal_request_serializes_worker_brief_and_comments_without_board_env(): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task( + conn, + title="grade memo", + body="Evaluate the attached investment memo.", + assignee="memo-evaluator", + ) + kb.add_comment(conn, task_id, "reviewer", "Use the current evidence rubric.") + assert kb.claim_task(conn, task_id) is not None + + request, _run_id = kanban_modal._build_modal_request(task_id, "/tmp/workspace") + + assert request["workspace"] == "/tmp/workspace" + assert "Evaluate the attached investment memo." in request["brief"] + assert "Use the current evidence rubric." in request["brief"] + assert "HERMES_KANBAN_DB" not in request + + +def test_modal_request_rejects_an_oversized_worker_brief(monkeypatch): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + assert kb.claim_task(conn, task_id) is not None + + monkeypatch.setattr( + kb, + "build_worker_context", + lambda *_args: "x" * (kanban_modal._MAX_MODAL_BRIEF_CHARS + 1), + ) + with pytest.raises(ValueError, match="too large"): + kanban_modal._build_modal_request(task_id, "/tmp/workspace") + + +def test_modal_cli_result_is_consumed_from_the_write_result_file(monkeypatch, tmp_path): + from hermes_cli import kanban_modal + + fake_modal = tmp_path / "modal" + fake_modal.write_text( + "#!/usr/bin/env python3\n" + "import json, pathlib, sys\n" + "out = pathlib.Path(sys.argv[sys.argv.index('--write-result') + 1])\n" + "out.write_text(json.dumps({'outcome': 'complete', 'summary': 'done', " + "'modal_call_id': 'fc-test', 'modal_log_url': 'https://modal.test/log'}))\n", + encoding="utf-8", + ) + fake_modal.chmod(0o755) + monkeypatch.setenv("PATH", f"{tmp_path}{os.pathsep}{os.environ.get('PATH', '')}") + + result = kanban_modal._run_modal({"task_id": "t_test", "brief": "grade this"}) + + assert result["modal_call_id"] == "fc-test" + assert result["modal_log_url"] == "https://modal.test/log" diff --git a/website/docs/user-guide/features/kanban-worker-lanes.md b/website/docs/user-guide/features/kanban-worker-lanes.md index 69f879c6b113..7cf4499fe64a 100644 --- a/website/docs/user-guide/features/kanban-worker-lanes.md +++ b/website/docs/user-guide/features/kanban-worker-lanes.md @@ -84,6 +84,20 @@ The shape every kanban worker takes today: the assignee is a profile name, the d When you create profiles for your fleet, choose names that match the *role* you want the orchestrator to route to. The orchestrator (when there is one) discovers your profile names via `hermes profile list` — there's no fixed roster the system assumes (the orchestrator side of the contract is part of the injected `KANBAN_GUIDANCE`). +### Modal memo-evaluator lane (opt-in) + +`memo-evaluator` is the only profile lane with a supported remote backend. It remains local by default. To send that lane through Modal, change one config value: + +```yaml +kanban: + worker_backends: + memo-evaluator: modal +``` + +The dispatcher still starts a local shim. The shim serializes the bounded worker brief (including comments), runs `modal run` synchronously, and applies the returned completion or block through the local Kanban database. The remote container never receives Kanban database paths or lifecycle credentials. A successful completion records the Modal function-call id and dashboard log URL in both run metadata and a `modal-shim` comment. + +The Modal image bakes only the `memo-evaluator` profile's `SOUL.md` and `skills/`; it reads `OPENAI_API_KEY` from the named `bloom-llm-proxy` Modal secret and routes LLM calls through `https://proxy.getbloom.app`. Before enabling the backend, ensure that profile assets exist locally and that the Modal secret has been provisioned. Values other than `memo-evaluator` remain local, even if a config entry asks for Modal. + ### Orchestrator profile lane A specialisation of the profile lane: an orchestrator is a Hermes profile whose toolset includes `kanban` but excludes `terminal` / `file` / `code` / `web` for implementation. Its job is decomposing a high-level goal into child tasks via `kanban_create` + `kanban_link` and stepping back. The orchestrator skill encodes the anti-temptation rules. From 8c4121a8d176b4cb624a7865fcca1b481551b476 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 18:20:35 -0400 Subject: [PATCH 02/16] fix(kanban): wire Modal memo evaluator to Anthropic proxy --- hermes_cli/kanban_modal_worker.py | 99 ++++++++++++++++++++++----- tests/hermes_cli/test_kanban_modal.py | 54 +++++++++++++++ 2 files changed, 137 insertions(+), 16 deletions(-) diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 585fa8fd2f58..6ee518a75497 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -23,22 +23,42 @@ ).expanduser() SOUL_SOURCE = PROFILE_SOURCE / "SOUL.md" SKILLS_SOURCE = PROFILE_SOURCE / "skills" -PROXY_BASE_URL = "https://proxy.getbloom.app" +# The memo-evaluator runs a Claude model through the shared billing proxy, which +# only speaks the Anthropic Messages API. The proxy hostname and its inbound gate +# key both live in the existing ``research-proxy`` Modal secret (ANTHROPIC_BASE_URL +# + ANTHROPIC_API_KEY, where the key equals the proxy gate key — Hermes sends it as +# ``x-api-key`` and the proxy strips it and injects the real OAuth subscription). PROXY_SECRET = modal.Secret.from_name( - "bloom-llm-proxy", required_keys=["OPENAI_API_KEY"] + "research-proxy", + required_keys=["ANTHROPIC_BASE_URL", "ANTHROPIC_API_KEY"], ) +# Model the memo-evaluator profile is pinned to (see its config.yaml). Passed +# explicitly because the image bakes only SOUL.md + skills/, not a config.yaml, +# so there is no on-disk model default inside the container. +MEMO_EVALUATOR_MODEL = "claude-fable-5" +MEMO_EVALUATOR_PROVIDER = "anthropic" + +# The profile source (SOUL.md + skills/) is only present on the machine that +# builds/launches the app; inside the Modal container the module is re-imported +# with only the baked ``/opt/memo-evaluator`` payload, so these local paths do +# not exist. Guard the source check and the ``add_local_*`` mounts on +# ``modal.is_local()`` — validating the source at container-import time crashes +# every remote run. +if modal.is_local(): + if not SOUL_SOURCE.is_file() or not SKILLS_SOURCE.is_dir(): + raise RuntimeError( + "Set HERMES_MODAL_MEMO_EVALUATOR_PROFILE to a profile containing SOUL.md and skills/." + ) -if not SOUL_SOURCE.is_file() or not SKILLS_SOURCE.is_dir(): - raise RuntimeError( - "Set HERMES_MODAL_MEMO_EVALUATOR_PROFILE to a profile containing SOUL.md and skills/." - ) - -image = ( - modal.Image.debian_slim(python_version="3.13") - .pip_install("hermes-agent>=0.18.2,<0.19") - .add_local_file(SOUL_SOURCE, "/opt/memo-evaluator/SOUL.md") - .add_local_dir(SKILLS_SOURCE, "/opt/memo-evaluator/skills") +_base_image = modal.Image.debian_slim(python_version="3.13").pip_install( + "hermes-agent>=0.18.2,<0.19" ) +if modal.is_local(): + image = _base_image.add_local_file( + SOUL_SOURCE, "/opt/memo-evaluator/SOUL.md" + ).add_local_dir(SKILLS_SOURCE, "/opt/memo-evaluator/skills") +else: + image = _base_image app = modal.App(APP_NAME) @@ -46,8 +66,46 @@ def _block(reason: str, *, kind: str = "transient") -> dict[str, Any]: return {"outcome": "block", "reason": reason, "kind": kind} +def _extract_last_json_object(text: str) -> str: + """Return the last balanced top-level JSON object in ``text``. + + Quiet-mode ``hermes chat -Q`` writes only the final response to stdout, but + a stray startup line (an interpreter warning, a security-scanner notice) can + still precede it. Scanning for the last balanced ``{...}`` recovers the model + verdict without assuming the whole stream is pure JSON. + """ + depth = 0 + start = -1 + candidates: list[str] = [] + in_string = False + escape = False + for i, ch in enumerate(text): + if in_string: + if escape: + escape = False + elif ch == "\\": + escape = True + elif ch == '"': + in_string = False + continue + if ch == '"': + in_string = True + elif ch == "{": + if depth == 0: + start = i + depth += 1 + elif ch == "}": + if depth > 0: + depth -= 1 + if depth == 0 and start >= 0: + candidates.append(text[start : i + 1]) + if not candidates: + raise ValueError("no JSON object found in worker output") + return candidates[-1] + + def _parse_worker_result(text: str) -> dict[str, Any]: - result = json.loads(text.strip()) + result = json.loads(_extract_last_json_object(text)) if not isinstance(result, dict): raise ValueError("worker result must be a JSON object") outcome = result.get("outcome") @@ -64,8 +122,6 @@ def _parse_worker_result(text: str) -> dict[str, Any]: timeout=3600, env={ "HERMES_HOME": "/opt/memo-evaluator", - "HERMES_INFERENCE_PROVIDER": "custom", - "OPENAI_BASE_URL": PROXY_BASE_URL, }, ) def evaluate_memo(request_json: str) -> str: @@ -88,7 +144,18 @@ def evaluate_memo(request_json: str) -> str: Task brief follows: """ + brief completed = subprocess.run( - ["hermes", "--cli", "chat", "-q", prompt], + [ + "hermes", + "--cli", + "chat", + "-Q", + "-q", + prompt, + "-m", + MEMO_EVALUATOR_MODEL, + "--provider", + MEMO_EVALUATOR_PROVIDER, + ], check=False, capture_output=True, text=True, diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index 58c0fec6440e..81102d3a2a58 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -132,3 +132,57 @@ def test_modal_cli_result_is_consumed_from_the_write_result_file(monkeypatch, tm assert result["modal_call_id"] == "fc-test" assert result["modal_log_url"] == "https://modal.test/log" + + +def _load_worker_module(monkeypatch, tmp_path): + """Import the Modal worker module with its import-time guards satisfied. + + The module imports ``modal`` and validates that a memo-evaluator profile + (SOUL.md + skills/) exists at import time, so point it at a throwaway + profile fixture. Skips cleanly when ``modal`` is not installed. + """ + modal = pytest.importorskip("modal") # noqa: F841 -- import guard only + profile = tmp_path / "profile" + (profile / "skills").mkdir(parents=True) + (profile / "SOUL.md").write_text("test soul", encoding="utf-8") + monkeypatch.setenv("HERMES_MODAL_MEMO_EVALUATOR_PROFILE", str(profile)) + import importlib + + return importlib.import_module("hermes_cli.kanban_modal_worker") + + +def test_worker_parses_verdict_even_with_a_stray_startup_line(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # Quiet mode can still emit a warning line before the final JSON response. + stdout = ( + " \u26a0 tirith security scanner enabled but not available\n" + '{"outcome":"complete","summary":"Memo passed the rubric.","metadata":{"score":8}}\n' + ) + result = worker._parse_worker_result(stdout) + assert result["outcome"] == "complete" + assert result["summary"] == "Memo passed the rubric." + assert result["metadata"]["score"] == 8 + + +def test_worker_extracts_the_last_object_when_prose_contains_braces(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # A JSON-looking snippet inside {braces} in prose must not shadow the real + # final verdict object. + stdout = ( + 'Example shape: {"outcome":"block"} is one option.\n' + '{"outcome":"complete","summary":"real verdict"}' + ) + result = worker._parse_worker_result(stdout) + assert result["summary"] == "real verdict" + + +def test_worker_binds_the_memo_evaluator_model_and_anthropic_proxy_secret(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # The memo-evaluator runs a Claude model through the Anthropic-format billing + # proxy; the worker must pin that model and require the Anthropic proxy secret + # keys, not an OpenAI-format secret that does not exist in the workspace. + assert worker.MEMO_EVALUATOR_MODEL == "claude-fable-5" + assert worker.MEMO_EVALUATOR_PROVIDER == "anthropic" From 3c2ef65cebed7096a9886cda1ad73d3f2495f767 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 18:34:47 -0400 Subject: [PATCH 03/16] fix(kanban): close parent log fd after spawning Modal worker The parent's log file handle was only closed on the exception path, leaking an open descriptor on every successful spawn. The child inherits its own dup, so release the parent handle in a finally block to stop fds accumulating as more Modal workers are launched. Addresses the graphite review thread on kanban_modal.py:144. --- hermes_cli/kanban_modal.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 6f47c02320ca..382538692a81 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -138,9 +138,11 @@ def spawn_modal_worker(task: Any, workspace: str, *, board: str | None = None) - start_new_session=True, creationflags=subprocess.CREATE_NO_WINDOW if os.name == "nt" else 0, ) - except Exception: + finally: + # The child inherits its own dup of the fd; the parent's handle is + # only needed for the spawn and must be released so open descriptors + # don't accumulate as more Modal workers are launched. log_f.close() - raise return proc.pid From 083ab032599616e4f3ee7055deeeeaa1c3f44f00 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 18:35:45 -0400 Subject: [PATCH 04/16] docs(kanban): correct memo-evaluator Modal secret + model The worker now loads the research-proxy secret (ANTHROPIC_BASE_URL + ANTHROPIC_API_KEY) and runs claude-fable-5 through the Anthropic-Messages proxy, but the lane docs still named the old bloom-llm-proxy/OPENAI_API_KEY setup. Align the operator instructions with the code so provisioning the documented secret actually works. Addresses the graphite + Codex doc-mismatch threads on kanban-worker-lanes.md. --- website/docs/user-guide/features/kanban-worker-lanes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/user-guide/features/kanban-worker-lanes.md b/website/docs/user-guide/features/kanban-worker-lanes.md index 7cf4499fe64a..dee0af0498a3 100644 --- a/website/docs/user-guide/features/kanban-worker-lanes.md +++ b/website/docs/user-guide/features/kanban-worker-lanes.md @@ -96,7 +96,7 @@ kanban: The dispatcher still starts a local shim. The shim serializes the bounded worker brief (including comments), runs `modal run` synchronously, and applies the returned completion or block through the local Kanban database. The remote container never receives Kanban database paths or lifecycle credentials. A successful completion records the Modal function-call id and dashboard log URL in both run metadata and a `modal-shim` comment. -The Modal image bakes only the `memo-evaluator` profile's `SOUL.md` and `skills/`; it reads `OPENAI_API_KEY` from the named `bloom-llm-proxy` Modal secret and routes LLM calls through `https://proxy.getbloom.app`. Before enabling the backend, ensure that profile assets exist locally and that the Modal secret has been provisioned. Values other than `memo-evaluator` remain local, even if a config entry asks for Modal. +The Modal image bakes only the `memo-evaluator` profile's `SOUL.md` and `skills/`; it reads `ANTHROPIC_API_KEY` and `ANTHROPIC_BASE_URL` from the named `research-proxy` Modal secret and routes LLM calls (model `claude-fable-5`) through that Anthropic-Messages proxy. Before enabling the backend, ensure that profile assets exist locally and that the Modal secret has been provisioned. Values other than `memo-evaluator` remain local, even if a config entry asks for Modal. ### Orchestrator profile lane From 880cf1e7029816d52ba67f10774dfffc23bf1488 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 18:47:05 -0400 Subject: [PATCH 05/16] chore: retrigger CI (synchronize event was dropped) From 9a27247be3fda15e08ef658cfad377708228bffa Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 19:35:03 -0400 Subject: [PATCH 06/16] fix(kanban): block memo tasks with unreadable attachments from Modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Modal worker has no Kanban attachments mount, so a memo task whose source doc is uploaded as an attachment would be shipped with only the file path in its brief — the remote container cannot read the bytes and could return a hallucinated complete. Refuse such tasks at dispatch and block them as a capability gap so they are rerouted to a mounted backend. Addresses Codex P1 (kanban_modal_worker.py:139). --- hermes_cli/kanban_modal.py | 34 ++++++++++++++ tests/hermes_cli/test_kanban_modal.py | 67 +++++++++++++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 382538692a81..61470eb4a620 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -22,6 +22,18 @@ _log = logging.getLogger(__name__) +class ModalUnsupportedTask(Exception): + """A memo task cannot be evaluated remotely and must run on a mounted backend. + + The Modal worker has no Kanban attachments mount, so a task whose memo is + supplied as an attachment (PDF / source document) would be evaluated with + only its file *path* in the brief — the remote container cannot read the + bytes and could return a hallucinated ``complete``. Surface this as a + capability block so a human reroutes the task instead of trusting a verdict + made against files the worker never saw. + """ + + def resolve_worker_backend(assignee: str | None, kanban_config: dict[str, Any]) -> str: """Return the configured backend, restricting Modal to memo-evaluator. @@ -156,6 +168,14 @@ def _build_modal_request(task_id: str, workspace: str) -> tuple[dict[str, Any], raise ValueError(f"Kanban task {task_id} is not running") if (task.assignee or "").strip().lower() != _MODAL_LANE: raise ValueError(f"Kanban task {task_id} is not assigned to {_MODAL_LANE}") + attachments = kb.list_attachments(conn, task.id) + if attachments: + names = ", ".join(sorted(a.filename for a in attachments)) + raise ModalUnsupportedTask( + f"Kanban task {task_id} has attachments ({names}) that the " + "mount-less Modal worker cannot read; run it on a backend with " + "the attachments directory mounted." + ) brief = kb.build_worker_context(conn, task.id) if len(brief) > _MAX_MODAL_BRIEF_CHARS: raise ValueError( @@ -223,6 +243,20 @@ def run_modal_shim(task_id: str, workspace: str) -> bool: return apply_modal_result( conn, task_id, result, expected_run_id=expected_run_id ) + except ModalUnsupportedTask as exc: + _log.warning("modal Kanban shim cannot run %s remotely: %s", task_id, exc) + with kb.connect_closing() as conn: + if expected_run_id is None: + task = kb.get_task(conn, task_id) + expected_run_id = task.current_run_id if task else None + kb.add_comment(conn, task_id, "modal-shim", str(exc)) + return kb.block_task( + conn, + task_id, + reason=str(exc), + kind="capability", + expected_run_id=expected_run_id, + ) except Exception as exc: _log.error("modal Kanban shim failed for %s: %s", task_id, exc) with kb.connect_closing() as conn: diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index 81102d3a2a58..bf4e60389a19 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -95,6 +95,73 @@ def test_modal_request_serializes_worker_brief_and_comments_without_board_env(): assert "HERMES_KANBAN_DB" not in request +def test_modal_request_refuses_a_task_with_unreadable_attachments(tmp_path): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task( + conn, + title="grade memo", + body="Evaluate the attached investment memo.", + assignee="memo-evaluator", + ) + assert kb.claim_task(conn, task_id) is not None + blob = tmp_path / "memo.pdf" + blob.write_bytes(b"%PDF-1.4 fake") + kb.add_attachment( + conn, + task_id, + filename="memo.pdf", + stored_path=str(blob), + content_type="application/pdf", + size=blob.stat().st_size, + ) + + # The mount-less Modal worker cannot read attachment bytes, so the shim must + # refuse rather than ship a path-only brief that invites a hallucinated verdict. + with pytest.raises(kanban_modal.ModalUnsupportedTask, match="memo.pdf"): + kanban_modal._build_modal_request(task_id, "/tmp/workspace") + + +def test_modal_shim_blocks_an_attachment_task_as_a_capability_gap(monkeypatch, tmp_path): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task( + conn, + title="grade memo", + body="Evaluate the attached investment memo.", + assignee="memo-evaluator", + ) + assert kb.claim_task(conn, task_id) is not None + blob = tmp_path / "memo.pdf" + blob.write_bytes(b"%PDF-1.4 fake") + kb.add_attachment( + conn, + task_id, + filename="memo.pdf", + stored_path=str(blob), + content_type="application/pdf", + size=blob.stat().st_size, + ) + + def _fail_run(*_args, **_kwargs): + raise AssertionError("Modal must not be invoked for an unreadable-attachment task") + + monkeypatch.setattr(kanban_modal, "_run_modal", _fail_run) + + assert kanban_modal.run_modal_shim(task_id, "/tmp/workspace") is True + + with kb.connect_closing() as conn: + task = kb.get_task(conn, task_id) + assert task is not None + assert task.status == "blocked" + + def test_modal_request_rejects_an_oversized_worker_brief(monkeypatch): from hermes_cli import kanban_db as kb from hermes_cli import kanban_modal From 43eca19c742f0ceb8518084adbee4317faba09b4 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 19:39:27 -0400 Subject: [PATCH 07/16] fix(kanban): resolve memo-evaluator profile via HERMES_HOME-aware helper The Modal worker hardcoded ~/.hermes/profiles/memo-evaluator and read a HERMES_MODAL_MEMO_EVALUATOR_PROFILE env var to override it. That breaks custom/Docker/profile HERMES_HOME layouts (AGENTS.md rule #9) and uses a HERMES_* env var for non-secret path config (rule #4). Resolve the profile via get_profile_dir('memo-evaluator') inside the modal.is_local() guard, which anchors to the profiles root and is HERMES_HOME-aware. Drops the env var entirely. Addresses claude[bot] CHANGES_REQUESTED (profile-safety, blocking). --- hermes_cli/kanban_modal_worker.py | 54 ++++++++++++++++----------- tests/hermes_cli/test_kanban_modal.py | 14 ++++--- 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 6ee518a75497..b39a65b71c07 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -7,7 +7,6 @@ from __future__ import annotations import json -import os import subprocess from pathlib import Path from typing import Any @@ -15,14 +14,28 @@ import modal APP_NAME = "hermes-kanban-memo-evaluator" -PROFILE_SOURCE = Path( - os.environ.get( - "HERMES_MODAL_MEMO_EVALUATOR_PROFILE", - "~/.hermes/profiles/memo-evaluator", - ) -).expanduser() -SOUL_SOURCE = PROFILE_SOURCE / "SOUL.md" -SKILLS_SOURCE = PROFILE_SOURCE / "skills" + + +def _default_profile_source() -> Path: + """Resolve the memo-evaluator profile dir, honoring HERMES_HOME. + + Anchored to the profiles root via ``get_profile_dir`` so custom / Docker / + profile-isolated ``HERMES_HOME`` layouts (e.g. ``/opt/data``) resolve + correctly instead of a hardcoded ``~/.hermes`` that would not exist there. + Only ever called under ``modal.is_local()`` (the local launcher), so the + ``hermes_cli`` import is always available. + """ + from hermes_cli.profiles import get_profile_dir + + return get_profile_dir("memo-evaluator") + + +# Model the memo-evaluator profile is pinned to (see its config.yaml). Passed +# explicitly because the image bakes only SOUL.md + skills/, not a config.yaml, +# so there is no on-disk model default inside the container. +MEMO_EVALUATOR_MODEL = "claude-fable-5" +MEMO_EVALUATOR_PROVIDER = "anthropic" + # The memo-evaluator runs a Claude model through the shared billing proxy, which # only speaks the Anthropic Messages API. The proxy hostname and its inbound gate # key both live in the existing ``research-proxy`` Modal secret (ANTHROPIC_BASE_URL @@ -32,22 +45,21 @@ "research-proxy", required_keys=["ANTHROPIC_BASE_URL", "ANTHROPIC_API_KEY"], ) -# Model the memo-evaluator profile is pinned to (see its config.yaml). Passed -# explicitly because the image bakes only SOUL.md + skills/, not a config.yaml, -# so there is no on-disk model default inside the container. -MEMO_EVALUATOR_MODEL = "claude-fable-5" -MEMO_EVALUATOR_PROVIDER = "anthropic" # The profile source (SOUL.md + skills/) is only present on the machine that # builds/launches the app; inside the Modal container the module is re-imported # with only the baked ``/opt/memo-evaluator`` payload, so these local paths do -# not exist. Guard the source check and the ``add_local_*`` mounts on -# ``modal.is_local()`` — validating the source at container-import time crashes -# every remote run. +# not exist. Guard the source resolution, check, and the ``add_local_*`` mounts +# on ``modal.is_local()`` — resolving/validating the source at container-import +# time crashes every remote run. if modal.is_local(): - if not SOUL_SOURCE.is_file() or not SKILLS_SOURCE.is_dir(): + _profile_source = _default_profile_source() + _soul_source = _profile_source / "SOUL.md" + _skills_source = _profile_source / "skills" + if not _soul_source.is_file() or not _skills_source.is_dir(): raise RuntimeError( - "Set HERMES_MODAL_MEMO_EVALUATOR_PROFILE to a profile containing SOUL.md and skills/." + f"memo-evaluator profile at {_profile_source} is missing SOUL.md " + "and skills/; create it with `hermes profile create memo-evaluator`." ) _base_image = modal.Image.debian_slim(python_version="3.13").pip_install( @@ -55,8 +67,8 @@ ) if modal.is_local(): image = _base_image.add_local_file( - SOUL_SOURCE, "/opt/memo-evaluator/SOUL.md" - ).add_local_dir(SKILLS_SOURCE, "/opt/memo-evaluator/skills") + _soul_source, "/opt/memo-evaluator/SOUL.md" + ).add_local_dir(_skills_source, "/opt/memo-evaluator/skills") else: image = _base_image app = modal.App(APP_NAME) diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index bf4e60389a19..a0a7bebcab9e 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -204,15 +204,17 @@ def test_modal_cli_result_is_consumed_from_the_write_result_file(monkeypatch, tm def _load_worker_module(monkeypatch, tmp_path): """Import the Modal worker module with its import-time guards satisfied. - The module imports ``modal`` and validates that a memo-evaluator profile - (SOUL.md + skills/) exists at import time, so point it at a throwaway - profile fixture. Skips cleanly when ``modal`` is not installed. + The module imports ``modal`` and validates that the ``memo-evaluator`` + profile (SOUL.md + skills/) exists at import time, resolving its path via + ``get_profile_dir`` under the per-test HERMES_HOME. Create that profile so + the import-time guard passes. Skips cleanly when ``modal`` is not installed. """ modal = pytest.importorskip("modal") # noqa: F841 -- import guard only - profile = tmp_path / "profile" - (profile / "skills").mkdir(parents=True) + from hermes_cli.profiles import get_profile_dir + + profile = get_profile_dir("memo-evaluator") + (profile / "skills").mkdir(parents=True, exist_ok=True) (profile / "SOUL.md").write_text("test soul", encoding="utf-8") - monkeypatch.setenv("HERMES_MODAL_MEMO_EVALUATOR_PROFILE", str(profile)) import importlib return importlib.import_module("hermes_cli.kanban_modal_worker") From d15b6041c8e43ac40c4f227a15110d6388256b1d Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 19:51:11 -0400 Subject: [PATCH 08/16] fix(kanban): harden Modal result handling (untrusted metadata + stdin request) Two findings from the re-review on the latest head: 1. Codex P1: an untrusted (prompt-injected/malformed) Modal result could set metadata._staged_artifacts / artifacts to arbitrary host paths, which complete_task turns into attachment records (and remove_attachment would later unlink). Strip reserved host-side directive keys before applying a remote result. 2. graphite (Windows): the brief can be up to 64KB but Windows caps the entire command line at 32,767 chars, so passing the request via --request-json overflows the modal spawn. Pipe the request over stdin instead; worker main() reads sys.stdin. Regression tests cover the metadata stripping and the stdin contract. --- hermes_cli/kanban_modal.py | 27 ++++++++++++++-- hermes_cli/kanban_modal_worker.py | 10 ++++-- tests/hermes_cli/test_kanban_modal.py | 46 +++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 61470eb4a620..0b84447c43ce 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -21,6 +21,14 @@ _MAX_MODAL_BRIEF_CHARS = 64_000 _log = logging.getLogger(__name__) +# Metadata keys the local Kanban lifecycle treats as trusted, host-side +# directives (they make ``complete_task`` copy/attach/unlink host files at the +# named paths). A remote Modal result is untrusted input — a prompt-injected or +# malformed worker response must never be able to smuggle these in and turn an +# arbitrary readable host file into a Kanban attachment (or get it unlinked). +# Strip them before applying the remote result. +_RESERVED_METADATA_KEYS = frozenset({"_staged_artifacts", "artifacts"}) + class ModalUnsupportedTask(Exception): """A memo task cannot be evaluated remotely and must run on a mounted backend. @@ -79,6 +87,17 @@ def apply_modal_result( raise ValueError("Modal completion is missing summary") remote_metadata = result.get("metadata") metadata = dict(remote_metadata) if isinstance(remote_metadata, dict) else {} + # The remote worker is untrusted: drop reserved host-side directive keys + # so a malicious/hallucinated result cannot attach or unlink host files. + stripped = _RESERVED_METADATA_KEYS.intersection(metadata) + if stripped: + _log.warning( + "modal result for %s carried reserved metadata keys %s; stripped", + task_id, + sorted(stripped), + ) + for key in stripped: + metadata.pop(key, None) metadata.update(audit) kb.add_comment( conn, @@ -198,16 +217,18 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s os.close(fd) result_path = Path(result_name) try: - completed = subprocess.run( # noqa: S603 -- fixed CLI plus serialized request + completed = subprocess.run( # noqa: S603 -- fixed CLI, request piped via stdin [ modal_bin, "run", "--write-result", str(result_path), _modal_runner_path(), - "--request-json", - json.dumps(request, separators=(",", ":")), ], + # Pass the (potentially ~64KB) request over stdin, not as an argv + # element: Windows caps the ENTIRE command line at 32,767 chars, so a + # large brief in ``--request-json`` would overflow the spawn there. + input=json.dumps(request, separators=(",", ":")), check=False, capture_output=True, text=True, diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index b39a65b71c07..7e2906542693 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -8,6 +8,7 @@ import json import subprocess +import sys from pathlib import Path from typing import Any @@ -183,8 +184,13 @@ def evaluate_memo(request_json: str) -> str: @app.local_entrypoint() -def main(request_json: str) -> str: - """Return the remote result plus the FunctionCall audit handle to the shim.""" +def main() -> str: + """Return the remote result plus the FunctionCall audit handle to the shim. + + The serialized request arrives on stdin (not an argv element) so a large + brief cannot overflow the Windows 32,767-char command-line limit. + """ + request_json = sys.stdin.read() call = evaluate_memo.spawn(request_json) result = json.loads(call.get()) result["modal_call_id"] = call.object_id diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index a0a7bebcab9e..a60ebeda7b8a 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -52,6 +52,47 @@ def test_modal_completion_is_written_locally_with_audit_metadata(): assert "fc-123" in kb.list_comments(conn, task_id)[-1].body +def test_modal_completion_strips_reserved_metadata_keys(): + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + + # A prompt-injected / malformed remote result tries to smuggle host-side + # attachment directives; the shim must drop them before completing so an + # arbitrary host file can't be attached (or later unlinked). + assert kanban_modal.apply_modal_result( + conn, + task_id, + { + "outcome": "complete", + "summary": "Memo passed the rubric.", + "metadata": { + "score": 8, + "_staged_artifacts": ["/etc/passwd"], + "artifacts": ["/home/user/.ssh/id_rsa"], + }, + "modal_call_id": "fc-123", + "modal_log_url": "https://modal.com/apps/example/logs/fc-123", + }, + expected_run_id=task.current_run_id, + ) + + completed = kb.get_task(conn, task_id) + assert completed is not None and completed.status == "done" + run = kb.list_runs(conn, task_id)[-1] + assert run.metadata is not None + assert "_staged_artifacts" not in run.metadata + assert "artifacts" not in run.metadata + assert run.metadata["score"] == 8 + # No attachment was created from the injected host paths. + assert kb.list_attachments(conn, task_id) == [] + + def test_configured_spawn_routes_only_memo_evaluator_to_modal(monkeypatch): from hermes_cli import kanban_db as kb from hermes_cli import kanban_modal @@ -187,6 +228,11 @@ def test_modal_cli_result_is_consumed_from_the_write_result_file(monkeypatch, tm fake_modal.write_text( "#!/usr/bin/env python3\n" "import json, pathlib, sys\n" + # The request must arrive on stdin (not argv) so a large brief stays + # under the Windows command-line limit; fail loudly on regression. + "assert '--request-json' not in sys.argv, 'request must be piped via stdin'\n" + "req = json.loads(sys.stdin.read())\n" + "assert req['brief'] == 'grade this'\n" "out = pathlib.Path(sys.argv[sys.argv.index('--write-result') + 1])\n" "out.write_text(json.dumps({'outcome': 'complete', 'summary': 'done', " "'modal_call_id': 'fc-test', 'modal_log_url': 'https://modal.test/log'}))\n", From 61f43c124846e491576ef0abb91ccfc7399202b4 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 20:01:38 -0400 Subject: [PATCH 09/16] fix(kanban): pin Modal shim to its spawned run id A delayed/previous shim that survives a reclaim built expected_run_id from whichever run was current when it finally started, so a stale Modal response could complete a re-claimed NEW attempt. Read the env-pinned HERMES_KANBAN_RUN_ID (set at spawn) as the authoritative run id, mirroring the local worker's _worker_run_id_for guard; fall back to the fresh read only when the env var is absent. Regression test proves a stale run id is rejected and the re-claimed task stays running. Addresses Codex P1 (kanban_modal.py:208). --- hermes_cli/kanban_modal.py | 28 +++++++++++++++-- tests/hermes_cli/test_kanban_modal.py | 44 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 0b84447c43ce..3af3e555840a 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -247,13 +247,37 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s result_path.unlink(missing_ok=True) +def _spawned_run_id(task_id: str) -> int | None: + """Return the run id pinned into the env when this shim was spawned. + + ``spawn_modal_worker`` records the claimed run in ``HERMES_KANBAN_RUN_ID``. + A delayed/previous shim that survives a reclaim must complete only the run + it was spawned for — not whatever run happens to be current when it finally + starts — so a stale Modal response cannot land on a re-claimed new attempt. + Mirrors the local worker's env-pinned guard (``_worker_run_id_for``). + """ + if os.environ.get("HERMES_KANBAN_TASK") not in (None, task_id): + return None + raw = os.environ.get("HERMES_KANBAN_RUN_ID") + if not raw: + return None + try: + return int(raw) + except ValueError: + return None + + def run_modal_shim(task_id: str, workspace: str) -> bool: """Run Modal then map its response to a local Kanban completion or block.""" from hermes_cli import kanban_db as kb - expected_run_id: int | None = None + # Prefer the run id pinned at spawn; only fall back to the request's fresh + # read when the env var is absent (e.g. a direct call in a test). + expected_run_id: int | None = _spawned_run_id(task_id) try: - request, expected_run_id = _build_modal_request(task_id, workspace) + request, request_run_id = _build_modal_request(task_id, workspace) + if expected_run_id is None: + expected_run_id = request_run_id timeout = None with kb.connect_closing() as conn: task = kb.get_task(conn, task_id) diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index a60ebeda7b8a..46b352bc6b00 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -93,6 +93,50 @@ def test_modal_completion_strips_reserved_metadata_keys(): assert kb.list_attachments(conn, task_id) == [] +def test_modal_shim_pins_the_spawned_run_id(monkeypatch): + """A stale shim must complete only the run it was spawned for. + + The shim reads ``HERMES_KANBAN_RUN_ID`` (pinned at spawn); if the task was + reclaimed into a new run after this shim was launched, applying its result + with the OLD run id must not touch the new attempt. + """ + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + stale_run_id = task.current_run_id + # Simulate a reclaim → re-claim: the task now runs under a NEW run. + assert kb.reclaim_task(conn, task_id) is True + reclaimed = kb.claim_task(conn, task_id) + assert reclaimed is not None + assert reclaimed.current_run_id != stale_run_id + + monkeypatch.setenv("HERMES_KANBAN_TASK", task_id) + monkeypatch.setenv("HERMES_KANBAN_RUN_ID", str(stale_run_id)) + + def _fake_run(_request, *, timeout=None): + return { + "outcome": "complete", + "summary": "stale result", + "modal_call_id": "fc-stale", + "modal_log_url": "https://modal.test/log", + } + + monkeypatch.setattr(kanban_modal, "_run_modal", _fake_run) + + # The stale run id no longer matches current_run_id, so the completion is + # rejected (apply_modal_result returns False) and the task stays running. + assert kanban_modal.run_modal_shim(task_id, "/tmp/workspace") is False + with kb.connect_closing() as conn: + task = kb.get_task(conn, task_id) + assert task is not None + assert task.status == "running" + + def test_configured_spawn_routes_only_memo_evaluator_to_modal(monkeypatch): from hermes_cli import kanban_db as kb from hermes_cli import kanban_modal From 23cf562076c3e3f0597f93ed9abb46b986301ceb Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 20:10:19 -0400 Subject: [PATCH 10/16] fix(kanban): install anthropic extra in the Modal memo-evaluator image The worker runs Hermes with --provider anthropic, but the image installed bare hermes-agent. The native Anthropic SDK is an optional extra, so the adapter raised ImportError and every Modal invocation blocked before evaluating the brief. Install hermes-agent[anthropic] so the provider resolves. Addresses Codex P1 (kanban_modal_worker.py:68). --- hermes_cli/kanban_modal_worker.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 7e2906542693..68f2a4b346b3 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -64,7 +64,10 @@ def _default_profile_source() -> Path: ) _base_image = modal.Image.debian_slim(python_version="3.13").pip_install( - "hermes-agent>=0.18.2,<0.19" + # The worker runs Hermes with ``--provider anthropic``; the native + # Anthropic SDK is an optional extra, so install it here or the adapter + # raises ImportError before evaluating any brief. + "hermes-agent[anthropic]>=0.18.2,<0.19" ) if modal.is_local(): image = _base_image.add_local_file( From 5828ed4ad9dfcfcce00a22c0182f6ecc34143145 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 20:17:49 -0400 Subject: [PATCH 11/16] docs(kanban): document Modal CLI prerequisite for the memo-evaluator lane The opt-in setup previously read as 'change one config value', but the modal CLI is an optional extra absent even from hermes-agent[all]; without it every run blocks transient. Document the 'uv pip install modal' + 'modal setup' prerequisite (and secret/profile assets) as an ordered setup, and make the missing-CLI RuntimeError name the fix so the block reason is actionable. Addresses Codex P1 (kanban_modal.py:215). --- hermes_cli/kanban_modal.py | 6 ++++- .../features/kanban-worker-lanes.md | 27 ++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 3af3e555840a..8b596d0925b1 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -212,7 +212,11 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s """Synchronously invoke the Modal app and parse its structured response.""" modal_bin = shutil.which("modal") if modal_bin is None: - raise RuntimeError("Modal CLI is not installed or not on PATH") + raise RuntimeError( + "Modal CLI is not installed or not on PATH; install it on the " + "dispatcher host with `uv pip install modal` and run `modal setup` " + "before enabling the memo-evaluator Modal backend" + ) fd, result_name = tempfile.mkstemp(prefix="hermes-kanban-modal-", suffix=".json") os.close(fd) result_path = Path(result_name) diff --git a/website/docs/user-guide/features/kanban-worker-lanes.md b/website/docs/user-guide/features/kanban-worker-lanes.md index dee0af0498a3..1b4477190094 100644 --- a/website/docs/user-guide/features/kanban-worker-lanes.md +++ b/website/docs/user-guide/features/kanban-worker-lanes.md @@ -86,17 +86,30 @@ When you create profiles for your fleet, choose names that match the *role* you ### Modal memo-evaluator lane (opt-in) -`memo-evaluator` is the only profile lane with a supported remote backend. It remains local by default. To send that lane through Modal, change one config value: +`memo-evaluator` is the only profile lane with a supported remote backend. It remains local by default. To send that lane through Modal: -```yaml -kanban: - worker_backends: - memo-evaluator: modal -``` +1. Install and configure the Modal CLI on the **dispatcher** host (it is an optional extra, absent even from `hermes-agent[all]`): + + ```bash + uv pip install modal # or: pip install modal + modal setup # authenticate the CLI once + ``` + + Without the `modal` CLI on `PATH`, every `memo-evaluator` run is blocked as `transient` (the shim never reaches the remote worker). + +2. Provision the `research-proxy` Modal secret (`ANTHROPIC_API_KEY` + `ANTHROPIC_BASE_URL`) and ensure the `memo-evaluator` profile's `SOUL.md` and `skills/` exist locally. + +3. Flip the backend for that lane: + + ```yaml + kanban: + worker_backends: + memo-evaluator: modal + ``` The dispatcher still starts a local shim. The shim serializes the bounded worker brief (including comments), runs `modal run` synchronously, and applies the returned completion or block through the local Kanban database. The remote container never receives Kanban database paths or lifecycle credentials. A successful completion records the Modal function-call id and dashboard log URL in both run metadata and a `modal-shim` comment. -The Modal image bakes only the `memo-evaluator` profile's `SOUL.md` and `skills/`; it reads `ANTHROPIC_API_KEY` and `ANTHROPIC_BASE_URL` from the named `research-proxy` Modal secret and routes LLM calls (model `claude-fable-5`) through that Anthropic-Messages proxy. Before enabling the backend, ensure that profile assets exist locally and that the Modal secret has been provisioned. Values other than `memo-evaluator` remain local, even if a config entry asks for Modal. +The Modal image bakes only the `memo-evaluator` profile's `SOUL.md` and `skills/`; it reads `ANTHROPIC_API_KEY` and `ANTHROPIC_BASE_URL` from the named `research-proxy` Modal secret and routes LLM calls (model `claude-fable-5`) through that Anthropic-Messages proxy. Values other than `memo-evaluator` remain local, even if a config entry asks for Modal. ### Orchestrator profile lane From cc2fbb9f559976ca2fc05ce043341d34ade2c4ea Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 20:26:56 -0400 Subject: [PATCH 12/16] fix(kanban): honor task runtime for the Modal memo-evaluator function timeout The remote evaluate_memo function was hardcoded to a 1h timeout, so a >1h or uncapped (max_runtime_seconds=None) memo task lost its Modal call at one hour and blocked as transient instead of honoring the task's runtime contract. The shim now threads max_runtime_seconds into the request; the worker's main() applies it via evaluate_memo.with_options(timeout=...), clamped to Modal's 24h ceiling, with uncapped tasks using that ceiling. Regression test proves the runtime reaches the request. Addresses Codex P2 (kanban_modal_worker.py:138). Also refutes the claude blocking concern: 'modal run -w/--write-result' is a real, documented flag (verified against modal 1.5.2) and FunctionCall.get_dashboard_url/object_id/get + Function.spawn/with_options all exist in the SDK. --- hermes_cli/kanban_modal.py | 3 +++ hermes_cli/kanban_modal_worker.py | 21 +++++++++++++-- tests/hermes_cli/test_kanban_modal.py | 39 +++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 8b596d0925b1..6f26489169fd 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -287,6 +287,9 @@ def run_modal_shim(task_id: str, workspace: str) -> bool: task = kb.get_task(conn, task_id) if task is not None and task.max_runtime_seconds: timeout = int(task.max_runtime_seconds) + # Thread the per-task runtime into the remote function timeout too, so a + # >1h or uncapped task isn't silently killed at the function default. + request["max_runtime_seconds"] = timeout result = _run_modal(request, timeout=timeout) with kb.connect_closing() as conn: return apply_modal_result( diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 68f2a4b346b3..27430676ca48 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -132,10 +132,17 @@ def _parse_worker_result(text: str) -> dict[str, Any]: raise ValueError("worker result must declare a complete or block outcome") +# Modal caps a function timeout at 24h; use that as the ceiling for an +# uncapped ``memo-evaluator`` task. The default here only applies when the shim +# doesn't override it via ``with_options`` (see ``main``). +_MODAL_MAX_TIMEOUT = 24 * 60 * 60 +_DEFAULT_TIMEOUT = 3600 + + @app.function( image=image, secrets=[PROXY_SECRET], - timeout=3600, + timeout=_DEFAULT_TIMEOUT, env={ "HERMES_HOME": "/opt/memo-evaluator", }, @@ -194,7 +201,17 @@ def main() -> str: brief cannot overflow the Windows 32,767-char command-line limit. """ request_json = sys.stdin.read() - call = evaluate_memo.spawn(request_json) + # Honor the task's runtime contract: a >1h or uncapped (None) task must not + # be killed at the 1h function default. Uncapped uses Modal's 24h ceiling. + fn = evaluate_memo + try: + max_runtime = json.loads(request_json).get("max_runtime_seconds") + except (TypeError, ValueError): + max_runtime = None + timeout = _MODAL_MAX_TIMEOUT if max_runtime is None else min(int(max_runtime), _MODAL_MAX_TIMEOUT) + if timeout != _DEFAULT_TIMEOUT: + fn = evaluate_memo.with_options(timeout=timeout) + call = fn.spawn(request_json) result = json.loads(call.get()) result["modal_call_id"] = call.object_id result["modal_log_url"] = call.get_dashboard_url() diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index 46b352bc6b00..75074cb36e7c 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -137,6 +137,45 @@ def _fake_run(_request, *, timeout=None): assert task.status == "running" +def test_modal_shim_threads_task_runtime_into_the_request(monkeypatch): + """A >1h / uncapped task's runtime must reach the remote so it isn't + killed at the function default. The shim adds max_runtime_seconds to the + request payload it hands to Modal.""" + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task( + conn, + title="long memo", + assignee="memo-evaluator", + max_runtime_seconds=7200, + ) + task = kb.claim_task(conn, task_id) + assert task is not None + + captured: dict = {} + + def _fake_run(request, *, timeout=None): + captured["request"] = request + captured["timeout"] = timeout + return { + "outcome": "complete", + "summary": "ok", + "modal_call_id": "fc-1", + "modal_log_url": "https://modal.test/log", + } + + monkeypatch.setenv("HERMES_KANBAN_TASK", task_id) + monkeypatch.setenv("HERMES_KANBAN_RUN_ID", str(task.current_run_id)) + monkeypatch.setattr(kanban_modal, "_run_modal", _fake_run) + + assert kanban_modal.run_modal_shim(task_id, "/tmp/workspace") is True + assert captured["request"]["max_runtime_seconds"] == 7200 + assert captured["timeout"] == 7200 + + def test_configured_spawn_routes_only_memo_evaluator_to_modal(monkeypatch): from hermes_cli import kanban_db as kb from hermes_cli import kanban_modal From ae2af15701692b95f7a71fdef690537ea46f99bf Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 21:08:09 -0400 Subject: [PATCH 13/16] fix(kanban): reclaim the Modal CLI child and reject over-cap runtimes Two Codex findings on the memo-evaluator Modal lane: P1 (kanban_modal.py): the shim spawned `modal run` as a blocking child, but a dispatcher reclaim/timeout signals only the recorded shim pid. Killing the shim orphaned the Modal CLI and its paid remote call, which a requeued attempt could then duplicate. Launch the child in its own process group, track it, and forward SIGTERM->SIGKILL to the whole group on the shim's own signal handler and on the per-task timeout path so no orphaned remote call survives. P2 (kanban_modal_worker.py): a max_runtime_seconds above Modal's 24h function cap was silently clamped, so a longer task would be killed mid-evaluation and requeued as a transient failure forever. Reject it up front as a capability block (extracted into the pure _resolve_function_timeout helper) so a human reroutes it before any paid spawn. --- hermes_cli/kanban_modal.py | 123 +++++++++++++++++++++++--- hermes_cli/kanban_modal_worker.py | 34 ++++++- tests/hermes_cli/test_kanban_modal.py | 100 +++++++++++++++++++++ 3 files changed, 245 insertions(+), 12 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 6f26489169fd..f6324c364e22 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -11,6 +11,7 @@ import logging import os import shutil +import signal import subprocess import sys import tempfile @@ -21,6 +22,13 @@ _MAX_MODAL_BRIEF_CHARS = 64_000 _log = logging.getLogger(__name__) +# The currently-running ``modal run`` child, tracked so a reclaim SIGTERM to the +# shim can forward termination to it. Without this the dispatcher would kill only +# the shim PID, orphaning the Modal CLI (and its paid remote call) to be +# duplicated when the task is requeued. Set while ``_run_modal`` blocks on the +# child; cleared in its ``finally``. +_active_modal_proc: subprocess.Popen | None = None + # Metadata keys the local Kanban lifecycle treats as trusted, host-side # directives (they make ``complete_task`` copy/attach/unlink host files at the # named paths). A remote Modal result is untrusted input — a prompt-injected or @@ -209,7 +217,16 @@ def _build_modal_request(task_id: str, workspace: str) -> tuple[dict[str, Any], def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[str, Any]: - """Synchronously invoke the Modal app and parse its structured response.""" + """Synchronously invoke the Modal app and parse its structured response. + + The ``modal run`` child is launched in its own process group and tracked in + ``_active_modal_proc`` so a reclaim/timeout SIGTERM delivered to this shim can + be forwarded to the whole group (see ``_terminate_active_modal``). Killing + only the shim would orphan the Modal CLI and its paid remote call, which a + requeued attempt would then duplicate. + """ + global _active_modal_proc + modal_bin = shutil.which("modal") if modal_bin is None: raise RuntimeError( @@ -220,8 +237,9 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s fd, result_name = tempfile.mkstemp(prefix="hermes-kanban-modal-", suffix=".json") os.close(fd) result_path = Path(result_name) + proc: subprocess.Popen | None = None try: - completed = subprocess.run( # noqa: S603 -- fixed CLI, request piped via stdin + proc = subprocess.Popen( # noqa: S603 -- fixed CLI, request piped via stdin [ modal_bin, "run", @@ -229,17 +247,30 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s str(result_path), _modal_runner_path(), ], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + # Own process group so the shim can forward SIGTERM/SIGKILL to the + # Modal CLI (and any grandchild it spawns), not just its direct pid. + start_new_session=True, + ) + _active_modal_proc = proc + try: # Pass the (potentially ~64KB) request over stdin, not as an argv # element: Windows caps the ENTIRE command line at 32,767 chars, so a # large brief in ``--request-json`` would overflow the spawn there. - input=json.dumps(request, separators=(",", ":")), - check=False, - capture_output=True, - text=True, - timeout=timeout, - ) - if completed.returncode != 0: - raise RuntimeError(f"Modal run exited with status {completed.returncode}") + _stdout, _stderr = proc.communicate( + input=json.dumps(request, separators=(",", ":")), + timeout=timeout, + ) + except subprocess.TimeoutExpired: + # The task's runtime cap elapsed. Tear the whole Modal group down so + # the remote call can't keep billing after we've given up on it. + _terminate_active_modal(proc) + raise + if proc.returncode != 0: + raise RuntimeError(f"Modal run exited with status {proc.returncode}") try: parsed = json.loads(result_path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: @@ -248,9 +279,52 @@ def _run_modal(request: dict[str, Any], *, timeout: int | None = None) -> dict[s raise RuntimeError("Modal run returned a non-object result") return parsed finally: + if proc is not None and _active_modal_proc is proc: + _active_modal_proc = None result_path.unlink(missing_ok=True) +def _terminate_active_modal(proc: subprocess.Popen) -> None: + """SIGTERM then SIGKILL the Modal CLI's whole process group. + + Called when the shim is being torn down (its own SIGTERM handler) or when + the per-task timeout elapses. Signalling the group — not just ``proc.pid`` — + stops any grandchild the Modal CLI spawned so no orphaned remote call keeps + running after the shim exits. + """ + if proc.poll() is not None: + return + _signal_process_group(proc, signal.SIGTERM) + try: + proc.wait(timeout=5) + return + except subprocess.TimeoutExpired: + pass + _signal_process_group(proc, getattr(signal, "SIGKILL", signal.SIGTERM)) + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + _log.warning("Modal CLI child %s did not exit after SIGKILL", proc.pid) + + +def _signal_process_group(proc: subprocess.Popen, sig: int) -> None: + """Signal the child's process group, falling back to the bare pid. + + ``start_new_session=True`` makes the child a group leader, so ``killpg`` on + its pid reaches the Modal CLI and any grandchild. On platforms without + ``killpg`` (Windows), fall back to signalling the process directly. + """ + killpg = getattr(os, "killpg", None) + getpgid = getattr(os, "getpgid", None) + try: + if killpg is not None and getpgid is not None: + killpg(getpgid(proc.pid), sig) + else: + proc.send_signal(sig) + except (ProcessLookupError, OSError): + pass + + def _spawned_run_id(task_id: str) -> int | None: """Return the run id pinned into the env when this shim was spawned. @@ -330,10 +404,39 @@ def run_modal_shim(task_id: str, workspace: str) -> bool: ) +def _install_shim_signal_forwarding() -> None: + """Forward a reclaim/timeout SIGTERM to the in-flight Modal CLI child. + + The dispatcher (``enforce_max_runtime`` / ``reclaim_task``) signals only the + recorded shim pid. Without this handler the shim would exit while its + ``modal run`` child — and the paid remote call — kept running, so a requeued + attempt could launch a duplicate evaluation. On SIGTERM/SIGINT we tear the + Modal process group down first, then exit non-zero so the run is recorded as + a failure/timeout rather than a phantom success. + """ + def _handler(signum, _frame): + proc = _active_modal_proc + if proc is not None: + _terminate_active_modal(proc) + # 128 + signal number is the conventional "terminated by signal" code. + raise SystemExit(128 + signum) + + for _sig_name in ("SIGTERM", "SIGINT"): + _sig = getattr(signal, _sig_name, None) + if _sig is not None: + try: + signal.signal(_sig, _handler) + except (ValueError, OSError): + # Not on the main thread (e.g. a direct in-process test call); + # signal forwarding is a best-effort dispatcher-path safeguard. + pass + + def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description="Run a Kanban memo evaluator via Modal") parser.add_argument("--task-id", required=True) args = parser.parse_args(argv) + _install_shim_signal_forwarding() workspace = os.environ.get("HERMES_KANBAN_WORKSPACE", "") return 0 if run_modal_shim(args.task_id, workspace) else 1 diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 27430676ca48..28d929a259f2 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -139,6 +139,32 @@ def _parse_worker_result(text: str) -> dict[str, Any]: _DEFAULT_TIMEOUT = 3600 +def _resolve_function_timeout(max_runtime: Any) -> int | dict[str, Any]: + """Map a task's ``max_runtime_seconds`` to a Modal function timeout. + + Returns an int timeout to apply, or a ``block`` result dict when the task's + runtime cannot be honored remotely. A runtime above Modal's hard 24h cap is + rejected (not silently clamped): clamping would kill a longer task + mid-evaluation and requeue it as a transient failure forever, so surface it + as a capability block up front — before any paid remote spawn — so a human + reroutes it to a backend without the cap. ``None`` / unparseable means + uncapped and uses Modal's 24h ceiling. + """ + if max_runtime is not None: + try: + max_runtime = int(max_runtime) + except (TypeError, ValueError): + max_runtime = None + if max_runtime is not None and max_runtime > _MODAL_MAX_TIMEOUT: + return _block( + f"Task max_runtime_seconds ({max_runtime}s) exceeds Modal's " + f"{_MODAL_MAX_TIMEOUT}s function-timeout cap; run it on a backend " + "without the 24h limit.", + kind="capability", + ) + return _MODAL_MAX_TIMEOUT if max_runtime is None else min(max_runtime, _MODAL_MAX_TIMEOUT) + + @app.function( image=image, secrets=[PROXY_SECRET], @@ -202,13 +228,17 @@ def main() -> str: """ request_json = sys.stdin.read() # Honor the task's runtime contract: a >1h or uncapped (None) task must not - # be killed at the 1h function default. Uncapped uses Modal's 24h ceiling. + # be killed at the 1h function default. Uncapped uses Modal's 24h ceiling; a + # value above the cap is rejected as a capability block (see the helper). fn = evaluate_memo try: max_runtime = json.loads(request_json).get("max_runtime_seconds") except (TypeError, ValueError): max_runtime = None - timeout = _MODAL_MAX_TIMEOUT if max_runtime is None else min(int(max_runtime), _MODAL_MAX_TIMEOUT) + resolved = _resolve_function_timeout(max_runtime) + if isinstance(resolved, dict): + return json.dumps(resolved) + timeout = resolved if timeout != _DEFAULT_TIMEOUT: fn = evaluate_memo.with_options(timeout=timeout) call = fn.spawn(request_json) diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index 75074cb36e7c..e7828813fa0b 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -384,3 +384,103 @@ def test_worker_binds_the_memo_evaluator_model_and_anthropic_proxy_secret(monkey # keys, not an OpenAI-format secret that does not exist in the workspace. assert worker.MEMO_EVALUATOR_MODEL == "claude-fable-5" assert worker.MEMO_EVALUATOR_PROVIDER == "anthropic" + + +def test_worker_uncapped_runtime_uses_modal_24h_ceiling(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # A task with no runtime cap must run to Modal's 24h function-timeout ceiling, + # not the 1h function default. + assert worker._resolve_function_timeout(None) == worker._MODAL_MAX_TIMEOUT + + +def test_worker_runtime_within_cap_is_honored(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # A 2h task keeps its exact runtime; a string is coerced. + assert worker._resolve_function_timeout(7200) == 7200 + assert worker._resolve_function_timeout("7200") == 7200 + + +def test_worker_rejects_runtime_above_modal_cap_as_capability_block(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # A 48h task exceeds Modal's 24h function-timeout cap. Silently clamping to + # 24h would kill it mid-evaluation and requeue it as a transient failure + # forever; the worker must reject it up front as a capability block so a + # human reroutes it — before any paid remote spawn. + result = worker._resolve_function_timeout(48 * 60 * 60) + assert isinstance(result, dict) + assert result["outcome"] == "block" + assert result["kind"] == "capability" + assert "exceeds Modal" in result["reason"] + + +def test_worker_unparseable_runtime_falls_back_to_ceiling(monkeypatch, tmp_path): + worker = _load_worker_module(monkeypatch, tmp_path) + + # A malformed value is treated as uncapped, not an error. + assert worker._resolve_function_timeout("not-a-number") == worker._MODAL_MAX_TIMEOUT + + +def test_run_modal_terminates_the_modal_child_group_on_timeout(monkeypatch, tmp_path): + """A per-task runtime timeout must tear down the Modal CLI process group. + + Killing only the shim would orphan the ``modal run`` child (and its paid + remote call); the shim forwards termination to the whole group so a requeued + attempt can't launch a duplicate evaluation. + """ + import subprocess + + from hermes_cli import kanban_modal + + monkeypatch.setattr(kanban_modal.shutil, "which", lambda _bin: "/usr/bin/modal") + + class _FakeProc: + def __init__(self): + self.pid = 4242 + self.returncode = None + self._alive = True + self.signals: list[int] = [] + + def communicate(self, input=None, timeout=None): + raise subprocess.TimeoutExpired(cmd="modal", timeout=timeout or 1) + + def poll(self): + return None if self._alive else self.returncode + + def wait(self, timeout=None): + # First SIGTERM: still alive (force the SIGKILL escalation). After a + # SIGKILL was delivered, report exit. + if any(s == getattr(kanban_modal.signal, "SIGKILL", None) for s in self.signals): + self._alive = False + self.returncode = -9 + return self.returncode + raise subprocess.TimeoutExpired(cmd="modal", timeout=timeout or 1) + + def send_signal(self, sig): + self.signals.append(sig) + + fake = _FakeProc() + monkeypatch.setattr(kanban_modal.subprocess, "Popen", lambda *a, **k: fake) + + captured_groups: list[tuple[int, int]] = [] + + def _fake_killpg(pgid, sig): + captured_groups.append((pgid, sig)) + fake.signals.append(sig) + + monkeypatch.setattr(kanban_modal.os, "killpg", _fake_killpg, raising=False) + monkeypatch.setattr(kanban_modal.os, "getpgid", lambda pid: pid, raising=False) + + with pytest.raises(subprocess.TimeoutExpired): + kanban_modal._run_modal({"task_id": "t_x", "brief": "grade"}, timeout=1) + + # SIGTERM then SIGKILL were sent to the child's process group (pid==pgid). + sigterm = kanban_modal.signal.SIGTERM + sigkill = getattr(kanban_modal.signal, "SIGKILL", sigterm) + assert (4242, sigterm) in captured_groups + assert (4242, sigkill) in captured_groups + # The global tracker is cleared so a later spawn isn't mistaken for this one. + assert kanban_modal._active_modal_proc is None + From 91aeed824894bd5ce80e77aaabe043e6f148e7a9 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 21:17:21 -0400 Subject: [PATCH 14/16] fix(kanban): skip the Modal call from a stale reclaimed shim Codex P2 re-review: a shim spawned for a reclaimed attempt still launched (and paid for) a duplicate remote evaluation because the env-pinned run id was only compared at result-application, not before invocation. Add a pre-invocation staleness guard in run_modal_shim: when the spawned run id no longer matches the task's current run, return without calling _run_modal, leaving the current attempt untouched for its own shim to own. --- hermes_cli/kanban_modal.py | 23 +++++++++++++- tests/hermes_cli/test_kanban_modal.py | 43 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index f6324c364e22..0d2acd840091 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -351,11 +351,32 @@ def run_modal_shim(task_id: str, workspace: str) -> bool: # Prefer the run id pinned at spawn; only fall back to the request's fresh # read when the env var is absent (e.g. a direct call in a test). - expected_run_id: int | None = _spawned_run_id(task_id) + spawned_run_id = _spawned_run_id(task_id) + expected_run_id: int | None = spawned_run_id try: request, request_run_id = _build_modal_request(task_id, workspace) if expected_run_id is None: expected_run_id = request_run_id + # Pre-invocation staleness guard: if this shim was spawned for a specific + # run (env-pinned) but the task has since been reclaimed into a newer run, + # skip the paid Modal call entirely. Without this the stale shim still + # launches — and pays for — a duplicate remote evaluation against the new + # task state; only the *result* was being rejected afterward. Leave the + # current attempt untouched (no lifecycle write) so its own shim owns it. + if ( + spawned_run_id is not None + and request_run_id is not None + and request_run_id != spawned_run_id + ): + _log.warning( + "modal shim for %s was spawned for run %s but the task is now on " + "run %s; skipping the remote call to avoid a duplicate paid " + "evaluation", + task_id, + spawned_run_id, + request_run_id, + ) + return False timeout = None with kb.connect_closing() as conn: task = kb.get_task(conn, task_id) diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index e7828813fa0b..e8d7c31318b6 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -137,6 +137,49 @@ def _fake_run(_request, *, timeout=None): assert task.status == "running" +def test_modal_shim_skips_the_remote_call_when_reclaimed(monkeypatch): + """A stale shim must not launch (and pay for) a duplicate remote evaluation. + + When a shim from a reclaimed attempt starts after the task was claimed again, + the env-pinned run id no longer matches the task's current run. The shim must + skip ``_run_modal`` entirely — not just reject the result afterward — so no + duplicate paid Modal call is made against the new task state. + """ + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + stale_run_id = task.current_run_id + assert kb.reclaim_task(conn, task_id) is True + reclaimed = kb.claim_task(conn, task_id) + assert reclaimed is not None + assert reclaimed.current_run_id != stale_run_id + + monkeypatch.setenv("HERMES_KANBAN_TASK", task_id) + monkeypatch.setenv("HERMES_KANBAN_RUN_ID", str(stale_run_id)) + + called = {"ran": False} + + def _fake_run(_request, *, timeout=None): + called["ran"] = True + raise AssertionError("stale shim must not invoke Modal") + + monkeypatch.setattr(kanban_modal, "_run_modal", _fake_run) + + # The pre-invocation guard returns False without ever calling _run_modal. + assert kanban_modal.run_modal_shim(task_id, "/tmp/workspace") is False + assert called["ran"] is False + # The current (reclaimed) attempt is left untouched for its own shim to own. + with kb.connect_closing() as conn: + task = kb.get_task(conn, task_id) + assert task is not None + assert task.status == "running" + + def test_modal_shim_threads_task_runtime_into_the_request(monkeypatch): """A >1h / uncapped task's runtime must reach the remote so it isn't killed at the function default. The shim adds max_runtime_seconds to the From 592eac7bb6c3f26555a4cb65e08bfa0dfe29df32 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 21:25:27 -0400 Subject: [PATCH 15/16] fix(kanban): gate Modal audit comment on validated completion; guard non-dict request Two re-review findings: - codex P2 (kanban_modal.py): apply_modal_result wrote the Modal call-id/log-url audit comment before complete_task's run-id validation, so a stale shim (task reclaimed mid-call) smeared old audit onto the new attempt even though the result was rejected. Complete first; add the comment only when the run-validated completion actually landed. - graphite (kanban_modal_worker.py): main's json.loads(...).get() raised an uncaught AttributeError on valid-but-non-dict JSON (list/str/number), crashing the entrypoint. Catch AttributeError/JSONDecodeError and treat as uncapped. --- hermes_cli/kanban_modal.py | 21 ++++++---- hermes_cli/kanban_modal_worker.py | 5 ++- tests/hermes_cli/test_kanban_modal.py | 59 +++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index 0d2acd840091..e4b32774a98c 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -107,19 +107,26 @@ def apply_modal_result( for key in stripped: metadata.pop(key, None) metadata.update(audit) - kb.add_comment( - conn, - task_id, - "modal-shim", - f"Modal audit: call {audit['modal_call_id']} — {audit['modal_log_url']}", - ) - return kb.complete_task( + # Complete first, then record the audit comment only if the run-validated + # completion actually landed. A stale shim (task reclaimed while its Modal + # call was in flight) has ``complete_task`` return False on the run-id + # mismatch; writing the comment unconditionally would smear the old call + # id / log url onto the new attempt's audit history. + completed = kb.complete_task( conn, task_id, summary=summary.strip(), metadata=metadata, expected_run_id=expected_run_id, ) + if completed: + kb.add_comment( + conn, + task_id, + "modal-shim", + f"Modal audit: call {audit['modal_call_id']} — {audit['modal_log_url']}", + ) + return completed if outcome == "block": reason = result.get("reason") diff --git a/hermes_cli/kanban_modal_worker.py b/hermes_cli/kanban_modal_worker.py index 28d929a259f2..32e11c5c23a7 100644 --- a/hermes_cli/kanban_modal_worker.py +++ b/hermes_cli/kanban_modal_worker.py @@ -233,7 +233,10 @@ def main() -> str: fn = evaluate_memo try: max_runtime = json.loads(request_json).get("max_runtime_seconds") - except (TypeError, ValueError): + except (TypeError, ValueError, AttributeError, json.JSONDecodeError): + # AttributeError guards valid-but-non-dict JSON (a list/str/number has no + # ``.get``); treat any malformed request as uncapped rather than crashing + # the entrypoint. max_runtime = None resolved = _resolve_function_timeout(max_runtime) if isinstance(resolved, dict): diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index e8d7c31318b6..19a863f15ac1 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -137,6 +137,46 @@ def _fake_run(_request, *, timeout=None): assert task.status == "running" +def test_modal_audit_comment_is_only_written_on_a_validated_completion(): + """A rejected (stale-run) completion must not smear its audit onto the task. + + ``apply_modal_result`` completes first and only records the Modal call-id / + log-url audit comment when the run-validated ``complete_task`` actually + landed. A stale shim whose run id no longer matches must leave no misleading + audit history on the new attempt. + """ + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + stale_run_id = task.current_run_id + assert kb.reclaim_task(conn, task_id) is True + reclaimed = kb.claim_task(conn, task_id) + assert reclaimed is not None + assert reclaimed.current_run_id != stale_run_id + + result = { + "outcome": "complete", + "summary": "stale verdict", + "modal_call_id": "fc-stale", + "modal_log_url": "https://modal.test/log", + } + # Applying with the STALE run id: complete_task returns False, so no audit + # comment is written. + assert ( + kanban_modal.apply_modal_result( + conn, task_id, result, expected_run_id=stale_run_id + ) + is False + ) + comments = kb.list_comments(conn, task_id) + assert not any("Modal audit" in (c.body or "") for c in comments) + + def test_modal_shim_skips_the_remote_call_when_reclaimed(monkeypatch): """A stale shim must not launch (and pay for) a duplicate remote evaluation. @@ -466,6 +506,25 @@ def test_worker_unparseable_runtime_falls_back_to_ceiling(monkeypatch, tmp_path) assert worker._resolve_function_timeout("not-a-number") == worker._MODAL_MAX_TIMEOUT +def test_worker_main_handles_non_dict_request_json(monkeypatch, tmp_path): + """A valid-JSON-but-non-dict request must not crash the entrypoint. + + ``json.loads`` of a bare list/string succeeds, but ``.get`` on it raises + AttributeError. The runtime resolution must swallow that and fall back to + uncapped rather than crashing the Modal local entrypoint. + """ + worker = _load_worker_module(monkeypatch, tmp_path) + import json as _json + + for payload in ("[1, 2, 3]", '"just a string"', "42"): + try: + max_runtime = _json.loads(payload).get("max_runtime_seconds") + except (TypeError, ValueError, AttributeError, _json.JSONDecodeError): + max_runtime = None + # Same recovery the entrypoint applies → treated as uncapped. + assert worker._resolve_function_timeout(max_runtime) == worker._MODAL_MAX_TIMEOUT + + def test_run_modal_terminates_the_modal_child_group_on_timeout(monkeypatch, tmp_path): """A per-task runtime timeout must tear down the Modal CLI process group. From 5e68b11f98242d69c4fb7f1f300108812b0b0821 Mon Sep 17 00:00:00 2001 From: exiao Date: Wed, 22 Jul 2026 21:36:48 -0400 Subject: [PATCH 16/16] fix(kanban): block untrusted prose-artifact promotion + gate failure audit on run Two more re-review findings on the untrusted-remote-result boundary: - codex P1: complete_task's legacy prose scanner promotes scratch-workspace file paths named in summary/result into Kanban attachments. A prompt-injected remote Modal verdict could name a workspace secret file and expose it to later board users; the metadata-key stripping did not cover the prose route. Add a scan_prose_artifacts flag (default True for trusted local workers) and pass False from the Modal shim so remote summaries cannot drive host-file promotion. - codex P2: the shim's capability/failure handlers wrote their audit comment before block_task's run-id validation, smearing a stale attempt's note onto the re-claimed run. Block first; write the comment only when the run-validated block landed. Mirrors the completion-path audit gating. --- hermes_cli/kanban_db.py | 18 +++++++++-- hermes_cli/kanban_modal.py | 33 ++++++++++++++------ tests/hermes_cli/test_kanban_db.py | 30 +++++++++++++++++++ tests/hermes_cli/test_kanban_modal.py | 43 +++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 12 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 398b199301fd..10d2411f9fce 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -5576,6 +5576,7 @@ def complete_task( metadata: Optional[dict] = None, created_cards: Optional[Iterable[str]] = None, expected_run_id: Optional[int] = None, + scan_prose_artifacts: bool = True, ) -> bool: """Transition ``running|ready -> done`` and record ``result``. @@ -5604,6 +5605,16 @@ def complete_task( Any suspected phantom references are recorded as a ``suspected_hallucinated_references`` event. This pass is advisory and never blocks. + + ``scan_prose_artifacts`` (default True) controls the legacy + prose-artifact promotion in :func:`_merge_completion_prose_artifacts`, + which discovers scratch-workspace file paths named in ``summary`` / + ``result`` and copies them into Kanban attachments. Callers applying an + **untrusted** completion (e.g. a remote Modal worker verdict) must pass + ``False``: a prompt-injected summary could otherwise name an arbitrary + workspace file (``/.env``) and expose it to later board + users. Trusted local workers keep the default so their promised + deliverables survive scratch cleanup. """ now = int(time.time()) @@ -5634,9 +5645,10 @@ def complete_task( else: verified_cards = [] - metadata = _merge_completion_prose_artifacts( - conn, task_id, metadata, summary=summary, result=result, - ) + if scan_prose_artifacts: + metadata = _merge_completion_prose_artifacts( + conn, task_id, metadata, summary=summary, result=result, + ) with write_txn(conn): if expected_run_id is None: cur = conn.execute( diff --git a/hermes_cli/kanban_modal.py b/hermes_cli/kanban_modal.py index e4b32774a98c..2072b5f3d75c 100644 --- a/hermes_cli/kanban_modal.py +++ b/hermes_cli/kanban_modal.py @@ -112,12 +112,19 @@ def apply_modal_result( # call was in flight) has ``complete_task`` return False on the run-id # mismatch; writing the comment unconditionally would smear the old call # id / log url onto the new attempt's audit history. + # + # ``scan_prose_artifacts=False``: the remote worker is untrusted and + # mount-less, so a prompt-injected summary must not be able to name a + # workspace file (e.g. ``/.env``) and have the host-side prose + # scanner promote it into Kanban attachments. Stripping the reserved + # metadata keys above does not cover the prose route. completed = kb.complete_task( conn, task_id, summary=summary.strip(), metadata=metadata, expected_run_id=expected_run_id, + scan_prose_artifacts=False, ) if completed: kb.add_comment( @@ -403,33 +410,41 @@ def run_modal_shim(task_id: str, workspace: str) -> bool: if expected_run_id is None: task = kb.get_task(conn, task_id) expected_run_id = task.current_run_id if task else None - kb.add_comment(conn, task_id, "modal-shim", str(exc)) - return kb.block_task( + # Block first; only record the shim note if the run-validated + # transition landed, so a stale shim (task reclaimed mid-call) can't + # smear this attempt's audit onto the new one. + blocked = kb.block_task( conn, task_id, reason=str(exc), kind="capability", expected_run_id=expected_run_id, ) + if blocked: + kb.add_comment(conn, task_id, "modal-shim", str(exc)) + return blocked except Exception as exc: _log.error("modal Kanban shim failed for %s: %s", task_id, exc) with kb.connect_closing() as conn: if expected_run_id is None: task = kb.get_task(conn, task_id) expected_run_id = task.current_run_id if task else None - kb.add_comment( - conn, - task_id, - "modal-shim", - "Modal worker invocation failed; see the worker log for details.", - ) - return kb.block_task( + # Same run-validated gating as the capability path above. + blocked = kb.block_task( conn, task_id, reason="Modal worker invocation failed; see the worker log.", kind="transient", expected_run_id=expected_run_id, ) + if blocked: + kb.add_comment( + conn, + task_id, + "modal-shim", + "Modal worker invocation failed; see the worker log for details.", + ) + return blocked def _install_shim_signal_forwarding() -> None: diff --git a/tests/hermes_cli/test_kanban_db.py b/tests/hermes_cli/test_kanban_db.py index c28631823db4..ab735b4dc3bb 100644 --- a/tests/hermes_cli/test_kanban_db.py +++ b/tests/hermes_cli/test_kanban_db.py @@ -4065,6 +4065,36 @@ def test_complete_task_preserves_legacy_artifact_path_from_summary(kanban_home): assert persisted.parent == kb.task_attachments_dir(t) +def test_complete_task_suppresses_prose_artifacts_when_disabled(kanban_home): + """An untrusted completion must not promote workspace files named in prose. + + A remote Modal verdict is untrusted: a prompt-injected summary could name an + existing workspace file (e.g. ``/.env``) and the host-side prose + scanner would otherwise copy it into Kanban attachments. Passing + ``scan_prose_artifacts=False`` disables that promotion. + """ + with kb.connect() as conn: + t = kb.create_task(conn, title="untrusted verdict") + task = kb.get_task(conn, t) + ws = kb.resolve_workspace(task) + kb.set_workspace_path(conn, t, ws) + secret = ws / ".env" + secret.write_text("API_KEY=super-secret", encoding="utf-8") + + assert kb.complete_task( + conn, + t, + summary=f"Graded — see {secret}", + scan_prose_artifacts=False, + ) + run = kb.latest_run(conn, t) + + # No artifact was promoted, and no attachment leaked the workspace file. + assert not (run.metadata or {}).get("artifacts") + attach_dir = kb.task_attachments_dir(t) + assert not attach_dir.exists() or not any(attach_dir.iterdir()) + + def test_complete_task_leaves_non_scratch_artifact_paths_unchanged( kanban_home, tmp_path, diff --git a/tests/hermes_cli/test_kanban_modal.py b/tests/hermes_cli/test_kanban_modal.py index 19a863f15ac1..2d0e858ff359 100644 --- a/tests/hermes_cli/test_kanban_modal.py +++ b/tests/hermes_cli/test_kanban_modal.py @@ -220,6 +220,49 @@ def _fake_run(_request, *, timeout=None): assert task.status == "running" +def test_modal_shim_failure_comment_is_gated_on_the_claimed_run(monkeypatch): + """A failure from a stale shim must not smear the new attempt's audit. + + When the Modal invocation errors after the shim was reclaimed and the task + re-claimed, the block is rejected on the run-id mismatch; the shim note must + only be written when that run-validated block actually landed. + """ + from hermes_cli import kanban_db as kb + from hermes_cli import kanban_modal + + kb.init_db() + with kb.connect_closing() as conn: + task_id = kb.create_task(conn, title="grade memo", assignee="memo-evaluator") + task = kb.claim_task(conn, task_id) + assert task is not None + stale_run_id = task.current_run_id + assert kb.reclaim_task(conn, task_id) is True + reclaimed = kb.claim_task(conn, task_id) + assert reclaimed is not None + assert reclaimed.current_run_id != stale_run_id + + # Pin the STALE run and skip the pre-invocation guard by making the request + # build report the stale run id, then fail inside _run_modal. + monkeypatch.setenv("HERMES_KANBAN_TASK", task_id) + monkeypatch.setenv("HERMES_KANBAN_RUN_ID", str(stale_run_id)) + monkeypatch.setattr( + kanban_modal, + "_build_modal_request", + lambda _tid, _ws: ({"task_id": task_id, "brief": "b"}, stale_run_id), + ) + + def _boom(_request, *, timeout=None): + raise RuntimeError("modal exploded") + + monkeypatch.setattr(kanban_modal, "_run_modal", _boom) + + # The block is rejected (stale run), so it returns False and writes no note. + assert kanban_modal.run_modal_shim(task_id, "/tmp/workspace") is False + with kb.connect_closing() as conn: + comments = kb.list_comments(conn, task_id) + assert not any("worker log" in (c.body or "") for c in comments) + + def test_modal_shim_threads_task_runtime_into_the_request(monkeypatch): """A >1h / uncapped task's runtime must reach the remote so it isn't killed at the function default. The shim adds max_runtime_seconds to the