From 02dfd43b3a0ad622f47499c391e2466df163ebaf Mon Sep 17 00:00:00 2001 From: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:35:35 +0800 Subject: [PATCH 1/2] fix(manager): name the refused read argument instead of a bare failure A manager read that fails validation returned only invalid_arguments. The caller is a model that can repair its own tool call, so a bare refusal makes it retry blind and the steward answer degrades into an unexplained failure. The reader now derives its allowlist and ranges from the published tool schema and returns every rejected entry as :, next to the allowed arguments, allowed views and a repair instruction naming the tool the caller actually used. Legal reads keep their existing shape. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> --- .../manager_context/inspection.py | 103 +++++++++++++----- 1 file changed, 78 insertions(+), 25 deletions(-) diff --git a/loopx/capabilities/manager_context/inspection.py b/loopx/capabilities/manager_context/inspection.py index 157ebca3f..f45de7f12 100644 --- a/loopx/capabilities/manager_context/inspection.py +++ b/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 ``:`` 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") From 75d7f076919d4c147643bdc52044b7e40909cbb5 Mon Sep 17 00:00:00 2001 From: huangruiteng <14976749+huangruiteng@users.noreply.github.com> Date: Mon, 21 Sep 2026 09:36:17 +0800 Subject: [PATCH 2/2] test(manager): pin the refused-read repair payload Cover each rejection rule, the schema-derived allowlist, the multi-argument case, the called-tool name in the repair instruction, and the unchanged shape of a legal read. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com> --- tests/test_chat_manager_inspection.py | 89 +++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/tests/test_chat_manager_inspection.py b/tests/test_chat_manager_inspection.py index 630cfe7b3..2412174e5 100644 --- a/tests/test_chat_manager_inspection.py +++ b/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(