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
47 changes: 31 additions & 16 deletions docs/SPEC-v0.10.md
Original file line number Diff line number Diff line change
Expand Up @@ -1241,11 +1241,17 @@ budget from an observed run is untouched.
| T499a | P7 as a test: an action tripping both `policy_unapproved` and a later refusal names `policy_unapproved` in **both** modes. The case that proves the list starts at `execute`'s entry, and the one a `_secure`-only refactor leaves broken while every other pair goes green |
| T500 | Its second: `policy_unapproved` against a later refusal, both modes name the same reason |
| T501 | Every pair the generator could not construct is named in the test's own output, and the list is asserted against the declared order so a shrinking pair set fails red |
| T502 | The four v0.9 regressions as regression tests: the resumed observed receipt's spend, the doubled `_Observation` event, the `effect_key` on the budget-refusal event, and the picklability of every `InvalidArgument` subclass across `verify`'s JSON-over-stdin children |
| T502 | The four v0.9 regressions as regression tests. **Two already existed under their own numbers** when this was audited at release: the `effect_key` on the budget-refusal event (`test_T461...`) and the picklability of `_UnmeasurableError` (`test_T462...`). The other two are `T502a`, the resumed *observed* receipt's spend — `T439` covers observe, `T447` covers resume, and neither covered the two together — and `T502b`, the doubled `ACTION_DENIED` on a resumed observed leg, which is the row that had no test under any name |

T502's last row is the one nothing in this repository would otherwise catch, which is why §13.8
named it, and it belongs to this item because this item rewrites the code that broke it.

**A number, not a property, is what was owed here.** The audit that closed this found two of the
four rows already tested under other numbers, which is the same defect §9.4 describes in a
different place: a table that describes the tree, checked by a human reading both. `T502b` is the
one that was genuinely absent, and it is absent-shaped for a reason worth keeping — every
assertion near it checked that the event *appeared*, and an event that appears twice appears.

---

## 6. The operator surfaces for a hop
Expand Down Expand Up @@ -1273,6 +1279,9 @@ envelope did the peer actually hold, and which hop narrowed it**.
| `depth` | derived by walking to the root, never read from the stored column (`v0.3 §5.5`) |
| `chain[]` | one entry per ancestor to the root: `id`, `depth`, `revoked_at`, and **the dimension on which each step narrows** |
| `revoked_at` | on the hop itself, or `null` |
| `schema` | `ctrlrun.hop/v1`, so a reader that is handed one document knows which shape it got. `ctrlrun inspect` without `--hop` answers about an **action** and `--hop` answers about an **authority record**, and the two are not interchangeable |
| `root_id` | the grant the walk ended at, so an operator can name the root without re-walking |
| `missing_parent_id` | the record the store could **not** read, or `null`. A chain that cannot be verified is refused rather than trusted (§3.2), and this is the id that says where it stopped |

**`chain[]` carrying which dimensions narrowed at each step is the part that answers the question.**
An operator looking at a refused action knows the chain is valid or it is not; what they cannot see
Expand Down Expand Up @@ -1537,24 +1546,30 @@ One justification per row. Anything not here is a spec amendment before it is co
### 9.4 Three rows in this table did not ship, and the table said they had

Written at release, against the shipped tree, because §9 is the section a reader trusts for the
public surface and a frozen name that names nothing is worse than a missing row: a missing row is a
gap, and a wrong one is an answer.
public surface and **a frozen name that names nothing is worse than a missing row**: a missing row
is a gap, and a wrong one is an answer.

This is §11's finding in its sharpest form. Every row above was justified before it was code, and
three of them were then not built. Nothing turned red, because **no test in this repository asserts
that a name §9 freezes exists.** The rule §11 states covers a sentence about a later item; these
are sentences about *this* document's own frozen table, and they need the same discipline.
three of them were then not built. Nothing turned red, because **no test in this repository asserted
that a name §9 freezes exists.** The rule §11 states covers a sentence about a later item; these are
sentences about *this document's own frozen table*, and they need the same discipline.

