From 418e197e428ff164f96089a89a8bc9d10aad2339 Mon Sep 17 00:00:00 2001 From: ZuyiZhou <144661423+ZuyiZhou@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:32:38 +0800 Subject: [PATCH 1/3] fix(agent): keep one cause one streak class in the media envelopes failure_class keys the loop's tool-failure streak on an envelope's "error" string alone, which holds only while that string comes from the small fixed vocabulary its producers are meant to keep. The media tools interpolated the vendor body, the unreadable reference path and the exception's own text into it, and the URL gate in front of both copies of the reader interpolated the address it refused. One dead endpoint, one bad reference or one blocked range therefore produced a class per call, the streak never reached the break threshold, and the stop-repeating nudge was unreachable for every tool here. "error" now carries only what a reader could enumerate; the variable part moves to "detail", which keeps it in front of the model and inside is_hard_tool_failure's transient-marker scan. A batch result forwards "detail" so a per-picture reason is not lost with it. Distinct causes stay distinct, which is the failure mode this trades against: two HTTP statuses, two exception types and the four vendor refusals each keep their own class. Co-authored-by: Claude (claude-opus-5) --- .../research-flow/research_flow/tools/web.py | 4 +- raven/agent/tools/media_gen.py | 34 +++-- raven/agent/tools/web.py | 4 +- tests/test_agent_loop_tool_loop_break.py | 4 +- tests/test_agent_tools_web_providers.py | 20 +++ tests/test_agents_research_tools.py | 17 +++ tests/test_media_gen_tool.py | 120 +++++++++++++++++- tests/test_security_web_ssrf.py | 4 +- 8 files changed, 185 insertions(+), 22 deletions(-) diff --git a/agents/raven-research/plugins/research-flow/research_flow/tools/web.py b/agents/raven-research/plugins/research-flow/research_flow/tools/web.py index 57e1b56ae..b82ecc4a1 100644 --- a/agents/raven-research/plugins/research-flow/research_flow/tools/web.py +++ b/agents/raven-research/plugins/research-flow/research_flow/tools/web.py @@ -1823,7 +1823,9 @@ async def _fetch( # seconds on a slow resolver, and one process serves every session's turn. is_valid, error_msg = await asyncio.to_thread(_judge_fetch_target, url) if not is_valid: - return json.dumps({"error": f"URL validation failed: {error_msg}", "url": url}, ensure_ascii=False) + # The same rule as the handlers below: most of these reasons name the + # hostname, and a reader whose every target is refused is one cause. + return json.dumps({"error": "URL validation failed", "detail": error_msg, "url": url}, ensure_ascii=False) # "no explicit proxy" is not "direct": with trust_env on, httpx # still honours HTTP(S)_PROXY from the environment. diff --git a/raven/agent/tools/media_gen.py b/raven/agent/tools/media_gen.py index 23ebc0e1c..1f439b62b 100644 --- a/raven/agent/tools/media_gen.py +++ b/raven/agent/tools/media_gen.py @@ -240,15 +240,21 @@ async def _chat(self, payload: dict[str, Any]) -> dict[str, Any]: return message def _format_http_error(self, e: httpx.HTTPStatusError) -> str: + # The status, not the body: ``failure_class`` keys the loop's tool-failure + # streak on this envelope's ``error`` alone, and a vendor body carries a + # request id, so interpolating it splits one dead endpoint into a class per + # call and the stop-repeating nudge never fires. The body moves to ``detail``, + # which keeps it in front of the model and inside ``is_hard_tool_failure``'s + # transient-marker scan. The same rule governs every envelope below. body = e.response.text[:400] - hint = "" + answer: dict[str, Any] = {"error": f"HTTP {e.response.status_code}", "detail": body} if e.response.status_code == 403: - hint = ( - " | Request denied (HTTP 403). You can route media calls through a " + answer["hint"] = ( + "Request denied (HTTP 403). You can route media calls through a " "proxy via tools.media.proxy (or HTTPS_PROXY) and retry." ) logger.error("{} HTTP {}: {}", self.name, e.response.status_code, body) - return json.dumps({"error": f"HTTP {e.response.status_code}: {body}{hint}"}, ensure_ascii=False) + return json.dumps(answer, ensure_ascii=False) _IMAGE_API_MODELS = ( @@ -558,7 +564,7 @@ async def execute( ) blank = next((i for i, spec in enumerate(wanted) if not str(spec.get("prompt") or "").strip()), None) if blank is not None: - return json.dumps({"error": f"the picture at index {blank} has no prompt"}, ensure_ascii=False) + return json.dumps({"error": "a picture has no prompt", "index": blank}, ensure_ascii=False) if len(wanted) == 1: return await self._generate_one(wanted[0], _safe_stem(wanted[0].get("filename")), model, output_dir) stems = _distinct_stems(wanted) @@ -598,7 +604,9 @@ def _batch_answer(self, wanted: list[dict[str, Any]], answers: list[str | BaseEx paths.extend(reply["paths"]) else: item["error"] = reply.get("error") or "no image returned" - for key in ("note", "finish_reason", "retryable"): + # ``detail`` travels with ``error``: the reason a picture failed is + # there now rather than in ``error``, which carries only the class. + for key in ("detail", "note", "finish_reason", "retryable"): if reply.get(key) is not None: item[key] = reply[key] if reply.get("model"): @@ -640,7 +648,7 @@ async def _generate_one(self, spec: dict[str, Any], stem: str, model: str | None for ref in images[:6]: content.append(self._image_part(ref)) except OSError as e: - return json.dumps({"error": f"could not read input image: {e}"}, ensure_ascii=False) + return json.dumps({"error": "could not read input image", "detail": str(e)}, ensure_ascii=False) else: content = prompt @@ -664,7 +672,7 @@ async def _generate_one(self, spec: dict[str, Any], stem: str, model: str | None return self._format_http_error(e) except Exception as e: logger.error("image_generate error: {}", e) - return json.dumps({"error": str(e)}, ensure_ascii=False) + return json.dumps({"error": type(e).__name__, "detail": str(e)}, ensure_ascii=False) out_images = msg.get("images") or [] if not out_images: @@ -720,7 +728,7 @@ async def _via_images_api( # edits form takes the bytes behind it. refs = [self._image_part(ref) for ref in (images or [])[:16]] except OSError as e: - return json.dumps({"error": f"could not read input image: {e}"}, ensure_ascii=False) + return json.dumps({"error": "could not read input image", "detail": str(e)}, ensure_ascii=False) try: async with httpx.AsyncClient(proxy=self._proxy, timeout=300.0) as client: if openrouter: @@ -745,7 +753,7 @@ async def _via_images_api( fetched = await guarded_fetch(client, url, what="image reference") if fetched is None: return json.dumps( - {"error": f"input image {url} refused: not a fetchable public target"}, + {"error": "input image refused: not a fetchable public target", "url": url}, ensure_ascii=False, ) fetched.raise_for_status() @@ -772,7 +780,7 @@ async def _via_images_api( return self._format_http_error(e) except Exception as e: logger.error("image_generate error: {}", e) - return json.dumps({"error": str(e)}, ensure_ascii=False) + return json.dumps({"error": type(e).__name__, "detail": str(e)}, ensure_ascii=False) paths: list[str] = [] for item in items: encoded = item.get("b64_json") if isinstance(item, dict) else None @@ -866,7 +874,7 @@ async def execute( return self._format_http_error(e) except Exception as e: logger.error("text_to_speech error: {}", e) - return json.dumps({"error": str(e)}, ensure_ascii=False) + return json.dumps({"error": type(e).__name__, "detail": str(e)}, ensure_ascii=False) if not pcm: return json.dumps( @@ -1107,7 +1115,7 @@ async def execute( return self._format_http_error(e) except Exception as e: logger.error("video_generate error: {}", e) - return json.dumps({"error": str(e)}, ensure_ascii=False) + return json.dumps({"error": type(e).__name__, "detail": str(e)}, ensure_ascii=False) path = self._output_path("mp4") path.write_bytes(data) diff --git a/raven/agent/tools/web.py b/raven/agent/tools/web.py index b14dd0c92..7deb412bb 100644 --- a/raven/agent/tools/web.py +++ b/raven/agent/tools/web.py @@ -796,7 +796,9 @@ async def execute(self, url: str, extractMode: str = "markdown", maxChars: int | max_chars = maxChars or self.max_chars is_valid, error_msg = validate_url_target(url) if not is_valid: - return json.dumps({"error": f"URL validation failed: {error_msg}", "url": url}, ensure_ascii=False) + # The same rule as the handlers below: most of these reasons name the + # hostname, and a reader whose every target is refused is one cause. + return json.dumps({"error": "URL validation failed", "detail": error_msg, "url": url}, ensure_ascii=False) try: logger.debug("WebFetch[{}]: {}", self.provider, "proxy enabled" if self.proxy else "direct connection") diff --git a/tests/test_agent_loop_tool_loop_break.py b/tests/test_agent_loop_tool_loop_break.py index 4a233e195..bcff0dfcb 100644 --- a/tests/test_agent_loop_tool_loop_break.py +++ b/tests/test_agent_loop_tool_loop_break.py @@ -55,7 +55,7 @@ def workspace(): # by the textual tests: the payload starts with "{" and spells the key # '"error":' rather than "error:". ('{"error": "Jina API key not configured", "url": "https://a.example"}', True), - ('{"error": "URL validation failed: blocked host", "url": "https://a.example"}', True), + ('{"error": "URL validation failed", "detail": "blocked host", "url": "https://a.example"}', True), ('{"url": "https://a.example", "text": "the page", "length": 9}', False), # Transient markers are tested before the envelope, so a retryable failure # stays retryable however it is wrapped. @@ -76,7 +76,7 @@ def test_different_envelope_failures_are_different_classes(): every envelope classified alike, a blocked URL followed by a reader's HTTP refusal would fire the stop-repeating nudge at a model that had changed both its cause and its approach -- the case this whole function exists to keep apart.""" - blocked = '{"error": "URL validation failed: blocked host", "url": "https://a.example"}' + blocked = '{"error": "URL validation failed", "detail": "blocked host", "url": "https://a.example"}' refused = '{"error": "Jina answered HTTP 403", "url": "https://b.example"}' keyless = '{"error": "Jina API key not configured. Set it in the config", "url": "https://c.example"}' diff --git a/tests/test_agent_tools_web_providers.py b/tests/test_agent_tools_web_providers.py index 7ea0596fb..1d55cc5af 100644 --- a/tests/test_agent_tools_web_providers.py +++ b/tests/test_agent_tools_web_providers.py @@ -466,6 +466,26 @@ async def test_two_vendor_refusals_stay_two_streak_classes(monkeypatch: pytest.M assert failure_class(spent) != failure_class(blocked) +async def test_one_blocked_reason_on_two_hosts_is_one_streak_class() -> None: + """The gate in front of the reader, which the two tests above never reach. + + Every reason this gate composes names the address it refused, so the reason + in the classification key gave a model walking an internal range one class + per address: the streak never reached the break threshold and the nudge that + would have sent it somewhere else never fired. The gate runs before any + transport, so nothing here needs a reader. + """ + envelopes = [ + await WebFetchTool(api_key="k", provider="jina").execute(u) + for u in ("http://10.0.0.1/p", "http://192.168.1.1/p") + ] + + assert all(is_hard_tool_failure(raw) for raw in envelopes) + assert len({failure_class(raw) for raw in envelopes}) == 1 + # The address the model has to stop reaching for is still in front of it. + assert "10.0.0.1" in json.loads(envelopes[0])["detail"] + + def test_every_reader_is_covered_by_the_fetch_table() -> None: assert set(_FETCH_CASES) == set(FETCH_PROVIDERS) diff --git a/tests/test_agents_research_tools.py b/tests/test_agents_research_tools.py index d384db60b..4dd5c7c8d 100644 --- a/tests/test_agents_research_tools.py +++ b/tests/test_agents_research_tools.py @@ -473,6 +473,23 @@ async def test_a_private_address_is_still_refused(monkeypatch): assert "URL validation failed" in answer +@pytest.mark.asyncio +async def test_one_blocked_reason_on_two_hosts_is_one_streak_class(monkeypatch): + """This copy of the reader carries the same gate, and drifting from the host's + is what let the envelope fix land on one of them last time. Every reason the + gate composes names the address, so the reason in the classification key gave + a model walking an internal range one class per address and the stop-repeating + nudge never came.""" + _patch_client(monkeypatch, _PageTransport()) + set_current_session("t") + + envelopes = [await WebFetchTool().execute(url=u) for u in ("http://10.0.0.1/p", "http://192.168.1.1/p")] + + assert all(is_hard_tool_failure(raw) for raw in envelopes) + assert len({failure_class(raw) for raw in envelopes}) == 1 + assert "10.0.0.1" in json.loads(envelopes[0])["detail"] + + @pytest.mark.asyncio async def test_a_non_http_scheme_is_still_refused(monkeypatch): _patch_client(monkeypatch, _PageTransport()) diff --git a/tests/test_media_gen_tool.py b/tests/test_media_gen_tool.py index 366236828..e0d61b10b 100644 --- a/tests/test_media_gen_tool.py +++ b/tests/test_media_gen_tool.py @@ -21,6 +21,7 @@ import httpx import pytest +from raven.agent.loop.failure_streak import failure_class, is_hard_tool_failure from raven.agent.tools import media_gen from raven.agent.tools.media_gen import ImageGenerateTool, SpeechGenerateTool, VideoGenerateTool @@ -341,8 +342,10 @@ def handler(request: httpx.Request) -> httpx.Response: async def test_a_picture_that_raises_is_that_picture_s_answer(monkeypatch, tmp_path) -> None: - """A reference outside a confined workspace raises rather than returning; the - batch gathers the exception as one result instead of losing every sibling.""" + """A reference outside a confined workspace raises; that picture wears the + refusal as its own result instead of taking every sibling down with it. The + reason rides in ``detail``, because ``error`` is the streak's classification + key and a path interpolated into it would split one bad reference per file.""" outside = tmp_path / "out.png" outside.write_bytes(_PNG) workspace = tmp_path / "ws" @@ -358,7 +361,8 @@ async def test_a_picture_that_raises_is_that_picture_s_answer(monkeypatch, tmp_p out = json.loads(await tool.execute(prompts=[{"prompt": "cover"}, {"prompt": "logo", "images": [str(outside)]}])) assert out["success"] and out["failed"] == 1 and len(out["paths"]) == 1 - assert "outside the workspace" in out["results"][1]["error"] + assert out["results"][1]["error"] == "could not read input image" + assert "outside the workspace" in out["results"][1]["detail"] async def test_the_pictures_of_a_batch_are_in_flight_at_once(monkeypatch, tmp_path) -> None: @@ -529,7 +533,8 @@ async def test_a_call_with_no_prompt_at_all_says_what_to_send(monkeypatch, tmp_p ) assert "nothing to generate" in json.loads(await tool.execute())["error"] - assert "index 1" in json.loads(await tool.execute(prompts=[{"prompt": "a"}, {"prompt": " "}]))["error"] + blank = json.loads(await tool.execute(prompts=[{"prompt": "a"}, {"prompt": " "}])) + assert blank["error"] == "a picture has no prompt" and blank["index"] == 1 # ── the speech path: what the stream carries, and what the file becomes ── @@ -933,3 +938,110 @@ def test_images_endpoint_accepts_openrouter_chat_style_usage(prompt, completion) assert usage["cost_usd"] == 0.04 assert usage["cache_read_tokens"] is None assert usage["cache_write_tokens"] is None + + +# ── what the loop reads off these envelopes ── + + +def _class_of(answer: str) -> str: + return failure_class(answer) + + +async def test_one_dead_endpoint_is_one_streak_class(monkeypatch, tmp_path) -> None: + """The loop breaks a tool-failure loop at two, keyed on ``(tool, failure_class)``, + and an envelope is classified by its ``error`` string alone. A vendor body carries + a request id, so a body interpolated into that key gave every call its own class + and the stop-repeating nudge was unreachable for this whole tool.""" + bodies = iter( + [ + '{"error":{"message":"upstream unavailable","request_id":"req_aaa111"}}', + '{"error":{"message":"upstream unavailable","request_id":"req_bbb222"}}', + ] + ) + tool = _image_tool( + monkeypatch, + lambda _r: httpx.Response(500, text=next(bodies)), + model="google/gemini-2.5-flash-image", + workspace=tmp_path / "ws", + ) + + first = await tool.execute("a circle") + second = await tool.execute("a square") + + assert _class_of(first) == _class_of(second) + # Not vacuous: both have to COUNT as failures for the streak to reach two at all. + assert is_hard_tool_failure(first) and is_hard_tool_failure(second) + # And the body the model needs is still in front of it, one key over. + assert "req_aaa111" in json.loads(first)["detail"] + + +async def test_two_different_statuses_stay_two_classes(monkeypatch, tmp_path) -> None: + """The control on the test above, and the failure mode it must not cause: a model + that hit a missing model and then a server fault has changed what it is doing, and + folding those together would fire the nudge at a model still adapting.""" + statuses = iter([404, 500]) + tool = _image_tool( + monkeypatch, + lambda _r: httpx.Response(next(statuses), text='{"error":"no"}'), + model="google/gemini-2.5-flash-image", + workspace=tmp_path / "ws", + ) + + assert _class_of(await tool.execute("a circle")) != _class_of(await tool.execute("a square")) + + +async def test_a_reference_it_cannot_read_is_one_class_across_files(monkeypatch, tmp_path) -> None: + """The same rule where the variable part is a path rather than a request id.""" + tool = _image_tool( + monkeypatch, + lambda _r: httpx.Response(200, json=_chat_reply()), + model="google/gemini-2.5-flash-image", + workspace=tmp_path / "ws", + ) + + first = await tool.execute("a circle", images=[str(tmp_path / "gone-a.png")]) + second = await tool.execute("a square", images=[str(tmp_path / "gone-b.png")]) + + assert _class_of(first) == _class_of(second) + assert "gone-a.png" in json.loads(first)["detail"] + + +async def test_two_transports_dying_the_same_way_are_one_class(tmp_path) -> None: + """The catch-all, where the exception's own text spells the host it failed to + reach. Two hosts behind one dead network are one cause; a different exception + type is not, which is the other half the type keeps apart.""" + + async def dies(exc: Exception, prompt: str) -> str: + # Each transport gets its own patch window: patching over a live patch + # would leave every tool here sharing the first one's transport. + with pytest.MonkeyPatch.context() as mp: + + def handler(_r: httpx.Request) -> httpx.Response: + raise exc + + tool = _image_tool(mp, handler, model="google/gemini-2.5-flash-image", workspace=tmp_path / "ws") + return await tool.execute(prompt) + + first = await dies(httpx.ConnectError("[Errno 61] Connection refused to a.example"), "a circle") + second = await dies(httpx.ConnectError("[Errno 61] Connection refused to b.example"), "a square") + other = await dies(httpx.ReadError("the reader gave up"), "a star") + + assert _class_of(first) == _class_of(second) + assert _class_of(first) != _class_of(other) + assert "a.example" in json.loads(first)["detail"] + + +async def test_a_denied_call_still_names_the_proxy_it_needs(monkeypatch, tmp_path) -> None: + """Splitting the envelope must not cost the operator the one thing a 403 is + worth saying: the media calls can be routed through a configured proxy.""" + tool = _image_tool( + monkeypatch, + lambda _r: httpx.Response(403, text="denied"), + model="google/gemini-2.5-flash-image", + workspace=tmp_path / "ws", + ) + + answer = json.loads(await tool.execute("a circle")) + + assert answer["error"] == "HTTP 403" + assert "tools.media.proxy" in answer["hint"] diff --git a/tests/test_security_web_ssrf.py b/tests/test_security_web_ssrf.py index f92df9644..17e5440bb 100644 --- a/tests/test_security_web_ssrf.py +++ b/tests/test_security_web_ssrf.py @@ -33,7 +33,9 @@ def _boom(*_a, **_k): out = await WebFetchTool().execute(url="http://totally-public.example.com/x") parsed = json.loads(out) assert "validation failed" in parsed["error"] - assert "private/internal" in parsed["error"] + # The reason sits in ``detail``: ``error`` is the tool-failure streak's + # classification key, so it carries the class and never the address. + assert "private/internal" in parsed["detail"] async def test_rejects_loopback(monkeypatch): From 88923896916b068bf219512bfb6eced049fe7031 Mon Sep 17 00:00:00 2001 From: ZuyiZhou <144661423+ZuyiZhou@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:48:16 +0800 Subject: [PATCH 2/3] test(agent): cover the envelope rule on the routes the chat path never takes The diff-coverage gate named four lines: a dedicated image model goes to the Images API and never touches chat/completions, so that route carries its own copies of both handlers, and speech and video each carry their own catch-all. The streak key is (tool, failure_class), so each of those tools accumulates its own streak and each one could lose the nudge on its own. Each new test makes the same claim as the ones it joins rather than only walking the line: two failures from one cause are one class, and the host or path that varies is still in front of the model under "detail". Co-authored-by: Claude (claude-opus-5) --- tests/test_media_gen_tool.py | 61 ++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/tests/test_media_gen_tool.py b/tests/test_media_gen_tool.py index e0d61b10b..142e1f5da 100644 --- a/tests/test_media_gen_tool.py +++ b/tests/test_media_gen_tool.py @@ -1045,3 +1045,64 @@ async def test_a_denied_call_still_names_the_proxy_it_needs(monkeypatch, tmp_pat assert answer["error"] == "HTTP 403" assert "tools.media.proxy" in answer["hint"] + + +def _raising_client(mp: pytest.MonkeyPatch, exc: Exception) -> None: + """Every client this module builds dies the same way, whatever the route.""" + + def _boom(*_a, **_kw): + raise exc + + mp.setattr(media_gen.httpx, "AsyncClient", _boom) + + +async def test_the_images_route_classes_its_failures_like_the_chat_route(tmp_path) -> None: + """A dedicated image model never touches chat/completions, so its own copies of + both handlers need the same rule -- one that drifted would put the streak back + where it was for every ``gpt-image`` call.""" + with pytest.MonkeyPatch.context() as mp: + tool = _image_tool( + mp, + lambda _r: httpx.Response(200, json={"data": []}), + model="openai/gpt-image-2", + workspace=tmp_path / "ws", + ) + unreadable = [await tool.execute("a poster", images=[str(tmp_path / f"gone-{n}.png")]) for n in ("a", "b")] + + assert len({failure_class(raw) for raw in unreadable}) == 1 + assert "gone-a.png" in json.loads(unreadable[0])["detail"] + + dead = [] + for host in ("alpha.example", "beta.example"): + with pytest.MonkeyPatch.context() as mp: + tool = _image_tool( + mp, lambda _r: httpx.Response(200), model="openai/gpt-image-2", workspace=tmp_path / "ws" + ) + _raising_client(mp, httpx.ConnectError(f"cannot reach {host}")) + dead.append(await tool.execute("a poster")) + + assert len({failure_class(raw) for raw in dead}) == 1 + assert "alpha.example" in json.loads(dead[0])["detail"] + + +@pytest.mark.parametrize( + "build,call", + [ + (lambda: SpeechGenerateTool(SimpleNamespace(api_base="https://api.test", model="", api_key="k")), "read me"), + (lambda: VideoGenerateTool(SimpleNamespace(api_base="https://api.test", model="", api_key="k")), "a river"), + ], + ids=["speech", "video"], +) +async def test_every_media_tool_reads_one_dead_network_as_one_class(build, call) -> None: + """The streak key is ``(tool, failure_class)``, so each tool accumulates its own. + Speech and video each carry their own catch-all, and an exception's text names the + host it could not reach: left there, neither tool could ever reach the nudge.""" + answers = [] + for host in ("alpha.example", "beta.example"): + with pytest.MonkeyPatch.context() as mp: + _raising_client(mp, httpx.ConnectError(f"[Errno 61] cannot reach {host}")) + answers.append(await build().execute(call)) + + assert all(is_hard_tool_failure(raw) for raw in answers) + assert len({failure_class(raw) for raw in answers}) == 1 + assert "alpha.example" in json.loads(answers[0])["detail"] From b24eb7570a7626b5837d43ef369a82b75c374d10 Mon Sep 17 00:00:00 2001 From: ZuyiZhou <144661423+ZuyiZhou@users.noreply.github.com> Date: Fri, 18 Sep 2026 22:57:47 +0800 Subject: [PATCH 3/3] fix(agent): carry every key the error split into through the batch fold Splitting "error" into a class plus the variable part gave the single call two new keys: "hint", where a 403 names the proxy the media calls can be routed through, and "url", where a guarded fetch names the reference it refused. The batch reducer was taught to forward "detail" and not those two, so the same 403 answered a batched picture with the status and the vendor body alone while the single call still told the operator how to get past it, and a refused reference reached the model with nothing saying which address was refused. Both keys now travel with "error" out of the fold. The batched picture is told what the single call is told, which is the property the split is supposed to preserve. Each test pins one key: reverting the fold to "detail" alone turns both red, restoring "hint" alone leaves the reference test red, and restoring "url" alone leaves the 403 test red. Co-authored-by: Claude (claude-opus-5) --- raven/agent/tools/media_gen.py | 9 +++-- tests/test_media_gen_tool.py | 60 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/raven/agent/tools/media_gen.py b/raven/agent/tools/media_gen.py index 1f439b62b..64e450539 100644 --- a/raven/agent/tools/media_gen.py +++ b/raven/agent/tools/media_gen.py @@ -604,9 +604,12 @@ def _batch_answer(self, wanted: list[dict[str, Any]], answers: list[str | BaseEx paths.extend(reply["paths"]) else: item["error"] = reply.get("error") or "no image returned" - # ``detail`` travels with ``error``: the reason a picture failed is - # there now rather than in ``error``, which carries only the class. - for key in ("detail", "note", "finish_reason", "retryable"): + # Every key ``error`` was split into travels with it: the reason a + # picture failed, the proxy a 403 can be routed through, the + # reference that was refused. ``error`` carries only the class now, + # so a key left behind here is a batched picture told strictly less + # than the single call is. + for key in ("detail", "hint", "url", "note", "finish_reason", "retryable"): if reply.get(key) is not None: item[key] = reply[key] if reply.get("model"): diff --git a/tests/test_media_gen_tool.py b/tests/test_media_gen_tool.py index 142e1f5da..02ecb89f4 100644 --- a/tests/test_media_gen_tool.py +++ b/tests/test_media_gen_tool.py @@ -262,6 +262,12 @@ def _chat_reply(*, count: int = 1) -> dict: return {"choices": [{"message": {"images": [{"image_url": {"url": "data:image/png;base64," + _B64}}] * count}}]} +async def _refuse_every_fetch(_client, _url, *, what="") -> None: + """Stand in for ``guarded_fetch`` judging a target private: it answers None, + which is the branch that names the refused address.""" + return None + + def _prompt_of(request: httpx.Request) -> str: content = json.loads(request.content)["messages"][0]["content"] return content if isinstance(content, str) else content[0]["text"] @@ -1106,3 +1112,57 @@ async def test_every_media_tool_reads_one_dead_network_as_one_class(build, call) assert all(is_hard_tool_failure(raw) for raw in answers) assert len({failure_class(raw) for raw in answers}) == 1 assert "alpha.example" in json.loads(answers[0])["detail"] + + +async def test_a_batched_403_names_the_proxy_the_single_call_names(monkeypatch, tmp_path) -> None: + """``error`` carrying only the class is what the streak needs; it is not a + reason to tell a batched picture less than a single one. The way out of a 403 + lives in ``hint`` now, so the reducer that folds eight answers into one has to + carry it, or asking for eight backdrops hides the proxy setting that asking for + one would have named.""" + + def handler(request: httpx.Request) -> httpx.Response: + if _prompt_of(request) == "chapter one": + return httpx.Response(403, text="denied") + return httpx.Response(200, json=_chat_reply()) + + tool = _image_tool(monkeypatch, handler, model="google/gemini-2.5-flash-image", workspace=tmp_path / "ws") + + out = json.loads(await tool.execute(prompts=[{"prompt": "cover"}, {"prompt": "chapter one"}])) + + failed = out["results"][1] + assert out["failed"] == 1 and failed["error"] == "HTTP 403" + assert "tools.media.proxy" in failed["hint"] + assert failed["detail"] == "denied" + + +async def test_a_batched_picture_is_told_which_reference_was_refused(monkeypatch, tmp_path) -> None: + """The other key ``error`` was split into: the address a guarded fetch refused + moved to ``url``, and a picture is one of several that could have carried a bad + reference, so dropping it in the fold leaves the model with a refusal it cannot + act on.""" + monkeypatch.setattr(media_gen, "guarded_fetch", _refuse_every_fetch) + + def handler(_r: httpx.Request) -> httpx.Response: + return httpx.Response(200, json={"data": [{"b64_json": _B64, "media_type": "image/png"}]}) + + tool = _image_tool( + monkeypatch, + handler, + model="openai/gpt-image-2", + workspace=tmp_path / "ws", + api_base="https://api.mycorp.example/v1", + ) + + out = json.loads( + await tool.execute( + prompts=[ + {"prompt": "cover"}, + {"prompt": "chapter one", "images": ["https://10.0.0.1/logo.png"]}, + ] + ) + ) + + failed = out["results"][1] + assert out["failed"] == 1 and failed["error"] == "input image refused: not a fetchable public target" + assert failed["url"] == "https://10.0.0.1/logo.png"