fix: report HTTP 404 as endpoint-not-found instead of "malformed event"

modules/hook-context-intelligence/amplifier_module_hook_context_intelligence/handlers/logging_handler.py

@@ -85,7 +85,12 @@
 #: errors, HTTP 5xx, HTTP 429, HTTP 401).
 _TRANSIENT: str = "transient"
 #: Returned by _post when the event must be skipped permanently (HTTP 4xx
-#: other than 401/429, i.e. malformed or forbidden).
+#: other than 401/429). This is a RETRY-CLASSIFICATION bucket only -- it does
+#: NOT imply a single cause. The cause varies by status and is asserted only
+#: where actually known: 400/413/422 = malformed/unprocessable payload,
+#: 403 = forbidden (credentials), 404/410 = endpoint not found/gone (routing
+#: or deployment, not the payload). Any other 4xx is non-retryable but its
+#: cause is NOT asserted (see the message-layer branches in _worker).
 _PERMANENT: str = "permanent"
 
 
@@ -100,7 +105,14 @@ def _classify_http_outcome(status_code: int) -> str:
     - ``_PERMANENT``:  300 <= status < 400 (3xx redirect — deliberately not following;
       authenticated POST redirects risk bearer-token leakage to another host)
     - ``_TRANSIENT``:  401, 429, or any 5xx (retry forever w/ backoff)
