diff --git a/README.md b/README.md index 84edaf8..8856136 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,12 @@ tools: per_page: {value: 25} describe: ListSearchResults: Search tickets. The kind is fixed to tickets. + omit: + ListSearchResults: [results.fields] # every search result, one by one + ShowTicket: + - ticket.fields # the custom fields again, under another name + - path: ticket.custom_fields # just the ones that hold no value + where_empty: value - openapi: spec: ./specs/helpdesk.yaml connection: helpdesk @@ -279,6 +285,21 @@ tools: that parameter's, as with `params`. Counting up what goes out fails safe as the description grows: a new field stays unexposed until someone declares it. +- **`omit` keeps results down to what the agent reads.** A published record + can carry far more than an agent needs — a helpdesk ticket may list its + custom fields twice, under two names — and every part of a result is + something the model may misread. A dotted path drops that part + of the result, stepping through every array on the way, so + `results.fields` drops it from each search result. With `where_empty`, the + path names an array that stays, minus the elements whose key holds + nothing: null, an empty string, an empty array or object, or no such key + at all. `false` and `0` are values and stay, and so does an element that + is no object. A result that does not carry the path is left as it came. + `validate` refuses a path the success responses rule out — one through a + value they declare as a scalar, or past an object that allows no other + properties — but not one they merely leave undeclared: published response + schemas routinely leave out fields the service sends. What is left passes + the policies' redaction like every other result. - **`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:`. diff --git a/src/gete/openapi.py b/src/gete/openapi.py index 402015b..4b4ce37 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -370,7 +370,7 @@ def declaration_problems(block: Mapping[str, Any], spec: Any) -> list[str]: ) continue found.extend(_operation_problems(operation, effect)) - for key in ("params", "describe", "only"): + for key in ("params", "describe", "only", "omit"): 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") @@ -388,6 +388,10 @@ def declaration_problems(block: Mapping[str, Any], spec: Any) -> list[str]: spec, operation, exposed, block.get("params", {}).get(name, {}) ) ) + for name, entries in block.get("omit", {}).items(): + operation = operations.get(name) + if operation is not None and name in selected: + found.extend(_omit_problems(spec, operation, entries)) return found @@ -683,6 +687,165 @@ def _only_problems( return found +def _omit_problems( + spec: Any, operation: Operation, entries: Iterable[Any] +) -> list[str]: + """The omit paths the success responses rule out. + + Only what is certain is reported. A published response schema routinely + leaves out fields the service sends, so a name it does not declare is no + evidence of anything; a path through a value it declares as a scalar is. + With several success responses, a path is ruled out only by all of them. + """ + schemas = _response_schemas(spec, operation) + if not schemas: + return [] + found: list[str] = [] + for entry in entries: + path = str(entry if isinstance(entry, str) else entry["path"]) + segments = tuple(path.split(".")) + walks = [_walk_result(spec, schema, segments) for schema in schemas] + if all(problem for problem, _ in walks): + found.append( + f"omit.{operation.id}: {path!r} cannot be in the result; {walks[0][0]}" + ) + continue + if isinstance(entry, str): + continue + types = [ + _declared_type(leaf) if leaf is not None else None for _, leaf in walks + ] + if all(declared not in (None, "array") for declared in types): + found.append( + f"omit.{operation.id}: where_empty filters the elements of an " + f"array, and {path!r} is {_a(str(types[0]))}" + ) + continue + key = str(entry["where_empty"]) + walks = [_walk_result(spec, schema, (*segments, key)) for schema in schemas] + if all(problem for problem, _ in walks): + found.append( + f"omit.{operation.id}: where_empty {key!r} cannot be in the " + f"elements of {path!r}; {walks[0][0]}" + ) + return found + + +def _response_schemas(spec: Any, operation: Operation) -> list[Mapping[str, Any]]: + """The JSON schemas the operation answers with when it succeeds.""" + responses = operation.document.get("responses") + if not isinstance(responses, Mapping): + return [] + found: list[Mapping[str, Any]] = [] + for code, response in responses.items(): + if not re.fullmatch(r"2(\d\d|XX)", str(code)): + continue + resolved = _resolve(spec, response) + content = resolved.get("content") if isinstance(resolved, Mapping) else None + if not isinstance(content, Mapping): + continue + for media, entry in content.items(): + if media == "application/json" or str(media).endswith("+json"): + schema = ( + _resolve(spec, entry.get("schema")) + if isinstance(entry, Mapping) + else None + ) + # A JSON answer with no schema may hold anything at all. + found.append(schema if isinstance(schema, Mapping) else {}) + return found + + +# JSON Schema types that hold no properties. +SCALAR_TYPES = frozenset({"string", "integer", "number", "boolean"}) + + +def _declared_type(schema: Mapping[str, Any]) -> str | None: + """The one type a schema declares, or None when it does not pin one. + + OpenAPI 3.1 writes a nullable value as a list such as ["string", "null"]; + a null holds no properties either, so the other type is what a path + meets whenever there is a value. A list of several other types is not + chosen between, as oneOf is not. + """ + declared = schema.get("type") + if isinstance(declared, str): + return declared + if not isinstance(declared, list): + return None + kinds = [kind for kind in declared if kind != "null"] + return kinds[0] if len(kinds) == 1 and isinstance(kinds[0], str) else None + + +def _a(word: str) -> str: + """The word with its indefinite article.""" + return f"{'an' if word[:1] in ('a', 'e', 'i', 'o', 'u') else 'a'} {word}" + + +def _walk_result( + spec: Any, schema: Mapping[str, Any], segments: tuple[str, ...] +) -> tuple[str | None, Mapping[str, Any] | None]: + """Follow a path into a response schema: why the schema rules it out, or + the schema declared at its end. Neither, once the walk leaves what the + schema declares - the path may still be there. + + Arrays on the way are stepped through to their items, as the runtime + steps through every element; the one at the end is the leaf itself. + oneOf and anyOf are not chosen between, so a path below one is never + ruled out. + """ + node: Any = schema + trail: list[str] = [] + for segment in segments: + holder, stepped = _object_schema(spec, node) + if holder is None: + return None, None + where = repr(".".join(trail)) if trail else "the result" + declared = _declared_type(holder) + if declared in SCALAR_TYPES: + if stepped: + return ( + f"{where} holds {declared} elements, which hold no properties", + None, + ) + return f"{where} is {_a(declared)}, which holds no properties", None + properties = holder.get("properties") + properties = properties if isinstance(properties, Mapping) else {} + if segment in properties: + node = properties[segment] + trail.append(segment) + continue + if holder.get("additionalProperties") is False: + known = ", ".join(sorted(map(str, properties))) + return ( + f"{segment!r} is not a property of {where}, which allows no " + f"others (properties: {known})", + None, + ) + return None, None + leaf = _resolve(spec, node) + return None, (_fold_allof(spec, leaf) if isinstance(leaf, Mapping) else None) + + +def _object_schema(spec: Any, node: Any) -> tuple[Mapping[str, Any] | None, bool]: + """The schema at node with references followed, allOf folded, and any + arrays stepped through to what their elements are; and whether an array + was stepped through.""" + stepped = False + # A YAML alias can make an array its own items; the walk stops there. + seen: set[int] = set() + while True: + node = _resolve(spec, node) + if not isinstance(node, Mapping) or id(node) in seen: + return None, stepped + seen.add(id(node)) + node = _fold_allof(spec, node) + if _declared_type(node) != "array": + return node, stepped + node = node.get("items") + stepped = True + + def allow_tree(entries: Iterable[str], literal: Iterable[str] = ()) -> dict[str, Any]: """An ``only`` list as a tree of what the model may write. diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index 5b11253..a53dce6 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -77,6 +77,64 @@ class NestedFix: suffix: str = "" +@dataclass(frozen=True) +class Omission: + """One part of a result the model is never shown, by a dotted path. + + With where_empty, the path names an array and only the elements whose + key holds nothing are dropped; the rest of the array stays. + """ + + path: tuple[str, ...] + where_empty: str | None = None + + @classmethod + def from_declaration(cls, entry: str | Mapping[str, Any]) -> "Omission": + if isinstance(entry, str): + return cls(path=tuple(entry.split("."))) + return cls( + path=tuple(str(entry["path"]).split(".")), + where_empty=str(entry["where_empty"]), + ) + + def apply(self, value: Any) -> Any: + return _omitted(value, self.path, self.where_empty) + + +def _is_empty(value: Any) -> bool: + """null, "", [] and {}. false and 0 are answers, not absences.""" + return value is None or (isinstance(value, str | list | dict) and not value) + + +def _omitted(value: Any, path: tuple[str, ...], where_empty: str | None) -> Any: + """The value without what path names; a copy, the answer itself untouched. + + An array anywhere on the way is stepped through, each element walked on + its own: results come back as lists of records far more often than as + one record. + """ + if isinstance(value, list): + return [_omitted(item, path, where_empty) for item in value] + if not isinstance(value, dict) or path[0] not in value: + return value + head, rest = path[0], path[1:] + if rest: + return {**value, head: _omitted(value[head], rest, where_empty)} + if where_empty is None: + return {key: item for key, item in value.items() if key != head} + elements = value[head] + if not isinstance(elements, list): + return value + return { + **value, + head: [ + element + for element in elements + if not (isinstance(element, dict) and _is_empty(element.get(where_empty))) + ], + } + + def _filtered(value: Any, tree: Mapping[str, Any] | None) -> Any: """What of the model's value may pass: the tree's subtrees of it. @@ -140,6 +198,7 @@ def __init__( connection: Connection, arguments: Iterable[Argument], nested_fixes: Iterable[NestedFix] = (), + omissions: Iterable[Omission] = (), declaration: Any, require_confirmation: bool, base_url: str | None = None, @@ -150,6 +209,7 @@ def __init__( self._connection = connection self._arguments = tuple(arguments) self._nested_fixes = tuple(nested_fixes) + self._omissions = tuple(omissions) self._declaration = declaration self._require_confirmation = require_confirmation self._base_url = base_url @@ -202,8 +262,17 @@ async def run_async(self, *, args: dict[str, Any], tool_context: Any) -> Any: if "{" in path: raise GeteError(f"{self.name}: a path parameter was not given") url = root.rstrip("/") + path + answer = await self._send( + url, query, body, getattr(tool_context, "state", None) + ) + for omission in self._omissions: + answer = omission.apply(answer) + return answer + + async def _send( + self, url: str, query: dict[str, Any], body: dict[str, Any], state: Any + ) -> Any: 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": @@ -309,6 +378,7 @@ def openapi_toolset( name=str(name), fixes=spec.get("params", {}).get(name, {}), only=spec.get("only", {}).get(name), + omit=spec.get("omit", {}).get(name, ()), description=describe.get(name), does_not=does_not, connection=connection, @@ -362,6 +432,7 @@ def _tool( name: str, fixes: Mapping[str, Mapping[str, Any]], only: Iterable[str] | None = None, + omit: Iterable[Any] = (), description: str | None, does_not: str | None, connection: Connection, @@ -445,6 +516,7 @@ def _tool( connection=connection, arguments=arguments, nested_fixes=nested, + omissions=[Omission.from_declaration(entry) for entry in omit], declaration=declaration, require_confirmation=require_confirmation, base_url=base_url, diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index 08159c5..2d289d9 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -336,6 +336,18 @@ } } }, + "omit": { + "description": "Parts of the result the model is never shown, by operationId. A dotted path steps through every array on the way. A published record can carry far more than an agent reads, and every part of it is something the model may misread or quote.", + "type": "object", + "additionalProperties": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "$ref": "#/$defs/openapi_omission" + } + } + }, "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", @@ -385,6 +397,35 @@ "type": "string", "pattern": "^https://[^/?#\\s]+(?:[/?#]\\S*)?$" }, + "dotted_path": { + "type": "string", + "pattern": "^[^.]+(\\.[^.]+)*$" + }, + "openapi_omission": { + "description": "A dotted path dropped from the result, or an array at a path of which only the elements whose where_empty key holds nothing (null, an empty string, array, or object, or no key at all) are dropped. false and 0 are values.", + "oneOf": [ + { + "$ref": "#/$defs/dotted_path" + }, + { + "type": "object", + "additionalProperties": false, + "required": [ + "path", + "where_empty" + ], + "properties": { + "path": { + "$ref": "#/$defs/dotted_path" + }, + "where_empty": { + "type": "string", + "pattern": "^[^.]+$" + } + } + } + ] + }, "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": [ diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 9bd110f..85210ab 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -300,6 +300,175 @@ def test_describe_must_name_a_selected_operation() -> None: assert found == ["describe: 'ShowTicket' is not one of this block's operations"] +def test_omit_must_name_a_selected_operation() -> None: + found = declaration_problems(block(omit={"ShowTicket": ["ticket.fields"]}), SPEC) + assert found == ["omit: 'ShowTicket' is not one of this block's operations"] + + +def with_ticket_response(schema: dict[str, Any], **components: Any) -> dict[str, Any]: + """SPEC whose ShowTicket answers 200 with the given JSON schema.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/tickets/{ticket_id}"]["get"]["responses"] = { + "200": { + "description": "ok", + "content": {"application/json": {"schema": schema}}, + } + } + spec["components"]["schemas"].update(components) + return spec + + +TICKET_RESPONSE: dict[str, Any] = { + "type": "object", + "properties": {"ticket": {"$ref": "#/components/schemas/Ticket"}}, +} + +TICKET: dict[str, Any] = { + "type": "object", + "properties": { + "id": {"type": "integer"}, + "subject": {"type": "string"}, + "tags": {"type": "array", "items": {"type": "string"}}, + "custom_fields": { + "type": "array", + "items": { + "type": "object", + "properties": {"id": {"type": "integer"}, "value": {}}, + }, + }, + }, +} + + +def omit_problems(spec: dict[str, Any], *entries: Any) -> list[str]: + return declaration_problems( + block(operations=["ShowTicket"], omit={"ShowTicket": list(entries)}), spec + ) + + +def test_an_omit_path_the_response_leaves_undeclared_passes() -> None: + """Published response schemas under-declare: a search result typed by + the kind of record it is may declare only the fields every kind shares, + and the ones to drop are exactly the ones it leaves out.""" + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + assert omit_problems(spec, "ticket.fields") == [] + + +def test_an_omit_path_through_a_declared_scalar_is_refused() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + assert omit_problems(spec, "ticket.subject.text") == [ + "omit.ShowTicket: 'ticket.subject.text' cannot be in the result; " + "'ticket.subject' is a string, which holds no properties" + ] + + +def test_an_omit_path_a_closed_object_does_not_declare_is_refused() -> None: + closed = {**TICKET, "additionalProperties": False} + spec = with_ticket_response(TICKET_RESPONSE, Ticket=closed) + assert omit_problems(spec, "ticket.fields") == [ + "omit.ShowTicket: 'ticket.fields' cannot be in the result; 'fields' is " + "not a property of 'ticket', which allows no others " + "(properties: custom_fields, id, subject, tags)" + ] + + +def test_an_omit_path_is_followed_through_the_items_of_an_array() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + assert omit_problems(spec, "ticket.custom_fields.value") == [] + assert omit_problems(spec, "ticket.custom_fields.id.x") == [ + "omit.ShowTicket: 'ticket.custom_fields.id.x' cannot be in the result; " + "'ticket.custom_fields.id' is an integer, which holds no properties" + ] + + +def test_an_omit_path_below_an_array_of_scalars_is_refused() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + assert omit_problems(spec, "ticket.tags.name") == [ + "omit.ShowTicket: 'ticket.tags.name' cannot be in the result; " + "'ticket.tags' holds string elements, which hold no properties" + ] + + +def test_where_empty_on_a_path_declared_as_no_array_is_refused() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + entry = {"path": "ticket.subject", "where_empty": "value"} + assert omit_problems(spec, entry) == [ + "omit.ShowTicket: where_empty filters the elements of an array, and " + "'ticket.subject' is a string" + ] + + +def test_where_empty_naming_a_key_the_elements_cannot_hold_is_refused() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + sound = {"path": "ticket.custom_fields", "where_empty": "value"} + assert omit_problems(spec, sound) == [] + entry = {"path": "ticket.tags", "where_empty": "value"} + assert omit_problems(spec, entry) == [ + "omit.ShowTicket: where_empty 'value' cannot be in the elements of " + "'ticket.tags'; 'ticket.tags' holds string elements, which hold no " + "properties" + ] + + +def test_an_omit_path_passes_when_no_success_response_declares_json() -> None: + assert omit_problems(SPEC, "ticket.subject.text") == [] + + +def test_an_omit_path_one_success_response_may_hold_passes() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=TICKET) + responses = spec["paths"]["/tickets/{ticket_id}"]["get"]["responses"] + responses["2XX"] = { + "description": "anything", + "content": {"application/json": {"schema": {"type": "object"}}}, + } + assert omit_problems(spec, "ticket.subject.text") == [] + + +def test_an_array_that_is_its_own_items_ends_the_walk() -> None: + """A YAML alias can build one; the walk must stop, not spin.""" + endless: dict[str, Any] = {"type": "array"} + endless["items"] = endless + spec = with_ticket_response(endless) + assert omit_problems(spec, "ticket.fields") == [] + + +NULLABLE_TICKET: dict[str, Any] = { + "type": ["object", "null"], + "properties": { + "subject": {"type": ["string", "null"]}, + "custom_fields": { + "type": ["array", "null"], + "items": {"type": "object", "properties": {"value": {}}}, + }, + "external_id": {"type": ["string", "integer"]}, + }, +} + + +def test_an_omit_path_is_followed_through_a_nullable_type_list() -> None: + """OpenAPI 3.1 writes a nullable value as a list of types; the value is + still what the one other type says when it is there.""" + spec = with_ticket_response(TICKET_RESPONSE, Ticket=NULLABLE_TICKET) + assert omit_problems(spec, "ticket.fields") == [] + assert omit_problems(spec, "ticket.subject.text") == [ + "omit.ShowTicket: 'ticket.subject.text' cannot be in the result; " + "'ticket.subject' is a string, which holds no properties" + ] + + +def test_where_empty_on_a_nullable_array_passes() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=NULLABLE_TICKET) + entry = {"path": "ticket.custom_fields", "where_empty": "value"} + assert omit_problems(spec, entry) == [] + + +def test_a_type_list_of_several_kinds_rules_nothing_out() -> None: + spec = with_ticket_response(TICKET_RESPONSE, Ticket=NULLABLE_TICKET) + assert omit_problems(spec, "ticket.external_id.x") == [] + entry = {"path": "ticket.external_id", "where_empty": "value"} + assert omit_problems(spec, entry) == [] + + def test_a_fix_naming_no_parameter_of_the_operation_is_reported() -> None: found = declaration_problems( block(params={"ListSearchResults": {"sort": {"value": "asc"}}}), SPEC diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 643806b..e469b8f 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -126,6 +126,7 @@ def toolset( params: dict[str, Any] | None = None, only: dict[str, list[str]] | None = None, describe: dict[str, str] | None = None, + omit: dict[str, list[Any]] | None = None, does_not: str | None = None, confirm: bool = False, confirm_names: list[str] | None = None, @@ -146,6 +147,8 @@ def toolset( spec["only"] = only if describe: spec["describe"] = describe + if omit: + spec["omit"] = omit if does_not: spec["does_not"] = does_not return openapi_toolset( @@ -706,6 +709,165 @@ async def test_a_delete_operation_uses_the_delete_verb( assert call["body"] is None +async def test_omit_drops_the_named_key_from_the_result( + tmp_path: Path, client: RecordingClient +) -> None: + client.answer = {"ticket": {"id": 7, "subject": "Help", "fields": [{"id": 1}]}} + built = toolset(tmp_path, omit={"ShowTicket": ["ticket.fields"]}) + result = await run(built, "ShowTicket", {"ticket_id": "7"}) + assert result == {"ticket": {"id": 7, "subject": "Help"}} + + +async def test_omit_walks_into_every_element_of_an_array_on_the_path( + tmp_path: Path, client: RecordingClient +) -> None: + client.answer = { + "results": [ + {"id": 1, "custom_fields": [{"id": 10, "value": None}], "fields": []}, + {"id": 2, "custom_fields": [], "fields": [{"id": 10, "value": None}]}, + ], + "count": 2, + } + built = toolset( + tmp_path, + omit={"ListSearchResults": ["results.custom_fields", "results.fields"]}, + ) + result = await run(built, "ListSearchResults", {"query": "x"}) + assert result == {"results": [{"id": 1}, {"id": 2}], "count": 2} + + +@pytest.mark.parametrize( + "answer", + [ + {"error": "RecordNotFound"}, + {"ticket": None}, + {"ticket": "not an object"}, + [{"ticket": {"id": 7}}], + "plain text", + ], +) +async def test_omit_leaves_a_result_without_the_path_as_it_came( + tmp_path: Path, client: RecordingClient, answer: Any +) -> None: + """An error body, or a record the service sent in another shape, is not a + broken declaration; nothing is there to drop.""" + client.answer = copy.deepcopy(answer) + built = toolset(tmp_path, omit={"ShowTicket": ["ticket.fields"]}) + assert await run(built, "ShowTicket", {"ticket_id": "7"}) == answer + + +async def test_where_empty_drops_just_the_elements_whose_key_holds_nothing( + tmp_path: Path, client: RecordingClient +) -> None: + """The fields that carry a value are what a lookup needs; false and 0 are + values, and an element that is no object has no key to judge by.""" + client.answer = { + "ticket": { + "id": 7, + "custom_fields": [ + {"id": 1, "value": None}, + {"id": 2, "value": ""}, + {"id": 3, "value": []}, + {"id": 4, "value": {}}, + {"id": 5}, + {"id": 6, "value": False}, + {"id": 7, "value": 0}, + {"id": 8, "value": "blue"}, + "not an object", + ], + } + } + built = toolset( + tmp_path, + omit={"ShowTicket": [{"path": "ticket.custom_fields", "where_empty": "value"}]}, + ) + result = await run(built, "ShowTicket", {"ticket_id": "7"}) + assert result == { + "ticket": { + "id": 7, + "custom_fields": [ + {"id": 6, "value": False}, + {"id": 7, "value": 0}, + {"id": 8, "value": "blue"}, + "not an object", + ], + } + } + + +async def test_where_empty_leaves_a_path_that_holds_no_array_as_it_came( + tmp_path: Path, client: RecordingClient +) -> None: + client.answer = {"ticket": {"custom_fields": {"value": None}}} + built = toolset( + tmp_path, + omit={"ShowTicket": [{"path": "ticket.custom_fields", "where_empty": "value"}]}, + ) + result = await run(built, "ShowTicket", {"ticket_id": "7"}) + assert result == {"ticket": {"custom_fields": {"value": None}}} + + +async def test_omit_on_one_operation_leaves_the_others_results_whole( + tmp_path: Path, client: RecordingClient +) -> None: + client.answer = {"ticket": {"id": 7, "fields": [{"id": 1}]}} + built = toolset(tmp_path, omit={"ShowTicket": ["ticket.fields"]}) + result = await run(built, "ListSearchResults", {"query": "x"}) + assert result == {"ticket": {"id": 7, "fields": [{"id": 1}]}} + + +async def test_redaction_still_covers_what_omit_leaves( + project: ProjectBuilder, client: RecordingClient +) -> None: + """Dropping parts of a result is no way around the policies: the agent + redacts whatever the tool returns, and omit runs inside the tool.""" + project.write_policies( + "p", [{"name": "mask", "when": "always", "redact": {"keys": ["email"]}}] + ) + project.write_project( + { + "version": 1, + "project": "example-project", + "location": "us-central1", + "policies": ["./policies/p.yaml"], + "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": ["ShowTicket"], + "effect": "read", + "omit": {"ShowTicket": ["ticket.fields"]}, + } + } + ], + }, + ) + (directory / "spec.yaml").write_text(yaml.safe_dump(SPEC, sort_keys=False)) + loaded = load_project(project.root / "gete.yaml") + path = directory / RESOLVED_FILE + path.write_text(yaml.safe_dump(resolve(loaded, loaded.agents[0]), sort_keys=False)) + agent = build(path) + client.answer = { + "ticket": {"id": 7, "email": "someone@example.com", "fields": [{"id": 1}]} + } + toolset_built = agent.tools[0] + [tool] = await toolset_built.get_tools(context()) + args = {"ticket_id": "7"} + answer = await tool.run_async(args=args, tool_context=context()) + shown = agent.after_tool_callback(tool, args, context(), answer) + assert shown == {"ticket": {"id": 7, "email": "[redacted]"}} + + async def test_the_callers_state_reaches_the_client( tmp_path: Path, client: RecordingClient ) -> None: diff --git a/tests/test_schema.py b/tests/test_schema.py index dc53d23..0be836b 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -391,6 +391,39 @@ def test_an_optional_scope_needs_a_non_empty_explanation() -> None: "base_url": "http://things.example.com", } }, + # An empty omit list drops nothing; leaving the operation out says so. + {"openapi": {**OPENAPI_TOOL["openapi"], "omit": {"ListThings": []}}}, + {"openapi": {**OPENAPI_TOOL["openapi"], "omit": {"ListThings": "a.b"}}}, + *( + {"openapi": {**OPENAPI_TOOL["openapi"], "omit": {"ListThings": [path]}}} + for path in ("", "a..b", ".a", "a.") + ), + # The object form exists for where_empty; without it, a string says it. + { + "openapi": { + **OPENAPI_TOOL["openapi"], + "omit": {"ListThings": [{"path": "thing.fields"}]}, + } + }, + # where_empty names one key of each element, never a path. + { + "openapi": { + **OPENAPI_TOOL["openapi"], + "omit": { + "ListThings": [{"path": "thing.fields", "where_empty": "a.b"}] + }, + } + }, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + "omit": { + "ListThings": [ + {"path": "thing.fields", "where_empty": "value", "x": 1} + ] + }, + } + }, ], ) def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: @@ -446,6 +479,17 @@ def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: "base_url": "https://things.example.com", } }, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + "omit": { + "ListThings": [ + "things.fields", + {"path": "things.custom_fields", "where_empty": "value"}, + ] + }, + } + }, ], ) def test_supported_tool_shapes_pass(tool: dict[str, Any]) -> None: