diff --git a/README.md b/README.md index 295b60f..0e23d9e 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,17 @@ 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. +- **The archive carries only what was declared.** Keep the vendor's + original in the repo; `gete archive` prunes it to the declared + operations, path-level parameters and every referenced component + riding along. Cutting a description down by hand breaks quietly — + path-level parameters fall away, a flattened `$ref` takes its arguments + with it, and `validate` cannot tell such a description from one that + never declared them — so the cutting is gete's job, and `validate` now + also reports a `{placeholder}` in a path that no path parameter + declares. The packing then holds the declaration against the pruned + description, so a reference pruning cannot keep is refused before + anything deploys. - **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 diff --git a/src/gete/archive.py b/src/gete/archive.py index 0c6d620..0d40c0d 100644 --- a/src/gete/archive.py +++ b/src/gete/archive.py @@ -18,7 +18,7 @@ from dataclasses import dataclass from importlib.metadata import requires from pathlib import Path, PurePosixPath -from typing import TextIO +from typing import Any, TextIO import yaml @@ -31,6 +31,7 @@ resolve, ) from gete.errors import DeclarationError +from gete.openapi import declaration_problems, load_spec, pruned_description from gete.templates import template_text from gete.validate import validate_project @@ -111,14 +112,41 @@ def build_archive(directory: Path, *, project: Project | None = None) -> Archive 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. + # agent's tools. What travels is pruned to the declared operations - + # cutting a description down by hand breaks quietly, so the archive does + # the cutting; a file two blocks share keeps the union of their choices. + descriptions: dict[str, tuple[Path, set[str]]] = {} + blocks: list[tuple[str, Any]] = [] for index, tool in enumerate(agent.tools): if "openapi" in tool: - spec_path = agent.directory / str(tool["openapi"]["spec"]) + block = tool["openapi"] + spec_path = agent.directory / str(block["spec"]) name = _archive_input( agent, spec_path, f"tools[{index}].openapi.spec", kind="file" ) - entries[name] = spec_path.read_bytes() + _, selected = descriptions.setdefault(name, (spec_path, set())) + selected.update(map(str, block["operations"])) + blocks.append((name, block)) + pruned_by_name: dict[str, Any] = {} + for name, (spec_path, selected) in descriptions.items(): + pruned = pruned_description(load_spec(spec_path), selected) + pruned_by_name[name] = pruned + entries[name] = yaml.safe_dump( + pruned, sort_keys=False, allow_unicode=True + ).encode() + # The runtime holds each block against the archived description, not the + # repo's file. A reference into another path's subtree resolves in the + # repo and dangles once that path is pruned away; holding the declaration + # against what actually travels keeps the miss from surfacing at the + # deployed agent's cold start. + for name, block in blocks: + problems = declaration_problems(block, pruned_by_name[name]) + if problems: + lines = "; ".join(problems) + raise DeclarationError( + f"{name}, pruned to the declared operations, cannot carry " + f"its declaration: {lines}" + ) 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/openapi.py b/src/gete/openapi.py index 4933d9a..402015b 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -25,6 +25,7 @@ "declaration_problems", "exposes", "load_spec", + "pruned_description", "read_operations", ] @@ -244,6 +245,103 @@ def read_operations(spec: Any) -> tuple[dict[str, Operation], list[str]]: return operations, duplicates +def pruned_description(spec: Any, selected: Iterable[str]) -> dict[str, Any]: + """The description reduced to the named operations. + + Cutting a published description down by hand breaks quietly: path-level + parameters fall away, a flattened $ref takes the arguments it carried, + and validate cannot tell a pruned description from one that never + declared them. So the cutting is done here, from what read_operations + already knows. A kept method rides with its whole path item - path-level + parameters included - and every node a kept part references is grafted + at its original pointer, transitively, so $ref keeps resolving. All the + rest stays behind: unselected operations, unreferenced components, and + the published servers, which nothing ever reads. + """ + wanted = set(map(str, selected)) + 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?" + ) + kept_paths: dict[str, Any] = {} + for path, item in paths.items(): + path_item = _resolve(spec, item) + if not isinstance(path_item, Mapping): + continue + kept = { + key: value + for key, value in path_item.items() + if key not in HTTP_METHODS + or (isinstance(value, Mapping) and value.get("operationId") in wanted) + } + if any(method in kept for method in HTTP_METHODS): + kept_paths[str(path)] = kept + document: dict[str, Any] = {} + for key in ("openapi", "info"): + if isinstance(spec, Mapping) and key in spec: + document[key] = spec[key] + document["paths"] = kept_paths + _graft_references(spec, document) + return document + + +def _graft_references(spec: Any, document: dict[str, Any]) -> None: + """Copy every locally referenced node into the document, transitively. + + Each target lands at its original JSON pointer, so the references it was + found under keep resolving. Pointers into ``paths`` stay behind: grafting + one would re-select what pruning just left out. + """ + queue: list[Any] = [document["paths"]] + grafted: set[str] = set() + while queue: + node = queue.pop(0) + if isinstance(node, Mapping): + for key, value in node.items(): + if ( + key == "$ref" + and isinstance(value, str) + and value.startswith("#/") + and value not in grafted + ): + grafted.add(value) + target = _graft(spec, document, value) + if target is not None: + queue.append(target) + else: + queue.append(value) + elif isinstance(node, list): + queue.extend(node) + + +def _graft(spec: Any, document: dict[str, Any], pointer: str) -> Any: + """Place one pointer's target into the document, returning it. + + A pointer that cannot be followed, or that leads into paths or through + anything but mappings, grafts nothing - the reference dangles exactly as + an unresolvable one always did, and the rules speak in their own words. + """ + parts = [ + part.replace("~1", "/").replace("~0", "~") for part in pointer[2:].split("/") + ] + if not parts or parts[0] == "paths": + return None + source: Any = spec + for part in parts: + if not isinstance(source, Mapping) or part not in source: + return None + source = source[part] + where = document + for part in parts[:-1]: + node = where.setdefault(part, {}) + if not isinstance(node, dict): + return None + where = node + where[parts[-1]] = source + return source + + def declaration_problems(block: Mapping[str, Any], spec: Any) -> list[str]: """Hold one ``openapi:`` block against the description it selects from. @@ -320,6 +418,19 @@ def _operation_problems(operation: Operation, effect: str) -> list[str]: f"operations: {name!r} requires {where} parameter " f"{parameter.get('name')!r}, which a declaration does not send" ) + declared_in_path = { + str(parameter.get("name")) + for parameter in operation.parameters + if parameter.get("in") == "path" + } + for placeholder in re.findall(r"\{([^{}]*)\}", operation.path): + if placeholder not in declared_in_path: + # The tool could never say which record the request addresses; + # hand-pruned descriptions lose path-level parameters this way. + found.append( + f"operations: {name!r} has {{{placeholder}}} in its path, " + "and no path parameter declares it" + ) if operation.body_media is not None and operation.body is None: found.append( f"operations: {name!r} takes a {operation.body_media} body; " diff --git a/tests/test_archive.py b/tests/test_archive.py index ddfeeaa..9f2e01b 100644 --- a/tests/test_archive.py +++ b/tests/test_archive.py @@ -395,7 +395,8 @@ def test_an_openapi_description_travels_in_the_archive( (directory / "specs").mkdir() (directory / "specs" / "service.yaml").write_text(OPENAPI_SPEC) files = members(build_archive(directory)) - assert files["specs/service.yaml"] == OPENAPI_SPEC.encode() + document = yaml.safe_load(files["specs/service.yaml"]) + assert document["paths"]["/things"]["get"]["operationId"] == "ListThings" resolved = yaml.safe_load(files[RESOLVED_FILE]) assert resolved["tools"][0]["openapi"]["spec"] == "./specs/service.yaml" @@ -407,3 +408,157 @@ def test_an_openapi_description_outside_the_agent_directory_is_refused( (project.agents_dir / "shared.yaml").write_text(OPENAPI_SPEC) with pytest.raises(DeclarationError, match="outside"): build_archive(directory) + + +WIDE_OPENAPI_SPEC = """ +openapi: 3.0.0 +info: {title: Example, version: "1.0"} +servers: [{url: "https://tenant.example.com"}] +paths: + /things: + get: + operationId: ListThings + responses: {"200": {description: ok}} + post: + operationId: CreateThing + responses: {"201": {description: made}} + /others: + get: + operationId: ListOthers + responses: {"200": {description: ok}} +""" + + +def test_the_archived_description_is_pruned_to_the_declared_operations( + project: ProjectBuilder, +) -> None: + """Cutting a published description down by hand breaks quietly - dropped + path-level parameters, flattened $refs - so the archive does the cutting + from the declaration.""" + directory = prepare_openapi(project, "./specs/service.yaml") + (directory / "specs").mkdir() + (directory / "specs" / "service.yaml").write_text(WIDE_OPENAPI_SPEC) + files = members(build_archive(directory)) + document = yaml.safe_load(files["specs/service.yaml"]) + assert list(document["paths"]) == ["/things"] + assert list(document["paths"]["/things"]) == ["get"] + assert "servers" not in document + + +def test_two_blocks_over_one_file_keep_the_union_of_their_operations( + project: ProjectBuilder, +) -> None: + """A read block and a write block commonly share one description; the + archived file must carry what either of them declared.""" + 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": "./specs/service.yaml", + "connection": "rooted-api", + "operations": ["ListThings"], + "effect": "read", + } + }, + { + "openapi": { + "spec": "./specs/service.yaml", + "connection": "rooted-api", + "operations": ["CreateThing"], + } + }, + ], + }, + ) + (directory / "specs").mkdir() + (directory / "specs" / "service.yaml").write_text(WIDE_OPENAPI_SPEC) + files = members(build_archive(directory)) + document = yaml.safe_load(files["specs/service.yaml"]) + assert set(document["paths"]["/things"]) == {"get", "post"} + assert "/others" not in document["paths"] + + +def test_a_pruned_description_archives_the_same_bytes_every_time( + project: ProjectBuilder, +) -> None: + """Pruning happens on the way in; it must not cost the determinism the + hash comparison rests on.""" + directory = prepare_openapi(project, "./specs/service.yaml") + (directory / "specs").mkdir() + (directory / "specs" / "service.yaml").write_text(WIDE_OPENAPI_SPEC) + assert build_archive(directory).archive == build_archive(directory).archive + + +POINTED_OPENAPI_SPEC = """ +openapi: 3.0.0 +info: {title: Example, version: "1.0"} +paths: + /things: + post: + operationId: CreateThing + requestBody: + content: + application/json: + schema: + type: object + properties: {name: {type: string}} + responses: {"201": {description: made}} + /things/{thing_id}: + parameters: + - {name: thing_id, in: path, required: true, schema: {type: string}} + put: + operationId: UpdateThing + requestBody: + content: + application/json: + schema: + $ref: "#/paths/~1things/post/requestBody/content/application~1json/schema" + responses: {"200": {description: ok}} +""" + + +def test_an_archive_refuses_a_declaration_pruning_cannot_keep( + project: ProjectBuilder, +) -> None: + """A reference into another path's subtree resolves in the repo's file + and dangles once that path is pruned away. The packing must say so; a + miss the archive carries would surface at the deployed agent's cold + start.""" + 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": "./specs/service.yaml", + "connection": "rooted-api", + "operations": ["UpdateThing"], + } + } + ], + }, + ) + (directory / "specs").mkdir() + (directory / "specs" / "service.yaml").write_text(POINTED_OPENAPI_SPEC) + with pytest.raises(DeclarationError, match="pruned"): + build_archive(directory) diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 17ce9d0..9bd110f 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -7,7 +7,12 @@ import pytest from gete.errors import DeclarationError -from gete.openapi import declaration_problems, load_spec, read_operations +from gete.openapi import ( + declaration_problems, + load_spec, + pruned_description, + read_operations, +) MINIMAL = """ openapi: 3.0.0 @@ -858,3 +863,105 @@ def test_an_operation_id_that_cannot_name_a_tool_is_reported() -> None: found = declaration_problems(block(operations=["list search results"]), spec) assert len(found) == 1 assert "name" in found[0] + + +def test_a_path_placeholder_no_parameter_declares_is_reported() -> None: + """A tool that cannot say which record it addresses would still validate; + hand-pruned descriptions lose path-level parameters exactly this way.""" + spec = json.loads(json.dumps(SPEC)) + del spec["paths"]["/tickets/{ticket_id}"]["parameters"] + found = declaration_problems(block(operations=["ShowTicket"]), spec) + assert len(found) == 1 + assert "{ticket_id}" in found[0] + + +def test_a_placeholder_satisfied_through_a_reference_passes() -> None: + """Path-level parameters are commonly written as $ref; resolution has + already happened by the time the placeholders are checked.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/tickets/{ticket_id}"]["parameters"] = [ + {"$ref": "#/components/parameters/TicketId"} + ] + spec["components"]["parameters"]["TicketId"] = { + "name": "ticket_id", + "in": "path", + "required": True, + "schema": {"type": "integer"}, + } + assert declaration_problems(block(operations=["ShowTicket"]), spec) == [] + + +def test_pruned_description_keeps_only_the_selected_operations() -> None: + pruned = pruned_description(SPEC, ["ShowTicket"]) + assert list(pruned["paths"]) == ["/tickets/{ticket_id}"] + assert set(pruned["paths"]["/tickets/{ticket_id}"]) == {"parameters", "get"} + + +def test_pruned_description_keeps_what_the_kept_operations_reference() -> None: + """The two ways hand-pruning breaks quietly: a dropped path-level + parameter, and a flattened $ref chain. Both survive here, and what + nothing references stays behind.""" + spec = with_update_body( + {"$ref": "#/components/schemas/TicketWrapper"}, + TicketWrapper={ + "type": "object", + "properties": {"ticket": {"$ref": "#/components/schemas/Ticket"}}, + }, + Ticket={"type": "object", "properties": {"status": {"type": "string"}}}, + Unrelated={"type": "object"}, + ) + pruned = pruned_description(spec, ["UpdateTicket", "ListSearchResults"]) + # The path-level parameter rides with the kept path item. + tickets = pruned["paths"]["/tickets/{ticket_id}"] + assert tickets["parameters"][0]["name"] == "ticket_id" + assert "get" not in tickets and "delete" not in tickets + # The $ref chain is grafted transitively; the unreferenced schema is not. + schemas = pruned["components"]["schemas"] + assert set(schemas) == {"TicketUpdate", "TicketWrapper", "Ticket"} + assert pruned["components"]["parameters"]["Query"]["name"] == "query" + + +def test_pruned_description_leaves_the_published_servers_behind() -> None: + pruned = pruned_description(SPEC, ["ShowTicket"]) + assert "servers" not in pruned + + +def test_a_pruned_description_still_carries_the_declaration() -> None: + sound = block( + operations=["ListSearchResults", "ShowTicket"], + params={"ListSearchResults": {"query": {"prefix": "type:ticket "}}}, + ) + pruned = pruned_description(SPEC, ["ListSearchResults", "ShowTicket"]) + assert declaration_problems(sound, pruned) == [] + + +def test_pruned_description_resolves_a_referenced_path_item() -> None: + """A path item may itself be a $ref; the resolved item is what travels.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/things"] = {"$ref": "#/components/pathItems/Things"} + spec["components"]["pathItems"] = { + "Things": { + "get": { + "operationId": "ListThings", + "responses": {"200": {"description": "ok"}}, + } + } + } + pruned = pruned_description(spec, ["ListThings"]) + assert "get" in pruned["paths"]["/things"] + + +def test_pruned_description_does_not_graft_a_pointer_into_paths() -> None: + """Grafting one would re-select what pruning just left out; the pointer + dangles exactly as an unresolvable one always did.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"] = [ + {"$ref": "#/paths/~1tickets~1{ticket_id}/parameters/0"} + ] + pruned = pruned_description(spec, ["ListSearchResults"]) + assert list(pruned["paths"]) == ["/search"] + + +def test_pruned_description_needs_a_document_with_paths() -> None: + with pytest.raises(DeclarationError, match="paths"): + pruned_description({"openapi": "3.0.0"}, ["ListThings"]) diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 921a55f..516fd55 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -11,6 +11,7 @@ from gete.connection import Registry from gete.declaration import RESOLVED_FILE, Agent, load_project, resolve from gete.errors import DeclarationError, GeteError +from gete.openapi import pruned_description from gete.request_context import clear_tool_call from gete.runtime import build from gete.runtime.openapi import OpenApiToolset, openapi_toolset @@ -719,3 +720,28 @@ def test_build_turns_the_declaration_into_a_toolset(project: ProjectBuilder) -> # The connection joins the reauthorization tool like an MCP one would. assert isinstance(asking, ReauthorizationToolset) assert asking.connection_ids == ("rooted-api",) + + +async def test_the_pruned_description_builds_the_same_tools(tmp_path: Path) -> None: + """gete archive packs the pruned description; the runtime must offer + exactly what the whole one would have.""" + operations = ["ListSearchResults", "ShowTicket", "UpdateTicket"] + whole_dir = tmp_path / "whole" + pruned_dir = tmp_path / "pruned" + whole_dir.mkdir() + pruned_dir.mkdir() + from_whole = toolset(whole_dir, operations=operations, effect="write") + from_pruned = toolset( + pruned_dir, + document=pruned_description(SPEC, operations), + operations=operations, + effect="write", + ) + whole_tools = await from_whole.get_tools(context()) + pruned_tools = await from_pruned.get_tools(context()) + assert [tool.name for tool in whole_tools] == [tool.name for tool in pruned_tools] + for ours, theirs in zip(whole_tools, pruned_tools, strict=True): + assert ( + ours._get_declaration().model_dump_json() + == theirs._get_declaration().model_dump_json() + )