diff --git a/README.md b/README.md index d6e259a..01b5168 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,54 @@ 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, 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 `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..6eee8d9 --- /dev/null +++ b/src/gete/openapi.py @@ -0,0 +1,388 @@ +"""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. +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", "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}") + + +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 _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]: + """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")) + 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)) + + +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 not in SUPPORTED_METHODS: + found.append( + f"operations: {name!r} is {operation.method.upper()}, " + "which is not one of GET, POST, PUT, PATCH, DELETE" + ) + 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 in BODYLESS_METHODS: + found.append( + f"operations: {name!r} is a {operation.method.upper()} with " + "a request body, which is not supported" + ) + 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] = [] + 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"): + 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) + 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 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(known)})" + ) + continue + if "prefix" in fix or "suffix" in fix: + 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( + 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..586f94a --- /dev/null +++ b/src/gete/runtime/openapi.py @@ -0,0 +1,336 @@ +"""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 Operation, 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) + 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, + "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[Operation]) -> dict[str, Any]: + """The description reduced to the selected operations. + + 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: + 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": ""}), + "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..c83fc88 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,40 @@ "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. A null value is refused: the client drops absent values, so a fixed null would silently send nothing at all.", + "oneOf": [ + { + "type": "object", + "additionalProperties": false, + "required": [ + "value" + ], + "properties": { + "value": { + "not": { + "type": "null" + } + } + } + }, + { + "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..701b868 --- /dev/null +++ b/tests/test_openapi.py @@ -0,0 +1,437 @@ +"""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 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"] = { + "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_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 "request body" 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_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)) + 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..d0d12cf --- /dev/null +++ b/tests/test_runtime_openapi.py @@ -0,0 +1,461 @@ +"""OpenAPI tools: operations become tools, and requests go through gete's client.""" + +import copy +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"}}, + }, + "delete": { + "operationId": "DeleteTicket", + "responses": {"204": {"description": "gone"}}, + }, + }, + }, +} + + +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, 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, + 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, document), + 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) + + 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: + 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"}} + + +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: + 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: + """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..e4b2b20 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,31 @@ 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": {}}}}}, + { + "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: - """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 +215,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