mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 07:17:54 +08:00
tools: check every prompting tool's arguments before the prompt
This commit is contained in:
@@ -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 `<key>` 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).
|
||||
|
||||
@@ -362,7 +362,7 @@ Safety defaults:
|
||||
`write_pty` is literal text only; use `write_pty_keys` for keys and bytes: `<c-c>` `<esc>` `<enter>` `<s-tab>` `<up>` … `<f12>`, `<c-up>` `<s-home>` `<c-s-f5>` for modifier combinations, `<up*3>` to repeat, `<m-x>` 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)
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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,
|
||||
*,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 ``<key>`` markers (never for normal typing).
|
||||
|
||||
|
||||
@@ -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
|
||||
@@ -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 <key> marker must not land in a shell prompt.
|
||||
(write_pty_keys, {"session_id": 1, "data": "<nope>"}, "unknown key <nope>"),
|
||||
(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": "<enter>"}))
|
||||
assert asked == ["write_pty_keys"]
|
||||
assert out.startswith(("[approval]", "error:"))
|
||||
Reference in New Issue
Block a user