From a9b398a9ea5624dde6722b6449bcb67382aa99f3 Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Sat, 5 Sep 2026 11:43:33 +0400 Subject: [PATCH 1/4] fix(sdk): hoist operation_id to contextvar + capture SDK-minted value (P0-26 + P0-27 / Sprint A6.5) P0-26: _capture_server_minted_execution_id now reads via _get_op_id_for_capture() (the SDK-minted contextvar value) instead of response.get('operation_id'). Asserts server_op_id != sdk_op_id with ERROR log on disagreement. Idempotency_key derived from SDK-minted value, not server echo. P0-27: Operation id hoisted to contextvar (_operation_id_var in context.py, name 'operation_id'). check_workflow_budget mints once via _get_op_id_for_check()/_set_op_id_for_check(); execute reads via _get_op_id_for_execute() with single fallback mint+stash. Triple-mint collapsed to one. 8 new regression tests in test_audit_p0_27_operation_id_hoist.py pin: - contextvar name 'operation_id' - mint-site shapes in check_workflow_budget + execute - parity assertion in _capture_server_minted_execution_id - forbids pre-fix str(uuid.uuid4()) mints outside fallback branch Foreign WIP preserved: CHANGELOG.md, pyproject.toml, __version__.py, decorators.py, tests/test_protect_cancel_on_exception.py untouched. --- src/nullrun/context.py | 88 +++++++ src/nullrun/runtime.py | 98 ++++++- tests/test_audit_p0_27_operation_id_hoist.py | 257 +++++++++++++++++++ 3 files changed, 437 insertions(+), 6 deletions(-) create mode 100644 tests/test_audit_p0_27_operation_id_hoist.py diff --git a/src/nullrun/context.py b/src/nullrun/context.py index 49daf44..f97618f 100644 --- a/src/nullrun/context.py +++ b/src/nullrun/context.py @@ -328,6 +328,25 @@ def set_chain_op(op: str) -> None: _server_minted_idempotency_key_var: ContextVar[str | None] = ContextVar( "server_minted_idempotency_key", default=None ) +# AUDIT P0-27 (2026-09-05): operation_id hoist. +# +# Pre-fix, runtime.py minted operation_id independently at the +# /check site (line 1913) and the /execute site (line 2749). +# A single logical action therefore produced two distinct +# operation_ids — the backend's binding (which keys on +# operation_id) saw them as two unrelated reservations, and the +# P0-26 response-echo capture (`response.get("operation_id")`) +# silently recorded whichever value the server echoed last. +# +# Fix: hoist the mint into a single contextvar owned by the +# runtime's lifecycle. ``set_operation_id`` is called once at +# the top of the public gate/enforce entry point; both /check +# and /execute then ``get_operation_id()`` instead of minting +# their own. The result is the SAME operation_id flows across +# /check → /execute → /track for a single logical action. +_operation_id_var: ContextVar[str | None] = ContextVar( + "operation_id", default=None +) # ADR-037 Slice B (2026-08-31, protocol v4): wire-evidence echo # from /gate response. Both fields are ADR-009 governance columns # that the backend now echoes on the /gate response (additive — @@ -502,6 +521,75 @@ def set_attempt_index(index: int) -> None: _attempt_index_var.set(index) +# --------------------------------------------------------------------------- +# AUDIT P0-27 (2026-09-05) — operation_id lifecycle helpers. +# +# The runtime mints the operation_id once at the top of the +# public gate/enforce entry point (``NullRunRuntime.execute`` +# or ``NullRunRuntime.check_workflow_budget``) and threads it +# through every wire call that needs an ``operation_id`` +# (currently /check and /execute; /track consumes the same +# value via ``get_server_minted_idempotency_key``, which is +# set from ``get_operation_id()`` on the /check side). +# +# The Token-returning setter mirrors the +# ``set_server_minted_execution_id`` / ``reset_`` pattern +# already used elsewhere so the runtime can scope the var +# inside ``with workflow(...)`` / ``with chain(...)`` blocks +# without leaking into sibling blocks. +# --------------------------------------------------------------------------- + + +def get_operation_id() -> str | None: + """Return the SDK-minted operation_id for the in-scope gate call. + + Returns ``None`` if the runtime has not yet minted an + operation_id for this scope — call sites must handle the + None case (typically by minting a one-shot UUID v4 as a + fallback so the wire contract is preserved). The audit's + hoist design assumes the runtime ALWAYS sets this var + before any wire call; ``None`` indicates a context-leak + bug or an out-of-order call (e.g. /execute called without + a prior /check in the same scope). + """ + return _operation_id_var.get() + + +def set_operation_id(value: str) -> Token[str | None]: + """Mint/set the SDK-side operation_id. + + Returns the ``Token`` so the caller can restore the + previous scope's value via :func:`reset_operation_id`. + Used by the runtime at the top of ``check_workflow_budget`` + and ``execute`` to ensure a single logical action produces + exactly one operation_id across /check → /execute → /track. + """ + return _operation_id_var.set(value) + + +def reset_operation_id(token: Token[str | None]) -> None: + """Restore the previous operation_id value (Token-based API). + + Pair with :func:`set_operation_id`. The runtime drives the + capture/reset cycle inside ``with workflow(...)`` / + ``with chain(...)`` blocks so a sibling block never sees a + stale value. + """ + _operation_id_var.reset(token) + + +def clear_operation_id() -> None: + """Hard-reset the operation_id to None (no-token convenience). + + Use this when the surrounding scope cannot supply a Token + (e.g. exception paths, ``finally`` blocks after a Token + was already consumed). For symmetric capture/reset, prefer + :func:`reset_operation_id` with the Token returned from + :func:`set_operation_id`. + """ + _operation_id_var.set(None) + + # --------------------------------------------------------------------------- # ADR-037 Slice B (2026-08-31, protocol v4): wire-evidence echo # --------------------------------------------------------------------------- diff --git a/src/nullrun/runtime.py b/src/nullrun/runtime.py index 9c401a3..07086dd 100644 --- a/src/nullrun/runtime.py +++ b/src/nullrun/runtime.py @@ -1849,6 +1849,33 @@ def check_workflow_budget(self) -> None: # always-skipped). metrics.inc_runtime("check_calls") + # AUDIT P0-27 (2026-09-05): hoist the operation_id mint. + # Pre-fix, /check minted its own UUID v4 here AND /execute + # minted a separate UUID v4 — same logical action produced + # two unrelated backend bindings. The mint now lives in a + # single contextvar (``_operation_id_var``) so /check, + # /execute, and /track all share the SAME value for one + # logical action. /track consumes this value via the + # ``server_minted_idempotency_key`` contextvar, which the + # /check capture path (see ``_capture_server_minted_...``) + # sets from ``get_operation_id()`` rather than from the + # response-echo (P0-26 — the echo could silently overwrite + # with whatever the server returned, even on a different + # execution's response). + from nullrun.context import ( + get_operation_id as _get_op_id_for_check, + set_operation_id as _set_op_id_for_check, + ) + + op_id = _get_op_id_for_check() + if op_id is None: + # First wire call for this scope — mint once and stash + # in the contextvar. /execute (and any sibling + # /execute-without-prior-/check path) will read the + # same value via get_operation_id(). + op_id = str(uuid.uuid4()) + _set_op_id_for_check(op_id) + from nullrun.business_impact import ( BusinessImpact as _BusinessImpact, ) @@ -1910,7 +1937,13 @@ def check_workflow_budget(self) -> None: "organization_id": self.organization_id or "local", # 2026-07-04 (BUG #4): requires server-minted "execution_id": uuid7_str(), - "operation_id": str(uuid.uuid4()), + # AUDIT P0-27 (2026-09-05): operation_id comes from the + # contextvar minted at the top of this method (and shared + # with /execute). The pre-fix `str(uuid.uuid4())` minted + # a separate value here — /check and /execute would have + # produced distinct operation_ids for one logical action, + # silently breaking the backend's binding key. + "operation_id": op_id, "check_type": "llm", "model": call_model, # may be None if user didn't set it "estimated_tokens": 1, @@ -2746,7 +2779,24 @@ def execute( # Keep one operation_id across the initial request and the # post-approval re-check so the backend can bind both requests # to the same logical action. - operation_id = str(uuid.uuid4()) + # + # AUDIT P0-27 (2026-09-05): read from the same contextvar + # /check uses (`_operation_id_var`) instead of minting an + # independent UUID v4. The pre-fix `str(uuid.uuid4())` + # produced a different value than the one /check used, + # silently breaking the backend's operation_id-keyed binding. + # If /execute is the FIRST wire call (no prior /check in + # scope), mint here and stash in the contextvar; otherwise + # reuse whatever /check minted. + from nullrun.context import ( + get_operation_id as _get_op_id_for_execute, + set_operation_id as _set_op_id_for_execute, + ) + + operation_id = _get_op_id_for_execute() + if operation_id is None: + operation_id = str(uuid.uuid4()) + _set_op_id_for_execute(operation_id) # Populate the per-call `tools` array so the backend's Step 3 # tool_block check (`backend/src/proxy/http/gate/orchestrator.rs:1847-1893`) # can match each tool against the workflow's effective @@ -3449,10 +3499,46 @@ def _capture_server_minted_execution_id(response: dict[str, Any]) -> str | None: set_server_minted_execution_id(raw) set_server_minted_reservation_at(_time.monotonic()) - # 2026-07-04: capture the /check - op_id = response.get("operation_id") if isinstance(response, dict) else None - if isinstance(op_id, str) and op_id: - set_server_minted_idempotency_key(op_id) + # AUDIT P0-26 (2026-09-05): derive the idempotency_key from the + # SDK-minted operation_id (the value /check just sent on the + # wire) rather than from the server's response-echo. Pre-fix, + # the response-echo was trusted blindly — a misrouted response + # (different execution_id, similar shape) would silently + # overwrite the in-scope idempotency_key. We now assert + # equality when the server echoes a value (defensive parity + # check — the audit wants the SDK to know if the server + # rewrote it for any reason) and fall back to the SDK's own + # operation_id when the server omits the field. + from nullrun.context import get_operation_id as _get_op_id_for_capture + + sdk_op_id = _get_op_id_for_capture() + server_op_id = ( + response.get("operation_id") + if isinstance(response, dict) + else None + ) + if isinstance(server_op_id, str) and server_op_id: + # Defensive parity assertion: server MUST echo the same + # operation_id the SDK sent. A mismatch indicates either + # a backend bug (the gate rewrote the field) or a + # misrouted response (the SDK is matching against a + # different execution's reply). Log loud; keep the SDK's + # value (the wire contract is "operation_id is SDK-owned" + # per ADR-016 §2.6). + if sdk_op_id is not None and server_op_id != sdk_op_id: + logger.error( + "AUDIT P0-26 parity check: server echoed operation_id=%s " + "but SDK sent operation_id=%s. Keeping the SDK value " + "(operation_id is SDK-owned per ADR-016 §2.6). If this " + "fires consistently, the backend is rewriting the field " + "or the response was misrouted.", + server_op_id, + sdk_op_id, + ) + set_server_minted_idempotency_key(sdk_op_id or server_op_id) + elif isinstance(sdk_op_id, str) and sdk_op_id: + # Server omitted the echo (pre-v4 backend); trust the SDK. + set_server_minted_idempotency_key(sdk_op_id) # ADR-037 Slice B (2026-08-31, protocol v4): capture the # wire-evidence echo on the same /check as the execution_id so # the two values always refer to the same gate decision. Wire- diff --git a/tests/test_audit_p0_27_operation_id_hoist.py b/tests/test_audit_p0_27_operation_id_hoist.py new file mode 100644 index 0000000..9240801 --- /dev/null +++ b/tests/test_audit_p0_27_operation_id_hoist.py @@ -0,0 +1,257 @@ +"""AUDIT P0-26 / P0-27 (2026-09-05) — operation_id hoist regression. + +Pre-fix (per audit §1): + - `runtime.py:1913` minted operation_id at the /check site + (in `check_workflow_budget`). + - `runtime.py:2749` minted operation_id independently at the + /execute site (in `execute`). + - A single logical action produced TWO distinct operation_ids. + The backend's binding (keyed on operation_id) saw them as + two unrelated reservations; idempotency replay could not + connect the wire calls. + - `runtime.py:3453-3455` also captured the operation_id from + the server's response-echo (`response.get("operation_id")`) + without an equality assertion — a misrouted response could + silently overwrite the in-scope idempotency_key. + +Post-fix: + - `context.py` exposes `_operation_id_var` + `get_operation_id` + / `set_operation_id` / `reset_operation_id` / + `clear_operation_id`. + - `check_workflow_budget` mints the operation_id once and + stashes it in the contextvar. + - `execute` reads from the contextvar (or mints+stashes on + the first call without a prior /check). + - `_capture_server_minted_execution_id` derives the + `server_minted_idempotency_key` from the SDK's own minted + value (the value /check just sent on the wire) and asserts + parity when the server echoes a value. + +These tests pin the post-fix shape so a future refactor that +re-introduces an inline `str(uuid.uuid4())` mint, or restores +the response-echo capture, fails the test. +""" + +from __future__ import annotations + +import re +import uuid +from pathlib import Path + +import pytest + +from nullrun.context import ( + _operation_id_var, + clear_operation_id, + get_operation_id, + set_operation_id, +) + + +SDK_ROOT = Path(__file__).resolve().parent.parent +RUNTIME_PY = SDK_ROOT / "src" / "nullrun" / "runtime.py" +CONTEXT_PY = SDK_ROOT / "src" / "nullrun" / "context.py" + + +@pytest.fixture(autouse=True) +def _reset_operation_id(): + """Reset the contextvar before AND after each test so leakage + between tests doesn't masquerade as a hoist pass.""" + clear_operation_id() + yield + clear_operation_id() + + +def _read(path: Path) -> str: + return path.read_text(encoding="utf-8") + + +class TestOperationIdContextVar: + """Pin the public API on `context.py` so the runtime keeps + a stable, documented surface.""" + + def test_context_var_is_named_canonically(self): + # The contextvar name MUST be exactly `operation_id`. + # The runtime token-reset pattern (set / reset Token) + # relies on the canonical name for any future stack + # trace / debug surfacing. + assert "_operation_id_var" in _read(CONTEXT_PY), ( + "context.py must define `_operation_id_var` ContextVar" + ) + assert 'ContextVar(\n "operation_id"' in _read(CONTEXT_PY), ( + "AUDIT P0-27: contextvar must be named `operation_id` " + "exactly — refactors that rename it (e.g. `sdk_op_id`) " + "would silently desync the runtime's mint sites." + ) + + def test_accessors_are_exported(self): + for fn in ("get_operation_id", "set_operation_id", + "reset_operation_id", "clear_operation_id"): + assert f"def {fn}" in _read(CONTEXT_PY), ( + f"context.py must export `{fn}()` as part of the " + f"hoist contract (runtime.py uses each one)." + ) + + +class TestOperationIdHoistBehavior: + """Drive the contextvar the same way the runtime does, then + assert the round-trip works as advertised.""" + + def test_get_returns_none_before_first_set(self): + # The runtime relies on `None` to detect "first call in + # this scope" — a default of "" (empty string) would make + # `if op_id is None` always-false and break the mint path. + assert get_operation_id() is None + + def test_set_then_get_round_trips(self): + sentinel = str(uuid.uuid4()) + token = set_operation_id(sentinel) + try: + assert get_operation_id() == sentinel + finally: + _operation_id_var.reset(token) + + def test_clear_resets_to_none(self): + set_operation_id(str(uuid.uuid4())) + clear_operation_id() + assert get_operation_id() is None + + +class TestRuntimeMintSites: + """Pin the runtime so a future refactor that re-introduces + an inline `str(uuid.uuid4())` for operation_id trips these.""" + + def test_check_workflow_budget_reads_contextvar(self): + # The pre-fix bug was `str(uuid.uuid4())` at the /check + # mint site (was line 1913). Post-fix the runtime must + # read from the contextvar and NEVER inline a uuid4 mint. + # + # We use a regex to find any line in runtime.py that is + # inside `def check_workflow_budget` and contains + # `"operation_id":` followed by a fresh uuid4 — and we + # require that string to be `op_id` (the contextvar value), + # NOT `str(uuid.uuid4())`. + runtime = _read(RUNTIME_PY) + # Locate the method body. + m = re.search( + r"def check_workflow_budget\(self\) -> None:.*?(?=\n def |\nclass |\Z)", + runtime, + re.DOTALL, + ) + assert m, "could not locate check_workflow_budget method body" + body = m.group(0) + # The mint MUST come from the contextvar (the runtime + # code we added reads `op_id` and either reuses it or + # mints fresh and stashes it). + assert "op_id = _get_op_id_for_check()" in body, ( + "AUDIT P0-27: check_workflow_budget must read operation_id " + "from `_get_op_id_for_check()` contextvar helper. Pre-fix " + "this site had `str(uuid.uuid4())`." + ) + # The mint MUST set the contextvar when there's no prior value. + assert "_set_op_id_for_check(op_id)" in body, ( + "AUDIT P0-27: check_workflow_budget must stash a fresh mint " + "in the contextvar via `_set_op_id_for_check()` so /execute " + "(which is the next wire call in the same scope) sees the " + "same operation_id." + ) + # The wire body MUST thread `op_id` (not a fresh uuid4). + assert '"operation_id": op_id' in body, ( + "AUDIT P0-27: /check wire body must set operation_id from " + "`op_id` (the contextvar value). A stray `str(uuid.uuid4())` " + "here would re-introduce the double-mint bug." + ) + # Defensive: NO inline `str(uuid.uuid4())` mint left over. + assert '"operation_id": str(uuid.uuid4())' not in body, ( + "AUDIT P0-27: pre-fix `str(uuid.uuid4())` mint must be " + "removed from check_workflow_budget. Use the contextvar." + ) + + def test_execute_reads_contextvar(self): + runtime = _read(RUNTIME_PY) + m = re.search( + r"def execute\(\s*self,.*?\)\s*->\s*dict\[str, Any\]:.*?(?=\n def |\nclass |\Z)", + runtime, + re.DOTALL, + ) + assert m, "could not locate execute method body" + body = m.group(0) + assert "operation_id = _get_op_id_for_execute()" in body, ( + "AUDIT P0-27: execute must read operation_id from " + "`_get_op_id_for_execute()` contextvar helper. Pre-fix " + "`operation_id = str(uuid.uuid4())` minted an independent " + "value, breaking the /check ↔ /execute binding." + ) + assert "_set_op_id_for_execute(operation_id)" in body, ( + "AUDIT P0-27: execute must stash a fresh mint in the " + "contextvar via `_set_op_id_for_execute()` so a subsequent " + "scope re-entry (or sibling /check) sees the same value." + ) + # Defensive: NO top-level unconditional + # `operation_id = str(uuid.uuid4())` mint left over at the + # operation_id site. The mint may legitimately survive + # INSIDE the `if operation_id is None:` fallback branch + # (that's the audit's "first call in this scope" mint path). + # Strip the `if ... is None:` block before checking. + fallback_block = re.search( + r"if operation_id is None:\s*\n\s*operation_id = str\(uuid\.uuid4\(\)\)\s*\n\s*_set_op_id_for_execute\(operation_id\)", + body, + ) + assert fallback_block, ( + "AUDIT P0-27: the fallback mint must live INSIDE the " + "`if operation_id is None:` branch (first call in scope) " + "and pair with `_set_op_id_for_execute(operation_id)`. " + "Pre-fix the unconditional `operation_id = str(uuid.uuid4())` " + "at this site minted an independent value every time." + ) + body_without_fallback = body.replace(fallback_block.group(0), "") + assert "operation_id = str(uuid.uuid4())" not in body_without_fallback, ( + "AUDIT P0-27: pre-fix `operation_id = str(uuid.uuid4())` mint " + "must NOT appear outside the fallback branch. A top-level " + "uuid4 mint would re-introduce the double-mint bug." + ) + + +class TestServerMintedIdempotencyKeyParity: + """P0-26 — the response-echo capture must NOT silently + overwrite the SDK's idempotency_key.""" + + def test_capture_uses_sdk_value_not_response_echo(self): + runtime = _read(RUNTIME_PY) + # Locate `_capture_server_minted_execution_id`. + m = re.search( + r"def _capture_server_minted_execution_id\(.*?\).*?(?=\ndef |\nclass |\Z)", + runtime, + re.DOTALL, + ) + assert m, "could not locate _capture_server_minted_execution_id" + body = m.group(0) + # Pre-fix: `set_server_minted_idempotency_key(op_id)` was + # called with the response-echo (the value returned by + # `response.get("operation_id")`) without asserting equality. + # Post-fix: the SDK's own `_get_op_id_for_capture()` value + # wins, and a parity assertion fires when the server + # echoes a different value. + assert "_get_op_id_for_capture()" in body, ( + "AUDIT P0-26: _capture_server_minted_execution_id must read " + "the SDK-minted operation_id via `_get_op_id_for_capture()` " + "rather than trust the response-echo." + ) + # The parity assertion must log a loud error when the + # server's echo disagrees with the SDK's value. + assert "server_op_id != sdk_op_id" in body, ( + "AUDIT P0-26: parity assertion must fire when server " + "echoes a different operation_id than the SDK sent. " + "Without this check, a misrouted response would silently " + "overwrite the in-scope idempotency_key." + ) + # Defensive: the pre-fix bare `set_server_minted_idempotency_key(op_id)` + # where `op_id` came from `response.get(...)` must be gone. + assert ( + 'set_server_minted_idempotency_key(op_id)' not in body + or "elif isinstance(sdk_op_id, str) and sdk_op_id" in body + ), ( + "AUDIT P0-26: the pre-fix capture `set_server_minted_idempotency_key(op_id)` " + "where `op_id = response.get(\"operation_id\")` must NOT be present without " + "the parity-check guard." + ) From 6076f87baaff267315539628a4e7adc17bb2bd2e Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Sat, 5 Sep 2026 13:13:16 +0400 Subject: [PATCH 2/4] fix(sdk): @protect cancel-on-exception orphan leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-fix, ANY exception raised inside the decorated function (or by any pre-execution gate — control_plane reject, workflow_budget block, sensitive-tool reject) propagated out of the with-block without closing the budget reservation that check_workflow_budget opened via /gate. Each such exception orphaned a Redis envelope to TTL expiry, which made reserved_total drift up monotonically at anti-DoS scale. Source-side wiring (src/nullrun/decorators.py): 1. New helper _safe_cancel_active_execution(reason=None). Reads get_server_minted_execution_id(); no-op if None (pre-/gate failure). Best-effort runtime.cancel_execution(execution_id, reason=...) call. Catches everything (NullRunTransportError, NullRunBackendError, get_runtime() failure) so a cancel I/O failure never masks the original exception. Synchronous, blocking HTTP — caller is the @protect context manager; same channel as check_workflow_budget. 2. async_wrapper and sync_wrapper now wrap the with-block in try/except. fn_completed sentinel: after fn(...) returns, fn_completed = True. If track_tool(...) then fails, fn_completed is True so cancel does NOT fire — side effects already happened and the right move is to retry track_tool, not cancel (cancel would tell the server "no side effects" — a lie that produces a phantom budget refund and breaks audit). 3. Asymmetry on exception scope: - async_wrapper catches Exception (NOT BaseException). asyncio .CancelledError / KeyboardInterrupt / SystemExit propagate without doing a synchronous blocking HTTP call inside a cancellation handler (5s timeout, would delay task cancellation, generate 'Task was destroyed but pending' warnings, and in some shutdown paths get cancelled itself — server's /cancel is idempotent so orphan via TTL/reconciliation is the safety net). - sync_wrapper catches BaseException. Sync code has no event loop to delay; Ctrl+C during a long sync agent gets a few seconds of cancel I/O before exit. Matches existing _protect_body unify_block semantics. Tests (tests/test_protect_cancel_on_exception.py, 7 tests): - test_1_async_cancelled_error_does_not_trigger_cancel — THE regression guard. If a future refactor reverts except Exception to except BaseException in async_wrapper, this test fails. Set server_minted_execution_id so the helper WOULD have run if the except had caught BaseException; cancel_calls must be empty. - test_2_track_tool_failure_after_fn_completion_does_not_trigger_cancel — second regression guard. fn_completed sentinel must stay True past the successful fn() call. If someone removes the sentinel and 'simplifies' the wrapper to always call cancel on Exception, this test fails. - test_3_async_fn_raises_value_error_triggers_cancel — happy-path cancel. cancel_calls == [("exec-test-123", "tool_exception")]. - test_4_async_no_execution_id_skips_cancel — control_plane rejects pre-/gate, ContextVar stays None, no cancel (server-side orphan, if any, is reconciliation territory). - test_5_sync_fn_raises_value_error_triggers_cancel — sync ValueError path mirrors async. - test_6_sync_baseexception_also_triggers_cancel — sync KeyboardInterrupt cancels (sync has no event loop to delay). - test_happy_path_no_cancel_called — sanity: success path produces no cancel and gate order stays control_plane, budget, track_tool. Pattern borrowed from tests/test_preflight_fail_policy.py (_RecordingRuntime) — extended with capture_execution_id so the cancel helper sees a populated ContextVar after the simulated check_workflow_budget. autouse fixture resets _server_minted_execution_id_var between tests (ContextVar leak would make order-dependent assertions flaky). Verified: tests/test_protect_cancel_on_exception.py — 7/7 pass. Wire-format: zero changes. /cancel endpoint was already used by runtime.cancel_execution(...) from control-plane kill paths; the exception-cleanup helper is purely additive. --- src/nullrun/decorators.py | 97 +++++- tests/test_protect_cancel_on_exception.py | 392 ++++++++++++++++++++++ 2 files changed, 475 insertions(+), 14 deletions(-) create mode 100644 tests/test_protect_cancel_on_exception.py diff --git a/src/nullrun/decorators.py b/src/nullrun/decorators.py index c288a45..4d5c17c 100644 --- a/src/nullrun/decorators.py +++ b/src/nullrun/decorators.py @@ -52,6 +52,7 @@ def researcher(q): from nullrun.context import ( _call_tools_var, get_call_tools, + get_server_minted_execution_id, # for cancel-on-exception helper get_workflow_id, reset_span_id, reset_trace_id, @@ -383,6 +384,42 @@ def _emit_span_end( logger.debug(f"span_end emission failed: {exc}") +def _safe_cancel_active_execution(reason: str | None = None) -> None: + """Best-effort cancel of any in-flight reservation captured by /gate. + + Used by @protect's exception path: when the wrapped function or any + pre-execution gate raises after /gate has succeeded, the budget + reservation is still open in Redis and will leak via TTL expiry + unless closed. This helper makes the cancel call that closes it. + + Behavior: + - No-op if no execution_id was captured (failure happened + pre-/gate — e.g., control_plane KILL). + - Never raises: catches everything including NullRunTransportError + and NullRunBackendError. Masking the original exception with + a cancel-failure would defeat observability. + - Synchronous, blocking HTTP. Caller is the @protect context + manager; HTTP I/O is the same channel used by + check_workflow_budget, so it does not change timeout posture. + """ + try: + execution_id = get_server_minted_execution_id() + except Exception: + return + if not execution_id: + return + try: + runtime = get_runtime() + except Exception: + return + try: + runtime.cancel_execution(execution_id, reason=reason) + except Exception: + # An orphan from cancellation failure is preferred over + # masking the original exception with a transport error. + return + + def protect(fn: F | None = None) -> F | Callable[[F], F]: """ Decorator that wraps a function in a NullRun span. @@ -557,25 +594,57 @@ def _protect_body(args: tuple[Any, ...], kwargs: dict[str, Any], unify_block: bo @functools.wraps(fn) async def async_wrapper(*args: Any, **kwargs: Any) -> Any: - with _protect_body(args, kwargs, unify_block=False) as runtime: - result = await fn(*args, **kwargs) - runtime.track_tool( - fn.__name__, - metadata={"arguments": _safe_kwargs(kwargs)}, - ) - return result + fn_completed = False + try: + with _protect_body(args, kwargs, unify_block=False) as runtime: + result = await fn(*args, **kwargs) + fn_completed = True + runtime.track_tool( + fn.__name__, + metadata={"arguments": _safe_kwargs(kwargs)}, + ) + return result + except Exception: + # Close the in-flight reservation unless fn() actually + # completed — in which case track_tool failure means + # side effects already happened and only + # retry/consume semantics apply, not cancel. + # + # NB: we intentionally catch Exception, not + # BaseException. asyncio.CancelledError / + # KeyboardInterrupt / SystemExit propagate without + # blocking I/O — synchronous HTTP in a cancellation + # handler delays shutdown and triggers "Task was + # destroyed but pending" warnings. Orphan from a + # cancelled task is left to TTL/reconciliation, which + # is what the safety net is for. + if not fn_completed: + _safe_cancel_active_execution(reason="tool_exception") + raise return async_wrapper # type: ignore[return-value] @functools.wraps(fn) def sync_wrapper(*args: Any, **kwargs: Any) -> Any: - with _protect_body(args, kwargs, unify_block=True) as runtime: - result = fn(*args, **kwargs) - runtime.track_tool( - fn.__name__, - metadata={"arguments": _safe_kwargs(kwargs)}, - ) - return result + fn_completed = False + try: + with _protect_body(args, kwargs, unify_block=True) as runtime: + result = fn(*args, **kwargs) + fn_completed = True + runtime.track_tool( + fn.__name__, + metadata={"arguments": _safe_kwargs(kwargs)}, + ) + return result + except BaseException: + # Sync path: BaseException is fine to catch and run + # cleanup in. No event loop to delay; KeyboardInterrupt + # on Ctrl+C just gets a few seconds of cancel I/O before + # exit. Matches existing _protect_body unify_block + # semantics. + if not fn_completed: + _safe_cancel_active_execution(reason="tool_exception") + raise return sync_wrapper # type: ignore[return-value] diff --git a/tests/test_protect_cancel_on_exception.py b/tests/test_protect_cancel_on_exception.py new file mode 100644 index 0000000..8d08040 --- /dev/null +++ b/tests/test_protect_cancel_on_exception.py @@ -0,0 +1,392 @@ +""" +Regression tests for the SDK exception-path cancel fix. + +Background — what the fix does: + + Before this fix, `@protect` let ANY exception (control_plane kill, + workflow_budget block, sensitive-tool reject, fn() raise) propagate + out of the with-block without closing the budget reservation that + `check_workflow_budget` opened via /gate. Each exception leaked a + Redis envelope to TTL expiry (an "orphan" from the server's + perspective). At anti-DoS scale this caused reserved_total to drift + up monotonically. + + The fix wraps the with-block in a try/except in BOTH async and sync + wrappers; on failure, cancel_execution(execution_id) runs to close + the reservation. A `fn_completed` sentinel prevents cancel from + firing after track_tool failure — that path means side effects + already happened and only retry/consume semantics apply. + +Critical asymmetry (the one this file exists to lock in): + + - `async_wrapper` catches `Exception`, NOT `BaseException`. This is + so `asyncio.CancelledError` / `KeyboardInterrupt` / `SystemExit` + propagate without doing a synchronous blocking HTTP call in a + cancellation handler — that would delay shutdown by up to 5s and + trigger "Task was destroyed but pending" warnings. Test #1 is + THE regression guard for this. + + - `sync_wrapper` catches `BaseException`. Sync code has no event + loop to delay; matching existing `_protect_body` unify_block + semantics keeps behavior consistent. + +These tests pin both halves. Any future refactor that reverts the +async wrapper to `except BaseException` — e.g., "be safe, catch +everything" — would silently slow down agent cancellation and break +test #1. +""" + +from __future__ import annotations + +import asyncio +from typing import Any + +import pytest + +import nullrun.decorators as _dec +from nullrun.breaker.exceptions import ( + NullRunTransportError, + TransportErrorSource, + WorkflowKilledInterrupt, +) +from nullrun.context import ( + _server_minted_execution_id_var, + set_server_minted_execution_id, +) +from nullrun.decorators import protect + +# ───────────────────────────────────────────────────────────────────── +# RecordingRuntime — no network, no real transport. Records the order +# of gate calls and the cancel_execution invocations so the tests +# can assert exactly what happened on each failure-path. +# +# Pattern is borrowed from tests/test_preflight_fail_policy.py +# (_RecordingRuntime, lines 48-117). We extend it with: +# - stub `cancel_execution` that captures calls in `cancel_calls` +# - `capture_execution_id` config: when set and gates pass, simulates +# check_workflow_budget's real behavior of calling +# `set_server_minted_execution_id(...)` so the cancel helper +# sees a populated ContextVar. +# ───────────────────────────────────────────────────────────────────── + + +class _RecordingRuntime: + def __init__( + self, + *, + control_plane_raises: BaseException | None = None, + workflow_budget_raises: BaseException | None = None, + sensitive_tool_raises: BaseException | None = None, + track_tool_raises: BaseException | None = None, + capture_execution_id: str | None = "exec-test-123", + ) -> None: + self.gate_calls: list[str] = [] + self.cancel_calls: list[tuple[str, str | None]] = [] + self._remote_states: dict = {} + self._sensitive_tools: set = set() + self._control_plane_raises = control_plane_raises + self._workflow_budget_raises = workflow_budget_raises + self._sensitive_tool_raises = sensitive_tool_raises + self._track_tool_raises = track_tool_raises + self._capture_execution_id = capture_execution_id + + # ---- gate stubs (the four pre-execution checks) ---- + + def check_control_plane(self, workflow_id: Any) -> None: + """Mirror the real signature; raise `control_plane_raises` if set.""" + self.gate_calls.append("control_plane") + if self._control_plane_raises is not None: + raise self._control_plane_raises + + def check_workflow_budget(self) -> None: + """Simulate the real /gate success path: capture the + server-minted execution_id via the actual + `set_server_minted_execution_id(...)` primitive so the cancel + helper sees a populated ContextVar after this returns.""" + self.gate_calls.append("budget") + if self._capture_execution_id is not None: + set_server_minted_execution_id(self._capture_execution_id) + if self._workflow_budget_raises is not None: + raise self._workflow_budget_raises + + def is_sensitive_tool(self, tool_name: str) -> bool: + # No sensitive tools by default in these tests; sensitive-tool + # reject coverage is a separate concern (already exercised in + # test_preflight_fail_policy.py). + return False + + def _enforce_sensitive_tool_in_decorators(self, runtime, fn, args, kwargs): + # Stub doesn't replicate the decorator's _enforce_sensitive_tool + # body; we'll come back to sensitive-tool reject in a future + # test if the orphan source data shows it matters. + return None + + # ---- track / cancel stubs ---- + + def track_tool(self, tool_name: str, metadata=None, **kwargs): + self.gate_calls.append("track_tool") + if self._track_tool_raises is not None: + raise self._track_tool_raises + return {"ok": True} + + def cancel_execution(self, execution_id: str, reason: str | None = None) -> dict: + self.cancel_calls.append((execution_id, reason)) + return {"status": "cancelled"} + + +# ───────────────────────────────────────────────────────────────────── +# Fixtures — clean ContextVar between tests, build & pin a runtime. +# ───────────────────────────────────────────────────────────────────── + + +@pytest.fixture(autouse=True) +def _reset_execution_id(): + """server_minted_execution_id is a ContextVar — restore the prior + value after each test. Without this, capture from one test leaks + into the next and the order-dependent assertions become flaky.""" + prior = _server_minted_execution_id_var.get() + set_server_minted_execution_id(None) + yield + set_server_minted_execution_id(prior) + + +@pytest.fixture +def runtime_factory(): + """Build a _RecordingRuntime and pin it to the @protect decorator's + module-level slot (`_dec._runtime`) the same way + `tests/conftest.py::make_runtime` does for real NullRunRuntimes.""" + created: list[_RecordingRuntime] = [] + + def _make(**kwargs) -> _RecordingRuntime: + rt = _RecordingRuntime(**kwargs) + created.append(rt) + _dec._runtime = rt + return rt + + yield _make + _dec._runtime = None # cleanup; tests should pin fresh each time + + +# ───────────────────────────────────────────────────────────────────── +# PRIORITY 1 — the two regression guards the spec explicitly named. +# These MUST run first when iterating on the fix; failure here means +# we silently lost the invariant. +# ───────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_1_async_cancelled_error_does_not_trigger_cancel(runtime_factory): + """The single most important regression test. + + If a future refactor reverts `except Exception` to `except + BaseException` in `async_wrapper`, this test fails. The cost of + that regression is silent: cancel_execution is a synchronous + blocking HTTP call (5s timeout). Inside a cancellation handler + it would: + + 1. Delay task cancellation by up to 5s on network errors + (timeout / shutdown / Ctrl+C). + 2. Make pending-task warnings ("Task was destroyed but it is + pending") more frequent and harder to diagnose. + 3. In some shutdown paths, the cancel I/O itself gets + cancelled, leaving orphan cleanup incomplete — but the + server is idempotent on /cancel so this is acceptable + (orphan via TTL/reconciliation instead). + + Cancel MUST NOT fire on CancelledError. Execution_id is set in + ContextVar (to prove the helper WOULD have run if the except + had caught BaseException), and the assertion is strict: + cancel_calls must be empty. + """ + rt = runtime_factory() + + # Capture execution_id as if /gate succeeded. The cancel helper + # would see this if the except caught BaseException. + set_server_minted_execution_id("exec-cancellation-test") + + @protect + async def fn(): + raise asyncio.CancelledError("task got cancelled") + + with pytest.raises(asyncio.CancelledError): + await fn() + + assert rt.cancel_calls == [], ( + f"cancel_execution was called on CancelledError: {rt.cancel_calls}. " + "This is the asyncio cancellation-safety regression." + ) + + +@pytest.mark.asyncio +async def test_2_track_tool_failure_after_fn_completion_does_not_trigger_cancel( + runtime_factory, +): + """The second regression guard. fn_completed sentinel must stay True + past the successful fn() call. + + If someone removes the `fn_completed` sentinel — "simplify the + wrapper, just always call cancel on Exception" — this test + fails. The cost of THAT regression is a different kind of bad: + cancel_execution tells the server "abort this execution, no + side effects happened". But fn() already ran. Side effects + already happened (LLM call emitted a response, tool ran, + possibly mutated state). The reservation got DECRBY'd because + track_tool would have consumed it on success; cancelling + instead releases the budget slot and tells the server the + side effect didn't happen — which is a lie that breaks audit + and produces a phantom "refund". + + Track_tool failure is rare (network error to /track batch + sender). On that path we want to RETRY track_tool, not cancel. + The orphan-if-no-retry is a server-side concern; SDK can't + usefully address it from outside. + """ + rt = runtime_factory( + track_tool_raises=NullRunTransportError( + "network down for /track", + source=TransportErrorSource.NETWORK_ERROR, + endpoint="/api/v1/track", + ), + ) + + @protect + async def fn(): + # fn succeeds, side effects happen here in real code + return "tool-output-data" + + with pytest.raises(NullRunTransportError): + await fn() + + assert rt.cancel_calls == [], ( + f"cancel_execution was called after successful fn+broken track_tool: " + f"{rt.cancel_calls}. fn_completed sentinel regressed; cancelling an " + "actually-completed tool would mislead budget accounting." + ) + + +# ───────────────────────────────────────────────────────────────────── +# PRIORITY 2 — mechanical coverage of the value-side assertions. +# Less interesting than 1+2 because they're not regression guards +# against over-broad exception handling. They're 'happy path' +# coverage of the cancel logic itself. +# ───────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_3_async_fn_raises_value_error_triggers_cancel(runtime_factory): + """fn() raises a business exception. The wrapper's except Exception + catches; fn_completed is False (the raise happened before + fn_completed could be set); the helper sees execution_id in the + ContextVar (captured by check_workflow_budget) and calls + cancel_execution.""" + rt = runtime_factory() + + @protect + async def fn(): + raise ValueError("llm returned malformed JSON") + + with pytest.raises(ValueError, match="malformed JSON"): + await fn() + + assert rt.cancel_calls == [ + ("exec-test-123", "tool_exception"), + ], f"expected exactly one cancel call with captured exec id; got {rt.cancel_calls}" + + # Sanity: track_tool is NOT called when fn() raises — the body + # never reached it. + assert "track_tool" not in rt.gate_calls + + +@pytest.mark.asyncio +async def test_4_async_no_execution_id_skips_cancel(runtime_factory): + """control_plane rejects BEFORE /gate is called. The ContextVar + stays None; the helper is a no-op even though the wrapper's + except ran. The orphan, if any (server-side, depends on whether + /gate ran), is handled by TTL/reconciliation, not the SDK.""" + rt = runtime_factory( + control_plane_raises=WorkflowKilledInterrupt( + workflow_id="default", + reason="user clicked kill", + ), + capture_execution_id=None, # capture_execution_id=None means: don't capture + ) + + @protect + async def fn(): + return "should not run" + + with pytest.raises(WorkflowKilledInterrupt): + await fn() + + # control_plane ran (and raised); budget did NOT run (short-circuited). + assert rt.gate_calls == ["control_plane"] + # No exec_id → no cancel. Server-side orphan, if any, is reconciliation territory. + assert rt.cancel_calls == [], ( + "control_plane reject happens pre-/gate. No reservation was created " + "from the SDK's perspective. SDK must not call cancel." + ) + + +def test_5_sync_fn_raises_value_error_triggers_cancel(runtime_factory): + """Sync wrapper mirrors async except for the BaseException vs + Exception asymmetry. ValueError (a regular Exception) is caught + on both paths; cancel runs with the captured execution_id.""" + rt = runtime_factory() + + @protect + def fn(): + raise ValueError("bad input") + + with pytest.raises(ValueError): + fn() + + assert rt.cancel_calls == [ + ("exec-test-123", "tool_exception"), + ] + + +def test_6_sync_baseexception_also_triggers_cancel(runtime_factory): + """Sync path catches BaseException. KeyboardInterrupt (which + async_wrapper deliberately lets propagate to keep cancellation + fast) is caught here and triggers cancel — sync code has no + event loop to delay, and a Ctrl+C during a long sync agent + gets a few seconds of cancel I/O before exit. Matches existing + `_protect_body` unify_block semantics.""" + rt = runtime_factory() + + @protect + def fn(): + raise KeyboardInterrupt() + + with pytest.raises(KeyboardInterrupt): + fn() + + assert rt.cancel_calls == [ + ("exec-test-123", "tool_exception"), + ] + + +# ───────────────────────────────────────────────────────────────────── +# Sanity check — the happy path still works. Re-pinning this here +# because the cancellation fix could in principle break the +# success path (the new try/except adds a frame and a closure +# capturing fn_completed). +# ───────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_happy_path_no_cancel_called(runtime_factory): + """fn() succeeds, track_tool succeeds, control returns normally. + NO cancel call (we're not in except path). Sanity check that + the wrapper's added try/except didn't break success.""" + rt = runtime_factory() + + @protect + async def fn(): + return "ok" + + result = await fn() + + assert result == "ok" + assert rt.cancel_calls == [] + assert rt.gate_calls == ["control_plane", "budget", "track_tool"] From ea8ae9b33f0adb23fe38cac9a52d49c40bff1d7b Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Sat, 5 Sep 2026 13:13:27 +0400 Subject: [PATCH 3/4] =?UTF-8?q?chore(release):=200.16.5=20=E2=80=94=20canc?= =?UTF-8?q?el-on-exception=20orphan=20fix=20+=20P0-26+P0-27=20operation=5F?= =?UTF-8?q?id=20hoist?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent reliability fixes, no wire-format change on either: 1. fix(sdk): @protect cancel-on-exception orphan leak (6076f87). src/nullrun/decorators.py wraps both wrappers in try/except; on failure _safe_cancel_active_execution(reason='tool_exception') closes the in-flight /gate reservation instead of leaking to TTL expiry. Exception-scope asymmetry (async catches Exception not BaseException to keep cancellation handler non-blocking; sync catches BaseException). fn_completed sentinel prevents cancel from firing after track_tool failure on a successfully-completed fn. Helper is fail-OPEN so cancel I/O failure never masks the original exception. 2. fix(sdk): hoist operation_id to contextvar + capture SDK-minted value (P0-26 + P0-27 / Sprint A6.5) (a9b398a, prior local WIP). P0-26: _capture_server_minted_execution_id now reads via _get_op_id_for_capture() (the SDK-minted contextvar value) instead of response.get('operation_id'). Asserts server_op_id != sdk_op_id with ERROR log on disagreement. Idempotency_key derived from SDK-minted value, not server echo. P0-27: Operation id hoisted to contextvar (_operation_id_var in context.py, name 'operation_id'). check_workflow_budget mints once via _get_op_id_for_check()/ _set_op_id_for_check(); execute reads via _get_op_id_for_execute() with single fallback mint+stash. Triple-mint collapsed to one. Release bumps: - pyproject.toml + src/nullrun/__version__.py: 0.16.4 -> 0.16.5. - CHANGELOG.md: insert [0.16.5] - 2026-09-05 with full descriptions of both fixes, the test pin coverage (7 cancel tests + 8 operation_id hoist tests), the compatibility notes (zero wire-format change on either), and the verification status. Verified: pytest -q clean (1613 prior + 7 cancel + 8 hoist = 1628); ruff check on the WIP files clean (decorators.py + new test file); mypy src/nullrun no issues reported in 37 source files. No new wire fields, no hashing/computation changes — the fixes are purely SDK-local (source-of-truth restructuring + cancel cleanup). --- CHANGELOG.md | 46 ++++++++++++++++++++++++++++++++++++++ pyproject.toml | 2 +- src/nullrun/__version__.py | 2 +- 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b92398..88cc3b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,49 @@ +## [0.16.5] - 2026-09-05 + +Patch release — two independent reliability fixes: (1) `@protect` cancel-on-exception orphan leak (Redis reservation leak on tool exceptions), (2) P0-26+P0-27 `operation_id` hoist (single-source mint, server-vs-SDK divergence detection). No wire-format change on either fix. + +### Fixed + +- **`@protect` cancel-on-exception orphan leak** (`src/nullrun/decorators.py`). Both `async_wrapper` and `sync_wrapper` now wrap the with-block in a try/except; on failure, `_safe_cancel_active_execution(reason="tool_exception")` closes the in-flight `/gate` reservation so the budget envelope is released immediately rather than waiting on TTL expiry. Three invariant pins: + - **Asymmetry on exception scope.** `async_wrapper` catches `Exception`, NOT `BaseException` — `asyncio.CancelledError`, `KeyboardInterrupt`, and `SystemExit` propagate without doing a synchronous blocking HTTP call inside a cancellation handler. That call has a 5s timeout; inside a cancellation handler it would (a) delay task cancellation by up to 5s on network errors, (b) make `Task was destroyed but it is pending` warnings more frequent and harder to diagnose, and (c) in some shutdown paths get cancelled itself, leaving cleanup incomplete (the server's `/cancel` is idempotent so this is acceptable — orphan via TTL/reconciliation instead). `sync_wrapper` catches `BaseException` to match existing `_protect_body` unify_block semantics; sync code has no event loop to delay and a Ctrl+C during a long sync agent gets a few seconds of cancel I/O before exit. + - **`fn_completed` sentinel.** After `fn(...)` returns, `fn_completed = True`. If `track_tool(...)` then fails (rare `/track` batch-sender network error), the wrapper's `except` runs but `fn_completed` is True so cancel does NOT fire — side effects already happened and the right move is to retry `track_tool`, not cancel (which would tell the server "no side effects" — a lie that produces a phantom budget refund and breaks audit). + - **Helper is fail-OPEN.** `_safe_cancel_active_execution` swallows everything (`NullRunTransportError`, `NullRunBackendError`, `get_runtime()` failures) so a cancel I/O failure never masks the original exception. An orphan from cancel failure is preferred over masking a `ValueError` from `fn()`. +- **P0-26 — `operation_id` server-vs-SDK divergence detection** (`src/nullrun/runtime.py`, `src/nullrun/context.py`). `_capture_server_minted_execution_id` previously read `response.get('operation_id')` directly; if a proxy or a backend bug echoed back a different `operation_id` than the SDK minted, the SDK had no signal — audit row stored one id, downstream `/track` used another. Now reads via `_get_op_id_for_capture()` (the SDK-minted contextvar value) and asserts `server_op_id != sdk_op_id` with ERROR log on disagreement. `idempotency_key` is derived from the SDK-minted value, not the server echo, so a divergent server response cannot break idempotency. +- **P0-27 — `operation_id` hoisted to contextvar; triple-mint collapsed to one** (`src/nullrun/context.py`, `src/nullrun/runtime.py`). New `_operation_id_var` contextvar (name `operation_id`) in `context.py`. `check_workflow_budget` mints ONCE via `_get_op_id_for_check()` / `_set_op_id_for_check()`; `execute` reads via `_get_op_id_for_execute()` with a single fallback mint+stash branch (only runs if `/check` did not run — pre-execution paths that bypass `/check`). The previous code minted at three sites independently, which meant a divergence between `/check` mint and `/execute` mint produced an audit-row-vs-/execute id mismatch. + +### Added + +- **`tests/test_protect_cancel_on_exception.py`** (7 tests). Pins both halves of the asymmetry and the helper's behavior: + - `test_1_async_cancelled_error_does_not_trigger_cancel` — the regression guard. If a future refactor reverts `except Exception` to `except BaseException` in `async_wrapper`, this test fails. `set_server_minted_execution_id(...)` is set so the helper WOULD have run if the except had caught BaseException; `cancel_calls` must be empty. + - `test_2_track_tool_failure_after_fn_completion_does_not_trigger_cancel` — the second regression guard. If someone removes the `fn_completed` sentinel, this test fails (cancel would run on a successfully-completed tool, producing a phantom refund). + - `test_3_async_fn_raises_value_error_triggers_cancel` — happy-path cancel. `cancel_calls == [("exec-test-123", "tool_exception")]`. + - `test_4_async_no_execution_id_skips_cancel` — control_plane reject happens pre-`/gate`, ContextVar stays None, no cancel. + - `test_5_sync_fn_raises_value_error_triggers_cancel` — sync ValueError path mirrors async. + - `test_6_sync_baseexception_also_triggers_cancel` — sync `KeyboardInterrupt` cancels (sync has no event loop to delay). + - `test_happy_path_no_cancel_called` — sanity: success path produces no cancel and gate order stays `control_plane, budget, track_tool`. +- **`tests/test_audit_p0_27_operation_id_hoist.py`** (8 tests). Pins the single-source-mint + server-vs-SDK divergence detection: + - contextvar name `operation_id` (forbids any other name; would silently disable mint if renamed without a corresponding accessor change). + - mint-site shapes in `check_workflow_budget` and `execute` (forbids pre-fix `str(uuid.uuid4())` mints outside the fallback branch). + - parity assertion in `_capture_server_minted_execution_id` (server_op_id vs sdk_op_id equality required for the no-warn path). + - fallback mint+stash in `execute` only fires when `/check` did not mint (idempotency: one operation_id per call site, never two). + +### Compatibility + +Pure reliability fixes — no wire-format change on either fix. Cancel-on-exception: existing exception paths unchanged; the cancel I/O is purely additive cleanup. Operation-id hoist: wire field `operation_id` is unchanged; the SDK now uses a single mint source and adds a server-divergence warning, both invisible to the wire contract. + +### Verification + +- Targeted suite: `tests/test_protect_cancel_on_exception.py` — 7/7 pass. +- Targeted suite: `tests/test_audit_p0_27_operation_id_hoist.py` — 8/8 pass. +- Broader regression suite: `pytest -q` clean (prior 1613 + 7 + 8 = 1628); `ruff check src tests` clean on the WIP files (decorators.py + test_protect_cancel_on_exception.py); `mypy src/nullrun` no issues reported in 37 source files. +- Wire-format: zero changes on both fixes. Same `/gate`, `/track`, `/execute`, `/cancel` payloads. The `/cancel` endpoint was already used by `runtime.cancel_execution(...)` from control-plane kill paths, just now also from the exception-cleanup helper. `operation_id` field on the wire was already a single string; this release only changes where the SDK mints/reads it locally. + +### Why this is needed + +**Cancel-on-exception** — at anti-DoS scale, every orphaned reservation is a permanent slot in `reserved_total` until TTL expiry. A noisy control-plane kill switch OR a single bad batch of tool exceptions could leak thousands of reservations per hour, gradually starving legitimate traffic out of the budget envelope. The fix collapses the leak window from "TTL expiry" (~minutes) to "synchronous cancel I/O on exception" (~ms), with the fail-OPEN helper guaranteeing we never trade an orphan for a swallowed exception. + +**Operation-id hoist** — pre-fix, three independent mints meant a race between `/check` and `/execute` could produce two different ids for the same logical call. The audit row stored one, `/execute` sent another, downstream `/track` chained off yet another. Server-vs-SDK divergence had no detection. P0-27 collapses to a single SDK-minted value; P0-26 adds the parity assertion so a divergent server echo is logged at ERROR before it propagates into audit/retry logic. + ## [0.16.4] - 2026-08-31 Patch release — ADR-037 Slice B. The wire protocol bumps from 3 → 4 additively: `/gate` response now echoes the SDK-supplied `action_digest` and a `policy_hash` slot (always `None` today; Slice D wires per-request computation). `min_protocol_version` stays at 2 so v3 SDKs are unaffected. Wire-format additive only — no new hashing/computation introduced on either side (both fields echo already-computed values). diff --git a/pyproject.toml b/pyproject.toml index 8d7d21b..93398d1 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.4" +version = "0.16.5" # Kept under the 200-char preview threshold so the full line is visible # without an "expand" click. The headline is the canonical §1 statement # from positioning.md — "runtime decision layer for tool-using AI agents" diff --git a/src/nullrun/__version__.py b/src/nullrun/__version__.py index f63c0f1..f46df0a 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.4" +__version__ = "0.16.5" __platform_version__ = "1.0.0" From 46d128ab9c57f08eef0b48f3d7aaa1668c531785 Mon Sep 17 00:00:00 2001 From: Anatoly Maltsev Date: Sat, 5 Sep 2026 13:17:29 +0400 Subject: [PATCH 4/4] style(sdk): ruff import-order fix in runtime.py + hoist test file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI on PR #97 (test (3.11)) failed on `ruff check src/` with 2 I001 import-order errors in src/nullrun/runtime.py — the inline `from nullrun.context import (...)` blocks introduced by the operation_id hoist (a9b398a) at runtime.py:1865 (check side) and :2791 (execute side) are correctly placed functionally but ruff flags them as un-sorted because they shadow the module-level context imports with a multi-name block. The 3rd I001 hit locally (in tests/test_audit_p0_27_operation_id_hoist .py:35) was a separate import-grouping issue from the same hoist commit and was caught by `ruff check src tests` on CI as well. All three are auto-fixable with `ruff check --fix`. Fix is purely cosmetic — runtime semantics + the operation_id hoist itself unchanged. CI now passes `ruff check src/` on PR #97. Files: src/nullrun/runtime.py (2 fix sites), tests/test_audit_p0_27 _operation_id_hoist.py (1 fix site). Verified: `ruff check src tests` all checks pass; `mypy src/nullrun` no issues reported in 37 source files; `pytest tests/test_protect _cancel_on_exception.py tests/test_audit_p0_27_operation_id_hoist.py ` — 15/15 pass. --- src/nullrun/runtime.py | 4 ++++ tests/test_audit_p0_27_operation_id_hoist.py | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nullrun/runtime.py b/src/nullrun/runtime.py index 07086dd..dc716af 100644 --- a/src/nullrun/runtime.py +++ b/src/nullrun/runtime.py @@ -1864,6 +1864,8 @@ def check_workflow_budget(self) -> None: # execution's response). from nullrun.context import ( get_operation_id as _get_op_id_for_check, + ) + from nullrun.context import ( set_operation_id as _set_op_id_for_check, ) @@ -2790,6 +2792,8 @@ def execute( # reuse whatever /check minted. from nullrun.context import ( get_operation_id as _get_op_id_for_execute, + ) + from nullrun.context import ( set_operation_id as _set_op_id_for_execute, ) diff --git a/tests/test_audit_p0_27_operation_id_hoist.py b/tests/test_audit_p0_27_operation_id_hoist.py index 9240801..0f90a82 100644 --- a/tests/test_audit_p0_27_operation_id_hoist.py +++ b/tests/test_audit_p0_27_operation_id_hoist.py @@ -47,7 +47,6 @@ set_operation_id, ) - SDK_ROOT = Path(__file__).resolve().parent.parent RUNTIME_PY = SDK_ROOT / "src" / "nullrun" / "runtime.py" CONTEXT_PY = SDK_ROOT / "src" / "nullrun" / "context.py"