diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bb4552f..e2cde7a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A read written mureo's own way is now recognised as a read, in the one + place that can safely act on it** (#549 follow-up). mureo's tools put the + verb LAST (`google_ads_campaigns_list`); the shared read vocabulary only + matched a verb FIRST (`list_campaigns`, the bridged convention). Every + native read therefore reached the rollback planner as a write with no + reversal hint, so a batch containing one showed the operator a read among + the items they "cannot revert" and reported `partial` coverage for a change + set that was in fact fully revertible. + + The fix is a SECOND predicate, `reads_as_a_report_only_action`, used by the + rollback planner alone. The shared `is_read_only_tool_name` is unchanged. + + That split is the substance of the change. The shared predicate has three + other callers and all three decide things about PLUGIN tools, whose names + mureo does not choose: the guardrail money pattern-scan registration + (a denial), promotion into `action_log`, and whether a change can restart a + learning period. Loosening the shared rule widens all three at once — and a + mutation admitted there silently loses a `## Guardrails` cap. Since no + hardcoded verb list can be complete for names mureo does not control, those + three keep the strict rule, and a test pins that they do. + + The trailing reading is still refused when a write verb appears in the same + segment, because this surface has its own honesty to keep: a plugin mutation + misread here would hide a real gap inside a batch claiming full coverage. + That vocabulary is single-sourced from + `mureo.byod._client_common._MUTATION_PREFIXES`, which AGENTS.md calls + authoritative, rather than hand-maintained a second time, and extended with + abbreviations and blunt synonyms (`del`, `rm`, `kill`, `terminate`, + `revert`, `undo`, …) after a review defeated the first list with + `campaign_del_list`. Names are NFKC-normalized before matching, so a + fullwidth `ｄｅｌ` is the same verb as `del`. None of that makes the + vocabulary complete — a verb in another script is not reachable from a list + of English words — which is precisely why the strict matcher, and not this + one, guards the money scan. - **An observed change no longer joins the operator's open batch** (#545 x #549). "Imports never join a batch" is stated in `docs/change-import.md` without qualification, but it was enforced in one place: the polled diff --git a/mureo/core/tool_names.py b/mureo/core/tool_names.py index a4f2bb74..cc2e08e5 100644 --- a/mureo/core/tool_names.py +++ b/mureo/core/tool_names.py @@ -1,18 +1,31 @@ """Shared vocabulary for reading intent out of a tool name. -Two safety surfaces need the same answer to "does this name describe a read?" -and they must not drift apart: +FOUR surfaces need an answer to "does this name describe a read?", and they +must not drift apart: -- :mod:`mureo.rollback.planner` — a read has nothing to undo, so it plans no - rollback. - :mod:`mureo.mcp.server`'s guardrail pattern-fallback registration — a read moves no money, so subjecting it to a heuristic budget/bid scan could only produce false DENIALS. +- :func:`mureo.mcp.plugin_semantics.derive_semantics` — a read is not promoted + into ``action_log`` (#517). +- :mod:`mureo.policy.learning_reset` — a read cannot restart a learning + period on any platform. +- :mod:`mureo.rollback.planner` — a read has nothing to undo, so it plans no + rollback. -Two copies of the prefix list would eventually disagree, and the disagreement +Copies of the prefix list would eventually disagree, and the disagreement would be silent in both directions (an unrollback-able read here, a denied read there), so the list and the matcher live here once. +**The first three take the strict answer; the fourth takes a looser one.** +:func:`is_read_only_tool_name` matches a verb at the START of a segment and +nothing else. :func:`reads_as_a_report_only_action` also accepts a verb at the +END, which is how mureo names its own tools, and is used by the rollback +planner alone. The split is deliberate and is documented on that function: +the first three decide things about PLUGIN tools, whose names mureo does not +choose, and a mutation admitted there loses a ``## Guardrails`` cap in +silence. Do not collapse the two. + **Namespace-aware by construction.** A bridged MCP server commonly namespaces its tools with a hyphen (``campaign_management-list_campaigns``), and a plain ``startswith`` against the whole name matches none of those. Matching anchors @@ -30,7 +43,14 @@ from __future__ import annotations -__all__ = ["READ_ONLY_PREFIXES", "is_read_only_tool_name"] +import unicodedata + +__all__ = [ + "READ_ONLY_PREFIXES", + "WRITE_VERBS", + "is_read_only_tool_name", + "reads_as_a_report_only_action", +] #: Verb prefixes that mark a tool name as a read. Anchored at the start of a #: namespace segment (see :func:`is_read_only_tool_name`); the trailing @@ -62,3 +82,132 @@ def is_read_only_tool_name(name: str) -> bool: for segment in lowered.split("-") for prefix in READ_ONLY_PREFIXES ) + + +#: Verbs that make a name a write whatever else it says. +#: +#: Single-sourced from ``mureo.byod._client_common._MUTATION_PREFIXES``, which +#: AGENTS.md calls the authoritative mutation vocabulary, so the two cannot +#: drift and a verb learned in one place is known in the other. The extras +#: below are shapes that vocabulary has no reason to carry (it names Python +#: client methods, not MCP tools). +#: +#: Only ever used to REFUSE a reading, never to assert that a name is a write, +#: so an omission costs a missed read rather than a missed mutation. +#: +#: Computed once at import; the ``_MUTATION_PREFIXES`` import is written inside +#: the function to keep the module-level import block free of a dependency on +#: a higher layer, not to defer it. +def _write_verbs() -> frozenset[str]: + from mureo.byod._client_common import _MUTATION_PREFIXES + + return frozenset(p.rstrip("_") for p in _MUTATION_PREFIXES) | { + "put", + "post", + "write", + "insert", + "replace", + "upsert", + "mutate", + "purge", + "drop", + "clear", + "reset", + "restore", + "archive", + "stop", + "start", + "activate", + "deactivate", + "toggle", + "promote", + "install", + "uninstall", + "register", + "deregister", + "revoke", + "grant", + "link", + "unlink", + "assign", + "schedule", + "rename", + "move", + "copy", + # Abbreviations and blunt synonyms. A review defeated an earlier + # version of this list with `campaign_del_list` and `budget_rm_check`: + # the trailing verb read as a report while the real verb was a + # shortening of one already listed. + "del", + "rm", + "kill", + "nuke", + "terminate", + "revert", + "undo", + "wipe", + "flush", + "prune", + "truncate", + "merge", + "execute", + "run", + "sync", + "import", + } + + +WRITE_VERBS: frozenset[str] = _write_verbs() + +#: The same verbs as :data:`READ_ONLY_PREFIXES`, as bare tokens. +_READ_VERBS: frozenset[str] = frozenset(p.rstrip("_") for p in READ_ONLY_PREFIXES) + + +def reads_as_a_report_only_action(name: str) -> bool: + """Like :func:`is_read_only_tool_name`, but also reads a TRAILING verb. + + **Do not use this to gate a denial or an exemption from one.** It exists + for exactly one caller — the rollback planner deciding whether an + ``action_log`` entry is something the operator could be asked to undo — + and the separation from :func:`is_read_only_tool_name` is the whole design, + not tidiness. + + The problem it solves. mureo's own tools put the verb LAST + (``google_ads_campaigns_list``) while the bridged convention puts it first + (``list_campaigns``), so the prefix-only rule read every native read as a + write. In a rollback batch that surfaced as a read listed among the items + the operator "cannot revert", and a change set that was in fact fully + revertible reporting ``partial`` coverage. + + Why it is not simply added to :func:`is_read_only_tool_name`. That + predicate has three other callers and every one of them is plugin-facing + and safety-relevant: ``mcp.server._register_plugin_pattern_fallbacks`` + skips the guardrail money pattern-scan for a name that reads as a read, + ``mcp.plugin_semantics.derive_semantics`` decides whether a call is + promoted into ``action_log`` at all, and ``policy.learning_reset`` + decides whether a change can restart a learning period. Loosening the + shared predicate widens all three at once, on names mureo does not + control — a plugin can ship any verb it likes. A mutation admitted there + silently loses a ``## Guardrails`` cap, which is the failure this + vocabulary exists to prevent, so those three keep the strict rule. + + The trailing reading is still guarded by :data:`WRITE_VERBS`, because this + surface has its own honesty to keep: a plugin mutation misread here would + be reported as nothing to revert, hiding a real gap in a batch's coverage. + That is a smaller harm than losing a money guardrail, which is why the + guarded rule is acceptable here and not there. + """ + # NFKC first: a fullwidth `ｄｅｌ` is the same verb as `del` to a reader and + # a different string to `in`, and the whole guard is a string comparison. + # It does not make the vocabulary complete — a verb written in another + # script (`削除_list`) is not reachable from a list of English words, and + # this predicate does not pretend otherwise; see the honesty note above + # about what a misread costs on this surface. + lowered = unicodedata.normalize("NFKC", name).lower() + for segment in lowered.split("-"): + if any(segment.startswith(prefix) for prefix in READ_ONLY_PREFIXES): + return True + tokens = segment.split("_") + if tokens[-1] in _READ_VERBS and not (set(tokens) & WRITE_VERBS): + return True + return False diff --git a/mureo/mcp/server.py b/mureo/mcp/server.py index 024c9e81..abc8e590 100644 --- a/mureo/mcp/server.py +++ b/mureo/mcp/server.py @@ -424,8 +424,16 @@ def _register_plugin_pattern_fallbacks( - ``annotations.readOnlyHint``, when the tool declares it; and - the tool NAME, via the shared read vocabulary in - :mod:`mureo.core.tool_names` (the same list and matcher the rollback - planner uses, single-sourced so the two cannot drift). + :mod:`mureo.core.tool_names`, single-sourced so the surfaces that use + it cannot drift. + + The matcher here is the STRICT one, :func:`~mureo.core.tool_names. + is_read_only_tool_name`, and that is deliberate. The rollback planner uses + a looser sibling that also reads a verb at the END of a name, because + mureo's own tools are named that way; this gate does not, because it + decides about PLUGIN tools whose names mureo does not choose, and a + mutation admitted here silently loses its ``## Guardrails`` cap. Do not + "unify" the two — see that sibling's docstring for the argument. The name check still matters after #517, which taught ``derive_semantics`` the same vocabulary for tools that declare NO ``readOnlyHint``: a plugin is diff --git a/mureo/rollback/planner.py b/mureo/rollback/planner.py index 8c67ba98..55e74ff9 100644 --- a/mureo/rollback/planner.py +++ b/mureo/rollback/planner.py @@ -33,7 +33,10 @@ import copy from typing import TYPE_CHECKING, Any -from mureo.core.tool_names import READ_ONLY_PREFIXES, is_read_only_tool_name +from mureo.core.tool_names import ( + READ_ONLY_PREFIXES, + reads_as_a_report_only_action, +) from mureo.rollback.models import RollbackPlan, RollbackStatus if TYPE_CHECKING: @@ -300,8 +303,17 @@ def _is_read_only(action: str) -> bool: the planner emit a NOT_SUPPORTED plan for an action with nothing to undo. A name with no hyphen is one segment, so native behaviour is unchanged — including the deliberate non-match of mid-word hits like ``listing_update``. + + This surface — and ONLY this surface — also reads a verb at the END of a + segment, because mureo's own tools are named that way + (``google_ads_campaigns_list``) and the prefix-only rule reported every + native read as an item the operator cannot revert. The looser predicate + is deliberately not the shared one: its other three callers all gate + plugin-facing safety decisions, including the guardrail money scan, and + widening those on names mureo does not control is how a mutation loses a + cap. See :func:`mureo.core.tool_names.reads_as_a_report_only_action`. """ - return is_read_only_tool_name(action) + return reads_as_a_report_only_action(action) def _not_supported(entry: ActionLogEntry, *, notes: str) -> RollbackPlan: diff --git a/tests/test_batch_revertible_unit.py b/tests/test_batch_revertible_unit.py index f0c9c6fa..2c9365a5 100644 --- a/tests/test_batch_revertible_unit.py +++ b/tests/test_batch_revertible_unit.py @@ -757,71 +757,23 @@ def test_already_reversed_member_is_not_offered_again( assert plan.members[0].status is BatchMemberStatus.ALREADY_REVERSED assert plan.apply_order == () - def test_native_read_only_member_is_not_counted_as_a_gap( - self, workspace: Path - ) -> None: + def test_a_read_in_the_batch_is_not_counted_as_a_gap(self, workspace: Path) -> None: """A read in the batch is not something the operator must undo by hand. - KNOWN DEFECT, pinned deliberately — read this before "fixing" it. - - **What is wrong.** ``mureo.core.tool_names.is_read_only_tool_name`` - anchors its verbs at the START of a hyphen-delimited name segment - (``list_campaigns``), but mureo's own tools put the verb at the END - (``google_ads_campaigns_list``). So:: - - is_read_only_tool_name("google_ads_campaigns_list") # False, wrong - - A NATIVE read therefore reaches ``plan_rollback`` as a write with no - ``reversible_params`` hint and is classified IRREVERSIBLE instead of - NOTHING_TO_REVERSE. The error direction is safe — nothing is offered - for reversal that should not be — but the batch report shows the - operator a read among the "cannot be reverted" items, which is untrue - and corrodes trust in exactly the surface #549 adds. The bridged - spelling (``campaign_management-list_campaigns``) is matched correctly - today, which is why both are asserted here. - - **Why it is not fixed in the #549 PR.** The obvious fix — also match a - verb at the END of a segment — is wrong, not merely broad. Three - modules share this vocabulary, and one of them gates a DENIAL: - ``mureo.mcp.server._register_pattern_fallbacks`` skips - ``register_pattern_fallback_tool(name)`` when the name reads as a read, - so a name wrongly classified as a read loses its guardrail money - pattern-scan. Measured on a 294-tool installed plugin surface, a naive - suffix rule flips 23 names, and **13 of them are** - ``ToolSemantics(mutating=True)`` — i.e. 13 real mutations would be - newly exempted from the money scan:: - - amc-execute_query - logly_ads_context_merge_adgroup_list - reporting-create_campaign_report - reporting-create_inventory_report - reporting-create_product_report - reporting-create_report - reporting-delete_report <- a DELETE reading as a read - yahoo_ads_create_placement_url_list - yahoo_ads_display_create_placement_url_list - yahoo_ads_display_remove_placement_url_list - yahoo_ads_display_update_placement_url_list - yahoo_ads_remove_placement_url_list - yahoo_ads_update_placement_url_list - - (On the native side the same rule flips 70 of 208 names, none carrying - a write verb — the native direction alone is safe.) - - **What a correct fix must do.** Match a trailing verb only when no - write verb (``create`` / ``update`` / ``delete`` / ``remove`` / ``set`` - / ``add`` / ``merge`` / ``execute`` …) appears elsewhere in the same - segment, so ``google_ads_campaigns_list`` becomes a read while - ``reporting-delete_report`` and ``yahoo_ads_update_placement_url_list`` - stay writes. It changes plugin guardrail registration, plugin - ``derive_semantics`` classification and ``mureo rollback list`` output, - so it needs its own tests in ``test_strategy_gate_pattern_fallback.py``, - ``test_mcp_plugin_semantics.py``, ``test_rollback.py`` and - ``test_cli_rollback.py``. - - **What flips here when it lands.** The ``by_index[0]`` assertion below - becomes ``BatchMemberStatus.NOTHING_TO_REVERSE``. ``by_index[1]``, - ``by_index[2]`` and ``apply_order`` are unchanged. + Both spellings of a read are recognised now. The bridged convention + puts the verb first (``campaign_management-list_campaigns``); mureo's + own puts it last (``google_ads_campaigns_list``), and for a long time + only the first was matched — so every native read in a batch reached + ``plan_rollback`` as a write with no ``reversible_params`` hint and + was reported IRREVERSIBLE. The direction was safe (nothing was ever + offered for reversal that should not be) but the operator was shown a + read among the "cannot be reverted" items, which is untrue and + corrodes the exact surface #549 adds. + + The trailing reading is guarded by ``WRITE_VERBS`` — see + ``mureo.core.tool_names`` and the write-verb cases in + ``tests/test_rollback.py``, which is where the measurement behind that + guard is recorded. """ state_file = workspace / "STATE.json" batch = begin_batch(state_file, label="a pass that also read things") @@ -842,14 +794,16 @@ def test_native_read_only_member_is_not_counted_as_a_gap( plan = plan_batch_rollback(read_state_file(state_file), batch.batch_id) by_index = {m.index: m for m in plan.members} - # Bridged spelling: correctly recognised as a read today. + # Native spelling, verb last. + assert by_index[0].status is BatchMemberStatus.NOTHING_TO_REVERSE + # Bridged spelling, verb first. assert by_index[1].status is BatchMemberStatus.NOTHING_TO_REVERSE - # Native spelling: misclassified today. Flip this to - # NOTHING_TO_REVERSE with the tool_names fix. - assert by_index[0].status is BatchMemberStatus.IRREVERSIBLE assert by_index[2].status is BatchMemberStatus.REVERSIBLE - # Either way a read is never offered for reversal. + # A read is never offered for reversal, and with both reads now + # recognised the batch reports as fully revertible rather than + # showing the operator a read they must somehow undo. assert plan.apply_order == (2,) + assert plan.coverage is BatchCoverage.FULL def test_unknown_batch_id_is_empty_not_a_lie(self, workspace: Path) -> None: plan = plan_batch_rollback(read_state_file(workspace / "STATE.json"), "nope") diff --git a/tests/test_learning_reset_preflight.py b/tests/test_learning_reset_preflight.py index 5c75ba40..458f7f9e 100644 --- a/tests/test_learning_reset_preflight.py +++ b/tests/test_learning_reset_preflight.py @@ -569,3 +569,28 @@ def test_plugin_can_advertise_its_own_rules(self) -> None: ) assert classify_change("demo_update_target", {}).risk is ResetRisk.RESETS assert classify_change("demo_update_name", {}).risk is ResetRisk.NO_RESET + + +@pytest.mark.unit +@pytest.mark.parametrize( + "tool_name", + [ + # Verb-last names on a platform with no enumerated trigger list. The + # rollback planner reads these as reads; this consumer must not, or a + # real mutation is declared unable to restart a learning period and + # the pre-flight goes quiet on exactly the change that matters. + "acme_ads_campaigns_list", + "acme_ads_budget_get", + "yahoo_ads_patch_placement_url_list", + "yahoo_ads_cancel_placement_url_list", + "campaign_del_list", + ], +) +def test_a_trailing_verb_does_not_make_a_change_a_read(tool_name: str) -> None: + """Pinned so pointing this consumer at the loose matcher goes red. + + ``mureo.core.tool_names`` deliberately has two matchers: a strict one for + the plugin-facing decisions (this among them) and a looser one, used only + by the rollback planner, that also reads a verb at the end of a name. + """ + assert classify_change(tool_name, {}).risk is not ResetRisk.NO_RESET diff --git a/tests/test_mcp_plugin_semantics.py b/tests/test_mcp_plugin_semantics.py index e4a13df7..d675e21e 100644 --- a/tests/test_mcp_plugin_semantics.py +++ b/tests/test_mcp_plugin_semantics.py @@ -157,8 +157,13 @@ def test_an_explicit_hint_always_wins_over_the_name(self) -> None: assert read.mutating is False def test_the_fallback_uses_the_shared_read_vocabulary(self) -> None: - """Same answer as the rollback planner and the guardrail - pattern-fallback registration — three surfaces, one list.""" + """One list across the surfaces that share the STRICT matcher. + + The rollback planner reads the same list through a looser matcher of + its own (it also accepts a verb at the end of a name). This surface + does not — see ``mureo.core.tool_names.reads_as_a_report_only_action`` + for why the two are separate. + """ from mureo.core.tool_names import is_read_only_tool_name for name in ("billing-list_invoice_summaries", "acme-create_thing"): @@ -166,6 +171,31 @@ def test_the_fallback_uses_the_shared_read_vocabulary(self) -> None: is_read_only_tool_name(name) ) + @pytest.mark.parametrize( + "name", + [ + # Verb-last names. The rollback planner reads these as reads; this + # surface must not, or a plugin mutation stops being recorded in + # action_log at all. + "google_ads_campaigns_list", + "acme_ads_budget_get", + "yahoo_ads_patch_placement_url_list", + "yahoo_ads_cancel_placement_url_list", + "campaign_del_list", + ], + ) + def test_a_trailing_verb_does_not_make_a_plugin_tool_a_read( + self, name: str + ) -> None: + """Pinned so pointing this consumer at the loose matcher goes red. + + ``derive_semantics`` defaults an undeclared tool to mutating, which is + what gets it promoted into ``action_log``. Reading a verb-last name as + a read here would drop the record of a real mutation, and the loss is + silent. + """ + assert derive_semantics(_tool(name=name)).mutating is True + @pytest.mark.unit class TestRecordMutationActionLog: diff --git a/tests/test_rollback.py b/tests/test_rollback.py index 86cd578a..ca6d792c 100644 --- a/tests/test_rollback.py +++ b/tests/test_rollback.py @@ -440,14 +440,83 @@ def test_a_hyphen_namespaced_write_is_still_plannable(self, monkeypatch) -> None @pytest.mark.parametrize( "action", [ - # Native names keep their exact behaviour: the read-only prefixes - # anchor at the start of the name, never mid-word. - "google_ads_campaigns_list", "update_budget", + # A verb that is only a prefix of a longer word is still not a + # verb: `listing` is not `list`, and the trailing-verb reading + # tokenises on `_` for the same reason. "listing_update", + "getter_config", + ], + ) + def test_a_native_write_is_still_a_write(self, action: str) -> None: + entry = _entry(action=action, reversible_params=None) + plan = plan_rollback(entry) + assert plan is not None + assert plan.status == RollbackStatus.NOT_SUPPORTED + + @pytest.mark.parametrize( + "action", + [ + # mureo's own convention puts the verb at the END, which the + # prefix-only rule never matched — so every native read reached + # here as a write with no hint and was reported as something the + # operator has to undo by hand. + "google_ads_campaigns_list", + "google_ads_budget_get", + "google_ads_search_terms_report", + "analysis_anomalies_check", + ], + ) + def test_a_native_read_is_read_only(self, action: str) -> None: + assert plan_rollback(_entry(action=action, reversible_params=None)) is None + + @pytest.mark.parametrize( + "action", + [ + # Real names from the installed plugin surface; each a mutation + # whose segment happens to END in a read verb. + "yahoo_ads_update_placement_url_list", + "yahoo_ads_remove_placement_url_list", + "yahoo_ads_create_placement_url_list", + "yahoo_ads_display_update_placement_url_list", + "yahoo_ads_display_remove_placement_url_list", + "yahoo_ads_display_create_placement_url_list", + "reporting-delete_report", + "reporting-create_report", + "reporting-create_campaign_report", + "reporting-create_inventory_report", + "reporting-create_product_report", + "amc-execute_query", + "logly_ads_context_merge_adgroup_list", + # Verbs no installed tool uses today. A hardcoded vocabulary is + # only as good as the next plugin's naming, so the ones a review + # found missing are pinned here rather than left to be + # rediscovered. + "yahoo_ads_patch_placement_url_list", + "yahoo_ads_replace_placement_url_list", + "yahoo_ads_cancel_placement_url_list", + "yahoo_ads_duplicate_placement_url_list", + "yahoo_ads_attach_placement_url_list", + "yahoo_ads_detach_placement_url_list", + "campaigns-publish_report", + "campaigns-restore_report", ], ) - def test_native_names_are_unchanged(self, action: str) -> None: + def test_a_write_verb_beats_a_trailing_read_verb(self, action: str) -> None: + """The guard that makes the trailing reading safe to have at all. + + This surface reports what an operator can be asked to undo, so a + mutation misread here is a real gap hidden inside a batch that claims + full coverage. The vocabulary is single-sourced from + ``mureo.byod._client_common._MUTATION_PREFIXES``, which AGENTS.md + calls authoritative, so a verb learned there is known here too. + + The guarded reading is confined to THIS surface. The plugin-facing + callers of ``is_read_only_tool_name`` — the guardrail money scan + above all — keep the strict verb-first rule, because mureo does not + name plugin tools and no hardcoded vocabulary can be complete for + names it does not control. + """ entry = _entry(action=action, reversible_params=None) plan = plan_rollback(entry) assert plan is not None diff --git a/tests/test_strategy_gate_pattern_fallback.py b/tests/test_strategy_gate_pattern_fallback.py index 59299432..64995666 100644 --- a/tests/test_strategy_gate_pattern_fallback.py +++ b/tests/test_strategy_gate_pattern_fallback.py @@ -1395,6 +1395,44 @@ def test_mutation_shaped_names_are_still_registered(self, name: str) -> None: _register_plugin_pattern_fallbacks(_semantics_for(name)) assert has_pattern_fallback(name) is True + @pytest.mark.parametrize( + "name", + [ + # mureo's own verb-last convention. The ROLLBACK planner reads + # these as reads (see reads_as_a_report_only_action); this gate + # deliberately does NOT, and that difference is the design. + "google_ads_campaigns_list", + "google_ads_budget_get", + "analysis_anomalies_check", + # A plugin could ship any verb it likes, including one no + # hardcoded vocabulary knows. Widening THIS gate on name shape + # is how such a mutation would silently lose its money scan. + "yahoo_ads_patch_placement_url_list", + "yahoo_ads_cancel_placement_url_list", + "yahoo_ads_duplicate_placement_url_list", + "reporting-delete_report", + "amc-execute_query", + ], + ) + def test_a_trailing_verb_never_exempts_a_plugin_tool(self, name: str) -> None: + """The denial gate keeps the STRICT rule, on purpose. + + A read whose verb is last is recognised for rollback reporting, where + the cost of being wrong is a misleading batch coverage line. It is NOT + recognised here, where the cost of being wrong is a ``## Guardrails`` + budget cap that never fires. mureo does not name plugin tools and + cannot enumerate the verbs a plugin might use, so this gate does not + try: the exemption stays keyed on the verb-first convention alone. + + The cost is stated rather than hidden: a genuine bridged read named + verb-last is scanned here for money-shaped arguments it does not + carry, which wastes a scan and denies nothing. + """ + from mureo.mcp.server import _register_plugin_pattern_fallbacks + + _register_plugin_pattern_fallbacks(_semantics_for(name)) + assert has_pattern_fallback(name) is True + def test_the_exemption_uses_the_shared_read_vocabulary(self) -> None: """One list, two safety surfaces — see mureo.core.tool_names.""" from mureo.core.tool_names import is_read_only_tool_name