From 880c7859bd0831153cdbc20b7f656609308e3c7a Mon Sep 17 00:00:00 2001 From: Rohit C Prasad Date: Sun, 16 Aug 2026 18:34:04 -0700 Subject: [PATCH] Work-item image attachments: content-addressed store + attach on every front door Blobs live in state-dir attachments/ (sha256-named); the log carries only attachment:// refs on a normal comment event. Attach authority = comment authority; images-only allowlist with magic-byte check, 10MB cap; in-app tool + API + CLI + MCP. --- coworker/server/app.py | 46 ++++++++++++++ coworker/server/manager.py | 11 +++- coworker/teams/attachments.py | 108 ++++++++++++++++++++++++++++++++ coworker/teams/cli.py | 37 +++++++++++ coworker/teams/dialect.py | 83 +++++++++++++++++++++++- coworker/teams/mcp_server.py | 19 ++++++ coworker/teams/tools.py | 26 ++++++++ tests/test_team_open_surface.py | 90 ++++++++++++++++++++++++++ 8 files changed, 417 insertions(+), 3 deletions(-) create mode 100644 coworker/teams/attachments.py diff --git a/coworker/server/app.py b/coworker/server/app.py index 77d6f7fa..34ed584d 100644 --- a/coworker/server/app.py +++ b/coworker/server/app.py @@ -921,6 +921,52 @@ def create_app(manager: SessionManager) -> FastAPI: ), ) + @app.post("/v1/board/items/attach") + def board_attach(request: Request, body: dict): + body = body or {} + + def run(actor): + raw = str(body.get("data_b64", "")) + # Cheap pre-decode bound: base64 is ~4/3 of the payload, so anything + # multiples over the cap is refused before allocating the decode. + if len(raw) > 15 * 1024 * 1024: + return JSONResponse( + {"error": "attachment exceeds 10MB"}, status_code=400 + ) + try: + data = base64.b64decode(raw, validate=True) + except (binascii.Error, ValueError): + return JSONResponse( + {"error": "data_b64 is not valid base64"}, status_code=400 + ) + ref = manager.attachment_store.put( + data, str(body.get("filename", "")) + ) + filename = str(body.get("filename", "")) + event = manager.team_store.comment( + str(body.get("space", "")), + actor, + int(body.get("id", 0)), + str(body.get("caption", "")) or f"attached {filename}", + refs=[ref], + ) + return {"ref": ref, "seq": event["seq"]} + + return _board(request, run) + + @app.get("/v1/board/attachment") + def board_attachment(request: Request, name: str): + def run(actor): + from fastapi.responses import Response + + path = manager.attachment_store.path_for(name) + return Response( + content=path.read_bytes(), + media_type=manager.attachment_store.mime_for(name), + ) + + return _board(request, run) + @app.get("/v1/board/policy") def board_get_policy(request: Request, space: str): return _board(request, lambda actor: manager.team_store.policy(space)) diff --git a/coworker/server/manager.py b/coworker/server/manager.py index fd76e1fe..7c6ba258 100644 --- a/coworker/server/manager.py +++ b/coworker/server/manager.py @@ -89,6 +89,7 @@ from ..teams import JournalStore, Role as TeamRole, TeamStore, board_tools, jour from ..teams.model import space_for_workspace from ..teams.chat import ChatStore from ..teams.registry import TeamRegistry, TeamWorker +from ..teams.attachments import AttachmentStore from ..teams.tokens import BoardTokens from ..skills import ( SessionSkillStore, @@ -216,6 +217,9 @@ class SessionManager: # External board clients (OPE-100): join tokens bind actor+role; the # `/v1/board` API resolves them and the store enforces authority. self.board_tokens = BoardTokens(base / "board-tokens.json") + # Work-item attachments (OPE-105): content-addressed blobs next to the + # board; the log carries only `attachment://` refs. + self.attachment_store = AttachmentStore(base / "attachments") self._team_inflight: set[str] = set() # Lead-session last-turn timestamps for the check-in backstop (monotonic-ish # wall clock; restart resets the clock rather than firing a wake storm). @@ -1542,7 +1546,12 @@ class SessionManager: persona=agent.name, session_id=session_id, ) - tools = board_tools(self.team_store, space=space, actor=actor) + journal_tools( + tools = board_tools( + self.team_store, + space=space, + actor=actor, + attachments=self.attachment_store, + ) + journal_tools( self.journal_store, actor=actor, space=space ) if role == "lead": diff --git a/coworker/teams/attachments.py b/coworker/teams/attachments.py new file mode 100644 index 00000000..807981c8 --- /dev/null +++ b/coworker/teams/attachments.py @@ -0,0 +1,108 @@ +"""Content-addressed attachments for board items — screenshots first. + +Review artifacts don't belong in the repo (they aren't source, and they die with +checkouts) and don't belong in the board log (events carry refs, never blobs — no +megabytes under the hash chain). They live here: files named by their sha256 in the +state dir, bridged into the board as a normal comment event carrying an +`attachment://.#` ref. + +Content addressing buys three things: dedupe for free (the same screenshot attached +twice stores once), immutability by construction (the ref can never dangle onto +changed bytes), and location independence — on a hosted board the same ref resolves +to object storage instead of this directory. + +Scope is images-only and ~10MB to start; the allowlist is the policy choke point +when that widens. +""" + +from __future__ import annotations + +import hashlib +import re +from pathlib import Path +from typing import Optional + +from .model import BoardError + +ATTACHMENT_SCHEME = "attachment://" +MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024 + +# Extension → mime for the types we accept. Sniffed magic must agree with the +# claimed extension — a .png that isn't a PNG is refused, not renamed. +_IMAGE_TYPES = { + "png": "image/png", + "jpg": "image/jpeg", + "jpeg": "image/jpeg", + "gif": "image/gif", + "webp": "image/webp", +} + +_MAGIC = { + "png": b"\x89PNG\r\n\x1a\n", + "jpg": b"\xff\xd8\xff", + "jpeg": b"\xff\xd8\xff", + "gif": b"GIF8", + "webp": b"RIFF", # RIFF….WEBP — checked with the fourcc below +} + +_STORED_NAME = re.compile(r"[0-9a-f]{64}\.[a-z0-9]{1,5}") + + +class AttachmentStore: + def __init__(self, root: str | Path) -> None: + self.root = Path(root).expanduser() + + def put(self, data: bytes, filename: str) -> str: + """Store one attachment; returns its `attachment://` ref. Idempotent — + identical bytes land on the same file.""" + ext = _validate(data, filename) + stored = f"{hashlib.sha256(data).hexdigest()}.{ext}" + self.root.mkdir(parents=True, exist_ok=True) + target = self.root / stored + if not target.exists(): + tmp = target.with_suffix(target.suffix + ".tmp") + tmp.write_bytes(data) + tmp.replace(target) + safe_name = Path(filename).name.replace("#", "_") + return f"{ATTACHMENT_SCHEME}{stored}#{safe_name}" + + def path_for(self, stored: str) -> Path: + """Resolve a stored name (`.`) to its file. The strict name + check is the traversal guard — nothing else reaches the filesystem.""" + stored = stored.strip() + if not _STORED_NAME.fullmatch(stored): + raise BoardError(f"not an attachment name: {stored!r}") + path = self.root / stored + if not path.exists(): + raise BoardError(f"no attachment {stored}") + return path + + def mime_for(self, stored: str) -> str: + return _IMAGE_TYPES.get(stored.rsplit(".", 1)[-1], "application/octet-stream") + + +def stored_name(ref: str) -> Optional[str]: + """`attachment://.#` → `.`; None for other refs.""" + if not ref.startswith(ATTACHMENT_SCHEME): + return None + return ref[len(ATTACHMENT_SCHEME):].split("#", 1)[0] + + +def _validate(data: bytes, filename: str) -> str: + if not data: + raise BoardError("attachment is empty") + if len(data) > MAX_ATTACHMENT_BYTES: + raise BoardError( + f"attachment exceeds {MAX_ATTACHMENT_BYTES // (1024 * 1024)}MB" + ) + ext = Path(filename).suffix.lstrip(".").lower() + if ext not in _IMAGE_TYPES: + raise BoardError( + f"unsupported attachment type .{ext or '?'} — images only for now" + f" ({', '.join(sorted(set(_IMAGE_TYPES)))})" + ) + if not data.startswith(_MAGIC[ext]) or ( + ext == "webp" and data[8:12] != b"WEBP" + ): + raise BoardError(f"file content does not look like .{ext}") + return "jpg" if ext == "jpeg" else ext diff --git a/coworker/teams/cli.py b/coworker/teams/cli.py index 18dce0cd..666030dc 100644 --- a/coworker/teams/cli.py +++ b/coworker/teams/cli.py @@ -93,6 +93,15 @@ def _parser() -> argparse.ArgumentParser: p.add_argument("id", type=int) p.add_argument("assignee") + p = cmd("attach", _cmd_attach, "attach a screenshot/image to an item") + p.add_argument("id", type=int) + p.add_argument("file", help="image file (png/jpg/gif/webp, ≤10MB)") + p.add_argument("--caption", default="") + + p = cmd("attachment", _cmd_attachment, "download an attachment by ref or name") + p.add_argument("ref", help="attachment:// ref or . name") + p.add_argument("-o", "--out", default="", help="output path (default: basename)") + p = cmd("link", _cmd_link, "link two items") p.add_argument("src", type=int) p.add_argument("kind", choices=("parent", "blocks")) @@ -328,6 +337,34 @@ def _cmd_assign(args) -> int: return 0 +def _cmd_attach(args) -> int: + source = Path(args.file).expanduser() + if not source.is_file(): + print(f"error: no such file: {source}", file=sys.stderr) + return 1 + result = _dialect(args).attach( + _space(args), args.id, source.read_bytes(), source.name, caption=args.caption + ) + ref = result.get("ref") or next( + (r for r in (result.get("payload") or {}).get("refs", [])), "" + ) + print(json.dumps(result, indent=2) if args.json else f"attached → {ref}") + return 0 + + +def _cmd_attachment(args) -> int: + from .attachments import stored_name + + stored = stored_name(args.ref) or args.ref + data, _mime = _dialect(args).attachment(stored) + out = Path(args.out) if args.out else Path( + args.ref.rsplit("#", 1)[-1] if "#" in args.ref else stored + ) + out.write_bytes(data) + print(str(out)) + return 0 + + def _cmd_link(args) -> int: _dialect(args).link(_space(args), args.src, args.kind, args.dst) print("ok" if not args.json else json.dumps({"ok": True})) diff --git a/coworker/teams/dialect.py b/coworker/teams/dialect.py index aa328044..540b0806 100644 --- a/coworker/teams/dialect.py +++ b/coworker/teams/dialect.py @@ -78,6 +78,16 @@ class BoardDialect(Protocol): def assign(self, space: str, item_id: int, assignee: str) -> dict[str, Any]: ... def claim(self, space: str, item_id: int) -> dict[str, Any]: ... def link(self, space: str, src: int, kind: str, dst: int) -> dict[str, Any]: ... + def attach( + self, + space: str, + item_id: int, + data: bytes, + filename: str, + *, + caption: str = "", + ) -> dict[str, Any]: ... + def attachment(self, stored: str) -> tuple[bytes, str]: ... def policy(self, space: str) -> dict[str, Any]: ... def set_policy(self, space: str, *, claims: str) -> dict[str, Any]: ... def pending(self, *, limit: int = 200) -> list[dict[str, Any]]: ... @@ -111,11 +121,17 @@ class LocalDialect: """Direct store access, one bound identity. The headless/standalone backing.""" def __init__( - self, store: TeamStore, journal: Optional[JournalStore], actor: Actor + self, + store: TeamStore, + journal: Optional[JournalStore], + actor: Actor, + *, + attachments: Any = None, ) -> None: self.store = store self.journal = journal self.actor = actor + self.attachments = attachments def whoami(self) -> dict[str, Any]: return {"actor": self.actor.id, "role": self.actor.role.value} @@ -187,6 +203,34 @@ class LocalDialect: def link(self, space: str, src: int, kind: str, dst: int) -> dict[str, Any]: return self.store.link(space, self.actor, src, kind, dst) + def attach( + self, + space: str, + item_id: int, + data: bytes, + filename: str, + *, + caption: str = "", + ) -> dict[str, Any]: + # Attach = store blob + a normal comment event carrying the ref. Comment + # authority IS attach authority (workers attach on their slice only). + if self.attachments is None: + raise BoardError("no attachment store is attached to this board") + ref = self.attachments.put(data, filename) + return self.store.comment( + space, + self.actor, + item_id, + caption or f"attached {filename}", + refs=[ref], + ) + + def attachment(self, stored: str) -> tuple[bytes, str]: + if self.attachments is None: + raise BoardError("no attachment store is attached to this board") + path = self.attachments.path_for(stored) + return path.read_bytes(), self.attachments.mime_for(stored) + def policy(self, space: str) -> dict[str, Any]: return self.store.policy(space) @@ -392,6 +436,36 @@ class RemoteDialect: "/v1/board/link", {"space": space, "src": src, "kind": kind, "dst": dst} ) + def attach( + self, + space: str, + item_id: int, + data: bytes, + filename: str, + *, + caption: str = "", + ) -> dict[str, Any]: + import base64 + + return self._post( + "/v1/board/items/attach", + { + "space": space, + "id": item_id, + "filename": filename, + "caption": caption, + "data_b64": base64.b64encode(data).decode("ascii"), + }, + ) + + def attachment(self, stored: str) -> tuple[bytes, str]: + response = self._client.get("/v1/board/attachment", params={"name": stored}) + if response.status_code >= 400: + self._unwrap(response) # raises with the server's message + return response.content, response.headers.get( + "content-type", "application/octet-stream" + ) + def policy(self, space: str) -> dict[str, Any]: return self._get("/v1/board/policy", {"space": space}) @@ -466,9 +540,14 @@ def local_dialect( backing for the CLI and MCP server when no OpenWorker server is running.""" from pathlib import Path + from .attachments import AttachmentStore + base = Path(db_dir).expanduser() journal = JournalStore(base / "journal.db") store = TeamStore(base / "teams.db", journal=journal) return LocalDialect( - store, journal, Actor(id=actor, role=Role(role)) + store, + journal, + Actor(id=actor, role=Role(role)), + attachments=AttachmentStore(base / "attachments"), ) diff --git a/coworker/teams/mcp_server.py b/coworker/teams/mcp_server.py index bff7cd90..d3f67c31 100644 --- a/coworker/teams/mcp_server.py +++ b/coworker/teams/mcp_server.py @@ -99,6 +99,25 @@ def build(dialect, *, space: str): belong here. `refs` attach artifact pointers.""" return _safe(dialect.comment, space, item, body, refs=list(refs or [])) + @mcp.tool() + def board_attach(item: int, path: str, caption: str = "") -> Any: + """Attach a screenshot or image (png/jpg/gif/webp, ≤10MB) from a local + file to a work item — so the lead/reviewer can SEE what you did. Give it + a caption saying what the image shows. Great with review hand-offs.""" + from pathlib import Path as _Path + + source = _Path(path).expanduser() + if not source.is_file(): + return {"error": f"no such file: {path}"} + return _safe( + dialect.attach, + space, + item, + source.read_bytes(), + source.name, + caption=caption, + ) + @mcp.tool() def board_pending() -> Any: """Your unconsumed deliveries — assignments addressed to you, cancel diff --git a/coworker/teams/tools.py b/coworker/teams/tools.py index fa4f8845..c704e270 100644 --- a/coworker/teams/tools.py +++ b/coworker/teams/tools.py @@ -63,6 +63,7 @@ def board_tools( space: str, actor: Actor, taint: Callable[[], bool] = lambda: False, + attachments=None, ) -> list: """The board verbs for one agent, pre-bound to its space and identity. @@ -150,7 +151,32 @@ def board_tools( `blocks` (src blocks dst).""" return _call(store.link, space, actor, src, kind, dst) + def attach_image(item: int, path: str, caption: str = "") -> dict: + """Attach a screenshot or image file (png/jpg/gif/webp, ≤10MB) to a work + item so the lead/reviewer can SEE what you did — pair it with your review + hand-off. `caption` says what the image shows.""" + from pathlib import Path as _Path + + source = _Path(path).expanduser() + if not source.is_file(): + return {"error": f"no such file: {path}"} + try: + ref = attachments.put(source.read_bytes(), source.name) + except (BoardError, ValueError) as error: + return {"error": str(error)} + return _call( + store.comment, + space, + actor, + item, + caption or f"attached {source.name}", + refs=[ref], + taint=taint(), + ) + verbs = LEAD_VERBS if actor.role in (Role.USER, Role.LEAD) else WORKER_VERBS + if attachments is not None: + verbs = verbs + ("attach_image",) local = locals() out = [] for name in verbs: diff --git a/tests/test_team_open_surface.py b/tests/test_team_open_surface.py index 04a36d91..9f90b860 100644 --- a/tests/test_team_open_surface.py +++ b/tests/test_team_open_surface.py @@ -319,6 +319,95 @@ def test_pending_and_consume_over_the_wire(api): assert nia.pending() == [] +# ------------------------------------------------------------------ attachments + +PNG = b"\x89PNG\r\n\x1a\n" + b"drill-bytes" + + +def test_attachment_store_is_content_addressed(tmp_path): + from coworker.teams.attachments import AttachmentStore, stored_name + + store = AttachmentStore(tmp_path / "attachments") + ref = store.put(PNG, "shot.png") + assert ref.startswith("attachment://") and ref.endswith("#shot.png") + # identical bytes dedupe to the same file, whatever the filename says + assert stored_name(store.put(PNG, "other.png")) == stored_name(ref) + data = store.path_for(stored_name(ref)).read_bytes() + assert data == PNG + assert store.mime_for(stored_name(ref)) == "image/png" + + +def test_attachment_store_validates(tmp_path): + from coworker.teams.attachments import AttachmentStore + + store = AttachmentStore(tmp_path / "attachments") + with pytest.raises(BoardError, match="images only"): + store.put(b"#!/bin/sh", "run.sh") + with pytest.raises(BoardError, match="does not look like"): + store.put(b"not a png at all", "fake.png") + with pytest.raises(BoardError, match="not an attachment name"): + store.path_for("../../etc/passwd") + + +def test_attach_over_the_wire_and_fetch(api): + client, manager, app = api + from fastapi.testclient import TestClient + + lead_token = _tokens(manager).mint("lead-1", "lead") + nia_token = _tokens(manager).mint("nia", "worker") + lead = RemoteDialect( + "http://board.test", + lead_token, + client=TestClient(app, base_url="http://board.test"), + ) + nia = RemoteDialect( + "http://board.test", + nia_token, + client=TestClient(app, base_url="http://board.test"), + ) + item = lead.create_item("proj", title="With screenshot", criteria="c") + nia.claim("proj", item["id"]) + result = nia.attach( + "proj", item["id"], PNG, "after.png", caption="statements page, dark mode" + ) + assert result["ref"].startswith("attachment://") + + # the ref lands on the item as a comment with the caption + shown = lead.get_item("proj", item["id"]) + assert result["ref"] in shown["refs"] + assert shown["comments"][-1]["body"] == "statements page, dark mode" + + # and the lead can fetch the bytes back + from coworker.teams.attachments import stored_name + + data, mime = lead.attachment(stored_name(result["ref"])) + assert data == PNG and mime == "image/png" + + # a worker cannot attach to an item outside its slice + other = lead.create_item("proj", title="Not nia's", criteria="c") + lead.assign("proj", other["id"], "someone-else") + with pytest.raises(BoardError, match="assigned items"): + nia.attach("proj", other["id"], PNG, "sneaky.png") + + +def test_attach_rejects_bad_payloads_over_the_wire(api): + client, manager, app = api + lead_token = _tokens(manager).mint("lead-1", "lead") + headers = {"Authorization": f"Bearer {lead_token}"} + client.post( + "/v1/board/items", + headers=headers, + json={"space": "proj", "title": "T", "criteria": "c"}, + ) + bad = client.post( + "/v1/board/items/attach", + headers=headers, + json={"space": "proj", "id": 1, "filename": "x.png", "data_b64": "!!!"}, + ) + assert bad.status_code == 400 + assert "base64" in bad.json()["error"] + + # ------------------------------------------------------------------ tokens @@ -361,6 +450,7 @@ def test_mcp_tool_surface_is_role_scoped(tmp_path): worker_names = names(worker) lead_names = names(lead) assert "board_claim" in worker_names + assert "board_attach" in worker_names assert "board_assign" not in worker_names assert "board_policy" not in worker_names assert {"board_assign", "board_link", "board_policy"} <= lead_names