fix(manager): name the refused read argument instead of a bare failure

loopx/capabilities/manager_context/inspection.py

@@ -63,6 +63,71 @@
     "Paginate with next_offset. No cross-Goal access, shell, writes or execution authority."}
 
 
+# The published tool schema is the contract the caller sees, so the reader takes
+# its allowlist and ranges from there instead of restating them in prose that can
+# drift from what a caller was offered.
+_READ_PROPERTIES = READ_TOOL["inputSchema"]["properties"]
+READ_ARGUMENT_NAMES = tuple(_READ_PROPERTIES)
+READ_VIEWS = tuple(_READ_PROPERTIES["view"]["enum"])
+READ_LIMIT_RANGE = (
+    _READ_PROPERTIES["limit"]["minimum"],
+    _READ_PROPERTIES["limit"]["maximum"],
+)
+READ_DAYS_RANGE = (
+    _READ_PROPERTIES["days"]["minimum"],
+    _READ_PROPERTIES["days"]["maximum"],
+)
+
+
+def rejected_read_arguments(arguments: dict[str, Any]) -> list[str]:
+    """Name every argument that keeps a manager read from running.
+
+    The caller is a model that can repair its own tool call, but only when the
+    refusal says which argument is wrong and what the tool accepts. Each entry
+    is ``<argument>:<what it must be>`` so the correction is mechanical instead
+    of a guess against a bare ``invalid_arguments``.
+    """
+
+    rejected = [
+        f"unknown_argument:{name}"
+        for name in sorted(set(arguments) - set(READ_ARGUMENT_NAMES))
+    ]
+    view = arguments.get("view")
+    if view not in READ_VIEWS:
+        rejected.append("view:must_be_one_of_" + ",".join(READ_VIEWS))
+    if "request_id" in arguments and view != "handoffs":
+        rejected.append("request_id:only_for_view_handoffs")
+    if "include_stopped" in arguments:
+        if view != "portfolio":
+            rejected.append("include_stopped:only_for_view_portfolio")
+        elif type(arguments["include_stopped"]) is not bool:
+            rejected.append("include_stopped:must_be_a_boolean")
+    offset = arguments.get("offset", 0)
+    if type(offset) is not int or offset < 0:
+        rejected.append("offset:must_be_an_integer_at_least_0")
+    limit = arguments.get("limit", 8)
+    if type(limit) is not int or not READ_LIMIT_RANGE[0] <= limit <= READ_LIMIT_RANGE[1]:
+        rejected.append(
+            "limit:must_be_an_integer_between_"
+            f"{READ_LIMIT_RANGE[0]}_and_{READ_LIMIT_RANGE[1]}"
+        )
+    goal_id = arguments.get("goal_id")
+    if goal_id is not None and not isinstance(goal_id, str):
+        rejected.append("goal_id:must_be_a_string")
+    if "days" in arguments:
+        days = arguments["days"]
+        if view != "deliveries":
+            rejected.append("days:only_for_view_deliveries")
+        elif type(days) is not int or not READ_DAYS_RANGE[0] <= days <= READ_DAYS_RANGE[1]:
+            rejected.append(
+                "days:must_be_an_integer_between_"
+                f"{READ_DAYS_RANGE[0]}_and_{READ_DAYS_RANGE[1]}"
+            )
+    if not isinstance(arguments.get("source_id", "local"), str):
+        rejected.append("source_id:must_be_a_string")
+    return rejected
+
+
 def manager_index(context: dict[str, Any]) -> dict[str, Any]:
     """A small directory, never a second mutable progress store."""
     read_tool = CONTEXT_TOOL_NAME if context.get("scope") == "owner_goal" else TOOL_NAME
@@ -143,34 +208,22 @@ def sources(self):
     def read(self, tool: str, arguments: Any) -> dict[str, Any]:
         if tool not in {TOOL_NAME, CONTEXT_TOOL_NAME} or not isinstance(arguments, dict):
             return {"ok": False, "error": "unsupported_read_tool"}
-        if set(arguments) - {
-            "view",
-            "goal_id",
-            "offset",
-            "limit",
-            "include_stopped",
-            "request_id",
-            "source_id",
-            "days",
-        }:
-            return {"ok": False, "error": "invalid_arguments"}
+        rejected = rejected_read_arguments(arguments)
+        if rejected:
+            return {
+                "ok": False,
+                "error": "invalid_arguments",
+                "rejected_arguments": rejected,
+                "allowed_arguments": list(READ_ARGUMENT_NAMES),
+                "allowed_views": list(READ_VIEWS),
+                "detail": (
+                    f"resend {tool} with only the allowed arguments; each rejected "
+                    "entry names the argument and what it must be"
+                ),
+            }
         view, goal_id = arguments.get("view"), arguments.get("goal_id")
         offset, limit = arguments.get("offset", 0), arguments.get("limit", 8)
         include_stopped = arguments.get("include_stopped", False)