| Row | What shipped | What follows from the gap |
|---|---|---|
| `hop=` and `task=` on `ctrlrun.adapter.needs_approval` | **nothing.** The signature is still `needs_approval(control, action, arguments, *, resource=None)` | The predicate evaluates against the receiver's whole candidate set while `execute` evaluates against the hop alone, exactly as the row above warned. **It is not an authority hole**: `Control.execute` is the enforcement point and still decides against the hop alone, so a call this predicate waves through is refused there. What it costs is the framework's own approval item — a human is not asked before invocation for a call that then refuses, which is a worse experience and a confusing receipt, not a wider grant |
| `ssl_context=` on `ctrlrun.gateway.transport.request` | **nothing on that function.** Check 3 was implemented on `ctrlrun.upstream.observe_upstream(url, *, verify=...)` and on the forwarder's `verify`, both new surfaces; `transport.request` still takes no context, no verify argument and no client | Check 3 runs where the gateway builds its forwarder, not inside the shared request helper. The module-level-default objection the row raises is satisfied by that placement rather than by the parameter, so the *reason* held and the *name* did not |
| `ctrlrun.hop/v1`'s key set (§6.2) | the table's keys **and three more**: `schema`, `root_id`, `missing_parent_id` | A consumer written against §6.2 alone gets keys it did not expect. The document under-describes what it emits, which is the safe direction for a reader and the wrong one for a frozen schema. `ctrlrun.hop/v1` is the version to amend, not to re-cut, and v0.11 owns it |

**What this asks of v0.11**, stated here so it is not rediscovered: a test that every name §9 freezes
is importable with the signature the row gives. It is cheap, it is the same shape as
`test_every_source_file_carries_its_copyright_and_license`, and it is the only thing that would have
caught all three.
**Two of the three are now built rather than recorded.** A gap that can be closed is closed; only
the one whose *name* was wrong and whose *reason* was right stays as a row.

| Row | Disposition |
|---|---|
| `hop=` and `task=` on `ctrlrun.adapter.needs_approval` | **Built.** The signature is `(control, action, arguments, *, resource=None, task=None, hop=None)` and both are threaded into `Control.evaluate`. Without them the predicate evaluated against the receiver's whole candidate set while `execute` evaluates against the hop **alone** (§2.3), so it answered "a human is needed" for a call `execute` then refuses: the framework surfaces an approval item, a human says yes, the call fails anyway. **It was never an authority hole** — `Control.execute` is the enforcement point and refuses either way, so nothing wider ever ran; what it cost was the framework's own approval item and a receipt nobody could explain. `T128c` drives both and asserts they agree, with the no-hop case as its negative control |
| `ctrlrun.hop/v1`'s key set (§6.2) | **Fixed in §6.2**, which listed eight of the eleven keys the document carries. `schema`, `root_id` and `missing_parent_id` are now in the table. Under-describing is the safe direction for a reader and the wrong one for a schema somebody writes a consumer against. `T506b` parses the table out of this file and compares it to the emitted document, so the two cannot drift again; `ctrlrun.hop/v1` is not re-cut, because nothing it emitted changed |
| `ssl_context=` on `ctrlrun.gateway.transport.request` | **Not built, deliberately, and this row is why.** Check 3 shipped on `ctrlrun.upstream.observe_upstream(url, *, verify=...)` and on the forwarder's `verify`; `transport.request` still takes no context, no verify argument and no client. The row's *reason* held and its *name* did not: its objection to a module-level default — one context pins every caller of a shared module to one certificate — is answered by putting check 3 where the gateway builds its forwarder, which is per-gateway rather than per-process. Adding the parameter now would add a second way to configure the same pin |

**The test this asked v0.11 for is in the same commit as this section**, because asking a later
milestone for it would be the exact mistake the section is about.
`test_every_v0_10_name_the_spec_freezes_is_importable_with_the_parameter_it_names` walks the rows
and checks each name imports with the parameter its row gives.
`test_the_row_of_section_9_that_did_not_ship_still_has_not` pins `ssl_context=` in the other
direction, so building it later fails there and this row comes out in the same commit: **the
document and the tree are wrong together or right together, never one of each.**