-    - ``_PERMANENT``:  403, 400, 413, 422, and any other 4xx (loud skip)
+    - ``_PERMANENT``:  403, 400, 413, 422, 404, 410, and any other 4xx (loud skip)
+
+    This is a RETRY decision only — every status below is uniformly non-retryable.
+    It does NOT imply they share one cause: 400/413/422 mean the payload was
+    rejected, 403 means forbidden/credentials, 404/410 mean the endpoint itself
+    is missing/gone (routing or deployment, not the payload). The message layer
+    (see ``_worker``) asserts the cause per status instead of blaming the payload
+    for all of them.
     """
     if status_code < 300:
         return _DELIVERED
@@ -111,7 +123,9 @@ def _classify_http_outcome(status_code: int) -> str:
         return _PERMANENT
     if status_code == 401 or status_code == 429 or status_code >= 500:
         return _TRANSIENT
-    # 403, 400, 413, 422, and any other 4xx
+    # 403, 400, 413, 422, 404, 410, and any other 4xx — all non-retryable, but
+    # NOT all "malformed"; see the message-layer branches in _worker for the
+    # per-status cause.
     return _PERMANENT
 
 
@@ -701,7 +715,26 @@ async def _worker(self) -> None:
                                 self._record_forwarding_issue(
                                     "permanent_reject", "rejected event (HTTP 403)"
                                 )
-                            else:
+                            elif self._last_status in (404, 410):
+                                # Endpoint itself is missing/gone — a routing or deployment
+                                # problem (e.g. an undeployed route behind Azure APIM), NOT a
+                                # payload problem. Do not blame the payload for a 4xx that
+                                # means "there is nothing here to receive it."
+                                logger.warning(
+                                    "%s (%s) rejected event (HTTP %d) — endpoint not found;"
+                                    " verify the route is deployed and reachable behind the"
+                                    " gateway (this is not a payload problem).",
+                                    self._name,
+                                    self._url,
+                                    self._last_status,
+                                )
+                                self._record_forwarding_issue(
+                                    "endpoint_not_found",
+                                    "endpoint not found (HTTP %s)" % self._last_status,
+                                )
+                            elif self._last_status in (400, 413, 422):
+                                # These are the ONLY statuses that genuinely mean the
+                                # payload was rejected as malformed/unprocessable.
                                 logger.warning(
                                     "%s (%s) rejected event (HTTP %d) — malformed event, skipped.",
                                     self._name,
@@ -712,6 +745,22 @@ async def _worker(self) -> None:
                                     "permanent_reject",
                                     "rejected event (HTTP %s) — malformed" % self._last_status,
                                 )
+                            else:
+                                # Any other non-retryable 4xx (405, 409, 421, 451, …). The
+                                # cause is NOT known here — do not assert "malformed"; that
+                                # claim is only true for the enumerated payload-rejection
+                                # set above. State only what is true: the server rejected it
+                                # and it will not be retried.
+                                logger.warning(
+                                    "%s (%s) rejected event (HTTP %d) — skipped.",
+                                    self._name,
+                                    self._url,
+                                    self._last_status,
+                                )
+                                self._record_forwarding_issue(
+                                    "permanent_reject",
+                                    "rejected event (HTTP %s)" % self._last_status,
+                                )
                     self._consecutive_failures = 0
                     self._auth_failures = 0
                     self._queue.task_done()
@@ -783,8 +832,12 @@ async def _post(self, event: str, data: dict[str, Any]) -> str:
             WriteTimeout, PoolTimeout), or httpx.RemoteProtocolError -- or HTTP
             401/429/5xx — caller should retry with backoff.
         _PERMANENT
-            HTTP 403 or any other 4xx (400, 413, 422, …) — event cannot be
-            delivered; caller should log loudly and skip.
+            HTTP 403, 404/410, 400/413/422, or any other 4xx — event cannot be
+            delivered; caller should log loudly and skip. This is a uniform
+            RETRY decision; the cause is NOT uniform — see the message-layer
+            branches in _worker, which assert 403=forbidden, 404/410=endpoint
+            not found/gone, 400/413/422=malformed payload, and stay
+            cause-neutral for any other 4xx.
 
         The Authorization header is produced PER REQUEST via self._strategy.headers().
         This ensures Entra tokens are refreshed by the azure-identity SDK when they

modules/hook-context-intelligence/tests/test_notifications.py

@@ -14,6 +14,8 @@
 from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import pytest
+
 from amplifier_module_hook_context_intelligence.handlers.logging_handler import (
     _DELIVERED,
     _TRANSIENT,
@@ -520,3 +522,118 @@ async def test_permanent_burst_is_rate_limited(self) -> None:
             f"Expected exactly 1 rate-limited PERMANENT warning (got {len(perm_calls)}): "
             f"{mock_logger.warning.call_args_list}"
         )
+
+
+# ---------------------------------------------------------------------------
+# TestPermanentMessagePolarity (404-wrong-classification fix, Option A')
+#
+# The catch-all branch used to hardcode "malformed event, skipped" for EVERY
+# _PERMANENT status not already special-cased (3xx, 403) -- asserting a
+# specific cause (bad payload) for statuses it cannot know are payload
+# problems (404, 405, 409, 410, 451, ...). This fixes the message layer only:
+# 404/410 now get their own "endpoint not found" branch, 400/413/422 remain
+# the genuine "malformed" set, and everything else falls into a cause-neutral
+# else that no longer claims "malformed". Retry/breaker behavior (_PERMANENT
+# classification itself) is unchanged -- see test_classification.py.
+# ---------------------------------------------------------------------------
+
+
+class TestPermanentMessagePolarity:
+    """Message-layer cause assertions must match what's actually known (Option A')."""
+
+    @pytest.mark.parametrize("status_code", [404, 410])
+    async def test_permanent_endpoint_not_found_logs_correct_cause(self, status_code: int) -> None:
+        """HTTP 404/410 -> warning says 'endpoint not found', NOT 'malformed'."""
+        d = _dispatcher()
+        d._client = _mock_client([_make_response(status_code)])
+        d._sleep_backoff = AsyncMock()  # type: ignore[method-assign]
+
+        with patch(LOGGER_PATH) as mock_logger:
+            d.enqueue("e1", {"session_id": "s1"})
+            await asyncio.wait_for(d._queue.join(), timeout=2.0)
+
+        rendered = str(mock_logger.warning.call_args_list)
+        assert "endpoint not found" in rendered, (
+            f"Expected 'endpoint not found' for HTTP {status_code}, got: {rendered}"
+        )
+        assert "malformed" not in rendered, (
+            f"HTTP {status_code} must NOT be labeled 'malformed' (it's a routing/deployment"
+            f" problem, not a payload problem): {rendered}"
+        )
+        await d.close()
+
+    @pytest.mark.parametrize("status_code", [400, 413, 422])
+    async def test_permanent_payload_statuses_still_log_malformed(self, status_code: int) -> None:
+        """HTTP 400/413/422 -> still 'malformed event, skipped' (regression anchor)."""
+        d = _dispatcher()
+        d._client = _mock_client([_make_response(status_code)])
+        d._sleep_backoff = AsyncMock()  # type: ignore[method-assign]
+
+        with patch(LOGGER_PATH) as mock_logger:
+            d.enqueue("e1", {"session_id": "s1"})
+            await asyncio.wait_for(d._queue.join(), timeout=2.0)
+
+        perm_calls = [
+            c for c in mock_logger.warning.call_args_list if "malformed event, skipped" in str(c)
+        ]
+        assert len(perm_calls) == 1, (
+            f"Expected 1 PERMANENT warning for {status_code} with 'malformed event, skipped', "
+            f"got {len(perm_calls)}: {mock_logger.warning.call_args_list}"
+        )
+        await d.close()
+
+    async def test_permanent_unenumerated_4xx_is_cause_neutral(self) -> None:
+        """HTTP 405 (not enumerated anywhere) -> warning does NOT say 'malformed'.
+
+        Proves the catch-all else is now cause-neutral instead of defaulting
+        every unrecognized _PERMANENT status to "malformed event, skipped".
+        """
+        d = _dispatcher()
+        d._client = _mock_client([_make_response(405)])
+        d._sleep_backoff = AsyncMock()  # type: ignore[method-assign]
+
+        with patch(LOGGER_PATH) as mock_logger:
+            d.enqueue("e1", {"session_id": "s1"})
+            await asyncio.wait_for(d._queue.join(), timeout=2.0)
+
+        rendered = str(mock_logger.warning.call_args_list)
+        assert "malformed" not in rendered, (
+            f"HTTP 405 must NOT be labeled 'malformed' -- cause is unknown: {rendered}"
+        )
+        # logger.warning is called with an unformatted template + positional args
+        # (e.g. "...(HTTP %d) — skipped.", name, url, 405) -- assert on the
+        # literal template text and the status code arg separately rather than
+        # a pre-interpolated substring.
+        assert "rejected event (HTTP %d) — skipped." in rendered, (
+            f"Expected the cause-neutral template for HTTP 405: {rendered}"
+        )
+        assert "405" in rendered, f"Expected status code 405 in the warning call args: {rendered}"
+        await d.close()
+
+    async def test_permanent_403_still_logs_check_credentials(self) -> None:
+        """HTTP 403 -> unchanged: still 'check credentials' (regression anchor)."""
+        d = _dispatcher()
+        d._client = _mock_client([_make_response(403)])
+        d._sleep_backoff = AsyncMock()  # type: ignore[method-assign]
+
+        with patch(LOGGER_PATH) as mock_logger:
+            d.enqueue("e1", {"session_id": "s1"})
+            await asyncio.wait_for(d._queue.join(), timeout=2.0)
+
+        rendered = str(mock_logger.warning.call_args_list)
+        assert "check credentials" in rendered
+        await d.close()
+
+    async def test_permanent_redirect_still_logs_redirect_message(self) -> None:
+        """HTTP 302 -> unchanged: still the redirect message (regression anchor)."""
+        d = _dispatcher()
+        d._client = _mock_client([_make_response(302)])
+        d._sleep_backoff = AsyncMock()  # type: ignore[method-assign]
+
+        with patch(LOGGER_PATH) as mock_logger:
+            d.enqueue("e1", {"session_id": "s1"})
+            await asyncio.wait_for(d._queue.join(), timeout=2.0)
+
+        rendered = str(mock_logger.warning.call_args_list)
+        assert "unexpected redirect" in rendered
+        await d.close()