Six of the seven merge-gating findings from the adversarial review

monarch-benchmark/workflowbench/tests/test_external_catalogue.py

@@ -153,3 +153,51 @@ def test_mixed_scalar_union_remains_exact_inside_root_json_payload(source,tmp_pa
     assert payload["type"] == "object"
     assert json.loads(payload["constraints"]["helper_text"].split("Source JSON Schema: ",1)[1]) == schema
     assert payload["required"] is True
+
+
+BULK_DELETE = {"delete": {"operationId": "bulk_delete", "summary": "Delete several messages",
+    "requestBody": {"required": True, "content": {"application/json": {
+        "schema": {"type": "object", "properties": {"ids": {"type": "string"}}, "required": ["ids"]}}}},
+    "responses": {"200": {"content": {"application/json": {
+        "schema": {"type": "object", "properties": {"ok": {"type": "string"}}, "required": ["ok"]}}}}}}}
+
+
+def test_an_operation_monarch_cannot_represent_is_excluded_and_reported(source, tmp_path):
+    """One unrepresentable operation used to abort the whole product's catalogue.
+
+    `_action` raised, nothing caught it, `monarch_setup` turned it into
+    `Stop(2, "generate", ...)` and no seed file was written -- so a task set naming any
+    of the four bodyless-body DELETEs in the real EnterpriseOps-Gym ITSM document could
+    not set Monarch up on that product at all. Being unable to express one operation is
+    not a reason to teach Monarch nothing.
+    """
+    product, tasks, doc, closed = source
+    doc["paths"]["/messages/bulk"] = copy.deepcopy(BULK_DELETE)
+    result = catalog.generate(product, tasks, tmp_path / "seeds", PUBLIC)
+    out = tmp_path / "seeds"
+    assert result.operations_in_spec == result.files_written == 1, "the representable operation is still taught"
+    assert [(e["service"], e["method"], e["path"]) for e in result.excluded] == [("gmail", "delete", "/messages/bulk")]
+    assert "bodyless" in result.excluded[0]["reason"]
+
+    # The pack says what Monarch was not taught, and never advertises it either.
+    assert json.loads((out / "ok.txt").read_text())["excluded_operations"] == result.excluded
+    assert "/messages/bulk" not in (out / "source-contracts.yaml").read_text()
+
+
+def test_a_catalogue_with_nothing_excluded_is_byte_for_byte_what_it_was(source, tmp_path):
+    """The manifest gains the field only when there is something to report, so every
+    pack frozen before this change still regenerates identically -- `generate` refuses
+    to replace a frozen catalogue file, so a new key on every pack would break setup
+    for anyone holding one."""
+    product, tasks, doc, closed = source
+    catalog.generate(product, tasks, tmp_path / "seeds", PUBLIC)
+    manifest = json.loads((tmp_path / "seeds" / "ok.txt").read_text())
+    assert "excluded_operations" not in manifest
+
+
+def test_an_unrepresentable_operation_is_the_only_thing_excluded(source, tmp_path):
+    """Excluding on any ValueError would hide real structural faults in a document."""
+    product, tasks, doc, closed = source
+    doc["paths"]["/messages/{message_id}"]["patch"]["servers"] = [{"url": "https://elsewhere.test"}]
+    with pytest.raises(ValueError, match="server overrides"):
+        catalog.generate(product, tasks, tmp_path / "seeds", PUBLIC)

monarch-benchmark/workflowbench/tests/test_external_customer.py

@@ -175,15 +175,20 @@ def test_attach_customer_failure_closes_budget_and_preserves_customer_billing(mo
     assert ledger.status().held_usd == ledger.reservations()[0].maximum_usd > 0
 
 
-@pytest.mark.parametrize("reason,kind", [
-    ("scope budget exhausted", "infra:budget"), ("run budget exhausted", "infra:budget"),
-    ("shared weekly budget exhausted", "infra:weekly_budget"),
-    ("recorded reservation overrun blocks further launches", "infra:weekly_budget")])
-def test_direct_arm_budget_refusal_is_infrastructure_with_partial_cost(monkeypatch, tmp_path, reason, kind):
+@pytest.mark.parametrize("reason,scope,kind", [
+    # Both budget refusals say `run budget exhausted`, because an attempt's own cap is
+    # a run reservation too. The scope separates them: the round's envelope stops the
+    # round, one attempt reaching its cap does not. See STOPS_THE_ROUND.
+    ("scope budget exhausted", "task/arm/t0#attempt-000", "infra:budget"),
+    ("run budget exhausted", "task/arm/t0#attempt-000", "infra:budget"),
+    ("run budget exhausted", "run-7#admission-000", "infra:run_budget"),
+    ("shared weekly budget exhausted", None, "infra:weekly_budget"),
+    ("recorded reservation overrun blocks further launches", None, "infra:weekly_budget")])
+def test_direct_arm_budget_refusal_is_infrastructure_with_partial_cost(monkeypatch, tmp_path, reason, scope, kind):
     config, ledger, calls = setup(monkeypatch, tmp_path, [], participant=False)
 
     def action(ep):
-        exc = BudgetExceeded(reason)
+        exc = BudgetExceeded(reason, scope_id=scope)
         exc.partial = ArmResult(cost_usd=0.2, tokens_output=4)
         raise exc
 
@@ -224,3 +229,83 @@ def action(ep):
     assert result.tokens_prompt == 1000 and result.flags == []
     assert calls[0][1] == [{"type": "function", **schema}]
     assert ledger.status().held_usd == 0
+
+
+def test_an_agent_timeout_keeps_its_verdict_when_the_customer_hit_the_same_deadline(monkeypatch, tmp_path):
+    """An expired episode deadline is a scored `timeout` verdict by deliberate rule
+    (`wb_arms/api_loop.py:670-673`), not an infrastructure retry.
+
+    The simulated customer is cut at that same deadline and records `infra:timeout`.
+    Letting the customer's sticky copy overwrite the arm's verdict dropped a real
+    agent timeout out of the pass denominator, so tau2 pass rates came out inflated
+    by exactly the attempts that ran out of time -- whenever the agent's last turn
+    happened to call the customer.
+    """
+    config, ledger, calls = setup(monkeypatch, tmp_path, [])
+    ep = Episode()
+
+    def action(ep):
+        assert "deadline" in fetch(ep)["error"]
+        return ArmResult(cost_usd=0.25, termination="timeout", final_text="ran out of time")
+
+    result = run(config, ledger, ep, action, deadline=time.monotonic() - 1)
+    assert result.termination == "timeout", "the customer overwrote the arm's scored verdict"
+    assert calls == [], "the deadline was already gone; no provider call should be made"
+    rows = [dict(task_id="task", arm="native", model="native", trial=0, passed=False,
+                 termination=result.termination, flags=result.flags, cost_usd=result.cost_usd)]
+    metrics = competitor_metrics(rows, 1)
+    assert metrics["strict_pass_denominator"] == 1 and metrics["infra"] == 0
+
+
+def test_a_customer_failure_that_is_not_the_deadline_still_wins(monkeypatch, tmp_path):
+    """The guard's own purpose, kept: a swallowed customer break must not be scored,
+    even when the arm goes on to finish and hand back a verdict."""
+    config, ledger, calls = setup(monkeypatch, tmp_path, [RuntimeError("provider disconnected")])
+    ep = Episode()
+
+    def action(ep):
+        assert "provider disconnected" in fetch(ep)["error"]
+        return ArmResult(cost_usd=0.25, termination="timeout", final_text="finished anyway")
+
+    with pytest.raises(InfraError) as caught:
+        run(config, ledger, ep, action)
+    assert caught.value.kind == "infra:customer"
+
+
+def test_only_the_round_wide_refusals_stop_the_round():
+    """The rule the kinds exist to carry, stated where it can be broken.
+
+    Stopping on `infra:budget` too would be a worse defect than the one the third kind
+    fixes: that kind is the correct, tested outcome of an attempt exhausting its own
+    scope cap (see the parametrized case above), so a round would die the first time
+    any single attempt reached its per-attempt cap.
+    """
+    from wb_orchestrator.orchestrator import STOPS_THE_ROUND
+    assert STOPS_THE_ROUND == {"infra:weekly_budget": "weekly_budget",
+                               "infra:run_budget": "run_budget"}
+    assert "infra:budget" not in STOPS_THE_ROUND
+
+
+def test_the_scope_tells_a_round_envelope_from_an_attempt_cap(tmp_path):
+    """The two refusals are word-for-word identical, so the scope is the discriminator.
+
+    An attempt's own cap is a run reservation too -- that is why matching on the
+    message classified every capped attempt as a round-wide stop. The round's envelope
+    is the one whose scope carries `ROUND_ENVELOPE_MARKER`.
+    """
+    from wb_orchestrator.budget import ROUND_ENVELOPE_MARKER, BudgetLedger
+    ledger = BudgetLedger(tmp_path / "ledger.sqlite", weekly_limit_usd="100")
+    refusals = {}
+    for scope in ("round-7" + ROUND_ENVELOPE_MARKER + "000", "task/arm/t0#attempt-000"):
+        ledger.reserve_run(scope, "1.00")
+        ledger.reserve(scope + "/first", "0.80", scope_id=scope, run_id=scope)
+        with pytest.raises(BudgetExceeded) as caught:
+            ledger.reserve(scope + "/second", "0.80", scope_id=scope, run_id=scope)
+        refusals[scope] = caught.value
+
+    messages = {str(exc) for exc in refusals.values()}
+    assert messages == {"run budget exhausted"}, "identical wording is the whole problem"
+    kinds = {scope: runtime._budget_failure(exc).kind for scope, exc in refusals.items()}
+    assert sorted(kinds.values()) == ["infra:budget", "infra:run_budget"]
+    assert kinds["round-7" + ROUND_ENVELOPE_MARKER + "000"] == "infra:run_budget"
+    assert ledger.status().available_usd > 0, "the week had room; only these scopes did not"

monarch-benchmark/workflowbench/tests/test_external_monarch_setup.py

@@ -149,7 +149,7 @@ def test_source_setup_refuses_import_with_unverified_result(source_setup, monkey
 
 def test_external_kb_rejects_legacy_service_slug(source_setup):
     path = source_setup.product.with_name("enterprise-ops-gym.monarch-kb.yaml")
-    monarch_setup._write_kb(source_setup.product, "enterprise-ops-gym", URL, {"bench-gym-itsm-mcp": "old"})
+    monarch_setup._write_kb(source_setup.product, "enterprise-ops-gym", URL, {"bench-gym-itsm-mcp": "old"}, env={})
     with pytest.raises(config.ConfigError, match=SLUG):
         config.load_monarch_kb(path, config.load_product(source_setup.product))
 

monarch-benchmark/workflowbench/tests/test_external_pipeline.py

@@ -316,3 +316,37 @@ def test_budget_capability_status_uses_fresh_exact_harness_verification(tmp_path
     record['checked_at'] = (datetime.now(timezone.utc)-timedelta(hours=3)).isoformat()
     monarch_probe.probe_path(site).write_text(json.dumps(record))
     assert approvals.capabilities([harness], env)['monarch'] == approvals.MONARCH_REASON
+
+
+def test_a_failure_after_admission_releases_the_round_envelope(tmp_path, monkeypatch):
+    """The envelope was closed by `_execute`'s finally alone, so anything raising
+    between `_admit` and `_execute` left the round's whole liability held against the
+    week with nothing spent.
+
+    A duplicate `--run-id` is enough: `_admit` reserves the envelope, `create_run`
+    raises on the unique constraint, and `_execute` is never entered. Nothing can
+    release a run envelope afterwards -- `wb budget release` only looks in
+    `budget_reservations` -- and the scope never appears in `status.unknown_ids`, so
+    the operator cannot even name what is holding the money.
+    """
+    from types import SimpleNamespace
+    from wb_orchestrator.budget import ROUND_ENVELOPE_MARKER, BudgetLedger
+    pp, pl = product_plan(tmp_path, external_task())
+    rc = config.resolve(pp, pl, env={})
+    ledger = BudgetLedger(tmp_path / 'ledger.sqlite3', weekly_limit_usd='100')
+    store = Store(tmp_path / 'wb.sqlite3')
+    engine = orchestrator.Orchestrator.from_config(store, rc, tmp_path / 'out', ledger=ledger)
+    # A paid competitor, so admission opens an envelope at all, and a world that needs
+    # no container: this test is about the round's lifecycle, not about either of those.
+    paid = SimpleNamespace(name='paid', provider_key='mock', run=lambda ep, deadline=None: None)
+    monkeypatch.setattr(engine, '_arms', lambda: [paid])
+    monkeypatch.setattr(engine, 'world', SimpleNamespace(prerequisites=lambda: []))
+
+    store.create_run('run-x', engine._hash(), engine.suite, engine._config())
+    with pytest.raises(Exception):
+        engine.run('run-x')                       # UNIQUE constraint on runs.run_id
+
+    envelopes = [r for r in ledger.run_reservations() if ROUND_ENVELOPE_MARKER in r.scope_id]
+    assert envelopes, 'admission should have opened one, or this proves nothing'
+    assert all(r.closed_at is not None for r in envelopes), 'the round envelope leaked'
+    assert ledger.status().held_usd == 0

monarch-benchmark/workflowbench/tests/test_front_door_relay.py

@@ -23,8 +23,9 @@
 from http.server import ThreadingHTTPServer
 
 import pytest
+import yaml
 
-from wb_orchestrator.monarch_setup import front_door_path
+from wb_orchestrator.monarch_setup import _write_kb, front_door_path
 from wb_studio.app import ROOT, Studio, front_door_target, handler
 from wb_world.episode import load_suite
 
@@ -142,3 +143,31 @@ def test_the_secret_segment_is_built_into_the_seed_url():
 
 def test_a_seed_url_without_a_secret_is_unchanged():
     assert front_door_path("https://studio.example.dev/front-door", {}) == "https://studio.example.dev/front-door"
+
+
+def test_the_tracked_knowledge_base_records_the_secret_by_name(tmp_path):
+    """`config/products/<product>.monarch-kb.yaml` is git-tracked and this repo is
+    public, so a secret written into it once is in history, not just in a file.
+    Only the config hash reads this field, and never as an address, so the name
+    carries everything the record needs."""
+    env = {"STUDIO_FRONT_DOOR_SECRET": SECRET}
+    live = front_door_path("https://studio.example.dev/front-door", env)
+    assert SECRET in live, "the address Monarch is handed must carry the secret"
+    path, _ = _write_kb(tmp_path / "tau2-retail.yaml", "tau2-retail", live,
+                        {"bench-gym-itsm-mcp": "sha"}, env=env)
+    text = path.read_text(encoding="utf-8")
+    assert SECRET not in text, "the secret reached a tracked file"
+    assert "${STUDIO_FRONT_DOOR_SECRET}" in text
+
+
+def test_rotating_the_secret_leaves_the_configuration_unchanged(tmp_path):
+    """Rotating per round is what the docstring tells an operator to do. Recording
+    the value made that read as a different configuration and moved the hash."""
+    written = []
+    for secret in ("round-47", "round-48"):
+        env = {"STUDIO_FRONT_DOOR_SECRET": secret}
+        live = front_door_path("https://studio.example.dev/front-door", env)
+        path, _ = _write_kb(tmp_path / "tau2-retail.yaml", "tau2-retail", live,
+                            {"bench-gym-itsm-mcp": "sha"}, env=env)
+        written.append(yaml.safe_load(path.read_text(encoding="utf-8"))["shim_public_url"])
+    assert written[0] == written[1]

monarch-benchmark/workflowbench/tests/test_worlds_enterprise_ops.py

@@ -242,6 +242,60 @@ def test_search_does_not_surface_them_either():
     assert not any(h["path"].startswith("/api/") for h in hits)
 
 
+class _Recording:
+    """A server that records the target it was handed instead of sending it."""
+
+    def __init__(self):
+        self.target = None
+
+    def request(self, method, path, body=None, extra=None):
+        self.target = path
+        return b'{"ok": true}'
+
+
+def _with_location_route():
+    world = _Offline()
+    world._specs["gym-itsm-mcp"]["paths"]["/locations/name/{name}"] = {
+        "get": {"summary": "location by name", "operationId": "location_by_name"}}
+    world.servers["gym-itsm-mcp"] = _Recording()
+    return world, world.servers["gym-itsm-mcp"]
+
+
+def test_a_path_value_with_a_space_reaches_the_world_still_encoded():
+    """`/locations/name/TechCorp%20NYC%20Headquarters` is a route and a value the
+    shipped ITSM document and its seed data both contain.
+
+    The decoded path was sent as the request target. `urllib` refuses a space
+    outright -- `InvalidURL: URL can't contain control characters` -- and a server
+    that accepted it would read the target as `/locations/name/TechCorp`. Either way
+    a real EOG task fails, and the approval rule reads that as the competitor
+    failing rather than as our own bug.
+    """
+    world, server = _with_location_route()
+    out = world._call("GET", "/locations/name/TechCorp%20NYC%20Headquarters", None, None)
+    assert json.loads(out) == {"ok": True}, out
+    assert " " not in server.target, "a space in the request target is not sendable"
+    assert server.target == "/locations/name/TechCorp%20NYC%20Headquarters"
+
+
+def test_the_permission_check_reads_the_decoded_path():
+    """Decoding still has to happen, just not on the way to the wire: an encoded
+    separator must not slip past the published-surface check as one segment."""
+    world, server = _with_location_route()
+    out = json.loads(world._call("GET", "/locations/name/Tech%2FCorp", None, None))
+    assert out["error"]["code"] == 403
+    assert server.target is None, "a refused call must not reach the world"
+
+
+def test_an_encoded_administrative_path_is_refused_as_administrative():
+    """The admin guard read the raw path while everything after it read the decoded
+    one. Two forms of the same path in one function is how a guard gets walked past."""
+    out = json.loads(_Offline()._call("POST", "/api/%73ql-runner", None,
+                                      json.dumps({"query": "UPDATE incident SET state='closed'"})))
+    assert out["error"]["code"] == 403
+    assert "administrative operation" in out["error"]["message"]
+
+
 def test_a_call_to_no_known_server_is_refused_by_name():
     out = json.loads(_Offline(("gym-itsm-mcp", "gym-calendar"))._call("GET", "/nowhere/x", None, None))
     assert out["error"]["code"] == 404