diff --git a/AGENTS.md b/AGENTS.md index bb2a78f2114a..63247b2bf474 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1094,14 +1094,16 @@ kanban task. - **CLI:** `hermes_cli/kanban.py` wires `hermes kanban` with verbs `init`, `create`, `list` (alias `ls`), `show`, `assign`, `link`, - `unlink`, `comment`, `complete`, `block`, `unblock`, `archive`, - `tail`, plus less-commonly-used `watch`, `stats`, `runs`, `log`, - `assignees`, `heartbeat`, `notify-*`, `dispatch`, `daemon`, `gc`. + `unlink`, `comment`, `attach`, `attachments`, `attach-rm`, `complete`, + `block`, `unblock`, `archive`, `tail`, plus less-commonly-used `watch`, + `stats`, `runs`, `log`, `assignees`, `heartbeat`, `notify-*`, + `dispatch`, `daemon`, `gc`. - **Worker/orchestrator toolset:** `tools/kanban_tools.py` exposes `kanban_show`, `kanban_complete`, `kanban_block`, `kanban_heartbeat`, - `kanban_comment`, `kanban_create`, `kanban_link`; profiles that - explicitly enable the `kanban` toolset outside a dispatcher-spawned - task also get `kanban_list` and `kanban_unblock` for board routing. + `kanban_comment`, `kanban_create`, `kanban_link`, `kanban_attach`, + `kanban_attach_url`, `kanban_attachments`; profiles that explicitly + enable the `kanban` toolset outside a dispatcher-spawned task also get + `kanban_list` and `kanban_unblock` for board routing. - **Dispatcher:** long-lived loop that (default every 60s) reclaims stale claims, promotes ready tasks, atomically claims, and spawns assigned profiles. Runs **inside the gateway** by default via diff --git a/hermes_cli/kanban.py b/hermes_cli/kanban.py index 7fc7bf948950..6f0766a738d1 100644 --- a/hermes_cli/kanban.py +++ b/hermes_cli/kanban.py @@ -522,6 +522,24 @@ def build_parser(parent_subparsers: argparse._SubParsersAction) -> argparse.Argu p_comment.add_argument("--max-len", type=int, default=None, help="Trim the stored comment body to this many characters") + # --- attach / attachments / attach-rm --- + p_attach = sub.add_parser("attach", help="Attach a local file to a task") + p_attach.add_argument("task_id") + p_attach.add_argument("path", help="Path to the local file to attach") + p_attach.add_argument("--content-type", default=None, + help="MIME type (default: guessed from the file extension)") + p_attach.add_argument("--name", default=None, + help="Stored filename (default: the source file's basename)") + p_attach.add_argument("--author", default=None, + help="uploaded_by label (default: $HERMES_PROFILE or 'user')") + + p_attachments = sub.add_parser("attachments", help="List a task's attachments") + p_attachments.add_argument("task_id") + p_attachments.add_argument("--json", action="store_true") + + p_attach_rm = sub.add_parser("attach-rm", help="Delete an attachment by id") + p_attach_rm.add_argument("attachment_id", type=int) + p_complete = sub.add_parser("complete", help="Mark one or more tasks done") p_complete.add_argument("task_ids", nargs="+", help="One or more task ids (only --result applies to all of them)") @@ -951,6 +969,9 @@ def kanban_command(args: argparse.Namespace) -> int: "unlink": _cmd_unlink, "claim": _cmd_claim, "comment": _cmd_comment, + "attach": _cmd_attach, + "attachments": _cmd_attachments, + "attach-rm": _cmd_attach_rm, "complete": _cmd_complete, "edit": _cmd_edit, "block": _cmd_block, @@ -1851,6 +1872,84 @@ def _cmd_comment(args: argparse.Namespace) -> int: return 0 +def _cmd_attach(args: argparse.Namespace) -> int: + """Attach a local file to a task. + + Reads the file off disk, writes it under the task's attachments dir, + and records the metadata row via the shared ``store_attachment_bytes`` + path (same code the dashboard upload and the agent tool use), so the + 25 MB cap and name-sanitisation behave identically everywhere. + """ + import mimetypes + + src = Path(args.path).expanduser() + if not src.is_file(): + print(f"kanban: no such file: {src}", file=sys.stderr) + return 1 + data = src.read_bytes() + name = args.name or src.name + content_type = args.content_type or mimetypes.guess_type(name)[0] + uploaded_by = args.author or _profile_author() + try: + with kb.connect_closing() as conn: + att_id = kb.store_attachment_bytes( + conn, + args.task_id, + name, + data, + content_type=content_type, + uploaded_by=uploaded_by, + ) + except kb.AttachmentTooLarge as exc: + print(f"kanban: {exc}", file=sys.stderr) + return 1 + print(f"Attached {name} to {args.task_id} (attachment {att_id}, {len(data)} bytes)") + return 0 + + +def _cmd_attachments(args: argparse.Namespace) -> int: + """List a task's attachments.""" + with kb.connect_closing() as conn: + if kb.get_task(conn, args.task_id) is None: + print(f"no such task: {args.task_id}", file=sys.stderr) + return 1 + atts = kb.list_attachments(conn, args.task_id) + if getattr(args, "json", False): + print(json.dumps([ + { + "id": a.id, + "filename": a.filename, + "content_type": a.content_type, + "size": a.size, + "uploaded_by": a.uploaded_by, + "stored_path": a.stored_path, + "created_at": a.created_at, + } + for a in atts + ], indent=2)) + return 0 + if not atts: + print(f"No attachments on {args.task_id}") + return 0 + print(f"Attachments on {args.task_id}:") + for a in atts: + ct = a.content_type or "-" + print(f" [{a.id}] {a.filename} ({a.size} bytes, {ct}, by {a.uploaded_by or '-'})") + print(f" {a.stored_path}") + return 0 + + +def _cmd_attach_rm(args: argparse.Namespace) -> int: + """Delete an attachment by id (removes the row and the on-disk blob).""" + with kb.connect_closing() as conn: + removed = kb.delete_attachment(conn, args.attachment_id) + if removed is None: + print(f"no such attachment: {args.attachment_id}", file=sys.stderr) + return 1 + print(f"Deleted attachment {args.attachment_id} ({removed.filename}) from {removed.task_id}") + return 0 + + def _worker_run_id_for(task_id: str) -> Optional[int]: if os.environ.get("HERMES_KANBAN_TASK") != task_id: return None @@ -2753,6 +2852,7 @@ Common subcommands: `stats` Per-status / per-assignee counts `create …` Create a task (auto-subscribes you to events) `comment <id> <msg>` Append a comment + `attach <id> <path>` Attach a local file; `attachments <id>` to list `complete <id>…` Mark task(s) done `block <id> [reason]` Mark blocked; `schedule <id> [reason]` parks time-delay work; `unblock <id>` to revive `assign <id> <profile>` Reassign diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 39c434d4021d..979f6fdc0a12 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -2963,6 +2963,118 @@ def list_comments(conn: sqlite3.Connection, task_id: str) -> list[Comment]: # Attachments # --------------------------------------------------------------------------- +# Cap a single attachment so a runaway upload can't fill the disk. 25 MB +# comfortably covers PDFs, images, and source docs — the kanban use case. +# Shared by the dashboard HTTP endpoint, the agent toolset, and the CLI so +# the limit cannot drift between surfaces. +_MAX_ATTACHMENT_BYTES = 25 * 1024 * 1024 + + +class AttachmentTooLarge(ValueError): + """Raised when an attachment exceeds the configured size cap. + + Subclasses :class:`ValueError` so generic ``except ValueError`` handlers + (e.g. the dashboard's 400 fallback) still catch it, while callers that + want a distinct user-facing message (the tool/CLI 413-equivalent) can + catch it specifically. + """ + + +def _safe_attachment_name(raw: str) -> str: + """Reduce a client-supplied filename to a safe basename. + + Strips any directory components (both separators) so a malicious + ``../../etc/passwd`` or ``C:\\x`` collapses to its leaf. Drops control + chars and leading dots so we never write a dotfile or a name with + embedded NULs/newlines. Rejects empty / dotfile-only names. The result + is only ever joined under the per-task attachments dir, never used + verbatim as a path from the client. + + Raises :class:`ValueError` on an unusable name; HTTP callers map that + to a 400. + """ + name = (raw or "").replace("\\", "/").split("/")[-1].strip() + name = "".join(ch for ch in name if ch.isprintable() and ch not in "\x00").strip() + name = name.lstrip(".").strip() + if not name: + raise ValueError("invalid attachment filename") + return name[:200] + + +def _collision_free_path(dest_dir: Path, safe_name: str) -> Path: + """Return a path under ``dest_dir`` that doesn't clobber an existing file. + + ``foo.pdf`` → ``foo.pdf``, then ``foo (1).pdf``, ``foo (2).pdf``, … + ``safe_name`` must already be sanitised via :func:`_safe_attachment_name`. + """ + stem, dot, ext = safe_name.partition(".") + candidate = safe_name + n = 1 + while (dest_dir / candidate).exists(): + candidate = f"{stem} ({n}){dot}{ext}" + n += 1 + return dest_dir / candidate + + +def store_attachment_bytes( + conn: sqlite3.Connection, + task_id: str, + filename: str, + data: bytes, + *, + content_type: Optional[str] = None, + uploaded_by: Optional[str] = None, + board: Optional[str] = None, + max_bytes: Optional[int] = None, +) -> int: + """Validate, size-check, persist a blob, and record its metadata row. + + This is the single write path shared by the dashboard endpoint, the + agent toolset (``kanban_attach`` / ``kanban_attach_url``), and the CLI + (``hermes kanban attach``) so name-sanitisation, the size cap, and the + collision-resolution all behave identically everywhere. + + Steps: enforce ``max_bytes``, sanitise ``filename`` to a safe basename, + write the bytes under :func:`task_attachments_dir` with a + collision-free name, then insert the ``task_attachments`` row via + :func:`add_attachment`. Returns the new attachment id. + + Raises :class:`AttachmentTooLarge` when ``data`` exceeds ``max_bytes``, + or :class:`ValueError` for a bad filename / unknown task. On any failure + after the blob is written (e.g. the task disappeared) the orphaned blob + is removed before re-raising. + """ + if max_bytes is None: + max_bytes = _MAX_ATTACHMENT_BYTES + if len(data) > max_bytes: + raise AttachmentTooLarge( + f"attachment exceeds {max_bytes // (1024 * 1024)} MB limit" + ) + safe_name = _safe_attachment_name(filename) + dest_dir = task_attachments_dir(task_id, board=board) + dest_dir.mkdir(parents=True, exist_ok=True) + dest_path = _collision_free_path(dest_dir, safe_name) + dest_path.write_bytes(data) + try: + return add_attachment( + conn, + task_id, + filename=dest_path.name, + stored_path=str(dest_path.resolve()), + content_type=content_type, + size=len(data), + uploaded_by=uploaded_by, + ) + except Exception: + # Don't leave an orphan blob if the metadata insert fails (most + # commonly: the task id doesn't exist). + try: + dest_path.unlink(missing_ok=True) + except OSError: + pass + raise + + def add_attachment( conn: sqlite3.Connection, task_id: str, diff --git a/plugins/kanban/dashboard/plugin_api.py b/plugins/kanban/dashboard/plugin_api.py index e280e54c8052..a66567556e94 100644 --- a/plugins/kanban/dashboard/plugin_api.py +++ b/plugins/kanban/dashboard/plugin_api.py @@ -660,28 +660,16 @@ def create_task(payload: CreateTaskBody, board: Optional[str] = Query(None)): # Attachments — upload / list / download / delete (#35338) # --------------------------------------------------------------------------- -# Cap a single upload so a runaway request can't fill the disk. 25 MB -# comfortably covers PDFs, images, and source docs — the kanban use case. -_MAX_ATTACHMENT_BYTES = kanban_db.KANBAN_ATTACHMENT_MAX_BYTES - - -def _safe_attachment_name(raw: str) -> str: - """Reduce a client-supplied filename to a safe basename. - - Strips any directory components (``os.path.basename`` on both - separators) so a malicious ``../../etc/passwd`` or ``C:\\x`` collapses - to its leaf. Rejects empty / dotfile-only names. The result is only - ever joined under the per-task attachments dir, never used verbatim - as a path from the client. - """ - name = (raw or "").replace("\\", "/").split("/")[-1].strip() - # Drop control chars and leading dots so we never write a dotfile or - # a name with embedded NULs/newlines. - name = "".join(ch for ch in name if ch.isprintable() and ch not in '\x00').strip() - name = name.lstrip(".").strip() - if not name: - raise HTTPException(status_code=400, detail="invalid attachment filename") - return name[:200] +# The size cap, filename sanitiser, and collision resolver now live in +# ``kanban_db`` so the dashboard, the agent toolset, and the CLI share one +# implementation and cannot drift. ``_safe_attachment_name`` raises a plain +# ``ValueError`` there; the upload handler's ``except ValueError`` below maps +# it to a 400, preserving the previous response. +from hermes_cli.kanban_db import ( # noqa: E402 + _MAX_ATTACHMENT_BYTES, + _collision_free_path, + _safe_attachment_name, +) @router.get("/tasks/{task_id}/attachments") @@ -727,13 +715,8 @@ async def upload_task_attachment( dest_dir.mkdir(parents=True, exist_ok=True) # Resolve name collisions: foo.pdf → foo (1).pdf, foo (2).pdf, … - stem, dot, ext = safe_name.partition(".") - candidate = safe_name - n = 1 - while (dest_dir / candidate).exists(): - candidate = f"{stem} ({n}){dot}{ext}" - n += 1 - dest_path = dest_dir / candidate + dest_path = _collision_free_path(dest_dir, safe_name) + candidate = dest_path.name total = 0 try: diff --git a/tests/hermes_cli/test_kanban_core_functionality.py b/tests/hermes_cli/test_kanban_core_functionality.py index 4884862e1f79..0e898999c1d2 100644 --- a/tests/hermes_cli/test_kanban_core_functionality.py +++ b/tests/hermes_cli/test_kanban_core_functionality.py @@ -890,7 +890,7 @@ def test_cli_gc_reports_counts(kanban_home): # run_slash parity — every verb returns a sensible, non-crashy string # --------------------------------------------------------------------------- -def test_run_slash_every_verb_returns_sensible_output(kanban_home): +def test_run_slash_every_verb_returns_sensible_output(kanban_home, tmp_path): """Smoke-test every verb with minimal args. None may raise, none may return the empty string (must either succeed or report a usage error).""" # Set up a pair of tasks to reference. @@ -901,6 +901,9 @@ def test_run_slash_every_verb_returns_sensible_output(kanban_home): finally: conn.close() + attach_src = tmp_path / "smoke.txt" + attach_src.write_text("smoke") + invocations = [ "", # no subcommand → help text "--help", @@ -914,6 +917,8 @@ def test_run_slash_every_verb_returns_sensible_output(kanban_home): f"unlink {tid_a} {tid_b}", f"claim {tid_a}", f"comment {tid_a} hello", + f"attach {tid_a} {attach_src}", + f"attachments {tid_a}", f"complete {tid_a}", f"block {tid_b} need input", f"unblock {tid_b}", diff --git a/tests/plugins/test_kanban_attachments.py b/tests/plugins/test_kanban_attachments.py index 3beb875a812e..bbd2862016d8 100644 --- a/tests/plugins/test_kanban_attachments.py +++ b/tests/plugins/test_kanban_attachments.py @@ -289,3 +289,149 @@ def test_upload_unknown_task_404(client): def test_download_unknown_attachment_404(client): assert client.get("/api/plugins/kanban/attachments/424242").status_code == 404 + + +# --------------------------------------------------------------------------- +# Shared helper — store_attachment_bytes (used by dashboard + tool + CLI) +# --------------------------------------------------------------------------- + + +def test_store_attachment_bytes_roundtrip(kanban_home): + conn = kb.connect() + try: + task_id = _make_task(conn) + att_id = kb.store_attachment_bytes( + conn, task_id, "doc.txt", b"some bytes", + content_type="text/plain", uploaded_by="tester", + ) + a = kb.get_attachment(conn, att_id) + assert a is not None + assert a.filename == "doc.txt" + assert a.size == len(b"some bytes") + assert a.uploaded_by == "tester" + assert Path(a.stored_path).read_bytes() == b"some bytes" + assert Path(a.stored_path).resolve().is_relative_to( + kb.task_attachments_dir(task_id).resolve() + ) + finally: + conn.close() + + +def test_store_attachment_bytes_rejects_oversize_and_leaves_no_blob(kanban_home): + conn = kb.connect() + try: + task_id = _make_task(conn) + with pytest.raises(kb.AttachmentTooLarge): + kb.store_attachment_bytes( + conn, task_id, "big.bin", b"0123456789", max_bytes=4, + ) + assert kb.list_attachments(conn, task_id) == [] + # No partial blob left behind. + d = kb.task_attachments_dir(task_id) + assert not d.exists() or list(d.iterdir()) == [] + finally: + conn.close() + + +def test_store_attachment_bytes_resolves_collisions(kanban_home): + conn = kb.connect() + try: + task_id = _make_task(conn) + kb.store_attachment_bytes(conn, task_id, "dup.txt", b"a") + kb.store_attachment_bytes(conn, task_id, "dup.txt", b"b") + names = sorted(a.filename for a in kb.list_attachments(conn, task_id)) + assert names == ["dup (1).txt", "dup.txt"] + finally: + conn.close() + + +def test_store_attachment_bytes_unknown_task_leaves_no_blob(kanban_home): + conn = kb.connect() + try: + with pytest.raises(ValueError): + kb.store_attachment_bytes(conn, "t_nope", "x.txt", b"x") + # The per-task dir may get created, but no blob should survive the + # failed metadata insert. + d = kb.task_attachments_dir("t_nope") + assert not d.exists() or list(d.iterdir()) == [] + finally: + conn.close() + + +# --------------------------------------------------------------------------- +# CLI — hermes kanban attach / attachments / attach-rm +# --------------------------------------------------------------------------- + + +def test_cli_attach_attachments_and_rm(kanban_home, tmp_path): + from hermes_cli.kanban import run_slash + + conn = kb.connect() + try: + task_id = _make_task(conn, title="cli-attach") + finally: + conn.close() + + src = tmp_path / "upload.txt" + src.write_bytes(b"cli file body") + + out = run_slash(f"attach {task_id} {src}") + assert "Attached" in out, out + + conn = kb.connect() + try: + atts = kb.list_attachments(conn, task_id) + assert len(atts) == 1 + att_id = atts[0].id + assert atts[0].filename == "upload.txt" + assert Path(atts[0].stored_path).read_bytes() == b"cli file body" + finally: + conn.close() + + listed = run_slash(f"attachments {task_id}") + assert "upload.txt" in listed + + removed = run_slash(f"attach-rm {att_id}") + assert "Deleted attachment" in removed + conn = kb.connect() + try: + assert kb.list_attachments(conn, task_id) == [] + finally: + conn.close() + + +def test_cli_attach_honors_name_override(kanban_home, tmp_path): + from hermes_cli.kanban import run_slash + + conn = kb.connect() + try: + task_id = _make_task(conn) + finally: + conn.close() + src = tmp_path / "raw.bin" + src.write_bytes(b"xyz") + run_slash(f"attach {task_id} {src} --name renamed.dat") + conn = kb.connect() + try: + assert kb.list_attachments(conn, task_id)[0].filename == "renamed.dat" + finally: + conn.close() + + +def test_cli_attach_missing_file(kanban_home, tmp_path): + from hermes_cli.kanban import run_slash + + conn = kb.connect() + try: + task_id = _make_task(conn) + finally: + conn.close() + out = run_slash(f"attach {task_id} {tmp_path / 'does-not-exist.txt'}") + assert "no such file" in out.lower() + + +def test_cli_attachments_unknown_task(kanban_home): + from hermes_cli.kanban import run_slash + + out = run_slash("attachments t_nope") + assert "no such task" in out.lower() diff --git a/tests/tools/test_kanban_tools.py b/tests/tools/test_kanban_tools.py index 5b1d9f6f7556..42bfc5e5bd0c 100644 --- a/tests/tools/test_kanban_tools.py +++ b/tests/tools/test_kanban_tools.py @@ -57,6 +57,7 @@ def test_kanban_tools_visible_with_env_var(monkeypatch, tmp_path): expected = { "kanban_show", "kanban_complete", "kanban_block", "kanban_heartbeat", "kanban_comment", "kanban_create", "kanban_link", + "kanban_attach", "kanban_attach_url", "kanban_attachments", } assert kanban == expected, f"expected {expected}, got {kanban}" @@ -138,6 +139,7 @@ def test_kanban_tools_visible_with_toolset_config(monkeypatch, tmp_path): "kanban_show", "kanban_complete", "kanban_block", "kanban_heartbeat", "kanban_comment", "kanban_create", "kanban_link", "kanban_unblock", + "kanban_attach", "kanban_attach_url", "kanban_attachments", } assert kanban == expected, f"expected {expected}, got {kanban}" @@ -2030,7 +2032,7 @@ def test_board_param_rejects_invalid_slug(multi_board_env): def test_board_param_in_all_schemas(): - """All nine kanban_* tool schemas must expose an optional ``board`` + """Every kanban_* tool schema must expose an optional ``board`` parameter. This pins the contract surfaced to the LLM — adding a new kanban tool without ``board`` will fail CI immediately.""" from tools import kanban_tools as kt @@ -2045,6 +2047,9 @@ def test_board_param_in_all_schemas(): kt.KANBAN_CREATE_SCHEMA, kt.KANBAN_UNBLOCK_SCHEMA, kt.KANBAN_LINK_SCHEMA, + kt.KANBAN_ATTACH_SCHEMA, + kt.KANBAN_ATTACH_URL_SCHEMA, + kt.KANBAN_ATTACHMENTS_SCHEMA, ] for schema in schemas: props = schema["parameters"]["properties"] @@ -2246,3 +2251,229 @@ def test_maybe_auto_subscribe_swallows_add_notify_sub_failure(monkeypatch, worke d = json.loads(out) assert d["ok"] is True, d assert d["subscribed"] is False, d + + +# --------------------------------------------------------------------------- +# Attachments — kanban_attach / kanban_attach_url / kanban_attachments +# --------------------------------------------------------------------------- + + +def test_attach_roundtrips_bytes_to_row_and_disk(worker_env): + """kanban_attach decodes base64, writes the blob, and records the row.""" + import base64 + from pathlib import Path + + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + content = b"hello attachment from a tool" + out = kt._handle_attach({ + "filename": "notes.txt", + "content_base64": base64.b64encode(content).decode(), + "content_type": "text/plain", + }) + d = json.loads(out) + assert d.get("ok") is True, out + assert d["size"] == len(content) + att_id = d["attachment_id"] + + conn = kb.connect() + try: + atts = kb.list_attachments(conn, worker_env) + assert [a.filename for a in atts] == ["notes.txt"] + a = atts[0] + assert a.id == att_id + assert a.content_type == "text/plain" + assert a.uploaded_by == "agent" + # Blob is on disk under the task's attachments dir with the bytes. + assert Path(a.stored_path).read_bytes() == content + assert Path(a.stored_path).resolve().is_relative_to( + kb.task_attachments_dir(worker_env).resolve() + ) + finally: + conn.close() + + +def test_attach_rejects_oversize(worker_env, monkeypatch): + """A decoded payload over the cap returns a clean tool error, no row.""" + import base64 + + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + # Shrink the cap so we don't have to build a 25 MB payload. + monkeypatch.setattr(kb, "_MAX_ATTACHMENT_BYTES", 8) + out = kt._handle_attach({ + "filename": "big.bin", + "content_base64": base64.b64encode(b"0123456789").decode(), + }) + d = json.loads(out) + assert "error" in d + assert "MB limit" in d["error"] + + conn = kb.connect() + try: + assert kb.list_attachments(conn, worker_env) == [] + finally: + conn.close() + + +def test_attach_rejects_bad_base64(worker_env): + from tools import kanban_tools as kt + + out = kt._handle_attach({"filename": "x.txt", "content_base64": "not base64!!!"}) + d = json.loads(out) + assert "error" in d and "base64" in d["error"] + + +def test_attach_requires_filename_and_content(worker_env): + from tools import kanban_tools as kt + + assert "error" in json.loads(kt._handle_attach({"content_base64": "QQ=="})) + assert "error" in json.loads(kt._handle_attach({"filename": "x.txt"})) + + +def test_attach_enforces_worker_task_ownership(worker_env): + """A worker scoped to its own task can't attach to a foreign task.""" + import base64 + + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + conn = kb.connect() + try: + other = kb.create_task(conn, title="someone else's task", assignee="peer") + finally: + conn.close() + + out = kt._handle_attach({ + "task_id": other, + "filename": "x.txt", + "content_base64": base64.b64encode(b"x").decode(), + }) + d = json.loads(out) + assert "error" in d + assert "scoped to task" in d["error"] + + +def test_attachments_lists_uploaded_files(worker_env): + import base64 + + from tools import kanban_tools as kt + + kt._handle_attach({ + "filename": "a.txt", + "content_base64": base64.b64encode(b"aaa").decode(), + }) + kt._handle_attach({ + "filename": "b.txt", + "content_base64": base64.b64encode(b"bbbb").decode(), + }) + out = kt._handle_attachments({}) + d = json.loads(out) + assert d.get("ok") is True + names = sorted(a["filename"] for a in d["attachments"]) + assert names == ["a.txt", "b.txt"] + sizes = {a["filename"]: a["size"] for a in d["attachments"]} + assert sizes == {"a.txt": 3, "b.txt": 4} + + +def test_attachments_unknown_task_errors(worker_env): + from tools import kanban_tools as kt + + out = kt._handle_attachments({"task_id": "t_nope"}) + assert "error" in json.loads(out) + + +def test_attach_url_fetches_local_fixture(worker_env): + """kanban_attach_url downloads from an http(s) URL and stores the bytes.""" + import http.server + import threading + from pathlib import Path + + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + payload = b"downloaded-by-url body" + + class _Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + def log_message(self, *a): # silence + pass + + srv = http.server.HTTPServer(("127.0.0.1", 0), _Handler) + threading.Thread(target=srv.serve_forever, daemon=True).start() + try: + port = srv.server_address[1] + out = kt._handle_attach_url({ + "url": f"http://127.0.0.1:{port}/files/report.bin", + }) + finally: + srv.shutdown() + d = json.loads(out) + assert d.get("ok") is True, out + assert d["size"] == len(payload) + + conn = kb.connect() + try: + atts = kb.list_attachments(conn, worker_env) + # Filename derived from the URL path leaf. + assert atts[0].filename == "report.bin" + assert Path(atts[0].stored_path).read_bytes() == payload + finally: + conn.close() + + +def test_attach_url_rejects_oversize_stream(worker_env, monkeypatch): + """An oversize response body is rejected during download, no row written.""" + import http.server + import threading + + from hermes_cli import kanban_db as kb + from tools import kanban_tools as kt + + big = b"x" * (64 * 1024) + + class _Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 + self.send_response(200) + self.send_header("Content-Type", "application/octet-stream") + self.send_header("Content-Length", str(len(big))) + self.end_headers() + self.wfile.write(big) + + def log_message(self, *a): + pass + + monkeypatch.setattr(kb, "_MAX_ATTACHMENT_BYTES", 1024) + srv = http.server.HTTPServer(("127.0.0.1", 0), _Handler) + threading.Thread(target=srv.serve_forever, daemon=True).start() + try: + port = srv.server_address[1] + out = kt._handle_attach_url({"url": f"http://127.0.0.1:{port}/big.bin"}) + finally: + srv.shutdown() + d = json.loads(out) + assert "error" in d + assert "MB limit" in d["error"] + + conn = kb.connect() + try: + assert kb.list_attachments(conn, worker_env) == [] + finally: + conn.close() + + +def test_attach_url_rejects_non_http_scheme(worker_env): + from tools import kanban_tools as kt + + out = kt._handle_attach_url({"url": "file:///etc/passwd"}) + d = json.loads(out) + assert "error" in d + assert "scheme" in d["error"] diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 5207c756aa6c..373748f55bee 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -838,6 +838,193 @@ def _handle_comment(args: dict, **kw) -> str: return tool_error(f"kanban_comment: {e}") +def _handle_attach(args: dict, **kw) -> str: + """Attach an inline (base64) file to a task. + + Mirrors the dashboard's upload endpoint for the agent surface: decode + the payload, enforce the shared size cap, write it under the per-task + attachments dir, and record the metadata row — all via + ``kanban_db.store_attachment_bytes`` so the three surfaces stay in lockstep. + """ + from hermes_cli import kanban_db as kb + + tid = _default_task_id(args.get("task_id")) + if not tid: + return tool_error( + "task_id is required (or set HERMES_KANBAN_TASK in the env)" + ) + ownership_err = _enforce_worker_task_ownership(tid) + if ownership_err: + return ownership_err + filename = args.get("filename") + if not filename or not str(filename).strip(): + return tool_error("filename is required") + content_b64 = args.get("content_base64") + if not content_b64 or not str(content_b64).strip(): + return tool_error("content_base64 is required") + import base64 + import binascii + try: + data = base64.b64decode(str(content_b64), validate=True) + except (binascii.Error, ValueError) as e: + return tool_error(f"content_base64 is not valid base64: {e}") + content_type = args.get("content_type") + board = args.get("board") + try: + _, conn = _connect(board=board) + try: + att_id = kb.store_attachment_bytes( + conn, + tid, + str(filename), + data, + content_type=content_type, + uploaded_by="agent", + board=board, + ) + return _ok(task_id=tid, attachment_id=att_id, size=len(data)) + finally: + conn.close() + except kb.AttachmentTooLarge as e: + return tool_error(f"kanban_attach: {e}") + except ValueError as e: + return tool_error(f"kanban_attach: {e}") + except Exception as e: + logger.exception("kanban_attach failed") + return tool_error(f"kanban_attach: {e}") + + +def _download_url_with_cap(url: str, max_bytes: int) -> tuple[bytes, Optional[str]]: + """Fetch ``url`` over http(s), aborting once ``max_bytes`` is exceeded. + + Returns ``(data, content_type)``. Raises ``ValueError`` for a non-http(s) + scheme or a body that overruns the cap (the caller maps it to a clean + tool error). Reads in chunks so an oversize response is rejected without + buffering the whole thing. + """ + from urllib.parse import urlparse + from urllib.request import Request, urlopen + + scheme = (urlparse(url).scheme or "").lower() + if scheme not in ("http", "https"): + raise ValueError(f"unsupported URL scheme {scheme!r}; only http/https are allowed") + req = Request(url, headers={"User-Agent": "hermes-kanban/attach"}) + chunks: list[bytes] = [] + total = 0 + with urlopen(req, timeout=30) as resp: # noqa: S310 — scheme checked above + content_type = resp.headers.get_content_type() if resp.headers else None + while True: + chunk = resp.read(1024 * 1024) + if not chunk: + break + total += len(chunk) + if total > max_bytes: + raise ValueError( + f"attachment exceeds {max_bytes // (1024 * 1024)} MB limit" + ) + chunks.append(chunk) + return b"".join(chunks), content_type + + +def _handle_attach_url(args: dict, **kw) -> str: + """Attach a file fetched server-side from a URL. + + The agent passes a URL; Hermes downloads it (with the shared size cap) + and stores it as a real attachment. Useful when the agent has a link + rather than the bytes. Only http/https URLs are accepted. + """ + from hermes_cli import kanban_db as kb + + tid = _default_task_id(args.get("task_id")) + if not tid: + return tool_error( + "task_id is required (or set HERMES_KANBAN_TASK in the env)" + ) + ownership_err = _enforce_worker_task_ownership(tid) + if ownership_err: + return ownership_err + url = args.get("url") + if not url or not str(url).strip(): + return tool_error("url is required") + url = str(url).strip() + filename = args.get("filename") or args.get("title") + if not filename or not str(filename).strip(): + # Derive a name from the URL path's leaf component. + from urllib.parse import unquote, urlparse + leaf = unquote(urlparse(url).path.rsplit("/", 1)[-1]).strip() + filename = leaf or "download" + content_type = args.get("content_type") + board = args.get("board") + try: + data, fetched_ct = _download_url_with_cap(url, kb._MAX_ATTACHMENT_BYTES) + except ValueError as e: + return tool_error(f"kanban_attach_url: {e}") + except Exception as e: + logger.exception("kanban_attach_url download failed") + return tool_error(f"kanban_attach_url: failed to fetch {url}: {e}") + try: + _, conn = _connect(board=board) + try: + att_id = kb.store_attachment_bytes( + conn, + tid, + str(filename), + data, + content_type=content_type or fetched_ct, + uploaded_by="agent", + board=board, + ) + return _ok(task_id=tid, attachment_id=att_id, size=len(data)) + finally: + conn.close() + except kb.AttachmentTooLarge as e: + return tool_error(f"kanban_attach_url: {e}") + except ValueError as e: + return tool_error(f"kanban_attach_url: {e}") + except Exception as e: + logger.exception("kanban_attach_url failed") + return tool_error(f"kanban_attach_url: {e}") + + +def _handle_attachments(args: dict, **kw) -> str: + """List a task's attachments (read-only; no ownership restriction).""" + tid = _default_task_id(args.get("task_id")) + if not tid: + return tool_error( + "task_id is required (or set HERMES_KANBAN_TASK in the env)" + ) + board = args.get("board") + try: + kb, conn = _connect(board=board) + try: + if kb.get_task(conn, tid) is None: + return tool_error(f"task {tid} not found") + atts = kb.list_attachments(conn, tid) + return json.dumps({ + "ok": True, + "task_id": tid, + "attachments": [ + { + "id": a.id, + "filename": a.filename, + "content_type": a.content_type, + "size": a.size, + "uploaded_by": a.uploaded_by, + "stored_path": a.stored_path, + "created_at": a.created_at, + } + for a in atts + ], + }) + finally: + conn.close() + except ValueError as e: + return tool_error(f"kanban_attachments: {e}") + except Exception as e: + logger.exception("kanban_attachments failed") + return tool_error(f"kanban_attachments: {e}") + + def _handle_create(args: dict, **kw) -> str: """Create a child task. Orchestrator workers use this to fan out. @@ -1396,6 +1583,102 @@ KANBAN_COMMENT_SCHEMA = { }, } +KANBAN_ATTACH_SCHEMA = { + "name": "kanban_attach", + "description": ( + "Attach a file to a task by passing its bytes inline (base64). " + "Use for genuine file artifacts the next worker or a human should " + "be able to download — generated reports, images, exports. The " + "file is stored as a real attachment (not a comment link) under " + "the task's attachments dir, capped at 25 MB. Prefer " + "kanban_attach_url when you only have a URL." + ), + "parameters": { + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": _DESC_TASK_ID_DEFAULT, + }, + "filename": { + "type": "string", + "description": ( + "File name to store it under (e.g. 'report.pdf'). " + "Directory components are stripped; only the leaf is kept." + ), + }, + "content_base64": { + "type": "string", + "description": "The file contents, base64-encoded. Max 25 MB decoded.", + }, + "content_type": { + "type": "string", + "description": "Optional MIME type (e.g. 'application/pdf').", + }, + "board": _board_schema_prop(), + }, + "required": ["filename", "content_base64"], + }, +} + +KANBAN_ATTACH_URL_SCHEMA = { + "name": "kanban_attach_url", + "description": ( + "Attach a file to a task by URL — Hermes downloads it server-side " + "and stores it as a real attachment (capped at 25 MB). Use when " + "you have a link rather than the bytes. Only http/https URLs are " + "accepted." + ), + "parameters": { + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": _DESC_TASK_ID_DEFAULT, + }, + "url": { + "type": "string", + "description": "http(s) URL to fetch and store.", + }, + "filename": { + "type": "string", + "description": ( + "Optional name to store it under. Defaults to the URL " + "path's leaf component." + ), + }, + "content_type": { + "type": "string", + "description": ( + "Optional MIME type override. Defaults to the " + "Content-Type the server returns." + ), + }, + "board": _board_schema_prop(), + }, + "required": ["url"], + }, +} + +KANBAN_ATTACHMENTS_SCHEMA = { + "name": "kanban_attachments", + "description": ( + "List the files attached to a task: id, filename, content_type, " + "size, who uploaded it, and the absolute on-disk path you can read." + ), + "parameters": { + "type": "object", + "properties": { + "task_id": { + "type": "string", + "description": _DESC_TASK_ID_DEFAULT, + }, + "board": _board_schema_prop(), + }, + "required": [], + }, +} + KANBAN_CREATE_SCHEMA = { "name": "kanban_create", "description": ( @@ -1653,6 +1936,33 @@ registry.register( emoji="💬", ) +registry.register( + name="kanban_attach", + toolset="kanban", + schema=KANBAN_ATTACH_SCHEMA, + handler=_handle_attach, + check_fn=_check_kanban_mode, + emoji="📎", +) + +registry.register( + name="kanban_attach_url", + toolset="kanban", + schema=KANBAN_ATTACH_URL_SCHEMA, + handler=_handle_attach_url, + check_fn=_check_kanban_mode, + emoji="📎", +) + +registry.register( + name="kanban_attachments", + toolset="kanban", + schema=KANBAN_ATTACHMENTS_SCHEMA, + handler=_handle_attachments, + check_fn=_check_kanban_mode, + emoji="📎", +) + registry.register( name="kanban_create", toolset="kanban", diff --git a/toolsets.py b/toolsets.py index 03e64fdba4c0..1be62780d0ca 100644 --- a/toolsets.py +++ b/toolsets.py @@ -75,6 +75,7 @@ _HERMES_CORE_TOOLS = [ "kanban_complete", "kanban_block", "kanban_heartbeat", "kanban_comment", "kanban_create", "kanban_link", "kanban_unblock", + "kanban_attach", "kanban_attach_url", "kanban_attachments", # Computer use (macOS, gated on cua-driver being installed via check_fn) "computer_use", ] @@ -264,14 +265,15 @@ TOOLSETS = { "set). The dispatcher runs inside the gateway by default; see " "`kanban.dispatch_in_gateway` in config.yaml. Lets workers mark " "tasks done with structured handoffs, block for human input, " - "heartbeat during long ops, comment on threads, and (for " - "orchestrators) list, unblock, and fan out tasks." + "heartbeat during long ops, comment on threads, attach files, and " + "(for orchestrators) list, unblock, and fan out tasks." ), "tools": [ "kanban_show", "kanban_list", "kanban_complete", "kanban_block", "kanban_heartbeat", "kanban_comment", "kanban_create", "kanban_link", "kanban_unblock", + "kanban_attach", "kanban_attach_url", "kanban_attachments", ], "includes": [], },