-        if (
-            view not in {"sources", "portfolio", "todos", "deliveries", "handoffs"}
-            or ("request_id" in arguments and view != "handoffs")
-            or type(include_stopped) is not bool
-            or ("include_stopped" in arguments and view != "portfolio")
-            or type(offset) is not int
-            or offset < 0
-            or type(limit) is not int
-            or not 1 <= limit <= 12
-            or (goal_id is not None and not isinstance(goal_id, str))
-            or ("days" in arguments and (view != "deliveries" or type(arguments["days"]) is not int or not 1 <= arguments["days"] <= 90))
-            or not isinstance(arguments.get("source_id", "local"), str)
-        ):
-            return {"ok": False, "error": "invalid_arguments"}
         if not self.scope_valid():
             return {"ok": False, "error": "authorization_changed"}
         source_id = arguments.get("source_id", "local")

tests/test_chat_manager_inspection.py

@@ -8,7 +8,11 @@
 import pytest
 
 from loopx.capabilities.manager_context.inspection import (
+    CONTEXT_TOOL_NAME,
     ManagerInspection,
+    READ_ARGUMENT_NAMES,
+    READ_TOOL,
+    READ_VIEWS,
     TOOL_NAME,
     manager_index,
 )
@@ -77,6 +81,91 @@ def forbidden(**_):
     assert not records
 
 
+@pytest.mark.parametrize(
+    "args, expected",
+    [
+        ({"view": "portfolio", "path": "/unknown"}, ["unknown_argument:path"]),
+        (
+            {"view": "shell"},
+            ["view:must_be_one_of_sources,portfolio,todos,deliveries,handoffs"],
+        ),
+        ({"view": "portfolio", "offset": True}, ["offset:must_be_an_integer_at_least_0"]),
+        (
+            {"view": "portfolio", "limit": 13},
+            ["limit:must_be_an_integer_between_1_and_12"],
+        ),
+        (
+            {"view": "todos", "goal_id": "alpha", "request_id": "a" * 64},
+            ["request_id:only_for_view_handoffs"],
+        ),
+        (
+            {"view": "portfolio", "include_stopped": 1},
+            ["include_stopped:must_be_a_boolean"],
+        ),
+        (
+            {"view": "todos", "goal_id": "alpha", "include_stopped": True},
+            ["include_stopped:only_for_view_portfolio"],
+        ),
+        ({"view": "todos", "goal_id": "alpha", "days": 7}, ["days:only_for_view_deliveries"]),
+        (
+            {"view": "deliveries", "goal_id": "alpha", "days": 91},
+            ["days:must_be_an_integer_between_1_and_90"],
+        ),
+        (
+            {"view": "todos", "goal_id": "alpha", "source_id": 3},
+            ["source_id:must_be_a_string"],
+        ),
+        ({"goal_id": "alpha"}, ["view:must_be_one_of_sources,portfolio,todos,deliveries,handoffs"]),
+    ],
+)
+def test_a_refused_read_names_the_argument_that_must_change(tmp_path, args, expected):
+    """A rejected tool call must be repairable without guessing.
+
+    The caller is a model, not a human reading a stack trace: a bare
+    ``invalid_arguments`` makes it retry blind, while this payload names the
+    offending argument and what the published schema accepts.
+    """
+
+    tool, records = inspector(tmp_path)
+    result = tool.read(TOOL_NAME, args)
+    assert result["ok"] is False and result["error"] == "invalid_arguments"
+    assert result["rejected_arguments"] == expected
+    # The offer is the published schema, so the correction cannot drift from
+    # what the caller was actually handed.
+    assert result["allowed_arguments"] == list(READ_ARGUMENT_NAMES)
+    assert result["allowed_views"] == list(READ_VIEWS)
+    assert result["allowed_arguments"] == list(READ_TOOL["inputSchema"]["properties"])
+    assert not records
+
+
+def test_a_refused_read_names_every_bad_argument_and_the_called_tool(tmp_path):
+    tool, records = inspector(tmp_path)
+    result = tool.read(
+        CONTEXT_TOOL_NAME,
+        {"view": "shell", "limit": 99, "path": "/x", "days": 0},
+    )
+    assert result["rejected_arguments"] == [
+        "unknown_argument:path",
+        "view:must_be_one_of_sources,portfolio,todos,deliveries,handoffs",
+        "limit:must_be_an_integer_between_1_and_12",
+        "days:only_for_view_deliveries",
+    ]
+    # The repair instruction names the tool the caller actually used.
+    assert CONTEXT_TOOL_NAME in result["detail"]
+    assert TOOL_NAME not in result["detail"]
+    assert not records
+
+
+def test_a_valid_read_keeps_its_existing_shape(tmp_path):
+    """The refusal payload is additive: legal reads are unchanged."""
+
+    tool, records = inspector(tmp_path)
+    result = tool.read(TOOL_NAME, {"view": "portfolio", "limit": 12})
+    assert result["ok"] is True
+    assert "rejected_arguments" not in result and "allowed_arguments" not in result
+    assert records
+
+
 def test_revocation_during_read_suppresses_result(monkeypatch, tmp_path):
     grants = iter([True, False])
     monkeypatch.setattr(