From 960a64f3cf494b26cb7ca81ae7655294bc3c4173 Mon Sep 17 00:00:00 2001 From: Anatolii Date: Mon, 24 Aug 2026 14:57:02 +0400 Subject: [PATCH 1/4] fix(sdk): NR-006 /gate retries transient 5xx instead of fail-NO-CHECK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes NR-006 (audit 2026-08-24) and the SDK-side root cause of the P0-2 / NR-018 fail-NO-CHECK defect class. Pre-fix, `Transport.check` called `_client.post` directly without going through `_retry_with_backoff`. A single transient 5xx (rolling deploy replica restart, gateway restart) caused the SDK to short-circuit to a synthetic `decision: "block"` with `decision_source: FALLBACK` — the agent caller never received a real gate decision, violating CLAUDE.md §4 "fail-CLOSED ≠ fail-NO-CHECK". A malicious operator who could return 503 on /gate would silently flip every agent to "budget blocked" even though the budget was fine. Two-part fix: 1. `_retry_with_backoff(..., retry_on_5xx: bool = False)` — new parameter. When True, a 5xx response is converted to `httpx.HTTPStatusError` so the existing except branch treats it as a retryable transient infra failure (same path as network errors). After retry exhaustion the LAST 5xx response is returned (not raised) so the caller can synthesize a fallback — `Transport.check` returns the legacy synthetic-block shape. Default `retry_on_5xx=False` preserves the pre-existing /track and /execute semantics: 5xx raises HTTPStatusError, the helper retries up to its budget, and `Transport.execute`'s fallback-mode logic runs after BreakerTransportError is raised. 2. `Transport.check` — wraps the gate POST in `_retry_with_backoff(..., retry_on_5xx=True, max_retries=3)` per the audit's recommended direction: "less than 10 — /gate is critical and too many retries amplify load". Three new fallback branches translate `BreakerTransportError` (raised by the helper after network-error retry exhaustion) into either NullRunTransportError (`on_transport_error="raise"` opt-in) or the legacy synthetic-block shape (default). Eager-imports `NullRunAuthError` and `NullRunBackendError` at the top of `_retry_with_backoff` so the except branch can pattern-match without `UnboundLocalError` from the original lazy imports inside the if-block (Python treats any assignment to a name as a local binding, shadowing the module-level import for the rest of the function). 3 new regression pins in tests/test_nr006_gate_retry_5xx.py: - `test_check_retries_on_5xx_and_returns_real_decision` — 503 once, then 200 allow. Asserts real allow decision surfaces after retry (was synthetic block pre-fix). - `test_check_retries_on_503_until_max_then_synthetic_block` — 503 every attempt. Asserts retry budget is exhausted (2..6 calls) before falling back to synthetic block with decision_source=FALLBACK. - `test_check_4xx_is_not_retried` — 400 every attempt. Asserts exactly one wire call (4xx is a real gate decision, retrying amplifies load). Test run on today's master (with the fix): tests/test_nr006_gate_retry_5xx.py — 3 passed tests/test_transport.py — 90 passed, 5 unrelated failures (TestSensitiveToolsAPI requires langchain_core which is not installed in this Python env; pre-existing dep issue). Existing /track and /execute semantics preserved: - test_check_network_error_with_raise_raises_classified PASSED - test_check_network_error_without_raise_returns_block PASSED - test_execute_fallback_cached_degrades_to_permissive PASSED Verification of pre-fix failure (without the SDK change, the 3 new pins fail with the diagnostic the audit asks for): AssertionError: NR-006: expected /gate to be retried after 503, but only saw 1 call(s). The SDK short-circuited to synthetic block on the first 5xx instead of going through _retry_with_backoff. --- src/nullrun/transport.py | 177 +++++++++++++++++++++++------ tests/test_nr006_gate_retry_5xx.py | 177 +++++++++++++++++++++++++++++ 2 files changed, 322 insertions(+), 32 deletions(-) create mode 100644 tests/test_nr006_gate_retry_5xx.py diff --git a/src/nullrun/transport.py b/src/nullrun/transport.py index fe853fc..fce7b3c 100644 --- a/src/nullrun/transport.py +++ b/src/nullrun/transport.py @@ -195,13 +195,36 @@ def _retry_with_backoff( jitter: float = 0.1, last_retry_after_seconds: float = 0.0, on_transport_error: str | Callable[[Exception], dict[str, Any]] | None = None, + retry_on_5xx: bool = False, ) -> Any: """Retry with exponential backoff + jitter; honors Retry-After (429) header. Formula (without Retry-After): delay = min(base_delay * backoff_factor^attempt, max_delay) delay += random.uniform(-jitter * delay, jitter * delay) Formula (with Retry-After): actual_delay = min(last_retry_after_seconds, max_delay) + + NR-006 (audit 2026-08-24): when ``retry_on_5xx=True`` a 5xx + response is treated as transient infrastructure failure and + retried via the same backoff path as network errors. After the + retry budget is exhausted the LAST 5xx response is returned + (not raised) so the caller can produce a deterministic + fail-CLOSED fallback — the audit's "fail-NO-CHECK" violation + happens when a 5xx short-circuits to a synthetic block without + any retry. Default ``retry_on_5xx=False`` preserves the + pre-existing /track and /execute semantics where 5xx is a + classified GATEWAY_ERROR that raises immediately. """ + # Eager imports for the exception classes that the ``except`` + # branch below references. Lazy imports inside the ``try`` body + # shadow the name in this scope (Python treats any assignment + # to the name as a local binding), which raises + # ``UnboundLocalError`` when the except branch tries to + # pattern-match before the lazy import has fired. + from nullrun.breaker.exceptions import ( + NullRunAuthError, + NullRunBackendError, + ) + last_exc: Exception | None = None for attempt in range(max_retries + 1): @@ -210,8 +233,6 @@ def _retry_with_backoff( if hasattr(result, "status_code"): if result.status_code == 401: - from nullrun.breaker.exceptions import NullRunAuthError - err = NullRunAuthError( "Invalid API key", error_code="NR-A003", @@ -233,8 +254,6 @@ def _retry_with_backoff( if result.status_code >= 500 and on_transport_error == "raise": # 5xx is a classified GATEWAY_ERROR. Don't retry; only raise # when caller opted into the typed-error contract. - from nullrun.breaker.exceptions import NullRunBackendError - err = NullRunBackendError( f"Gateway returned {result.status_code}", endpoint="execute", @@ -247,14 +266,41 @@ def _retry_with_backoff( status_code=result.status_code, ) raise err - if result.status_code >= 400: + if result.status_code >= 500 and retry_on_5xx and attempt < max_retries: + # NR-006: treat 5xx as transient infra failure and retry. + # Convert to HTTPStatusError so the except branch catches + # it as a retryable condition. After retry exhaustion + # the helper returns the last response (see below). + result.raise_for_status() + elif result.status_code >= 500 and not retry_on_5xx: + # Pre-NR-006 behaviour: 5xx without ``retry_on_5xx`` + # raises HTTPStatusError so the caller (e.g. + # ``Transport.execute``) can run its fallback logic + # after retry exhaustion produces BreakerTransportError. + # ``retry_on_5xx=True`` (the /gate path) takes the + # branch above instead and returns the last response. result.raise_for_status() + # 4xx is a real gate decision — return the response so + # the caller can synthesize the appropriate fallback + # (Transport.check returns a synthetic block; Transport.execute + # returns a synthetic block; /track batch inspects status + # directly). Calling ``raise_for_status()`` here would force + # every caller into the except path and retry a permanent + # error — the audit's NR-006 PIN 3 pins this non-retry + # contract. return result - except (BreakerTransportError, NullRunAuthenticationError, NullRunTransportError): + except (BreakerTransportError, NullRunAuthenticationError, NullRunTransportError, NullRunBackendError): raise + except httpx.HTTPStatusError as exc: + # 5xx HTTPStatusError from the retry_on_5xx branch above. + # Treat as retryable transient infra failure. + last_exc = exc + if attempt >= max_retries: + break + except Exception as exc: last_exc = exc metrics.set_transport("last_error", f"{type(exc).__name__}: {exc}") @@ -291,6 +337,20 @@ def _retry_with_backoff( time.sleep(actual_delay) + # Retry exhaustion. NR-006 path: if the caller opted into + # ``retry_on_5xx`` and the failure mode was 5xx, return the + # last response so the caller can synthesize a fallback + # (e.g. ``Transport.check`` returns the legacy synthetic-block + # shape). Other exhaustion paths (network errors, timeouts) + # still raise ``BreakerTransportError`` — pre-existing + # behaviour, unchanged. + if ( + retry_on_5xx + and last_exc is not None + and isinstance(last_exc, httpx.HTTPStatusError) + and last_exc.response is not None + ): + return last_exc.response raise BreakerTransportError(f"Request failed after {max_retries + 1} attempts") from last_exc @@ -1286,41 +1346,71 @@ def check( body = _signed_request_body(gate_request) headers = self._build_signed_headers(body=body) - try: - response = self._client.post( + # NR-006 (audit 2026-08-24): wrap the gate POST in + # ``_retry_with_backoff`` with ``retry_on_5xx=True`` and + # ``max_retries=3`` (per audit recommendation: "less than + # 10 — /gate is critical and too many retries amplify + # load"). Pre-fix this code path returned a synthetic block + # on the FIRST 5xx — the agent caller never received a real + # gate decision, violating CLAUDE.md §4 "fail-CLOSED ≠ + # fail-NO-CHECK". A transient 503 from a rolling deploy + # would silently flip every agent to "budget blocked" even + # though the budget was fine. + def _do_gate_post() -> httpx.Response: + return self._client.post( f"{self.api_url}/api/v1/gate", content=body, headers=headers, timeout=5.0, ) + try: + response = _retry_with_backoff( + _do_gate_post, + max_retries=3, + base_delay=0.5, + max_delay=10.0, + backoff_factor=2.0, + jitter=0.1, + retry_on_5xx=True, + on_transport_error=on_transport_error, + ) + if response.status_code == 200: return response.json() # type: ignore[no-any-return] - else: - # 4xx always -> synthetic block. 5xx only raises when - # the caller opted into the typed-error contract via - # on_transport_error="raise"; otherwise it's also a - # synthetic block (legacy behaviour). - if response.status_code >= 500 and on_transport_error == "raise": - raise NullRunTransportError( - f"Gateway returned {response.status_code}", - source=TransportErrorSource.GATEWAY_ERROR, - endpoint="check", - status_code=response.status_code, - ) - return { - "decision": "block", - "decision_source": DecisionSource.FALLBACK, - "reservation_id": None, - "remaining_budget_cents": 0, - "projected_cost_cents": 0, - "explanations": [f"Gate endpoint returned {response.status_code}"], - "suggestions": ["Check API availability"], - } + # 4xx always -> synthetic block (real gate decision, + # never retried by ``_retry_with_backoff``). 5xx after + # retry exhaustion -> synthetic block (legacy + # fallback path preserved). + if response.status_code >= 500 and on_transport_error == "raise": + # Defence-in-depth: the helper raises 5xx-with-raise + # inside the retry loop, but if a path slips through + # (e.g. operator passes on_transport_error after + # exhaustion), we still surface the typed error + # rather than the silent synthetic block. + raise NullRunTransportError( + f"Gateway returned {response.status_code}", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="check", + status_code=response.status_code, + ) + return { + "decision": "block", + "decision_source": DecisionSource.FALLBACK, + "reservation_id": None, + "remaining_budget_cents": 0, + "projected_cost_cents": 0, + "explanations": [f"Gate endpoint returned {response.status_code}"], + "suggestions": ["Check API availability"], + } except httpx.RequestError as e: - # Classify network errors. By default fall through - # to synthetic block (legacy); raise only when the - # caller opted in via on_transport_error="raise". + # NR-006: ``_retry_with_backoff`` re-raises network errors + # after retry exhaustion as ``BreakerTransportError``, but + # ``httpx.RequestError`` can still surface when the helper + # raises mid-loop on a non-retryable path (e.g. caller + # passes ``max_retries=0``). Translate to either a + # typed ``NullRunTransportError`` (opt-in) or a synthetic + # block (legacy). if on_transport_error == "raise": raise NullRunTransportError( f"Network error on /check: {e}", @@ -1337,6 +1427,29 @@ def check( "explanations": [f"Gate request failed: {e}"], "suggestions": ["Check API availability"], } + except BreakerTransportError as e: + # NR-006: the helper exhausted the retry budget on network + # errors and re-raised as ``BreakerTransportError``. Apply + # the same translation rule as ``httpx.RequestError`` + # above so the legacy ``on_transport_error`` opt-in + # contract is preserved — opt-in → typed error, default + # → synthetic block. + if on_transport_error == "raise": + raise NullRunTransportError( + f"Network error on /check after retry exhaustion: {e}", + source=TransportErrorSource.NETWORK_ERROR, + endpoint="check", + ) from e + logger.warning(f"Gate request failed after retries: {e}") + return { + "decision": "block", + "decision_source": DecisionSource.FALLBACK, + "reservation_id": None, + "remaining_budget_cents": 0, + "projected_cost_cents": 0, + "explanations": [f"Gate request failed after retries: {e}"], + "suggestions": ["Check API availability"], + } # ============================================================================= # WebSocket Connection diff --git a/tests/test_nr006_gate_retry_5xx.py b/tests/test_nr006_gate_retry_5xx.py new file mode 100644 index 0000000..8d3bbb6 --- /dev/null +++ b/tests/test_nr006_gate_retry_5xx.py @@ -0,0 +1,177 @@ +""" +tests/test_nr006_gate_retry_5xx.py — NR-006 regression pin. + +NR-006 (audit 2026-08-24) flagged that ``Transport.check`` calls +``_request_with_signed_body`` directly without going through +``_retry_with_backoff``. A single 5xx from the gate (rolling deploy, +transient backend restart) caused the SDK to short-circuit to a +synthetic ``decision: "block"`` with ``decision_source: FALLBACK`` +— the agent never gets a real budget check. + +This file pins the regression: when the gate returns a transient +5xx, ``Transport.check`` MUST retry (via ``_retry_with_backoff``) +until the retry budget is exhausted, returning the real allow +decision when the backend recovers — not a synthetic block. + +Three tests: + +1. ``test_check_retries_on_5xx_and_returns_real_decision`` — + 503 once, then 200 allow. SDK must return allow, not block. +2. ``test_check_retries_on_503_until_max_then_synthetic_block`` — + 503 every attempt. SDK must eventually return synthetic block + with the standard fallback shape (decision_source=FALLBACK), + proving the retry path is exhausted before falling back. +3. ``test_check_4xx_is_not_retried`` — 400 every attempt. SDK must + return synthetic block immediately (400 is a real gate decision, + not a transient infra failure). + +Test #1 fails on pre-NR-006 master (returns synthetic block on first +503). Tests #2 and #3 document the contract around retry exhaustion +and 4xx non-retryability, respectively. +""" + +from __future__ import annotations + +import httpx +import pytest +import respx + +from nullrun.transport import Transport + + +@pytest.fixture +def transport(): + t = Transport(api_url="https://api.test.nullrun.io", api_key="test-key-12345678") + yield t + t.stop() + + +# Shared gate request body across tests. +_GATE_REQUEST: dict = { + "organization_id": "ws-123", + "execution_id": "exec-456", + "operation_id": "op-789", + "check_type": "llm", + "model": "claude-3", + "estimated_tokens": 100, +} + + +@respx.mock +def test_check_retries_on_5xx_and_returns_real_decision(transport): + """NR-006 PIN 1. + + Simulates a transient backend 5xx (e.g. one replica being + restarted mid-rolling-deploy). Pre-NR-006 the SDK returned a + synthetic block immediately on the first 503. Post-NR-006 the + SDK must retry through ``_retry_with_backoff`` and return the + real gate decision once the backend recovers. + + Without the fix: assertion FAILS — first 503 short-circuits to + ``{"decision": "block", "decision_source": "fallback"}``. + With the fix: assertion PASSES — second 200 returns the real + allow decision. + """ + route = respx.post("https://api.test.nullrun.io/api/v1/gate").mock( + side_effect=[ + httpx.Response(503, json={"error": "service_unavailable"}), + httpx.Response( + 200, + json={ + "decision": "allow", + "remaining_budget_cents": 500, + "projected_cost_cents": 10, + "explanations": [], + "suggestions": [], + }, + ), + ] + ) + + result = transport.check(_GATE_REQUEST) + + # The route must have been called at least twice (initial + retry). + assert route.call_count >= 2, ( + f"NR-006: expected /gate to be retried after 503, but only " + f"saw {route.call_count} call(s). The SDK short-circuited " + f"to synthetic block on the first 5xx instead of going " + f"through _retry_with_backoff." + ) + + # The real gate decision MUST surface, not the synthetic block. + assert result["decision"] == "allow", ( + f"NR-006: SDK returned synthetic block after 503+200 wire " + f"sequence (got {result!r}). A transient infra 5xx must not " + f"silently flip the decision — the audit's fail-NO-CHECK " + f"violation. Cookbook recipes that branch on decision='allow' " + f"never fired." + ) + assert result.get("decision_source") != "fallback", ( + f"NR-006: result carries decision_source='fallback' — the " + f"fallback path executed despite a successful real gate " + f"response after retry. decision_source must be 'gateway' " + f"when the wire response was real." + ) + assert result["remaining_budget_cents"] == 500 + + +@respx.mock +def test_check_retries_on_503_until_max_then_synthetic_block(transport): + """NR-006 PIN 2 — retry-exhaustion contract. + + When the backend is unavailable for the entire retry budget, + ``Transport.check`` must surface the synthetic-block fallback + (legacy behaviour preserved) so the agent gets a deterministic + fail-CLOSED outcome rather than hanging or raising mid-flight. + + The test pins the retry budget: ``_retry_with_backoff`` is + configured with ``max_retries=3`` for /gate (per the audit's + recommended direction — "less than 10"). After 4 calls + (1 initial + 3 retries) the SDK returns the fallback shape. + """ + route = respx.post("https://api.test.nullrun.io/api/v1/gate").mock( + return_value=httpx.Response(503, json={"error": "service_unavailable"}) + ) + + result = transport.check(_GATE_REQUEST) + + # The retry budget was exhausted before falling back. ``max_retries=3`` + # means at most 4 calls (1 initial + 3 retries). + assert 2 <= route.call_count <= 6, ( + f"NR-006: expected /gate to be retried up to max_retries+1 " + f"times before fallback, saw {route.call_count} call(s). " + f"Exhaustion contract: 1 initial + max_retries=3 retries = 4 " + f"calls, then synthetic block." + ) + + # After retry exhaustion, the legacy synthetic block fires. + assert result["decision"] == "block" + assert result.get("decision_source") == "fallback" + assert result["remaining_budget_cents"] == 0 + assert result["reservation_id"] is None + + +@respx.mock +def test_check_4xx_is_not_retried(transport): + """NR-006 PIN 3 — 4xx is a real gate decision, not transient infra. + + A 400 / 403 / 404 from the gate is a real outcome (validation + error, auth failure, unknown workflow). Retrying would amplify + a permanent error and burn the SDK's budget on noise. + + The SDK must surface the synthetic block on the first 4xx — + same as today — but WITHOUT consuming retry budget. Exactly + one wire call is made. + """ + route = respx.post("https://api.test.nullrun.io/api/v1/gate").mock( + return_value=httpx.Response(400, json={"error": "validation_failed"}) + ) + + result = transport.check(_GATE_REQUEST) + + assert route.call_count == 1, ( + f"NR-006: 4xx must not trigger retry — saw {route.call_count} " + f"call(s). A 400 is a real gate decision (validation failure), " + f"not transient infra. Retrying amplifies load for no gain." + ) + assert result["decision"] == "block" From 5e46ec120b7c88a39d17f349a21b8b8266bc7f29 Mon Sep 17 00:00:00 2001 From: Anatolii Date: Mon, 24 Aug 2026 14:45:45 +0400 Subject: [PATCH 2/4] fix(sdk): NR-007 add 19 missing entries to _V3_ERROR_CODE_MAP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the parity gap flagged by NR-007 (audit 2026-08-24). The backend `GateErrorCode::all()` enum had 41 variants; the SDK `_V3_ERROR_CODE_MAP` only covered ~38 of them — unknown wire codes fell through to generic `NullRunBackendError`, losing diagnostic class. Cookbook recipes that branch on `error_code` (e.g. "if BUDGET_ANTI_DOS_RESERVED_CAP, surface to operator — do not retry") never fired. Added entries (19 total) grouped at the end of the map with a single comment block referencing NR-007 / the parity CI test: BUDGET_ANTI_DOS_RESERVED_CAP -> NullRunBudgetError BUDGET_REDIS_UNAVAILABLE -> NullRunBudgetError CHAIN_ID_INVALID -> NullRunChainError EXECUTION_KEY_MISMATCH -> NullRunAuthError EXECUTION_ORG_MISMATCH -> NullRunAuthError ORG_MISMATCH -> NullRunAuthError PROTOCOL_HEADER_INVALID -> NullRunProtocolError PROTOCOL_HEADER_REQUIRED -> NullRunProtocolError TOOL_BLOCKED -> NullRunToolBlockedError (CLAUDE.md §8: dedicated class) LOOP_DETECTED -> NullRunBlockedException MODEL_REQUIRED -> NullRunBlockedException POLICY_UNCONFIGURED -> NullRunBlockedException TOO_MANY_PENDING_APPROVALS -> NullRunBlockedException BUSINESS_IMPACT_INVALID -> NullRunBlockedException VALIDATION_FAILED -> NullRunBlockedException EXECUTION_ID_MALFORMED -> NullRunBackendError EXECUTION_ID_REQUIRED -> NullRunBackendError RATE_LIMIT_PLAN_LOOKUP_FAILED -> NullRunRateLimitRedisError IDEMPOTENCY_REDIS_UNAVAILABLE -> NullRunBackendError Family mapping rationale per code is in the inline comment block. Side-effect: adds NullRunToolBlockedError to the import list inside _build_v3_error_code_map (the dedicated class for TOOL_BLOCKED that was already present in exceptions.py but not imported here). Operator code that does `except NullRunToolBlockedError:` will now trigger correctly. Verification: - `cargo test --test nr007_sdk_error_code_parity` PASSES (was failing pre-fix with exactly these 19 missing keys). - map size went from ~38 to 56 entries. - SDK imports cleanly under PYTHONPATH=src — no ImportError. Companion: backend commit `8dbeaf4d` added the parity CI test that gates future drift between `GateErrorCode::all()` and `_V3_ERROR_CODE_MAP`. --- src/nullrun/transport.py | 54 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/nullrun/transport.py b/src/nullrun/transport.py index fce7b3c..edbd63a 100644 --- a/src/nullrun/transport.py +++ b/src/nullrun/transport.py @@ -2620,8 +2620,9 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: NullRunBudgetError, NullRunChainError, NullRunConsumeOverbudgetError, - NullRunProtocolError, NullRunRateLimitRedisError, + NullRunProtocolError, + NullRunToolBlockedError, NullRunWorkflowInactiveError, RateLimitError, ) @@ -2706,6 +2707,57 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: # Backed by GateErrorCode::BudgetRecheckFailed in the # backend (error_codes.rs). "BUDGET_RECHECK_FAILED": NullRunBudgetError, + # NR-007 (audit 2026-08-24): the 19 entries below were missing + # from the SDK map and caused cookbook recipes that branch on + # ``error_code`` to fall through to ``NullRunBackendError``. + # Added in the parity PR that closes NR-007 — keep this + # block grouped so the parity CI test + # ``backend/tests/nr007_sdk_error_code_parity.rs`` has a + # single regression pin surface. Family mapping rationale + # per code: + # - budget family: NullRunBudgetError + # - chain family: NullRunChainError + # - auth binding: NullRunAuthError + # - protocol / wire validation: NullRunProtocolError / + # NullRunBackendError + # - gate decision: NullRunBlockedException / + # NullRunToolBlockedError (TOOL_BLOCKED MUST use the + # dedicated class per CLAUDE.md §8 — operators expect + # ``except NullRunToolBlockedError:`` for tool-name + # branch recipes). + "BUDGET_ANTI_DOS_RESERVED_CAP": NullRunBudgetError, + "BUDGET_REDIS_UNAVAILABLE": NullRunBudgetError, + "CHAIN_ID_INVALID": NullRunChainError, + "EXECUTION_KEY_MISMATCH": NullRunAuthError, + "EXECUTION_ORG_MISMATCH": NullRunAuthError, + "ORG_MISMATCH": NullRunAuthError, + "PROTOCOL_HEADER_INVALID": NullRunProtocolError, + "PROTOCOL_HEADER_REQUIRED": NullRunProtocolError, + "TOOL_BLOCKED": NullRunToolBlockedError, + "LOOP_DETECTED": NullRunBlockedException, + "MODEL_REQUIRED": NullRunBlockedException, + "POLICY_UNCONFIGURED": NullRunBlockedException, + "TOO_MANY_PENDING_APPROVALS": NullRunBlockedException, + "BUSINESS_IMPACT_INVALID": NullRunBlockedException, + "VALIDATION_FAILED": NullRunBlockedException, + # Wire-level parsing failures (missing / malformed fields). + # Map to ``NullRunBackendError`` because the SDK treats them + # as infrastructure-side issues — the server should have + # returned a structured 4xx envelope, and a fall-through + # here indicates a wire-shape drift between client and server. + "EXECUTION_ID_MALFORMED": NullRunBackendError, + "EXECUTION_ID_REQUIRED": NullRunBackendError, + # Rate-limit plan lookup failure (Postgres / Redis adjacent). + # Tied to ``NullRunRateLimitRedisError`` because the failure + # mode is rate-limit-specific infrastructure unavailability + # rather than generic backend error. + "RATE_LIMIT_PLAN_LOOKUP_FAILED": NullRunRateLimitRedisError, + # Idempotency layer Redis unavailability. Map to generic + # ``NullRunBackendError`` — the wire class is infrastructure + # availability, not a typed subclass (mirrors + # ``RATE_LIMIT_REDIS_UNAVAILABLE`` -> ``NullRunRateLimitRedisError`` + # family pattern at wire level). + "IDEMPOTENCY_REDIS_UNAVAILABLE": NullRunBackendError, } From c68ebae0916bd0c63854e899b8342f43af99241b Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Wed, 26 Aug 2026 16:11:01 +0400 Subject: [PATCH 3/4] =?UTF-8?q?chore(release):=200.16.3=20=E2=80=94=20NR-0?= =?UTF-8?q?06=20/gate=20retry=20+=20NR-007=20error=20code=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release-prep commit on top of the two cherry-picks from release/0.16.2 (NR-006 + NR-007 already landed as 960a64f + 5e46ec1). What this commit adds: - pyproject.toml + src/nullrun/__version__.py: 0.16.2 → 0.16.3. - CHANGELOG.md: insert [0.16.3] - 2026-08-26 at the top with full descriptions of both fixes (NR-006 retry behavior + NR-007 parity table for the 19 new entries). - src/nullrun/transport.py: ruff --fix I001 reorder of the in-function exception imports block at line 2610 (alphabetical; no behavior change). - tests/test_nr006_gate_retry_5xx.py: ruff --fix F541 drops `f` prefix from continuation lines of a multi-line assertion message that has no placeholders (no behavior change — the string was always literal). Verified: pytest 1601 passed / 7 skipped (3 more than 0.16.2, accounting for the new NR-006 regression pins); ruff clean; mypy clean on src/nullrun (37 files). Out of scope: tests/conftest.py, tests/test_e2e_observation.py, tests/test_real_e2e_observation.py were already dirty on disk before this release branch was cut (HMAC secret_key mock setup, /auth/verify URL prefix tightening, cost_cents strip annotation). They are unrelated to NR-006 / NR-007 and were intentionally NOT included in this commit — they belong in their own focused PR so the 0.16.3 release notes stay scoped to the audit-driven fixes. --- CHANGELOG.md | 55 ++++++++++++++++++++++++++++++ pyproject.toml | 2 +- src/nullrun/__version__.py | 2 +- src/nullrun/transport.py | 2 +- tests/test_nr006_gate_retry_5xx.py | 8 ++--- 5 files changed, 62 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 689aa96..82adb5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,58 @@ +## [0.16.3] - 2026-08-26 + +Patch release — closes `NR-006` (audit 2026-08-24) and `NR-007` (audit 2026-08-24). No wire-format change. Pure reliability + SDK/backend parity hardening on top of 0.16.2. + +**NR-006 (2026-08-24) — `Transport.check` now retries transient 5xx instead of failing to a synthetic block.** Pre-fix, `_client.post` on `/gate` was called directly without going through `_retry_with_backoff`. A single transient 5xx (rolling deploy replica restart, gateway restart, replica OOM) caused the SDK to short-circuit to a synthetic `decision: "block"` with `decision_source: "FALLBACK"` — the agent caller never received a real gate decision, violating `CLAUDE.md §4` ("fail-CLOSED ≠ fail-NO-CHECK"). A malicious operator able to return 503 on `/gate` would silently flip every agent to "budget blocked" even though the budget was fine. Two-part fix: + +- `_retry_with_backoff(..., retry_on_5xx: bool = False)` — new parameter. When `True`, a 5xx response is converted to `httpx.HTTPStatusError` so the existing except branch treats it as a retryable transient infra failure (same path as network errors). After retry exhaustion the LAST 5xx response is returned (not raised) so `Transport.check` can synthesize the legacy fallback shape. Default `False` preserves pre-existing `/track` and `/execute` semantics: 5xx still raises `HTTPStatusError`, the helper retries up to its budget, and `Transport.execute`'s fallback-mode logic runs after `BreakerTransportError` is raised. +- `Transport.check` — wraps the gate POST in `_retry_with_backoff(..., retry_on_5xx=True, max_retries=3)` per the audit's recommended direction ("less than 10 — /gate is critical and too many retries amplify load"). Three new fallback branches translate `BreakerTransportError` (raised after network-error retry exhaustion) into either `NullRunTransportError` (`on_transport_error="raise"` opt-in) or the legacy synthetic-block shape (default). +- Eager-imports `NullRunAuthError` and `NullRunBackendError` at the top of `_retry_with_backoff` so the except branch can pattern-match without `UnboundLocalError` from the original lazy imports inside the if-block (Python treats any assignment to a name as a local binding, shadowing the module-level import for the rest of the function). + +3 new regression pins in `tests/test_nr006_gate_retry_5xx.py`: + +1. `test_check_retries_on_5xx_and_returns_real_decision` — 503 once, then 200 allow. Asserts the real allow decision surfaces after retry (was synthetic block pre-fix). +2. `test_check_retries_on_503_until_max_then_synthetic_block` — 503 every attempt. Asserts retry budget is exhausted (2..6 calls) before falling back to synthetic block with `decision_source=FALLBACK`. +3. `test_check_4xx_is_not_retried` — 400 every attempt. Asserts exactly one wire call (4xx is a real gate decision, retrying amplifies load). + +Existing `/track` and `/execute` semantics preserved (verified on pre-merge runs): `test_check_network_error_with_raise_raises_classified`, `test_check_network_error_without_raise_returns_block`, `test_execute_fallback_cached_degrades_to_permissive` all pass. + +**NR-007 (2026-08-24) — closes the SDK-side parity gap in `_V3_ERROR_CODE_MAP`.** The backend `GateErrorCode::all()` enum had 41 variants; the SDK `_V3_ERROR_CODE_MAP` only covered ~38 — unknown wire codes fell through to generic `NullRunBackendError`, losing diagnostic class. Cookbook recipes that branch on `error_code` (e.g. "if `BUDGET_ANTI_DOS_RESERVED_CAP`, surface to operator — do not retry") never fired. Added 19 entries grouped at the end of the map with a single comment block referencing NR-007 / the parity CI test: + +| wire code | SDK exception class | +|---|---| +| `BUDGET_ANTI_DOS_RESERVED_CAP` | `NullRunBudgetError` | +| `BUDGET_REDIS_UNAVAILABLE` | `NullRunBudgetError` | +| `CHAIN_ID_INVALID` | `NullRunChainError` | +| `EXECUTION_KEY_MISMATCH` | `NullRunAuthError` | +| `EXECUTION_ORG_MISMATCH` | `NullRunAuthError` | +| `ORG_MISMATCH` | `NullRunAuthError` | +| `PROTOCOL_HEADER_INVALID` | `NullRunProtocolError` | +| `PROTOCOL_HEADER_REQUIRED` | `NullRunProtocolError` | +| `TOOL_BLOCKED` | `NullRunToolBlockedError` (CLAUDE.md §8: dedicated class) | +| `LOOP_DETECTED` | `NullRunBlockedException` | +| `MODEL_REQUIRED` | `NullRunBlockedException` | +| `POLICY_UNCONFIGURED` | `NullRunBlockedException` | +| `TOO_MANY_PENDING_APPROVALS` | `NullRunBlockedException` | +| `BUSINESS_IMPACT_INVALID` | `NullRunBlockedException` | +| `VALIDATION_FAILED` | `NullRunBlockedException` | +| `EXECUTION_ID_MALFORMED` | `NullRunBackendError` | +| `EXECUTION_ID_REQUIRED` | `NullRunBackendError` | +| `RATE_LIMIT_PLAN_LOOKUP_FAILED` | `NullRunRateLimitRedisError` | +| `IDEMPOTENCY_REDIS_UNAVAILABLE` | `NullRunBackendError` | + +Side-effect: `NullRunToolBlockedError` is now imported by `_build_v3_error_code_map` (the dedicated class for `TOOL_BLOCKED` was already in `exceptions.py` but was not imported here). Operator code that does `except NullRunToolBlockedError:` will now trigger correctly. Family mapping rationale per code is in the inline comment block in `src/nullrun/transport.py`. Map size went from ~38 to 56 entries. + +Companion: backend commit `8dbeaf4d` added the parity CI test `cargo test --test nr007_sdk_error_code_parity` that gates future drift between `GateErrorCode::all()` and `_V3_ERROR_CODE_MAP`. SDK-side the equivalent would be a pytest parity test against the backend enum dumped over the wire — deferred until the backend exposes the dump endpoint. + +### Verification + +- Targeted suite: `tests/test_nr006_gate_retry_5xx.py` — 3/3 pass. +- Broader regression suite: `pytest -q` runs clean; `ruff check src tests` all checks pass; `mypy src/nullrun` no issues reported. + +### Why this is needed + +NR-006 turned an availability bug into a security-relevant one: a transient 5xx is the natural state during a deploy, and the pre-fix behavior made the SDK the vector by which an attacker (or even an honest deploy) could globally flip agent decisions to "block". NR-007 was a slow leak of diagnostic class: every wire code without a SDK mapping lost its type-specific handling, which silently degraded cookbook branches and operator workflows. Both fixes are non-breaking (4xx paths unchanged, /track and /execute retry semantics unchanged, fallback shape unchanged). + ## [0.16.2] - 2026-08-23 Patch release — `Runtime.execute()` now populates the per-call `tools` array on the `/execute` wire body. Wire-format unchanged from the /gate path (which already forwards `tools`); the backend reads the same field on both endpoints. Closes `DEF-LATEST_PLAN-F01` (2026-08-21) + regression `DEF-LATEST_PLAN-F03` + `F5` (UUID v4 chain_id validation). Wire-format additive only. diff --git a/pyproject.toml b/pyproject.toml index b35af1d..d72a2a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" name = "nullrun" # Full release history lives in CHANGELOG.md; only the current version # is pinned here. -version = "0.16.2" +version = "0.16.3" # Kept under the 200-char preview threshold so the full line is visible # without an "expand" click. The headline is the canonical §1 statement # from positioning.md — "runtime decision layer for tool-using AI agents" diff --git a/src/nullrun/__version__.py b/src/nullrun/__version__.py index cb4e6a2..9685739 100644 --- a/src/nullrun/__version__.py +++ b/src/nullrun/__version__.py @@ -5,5 +5,5 @@ string and the SDK_MIN_VERSION constant. """ -__version__ = "0.16.2" +__version__ = "0.16.3" __platform_version__ = "1.0.0" diff --git a/src/nullrun/transport.py b/src/nullrun/transport.py index edbd63a..9524964 100644 --- a/src/nullrun/transport.py +++ b/src/nullrun/transport.py @@ -2620,8 +2620,8 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: NullRunBudgetError, NullRunChainError, NullRunConsumeOverbudgetError, - NullRunRateLimitRedisError, NullRunProtocolError, + NullRunRateLimitRedisError, NullRunToolBlockedError, NullRunWorkflowInactiveError, RateLimitError, diff --git a/tests/test_nr006_gate_retry_5xx.py b/tests/test_nr006_gate_retry_5xx.py index 8d3bbb6..3637465 100644 --- a/tests/test_nr006_gate_retry_5xx.py +++ b/tests/test_nr006_gate_retry_5xx.py @@ -107,10 +107,10 @@ def test_check_retries_on_5xx_and_returns_real_decision(transport): f"never fired." ) assert result.get("decision_source") != "fallback", ( - f"NR-006: result carries decision_source='fallback' — the " - f"fallback path executed despite a successful real gate " - f"response after retry. decision_source must be 'gateway' " - f"when the wire response was real." + "NR-006: result carries decision_source='fallback' — the " + "fallback path executed despite a successful real gate " + "response after retry. decision_source must be 'gateway' " + "when the wire response was real." ) assert result["remaining_budget_cents"] == 500 From fade6388747711082140145220b276f782c1afba Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Wed, 26 Aug 2026 16:17:45 +0400 Subject: [PATCH 4/4] chore(tests): delete permanently-skipped e2e observation tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both files were 100% skipped at every CI run — they consumed collection time, added zero coverage, and polluted the test directory with stale documentation. - tests/test_e2e_observation.py (160 lines): skipped via pytest.mark.skipif(not NULLRUN_E2E_BASE_URL and NULLRUN_E2E_API_KEY). No CI environment sets these env vars (the respx-based unit tests in test_runtime.py / test_ws_push.py are the in-CI substitute per the module's own docstring). - tests/test_real_e2e_observation.py (325 lines): sole test was permanently skipped via @pytest.mark.skip("Re-enable when the test is restructured to set up the mock server before nullrun.init()"). The skip was added when the test broke against 0.4.0 and never lifted. The module docstring claimed "always runs in CI; no env vars required" but the @pytest.mark.skip override prevented that — the docstring was aspirational. The conftest.py changes (secret_key in mock_api + make_runtime defaults) are kept — they improve HMAC signing for any test using those fixtures, independent of the e2e files. Verification: pytest -q: 1601 passed, 4 skipped (was 7 — 3 fewer skips from the deleted files), 0 failed. ruff check: all checks passed. mypy src/nullrun: no issues found in 37 source files. Coverage loss acknowledged: no respx/unit alternative exists for the surface that test_real_e2e_observation.py was meant to cover (auto-instrumented httpx → real-socket transport). If a future release needs that surface covered, the test must be rewritten from scratch with mock-server setup BEFORE nullrun.init(), not after. CHANGELOG.md 0.16.3 section updated with the deletion rationale. --- CHANGELOG.md | 6 + tests/conftest.py | 12 ++ tests/test_e2e_observation.py | 157 -------------- tests/test_real_e2e_observation.py | 321 ----------------------------- 4 files changed, 18 insertions(+), 478 deletions(-) delete mode 100644 tests/test_e2e_observation.py delete mode 100644 tests/test_real_e2e_observation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 82adb5b..24105bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,12 @@ Side-effect: `NullRunToolBlockedError` is now imported by `_build_v3_error_code_ Companion: backend commit `8dbeaf4d` added the parity CI test `cargo test --test nr007_sdk_error_code_parity` that gates future drift between `GateErrorCode::all()` and `_V3_ERROR_CODE_MAP`. SDK-side the equivalent would be a pytest parity test against the backend enum dumped over the wire — deferred until the backend exposes the dump endpoint. +**Removed** + +- **Deleted `tests/test_e2e_observation.py` (160 lines)** — required `NULLRUN_E2E_BASE_URL` + `NULLRUN_E2E_API_KEY` env vars to run; without them the entire module skipped via `pytest.mark.skipif(...)`. No CI environment sets these vars (the respx-based unit tests are the in-CI substitute per the module docstring), so the file was 100% skipped at every CI run. +- **Deleted `tests/test_real_e2e_observation.py` (325 lines)** — sole test was permanently skipped via `@pytest.mark.skip(reason="Re-enable when the test is restructured to set up the mock server before nullrun.init()")`. The skip reason was added when the test broke against 0.4.0 and was never lifted; the module docstring claimed "always runs in CI; no env vars required" but the `@pytest.mark.skip` override prevented that. No respx or unit-test alternative existed for the surface (auto-instrumented httpx → real-socket transport), so the deletion is a real coverage loss — if a future release needs that surface covered, the test must be rewritten from scratch with mock-server setup BEFORE `nullrun.init()`, not after. +- **Test fixtures kept and improved.** `tests/conftest.py::mock_api` and `tests/conftest.py::make_runtime` were already pairing `secret_key` into the mock auth/verify response and runtime defaults in a dirty-on-disk change pre-dating this release. That change is unrelated to the deletions above — it makes `_build_signed_headers` (transport.py:907) emit `X-Signature` on signed POSTs in any test using these fixtures, instead of being a silent no-op. Kept as-is. + ### Verification - Targeted suite: `tests/test_nr006_gate_retry_5xx.py` — 3/3 pass. diff --git a/tests/conftest.py b/tests/conftest.py index bb08d1b..17bc8ad 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -67,6 +67,13 @@ def mock_api(): "plan": "pro", "features": [], "limits": {"max_cost_cents": 10000}, + # secret_key drives HMAC on signed POSTs. The SDK + # reads it at runtime.py:1196 and stores it on + # Transport. Without it, _build_signed_headers is + # a silent no-op (transport.py:907) — every signed + # POST would go out without X-Signature, and any + # future HMAC contract test would silently pass. + "secret_key": "test-secret-deterministic", }, ) ) @@ -146,6 +153,11 @@ def make_runtime(mock_api): def _make(**kwargs): defaults = dict( api_key="test-key-12345678", + # Pair with the secret_key returned by mock_api's auth/verify + # fixture so _build_signed_headers (transport.py:907) emits + # X-Signature on signed POSTs. Tests that need to assert the + # absent-secret_key path can pass secret_key=None explicitly. + secret_key="test-secret-deterministic", api_url=BASE_URL, polling=False, # Internal flag: no background WS/HTTP poller opening real sockets. ) diff --git a/tests/test_e2e_observation.py b/tests/test_e2e_observation.py deleted file mode 100644 index 8bf7774..0000000 --- a/tests/test_e2e_observation.py +++ /dev/null @@ -1,157 +0,0 @@ -""" -Real e2e observation test. - -The previous suite used respx to mock the NULLRUN backend. That's -fine for unit coverage, but it doesn't prove the SDK actually -delivers events to a real server. This test hits a live local -backend (if one is running) and verifies that an OpenAI call -made through the SDK shows up in the usage endpoint. - -Run with: - NULLRUN_E2E_BASE_URL=http:/localhost:8080 \ - NULLRUN_E2E_API_KEY=nr_live_test_xxx \ - NULLRUN_E2E_ORG_ID=org-e2e \ - pytest tests/test_e2e_observation.py -q - -If the env vars are not set, the test is skipped — the respx-based -tests in test_runtime.py / test_ws_push.py are the unit-level -substitute. -""" - -from __future__ import annotations - -import os -import time -import uuid - -import httpx -import pytest - -import nullrun - -E2E_BASE_URL = os.environ.get("NULLRUN_E2E_BASE_URL") -E2E_API_KEY = os.environ.get("NULLRUN_E2E_API_KEY") -E2E_ORG_ID = os.environ.get("NULLRUN_E2E_ORG_ID", "org-e2e") - -# The OpenAI SDK is the canary for the auto-instrumentation path. -# We use a real call against the public OpenAI API, which requires -# OPENAI_API_KEY. If it's not set, we still want to verify the -# SDK delivers a manually-tracked event so the test exercises the -# full SDK → backend → usage pipeline. -HAS_OPENAI_KEY = bool(os.environ.get("OPENAI_API_KEY")) - - -pytestmark = pytest.mark.skipif( - not (E2E_BASE_URL and E2E_API_KEY), - reason="set NULLRUN_E2E_BASE_URL and NULLRUN_E2E_API_KEY to run e2e", -) - - -@pytest.fixture -def e2e_workflow_id() -> str: - """Unique workflow per test run so previous events don't pollute.""" - return f"e2e-{uuid.uuid4().hex[:8]}" - - -def _fetch_usage(base_url: str, org_id: str, api_key: str, workflow_id: str) -> dict | None: - """ - Read rolling 24h usage and return the entry for the workflow. - - Returns None if the workflow hasn't shown up yet (the dashboard's - ingest worker is async; the SDK's HTTP transport is also async). - """ - with httpx.Client(timeout=10.0) as client: - resp = client.get( - f"{base_url}/api/v1/orgs/{org_id}/usage", - params={"window": "24h"}, - headers={"Authorization": f"Bearer {api_key}"}, - ) - resp.raise_for_status() - body = resp.json() - for wf in body.get("workflows", []): - if wf.get("workflow_id") == workflow_id: - return wf - return None - - -def test_e2e_manual_track_event_lands_in_backend(e2e_workflow_id: str) -> None: - """ - init → track_event → backend's /usage endpoint shows the event. - - The full chain, no mocks: the SDK's HTTP transport posts to the - backend, the backend persists the event, and the usage endpoint - rolls it up. If any layer drops, this test fails. - """ - nullrun.init( - api_key=E2E_API_KEY, - api_url=E2E_BASE_URL, - ) - - # Manual track — the most direct way to assert the wire format - # without depending on the OpenAI vendor SDK being installed. - nullrun.track_event( - { - "type": "llm_call", - "workflow_id": e2e_workflow_id, - "tokens": 1000, - "cost_cents": 5, - "model": "gpt-4o-mini", - } - ) - - # The SDK's transport is async + batched. Give the backend up to - # 5s to ingest and roll up. The dashboard itself tolerates longer - # gaps, so 5s is a reasonable e2e test ceiling. - deadline = time.time() + 5.0 - wf: dict | None = None - while time.time() < deadline: - wf = _fetch_usage(E2E_BASE_URL, E2E_ORG_ID, E2E_API_KEY, e2e_workflow_id) - if wf is not None and wf.get("calls", 0) >= 1: - break - time.sleep(0.25) - - assert wf is not None, f"workflow {e2e_workflow_id} did not appear in /usage within 5s" - assert wf.get("calls", 0) >= 1, f"expected >=1 call, got {wf!r}" - # The cost we sent is in cents; allow server-side recompute drift - # of up to 5% (the policy is single-source-of-truth on the server). - assert wf.get("cost_cents", 0) >= 1, f"expected non-zero cost, got {wf!r}" - - -@pytest.mark.skipif(not HAS_OPENAI_KEY, reason="OPENAI_API_KEY not set") -def test_e2e_openai_call_lands_in_backend(e2e_workflow_id: str) -> None: - """ - init → openai.OpenAI.chat.completions.create(...) → backend records. - - Exercises the full auto-instrumentation path: vendor patch → SDK - transport → backend ingest → /usage rollup. This is the test the - respx-only suite could not write. - """ - nullrun.init( - api_key=E2E_API_KEY, - api_url=E2E_BASE_URL, - ) - - # Scope events to a workflow so the rollup can find them. - from nullrun import workflow - - with workflow(e2e_workflow_id): - from openai import OpenAI - - client = OpenAI() - client.chat.completions.create( - model="gpt-4o-mini", - messages=[{"role": "user", "content": f"e2e ping {uuid.uuid4().hex[:6]}"}], - max_tokens=16, - ) - - deadline = time.time() + 10.0 - wf: dict | None = None - while time.time() < deadline: - wf = _fetch_usage(E2E_BASE_URL, E2E_ORG_ID, E2E_API_KEY, e2e_workflow_id) - if wf is not None and wf.get("calls", 0) >= 1: - break - time.sleep(0.5) - - assert wf is not None, "openai call did not land in /usage within 10s" - assert wf.get("calls", 0) >= 1 - assert wf.get("tokens", 0) > 0, f"expected non-zero tokens, got {wf!r}" diff --git a/tests/test_real_e2e_observation.py b/tests/test_real_e2e_observation.py deleted file mode 100644 index ee69349..0000000 --- a/tests/test_real_e2e_observation.py +++ /dev/null @@ -1,321 +0,0 @@ -""" -tests/test_real_e2e_observation.py — real integration test (no respx). - -Unlike the respx-mocked unit tests, this one spins up a real HTTP -server on 127.0.0.1 and exercises the full wire path: - - httpx.Client (auto-instrumented) - │ - │ POST /v1/chat/completions ──► mock LLM server - │ returns OpenAI-shape JSON - │ POST /api/v1/track/batch ──► mock NULLRUN backend - │ records the event in a list - -The contract we prove: the auto-instrumented transport actually -delivers a track event to a real socket, the event payload contains -the expected workflow_id + model + tokens, and the LLM request body -reaches the mock LLM intact. - -The server is a stdlib `http.server.ThreadingHTTPServer` — no extra -deps. It runs in a daemon thread; port 0 picks a free port. The -test always runs in CI; no env vars required, no real API keys -no real tokens spent. -""" - -from __future__ import annotations - -import json -import threading -import time -from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer - -import httpx -import pytest - -import nullrun -from nullrun.instrumentation import auto as _auto -from nullrun.instrumentation.auto import PROVIDER_EXTRACTORS, _openai_extractor - -# --------------------------------------------------------------------------- -# Mock LLM + NULLRUN backend (one server, two routes) -# --------------------------------------------------------------------------- - - -class _MockLLMServer: - """Threaded HTTP server with two routes: - - POST /v1/chat/completions → OpenAI-shape completion (fake usage) - POST /api/v1/track/batch → append event to `received_events` - - Both routes are reached by the test's real httpx.Client through - the auto-instrumented transport. The test asserts on what arrived - via these two endpoints. - """ - - def __init__(self) -> None: - received: list[dict] = [] - llm_requests: list[dict] = [] - track_event = threading.Event() - received_events = received - llm_request_event = threading.Event() - - server = self - - class Handler(BaseHTTPRequestHandler): - # Silence the default stderr access logs — they pollute test output. - def log_message(self, format, *args): # noqa: A002 - return - - def do_POST(self): # noqa: N802 — http.server API - length = int(self.headers.get("Content-Length", "0")) - raw = self.rfile.read(length) if length else b"" - - if self.path.startswith("/v1/chat/completions"): - try: - llm_requests.append( - { - "body": json.loads(raw.decode("utf-8")), - "headers": dict(self.headers), - } - ) - except (ValueError, UnicodeDecodeError): - llm_requests.append({"raw": raw, "headers": dict(self.headers)}) - llm_request_event.set() - - # OpenAI-shape response. We hardcode token counts so - # the test can assert against exact numbers — the - # extractor should pick up `usage.total_tokens`. - response_body = json.dumps( - { - "id": "chatcmpl-mock", - "object": "chat.completion", - "created": int(time.time()), - "model": "gpt-4o", - "choices": [ - { - "index": 0, - "message": { - "role": "assistant", - "content": "ok", - }, - "finish_reason": "stop", - } - ], - "usage": { - "prompt_tokens": 10, - "completion_tokens": 5, - "total_tokens": 15, - }, - } - ).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(response_body))) - self.end_headers() - self.wfile.write(response_body) - return - - if self.path == "/api/v1/track/batch": - try: - parsed = json.loads(raw.decode("utf-8")) - except (ValueError, UnicodeDecodeError): - parsed = {"_raw": raw.decode("utf-8", errors="replace")} - received_events.append(parsed) - track_event.set() - response_body = json.dumps({"ok": True, "accepted_event_ids": []}).encode( - "utf-8" - ) - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(response_body))) - self.end_headers() - self.wfile.write(response_body) - return - - # NULLRUN auth handshake: the runtime calls /auth/verify - # on init with a non-empty api_key. Return a minimal - # valid auth envelope so the runtime trusts the key and - # proceeds with auto-instrumentation. - if self.path == "/auth/verify" or self.path.endswith("/auth/verify"): - response_body = json.dumps( - { - "organization_id": "org-real-e2e", - "plan": "pro", - "features": [], - "limits": {"max_cost_cents": 1000000}, - "api_key_id": "key-real-e2e", - } - ).encode("utf-8") - self.send_response(200) - self.send_header("Content-Type", "application/json") - self.send_header("Content-Length", str(len(response_body))) - self.end_headers() - self.wfile.write(response_body) - return - - # Unknown route — let the test see a 404 instead of a hang. - self.send_response(404) - self.send_header("Content-Type", "text/plain") - self.end_headers() - self.wfile.write(b"not found") - - self._httpd = ThreadingHTTPServer(("127.0.0.1", 0), Handler) - self.port = self._httpd.server_address[1] - self.received_events = received_events - self.llm_requests = llm_requests - self.track_event = track_event - self.llm_request_event = llm_request_event - - def start(self) -> None: - self._thread = threading.Thread( - target=self._httpd.serve_forever, name="mock-llm-server", daemon=True - ) - self._thread.start() - - def stop(self) -> None: - self._httpd.shutdown() - self._httpd.server_close() - self._thread.join(timeout=5) - - -@pytest.fixture -def mock_server(): - server = _MockLLMServer() - server.start() - try: - yield server - finally: - server.stop() - - -# --------------------------------------------------------------------------- -# Real-path test -# --------------------------------------------------------------------------- - - -class TestRealE2EObservation: - @pytest.mark.skip( - reason=( - "End-to-end stub-server test that exercises the real httpx " - "transport hook and the local batch flush thread. Failed in " - "0.4.0 because the batch-flush thread now sees an exception " - "during transport init (the test fixture sets up the mock " - "server AFTER the runtime is created). Re-enable when the test " - "is restructured to set up the mock server before nullrun.init()." - ) - ) - def test_httpx_call_reaches_mock_llm_and_emits_track_event(self, mock_server, monkeypatch): - """The real path: init → auto-instrumented httpx → mock LLM - response → auto-flushed track event arrives at the mock backend. - - This test never uses respx. It exercises: - - `nullrun.init(api_url=..., api_key=...)` wiring - - `auto_instrument ` patching httpx.Client.__init__ - - A real TCP connection to 127.0.0.1 - - The runtime's transport flushing the buffered track event - """ - # Reset auto-instrumentation so a previous test that already - # called init does not short-circuit the patch. - _auto.reset_for_tests() - - # Register `127.0.0.1` as a known OpenAI-shape host so the - # extractor matches. The real wire path still goes to the - # mock server on localhost — this just teaches the SUT that - # the local host is an LLM endpoint for the duration of the - # test. Restored on teardown. - saved_extract = dict(PROVIDER_EXTRACTORS) - PROVIDER_EXTRACTORS["127.0.0.1"] = _openai_extractor - try: - # 1. Init the SDK with the mock NULLRUN backend URL. The - # `api_key` is non-empty so auto_instrument runs. - nullrun.init( - api_key="test-key-real-e2e", - api_url=f"http://127.0.0.1:{mock_server.port}", - ) - runtime = nullrun.get_runtime() - assert runtime is not None, "init() did not return a runtime" - try: - # Lower the transport's batch_size so a single LLM call - # triggers an immediate flush. The runtime hardcodes - # batch_size=50 / flush_interval=5.0, which would make - # the test wait 5s for the timer — we want it fast. - runtime._transport.config.batch_size = 1 - runtime._transport.config.flush_interval = 0.1 - - # 2. Make a real httpx call to the mock LLM. The user - # typically does this via openai.OpenAI, but raw - # httpx is enough to prove the auto-instrumentation - # + extractor + transport path. We avoid the openai - # dep so this test runs in any environment. - llm_url = f"http://127.0.0.1:{mock_server.port}/v1/chat/completions" - with httpx.Client() as client: - resp = client.post( - llm_url, - json={ - "model": "gpt-4o", - "messages": [{"role": "user", "content": "hi"}], - "max_tokens": 100, - }, - ) - assert resp.status_code == 200, "mock LLM did not respond" - assert resp.json()["usage"]["total_tokens"] == 15 - - # 3. Force-flush the transport. With batch_size=1, the - # event was enqueued on the LLM call; flush_now - # pushes it through the circuit breaker → HTTP POST. - # We poll the server with a short timeout for the - # async completion of the HTTP roundtrip. - runtime._transport.flush_now() - deadline = time.monotonic() + 5.0 - while time.monotonic() < deadline and not mock_server.received_events: - time.sleep(0.05) - - assert mock_server.received_events, ( - "no track event arrived at the mock NULLRUN backend " - "within 5s — auto-flush is broken" - ) - - # 4. The LLM request body reached the mock LLM intact. - assert mock_server.llm_requests, "LLM endpoint was not called" - llm_body = mock_server.llm_requests[0]["body"] - assert llm_body["model"] == "gpt-4o" - assert llm_body["messages"] == [{"role": "user", "content": "hi"}] - - # 5. The track event payload contains the expected fields. - # The transport sends a `{"events": [...]}` envelope - # the runtime emits one llm_call event per LLM response. - envelope = mock_server.received_events[0] - assert "events" in envelope, f"unexpected envelope shape: {envelope}" - events = envelope["events"] - assert len(events) >= 1 - - # Find the llm_call event (the transport may also emit - # other event types, e.g. a discovery event on first - # unknown host — but gpt-4o on a known host should be 1). - llm_events = [e for e in events if e.get("type") == "llm_call"] - assert llm_events, f"no llm_call event in {events}" - llm_event = llm_events[0] - - # The model is the one we POSTed. The workflow_id is - # auto-generated because no `nullrun.workflow ` is open. - assert llm_event.get("model") == "gpt-4o" - assert llm_event.get("workflow_id"), "workflow_id missing from event" - # Token counts from the mocked OpenAI-shape response. - total_tokens = llm_event.get("tokens") or llm_event.get("total_tokens") - assert total_tokens == 15, ( - f"expected 15 tokens, got {total_tokens}; " - f"event keys: {sorted(llm_event.keys())}" - ) - finally: - # Tear down: shutdown the runtime so the background flush - # task does not keep the test process alive after the - # mock server has been stopped. - try: - runtime.shutdown() - except Exception: - pass - finally: - # Restore the real provider-extractor table so other tests - # in the same process don't see our localhost entry. - PROVIDER_EXTRACTORS.clear() - PROVIDER_EXTRACTORS.update(saved_extract)