From 52be7dbe65525ff61fbadab731bdd50a83de81d3 Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 13 Sep 2026 14:48:16 +0530 Subject: [PATCH 1/3] fix: G22 was passing for a reason that had nothing to do with G22 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A second adversarial review round found eight defects, four of them regressions introduced by answering the first round. This is the one that matters most, and the review did not find it: it fell out of a document written to test one of the review's mutation survivors. G22 resolved the charges it fills the budget with through Control._charges_for, which answers from _AUTHORITY_RESULT, a context variable only execute sets, and G22 runs it before its own control leg. It therefore returned () whenever nothing had executed in that context yet: the synthetic hold reserved nothing, the budget was never filled, and the next action ran. On examples/authority/payments.yaml, ctrlrun verify --only G22 reported FAIL, which is the status that means the kernel is broken. In a full run it got the right charges only because an earlier scenario's execute had left its result in that variable. The milestone's headline guarantee, the one that proves a budget holds a charge at all, was passing because of what ran before it. G22 now resolves authority for its own action and asks the Authority directly, and a control leg asserts the charges are non-empty rather than filling nothing quietly. T413s grades G22 alone and in a full run and requires the two to agree. Four regressions from the first round, all demonstrated by the review: - An observed resumed receipt reported another action's spend. execute clears _BUDGET_CHARGES at its top; resume relied on _resumed_charges to do it, and the observe path skipped that call. A receipt for an action whose metric cannot be measured claimed a charge of 700 belonging to a different effect, on the one receipt §8.3 makes the whole evidence for an MCP multi round-trip. That is T448's defect on the observe path. - The resumed recompute was handed a throwaway _Observation(), so the block was discarded and the event written twice: the receipt said ALLOW with no blocked reason while two ACTION_DENIED events beside it said otherwise. An answer and the evidence may not disagree about the same action, which is the rule acs.py states one boundary lower. - Splitting the observe check dropped effect_key from the observed budget refusal, so the one event naming which effect a budget would refuse stopped naming it. - _UnmeasurableError was not picklable and InvalidArgument is. A caller fanning Control.execute across a ProcessPoolExecutor lost the pool instead of catching the refusal. Nothing in this repository pickles it, so only probing the type could have found it. And verify still exited 3 and still reported FAIL on ordinary documents, because _deciding_grant was wired into the resize loop but not the early return, and because it applied two of the four predicates Authority.evaluate applies. A task-bound grant carrying no budget was returned as the decider for an action outside its task, so a sibling's budget went unchecked. It now applies all four, in the same order. T413l to T413s pin these, plus four guards a mutation run found removable with the whole suite green: the grant-identity check in the resize loop, the resource-miss refinement, the metric miss note, and the per-candidate unmeasurable reset. Signed-off-by: arpan --- src/ctrlrun/control.py | 82 +++++-- src/ctrlrun/verify/scenarios.py | 128 ++++++++--- tests/test_budget_holds.py | 112 ++++++++++ tests/test_verify_authority.py | 364 ++++++++++++++++++++++++++++++++ 4 files changed, 636 insertions(+), 50 deletions(-) diff --git a/src/ctrlrun/control.py b/src/ctrlrun/control.py index 2691413b..3d00428c 100644 --- a/src/ctrlrun/control.py +++ b/src/ctrlrun/control.py @@ -573,6 +573,22 @@ def __init__(self, message: str, *, reason: str) -> None: super().__init__(message) self.reason = reason + def __reduce__(self) -> tuple[Any, tuple[Any, ...]]: + """Keep this picklable, because `InvalidArgument` is. + + The default reconstruction is `(cls, self.args)`, and this `__init__` takes `reason` + keyword-only, so unpickling raised `TypeError` and a caller fanning `Control.execute` + across a `ProcessPoolExecutor` lost the pool instead of catching the refusal. Nothing + in this repository pickles it -- verify's children speak JSON over stdin -- so an + independent review found it by probing the type rather than by a failing run. + """ + return (_rebuild_unmeasurable, (str(self), self.reason)) + + +def _rebuild_unmeasurable(message: str, reason: str) -> _UnmeasurableError: + """Module-level so `pickle` can find it by name.""" + return _UnmeasurableError(message, reason=reason) + class _ObservedRefusalError(Exception): """SPEC-v0.9 §5.2.2 — observe mode's would-have-refused, which escapes `_in_scope` and is @@ -1699,7 +1715,7 @@ def _observe_secure( # budget first told an operator to raise a limit when the real answer was that the effect # had already happened. The clauses above have returned by now on every refusal enforce # mode would have hit first, so what reaches here is what the budget would decide. T452. - self._observe_spend(action, charges, observation) + self._observe_spend(action, charges, observation, effect_key) return approval, reservation def _observe_take( @@ -1804,10 +1820,16 @@ def resume(self, continuation: str, executor: Callable[[], Any]) -> Receipt: # for an action that spent, and a gateway that ran another action in this context since # the suspension would report *that* action's spend. The ledger is the record; the first # leg wrote it inside the reservation's own transaction. T447, T448. - # In observe mode the ledger is empty by design, so it is recomputed below, after the - # authority result this needs exists. §4.2.1a, T457. - if not self._observing: - self._resumed_charges(held.effect_key, held.record.attempt) + # **Unconditional, because this call is also the reset.** `execute` clears + # `_BUDGET_CHARGES` at its own top (§6.3.1's reason: a refusal carrying the previous + # action's numbers); `resume` has no such line, so skipping this in observe mode left the + # contextvar holding whatever the last action in this context had put there. An + # independent review demonstrated the consequence: a resumed `observed` receipt for an + # action whose own metric could not be measured reported a charge of 700 belonging to a + # different effect. That is T448's defect on the observe path, on the one receipt §8.3 + # makes the whole evidence for an MCP multi round-trip. In observe mode the ledger is + # empty by design, so this sets `()` and §4.2.1a's counterfactual is computed below. + self._resumed_charges(held.effect_key, held.record.attempt) # SPEC-v0.3 §2.5 — a continuation is a store-wide token, so a Control in another # environment can reach one. Evaluating a staging action inside a production # deployment is the fail-open §2.5 exists to close. @@ -1849,7 +1871,12 @@ def resume(self, continuation: str, executor: Callable[[], Any]) -> Receipt: # # Enforce mode keeps the ledger read: there the row is evidence of a spend that # happened, and a recomputed number would be a claim about it instead. - self._observe_charges(action, held.effect_key, _Observation()) + # + # Computed below, once the **real** observation exists. It used to be handed a + # throwaway `_Observation()`, which swallowed the block: the resumed receipt then + # said `decision=ALLOW, blocked_reason=None` for an action enforce mode refuses, + # while the two `ACTION_DENIED` events beside it said otherwise. + pass # SPEC-v0.3 §6.3 — a resumption in observe mode gets the same `observed` receipt its # first leg did. It is the *only* receipt an MCP multi round-trip ever gets (§8.3), so # a resumed leg reporting `committed` under a mode that enforces nothing would put the @@ -1861,6 +1888,9 @@ def resume(self, continuation: str, executor: Callable[[], Any]) -> Receipt: observation.decided(evaluation) if evaluation.decision is Decision.DENY: observation.block(evaluation.reason) + # §4.2.1a's counterfactual, on the real observation and announcing nothing: the + # first leg already wrote this action's `ACTION_DENIED`. + self._observe_charges(action, held.effect_key, observation, announce=False) # SPEC-v0.7 §6.8: **no recheck on a resumed leg**, for `v0.6 §7.2.3`'s reason. The # approval was consumed on the first leg, after that leg's recheck, and refusing here # would strand a reservation the remote may already be acting on. The receipt says so: @@ -3279,6 +3309,7 @@ def _charges_for( action: Action, effect_key: str | None, observation: _Observation | None = None, + announce: bool = True, ) -> tuple[Charge, ...]: """What this action spends, one `Charge` per ancestor (SPEC-v0.9 §2.7). @@ -3301,7 +3332,7 @@ def _charges_for( # §2.3. The kernel cannot measure what this action spends, so it cannot hold the # grant to its budget, so it declines to run it. Recorded before it is re-raised. raise self._refuse_unmeasurable( - action, BUDGET_UNMEASURABLE, unmeasurable, observation + action, BUDGET_UNMEASURABLE, unmeasurable, observation, announce ) from None if charges and effect_key is None: raise self._refuse_unmeasurable( @@ -3314,11 +3345,16 @@ def _charges_for( "grant (SPEC-v0.9 §2.4.1)" ), observation, + announce, ) from None return charges def _observe_charges( - self, action: Action, effect_key: str | None, observation: _Observation + self, + action: Action, + effect_key: str | None, + observation: _Observation, + announce: bool = True, ) -> tuple[Charge, ...]: """§4.2.1's first half: what this action *would have* been charged, charging nothing. @@ -3331,8 +3367,11 @@ def _observe_charges( Called above the approval gate, where `_secure` computes the same thing, so the two modes agree about which refusal comes first (T451). """ + # **Cleared first, on every path.** Returning early without touching it left the + # previous action's charges on this action's receipt, which is the defect above. + _BUDGET_CHARGES.set(()) try: - charges = self._charges_for(action, effect_key, observation) + charges = self._charges_for(action, effect_key, observation, announce) except InvalidArgument: # Already reported by `_refuse_unmeasurable`, which blocked rather than denying. return () @@ -3352,7 +3391,11 @@ def _observe_charges( return charges def _observe_spend( - self, action: Action, charges: tuple[Charge, ...], observation: _Observation + self, + action: Action, + charges: tuple[Charge, ...], + observation: _Observation, + effect_key: str | None = None, ) -> None: """§4.2.1's second half: whether the budget would have refused, writing nothing. @@ -3399,6 +3442,9 @@ def spent(charge: Charge) -> int: "window": int(exhausted.window.total_seconds()), "observed": True, }, + # Splitting this method dropped the key, so the one event that names which + # effect the budget refused stopped naming it. Nothing noticed. + effect_key, ) except InvalidArgument: return @@ -3427,6 +3473,7 @@ def _refuse_unmeasurable( reason: str, error: InvalidArgument, observation: _Observation | None = None, + announce: bool = True, ) -> InvalidArgument: """§2.3 and §2.4.1's refusals, with the events and the receipt they were missing. @@ -3441,11 +3488,16 @@ def _refuse_unmeasurable( # the store, alongside the `observed` receipt for the run that went ahead: two # receipts for one action, disagreeing. T439c. observation.block(reason) - self._append( - EventType.ACTION_DENIED, - action, - {"reason": reason, "error": str(error), "observed": True}, - ) + if announce: + # **Not on a resumed leg**: its first leg already wrote this event for this + # action, and a second one made the evidence say the action was denied twice + # while the receipt beside it said `ALLOW`. An independent review found the two + # disagreeing, which is what `acs.py`'s clause forbids one boundary lower. + self._append( + EventType.ACTION_DENIED, + action, + {"reason": reason, "error": str(error), "observed": True}, + ) return _UnmeasurableError(str(error), reason=reason) self._append(EventType.ACTION_DENIED, action, {"reason": reason, "error": str(error)}) self._record( diff --git a/src/ctrlrun/verify/scenarios.py b/src/ctrlrun/verify/scenarios.py index 176d9222..c8ade75b 100644 --- a/src/ctrlrun/verify/scenarios.py +++ b/src/ctrlrun/verify/scenarios.py @@ -936,8 +936,16 @@ def _deciding_grant(self, action: Action) -> Grant | None: return None for grant_id in sorted(self.authority.grants): grant = self.authority.grants[grant_id] - if grant.matches_shape(action) and grant.constraints_hold(action): - return grant + # The same four predicates `Authority.evaluate` applies, in the same order + # (`authority.py:1237-1255`). **`task_holds` and `is_expired` are not optional + # here**: an independent review found a task-bound grant carrying no budget being + # returned as the decider for an action outside its task, so a sibling grant's + # budget was never checked and `ctrlrun verify` exited 3 on it. + if not grant.matches_shape(action) or not grant.constraints_hold(action): + continue + if grant.is_expired(self._t0) or not grant.task_holds(self._task): + continue + return grant return None #: How many spends of the chosen vector a scenario may take. G4's control leg runs @@ -968,6 +976,46 @@ def _fits_budgets(self, grant: Grant, action: Action, *, room: int = 1) -> bool return False return True + def _budget_verdict(self, grants: tuple[Grant, ...], action: Action, room: int) -> bool | None: + """`_fits_budgets` over **every** grant that could hold this action to a budget. + + Two of them, and an independent review demonstrated why both are needed. + `_deciding_grant` answers who `Authority.evaluate` resolves, and `select`'s own + `grant_filter` answers who the scenario is *about*: G9 delegates from the grant it + selected, so that grant's budget binds the delegation whatever the resolver says. Sizing + against only the resolver let a lexicographically earlier grant with no budget shadow a + delegable parent whose limit was 50 times smaller, and `ctrlrun verify` exited 3 on the + delegation's own budget. + """ + seen: dict[str, Grant] = {grant.id: grant for grant in grants} + for grant in seen.values(): + verdict = self._fits_budgets(grant, action, room=room) + if verdict is not True: + return verdict + return True + + def _shrunk( + self, arguments: dict[str, Any], grants: tuple[Grant, ...], action: Action, divisor: int + ) -> dict[str, Any] | None: + """One vector with **every** over-limit metric brought under its own budget. + + `_fitted_to_budgets` used to resize the first budgeted metric alone, so a grant with + budgets on two metrics was declined even when a fitting vector existed: an independent + review found a document where adding one `tip` budget took verify from grading thirteen + guarantees to grading none, silently and with exit 0. A budget per metric is an ordinary + shape, and each metric needs its own number. + """ + tried = dict(arguments) + for grant in grants: + for budget in grant.budgets or (): + try: + value = _metric_value(action, budget.metric, grant.id) + except InvalidArgument: + return None + if value * self._BUDGET_HEADROOM > budget.limit: + tried[budget.metric] = max(1, budget.limit // divisor) + return tried if tried != arguments else None + def _fitted_to_budgets( self, name: str, @@ -977,7 +1025,7 @@ def _fitted_to_budgets( grant: Grant, action: Action, ) -> tuple[dict[str, Any], Action] | None: - """Size verify's own action vector to the budgets that will actually decide it. + """Size verify's own action vector to the budgets that will decide it. **Verify grades a guarantee, not the operator's budget sizing.** `_synthesize` picks a vector to land in a rule, and a grant whose budget is smaller than that vector refuses @@ -986,17 +1034,16 @@ def _fitted_to_budgets( on G1, which is about approvals. That is `_identity_the_document_needs`'s case in the budget dimension, and it gets the same answer: verify supplies what the document needs. - Every candidate is checked against **the grant that would decide it**, not the grant the - caller is holding, because a resize can move the action between grants. The vector is - only changed when a budget would refuse it, so every document without budgets keeps the - vector it had, and a replacement must land in the same rule with the same reason because - `select`'s contract is the decision it was asked for. - - Where nothing fits, the candidate is declined and `select` moves on, so the guarantee - reports `N/A` with a true reason rather than failing a control leg. + The vector is only changed when a budget would refuse it, so every document without + budgets keeps the vector it had, and a replacement must land in the same rule with the + same reason because `select`'s contract is the decision it was asked for. Where nothing + fits, the candidate is declined and `select` moves on, so the guarantee reports `N/A` + with a true reason rather than failing a control leg. """ - deciding = self._deciding_grant(action) or grant - verdict = self._fits_budgets(deciding, action, room=self._BUDGET_HEADROOM) + deciding = self._deciding_grant(action) + bound = (grant,) if deciding is None else (grant, deciding) + room = self._BUDGET_HEADROOM + verdict = self._budget_verdict(bound, action, room) if verdict is True: return arguments, action if verdict is None: @@ -1005,34 +1052,25 @@ def _fitted_to_budgets( # because raising a limit fixes one and nothing about the other. self._unmeasurable = True return None - limits = [budget.limit for budget in (deciding.budgets or ()) if budget.limit > 0] - smallest = min(limits) if limits else 0 - metric = next(iter(deciding.budgets or ())).metric - # **Room for a scenario that acts more than once**, not for one action. G4's control leg - # alone runs `PROCESSES` children on distinct keys and then contends `PROCESSES` more on - # one, so it needs nine spends to fit; a vector sized to `limit` exactly made its control - # leg pass, its contended leg find zero winners, and the guarantee report **FAIL** -- the - # status that means the kernel is broken -- for a budget that was merely small. Verify - # may say it could not grade a configuration; it may not accuse the kernel of a defect. - headroom = self._BUDGET_HEADROOM - for candidate in (1, smallest // headroom, smallest // 8, smallest // 4, smallest // 2): - if candidate < 1: + for divisor in (room, 8, 4, 2, 1): + tried = self._shrunk(arguments, bound, action, divisor) + if tried is None: continue - tried = {**arguments, metric: candidate} rebuilt = replace(action, arguments=tried) evaluation = self.policy.evaluate(rebuilt) if evaluation.decision is not decision or evaluation.reason != reason: continue - # **Re-resolved, and it must settle on the same grant.** A resize can move the - # action between grants, and `_bind` is building a selection that names *this* one: - # a vector graded against a different grant's budget would report the wrong grant in - # the result and check a budget nobody will apply. Requiring the same grant subsumes - # re-checking its shape and constraints, because `_deciding_grant` only returns a - # grant that matched both. + # **Re-resolved, and it must settle on the same grant.** A resize can move the action + # between grants, and `_bind` is building a selection that names *this* one: a vector + # graded against a different grant's budget would report the wrong grant in the + # result and check a budget nobody will apply. settled = self._deciding_grant(rebuilt) - if settled is None or settled.id != grant.id: + if settled is not None and settled.id != grant.id and deciding is not None: continue - if self._fits_budgets(settled, rebuilt, room=headroom) is not True: + if not grant.matches_shape(rebuilt) or not grant.constraints_hold(rebuilt): + continue + after = (grant,) if settled is None else (grant, settled) + if self._budget_verdict(after, rebuilt, room) is not True: continue return tried, rebuilt return None @@ -3938,7 +3976,27 @@ def body(detail: dict[str, Any]) -> None: detail["limit"] = budget.limit detail["per_action"] = per_action action = selection.build() - charges = control._charges_for(action, selection.effect_key) + # **Resolved here, not read out of a context variable.** `Control._charges_for` + # answers from `_AUTHORITY_RESULT`, which only `execute` sets, and this runs before + # the control leg. It therefore returned `()` whenever nothing had executed in this + # context yet, the synthetic hold below reserved nothing, the budget was never + # filled, and G22 reported **FAIL** -- the kernel is broken -- on the shipped + # example under `ctrlrun verify --only G22`. + # + # In a full run it returned the right charges only because a previous scenario's + # `execute` had left its own result in that variable, so this guarantee was passing + # for a reason that had nothing to do with it. That is the false green this + # repository keeps finding, on the guarantee that proves budgets work at all. + assert self.authority is not None # `budgeted` is non-empty, so there is one + resolved = self.authority.evaluate( + action, now=control._clock(), store=store, task=selection.task + ) + charges = self.authority._charges_for(action, resolved, store=store) + _expect_control( + bool(charges), + "the selected action charges the budgeted grant", + f"no charge resolved for {selection.action} on {budgeted[0]}: {resolved.reason}", + ) # The control: with room, it runs. executor = _Executor() diff --git a/tests/test_budget_holds.py b/tests/test_budget_holds.py index bc2d8c02..44102cc2 100644 --- a/tests/test_budget_holds.py +++ b/tests/test_budget_holds.py @@ -1212,3 +1212,115 @@ def test_T458_observe_and_enforce_agree_when_an_action_is_both_out_of_scope_and_ assert receipt.would_have.blocked_reason == enforced, ( f"enforce refused {enforced!r} and the pilot was told {receipt.would_have.blocked_reason!r}" ) + + +# --- the second review round: four regressions on the observe and resume paths --------------- + + +def test_T459_an_observed_resumed_receipt_never_carries_another_actions_charges( + store, clock +) -> None: + """**T448's defect, reintroduced on the observe path**, on the one receipt §8.3 makes the + whole evidence for an MCP multi round-trip. + + `execute` clears `_BUDGET_CHARGES` at its own top; `resume` has no such line and relied on + `_resumed_charges` to do it, and an earlier fix skipped that call in observe mode. An + independent review demonstrated the result: a resumed `observed` receipt for an action whose + own metric cannot be measured reported a charge of 700 belonging to a different effect. + + The action here is unmeasurable on purpose, because that is the path that returns without + computing anything and so leaves whatever the contextvar held. + """ + from ctrlrun import Suspended + + control = _observing_control(store, clock) + unmeasurable = Action( + name="payments.refund", + arguments={"amount": -250, "id": "1"}, + principal=AGENT, + environment="prod", + ) + + def suspends() -> Any: + raise Suspended("round-1") + + with pytest.raises(Suspended): + control.execute(unmeasurable, suspends, "refund:1") + # A different action runs in the same context, leaving its own charges behind. + control.execute(_action("2", 100), lambda: {"ok": True}, "refund:2") + + receipt = control.resume("round-1", lambda: {"ok": True}) + + assert receipt.budget_charges == (), ( + f"the resumed receipt for {dict(receipt.arguments)} carried {receipt.budget_charges}" + ) + + +def test_T460_the_resumed_leg_records_one_denial_and_the_receipt_agrees_with_it( + store, clock +) -> None: + """**An answer and the evidence may not disagree about the same action**, which is the rule + `acs.py`'s own clause states one boundary lower. + + The resumed recompute was handed a throwaway `_Observation()`, so the block was discarded and + the event written a second time: the receipt said `decision=ALLOW, blocked_reason=None` while + the two `ACTION_DENIED` events beside it said the action was refused twice. + """ + from ctrlrun import Suspended + + control = _observing_control(store, clock) + unmeasurable = Action( + name="payments.refund", + arguments={"amount": -250, "id": "1"}, + principal=AGENT, + environment="prod", + ) + + def suspends() -> Any: + raise Suspended("round-1") + + with pytest.raises(Suspended): + control.execute(unmeasurable, suspends, "refund:1") + receipt = control.resume("round-1", lambda: {"ok": True}) + + denials = [e for e in store.events() if str(e.type) == "ACTION_DENIED"] + assert len(denials) == 1, [e.data.get("reason") for e in denials] + assert receipt.would_have is not None + assert receipt.would_have.blocked_reason == "budget_unmeasurable", receipt.would_have + + +def test_T461_the_observed_budget_refusal_names_the_effect_it_refused(store, clock) -> None: + """Splitting the observe check dropped `effect_key` from the one event that says which effect + a budget would have refused. Nothing noticed, which is why this exists.""" + enforcing = _control(store, clock) + enforcing.execute(_action("1", 250), lambda: {"ok": True}, "refund:1") + + observing = _observing_control(store, clock) + observing.execute(_action("2", 250), lambda: {"ok": True}, "refund:2") + + exhausted = [ + event + for event in store.events() + if str(event.type) == "ACTION_DENIED" and event.data.get("reason") == "budget_exhausted" + ] + assert exhausted, "no observed budget refusal was recorded" + assert exhausted[-1].effect_key == "refund:2", exhausted[-1].effect_key + + +def test_T462_the_unmeasurable_refusal_survives_a_pickle_as_InvalidArgument_did() -> None: + """`InvalidArgument` round-trips; a subclass with a keyword-only `reason` did not, because + the default reconstruction is `(cls, self.args)`. + + Nothing in this repository pickles it, so no test would have caught it by running: verify's + children speak JSON over stdin. A caller fanning `Control.execute` across a + `ProcessPoolExecutor` lost the pool instead of catching the refusal. + """ + import pickle + + from ctrlrun.control import _UnmeasurableError + + back = pickle.loads(pickle.dumps(_UnmeasurableError("nope", reason="budget_unkeyed"))) + + assert isinstance(back, InvalidArgument) + assert back.reason == "budget_unkeyed" + assert str(back) == "nope" diff --git a/tests/test_verify_authority.py b/tests/test_verify_authority.py index 8bd949ca..efc92179 100644 --- a/tests/test_verify_authority.py +++ b/tests/test_verify_authority.py @@ -671,3 +671,367 @@ def test_T413h_a_resize_never_silently_grades_a_different_rule(tmp_path): ) else: assert result.status is Status.NOT_APPLICABLE, result.reason + + +SHADOWED_DELEGABLE = """ +authority: + max_delegation_depth: 3 + grants: + - id: aa-broad + subject: { agent: "zz-*" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_gte: 0, amount_lte: 10000000 } + - id: zz-parent + subject: { agent: "zz-agent", user: "dana@example.com" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_gte: 0, amount_lte: 10000000 } + delegable: true + expires_at: "2027-01-01T00:00:00Z" + budgets: + - { metric: amount, limit: 1000, window: PT24H } +""" + +FLOORED_ALLOW = """ +actions: + acme.refund: + effect: "refund:{payment_id}" + resource: "payment:{payment_id}" + rules: + - when: { amount_gte: 50000, amount_lte: 10000000 } + decision: allow + - decision: deny +""" + +TWO_METRICS = """ +authority: + grants: + - id: g1 + subject: { agent: "payer" } + actions: ["pay.send"] + resources: ["acct:*"] + budgets: + - { metric: amount, limit: 100000, window: PT24H } + - { metric: tip, limit: 100000, window: PT24H } + +actions: + pay.send: + effect: "pay:{ref}" + resource: "acct:{ref}" + rules: + - when: { amount_lte: 100000, tip_lte: 100000 } + decision: allow + - decision: deny +""" + + +def test_T413i_a_grant_the_scenario_selected_is_held_to_its_budget_even_when_shadowed(tmp_path): + """**`select`'s `grant_filter` names the grant a scenario is about; the resolver may name + another.** G9 delegates from the grant it selected, so that grant's budget binds the + delegation whatever `Authority.evaluate` resolves for the parent action. + + `_deciding_grant` answers only the resolver, so a lexicographically earlier grant carrying no + budget shadowed a delegable parent whose limit was fifty times smaller: the vector was left + at the band floor, G9 delegated, and `ctrlrun verify` exited 3 on the delegation's own + budget. An independent review demonstrated it. + """ + path = _write(tmp_path, V7 + SHADOWED_DELEGABLE + FLOORED_ALLOW) + + graded = _by_id(run(path, only=("G9", "G22"))) + + for gid in ("G9", "G22"): + assert graded[gid].status is not Status.FAIL, f"{gid}: {graded[gid].reason}" + + +def test_T413j_a_grant_budgeting_two_metrics_is_graded_not_declined(tmp_path): + """A budget per metric is an ordinary shape, and each metric needs its own number. + + The resize set the **first** budgeted metric alone, so a vector over two budgets could never + be brought under both and every candidate was rejected. An independent review found a + document where adding one `tip` budget took verify from grading thirteen guarantees to + grading none, **silently, with exit 0**: every guarantee reported `N/A` and an operator + reading that would think their configuration had been checked. + """ + path = _write(tmp_path, V7 + TWO_METRICS) + + graded = _by_id(run(path, only=("G3", "G4", "G22"))) + + for gid in ("G3", "G4", "G22"): + assert graded[gid].status is Status.PASS, f"{gid} was not graded: {graded[gid].reason}" + + +def test_T413k_the_two_metric_vector_is_under_both_budgets(tmp_path): + """And the vector it picked really does satisfy both, rather than one of them twice.""" + path = _write(tmp_path, V7 + TWO_METRICS) + + result = _by_id(run(path, only=("G3",)))["G3"] + + graded = dict(result.arguments or {}) + assert graded["amount"] * 18 <= 100000, graded + assert graded["tip"] * 18 <= 100000, graded + + +TASKED_SHADOW = """ +authority: + grants: + - id: aa-tasked + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + tasks: ["nightly-run:*"] + - id: bb-budgeted + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + budgets: + - { metric: amount, limit: 100000, window: PT24H } + - { metric: tokens, limit: 100000, window: PT24H } +""" + +UPPER_ONLY = """ +actions: + acme.refund: + effect: "refund:{payment_id}" + resource: "payment:{payment_id}" + rules: + - when: { amount_lte: 100000 } + decision: allow + - decision: deny +""" + + +def test_T413l_the_deciding_grant_applies_every_predicate_evaluate_applies(tmp_path): + """`_deciding_grant` must answer the question `Authority.evaluate` answers, which means all + four of its predicates and not two. + + Checking only `matches_shape` and `constraints_hold` returned a **task-bound** grant as the + decider for an action outside its task. That grant carries no budget, so its sibling's was + never checked, and `ctrlrun verify` exited 3 on a refusal the kernel made correctly. An + independent review demonstrated it. + """ + path = _write(tmp_path, V7 + TASKED_SHADOW + UPPER_ONLY) + + result = _by_id(run(path, only=("G22",)))["G22"] + + assert result.status is not Status.FAIL, result.reason + assert "internal" not in str(result.reason or "").lower() + + +def test_T413m_a_resize_that_moves_the_action_to_another_grant_is_rejected(tmp_path): + """The grant-identity guard inside the resize loop, which was the headline of the commit that + added it and which a mutation run found untested: deleting it left every test green. + + `aa-narrow` sorts first and admits only tiny amounts; `bb-broad` is the grant the band puts + the action in. A candidate small enough for `bb-broad`'s budget falls inside `aa-narrow`, and + grading it there would report the wrong grant and check a budget nobody applies. + """ + document = ( + V7 + + """ +authority: + grants: + - id: aa-narrow + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_lte: 10 } + - id: bb-broad + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_lte: 500000 } + budgets: + - { metric: amount, limit: 90000, window: PT24H } +""" + + UPPER_ONLY + ) + path = _write(tmp_path, document) + + result = _by_id(run(path, only=("G3",)))["G3"] + + assert result.status is not Status.FAIL, result.reason + if result.status is Status.PASS: + # Whatever it graded, the grant it names must be the one that actually decides it. + assert dict(result.arguments or {})["amount"] > 10, ( + f"graded a vector that aa-narrow would decide: {result.arguments}" + ) + + +def test_T413n_a_budget_decline_does_not_claim_a_resource_miss(tmp_path): + """`select` records a grant miss for every failed candidate, so without the refinement a + single-grant document was told no grant's `resources:` matched about a pattern that matched + perfectly. A mutation run found the refinement untested.""" + path = _write(tmp_path, V7 + TIGHT_BUDGET + FLOORED_RULE) + + result = _by_id(run(path, only=("G4",)))["G4"] + + assert result.status is Status.NOT_APPLICABLE + assert reg.NO_GRANT_COVERS_SELECTION not in result.reason, result.reason + assert reg.NO_ACTION_FITS_THE_BUDGET in result.reason, result.reason + + +def test_T413o_the_metric_miss_carries_the_budget_note(tmp_path): + """The note that tells an operator what to do about it. Its *reason* was tested; the note + beside it was not, and a mutation dropping it left every test green.""" + path = _write(tmp_path, V7 + UNMEASURABLE_BUDGET + ACTIONS) + + result = _by_id(run(path, only=("G3",)))["G3"] + + assert result.reason.startswith(reg.NO_METRIC_TO_MEASURE), result.reason + assert reg.BUDGET_MISS_NOTE in str(result.detail), result.detail + + +def test_T413p_an_unmeasurable_grant_does_not_mislabel_a_later_budget_miss(tmp_path): + """The per-candidate reset. Without it a `True` left by an earlier grant makes the next + action's ordinary budget miss report as a metric miss, which is a different fix entirely. + + `aaa.unmeasurable` is budgeted on a metric it does not carry; `zzz.refund` is budgeted on one + it does and simply cannot afford. The second must be reported as what it is. + """ + document = ( + V7 + + """ +authority: + grants: + - id: aaa-grant + subject: { agent: "head-of-support" } + actions: ["aaa.unmeasurable"] + resources: ["payment:*"] + budgets: + - { metric: items, limit: 900, window: PT24H } + - id: zzz-grant + subject: { agent: "head-of-support" } + actions: ["zzz.refund"] + resources: ["payment:*"] + budgets: + - { metric: amount, limit: 900, window: PT24H } + +actions: + aaa.unmeasurable: + effect: "u:{payment_id}" + resource: "payment:{payment_id}" + rules: + - when: { amount_lte: 100000 } + decision: approve + - decision: deny + zzz.refund: + effect: "refund:{payment_id}" + resource: "payment:{payment_id}" + rules: + - when: { amount_gte: 50000, amount_lte: 100000 } + decision: approve + - decision: deny +""" + ) + path = _write(tmp_path, document) + + result = _by_id(run(path, only=("G1",)))["G1"] + + assert result.status is Status.NOT_APPLICABLE + assert reg.NO_ACTION_FITS_THE_BUDGET in result.reason, result.reason + + +def test_T413q_the_result_names_the_grant_that_actually_decides_the_graded_vector(tmp_path): + """The grant-identity guard's real subject: **the report must not name the wrong grant.** + + A resize can move the action into a lexicographically earlier grant. The budget re-check + below catches that when the new grant has a budget to violate; when it does not, the only + consequence is that the result names one grant while `Authority.evaluate` resolves another, + and a mutation run found nothing asserting otherwise. + """ + document = ( + V7 + + """ +authority: + grants: + - id: aa-narrow + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_lte: 10 } + - id: bb-broad + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + constraints: { amount_lte: 500000 } + budgets: + - { metric: amount, limit: 90000, window: PT24H } +""" + + UPPER_ONLY + ) + path = _write(tmp_path, document) + + result = _by_id(run(path, only=("G3",)))["G3"] + + if result.status is not Status.PASS: + pytest.skip(f"nothing graded: {result.reason}") + graded = dict(result.arguments or {}) + # Whichever grant the report names, it has to be the one that would decide this vector. + deciding = "aa-narrow" if graded["amount"] <= 10 else "bb-broad" + assert result.grant_id in (None, deciding), ( + f"the report names {result.grant_id!r} for a vector {deciding!r} decides: {graded}" + ) + + +def test_T413r_an_earlier_grants_budget_binds_a_task_filtered_selection(tmp_path): + """G24 selects by task, and a grant naming **no** `tasks:` authorises every task, so an + earlier one still decides and its budget still binds. + + Sizing the vector against only the selected grant let an earlier, tighter budget go + unchecked. This is the shape that makes checking the deciding grant load-bearing rather than + merely principled. + """ + document = ( + V7 + + """ +authority: + grants: + - id: aa-tight + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + budgets: + - { metric: amount, limit: 90, window: PT24H } + - id: zz-tasked + subject: { agent: "head-of-support" } + actions: ["acme.refund"] + resources: ["payment:*"] + tasks: ["refund-run:*"] + budgets: + - { metric: amount, limit: 900000, window: PT24H } +""" + + UPPER_ONLY + ) + path = _write(tmp_path, document) + + graded = _by_id(run(path, only=("G22", "G24"))) + + for gid in ("G22", "G24"): + assert graded[gid].status is not Status.FAIL, f"{gid}: {graded[gid].reason}" + assert "internal" not in str(graded[gid].reason or "").lower(), graded[gid].reason + + +def test_T413s_G22_grades_the_same_alone_as_it_does_in_a_full_run(tmp_path): + """**The milestone's headline guarantee was passing for the wrong reason.** + + G22 resolved the charges it fills the budget with through `Control._charges_for`, which + answers from `_AUTHORITY_RESULT` -- a context variable only `execute` sets -- and it runs + before its own control leg. So it returned `()` whenever nothing had executed in that context + yet: the synthetic hold reserved nothing, the budget was never filled, and `ctrlrun verify + --only G22` reported **FAIL** on `examples/authority/payments.yaml`, telling an operator the + kernel is broken. + + In a full run it got the right charges only because an earlier scenario's `execute` had left + its result in that variable. A guarantee that passes because of what ran before it is not + graded, and `--only` is the switch that shows it. + """ + path = _write(tmp_path, V7 + TIGHT_BUDGET.replace("limit: 900", "limit: 9000") + ACTIONS) + + alone = _by_id(run(path, only=("G22",)))["G22"] + together = _by_id(run(path))["G22"] + + assert alone.status is together.status, ( + f"alone: {alone.status} ({alone.reason}); in a full run: {together.status}" + ) + assert alone.status is not Status.FAIL, alone.reason From 055454773f7954630af6a7f8c3cab9ec02d276fd Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 13 Sep 2026 14:52:08 +0530 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20observe=20mode=20reports=20one=20ref?= =?UTF-8?q?usal,=20and=20=C2=A74.2.1=20stops=20overclaiming=20about=20whic?= =?UTF-8?q?h?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enforce mode raises at the first refusal and never reaches the budget; observe mode runs every check and kept going, so it wrote a budget_exhausted event for an action enforce mode refuses out of scope. An operator reading the log saw a refusal that would never have happened. _observe_spend skips itself once anything has blocked. Its own docstring claimed the earlier clauses had already returned by then, which is not true of the scope block or the approval gate, and that sentence is corrected rather than left as the reason the bug was invisible. And §4.2.1 said the report and the enforcement cannot drift. That is true of the arithmetic and false of the ordering, which an independent review demonstrated twice more after three cases had already been fixed. §4.2.1b now states the limit: observe mode reports a refusal exactly when enforce mode would refuse, and does not promise to name the same one. The two cases still unaligned are named, with what they cost, and neither is an enforcement difference. Aligning the rest means one ordered list of checks both modes walk, which is a refactor of _secure and _observe_secure together rather than a fourth reordering. Three reorderings in this milestone produced four regressions between them. That is the argument for stopping here and saying so. Signed-off-by: arpan --- docs/SPEC-v0.9.md | 33 +++++++++++++++++++++++++++++-- src/ctrlrun/control.py | 19 ++++++++++++++---- tests/test_budget_holds.py | 40 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) diff --git a/docs/SPEC-v0.9.md b/docs/SPEC-v0.9.md index 328ea89a..36e1ff35 100644 --- a/docs/SPEC-v0.9.md +++ b/docs/SPEC-v0.9.md @@ -835,8 +835,37 @@ entire purpose is to hit nothing. **What it does instead**: the observe report says the action *would have been* refused on a budget, naming the grant and the metric, exactly as it reports what a policy would have decided. The predicate is `check_charges`, the same function all three stores enforce with, so the report and the -enforcement cannot drift: a pilot that says "this would have been fine" about an action enforce mode -refuses is worse than no pilot. +enforcement cannot drift **on the arithmetic**: a pilot that says "this would have been fine" about +an action enforce mode refuses is worse than no pilot. + +### 4.2.1b What observe mode does not promise about *which* refusal + +**The arithmetic is shared; the ordering is not**, and that limit is stated here rather than +discovered. `_secure` and `_observe_secure` are separate implementations, for the reason +`_observe_secure`'s docstring gives: they differ in almost every branch, and one writes no receipt +and raises nothing. What they do not share is the order their checks run in, and `_Observation` +keeps the **first** reason it is given. + +So for an action that trips more than one refusal, observe mode names the one *it* reached first, +which is not always the one enforce mode would raise. Three cases were found and aligned, each with +a test: the approval gate (T451), the reservation (T452), and the scope provider (T458). Two more +are known and **not** aligned in v0.9: + +- **Scope against the approval gate.** `_secure` presents the approval above `_in_scope`; + `_observe_secure` checks scope first. An action both out of scope and awaiting approval is + reported `out_of_scope` by the pilot while enforce mode pages a human. +- **`policy_unapproved` against anything decided after it.** Enforce refuses it above authority and + policy; observe records it below both, so it is lost whenever something later blocks first. + +Neither is an enforcement difference: observe mode refuses nothing either way, and the action runs +in both. What an operator loses is the **category** of a refusal that would have happened, on an +action that would have been refused regardless. **The rule to rely on is that observe mode reports +a refusal exactly when enforce mode would refuse, and not that it always names the same one.** + +Aligning the rest means one ordered list of checks both modes walk, which is a refactor of +`_secure` and `_observe_secure` together rather than a fourth reordering. Three reorderings in this +milestone produced four regressions between them, and this section is the honest statement of where +that stopped. §2.3's and §2.4.1's refusals are reported the same way, under their own reasons. Enforce mode refuses those actions, so saying so is what observe mode is for, and an operator needs to know diff --git a/src/ctrlrun/control.py b/src/ctrlrun/control.py index 3d00428c..3dcc9147 100644 --- a/src/ctrlrun/control.py +++ b/src/ctrlrun/control.py @@ -1713,8 +1713,11 @@ def _observe_secure( # `plan_reservation` runs before `check_charges` (§3.3), so an effect that is already # committed raises `DuplicateEffect` and the budget is never consulted. Reporting the # budget first told an operator to raise a limit when the real answer was that the effect - # had already happened. The clauses above have returned by now on every refusal enforce - # mode would have hit first, so what reaches here is what the budget would decide. T452. + # had already happened. T452. + # + # Not every earlier refusal returns before this: the scope block and the approval gate + # record and carry on. `_observe_spend` skips itself once anything has blocked, which is + # what keeps the report to the one refusal enforce mode would have raised. self._observe_spend(action, charges, observation, effect_key) return approval, reservation @@ -3400,7 +3403,8 @@ def _observe_spend( """§4.2.1's second half: whether the budget would have refused, writing nothing. The predicate is `check_charges`, the same function all three stores decide with, so the - report and the enforcement cannot drift: a pilot that said "this would have been fine" + report and the enforcement cannot drift **on the arithmetic** (§4.2.1b states what is not + promised about *which* refusal is named): a pilot that said "this would have been fine" about an action enforce mode refuses is worse than no pilot. **The sum is a lock-free read** off the public `consumptions()` rather than a store's @@ -3408,12 +3412,19 @@ def _observe_spend( write nothing. It is therefore stale under concurrency, which is correct for a counterfactual and would not be for a decision. + **Skipped once something else has blocked.** Enforce mode raises at the first refusal and + never reaches the budget; observe mode runs every check, so without this it wrote a + `budget_exhausted` event for an action enforce mode refuses out of scope, and an operator + reading the log saw a refusal that would never have happened. An earlier version of this + docstring claimed the clauses above had already returned by then, which was not true of + the scope block or the approval gate. + `check_charges` can also raise `InvalidArgument` for two charges on one grant and metric carrying different amounts (§3.3.1). Nothing reachable produces that shape -- §2.7's ancestors are distinct grants, and one grant's two budgets on one metric always agree -- and observe mode is not the place to raise about it if something ever does. """ - if not charges: + if not charges or observation.blocked_reason is not None: return now = self._clock() diff --git a/tests/test_budget_holds.py b/tests/test_budget_holds.py index 44102cc2..b251efaf 100644 --- a/tests/test_budget_holds.py +++ b/tests/test_budget_holds.py @@ -1324,3 +1324,43 @@ def test_T462_the_unmeasurable_refusal_survives_a_pickle_as_InvalidArgument_did( assert isinstance(back, InvalidArgument) assert back.reason == "budget_unkeyed" assert str(back) == "nope" + + +def test_T463_observe_reports_one_refusal_and_not_every_check_that_would_have_failed( + store, clock +) -> None: + """Enforce mode raises at the first refusal and never reaches the budget. Observe mode runs + every check, so it wrote a `budget_exhausted` event for an action enforce mode refuses out of + scope, and an operator reading the log saw a refusal that would never have happened. + + `_observe_spend`'s own docstring used to claim the earlier clauses had returned by then. The + scope block and the approval gate do not return; they record and carry on. + """ + document = DOC.replace( + ' effect: "refund:{id}"', ' effect: "refund:{id}"\n resource: "payment:{id}"' + ) + enforcing, observing, watcher = _both_modes(store, clock, document) + # Fill the budget in both stores, so the budget really would refuse if it were reached. + for control in (enforcing, observing): + control.execute(_action("1", 250), lambda: {"ok": True}, "refund:1") + + somebody_else = lambda _action: {"resources": ["payment:999"]} # noqa: E731 + later = Action( + name="payments.refund", + arguments={"amount": 250, "id": "2"}, + principal=AGENT, + resource="payment:2", + environment="prod", + ) + with pytest.raises(ActionDenied) as refused: + enforcing.execute(later, lambda: {"ok": True}, "refund:2", scope=somebody_else) + assert refused.value.reason == "out_of_scope" + + observing.execute(later, lambda: {"ok": True}, "refund:2", scope=somebody_else) + + reasons = [ + event.data.get("reason") for event in watcher.events() if str(event.type) == "ACTION_DENIED" + ] + assert reasons == ["out_of_scope"], ( + f"the pilot recorded a refusal enforce mode never reached: {reasons}" + ) From dec152339fb2dd65e2a56c0bea41252f66c6020c Mon Sep 17 00:00:00 2001 From: arpan Date: Sun, 13 Sep 2026 14:56:07 +0530 Subject: [PATCH 3/3] test: no v0.9 guarantee may grade differently on its own The invariant G22 broke. A guarantee that grades differently under --only is reading state an earlier scenario left behind, and then the report cannot be trusted either way round: whichever answer is right, one of them is produced for the wrong reason. Checked by hand across all twenty-four against the shipped example after the G22 fix, and all twenty-four now agree; G22 was the only one. The test keeps the three this milestone added honest, against the fixture the rest of the file uses so it stays cheap. Signed-off-by: arpan --- tests/test_verify_authority.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_verify_authority.py b/tests/test_verify_authority.py index efc92179..792663b6 100644 --- a/tests/test_verify_authority.py +++ b/tests/test_verify_authority.py @@ -1035,3 +1035,23 @@ def test_T413s_G22_grades_the_same_alone_as_it_does_in_a_full_run(tmp_path): f"alone: {alone.status} ({alone.reason}); in a full run: {together.status}" ) assert alone.status is not Status.FAIL, alone.reason + + +@pytest.mark.parametrize("gid", ["G22", "G23", "G24"]) +def test_T413t_a_v09_guarantee_grades_the_same_alone_as_in_a_full_run(gid, tmp_path): + """The invariant G22 broke, over all three guarantees this milestone added. + + A guarantee that grades differently on its own is reading state an earlier scenario left + behind, and the report cannot be trusted either way round: whichever answer is right, one of + them is being produced for the wrong reason. Checked against the same fixture the rest of + this file uses, so it stays cheap enough to keep. + """ + path = _write(tmp_path, V7 + FULL_AUTHORITY + ACTIONS) + + alone = _by_id(run(path, only=(gid,)))[gid] + together = _by_id(run(path))[gid] + + assert alone.status is together.status, ( + f"{gid} alone: {alone.status} ({alone.reason}); in a full run: {together.status} " + f"({together.reason})" + )