From 7ac425be71c5020f4603069ae2558df47bef2a77 Mon Sep 17 00:00:00 2001 From: arpan Date: Tue, 15 Sep 2026 23:44:56 +0530 Subject: [PATCH] Describe every operator tool argument, and say when to use which read tool Signed-off-by: arpan --- src/ctrlrun/gateway/operator.py | 122 +++++++++++++++++++++++++++----- tests/test_mcp_operator.py | 29 ++++++++ 2 files changed, 133 insertions(+), 18 deletions(-) diff --git a/src/ctrlrun/gateway/operator.py b/src/ctrlrun/gateway/operator.py index 8e63f43c..83454c50 100644 --- a/src/ctrlrun/gateway/operator.py +++ b/src/ctrlrun/gateway/operator.py @@ -325,50 +325,109 @@ def schema(self) -> dict[str, Any]: } -_STRING: Final = {"type": "string"} +def _string(description: str) -> dict[str, Any]: + return {"type": "string", "description": description} -def _bounded(default: int) -> dict[str, Any]: - return {"type": "integer", "minimum": 1, "maximum": _MAX_LIMIT, "default": default} +def _bounded(default: int, description: str) -> dict[str, Any]: + return { + "type": "integer", + "minimum": 1, + "maximum": _MAX_LIMIT, + "default": default, + "description": description, + } #: §4.6 — a write tool's description says in its first clause that it writes, that it needs an #: authenticated human, and that the answer is recorded under their name. The assistant renders #: it, and an approver who did not know their name was going into the evidence log should learn #: it before they answer rather than after. +#: +#: Every argument carries a `description` too. An input schema says an argument is a string; it +#: cannot say that `control` filters rather than selects, that `since` accepts `24h` as readily +#: as a timestamp, or that a `failed` resolution is what unblocks a retry. A caller that has to +#: infer those from the name guesses, and this server exists so that nobody guesses. TOOLS: Final[tuple[_Tool, ...]] = ( _Tool( "list_pending_approvals", False, "Read-only. The approval requests waiting for a human, oldest first, with the action, " - "its arguments and when the request expires.", - {"limit": _bounded(_DEFAULT_PENDING_LIMIT)}, + "its arguments and when the request expires. Start here: the request ids approve and " + "deny take come from this list.", + { + "limit": _bounded( + _DEFAULT_PENDING_LIMIT, + "How many pending requests to return, 1 to 200, default 50. This bounds the " + "response and not the scan, so a store holding many answered requests still " + "walks them to find the pending ones.", + ) + }, ), _Tool( "inspect_action", False, "Read-only. One action's whole history: what was proposed, what the policy decided, " - "which approval was involved, what happened to the effect, and the receipt.", - {"action_id": _STRING}, + "which approval was involved, what happened to the effect, and the receipt. Use this " + "when you know which action you care about; use receipts to browse, stats for totals.", + { + "action_id": _string( + "The action to inspect. Action ids appear on receipts and on the pending " + "requests list_pending_approvals returns." + ) + }, ("action_id",), ), _Tool( "receipts", False, - "Read-only. The evidence this store holds, as portable JSON, oldest last.", - {"limit": _bounded(_DEFAULT_RECEIPT_LIMIT), "control": _STRING}, + "Read-only. The individual receipts this store holds, as portable JSON, oldest last, " + "one for every attempt including the refusals. Use this to read what happened; use " + "stats for totals, and inspect_action for one action end to end.", + { + "limit": _bounded( + _DEFAULT_RECEIPT_LIMIT, + "How many receipts to return, 1 to 200, default 20, oldest last.", + ), + "control": _string( + "Return only receipts citing this control id, exactly as " + "`ctrlrun receipts --control` filters. Omit it to return every receipt." + ), + }, ), _Tool( "effects", False, - "Read-only. The logical effects this store knows about, optionally one state only.", - {"state": {"type": "string", "enum": [str(state) for state in EffectState]}}, + "Read-only. The logical effects this store knows about, one row per effect key. Use " + "this to find the effects whose outcome is unknown, which are the ones resolve can " + "move on and the ones blocking a retry.", + { + "state": { + "type": "string", + "enum": [str(state) for state in EffectState], + "description": ( + "Return only effects in this state, or omit it for every effect. " + "'reserved' and 'executing' are in flight, 'committed' and 'failed' are " + "settled, and 'ambiguous' is the one that needs a human: the effect may " + "or may not have happened, and a retry stays refused until resolve says " + "which." + ), + } + }, ), _Tool( "stats", False, - "Read-only. What this store's receipts say, over an optional window.", - {"since": _STRING}, + "Read-only. One summary of what this store's receipts say over an optional window: " + "how many actions were allowed, refused or sent to a human, and how their effects " + "ended. Use this for totals; use receipts when you need the records themselves.", + { + "since": _string( + "How far back to count: an ISO-8601 timestamp carrying an offset, or a " + "relative window written as m, h or d, such as 30m, 24h or 7d. " + "Omit it to count every receipt in the store." + ) + }, ), _Tool( "approve", @@ -376,7 +435,13 @@ def _bounded(default: int) -> dict[str, Any]: "WRITES. Grants one pending approval request, letting the agent run that exact " "action once. Requires an authenticated human; the answer is recorded under their " "name and is visible in the receipt the action leaves.", - {"request_id": _STRING}, + { + "request_id": _string( + "The pending request to grant, as returned by list_pending_approvals. The " + "grant is bound to the hash of the action that request already names, so " + "there is no argument here by which a different action could be approved." + ) + }, ("request_id",), ), _Tool( @@ -384,7 +449,11 @@ def _bounded(default: int) -> dict[str, Any]: True, "WRITES. Refuses one pending approval request. Requires an authenticated human; the " "answer is recorded under their name.", - {"request_id": _STRING}, + { + "request_id": _string( + "The pending request to refuse, as returned by list_pending_approvals." + ) + }, ("request_id",), ), _Tool( @@ -394,14 +463,31 @@ def _bounded(default: int) -> dict[str, Any]: "Requires an authenticated human and a reason; the answer is recorded under that " "person's name, and a 'failed' resolution permits a retry that is currently blocked.", { - "effect_key": _STRING, - "outcome": {"type": "string", "enum": sorted(RESOLUTIONS)}, - "reason": _STRING, + "effect_key": _string( + "The effect to settle, as it appears in the effects list, for example " + "refund:txn_1. Only an effect whose outcome is unknown can be resolved." + ), + "outcome": { + "type": "string", + "enum": sorted(RESOLUTIONS), + "description": ( + "What you established actually happened at the remote. 'committed' means " + "the effect took hold and must never run again; 'failed' means it did " + "not, and is what releases the blocked retry. Check the remote before " + "answering: this is a claim about the world, not a guess." + ), + }, + "reason": _string( + "How you know, in plain words. Must be non-empty. A resolution answered " + "through an assistant has a conversation behind it and no record of it, so " + "this reason is that record." + ), }, ("effect_key", "outcome", "reason"), ), ) + _BY_NAME: Final = {tool.name: tool for tool in TOOLS} diff --git a/tests/test_mcp_operator.py b/tests/test_mcp_operator.py index 449347ea..66433218 100644 --- a/tests/test_mcp_operator.py +++ b/tests/test_mcp_operator.py @@ -800,6 +800,35 @@ def test_T185_a_write_tools_description_says_what_it_costs_the_approver(): assert tool.description.startswith("Read-only."), tool.name +def test_every_tool_argument_carries_a_description_and_enums_explain_their_values(): + """§4.6 says what a tool's own description must carry; this is the same argument one level + down. An input schema can say `control` is a string and `since` is a string, and it cannot + say that one filters rather than selects, or that the other takes `24h` as readily as a + timestamp. A caller that has to infer those from the name guesses, and the whole point of + this server is that the human answering does not guess. + + The length floor is what stops the description being the parameter name again; it is + deliberately the only shape rule, because a first draft also banned the argument's own name + from its description and that failed on `control`, whose description has to say "control id" + to be any use at all. The enum rule is there because `state` and `outcome` are the two + arguments where the values, not the argument, are the thing needing explanation: + `ambiguous` is not self-evidently the state that blocks a retry, and `failed` is not + self-evidently the answer that releases one. + """ + from ctrlrun.gateway.operator import TOOLS + + for tool in TOOLS: + assert tool.properties, tool.name + for argument, schema in tool.properties.items(): + where = f"{tool.name}.{argument}" + description = schema.get("description", "") + assert description, where + assert len(description) >= 40, where + if "enum" in schema: + named = [value for value in schema["enum"] if value in description] + assert len(named) >= 2, f"{where} explains {named} of {schema['enum']}" + + # --- T186 — expiry, on both sides of the clock ------------------------------------------