diff --git a/CHANGELOG.md b/CHANGELOG.md index f5810d5308..8e8633515b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,60 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Fix a live crash: `noema-review` failed with an unhandled `HTTPError` instead + of failing closed.** Live incident on `ContextualWisdomLab/naruon#1486`: + `scripts/ci/noema_review_gate.py::call_llm`'s `opener.open(request)` call sat + outside the surrounding `try`/`except`, which only guarded the JSON-decode and + validation steps after a successful response. A genuine `HTTP Error 502: Bad + Gateway` from the completion request therefore crashed the whole required + check with an unhandled traceback instead of getting the same one-time + repair-retry the malformed-verdict path already has. Widened the `try` to + also cover the request itself and added `urllib.error.URLError` alongside + `RuntimeError` to the existing repair-retry `except` clause — a transient + transport failure now gets one retry, then fails closed with a clean + `RuntimeError` on a second failure, exactly like a malformed verdict already + does. Verified genuine RED (the exact `HTTPError: Bad Gateway` reproduced + uncaught) before the fix, GREEN after; full suite 2248 passed, 1 skipped, 21 + subtests. (Repo-wide coverage independently confirmed at 99% both before and + after this change — a pre-existing gap in + `pr_review_fix_scheduler.py`/`pr_review_merge_scheduler.py` unrelated to this + diff.) Devin Review then found the transport-error boundary still missed a + mid-response failure: `response.read()` can raise `http.client + .IncompleteRead` (or another `http.client.HTTPException`/raw `OSError`) when + the server closes the connection before delivering the full + `Content-Length` body, and none of those are `RuntimeError` or + `urllib.error.URLError`. Widened the `except` clause to + `(RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError)` + and simplified the repair-retry re-raise to "re-raise as-is only when it's + already our own `RuntimeError`; otherwise wrap in a clean `RuntimeError`" so + the fail-closed behavior generalizes to any transport exception type rather + than needing another isinstance check added per exception class. Verified + genuine RED (`IncompleteRead` reproduced uncaught) before this second fix, + GREEN after. A third distinct exception path (a raw `TimeoutError` reaching + `opener.open()` directly, never wrapped as `URLError`) was added per the + repo owner's explicit request on `#1566` for at least one timeout/disconnect + family exercising a genuinely different branch than the HTTPError/URLError + and IncompleteRead cases above — also RED→GREEN verified. Full suite 2252 + passed, 1 skipped, 21 subtests; `noema_review_gate.py` itself at 100% + line/branch coverage. (A separate, pre-existing SIGPIPE flake in + `tests/test_opencode_required_verdict_regression.py`, unrelated to this + file, was also reproduced and fixed in its own PR during this verification.) + Devin Review then found a fourth, distinct bug in the fix itself: gating the + retry-vs-fail-closed decision on `repair_error`'s truthiness conflated "is + this the second attempt" with "does the caught exception have display + text" — several transport exceptions (a bare `OSError()`/`TimeoutError()`, + or an `http.client.HTTPException` raised with no message) stringify to an + empty string, so an empty-message failure on the first attempt would keep + `repair_error` falsy on the recursive call too and retry unboundedly instead + of failing closed after one attempt. Added an explicit `is_retry: bool` + parameter to track retry state independently of the exception's text, used + it (not `repair_error`) as the sole gate in both the prompt-injection branch + and the except clause, and threaded it through the recursive call. Verified + genuine RED with a bounded-recursion regression test (an `AssertionError` + fires if `call_llm` retries more than once, rather than letting it recurse + to CPython's own limit) before this fourth fix, GREEN after. Full suite 2254 + passed, 1 skipped, 21 subtests; `noema_review_gate.py` still at 100% + line/branch coverage, 100% docstrings. - Avoid redundant merge-scheduler wakes when the trusted receipt predicate already finds a substantive exact-head OpenCode verdict. Missing, stale, or fallback-only evidence still dispatches review work, while receipt lookup or diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 76d85b949b..9367d54f67 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2344,6 +2344,84 @@ contract assertion, and `docs/adr/0003-contextual-orchestrator-vendored-free-zdr "today" reference. Landed in the same PR (`#1463`) as the streaming revert, not split out, since the revert is unsafe without it. +## 2026-09-01 naruon#1486 transport-crash: root cause, owner, status + +**Live incident**: the required `noema-review` check on `ContextualWisdomLab/naruon#1486` crashed with an +unhandled `urllib.error.HTTPError: HTTP Error 502: Bad Gateway`. Root cause: `call_llm` in +`scripts/ci/noema_review_gate.py` had `with opener.open(request) as response:` sitting outside the +`try`/`except` that only guarded the JSON-decode/validation steps *after* a successful response -- +identical in shape to, but a distinct bug from, the malformed-verdict crash fixed in `#1507` +(2026-08-31 entries above). Confirmed via direct fetch that `#1546`'s own `call_llm` (main tip at the +time, `5686de41`) carried the same unguarded line, so this crash is orthogonal to, and survives +regardless of, the `#1438`/`#1546` wall-clock-deadline policy question -- `#1438` was closed by the +repo owner as a stale mixed branch unrelated to this specific bug. + +**Fix, round 1**: widened the `try` to cover the request itself and added `urllib.error.URLError` +alongside `RuntimeError` to the existing repair-retry `except` clause -- one retry on a transient +transport failure, then a clean `RuntimeError` on a second failure, matching the malformed-verdict +path's contract. RED (`HTTPError: Bad Gateway` reproduced uncaught) confirmed before, GREEN after. + +**Fix, round 2 (Devin Review, then owner confirmation, on `#1566` itself)**: Devin correctly found that +`response.read()` can raise `http.client.IncompleteRead` -- and, more generally, any +`http.client.HTTPException` or raw `OSError` (a bare socket timeout/disconnect reaching `opener.open()` +before urllib gets a chance to wrap it as `URLError`) -- none of which are `RuntimeError` or +`urllib.error.URLError`, so they still escaped the round-1 boundary. The owner's review comment and +follow-up issue comment on `#1566` confirmed this independently and specified the exact contract: widen +to the bounded transport/read exception families without swallowing JSON/validator/programming errors, +add RED->GREEN regressions for a truncated-body success-after-retry and a repeated-failure case, and at +least one timeout/disconnect family exercising a distinct exception path -- while preserving `#1546`'s +unbounded inference semantics (no fixed inference timeout, no direct-provider fallback, no bypass). + +Widened the `except` clause to `(RuntimeError, urllib.error.URLError, http.client.HTTPException, +OSError)` and simplified the repair-retry re-raise from an `isinstance(exc, urllib.error.URLError)` +check to `isinstance(exc, RuntimeError)`: re-raise as-is only when the second failure is already this +module's own `RuntimeError` (a malformed verdict, an invalid finding, etc.); otherwise wrap in a clean +`RuntimeError`. This generalizes the fail-closed contract to any transport exception type without +needing another `isinstance` branch added per exception class encountered. Three genuinely distinct +exception paths are now each covered by their own RED->GREEN success-after-retry and repeated-failure +regression pair (`test_call_llm_repairs_once_after_a_transport_error_then_succeeds` / +`test_call_llm_fails_closed_after_a_repeated_transport_error` for `HTTPError`/`URLError`; +`test_call_llm_repairs_once_after_a_truncated_response_then_succeeds` / +`test_call_llm_fails_closed_after_a_repeated_truncated_response` for `http.client.IncompleteRead`; +`test_call_llm_repairs_once_after_a_socket_timeout_then_succeeds` / +`test_call_llm_fails_closed_after_a_repeated_socket_timeout` for a raw `TimeoutError` reaching +`opener.open()` directly) -- each verified genuinely RED against the pre-fix boundary before being +folded in, never transferred from an earlier case as substitute proof. Full suite: 2252 passed, 1 +skipped, 21 subtests; `noema_review_gate.py` at 100% line/branch coverage; 100% docstring coverage. + +**Fix, round 3 (Devin Review again, same `#1566`)**: a fourth, distinct bug in the fix itself -- +gating the retry-vs-fail-closed decision on `repair_error`'s truthiness conflated "is this the +second attempt" with "does the caught exception have display text". Several transport exceptions +(a bare `OSError()`/`TimeoutError()`, or an `http.client.HTTPException` raised with no message) all +stringify to `''`, so an empty-message failure on the *first* attempt would leave `repair_error` +falsy on the recursive call too -- the retry-state signal was lost, and `call_llm` would retry +unboundedly (each recursive call itself another live-gateway request) rather than failing closed +after one attempt, eventually crashing on an uncaught `RecursionError` once the interpreter's call +stack was exhausted. Added an explicit `is_retry: bool = False` parameter to track retry state +independently of the exception's text; it (not `repair_error`) now gates both the prompt-injection +branch (falling back to a generic message when `repair_error` is empty) and the except clause's +retry-vs-fail-closed decision, and is threaded through as `is_retry=True` on the recursive call. +Verified genuine RED with a bounded-recursion regression test +(`test_call_llm_fails_closed_after_a_repeated_empty_message_transport_error`, which raises a +diagnostic `AssertionError` if `call_llm` retries more than once instead of letting it recurse to +CPython's own limit) before this fourth fix, GREEN after -- paired with +`test_call_llm_repairs_once_after_an_empty_message_transport_error_then_succeeds` for the +happy-path case. Full suite: 2254 passed, 1 skipped, 21 subtests; `noema_review_gate.py` still at +100% line/branch coverage, 100% docstring coverage. + +**Owner**: this repo (`ContextualWisdomLab/.github`), `scripts/ci/noema_review_gate.py`. +**Status**: fixed on `ContextualWisdomLab/.github#1566` (branch `fix/noema-review-transport-error-retry`), +pending required checks and final review. + +While verifying this fix's full-suite run, an unrelated, pre-existing SIGPIPE (exit 141) flake was also +found and root-caused in `tests/test_opencode_required_verdict_regression.py::test_scheduler_wake_reuses_trusted_receipt_predicate`: +its fake `gh` fixture never drains the JSON piped into it via `--input -` for the dispatch call, so under +`set -euo pipefail` the pipeline's writer (`jq`) can be killed by `SIGPIPE` if the fake reader exits +first -- reproduced locally at roughly a 60% failure rate over 15 runs in complete isolation (not merely +under CI load), and eliminated (30/30 clean runs) by draining stdin (`cat >/dev/null`) before the fixture +writes its own output. Fixed separately, since it is unrelated to the transport-crash file above; see +that PR for its own evidence. + ## 5. 실행 루프와 고객의 다음 행동 각 hourly pass는 아래 순서를 유지한다. diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 249f94f6b7..b77ed11c03 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -7,6 +7,7 @@ import ast import base64 import hashlib +import http.client import ipaddress import json import os @@ -922,6 +923,7 @@ def call_llm( review_context: str = "", changed_paths: Sequence[str] = (), repair_error: str = "", + is_retry: bool = False, ) -> dict[str, Any]: """Call the configured OpenAI-compatible LLM endpoint for a review verdict. @@ -935,6 +937,13 @@ def call_llm( discard anyway once this function returns. See ``fetch_pr`` for the live lookup and ``StaleHeadDuringRepairRetryError`` for how that stale condition is reported distinctly to the caller. + + ``is_retry`` tracks retry state independently of ``repair_error``'s text: + several transport exceptions (a bare ``OSError``/``TimeoutError`` or + ``http.client.HTTPException`` raised with no message) stringify to an + empty string, so gating on ``repair_error``'s truthiness alone would let + an empty-message failure retry unboundedly instead of failing closed + after one attempt. """ api_url = os.environ.get("NOEMA_LLM_API_URL", "").strip() api_key = os.environ.get("NOEMA_LLM_API_KEY", "").strip() @@ -994,10 +1003,11 @@ def call_llm( "Use request_changes only for blocking, concrete issues. A generic no-issues statement is not review evidence.", *( [ - f"Your prior verdict was rejected by the trusted validator: {repair_error}", + "Your prior verdict was rejected by the trusted validator: " + f"{repair_error or 'no diagnostic message was available'}", "Return one corrected JSON verdict using only exact changed-side locations from the supplied diff.", ] - if repair_error + if is_retry else [] ), f"Repository: {repo}", @@ -1030,9 +1040,9 @@ def call_llm( method="POST", ) opener = urllib.request.build_opener(NoRedirectHandler()) - with opener.open(request) as response: # nosec B310 - raw_bytes = response.read() try: + with opener.open(request) as response: # nosec B310 + raw_bytes = response.read() raw = decode_llm_response_body(raw_bytes) content = extract_llm_message_content(raw) verdict = extract_json_object(content) @@ -1060,9 +1070,11 @@ def call_llm( if decision == "request_changes" and not findings: raise RuntimeError("Noema LLM request_changes response did not contain a substantive finding") validate_substantive_verdict(verdict, diff, changed_paths) - except RuntimeError as exc: - if repair_error: - raise + except (RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError) as exc: + if is_retry: + if isinstance(exc, RuntimeError): + raise + raise RuntimeError(str(exc)) from exc if str(fetch_pr(repo, number).get("headRefOid") or "").lower() != expected_head: raise StaleHeadDuringRepairRetryError( "Pull request head changed during review; stale before repair retry." @@ -1077,6 +1089,7 @@ def call_llm( review_context, changed_paths, str(exc), + is_retry=True, ) return verdict diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 6272ff2b59..43aaf46e81 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -1,5 +1,6 @@ import base64 import hashlib +import http.client import json import os import shlex @@ -1338,6 +1339,321 @@ def open_response(_opener, request, **_kwargs): assert "prior verdict was rejected" in json.loads(open_calls[1].data)["messages"][1]["content"] +def test_call_llm_repairs_once_after_a_transport_error_then_succeeds(monkeypatch): + """A transport-level failure (e.g. a genuine HTTP 502 from the gateway) + must not crash the job with an unhandled traceback. + + Live incident (ContextualWisdomLab/naruon#1486): ``opener.open(request)`` + sat outside the surrounding try/except, which only guarded the + JSON-decode/validation step after a successful response. Any transport + exception (HTTPError, URLError) from the request itself propagated as an + unhandled traceback instead of getting the same one-time repair-retry the + malformed-verdict path already has. Widening the try to also cover the + request itself, and catching ``urllib.error.URLError`` alongside + ``RuntimeError``, integrates it with that existing boundary.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + attempts = [] + + class Response: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + return json.dumps( + { + "choices": [ + { + "message": { + "content": json.dumps( + {"decision": "comment", "summary": "Recovered", "findings": []} + ) + } + } + ] + } + ).encode() + + def open_response(_opener, request, **_kwargs): + attempts.append(request) + if len(attempts) == 1: + raise noema.urllib.error.HTTPError(request.full_url, 502, "Bad Gateway", {}, None) + return Response() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + verdict = noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + + assert verdict["summary"] == "Recovered" + assert len(attempts) == 2 + + +def test_call_llm_fails_closed_after_a_repeated_transport_error(monkeypatch): + """Two consecutive transport errors must produce a single clean + RuntimeError diagnostic, never an unhandled traceback -- the first still + gets a repair-retry request like any other recoverable failure would.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + open_calls = [] + + def open_response(_opener, request, **_kwargs): + open_calls.append(request) + raise noema.urllib.error.HTTPError(request.full_url, 502, "Bad Gateway", {}, None) + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + with pytest.raises(RuntimeError, match="Bad Gateway"): + noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + assert len(open_calls) == 2 + + +def test_call_llm_repairs_once_after_a_truncated_response_then_succeeds(monkeypatch): + """A truncated response body must not crash the job either. + + ``response.read()`` can raise ``http.client.IncompleteRead`` when the + server closes the connection before delivering the full + ``Content-Length`` body. That exception is neither a ``RuntimeError`` + nor a ``urllib.error.URLError`` -- it is a plain ``http.client + .HTTPException`` -- so it slipped through the transport-error boundary + added for the HTTPError/URLError case and still crashed the required + check with an unhandled traceback.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + attempts = [] + + class TruncatedResponse: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + raise http.client.IncompleteRead(b"", 10) + + class Response: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + return json.dumps( + { + "choices": [ + { + "message": { + "content": json.dumps( + {"decision": "comment", "summary": "Recovered", "findings": []} + ) + } + } + ] + } + ).encode() + + def open_response(_opener, request, **_kwargs): + attempts.append(request) + if len(attempts) == 1: + return TruncatedResponse() + return Response() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + verdict = noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + + assert verdict["summary"] == "Recovered" + assert len(attempts) == 2 + + +def test_call_llm_fails_closed_after_a_repeated_truncated_response(monkeypatch): + """Two consecutive truncated reads must produce a single clean + RuntimeError diagnostic, never an unhandled traceback -- the first still + gets a repair-retry request like any other recoverable failure would.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + open_calls = [] + + class TruncatedResponse: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + raise http.client.IncompleteRead(b"", 10) + + def open_response(_opener, request, **_kwargs): + open_calls.append(request) + return TruncatedResponse() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + with pytest.raises(RuntimeError, match="IncompleteRead"): + noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + assert len(open_calls) == 2 + + +def test_call_llm_repairs_once_after_a_socket_timeout_then_succeeds(monkeypatch): + """A raw socket-level failure during the request/connect phase -- not + wrapped as a ``urllib.error.URLError`` -- must not crash the job either. + + ``opener.open(request)`` can raise a bare ``OSError`` subtype (e.g. a + ``TimeoutError``/``socket.timeout``, or a connection reset) directly from + the underlying ``http.client`` connection when the failure happens before + urllib gets a chance to wrap it as ``URLError``. This exercises the + ``OSError`` branch of the transport-failure boundary on a distinct + exception path from the ``http.client.HTTPException`` branch + (``IncompleteRead``, above) and the ``urllib.error.URLError`` branch + (``HTTPError``, further above).""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + attempts = [] + + class Response: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + return json.dumps( + { + "choices": [ + { + "message": { + "content": json.dumps( + {"decision": "comment", "summary": "Recovered", "findings": []} + ) + } + } + ] + } + ).encode() + + def open_response(_opener, request, **_kwargs): + attempts.append(request) + if len(attempts) == 1: + raise TimeoutError("timed out waiting for the gateway") + return Response() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + verdict = noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + + assert verdict["summary"] == "Recovered" + assert len(attempts) == 2 + + +def test_call_llm_fails_closed_after_a_repeated_socket_timeout(monkeypatch): + """Two consecutive raw socket timeouts must produce a single clean + RuntimeError diagnostic, never an unhandled traceback -- the first still + gets a repair-retry request like any other recoverable failure would.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + open_calls = [] + + def open_response(_opener, request, **_kwargs): + open_calls.append(request) + raise TimeoutError("timed out waiting for the gateway") + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + with pytest.raises(RuntimeError, match="timed out waiting for the gateway"): + noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + assert len(open_calls) == 2 + + +def test_call_llm_repairs_once_after_an_empty_message_transport_error_then_succeeds(monkeypatch): + """A transport exception whose ``str()`` is empty (a bare ``OSError()``/ + ``TimeoutError()``, or an ``http.client.HTTPException`` raised with no + message -- all of these stringify to ``''`` in practice) must still get + exactly one repair retry, the same as any other transport failure. + + Devin Review on #1566: gating the retry-vs-fail-closed decision on + ``repair_error``'s truthiness conflated "is this the second attempt" + with "does the caught exception have display text" -- an empty-message + failure on the first attempt would keep ``repair_error`` falsy on the + recursive call too, so the retry state was lost. ``is_retry`` now tracks + that state explicitly and independently of the exception's text.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + attempts = [] + + class Response: + def __enter__(self): + return self + + def __exit__(self, *args): + return None + + def read(self): + return json.dumps( + { + "choices": [ + { + "message": { + "content": json.dumps( + {"decision": "comment", "summary": "Recovered", "findings": []} + ) + } + } + ] + } + ).encode() + + def open_response(_opener, request, **_kwargs): + attempts.append(request) + if len(attempts) == 1: + raise OSError() + return Response() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + verdict = noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + + assert verdict["summary"] == "Recovered" + assert len(attempts) == 2 + + +def test_call_llm_fails_closed_after_a_repeated_empty_message_transport_error(monkeypatch): + """Two consecutive empty-message transport failures must still fail + closed after exactly one repair retry, never retry unboundedly. + + Bounds the fixture at 6 open() calls so a regression that reintroduces + unbounded recursion fails this test fast with a clear AssertionError + instead of recursing until CPython's own recursion limit.""" + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example/v1/chat/completions") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "test-key") + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + open_calls = [] + + def open_response(_opener, request, **_kwargs): + open_calls.append(request) + if len(open_calls) > 5: + raise AssertionError("call_llm retried more than once on an empty-message transport error") + raise OSError() + + monkeypatch.setattr(noema.urllib.request.OpenerDirector, "open", open_response) + + with pytest.raises(RuntimeError): + noema.call_llm("owner/repo", 7, make_pr(), "diff", False, "head") + assert len(open_calls) == 2 + + @pytest.mark.parametrize("choices", [{"a": 1}, 5]) def test_call_llm_fails_closed_on_wrong_shaped_gateway_choices(monkeypatch, choices): """A malformed (non-list) choices field surfaces through call_llm's