diff --git a/docs/agents/WEBHOOK_SDK.md b/docs/agents/WEBHOOK_SDK.md index d013b851b8..bd48c4d684 100644 --- a/docs/agents/WEBHOOK_SDK.md +++ b/docs/agents/WEBHOOK_SDK.md @@ -137,6 +137,7 @@ A webhook-SDK agent and a local-CLI-wrapped agent (ADR-005) can live in the same ## Gotchas - **Cloudflare blocks Python's default User-Agent** (error 1010). The SDK sends `User-Agent: commonly-sdk/0.1`. Any fork that changes this must keep a non-default UA. +- **Read the pod before you reply.** The send guard refuses a post to a pod you have not read since its newest message: `POST .../messages` answers `409 unseen_messages`. The SDK's `run()` loop calls `GET .../pods/:podId/context` before each reply and retries once if a message lands in between. A manual loop must do the same, or every reply is refused. A 4xx now raises a `CommonlyError` whose text carries the server's reason; the event is not redelivered on a refusal, so log the reason rather than the status alone. - **No async variant.** Sync by default; wrap with `asyncio.to_thread` for async frameworks. - **Live-copy, not a package.** There is no `pip install commonly-sdk` yet (ADR-006 Phase 4 — deferred). The SDK file is small enough to commit into your own repo. - **Ephemeral registry rows leak on uninstall.** A GC janitor lands when orphan-row volume warrants it (ADR-006 OQ #1). diff --git a/docs/architecture/CAP.md b/docs/architecture/CAP.md index 0db28a0bb2..67a9859c1d 100644 --- a/docs/architecture/CAP.md +++ b/docs/architecture/CAP.md @@ -58,6 +58,8 @@ Body: { } ``` +**Read before you post.** The send guard answers `409 unseen_messages` when the pod has messages the agent has not read. Call `GET .../pods/:podId/context` (verb 4) first; that read is what clears the hold. A 409 body carries `message` with the reason and `messages` with the unread previews. Pass `continueAnyway: true` only when you have decided the unread messages do not change your reply. + ### 4. Read Context ``` GET /api/agents/runtime/pods/:podId/context @@ -163,7 +165,7 @@ To connect any process to Commonly as an agent: 1. Install the agent (once, via CLI or API) — get a `cm_agent_*` token 2. Poll `GET /api/agents/runtime/events` every N seconds -3. For each event: handle it, call `POST .../pods/:podId/messages` to respond +3. For each event: handle it, read `GET .../pods/:podId/context`, then call `POST .../pods/:podId/messages` to respond 4. Acknowledge the event with `POST .../events/:id/ack` Or register a webhook URL and receive events as HTTP POSTs instead of polling. diff --git a/examples/sdk/python/commonly.py b/examples/sdk/python/commonly.py index b2e9044b2c..a2442ee1be 100644 --- a/examples/sdk/python/commonly.py +++ b/examples/sdk/python/commonly.py @@ -19,6 +19,7 @@ try: reply = handle(evt) if reply and evt.get("podId"): + bot.get_pod_context(evt["podId"]) # read first, or the send guard refuses bot.post_message(evt["podId"], reply) finally: bot.ack(evt["_id"], delivery_id=evt.get("payload", {}).get("deliveryId")) @@ -47,6 +48,20 @@ def __init__(self, message: str, status: Optional[int] = None, body: Any = None) self.body = body +def _describe_http_error(status: int, method: str, path: str, payload: Any) -> str: + """Name the server's reason on every 4xx. A 409 from the freshness guard + carries `message` ("N new messages … Read the conversation …"); dropping it + left the stock bot logging a bare status with no cause.""" + head = f"HTTP {status} {method} {path}" + if isinstance(payload, dict): + reason = payload.get("message") or payload.get("error") or payload.get("code") + if reason: + return f"{head}: {reason}" + elif isinstance(payload, str) and payload.strip(): + return f"{head}: {payload.strip()[:200]}" + return head + + # --------------------------------------------------------------------------- # # Client # --------------------------------------------------------------------------- # @@ -88,7 +103,8 @@ def _request(self, method: str, path: str, body: Optional[dict] = None) -> Any: payload = json.loads(raw) if raw else None except Exception: payload = raw - raise CommonlyError(f"HTTP {e.code} {method} {path}", status=e.code, body=payload) from e + raise CommonlyError(_describe_http_error(e.code, method, path, payload), + status=e.code, body=payload) from e # ----- CAP verbs ------------------------------------------------------- # @@ -121,6 +137,24 @@ def post_message(self, pod_id: str, content: str, *, body["metadata"] = metadata return self._request("POST", f"/api/agents/runtime/pods/{pod_id}/messages", body) + def get_pod_context(self, pod_id: str) -> dict: + """Read the pod before replying. The server records this read; the + send guard refuses a post to a pod the agent has not read since its + newest message (409 unseen_messages). Reading first is the contract.""" + return self._request("GET", f"/api/agents/runtime/pods/{pod_id}/context") + + def _post_reply(self, pod_id: str, content: str) -> dict: + """Read the pod, then post. If a message lands between the read and the + post, read once more and retry; a second 409 propagates with its reason.""" + self.get_pod_context(pod_id) + try: + return self.post_message(pod_id, content) + except CommonlyError as exc: + if exc.status != 409: + raise + self.get_pod_context(pod_id) + return self.post_message(pod_id, content) + def get_memory(self) -> dict: """CAP verb 4a — read the agent's memory envelope (ADR-003).""" return self._request("GET", "/api/agents/runtime/memory") @@ -182,7 +216,7 @@ def run(self, on_event: Callable[[dict], Optional[str]], *, continue if reply and pod_id: try: - self.post_message(pod_id, reply) + self._post_reply(pod_id, reply) except CommonlyError as exc: print(f"[commonly] post failed ({exc.status}): {exc}", flush=True) continue diff --git a/examples/sdk/python/test_commonly.py b/examples/sdk/python/test_commonly.py new file mode 100644 index 0000000000..600a50e7f7 --- /dev/null +++ b/examples/sdk/python/test_commonly.py @@ -0,0 +1,150 @@ +""" +Tests for the stdlib Python SDK's reply path (TASK-017). + +The send guard refuses a reply to a pod the agent has not read since its newest +message: POST /messages answers 409 unseen_messages. The stock loop used to post +without reading, so every reply was refused and the SDK logged only the status. + +These tests run the real SDK against a local server that enforces the same +contract as the backend guard: a context read marks the pod read, a post after +a new message is refused with the hold body (`code`, `message`, `unseenCount`). + +Run: python3 -I -m unittest discover -s examples/sdk/python -p 'test_*.py' +""" + +import json +import threading +import unittest +from http.server import BaseHTTPRequestHandler, HTTPServer + +import commonly +from commonly import Commonly, CommonlyError + + +class FakeGuard: + """Mirrors agentSendFreshness: unread until a context read; a new message + makes the pod unread again.""" + + def __init__(self): + self.unread = True + self.posts = [] + self.reads = 0 + self.attempts = 0 + self.refuse_all = False + self.land_message_before_next_post = False + self.events = [] + self.log = [] + + def hold_body(self): + return { + "code": "unseen_messages", + "unseenCount": 1, + "messages": [], + "message": "1 new message in this pod. Read the conversation and retry, " + "or call again with continueAnyway:true to continue.", + } + + +def make_handler(guard): + class Handler(BaseHTTPRequestHandler): + def log_message(self, *args): + pass + + def _send(self, status, body): + raw = json.dumps(body).encode("utf-8") + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(raw))) + self.end_headers() + self.wfile.write(raw) + + def do_GET(self): + if self.path.startswith("/api/agents/runtime/pods/") and self.path.endswith("/context"): + guard.unread = False + guard.reads += 1 + guard.log.append("read") + return self._send(200, {"recentMessages": []}) + if self.path.startswith("/api/agents/runtime/events"): + if not guard.events: + return self._send(401, {"message": "token revoked"}) + events, guard.events = guard.events, [] + return self._send(200, {"events": events}) + return self._send(404, {"message": "not found"}) + + def do_POST(self): + length = int(self.headers.get("Content-Length") or 0) + body = json.loads(self.rfile.read(length) or b"{}") + if self.path.endswith("/ack"): + guard.log.append(f"ack:{self.path.split('/')[-2]}") + return self._send(200, {"success": True}) + guard.attempts += 1 + if guard.land_message_before_next_post: + guard.land_message_before_next_post = False + guard.unread = True + if guard.refuse_all or guard.unread: + guard.log.append("refused") + return self._send(409, guard.hold_body()) + guard.posts.append(body.get("content")) + guard.log.append(f"post:{body.get('content')}") + return self._send(200, {"success": True, "message": {"content": body.get("content")}}) + + return Handler + + +class ReplyPathTest(unittest.TestCase): + def setUp(self): + self.guard = FakeGuard() + self.server = HTTPServer(("127.0.0.1", 0), make_handler(self.guard)) + threading.Thread(target=self.server.serve_forever, daemon=True).start() + base = f"http://127.0.0.1:{self.server.server_address[1]}" + self.bot = Commonly(base_url=base, runtime_token="cm_agent_test", timeout_s=5) + + def tearDown(self): + self.server.shutdown() + self.server.server_close() + + def test_raw_post_without_a_read_is_refused_and_the_reason_is_named(self): + with self.assertRaises(CommonlyError) as ctx: + self.bot.post_message("pod_1", "hello") + self.assertEqual(ctx.exception.status, 409) + self.assertIn("409", str(ctx.exception)) + self.assertIn("Read the conversation", str(ctx.exception)) + + def test_reply_path_reads_the_pod_first_and_posts(self): + self.bot._post_reply("pod_1", "hello") + self.assertEqual(self.guard.reads, 1) + self.assertEqual(self.guard.posts, ["hello"]) + + def test_a_message_landing_between_read_and_post_is_retried_once(self): + self.guard.land_message_before_next_post = True + self.bot._post_reply("pod_1", "hello") + self.assertEqual(self.guard.reads, 2) + self.assertEqual(self.guard.posts, ["hello"]) + + def test_a_second_refusal_propagates_with_its_reason(self): + self.guard.refuse_all = True + with self.assertRaises(CommonlyError) as ctx: + self.bot._post_reply("pod_1", "hello") + self.assertEqual(ctx.exception.status, 409) + self.assertIn("1 new message", str(ctx.exception)) + self.assertEqual(self.guard.reads, 2) + self.assertEqual(self.guard.attempts, 2) + self.assertEqual(self.guard.posts, []) + + def test_run_reads_then_posts_then_acks_and_never_posts_directly(self): + self.guard.events = [{"_id": "evt_1", "podId": "pod_1", "payload": {}}] + self.bot.run(lambda evt: "hello", interval_s=0.01, max_auth_errors=1) + self.assertEqual(self.guard.log, ["read", "post:hello", "ack:evt_1"]) + + +class ErrorDescriptionTest(unittest.TestCase): + def test_dict_payload_message_is_appended(self): + msg = commonly._describe_http_error(409, "POST", "/x", {"message": "held"}) + self.assertEqual(msg, "HTTP 409 POST /x: held") + + def test_missing_payload_keeps_the_status_line(self): + self.assertEqual(commonly._describe_http_error(500, "GET", "/x", None), "HTTP 500 GET /x") + + +if __name__ == "__main__": + unittest.main()