commune-dev · shanjairaj7 · Mar 1, 2026 · Mar 1, 2026
diff --git a/tests/review/__init__.py b/tests/review/__init__.py
diff --git a/tests/review/test_idempotency_spec.py b/tests/review/test_idempotency_spec.py
@@ -0,0 +1,117 @@
+"""
+Behavioral specification: idempotent email sending.
+
+These tests describe what correct idempotency key usage looks like.
+An agent that sends emails without idempotency keys will send duplicates
+when webhook retries or agent restarts occur.
+
+Correct pattern: derive the key from the triggering message ID so that
+every retry of the same webhook event produces the same key, and the
+Commune API deduplicates the send automatically.
+"""
+import pytest
+
+
+class TestIdempotencyKeyPattern:
+    """Specify that idempotency keys must be deterministic and message-scoped."""
+
+    def test_idempotency_key_is_deterministic(self):
+        """The same input must always produce the same idempotency key.
+
+        An idempotency key derived from random values (uuid4, timestamp)
+        changes on every call and defeats deduplication. The key must be
+        a pure function of the triggering event's stable identifiers.
+        """
+        webhook_message_id = "msg_inbound_abc123"
+
+        # Correct pattern: deterministic key derived from the trigger
+        key1 = f"reply-{webhook_message_id}"
+        key2 = f"reply-{webhook_message_id}"  # Same trigger, same retry
+
+        assert key1 == key2, "Idempotency key must be deterministic across retries"
+
+    def test_idempotency_key_includes_message_scope(self):
+        """Keys must be scoped to the specific trigger message, not globally unique.
+
+        A globally unique key (uuid4) is different on every invocation.
+        When a webhook is retried, the new uuid4 key would result in a second
+        send. A key derived from the triggering message ID is safe to retry.
+        """
+        import uuid
+
+        # Bad: globally unique — a different key every time the handler runs
+        bad_key = str(uuid.uuid4())
+
+        # Good: derived from the triggering message ID — stable across retries
+        trigger_message_id = "msg_webhook_xyz789"
+        good_key = f"reply-{trigger_message_id}"
+
+        # The good key is predictable and reproducible from the same input
+        assert good_key == f"reply-{trigger_message_id}"
+        # The bad key is guaranteed to differ from any deterministic key
+        assert bad_key != good_key
+
+    def test_send_with_idempotency_key_is_correct_pattern(self):
+        """Demonstrate the correct messages.send() call structure for production use.
+
+        In production, the call should be wrapped with an idempotency key so
+        that webhook retries don't produce duplicate emails. The mock confirms
+        the call is made exactly once per handler invocation.
+        """
+        from unittest.mock import MagicMock
+
+        mock_client = MagicMock()
+        mock_client.messages.send.return_value = MagicMock(
+            thread_id="t_xyz",
+            message_id="<msg_sent_001@mail.commune.email>",
+            status="queued",
+        )
+
+        # Correct: one send per webhook event, keyed to the inbound message ID
+        mock_client.messages.send(
+            to="customer@example.com",
+            subject="Re: Your question",
+            text="Here is the answer.",
+            inbox_id="i_support",
+            thread_id="t_original",
+        )
+
+        # The handler must call send() exactly once per event
+        mock_client.messages.send.assert_called_once()
+
+    def test_retry_with_same_key_does_not_duplicate(self):
+        """Simulates that the same idempotency_key on retry returns the same result.
+
+        When the Commune API receives a duplicate send request with the same
+        idempotency key, it returns the original result without re-sending.
+        Both calls must yield a result with the same message_id — confirming
+        that no duplicate email was delivered.
+        """
+        from unittest.mock import MagicMock
+
+        first_result = MagicMock(
+            thread_id="t_abc",
+            message_id="<msg_001@mail.commune.email>",
+            status="queued",
+        )
+        mock_send = MagicMock(return_value=first_result)
+
+        # First attempt
+        result1 = mock_send(
+            to="c@example.com",
+            subject="Re: Order",
+            text="Your order is ready.",
+            inbox_id="i_support",
+            thread_id="t_abc",
+        )
+        # Retry — in the real Commune API this returns the cached result
+        result2 = mock_send(
+            to="c@example.com",
+            subject="Re: Order",
+            text="Your order is ready.",
+            inbox_id="i_support",
+            thread_id="t_abc",
+        )
+
+        # Both results must reference the same sent message — no duplicate
+        assert result1.message_id == result2.message_id
diff --git a/tests/review/test_secure_webhook_spec.py b/tests/review/test_secure_webhook_spec.py
@@ -0,0 +1,130 @@
+"""
+Behavioral specification: secure webhook handler.
+
+These tests describe what a production-quality webhook handler MUST do.
+They serve as a specification — each test corresponds to a correctness or
+security requirement that reviewers should check.
+
+Signature format: v1={HMAC-SHA256(secret, "{timestamp_ms}.{body}")}
+Timestamp: Unix milliseconds (same as backend Date.now())
+Headers:   x-commune-signature, x-commune-timestamp
+"""
+import hashlib
+import hmac
+import json
+import time
+
+import pytest
+
+from commune.webhooks import verify_signature, WebhookVerificationError
+
+
+def _sign(payload: bytes, secret: str, timestamp_ms: str) -> str:
+    """Replicate the backend signing protocol: v1=HMAC-SHA256(secret, '{ts}.{body}')."""
+    digest = hmac.new(
+        secret.encode("utf-8"),
+        f"{timestamp_ms}.".encode("utf-8") + payload,
+        hashlib.sha256,
+    ).hexdigest()
+    return f"v1={digest}"
+
+
+class TestWebhookSignatureVerification:
+    """Verify that the webhook handler enforces signature validation."""
+
+    def test_valid_signature_is_accepted(self):
+        """A correctly signed payload should pass verification."""
+        payload = json.dumps({"event": "message.received"}).encode()
+        secret = "test_secret_abc123"
+        timestamp_ms = str(int(time.time() * 1000))
+        sig = _sign(payload, secret, timestamp_ms)
+        # Should not raise and should return True
+        result = verify_signature(payload=payload, signature=sig, secret=secret, timestamp=timestamp_ms)
+        assert result is True
+
+    def test_invalid_signature_raises(self):
+        """A payload with a wrong signature must be rejected."""
+        payload = json.dumps({"event": "message.received"}).encode()
+        timestamp_ms = str(int(time.time() * 1000))
+        with pytest.raises(WebhookVerificationError):
+            verify_signature(
+                payload=payload,
+                signature="v1=invalid_sig_hex_that_will_not_match",
+                secret="secret",
+                timestamp=timestamp_ms,
+            )
+
+    def test_raw_bytes_not_parsed_dict(self):
+        """Verification must use raw bytes, not a re-serialized dict.
+
+        Parsing and re-serializing a JSON body changes whitespace and key
+        ordering, so the HMAC computed over re-serialized bytes will not match
+        the signature computed by the backend over the original wire bytes.
+        Webhook handlers MUST pass request.body / request.get_data() directly.
+        """
+        raw = b'{"event":"message.received","data":{"id":1}}'
+        re_serialized = json.dumps(json.loads(raw)).encode()
+        # Confirm test setup: bytes differ after round-trip
+        assert raw != re_serialized, "Test setup: re-serialization should produce different bytes"
+
+        secret = "test_secret"
+        timestamp_ms = str(int(time.time() * 1000))
+        valid_sig = _sign(raw, secret, timestamp_ms)
+
+        # Signature is valid for the original raw bytes
+        assert verify_signature(payload=raw, signature=valid_sig, secret=secret, timestamp=timestamp_ms) is True
+
+        # But NOT for re-serialized bytes — they produce a different HMAC
+        with pytest.raises(WebhookVerificationError):
+            verify_signature(payload=re_serialized, signature=valid_sig, secret=secret, timestamp=timestamp_ms)
+
+    def test_expired_timestamp_is_rejected(self):
+        """Payloads more than 5 minutes old should be rejected (replay protection).
+
+        The default tolerance is 300 seconds. A timestamp 6+ minutes old
+        (360 000 ms) must be refused regardless of signature correctness.
+        """
+        payload = json.dumps({"event": "message.received"}).encode()
+        secret = "test_secret"
+        # 10 minutes ago in milliseconds
+        old_ts_ms = str(int(time.time() * 1000) - 600_000)
+        old_sig = _sign(payload, secret, old_ts_ms)
+        with pytest.raises(WebhookVerificationError, match="too old"):
+            verify_signature(
+                payload=payload,
+                signature=old_sig,
+                secret=secret,
+                timestamp=old_ts_ms,
+                tolerance_seconds=300,
+            )
+
+    def test_empty_secret_is_rejected(self):
+        """An empty webhook secret must never accept any payload.
+
+        An empty secret means the signing key has not been configured.
+        Accepting requests without a real secret would allow anyone to
+        craft a valid-looking webhook delivery.
+        """
+        payload = b'{"event": "test"}'
+        with pytest.raises((WebhookVerificationError, ValueError)):
+            verify_signature(
+                payload=payload,
+                signature="v1=any_sig",
+                secret="",
+                timestamp=str(int(time.time() * 1000)),
+            )
+
+    def test_missing_signature_is_rejected(self):
+        """A missing signature header must fail verification.
+
+        An empty signature string represents a missing x-commune-signature
+        header. The handler must reject the request before any HMAC work.
+        """
+        payload = b'{"event": "test"}'
+        with pytest.raises(WebhookVerificationError, match="Missing signature"):
+            verify_signature(
+                payload=payload,
+                signature="",
+                secret="secret",
+                timestamp=str(int(time.time() * 1000)),
+            )
diff --git a/tests/review/test_thread_continuity_spec.py b/tests/review/test_thread_continuity_spec.py
@@ -0,0 +1,129 @@
+"""
+Behavioral specification: email thread continuity.
+
+These tests describe the invariants that any email agent MUST maintain
+to ensure conversations are properly threaded in the recipient's email client.
+
+Key rule: every outbound reply must carry the thread_id from the triggering
+inbound message. Omitting thread_id starts a new thread instead of replying,
+breaking conversation history for both the agent and the customer.
+"""
+import pytest
+
+
+class TestThreadIdPropagation:
+    """Specify that thread_id must be preserved and propagated through reply chains."""
+
+    def test_send_result_has_thread_id(self):
+        """messages.send() must return a thread_id for use in subsequent replies.
+
+        The thread_id from SendMessageResult is the value to pass back into
+        messages.send(thread_id=...) for the next turn. Agents that discard
+        this value will lose thread continuity on the second reply.
+        """
+        from commune.types import SendMessageResult
+
+        result = SendMessageResult(
+            id="doc_001",
+            message_id="<msg001@mail.commune.email>",
+            thread_id="t_abc123",
+            status="queued",
+        )
+        assert result.thread_id is not None, "send() result must include thread_id"
+        assert result.thread_id.startswith("t_"), "thread_id should have expected prefix format"
+
+    def test_reply_requires_thread_id(self):
+        """A reply to a customer must always include the original thread_id.
+
+        Code that omits thread_id when replying is a correctness bug.
+        The correct call signature always passes thread_id as a keyword argument.
+        """
+        from unittest.mock import MagicMock
+
+        mock_client = MagicMock()
+        mock_client.messages.send.return_value = MagicMock(
+            thread_id="t_xyz",
+            message_id="<msg002@mail.commune.email>",
+            status="queued",
+        )
+
+        # Correct pattern: thread_id is explicitly passed
+        mock_client.messages.send(
+            to="customer@example.com",
+            subject="Re: Help with order",
+            text="Here is your answer...",
+            inbox_id="i_support",
+            thread_id="t_original",
+        )
+
+        call_kwargs = mock_client.messages.send.call_args.kwargs
+        assert "thread_id" in call_kwargs, "Reply MUST include thread_id"
+        assert call_kwargs["thread_id"] == "t_original"
+
+    def test_thread_id_from_webhook_payload(self):
+        """The thread_id required for replies is available in the webhook payload.
+
+        A Commune webhook payload for an inbound message includes thread_id
+        at data.thread_id. Agents MUST read from this path — not from a
+        stored lookup — so that replies stay in the correct thread even when
+        the customer starts a new conversation.
+        """
+        webhook_payload = {
+            "event": "message.received",
+            "data": {
+                "thread_id": "t_abc123",
+                "inbox_id": "i_support",
+                "sender": "customer@example.com",
+                "subject": "I need help",
+                "text": "Please help me with my order",
+            },
+        }
+
+        # The thread_id MUST be accessible at this exact path
+        thread_id = webhook_payload["data"]["thread_id"]
+        assert thread_id == "t_abc123"
+        assert thread_id is not None, "webhook payload must provide thread_id"
+
+    def test_thread_list_provides_thread_ids(self):
+        """threads.list() must return Thread objects with thread_id for polling workflows.
+
+        Agents that poll instead of using webhooks iterate threads.list().data
+        and must find a usable thread_id on each Thread object. The last_direction
+        field tells the agent whether a reply is needed.
+        """
+        from commune.types import Thread, ThreadList
+
+        mock_thread = Thread(
+            thread_id="t_abc123",
+            subject="Customer inquiry",
+            message_count=2,
+            last_direction="inbound",
+            last_message_at="2024-01-15T10:00:00Z",
+        )
+        thread_list = ThreadList(data=[mock_thread], has_more=False)
+
+        assert len(thread_list.data) == 1
+        assert thread_list.data[0].thread_id == "t_abc123"
+        assert thread_list.data[0].last_direction == "inbound"
+
+    def test_message_has_thread_id_field(self):
+        """Every Message object must have a thread_id field for building reply context.
+
+        When an agent loads a conversation via threads.messages(thread_id),
+        each returned Message carries its own thread_id. This allows the agent
+        to reconstruct the reply target even when messages are processed out of order.
+        """
+        from commune.types import Message, MessageMetadata, Participant
+
+        msg = Message(
+            message_id="<msg001@mail.example.com>",
+            thread_id="t_abc123",
+            direction="inbound",
+            content="Hello, I need help with my order.",
+            created_at="2024-01-15T10:00:00Z",
+            metadata=MessageMetadata(created_at="2024-01-15T10:00:00Z"),
+        )
+
+        assert msg.thread_id == "t_abc123"
+        assert msg.content == "Hello, I need help with my order."
+        assert msg.direction == "inbound"