diff --git a/src/nullrun/decorators.py b/src/nullrun/decorators.py index 0c76e9e..1000eb8 100644 --- a/src/nullrun/decorators.py +++ b/src/nullrun/decorators.py @@ -608,7 +608,31 @@ def _enforce_sensitive_tool( case; a real `decision=block` from the gateway is still honored and still raises `NullRunBlockedException`. """ - if not runtime.is_sensitive_tool(fn.__name__): + # 2026-07-24 (Root-cause fix): the previous code used + # ``is_sensitive_tool(fn.__name__)`` as the single source of + # truth. That looked up the name in ``runtime._sensitive_tools``, + # which is populated by the ``@sensitive`` decorator at + # *decoration time*. If the user calls ``init_or_die()`` (or any + # other runtime singleton reinit path) AFTER the module-level + # decorators run — which is the common pattern in the + # examples — the registration lands on the OLD runtime, the + # new runtime has an empty ``_sensitive_tools`` set, and this + # gate returns early before reading ``_nullrun_extractor``. + # The function carries the typed impact extractor as an + # attribute on the callable itself, so use the presence of + # the extractor as a second source of truth: if either the + # runtime registry knows the name OR the function carries + # ``_nullrun_extractor``, this is a sensitive tool and the + # gate must run. This avoids the four-cell state space + # (extractor × registered) collapsing to the silent-skip + # "your bug" cell. + # + # The ``@sensitive`` decorator now stamps the attribute on the + # innermost callable (via ``_stamp_extractor_on_innermost``), + # so the bare ``fn`` parameter here carries it directly and a + # single ``getattr`` is enough. + extractor = getattr(fn, "_nullrun_extractor", None) + if not runtime.is_sensitive_tool(fn.__name__) and extractor is None: return masked = _safe_kwargs(kwargs) # P0-1: positional args are masked the same way as kwargs. Without @@ -633,7 +657,10 @@ def _enforce_sensitive_tool( # try/except below wraps it as NullRunBlockedException. business_impact_dict: dict[str, Any] | None = None action_digest_hex: str | None = None - extractor = getattr(fn, "_nullrun_extractor", None) + # ``extractor`` was already resolved at the top of this + # function (line 626) for the gate-skip check; reuse the + # binding here so we do not pay for a second ``getattr`` and + # so a future change to that lookup applies to both sites. if extractor is not None: try: from nullrun.business_impact import compute_action_digest @@ -916,22 +943,69 @@ def refund_customer(amount_cents: int, customer_id: str): # closes over the impact extractor. We stamp the extractor onto # the function later (when the decorator is invoked) so users # can mix @sensitive(impact=...) with @protect in any order. + # + # 2026-07-24 (Root-cause fix): the user-typical spelling is + # + # @sensitive(impact=money_outflow(...)) + # @protect + # def refund_customer(...): + # ... + # + # Python applies decorators bottom-up, so @protect runs first + # and ``_attach_decorator`` receives the @protect-wrapped + # function. The pre-fix code stamped ``_nullrun_extractor`` on + # the wrapper (``_fn``) directly, so the gate later saw the + # extractor on the @protect wrapper but not on the bare + # user function that ``@protect`` captured as ``fn``. The + # gate's ``_enforce_sensitive_tool`` therefore found no + # extractor on ``fn``, returned early, and never built the + # typed ``business_impact`` for the /execute payload. To fix + # the root cause, walk ``__wrapped__`` (set on the @protect + # wrapper by ``functools.wraps``) to find the innermost + # user function and stamp the attribute there. This way the + # gate can find the extractor via a single ``getattr`` on + # the bare function — no chain walk needed at gate time. if fn is None: def _attach_decorator(_fn: F) -> F: if impact is not None: - # `setattr` keeps mypy happy without a TYPE_CHECKING - # forward-reference declaration; ruff B010 is a - # stylistic preference (no functional risk here). - setattr(_fn, "_nullrun_extractor", impact) # noqa: B010 + _stamp_extractor_on_innermost(_fn, impact) return _do_sensitive_register(_fn) return _attach_decorator # type: ignore[return-value] # Bare form: @sensitive. if impact is not None: - setattr(fn, "_nullrun_extractor", impact) # noqa: B010 + _stamp_extractor_on_innermost(fn, impact) return _do_sensitive_register(fn) +def _stamp_extractor_on_innermost(fn: F, impact: Any) -> None: + """Stamp ``_nullrun_extractor`` on the innermost callable. + + Walks the ``__wrapped__`` chain (set by ``functools.wraps``) + to find the deepest user function. Falls back to ``fn`` + itself if no chain is present. Setting the attribute on the + innermost callable means the gate's ``_enforce_sensitive_tool`` + can read it from the bare user function via a single + ``getattr`` call — no chain walk needed. + """ + # `setattr` keeps mypy happy without a TYPE_CHECKING + # forward-reference declaration; ruff B010 is a stylistic + # preference (no functional risk here). + seen: set[int] = set() + current: Any = fn + while current is not None and id(current) not in seen: + seen.add(id(current)) + next_current = getattr(current, "__wrapped__", None) + if next_current is None: + setattr(current, "_nullrun_extractor", impact) # noqa: B010 + return + current = next_current + # Fallback: chain exhausted without finding a leaf. Stamp + # on the input itself so the attribute is at least present + # on the outermost wrapper @protect captured. + setattr(fn, "_nullrun_extractor", impact) # noqa: B010 + + def _do_sensitive_register(fn: F) -> F: try: # Use the same slot the @protect wrapper uses so the @@ -941,6 +1015,23 @@ def _do_sensitive_register(fn: F) -> F: # tests that build a custom runtime. rt = _get_or_create_runtime() rt.add_sensitive_tool(fn.__name__) + # 2026-07-24 (Root-cause fix): the runtime singleton + # above is the one that was active at *decoration time*. + # If the user calls ``init_or_die()`` (or any other + # runtime reinit path) after the module-level + # decorators run — which is the common pattern in the + # examples — the new runtime starts with an empty + # ``_sensitive_tools`` set and the previous + # registration is lost. Stamping the tool name in + # the module-level ``_STRICT_MODE_FORCED`` set as well + # gives ``runtime.execute`` a second source of truth + # that survives the singleton churn. Importing here + # rather than at module top so this module stays + # import-cycle-free against ``nullrun.decorators`` (the + # only legitimate consumer is itself). + from nullrun.runtime import register_strict_mode_forced + + register_strict_mode_forced(fn.__name__) except Exception as exc: # Sensitive tool registration is part of the fail-CLOSED contract # (ADR-008 / sensitive-tool-fail-closed memory). If we diff --git a/src/nullrun/runtime.py b/src/nullrun/runtime.py index 9dc367f..6882467 100644 --- a/src/nullrun/runtime.py +++ b/src/nullrun/runtime.py @@ -129,6 +129,44 @@ _GATE_CACHE: dict[tuple[str, str | None, str | None], tuple[float, dict[str, Any]]] = {} _GATE_CACHE_TTL_SECONDS: float = 5.0 +# 2026-07-24 (Root-cause fix for the ``@sensitive`` reinit gap): +# a process-level set of tool names that the ``@sensitive`` +# decorator has stamped as needing strict mode. The runtime +# singleton also tracks this via ``_sensitive_tools``, but +# that set is populated at decoration time and can be lost +# across ``init_or_die()`` calls if the user re-initializes +# the runtime (the registration landed on the OLD instance +# and the new instance starts with an empty set). The +# module-level set is decorator-driven and survives any +# runtime singleton churn, so ``is_strict_mode_forced`` is +# the second source of truth that ``runtime.execute`` consults +# before falling through to inline mode. +_STRICT_MODE_FORCED: set[str] = set() + + +def register_strict_mode_forced(tool_name: str) -> None: + """Mark ``tool_name`` as needing strict mode. + + Called by ``@sensitive(impact=...)`` at decoration time. The + name stays in the module-level set until process exit; it + is intentionally not cleared by ``init_or_die()`` so a + second-runtime reinit does not silently drop a tool out of + strict mode. + """ + _STRICT_MODE_FORCED.add(tool_name) + + +def is_strict_mode_forced(tool_name: str) -> bool: + """Return True if ``tool_name`` was decorated with ``@sensitive``. + + Complements ``runtime.is_sensitive_tool(tool_name)`` which + reads the per-runtime registry. The two are OR'd in + ``runtime.execute`` so that a tool whose registration is + lost to runtime reinit still gets the strict /execute + round-trip it asked for. + """ + return tool_name in _STRICT_MODE_FORCED + # 2026-07-04 (v0.12.0 wiring fix — ): # the maximum age (seconds) for a captured ``reservation_id`` # to be eligible for forwarding onto a /track payload. Past @@ -2460,14 +2498,28 @@ def execute( 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. if mode == "auto": - if self.is_sensitive_tool(tool_name): + if self.is_sensitive_tool(tool_name) or is_strict_mode_forced(tool_name): mode = "strict" else: mode = "inline" # For inline mode with non-sensitive tools, skip execute and use local enforcement - if mode == "inline" and not self.is_sensitive_tool(tool_name): + 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, diff --git a/src/nullrun/transport.py b/src/nullrun/transport.py index 979143c..9a62785 100644 --- a/src/nullrun/transport.py +++ b/src/nullrun/transport.py @@ -1335,6 +1335,18 @@ def execute( fallback_mode: str = FallbackMode.PERMISSIVE, operation_id: str | None = None, approval_id: str | None = None, + # Phase 1 / MVP 1.0: typed-impact + digest-bound approval. + # The runtime.execute() helper builds these kwargs and the + # transport includes them on the wire so the backend can + # stamp the approval row with the digest and verify it on + # the post-approval re-check. Pre-fix these kwargs were + # constructed in runtime.execute but never accepted by + # Transport.execute (which raised TypeError and was + # classified as a transport error by the on_transport_error + # arm below — the body was blocked even though no real + # policy violation happened). + business_impact: dict[str, Any] | None = None, + action_digest: str | None = None, on_transport_error: Callable[[Exception], dict[str, Any]] | None = None, ) -> dict[str, Any]: """ @@ -1395,6 +1407,17 @@ def execute( } if approval_id is not None: gate_request["approval_id"] = approval_id + # Phase 1 / MVP 1.0: typed-impact + digest-bound approval. + # Forward both fields on the wire when supplied. The backend + # stamps the approval row with the digest and verifies it on + # the post-approval re-check. The keys are only included + # when the runtime layer actually built them (i.e. when + # ``@sensitive(impact=...)`` was applied) so the wire stays + # quiet for legacy Phase 0 callers. + if business_impact is not None: + gate_request["business_impact"] = business_impact + if action_digest is not None: + gate_request["action_digest"] = action_digest # 2026-07-02 (v0.11.0 refactor): route through the canonical # signed-headers helper — produces Content-Type + X-API-Key + diff --git a/tests/test_approval_timeout_field.py b/tests/test_approval_timeout_field.py index c938d58..e223fe9 100644 --- a/tests/test_approval_timeout_field.py +++ b/tests/test_approval_timeout_field.py @@ -193,22 +193,28 @@ def test_env_fallback_when_server_value_is_zero(self): # on the very first event.wait(), so we explicitly reject # non-positive values. # - # Sprint 0 (coverage): this test is rare-flaky under + # Sprint 0 (coverage): this test was rare-flaky under # pytest-xdist on CI (linux, Python 3.12) — the spawned - # wait thread occasionally misses the release_after_ms - # window when the main thread is mid-test-collection, and - # the entry stays empty so ``result_box.get("result")`` - # is None. ``pytest-rerunfailures`` (already in dev-deps) - # retries up to 2 times. Local pytest on Windows is - # unaffected; the failure mode is xdist worker scheduling - # under load. - @pytest.mark.rerunfailures(max_retries=2) + # wait thread occasionally missed the 50ms release window + # when the main thread was mid-test-collection, and the + # entry stayed empty so ``result_box.get("result")`` was + # None. Two fixes applied together: + # + # 1. ``@pytest.mark.rerunfailures(reruns=2)`` (dev plugin + # pytest-rerunfailures>=14.0,<16.0) retries the flaky + # inner helper up to 2 times. + # 2. ``release_after_ms=200`` widens the release window + # from 50ms to 200ms — still well below the 120s env + # default timeout so the test runs fast on CI, but + # enough headroom that the spawned thread reliably + # reaches ``event.wait()`` before the release fires. + @pytest.mark.rerunfailures(reruns=2) def _check_zero(bad_value: float) -> None: rt = _make_runtime(env_timeout=120.0) try: result_box = _run_wait_and_release( rt, "appr-zero", timeout_seconds=bad_value, - release_after_ms=50, + release_after_ms=200, ) assert result_box.get("result") is not None assert result_box["result"]["timeout_seconds"] == 120.0, (