mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 07:17:54 +08:00
agent/tools: report missing required arguments before running
This commit is contained in:
@@ -201,6 +201,30 @@ def tool[**PS, R](
|
||||
return decorator
|
||||
|
||||
|
||||
def _missing_required_error(definition: ToolDefinition, args: Mapping[str, object]) -> str | None:
|
||||
"""Model-facing error when a call omits a required argument.
|
||||
|
||||
The JSON schema already declares what is required, so the model gets the
|
||||
exact argument names back instead of the handler's bare ``TypeError``
|
||||
(``missing 1 required positional argument``). Unknown *extra* arguments are
|
||||
left to the handler: a declared schema is not proof that no other key is
|
||||
accepted (MCP servers may document only part of their input).
|
||||
"""
|
||||
schema = definition.parameters
|
||||
required = schema.get("required")
|
||||
if not isinstance(required, list):
|
||||
return None
|
||||
missing = [name for name in cast("list[object]", required) if isinstance(name, str) and name not in args]
|
||||
if not missing:
|
||||
return None
|
||||
properties = schema.get("properties")
|
||||
accepted: list[str] = []
|
||||
if isinstance(properties, dict):
|
||||
accepted = sorted(str(key) for key in cast("dict[object, object]", properties))
|
||||
hint = f"; it accepts: {', '.join(accepted)}" if accepted else ""
|
||||
return f"error: `{definition.name}` is missing required argument(s): {', '.join(missing)}{hint}"
|
||||
|
||||
|
||||
class ToolRegistry:
|
||||
"""Name → tool definition map with execution helpers."""
|
||||
|
||||
@@ -434,6 +458,11 @@ class ToolRegistry:
|
||||
if not isinstance(raw_args, dict):
|
||||
return "error: tool arguments must be a JSON object"
|
||||
args = {str(key): value for key, value in cast("dict[object, object]", raw_args).items()}
|
||||
# Argument shape/coverage problems are answered before any confirm prompt:
|
||||
# the human should never be asked about a call that cannot run.
|
||||
missing_error = _missing_required_error(definition, args)
|
||||
if missing_error is not None:
|
||||
return missing_error
|
||||
|
||||
async def _run() -> str:
|
||||
missing = self._check_state_tags(definition)
|
||||
|
||||
@@ -70,6 +70,46 @@ async def test_registry_handler_error() -> None:
|
||||
assert "failed" in result
|
||||
|
||||
|
||||
async def test_registry_reports_missing_required_arguments() -> None:
|
||||
"""A missing required arg is explained with the accepted names, not a TypeError."""
|
||||
called: list[str] = []
|
||||
|
||||
@tool(register=False)
|
||||
def pick(question: str, options: list[str], *, allow_custom: bool = True) -> str:
|
||||
del allow_custom
|
||||
called.append(question)
|
||||
return options[0]
|
||||
|
||||
registry = ToolRegistry([pick])
|
||||
result = await registry.execute("pick", '{"question": "Which?"}')
|
||||
assert result == (
|
||||
"error: `pick` is missing required argument(s): options; it accepts: allow_custom, options, question"
|
||||
)
|
||||
assert called == [] # the handler never ran
|
||||
assert await registry.execute("pick", '{"question": "Which?", "options": ["a"]}') == "a"
|
||||
assert called == ["Which?"]
|
||||
|
||||
|
||||
async def test_registry_missing_arg_skips_confirm_hook() -> None:
|
||||
"""The human is not asked to confirm a call that can never run."""
|
||||
confirms: list[str] = []
|
||||
|
||||
def confirm(name: str, args: object, reason: str) -> bool:
|
||||
del args, reason
|
||||
confirms.append(name)
|
||||
return True
|
||||
|
||||
@tool(register=False)
|
||||
def write(path: str, content: str) -> str:
|
||||
del content
|
||||
return path
|
||||
|
||||
registry = ToolRegistry([write], danger=lambda name, args: "writes a file", on_confirm=confirm)
|
||||
result = await registry.execute("write", '{"path": "x"}')
|
||||
assert "missing required argument(s): content" in result
|
||||
assert confirms == []
|
||||
|
||||
|
||||
def test_tool_items() -> None:
|
||||
@tool(register=False)
|
||||
def f(x: int) -> int:
|
||||
|
||||
@@ -333,3 +333,29 @@ def test_wait_prompt_renders_fractional_seconds() -> None:
|
||||
prompt = wait_module._wait_prompt(2.5, reason=None)
|
||||
assert prompt.startswith("Waiting for 2.5s.")
|
||||
assert wait_module._wait_prompt(5, reason=None).startswith("Waiting for 5s.")
|
||||
|
||||
|
||||
async def test_choose_user_reports_missing_options() -> None:
|
||||
"""A call without the required ``options`` lists what the tool accepts."""
|
||||
backend = ScriptedBackend(["1"])
|
||||
with temporary_backend(backend):
|
||||
registry = ToolRegistry([choose_user])
|
||||
out = await registry.execute("ask_user_choice", json.dumps({"question": "Pick"}))
|
||||
assert out.startswith("error:")
|
||||
assert "missing required argument(s): options" in out
|
||||
assert "it accepts: allow_custom, default, options, question" in out
|
||||
assert backend.lines == ["1"] # the human was never asked
|
||||
|
||||
|
||||
async def test_form_user_reports_missing_fields() -> None:
|
||||
registry = ToolRegistry([form_user])
|
||||
out = await registry.execute("ask_user_form", json.dumps({"title": "Setup"}))
|
||||
assert out.startswith("error:")
|
||||
assert "missing required argument(s): fields" in out
|
||||
|
||||
|
||||
async def test_wait_reports_missing_duration() -> None:
|
||||
registry = ToolRegistry([wait])
|
||||
out = await registry.execute("wait", "{}")
|
||||
assert out.startswith("error:")
|
||||
assert "missing required argument(s): duration" in out
|
||||
|
||||
Reference in New Issue
Block a user