diff --git a/AGENTS.md b/AGENTS.md index 172bc5d..886a4c7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -85,7 +85,7 @@ Async SQLAlchemy + aiosqlite. `MemoryStore`: schema init (+ lightweight SQLite ` - **`messages_bridge` / `messages_dispatch`**: chat ↔ Anthropic Messages; system fold; tool_use/tool_result; stream text + tool argument fragments; `thinking` from the request's `reasoning_effort` (level → token budget, floor 1024, `max_tokens` grown when the budget would leave no room) plus the client's configured `thinking_budget_tokens`; a thinking block the model refuses (from Claude 4.7 on, `thinking.type.enabled` is a 400 asking for adaptive thinking + `output_config.effort`; a model whose thinking is always on refuses `thinking.type.disabled`) gets one retry with the shape it takes — adaptive thinking with the level as the effort, or no block at all — and the refusal is remembered per model on the client, so only its first request pays the round trip (only a refusal that arrives before the stream yields anything is retried; a later failure has already been seen); a server tool (`server_tool_use` + its `web_search_tool_result`) is the surface's own business — it streams `input_json_delta` fragments like a call but nothing local asked for it, so only a `tool_use` block becomes a tool call and nothing is executed; `thinking` and the two search blocks are kept as `AssistantChatMessage.provider_blocks` (the thinking text also lands in `reasoning_content`, so it streams as reasoning) and go back with the round, which is also why a round that kept only those blocks is not an empty completion; a chat-completions request drops the field (`compat.coerce_chat_messages`). - **Images** (see `doc/images.md`): a user turn's `content` may be a part array (`ChatTextPart` + `ChatImagePart`), and a tool result carries its pictures in `ToolChatMessage.images`; each surface lifts them into what it can express — chat-completions content blocks (tool-result images move into the user turn after the tool run, since a `tool` message holds text only), Anthropic `image` blocks (inside `tool_result` for a tool result), Responses `input_image` parts (in a `function_call_output` item). Pictures belong to user turns: an image in a system or assistant message is a provider 400. `agent/images.py` sniffs the format from the bytes and caps an inline picture at 8 MiB, because every round re-sends it; `[providers.*.models.*].image_in` is the opt-in, and every image-carrying path checks it — an `@path` mention, `view_image`, `fetch_local`'s image body, and (as a safety net in the registry) an MCP server's or plugin's `ToolOutput`. - **Provider-side tools**: `provider_tools` on the OpenAI **and** DeepSeek presets (list of dicts; `[{type="web_search"}]` when omitted; `[]` disables) threaded via `ChatAgent`/`run_chat_loop` into Responses and Messages; never executed by `ToolRegistry`. The Messages surface declares its own server tool instead — a bare `web_search` becomes `{type="web_search_20250305", name="web_search"}` (the search then runs inside the response, and its `encrypted_content` results go back verbatim with the round — `AssistantChatMessage.provider_blocks`) — and a DeepSeek provider on a surface that ignores hosted tools (`config.routing.local_web_search`) gets the local `web_search` tool from `tools/net/search.py` instead. -- **`@tool` / `ToolRegistry`**: decorator infers JSON Schema from type hints; execute tools by name. A handler returns its text, or a `ToolOutput(text, images)` when it also produced a picture: `execute` keeps the text-only contract and `execute_output` returns the pair, which `run_chat_loop` stores as the tool message's text plus its `images` (`view_image`, an image body from `fetch_local`, and an MCP `image` content block all take that path). Before running, `execute` answers a call that omits a required argument with the accepted names (`_missing_required_error`) — the handler's bare `TypeError` never reaches the model, and no confirm prompt fires for a call that cannot run. Shape mistakes inside a *present* argument are the tool's own job (see the `chat` bullet), unless the tool declares its own pre-prompt check (`@tool(validate=…)`, answered in that same place): `run_argv` / `run_argv_batch` answer a `timeout_seconds` that is not a positive number and a batch tree that cannot be built, so a value that would only fail inside `wait_for` never spawns a process, and `prepare_batch_confirm` leaves such a call out of the round's prompt. `@tool(writes=("path",))` names a handler's file arguments: one batch applies calls that name the same file one at a time, in call order — the lock is taken before the confirm prompt, so a pending prompt cannot be overtaken — while different files keep running in parallel. +- **`@tool` / `ToolRegistry`**: decorator infers JSON Schema from type hints; execute tools by name. A handler returns its text, or a `ToolOutput(text, images)` when it also produced a picture: `execute` keeps the text-only contract and `execute_output` returns the pair, which `run_chat_loop` stores as the tool message's text plus its `images` (`view_image`, an image body from `fetch_local`, and an MCP `image` content block all take that path). Before running, `execute` answers a call that omits a required argument with the accepted names (`_missing_required_error`) — the handler's bare `TypeError` never reaches the model, and no confirm prompt fires for a call that cannot run. Shape mistakes inside a *present* argument are the tool's own job (see the `chat` bullet), unless the tool declares its own pre-prompt check (`@tool(validate=…)`, answered in that same place). `run_argv` / `run_argv_batch` / `open_pty` / `write_pty_keys` / `edit_replace` / `edit_lineno` / `fetch_local` do: a `timeout_seconds` that is not a positive number (so a value that would only fail inside `wait_for` never spawns a process), an `argv` sent as a string, an unknown `` marker, an empty `old_string` or `max_replaces < 1`, a backwards line range, a verb outside GET/POST/PUT/DELETE and a URL that cannot be parsed. `prepare_batch_confirm` leaves such a call out of the round's prompt, so nobody is asked about a call that cannot run. `@tool(writes=("path",))` names a handler's file arguments: one batch applies calls that name the same file one at a time, in call order — the lock is taken before the confirm prompt, so a pending prompt cannot be overtaken — while different files keep running in parallel. - **`run_chat_loop`**: multi-round tool loop; default **streaming** text deltas + stream tool-call merge; parallel tools; request-prep tool-result cap (each result stored whole, trimmed on the request copy with a `document_id=...` marker); soft context compact on request (**API-calibrated** after first usage when available); cooperative cancel points; optional `on_limit`; resolved `reasoning_effort` on every request (see Config). A round the provider cut off at the output limit is kept, not failed: the answer and its thinking blocks stay in `messages`, the round's tool calls are dropped (a half-written call must not run — the model re-issues it), and up to `max_truncation_continues` (2) further rounds carry on from a truncation notice; 0 spends the turn as unfinished instead, for `/retry`. - **`ChatAgent`**: optional `MemoryStore` (user message persisted immediately; **completed tool batches checkpointed** mid-turn; unfinished assistant suffix rolled back on failure); `stream`; system prompt; `provider_tools`; `retry()` continues incomplete turns (user-only, after committed tools — does not re-run those tools — or after a kept truncated answer) and `pending_retry_text` reads a truncation notice as unfinished work. - **`/compact`**: soft-compact tool dumps → model summary (no tools) via the same kind dispatch → **new** session seeded with summary message. Soft-compact keeps the last **12** tool results full-size by default (`DEFAULT_RECENT_TOOL_RESULTS`); older tool payloads shrink first. The default summarizer system prompt states that tool calls are unavailable (a `compact_system_prompt` override replaces that default wholesale). diff --git a/README.md b/README.md index d63c9fc..10c6bd8 100644 --- a/README.md +++ b/README.md @@ -362,7 +362,7 @@ Safety defaults: `write_pty` is literal text only; use `write_pty_keys` for keys and bytes: `` `` `` `` `` … ``, `` `` `` for modifier combinations, `` to repeat, `` for Alt, and `\xHH` / `\uHHHH` for raw bytes. An unknown `<…>` is an error, never text typed into the terminal. For passwords/sudo/ssh prompts use `ask_into_pty` (human types locally; answer never returns to the model). -- Mis-typed tool arguments are answered, never guessed: a string where an array is expected (`run_argv` `argv`, `ask_user_choice` `options`, `ask_user_form` `fields`), a call that omits a required argument, a non-numeric `wait` duration (numeric strings are accepted), or a `timeout_seconds` that is not a positive number returns an error showing the shape/names to send — the human never sees a garbage prompt, and no confirm prompt fires for a call that cannot run (a tool declares that pre-prompt check with `@tool(validate=…)`; `run_argv` / `run_argv_batch` do, the batch also for a tree it cannot build). +- Mis-typed tool arguments are answered, never guessed: a string where an array is expected (`run_argv` `argv`, `ask_user_choice` `options`, `ask_user_form` `fields`), a call that omits a required argument, a non-numeric `wait` duration (numeric strings are accepted), or a `timeout_seconds` that is not a positive number returns an error showing the shape/names to send — the human never sees a garbage prompt, and no confirm prompt fires for a call that cannot run (a tool declares that pre-prompt check with `@tool(validate=…)`; `run_argv`, `run_argv_batch` (also for a tree it cannot build), `open_pty`, `write_pty_keys`, `edit_replace`, `edit_lineno` and `fetch_local` do). ## Usage / context (CLI) diff --git a/src/plyngent/tools/chat/shape.py b/src/plyngent/tools/chat/shape.py index 9955708..b397e43 100644 --- a/src/plyngent/tools/chat/shape.py +++ b/src/plyngent/tools/chat/shape.py @@ -14,20 +14,9 @@ tools themselves stay free of unnecessary-isinstance warnings. from __future__ import annotations -_SHAPES: dict[type, str] = { - str: "a string", - list: "an array", - dict: "an object", - bool: "a boolean", - int: "a number", - float: "a number", - type(None): "null", -} +from plyngent.tools.shape import shape_of - -def shape_of(value: object) -> str: - """Model-facing name for a decoded JSON value (``a string``, ``an array``…).""" - return _SHAPES.get(type(value), type(value).__name__) +__all__ = ["first_error", "list_error", "number_arg", "shape_of", "string_error"] def first_error(*errors: str | None) -> str | None: diff --git a/src/plyngent/tools/file/edit_lineno.py b/src/plyngent/tools/file/edit_lineno.py index ccbedec..f3c83df 100644 --- a/src/plyngent/tools/file/edit_lineno.py +++ b/src/plyngent/tools/file/edit_lineno.py @@ -3,9 +3,11 @@ from __future__ import annotations from typing import TYPE_CHECKING from plyngent.agent import ToolTag, lineno_read_lines, lineno_read_mtime, tool +from plyngent.tools.shape import shape_of from plyngent.tools.workspace import AccessMode, resolve_path if TYPE_CHECKING: + from collections.abc import Mapping from pathlib import Path # How many unread line numbers to preview in the error message. @@ -15,6 +17,28 @@ _UNREAD_PREVIEW = 8 _READ_HINT_MARGIN = 3 +def _range_shape_error(start_line: object, end_line: object) -> str | None: + """The two range checks that need nothing but the two numbers.""" + if not isinstance(start_line, int) or not isinstance(end_line, int): + return None # the schema declares integers; a bad pair stays the handler's to answer + if start_line < 1: + return "error: start_line must be >= 1" + if end_line < start_line: + return "error: end_line must be >= start_line" + return None + + +def edit_lineno_shape(args: Mapping[str, object]) -> str | None: + """Answer a range that cannot be applied, before anyone is asked about it.""" + shape_error = _range_shape_error(args.get("start_line"), args.get("end_line")) + if shape_error is not None: + return shape_error + content = args.get("new_content") + if content is not None and not isinstance(content, str): + return f"error: `edit_lineno` expects `new_content` as a string, got {shape_of(content)}" + return None + + def _detect_newline(lines: list[str]) -> str: for sample in lines: if sample.endswith("\r\n"): @@ -43,10 +67,9 @@ def _to_keepends(new_content: str, newline: str, *, force_trailing: bool) -> lis def _validate_range(start_line: int, end_line: int, n: int) -> str | None: - if start_line < 1: - return "error: start_line must be >= 1" - if end_line < start_line: - return "error: end_line must be >= start_line" + shape_error = _range_shape_error(start_line, end_line) + if shape_error is not None: + return shape_error if start_line > n + 1: return f"error: start_line {start_line} past end of file ({n} lines)" if start_line == n + 1 and end_line != start_line: @@ -124,7 +147,7 @@ def _replace_range( ) -@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, writes=("path",)) +@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, writes=("path",), validate=edit_lineno_shape) async def edit_lineno(path: str, start_line: int, end_line: int, new_content: str) -> str: """Replace lines ``start_line``..``end_line`` (1-based, inclusive) in a file. diff --git a/src/plyngent/tools/file/edit_replace.py b/src/plyngent/tools/file/edit_replace.py index 598eb38..6f08aae 100644 --- a/src/plyngent/tools/file/edit_replace.py +++ b/src/plyngent/tools/file/edit_replace.py @@ -1,8 +1,27 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from plyngent.agent import ToolTag, tool +from plyngent.tools.shape import shape_of from plyngent.tools.workspace import AccessMode, resolve_path +if TYPE_CHECKING: + from collections.abc import Mapping + + +def edit_replace_shape(args: Mapping[str, object]) -> str | None: + """Answer a replacement that cannot apply, before anyone is asked about it.""" + old_string = args.get("old_string") + if isinstance(old_string, str) and not old_string: + return "error: old_string must not be empty" + if not isinstance(old_string, str) and old_string is not None: + return f"error: `edit_replace` expects `old_string` as a string, got {shape_of(old_string)}" + limit = args.get("max_replaces") + if isinstance(limit, int) and not isinstance(limit, bool) and limit < 1: + return "error: max_replaces must be >= 1" + return None + def _count_non_overlapping(text: str, needle: str) -> int: """Count left-to-right non-overlapping matches (same as ``str.replace``).""" @@ -48,7 +67,7 @@ def _success_message( ) -@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, writes=("path",)) +@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, writes=("path",), validate=edit_replace_shape) async def edit_replace(path: str, old_string: str, new_string: str, max_replaces: int = 1) -> str: """Replace occurrences of ``old_string`` with ``new_string`` in a file. diff --git a/src/plyngent/tools/net/fetch_local.py b/src/plyngent/tools/net/fetch_local.py index 3c004f2..04abb16 100644 --- a/src/plyngent/tools/net/fetch_local.py +++ b/src/plyngent/tools/net/fetch_local.py @@ -2,6 +2,8 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from plyngent.agent import ToolOutput, ToolTag, tool from plyngent.agent.images import image_part_from_bytes from plyngent.tools.context import is_read_only_context @@ -17,8 +19,36 @@ from plyngent.tools.net.policy import ( parse_fetch_url, soft_confirm_reason, ) +from plyngent.tools.shape import shape_of, timeout_shape_error from plyngent.tools.workspace import image_input_refusal +if TYPE_CHECKING: + from collections.abc import Mapping + + +def fetch_local_shape(args: Mapping[str, object]) -> str | None: + """Answer a request that cannot be built, before anyone is asked about it. + + The verb and the URL are checked here (both are the request itself, and + ``parse_fetch_url`` touches nothing but the string); a host that needs a + human grant still asks in the handler, once the call can actually run. + """ + url = args.get("url") + if not isinstance(url, str): + return f"error: `fetch_local` expects `url` as a string, got {shape_of(url)}" + method = args.get("method", "GET") + if not isinstance(method, str): + return f"error: `fetch_local` expects `method` as a string, got {shape_of(method)}" + timeout_error = timeout_shape_error(args.get("timeout_seconds")) + if timeout_error is not None: + return f"error: `fetch_local` {timeout_error}" + try: + _ = normalize_method(method) + _ = parse_fetch_url(url) + except FetchPolicyError as exc: + return f"error: {exc}" + return None + def format_fetch_result( *, @@ -59,7 +89,10 @@ def format_fetch_result( return "\n".join(parts) -@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO | ToolTag.TRUSTABLE | ToolTag.READ_ONLY) +@tool( + tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO | ToolTag.TRUSTABLE | ToolTag.READ_ONLY, + validate=fetch_local_shape, +) async def fetch_local( url: str, *, diff --git a/src/plyngent/tools/process/command_exec.py b/src/plyngent/tools/process/command_exec.py index 8c0484c..f150897 100644 --- a/src/plyngent/tools/process/command_exec.py +++ b/src/plyngent/tools/process/command_exec.py @@ -45,26 +45,6 @@ def format_command_result( return "\n".join(parts) -def timeout_shape_error(value: object) -> str | None: - """Model-facing error when ``timeout_seconds`` is not a positive number. - - Numeric strings are accepted (models send them), anything else is answered - before the call can start: a value that only fails inside ``wait_for`` has - already spawned the process, so nothing would stop it. - """ - if value is None: - return None - if isinstance(value, bool) or not isinstance(value, int | float | str): - return f"timeout_seconds must be a number, not {type(value).__name__}" - try: - seconds = float(value) - except ValueError: - return f"timeout_seconds must be a number, not {value!r}" - if seconds <= 0: - return f"timeout_seconds must be greater than 0 (got {value!r})" - return None - - def resolve_workdir(cwd: str) -> tuple["Path", str]: """Return (absolute workdir, display path relative to workspace when possible).""" workdir = resolve_path(cwd, required=AccessMode.EXEC) diff --git a/src/plyngent/tools/process/open_pty.py b/src/plyngent/tools/process/open_pty.py index 8da4908..1576122 100644 --- a/src/plyngent/tools/process/open_pty.py +++ b/src/plyngent/tools/process/open_pty.py @@ -1,14 +1,26 @@ from __future__ import annotations import shlex +from typing import TYPE_CHECKING from plyngent.agent import ToolTag, tool -from plyngent.tools.workspace import WorkspaceError +from plyngent.tools.workspace import WorkspaceError, argv_shape_error from .pty_session import active_pty_manager +if TYPE_CHECKING: + from collections.abc import Mapping -@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO) + +def open_pty_shape(args: Mapping[str, object]) -> str | None: + """Answer a session that cannot start, before anyone is asked about it.""" + shape_error = argv_shape_error(args.get("command")) + if shape_error is not None: + return f"error: `open_pty` {shape_error}" + return None + + +@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, validate=open_pty_shape) async def open_pty(command: list[str], *, cwd: str = ".") -> str: """Open a PTY session running ``command`` (argv) under the workspace. diff --git a/src/plyngent/tools/process/run_argv.py b/src/plyngent/tools/process/run_argv.py index 1dc36f5..acf80b2 100644 --- a/src/plyngent/tools/process/run_argv.py +++ b/src/plyngent/tools/process/run_argv.py @@ -3,13 +3,13 @@ from __future__ import annotations from typing import TYPE_CHECKING from plyngent.agent import ToolTag, tool +from plyngent.tools.shape import timeout_shape_error from plyngent.tools.workspace import WorkspaceError, argv_shape_error from .command_exec import ( DEFAULT_TIMEOUT_SECONDS, execute_argv, format_command_result, - timeout_shape_error, ) if TYPE_CHECKING: diff --git a/src/plyngent/tools/process/run_argv_batch.py b/src/plyngent/tools/process/run_argv_batch.py index 990190a..8c44e85 100644 --- a/src/plyngent/tools/process/run_argv_batch.py +++ b/src/plyngent/tools/process/run_argv_batch.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any, cast from plyngent.agent import ToolTag, tool +from plyngent.tools.shape import timeout_shape_error from plyngent.tools.workspace import WorkspaceError if TYPE_CHECKING: @@ -17,7 +18,6 @@ from .command_exec import ( MAX_BATCH_DEPTH, CommandStepResult, execute_argv, - timeout_shape_error, ) # Execution budget: stop the batch once accumulated output crosses this factor diff --git a/src/plyngent/tools/process/write_pty_keys.py b/src/plyngent/tools/process/write_pty_keys.py index fb30b2c..ffb714c 100644 --- a/src/plyngent/tools/process/write_pty_keys.py +++ b/src/plyngent/tools/process/write_pty_keys.py @@ -1,13 +1,31 @@ from __future__ import annotations +from typing import TYPE_CHECKING + from plyngent.agent import ToolTag, tool +from plyngent.tools.shape import shape_of from plyngent.tools.workspace import WorkspaceError from .pty_terminal import KeySyntaxError, decode_write_data from .write_pty import write_pty_payload +if TYPE_CHECKING: + from collections.abc import Mapping -@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO) + +def write_pty_keys_shape(args: Mapping[str, object]) -> str | None: + """Answer a bad marker list, before anyone is asked about the call.""" + data = args.get("data") + if not isinstance(data, str): + return f"error: `write_pty_keys` expects `data` as a string, got {shape_of(data)}" + try: + _ = decode_write_data(data) + except KeySyntaxError as exc: + return f"error: {exc}" + return None + + +@tool(tags=ToolTag.LOCAL | ToolTag.INSTANCE_STATE | ToolTag.YOLO, validate=write_pty_keys_shape) async def write_pty_keys(session_id: int, data: str) -> str: """Write to a PTY after expanding ```` markers (never for normal typing). diff --git a/src/plyngent/tools/shape.py b/src/plyngent/tools/shape.py new file mode 100644 index 0000000..b78702c --- /dev/null +++ b/src/plyngent/tools/shape.py @@ -0,0 +1,69 @@ +"""Pre-prompt argument checks shared by tools. + +Tool arguments arrive as decoded JSON, so a parameter declared ``float`` or +``list[str]`` can still hold anything at runtime. A value the tool can judge +without running a thing is answered *before* the call reaches a confirm prompt +(a tool declares the check with ``@tool(validate=…)``, see +:class:`plyngent.agent.tools.ToolDefinition`): the human is never asked about a +call that could only fail, and a value that would only break halfway through +(e.g. a deadline that fails inside ``wait_for``, after the process was spawned) +never gets that far. + +Every check takes ``object``, so the isinstance calls stay meaningful and the +tools themselves stay free of unnecessary-isinstance warnings. +""" + +from __future__ import annotations + +_SHAPES: dict[type, str] = { + str: "a string", + list: "an array", + dict: "an object", + bool: "a boolean", + int: "a number", + float: "a number", + type(None): "null", +} + + +def shape_of(value: object) -> str: + """Model-facing name for a decoded JSON value (``a string``, ``an array``…).""" + return _SHAPES.get(type(value), type(value).__name__) + + +def got(value: object) -> str: + """What a rejected value is, quoting a string (``'soon'``) instead of naming it.""" + return repr(value) if isinstance(value, str) else shape_of(value) + + +def number_value(value: object) -> float | None: + """The number *value* carries, or ``None`` when it is not one. + + Numeric strings are accepted (models send them); a boolean is not a number + here, and neither is non-numeric text. + """ + if isinstance(value, bool): + return None + if isinstance(value, int | float): + return float(value) + if isinstance(value, str): + try: + return float(value.strip()) + except ValueError: + return None + return None + + +def timeout_shape_error(value: object, *, name: str = "timeout_seconds") -> str | None: + """Error when *name* is not a positive number of seconds; ``None`` when it is. + + ``None`` means the argument was omitted (the tool's default applies). + """ + if value is None: + return None + seconds = number_value(value) + if seconds is None: + return f"{name} must be a number, not {got(value)}" + if seconds <= 0: + return f"{name} must be greater than 0 (got {value!r})" + return None diff --git a/tests/test_agent/test_tool_shape.py b/tests/test_agent/test_tool_shape.py new file mode 100644 index 0000000..a33aa6c --- /dev/null +++ b/tests/test_agent/test_tool_shape.py @@ -0,0 +1,82 @@ +"""A tool's own shape check answers a call before anyone is asked about it.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING + +import pytest + +from plyngent.agent import ToolRegistry +from plyngent.tools.file.edit_lineno import edit_lineno +from plyngent.tools.file.edit_replace import edit_replace +from plyngent.tools.net.fetch_local import fetch_local +from plyngent.tools.process.open_pty import open_pty +from plyngent.tools.process.write_pty_keys import write_pty_keys + +if TYPE_CHECKING: + from collections.abc import Mapping + + from plyngent.agent import ToolDefinition + + +def _everything_needs_a_confirm(name: str, _args: Mapping[str, object]) -> str: + """A danger classifier that prompts for every call, so a suppressed prompt is visible.""" + return f"run {name}" + + +def _registry(definition: ToolDefinition) -> tuple[ToolRegistry, list[str]]: + asked: list[str] = [] + + def on_confirm(name: str, _args: Mapping[str, object], _reason: str) -> bool: + asked.append(name) + return True + + return ToolRegistry([definition], danger=_everything_needs_a_confirm, on_confirm=on_confirm), asked + + +@pytest.mark.parametrize( + ("definition", "args", "expected"), + [ + # A string where the tool needs an argv array would splat into characters. + (open_pty, {"command": "ls -la"}, "only accepts an array of args"), + # A typo in a marker must not land in a shell prompt. + (write_pty_keys, {"session_id": 1, "data": ""}, "unknown key "), + (edit_replace, {"path": "a.py", "old_string": "", "new_string": "x"}, "old_string must not be empty"), + ( + edit_replace, + {"path": "a.py", "old_string": "x", "new_string": "y", "max_replaces": 0}, + "max_replaces must be >= 1", + ), + ( + edit_lineno, + {"path": "a.py", "start_line": 0, "end_line": 1, "new_content": "x"}, + "start_line must be >= 1", + ), + ( + edit_lineno, + {"path": "a.py", "start_line": 3, "end_line": 2, "new_content": "x"}, + "end_line must be >= start_line", + ), + (fetch_local, {"url": "https://example.com", "method": "PATCH"}, "method must be one of"), + (fetch_local, {"url": "ftp://example.com"}, "url scheme must be http or https"), + (fetch_local, {"url": "https://example.com", "timeout_seconds": "soon"}, "timeout_seconds must be a number"), + ], +) +async def test_a_bad_argument_is_answered_before_the_prompt( + definition: ToolDefinition, + args: dict[str, object], + expected: str, +) -> None: + registry, asked = _registry(definition) + out = await registry.execute(definition.name, json.dumps(args)) + assert asked == [] + assert expected in out + + +async def test_a_call_that_can_run_still_asks() -> None: + """The check answers only what it can judge; a runnable call keeps its prompt.""" + registry, asked = _registry(write_pty_keys) + out = await registry.execute("write_pty_keys", json.dumps({"session_id": 1, "data": ""})) + assert asked == ["write_pty_keys"] + assert out.startswith(("[approval]", "error:"))