fix: keyless rescue no longer re-fetches policy-blocked URLs
The one-shot keyless extract rescue (d1eefe6ac) treated ANY whole-batch
failure as a backend outage. A website-policy refusal also arrives as a
failed batch, so blocked URLs were routed through the free-tier ring:
in CI the ring's live fetch attempt returned a result for the wrong URL
or a bare None error, turning test_website_policy reds on main (slices
8/12 and 12/12) — and in production it would fetch content the user
explicitly blocked.
_rescue_extract now partitions policy blocks (blocked_by_policy flag or
policy error text) out of the rescue set: they are preserved verbatim,
only genuine failures ride the ring, and order/merge parity is kept.
Two sabotage-verified regression tests pin the class.
This commit is contained in:
@@ -301,3 +301,66 @@ def test_check_website_access_fails_open_on_malformed_config(tmp_path, monkeypat
|
||||
# With default path, errors are caught and fail open
|
||||
result = check_website_access("https://example.com")
|
||||
assert result is None # allowed, not crashed
|
||||
|
||||
|
||||
# ─── Keyless rescue must never re-fetch policy-blocked URLs ──────────────────
|
||||
|
||||
|
||||
def test_rescue_extract_skips_policy_blocked_results():
|
||||
"""A whole-batch failure that is actually a policy refusal must NOT be
|
||||
routed through the keyless rescue ring — that would fetch content the
|
||||
user explicitly blocked. Regression for CI reds on main (Aug 2026)."""
|
||||
from tools import web_tools
|
||||
|
||||
urls = ["https://blocked.test"]
|
||||
results = [{
|
||||
"url": "https://blocked.test",
|
||||
"title": "",
|
||||
"content": "",
|
||||
"error": "Blocked by website policy (rule: blocked.test)",
|
||||
"blocked_by_policy": {"rule": "blocked.test"},
|
||||
}]
|
||||
|
||||
def _boom(*a, **kw):
|
||||
raise AssertionError("keyless ring must not be called for policy blocks")
|
||||
|
||||
import plugins.web.keyless_mcp as ring
|
||||
orig = ring.extract_with_failover
|
||||
ring.extract_with_failover = _boom
|
||||
try:
|
||||
out = web_tools._rescue_extract("firecrawl", urls, results)
|
||||
finally:
|
||||
ring.extract_with_failover = orig
|
||||
|
||||
assert out == results # blocked results preserved verbatim
|
||||
|
||||
|
||||
def test_rescue_extract_mixed_batch_only_rescues_real_failures(monkeypatch):
|
||||
"""Mixed batch: the policy-blocked entry is preserved; only the genuine
|
||||
backend failure goes through the ring, and order is preserved."""
|
||||
from tools import web_tools
|
||||
|
||||
urls = ["https://blocked.test", "https://down.test"]
|
||||
results = [
|
||||
{"url": "https://blocked.test", "title": "", "content": "",
|
||||
"error": "Blocked by website policy", "blocked_by_policy": {"rule": "blocked.test"}},
|
||||
{"url": "https://down.test", "title": "", "content": "",
|
||||
"error": "backend 500"},
|
||||
]
|
||||
|
||||
seen = {}
|
||||
|
||||
def fake_failover(provider_name, rescue_urls):
|
||||
seen["urls"] = list(rescue_urls)
|
||||
return [{"url": u, "title": "ok", "content": "rescued", "error": ""} for u in rescue_urls]
|
||||
|
||||
monkeypatch.setattr(
|
||||
"plugins.web.keyless_mcp.extract_with_failover", fake_failover
|
||||
)
|
||||
|
||||
out = web_tools._rescue_extract("firecrawl", urls, results)
|
||||
|
||||
assert seen["urls"] == ["https://down.test"]
|
||||
assert out[0]["error"] == "Blocked by website policy" # untouched
|
||||
assert out[1]["content"] == "rescued"
|
||||
assert out[1]["metadata"]["rescued_from"] == "firecrawl"
|
||||
|
||||
+32
-3
@@ -520,23 +520,47 @@ def _rescue_search(provider_name: str, original_error: str, query: str, limit: i
|
||||
}
|
||||
|
||||
|
||||
def _policy_blocked_result(result: dict) -> bool:
|
||||
"""True when an extract result failed because of the user's website
|
||||
policy — an intentional refusal, never a backend outage. Policy blocks
|
||||
must NOT be rescued: routing the same URL through the keyless ring
|
||||
would fetch content the user explicitly blocked."""
|
||||
if result.get("blocked_by_policy"):
|
||||
return True
|
||||
return "blocked by website policy" in str(result.get("error") or "").lower()
|
||||
|
||||
|
||||
def _rescue_extract(provider_name: str, urls: list, results: list) -> list:
|
||||
"""One-shot keyless-ring rescue for a failed keyed/configured extract.
|
||||
|
||||
Fires only when EVERY url failed (whole-backend failure); partial
|
||||
results are page problems and pass through untouched. Stateless —
|
||||
the next web_extract call attempts the chosen backend again.
|
||||
|
||||
Website-policy refusals are intentional, not failures: entries flagged
|
||||
by ``_policy_blocked_result`` are never re-fetched through the ring and
|
||||
their original (blocked) results are preserved verbatim.
|
||||
"""
|
||||
from plugins.web.keyless_mcp import extract_with_failover
|
||||
|
||||
# Partition out policy blocks. Rescue only genuine backend failures.
|
||||
if len(results) == len(urls):
|
||||
rescue_idx = [i for i, r in enumerate(results) if not _policy_blocked_result(r)]
|
||||
else: # defensive: provider broke order parity — treat all as rescueable
|
||||
rescue_idx = list(range(len(results)))
|
||||
if not rescue_idx:
|
||||
return results # every failure is an intentional policy block
|
||||
|
||||
rescue_urls = [urls[i] for i in rescue_idx] if len(results) == len(urls) else list(urls)
|
||||
original_error = next(
|
||||
(r.get("error") for r in results if r.get("error")), "extract failed"
|
||||
(results[i].get("error") for i in rescue_idx if results[i].get("error")),
|
||||
"extract failed",
|
||||
)
|
||||
logger.warning(
|
||||
"web_extract backend '%s' failed all %d URL(s) (%s); one-shot keyless rescue",
|
||||
provider_name, len(urls), (original_error or "")[:200],
|
||||
provider_name, len(rescue_urls), (original_error or "")[:200],
|
||||
)
|
||||
rescued = extract_with_failover(provider_name, list(urls))
|
||||
rescued = extract_with_failover(provider_name, list(rescue_urls))
|
||||
rescued_errors = [r.get("error", "") for r in rescued]
|
||||
if rescued and all(e for e in rescued_errors):
|
||||
return results # rescue also failed everywhere: keep original errors
|
||||
@@ -546,6 +570,11 @@ def _rescue_extract(provider_name: str, urls: list, results: list) -> list:
|
||||
if isinstance(meta, dict):
|
||||
meta["rescued_from"] = provider_name
|
||||
meta["backend_error"] = (original_error or "")[:300]
|
||||
if len(rescued) == len(rescue_idx) and len(results) == len(urls):
|
||||
merged = list(results)
|
||||
for pos, i in enumerate(rescue_idx):
|
||||
merged[i] = rescued[pos]
|
||||
return merged
|
||||
return rescued
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user