From 68bf20c9a9d6c6e5a55310739a600a4374ce0a10 Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:53:20 +0900 Subject: [PATCH 1/4] Declare tools from an OpenAPI description MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A service that publishes an OpenAPI description but runs no MCP server could only be reached by writing Python. An openapi: block now turns selected operations into tools, with the constraints such code used to enforce surviving the move into declaration: - operations is required, never defaulted. A published description holds hundreds of operations, the toolset builder does not fall over on any of them, and forgetting to choose must not mean offering everything to the model. - Request URLs are built from the connection's base_url; the description's servers are never read. Published roots carry variables, stale defaults, or another tenant, and the client's destination check must keep holding. - params keeps pinned arguments pinned: value fixes a parameter and takes it out of the model's declaration entirely, prefix/suffix wrap what the model writes with the declared text in front, so the model's own qualifiers cannot displace a declared scope. - describe replaces vendor text written for developers next to the docs, and does_not rides on every tool as with mcp:. - The description is read at packing time and travels in the archive, so a vendor editing their published file changes nothing until someone re-archives deliberately. - PUT and PATCH operations must sit in an effect: write block, ride the same confirmation and redaction paths, and are never resent. DELETE cannot be declared; the client offers no delete verb. Declarations are parsed into Gemini function declarations by ADK's spec parser; execution stays in gete's ConnectionClient so the token and destination rules are the same as for every other request. 🤖 Generated with Claude Code --- README.md | 50 ++++- src/gete/archive.py | 10 + src/gete/graph.py | 9 + src/gete/openapi.py | 324 ++++++++++++++++++++++++++++ src/gete/policies.py | 5 +- src/gete/runtime/openapi.py | 326 ++++++++++++++++++++++++++++ src/gete/runtime/tools.py | 13 ++ src/gete/schema/agent.json | 96 +++++++++ src/gete/validate.py | 40 ++++ tests/test_archive.py | 73 +++++++ tests/test_graph.py | 22 ++ tests/test_openapi.py | 317 +++++++++++++++++++++++++++ tests/test_policies.py | 17 ++ tests/test_runtime_openapi.py | 390 ++++++++++++++++++++++++++++++++++ tests/test_schema.py | 37 +++- tests/test_validate.py | 110 ++++++++++ 16 files changed, 1836 insertions(+), 3 deletions(-) create mode 100644 src/gete/openapi.py create mode 100644 src/gete/runtime/openapi.py create mode 100644 tests/test_openapi.py create mode 100644 tests/test_runtime_openapi.py diff --git a/README.md b/README.md index 07a4715..080eecd 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,8 @@ connections requirements.txt registration → en whoever calls it. The tools and their guardrails ship with gete; a declaration can only switch them on. - **Runtime** — builds the ADK agent from `agent.resolved.yaml`, carries the - user's token to the tools (builtin, MCP, python), redacts what comes back. + user's token to the tools (builtin, MCP, OpenAPI, python), redacts what + comes back. - **Delivery** — a deterministic archive, a Terraform module, and `register` for the parts Terraform has no resources for. @@ -143,6 +144,53 @@ writes are split by naming it twice — the same `url` and `connection`, two lists of tool names, two effects. Where a server hands out one grant for both, that is the only place an agent can say it means to read. +### Tools from an OpenAPI description + +A service that publishes an OpenAPI description but runs no MCP server can be +declared without writing Python: + +```yaml +tools: + - openapi: + spec: ./specs/helpdesk.yaml # read at packing time, travels in the archive + connection: helpdesk + operations: [ListSearchResults, ShowTicket, ListTicketComments] + effect: read + does_not: Results are the caller's own view; not found is not proof of absence. + params: + ListSearchResults: + query: {prefix: "type:ticket "} + per_page: {value: 25} + describe: + ListSearchResults: Search tickets. The kind is fixed to tickets. +``` + +- **`operations` is required, never defaulted.** A published description + holds far more than an agent means to expose — hundreds of operations is + normal — and forgetting to choose must not mean offering everything. + Operations are picked by `operationId`, which also becomes the tool's name. +- **Request URLs are built from the connection's `base_url`.** The + description's own `servers` are never read: a published root may carry + variables, a stale default, or another tenant. The client's destination + check and token rules hold exactly as for every other request. +- **`params` keeps what the code it replaces used to enforce.** `value` + fixes a parameter and takes it out of what the model sees — its value is + declared, so there is nothing left for the model to say. `prefix` and + `suffix` wrap what the model writes; the declared text comes first, so + nothing the model writes can displace it. +- **`describe` replaces the vendor's text.** Vendor descriptions are written + for developers sitting next to the docs and often cite links a model + cannot follow; `does_not` is appended to every tool, as with `mcp:`. +- **The description is fixed at packing time.** `gete archive` takes the + file into the archive and the runtime reads it from there, so a vendor + editing their published description changes nothing until someone + re-archives deliberately. +- **Writes ride the same rails.** PUT and PATCH operations must sit in a + block declared `effect: write`, which the confirmation policies key on, + and results pass the same redaction as every other tool. DELETE cannot be + declared at all — the client offers no delete verb, and the reason is + written where the verbs live. + ### Connections `gete connections` lists what ships: `freee`, `google`, `github`, `notion-mcp`, diff --git a/src/gete/archive.py b/src/gete/archive.py index 67741f4..0c6d620 100644 --- a/src/gete/archive.py +++ b/src/gete/archive.py @@ -109,6 +109,16 @@ def build_archive(directory: Path, *, project: Project | None = None) -> Archive if instruction is not None: name = _archive_input(agent, instruction, "instruction", kind="file") entries[name] = instruction.read_bytes() + # OpenAPI descriptions are read from the archive, never the network: a + # vendor changing a published description must not change a deployed + # agent's tools. + for index, tool in enumerate(agent.tools): + if "openapi" in tool: + spec_path = agent.directory / str(tool["openapi"]["spec"]) + name = _archive_input( + agent, spec_path, f"tools[{index}].openapi.spec", kind="file" + ) + entries[name] = spec_path.read_bytes() if agent.source is not None: # Agent Engine imports from the archive root, so the source directory's # contents go there and the resolved declaration points at ".". diff --git a/src/gete/graph.py b/src/gete/graph.py index 15d4967..c9fc315 100644 --- a/src/gete/graph.py +++ b/src/gete/graph.py @@ -59,6 +59,15 @@ def mermaid(project: Project, names: list[str] | None = None) -> str: if connection: connected.add(connection) lines.append(f" {node} -. {connection} .-> {tool_node}") + elif "openapi" in tool: + count = len(tool["openapi"]["operations"]) + noun = "operation" if count == 1 else "operations" + lines.append( + f' {node} --> {tool_node}[("openapi
{count} {noun}")]' + ) + connection = tool["openapi"]["connection"] + connected.add(connection) + lines.append(f" {node} -. {connection} .-> {tool_node}") for name in agent.shared_credentials: # Marked as the bot it is: the diagram must not read as if these # tools acted with the caller's authorization. diff --git a/src/gete/openapi.py b/src/gete/openapi.py new file mode 100644 index 0000000..5bde86d --- /dev/null +++ b/src/gete/openapi.py @@ -0,0 +1,324 @@ +"""Reading an OpenAPI description: what an ``openapi:`` block selects from it. + +The description is read where the agent is packed and travels inside the +archive; it is never fetched at run time. A vendor changing a published +description must not silently change the tools an agent offers - operations +appearing, or an argument's meaning shifting, without anyone declaring it. + +Everything here is plain document walking, free of ADK, so validate can hold +a declaration against the description on machines that never deploy. +""" + +import re +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +import yaml + +from gete.errors import DeclarationError + +__all__ = ["Operation", "declaration_problems", "load_spec", "read_operations"] + +# The keys of a path item that name operations (RFC 9110 methods, lowercase). +HTTP_METHODS = ("get", "put", "post", "delete", "options", "head", "patch", "trace") + +# The verbs the connection client offers. DELETE is deliberately not one of +# them; the client's docstring holds the reason. +SUPPORTED_METHODS = frozenset({"get", "post", "put", "patch"}) + +# Methods that always change state. POST is not among them: search endpoints +# commonly take one, and the declaration's effect is where the difference is +# said. +WRITE_METHODS = frozenset({"put", "patch"}) + +# What a Gemini function may be called; the operationId becomes the tool name. +TOOL_NAME = re.compile(r"[A-Za-z_][A-Za-z0-9_-]{0,63}") + + +class _SpecLoader(yaml.SafeLoader): + """SafeLoader that survives YAML 1.1 leftovers published specs carry. + + A ``=`` mapping key is typed as ``tag:yaml.org,2002:value``, which + SafeLoader has no constructor for; real vendors publish specs that use + it. It stands for the literal string, so that is what it becomes. + """ + + +def _construct_value(loader: _SpecLoader, node: yaml.Node) -> Any: + return loader.construct_yaml_str(node) + + +_SpecLoader.add_constructor("tag:yaml.org,2002:value", _construct_value) + + +def load_spec(path: Path) -> Any: + """One OpenAPI document from a YAML or JSON file. + + JSON parses as YAML, so one loader reads both serializations. + """ + try: + text = path.read_text(encoding="utf-8") + except OSError as error: + raise DeclarationError(f"{path} could not be read: {error}") from error + try: + return yaml.load(text, Loader=_SpecLoader) # noqa: S506 - SafeLoader subclass + except yaml.YAMLError as error: + raise DeclarationError( + f"{path} could not be parsed as an OpenAPI description: {error}" + ) from error + + +@dataclass(frozen=True) +class Operation: + """One operation as the rules need it: located, with references resolved.""" + + id: str + path: str + method: str + document: Mapping[str, Any] + # Path-level and operation-level parameters merged, each resolved. + parameters: tuple[Mapping[str, Any], ...] + # The JSON request body schema, resolved one level, or None. + body: Mapping[str, Any] | None + # The media type the body was found under, or None. + body_media: str | None + + +def _resolve(spec: Any, node: Any) -> Any: + """Follow local $ref pointers until a plain node is reached. + + Only ``#/...`` pointers: an external reference would reach outside the + file the archive holds. A pointer that cannot be followed, or that loops, + resolves to nothing rather than raising - the rules report what is + missing in their own words. + """ + seen: set[str] = set() + while isinstance(node, Mapping) and isinstance(node.get("$ref"), str): + reference: str = node["$ref"] + if not reference.startswith("#/") or reference in seen: + return None + seen.add(reference) + node = spec + for part in reference[2:].split("/"): + name = part.replace("~1", "/").replace("~0", "~") + if not isinstance(node, Mapping) or name not in node: + return None + node = node[name] + return node + + +def _resolved_parameters( + spec: Any, path_item: Mapping[str, Any], operation: Mapping[str, Any] +) -> tuple[Mapping[str, Any], ...]: + """The operation's parameters: path-level first, resolved, mappings only.""" + merged: list[Mapping[str, Any]] = [] + for owner in (path_item, operation): + for entry in owner.get("parameters", ()): + resolved = _resolve(spec, entry) + if isinstance(resolved, Mapping) and "name" in resolved: + merged.append(resolved) + return tuple(merged) + + +def _resolved_body( + spec: Any, operation: Mapping[str, Any] +) -> tuple[Mapping[str, Any] | None, str | None]: + """The JSON request body schema and its media type, or (None, None). + + Only JSON is looked for; other bodies are reported by the rules, not + silently sent as JSON. allOf is folded one level so a schema written as + a composition still shows its properties. + """ + request_body = _resolve(spec, operation.get("requestBody")) + if not isinstance(request_body, Mapping): + return None, None + content = request_body.get("content") + if not isinstance(content, Mapping) or not content: + return None, None + for media, entry in content.items(): + if media == "application/json" or media.endswith("+json"): + schema = None + if isinstance(entry, Mapping): + schema = _resolve(spec, entry.get("schema")) + return (schema if isinstance(schema, Mapping) else {}), media + # No JSON among the media types; the first one names what was found. + return None, next(iter(content)) + + +def read_operations(spec: Any) -> tuple[dict[str, Operation], list[str]]: + """Every operation with an operationId, and the ids that appear twice. + + A duplicated id cannot select one operation, so the caller reports it + instead of quietly taking either. + """ + operations: dict[str, Operation] = {} + duplicates: list[str] = [] + paths = spec.get("paths") if isinstance(spec, Mapping) else None + if not isinstance(paths, Mapping): + raise DeclarationError( + "the document has no paths; is it an OpenAPI description?" + ) + for path, item in paths.items(): + path_item = _resolve(spec, item) + if not isinstance(path_item, Mapping): + continue + for method in HTTP_METHODS: + operation = path_item.get(method) + if not isinstance(operation, Mapping): + continue + operation_id = operation.get("operationId") + if not isinstance(operation_id, str) or not operation_id: + continue + if operation_id in operations: + if operation_id not in duplicates: + duplicates.append(operation_id) + continue + body, body_media = _resolved_body(spec, operation) + operations[operation_id] = Operation( + id=operation_id, + path=str(path), + method=method, + document=operation, + parameters=_resolved_parameters(spec, path_item, operation), + body=body, + body_media=body_media, + ) + return operations, duplicates + + +def declaration_problems(block: Mapping[str, Any], spec: Any) -> list[str]: + """Hold one ``openapi:`` block against the description it selects from. + + The block has passed the schema; these are the rules the schema cannot + see. Returned messages carry no prefix; validate adds where they were + found. + """ + try: + operations, duplicates = read_operations(spec) + except DeclarationError as error: + return [str(error)] + found: list[str] = [] + selected: tuple[str, ...] = tuple(block.get("operations", ())) + effect = str(block.get("effect", "write")) + for name in selected: + if name in duplicates: + found.append( + f"operations: {name!r} appears more than once in the " + "description; it cannot select one operation" + ) + continue + operation = operations.get(name) + if operation is None: + found.append( + f"operations: {name!r} is not an operationId in the description" + ) + continue + found.extend(_operation_problems(operation, effect)) + for key in ("params", "describe"): + for name in block.get(key, {}): + if name not in selected: + found.append(f"{key}: {name!r} is not one of this block's operations") + for name, fixes in block.get("params", {}).items(): + operation = operations.get(name) + if operation is not None and name in selected: + found.extend(_fix_problems(operation, fixes)) + return found + + +def _operation_problems(operation: Operation, effect: str) -> list[str]: + """What keeps one selected operation from becoming a tool.""" + found: list[str] = [] + name = operation.id + if not TOOL_NAME.fullmatch(name): + found.append( + f"operations: {name!r} cannot name a tool; letters, digits, " + "underscores, and hyphens only" + ) + if operation.method == "delete": + found.append( + f"operations: {name!r} is a DELETE, which gete does not send: " + "a deletion cannot be read back or corrected afterwards, so it " + "stays outside what a declaration can reach" + ) + return found + if operation.method not in SUPPORTED_METHODS: + found.append( + f"operations: {name!r} is {operation.method.upper()}, " + "which is not one of GET, POST, PUT, PATCH" + ) + return found + if operation.method in WRITE_METHODS and effect != "write": + found.append( + f"operations: {name!r} is a {operation.method.upper()}, which " + "changes state; declare it in a block with effect: write" + ) + for parameter in operation.parameters: + where = parameter.get("in") + if where in ("header", "cookie") and parameter.get("required"): + found.append( + f"operations: {name!r} requires {where} parameter " + f"{parameter.get('name')!r}, which a declaration does not send" + ) + if operation.body_media is not None and operation.body is None: + found.append( + f"operations: {name!r} takes a {operation.body_media} body; " + "only a JSON body can be declared" + ) + elif operation.body is not None: + if operation.method == "get": + found.append( + f"operations: {name!r} is a GET with a request body, " + "which is not supported" + ) + body_type = operation.body.get("type") + if body_type is not None and body_type != "object": + found.append( + f"operations: {name!r} has a {body_type} JSON body; " + "only an object body can be declared" + ) + return found + + +def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: + """What keeps the declared parameter fixes from being applied.""" + found: list[str] = [] + schemas: dict[str, Any] = {} + header_names: set[str] = set() + for parameter in operation.parameters: + name = str(parameter.get("name")) + if parameter.get("in") in ("query", "path"): + schemas[name] = parameter.get("schema") + else: + header_names.add(name) + properties = (operation.body or {}).get("properties") + enumerable = operation.body is None or isinstance(properties, Mapping) + if isinstance(properties, Mapping): + for name, schema in properties.items(): + schemas.setdefault(str(name), schema) + for name, fix in fixes.items(): + if name in header_names: + found.append( + f"params.{operation.id}: {name!r} is a header parameter, " + "which a declaration does not send" + ) + continue + if name not in schemas: + # A body whose properties cannot be enumerated may still hold + # the name; only a miss that is certain is reported. + if enumerable: + found.append( + f"params.{operation.id}: {name!r} names no parameter of " + f"{operation.id} (parameters: {', '.join(sorted(schemas))})" + ) + continue + if "prefix" in fix or "suffix" in fix: + schema = schemas[name] + declared = schema.get("type") if isinstance(schema, Mapping) else None + if declared is not None and declared != "string": + found.append( + f"params.{operation.id}.{name}: prefix and suffix need a " + f"string parameter, and {name!r} is {declared}" + ) + return found diff --git a/src/gete/policies.py b/src/gete/policies.py index fe5cc81..6cc3cd1 100644 --- a/src/gete/policies.py +++ b/src/gete/policies.py @@ -103,10 +103,13 @@ def load_policies(paths: Iterable[Path]) -> list[Policy]: def tool_effect(tool: Mapping[str, Any]) -> str: - """read or write. Only mcp and python tools can declare read; builtin is write.""" + """read or write. mcp, openapi, and python can declare read; builtin is write.""" if "mcp" in tool: effect: str = tool["mcp"].get("effect", DEFAULT_EFFECT) return effect + if "openapi" in tool: + openapi_effect: str = tool["openapi"].get("effect", DEFAULT_EFFECT) + return openapi_effect if "python" in tool: spec = tool["python"] if isinstance(spec, Mapping): diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py new file mode 100644 index 0000000..c2ae2f5 --- /dev/null +++ b/src/gete/runtime/openapi.py @@ -0,0 +1,326 @@ +"""OpenAPI tools: operations selected from a description, sent through gete's client. + +The description travelled inside the archive; nothing is fetched when the +agent starts. The published ``servers`` are never read - a definition's own +root may hold variables, a stale default, or another tenant - so request +URLs are built from the connection's ``base_url``, and the client's +destination check and token rules hold exactly as for every other request. + +ADK's spec parser turns the pruned description into declarations the model +can call. Execution stays here: ADK's own OpenAPI tools carry requests and +credentials themselves, outside the connection's guards. +""" + +import urllib.parse +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from typing import Any + +from google.adk.tools.base_tool import BaseTool +from google.adk.tools.base_toolset import BaseToolset +from google.adk.tools.openapi_tool.openapi_spec_parser.openapi_spec_parser import ( + OpenApiSpecParser, +) +from google.adk.tools.openapi_tool.openapi_spec_parser.rest_api_tool import ( + RestApiTool, +) + +from gete.connection.client import shared_client +from gete.connection.registry import Connection, Registry +from gete.connection.runtime import usable_token +from gete.declaration import Agent +from gete.errors import DeclarationError, GeteError +from gete.openapi import declaration_problems, load_spec, read_operations + + +@dataclass(frozen=True) +class Argument: + """How one request argument is produced: by the model, or declared. + + A fixed argument carries the declared value and is not offered to the + model at all. prefix and suffix wrap what the model wrote; they are how + a declaration keeps a constraint the code it replaces used to enforce. + """ + + name: str + py_name: str + location: str + fixed: bool = False + value: Any = None + prefix: str = "" + suffix: str = "" + + +class OpenApiTool(BaseTool): + """One operation as a tool, run with the caller's token.""" + + def __init__( + self, + *, + name: str, + description: str, + method: str, + path: str, + connection: Connection, + arguments: Iterable[Argument], + declaration: Any, + require_confirmation: bool, + ) -> None: + super().__init__(name=name, description=description) + self._method = method + self._path = path + self._connection = connection + self._arguments = tuple(arguments) + self._declaration = declaration + self._require_confirmation = require_confirmation + + def _get_declaration(self) -> Any: + return self._declaration + + async def check_require_confirmation( + self, args: dict[str, Any], tool_context: Any + ) -> bool: + return self._require_confirmation + + async def run_async(self, *, args: dict[str, Any], tool_context: Any) -> Any: + root = self._connection.base_url + if root is None: + # validate refuses this; guarded again because the archive that + # reached the runtime is whatever was packed. + raise GeteError( + f"connection {self._connection.id} declares no base_url; " + "there is no root to build the request URL from" + ) + path = self._path + query: dict[str, Any] = {} + body: dict[str, Any] = {} + for argument in self._arguments: + value = argument.value if argument.fixed else args.get(argument.py_name) + if value is None: + continue + if argument.prefix or argument.suffix: + value = f"{argument.prefix}{value}{argument.suffix}" + if argument.location == "path": + # Quoted with no safe characters, so a value cannot climb + # out of its segment and address another route. + path = path.replace( + "{" + argument.name + "}", + urllib.parse.quote(str(value), safe=""), + ) + elif argument.location == "query": + query[argument.name] = value + else: + body[argument.name] = value + if "{" in path: + raise GeteError(f"{self.name}: a path parameter was not given") + url = root.rstrip("/") + path + client = shared_client(self._connection.id) + state = getattr(tool_context, "state", None) + if self._method == "get": + return await client.get_json(url, params=query or None, state=state) + send = { + "post": client.post_json, + "put": client.put_json, + "patch": client.patch_json, + }[self._method] + return await send(url, body or None, params=query or None, state=state) + + +class OpenApiToolset(BaseToolset): + """Offers the block's tools, and only to a caller with a usable token. + + Without one the service is not spoken for at all: nothing is offered, + and the way back to authorizing belongs to the connection's + reauthorization tool, offered by the agent. + """ + + def __init__( + self, + *, + tools: Iterable[OpenApiTool], + connection: Connection, + authorization_key: str, + ) -> None: + super().__init__() + self._tools = tuple(tools) + self._connection = connection + self._key = authorization_key + + @property + def connection(self) -> Connection: + return self._connection + + @property + def connection_id(self) -> str: + return self._connection.id + + async def get_tools(self, readonly_context: Any = None) -> list[Any]: + # No context at all counts as no token: the Agent Card is built that + # way, and it must not promise what an unauthorized user cannot call. + state = getattr(readonly_context, "state", None) + if usable_token(self._connection, self._key, state) is None: + return [] + return list(self._tools) + + async def close(self) -> None: + """Nothing is held open; requests go through the shared client.""" + + +def openapi_toolset( + spec: Mapping[str, Any], + *, + agent: Agent, + authorizations: Mapping[str, str], + registry: Registry, + confirm: bool, + confirm_names: Iterable[str] = (), + denied: Iterable[str] = (), +) -> OpenApiToolset: + """Build the toolset for one ``openapi:`` entry of a resolved declaration.""" + connection_id = str(spec["connection"]) + connection = registry.get(connection_id) + document = load_spec(agent.directory / str(spec["spec"])) + found = declaration_problems(spec, document) + if found: + raise DeclarationError("openapi: " + "; ".join(found)) + operations, _ = read_operations(document) + selected = [operations[str(name)] for name in spec["operations"]] + parsed_by_id: dict[str, Any] = {} + for entry in OpenApiSpecParser().parse(_pruned(document, selected)): + operation_id = entry.operation.operationId + if operation_id: + parsed_by_id[operation_id] = entry + confirmed = frozenset(confirm_names) + excluded = frozenset(denied) + describe: Mapping[str, str] = spec.get("describe", {}) + does_not: str | None = spec.get("does_not") + tools: list[OpenApiTool] = [] + for name in spec["operations"]: + if name in excluded: + continue + parsed = parsed_by_id.get(name) + if parsed is None: + # read_operations saw it, the parser did not; something in the + # description defeats the parser, and silence would offer less + # than what was declared. + raise DeclarationError( + f"openapi: {name!r} was not parsed from the description" + ) + tools.append( + _tool( + parsed, + name=str(name), + fixes=spec.get("params", {}).get(name, {}), + description=describe.get(name), + does_not=does_not, + connection=connection, + require_confirmation=confirm or name in confirmed, + ) + ) + return OpenApiToolset( + tools=tools, + connection=connection, + authorization_key=authorizations.get(connection_id, connection_id), + ) + + +def _pruned(spec: Mapping[str, Any], selected: Iterable[Any]) -> dict[str, Any]: + """The description reduced to the selected operations. + + The parser resolves references itself, so components ride along whole. + Security schemes are left out: authorization is the client's, never the + parser's. + """ + paths: dict[str, dict[str, Any]] = {} + for operation in selected: + item = spec["paths"][operation.path] + entry = paths.setdefault(operation.path, {}) + if operation.method in item: + if "parameters" in item: + entry["parameters"] = item["parameters"] + entry[operation.method] = item[operation.method] + else: + # The path item hides behind a reference; take it whole and let + # the selection below pick the operations out of the parse. + paths[operation.path] = dict(item) + return { + "openapi": str(spec.get("openapi", "3.0.0")), + "info": dict(spec.get("info") or {"title": "", "version": ""}), + "paths": paths, + "components": dict(spec.get("components") or {}), + } + + +def _tool( + parsed: Any, + *, + name: str, + fixes: Mapping[str, Mapping[str, Any]], + description: str | None, + does_not: str | None, + connection: Connection, + require_confirmation: bool, +) -> OpenApiTool: + """One tool: the declaration without the fixed parameters, and the + arguments that rebuild the request from what the model writes.""" + arguments: list[Argument] = [] + visible: list[Any] = [] + applied: set[str] = set() + for parameter in parsed.parameters: + if parameter.param_location in ("header", "cookie"): + # Never model-driven; validate refused the required ones. + continue + fix = fixes.get(parameter.original_name, {}) + if fix: + applied.add(parameter.original_name) + if "value" in fix: + arguments.append( + Argument( + name=parameter.original_name, + py_name=parameter.py_name, + location=parameter.param_location, + fixed=True, + value=fix["value"], + ) + ) + continue + arguments.append( + Argument( + name=parameter.original_name, + py_name=parameter.py_name, + location=parameter.param_location, + prefix=str(fix.get("prefix", "")), + suffix=str(fix.get("suffix", "")), + ) + ) + visible.append(parameter) + unapplied = sorted(set(fixes) - applied) + if unapplied: + # validate could not see into this body; failing here is still + # better than a constraint that silently never held. + raise DeclarationError( + f"openapi: params.{name}: {', '.join(map(repr, unapplied))} " + "name no parameter the description declares" + ) + # The declaration is built from the visible parameters only. ADK reads + # them from the parsed operation, so the list is narrowed before the + # tool is derived from it. + parsed.parameters = visible + rest = RestApiTool.from_parsed_operation(parsed) + declaration = rest._get_declaration() # noqa: SLF001 - ADK offers no public way + operation = parsed.operation + text = description or operation.description or operation.summary or "" + if does_not: + text = f"{text}\n\nDoes not: {does_not}".strip() + declaration.name = name + declaration.description = text + return OpenApiTool( + name=name, + description=text, + method=str(parsed.endpoint.method).lower(), + path=str(parsed.endpoint.path), + connection=connection, + arguments=arguments, + declaration=declaration, + require_confirmation=require_confirmation, + ) diff --git a/src/gete/runtime/tools.py b/src/gete/runtime/tools.py index e79a3da..0e7e088 100644 --- a/src/gete/runtime/tools.py +++ b/src/gete/runtime/tools.py @@ -10,6 +10,7 @@ from gete.errors import DeclarationError from gete.policies import Policy, tool_effect from gete.runtime.mcp import mcp_toolset +from gete.runtime.openapi import openapi_toolset from gete.runtime.reauthorization import reauthorization_toolset from gete.shared_credentials import SHARED_CREDENTIALS @@ -88,6 +89,18 @@ def build_tools( # may back several toolsets, and each would offer the same # function under the same name. authorized.append(toolset.connection) + elif "openapi" in tool: + openapi = openapi_toolset( + tool["openapi"], + agent=agent, + authorizations=authorizations, + registry=registry, + confirm=confirm.for_effect(effect), + confirm_names=confirm.names, + denied=denied, + ) + tools.append(openapi) + authorized.append(openapi.connection) # Shared credential tools carry their effects with them; the write among # them is confirmed and denied like any declared write tool. for name in agent.shared_credentials: diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index c838214..2872570 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -217,6 +217,72 @@ } } }, + { + "type": "object", + "additionalProperties": false, + "required": [ + "openapi" + ], + "properties": { + "openapi": { + "type": "object", + "additionalProperties": false, + "required": [ + "spec", + "connection", + "operations" + ], + "properties": { + "spec": { + "description": "Path to the OpenAPI description, relative to agent.yaml. It is read where the agent is packed and travels inside the archive; nothing is fetched at run time.", + "type": "string", + "minLength": 1 + }, + "connection": { + "description": "Request URLs are built from this connection's base_url, and the caller's token rides along.", + "type": "string", + "pattern": "^[a-z][a-z0-9-]*$" + }, + "operations": { + "description": "operationIds to offer, out of everything the description holds. Required, never defaulted: a published description holds far more than an agent means to expose, and forgetting to choose must not mean offering everything.", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + }, + "effect": { + "$ref": "#/$defs/effect" + }, + "does_not": { + "type": "string", + "minLength": 1 + }, + "params": { + "description": "Constraints on the operations' parameters, by operationId, then by parameter or body property name. What was pinned in code must not be lost by moving to a declaration.", + "type": "object", + "additionalProperties": { + "type": "object", + "minProperties": 1, + "additionalProperties": { + "$ref": "#/$defs/openapi_param" + } + } + }, + "describe": { + "description": "Replaces an operation's description. The vendor's text is written for developers next to the docs, not for a model; it may cite links and syntax the model cannot follow.", + "type": "object", + "additionalProperties": { + "type": "string", + "minLength": 1 + } + } + } + } + } + }, { "type": "object", "additionalProperties": false, @@ -253,6 +319,36 @@ "https_url": { "type": "string", "pattern": "^https://[^/?#\\s]+(?:[/?#]\\S*)?$" + }, + "openapi_param": { + "description": "One parameter's constraint: a fixed value the model never sees, or text put around what the model writes. A fixed parameter is taken out of the declaration shown to the model - its value is declared, so there is nothing left for the model to say, and prefix or suffix would have nothing to wrap.", + "oneOf": [ + { + "type": "object", + "additionalProperties": false, + "required": [ + "value" + ], + "properties": { + "value": {} + } + }, + { + "type": "object", + "additionalProperties": false, + "minProperties": 1, + "properties": { + "prefix": { + "type": "string", + "minLength": 1 + }, + "suffix": { + "type": "string", + "minLength": 1 + } + } + } + ] } } } diff --git a/src/gete/validate.py b/src/gete/validate.py index f675b94..85b4ba5 100644 --- a/src/gete/validate.py +++ b/src/gete/validate.py @@ -10,6 +10,7 @@ from gete.connection.registry import missing_base_url from gete.declaration import Agent, Problem, Project from gete.errors import DeclarationError, GeteError +from gete.openapi import declaration_problems, load_spec from gete.policies import duplicate_policy_names from gete.schema import problems as schema_problems from gete.shared_credentials import SHARED_CREDENTIALS @@ -192,6 +193,8 @@ def _tool_problems( return _python_ref_problems(project, agent, ref) if "mcp" in tool: return _mcp_problems(tool["mcp"], registry, known) + if "openapi" in tool: + return _openapi_problems(project, agent, tool["openapi"], registry, known) return [] @@ -225,6 +228,43 @@ def _python_ref_problems(project: Project, agent: Agent, ref: str) -> list[str]: return [] +def _openapi_problems( + project: Project, + agent: Agent, + spec: Mapping[str, Any], + registry: Registry, + known: set[str], +) -> list[str]: + """The openapi rules: the connection gives a root, and the description + must actually hold what the declaration selects from it.""" + connection_id: str = spec["connection"] + if connection_id not in known: + return [ + f"openapi: connection {connection_id!r} is not in this agent's " + "connections, so no token would be available for it" + ] + connection = registry.get(connection_id) + found: list[str] = [] + if connection.base_url is None and not connection.needs_base_url: + # An open root is already refused at the agent level; this catches a + # rooted connection that never declared one, such as a catalog entry + # whose host list was written directly. + found.append( + f"openapi: connection {connection_id!r} declares no base_url, and " + "request URLs are built from it. Set " + f"connections.{connection_id}.base_url in gete.yaml" + ) + spec_path = agent.directory / str(spec["spec"]) + try: + document = load_spec(spec_path) + except DeclarationError as error: + return [*found, f"openapi: {error}"] + found.extend( + f"openapi: {message}" for message in declaration_problems(spec, document) + ) + return found + + def _mcp_problems( mcp: Mapping[str, Any], registry: Registry, known: set[str] ) -> list[str]: diff --git a/tests/test_archive.py b/tests/test_archive.py index 0a08e0f..ddfeeaa 100644 --- a/tests/test_archive.py +++ b/tests/test_archive.py @@ -334,3 +334,76 @@ def test_this_agents_own_problem_still_blocks_it(project: ProjectBuilder) -> Non prepare(project, name="mail", connections=["salesforce"]) with pytest.raises(DeclarationError, match="salesforce"): build_archive(project.root / "agents" / "mail") + + +OPENAPI_SPEC = """ +openapi: 3.0.0 +info: {title: Example, version: "1.0"} +paths: + /things: + get: + operationId: ListThings + responses: {"200": {description: ok}} +""" + +ROOTED_API = { + "display_name": "Rooted API", + "hosts": [], + "token_prefixes": ["rt_"], + "base_url": "https://acme.example.com", + "oauth": { + "authorization_url": "https://acme.example.com/oauth/authorize", + "token_url": "https://acme.example.com/oauth/token", + "scopes": {"read": "Read data"}, + }, +} + + +def prepare_openapi(project: ProjectBuilder, spec: str) -> Path: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": {"rooted-api": ROOTED_API}, + } + ) + directory = project.write_agent( + "mail-triage", + { + "connections": ["rooted-api"], + "tools": [ + { + "openapi": { + "spec": spec, + "connection": "rooted-api", + "operations": ["ListThings"], + "effect": "read", + } + } + ], + }, + ) + return directory + + +def test_an_openapi_description_travels_in_the_archive( + project: ProjectBuilder, +) -> None: + """The runtime reads the description from the archive, never the network.""" + directory = prepare_openapi(project, "./specs/service.yaml") + (directory / "specs").mkdir() + (directory / "specs" / "service.yaml").write_text(OPENAPI_SPEC) + files = members(build_archive(directory)) + assert files["specs/service.yaml"] == OPENAPI_SPEC.encode() + resolved = yaml.safe_load(files[RESOLVED_FILE]) + assert resolved["tools"][0]["openapi"]["spec"] == "./specs/service.yaml" + + +def test_an_openapi_description_outside_the_agent_directory_is_refused( + project: ProjectBuilder, +) -> None: + directory = prepare_openapi(project, "../shared.yaml") + (project.agents_dir / "shared.yaml").write_text(OPENAPI_SPEC) + with pytest.raises(DeclarationError, match="outside"): + build_archive(directory) diff --git a/tests/test_graph.py b/tests/test_graph.py index f41a117..8e9ea0e 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -111,3 +111,25 @@ def test_shared_credential_tools_appear(project: ProjectBuilder) -> None: text = graph(project) assert "slack_post" in text assert "bot" in text + + +def test_openapi_tools_appear_with_their_connection(project: ProjectBuilder) -> None: + project.write_agent( + "desk", + { + "connections": ["freee"], + "tools": [ + { + "openapi": { + "spec": "./specs/service.yaml", + "connection": "freee", + "operations": ["ListThings", "ShowThing"], + "effect": "read", + } + } + ], + }, + ) + text = graph(project) + assert 'desk --> desk_tool_0[("openapi
2 operations")]' in text + assert "desk -. freee .-> desk_tool_0" in text diff --git a/tests/test_openapi.py b/tests/test_openapi.py new file mode 100644 index 0000000..db4901a --- /dev/null +++ b/tests/test_openapi.py @@ -0,0 +1,317 @@ +"""Reading an OpenAPI description and holding a declaration against it.""" + +import json +from pathlib import Path +from typing import Any + +import pytest + +from gete.errors import DeclarationError +from gete.openapi import declaration_problems, load_spec, read_operations + +MINIMAL = """ +openapi: 3.0.0 +info: {title: Example, version: "1.0"} +paths: + /things: + get: + operationId: ListThings + responses: {"200": {description: ok}} +""" + + +def test_load_spec_reads_yaml(tmp_path: Path) -> None: + path = tmp_path / "spec.yaml" + path.write_text(MINIMAL) + spec = load_spec(path) + assert "ListThings" in json.dumps(spec) + + +def test_load_spec_reads_json_too(tmp_path: Path) -> None: + """Published descriptions come in either serialization.""" + path = tmp_path / "spec.json" + path.write_text( + json.dumps( + { + "openapi": "3.0.0", + "info": {"title": "Example", "version": "1.0"}, + "paths": {}, + } + ) + ) + assert load_spec(path)["openapi"] == "3.0.0" + + +def test_load_spec_survives_the_yaml_value_key(tmp_path: Path) -> None: + """Real vendors publish specs with a bare = key, which SafeLoader refuses.""" + path = tmp_path / "spec.yaml" + path.write_text("openapi: 3.0.0\npaths: {}\nx-legacy: {=: fallback}\n") + assert load_spec(path)["x-legacy"] == {"=": "fallback"} + + +def test_load_spec_says_which_file_could_not_be_parsed(tmp_path: Path) -> None: + path = tmp_path / "broken.yaml" + path.write_text("openapi: [unclosed") + with pytest.raises(DeclarationError, match="broken.yaml"): + load_spec(path) + + +def test_load_spec_says_when_the_file_is_missing(tmp_path: Path) -> None: + with pytest.raises(DeclarationError, match="nowhere.yaml"): + load_spec(tmp_path / "nowhere.yaml") + + +SPEC: dict[str, Any] = { + "openapi": "3.0.0", + "info": {"title": "Example", "version": "1.0"}, + # The published servers cannot be trusted; nothing below reads them. + "servers": [{"url": "https://{tenant}.example.com"}], + "paths": { + "/search": { + "get": { + "operationId": "ListSearchResults", + "description": "Vendor text.", + "parameters": [ + {"$ref": "#/components/parameters/Query"}, + {"name": "per_page", "in": "query", "schema": {"type": "integer"}}, + ], + "responses": {"200": {"description": "ok"}}, + } + }, + "/tickets/{ticket_id}": { + "parameters": [ + { + "name": "ticket_id", + "in": "path", + "required": True, + "schema": {"type": "integer"}, + } + ], + "get": { + "operationId": "ShowTicket", + "responses": {"200": {"description": "ok"}}, + }, + "put": { + "operationId": "UpdateTicket", + "requestBody": { + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/TicketUpdate"} + } + } + }, + "responses": {"200": {"description": "ok"}}, + }, + "delete": { + "operationId": "DeleteTicket", + "responses": {"204": {"description": "gone"}}, + }, + }, + }, + "components": { + "parameters": { + "Query": { + "name": "query", + "in": "query", + "required": True, + "schema": {"type": "string"}, + } + }, + "schemas": { + "TicketUpdate": { + "type": "object", + "properties": { + "ticket": { + "type": "object", + "properties": {"status": {"type": "string"}}, + } + }, + } + }, + }, +} + + +def test_read_operations_indexes_by_operation_id() -> None: + operations, duplicates = read_operations(SPEC) + assert duplicates == [] + search = operations["ListSearchResults"] + assert (search.path, search.method) == ("/search", "get") + assert [p["name"] for p in search.parameters] == ["query", "per_page"] + + +def test_read_operations_resolves_parameter_references() -> None: + operations, _ = read_operations(SPEC) + query = operations["ListSearchResults"].parameters[0] + assert query["required"] is True + assert query["schema"]["type"] == "string" + + +def test_read_operations_merges_path_level_parameters() -> None: + """A parameter declared on the path applies to every operation under it.""" + operations, _ = read_operations(SPEC) + show = operations["ShowTicket"] + assert [p["name"] for p in show.parameters] == ["ticket_id"] + + +def test_read_operations_resolves_the_request_body_schema() -> None: + operations, _ = read_operations(SPEC) + update = operations["UpdateTicket"] + assert update.body is not None + assert "ticket" in update.body["properties"] + + +def test_read_operations_reports_a_duplicated_operation_id() -> None: + doubled = json.loads(json.dumps(SPEC)) + doubled["paths"]["/search"]["post"] = { + "operationId": "ListSearchResults", + "responses": {"200": {"description": "ok"}}, + } + _, duplicates = read_operations(doubled) + assert duplicates == ["ListSearchResults"] + + +def block(**overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "spec": "./spec.yaml", + "connection": "example", + "operations": ["ListSearchResults"], + "effect": "read", + } + return {**base, **overrides} + + +def test_a_sound_declaration_has_no_problems() -> None: + sound = block( + operations=["ListSearchResults", "ShowTicket"], + params={"ListSearchResults": {"query": {"prefix": "type:ticket "}}}, + describe={"ListSearchResults": "Search tickets."}, + ) + assert declaration_problems(sound, SPEC) == [] + + +def test_an_operation_the_description_does_not_hold_is_reported() -> None: + found = declaration_problems(block(operations=["NoSuchOperation"]), SPEC) + assert found == [ + "operations: 'NoSuchOperation' is not an operationId in the description" + ] + + +def test_a_duplicated_operation_id_cannot_be_selected() -> None: + doubled = json.loads(json.dumps(SPEC)) + doubled["paths"]["/search"]["post"] = { + "operationId": "ListSearchResults", + "responses": {"200": {"description": "ok"}}, + } + found = declaration_problems(block(), doubled) + assert len(found) == 1 + assert "more than once" in found[0] + + +def test_delete_operations_are_refused_with_the_reason() -> None: + found = declaration_problems(block(operations=["DeleteTicket"]), SPEC) + assert len(found) == 1 + assert "DELETE" in found[0] + + +def test_a_write_method_may_not_be_declared_as_a_read() -> None: + found = declaration_problems( + block(operations=["UpdateTicket"], effect="read"), SPEC + ) + assert len(found) == 1 + assert "effect: write" in found[0] + + +def test_a_write_method_passes_under_effect_write() -> None: + sound = block(operations=["UpdateTicket"], effect="write") + assert declaration_problems(sound, SPEC) == [] + + +def test_params_must_name_a_selected_operation() -> None: + found = declaration_problems( + block(params={"ShowTicket": {"ticket_id": {"value": 1}}}), SPEC + ) + assert found == ["params: 'ShowTicket' is not one of this block's operations"] + + +def test_describe_must_name_a_selected_operation() -> None: + found = declaration_problems(block(describe={"ShowTicket": "A ticket."}), SPEC) + assert found == ["describe: 'ShowTicket' is not one of this block's operations"] + + +def test_a_fix_naming_no_parameter_of_the_operation_is_reported() -> None: + found = declaration_problems( + block(params={"ListSearchResults": {"sort": {"value": "asc"}}}), SPEC + ) + assert len(found) == 1 + assert "sort" in found[0] + # The parameters that do exist are named, so the fix can be corrected. + assert "query" in found[0] and "per_page" in found[0] + + +def test_a_fix_may_name_a_body_property() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket": {"value": {"status": "open"}}}}, + ) + assert declaration_problems(sound, SPEC) == [] + + +def test_prefix_needs_a_string_parameter() -> None: + found = declaration_problems( + block(params={"ListSearchResults": {"per_page": {"prefix": "p"}}}), SPEC + ) + assert len(found) == 1 + assert "string" in found[0] + + +def test_a_fixed_value_needs_no_particular_type() -> None: + sound = block(params={"ListSearchResults": {"per_page": {"value": 25}}}) + assert declaration_problems(sound, SPEC) == [] + + +def test_a_required_header_parameter_cannot_be_declared() -> None: + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + { + "name": "X-Team", + "in": "header", + "required": True, + "schema": {"type": "string"}, + } + ) + found = declaration_problems(block(), spec) + assert len(found) == 1 + assert "header" in found[0] + + +def test_a_body_that_is_not_json_is_refused() -> None: + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/tickets/{ticket_id}"]["put"]["requestBody"]["content"] = { + "application/x-www-form-urlencoded": {"schema": {"type": "object"}} + } + found = declaration_problems( + block(operations=["UpdateTicket"], effect="write"), spec + ) + assert len(found) == 1 + assert "JSON" in found[0] + + +def test_a_get_with_a_request_body_is_refused() -> None: + """The client sends no body on a GET; the operation would lose arguments.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["requestBody"] = { + "content": {"application/json": {"schema": {"type": "object"}}} + } + found = declaration_problems(block(), spec) + assert len(found) == 1 + assert "GET with a request body" in found[0] + + +def test_an_operation_id_that_cannot_name_a_tool_is_reported() -> None: + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["operationId"] = "list search results" + found = declaration_problems(block(operations=["list search results"]), spec) + assert len(found) == 1 + assert "name" in found[0] diff --git a/tests/test_policies.py b/tests/test_policies.py index ce44890..8d3876e 100644 --- a/tests/test_policies.py +++ b/tests/test_policies.py @@ -66,6 +66,23 @@ def test_policy_without_prefix_leaves_the_instruction_alone() -> None: [{"python": {"ref": "pkg.a:R", "effect": "read"}}, {"python": "pkg.b:W"}], True, ), + ( + [{"openapi": {"spec": "./s.yaml", "connection": "c", "operations": ["A"]}}], + True, + ), + ( + [ + { + "openapi": { + "spec": "./s.yaml", + "connection": "c", + "operations": ["A"], + "effect": "read", + } + } + ], + False, + ), ], ) def test_has_write_tools_treats_undeclared_effect_as_write( diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py new file mode 100644 index 0000000..94b1028 --- /dev/null +++ b/tests/test_runtime_openapi.py @@ -0,0 +1,390 @@ +"""OpenAPI tools: operations become tools, and requests go through gete's client.""" + +from pathlib import Path +from typing import Any + +import pytest +import yaml +from conftest import ProjectBuilder + +from gete.connection import Registry +from gete.declaration import RESOLVED_FILE, Agent, load_project, resolve +from gete.errors import DeclarationError +from gete.request_context import clear_tool_call +from gete.runtime import build +from gete.runtime.openapi import OpenApiToolset, openapi_toolset +from gete.runtime.reauthorization import ReauthorizationToolset + +TOKEN = "rt_0123456789abcdef" + +ROOTED_API: dict[str, Any] = { + "id": "rooted-api", + "display_name": "Rooted API", + "hosts": [], + "token_prefixes": ["rt_"], + "base_url": "https://acme.example.com", + "oauth": { + "authorization_url": "https://acme.example.com/oauth/authorize", + "token_url": "https://acme.example.com/oauth/token", + "scopes": {"read": "Read data"}, + }, +} + +REGISTRY = Registry.from_documents({"rooted-api": ROOTED_API}) + +SPEC: dict[str, Any] = { + "openapi": "3.0.0", + "info": {"title": "Example", "version": "1.0"}, + # Untrustworthy on purpose: nothing may ever read the published servers. + "servers": [{"url": "https://{tenant}.evil.example"}], + "paths": { + "/search": { + "get": { + "operationId": "ListSearchResults", + "description": "Vendor text. See [Query syntax](#query-syntax).", + "parameters": [ + { + "name": "query", + "in": "query", + "required": True, + "schema": {"type": "string"}, + }, + {"name": "per_page", "in": "query", "schema": {"type": "integer"}}, + ], + "responses": {"200": {"description": "ok"}}, + } + }, + "/tickets/{ticket_id}": { + "parameters": [ + { + "name": "ticket_id", + "in": "path", + "required": True, + "schema": {"type": "string"}, + } + ], + "get": { + "operationId": "ShowTicket", + "responses": {"200": {"description": "ok"}}, + }, + "put": { + "operationId": "UpdateTicket", + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "ticket": { + "type": "object", + "properties": {"status": {"type": "string"}}, + } + }, + } + } + } + }, + "responses": {"200": {"description": "ok"}}, + }, + }, + }, +} + + +class Context: + """The part of ADK's ReadonlyContext / ToolContext the runtime reads.""" + + def __init__(self, state: dict[str, Any]) -> None: + self.state = state + self.user_id = "user" + + +def teardown_function() -> None: + clear_tool_call() + + +def agent_with_spec(tmp_path: Path) -> Agent: + (tmp_path / "spec.yaml").write_text(yaml.safe_dump(SPEC, sort_keys=False)) + return Agent(directory=tmp_path, data={"name": "mail-triage"}) + + +def toolset( + tmp_path: Path, + *, + operations: list[str] | None = None, + effect: str = "read", + params: dict[str, Any] | None = None, + describe: dict[str, str] | None = None, + does_not: str | None = None, + confirm: bool = False, + confirm_names: list[str] | None = None, + denied: list[str] | None = None, +) -> OpenApiToolset: + spec: dict[str, Any] = { + "spec": "./spec.yaml", + "connection": "rooted-api", + "operations": operations or ["ListSearchResults", "ShowTicket"], + "effect": effect, + } + if params: + spec["params"] = params + if describe: + spec["describe"] = describe + if does_not: + spec["does_not"] = does_not + return openapi_toolset( + spec, + agent=agent_with_spec(tmp_path), + authorizations={"rooted-api": "mail-triage-rooted-api"}, + registry=REGISTRY, + confirm=confirm, + confirm_names=confirm_names or (), + denied=denied or (), + ) + + +def context() -> Context: + return Context({"mail-triage-rooted-api": TOKEN}) + + +async def test_each_operation_becomes_a_tool_named_by_its_operation_id( + tmp_path: Path, +) -> None: + tools = await toolset(tmp_path).get_tools(context()) + assert [tool.name for tool in tools] == ["ListSearchResults", "ShowTicket"] + + +async def test_nothing_is_offered_without_a_usable_token(tmp_path: Path) -> None: + built = toolset(tmp_path) + assert await built.get_tools(Context({})) == [] + assert await built.get_tools(Context({"mail-triage-rooted-api": "ya29.x"})) == [] + # No context at all counts as no token; the Agent Card is built that way. + assert await built.get_tools(None) == [] + + +async def test_denied_tools_are_not_offered(tmp_path: Path) -> None: + built = toolset(tmp_path, denied=["ShowTicket"]) + tools = await built.get_tools(context()) + assert [tool.name for tool in tools] == ["ListSearchResults"] + + +async def test_a_fixed_parameter_is_not_in_the_declaration(tmp_path: Path) -> None: + """Its value is declared, so there is nothing for the model to say.""" + built = toolset( + tmp_path, + params={"ListSearchResults": {"per_page": {"value": 25}}}, + ) + search = (await built.get_tools(context()))[0] + declared = search._get_declaration().model_dump_json(exclude_none=True) + assert "query" in declared + assert "per_page" not in declared + + +async def test_describe_replaces_the_vendor_text_and_does_not_rides_along( + tmp_path: Path, +) -> None: + built = toolset( + tmp_path, + describe={"ListSearchResults": "Search tickets."}, + does_not="Does not read other kinds.", + ) + search, show = await built.get_tools(context()) + assert ( + search.description == "Search tickets.\n\nDoes not: Does not read other kinds." + ) + assert "Vendor text" not in search.description + # Without describe, the vendor's text stays, with the same rider. + assert show.description.endswith("Does not: Does not read other kinds.") + + +async def test_write_tools_ask_for_confirmation_when_told_to(tmp_path: Path) -> None: + built = toolset(tmp_path, operations=["UpdateTicket"], effect="write", confirm=True) + update = (await built.get_tools(context()))[0] + assert await update.check_require_confirmation({}, context()) is True + + +async def test_confirmation_can_name_a_single_tool(tmp_path: Path) -> None: + built = toolset(tmp_path, confirm_names=["ShowTicket"]) + search, show = await built.get_tools(context()) + assert await search.check_require_confirmation({}, context()) is False + assert await show.check_require_confirmation({}, context()) is True + + +def test_build_refuses_a_declaration_the_description_cannot_carry( + tmp_path: Path, +) -> None: + with pytest.raises(DeclarationError, match="Nope"): + toolset(tmp_path, operations=["Nope"]) + + +def test_a_fix_the_description_cannot_place_fails_at_build(tmp_path: Path) -> None: + """A constraint that silently never held would be worse than a failure.""" + with pytest.raises(DeclarationError, match="nested"): + toolset( + tmp_path, + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"nested": {"value": 1}}}, + ) + + +class RecordingClient: + """Stands in for the shared ConnectionClient and records every request.""" + + def __init__(self, answer: Any = None) -> None: + self.answer = answer if answer is not None else {"ok": True} + self.calls: list[dict[str, Any]] = [] + + async def get_json(self, url: str, params: Any = None, **kwargs: Any) -> Any: + return self._record("GET", url, params, None, kwargs) + + async def post_json( + self, url: str, body: Any = None, params: Any = None, **kwargs: Any + ) -> Any: + return self._record("POST", url, params, body, kwargs) + + async def put_json( + self, url: str, body: Any = None, params: Any = None, **kwargs: Any + ) -> Any: + return self._record("PUT", url, params, body, kwargs) + + async def patch_json( + self, url: str, body: Any = None, params: Any = None, **kwargs: Any + ) -> Any: + return self._record("PATCH", url, params, body, kwargs) + + def _record( + self, method: str, url: str, params: Any, body: Any, kwargs: Any + ) -> Any: + self.calls.append( + { + "method": method, + "url": url, + "params": params, + "body": body, + "state": kwargs.get("state"), + } + ) + return self.answer + + +@pytest.fixture +def client(monkeypatch: pytest.MonkeyPatch) -> RecordingClient: + recorder = RecordingClient() + monkeypatch.setattr( + "gete.runtime.openapi.shared_client", lambda connection_id: recorder + ) + return recorder + + +async def run(built: OpenApiToolset, name: str, args: dict[str, Any]) -> Any: + tool = next(t for t in await built.get_tools(context()) if t.name == name) + return await tool.run_async(args=args, tool_context=context()) + + +async def test_requests_go_under_the_connections_base_url_never_the_specs( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset(tmp_path) + result = await run(built, "ListSearchResults", {"query": "invoice"}) + assert result == {"ok": True} + call = client.calls[0] + assert call["method"] == "GET" + assert call["url"] == "https://acme.example.com/search" + assert call["params"] == {"query": "invoice"} + + +async def test_a_prefix_is_put_in_front_of_what_the_model_wrote( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset( + tmp_path, + params={"ListSearchResults": {"query": {"prefix": "type:ticket "}}}, + ) + await run(built, "ListSearchResults", {"query": "type:user urgent"}) + # The declared kind comes first, so the model's own type: does not win. + assert client.calls[0]["params"] == {"query": "type:ticket type:user urgent"} + + +async def test_a_fixed_value_rides_on_every_request( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset( + tmp_path, + params={"ListSearchResults": {"per_page": {"value": 25}}}, + ) + # The model cannot see per_page; even a value smuggled into the + # arguments is overridden by the declared one. + await run(built, "ListSearchResults", {"query": "x", "per_page": 100}) + assert client.calls[0]["params"] == {"query": "x", "per_page": 25} + + +async def test_a_path_parameter_cannot_climb_out_of_its_segment( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset(tmp_path) + await run(built, "ShowTicket", {"ticket_id": "1/../../admin"}) + assert client.calls[0]["url"] == ( + "https://acme.example.com/tickets/1%2F..%2F..%2Fadmin" + ) + + +async def test_a_put_operation_sends_the_json_body_with_the_put_verb( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset(tmp_path, operations=["UpdateTicket"], effect="write") + await run(built, "UpdateTicket", {"ticket_id": "7", "ticket": {"status": "solved"}}) + call = client.calls[0] + assert call["method"] == "PUT" + assert call["url"] == "https://acme.example.com/tickets/7" + assert call["body"] == {"ticket": {"status": "solved"}} + + +async def test_the_callers_state_reaches_the_client( + tmp_path: Path, client: RecordingClient +) -> None: + """The client takes the token from the state; without it every user of + the instance would share whatever call came first.""" + built = toolset(tmp_path) + await run(built, "ListSearchResults", {"query": "x"}) + assert client.calls[0]["state"] == {"mail-triage-rooted-api": TOKEN} + + +def test_build_turns_the_declaration_into_a_toolset(project: ProjectBuilder) -> None: + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "connections": { + "rooted-api": {k: v for k, v in ROOTED_API.items() if k != "id"} + }, + } + ) + directory = project.write_agent( + "mail-triage", + { + "connections": ["rooted-api"], + "tools": [ + { + "openapi": { + "spec": "./spec.yaml", + "connection": "rooted-api", + "operations": ["ListSearchResults"], + "effect": "read", + } + } + ], + }, + ) + (directory / "spec.yaml").write_text(yaml.safe_dump(SPEC, sort_keys=False)) + loaded = load_project(project.root / "gete.yaml") + resolved = resolve(loaded, loaded.agents[0]) + path = directory / RESOLVED_FILE + path.write_text(yaml.safe_dump(resolved, sort_keys=False)) + built, asking = build(path).tools + assert isinstance(built, OpenApiToolset) + assert built.connection_id == "rooted-api" + # The connection joins the reauthorization tool like an MCP one would. + assert isinstance(asking, ReauthorizationToolset) + assert asking.connection_ids == ("rooted-api",) diff --git a/tests/test_schema.py b/tests/test_schema.py index e72f799..c98a46e 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -40,6 +40,14 @@ MCP_TOOL: dict[str, Any] = {"mcp": {"url": "https://mcp.example.com/mcp"}} +OPENAPI_TOOL: dict[str, Any] = { + "openapi": { + "spec": "./specs/example.yaml", + "connection": "example", + "operations": ["ListThings"], + } +} + @pytest.mark.parametrize( ("kind", "document"), @@ -160,11 +168,23 @@ def test_connections_must_be_unique() -> None: {"mcp": {"url": "https://mcp.example.com/mcp", "allow": []}}, {"python": {"effect": "read"}}, {"python": "no_colon_here"}, + # openapi needs the spec, the connection, and a choice of operations. {"openapi": {"spec": "./spec.yaml"}}, + {"openapi": {**OPENAPI_TOOL["openapi"], "operations": []}}, + {"openapi": {**OPENAPI_TOOL["openapi"], "params": {"ListThings": {}}}}, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + # A fixed parameter is taken away from the model, so there is + # no value of the model's left to put a prefix in front of. + "params": {"ListThings": {"q": {"value": "x", "prefix": "y"}}}, + } + }, + {"openapi": {**OPENAPI_TOOL["openapi"], "params": {"ListThings": {"q": {}}}}}, ], ) def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: - """One key per tool, https for MCP, effect read or write; openapi is not in.""" + """One key per tool, https for MCP, effect read or write, whole openapi blocks.""" with pytest.raises(DeclarationError, match="tools"): validate_document("agent", {**AGENT, "tools": [tool]}, source="agent.yaml") @@ -187,6 +207,21 @@ def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: }, {"python": "my_agent.agent:TOOLS"}, {"python": {"ref": "my_agent.agent:TOOLS", "effect": "read"}}, + {**OPENAPI_TOOL}, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + "effect": "read", + "does_not": "Does not write anything.", + "params": { + "ListThings": { + "query": {"prefix": "type:thing ", "suffix": " sorted"}, + "per_page": {"value": 25}, + } + }, + "describe": {"ListThings": "List the things."}, + } + }, ], ) def test_supported_tool_shapes_pass(tool: dict[str, Any]) -> None: diff --git a/tests/test_validate.py b/tests/test_validate.py index f8e46cc..472f93e 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -439,3 +439,113 @@ def test_an_mcp_url_under_the_root_is_checked_against_the_filled_in_hosts( }, ) assert problems(project) == [] + + +# One operation is enough for the rules; gete.openapi's own tests cover the rest. +OPENAPI_SPEC = """ +openapi: 3.0.0 +info: {title: Example, version: "1.0"} +paths: + /things: + get: + operationId: ListThings + parameters: + - {name: query, in: query, schema: {type: string}} + responses: {"200": {description: ok}} +""" + + +def write_openapi_agent( + project: ProjectBuilder, + tool: dict[str, Any], + *, + connections: list[str] | None = None, + spec_text: str | None = OPENAPI_SPEC, +) -> None: + agent_dir = project.write_agent( + "mail-triage", + { + **({"connections": connections} if connections is not None else {}), + "tools": [{"openapi": tool}], + }, + ) + if spec_text is not None: + (agent_dir / "spec.yaml").write_text(spec_text) + + +def test_a_sound_openapi_declaration_passes(project: ProjectBuilder) -> None: + write_rooted_api(project, "https://acme.example.com") + write_openapi_agent( + project, + { + "spec": "./spec.yaml", + "connection": "rooted-api", + "operations": ["ListThings"], + "effect": "read", + "params": {"ListThings": {"query": {"prefix": "type:thing "}}}, + }, + connections=["rooted-api"], + ) + assert problems(project) == [] + + +def test_openapi_connection_must_be_declared_by_the_agent( + project: ProjectBuilder, +) -> None: + """Without the connection there is no token, and no root to send requests to.""" + write_rooted_api(project, "https://acme.example.com") + write_openapi_agent( + project, + { + "spec": "./spec.yaml", + "connection": "rooted-api", + "operations": ["ListThings"], + }, + ) + found = problems(project) + assert any("rooted-api" in p and "connections" in p for p in found), found + + +def test_openapi_connection_must_have_a_base_url(project: ProjectBuilder) -> None: + """internal-api names hosts but no root; there is nothing to build URLs from.""" + write_internal_api(project) + write_openapi_agent( + project, + { + "spec": "./spec.yaml", + "connection": "internal-api", + "operations": ["ListThings"], + }, + connections=["internal-api"], + ) + found = problems(project) + assert any("base_url" in p for p in found), found + + +def test_openapi_missing_spec_file_is_reported(project: ProjectBuilder) -> None: + write_rooted_api(project, "https://acme.example.com") + write_openapi_agent( + project, + { + "spec": "./spec.yaml", + "connection": "rooted-api", + "operations": ["ListThings"], + }, + connections=["rooted-api"], + spec_text=None, + ) + found = problems(project) + assert any("spec.yaml" in p for p in found), found + + +def test_openapi_operations_are_held_against_the_description( + project: ProjectBuilder, +) -> None: + write_rooted_api(project, "https://acme.example.com") + write_openapi_agent( + project, + {"spec": "./spec.yaml", "connection": "rooted-api", "operations": ["Nope"]}, + connections=["rooted-api"], + ) + found = problems(project) + assert any("Nope" in p for p in found), found From d4acea82945347eca7eaa3d3d2493a952ee183dd Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:00:23 +0900 Subject: [PATCH 2/4] Let a declaration select DELETE operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client offers DELETE now, so refusing it here would only push a deleting tool off the guarded path. A DELETE operation always changes state, so like PUT and PATCH it must sit in a block declared effect: write, where the confirmation policies see it. One with a request body is refused: the client sends none (RFC 9110 gives it no meaning), and the operation would lose arguments silently. 🤖 Generated with Claude Code --- README.md | 11 ++++++----- src/gete/openapi.py | 27 ++++++++++++--------------- src/gete/runtime/openapi.py | 4 ++++ tests/test_openapi.py | 26 +++++++++++++++++++++++--- tests/test_runtime_openapi.py | 18 ++++++++++++++++++ 5 files changed, 63 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 080eecd..51d71bf 100644 --- a/README.md +++ b/README.md @@ -185,11 +185,12 @@ tools: file into the archive and the runtime reads it from there, so a vendor editing their published description changes nothing until someone re-archives deliberately. -- **Writes ride the same rails.** PUT and PATCH operations must sit in a - block declared `effect: write`, which the confirmation policies key on, - and results pass the same redaction as every other tool. DELETE cannot be - declared at all — the client offers no delete verb, and the reason is - written where the verbs live. +- **Writes ride the same rails.** PUT, PATCH, and DELETE operations must + sit in a block declared `effect: write`, which the confirmation policies + key on, and results pass the same redaction as every other tool. A change + that may already have been applied is never resent — for a DELETE, what + it removed usually cannot be brought back, so a confirmation policy on + write tools is worth having before declaring one. ### Connections diff --git a/src/gete/openapi.py b/src/gete/openapi.py index 5bde86d..0785a65 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -24,14 +24,18 @@ # The keys of a path item that name operations (RFC 9110 methods, lowercase). HTTP_METHODS = ("get", "put", "post", "delete", "options", "head", "patch", "trace") -# The verbs the connection client offers. DELETE is deliberately not one of -# them; the client's docstring holds the reason. -SUPPORTED_METHODS = frozenset({"get", "post", "put", "patch"}) +# The verbs the connection client offers. +SUPPORTED_METHODS = frozenset({"get", "post", "put", "patch", "delete"}) # Methods that always change state. POST is not among them: search endpoints # commonly take one, and the declaration's effect is where the difference is # said. -WRITE_METHODS = frozenset({"put", "patch"}) +WRITE_METHODS = frozenset({"put", "patch", "delete"}) + +# Methods the client sends without a body. A GET's is dropped by convention, +# a DELETE's has no meaning (RFC 9110); an operation that needs one would +# lose arguments silently. +BODYLESS_METHODS = frozenset({"get", "delete"}) # What a Gemini function may be called; the operationId becomes the tool name. TOOL_NAME = re.compile(r"[A-Za-z_][A-Za-z0-9_-]{0,63}") @@ -236,17 +240,10 @@ def _operation_problems(operation: Operation, effect: str) -> list[str]: f"operations: {name!r} cannot name a tool; letters, digits, " "underscores, and hyphens only" ) - if operation.method == "delete": - found.append( - f"operations: {name!r} is a DELETE, which gete does not send: " - "a deletion cannot be read back or corrected afterwards, so it " - "stays outside what a declaration can reach" - ) - return found if operation.method not in SUPPORTED_METHODS: found.append( f"operations: {name!r} is {operation.method.upper()}, " - "which is not one of GET, POST, PUT, PATCH" + "which is not one of GET, POST, PUT, PATCH, DELETE" ) return found if operation.method in WRITE_METHODS and effect != "write": @@ -267,10 +264,10 @@ def _operation_problems(operation: Operation, effect: str) -> list[str]: "only a JSON body can be declared" ) elif operation.body is not None: - if operation.method == "get": + if operation.method in BODYLESS_METHODS: found.append( - f"operations: {name!r} is a GET with a request body, " - "which is not supported" + f"operations: {name!r} is a {operation.method.upper()} with " + "a request body, which is not supported" ) body_type = operation.body.get("type") if body_type is not None and body_type != "object": diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index c2ae2f5..b55fbf8 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -118,6 +118,10 @@ async def run_async(self, *, args: dict[str, Any], tool_context: Any) -> Any: state = getattr(tool_context, "state", None) if self._method == "get": return await client.get_json(url, params=query or None, state=state) + if self._method == "delete": + # No body: DELETE gives one no meaning, and validate refused any + # operation that declares one. + return await client.delete_json(url, params=query or None, state=state) send = { "post": client.post_json, "put": client.put_json, diff --git a/tests/test_openapi.py b/tests/test_openapi.py index db4901a..a67f978 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -208,10 +208,30 @@ def test_a_duplicated_operation_id_cannot_be_selected() -> None: assert "more than once" in found[0] -def test_delete_operations_are_refused_with_the_reason() -> None: - found = declaration_problems(block(operations=["DeleteTicket"]), SPEC) +def test_a_delete_may_be_declared_as_a_write() -> None: + sound = block(operations=["DeleteTicket"], effect="write") + assert declaration_problems(sound, SPEC) == [] + + +def test_a_delete_may_not_be_declared_as_a_read() -> None: + found = declaration_problems( + block(operations=["DeleteTicket"], effect="read"), SPEC + ) + assert len(found) == 1 + assert "effect: write" in found[0] + + +def test_a_delete_with_a_request_body_is_refused() -> None: + """The client sends no body on a DELETE; RFC 9110 gives one no meaning.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/tickets/{ticket_id}"]["delete"]["requestBody"] = { + "content": {"application/json": {"schema": {"type": "object"}}} + } + found = declaration_problems( + block(operations=["DeleteTicket"], effect="write"), spec + ) assert len(found) == 1 - assert "DELETE" in found[0] + assert "request body" in found[0] def test_a_write_method_may_not_be_declared_as_a_read() -> None: diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 94b1028..0b1ddd5 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -86,6 +86,10 @@ }, "responses": {"200": {"description": "ok"}}, }, + "delete": { + "operationId": "DeleteTicket", + "responses": {"204": {"description": "gone"}}, + }, }, }, } @@ -253,6 +257,9 @@ async def patch_json( ) -> Any: return self._record("PATCH", url, params, body, kwargs) + async def delete_json(self, url: str, params: Any = None, **kwargs: Any) -> Any: + return self._record("DELETE", url, params, None, kwargs) + def _record( self, method: str, url: str, params: Any, body: Any, kwargs: Any ) -> Any: @@ -340,6 +347,17 @@ async def test_a_put_operation_sends_the_json_body_with_the_put_verb( assert call["body"] == {"ticket": {"status": "solved"}} +async def test_a_delete_operation_uses_the_delete_verb( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset(tmp_path, operations=["DeleteTicket"], effect="write") + await run(built, "DeleteTicket", {"ticket_id": "7"}) + call = client.calls[0] + assert call["method"] == "DELETE" + assert call["url"] == "https://acme.example.com/tickets/7" + assert call["body"] is None + + async def test_the_callers_state_reaches_the_client( tmp_path: Path, client: RecordingClient ) -> None: From e2b701f81a32d21b04ab084fe1df061a2b899c4a Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:23:48 +0900 Subject: [PATCH 3/4] Hold a JSON body to what the parser can actually offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser only expands a body's properties into arguments when the schema spells type: object, and published descriptions often leave the type implicit or compose the body with allOf. Everything else fell to a single opaque body argument, and the request then carried the payload wrapped under a key the service never declared - silently, since validate was satisfied with any body that did not declare a non-object type. The description handed to the parser now carries each selected operation with its parameters resolved and its body schema folded and typed, so the properties always expand. allOf is folded one level, as _resolved_body's docstring already promised without the code keeping the promise. A body that still shows no properties is refused outright: there is nothing to offer the model, and nothing the rules could hold a fix against. A fix naming both a request parameter and a body property is refused for the same reason: the runtime applies fixes by name, and one name in two places would fix both. 🤖 Generated with Claude Code --- src/gete/openapi.py | 95 +++++++++++++++++++++++++++----- src/gete/runtime/openapi.py | 36 +++++++----- tests/test_openapi.py | 100 ++++++++++++++++++++++++++++++++++ tests/test_runtime_openapi.py | 59 +++++++++++++++++++- 4 files changed, 258 insertions(+), 32 deletions(-) diff --git a/src/gete/openapi.py b/src/gete/openapi.py index 0785a65..6eee8d9 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -126,6 +126,49 @@ def _resolved_parameters( return tuple(merged) +def _fold_allof(spec: Any, schema: Mapping[str, Any]) -> Mapping[str, Any]: + """One level of allOf folded into a plain schema. + + Published bodies are often a composition - a shared core plus the + operation's own fields - and without folding, neither the rules nor the + parser would see the composed properties. One level covers that shape; + deeper nesting stays unread, and a body it hides ends up refused for + declaring no properties rather than half-read. + """ + parts = schema.get("allOf") + if not isinstance(parts, list): + return schema + properties: dict[str, Any] = {} + required: list[str] = [] + folded_type = schema.get("type") + for part in parts: + resolved = _resolve(spec, part) + if not isinstance(resolved, Mapping): + continue + if folded_type is None: + folded_type = resolved.get("type") + part_properties = resolved.get("properties") + if isinstance(part_properties, Mapping): + properties.update(part_properties) + part_required = resolved.get("required") + if isinstance(part_required, list): + required.extend(name for name in part_required if name not in required) + own = schema.get("properties") + if isinstance(own, Mapping): + properties.update(own) + own_required = schema.get("required") + if isinstance(own_required, list): + required.extend(name for name in own_required if name not in required) + folded = {key: value for key, value in schema.items() if key != "allOf"} + if properties: + folded["properties"] = properties + if required: + folded["required"] = required + if folded_type is not None: + folded["type"] = folded_type + return folded + + def _resolved_body( spec: Any, operation: Mapping[str, Any] ) -> tuple[Mapping[str, Any] | None, str | None]: @@ -146,6 +189,8 @@ def _resolved_body( schema = None if isinstance(entry, Mapping): schema = _resolve(spec, entry.get("schema")) + if isinstance(schema, Mapping): + schema = _fold_allof(spec, schema) return (schema if isinstance(schema, Mapping) else {}), media # No JSON among the media types; the first one names what was found. return None, next(iter(content)) @@ -269,31 +314,43 @@ def _operation_problems(operation: Operation, effect: str) -> list[str]: f"operations: {name!r} is a {operation.method.upper()} with " "a request body, which is not supported" ) - body_type = operation.body.get("type") - if body_type is not None and body_type != "object": - found.append( - f"operations: {name!r} has a {body_type} JSON body; " - "only an object body can be declared" - ) + else: + body_type = operation.body.get("type") + properties = operation.body.get("properties") + if body_type is not None and body_type != "object": + found.append( + f"operations: {name!r} has a {body_type} JSON body; " + "only an object body can be declared" + ) + elif not (isinstance(properties, Mapping) and properties): + # The parser would offer the model one opaque body argument + # and the request would carry the payload wrapped under a + # key the service never declared. + found.append( + f"operations: {name!r} has a JSON body that declares no " + "properties; there is nothing to offer the model" + ) return found def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: """What keeps the declared parameter fixes from being applied.""" found: list[str] = [] - schemas: dict[str, Any] = {} + parameters: dict[str, Any] = {} header_names: set[str] = set() for parameter in operation.parameters: name = str(parameter.get("name")) if parameter.get("in") in ("query", "path"): - schemas[name] = parameter.get("schema") + parameters[name] = parameter.get("schema") else: header_names.add(name) properties = (operation.body or {}).get("properties") + body_properties: dict[str, Any] = ( + {str(name): schema for name, schema in properties.items()} + if isinstance(properties, Mapping) + else {} + ) enumerable = operation.body is None or isinstance(properties, Mapping) - if isinstance(properties, Mapping): - for name, schema in properties.items(): - schemas.setdefault(str(name), schema) for name, fix in fixes.items(): if name in header_names: found.append( @@ -301,17 +358,27 @@ def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: "which a declaration does not send" ) continue - if name not in schemas: + if name in parameters and name in body_properties: + # The runtime applies a fix by name; one name in two places + # would be fixed in both, and the declaration said neither. + found.append( + f"params.{operation.id}: {name!r} names both a request " + f"parameter and a body property of {operation.id}; the fix " + "cannot choose between them" + ) + continue + if name not in parameters and name not in body_properties: # A body whose properties cannot be enumerated may still hold # the name; only a miss that is certain is reported. if enumerable: + known = sorted({**body_properties, **parameters}) found.append( f"params.{operation.id}: {name!r} names no parameter of " - f"{operation.id} (parameters: {', '.join(sorted(schemas))})" + f"{operation.id} (parameters: {', '.join(known)})" ) continue if "prefix" in fix or "suffix" in fix: - schema = schemas[name] + schema = parameters[name] if name in parameters else body_properties[name] declared = schema.get("type") if isinstance(schema, Mapping) else None if declared is not None and declared != "string": found.append( diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index b55fbf8..586f94a 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -30,7 +30,7 @@ from gete.connection.runtime import usable_token from gete.declaration import Agent from gete.errors import DeclarationError, GeteError -from gete.openapi import declaration_problems, load_spec, read_operations +from gete.openapi import Operation, declaration_problems, load_spec, read_operations @dataclass(frozen=True) @@ -228,25 +228,31 @@ def openapi_toolset( ) -def _pruned(spec: Mapping[str, Any], selected: Iterable[Any]) -> dict[str, Any]: +def _pruned(spec: Mapping[str, Any], selected: Iterable[Operation]) -> dict[str, Any]: """The description reduced to the selected operations. - The parser resolves references itself, so components ride along whole. - Security schemes are left out: authorization is the client's, never the - parser's. + Each operation rides with its parameters already resolved and merged - + path-level ones included - and its JSON body schema folded, with + ``type: object`` said out loud: the parser only expands a body's + properties into arguments when the type is spelled, and published + schemas often leave it implicit or compose it with allOf. Property-level + references stay, and components ride along whole for the parser to + resolve. Security schemes are left out of the requests: authorization is + the client's, never the parser's. """ paths: dict[str, dict[str, Any]] = {} for operation in selected: - item = spec["paths"][operation.path] - entry = paths.setdefault(operation.path, {}) - if operation.method in item: - if "parameters" in item: - entry["parameters"] = item["parameters"] - entry[operation.method] = item[operation.method] - else: - # The path item hides behind a reference; take it whole and let - # the selection below pick the operations out of the parse. - paths[operation.path] = dict(item) + entry = dict(operation.document) + entry["parameters"] = [dict(parameter) for parameter in operation.parameters] + if operation.body is not None: + entry["requestBody"] = { + "content": { + operation.body_media: { + "schema": {**operation.body, "type": "object"} + } + } + } + paths.setdefault(operation.path, {})[operation.method] = entry return { "openapi": str(spec.get("openapi", "3.0.0")), "info": dict(spec.get("info") or {"title": "", "version": ""}), diff --git a/tests/test_openapi.py b/tests/test_openapi.py index a67f978..701b868 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -161,6 +161,42 @@ def test_read_operations_resolves_the_request_body_schema() -> None: assert "ticket" in update.body["properties"] +def with_update_body(schema: dict[str, Any], **components: Any) -> dict[str, Any]: + """SPEC with UpdateTicket's body schema replaced, plus extra components.""" + spec = json.loads(json.dumps(SPEC)) + spec["components"]["schemas"] = { + **spec["components"]["schemas"], + **components, + "TicketUpdate": schema, + } + return spec + + +def test_read_operations_folds_an_allof_body_one_level() -> None: + """Published bodies are often a shared core plus the operation's own + fields; the properties have to show for the rules and the model alike.""" + spec = with_update_body( + { + "allOf": [ + {"$ref": "#/components/schemas/TicketCore"}, + {"properties": {"comment": {"type": "string"}}}, + ] + }, + TicketCore={ + "type": "object", + "properties": {"status": {"type": "string"}}, + "required": ["status"], + }, + ) + operations, _ = read_operations(spec) + update = operations["UpdateTicket"] + assert update.body is not None + assert set(update.body["properties"]) == {"status", "comment"} + assert update.body["required"] == ["status"] + assert update.body["type"] == "object" + assert "allOf" not in update.body + + def test_read_operations_reports_a_duplicated_operation_id() -> None: doubled = json.loads(json.dumps(SPEC)) doubled["paths"]["/search"]["post"] = { @@ -318,6 +354,70 @@ def test_a_body_that_is_not_json_is_refused() -> None: assert "JSON" in found[0] +def test_a_body_whose_type_is_left_implicit_still_counts_as_an_object() -> None: + """Published schemas often write properties without spelling type: object.""" + spec = with_update_body({"properties": {"ticket": {"type": "object"}}}) + sound = block(operations=["UpdateTicket"], effect="write") + assert declaration_problems(sound, spec) == [] + + +def test_a_fix_missing_from_an_allof_body_is_reported() -> None: + """Folding makes the composed properties enumerable, so a miss is certain.""" + spec = with_update_body( + {"allOf": [{"type": "object", "properties": {"status": {"type": "string"}}}]} + ) + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"nope": {"value": 1}}}, + ), + spec, + ) + assert len(found) == 1 + assert "nope" in found[0] + + +def test_a_body_that_declares_no_properties_is_refused() -> None: + """The parser would offer the model a single opaque body argument, and the + request would carry the payload wrapped under a key the service never + declared.""" + spec = with_update_body({"type": "object"}) + found = declaration_problems( + block(operations=["UpdateTicket"], effect="write"), spec + ) + assert len(found) == 1 + assert "properties" in found[0] + + +def test_a_body_declared_only_as_alternatives_is_refused() -> None: + """oneOf without properties leaves nothing the rules or the model can hold.""" + spec = with_update_body({"oneOf": [{"type": "object"}, {"type": "string"}]}) + found = declaration_problems( + block(operations=["UpdateTicket"], effect="write"), spec + ) + assert len(found) == 1 + assert "properties" in found[0] + + +def test_a_fix_matching_a_parameter_and_a_body_property_is_refused() -> None: + """The runtime applies fixes by name; one name in two places would fix both.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/tickets/{ticket_id}"]["put"]["parameters"] = [ + {"name": "ticket", "in": "query", "schema": {"type": "string"}} + ] + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket": {"value": "x"}}}, + ), + spec, + ) + assert len(found) == 1 + assert "both" in found[0] + + def test_a_get_with_a_request_body_is_refused() -> None: """The client sends no body on a GET; the operation would lose arguments.""" spec = json.loads(json.dumps(SPEC)) diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 0b1ddd5..d0d12cf 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -1,5 +1,6 @@ """OpenAPI tools: operations become tools, and requests go through gete's client.""" +import copy from pathlib import Path from typing import Any @@ -107,14 +108,17 @@ def teardown_function() -> None: clear_tool_call() -def agent_with_spec(tmp_path: Path) -> Agent: - (tmp_path / "spec.yaml").write_text(yaml.safe_dump(SPEC, sort_keys=False)) +def agent_with_spec(tmp_path: Path, document: dict[str, Any] | None = None) -> Agent: + (tmp_path / "spec.yaml").write_text( + yaml.safe_dump(document or SPEC, sort_keys=False) + ) return Agent(directory=tmp_path, data={"name": "mail-triage"}) def toolset( tmp_path: Path, *, + document: dict[str, Any] | None = None, operations: list[str] | None = None, effect: str = "read", params: dict[str, Any] | None = None, @@ -138,7 +142,7 @@ def toolset( spec["does_not"] = does_not return openapi_toolset( spec, - agent=agent_with_spec(tmp_path), + agent=agent_with_spec(tmp_path, document), authorizations={"rooted-api": "mail-triage-rooted-api"}, registry=REGISTRY, confirm=confirm, @@ -347,6 +351,55 @@ async def test_a_put_operation_sends_the_json_body_with_the_put_verb( assert call["body"] == {"ticket": {"status": "solved"}} +def update_body(schema: dict[str, Any], **components: Any) -> dict[str, Any]: + """SPEC with UpdateTicket's body schema replaced, plus extra components.""" + spec = copy.deepcopy(SPEC) + spec["paths"]["/tickets/{ticket_id}"]["put"]["requestBody"]["content"][ + "application/json" + ]["schema"] = schema + if components: + spec["components"] = {"schemas": components} + return spec + + +async def test_a_body_whose_type_is_left_implicit_sends_its_properties( + tmp_path: Path, client: RecordingClient +) -> None: + """Published schemas often leave type: object implicit; the payload must + not end up wrapped under a body argument the service never declared.""" + spec = update_body({"properties": {"ticket": {"type": "object"}}}) + built = toolset( + tmp_path, document=spec, operations=["UpdateTicket"], effect="write" + ) + update = (await built.get_tools(context()))[0] + declared = update._get_declaration().model_dump_json(exclude_none=True) + assert '"ticket"' in declared + assert '"body"' not in declared + await run(built, "UpdateTicket", {"ticket_id": "7", "ticket": {"status": "solved"}}) + assert client.calls[0]["body"] == {"ticket": {"status": "solved"}} + + +async def test_an_allof_body_sends_its_properties_at_the_top_level( + tmp_path: Path, client: RecordingClient +) -> None: + spec = update_body( + { + "allOf": [ + {"$ref": "#/components/schemas/TicketCore"}, + {"properties": {"comment": {"type": "string"}}}, + ] + }, + TicketCore={"type": "object", "properties": {"status": {"type": "string"}}}, + ) + built = toolset( + tmp_path, document=spec, operations=["UpdateTicket"], effect="write" + ) + await run( + built, "UpdateTicket", {"ticket_id": "7", "status": "open", "comment": "hi"} + ) + assert client.calls[0]["body"] == {"status": "open", "comment": "hi"} + + async def test_a_delete_operation_uses_the_delete_verb( tmp_path: Path, client: RecordingClient ) -> None: From 495120093aa6a2f6b1fb01517500b992a51fbc8d Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:23:53 +0900 Subject: [PATCH 4/4] Refuse a fixed null before it silently drops the parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client sends no absent values, so a params entry fixing a value to null would take the parameter away from the model and then send nothing at all - a constraint that reads as a pin but acts as a deletion. The schema now refuses null where a fixed value is required. 🤖 Generated with Claude Code --- src/gete/schema/agent.json | 8 ++++++-- tests/test_schema.py | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index 2872570..c83fc88 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -321,7 +321,7 @@ "pattern": "^https://[^/?#\\s]+(?:[/?#]\\S*)?$" }, "openapi_param": { - "description": "One parameter's constraint: a fixed value the model never sees, or text put around what the model writes. A fixed parameter is taken out of the declaration shown to the model - its value is declared, so there is nothing left for the model to say, and prefix or suffix would have nothing to wrap.", + "description": "One parameter's constraint: a fixed value the model never sees, or text put around what the model writes. A fixed parameter is taken out of the declaration shown to the model - its value is declared, so there is nothing left for the model to say, and prefix or suffix would have nothing to wrap. A null value is refused: the client drops absent values, so a fixed null would silently send nothing at all.", "oneOf": [ { "type": "object", @@ -330,7 +330,11 @@ "value" ], "properties": { - "value": {} + "value": { + "not": { + "type": "null" + } + } } }, { diff --git a/tests/test_schema.py b/tests/test_schema.py index c98a46e..e4b2b20 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -181,6 +181,14 @@ def test_connections_must_be_unique() -> None: } }, {"openapi": {**OPENAPI_TOOL["openapi"], "params": {"ListThings": {"q": {}}}}}, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + # The client drops absent values, so a fixed null would + # silently send nothing at all. + "params": {"ListThings": {"q": {"value": None}}}, + } + }, ], ) def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: