fix(web): fail over keyless auth rejections

plugins/web/keyless_mcp.py

@@ -47,6 +47,23 @@ def _is_rate_limitish(message: str) -> bool:
     return any(marker in (message or "").lower() for marker in _RATE_LIMIT_MARKERS)
 
 
+_AUTH_STATUS_RE = re.compile(
+    r"\b(?:http(?:\s+status)?|status(?:\s+code)?|client\s+error|error(?:\s+code)?)"
+    r"\s*[:=']*\s*(?:401|403)\b",
+    re.IGNORECASE,
+)
+
+
+def _is_search_failover_eligible(message: str) -> bool:
+    """Return whether another anonymous vendor may serve the search.
+
+    Rate limits and structured HTTP 401/403 provider rejections are local to
+    one free search endpoint. Free-text markers are deliberately ignored:
+    vendors may echo the query in an otherwise terminal error.
+    """
+    return _is_rate_limitish(message) or bool(_AUTH_STATUS_RE.search(message or ""))
+
+
 def _fail_msg(vendor: str, kind: str, exc: Any, *, other_backends: bool = True) -> str:
     label, env_key, site = _VENDOR_HINTS[vendor]
     alt = " or another web backend via `hermes tools`" if other_backends else ""
@@ -369,31 +386,32 @@ def _walk_ring(name: str, kind: str, call, throttled) -> tuple:
         if not throttled(result):
             return order, vendor, result, False
         if i + 1 < len(order):
-            logger.info("keyless %s %s throttled; failing over to %s", vendor, kind, order[i + 1])
+            logger.info("keyless %s %s unavailable; failing over to %s", vendor, kind, order[i + 1])
     return order, vendor, result, True
 
 
 def search_with_failover(name: str, query: str, limit: int = 5) -> Dict[str, Any]:
-    """Rate-limit-shaped errors advance to the next vendor, other errors stop the walk
-    (a malformed query fails everywhere). ``data.served_by`` is set when the serving
-    vendor differs from *name*."""
+    """Rate limits and anonymous-endpoint auth/policy rejections advance to the next
+    vendor, other errors stop the walk (a malformed query fails everywhere).
+    ``data.served_by`` is set when the serving vendor differs from *name*."""
 
     def _throttled(result: Dict[str, Any]) -> bool:
-        return not result.get("success") and _is_rate_limitish(result.get("error", ""))
+        return not result.get("success") and _is_search_failover_eligible(result.get("error", ""))
 
     order, vendor, result, exhausted = _walk_ring(name, "search", lambda v: _KEYLESS_SEARCHERS[v](query, limit), _throttled)
     if not order:
         return search_fail(_ALL_PAID_MSG)
     if exhausted:
-        result["error"] = f"{result.get('error', '')} (all keyless vendors throttled: {', '.join(order)})"
+        result["error"] = f"{result.get('error', '')} (all keyless vendors unavailable: {', '.join(order)})"
     elif result.get("success") and vendor != name:
         result.setdefault("data", {})["served_by"] = vendor
     return result
 
 
 def extract_with_failover(name: str, urls: List[str]) -> List[Dict[str, Any]]:
-    """Fails over only when EVERY url in a batch is rate-limit-shaped (partial failures
-    are page problems, returned as-is)."""
+    """Advances to the next ring vendor only when EVERY url in a batch comes
+    back with a rate-limit-shaped error — partial failures and HTTP 403s can
+    be page problems, not provider throttling, and return as-is."""
 
     def _all_throttled(results: List[Dict[str, Any]]) -> bool:
         return bool(results) and all(r.get("error", "") and _is_rate_limitish(r.get("error", "")) for r in results)

tests/tools/test_web_keyless_fallback.py

@@ -509,6 +509,26 @@ class TestKeylessFailover:
         """Pin *name* so the ring starts there deterministically."""
         monkeypatch.setattr(keyless_mcp, "_vendor_pinned", lambda n: n == name)
 
+    @pytest.mark.parametrize(
+        ("message", "expected"),
+        [
+            ("free MCP rate limit", True),
+            ("Client error '403 Forbidden'", True),
+            ("HTTP status 401: unauthorized", True),
+            ("status code=403", True),
+            ("HTTP 2403", False),
+            ("error 4031", False),
+            ("FORBIDDEN", False),
+            ("extract failed for 'forbidden kingdom trailer': connection timeout", False),
+            ("HTTP 400: malformed request", False),
+            ("", False),
+        ],
+    )
+    def test_search_failover_eligibility_requires_structured_status(
+        self, message, expected
+    ):
+        assert keyless_mcp._is_search_failover_eligible(message) is expected
+
     def test_search_fails_over_on_rate_limit(self, monkeypatch):
         self._pin(monkeypatch, "exa")
         monkeypatch.setitem(keyless_mcp._KEYLESS_SEARCHERS, "exa", lambda q, l: self._throttled("Exa"))
@@ -517,6 +537,27 @@ class TestKeylessFailover:
         assert out["success"] is True
         assert out["data"]["served_by"] == "parallel"
 
+    def test_search_fails_over_on_anonymous_provider_forbidden(self, monkeypatch):
+        self._pin(monkeypatch, "firecrawl")
+        monkeypatch.setitem(
+            keyless_mcp._KEYLESS_SEARCHERS,
+            "firecrawl",
+            lambda q, l: {
+                "success": False,
+                "error": "Keyless Firecrawl search failed: Client error '403 Forbidden'",
+            },
+        )
+        monkeypatch.setitem(
+            keyless_mcp._KEYLESS_SEARCHERS,
+            "keenable",
+            lambda q, l: self._ok("keenable"),
+        )
+
+        out = keyless_mcp.search_with_failover("firecrawl", "q")
+
+        assert out["success"] is True
+        assert out["data"]["served_by"] == "keenable"
+
     def test_search_no_failover_on_non_throttle_error(self, monkeypatch):
         self._pin(monkeypatch, "exa")
         monkeypatch.setitem(
@@ -541,7 +582,7 @@ class TestKeylessFailover:
             )
         out = keyless_mcp.search_with_failover("exa", "q")
         assert out["success"] is False
-        assert "all keyless vendors throttled" in out["error"]
+        assert "all keyless vendors unavailable" in out["error"]
 
     def test_search_walks_ring_past_multiple_throttles(self, monkeypatch):
         # exa -> parallel all throttled; firecrawl serves.
@@ -596,6 +637,31 @@ class TestKeylessFailover:
         out = keyless_mcp.extract_with_failover("exa", ["https://a", "https://b"])
         assert out == good
 
+    def test_extract_all_forbidden_stays_on_primary(self, monkeypatch):
+        self._pin(monkeypatch, "firecrawl")
+        forbidden = [
+            {"url": url, "title": "", "content": "", "error": "HTTP 403"}
+            for url in ("https://a", "https://b")
+        ]
+        called = []
+        monkeypatch.setitem(
+            keyless_mcp._KEYLESS_EXTRACTORS,
+            "firecrawl",
+            lambda urls: forbidden,
+        )
+        monkeypatch.setitem(
+            keyless_mcp._KEYLESS_EXTRACTORS,
+            "keenable",
+            lambda urls: called.append(1) or [],
+        )
+
+        out = keyless_mcp.extract_with_failover(
+            "firecrawl", ["https://a", "https://b"]
+        )
+
+        assert out == forbidden
+        assert not called
+
     def test_extract_partial_failure_stays_on_primary(self, monkeypatch):
         self._pin(monkeypatch, "exa")
         partial = [