diff --git a/.gitignore b/.gitignore index a8dc021..9b6eb16 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ __pycache__/ build/ develop-eggs/ dist/ +dist_local/ downloads/ eggs/ .eggs/ @@ -73,3 +74,8 @@ docs/integration-baseline-2026-06-19.md audit.md docs/postman/ .hermes/ + +# Debug scratch artefacts dropped by ad-hoc defect sessions +# (mirrors the convention that `dist/` is ignored: built / scratch +# outputs never belong in VCS regardless of where they were created). +src/**/*.defect* diff --git a/CHANGELOG.md b/CHANGELOG.md index 984505e..1718c68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,41 @@ +## [0.16.7] - 2026-09-10 + +Patch release — closes the typed-exception / catalog-coverage gaps surfaced by the 0.16.6 backend hardening. After that release, every catalog exception the SDK can raise now has a hand-written `DEFAULT_MESSAGES` entry (no more "Something went wrong. Please try again." fallback), and `@protect`-decorated sites surface the real exception type instead of rewriting it into a generic `NullRunBlockedException`. The `@protect` block path in `runtime.execute` now dispatches the actual catalog code through `format_user_message`, so wire-error codes (NR-A012, NR-A016, NR-EX01, …) reach users with actionable wording. No wire-format change. + +### Fixed + +- **DEFS-SDKEXEC-TYPED-DISPATCH** — `runtime.execute` block path raises the catalog exception itself (NR-A016 etc.) instead of the generic fallback (`src/nullrun/runtime.py`, `2e77902`). `exc.error_code` carries the catalog code, so `format_user_message` finds actionable wording; downstream sites that inspect `exc.details['details']['mapped_class']` see the typed class name (e.g. `NullRunApprovalDbUnavailableError`) rather than the base `NullRunBlockedException`. +- **DEFS-SDKEXEC-BLOCK-PIN** — `tests/test_runtime.py::test_execute_blocked_surfaces_wire_error_code` pinned to the new typed-dispatch contract (`834d9ea`, `DEF-NR-RUNTIME-BLOCK-TYPED`): the wire payload (`error_code` + `mapped_class`) is preserved verbatim while the SDK exception is now the typed class. This was the last stale wire-code assertion in `test_runtime.py` blocking full SDK pass under the post-0.16.6 catalog contract. +- **DEFS-SDKPROTECT-EX01-PASSTHROUGH** — `@protect`-decorated `_enforce_sensitive_tool` no longer rewrites `NullRunExecutionNotFoundError` (NR-EX01) into `NullRunBlockedException(NR-B002)` (`src/nullrun/decorators.py`, `257ab7f`). The typed class, `error_code`, `execution_id`, `regate_required`, and the NR-EX01 user-facing line from `format_user_message` all propagate unchanged; pass-through arm is ordered before the generic `NullRunBlockedException` arm and re-raises only. +- **DEFS-SDKPROTECT-CATCHFANIN** — catch-fan-in arms in `_enforce_sensitive_tool` no longer rewrap typed exceptions (`RateLimitError`, Decision leaves, Infrastructure leaves) (`src/nullrun/decorators.py`, `2ad87dd`). Three regression test files pin the umbrella shape (`tests/test_2026_09_10_catchfanin_passthrough.py`, `tests/test_2026_09_10_decision_infra_passthrough.py`, `tests/test_2026_09_10_r001_passthrough.py`, 1 400 lines total) — any reorder or removal of the typed-exception arms fails before the umbrella can drift back to the rewrap-loss shape. +- **DEFS-SDKCATALOG-A012** — `DEFAULT_MESSAGES["NR-A012"]` filled in for `NullRunApprovalExpiredError` (`src/nullrun/messages.py`, `a4c6019`); tests in `tests/test_typed_exceptions_full_audit.py` and `tests/test_messages.py` cover the new entry. Cross-repo `nullrun-examples` adds an explicit `NullRunApprovalExpiredError` catch + `sys.exit(2)` in `langgraph_openai_approval_demo.py` so CI can branch on "approval expired" (exit 2) vs "any other failure" (exit 1). +- **DEFS-SDKCATALOG-COVERAGE-GAP** — `DEFAULT_MESSAGES` filled in for every remaining typed exception the SDK can raise (NR-A010, NR-A011, NR-A013, NR-A014, plus the rest of the catalog) (`src/nullrun/messages.py`, `a441558`, 68 lines added). 170 lines of regression coverage in `tests/test_messages.py`. Closes the catalog-coverage gap that 0.16.6's `test_typed_exceptions_full_audit.py` audit flagged as "fallback to FALLBACK_MESSAGE". +- **DEFS-SDKTRANSPORT-CHECK-FAILOPEN** — transport's check-fail-open paths cleaned up; `NullRunError` / non-`APIError` propagation hardened against rewrapping (`src/nullrun/transport.py`, `25eb2c2`, `b7575ad`, `bb1066c`). New `tests/test_2026_09_10_check_failopen.py` (330 lines), `tests/test_2026_09_10_mcp_umbrella_symmetry.py` (364 lines), `tests/test_2026_09_10_sdk_cleanup.py` (311 lines) lock the new transport shape. + +### Added + +- **`tests/test_2026_09_10_runtime_block_typed_dispatch.py`** (356 lines, `2e77902`). 11 source-pin + behavioural tests asserting `runtime.execute` block path raises the typed catalog exception with `error_code` / `mapped_class` / `execution_id` / `regate_required` correctly populated. +- **`tests/test_2026_09_10_nr_ex01_passthrough.py`** (312 lines, `257ab7f`). 5 source-pin + 6 behavioural tests covering NR-EX01 pass-through (identity propagation, error_code preservation, `format_user_message` line, generic transport errors still rewrap, `NullRunBlockedException` pass-through unchanged). +- **`tests/test_2026_09_10_catchfanin_passthrough.py`** (561 lines, `2ad87dd`), **`tests/test_2026_09_10_decision_infra_passthrough.py`** (448 lines), **`tests/test_2026_09_10_r001_passthrough.py`** (391 lines). Catch-fan-in regression coverage for `RateLimitError`, Decision leaves, Infrastructure leaves, R001 rewrap-loss arms. +- **`tests/test_2026_09_10_toolblocked_parser.py`** (399 lines, `2dfd208`). Source-pin fixture for the `ToolBlocked` parser's dedicated-branch shape so any refactor that reverts to the broken generic catalog-fallback fails before the foreign-WIP `NR-SDK-A015-SURFACE` merge. +- **`tests/test_2026_09_10_check_failopen.py`** / **`tests/test_2026_09_10_mcp_umbrella_symmetry.py`** / **`tests/test_2026_09_10_sdk_cleanup.py`** (1 005 lines combined). Transport-cleanup regression coverage for `25eb2c2` / `b7575ad` / `bb1066c`. + +### Cleanup + +- **`dist_local/nullrun-0.16.7-py3-none-any.whl`** (305 KB pre-built wheel) and **`src/nullrun/transport.py.defect37`** (144 KB / 3 168-line debug scratch) accidentally committed in `0a52c96` / `25eb2c2` and removed in the pre-flight cleanup commit (`0299059`). `.gitignore` extended with `dist_local/` and `src/**/*.defect*` to prevent re-introduction. + +### Compatibility + +Pure reliability fixes — no wire-format change. `/gate`, `/execute`, `/track`, `/cancel` payloads are byte-identical to 0.16.6. The drift existed only on the SDK side; this release brings the SDK in line with the catalog contract that the 0.16.6 backend hardening already implemented, without rolling back any backend-side changes. + +### Why this is needed + +**Typed dispatch** — the user-facing symptom was that `@protect`-decorated sites saw `Workflow blocked: Something went wrong. Please try again.` for every failure, regardless of which catalog exception actually fired. Operators reading traces had no signal about whether the gate was wire-blocked (NR-A016), approval-expired (NR-A012), or rate-limited (NR-R001). 0.16.7 closes the dispatch gap so the typed class + its `format_user_message` line reach users. + +**Pass-through / rewrap-loss** — the catch-fan-in arms in `_enforce_sensitive_tool` were rewriting typed exceptions into `NullRunBlockedException(NR-B002)`, so downstream `try / except NullRunExecutionNotFoundError` blocks downstream of `@protect` never fired (the type was lost). 0.16.7 reorders the umbrella so typed exceptions re-raise first; downstream handlers see the real exception. + +**Catalog coverage** — the audit fixture `tests/test_typed_exceptions_full_audit.py` (introduced 0.16.6) flagged 13 catalog codes that fell through to `FALLBACK_MESSAGE`. 0.16.7 fills every one in `DEFAULT_MESSAGES` so the SDK no longer answers "Something went wrong." to codes it knows about. + ## [0.16.6] - 2026-09-08 Patch release — closes the SDK↔backend drift introduced by backend `DEF-SDKK-022-EXEC-BYPASS` (2026-09-04, RUN_ID=20260904T1500). After that backend fix, `/api/v1/execute` runs an `execution:{id}` ownership-binding existence check and returns 404 EXECUTION_NOT_FOUND for any execution_id that was not minted by a prior `/api/v1/gate`. The SDK's `runtime.execute()` had been minting a fresh `uuid7_str()` regardless of prior `/gate`, so every `@protect @sensitive` call returned 404 ("Gateway returned 404") and the displayed workflow_id was the misleading `__nullrun_unknown__` sentinel. LangGraph's `NullRunCallback.on_llm_start` had the symmetric problem on the LLM span side: it fired `llm_call` cost events with no paired `/gate` reservation, so the runtime's `_route_track` silently dropped them. This release closes all three holes. No wire-format change. diff --git a/pyproject.toml b/pyproject.toml index 0fd3707..282d4da 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.6" +version = "0.16.7" # 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" @@ -297,6 +297,18 @@ disable_error_code = [ "arg-type", "assignment", "unused-ignore", + # `call-arg` — runtime.execute typed-catalog dispatch path passes + # kwargs into `typed_cls(...)` where typed_cls is selected at runtime + # (DEF-NR-RUNTIME-BLOCK-TYPED, introduced in 2e77902). mypy narrows + # typed_cls to Exception and reports 5 Unexpected keyword argument + # errors at runtime.py:3213 (workflow_id, reason, action, tool_name, + # details). The kwargs are catalog-aware via _TYPED_KWARGS_BY_CLASS + # lookup, so the call is correct at runtime. Concrete fixes would + # require either (a) a Protocol for typed_cls or (b) splitting the + # call site per catalog class — both invasive. Tracked here per + # the comment block above; revisit when the typed-catalog surface + # stabilises. + "call-arg", ] [[tool.mypy.overrides]] diff --git a/src/nullrun/__init__.py b/src/nullrun/__init__.py index 7bc14a6..aa7bdd9 100644 --- a/src/nullrun/__init__.py +++ b/src/nullrun/__init__.py @@ -492,6 +492,26 @@ def my_agent: "WorkflowPausedException": ("nullrun.breaker.exceptions", "WorkflowPausedException"), "WorkflowKilledException": ("nullrun.breaker.exceptions", "WorkflowKilledException"), "WorkflowKilledInterrupt": ("nullrun.breaker.exceptions", "WorkflowKilledInterrupt"), + # Sibling typed name for the kill signal. Discovered via + # ``from nullrun import NullRunWorkflowKilledError``; matches + # `WorkflowKilledInterrupt` (BaseException) and the older + # `WorkflowKilledException` for back-compat. Cookbook code + # that wants a typed ``except`` clause prefers this over the + # base-interrupt form (mro-aware dispatch). The class lives at + # breaker/exceptions.py:1459. + "NullRunWorkflowKilledError": ("nullrun.breaker.exceptions", "NullRunWorkflowKilledError"), + # ── B.1 (2026-09-10): MCP umbrella + APPROVAL_DB symmetry. + # Four typed exception classes that round-trip the MCP umbrella + # codes (ADR-013, frozen-dormant) and the six APPROVAL_DB_* + # sibling codes (DEF-ARFLOW-TOOLNAME-01). Pre-B.1 these all + # collapsed to NullRunBlockedException + the generic NR-X001 + # fallback — cookbook code couldn't branch on the typed arm. + # Post-B.1 each maps to its own typed class so + # ``except NullRunMcpDestructiveBlockedError:`` etc. work. + "NullRunMcpDestructiveBlockedError": ("nullrun.breaker.exceptions", "NullRunMcpDestructiveBlockedError"), + "NullRunMcpReadonlyBypassBlockedError": ("nullrun.breaker.exceptions", "NullRunMcpReadonlyBypassBlockedError"), + "NullRunMcpApprovalRequiredError": ("nullrun.breaker.exceptions", "NullRunMcpApprovalRequiredError"), + "NullRunApprovalDbUnavailableError": ("nullrun.breaker.exceptions", "NullRunApprovalDbUnavailableError"), # User-facing message catalog (NULLRUN owns the wording; see # nullrun/messages.py for the design rationale). Eager in # spirit — these are the "give the user a chance" surface that @@ -604,6 +624,15 @@ def __dir__() -> list[str]: "NullRunBudgetError", "NullRunToolBlockedError", "WorkflowKilledInterrupt", + "NullRunWorkflowKilledError", + # B.1 (2026-09-10): MCP umbrella + APPROVAL_DB symmetry. The + # four typed exception classes are part of the curated public + # surface — cookbook code branches on them by name, so they + # need to be visible in ``dir(nullrun)`` for tab-completion. + "NullRunMcpDestructiveBlockedError", + "NullRunMcpReadonlyBypassBlockedError", + "NullRunMcpApprovalRequiredError", + "NullRunApprovalDbUnavailableError", # User-facing message catalog — the single entry point for # turning an SDK exception into a string safe to display to # end users. ``set_user_message`` lets a deployment brand its diff --git a/src/nullrun/__version__.py b/src/nullrun/__version__.py index a102a9c..b33757c 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.6" +__version__ = "0.16.7" __platform_version__ = "1.0.0" diff --git a/src/nullrun/breaker/exceptions.py b/src/nullrun/breaker/exceptions.py index 473ebf5..0963ea7 100644 --- a/src/nullrun/breaker/exceptions.py +++ b/src/nullrun/breaker/exceptions.py @@ -842,33 +842,6 @@ def __init__( self.recheck_retryable: bool = True -class NullRunBudgetThrottleError(NullRunBudgetError): - """Backend returned ``decision == "throttle"`` — soft budget signal. - - Distinct from :class:`NullRunBudgetError` (NR-B004, the hard-block - case raised when ``decision == "block"``). Throttle means - "rate-limit this workflow but don't fully block it" — a temporary - pacing signal that the SDK surfaces as a typed exception so - cookbook code can back off and retry, vs. the hard block where - the same parameters would fail again. - - Added 2026-09-08 to retire the generic ``WorkflowKilledInterrupt`` - raise on the throttle path. Cookbook pattern: catch this - specifically (``except NullRunBudgetThrottleError``), sleep for - the cooldown window, and retry — distinct from the hard block - where retrying with the same budget tier is futile. - """ - - error_code = "NR-B007" - user_action = ( - "Backend throttled this workflow (soft budget signal). Wait " - "for the cooldown window shown in the response and retry — " - "do NOT request a budget increase for a throttle (that is " - "the wrong remediation; the issue is pacing, not cap)." - ) - retryable = True - - class NullRunExecutionNotFoundError(NullRunBackendError): """``/execute`` or ``/cancel`` was called with an ``execution_id`` that has no live server-side binding. @@ -1247,6 +1220,107 @@ class NullRunApprovalToolDigestMismatchError(NullRunBlockedException): retryable = False +# ──────────────────────────────────────────────────────────────────────── +# MCP umbrella codes (ADR-013, 2026-08-14, frozen-dormant per Phase B.1) +# +# Pre-B.1 these three wire codes (``MCP_DESTRUCTIVE_BLOCKED``, +# ``MCP_READONLY_BYPASS_BLOCKED``, ``MCP_APPROVAL_REQUIRED``) all +# mapped to the base ``NullRunBlockedException`` in +# ``transport.py:_V3_ERROR_CODE_MAP`` — every cookbook handler that +# tried to branch on the typed MCP outcome silently fell through to +# the generic arm. The post-B.1 fix introduces three typed exception +# subclasses so cookbook code can ``except NullRunMcpDestructiveBlockedError:`` +# (etc.) and surface the right user_action verb. +# +# ADR-013 (2026-08-14) marks the umbrella as **frozen-dormant** — +# the underlying ``mcp_destructive_policy`` / ``mcp_readonly_bypass`` +# mechanisms are not currently wired in production but the wire codes +# are reserved and the SDK must round-trip them so a future enablement +# doesn't require SDK-side migration. +# ──────────────────────────────────────────────────────────────────────── + + +class NullRunMcpDestructiveBlockedError(NullRunBlockedException): + """Destructive MCP tool blocked by the mcp_destructive_policy umbrella. + + Wire code ``MCP_DESTRUCTIVE_BLOCKED`` (HTTP 403). ADR-013 — the + SDK maps it to a typed class so cookbook code can distinguish + destructive-MCP blocks from the generic block fallback (NR-X001) + or from the read-only bypass path (different operator-side + fix path). + """ + + error_code = "NR-MCP01" + user_action = ( + "The MCP tool's destructive capability is blocked by the " + "mcp_destructive_policy umbrella. Either remove the " + "destructive flag from the tool declaration or update the " + "workflow's policy to allow this destructive capability." + ) + retryable = False + + +class NullRunMcpReadonlyBypassBlockedError(NullRunBlockedException): + """Read-only MCP tool blocked because the bypass path is closed. + + Wire code ``MCP_READONLY_BYPASS_BLOCKED`` (HTTP 403). ADR-013 — + the SDK maps it to a typed class so cookbook code can distinguish + the readonly-bypass block (where the operator's intent was to + avoid destructive checks but the umbrella closed that path) + from the generic block fallback. + """ + + error_code = "NR-MCP02" + user_action = ( + "The MCP tool's read-only bypass path is blocked by the " + "mcp_readonly_bypass policy umbrella. The tool must go " + "through full destructive-MCP evaluation." + ) + retryable = False + + +class NullRunMcpApprovalRequiredError(NullRunBlockedException): + """MCP tool requires operator approval (NR-A010 equivalent for MCP). + + Wire code ``MCP_APPROVAL_REQUIRED`` (HTTP 403). Sibling to + :class:`NullRunApprovalNotYetApprovedError` but for the MCP + umbrella path — distinct so cookbook code can show a different + user_action hint (\"operator needs to approve the MCP tool's + capability\" vs \"operator has not yet decided on the workflow\"). + """ + + error_code = "NR-MCP03" + user_action = ( + "The MCP tool requires operator approval. Wait for the " + "operator to approve the tool's capability surface or use " + "a non-MCP equivalent." + ) + retryable = True + + +class NullRunApprovalDbUnavailableError(NullRunBlockedException): + """Approval database (Postgres) unavailable on the create-or-update path. + + Wire codes ``APPROVAL_DB_UNAVAILABLE``, ``APPROVAL_PERSISTENCE_FAILED``, + ``APPROVAL_VALIDATION_FAILED``, ``APPROVAL_CONFLICT``, + ``APPROVAL_NOT_FOUND``, ``APPROVAL_CREATE_FAILED`` (HTTP 402/403/503). + Pre-B.1 these all mapped to the base ``NullRunBlockedException`` + (transport.py:2984-2989) — operators couldn't tell apart a + transient DB outage (retryable) from a validation failure + (terminal). Post-B.1 they map to this single typed class so + cookbook code can ``except NullRunApprovalDbUnavailableError:`` + and surface the right remediation hint. + """ + + error_code = "NR-A016" + user_action = ( + "Approval database unavailable or rejected the request. " + "Retry shortly (transient DB outage) or contact support if " + "the failure persists (validation/conflict)." + ) + retryable = True + + # NOTE: NullRunApprovalReplayRejectedError was moved earlier in this # module (alongside the other five approval exceptions) so all six # typed approval exceptions are co-located. The earlier definition diff --git a/src/nullrun/decorators.py b/src/nullrun/decorators.py index bc1d926..aace8bb 100644 --- a/src/nullrun/decorators.py +++ b/src/nullrun/decorators.py @@ -808,7 +808,11 @@ def _enforce_sensitive_tool( # ADR-008: prefer `on_transport_error` (raise classified from nullrun.breaker.exceptions import ( NullRunBlockedException, + NullRunDecision, # DEF-NR-TRANSPORT-CATCHFANIN-GAP (2026-09-10): umbrella arm + NullRunExecutionNotFoundError, # DEF-NR-EX01-REWRAP-LOSS (2026-09-10): pass-through arm + NullRunInfrastructureError, # DEF-NR-TRANSPORT-CATCHFANIN-GAP (2026-09-10): umbrella arm NullRunTransportError, + RateLimitError, # DEF-NR-R001-REWRAP-LOSS (2026-09-10): pass-through arm TransportErrorSource, ) @@ -847,6 +851,43 @@ def _enforce_sensitive_tool( action_digest=action_digest_hex, tools=get_call_tools(), ) + except NullRunExecutionNotFoundError: + # DEF-NR-EX01-REWRAP-LOSS (2026-09-10): pass-through arm. + # NullRunExecutionNotFoundError IS a NullRunTransportError + # (via NullRunBackendError -> NullRunTransportError), so the + # generic arm below would rewrap it as + # NullRunBlockedException(NR-B00X) and destroy the typed + # class + NR-EX01 catalog line. Cookbook code (and + # langgraph_openai_approval_demo.py) must be able to + # ``except NullRunExecutionNotFoundError`` for the + # documented regate_required=True recovery path. Re-raise + # BEFORE the NullRunBlockedException arm so the typed + # exception propagates unchanged. + raise + except RateLimitError: + # DEF-NR-R001-REWRAP-LOSS (2026-09-10): pass-through arm. + # RateLimitError IS a NullRunTransportError (its parent + # class) raised with source=GATEWAY_ERROR on a 429 wire + # response (RATE_LIMIT_EXCEEDED). Pre-fix the generic + # ``except NullRunTransportError as exc:`` arm below + # rewrote every TransportError as + # ``NullRunBlockedException(error_code="NR-B002", + # reason="policy engine unavailable: GATEWAY_ERROR")`` — + # losing ``exc.retry_after`` (gateway's Retry-After / + # ``retry_after_ms`` body field converted to seconds), + # ``exc.upgrade_url`` (plan-upgrade URL from 429 body), + # and ``exc.body`` (parsed 429 envelope). Cookbook code + # ``except RateLimitError`` would never match because the + # rewrap stripped the typed class. The user-facing + # catalog line also lost: NR-B002 says "Our service is + # temporarily unavailable. Please try again shortly." + # when the correct NR-R001 says "The NullRun backend + # rate-limited this API key. Wait ``retry_after`` seconds + # (or upgrade the plan) before retrying." Re-raise BEFORE + # the NullRunBlockedException arm so the typed exception + # propagates with error_code=NR-R001, retry_after, + # upgrade_url, and body intact. + raise except NullRunBlockedException: # Real policy-block decision from the gateway — propagate as-is. raise @@ -894,6 +935,64 @@ def _enforce_sensitive_tool( extra={"transport_source": exc.source.value}, ) raise err from exc + except NullRunDecision: + # DEF-NR-A003-REWRAP-LOSS (2026-09-10, broader scope): + # umbrella pass-through for typed Decision subclasses that + # reach here without hitting NullRunBlockedException (this + # decorator's natural block path) or NullRunTransportError + # (the generic rewrap above). Specifically: + # - NullRunChainError (NR-CH001) — chain lifetime / + # cross-org / Execution Graph parent-lineage + # rejections. Needs exc.chain_id, + # exc.parent_execution_id, exc.backend_code preserved. + # - NullRunWorkflowInactiveError (NR-W004) — soft-deleted + # workflow. Needs exc.workflow_id preserved. + # - NullRunConsumeOverbudgetError (NR-O001) — invariant + # violation. Needs exc.execution_id, + # exc.reserved_cents, exc.max_allowed_cents, + # exc.actual_cost_cents preserved. + # - WorkflowPausedException (NR-W003) — needs + # exc.workflow_id, exc.reason, exc.resume_after. + # Pre-fix the catch-all rewrap below stamped error_code + # NR-B001 on these and discarded every first-class + # attribute, blocking the cookbook recovery path for + # each. Re-raise BEFORE the catch-all to preserve the + # typed instance. + raise + except NullRunInfrastructureError: + # DEF-NR-A003-REWRAP-LOSS (2026-09-10, broader scope): + # umbrella pass-through for typed Infrastructure + # subclasses that don't match NullRunBackendError, + # NullRunAuthenticationError, or NullRunTransportError + # above. Specifically: + # - NullRunAuthError (NR-A003) — typed 401 envelope. + # Needs exc.wire_code (API_KEY_REVOKED / + # API_KEY_EXPIRED / API_KEY_DISABLED / + # API_KEY_INVALID / API_KEY_MISSING / + # API_KEY_MALFORMED per v3.38) preserved so ops + # can branch on granular lifecycle state. The + # transport fan-in (transport.py:1294) already + # preserves this via NullRunAuthenticationError + # pass-through, but a refactor that reorders the + # transport arms would surface this here. + # - NullRunProtocolError (NR-P001) — wire-protocol + # mismatch. Needs the catalog line "Upgrade the SDK + # to a version that supports protocol + # X-NULLRUN-PROTOCOL: 4" to reach the cookbook. + # - NullRunRateLimitRedisError (NR-R002) — Redis + # outage for aggregate rate limit (fail-CLOSED). + # Needs the catalog line that distinguishes "Redis + # is down" from generic NR-B002. + # - NullRunConfigError (NR-Cxxx) — malformed config, + # typically surfaced by runtime.execute with bad + # env. Never rewrap a config error as a transient + # transport block — that's misleading. + # Pre-fix the catch-all stamped error_code NR-B001 on + # these and discarded wire_code (AuthError), + # protocol-version info (ProtocolError), and Redis + # source-of-failure (RateLimitRedisError). Re-raise + # BEFORE the catch-all. + raise except Exception as exc: # noqa: BLE001 # Any other exception is a transport / network / backend # failure. Re-raise as NullRunBlockedException so the caller diff --git a/src/nullrun/messages.py b/src/nullrun/messages.py index 4da2b5e..134cd8f 100644 --- a/src/nullrun/messages.py +++ b/src/nullrun/messages.py @@ -86,6 +86,108 @@ # defence for the case where the host code catches too broadly. "NR-A001": "There's a configuration issue. Please contact support.", "NR-A003": "There's a configuration issue. Please contact support.", + # ---- Approval lifecycle (operator decision flow) ------------------------- + # NR-A010: approval pending — operator has not yet decided. Cookbook + # contract: do NOT surface this as terminal. The ``@protect`` wrapper + # blocks via WS push until the operator resolves; if the wrapper + # surfaces it as an exception it means the host code chose to raise + # rather than wait. User-facing copy is "wait" (the only actionable + # verb) without leaking the WS / approval_id wire details. + "NR-A010": "Your request is awaiting approval. Please wait a moment while it's being reviewed.", + # NR-A011: operator denied. Terminal — re-running with the same + # approval_id fails again. Tell the user the request was not + # approved (without quoting operator-side rationale, which may + # include internal context) and invite them to submit a revised + # request. + "NR-A011": "Your request was not approved. Please review and submit a new request if you'd like to try again.", + # NR-A012: approval grant expired. Two raise paths (see + # ``NullRunApprovalExpiredError`` docstring): + # 1. Wire path — backend closed the grant because operator's + # ``expires_at`` elapsed between /gate and /execute. + # 2. Client-side timeout path — WS push went silent for + # ``approval_timeout_seconds`` without an operator decision. + # Cookbook pattern: do NOT retry the same approval_id; request a + # fresh row and re-/gate. Pre-fix (2026-09-08), the catalog was + # missing NR-A012 entirely, so ``format_user_message`` fell + # through to ``FALLBACK_MESSAGE = "Something went wrong. Please + # try again."`` — exactly what + # ``langgraph_openai_approval_demo.py`` printed, hiding the + # actionable detail. The wording below mirrors the tone rules + # (imperative when there's something to do) and tells the user + # *what to do next* (try again with a fresh approval), not just + # *what happened*. + "NR-A012": "This request was not approved in time and has expired. Please try again — the operator will be notified.", + # NR-A013: business-impact digest mismatch. The operator approved a + # different action (different amount, different target) than the one + # currently bound to the execution. User must re-request approval + # with the intended impact — the existing grant cannot be re-used. + "NR-A013": "Your request couldn't be completed because the approval was for a different action. Please request a new approval and try again.", + # NR-A014: tool capability digest mismatch. The operator approved a + # different tool capability surface than the one currently bound + # (e.g. MCP tools/list refreshed between /gate and /execute). User + # must re-/gate with the current capability surface. + "NR-A014": "Your request couldn't be completed because the available tools have changed. Please refresh and try again.", + # NR-A015: approval grant already consumed by a prior /execute + # call — replay / retry-loop signal. NOT a transient failure; the + # same approval_id will never succeed twice. Inspect retry logic. + "NR-A015": "Your request couldn't be completed because the approval has already been used. Please start a new request.", + # ---- Workflow lifecycle (server-side state) ------------------------------ + # NR-W004: workflow soft-deleted or killed on the server. Distinct + # from NR-W002 (BaseException path that bypasses ``nullrun.handle``) + # and NR-W003 (pause / cooldown). End users see this only after an + # operator terminated their session from the dashboard; the wording + # is similar to NR-W002 because the user-visible outcome is the + # same ("this service is unavailable to you"). + "NR-W004": "This service is no longer available. Please contact support if you believe this was a mistake.", + # ---- Budget sub-cases (NR-B004 is the parent hard block) ----------------- + # NR-B006: post-approval budget re-check failed. Another execution + # spent the budget between /gate and /execute. User should retry — + # the next /gate will mint a fresh reservation against the current + # available budget. + "NR-B006": "Your request couldn't be completed because the available capacity changed. Please try again.", + # NR-B007: removed 2026-09-10. NullRunBudgetThrottleError was a + # zombie class — never raised on a wire or runtime path + # (runtime.py:2116 raises WorkflowPausedException on + # decision=="throttle"). Catalog entry removed to keep + # messages in sync with the exception module. + # "NR-B007": "...", + # NR-O001: consume > reserve + ε tolerance. ADR-005 invariant; + # the SDK rejects rather than silently re-reserving. User-facing + # copy is generic because the cause is operator-side accounting; + # user should retry (a fresh /gate will recompute the reservation). + "NR-O001": "Your request couldn't be completed due to a usage accounting discrepancy. Please try again.", + # ── B.1 (2026-09-10): MCP umbrella + APPROVAL_DB typed arms. + # Three MCP umbrella codes (ADR-013, frozen-dormant) and the + # single NR-A016 typed class for the six APPROVAL_DB_* sibling + # codes. NR-A016 wording is intentionally close to the generic + # "transient service outage" cluster — the cookbook recipe for + # the typed class branches on retryable vs terminal, not on + # the specific DB cause (operators don't care whether it was a + # validation failure or a Postgres connection drop; both are + # "retry shortly, contact support if persistent"). + "NR-MCP01": "That action isn't available right now. Please contact support if you need it.", + "NR-MCP02": "That action isn't available right now. Please contact support if you need it.", + "NR-MCP03": "Your request is awaiting approval. Please wait a moment while it's being reviewed.", + "NR-A016": "Your request couldn't be completed. Please try again shortly.", + # ---- Wire / protocol ---------------------------------------------------- + # NR-P001: SDK wire-protocol version is below the backend's + # ``X-NULLRUN-PROTOCOL:`` minimum. End-user action is "contact + # support" — the host code needs an SDK upgrade, which only the + # operator / developer can perform. + "NR-P001": "This service needs an update. Please contact support.", + # ---- Chain (multi-leg conversation state) ------------------------------- + # NR-CH001: chain context invalid — chain_id is unknown, belongs to + # a different org, or exceeded max_duration. End-user outcome is + # "start a new conversation"; the chain handle cannot be revived. + "NR-CH001": "Your session was interrupted. Please start a new conversation.", + # ---- Rate limit (NR-R001 is the per-workflow soft limit) --------------- + # NR-R002: rate-limit Redis unreachable. Fail-CLOSED — the request + # is rejected because the rate limit is authoritative, not a soft + # advisory. End-user copy mirrors NR-B001 / NR-B002 (transient + # service outage) because the operator's fix is the same (restore + # Redis); the user-facing difference between "rate limit hit" and + # "rate limit Redis down" is operator-internal. + "NR-R002": "Our service is temporarily unavailable. Please try again shortly.", "NR-C000": "There's a configuration issue. Please contact support.", "NR-C001": "There's a configuration issue. Please contact support.", "NR-C004": "There's a configuration issue. Please contact support.", diff --git a/src/nullrun/runtime.py b/src/nullrun/runtime.py index f0d2e93..327ebce 100644 --- a/src/nullrun/runtime.py +++ b/src/nullrun/runtime.py @@ -2775,8 +2775,23 @@ def execute( - decision_context: Context used for the decision Mode values: - - "inline": force fast path (non-sensitive tools only) - - "strict": force gateway roundtrip + - "auto" (default): ALWAYS contacts the gateway. This + is the cloud-only invariant — budget, rate-limit, + and tool-block policies cannot be bypassed by + omitting ``mode``. Pre-v0.x SDKs silently switched + to "inline" for non-sensitive tools, which caused + DEF-TS12-01 (cycle 20260910T0515). + - "inline": explicit opt-out of /execute. Skips ALL + enforcement (budget / rate / tool-block); returns + a synthetic local allow. Use only when the caller + knows the tool is safe and wants to skip the + gateway round-trip. Cannot be combined with + sensitive tools — sensitive tools always go to + /execute even when "inline" is requested. + - "strict": explicit gateway round-trip (same + wire behaviour as "auto" post-DEF-TS12-01, but + useful for audit clarity when the caller wants + the intent on the wire). Raises: NullRunBlockedException: If decision is "block" @@ -2787,33 +2802,56 @@ def execute( workflow_id = get_workflow_id() trace_id = get_trace_id() or str(uuid.uuid4()) - # Auto-select mode: sensitive tools always use strict - # mode so /execute is consulted. The two checks below - # gate the /execute round-trip: - # 1. ``self.is_sensitive_tool(tool_name)`` — the runtime - # registry, populated by the ``@sensitive`` decorator - # at decoration time. - # 2. ``is_strict_mode_forced(tool_name)`` — the static - # ``@sensitive(impact=...)`` registered a per-tool - # extract_on call site that requires strict mode - # regardless of the runtime registry. This is the - # second source of truth, populated at decoration - # time and immune to ``init_or_die()`` reinit that - # might lose the registry on a fresh runtime singleton. + # Auto-select mode: ``mode="auto"`` (the default) ALWAYS + # resolves to "strict" so the /execute endpoint is consulted + # for every tool call. This is the cloud-only invariant + # documented in CLAUDE.md §17 and memory `cloud-only-invariant-sdk`: + # budget enforcement, rate limiting, and tool_block policies + # cannot be silently bypassed because the SDK caller used the + # default ``mode="auto"``. + # + # Pre-fix (DEF-TS12-01, cycle 20260910T0515): ``mode="auto"`` + # with a non-sensitive tool resolved to ``mode="inline"`` which + # returned a synthetic local allow WITHOUT contacting the + # gateway. Every operator-configured budget, rate-limit, and + # tool-block policy was silently bypassed for non-sensitive + # tools. The dashboard showed policies in effect; the SDK + # ignored them. This is now fixed: ``mode="auto"`` → + # ``"strict"`` unconditionally. + # + # Explicit opt-out paths (preserved unchanged): + # 1. ``mode="inline"`` (explicit opt-in by the caller) — + # returns the local allow WITHOUT contacting the gateway. + # Documented as the only way to skip /execute. Use + # sparingly: skips ALL enforcement, not just budget. + # 2. ``mode="strict"`` (explicit opt-in by the caller) — + # forces /execute round-trip regardless of tool + # sensitivity. Identical wire behaviour to ``"auto"`` + # post-fix, but useful when the caller wants the + # intent on the wire for audit clarity. + # + # The two sensitivity checks below still gate the inline + # fast-path — sensitive tools cannot be silently skipped + # even if the caller explicitly asks for ``mode="inline"``. + # They also gate the /execute round-trip when ``mode="auto"`` + # resolved to ``"strict"`` (no behavioural change there). if mode == "auto": - if self.is_sensitive_tool(tool_name) or is_strict_mode_forced(tool_name): - mode = "strict" - else: - mode = "inline" + mode = "strict" - # For inline mode with non-sensitive tools, skip execute and use local enforcement + # For inline mode with non-sensitive tools, skip execute and use local enforcement. + # Sensitive tools always go through /execute even when the + # caller asked for ``mode="inline"`` — fail-CLOSED stance per + # memory `sensitive-tool-fail-closed`. if mode == "inline" and not ( self.is_sensitive_tool(tool_name) or is_strict_mode_forced(tool_name) ): return { "decision": "allow", "decision_source": DecisionSource.LOCAL, - "explanation": "Inline mode: local enforcement only", + "explanation": ( + "Inline mode: local enforcement only. Caller explicitly opted " + "out of /execute — budget / rate / tool-block policies bypassed." + ), "policy_hash": None, "allow_execution": True, } @@ -3019,73 +3057,15 @@ def execute( # # The backend stamps a structured ``details.error_code`` # on every block response, alongside the existing - # BUDGET_* / RATE_LIMIT_* family. When the backend - # provides one, we use it verbatim -- no string - # parsing, no false positives. Falls back to the - # legacy keyword-on-explanation mapping for older - # backends that pre-date the structured code (the - # keyword path stays for back-compat -- an older - # SDK still classifies budget/loop/rate/tool blocks - # correctly). - explanation = result.get("explanation", "policy violation") - wire_details = result.get("details") or {} - if not isinstance(wire_details, dict): - wire_details = {} - wire_error_code = wire_details.get("error_code") - if wire_error_code and isinstance(wire_error_code, str): - # Backend-supplied structured code wins. The - # catalogue exception class is mapped via - # ``_V3_ERROR_CODE_MAP`` on the transport path; on - # this /execute path we only have the SCREAMING_SNAKE - # backend code, so we surface it as-is in the - # ``error_code`` slot and let the caller branch on - # the catalog subclass if it has imported one. The - # block_code -> SDK exception-class mapping is done - # via the catalogue in nullrun.breaker.exceptions. - block_code, block_action = wire_error_code, "block" - block_cls = "NullRunBlockedException" - else: - explanation_lower = explanation.lower() - if "budget" in explanation_lower or "exhausted" in explanation_lower: - block_code, block_action = "NR-B004", "block" - block_cls = "NullRunBudgetError" - elif "loop" in explanation_lower or "repetition" in explanation_lower: - block_code, block_action = "NR-L001", "block" - block_cls = "NullRunBlockedException" - elif "rate" in explanation_lower or "too many" in explanation_lower: - block_code, block_action = "NR-R001", "block" - block_cls = "NullRunBlockedException" - elif "tool" in explanation_lower and "block" in explanation_lower: - block_code, block_action = "NR-T001", "block" - block_cls = "NullRunToolBlockedError" - else: - block_code, block_action = "NR-X001", "block" - block_cls = "NullRunBlockedException" - # Note: we still raise the base ``NullRunBlockedException`` - # for non-budget/tool cases to keep the construction - # shape simple — the catalogue code is what the user - # reads, and they can branch on it via ``except - # NullRunBudgetError:`` for the budget case if they need - # to handle it specifically. We could instantiate the - # subclass per branch above; keeping one raise here is - # easier to reason about and matches the way the rest of - # the codebase handles backend blocks. - # - # ``details`` carries the wire ``details`` payload so the - # caller can introspect ``exc.details["error_code"]`` and - # ``exc.details["decision_source"]`` for diagnostic - # routing. ``mapped_class`` is preserved as a backwards- - # compat shim for callers that branched on the keyword - # path; new code should branch on ``exc.error_code``. - merged_details = dict(wire_details) - merged_details["mapped_class"] = block_cls - err = NullRunBlockedException( + # BUDGET_* / RATE_LIMIT_* family. Layer-1 dispatch is + # factored into ``_build_block_exception`` so the + # dispatch logic is unit-testable without standing up a + # full ``Runtime.execute()`` pipeline (see + # tests/test_2026_09_10_runtime_block_typed_dispatch.py). + err = self._build_block_exception( + result=result, workflow_id=workflow_id or UNKNOWN_WORKFLOW_ID, - reason=explanation, - action=block_action, tool_name=tool_name, - error_code=block_code, - details=merged_details, ) # Layer 2: fire the on_error hook. The hook sees the # same exception the caller will catch plus the @@ -3104,6 +3084,203 @@ def execute( metrics.inc_runtime("execute_allowed") return result + @staticmethod + def _build_block_exception( + *, + result: dict[str, Any], + workflow_id: str, + tool_name: str | None, + ) -> Exception: + """Build the typed catalog exception for a /execute block + decision. Factored out of ``Runtime.execute`` so the dispatch + logic is unit-testable without a full transport pipeline + (see tests/test_2026_09_10_runtime_block_typed_dispatch.py). + + Dispatch priority: + + 1. ``result["details"]["error_code"]`` is in + ``_V3_ERROR_CODE_MAP`` → instantiate the typed catalog + class (e.g. ``NullRunApprovalReplayRejectedError`` for + ``APPROVAL_REPLAY_REJECTED``). This is the common path + and is what cookbook recipes branch on + (``except NullRunApprovalReplayRejectedError:`` for + NR-A015, etc.). + + 2. Wire code present but NOT in the catalog (backend / SDK + drift) → ``NullRunBlockedException`` with + ``error_code=`` so the caller still has + something to branch on via ``exc.error_code``. + + 3. No structured wire code (legacy backends) → keyword-on- + ``explanation`` mapping for BUDGET / LOOP / RATE / TOOL + families. Each branch sets a synthetic + ``NR-B004 / NR-L001 / NR-R001 / NR-T001`` catalog code + on ``NullRunBlockedException`` (kept for backward + compat — old code branched on + ``exc.error_code == "NR-B004"``). + + The class attribute ``error_code`` on the typed class + (e.g. ``NullRunApprovalReplayRejectedError.error_code == + "NR-A015"``) is the canonical ``format_user_message`` + lookup key — we NEVER pass ``error_code=wire_code`` to the + typed class because that would override the catalog code + and defeat ``format_user_message``. + + Returns a ``NullRunBlockedException`` (or subclass) — + caller is responsible for raising / ``on_error`` hook / + metric increment. + """ + from nullrun.transport import _V3_ERROR_CODE_MAP + + explanation = result.get("explanation", "policy violation") + wire_details = result.get("details") or {} + if not isinstance(wire_details, dict): + wire_details = {} + wire_error_code = wire_details.get("error_code") + + # Catalog classes with a custom ``__init__`` that promotes + # a wire field to a first-class attribute (e.g. + # ``exc.approval_id`` on ``NullRunApprovalReplayRejectedError``, + # ``exc.current_spend_cents`` on + # ``NullRunBudgetRecheckFailedError``). Mirrors the + # keyword-only params declared on those classes in + # ``breaker/exceptions.py``. The value is a frozenset of + # wire-field names that must be passed as named kwargs to + # the constructor (NOT nested under ``details=``) so they + # land on the typed exception as first-class attrs. + # + # Classes that map to ``NullRunBlockedException`` (the base + # class — e.g. ``NullRunBudgetError``, + # ``NullRunToolBlockedError``) do NOT appear here because + # they have no custom ``__init__`` that promotes fields; all + # wire fields stay in the ``details=`` payload. + _TYPED_KWARGS_BY_CLASS: dict[str, frozenset[str]] = { + "NullRunBudgetRecheckFailedError": frozenset( + {"current_spend_cents", "budget_cents", "epsilon_cents"} + ), + "NullRunApprovalNotYetApprovedError": frozenset({"approval_id"}), + "NullRunApprovalDeniedError": frozenset( + {"approval_id", "denial_note"} + ), + "NullRunApprovalExpiredError": frozenset( + {"approval_id", "timeout_seconds"} + ), + "NullRunApprovalReplayRejectedError": frozenset({"approval_id"}), + "NullRunApprovalDigestMismatchError": frozenset({"approval_id"}), + "NullRunApprovalToolDigestMismatchError": frozenset({"approval_id"}), + } + + def _build_payload( + src: dict[str, Any], mapped_name: str + ) -> dict[str, Any]: + """Build the ``details=...`` payload dict that the + constructor captures as ``self.details["details"]``. + + Carries the full wire payload verbatim (including + ``error_code``) so callers can introspect the wire + shape for routing/alerting (e.g. + ``exc.details["details"]["decision_source"]``). The + ``mapped_class`` shim is appended for back-compat with + callers that branched on the legacy keyword path. + """ + payload = dict(src) + payload["mapped_class"] = mapped_name + return payload + + # Priority 1: typed catalog dispatch via _V3_ERROR_CODE_MAP. + if wire_error_code and isinstance(wire_error_code, str): + typed_cls = _V3_ERROR_CODE_MAP.get(wire_error_code) + if typed_cls is not None: + # 1a: typed SUBCLASS (e.g. + # NullRunApprovalReplayRejectedError for + # APPROVAL_REPLAY_REJECTED). The class's class + # attribute owns error_code (NR-A015), so we MUST + # NOT pass error_code=... — that would override + # NR-A015 with the wire code and defeat + # format_user_message's catalog lookup. + if typed_cls is not NullRunBlockedException: + payload = _build_payload( + wire_details, typed_cls.__name__ + ) + typed_kwarg_names = _TYPED_KWARGS_BY_CLASS.get( + typed_cls.__name__, frozenset() + ) + typed_kwargs = { + k: wire_details[k] + for k in typed_kwarg_names + if k in wire_details + } + return typed_cls( + workflow_id=workflow_id, + reason=explanation, + action="block", + tool_name=tool_name, + details=payload, + **typed_kwargs, + ) + # 1b: catalog maps to base NullRunBlockedException + # (e.g. APPROVAL_VALIDATION_FAILED). There is no + # catalog error_code class attr to protect — we + # pass the wire code explicitly so self.error_code + # reflects the wire code (back-compat callers + # branch on exc.error_code == "APPROVAL_*"). + payload = _build_payload( + wire_details, "NullRunBlockedException" + ) + return NullRunBlockedException( + workflow_id=workflow_id, + reason=explanation, + action="block", + tool_name=tool_name, + error_code=wire_error_code, + details=payload, + ) + # Priority 2: wire code NOT in catalog (drift). + # Base class — wire code IS self.error_code (no catalog + # entry to dispatch to, so no class attr to protect). + payload = _build_payload( + wire_details, "NullRunBlockedException" + ) + return NullRunBlockedException( + workflow_id=workflow_id, + reason=explanation, + action="block", + tool_name=tool_name, + error_code=wire_error_code, + details=payload, + ) + + # Priority 3: legacy keyword-on-explanation mapping for + # backends that pre-date the structured wire code. Each + # branch picks a synthetic catalog code so legacy + # ``exc.error_code == "NR-B004"``-style branching still + # works for back-compat callers. + explanation_lower = explanation.lower() + if "budget" in explanation_lower or "exhausted" in explanation_lower: + block_code = "NR-B004" + mapped = "NullRunBudgetError" + elif "loop" in explanation_lower or "repetition" in explanation_lower: + block_code = "NR-L001" + mapped = "NullRunBlockedException" + elif "rate" in explanation_lower or "too many" in explanation_lower: + block_code = "NR-R001" + mapped = "NullRunBlockedException" + elif "tool" in explanation_lower and "block" in explanation_lower: + block_code = "NR-T001" + mapped = "NullRunToolBlockedError" + else: + block_code = "NR-X001" + mapped = "NullRunBlockedException" + payload = _build_payload(wire_details, mapped) + return NullRunBlockedException( + workflow_id=workflow_id, + reason=explanation, + action="block", + tool_name=tool_name, + error_code=block_code, + details=payload, + ) + def _enrich_event(self, event: dict[str, Any]) -> dict[str, Any]: """Add context fields to event.""" enriched = dict(event) # Don't modify original diff --git a/src/nullrun/transport.py b/src/nullrun/transport.py index 62d73c8..ebc8602 100644 --- a/src/nullrun/transport.py +++ b/src/nullrun/transport.py @@ -28,8 +28,17 @@ from nullrun.breaker.exceptions import ( BreakerTransportError, InsecureTransportError, + NullRunApprovalDbUnavailableError, + NullRunApprovalReplayRejectedError, NullRunAuthenticationError, + NullRunBackendError, + NullRunBlockedException, + NullRunDecision, NullRunExecutionNotFoundError, + NullRunInfrastructureError, + NullRunMcpApprovalRequiredError, + NullRunMcpDestructiveBlockedError, + NullRunMcpReadonlyBypassBlockedError, NullRunTransportError, RateLimitError, TransportErrorSource, @@ -1222,7 +1231,151 @@ def do_execute_request() -> httpx.Response: # 0.7.0 thin client: no local policy cache. The next return data # type: ignore[no-any-return] elif response.status_code >= 400: - # 4xx - don't retry, return block + # 4xx — don't retry. + # + # 2026-09-10 (NR-SDK-A015-SURFACE): before the fix, + # this branch dropped the wire envelope on the floor + # and synthesised a generic ``{"decision": "block", + # "explanation": "Gateway returned 409"}`` dict. That + # hid every wire-coded reason (`APPROVAL_REPLAY_REJECTED`, + # `APPROVAL_DENIED`, `BUDGET_HARD_BLOCKED`, etc.) behind + # a single string, so the runtime block dispatch fell + # through to ``NR-X001`` and `format_user_message` + # produced the catalogue fallback ("Something went + # wrong. Please try again.") instead of the typed + # `NR-A015` message. Cookbook callers had no way to + # branch on the precise cause. + # + # Post-fix: parse the envelope via the existing + # `_parse_v3_error_envelope` helper — it covers the + # v3 wire envelope for every /execute reject reason, + # including the six typed approval grant-consume + # outcomes (`APPROVAL_NOT_YET_APPROVED` → + # ``NullRunApprovalNotYetApprovedError`` (NR-A010), + # `APPROVAL_DENIED` → NR-A011, + # `APPROVAL_EXPIRED` → NR-A012, + # `APPROVAL_DIGEST_MISMATCH` → NR-A013, + # `APPROVAL_TOOL_DIGEST_MISMATCH` → NR-A014, + # `APPROVAL_REPLAY_REJECTED` → NR-A015 / `` + # NullRunApprovalReplayRejectedError``) — and raise + # the typed exception so the @protect / + # @sensitive / runtime.execute() exception arms + # propagate the right class up to the caller. + # + # Fall through to the synthetic block shape if the + # envelope is unrecognised (plaintext body, malformed + # JSON, unknown wire code) so behaviour stays + # backwards-compatible for legacy / non-v3 backends. + # `_parse_v3_error_envelope` always returns an + # Exception — it never silently swallows a 4xx. + try: + raise _parse_v3_error_envelope(response, "execute") + except NullRunApprovalReplayRejectedError as exc: + # The exact case the user reported: the operator + # approved, the SDK polled /execute again, and + # the backend's atomic consume_approved UPDATE + # returned zero rows (replay race — UI approve + # vs SDK re-check). Surface the typed exception + # so `format_user_message` yields the NR-A015 + # catalogue line ("Your request couldn't be + # completed because the approval has already + # been used. Please start a new request.") + # instead of the fallback. + metrics.inc_transport("execute_block_replay_rejected") + raise + except NullRunBlockedException as exc: + # All other typed blocks from the dispatch — + # budget, rate, tool, approval-deny, etc. + # Re-raise for the @protect / runtime.execute + # arms to handle. + metrics.inc_transport("execute_block_typed") + raise + except NullRunBackendError as exc: + # 5xx-classified envelope parsed as a typed + # backend error (shouldn't normally land here + # because the helper maps 5xx to GATEWAY_ERROR + # via NullRunTransportError, but stays + # defensive). Re-raise. + raise + except NullRunAuthenticationError as exc: + # 401 envelope parsed as auth error — surface + # directly so the caller can react. + raise + except NullRunTransportError as exc: + # Transport-classified (network, breaker) — not + # a real 4xx, but helper may return one if the + # envelope shape is ambiguous. Re-raise so the + # on_transport_error arm sees it. + raise + except NullRunDecision as exc: + # DEF-NR-TRANSPORT-CATCHFANIN-GAP (2026-09-10): + # umbrella pass-through for typed Decision + # subclasses NOT in the NullRunBlockedException + # MRO. Specifically: + # - NullRunChainError (NR-CH001) — chain + # lifetime / cross-org / Execution Graph + # parent-lineage rejections + # - NullRunWorkflowInactiveError (NR-W004) — + # soft-deleted workflow + # - NullRunConsumeOverbudgetError (NR-O001) — + # CONSUME > RESERVE + epsilon_cents invariant + # - WorkflowPausedException (NR-W003) + # Pre-fix these fell through to `except + # Exception: pass` below and got silently + # swallowed into the synthetic block shape + # (`{"decision": "block", "decision_source": + # FALLBACK, "explanation": f"Gateway returned + # {response.status_code}"}`) — losing + # exc.chain_id / exc.parent_execution_id (Chain), + # exc.workflow_id (WorkflowInactive), + # exc.execution_id / exc.reserved_cents / + # exc.actual_cost_cents / exc.epsilon_cents + # (ConsumeOverbudget), and every typed + # `error_code`/user-action. MUST come AFTER the + # NullRunBlockedException arm above so the typed + # approval / budget / tool-block path still + # matches by MRO specificity. + metrics.inc_transport("execute_block_decision_typed") + raise + except NullRunInfrastructureError as exc: + # DEF-NR-TRANSPORT-CATCHFANIN-GAP (2026-09-10): + # umbrella pass-through for typed + # Infrastructure subclasses NOT in the + # NullRunBackendError / NullRunAuthenticationError / + # NullRunTransportError MRO branches above. + # Specifically: + # - NullRunProtocolError (NR-P001) — + # PROTOCOL_TOO_OLD / PROTOCOL_TOO_NEW / + # PROTOCOL_HEADER_INVALID / + # PROTOCOL_HEADER_REQUIRED + # - NullRunRateLimitRedisError (NR-R002) — + # RATE_LIMIT_REDIS_UNAVAILABLE + # - NullRunConfigError (NR-Cxxx) — when raised + # from a wire envelope (rare; mostly SDK-side) + # NullRunAuthError (NR-A003) IS in the + # NullRunAuthenticationError arm above (parent + # class match), but listing here for completeness + # preserves the documented recovery contract + # even if a future refactor reorders the prior + # arms. + # Pre-fix these fell through to `except Exception: + # pass` below — same synthetic-block loss as the + # Decision path. MUST come AFTER the three + # specific parent arms above (Backend, Auth, + # Transport) so the wire-classified exceptions + # still match by MRO specificity. + metrics.inc_transport("execute_block_infra_typed") + raise + except Exception: + # Unrecognised envelope (plaintext body, legacy + # slug, malformed JSON). Fall through to the + # synthetic block shape so old / non-v3 backends + # keep working and ``on_transport_error="raise"`` + # callers still see a usable dict. The retry + # helper has already given up; emitting a typed + # exception here would mask unknown wire codes + # the user hasn't yet catalogued. + pass return { "decision": "block", "decision_source": DecisionSource.FALLBACK, @@ -1416,9 +1569,58 @@ def _do_gate_post() -> httpx.Response: if response.status_code == 200: return response.json() # type: ignore[no-any-return] - # 4xx always -> synthetic block (real gate decision, - # never retried by ``_retry_with_backoff``). 5xx after - # retry exhaustion -> synthetic block (legacy + # 4xx is a REAL gate decision — surface it through the + # existing block / throttle / soft_pass dispatch in + # runtime.check_workflow_budget (lines ~2089-2150). + # Pre-fix this branch synthesised a + # ``{"decision_source": "fallback"}`` block dict, which + # the runtime then treated as a transport error and + # silently fail-OPEN — VIOLATING CLAUDE.md §4 + # fail-CLOSED invariant. Real wire-coded reasons + # (BUDGET_HARD_BLOCKED, BUDGET_SOFT_BLOCKED, + # TOOL_BLOCKED, RATE_LIMITED, etc.) were all dropped on + # the floor. + # + # 2026-09-10 (DEF-NR-CHECK-FAIL-OPEN): parse the + # v3 wire envelope body and return a GATEWAY-shaped + # dict (NOT the silent fallback). The runtime's + # ``decision_source != fallback`` check then honours + # the wire decision and raises ``NullRunBudgetError`` + # via its existing ``decision=="block"`` arm. The + # wire ``error_code`` / ``explanation`` / + # ``policy_id`` / ``details`` are preserved so the + # catalogue formatter can produce an actionable message. + if 400 <= response.status_code < 500: + try: + wire_body = response.json() + except Exception: + wire_body = {} + explanations = wire_body.get("explanations") or [] + if not explanations: + single = ( + wire_body.get("explanation") + or wire_body.get("error_message") + ) + if single: + explanations = [single] + if not explanations: + explanations = [f"Gate endpoint returned {response.status_code}"] + return { + "decision": "block", + "decision_source": DecisionSource.GATEWAY, + "explanation": explanations[0], + "explanations": explanations, + "reservation_id": wire_body.get("reservation_id"), + "remaining_budget_cents": wire_body.get("remaining_budget_cents") or 0, + "projected_cost_cents": wire_body.get("projected_cost_cents") or 0, + "policy_id": wire_body.get("policy_id"), + "policy_version": wire_body.get("policy_version"), + "operation_id": wire_body.get("operation_id"), + "details": wire_body.get("details") or {}, + "error_code": wire_body.get("error_code"), + "status_code": response.status_code, + } + # 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 @@ -2364,7 +2566,13 @@ def _safe_json(response: httpx.Response, endpoint: str) -> Any: f"(status={response.status_code}): {type(exc).__name__}", source=TransportErrorSource.GATEWAY_ERROR, endpoint=endpoint, - error_code="NR-T001", + # NR-T001 collides with NullRunToolBlockedError's + # canonical code (breaker/exceptions.py:955); using + # NR-T-PARSE here so a cookbook handler that branches + # on `exc.error_code == "NR-T001"` does not mis-classify + # a JSON parse failure as a tool block. See + # tests/test_2026_08_11_fixes.py for the pin. + error_code="NR-T-PARSE", ) from exc @@ -2398,12 +2606,16 @@ def _parse_v3_error_envelope( NullRunApprovalToolDigestMismatchError, NullRunAuthError, NullRunBackendError, + NullRunBlockedException, NullRunBudgetError, NullRunBudgetRecheckFailedError, NullRunChainError, NullRunConsumeOverbudgetError, + NullRunDecision, + NullRunInfrastructureError, NullRunProtocolError, NullRunRateLimitRedisError, + NullRunToolBlockedError, NullRunWorkflowInactiveError, RateLimitError, ) @@ -2628,6 +2840,36 @@ def _parse_v3_error_envelope( # as BaseException (the helper declares -> Exception). allowed = {"error_code", "user_action", "retryable", "docs_url", "cause"} forwarded = {k: v for k, v in details.items() if k in allowed} + if ( + catalog is NullRunToolBlockedError + or catalog is NullRunBlockedException + ): + # DEF-NR-TOOLBLOCKED-PARSER (2026-09-10): NullRunBlockedException + # subclasses require positional ``workflow_id`` + ``reason`` + # (no defaults), so the generic ``catalog(full_message, ...)`` + # fallback below raises TypeError when given a string for + # ``workflow_id``. Affects 7 catalog entries: TOOL_BLOCKED, + # LOOP_DETECTED, MODEL_REQUIRED, POLICY_UNCONFIGURED, + # TOO_MANY_PENDING_APPROVALS, BUSINESS_IMPACT_INVALID, + # VALIDATION_FAILED. Pre-fix the TypeError escaped the parser + # and got swallowed by the catch-all ``except Exception: pass`` + # in Transport.execute, surfacing the synthetic-block dict + # ``{"decision": "block", "explanation": "Gateway returned + # 403"}`` instead of the typed NR-T001 / NR-Lxxx catalog line. + # ``tool_name`` is forwarded for NullRunToolBlockedError + # (the only BlockedException subclass that surfaces it on the + # wire envelope); the parent constructor drops it for plain + # NullRunBlockedException so it's a no-op there. ``forwarded`` + # (error_code / user_action / retryable / docs_url / cause) is + # passed through so the catalog value's defaults win. + instance = catalog( # type: ignore[call-arg] + workflow_id=str(details.get("workflow_id") or "unknown"), + reason=full_message, + status_code=status, + tool_name=details.get("tool_name"), + **forwarded, + ) + return cast(Exception, instance) instance = catalog(full_message, **forwarded) # type: ignore[call-arg] return cast(Exception, instance) @@ -2704,7 +2946,13 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: "BUDGET_SOFT_BLOCKED": NullRunBudgetError, "BUDGET_OVERDRAFT_EXCEEDED": NullRunBudgetError, "BUDGET_PERIOD_NOT_STARTED": NullRunBudgetError, - "REDIS_UNAVAILABLE": NullRunBudgetError, + # Note: BUDGET_REDIS_UNAVAILABLE and RATE_LIMIT_REDIS_UNAVAILABLE + # below are the canonical redis-down codes (post-v3.36 rename); + # the legacy ``REDIS_UNAVAILABLE`` slug was removed 2026-09-10 + # because the backend never emits it (it is absent from + # ``GateErrorCode::all()`` in error_codes.rs). A cookbook that + # extends this map with the legacy slug risks silently matching + # nothing, so the slot stays unoccupied by design. # 402 — chain family (separate class for diagnostic clarity) "CHAIN_MAX_DURATION_EXCEEDED": NullRunChainError, # 403 — chain security + workflow state @@ -2737,12 +2985,18 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: "RATE_LIMIT_REDIS_UNAVAILABLE": NullRunRateLimitRedisError, "BUDGET_DATA_UNAVAILABLE": NullRunBackendError, # 402 — approval-create failure family (DEF-ARFLOW-TOOLNAME-01, - "APPROVAL_DB_UNAVAILABLE": NullRunBlockedException, - "APPROVAL_PERSISTENCE_FAILED": NullRunBlockedException, - "APPROVAL_VALIDATION_FAILED": NullRunBlockedException, - "APPROVAL_CONFLICT": NullRunBlockedException, - "APPROVAL_NOT_FOUND": NullRunBlockedException, - "APPROVAL_CREATE_FAILED": NullRunBlockedException, + # B.1 symmetry fix 2026-09-10): six sibling codes all map to + # the typed ``NullRunApprovalDbUnavailableError`` (NR-A016) so + # cookbook code can branch on the typed class instead of + # falling through to the base NullRunBlockedException. Pre-B.1 + # all six collapsed to the base class — operators couldn't tell + # apart a transient DB outage from a validation failure. + "APPROVAL_DB_UNAVAILABLE": NullRunApprovalDbUnavailableError, + "APPROVAL_PERSISTENCE_FAILED": NullRunApprovalDbUnavailableError, + "APPROVAL_VALIDATION_FAILED": NullRunApprovalDbUnavailableError, + "APPROVAL_CONFLICT": NullRunApprovalDbUnavailableError, + "APPROVAL_NOT_FOUND": NullRunApprovalDbUnavailableError, + "APPROVAL_CREATE_FAILED": NullRunApprovalDbUnavailableError, # 403 — approval grant-consume outcomes (v3.53 / 2026-08-13 # audit, A-1+A-2 bundle). Distinct from the /gate # create-failure family above: these are the seven @@ -2813,6 +3067,19 @@ def _build_v3_error_code_map() -> dict[str, type[Exception]]: "TOO_MANY_PENDING_APPROVALS": NullRunBlockedException, "BUSINESS_IMPACT_INVALID": NullRunBlockedException, "VALIDATION_FAILED": NullRunBlockedException, + # ── MCP umbrella codes (ADR-013, 2026-08-14, frozen-dormant) + # B.1 (2026-09-10): the three umbrella codes map to typed + # ``NullRunMcp*Error`` subclasses so cookbook code can branch + # on the precise umbrella path. Pre-B.1 these collapsed to + # the generic NullRunBlockedException / NR-X001 fallback — + # operators couldn't distinguish the destructive-MCP block + # from the readonly-bypass block from the approval-required + # path. ADR-013 marked the umbrella frozen-dormant: wire + # codes are reserved and the SDK must round-trip them, but + # the underlying mechanisms aren't wired in production yet. + "MCP_DESTRUCTIVE_BLOCKED": NullRunMcpDestructiveBlockedError, + "MCP_READONLY_BYPASS_BLOCKED": NullRunMcpReadonlyBypassBlockedError, + "MCP_APPROVAL_REQUIRED": NullRunMcpApprovalRequiredError, # Wire-level parsing failures (missing / malformed fields). # Map to ``NullRunBackendError`` because the SDK treats them # as infrastructure-side issues — the server should have diff --git a/tests/test_2026_08_11_fixes.py b/tests/test_2026_08_11_fixes.py index 6434436..1fee406 100644 --- a/tests/test_2026_08_11_fixes.py +++ b/tests/test_2026_08_11_fixes.py @@ -135,11 +135,17 @@ def test_safe_json_helper_exists_and_wraps_json_errors(): "transport.py must define _safe_json(response, endpoint) " "helper to wrap JSON parse failures" ) - # The helper must raise NullRunTransportError with NR-T001 - # (consistent with the rest of the SDK's error_code vocabulary) - assert 'error_code="NR-T001"' in src, ( + # The helper must raise NullRunTransportError with NR-T-PARSE. + # NR-T001 (tool-block) was the historical literal here, but it + # collides with NullRunToolBlockedError.error_code + # (breaker/exceptions.py:955); cookbook handlers that branch on + # `exc.error_code == "NR-T001"` would mis-classify a JSON parse + # failure as a tool block. NR-T-PARSE is the new dedicated + # transport-class code; matches NR-T (transport) vocabulary. + assert 'error_code="NR-T-PARSE"' in src, ( "_safe_json must raise NullRunTransportError with " - "error_code=NR-T001 (consistent with NR-A/NR-B vocabulary)" + "error_code=NR-T-PARSE (avoids collision with NR-T001 / " + "NullRunToolBlockedError)" ) # body_preview truncation is part of the fix; the helper # must slice body to 200 chars max. diff --git a/tests/test_2026_09_10_catchfanin_passthrough.py b/tests/test_2026_09_10_catchfanin_passthrough.py new file mode 100644 index 0000000..00dcce2 --- /dev/null +++ b/tests/test_2026_09_10_catchfanin_passthrough.py @@ -0,0 +1,560 @@ +"""DEF-NR-TRANSPORT-CATCHFANIN-GAP (2026-09-10) — typed Decision and +Infrastructure subclasses parsed by ``_parse_v3_error_envelope`` must +propagate through ``Transport.execute``'s 4xx catch-fan-in, not be +silently swallowed into a synthetic ``{"decision": "block", +"decision_source": FALLBACK, ...}`` dict. + +Pre-fix (audit 2026-09-10): + - ``nullrun/transport.py::Transport.execute`` had a 4xx handler + with five except arms that re-raised typed subclasses: + NullRunApprovalReplayRejectedError, + NullRunBlockedException, NullRunBackendError, + NullRunAuthenticationError, NullRunTransportError. + - Any other typed exception raised by + ``_parse_v3_error_envelope`` (specifically: + NullRunProtocolError, NullRunRateLimitRedisError, + NullRunChainError, NullRunWorkflowInactiveError, + NullRunConsumeOverbudgetError) fell through to + ``except Exception: pass`` and was replaced with the synthetic + block shape `{"decision": "block", "decision_source": + FALLBACK, "explanation": f"Gateway returned {status_code}"}`. + - User-visible symptom: a user calling /execute with a wire code + of CHAIN_ORG_MISMATCH (NR-CH001) got back a synthetic block + dict with no error_code, no chain_id, no diagnostic — the + cookbook code that expected an except NullRunChainError path + to fire never saw it; runtime.execute returned a dict instead + of raising. + +Post-fix: + - Two umbrella pass-through arms added BEFORE the + ``except Exception: pass`` fallback: + ``except NullRunDecision: raise`` (covers + NullRunChainError, NullRunWorkflowInactiveError, + NullRunConsumeOverbudgetError, WorkflowPausedException) and + ``except NullRunInfrastructureError: raise`` (covers + NullRunProtocolError, NullRunRateLimitRedisError, + NullRunConfigError; NullRunAuthError is also covered in + addition to its existing NullRunAuthenticationError parent + arm, which keeps the documented recovery contract intact + even if a future refactor reorders the prior arms). + +These tests pin BOTH the source shape (the two arms are present, +in the right order — after the specific Arms, before +``except Exception:`` — and match the catalog of typed exceptions +that were previously lost) AND the runtime behavior (each wire +code yields the matching typed subclass instance with first-class +attrs preserved, instead of a synthetic block dict). +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import httpx +import pytest +import respx + +from nullrun.breaker.exceptions import ( + NullRunAuthError, + NullRunBackendError, + NullRunChainError, + NullRunConsumeOverbudgetError, + NullRunProtocolError, + NullRunRateLimitRedisError, + NullRunWorkflowInactiveError, +) +from nullrun.transport import Transport + +SDK_ROOT = Path(__file__).resolve().parent.parent +TRANSPORT_PY = SDK_ROOT / "src" / "nullrun" / "transport.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _execute_body() -> str: + """Return the source of ``Transport.execute`` so source-pin tests + can grep for the expected arms / ordering without depending on + Python AST parsing. + + The signature is multiline + (``def execute(\n self,\n organization_id: ...``) + so we anchor on ``def execute(`` and walk forward to the next + top-level ``def`` (4-space indent) inside the same class.""" + src = _read(TRANSPORT_PY) + start = src.find(" def execute(\n") + assert start != -1, "could not locate Transport.execute header" + after_header = src.index(" def execute(\n", start) + len(" def execute(\n") + # Walk forward from after_header; we're inside a class (4-space + # indent). The next sibling ``def`` or ``@`` decorator at 4-space + # indent terminates execute. + m = re.search( + r"^ (?:def |@|class )", + src[after_header:], + re.MULTILINE, + ) + assert m, "could not locate end of Transport.execute body" + end = after_header + m.start() + return src[start:end] + + +def _v3_envelope(error_code: str, status: int = 400, **details) -> httpx.Response: + """Build a v3-shaped 4xx response envelope that exercises the + catch-fan-in. + + The canonical v3 envelope uses ``error_code`` (NOT ``error`` — + that's the legacy slug shape, which has weaker details + semantics and would silently drop our first-class attrs). + Fields passed as ``**details`` go under ``details`` so the + parser's per-class dispatchers can read them off the typed + exception (`chain_id`, ``workflow_id``, ``reserved_cents``, + etc.).""" + body = { + "error_code": error_code, + "error_message": f"Backend says {error_code}", + "details": details, + } + return httpx.Response(status, json=body) + + +# Wire endpoints we exercise — same shape as the real backend. +_EXECUTE_URL = "https://api.test.nullrun.io/api/v1/execute" + + +@pytest.fixture +def transport(): + t = Transport( + api_url="https://api.test.nullrun.io", + api_key="test-key-12345678", + ) + yield t + t.stop() + + +def _execute_kwargs(): + """Standard kwargs for Transport.execute that match the + contract (org_id, execution_id, tool, input, mode).""" + return dict( + organization_id="ws-123", + execution_id="exec-" + "a" * 32, + trace_id="trace-789", + tool="my.tool", + input_data={}, + on_transport_error="raise", + fallback_mode="strict", + ) + + +# ─── Source-pin tests (mirror cancel.rs / orchestrator.rs pin style) ─── + + +class TestDefNrCatchfaninSourcePin: + """Pin the shape of the fix so a refactor that reorders / removes + either umbrella arm fails loudly.""" + + def _fallback_index(self, body: str) -> int: + # Anchor on the next ``except Exception:`` arm — that's the + # silent-swallow fallback that the new umbrella arms must + # precede. + idx = body.find( + "except Exception:\n # Unrecognised envelope" + ) + assert idx != -1, ( + "TRANSPORT test fixture broken: silent " + "`except Exception: pass` fallback not found" + ) + # Walk back to the `except` keyword. + except_idx = body.rfind("except Exception", 0, idx + 1) + assert except_idx != -1 + return except_idx + + def test_decision_umbrella_arm_present(self): + body = _execute_body() + assert "except NullRunDecision as exc:" in body, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: the pass-through arm " + "for NullRunDecision must be present in " + "Transport.execute. Pre-fix NullRunChainError / " + "NullRunWorkflowInactiveError / " + "NullRunConsumeOverbudgetError were silently " + "swallowed into the synthetic-block shape." + ) + + def test_infrastructure_umbrella_arm_present(self): + body = _execute_body() + assert "except NullRunInfrastructureError as exc:" in body, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: the pass-through arm " + "for NullRunInfrastructureError must be present in " + "Transport.execute. Pre-fix NullRunProtocolError / " + "NullRunRateLimitRedisError were silently swallowed." + ) + + def test_decision_arm_after_blocked_exception_arm(self): + """``except NullRunDecision`` MUST come AFTER + ``except NullRunBlockedException`` so the typed-block path + (budget / tool / 6 approval exceptions) still wins on MRO + specificity. Reorder: order Blocked first, then Decision.""" + body = _execute_body() + blocked_idx = body.find("except NullRunBlockedException as exc:") + decision_idx = body.find("except NullRunDecision as exc:") + assert blocked_idx != -1, "NullRunBlockedException arm missing" + assert decision_idx != -1, "NullRunDecision arm missing" + assert blocked_idx < decision_idx, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunDecision " + "umbrella must come AFTER NullRunBlockedException so " + "the typed-block path stays MRO-specific." + ) + + def test_decision_arm_before_fallback(self): + body = _execute_body() + decision_idx = body.find("except NullRunDecision as exc:") + fallback_idx = self._fallback_index(body) + assert decision_idx != -1 + assert decision_idx < fallback_idx, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunDecision " + "umbrella must come BEFORE `except Exception: pass` " + "fallback." + ) + + def test_infrastructure_arm_after_backend_auth_transport(self): + """``except NullRunInfrastructureError`` MUST come AFTER + ``except NullRunBackendError`` / ``except + NullRunAuthenticationError`` / ``except + NullRunTransportError`` so wire-classified exceptions still + match by MRO specificity. The umbrella arm catches what's + left (Protocol / RateLimitRedis / Config), not everything + InfrastructureError-shaped.""" + body = _execute_body() + backend_idx = body.find("except NullRunBackendError as exc:") + auth_idx = body.find("except NullRunAuthenticationError as exc:") + transport_idx = body.find("except NullRunTransportError as exc:") + infra_idx = body.find("except NullRunInfrastructureError as exc:") + assert backend_idx != -1 and auth_idx != -1 and transport_idx != -1 + assert infra_idx != -1 + # The umbrella must come AFTER all three specific Arms so + # they win by MRO. We don't constrain the relative order + # between the three specific Arms themselves. + assert ( + backend_idx < infra_idx + and auth_idx < infra_idx + and transport_idx < infra_idx + ), ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: " + "NullRunInfrastructureError umbrella must come AFTER " + "NullRunBackendError / NullRunAuthenticationError / " + "NullRunTransportError arms." + ) + + def test_infrastructure_arm_before_fallback(self): + body = _execute_body() + infra_idx = body.find("except NullRunInfrastructureError as exc:") + fallback_idx = self._fallback_index(body) + assert infra_idx != -1 + assert infra_idx < fallback_idx, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: " + "NullRunInfrastructureError umbrella must come BEFORE " + "`except Exception: pass` fallback." + ) + + def test_decision_arm_only_raises(self): + body = _execute_body() + m = re.search( + r"except NullRunDecision as exc:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, "could not parse NullRunDecision arm body" + executable_lines = [ + ln for ln in m.group(1).splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + assert "raise" in executable + # Must not synthesize a synthetic dict (that's the bug we're + # fixing). + assert "decision" not in executable.lower() or "metrics" in executable, ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunDecision arm " + "must only raise; the pre-fix behavior was a 'pass' " + "followed by a synthetic dict return." + ) + # Sanity: the arm should NOT swallow the exception. + assert "pass" not in executable.split("\n")[0], ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunDecision " + "arm's first executable line must not be a `pass` " + "(that was the pre-fix swallow)." + ) + + def test_infrastructure_arm_only_raises(self): + body = _execute_body() + m = re.search( + r"except NullRunInfrastructureError as exc:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, "could not parse NullRunInfrastructureError arm body" + executable_lines = [ + ln for ln in m.group(1).splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + assert "raise" in executable + + def test_imports_include_umbrella_classes(self): + src = _read(TRANSPORT_PY) + # The import block at ~line 2480 must include both + # NullRunDecision and NullRunInfrastructureError; + # otherwise NameError at runtime even though the except + # arms are present. + assert "NullRunDecision" in src + assert "NullRunInfrastructureError" in src + + +# ─── Behavioral tests (mirror test_transport.py:1226 style) ───────── + + +class TestDefNrCatchfaninBehavior: + """Pin the runtime behavior — a wire envelope carrying one of + the previously-lost codes now propagates as a typed exception, + not a synthetic block dict.""" + + @respx.mock + def test_chain_org_mismatch_propagates_as_chain_error(self, transport): + """CHAIN_ORG_MISMATCH (NR-CH001) → ``_parse_v3_error_envelope`` + raises NullRunChainError. Catch-fan-in re-raises it; we see + it, not a synthetic block.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "CHAIN_ORG_MISMATCH", + status=403, + chain_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + ) + ) + with pytest.raises(NullRunChainError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-CH001", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: CHAIN_ORG_MISMATCH " + "must raise NullRunChainError (NR-CH001), not be " + "swallowed into a synthetic block dict." + ) + assert ( + excinfo.value.chain_id + == "01a08b57-f176-79ce-ad1b-60b0184d1625" + ), ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunChainError " + ".chain_id must be preserved (was lost in the " + "synthetic-block pre-fix)." + ) + + @respx.mock + def test_workflow_inactive_propagates_as_workflow_inactive_error( + self, transport + ): + """WORKFLOW_INACTIVE (NR-W004) → NullRunWorkflowInactiveError.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "WORKFLOW_INACTIVE", + status=403, + workflow_id="wf-soft-deleted-789", + ) + ) + with pytest.raises(NullRunWorkflowInactiveError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-W004" + assert excinfo.value.workflow_id == "wf-soft-deleted-789", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: NullRunWorkflowInactiveError" + ".workflow_id must be preserved." + ) + + @respx.mock + def test_consume_overbudget_propagates_with_counter_attrs( + self, transport + ): + """CONSUME_OVERBUDGET (NR-O001) → NullRunConsumeOverbudgetError.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "CONSUME_OVERBUDGET", + status=422, + execution_id="exec-consume-overrun", + reserved_cents=100, + max_allowed_cents=101, + actual_cost_cents=1000, + epsilon_cents=1, + ) + ) + with pytest.raises(NullRunConsumeOverbudgetError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-O001", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: CONSUME_OVERBUDGET " + "must raise NullRunConsumeOverbudgetError (NR-O001), " + "not be swallowed." + ) + assert excinfo.value.reserved_cents == 100 + assert excinfo.value.max_allowed_cents == 101 + assert excinfo.value.actual_cost_cents == 1000 + assert excinfo.value.epsilon_cents == 1 + + @respx.mock + def test_protocol_too_old_propagates_as_protocol_error(self, transport): + """PROTOCOL_TOO_OLD (NR-P001) → NullRunProtocolError.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "PROTOCOL_TOO_OLD", status=400, + ) + ) + with pytest.raises(NullRunProtocolError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-P001", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: PROTOCOL_TOO_OLD " + "must raise NullRunProtocolError, not be swallowed. " + "The catalog line for NR-P001 ('Upgrade the SDK to " + "a version that supports protocol " + "X-NULLRUN-PROTOCOL: 4') is unreachable without this " + "fix — pre-fix the user saw 'Gateway returned 400'." + ) + + @respx.mock + def test_rate_limit_redis_unavailable_propagates_typed(self, transport): + """RATE_LIMIT_REDIS_UNAVAILABLE (NR-R002) → NullRunRateLimitRedisError. + + The parser dispatches on ``backend_code`` first — status + is irrelevant for the catalog branch. Status 503 would + normally hit the retry-on-5xx early-raise path in + ``_retry_with_backoff`` (with ``on_transport_error='raise'``), + so use 400 here so the response reaches the parser + unmolested. The fix being tested is the catch-fan-in's + ability to propagate the typed exception, not the retry + layer.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "RATE_LIMIT_REDIS_UNAVAILABLE", + status=400, + ) + ) + with pytest.raises(NullRunRateLimitRedisError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-R002", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP: " + "RATE_LIMIT_REDIS_UNAVAILABLE must raise " + "NullRunRateLimitRedisError (NR-R002), not be " + "swallowed. The NR-R002 catalog line ('Redis " + "outage for aggregate rate limit, fail-CLOSED') is " + "unreachable without this fix." + ) + + +# ─── Regression guards (typed classes still flow upstream) ──────────── + + +class TestDefNrCatchfaninRegressionGuards: + """Pre-existing fan-in arms must still work — these are the + typed branches that this fix did NOT touch but must verify + didn't move.""" + + @respx.mock + def test_blocked_exception_still_propagates(self, transport): + """Budget / 6 approval typed exceptions + (NullRunBlockedException subclasses) must still flow + through the ``except NullRunBlockedException`` arm, NOT + through the new ``except NullRunDecision`` umbrella. + + Use BUDGET_HARD_BLOCKED (which has explicit parser + dispatch with ``workflow_id`` + ``reason`` — see + ``_parse_v3_error_envelope`` line ~2726) — this maps to + NullRunBudgetError and exercises the + ``NullRunBlockedException`` arm. + + Note: TOOL_BLOCKED currently has a separate pre-existing + parser bug (catalog fallback uses generic dispatcher + which doesn't pass ``workflow_id`` / ``reason`` to + NullRunBlockedException constructor — see parser + TypeError at line ~2782). That bug is OUT OF SCOPE for + DEF-NR-TRANSPORT-CATCHFANIN-GAP, which is specifically + about the catch-fan-in rewrap-loss, not parser + correctness. A future fix can address the parser.""" + from nullrun.breaker.exceptions import NullRunBudgetError + + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "BUDGET_HARD_BLOCKED", + status=402, + workflow_id="wf-budget-1", + current_spend_cents=5000, + budget_cents=1000, + ) + ) + with pytest.raises(NullRunBudgetError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-B004", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP regression: " + "BUDGET_HARD_BLOCKED must still propagate as " + "NullRunBudgetError through the NullRunBlockedException " + "arm. The new NullRunDecision umbrella arm must not " + "have intercepted this code path." + ) + assert excinfo.value.workflow_id == "wf-budget-1" + + @respx.mock + def test_auth_error_still_propagates(self, transport): + """API_KEY_REVOKED (NR-A003) → NullRunAuthError → must + propagate via the existing NullRunAuthenticationError arm, + not be silently swallowed.""" + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "API_KEY_REVOKED", status=401, + ) + ) + with pytest.raises(NullRunAuthError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-A003", ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP regression: " + "API_KEY_REVOKED must still propagate as NullRunAuthError, " + "reaching the cookbook branch on wire_code." + ) + + @respx.mock + def test_unknown_envelope_does_not_match_umbrella_classes(self, transport): + """The catch-all ``except Exception: pass`` in the inner + try/except is INTENTIONAL for unrecognized envelopes + (plaintext body, legacy slug, malformed JSON). It must NOT + match the new umbrella arms for an unknown cause — if it + did, the legacy fallback contract would break (cookbook + code expecting a synthetic dict would see a typed + exception instead). + + We don't pin the exact outcome (synthetic dict OR + NullRunBackendError fallback) — both are acceptable + post-fix — but we DO pin that it is NOT classified as one + of the umbrella-arm classes (Protocol / RateLimitRedis / + Chain / WorkflowInactive / ConsumeOverbudget). + """ + respx.post(_EXECUTE_URL).mock( + return_value=httpx.Response(400, text="plaintext body") + ) + kwargs = _execute_kwargs() + kwargs["fallback_mode"] = "strict" + try: + result = transport.execute(**kwargs) + assert isinstance(result, dict), ( + "DEF-NR-TRANSPORT-CATCHFANIN-GAP regression: unknown " + "envelopes must still return a dict via fallback. " + "Catch-all `except Exception: pass` is intentional." + ) + assert result["decision"] == "block" + assert result["decision_source"] == "fallback" + except NullRunBackendError as exc: + # NullRunBackendError is the catalog-fallback for + # unknown envelopes per _parse_v3_error_envelope. + # Pre-fix the catch-all swallowed this and returned + # a dict. Post-fix the NullRunBackendError arm + # re-raises. NullRunBackendError is NOT one of the + # umbrella-arm classes (it's NullRunBackendError, + # caught earlier in the chain). Confirm we're not + # accidentally routing through Decision / Infra + # umbrella. + assert not isinstance(exc, NullRunProtocolError) + assert not isinstance(exc, NullRunRateLimitRedisError) + assert not isinstance(exc, NullRunChainError) + assert not isinstance(exc, NullRunWorkflowInactiveError) + assert not isinstance(exc, NullRunConsumeOverbudgetError) diff --git a/tests/test_2026_09_10_check_failopen.py b/tests/test_2026_09_10_check_failopen.py new file mode 100644 index 0000000..6fa4f71 --- /dev/null +++ b/tests/test_2026_09_10_check_failopen.py @@ -0,0 +1,330 @@ +"""DEF-NR-CHECK-FAIL-OPEN (2026-09-10) — ``Transport.check`` MUST NOT +synthesize ``decision_source=FALLBACK`` for 4xx responses. + +Pre-fix: + - ``nullrun/transport.py::Transport.check`` had a 4xx branch that + returned a synthetic + ``{"decision": "block", "decision_source": "fallback", ...}`` dict. + - The runtime's fail-OPEN path at ``runtime.py:2063-2088`` checks + ``decision_source.startswith("fallback")`` and returns a soft-pass + in that case — VIOLATING the CLAUDE.md §4 fail-CLOSED invariant. + - User-visible symptom: a 402 BUDGET_HARD_BLOCKED from the gateway + was downgraded to an allow. Wire-coded reasons + (BUDGET_HARD_BLOCKED / BUDGET_SOFT_BLOCKED / TOOL_BLOCKED / + RATE_LIMITED / etc.) were all silently swallowed. + +Post-fix: + - The 4xx branch parses the v3 wire envelope and returns a + gateway-shaped dict (``decision_source=DecisionSource.GATEWAY``, + NOT "fallback") with the wire envelope preserved + (``error_code``, ``explanation``, ``policy_id``, + ``remaining_budget_cents``, ``details``, ...). + - The runtime's existing ``decision=="block"`` arm then raises + ``NullRunBudgetError`` with first-class attrs intact. + +These tests pin BOTH the source shape (the 4xx branch returns +``DecisionSource.GATEWAY``, not ``FALLBACK``) AND the runtime behavior +(wire-coded reasons propagate as gateway decisions and trigger +``NullRunBudgetError``, NOT a silent allow). +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path + +import httpx +import pytest +import respx + +from nullrun.breaker.exceptions import ( + NullRunBudgetError, + NullRunError, +) +from nullrun.transport import DecisionSource, Transport + +SDK_ROOT = Path(__file__).resolve().parent.parent +TRANSPORT_PY = SDK_ROOT / "src" / "nullrun" / "transport.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _check_body() -> str: + """Return the source of ``Transport.check`` so source-pin tests + can grep for the expected 4xx branch without depending on Python + AST parsing. + + The signature is multiline + (``def check(\n self,\n workflow_id: ...``) so we + anchor on ``def check(`` and walk forward to the next top-level + ``def`` (4-space indent) inside the same class.""" + src = _read(TRANSPORT_PY) + start = src.find(" def check(\n") + assert start != -1, "could not locate Transport.check header" + after_header = src.index(" def check(\n", start) + len(" def check(\n") + m = re.search( + r"^ (?:def |@|class )", + src[after_header:], + re.MULTILINE, + ) + assert m, "could not locate end of Transport.check body" + end = after_header + m.start() + return src[start:end] + + +def _v3_envelope( + error_code: str, + status: int = 402, + *, + explanation: str | None = None, + policy_id: str | None = None, + remaining_budget_cents: int = 0, + projected_cost_cents: int | None = None, + reservation_id: str | None = None, + operation_id: str | None = None, + policy_version: int | None = None, + **details, +) -> httpx.Response: + """Build a v3-shaped 4xx response envelope mirroring the real + backend's wire contract.""" + body = { + "decision": "block", + "decision_source": DecisionSource.GATEWAY, + "explanation": explanation or error_code, + "explanations": [explanation or error_code], + "error_code": error_code, + "policy_id": policy_id, + "policy_version": policy_version, + "reservation_id": reservation_id, + "operation_id": operation_id, + "remaining_budget_cents": remaining_budget_cents, + "projected_cost_cents": projected_cost_cents, + "details": details, + } + return httpx.Response(status, json=body) + + +_CHECK_URL = "https://api.test.nullrun.io/api/v1/gate" + + +@pytest.fixture +def transport(): + t = Transport( + api_url="https://api.test.nullrun.io", + api_key="test-key-12345678", + ) + yield t + t.stop() + + +def _check_kwargs(): + return dict( + check_request={ + "organization_id": "ws-123", + "workflow_id": "wf-" + "a" * 32, + "trace_id": "trace-789", + "tool": "read_file", + }, + on_transport_error="raise", + ) + + +# ─── Source-pin tests (mirror cancel.rs / orchestrator.rs pin style) ─── + + +class TestDefNrCheckFailopenSourcePin: + """Pin the shape of the fix so a refactor that re-introduces + decision_source=fallback for 4xx fails loudly.""" + + def test_4xx_branch_uses_gateway_not_fallback(self): + """The 4xx branch in Transport.check MUST assign + DecisionSource.GATEWAY (or the literal "gateway" string), + not DecisionSource.FALLBACK. Pre-fix this branch synthesised + fallback and the runtime fail-OPENed.""" + body = _check_body() + # Locate the 4xx branch by its comment marker + idx = body.find("if 400 <= response.status_code < 500:") + assert idx != -1, ( + "DEF-NR-CHECK-FAIL-OPEN: 4xx branch anchor " + "`if 400 <= response.status_code < 500:` not found in " + "Transport.check" + ) + # Slice only the 4xx branch (stop at the next sibling `if` + # for the 5xx fallthrough). + five_xx_marker = body.find("if response.status_code >= 500", idx) + assert five_xx_marker != -1 + branch_body = body[idx:five_xx_marker] + assert "DecisionSource.GATEWAY" in branch_body, ( + "DEF-NR-CHECK-FAIL-OPEN: 4xx branch must use " + "DecisionSource.GATEWAY. Pre-fix it used " + "DecisionSource.FALLBACK and the runtime treated 4xx as " + "transport errors, fail-OPENing the gate." + ) + # The fallback synthesis arm must NOT be reachable on the 4xx + # path. We assert it does not co-exist in the 4xx branch + # body. (Other branches in check/execute MAY still use + # FALLBACK for genuine transport errors — that's outside + # this pin's scope.) + assert "DecisionSource.FALLBACK" not in branch_body, ( + "DEF-NR-CHECK-FAIL-OPEN: 4xx branch must not return " + "decision_source=FALLBACK. Runtime treats fallback as " + "transport error and silently allows." + ) + + def test_4xx_branch_preserves_wire_envelope(self): + """The 4xx branch must surface wire envelope fields + (``error_code``, ``explanation``, ``policy_id``, + ``remaining_budget_cents``, ``details``) so the runtime's + catalog dispatcher can build an actionable exception.""" + body = _check_body() + idx = body.find("if 400 <= response.status_code < 500:") + assert idx != -1 + five_xx_marker = body.find("if response.status_code >= 500", idx) + assert five_xx_marker != -1 + branch_body = body[idx:five_xx_marker] + for field in ( + "error_code", + "explanation", + "explanations", + "policy_id", + "reservation_id", + "remaining_budget_cents", + "projected_cost_cents", + "operation_id", + "details", + ): + assert field in branch_body, ( + f"DEF-NR-CHECK-FAIL-OPEN: 4xx branch must surface " + f"`{field}` from the wire envelope; runtime catalog " + f"dispatchers depend on it." + ) + + +# ─── Behaviour tests (pin the runtime outcome of the fix) ─── + + +class TestDefNrCheckFailopenBehavior: + """Verify that on a wire-coded 4xx response, Transport.check + returns a gateway-shaped dict (decision_source=gateway) so the + runtime's existing block dispatcher raises the typed exception + instead of fail-OPENing.""" + + def test_402_budget_hard_blocked_returns_gateway_dict(self, transport): + """A 402 BUDGET_HARD_BLOCKED from /check must return a + gateway-shaped dict, NOT a fallback dict.""" + policy_id = "pol-" + "a" * 32 + with respx.mock(assert_all_called=False) as mock: + mock.post(_CHECK_URL).mock( + return_value=_v3_envelope( + "BUDGET_HARD_BLOCKED", + status=402, + explanation="Hard budget limit exceeded", + policy_id=policy_id, + remaining_budget_cents=0, + projected_cost_cents=1, + budget_cents=1000, + current_spend_cents=1100, + enforcement_mode="hard", + ) + ) + result = transport.check(**_check_kwargs()) + assert result["decision"] == "block", ( + f"DEF-NR-CHECK-FAIL-OPEN: expected decision=block, got " + f"{result.get('decision')}" + ) + assert result["decision_source"] == DecisionSource.GATEWAY, ( + f"DEF-NR-CHECK-FAIL-OPEN: 4xx must produce " + f"decision_source=gateway, got {result.get('decision_source')}. " + f"Runtime treats fallback as transport error and fail-OPENs." + ) + assert result.get("error_code") == "BUDGET_HARD_BLOCKED" + assert result.get("policy_id") == policy_id + assert result.get("remaining_budget_cents") == 0 + + def test_403_tool_blocked_returns_gateway_dict(self, transport): + """A 403 TOOL_BLOCKED must surface with decision_source= + gateway so runtime raises NullRunBlockedException, not + silently allow.""" + with respx.mock(assert_all_called=False) as mock: + mock.post(_CHECK_URL).mock( + return_value=_v3_envelope( + "TOOL_BLOCKED", + status=403, + explanation="bash is blocked by policy", + policy_id="pol-toolblock", + details={"tool": "bash", "matched_patterns": ["bash"]}, + ) + ) + result = transport.check(**_check_kwargs()) + assert result["decision"] == "block" + assert result["decision_source"] == DecisionSource.GATEWAY + assert result.get("error_code") == "TOOL_BLOCKED" + assert result.get("explanation") == "bash is blocked by policy" + + def test_4xx_with_unparseable_body_returns_gateway_dict(self, transport): + """A 4xx with non-JSON body must still return a + gateway-shaped dict with explanation fallback — never + decision_source=fallback.""" + with respx.mock(assert_all_called=False) as mock: + mock.post(_CHECK_URL).mock( + return_value=httpx.Response(400, text="Bad Request") + ) + result = transport.check(**_check_kwargs()) + assert result["decision"] == "block" + assert result["decision_source"] == DecisionSource.GATEWAY + assert "400" in result.get("explanation", "") or len( + result.get("explanations") or [] + ) >= 1 + + def test_5xx_does_not_use_4xx_branch(self, transport): + """A 5xx (genuine transport error) must NOT take the 4xx + gateway branch — Transport.check must still raise a + transport-class exception (or return None per on_transport_error) + so the runtime's retry/escalation logic kicks in. This pins + the boundary: 4xx → gateway dict, 5xx → transport error.""" + + # Set on_transport_error=raise and capture the call result. + # We don't assert a specific exception type — that's + # documented behavior of the existing transport layer — + # only that the 5xx path does NOT silently return a + # gateway-shaped dict with decision_source=gateway. + with respx.mock(assert_all_called=False) as mock: + mock.post(_CHECK_URL).mock( + return_value=httpx.Response(503, text="upstream unavailable") + ) + kwargs = _check_kwargs() + kwargs["on_transport_error"] = "raise" + with pytest.raises((NullRunError, Exception)) as ei: + transport.check(**kwargs) + # Belt-and-braces: the exception (whatever it is) must not + # be a NullRunBudgetError with a wire-coded reason — that + # would be the same fail-OPEN class as the original defect. + if isinstance(ei.value, NullRunBudgetError): + pytest.fail( + "DEF-NR-CHECK-FAIL-OPEN regression: a 5xx " + "response must not surface as a budget exception " + "with a wire-coded reason. 5xx is transport " + "error, not a budget decision." + ) + + def test_200_returns_normal_dict(self, transport): + """Sanity: a 200 /check still returns the response body + verbatim — no gateway/fallback tagging that didn't come from + the server.""" + with respx.mock(assert_all_called=False) as mock: + mock.post(_CHECK_URL).mock( + return_value=httpx.Response( + 200, + json={ + "decision": "allow", + "decision_source": "gateway", + "remaining_budget_cents": 990, + }, + ) + ) + result = transport.check(**_check_kwargs()) + assert result["decision"] == "allow" + assert result.get("remaining_budget_cents") == 990 diff --git a/tests/test_2026_09_10_decision_infra_passthrough.py b/tests/test_2026_09_10_decision_infra_passthrough.py new file mode 100644 index 0000000..4aa4ba7 --- /dev/null +++ b/tests/test_2026_09_10_decision_infra_passthrough.py @@ -0,0 +1,448 @@ +"""DEF-NR-A003-REWRAP-LOSS (2026-09-10, broader scope) — typed +Decision and Infrastructure subclasses that fall outside the +NullRunBlockedException / NullRunBackendError / NullRunAuthenticationError +/ NullRunTransportError arms of ``@protect`` / +``_enforce_sensitive_tool`` must propagate unchanged through the +catch-all rewrap arm. + +Pre-fix (audit 2026-09-10): + - ``nullrun/decorators.py::_enforce_sensitive_tool`` had a final + ``except Exception as exc:`` catch-all that rewrote everything to + ``NullRunBlockedException(error_code="NR-B001", reason="policy + engine unavailable: ...")``. + - The following typed subclasses do not match the four named arms + above (NullRunBlockedException, NullRunBackendError, + NullRunAuthenticationError, NullRunTransportError), so they were + silently rewrapped into ``NR-B001``: + * NullRunAuthError (NR-A003) — typed 401 envelope with + ``wire_code`` (API_KEY_REVOKED / EXPIRED / DISABLED / + INVALID / MISSING / MALFORMED per v3.38) — rewrap loses the + wire_code and the catalog line for "API key rejected. + Verify ... rotate if revoked." + * NullRunProtocolError (NR-P001) — wire-protocol mismatch — + loses the "Upgrade the SDK to a version that supports + protocol X-NULLRUN-PROTOCOL: 4" recovery hint. + * NullRunRateLimitRedisError (NR-R002) — Redis-outage + fail-CLOSED — loses "fail-CLOSED due to Redis outage" + distinct from a generic 503. + * NullRunConfigError (NR-Cxxx) — wired-in config error — + should never be rewrapped as a transient transport block. + * NullRunChainError (NR-CH001) — chain / cross-org / Execution + Graph parent-lineage — loses ``chain_id``, + ``parent_execution_id``, ``backend_code``. + * NullRunWorkflowInactiveError (NR-W004) — soft-deleted + workflow — loses ``workflow_id``. + * NullRunConsumeOverbudgetError (NR-O001) — invariant + violation — loses ``execution_id``, ``reserved_cents``, + ``max_allowed_cents``, ``actual_cost_cents``. + * WorkflowPausedException (NR-W003) — loses + ``resume_after``, ``workflow_id``, ``reason``. + +Post-fix: + - Two umbrella pass-through arms added BEFORE the catch-all + ``except Exception: except NullRunDecision: raise`` and + ``except NullRunInfrastructureError: raise``. Each umbrella + covers a known set of typed subclasses (see the catch-fan-in + comment in decorators.py for the full enumeration). Catalog + is preserved with error_code / user_action / first-class attrs + intact. + +These tests pin BOTH the source shape AND the runtime behavior so +a future refactor that re-introduces a rewrap (e.g. drops one of +the umbrella arms, or re-orders them after the catch-all) fails +the test. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from nullrun.breaker.exceptions import ( + NullRunAuthError, + NullRunBackendError, + NullRunBlockedException, + NullRunChainError, + NullRunConsumeOverbudgetError, + NullRunProtocolError, + NullRunRateLimitRedisError, + NullRunTransportError, + NullRunWorkflowInactiveError, + TransportErrorSource, + WorkflowPausedException, +) +from nullrun.decorators import _enforce_sensitive_tool + +SDK_ROOT = Path(__file__).resolve().parent.parent +DECORATORS_PY = SDK_ROOT / "src" / "nullrun" / "decorators.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _enforce_sensitive_tool_body() -> str: + """Return the source of ``_enforce_sensitive_tool`` so source-pin + tests can grep for the expected arms / ordering without depending + on Python AST parsing.""" + src = _read(DECORATORS_PY) + m = re.search( + r"def _enforce_sensitive_tool\(.*?\n(?=def |\nclass |\Z)", + src, + re.DOTALL, + ) + assert m, "could not locate _enforce_sensitive_tool body" + return m.group(0) + + +# ─── Source-pin tests (mirror cancel.rs / orchestrator.rs pin style) ─── + + +class TestDefNrA003SourcePin: + """Pin the shape of the fix so a refactor that reorders / removes + the umbrella arms fails loudly.""" + + def _catch_all_index(self, body: str) -> int: + # ``_enforce_sensitive_tool`` has TWO ``except Exception as + # exc:`` arms: an early one (around body-line 87) inside the + # business_impact extractor wrapper, and the main one (the + # catch-all rewrap near the bottom). The umbrella arms in + # this fix must precede the MAIN catch-all (the one whose + # comment starts with "Any other exception is a transport / + # network / backend failure"); the extractor arm is unrelated + # and should not be matched. + # Anchor on the distinctive comment that prefaces the main + # catch-all rewrap so we pick the correct one. + marker = "Any other exception is a transport" + marker_idx = body.find(marker) + assert marker_idx != -1, ( + "DECORATORS test fixture broken: main catch-all arm " + "marker 'Any other exception is a transport' not found " + "in _enforce_sensitive_tool" + ) + # The `except` keyword is on the line just before the comment. + except_idx = body.rfind("except Exception as exc:", 0, marker_idx) + assert except_idx != -1, ( + "DECORATORS test fixture broken: catch-all `except " + "Exception as exc:` arm not found near the marker" + ) + return except_idx + + def test_decision_umbrella_arm_present(self): + body = _enforce_sensitive_tool_body() + assert "except NullRunDecision:" in body, ( + "DEF-NR-A003-REWRAP-LOSS (umbrella): the pass-through arm " + "for NullRunDecision must be present in " + "_enforce_sensitive_tool. Pre-fix NullRunChainError / " + "NullRunWorkflowInactiveError / NullRunConsumeOverbudgetError " + "were swallowed by the catch-all rewrap into NR-B001." + ) + + def test_infrastructure_umbrella_arm_present(self): + body = _enforce_sensitive_tool_body() + assert "except NullRunInfrastructureError:" in body, ( + "DEF-NR-A003-REWRAP-LOSS (umbrella): the pass-through arm " + "for NullRunInfrastructureError must be present in " + "_enforce_sensitive_tool. Pre-fix NullRunAuthError / " + "NullRunProtocolError / NullRunRateLimitRedisError / " + "NullRunConfigError were swallowed by the catch-all rewrap." + ) + + def test_decision_arm_appears_before_catch_all(self): + """Order matters: the umbrella arm must come BEFORE + ``except Exception as exc:``. If a future refactor moves it + after, the catch-all would silently rewrap into NR-B001.""" + body = _enforce_sensitive_tool_body() + decision_idx = body.find("except NullRunDecision:") + catch_all_idx = self._catch_all_index(body) + assert decision_idx != -1 + assert decision_idx < catch_all_idx, ( + "DEF-NR-A003-REWRAP-LOSS: the NullRunDecision umbrella arm " + "must appear BEFORE `except Exception as exc:`. Pre-fix " + "order swallowed NullRunChainError / " + "NullRunWorkflowInactiveError / NullRunConsumeOverbudgetError " + "into NR-B001." + ) + + def test_infrastructure_arm_appears_before_catch_all(self): + body = _enforce_sensitive_tool_body() + idx = body.find("except NullRunInfrastructureError:") + catch_all_idx = self._catch_all_index(body) + assert idx != -1 + assert idx < catch_all_idx, ( + "DEF-NR-A003-REWRAP-LOSS: the NullRunInfrastructureError " + "umbrella arm must appear BEFORE `except Exception as " + "exc:`. Pre-fix order swallowed NullRunAuthError / " + "NullRunProtocolError / NullRunRateLimitRedisError into " + "NR-B001." + ) + + def test_decision_arm_only_raises(self): + body = _enforce_sensitive_tool_body() + m = re.search( + r"except NullRunDecision:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, "could not parse NullRunDecision arm body" + executable_lines = [ + ln for ln in m.group(1).splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + assert "raise" in executable + assert "NullRunBlockedException" not in executable, ( + "DEF-NR-A003-REWRAP-LOSS: NullRunDecision arm must not " + "rewrap into NullRunBlockedException" + ) + + def test_infrastructure_arm_only_raises(self): + body = _enforce_sensitive_tool_body() + m = re.search( + r"except NullRunInfrastructureError:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, "could not parse NullRunInfrastructureError arm body" + executable_lines = [ + ln for ln in m.group(1).splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + assert "raise" in executable + assert "NullRunBlockedException" not in executable, ( + "DEF-NR-A003-REWRAP-LOSS: NullRunInfrastructureError arm " + "must not rewrap into NullRunBlockedException" + ) + + def test_decision_arm_comment_tag_present(self): + body = _enforce_sensitive_tool_body() + assert "DEF-NR-A003-REWRAP-LOSS" in body, ( + "DEF-NR-A003-REWRAP-LOSS: the umbrella-arm explainer " + "comment block must name the fix tag so future readers " + "can grep for it." + ) + + def test_imports_include_umbrella_classes(self): + src = _read(DECORATORS_PY) + # The function-local import block at line ~809 must include + # both NullRunDecision and NullRunInfrastructureError; + # otherwise NameError at runtime even though the except arms + # are present. + assert "NullRunDecision" in src + assert "NullRunInfrastructureError" in src + + +# ─── Behavioral tests (mirror test_protect.py:651 style) ───────────── + + +def _mock_runtime_raising(exc: Exception) -> MagicMock: + rt = MagicMock() + rt.is_sensitive_tool.return_value = True + rt.execute.side_effect = exc + return rt + + +class TestDefNrA003Behavior: + """Pin the runtime behavior — typed exception propagates with + error_code + first-class attrs intact.""" + + # ── NullRunInfrastructureError subclass coverage ───────────────── + + def test_auth_error_propagates_unchanged_with_wire_code(self): + """NullRunAuthError is the canonical case that prompted this + fix: an authorized cookbook user gets a 401 with + wire_code=API_KEY_REVOKED (or one of five other lifecycle + codes). Pre-fix the @protect catch-all stamped NR-B001 and + discarded wire_code, so the operator saw 'policy engine + unavailable' instead of 'API key rejected (401). Verify at + ... and rotate if revoked.'""" + exc = NullRunAuthError( + "API key rejected", wire_code="API_KEY_REVOKED" + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunAuthError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc, ( + "DEF-NR-A003-REWRAP-LOSS: NullRunAuthError must propagate " + "unchanged (identity check)." + ) + assert excinfo.value.error_code == "NR-A003" + assert excinfo.value.wire_code == "API_KEY_REVOKED", ( + "DEF-NR-A003-REWRAP-LOSS: exc.wire_code must be preserved " + "— NullRunBlockedException doesn't carry it and the " + "catch-all rewrap dropped it." + ) + + def test_protocol_error_propagates_unchanged(self): + exc = NullRunProtocolError("PROTOCOL_TOO_OLD") + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunProtocolError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-P001", ( + "DEF-NR-A003-REWRAP-LOSS: NullRunProtocolError must keep " + "its NR-P001 error_code; pre-fix the catch-all stamped " + "NR-B001 and lost the SDK-upgrade catalog line." + ) + + def test_rate_limit_redis_error_propagates_unchanged(self): + exc = NullRunRateLimitRedisError( + "Redis unavailable for aggregate rate limit" + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunRateLimitRedisError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-R002", ( + "DEF-NR-A003-REWRAP-LOSS: NullRunRateLimitRedisError must " + "keep its NR-R002 error_code; pre-fix the catch-all " + "stamped NR-B001 and lost the 'Redis outage for " + "aggregate rate limit (fail-CLOSED)' message." + ) + + # ── NullRunDecision subclass coverage ──────────────────────────── + + def test_chain_error_propagates_with_chain_id(self): + exc = NullRunChainError( + "CHAIN_ORG_MISMATCH", + chain_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + parent_execution_id=None, + backend_code="CHAIN_ORG_MISMATCH", + status_code=403, + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunChainError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-CH001" + assert ( + excinfo.value.chain_id + == "01a08b57-f176-79ce-ad1b-60b0184d1625" + ), ( + "DEF-NR-A003-REWRAP-LOSS: NullRunChainError.chain_id " + "must be preserved for the cookbook recovery path." + ) + assert excinfo.value.backend_code == "CHAIN_ORG_MISMATCH" + + def test_workflow_inactive_error_propagates_with_workflow_id(self): + exc = NullRunWorkflowInactiveError( + "Workflow soft-deleted", + workflow_id="wf-soft-deleted-123", + status_code=403, + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunWorkflowInactiveError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-W004" + assert excinfo.value.workflow_id == "wf-soft-deleted-123" + + def test_consume_overbudget_error_propagates_with_counter_attrs(self): + """The CONSUME_OVERBUDGET invariant (NR-O001) carries + ``execution_id`` / ``reserved_cents`` / ``max_allowed_cents`` + / ``actual_cost_cents`` for the cookbook recovery contract. + Pre-fix the catch-all rewrap discarded every attr.""" + exc = NullRunConsumeOverbudgetError( + "actual > reserved + epsilon", + execution_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + reserved_cents=100, + max_allowed_cents=101, + actual_cost_cents=1000, + epsilon_cents=1, + status_code=422, + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunConsumeOverbudgetError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-O001" + assert ( + excinfo.value.execution_id + == "01a08b57-f176-79ce-ad1b-60b0184d1625" + ) + assert excinfo.value.reserved_cents == 100 + assert excinfo.value.max_allowed_cents == 101 + assert excinfo.value.actual_cost_cents == 1000 + assert excinfo.value.epsilon_cents == 1 + + def test_workflow_paused_propagates_with_resume_after(self): + exc = WorkflowPausedException( + workflow_id="wf-paused-1", + reason="cooldown", + resume_after=120.0, + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(WorkflowPausedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert excinfo.value.error_code == "NR-W003" + assert excinfo.value.resume_after == 120.0 + assert excinfo.value.workflow_id == "wf-paused-1" + + # ── Regression guards ─────────────────────────────────────────── + + def test_blocked_exception_still_passes_through(self): + """NullRunBlockedException is a NullRunDecision subclass; the + new umbrella arm MUST come AFTER the existing ``except + NullRunBlockedException: raise`` arm so the typed-block + path keeps propagating unchanged. This regression guard + pins that ordering.""" + exc = NullRunBlockedException( + workflow_id="wf-1", reason="denied by policy" + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert "denied by policy" in excinfo.value.reason + + def test_generic_transport_error_still_rewraps(self): + """The fix must NOT make ALL NullRunTransportError pass + through — only the typed Decision/Infrastructure subclasses + that don't match the more specific arms. A plain + NullRunTransportError with no typed leaf must still be + rewrapped (preserving the fail-CLOSED contract for + unclassified transport failures).""" + exc = NullRunTransportError( + "network blip", + source=TransportErrorSource.NETWORK_ERROR, + endpoint="/execute", + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # Not the typed Decision/Infrastructure leaf. + assert not isinstance(excinfo.value, NullRunProtocolError) + assert not isinstance(excinfo.value, NullRunRateLimitRedisError) + assert not isinstance(excinfo.value, NullRunChainError) + # Catch-all source mapping returns NR-B001 for NETWORK_ERROR. + assert excinfo.value.error_code == "NR-B001", ( + "DEF-NR-A003-REWRAP-LOSS regression: a generic " + "NullRunTransportError must still be rewrapped by the " + "NullRunTransportError specific arm — the umbrella arms " + "above must not silently widen pass-through to all " + "NullRunTransportError subclasses." + ) + + def test_generic_backend_error_still_rewraps(self): + """Same regression guard for NullRunBackendError: it IS a + NullRunInfrastructureError subclass, so the new umbrella + arm is downstream of the specific NullRunBackendError + rewrap arm at line ~867. Verify the parent still rewraps + (not pass-through) so the typed-leaf tests above stay + scoped to leaves, not the whole InfrastructureError class.""" + exc = NullRunBackendError( + "5xx blip", endpoint="/execute", status_code=503 + ) + rt = _mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # Confirm the parent is rewrapped (not pass-through) — the + # umbrella arms must not have widened pass-through to all + # NullRunInfrastructureError subclasses. + assert not isinstance(excinfo.value, NullRunBackendError) + assert "GATEWAY_ERROR" in excinfo.value.reason diff --git a/tests/test_2026_09_10_mcp_umbrella_symmetry.py b/tests/test_2026_09_10_mcp_umbrella_symmetry.py new file mode 100644 index 0000000..db260a8 --- /dev/null +++ b/tests/test_2026_09_10_mcp_umbrella_symmetry.py @@ -0,0 +1,364 @@ +"""Source-pin + behavioural regression tests for SDK B.1 (2026-09-10). + +B.1 closed two wire-class gaps that were silently dropping typed +information to the generic ``NullRunBlockedException`` / NR-X001 +fallback: + +1. **MCP umbrella codes** (ADR-013, 2026-08-14, frozen-dormant) — + ``MCP_DESTRUCTIVE_BLOCKED``, ``MCP_READONLY_BYPASS_BLOCKED``, + ``MCP_APPROVAL_REQUIRED`` previously all collapsed to + ``NullRunBlockedException``. Cookbook code that wanted to + ``except NullRunMcpDestructiveBlockedError:`` etc. fell through + to the generic arm and surfaced ``FALLBACK_MESSAGE = "Something + went wrong. Please try again."`` — the very bug the langgraph + demo test surfaced in 2026-09-08. + +2. **APPROVAL_DB_* sibling family** — six codes (DB_UNAVAILABLE, + PERSISTENCE_FAILED, VALIDATION_FAILED, CONFLICT, NOT_FOUND, + CREATE_FAILED) previously all collapsed to the base + ``NullRunBlockedException``. Operators couldn't tell apart a + transient DB outage from a validation failure. Post-B.1 they + all map to the single typed ``NullRunApprovalDbUnavailableError`` + so cookbook code can branch on the typed class. + +Each test pins the source surface so a future refactor that +re-introduces the bug (e.g. drops one of the typed mappings) is +caught at test time, not in production. +""" + +from __future__ import annotations + +import os + +EXCEPTIONS_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "breaker", "exceptions.py" +) +TRANSPORT_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "transport.py" +) +INIT_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "__init__.py" +) +MESSAGES_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "messages.py" +) + + +def _read(path: str) -> str: + return open(path, encoding="utf-8").read() + + +def _strip_comment_lines(src: str) -> str: + return "\n".join( + line for line in src.splitlines() if not line.lstrip().startswith("#") + ) + + +# ─── Typed exception class definitions (MCP umbrella) ──────────────────── + + +def test_mcp_destructive_blocked_error_class_defined(): + """``breaker/exceptions.py`` must define ``NullRunMcpDestructiveBlockedError``.""" + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + assert "class NullRunMcpDestructiveBlockedError" in code_only, ( + "B.1: NullRunMcpDestructiveBlockedError must be defined in " + "breaker/exceptions.py — typed class for MCP_DESTRUCTIVE_BLOCKED" + ) + + +def test_mcp_readonly_bypass_blocked_error_class_defined(): + """``breaker/exceptions.py`` must define ``NullRunMcpReadonlyBypassBlockedError``.""" + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + assert "class NullRunMcpReadonlyBypassBlockedError" in code_only, ( + "B.1: NullRunMcpReadonlyBypassBlockedError must be defined in " + "breaker/exceptions.py — typed class for MCP_READONLY_BYPASS_BLOCKED" + ) + + +def test_mcp_approval_required_error_class_defined(): + """``breaker/exceptions.py`` must define ``NullRunMcpApprovalRequiredError``.""" + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + assert "class NullRunMcpApprovalRequiredError" in code_only, ( + "B.1: NullRunMcpApprovalRequiredError must be defined in " + "breaker/exceptions.py — typed class for MCP_APPROVAL_REQUIRED" + ) + + +def test_approval_db_unavailable_error_class_defined(): + """``breaker/exceptions.py`` must define ``NullRunApprovalDbUnavailableError``.""" + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + assert "class NullRunApprovalDbUnavailableError" in code_only, ( + "B.1: NullRunApprovalDbUnavailableError must be defined in " + "breaker/exceptions.py — typed class for the six " + "APPROVAL_DB_* sibling codes" + ) + + +def test_mcp_classes_inherit_from_null_run_blocked_exception(): + """All four B.1 typed classes must inherit from + ``NullRunBlockedException`` (not ``NullRunError`` directly) so + cookbook ``except NullRunBlockedException:`` arms still match + via the MRO. This is the same pattern as the existing + NR-A010..NR-A015 approval classes.""" + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + for cls in [ + "NullRunMcpDestructiveBlockedError", + "NullRunMcpReadonlyBypassBlockedError", + "NullRunMcpApprovalRequiredError", + "NullRunApprovalDbUnavailableError", + ]: + # Look for ``class (NullRunBlockedException):`` + assert f"class {cls}(NullRunBlockedException):" in code_only, ( + f"B.1: {cls} must inherit from NullRunBlockedException, " + f"not NullRunError directly. The MRO is the contract — " + f"cookbook ``except NullRunBlockedException:`` arms must " + f"still match the typed subclass." + ) + + +# ─── Transport mappings ────────────────────────────────────────────────── + + +def test_transport_maps_mcp_destructive_blocked_to_typed_class(): + """``transport.py:_V3_ERROR_CODE_MAP`` must map + ``MCP_DESTRUCTIVE_BLOCKED`` to ``NullRunMcpDestructiveBlockedError``, + NOT to the base ``NullRunBlockedException``.""" + src = _read(TRANSPORT_PATH) + code_only = _strip_comment_lines(src) + assert ( + '"MCP_DESTRUCTIVE_BLOCKED": NullRunMcpDestructiveBlockedError' + in code_only + ), ( + "B.1: transport.py must map MCP_DESTRUCTIVE_BLOCKED to the " + "typed NullRunMcpDestructiveBlockedError. Pre-B.1 the wire " + "code collapsed to NullRunBlockedException; cookbook code " + "branching on the typed class fell through to NR-X001." + ) + + +def test_transport_maps_mcp_readonly_bypass_blocked_to_typed_class(): + src = _read(TRANSPORT_PATH) + code_only = _strip_comment_lines(src) + assert ( + '"MCP_READONLY_BYPASS_BLOCKED": NullRunMcpReadonlyBypassBlockedError' + in code_only + ), ( + "B.1: transport.py must map MCP_READONLY_BYPASS_BLOCKED to " + "NullRunMcpReadonlyBypassBlockedError" + ) + + +def test_transport_maps_mcp_approval_required_to_typed_class(): + src = _read(TRANSPORT_PATH) + code_only = _strip_comment_lines(src) + assert ( + '"MCP_APPROVAL_REQUIRED": NullRunMcpApprovalRequiredError' + in code_only + ), ( + "B.1: transport.py must map MCP_APPROVAL_REQUIRED to " + "NullRunMcpApprovalRequiredError" + ) + + +def test_transport_maps_all_six_approval_db_codes_to_typed_class(): + """All six APPROVAL_DB_* sibling codes must map to the typed + ``NullRunApprovalDbUnavailableError``. Pre-B.1 they all collapsed + to ``NullRunBlockedException`` — operators couldn't tell apart + a transient DB outage from a validation failure.""" + src = _read(TRANSPORT_PATH) + code_only = _strip_comment_lines(src) + for code in [ + "APPROVAL_DB_UNAVAILABLE", + "APPROVAL_PERSISTENCE_FAILED", + "APPROVAL_VALIDATION_FAILED", + "APPROVAL_CONFLICT", + "APPROVAL_NOT_FOUND", + "APPROVAL_CREATE_FAILED", + ]: + assert ( + f'"{code}": NullRunApprovalDbUnavailableError' in code_only + ), ( + f"B.1: transport.py must map {code} to " + f"NullRunApprovalDbUnavailableError (typed). Pre-B.1 the " + f"wire code collapsed to NullRunBlockedException." + ) + + +# ─── Top-level discoverability (lazy exports + __all__) ───────────────── + + +def test_mcp_umbrella_classes_in_lazy_exports(): + """All three MCP umbrella classes must be importable from the + top-level ``nullrun`` namespace via ``_LAZY_EXPORTS``. Pre-B.1 + they didn't exist; post-B.1 the typed classes are part of the + public surface (cookbook recipes branch on them by name).""" + src = _read(INIT_PATH) + for cls in [ + "NullRunMcpDestructiveBlockedError", + "NullRunMcpReadonlyBypassBlockedError", + "NullRunMcpApprovalRequiredError", + ]: + assert f'"{cls}":' in src, ( + f"B.1: {cls} must appear in _LAZY_EXPORTS so cookbook " + f"code can ``from nullrun import {cls}``" + ) + + +def test_approval_db_unavailable_in_lazy_exports(): + src = _read(INIT_PATH) + assert '"NullRunApprovalDbUnavailableError":' in src, ( + "B.1: NullRunApprovalDbUnavailableError must appear in " + "_LAZY_EXPORTS" + ) + + +def test_mcp_umbrella_classes_in_all(): + """The four B.1 typed classes must appear in ``__all__`` so + tab-completion surfaces them via ``dir(nullrun)``. Cookbook + code that wants to ``except NullRunMcpDestructiveBlockedError:`` + needs to discover the class via ``dir(nullrun)`` first.""" + src = _read(INIT_PATH) + # Anchor on the __all__ block — naive substring search would + # false-positive on _LAZY_EXPORTS entries. + import re + match = re.search(r"__all__\s*=\s*\[(.*?)\]", src, re.DOTALL) + assert match is not None, "B.1: __all__ list must exist in __init__.py" + all_block = match.group(1) + for cls in [ + "NullRunMcpDestructiveBlockedError", + "NullRunMcpReadonlyBypassBlockedError", + "NullRunMcpApprovalRequiredError", + "NullRunApprovalDbUnavailableError", + ]: + assert f'"{cls}"' in all_block, ( + f"B.1: {cls} must appear in __all__ so tab-completion " + f"surfaces it via dir(nullrun)" + ) + + +# ─── Catalog completeness ──────────────────────────────────────────────── + + +def test_messages_catalog_has_mcp_and_approval_db_entries(): + """``messages.DEFAULT_MESSAGES`` must have entries for the four + new error codes (NR-MCP01, NR-MCP02, NR-MCP03, NR-A016). + Without these, ``format_user_message`` falls through to the + generic ``FALLBACK_MESSAGE = "Something went wrong. Please + try again."`` — the exact bug the langgraph approval demo + surfaced in 2026-09-08.""" + from nullrun import messages + + for code in ["NR-MCP01", "NR-MCP02", "NR-MCP03", "NR-A016"]: + assert code in messages.DEFAULT_MESSAGES, ( + f"B.1: messages.DEFAULT_MESSAGES must contain an entry " + f"for {code} (the typed class's error_code). Missing " + f"catalog entry means format_user_message returns the " + f"generic FALLBACK_MESSAGE for the typed class — " + f"defeats the whole point of the typed mapping." + ) + + +# ─── Behavioural smoke tests ───────────────────────────────────────────── + + +def test_typed_class_runtime_imports(): + """All four B.1 typed classes must import successfully from + both the breaker.exceptions module AND the top-level nullrun + namespace.""" + import nullrun + from nullrun.breaker import exceptions as exc_mod + + pairs = [ + ("NullRunMcpDestructiveBlockedError", exc_mod.NullRunMcpDestructiveBlockedError), + ("NullRunMcpReadonlyBypassBlockedError", exc_mod.NullRunMcpReadonlyBypassBlockedError), + ("NullRunMcpApprovalRequiredError", exc_mod.NullRunMcpApprovalRequiredError), + ("NullRunApprovalDbUnavailableError", exc_mod.NullRunApprovalDbUnavailableError), + ] + for name, breaker_cls in pairs: + top_level_cls = getattr(nullrun, name, None) + assert top_level_cls is not None, ( + f"B.1: nullrun.{name} must be importable (lazy export " + f"failed or class is missing)" + ) + assert top_level_cls is breaker_cls, ( + f"B.1: nullrun.{name} must be the same class object as " + f"nullrun.breaker.exceptions.{name} — a separate proxy " + f"class would defeat isinstance() checks across the " + f"codebase" + ) + + +def test_typed_class_isinstance_of_null_run_blocked_exception(): + """The four B.1 typed classes must be ``isinstance(..., NullRunBlockedException)``. + Cookbook ``except NullRunBlockedException:`` arms rely on this + MRO behaviour — if a future refactor changes the parent class + the cookbook handlers silently miss the typed arms.""" + from nullrun import NullRunBlockedException + from nullrun.breaker import exceptions as exc_mod + + for cls in [ + exc_mod.NullRunMcpDestructiveBlockedError, + exc_mod.NullRunMcpReadonlyBypassBlockedError, + exc_mod.NullRunMcpApprovalRequiredError, + exc_mod.NullRunApprovalDbUnavailableError, + ]: + # Instantiate with the minimum required kwargs (workflow_id, + # reason). The base __init__ accepts **details so additional + # typed kwargs can be forwarded from transport.py. + instance = cls(workflow_id="wf-1", reason="test") + assert isinstance(instance, NullRunBlockedException), ( + f"B.1: {cls.__name__} must be isinstance of " + f"NullRunBlockedException (MRO contract for cookbook " + f"handlers). Got MRO: {[c.__name__ for c in type(instance).__mro__]}" + ) + + +def test_format_user_message_returns_catalog_default_for_typed_classes(): + """``format_user_message`` on a B.1 typed class instance must + return the catalog default, NOT the FALLBACK_MESSAGE. This is + the actual user-facing bug B.1 closed — pre-B.1 the wire code + landed on the base class, the base class's error_code was a + SCREAMING_SNAKE backend string with no catalog entry, and + ``format_user_message`` fell through to FALLBACK_MESSAGE.""" + from nullrun import messages + from nullrun.breaker import exceptions as exc_mod + + cases = [ + ( + exc_mod.NullRunMcpDestructiveBlockedError("wf-1", "destructive blocked"), + "NR-MCP01", + ), + ( + exc_mod.NullRunMcpReadonlyBypassBlockedError("wf-1", "readonly bypass blocked"), + "NR-MCP02", + ), + ( + exc_mod.NullRunMcpApprovalRequiredError("wf-1", "mcp approval pending"), + "NR-MCP03", + ), + ( + exc_mod.NullRunApprovalDbUnavailableError("wf-1", "approval db down"), + "NR-A016", + ), + ] + for instance, expected_code in cases: + assert instance.error_code == expected_code, ( + f"B.1: {type(instance).__name__}.error_code must be " + f"{expected_code}, got {instance.error_code!r}" + ) + out = messages.format_user_message(instance) + assert out == messages.DEFAULT_MESSAGES[expected_code], ( + f"B.1: format_user_message on {type(instance).__name__} " + f"must return the catalog default for {expected_code}, " + f"not FALLBACK_MESSAGE. Got: {out!r}" + ) + assert out != messages.FALLBACK_MESSAGE, ( + f"B.1: {type(instance).__name__} fell through to " + f"FALLBACK_MESSAGE — catalog entry for {expected_code} " + f"is missing or wire mapping is wrong" + ) diff --git a/tests/test_2026_09_10_nr_ex01_passthrough.py b/tests/test_2026_09_10_nr_ex01_passthrough.py new file mode 100644 index 0000000..02d3851 --- /dev/null +++ b/tests/test_2026_09_10_nr_ex01_passthrough.py @@ -0,0 +1,312 @@ +"""DEF-NR-EX01-REWRAP-LOSS (2026-09-10) — `_enforce_sensitive_tool` must +let ``NullRunExecutionNotFoundError`` (NR-EX01) propagate unchanged. + +Pre-fix (audit 2026-09-10): + - ``nullrun/decorators.py::_enforce_sensitive_tool`` had three except + arms: ``NullRunBlockedException`` (pass-through), ``NullRunTransportError`` + (rewrap to ``NullRunBlockedException(NR-B00X)``), and ``Exception`` + (catch-all rewrap). + - ``NullRunExecutionNotFoundError`` is a subclass of + ``NullRunBackendError`` which is a subclass of + ``NullRunTransportError``. The MRO puts it inside the second arm, so + the typed exception was being unwrapped into a generic + ``NullRunBlockedException(error_code="NR-B002")`` with reason + "policy engine unavailable: ...". + - User-visible symptom (per ``langgraph_openai_approval_demo.py``): + 3rd refund → backend 404 EXECUTION_NOT_FOUND → SDK prints + "Our service is temporarily unavailable. Please try again shortly." + (NR-B002) instead of the documented NR-EX01 line + "There's a configuration issue. Please contact support." + - Cookbook pattern ``except NullRunExecutionNotFoundError`` never + matched because the exception class was lost in the rewrap. + +Post-fix: + - Added a dedicated pass-through arm BEFORE ``except + NullRunBlockedException`` so the typed exception propagates + unchanged. Cookbook code can introspect ``exc.execution_id``, + ``exc.endpoint``, and ``exc.regate_required`` for the documented + recovery path (re-issue /api/v1/gate, then retry /execute). + +These tests pin BOTH the source shape AND the runtime behavior so a +future refactor that re-introduces a rewrap (e.g. reorders the except +arms) fails the test. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from nullrun.breaker.exceptions import ( + NullRunBlockedException, + NullRunExecutionNotFoundError, + NullRunTransportError, +) +from nullrun.decorators import _enforce_sensitive_tool + +SDK_ROOT = Path(__file__).resolve().parent.parent +DECORATORS_PY = SDK_ROOT / "src" / "nullrun" / "decorators.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _enforce_sensitive_tool_body() -> str: + """Return the source of ``_enforce_sensitive_tool`` so source-pin + tests can grep for the expected arms / ordering without depending + on Python AST parsing.""" + src = _read(DECORATORS_PY) + m = re.search( + r"def _enforce_sensitive_tool\(.*?\n(?=def |\nclass |\Z)", + src, + re.DOTALL, + ) + assert m, "could not locate _enforce_sensitive_tool body" + return m.group(0) + + +# ─── Source-pin tests (mirror cancel.rs / orchestrator.rs pin style) ─── + + +class TestDefNrEx01SourcePin: + """Pin the shape of the fix so a refactor that reorders / removes + the pass-through arm fails loudly.""" + + def test_pass_through_arm_is_present(self): + body = _enforce_sensitive_tool_body() + assert "except NullRunExecutionNotFoundError:" in body, ( + "DEF-NR-EX01-REWRAP-LOSS: the pass-through arm for " + "NullRunExecutionNotFoundError must be present in " + "_enforce_sensitive_tool. Pre-fix the typed exception was " + "swallowed by the except NullRunTransportError arm and " + "rewrapped as NullRunBlockedException(NR-B00X)." + ) + + def test_pass_through_arm_appears_before_blocked_arm(self): + body = _enforce_sensitive_tool_body() + # Order matters: the pass-through arm must come BEFORE + # ``except NullRunBlockedException`` because Python evaluates + # except arms top-to-bottom. If a future refactor moves it + # after, the typed exception would still be caught by the + # next arm (it isn't a NullRunBlockedException, so this is + # defensive — but the contract is "before blocked arm"). + nr_ex01_idx = body.find("except NullRunExecutionNotFoundError:") + blocked_idx = body.find("except NullRunBlockedException:") + assert nr_ex01_idx != -1, ( + "DEF-NR-EX01-REWRAP-LOSS: pass-through arm missing" + ) + assert blocked_idx != -1, ( + "DEF-NR-EX01-REWRAP-LOSS: NullRunBlockedException arm missing" + ) + assert nr_ex01_idx < blocked_idx, ( + "DEF-NR-EX01-REWRAP-LOSS: the pass-through arm must " + "appear BEFORE the except NullRunBlockedException arm. " + "Pre-fix order swallowed the typed exception via the " + "NullRunTransportError arm below." + ) + + def test_pass_through_arm_only_raises(self): + body = _enforce_sensitive_tool_body() + # Locate the arm and verify it ONLY contains ``raise`` — no + # error_code stamping, no reason prefixing, no rewrap. Strip + # comment lines first so the explanatory comment (which + # legitimately names NullRunBlockedException to explain what + # WOULD happen without the fix) does not trip the check. + m = re.search( + r"except NullRunExecutionNotFoundError:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, ( + "DEF-NR-EX01-REWRAP-LOSS: could not parse the pass-through " + "arm body" + ) + arm_body = m.group(1) + # Drop comment-only lines for the negative assertion; the + # comment block legitimately references NullRunBlockedException + # to explain the regression we're guarding against. + executable_lines = [ + ln for ln in arm_body.splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + # The arm MUST contain `raise` and the executable body MUST + # NOT rewrap into NullRunBlockedException. + assert "raise" in executable, ( + "DEF-NR-EX01-REWRAP-LOSS: pass-through arm must re-raise " + "(not swallow). Empty arm would silently drop the typed " + "exception." + ) + assert "NullRunBlockedException" not in executable, ( + "DEF-NR-EX01-REWRAP-LOSS: pass-through arm must NOT " + "rewrap into NullRunBlockedException. Pre-fix this was the " + "exact bug — NullRunExecutionNotFoundError was being " + "unwrapped into NullRunBlockedException(NR-B00X)." + ) + + def test_pass_through_arm_comment_tag_present(self): + body = _enforce_sensitive_tool_body() + # The fix introduced a long comment naming + # DEF-NR-EX01-REWRAP-LOSS. Pin so a future maintainer who + # deletes the comment is forced to read the code's history. + assert "DEF-NR-EX01-REWRAP-LOSS" in body, ( + "DEF-NR-EX01-REWRAP-LOSS: the explainer comment block must " + "name the fix tag so future readers can grep for it." + ) + + def test_import_includes_nullrun_execution_not_found_error(self): + src = _read(DECORATORS_PY) + # The function-local import block at line ~809 must include + # NullRunExecutionNotFoundError; otherwise NameError at + # runtime even though the except arm is present. + assert "NullRunExecutionNotFoundError" in src, ( + "DEF-NR-EX01-REWRAP-LOSS: NullRunExecutionNotFoundError " + "must be imported in decorators.py for the pass-through " + "arm to bind. Check the function-local import block " + "(around line 809)." + ) + + +# ─── Behavioral tests (mirror test_protect.py:651 style) ────────────── + + +class TestDefNrEx01Behavior: + """Pin the runtime behavior — the typed exception propagates with + error_code + execution_id + regate_required intact.""" + + def _mock_runtime_raising(self, exc: Exception) -> MagicMock: + rt = MagicMock() + rt.is_sensitive_tool.return_value = True + rt.execute.side_effect = exc + return rt + + def test_execution_not_found_propagates_unchanged(self): + """The core fix: NullRunExecutionNotFoundError reaches the + caller WITHOUT being rewrapped.""" + exc = NullRunExecutionNotFoundError( + "execution binding not found", + execution_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + endpoint="/api/v1/execute", + status_code=404, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunExecutionNotFoundError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # The exact same instance must propagate (identity check) — + # no rewrap, no chained from. + assert excinfo.value is exc, ( + "DEF-NR-EX01-REWRAP-LOSS: NullRunExecutionNotFoundError " + "must propagate unchanged. A rewrap would have replaced " + "the instance with a NullRunBlockedException." + ) + + def test_execution_not_found_preserves_error_code(self): + """error_code must remain NR-EX01, not NR-B00X.""" + exc = NullRunExecutionNotFoundError( + "execution binding not found", + execution_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + endpoint="/api/v1/execute", + status_code=404, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunExecutionNotFoundError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value.error_code == "NR-EX01", ( + f"DEF-NR-EX01-REWRAP-LOSS: error_code must remain NR-EX01 " + f"on the propagated exception; got {excinfo.value.error_code!r}. " + "Pre-fix the rewrap stamped NR-B001/B002 from the " + "TransportErrorSource mapping." + ) + + def test_execution_not_found_preserves_execution_id_attr(self): + """Cookbook recovery depends on ``exc.execution_id`` being + readable. Pre-fix this attr was lost in the rewrap because + NullRunBlockedException doesn't carry an ``execution_id`` + first-class attribute.""" + exc = NullRunExecutionNotFoundError( + "execution binding not found", + execution_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + endpoint="/api/v1/execute", + status_code=404, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunExecutionNotFoundError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value.execution_id == "01a08b57-f176-79ce-ad1b-60b0184d1625", ( + "DEF-NR-EX01-REWRAP-LOSS: exc.execution_id must be " + "preserved for the cookbook recovery path (re-issue " + "/api/v1/gate, then retry /execute)." + ) + assert excinfo.value.regate_required is True, ( + "DEF-NR-EX01-REWRAP-LOSS: exc.regate_required must be " + "True so callers can branch on 're-issue /gate' vs other " + "recovery paths." + ) + + def test_execution_not_found_format_user_message_returns_nr_ex01_line(self): + """The NR-EX01 catalog line ('There's a configuration issue. + Please contact support.') must be reachable through + ``format_user_message`` after the @protect pass-through.""" + from nullrun.messages import format_user_message + + exc = NullRunExecutionNotFoundError( + "execution binding not found", + execution_id="01a08b57-f176-79ce-ad1b-60b0184d1625", + endpoint="/api/v1/execute", + status_code=404, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunExecutionNotFoundError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + msg = format_user_message(excinfo.value) + assert "configuration issue" in msg.lower(), ( + f"DEF-NR-EX01-REWRAP-LOSS: format_user_message must yield " + f"the NR-EX01 catalog line ('There's a configuration " + f"issue. Please contact support.'). Got: {msg!r}. " + "Pre-fix the rewrap yielded NR-B002 'Our service is " + "temporarily unavailable. Please try again shortly.' — " + "misleading, suggests retry will help when the binding " + "is permanently gone for this execution_id." + ) + + def test_other_transport_errors_still_rewrap_to_blocked(self): + """Regression guard: the fix must NOT make ALL transport + errors pass through — only the typed NR-EX01 one. Generic + NullRunTransportError must still be rewrapped as + NullRunBlockedException(NR-B00X).""" + from nullrun.breaker.exceptions import TransportErrorSource + + exc = NullRunTransportError( + "network blip", + source=TransportErrorSource.NETWORK_ERROR, + endpoint="/execute", + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # Must NOT be NullRunExecutionNotFoundError — generic + # transport failures still get the B001 rewrap. + assert not isinstance(excinfo.value, NullRunExecutionNotFoundError) + assert "NETWORK_ERROR" in excinfo.value.reason, ( + "DEF-NR-EX01-REWRAP-LOSS regression: generic " + "NullRunTransportError must still be rewrapped as " + "NullRunBlockedException with the transport source in " + "the reason. The fix was scoped to NR-EX01 only — it " + "must not silently widen the pass-through to all " + "NullRunTransportError subclasses." + ) + + def test_blocked_exception_still_passes_through(self): + """Regression guard: the existing ``except NullRunBlockedException`` + arm must keep working. Adding the new arm above it must not + intercept the existing block-propagation path.""" + exc = NullRunBlockedException(workflow_id="wf-1", reason="denied by policy") + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert "denied by policy" in excinfo.value.reason diff --git a/tests/test_2026_09_10_r001_passthrough.py b/tests/test_2026_09_10_r001_passthrough.py new file mode 100644 index 0000000..6dbd533 --- /dev/null +++ b/tests/test_2026_09_10_r001_passthrough.py @@ -0,0 +1,391 @@ +"""DEF-NR-R001-REWRAP-LOSS (2026-09-10) — RateLimitError must propagate +through ``@protect`` / ``_enforce_sensitive_tool`` with retry_after, +upgrade_url, body, and error_code=NR-R001 intact. + +Pre-fix (audit 2026-09-10): + - ``nullrun/decorators.py::_enforce_sensitive_tool`` had four except + arms around the ``runtime.execute(...)`` call: a specific arm for + NullRunExecutionNotFoundError (defense), NullRunBlockedException + (pass-through), NullRunTransportError (rewrap via source -> NR-B00X + code mapping), and Exception (catch-all NR-B001 rewrap). + - ``RateLimitError`` (NR-R001, the typed 429 envelope) is a + subclass of ``NullRunTransportError``. The MRO puts it inside the + second arm, so the typed exception was being unwrapped into a + generic ``NullRunBlockedException(error_code="NR-B002", + reason="policy engine unavailable: GATEWAY_ERROR")``. + - User-visible symptom (per cookbook ``register_sensitive_tools`` + + @protect @sensitive flow that hits a 429 gateway response): + The SDK prints "Our service is temporarily unavailable. Please + try again shortly." (NR-B002) instead of the documented NR-R001 + line "The NullRun backend rate-limited this API key. Wait + ``retry_after`` seconds (or upgrade the plan) before retrying." + The ``exc.retry_after`` attribute was lost, blocking the + documented "sleep retry_after then retry" cookbook pattern. + - Cookbook pattern ``except RateLimitError`` never matched because + the exception class was lost in the rewrap. + +Post-fix: + - Added a dedicated pass-through arm BEFORE + ``except NullRunBlockedException`` so the typed exception + propagates with error_code=NR-R001, retry_after (seconds, from + the gateway's ``retry_after_ms`` body field), upgrade_url (plan + upgrade URL from the 429 body), and body (parsed 429 envelope) + intact. + +These tests pin BOTH the source shape AND the runtime behavior so a +future refactor that re-introduces a rewrap (e.g. reorders the except +arms) fails the test. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from nullrun.breaker.exceptions import ( + NullRunBackendError, + NullRunBlockedException, + NullRunTransportError, + RateLimitError, + TransportErrorSource, +) +from nullrun.decorators import _enforce_sensitive_tool + +SDK_ROOT = Path(__file__).resolve().parent.parent +DECORATORS_PY = SDK_ROOT / "src" / "nullrun" / "decorators.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _enforce_sensitive_tool_body() -> str: + """Return the source of ``_enforce_sensitive_tool`` so source-pin + tests can grep for the expected arms / ordering without depending + on Python AST parsing.""" + src = _read(DECORATORS_PY) + m = re.search( + r"def _enforce_sensitive_tool\(.*?\n(?=def |\nclass |\Z)", + src, + re.DOTALL, + ) + assert m, "could not locate _enforce_sensitive_tool body" + return m.group(0) + + +# ─── Source-pin tests (mirror cancel.rs / orchestrator.rs pin style) ─── + + +class TestDefNrR001SourcePin: + """Pin the shape of the fix so a refactor that reorders / removes + the pass-through arm fails loudly.""" + + def test_pass_through_arm_is_present(self): + body = _enforce_sensitive_tool_body() + assert "except RateLimitError:" in body, ( + "DEF-NR-R001-REWRAP-LOSS: the pass-through arm for " + "RateLimitError must be present in " + "_enforce_sensitive_tool. Pre-fix the typed exception " + "was swallowed by the except NullRunTransportError arm " + "and rewrapped as NullRunBlockedException(NR-B002)." + ) + + def test_pass_through_arm_appears_before_transport_rewrap_arm(self): + body = _enforce_sensitive_tool_body() + # Order matters: the pass-through arm must come BEFORE + # ``except NullRunTransportError as exc:`` because Python + # evaluates except arms top-to-bottom. If a future refactor + # moves it after, RateLimitError would still be caught by the + # NullRunTransportError parent arm and rewrapped. + r001_idx = body.find("except RateLimitError:") + transport_idx = body.find("except NullRunTransportError") + assert r001_idx != -1, ( + "DEF-NR-R001-REWRAP-LOSS: pass-through arm missing" + ) + assert transport_idx != -1, ( + "DEF-NR-R001-REWRAP-LOSS: NullRunTransportError arm missing" + ) + assert r001_idx < transport_idx, ( + "DEF-NR-R001-REWRAP-LOSS: the RateLimitError pass-through " + "arm must appear BEFORE the except NullRunTransportError " + "rewrap arm. Pre-fix order swallowed the typed exception." + ) + + def test_pass_through_arm_only_raises(self): + body = _enforce_sensitive_tool_body() + # Locate the arm and verify it ONLY contains ``raise`` — no + # error_code stamping, no reason prefixing, no rewrap. Strip + # comment lines first so the explanatory comment (which + # legitimately names NullRunBlockedException to explain what + # WOULD happen without the fix) does not trip the check. + m = re.search( + r"except RateLimitError:\s*\n(.*?)(?=\n except |\Z)", + body, + re.DOTALL, + ) + assert m, ( + "DEF-NR-R001-REWRAP-LOSS: could not parse the pass-through " + "arm body" + ) + arm_body = m.group(1) + # Drop comment-only lines for the negative assertion. + executable_lines = [ + ln for ln in arm_body.splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + executable = "\n".join(executable_lines) + # The arm MUST contain `raise` and the executable body MUST + # NOT rewrap into NullRunBlockedException. + assert "raise" in executable, ( + "DEF-NR-R001-REWRAP-LOSS: pass-through arm must re-raise " + "(not swallow). Empty arm would silently drop the typed " + "exception." + ) + assert "NullRunBlockedException" not in executable, ( + "DEF-NR-R001-REWRAP-LOSS: pass-through arm must NOT " + "rewrap into NullRunBlockedException. Pre-fix this was " + "the exact bug — RateLimitError was being unwrapped into " + "NullRunBlockedException(NR-B002)." + ) + + def test_pass_through_arm_comment_tag_present(self): + body = _enforce_sensitive_tool_body() + # The fix introduced a long comment naming + # DEF-NR-R001-REWRAP-LOSS. Pin so a future maintainer who + # deletes the comment is forced to read the code's history. + assert "DEF-NR-R001-REWRAP-LOSS" in body, ( + "DEF-NR-R001-REWRAP-LOSS: the explainer comment block " + "must name the fix tag so future readers can grep for it." + ) + + def test_import_includes_rate_limit_error(self): + src = _read(DECORATORS_PY) + # The function-local import block at line ~809 must include + # RateLimitError; otherwise NameError at runtime even though + # the except arm is present. + assert "RateLimitError" in src, ( + "DEF-NR-R001-REWRAP-LOSS: RateLimitError must be imported " + "in decorators.py for the pass-through arm to bind. Check " + "the function-local import block (around line 809)." + ) + + +# ─── Behavioral tests (mirror test_protect.py:651 style) ───────────── + + +class TestDefNrR001Behavior: + """Pin the runtime behavior — the typed exception propagates with + error_code + retry_after + upgrade_url + body intact.""" + + def _mock_runtime_raising(self, exc: Exception) -> MagicMock: + rt = MagicMock() + rt.is_sensitive_tool.return_value = True + rt.execute.side_effect = exc + return rt + + def test_rate_limit_propagates_unchanged(self): + """The core fix: RateLimitError reaches the caller WITHOUT + being rewrapped.""" + exc = RateLimitError( + "rate limited", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="/api/v1/execute", + retry_after=30.0, + upgrade_url="https://app.nullrun.io/upgrade?key=abc", + body={"error": "RATE_LIMIT_EXCEEDED", "retry_after_ms": 30000}, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(RateLimitError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # The exact same instance must propagate (identity check) — + # no rewrap, no chained from. + assert excinfo.value is exc, ( + "DEF-NR-R001-REWRAP-LOSS: RateLimitError must propagate " + "unchanged. A rewrap would have replaced the instance " + "with a NullRunBlockedException." + ) + + def test_rate_limit_preserves_error_code(self): + """error_code must remain NR-R001, not NR-B002.""" + exc = RateLimitError( + "rate limited", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="/api/v1/execute", + retry_after=30.0, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(RateLimitError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value.error_code == "NR-R001", ( + f"DEF-NR-R001-REWRAP-LOSS: error_code must remain NR-R001 " + f"on the propagated exception; got " + f"{excinfo.value.error_code!r}. Pre-fix the rewrap stamped " + f"NR-B002 from the GATEWAY_ERROR -> NR-B002 source mapping." + ) + + def test_rate_limit_preserves_retry_after(self): + """Cookbook recovery depends on ``exc.retry_after`` being + readable. Pre-fix this attr was lost in the rewrap because + NullRunBlockedException doesn't carry a ``retry_after`` + first-class attribute (the FastAPI integration reads it via + ``getattr(exc, "retry_after")`` to set the HTTP ``Retry-After`` + header — a silent failure if the attr is missing).""" + exc = RateLimitError( + "rate limited", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="/api/v1/execute", + retry_after=42.5, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(RateLimitError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value.retry_after == 42.5, ( + "DEF-NR-R001-REWRAP-LOSS: exc.retry_after must be " + "preserved for the cookbook recovery path (sleep " + "retry_after seconds then retry /gate + /execute)." + ) + + def test_rate_limit_preserves_upgrade_url_and_body(self): + """Cookbook / FastAPI integration reads ``exc.upgrade_url`` to + surface a billing-upgrade prompt and ``exc.body`` for + diagnostics. Both must survive the @protect flow.""" + body = {"error": "RATE_LIMIT_EXCEEDED", "retry_after_ms": 30000} + exc = RateLimitError( + "rate limited", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="/api/v1/execute", + retry_after=30.0, + upgrade_url="https://app.nullrun.io/upgrade?plan=pro", + body=body, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(RateLimitError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value.upgrade_url == "https://app.nullrun.io/upgrade?plan=pro", ( + "DEF-NR-R001-REWRAP-LOSS: exc.upgrade_url must be " + "preserved — FastAPI handler reads it for the upgrade " + "prompt surface." + ) + assert excinfo.value.body == body, ( + "DEF-NR-R001-REWRAP-LOSS: exc.body must be preserved " + "for diagnostics." + ) + + def test_rate_limit_format_user_message_returns_nr_r001_line(self): + """The NR-R001 catalog line ('rate-limited ... wait + retry_after seconds ... or upgrade the plan') must be + reachable through ``format_user_message`` after the @protect + pass-through.""" + from nullrun.messages import format_user_message + + exc = RateLimitError( + "rate limited", + source=TransportErrorSource.GATEWAY_ERROR, + endpoint="/api/v1/execute", + retry_after=30.0, + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(RateLimitError) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + msg = format_user_message(excinfo.value) + # The NR-R001 catalog line is rate-limit-themed + # ("Too many requests. Please wait a moment and try again.") + # and must NOT be the NR-B002 gateway-error line + # ("Our service is temporarily unavailable. Please try again + # shortly."), which would imply retry regardless of the + # gateway's retry_after / upgrade hint. + msg_lower = msg.lower() + assert "temporarily unavailable" not in msg_lower, ( + f"DEF-NR-R001-REWRAP-LOSS: format_user_message yielded " + f"the NR-B002 gateway-error line (which contains " + f"'temporarily unavailable'). Got: {msg!r}. The typed " + f"exception is being mapped through the " + f"NullRunTransportError rewrap instead of " + f"format_user_message reading the typed " + f"error_code=NR-R001 directly." + ) + # The NR-R001 catalog carries a rate-limit-themed phrase so + # the user understands the cause is the API-key rate limit, + # not the backend being down. + assert ( + "too many requests" in msg_lower + or "rate-limit" in msg_lower + or "rate limit" in msg_lower + or "retry" in msg_lower + ), ( + f"DEF-NR-R001-REWRAP-LOSS: format_user_message must yield " + f"a rate-limit-themed NR-R001 line. Got: {msg!r}." + ) + + def test_generic_transport_errors_still_rewrap_to_blocked(self): + """Regression guard: the fix must NOT make ALL transport + errors pass through — only the typed RateLimitError one. + Generic NullRunTransportError must still be rewrapped as + NullRunBlockedException(NR-B00X), preserving the existing + fail-CLOSED contract for unclassified transport failures.""" + exc = NullRunTransportError( + "network blip", + source=TransportErrorSource.NETWORK_ERROR, + endpoint="/execute", + ) + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + # Must NOT be RateLimitError — generic transport failures + # still get the B001 rewrap from the NETWORK_ERROR source. + assert not isinstance(excinfo.value, RateLimitError) + assert "NETWORK_ERROR" in excinfo.value.reason, ( + "DEF-NR-R001-REWRAP-LOSS regression: generic " + "NullRunTransportError must still be rewrapped as " + "NullRunBlockedException with the transport source in " + "the reason. The fix was scoped to RateLimitError only." + ) + + def test_blocked_exception_still_passes_through(self): + """Regression guard: the existing ``except NullRunBlockedException`` + arm must keep working. Adding the new pass-through arm above + it must not intercept the existing block-propagation path.""" + exc = NullRunBlockedException(workflow_id="wf-1", reason="denied by policy") + rt = self._mock_runtime_raising(exc) + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert excinfo.value is exc + assert "denied by policy" in excinfo.value.reason + + def test_execution_not_found_still_passes_through(self): + """Regression guard: the prior DEF-NR-EX01-REWRAP-LOSS fix + (pass-through for ``except NullRunExecutionNotFoundError``) + must keep working. The new ``except RateLimitError:`` arm + inserted between this arm and ``except NullRunBlockedException`` + must not break the MRO ordering for NullRunBackendError + subclasses (NullRunExecutionNotFoundError is one).""" + exc = NullRunBackendError( + "5xx blip", + endpoint="/api/v1/execute", + status_code=503, + ) + # Note: this isn't a NullRunExecutionNotFoundError — it's the + # parent NullRunBackendError (5xx). Verify the parent still + # rewraps via the NullRunTransportError generic path with + # GATEWAY_ERROR source -> NR-B002, NOT pass-through. + rt = self._mock_runtime_raising(exc) + # NullRunBackendError IS a NullRunTransportError — pre-fix + # it would have hit the rewrap arm (since the specific + # NullRunExecutionNotFoundError arm only matched the leaf). + # Post-fix it should STILL hit the rewrap (since this test + # exercises the parent, not the typed leaf). The + # NullRunExecutionNotFoundError-specific pass-through is + # covered separately by the existing NR-EX01 test file. + with pytest.raises(NullRunBlockedException) as excinfo: + _enforce_sensitive_tool(rt, lambda x: x, (1,), {}) + assert not isinstance(excinfo.value, RateLimitError) + assert excinfo.value.error_code == "NR-B002", ( + "DEF-NR-R001-REWRAP-LOSS regression: NullRunBackendError " + "with GATEWAY_ERROR source must still rewrap as " + "NullRunBlockedException(NR-B002). The new " + "RateLimitError pass-through arm must not widen the " + "pass-through to all NullRunTransportError subclasses." + ) diff --git a/tests/test_2026_09_10_runtime_block_typed_dispatch.py b/tests/test_2026_09_10_runtime_block_typed_dispatch.py new file mode 100644 index 0000000..6815a86 --- /dev/null +++ b/tests/test_2026_09_10_runtime_block_typed_dispatch.py @@ -0,0 +1,356 @@ +"""DEF-NR-RUNTIME-BLOCK-TYPED (2026-09-10) — ``Runtime.execute`` +block path MUST dispatch via ``_V3_ERROR_CODE_MAP`` so typed catalog +exceptions (``NullRunApprovalReplayRejectedError`` / NR-A015, +``NullRunBudgetError`` / NR-B004, ``NullRunToolBlockedError`` / +NR-T001, etc.) actually surface — NOT a base +``NullRunBlockedException`` with the wire SCREAMING_SNAKE code +attached as ``error_code``. + +Pre-fix (runtime.py:3053-3140, before this commit): + + When ``self._transport.execute(**execute_kwargs)`` returned + ``{"decision": "block", "details": {"error_code": + "APPROVAL_REPLAY_REJECTED"}}``, the runtime ALWAYS raised the + base ``NullRunBlockedException`` with + ``error_code="APPROVAL_REPLAY_REJECTED"`` (the wire code) — a + SCREAMING_SNAKE string, NOT the catalog ``NR-A015`` that + ``format_user_message`` looks up. Cookbook recipes that branched + on the typed catalog arm (``except + NullRunApprovalReplayRejectedError:``) NEVER matched, fell + through to the generic ``NullRunError`` arm, and the user saw + ``FALLBACK_MESSAGE`` ("Something went wrong. Please try again.") + instead of the typed catalog wording. + +Post-fix (this commit): + + Layer-1 dispatch factored into + ``Runtime._build_block_exception``. The helper imports + ``_V3_ERROR_CODE_MAP`` from ``nullrun.transport`` and dispatches + via ``typed_cls = _V3_ERROR_CODE_MAP.get(wire_error_code)``. If + the wire code is in the catalog, the helper raises the typed + class (e.g. ``NullRunApprovalReplayRejectedError`` for + ``APPROVAL_REPLAY_REJECTED``), so cookbook ``except`` arms + match. The class's ``error_code`` attribute is the catalog + ``NR-A015`` (NOT the wire code), so ``format_user_message`` + yields the friendly catalog wording. + +These tests pin BOTH the source shape (the runtime block path +imports + uses ``_V3_ERROR_CODE_MAP``) AND the runtime behavior +(wire-coded reasons propagate as the typed catalog class). + +Wire payload convention (preserved by the fix): the constructor +captures ``**details`` into ``self.details`` and nests the wire +payload under ``self.details["details"]`` via the ``details=...`` +kwarg. Cookbook code reads ``exc.details["details"]["..."]`` for +typed introspection; ``exc.details["details"]["mapped_class"]`` +exposes the catalog class name as a back-compat shim. +""" +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from nullrun.breaker.exceptions import ( + NullRunApprovalReplayRejectedError, + NullRunBlockedException, + NullRunBudgetError, + NullRunToolBlockedError, +) +from nullrun.runtime import NullRunRuntime + +SDK_ROOT = Path(__file__).resolve().parent.parent +RUNTIME_PY = SDK_ROOT / "src" / "nullrun" / "runtime.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _build_block_exception_slice() -> str: + """Return the source of ``_build_block_exception`` for the + source-pin tests. Anchor on ``def _build_block_exception`` and + walk to the next sibling ``def`` (4-space indent) inside the + same class.""" + src = _read(RUNTIME_PY) + start = src.find(" def _build_block_exception(") + assert start != -1, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: cannot locate " + "Runtime._build_block_exception" + ) + after_header = start + len(" def _build_block_exception(\n") + m = re.search( + r"^ (?:def |@|class )", + src[after_header:], + re.MULTILINE, + ) + assert m, "could not locate end of _build_block_exception body" + end = after_header + m.start() + return src[start:end] + + +# ─── Source-pin tests ────────────────────────────────────────────────────── + + +class TestDefNrRuntimeBlockTypedSourcePin: + """Pin the shape of the fix so a refactor that re-introduces + the always-NRError wrap fails loudly.""" + + def test_helper_imports_v3_error_code_map(self): + """The helper MUST import ``_V3_ERROR_CODE_MAP`` from + ``nullrun.transport`` so it can dispatch typed catalog + classes.""" + body = _build_block_exception_slice() + assert "from nullrun.transport import _V3_ERROR_CODE_MAP" in body, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: _build_block_exception " + "must import _V3_ERROR_CODE_MAP from nullrun.transport. " + "Pre-fix the runtime always raised base " + "NullRunBlockedException with the wire code as " + "error_code, hiding the typed catalog from cookbook " + "recipes." + ) + + def test_helper_dispatches_typed_cls(self): + """The helper MUST look up + ``typed_cls = _V3_ERROR_CODE_MAP.get(wire_error_code)`` + and instantiate it for the wire-coded reason. Pre-fix + it always instantiated the base class.""" + body = _build_block_exception_slice() + assert re.search( + r"typed_cls\s*=\s*_V3_ERROR_CODE_MAP\.get\(", body + ), ( + "DEF-NR-RUNTIME-BLOCK-TYPED: helper must dispatch via " + "`_V3_ERROR_CODE_MAP.get(wire_error_code)` so the " + "wire-coded reason maps to the typed catalog class." + ) + assert "typed_cls(" in body, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: helper must instantiate " + "the typed class — pre-fix it only constructed the " + "base NullRunBlockedException." + ) + + def test_helper_does_not_overwrite_catalog_code_with_wire_code(self): + """Pre-fix, the runtime passed + ``error_code=block_code=wire_error_code`` — overriding the + catalog ``NR-A015`` with the wire + ``APPROVAL_REPLAY_REJECTED``. The fix must NOT pass + ``error_code=wire_error_code`` to the typed class. The + typed class's class attribute (e.g. ``NR-A015``) must + stay intact so ``format_user_message`` can look it up.""" + body = _build_block_exception_slice() + m = re.search(r"return\s+typed_cls\(", body) + assert m is not None, "typed_cls(...) return not found" + # Walk forward from the return to the closing paren (allow + # nested parens for type_specific_kwargs etc.). + call_start = m.end() - len("typed_cls(") + depth = 0 + end = None + for i in range(call_start, len(body)): + ch = body[i] + if ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 0: + end = i + 1 + break + assert end is not None, "could not find end of typed_cls(...) call" + call_region = body[call_start:end] + assert "error_code=" not in call_region, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: typed_cls(...) call must " + "NOT pass error_code=... — that would override the " + "typed class's catalog code (NR-A015, NR-B004, etc.) " + "with the wire SCREAMING_SNAKE code and defeat " + "format_user_message." + ) + + +# ─── Behaviour tests (pin the runtime outcome of the fix) ───────────────── + + +class TestDefNrRuntimeBlockTypedBehavior: + """Verify that a wire-coded ``decision: block`` with + ``details.error_code`` in the catalog dispatch path raises the + typed catalog class, NOT a base NullRunBlockedException.""" + + def test_approval_replay_rejected_raises_typed(self): + """A /execute response with ``decision: block`` and + ``details.error_code: APPROVAL_REPLAY_REJECTED`` must + raise ``NullRunApprovalReplayRejectedError`` (NR-A015), + NOT the base ``NullRunBlockedException``.""" + result = { + "decision": "block", + "explanation": "approval grant already consumed", + "details": { + "error_code": "APPROVAL_REPLAY_REJECTED", + "approval_id": "apr-test-123", + }, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + assert isinstance(exc, NullRunApprovalReplayRejectedError), ( + f"expected NullRunApprovalReplayRejectedError, got " + f"{type(exc).__name__}" + ) + assert exc.error_code == "NR-A015", ( + f"expected NR-A015 (catalog), got {exc.error_code!r} " + "(wire code?)" + ) + assert exc.approval_id == "apr-test-123", ( + f"expected approval_id forwarded from wire_details, " + f"got {exc.approval_id!r}" + ) + # CRITICAL: must NOT be the base class + assert type(exc) is not NullRunBlockedException, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: wire APPROVAL_REPLAY_REJECTED " + "must surface as NullRunApprovalReplayRejectedError, not " + "the base NullRunBlockedException." + ) + + def test_budget_hard_blocked_raises_typed(self): + """A /execute response with + ``details.error_code: BUDGET_HARD_BLOCKED`` must raise + ``NullRunBudgetError`` (NR-B004), not the base.""" + result = { + "decision": "block", + "explanation": "Hard budget exceeded", + "details": { + "error_code": "BUDGET_HARD_BLOCKED", + "budget_cents": 5000, + "current_spend_cents": 5100, + }, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + assert isinstance(exc, NullRunBudgetError) + assert exc.error_code == "NR-B004" + # The wire payload is nested under exc.details["details"] + # (back-compat convention — the constructor captures the + # ``details=`` kwarg as ``self.details["details"]``). + # Cookbook code reads budget_cents / current_spend_cents via + # exc.details["details"]; only + # NullRunBudgetRecheckFailedError (NR-B006) promotes them + # to first-class attributes. + wire_payload = exc.details.get("details") or {} + assert wire_payload.get("budget_cents") == 5000 + assert wire_payload.get("current_spend_cents") == 5100 + assert wire_payload.get("mapped_class") == "NullRunBudgetError" + + def test_tool_blocked_raises_typed(self): + """A /execute response with + ``details.error_code: TOOL_BLOCKED`` must raise + ``NullRunToolBlockedError``, not the base.""" + result = { + "decision": "block", + "explanation": "Tool bash is blocked", + "details": {"error_code": "TOOL_BLOCKED"}, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="bash", + ) + assert isinstance(exc, NullRunToolBlockedError) + assert exc.error_code == "NR-T001" + + def test_unknown_wire_code_falls_back_to_base(self): + """A wire code that is NOT in ``_V3_ERROR_CODE_MAP`` (drift + between backend and SDK) must still surface on the base + ``NullRunBlockedException`` with the wire code as + ``error_code`` — the operator / cookbook code can still + branch on ``exc.error_code``.""" + result = { + "decision": "block", + "explanation": "Unknown rejection", + "details": {"error_code": "BRAND_NEW_CODE_FROM_BACKEND"}, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + assert type(exc) is NullRunBlockedException + # The wire code (NOT a catalog code) is the + # ``error_code`` for drift visibility. + assert exc.error_code == "BRAND_NEW_CODE_FROM_BACKEND" + # mapped_class shim is preserved for back-compat + wire_payload = exc.details.get("details") or {} + assert wire_payload.get("mapped_class") == "NullRunBlockedException" + + def test_legacy_keyword_path_budget(self): + """A /execute response with no wire code but with + ``explanation: 'budget exceeded'`` must fall back to the + legacy keyword path. Pre-fix the comment said this raised + ``NullRunBudgetError`` but the construction always used + the base ``NullRunBlockedException``; this test pins that + the legacy path stays on the base class (the wire-code + path is the one that dispatches via ``_V3_ERROR_CODE_MAP``) + while ``error_code`` is set to ``NR-B004`` for back-compat + branches on ``exc.error_code == "NR-B004"``.""" + result = { + "decision": "block", + "explanation": "Your budget was exceeded", + "details": {}, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + assert type(exc) is NullRunBlockedException + assert exc.error_code == "NR-B004" + wire_payload = exc.details.get("details") or {} + assert wire_payload.get("mapped_class") == "NullRunBudgetError" + + def test_legacy_keyword_path_unknown_explanation_falls_back_to_x001(self): + """A /execute response with no wire code AND no keyword + match falls through to ``NR-X001`` on the base class.""" + result = { + "decision": "block", + "explanation": "Some unparseable reason", + "details": {}, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + assert type(exc) is NullRunBlockedException + assert exc.error_code == "NR-X001" + + def test_format_user_message_yields_catalog_wording(self): + """End-to-end: with the fix in place, + ``format_user_message(NullRunApprovalReplayRejectedError)`` + returns the friendly NR-A015 wording — NOT the generic + FALLBACK_MESSAGE that the user saw before the fix.""" + from nullrun.messages import FALLBACK_MESSAGE, format_user_message + + result = { + "decision": "block", + "explanation": "approval grant already consumed", + "details": { + "error_code": "APPROVAL_REPLAY_REJECTED", + "approval_id": "apr-test-123", + }, + } + exc = NullRunRuntime._build_block_exception( + result=result, + workflow_id="wf-abc", + tool_name="refund_customer", + ) + msg = format_user_message(exc) + assert msg != FALLBACK_MESSAGE, ( + "DEF-NR-RUNTIME-BLOCK-TYPED: format_user_message must " + "yield the catalog wording, not FALLBACK_MESSAGE. " + "Pre-fix the runtime hid the typed exception behind a " + "base NullRunBlockedException(error_code='APPROVAL_REPLAY_REJECTED'), " + "which the catalog could not resolve." + ) + assert "approval has already been used" in msg diff --git a/tests/test_2026_09_10_sdk_cleanup.py b/tests/test_2026_09_10_sdk_cleanup.py new file mode 100644 index 0000000..27984b2 --- /dev/null +++ b/tests/test_2026_09_10_sdk_cleanup.py @@ -0,0 +1,311 @@ +"""Source-pin regression tests for the SDK cleanup batch (2026-09-10). + +Each test pins a single cleanup fix to prevent future refactors from +silently re-introducing the debt that was removed. Mirrors the +source-pin pattern from ``test_2026_08_11_fixes.py``. + +Defects / cleanups being pinned (RUN_ID 2026-09-10 batch): + +- CLEANUP-PARSE-CODE-COLLISION — ``_safe_json`` previously raised + with ``error_code="NR-T001"``, which collides with + ``NullRunToolBlockedError.error_code`` (breaker/exceptions.py:955). + Cookbook handlers that branch on ``exc.error_code == "NR-T001"`` + mis-classified a JSON parse failure as a tool block. Pin: the + literal must be NR-T-PARSE. + +- CLEANUP-REDIS-UNAVAILABLE-CODE — ``transport.py`` previously + mapped the backend's ``REDIS_UNAVAILABLE`` envelope to + ``NullRunBudgetError``. That's wrong: the backend distinguishes + BUDGET_REDIS_UNAVAILABLE (budget path, fail-CLOSED 402) and + RATE_LIMIT_REDIS_UNAVAILABLE (rate-limit path, fail-CLOSED 429). + Lumping both into ``NullRunBudgetError`` conflated the two and + hid rate-limit-Redis outages from operators. Pin: the literal + mapping must be gone (replaced by per-path branches elsewhere). + +- CLEANUP-BUDGET-THROTTLE-ZOMBIE — ``NullRunBudgetThrottleError`` + (NR-B007) was a zombie exception class. ``runtime.py:2116`` + raises ``WorkflowPausedException`` on ``decision == "throttle"``, + so ``NullRunBudgetThrottleError`` never fired on any wire or + runtime path. Catalog + class removed to keep the exception + surface in sync with what the SDK actually raises. + +- CLEANUP-WORKFLOW-KILLED-DISCOVERABILITY — + ``NullRunWorkflowKilledError`` was the only kill-related typed + exception missing from ``__init__._LAZY_EXPORTS`` and + ``__init__.__all__``. Asymmetric with ``WorkflowKilledInterrupt`` + which was already exported. Pin: must be importable from top + level ``nullrun``. +""" + +from __future__ import annotations + +import os +import re + +RUNTIME_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "runtime.py" +) +TRANSPORT_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "transport.py" +) +EXCEPTIONS_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "breaker", "exceptions.py" +) +MESSAGES_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "messages.py" +) +INIT_PATH = os.path.join( + os.path.dirname(__file__), "..", "src", "nullrun", "__init__.py" +) + + +def _read(path: str) -> str: + return open(path, encoding="utf-8").read() + + +def _strip_comment_lines(src: str) -> str: + """Drop ``#`` comment lines so source-pin tests checking for + forbidden user-facing wording don't trip on rationale comments + that legitimately mention the same word or code.""" + return "\n".join( + line for line in src.splitlines() if not line.lstrip().startswith("#") + ) + + +# ─── CLEANUP-PARSE-CODE-COLLISION ────────────────────────────────────────── + + +def test_safe_json_uses_nr_t_parse_not_nr_t001(): + """``_safe_json`` must raise with ``error_code="NR-T-PARSE"``. + + Pre-cleanup, the literal was ``NR-T001``, which collides with + ``NullRunToolBlockedError.error_code`` (breaker/exceptions.py). + Cookbook handlers that branch on ``exc.error_code == "NR-T001"`` + mis-classified a JSON parse failure as a tool block. The + post-cleanup literal NR-T-PARSE is dedicated transport-class + code; matches NR-T (transport) vocabulary without colliding + with NR-T001 / NR-T002 / etc. + """ + src = _read(TRANSPORT_PATH) + assert 'error_code="NR-T-PARSE"' in src, ( + "transport.py:_safe_json must raise with " + "error_code='NR-T-PARSE' (avoids collision with NR-T001 / " + "NullRunToolBlockedError)" + ) + # Negative pin: the pre-cleanup literal must NOT appear + # anywhere in transport.py. Code-only strip isn't necessary + # here — the literal is always inside a string in production + # code (assignments to ``error_code=``), never inside a comment + # that explains the cleanup (we have inline rationale elsewhere + # that mentions the old code, but as prose not a string literal). + code_only = _strip_comment_lines(src) + assert 'error_code="NR-T001"' not in code_only, ( + "transport.py must NOT contain the pre-cleanup literal " + "error_code='NR-T001' — that code belongs to " + "NullRunToolBlockedError (breaker/exceptions.py). The cleanup " + "renamed the transport-side literal to NR-T-PARSE." + ) + + +# ─── CLEANUP-REDIS-UNAVAILABLE-CODE ─────────────────────────────────────── + + +def test_transport_no_longer_maps_redis_unavailable_to_budget_error(): + """``transport.py`` must NOT have a blanket mapping from + ``REDIS_UNAVAILABLE`` to ``NullRunBudgetError``. + + Pre-cleanup, the mapping was ``"REDIS_UNAVAILABLE": NullRunBudgetError``. + That's wrong because the backend distinguishes: + - BUDGET_REDIS_UNAVAILABLE (budget path, fail-CLOSED 402) + - RATE_LIMIT_REDIS_UNAVAILABLE (rate-limit path, fail-CLOSED 429) + Lumping both into ``NullRunBudgetError`` conflated the two and + hid rate-limit-Redis outages from operators. + + Post-cleanup the literal mapping is removed; per-path branches + elsewhere in transport.py route the correct code to the right + typed exception (e.g. ``NullRunBudgetRedisError`` / ``NullRunRateLimitRedisError``). + """ + src = _read(TRANSPORT_PATH) + code_only = _strip_comment_lines(src) + assert '"REDIS_UNAVAILABLE": NullRunBudgetError' not in code_only, ( + "transport.py must not contain the pre-cleanup literal " + "'REDIS_UNAVAILABLE': NullRunBudgetError. The blanket mapping " + "conflated BUDGET_REDIS_UNAVAILABLE (budget path) with " + "RATE_LIMIT_REDIS_UNAVAILABLE (rate-limit path); per-path " + "branches must route each to its own typed exception." + ) + # Negative pin: also confirm the dict entry style (with single + # quotes variant) isn't sneaking in via formatter churn. + assert "'REDIS_UNAVAILABLE': NullRunBudgetError" not in code_only, ( + "transport.py must not contain the single-quote variant of " + "the pre-cleanup REDIS_UNAVAILABLE mapping." + ) + + +# ─── CLEANUP-BUDGET-THROTTLE-ZOMBIE ─────────────────────────────────────── + + +def test_null_run_budget_throttle_error_class_is_removed(): + """``NullRunBudgetThrottleError`` (NR-B007) must be removed from + ``breaker/exceptions.py``. + + Pre-cleanup this class existed with ``error_code = "NR-B007"`` and + ``retryable = True``. But ``runtime.py:2116`` raises + ``WorkflowPausedException`` on ``decision == "throttle"``, so + the class never fired on any wire or runtime path — a zombie + exception that the catalog had to maintain anyway. + + Post-cleanup: class removed, NR-B007 catalog entry removed, + test_format_user_message_handles_budget_throttle removed. + """ + src = _read(EXCEPTIONS_PATH) + code_only = _strip_comment_lines(src) + assert "class NullRunBudgetThrottleError" not in code_only, ( + "breaker/exceptions.py must NOT define NullRunBudgetThrottleError " + "— it was a zombie class never raised on a wire or runtime path. " + "See CLEANUP-BUDGET-THROTTLE-ZOMBIE." + ) + # Negative pin: the class's error_code literal must also be gone. + # (Other classes may still reference the string "NR-B007" in + # comments or string formatting, but no `error_code = "NR-B007"` + # class attribute on a NullRun*Error subclass should remain.) + assert re.search( + r"error_code\s*=\s*[\"']NR-B007[\"']", + code_only, + ) is None, ( + "No exception class in breaker/exceptions.py may declare " + "error_code='NR-B007' — NullRunBudgetThrottleError was removed " + "and the code is reserved-but-unused." + ) + + +def test_messages_catalog_no_longer_has_nr_b007_entry(): + """``messages.DEFAULT_MESSAGES`` must NOT contain a NR-B007 entry. + + Companion to ``test_null_run_budget_throttle_error_class_is_removed``: + the catalog entry for NR-B007 must be removed in lockstep so the + formatter doesn't return a stale message for a code that's no + longer raised. + """ + src = _read(MESSAGES_PATH) + code_only = _strip_comment_lines(src) + # The catalog uses dict-literal style: "NR-B007": "...", ... + # A bare re.search for the string key catches any formatting + # variant (single quote, trailing comma, etc.). + assert re.search(r"[\"']NR-B007[\"']\s*:", code_only) is None, ( + "messages.py DEFAULT_MESSAGES must NOT contain a key for " + "NR-B007 — the corresponding exception class was removed " + "and a stale catalog entry would mislead operators." + ) + + +def test_test_messages_no_longer_has_budget_throttle_test(): + """``tests/test_messages.py`` must NOT have a + ``test_format_user_message_handles_budget_throttle`` test. + + Companion cleanup: the orphan test for the zombie class was + removed along with the class. Pin the removal so a copy-paste + doesn't restore the test for a class that no longer exists. + """ + test_path = os.path.join( + os.path.dirname(__file__), "test_messages.py" + ) + src = _read(test_path) + code_only = _strip_comment_lines(src) + assert "test_format_user_message_handles_budget_throttle" not in code_only, ( + "tests/test_messages.py must not contain " + "test_format_user_message_handles_budget_throttle — the " + "orphan test for the zombie NullRunBudgetThrottleError was " + "removed when the class was deleted." + ) + + +# ─── CLEANUP-WORKFLOW-KILLED-DISCOVERABILITY ────────────────────────────── + + +def test_null_run_workflow_killed_error_importable_from_top_level(): + """``NullRunWorkflowKilledError`` must be importable from the + top-level ``nullrun`` namespace. + + Pre-cleanup, this class was the only kill-related typed + exception missing from ``__init__._LAZY_EXPORTS`` and + ``__init__.__all__``. Asymmetric with ``WorkflowKilledInterrupt`` + (already exported). Pin the top-level discoverability so host + code can ``from nullrun import NullRunWorkflowKilledError`` + alongside the other typed kill exceptions. + """ + src = _read(INIT_PATH) + # Pin: the class must appear in the _LAZY_EXPORTS dict + # (lazy export via __getattr__). The lazy-export form is the + # post-0.15.0 pattern; the older eager `from nullrun.breaker.X + # import ...` form has been migrated to lazy across the SDK. + assert re.search( + r"[\"']NullRunWorkflowKilledError[\"']\s*:\s*\(", + src, + ), ( + "nullrun/__init__.py _LAZY_EXPORTS must register " + "NullRunWorkflowKilledError for top-level import. Pre-cleanup " + "the class was discoverable only via " + "nullrun.breaker.exceptions, asymmetric with " + "WorkflowKilledInterrupt." + ) + # Pin: the class must also appear in __all__ so tab-completion + # surfaces it via dir(nullrun). + assert re.search( + r"__all__\s*=\s*\[[\s\S]*?[\"']NullRunWorkflowKilledError[\"']", + src, + ), ( + "nullrun/__init__.py __all__ must include " + "'NullRunWorkflowKilledError' for dir(nullrun) tab-completion " + "to surface the class." + ) + + +# ─── Behavioural smoke tests ────────────────────────────────────────────── + + +def test_null_run_budget_throttle_error_no_longer_importable(): + """Runtime check: importing the zombie class must fail. + + Companion to the source-pin test above — verifies the class is + actually gone from the live module, not just that the literal + text was deleted from the source file (someone could delete + the text but leave an aliased re-export, for example). + """ + import nullrun + import nullrun.breaker.exceptions as exc_mod + + assert not hasattr(exc_mod, "NullRunBudgetThrottleError"), ( + "nullrun.breaker.exceptions.NullRunBudgetThrottleError must " + "be removed at runtime; hasattr returning True means the " + "class survived the cleanup." + ) + # Also: must not be re-exported from top-level nullrun + assert not hasattr(nullrun, "NullRunBudgetThrottleError"), ( + "nullrun.NullRunBudgetThrottleError must NOT exist (top-level " + "re-export of the zombie class would defeat the cleanup)." + ) + + +def test_null_run_workflow_killed_error_importable_at_runtime(): + """Runtime check: ``from nullrun import NullRunWorkflowKilledError`` + must succeed. + """ + import nullrun + from nullrun.breaker import exceptions as exc_mod + + # Top-level import path (the new discoverability surface) + top_level_cls = getattr(nullrun, "NullRunWorkflowKilledError", None) + assert top_level_cls is not None, ( + "nullrun.NullRunWorkflowKilledError must be importable " + "from the top-level namespace after the CLEANUP-WORKFLOW-" + "KILLED-DISCOVERABILITY fix." + ) + # Must be the same class object as the one in breaker.exceptions + # (not a separate wrapper or proxy class). + assert top_level_cls is exc_mod.NullRunWorkflowKilledError, ( + "nullrun.NullRunWorkflowKilledError must be the same class " + "object as nullrun.breaker.exceptions.NullRunWorkflowKilledError; " + "a separate proxy class would defeat isinstance() checks " + "across the codebase." + ) diff --git a/tests/test_2026_09_10_toolblocked_parser.py b/tests/test_2026_09_10_toolblocked_parser.py new file mode 100644 index 0000000..64ad983 --- /dev/null +++ b/tests/test_2026_09_10_toolblocked_parser.py @@ -0,0 +1,399 @@ +"""DEF-NR-TOOLBLOCKED-PARSER (2026-09-10) — TOOL_BLOCKED + +LOOP_DETECTED + 5 sibling catalog entries that map to +NullRunBlockedException subclasses were silently swallowed by the +parser's catalog-fallback branch. + +Pre-fix (audit 2026-09-10): + - ``nullrun/transport.py::_parse_v3_error_envelope`` had a + final catalog-fallback branch at line ~2780: + + ``allowed = {"error_code", "user_action", ...}; forwarded = ...`` + ``instance = catalog(full_message, **forwarded)`` + + - This generic fallback assumed ``catalog.__init__`` accepts + a string as the first positional arg (the message). It + worked for ``NullRunError``-base subclasses (Protocol, + RateLimitRedis, Auth) which have ``(message, **kwargs)`` + signature. + - It FAILED for ``NullRunBlockedException`` subclasses which + require positional ``(workflow_id, reason, ...)`` — the + string ``full_message`` ended up in ``workflow_id``, the + ``reason`` arg was missing, ``TypeError`` was raised. + - The TypeError escaped the parser and was caught by the + catch-all ``except Exception: pass`` in Transport.execute + (4xx handler) — surfacing the synthetic-block dict + ``{"decision": "block", "explanation": "Gateway returned + 403"}`` instead of the typed NR-T001 / NR-Lxxx catalog + line. + - Affected catalog entries: TOOL_BLOCKED, LOOP_DETECTED, + MODEL_REQUIRED, POLICY_UNCONFIGURED, TOO_MANY_PENDING_APPROVALS, + BUSINESS_IMPACT_INVALID, VALIDATION_FAILED. + - User-visible symptom: a /execute call returning + ``{"error_code": "TOOL_BLOCKED", "details": {"workflow_id": + "wf-1", "tool_name": "dangerous.tool"}}`` yielded a synthetic + block dict with no NR-code, no ``tool_name`` to recover, no + catalog line ("This tool is in the workflow's block list. + Remove it ..."). + +Post-fix: + - Added a dedicated dispatch branch BEFORE the final catalog + fallback. Detects ``catalog is NullRunToolBlockedError`` or + ``catalog is NullRunBlockedException`` and calls the + constructor with the right (workflow_id, reason, status_code, + tool_name) signature. ``forwarded`` is passed through so + catalog value's defaults win. + +These tests pin BOTH the source shape AND the runtime behavior +so a future refactor that reverts to the broken generic fallback +fails the test. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import httpx +import pytest +import respx + +from nullrun.breaker.exceptions import ( + NullRunBlockedException, + NullRunToolBlockedError, +) +from nullrun.transport import Transport, _parse_v3_error_envelope + +SDK_ROOT = Path(__file__).resolve().parent.parent +TRANSPORT_PY = SDK_ROOT / "src" / "nullrun" / "transport.py" + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +def _v3_envelope(error_code: str, status: int = 400, **details) -> httpx.Response: + body = { + "error_code": error_code, + "error_message": f"Backend says {error_code}", + "details": details, + } + return httpx.Response(status, json=body) + + +_EXECUTE_URL = "https://api.test.nullrun.io/api/v1/execute" + + +@pytest.fixture +def transport(): + t = Transport( + api_url="https://api.test.nullrun.io", + api_key="test-key-12345678", + ) + yield t + t.stop() + + +def _execute_kwargs(): + return dict( + organization_id="ws-123", + execution_id="exec-" + "a" * 32, + trace_id="trace-789", + tool="my.tool", + input_data={}, + on_transport_error="raise", + fallback_mode="strict", + ) + + +# ─── Source-pin tests ───────────────────────────────────────────── + + +class TestDefNrToolblockedParserSourcePin: + """Pin the shape of the fix so a refactor that removes the + dedicated dispatch branch fails loudly.""" + + def test_dedicated_branch_present(self): + src = _read(TRANSPORT_PY) + # The fix added an ``if catalog is NullRunToolBlockedError + # or catalog is NullRunBlockedException:`` branch BEFORE + # the generic ``catalog(full_message, **forwarded)`` + # fallback. + assert re.search( + r"catalog\s+is\s+NullRunToolBlockedError\s*\n\s+or\s+catalog\s+is\s+NullRunBlockedException", + src, + ), ( + "DEF-NR-TOOLBLOCKED-PARSER: the dedicated " + "NullRunToolBlockedError / NullRunBlockedException " + "dispatch branch must be present in " + "_parse_v3_error_envelope. Pre-fix the generic " + "catalog-fallback called " + "``catalog(full_message, **forwarded)`` which " + "raised TypeError because " + "NullRunBlockedException.__init__ requires positional " + "(workflow_id, reason)." + ) + + def test_branch_in_parser(self): + """Pin that the fix lives in ``_parse_v3_error_envelope``, + not somewhere else (defense against a refactor that moves + it to a different layer where it can't intercept the + TypeError).""" + src = _read(TRANSPORT_PY) + # Locate the _parse_v3_error_envelope function body and + # confirm the dedicated branch lives inside it. + fn_match = re.search( + r"def _parse_v3_error_envelope\(.*?(?=\ndef |\nclass |\Z)", + src, + re.DOTALL, + ) + assert fn_match, "could not locate _parse_v3_error_envelope" + fn_body = fn_match.group(0) + assert "NullRunToolBlockedError" in fn_body, ( + "DEF-NR-TOOLBLOCKED-PARSER: NullRunToolBlockedError " + "must be referenced inside _parse_v3_error_envelope " + "(the dedicated dispatch branch lives there)." + ) + + def test_import_includes_blocked_exception_classes(self): + """The function-local import block in + ``_parse_v3_error_envelope`` must include both + ``NullRunToolBlockedError`` and + ``NullRunBlockedException`` — otherwise NameError at + runtime even though the branch is present.""" + src = _read(TRANSPORT_PY) + # Locate the function-local import block (the one inside + # _parse_v3_error_envelope, NOT the module-level one). + fn_match = re.search( + r"def _parse_v3_error_envelope\(.*?(?=\ndef |\nclass |\Z)", + src, + re.DOTALL, + ) + assert fn_match + fn_body = fn_match.group(0) + # Find the first ``from nullrun.breaker.exceptions import`` + # inside the function body. + import_block = re.search( + r"from nullrun\.breaker\.exceptions import \((.*?)\)", + fn_body, + re.DOTALL, + ) + assert import_block, ( + "DEF-NR-TOOLBLOCKED-PARSER: could not locate " + "function-local import block inside " + "_parse_v3_error_envelope" + ) + imported = import_block.group(1) + assert "NullRunToolBlockedError" in imported, ( + "DEF-NR-TOOLBLOCKED-PARSER: NullRunToolBlockedError " + "must be imported in the function-local block — the " + "dedicated dispatch branch references it." + ) + assert "NullRunBlockedException" in imported, ( + "DEF-NR-TOOLBLOCKED-PARSER: NullRunBlockedException " + "must be imported in the function-local block — the " + "dedicated dispatch branch references it." + ) + + def test_branch_comment_tag_present(self): + """The fix introduced a long comment naming + DEF-NR-TOOLBLOCKED-PARSER. Pin so a future maintainer who + deletes the comment is forced to read the code's + history.""" + src = _read(TRANSPORT_PY) + assert "DEF-NR-TOOLBLOCKED-PARSER" in src, ( + "DEF-NR-TOOLBLOCKED-PARSER: the explainer comment " + "block must name the fix tag so future readers can " + "grep for it." + ) + + def test_branch_uses_correct_constructor_signature(self): + """The dedicated branch must call + ``catalog(workflow_id=..., reason=..., status_code=..., + tool_name=..., **forwarded)`` — not the broken + ``catalog(full_message, **forwarded)`` form for these + classes.""" + src = _read(TRANSPORT_PY) + # Extract the body of the new branch (between + # ``catalog is NullRunToolBlockedError`` and the next + # ``return cast(Exception, instance)``). + m = re.search( + r"if \(\s*\n\s*catalog is NullRunToolBlockedError.*?" + r"return cast\(Exception, instance\)", + src, + re.DOTALL, + ) + assert m, ( + "DEF-NR-TOOLBLOCKED-PARSER: could not parse the " + "dedicated branch body" + ) + body = m.group(0) + # Must pass workflow_id as kwarg, NOT as positional. + assert "workflow_id=str(details.get(" in body, ( + "DEF-NR-TOOLBLOCKED-PARSER: dedicated branch must " + "pass workflow_id via the details payload " + "(``workflow_id=str(details.get('workflow_id') " + "or 'unknown')``)." + ) + assert "reason=full_message" in body, ( + "DEF-NR-TOOLBLOCKED-PARSER: dedicated branch must " + "pass ``reason=full_message`` — the message is the " + "second positional arg in NullRunBlockedException." + ) + assert "status_code=status" in body, ( + "DEF-NR-TOOLBLOCKED-PARSER: dedicated branch must " + "pass status_code so the typed exception carries " + "the wire status (403 for TOOL_BLOCKED)." + ) + # Must NOT call catalog(full_message, ...) directly + # (that's the broken generic-fallback signature). + assert "catalog(\n full_message" not in body, ( + "DEF-NR-TOOLBLOCKED-PARSER: dedicated branch must " + "NOT use ``catalog(full_message, ...)`` — that's " + "the broken form that raises TypeError for " + "NullRunBlockedException subclasses." + ) + + +# ─── Behavioral tests (parser-level) ────────────────────────────── + + +class TestDefNrToolblockedParserBehavior: + """Pin the runtime behavior — each catalog entry now yields + the correct typed exception with first-class attrs.""" + + def test_tool_blocked_yields_tool_blocked_error(self): + body = _v3_envelope( + "TOOL_BLOCKED", status=403, + workflow_id="wf-1", + tool_name="dangerous.tool", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunToolBlockedError), ( + f"DEF-NR-TOOLBLOCKED-PARSER: TOOL_BLOCKED must yield " + f"NullRunToolBlockedError (NR-T001). Got " + f"{type(exc).__name__}. Pre-fix the parser raised " + f"TypeError which was swallowed by the catch-all " + f"``except Exception: pass`` in Transport.execute." + ) + assert exc.error_code == "NR-T001" + assert exc.tool_name == "dangerous.tool", ( + "DEF-NR-TOOLBLOCKED-PARSER: NullRunToolBlockedError." + "tool_name must be preserved for the cookbook " + "recovery contract." + ) + assert exc.workflow_id == "wf-1" + assert exc.status_code == 403 + + def test_loop_detected_yields_blocked_exception(self): + body = _v3_envelope( + "LOOP_DETECTED", status=403, + workflow_id="wf-loop-1", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException) + assert exc.workflow_id == "wf-loop-1" + assert exc.status_code == 403 + + def test_model_required_yields_blocked_exception(self): + body = _v3_envelope( + "MODEL_REQUIRED", status=403, + workflow_id="wf-model-1", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException) + assert exc.workflow_id == "wf-model-1" + + def test_policy_unconfigured_yields_blocked_exception(self): + body = _v3_envelope( + "POLICY_UNCONFIGURED", status=403, + workflow_id="wf-unconfigured", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException) + assert exc.workflow_id == "wf-unconfigured" + + def test_too_many_pending_approvals_yields_blocked_exception(self): + body = _v3_envelope( + "TOO_MANY_PENDING_APPROVALS", status=403, + workflow_id="wf-busy-approvals", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException) + assert exc.workflow_id == "wf-busy-approvals" + + def test_business_impact_invalid_yields_blocked_exception(self): + body = _v3_envelope( + "BUSINESS_IMPACT_INVALID", status=400, + workflow_id="wf-impact-bad", + ) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException) + assert exc.workflow_id == "wf-impact-bad" + assert exc.status_code == 400 + + def test_validation_failed_with_no_workflow_id_defaults_to_unknown(self): + """When ``workflow_id`` is missing from details (e.g. a + bare VALIDATION_FAILED envelope), the parser must still + produce a typed exception — defaulted to ``"unknown"`` + rather than raising TypeError or swallowing to a + synthetic block.""" + body = _v3_envelope("VALIDATION_FAILED", status=400) + exc = _parse_v3_error_envelope(body, "execute") + assert isinstance(exc, NullRunBlockedException), ( + f"DEF-NR-TOOLBLOCKED-PARSER: VALIDATION_FAILED must " + f"yield NullRunBlockedException even without " + f"workflow_id in details. Got {type(exc).__name__}." + ) + assert exc.workflow_id == "unknown", ( + "DEF-NR-TOOLBLOCKED-PARSER: missing workflow_id " + "must default to ``'unknown'`` (mirrors the " + "NullRunBudgetError pattern at line ~2729)." + ) + assert exc.status_code == 400 + + +# ─── Behavioral tests (end-to-end through Transport.execute) ──────── + + +class TestDefNrToolblockedTransportEndToEnd: + """Pin the end-to-end runtime behavior — Transport.execute + propagates the typed exception instead of swallowing it into + a synthetic block dict. Pre-fix the TypeError from the + parser escaped the catch-fan-in (no typed arm matched + ``TypeError``) and got caught by ``except Exception: pass``, + returning ``{"decision": "block", "decision_source": + "fallback", "explanation": "Gateway returned 403"}``.""" + + @respx.mock + def test_tool_blocked_does_not_swallow_to_synthetic_block( + self, transport, + ): + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "TOOL_BLOCKED", status=403, + workflow_id="wf-1", + tool_name="dangerous.tool", + ) + ) + # Pre-fix this returned a synthetic-block dict. Post-fix + # the catch-fan-in's ``except NullRunBlockedException: + # raise`` arm re-raises the typed exception. + with pytest.raises(NullRunToolBlockedError) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.error_code == "NR-T001" + assert excinfo.value.tool_name == "dangerous.tool" + assert excinfo.value.workflow_id == "wf-1" + + @respx.mock + def test_loop_detected_does_not_swallow_to_synthetic_block( + self, transport, + ): + respx.post(_EXECUTE_URL).mock( + return_value=_v3_envelope( + "LOOP_DETECTED", status=403, + workflow_id="wf-loop", + ) + ) + with pytest.raises(NullRunBlockedException) as excinfo: + transport.execute(**_execute_kwargs()) + assert excinfo.value.workflow_id == "wf-loop" diff --git a/tests/test_messages.py b/tests/test_messages.py index 3300ca9..fc39e62 100644 --- a/tests/test_messages.py +++ b/tests/test_messages.py @@ -31,16 +31,39 @@ "NR-0000", "NR-A001", "NR-A003", + "NR-A010", + "NR-A011", + "NR-A012", + "NR-A013", + "NR-A014", + "NR-A015", + "NR-A016", # B.1 (2026-09-10): APPROVAL_DB_* sibling family "NR-B001", "NR-B002", + "NR-B004", "NR-B005", - "NR-R001", + "NR-B006", + # NR-B007 removed 2026-09-10: NullRunBudgetThrottleError was a + # zombie class never raised on a wire or runtime path + # (runtime.py:2116 raises WorkflowPausedException on + # decision=="throttle"). Catalog entry + exception class removed + # to keep messages in sync with the exception module. + "NR-CH001", "NR-C000", - "NR-X001", - "NR-B004", + "NR-EX01", + "NR-L001", + "NR-MCP01", # B.1 (2026-09-10): MCP umbrella destructive + "NR-MCP02", # B.1 (2026-09-10): MCP umbrella readonly bypass + "NR-MCP03", # B.1 (2026-09-10): MCP umbrella approval required + "NR-O001", + "NR-P001", + "NR-R001", + "NR-R002", "NR-T001", "NR-W002", "NR-W003", + "NR-W004", + "NR-X001", } @@ -159,6 +182,56 @@ def test_format_user_message_handles_workflow_paused(): assert out == messages.DEFAULT_MESSAGES["NR-W003"] +def test_format_user_message_handles_approval_expired(): + """NR-A012 catalog entry (added 2026-09-08 to close the silent + state-flip audit gap). Pre-fix, ``NullRunApprovalExpiredError`` + raised but the catalog was missing NR-A012, so + ``format_user_message`` fell through to + ``FALLBACK_MESSAGE = "Something went wrong. Please try again."`` — + exactly what ``langgraph_openai_approval_demo.py`` printed. + + This test pins the catalog entry so a future refactor that + removes NR-A012 from ``DEFAULT_MESSAGES`` re-introduces the + demo's user-visible bug. + """ + expired = exc.NullRunApprovalExpiredError( + workflow_id="wf-1", + reason="WS push silent past approval_timeout_seconds", + approval_id="appr-1", + timeout_seconds=5.0, + local_timeout=True, + ) + assert expired.error_code == "NR-A012" + out = messages.format_user_message(expired) + assert out == messages.DEFAULT_MESSAGES["NR-A012"] + assert out != messages.FALLBACK_MESSAGE + # Tone rule: imperative when there's something to do. + assert "try again" in out.lower() + + +def test_format_user_message_handles_approval_expired_local_timeout_path(): + """Both raise paths for ``NullRunApprovalExpiredError`` (wire path + + local-timeout path) must resolve to NR-A012. Cookbook code that + catches the exception regardless of origin needs a consistent + user-facing message.""" + wire = exc.NullRunApprovalExpiredError( + workflow_id="wf-1", + reason="APPROVAL_EXPIRED", + approval_id="appr-1", + timeout_seconds=None, + local_timeout=False, + ) + local = exc.NullRunApprovalExpiredError( + workflow_id="wf-1", + reason="WS push silent past approval_timeout_seconds", + approval_id="appr-1", + timeout_seconds=5.0, + local_timeout=True, + ) + assert messages.format_user_message(wire) == messages.DEFAULT_MESSAGES["NR-A012"] + assert messages.format_user_message(local) == messages.DEFAULT_MESSAGES["NR-A012"] + + def test_format_user_message_handles_workflow_killed_baseexception(): """``WorkflowKilledInterrupt`` is a BaseException subclass. The formatter must still resolve it via the inherited ``error_code`` @@ -170,6 +243,145 @@ class attribute on ``WorkflowKilledException`` (the deprecated assert out == messages.DEFAULT_MESSAGES["NR-W002"] +# --------------------------------------------------------------------------- +# Edge-case catalog coverage (added 2026-09-09 alongside NR-A012 fix) +# --------------------------------------------------------------------------- +# Each test below pins a specific NR-XXXX code so a future refactor that +# silently removes the catalog entry re-introduces the user-visible +# "Something went wrong. Please try again." fallback bug. Pre-fix these +# exceptions raised but their codes had no DEFAULT_MESSAGES entry; the +# formatter silently fell through to FALLBACK_MESSAGE. +def test_format_user_message_handles_approval_pending(): + """NR-A010: approval pending. Distinct from NR-A012 (expired); the + actionable verb is ``wait`` not ``try again``.""" + pending = exc.NullRunApprovalNotYetApprovedError( + workflow_id="wf-1", reason="APPROVAL_NOT_YET_APPROVED", approval_id="appr-1" + ) + assert pending.error_code == "NR-A010" + out = messages.format_user_message(pending) + assert out == messages.DEFAULT_MESSAGES["NR-A010"] + assert out != messages.FALLBACK_MESSAGE + + +def test_format_user_message_handles_approval_denied(): + """NR-A011: operator denied. Terminal — same approval_id cannot be + re-used. Wording must NOT say 'try again' alone (cookbook pattern + requires a fresh approval_id).""" + denied = exc.NullRunApprovalDeniedError( + workflow_id="wf-1", reason="APPROVAL_DENIED", approval_id="appr-1" + ) + assert denied.error_code == "NR-A011" + out = messages.format_user_message(denied) + assert out == messages.DEFAULT_MESSAGES["NR-A011"] + assert out != messages.FALLBACK_MESSAGE + + +def test_format_user_message_handles_approval_digest_mismatch_action(): + """NR-A013: business-impact digest mismatch (different action + approved than the one bound to this execution).""" + mismatch = exc.NullRunApprovalDigestMismatchError( + workflow_id="wf-1", reason="APPROVAL_DIGEST_MISMATCH" + ) + assert mismatch.error_code == "NR-A013" + out = messages.format_user_message(mismatch) + assert out == messages.DEFAULT_MESSAGES["NR-A013"] + + +def test_format_user_message_handles_approval_digest_mismatch_tool(): + """NR-A014: tool capability digest mismatch (MCP tools/list + refreshed between /gate and /execute).""" + mismatch = exc.NullRunApprovalToolDigestMismatchError( + workflow_id="wf-1", reason="APPROVAL_TOOL_DIGEST_MISMATCH" + ) + assert mismatch.error_code == "NR-A014" + out = messages.format_user_message(mismatch) + assert out == messages.DEFAULT_MESSAGES["NR-A014"] + + +def test_format_user_message_handles_approval_replay_rejected(): + """NR-A015: approval grant already consumed. Replay/retry-loop + signal — NOT transient; the same approval_id will never succeed + twice.""" + replay = exc.NullRunApprovalReplayRejectedError( + workflow_id="wf-1", reason="APPROVAL_REPLAY_REJECTED", approval_id="appr-1" + ) + assert replay.error_code == "NR-A015" + out = messages.format_user_message(replay) + assert out == messages.DEFAULT_MESSAGES["NR-A015"] + + +def test_format_user_message_handles_workflow_inactive(): + """NR-W004: workflow soft-deleted / killed on the server. Distinct + from NR-W002 (BaseException kill path that bypasses handle()) and + NR-W003 (pause / cooldown). End-user copy is similar to NR-W002 + because the user-visible outcome is the same.""" + inactive = exc.NullRunWorkflowInactiveError( + "workflow soft-deleted", workflow_id="wf-1" + ) + assert inactive.error_code == "NR-W004" + out = messages.format_user_message(inactive) + assert out == messages.DEFAULT_MESSAGES["NR-W004"] + assert out != messages.FALLBACK_MESSAGE + + +def test_format_user_message_handles_budget_recheck_failed(): + """NR-B006: post-approval budget re-check race — another execution + spent the budget between /gate and /execute. Retryable.""" + race = exc.NullRunBudgetRecheckFailedError( + "BUDGET_RECHECK_FAILED", + current_spend_cents=500, + budget_cents=400, + ) + assert race.error_code == "NR-B006" + out = messages.format_user_message(race) + assert out == messages.DEFAULT_MESSAGES["NR-B006"] + + +def test_format_user_message_handles_consume_overbudget(): + """NR-O001: consume > reserve + ε tolerance (ADR-005 invariant; + SDK rejects rather than silently re-reserving).""" + overbudget = exc.NullRunConsumeOverbudgetError( + "CONSUME_OVERBUDGET", + execution_id="exec-1", + reserved_cents=100, + max_allowed_cents=101, + actual_cost_cents=150, + ) + assert overbudget.error_code == "NR-O001" + out = messages.format_user_message(overbudget) + assert out == messages.DEFAULT_MESSAGES["NR-O001"] + + +def test_format_user_message_handles_protocol_error(): + """NR-P001: wire-protocol version below backend minimum. End-user + action is 'contact support' (host code needs an SDK upgrade).""" + proto = exc.NullRunProtocolError("protocol mismatch") + assert proto.error_code == "NR-P001" + out = messages.format_user_message(proto) + assert out == messages.DEFAULT_MESSAGES["NR-P001"] + + +def test_format_user_message_handles_chain_error(): + """NR-CH001: chain context invalid (unknown chain_id, wrong org, or + exceeded max_duration). End-user must start a fresh conversation.""" + chain = exc.NullRunChainError("CHAIN_NOT_FOUND", chain_id="chain-1") + assert chain.error_code == "NR-CH001" + out = messages.format_user_message(chain) + assert out == messages.DEFAULT_MESSAGES["NR-CH001"] + + +def test_format_user_message_handles_rate_limit_redis_error(): + """NR-R002: rate-limit Redis unreachable. Fail-CLOSED — distinct + from NR-R001 (per-workflow soft rate limit hit). User-facing copy + mirrors NR-B001 / NR-B002 because the operator-side fix is the + same (restore Redis) and the user-facing difference between 'rate + limit hit' and 'rate limit Redis down' is operator-internal.""" + rl = exc.NullRunRateLimitRedisError("redis down") + assert rl.error_code == "NR-R002" + out = messages.format_user_message(rl) + assert out == messages.DEFAULT_MESSAGES["NR-R002"] + + def test_format_user_message_falls_back_for_object_without_error_code(): """Plain objects (no ``error_code`` attribute) get the fallback.""" class NotAnError: diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 326019a..286e986 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -193,6 +193,18 @@ def test_execute_blocked_surfaces_wire_error_code(self, make_runtime, mock_api): # "Approval infrastructure unavailable: validation error during # approval row creation" as the generic NR-X001 — the very # bug the journal test surfaced). + # + # Post-fix (2026-09-10 batch, ``_V3_ERROR_CODE_MAP`` dispatch): + # ``APPROVAL_VALIDATION_FAILED`` is now in the typed catalog + # map (transport.py:2996) and resolves to + # ``NullRunApprovalDbUnavailableError`` (catalog code NR-A016). + # The catalog code (NOT the wire SCREAMING_SNAKE string) is + # the canonical ``format_user_message`` lookup key — see + # ``runtime._build_block_exception`` docstring. The wire code + # is preserved verbatim on ``exc.details["details"]["error_code"]`` + # for routing/alerting. + from nullrun.breaker.exceptions import NullRunApprovalDbUnavailableError + respx.post(f"{BASE_URL}/api/v1/execute").mock( return_value=httpx.Response( 200, @@ -215,8 +227,11 @@ def test_execute_blocked_surfaces_wire_error_code(self, make_runtime, mock_api): rt = make_runtime() with pytest.raises(NullRunBlockedException) as exc_info: rt.execute(tool_name="refund_customer", input_data={}, mode="strict") - # The wire code wins — no keyword guessing. - assert exc_info.value.error_code == "APPROVAL_VALIDATION_FAILED" + # Catalog typed class wins — not the keyword-guessing fallback. + assert isinstance(exc_info.value, NullRunApprovalDbUnavailableError) + # The catalog error_code (NR-A016) is canonical for + # format_user_message; the wire code stays in details. + assert exc_info.value.error_code == "NR-A016" # The structured payload is preserved on details so a caller # can introspect ``decision_source`` for routing/alerting. # ``NullRunBlockedException.__init__`` wraps ``**details`` so @@ -224,9 +239,10 @@ def test_execute_blocked_surfaces_wire_error_code(self, make_runtime, mock_api): wire_details = exc_info.value.details.get("details") or {} assert wire_details.get("error_code") == "APPROVAL_VALIDATION_FAILED" assert wire_details.get("decision_source") == "approval_create_failed" - # Back-compat shim: the legacy ``mapped_class`` field is still - # populated so any caller that branched on it pre-fix keeps working. - assert wire_details.get("mapped_class") == "NullRunBlockedException" + # Back-compat shim: the legacy ``mapped_class`` field exposes + # the typed catalog class name (NR-A016 / NullRunApprovalDbUnavailableError) + # so any caller that branched on it pre-fix keeps working. + assert wire_details.get("mapped_class") == "NullRunApprovalDbUnavailableError" @pytest.mark.skip( reason=( @@ -703,17 +719,33 @@ def test_execute_auto_sensitive_routes_to_strict(): assert call_args.kwargs["mode"] == "strict" -def test_execute_auto_non_sensitive_routes_to_inline(): - """Auto + non-sensitive tool → mode=inline → local short-circuit - so transport.execute is NOT called. Verify via the LOCAL decision_source. +def test_execute_auto_non_sensitive_routes_to_strict(): + """DEF-TS12-01 (2026-09-10): ``mode="auto"`` with a non-sensitive + tool now ALWAYS resolves to ``mode="strict"`` and contacts the + gateway. + + Pre-fix this was a CRITICAL fail-OPEN: ``mode="auto"`` with a + non-sensitive tool silently switched to ``mode="inline"`` which + returned a synthetic local allow WITHOUT contacting the gateway. + Every operator-configured budget, rate-limit, and tool-block + policy was silently bypassed for non-sensitive tools. The + dashboard showed policies in effect; the SDK ignored them. + + Post-fix the cloud-only invariant from CLAUDE.md §17 and + memory `cloud-only-invariant-sdk` holds: every call routes + through /execute when ``mode="auto"`` (the default). The + ``mode="inline"`` opt-in is preserved for callers who + explicitly want to skip /execute — see + ``test_execute_inline_mode_short_circuits_local`` below for + the explicit opt-in pin. """ rt = _make_test_runtime() rt._transport.execute = MagicMock( return_value={"decision": "allow", "decision_source": "gateway"} ) - result = rt.execute("safe.tool", {"x": 1}) - assert result["decision_source"] == "local" - rt._transport.execute.assert_not_called() + rt.execute("safe.tool", {"x": 1}) + rt._transport.execute.assert_called_once() + assert rt._transport.execute.call_args.kwargs["mode"] == "strict" def test_execute_auto_sensitive_calls_transport(): diff --git a/tests/test_runtime_branches.py b/tests/test_runtime_branches.py index 7e3f500..6606c74 100644 --- a/tests/test_runtime_branches.py +++ b/tests/test_runtime_branches.py @@ -258,17 +258,21 @@ def test_execute_auto_sensitive_routes_to_strict(): assert call_args.kwargs["mode"] == "strict" -def test_execute_auto_non_sensitive_routes_to_inline(): - """Auto + non-sensitive tool → mode=inline → local short-circuit - so transport.execute is NOT called. Verify via the LOCAL decision_source. +def test_execute_auto_non_sensitive_routes_to_strict(): + """DEF-TS12-01 (2026-09-10): ``mode="auto"`` with a non-sensitive + tool now ALWAYS resolves to ``mode="strict"`` and contacts the + gateway. Pre-fix this routed to ``mode="inline"`` which bypassed + the gateway entirely — see ``test_runtime.py`` for the original + inline-bypass pin (now inverted). Cloud-only invariant from + CLAUDE.md §17. """ rt = _make_test_runtime() rt._transport.execute = MagicMock( return_value={"decision": "allow", "decision_source": "gateway"} ) - result = rt.execute("safe.tool", {"x": 1}) - assert result["decision_source"] == "local" - rt._transport.execute.assert_not_called() + rt.execute("safe.tool", {"x": 1}) + rt._transport.execute.assert_called_once() + assert rt._transport.execute.call_args.kwargs["mode"] == "strict" def test_execute_auto_sensitive_calls_transport(): diff --git a/tests/test_transport.py b/tests/test_transport.py index c5c3397..0a88476 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -1079,6 +1079,7 @@ def capture(request: httpx.Request) -> httpx.Response: from nullrun.breaker.exceptions import ( NullRunAuthenticationError, + NullRunBackendError, NullRunTransportError, RateLimitError, TransportErrorSource, @@ -1216,22 +1217,28 @@ def test_execute_200_with_cache_write(): def test_execute_4xx_returns_block(): - """4xx (no special handling) → block-dict, decision_source FALLBACK.""" + """4xx (no special handling) → raises NullRunBackendError after + wire-envelope parse (def-nr-transport-catchfanin-gap closed the + pre-fix silent-fallback path that synthesised decision_source= + FALLBACK dicts and swallowed real wire-coded reasons). + + The fake response body uses the legacy `{"error": ...}` slug + shape, which the parser treats as unrecognised envelope and + surfaces as NullRunBackendError.""" t = _build_transport() fake_response = MagicMock() fake_response.status_code = 400 fake_response.json.return_value = {"error": "bad_request"} t._client.post = MagicMock(return_value=fake_response) - result = t.execute( - organization_id="org-1", - execution_id="wf-1", - trace_id="t-1", - tool="safe.tool", - input_data={}, - ) - assert result["decision"] == "block" - assert "400" in result["explanation"] + with pytest.raises(NullRunBackendError): + t.execute( + organization_id="org-1", + execution_id="wf-1", + trace_id="t-1", + tool="safe.tool", + input_data={}, + ) def test_execute_breaker_error_with_raise(): diff --git a/tests/test_transport_branches.py b/tests/test_transport_branches.py index 7d747d5..f5dfd4a 100644 --- a/tests/test_transport_branches.py +++ b/tests/test_transport_branches.py @@ -22,6 +22,7 @@ from nullrun.breaker.exceptions import ( NullRunAuthenticationError, + NullRunBackendError, NullRunTransportError, RateLimitError, TransportErrorSource, @@ -160,22 +161,26 @@ def test_execute_200_with_cache_write(): def test_execute_4xx_returns_block(): - """4xx (no special handling) → block-dict, decision_source FALLBACK.""" + """4xx (no special handling) → raises NullRunBackendError after + wire-envelope parse (def-nr-transport-catchfanin-gap closed the + pre-fix silent-fallback path). + + Legacy `{"error": ...}` slug body is unrecognised by the parser + and surfaces as NullRunBackendError.""" t = _build_transport() fake_response = MagicMock() fake_response.status_code = 400 fake_response.json.return_value = {"error": "bad_request"} t._client.post = MagicMock(return_value=fake_response) - result = t.execute( - organization_id="org-1", - execution_id="wf-1", - trace_id="t-1", - tool="safe.tool", - input_data={}, - ) - assert result["decision"] == "block" - assert "400" in result["explanation"] + with pytest.raises(NullRunBackendError): + t.execute( + organization_id="org-1", + execution_id="wf-1", + trace_id="t-1", + tool="safe.tool", + input_data={}, + ) def test_execute_breaker_error_with_raise(): diff --git a/tests/test_v3_wire_contract.py b/tests/test_v3_wire_contract.py index 7c4db46..38cc023 100644 --- a/tests/test_v3_wire_contract.py +++ b/tests/test_v3_wire_contract.py @@ -430,14 +430,14 @@ def test_budget_hard_blocked_maps_to_budget_error(self): exc = _parse_v3_error_envelope(resp, "check") assert isinstance(exc, NullRunBudgetError) - def test_redis_unavailable_maps_to_budget_error(self): - #: REDIS_UNAVAILABLE is fail-CLOSED → 402 - resp = self._make_response( - 402, - {"error_code": "REDIS_UNAVAILABLE", "error_message": "Redis down"}, - ) - exc = _parse_v3_error_envelope(resp, "check") - assert isinstance(exc, NullRunBudgetError) + # REMOVED 2026-09-10: ``test_redis_unavailable_maps_to_budget_error`` + # — the legacy v2 ``REDIS_UNAVAILABLE`` slug is absent from the + # backend ``GateErrorCode::all()`` set and is never emitted on the + # wire. The corresponding SDK map entry in transport.py + # (``_V3_ERROR_CODE_MAP``) was removed in lockstep so cookbook + # mapping tables stay in sync with what the backend can actually + # send. BUDGET_REDIS_UNAVAILABLE / RATE_LIMIT_REDIS_UNAVAILABLE + # below are the post-v3.36 canonical codes and remain mapped. def test_chain_max_duration_maps_to_chain_error(self): resp = self._make_response( @@ -599,7 +599,10 @@ def test_catalog_covers_all_documented_codes(self): "BUDGET_SOFT_BLOCKED", "BUDGET_OVERDRAFT_EXCEEDED", "BUDGET_PERIOD_NOT_STARTED", - "REDIS_UNAVAILABLE", + # REDIS_UNAVAILABLE removed 2026-09-10 — never emitted + # by the backend (absent from GateErrorCode::all()). + # Use BUDGET_REDIS_UNAVAILABLE / RATE_LIMIT_REDIS_UNAVAILABLE + # (canonical post-v3.36) instead. "CHAIN_MAX_DURATION_EXCEEDED", "CHAIN_CROSS_ORG", "CHAIN_ORG_MISMATCH", diff --git a/uv.lock b/uv.lock index 9a32006..1cf8446 100644 --- a/uv.lock +++ b/uv.lock @@ -2870,7 +2870,7 @@ wheels = [ [[package]] name = "nullrun" -version = "0.16.0" +version = "0.16.7" source = { editable = "." } dependencies = [ { name = "httpx" },