**No new error type.** `errors.py`'s closed set already covers every refusal here: `authority_hop`
is an `AuthorityDenied` reason, and both upstream reasons are `ActionDenied` reasons. If an item
Expand Down
11 changes: 10 additions & 1 deletion src/ctrlrun/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,8 @@ def needs_approval(
arguments: Mapping[str, Any],
*,
resource: str | None = None,
task: str | None = None,
hop: str | None = None,
) -> bool:
"""Does this call need a human? For a framework that asks before it invokes (SPEC-v0.5 §3.5).

Expand All @@ -459,6 +461,13 @@ def needs_approval(
`resource:` is used where none is given -- the same precedence `@protect` applies. It
matters: authority matches on resource patterns (SPEC-v0.3 §4.2), so a predicate that
skipped it would evaluate a different action from the one that runs.

`task` and `hop` are that same argument one frame further out (SPEC-v0.10 §9). Without them
this predicate evaluates against the receiver's **whole candidate set** while `execute`
evaluates against the hop **alone** (§2.3), so it answers "no human needed" for a call
`execute` then refuses. That is not a wider grant -- `Control.execute` is the enforcement
point and decides against the hop either way -- but it costs the framework its own approval
item, and a human is not asked before an invocation that then fails.
"""
principal = control.resolve_principal(action)
template = resource if resource is not None else control.policy.resource_template(action)
Expand All @@ -469,7 +478,7 @@ def needs_approval(
resource=None if template is None else resolve_resource(template, arguments),
environment=control.environment,
)
return control.evaluate(proposed).decision is Decision.APPROVE
return control.evaluate(proposed, task=task, hop=hop).decision is Decision.APPROVE


def banner(control: Control) -> None:
Expand Down
100 changes: 100 additions & 0 deletions tests/test_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1195,3 +1195,103 @@ def issue_refund(payment_id: str, amount: int, mode: str = "live") -> str:
assert issue_refund(payment_id="t", amount=1) == "ok"

assert len(double.calls) == 1


# --- T128c: the predicate and `execute` decide the same action --------------------------------

#: A receiver that holds a wide root grant of its own **and** is handed a narrow hop. The
#: disagreement `SPEC-v0.10 §9` names lives exactly here: without `hop=` the predicate evaluates
#: against `broad` while `execute` evaluates against the hop alone (§2.3).
HOP_AND_A_ROOT_GRANT = """
schema: ctrlrun.policy/v3
mode: enforce
authority:
grants:
- id: broad
subject: {agent: finance-agent}
actions: ["stripe.*"]
resources: ["payment:*"]
environments: [production]
- id: issuer
subject: {agent: planner}
actions: ["stripe.*"]
resources: ["payment:*"]
environments: [production]
delegable: true
expires_at: "2027-01-01T00:00:00Z"
actions:
stripe.refund:
resource: "payment:{payment_id}"
rules:
- when: {amount_lte: 500000}
decision: approve
- decision: deny
"""


@pytest.mark.authority
def test_T128c_needs_approval_under_a_hop_decides_what_execute_decides(tmp_path):
"""SPEC-v0.10 §9's `hop=`/`task=` row, and the defect it names.

A framework asks this predicate **before** it invokes. Without `hop=` the predicate evaluated
against the receiver's whole candidate set -- here a root grant covering `payment:*` -- while
`execute` evaluates against the hop **alone** (§2.3, no fallback). So it answered "a human is
needed" for a call `execute` then refuses outright: the framework surfaces an approval item, a
human says yes, and the tool call fails anyway.

**Not an authority hole, and the test says which it is.** `Control.execute` is the enforcement
point and refuses either way, so nothing wider ever runs. What the gap cost was the framework's
own approval item and a receipt nobody could explain.

The `without` case is what makes this discriminating: it proves the hop is what changed the
answer, rather than a fixture in which everything is denied anyway (mutation pattern 3).
"""
from datetime import UTC, datetime

from ctrlrun.authority import grant_from_yaml

control, store, _ = build(HOP_AND_A_ROOT_GRANT)
assert control._authority is not None
narrow = grant_from_yaml(
"""
subject: {agent: finance-agent}
actions: ["stripe.refund"]
resources: ["payment:US-*"]
environments: [production]
expires_at: "2026-12-01T00:00:00Z"
""",
source="<test>",
)
planned = control._authority.plan_delegation(
"issuer", narrow, by=Principal(agent="planner"), store=store, now=datetime.now(UTC)
)
store.put_delegation(planned.to_record())
hop = planned.delegation_id

off_envelope = {"payment_id": "EU-1", "amount": 2000}
with context("finance-agent"):
# Without the hop the root grant reaches it, so a human *is* needed. This is the answer
# the predicate used to give under a hop as well.
assert needs_approval(control, REFUND, off_envelope) is True

# Under the hop, the resource is outside the envelope and nothing falls back to `broad`.
assert needs_approval(control, REFUND, off_envelope, hop=hop) is False

# And that is the same answer `execute` reaches, which is the whole point.
with pytest.raises(AuthorityDenied):
control.execute(
Action(
name=REFUND,
arguments=off_envelope,
principal=Principal(agent="finance-agent"),
resource="payment:EU-1",
environment="production",
),
lambda: "ok",
hop=hop,
)

