Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion src/nullrun/__version__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
string and the SDK_MIN_VERSION constant.
"""

__version__ = "0.16.4"
__version__ = "0.16.5"
__platform_version__ = "1.0.0"
88 changes: 88 additions & 0 deletions src/nullrun/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down Expand Up @@ -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
# ---------------------------------------------------------------------------
Expand Down
97 changes: 83 additions & 14 deletions src/nullrun/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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]

Expand Down
Loading
Loading