Repository navigation
chore(release): 0.23.0 — MCP approval re-entry + zero-code budget pre-flight - #119
Merged
Merged
Conversation
…ding, canonical argument bag, missing /gate pre-flight (NR-058 / NR-015, SDK half)
## Summary
The MCP path is the second enforcement path, and it could not complete
an approval round trip at all. `MCPAdapter.call_tool` had no
`approval_id` parameter, so a retry after the operator approved landed
in `**passthrough` and was forwarded to the MCP SERVER — the gate never
saw the grant. The adapter sent neither `business_impact` nor
`action_digest`, so the approval row was never digest-bound and the
backend could not detect a tampered payload at consume time. The tool
arguments travelled only as the legacy `input` field, while the backend
resolves the canonical bag as `tool_arguments` → `tool_params` → Null
(ADR-060 §4) — so every params-based rule saw "no bag". And the adapter
called `runtime.execute` with NO pre-flight: the backend's
`/api/v1/execute` runs an existence check on the Redis binding
`execution:{id}` and answers 404 EXECUTION_NOT_FOUND when no prior
`/gate` minted it, so a direct `adapter.call_tool(...)` never reached
the MCP server.
### Fixed
- **NR-058 (SDK half)** — `MCPAdapter.call_tool` gains three NAMED
keyword-only parameters (`approval_id`, `business_impact`,
`action_digest`) placed BEFORE `**passthrough`, so they can never fall
into the passthrough and be forwarded to the MCP server. The adapter
forwards them into `runtime.execute(...)`.
- **NR-058 (SDK half, wire)** — `NullRunRuntime.execute` gains
`approval_id` and `tool_arguments` and forwards them into the
`execute_kwargs` it already builds, using the same conditional
pattern as `business_impact` / `action_digest`: the key is added only
when the value is not None, because an absent field means "no grant"
and that is not the same wire statement as a null.
`Transport.execute` already accepted both and put them on the wire;
the runtime was the only missing link. The two new parameters are
appended AFTER `tools` so the existing positional order of the
parameters above is unchanged.
- **NR-015 (SDK half)** — the same argument dict now reaches the wire
as BOTH `input` (the tool payload, unchanged) and `tool_arguments`
(the canonical bag). No new wire field is invented:
`tool_arguments` is the field the backend already resolves.
- **Pre-flight** — before `runtime.execute(...)` the adapter now
ensures a `/gate` call exists for this logical action: when
`nullrun.context.get_server_minted_execution_id()` returns None (no
prior `/gate` in this context) it calls
`runtime.check_workflow_budget()` first; otherwise it SKIPS it. The
pre-flight RESERVES money at the gate, so running it twice would
charge the org twice for one logical call — the condition is the same
contextvar `runtime.execute` reads to decide whether to reuse the
execution id, so the two agree by construction. A refusal from the
pre-flight propagates and the MCP client is NOT called. Ordering
mirrors `@protect` (`decorators._protect_body`): contextvar stamp →
`/gate` pre-flight → `/execute` → body.
### Verification
| Check | Result |
|---|---|
| `pytest tests/test_mcp_approval_wire.py` (pre-fix source) | 10 failed, 5 passed — RED |
| `pytest tests/test_mcp_approval_wire.py` (post-fix) | 15 passed — GREEN |
| `pytest tests/` | 1928 passed, 12 skipped (pristine HEAD: 1925 collected; +15 new) |
| `ruff check src/` (the CI gate) | All checks passed |
| `ruff check` on the touched test files | All checks passed |
| `ruff format --check` | broken repo-wide on the installed ruff 0.16.8 — 114 pristine-HEAD files would be reformatted; pre-existing, untouched by this change |
| `mypy src/` | 5 errors, all in `src/nullrun/instrumentation/auto.py` — identical on pristine HEAD; pre-existing, untouched by this change |
New test file: `tests/test_mcp_approval_wire.py` (15 tests). The two
existing MCP test modules gained a `check_workflow_budget` method on
their runtime doubles — the adapter now performs a pre-flight that a
runtime double has to model.
…йдеру (NR-059) Проблема: на zero-code пути (транспорт httpx/requests, без @Protect и без set_call_context) SDK отправлял llm_call ТОЛЬКО ПОСЛЕ ответа провайдера. Событие — единственное, что двигает счётчик бюджета, поэтому звонок, который потолок должен был остановить, был учтён, а не остановлен. Контроль-центр показывал сумму уже потраченного. Что сделано: - check_workflow_budget() принимает model / estimated_tokens / max_output_tokens / use_gate_cache и возвращает id резерва. - Транспорт httpx (sync + async) и requests делают пре-флайт ДО запроса к провайдеру: читают модель, размер промпта и потолок вывода из тела запроса и резервируют их. Отказ гейта бросается наружу — запрос к провайдеру НЕ уходит. - Резерв едет с событием явно (track(..., reservation_id=...)), а не через контеквар: контеквар — один слот на задачу, два параллельных звонка поменялись бы резервами. - Звонок, который не дал usage (ошибка сети, пустое тело, нет блока usage), освобождает свой резерв нулевым /track — иначе резерв держал бы бюджет до воркера reclaim. - Если в области уже есть свежий резерв (путь @Protect резервирует один раз на всю область), пре-флайт НЕ делается: второй резерв осиротел бы первый. - Оценка входа: ~3 символа на токен, CJK — 1 токен на символ, base64 не считается текстом, потолок 32768 токенов. Потолок вывода объявляется только если его объявил вызывающий: без него верхней границы нет, а цена по каталожному максимуму заморозила бы месячный бюджет на одном звонке. - README: правда по каждому пути — что потолок останавливает, а что может остановить только следующий звонок. Проверено: - 1948 тестов проходят, 12 пропущено; ruff чист; mypy — базовая линия (5 существовавших ошибок в auto.py, к этой правке отношения не имеют). - Новое: tests/test_zero_code_preflight.py (20 тестов). - Мутациями доказано, что тесты ловят поломку: A (потеря резерва) — 6 красных; B (отключённое освобождение) — 3 красных; C (однопоточная передача резерва) — конкурентный тест красный, для этого в тесте стоит барьер, без него тест был вакуумным; D (игнор резерва области) — красный. - Пин в tests/test_audit_p0_27_operation_id_hoist.py пришлось переписать: он пиннил старую сигнатуру def check_workflow_budget(self) -> None. Теперь срез идёт по границе метода, а не по сигнатуре.
…-flight ## Summary Minor release. Two enforcement paths that could not complete their own contract are closed. The MCP path could not finish an approval round trip at all: a retry after the operator approved landed in `**passthrough` and was forwarded to the MCP server, so the gate never saw the grant. The zero-code LLM path booked the call only AFTER the provider had already answered — the `llm_call` event is the only thing that moves the budget counter, so a call the ceiling was supposed to stop was counted instead of stopped, and the control centre showed the sum of spend that had already happened. Both fixes are SDK-side; there is no wire-protocol change. The public surface grows by ten keyword-only parameters across four public methods, and one changed return value. ## Fixed ### 1. MCP approval re-entry (NR-058 / NR-015, SDK half) — 5021ed3 `MCPAdapter.call_tool` had no `approval_id` parameter, so a retry after the operator approved landed in `**passthrough` and was forwarded to the MCP SERVER — the gate never saw the grant. The adapter sent neither `business_impact` nor `action_digest`, so the approval row was never digest-bound and the backend could not detect a tampered payload at consume time. The tool arguments travelled only as the legacy `input` field, while the backend resolves the canonical bag as `tool_arguments` → `tool_params` → Null (ADR-060 §4) — so every params-based rule saw "no bag". Three NAMED keyword-only parameters (`approval_id`, `business_impact`, `action_digest`) are placed BEFORE `**passthrough`, so they can never fall into the passthrough and be forwarded to the MCP server. `NullRunRuntime.execute` gains `approval_id` and `tool_arguments` and forwards them into the `execute_kwargs` it already builds, using the same conditional pattern as `business_impact` / `action_digest`: the key is added only when the value is not None, because an absent field means "no grant" and that is not the same wire statement as a null. The two new parameters are appended AFTER `tools` so the existing positional order of the parameters above is unchanged. The same argument dict now reaches the wire as BOTH `input` (the tool payload, unchanged) and `tool_arguments` (the canonical bag). No new wire field is invented: `tool_arguments` is the field the backend already resolves. Separately, `/api/v1/execute` runs an existence check on the Redis binding `execution:{id}` and answers 404 EXECUTION_NOT_FOUND when no prior `/gate` minted it, so a DIRECT `adapter.call_tool(...)` never reached the MCP server at all. Before `runtime.execute(...)` the adapter now ensures a `/gate` call exists for this logical action: when `get_server_minted_execution_id()` returns None (no prior `/gate` in this context) it calls `runtime.check_workflow_budget()` first; otherwise it SKIPS it. The pre-flight RESERVES money at the gate, so running it twice would charge the org twice for one logical call — the condition is the same contextvar `runtime.execute` reads to decide whether to reuse the execution id, so the two agree by construction. A refusal from the pre-flight propagates and the MCP client is NOT called. Ordering mirrors the `@protect` wrapper (contextvar stamp → `/gate` pre-flight → `/execute` → body). (+558/-4; test file `tests/test_mcp_approval_wire.py`, 15 tests) ### 2. Zero-code budget pre-flight (NR-059) — 4bc9322 On the zero-code path (httpx / requests transport, no `@protect`, no `set_call_context`) the SDK sent the `llm_call` event ONLY AFTER the provider's response. The event is the only thing that moves the budget counter, so a call the cap should have stopped was billed, not stopped. The httpx (sync + async) and requests transports now run the pre-flight BEFORE the request reaches the provider: they read the model, the prompt size and the output cap out of the request body and reserve them. A gate refusal is raised outward — the provider request does NOT leave. The reservation rides with the event explicitly (`track(..., reservation_id=...)`) rather than through a contextvar: the contextvar is one slot per task, so two parallel calls would swap reservations. A call that produced no usage (network error, empty body, no usage block) releases its reservation with a zero-valued `/track` — otherwise the reservation would hold budget until the reclaim worker ran. When the scope already holds a fresh reservation (the `@protect` path reserves once for the whole scope) the pre-flight is SKIPPED: a second reservation would orphan the first. `check_workflow_budget` gains `model` / `estimated_tokens` / `max_output_tokens` / `use_gate_cache` and returns the server-minted reservation id (`str | None`). `NullRunRuntime.track` gains `reservation_id`. Token estimation is ~3 characters per token, 1 token per CJK character, base64 is not counted as text, and the input estimate is capped at 32768 tokens. The output cap is declared ONLY if the caller declared one — without it there is no upper bound, and pricing at the catalog maximum would freeze a monthly budget on a single call. The README now states, per path, what the ceiling stops and what only the NEXT call can stop. (+1210/-35; test file `tests/test_zero_code_preflight.py`, 20 tests) ### 3. Lint hygiene — pre-flight `ruff check src tests` was red on `origin/master`: `tests/test_websocket_extra_is_declared.py` imported `sys` twice (F811 + I001), a duplicate left behind when the 3.10 `tomli` fallback was added in 0.22.2, and the new `tests/test_zero_code_preflight.py` had one blank line too many after its import block. CI only gates `ruff check src/`, so neither was caught there; the release runbook's local gate is `ruff check src tests`. Both edits are import-ordering only — no behaviour changed. ## Verification | Check | Result | |---|---| | `ruff check src tests` | All checks passed | | `ruff check src/` (CI gate) | All checks passed | | `mypy src/nullrun` | Success: no issues found in 39 source files | | `mypy src/` (CI gate) | Success: no issues found in 39 source files | | `pytest -q` | **1948 passed, 12 skipped** in 192.05s (vs baseline 1913 / 12 — **+35 new tests** from `test_mcp_approval_wire` (15) and `test_zero_code_preflight` (20)) | | Scratch diff | clean (`dist_local/`, `*.defect*` absent) | | `nullrun.__version__` | `0.23.0` | | Wire-format compatibility | unchanged from 0.22.2 — no protocol change; `approval_id` and `tool_arguments` are the fields the backend already resolves, and each is sent only when set (absent ≠ null) |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Minor release. Two enforcement paths that could not complete their own
contract are closed. The MCP path could not finish an approval round
trip at all:
MCPAdapter.call_toolhad noapproval_idparameter, so aretry after the operator approved landed in
**passthroughand wasforwarded to the MCP server — the gate never saw the grant. The adapter
sent neither
business_impactnoraction_digest, so the approval rowwas never digest-bound; and the tool arguments travelled only as the
legacy
inputfield, while the backend resolves the canonical bag astool_arguments→tool_params→ Null (ADR-060 §4) — so everyparams-based rule saw "no bag". On top of that,
/api/v1/executeanswers 404 EXECUTION_NOT_FOUND when no prior
/gateminted theexecution id, so a direct
adapter.call_tool(...)never reached the MCPserver at all.
The zero-code LLM path had the mirror-image defect: it sent the
llm_callevent ONLY AFTER the provider had answered. That event isthe only thing that moves the budget counter, so a call the ceiling was
supposed to stop was counted instead of stopped — the control centre
showed the sum of spend that had already happened. The httpx (sync and
async) and requests transports now reserve the call's expected cost
BEFORE the request leaves the process, so a refusal at the gate means
the provider is never called.
Both fixes are SDK-side; there is no wire-protocol change. The public
surface grows by ten keyword-only parameters across four public methods,
and one changed return value.
Migration
MCPAdapter.call_toolcan carry an approval grant. Three NAMEDkeyword-only parameters —
approval_id,business_impact,action_digest— placed BEFORE**passthroughso they can neverfall into the passthrough and be forwarded to the MCP server. A
caller that wrote
adapter.call_tool("x", {}, approval_id="...")was sending the approval id to the MCP server as a tool argument;
now it goes to the gate.
NullRunRuntime.executegainsapproval_idandtool_arguments. Both are appended AFTERtools, so the existingpositional order of the parameters above is unchanged. Each key is
added to the wire only when the value is not
None— an absentfield means "no grant", which is not the same wire statement as a
null.
check_workflow_budgetreturns the reservation id. The methodgains
model,estimated_tokens,max_output_tokensanduse_gate_cache, and its return type changes fromNonetostr | None. Callers that ignored the return value are unaffected.The
@protectpre-flight still sendsestimated_tokens=1, becausethat path answers "is ANY budget left?" and not "what will THIS call
cost?".
NullRunRuntime.trackgainsreservation_id. The reservationtravels with the cost event explicitly rather than through a
contextvar: the contextvar is one slot per task, so two concurrent
calls would swap reservations.
A call that the ceiling rejects never reaches the provider. This is
visible: a capped zero-code caller will now see refusals where it
previously saw successful calls that were billed anyway.
Fixed
NR-058 (SDK half) — the MCP path could not complete an approval
round trip.
MCPAdapter.call_toolhad noapproval_idparameter, so a retry after the operator approved landed in
**passthroughand was forwarded to the MCP SERVER — the gate neversaw the grant. The adapter also sent neither
business_impactnoraction_digest, so the approval row was never digest-bound and thebackend could not detect a tampered payload at consume time. The
three parameters are now named keyword-only members of the public
signature and are forwarded into
runtime.execute(...), whichgained the matching
approval_idparameter —Transport.executealready accepted it and put it on the wire; the runtime was the only
missing link. Separately, the adapter now runs the
/gatepre-flightitself when
get_server_minted_execution_id()isNoneand SKIPS itotherwise — that path reserves money at the gate, and a second
/gatewould charge the org twice for one logical call. Thecondition is the same contextvar
runtime.executereads, so the twoagree by construction. A refusal from the pre-flight propagates and
the MCP client is NOT called. (
5021ed3, +558/-4; test filetests/test_mcp_approval_wire.py, 15 tests)NR-015 (SDK half) — the canonical argument bag never reached the
wire. The tool arguments travelled only as the legacy
inputfield, while the backend resolves the canonical bag as
tool_arguments→tool_params→ Null (ADR-060 §4) — so everyparams-based rule saw "no bag", and the approval decision had no
argument snapshot to bind to. The same dict now reaches the wire as
BOTH
input(the tool payload, unchanged) andtool_arguments(thecanonical bag). No new wire field is invented:
tool_argumentsisthe field the backend already resolves. (
5021ed3)NR-059 — the zero-code LLM path booked the call after the provider
had already been paid. On the zero-code path (httpx / requests
transport, no
@protect, noset_call_context) the SDK sent thellm_callevent ONLY AFTER the provider's response. The event isthe only thing that moves the budget counter, so a call the cap
should have stopped was counted, not stopped. Now the httpx (sync
and async) and requests transports run the pre-flight BEFORE the
request leaves: they read the model, the prompt size and the output
cap out of the request body and reserve them, and a gate refusal is
raised outward so the provider request is never sent. The
reservation travels with the event explicitly
(
track(..., reservation_id=...)), because the contextvar carrier isone slot per task and two parallel calls would swap reservations. A
call that produced no usage (network error, empty body, no usage
block) releases its reservation with a zero-valued
/track—otherwise the reservation would hold budget until the reclaim worker
ran. When the scope already holds a fresh reservation (the
@protectpath reserves once for the whole scope) the pre-flight isSKIPPED: a second reservation would orphan the first. Token
estimation is ~3 characters per token, 1 token per CJK character,
base64 is not counted as text, and the estimate is capped at 32768
tokens. The output cap is declared ONLY if the caller declared one —
without it there is no upper bound, and pricing at the catalog
maximum would freeze a monthly budget on a single call. The README
now states, per path, what the ceiling stops and what only the NEXT
call can stop. (
4bc9322, +1210/-35; test filetests/test_zero_code_preflight.py, 20 tests)Cleanup
ruff check src testswas redon
origin/master:tests/test_websocket_extra_is_declared.pyimported
systwice (F811 + I001), a duplicate left behind when the3.10
tomlifallback was added in 0.22.2, and the newtests/test_zero_code_preflight.pyhad one blank line too manyafter its import block. CI only gates
ruff check src/, so neitherwas caught there; the release runbook's local gate is
ruff check src tests. Both edits are import-ordering only — nobehaviour changed.
Tooling
mypy src/(the CI gate): Success: no issues found in 39 sourcefiles. The two fix commits record 5 errors in
instrumentation/auto.pyunder their local mypy build; under thepinned CI mypy this tree is clean, and
mypy src/nullrunis cleantoo.
pyproject.toml:9,src/nullrun/__version__.py:8,CHANGELOG.md(full 0.23.0 block), and the one-line
uv.lockstamp0.22.2 → 0.23.0.Verification
ruff check src testsruff check src/(CI gate)mypy src/nullrunmypy src/(CI gate)pytest -qtest_mcp_approval_wire(15) andtest_zero_code_preflight(20))dist_local/,*.defect*absent)nullrun.__version__0.23.0approval_idandtool_argumentsare the fields the backend already resolves, and each is sent only when set (absent ≠ null)Commits included