# The negative control: inside the envelope both still say a human is needed, so the
# hop narrows rather than refusing everything.
inside = {"payment_id": "US-9", "amount": 2000}
assert needs_approval(control, REFUND, inside, hop=hop) is True
99 changes: 99 additions & 0 deletions tests/test_budget_holds.py
Original file line number Diff line number Diff line change
Expand Up @@ -1366,3 +1366,102 @@ def test_T463_observe_reports_one_refusal_and_not_every_check_that_would_have_fa
assert reasons == ["out_of_scope"], (
f"the pilot recorded a refusal enforce mode never reached: {reasons}"
)


# --- T502: the four v0.9 regressions, as regression tests (SPEC-v0.10 §5.4) --------------------


def test_T502a_a_resumed_observed_leg_carries_the_counterfactual_its_first_leg_computed(
store, clock
) -> None:
"""T502, row one. `T439` proves observe mode charges nothing; `T447` proves a resumed leg
reports what its first leg charged. **Neither covers the two together**, and the two together
are the case that broke.

The first version of this test asserted the resumed observed receipt reports **no** spend,
reasoning that the ledger is empty. That is wrong, and `§4.2.1a` says why: under observation
every number on a receipt is a counterfactual, and if the receipt does not carry what the
action *would* have been charged then nothing anywhere records it and **a budget cannot be
sized from an observed run** -- which is the entire reason to run one. `T439d` pins that for
a single leg.

So the property here is that a **resumed** leg keeps it. The resumed receipt is the whole
evidence an MCP or ACS action ever gets, and `_resumed_charges` reads the ledger, which is
empty under observation. A resumed observed leg that reported `()` would silently drop the
counterfactual for exactly the actions that take more than one round trip.
"""
from ctrlrun import Suspended

observing = _observing_control(store, clock)

def suspends() -> Any:
raise Suspended("observed-round")

with pytest.raises(Suspended):
observing.execute(_action("1", 100), suspends, "refund:1")
assert store.consumptions() == (), "observe mode wrote to the ledger on the first leg"

receipt = contextvars.Context().run(observing.resume, "observed-round", lambda: {"ok": True})

assert receipt.result is ReceiptResult.OBSERVED
assert receipt.budget_charges == ({"grant_id": "payer", "metric": "amount", "amount": 100},), (
f"the resumed observed leg dropped the counterfactual spend: {receipt.budget_charges}"
)
assert store.consumptions() == (), (
"the resumed observed leg wrote to the ledger; the number on the receipt is a "
"counterfactual and must stay one"
)


def test_T502b_a_resumed_observed_leg_does_not_announce_the_refusal_twice(store, clock) -> None:
"""T502, row two, and the only one of the four with no test under any name.

Row three is `test_T461...` and row four is `test_T462...`, both above and both under their
own numbers -- which is why §8 calls T502 owed while the tree already had most of it.

The regression: `_refuse_unmeasurable` announces `ACTION_DENIED` under observation, and a
resumed leg announced it **again** for the same action. The evidence then said the action was
denied twice while the `observed` receipt beside it said it ran. An answer and its evidence
disagreeing about one action is what `acs.py`'s clause forbids one boundary lower, and it is
why `announce=False` exists on the resumed path.

**Counted, not merely present.** A test asserting the event appears passes whether it appears
once or twice, which is how this shipped in the first place. `announce` guards the
*unmeasurable* refusal (§2.3, §2.4.1), so the action carries a metric the budget cannot read
-- a negative amount -- rather than one that exhausts it.
"""
from ctrlrun import Suspended

observing = _observing_control(store, clock)
unmeasurable = Action(
name="payments.refund",
arguments={"amount": -250, "id": "1"},
principal=AGENT,
environment="prod",
)

def suspends() -> Any:
raise Suspended("observed-round")

with pytest.raises(Suspended):
observing.execute(unmeasurable, suspends, "refund:1")

def denials() -> list:
return [
event
for event in store.events()
if str(event.type) == "ACTION_DENIED"
and event.data.get("reason") == "budget_unmeasurable"
]

assert len(denials()) == 1, (
f"the first leg did not announce the observed refusal exactly once: {len(denials())}"
)

receipt = contextvars.Context().run(observing.resume, "observed-round", lambda: {"ok": True})

assert receipt.result is ReceiptResult.OBSERVED, "observe mode refused a resumed leg"
assert len(denials()) == 1, (
f"the resumed leg announced the refusal again: {len(denials())} ACTION_DENIED rows for "
"one action, beside an `observed` receipt saying it ran"
)
Loading