From 7f50173871cb641bd611d8104d12805d425edeba Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:14:41 +0900 Subject: [PATCH 1/3] Let a declaration reach into the body and count up what goes out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A helpdesk ticket update carries its comment inside the record: {"ticket": {"comment": {"body": ..., "public": true}}}. Whether that comment goes out to the requester is a boolean two levels down, and params could only name arguments and top-level body properties - the one value that separates an internal note from an outgoing mail was the model's to write. The code such a declaration replaces kept the two apart as differently named functions; a declaration must be able to pin the same value structurally, not ask nicely in the instruction. params now reads a dotted name as a path into the JSON body when no parameter carries the name literally. A fixed leaf disappears from the declaration and is written wherever its parent object is sent - overwriting anything found there, never conjuring the parent up. Published update operations also accept the whole record - status, assignee, tags - when an agent is only meant to add a comment. Pinning every unwanted field enumerates a list that goes stale silently as the description grows. The new only key turns the enumeration around: it names what the model may write, everything else leaves the declaration and is never sent, and a new field stays unexposed until someone declares it. The two are one mechanism seen from both sides, so their rules hold each other: a fix cannot pin what only hands the model, a prefix cannot wrap what only withholds, and a required argument cannot fall between them. 🤖 Generated with Claude Code --- README.md | 23 +++ src/gete/openapi.py | 263 ++++++++++++++++++++++++- src/gete/runtime/openapi.py | 180 ++++++++++++++++- src/gete/schema/agent.json | 15 +- tests/test_openapi.py | 357 ++++++++++++++++++++++++++++++++++ tests/test_runtime_openapi.py | 224 ++++++++++++++++++++- tests/test_schema.py | 11 ++ 7 files changed, 1063 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 3aa54e6..88c8143 100644 --- a/README.md +++ b/README.md @@ -163,6 +163,16 @@ tools: per_page: {value: 25} describe: ListSearchResults: Search tickets. The kind is fixed to tickets. + - openapi: + spec: ./specs/helpdesk.yaml + connection: helpdesk + operations: [UpdateTicket] + effect: write + only: + UpdateTicket: [ticket_id, ticket.comment.body] # all the model may write + params: + UpdateTicket: + ticket.comment.public: {value: false} # internal note, never mail ``` - **`operations` is required, never defaulted.** A published description @@ -178,6 +188,19 @@ tools: 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. +- **A dotted name reaches into the JSON body.** Services commonly nest what + matters: whether a helpdesk comment goes out to the requester is a boolean + two levels down. `ticket.comment.public: {value: false}` pins it there — + the leaf disappears from what the model sees, and the declared value is + written wherever its parent object is sent, overwriting anything found + there and never conjuring the parent up. A name that matches a parameter + literally keeps meaning that parameter. +- **`only` names what the model may write.** Published update operations + accept the whole record — status, assignee, tags — when an agent is only + meant to add a comment. Everything `only` leaves unlisted is taken out of + the declaration and never sent, even smuggled into the arguments; a + `params` value still rides. Counting up what goes out fails safe as the + description grows: a new field stays unexposed until someone declares 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:`. diff --git a/src/gete/openapi.py b/src/gete/openapi.py index 6eee8d9..5b68d20 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -10,7 +10,7 @@ """ import re -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from dataclasses import dataclass from pathlib import Path from typing import Any @@ -19,7 +19,14 @@ from gete.errors import DeclarationError -__all__ = ["Operation", "declaration_problems", "load_spec", "read_operations"] +__all__ = [ + "Operation", + "allow_tree", + "declaration_problems", + "exposes", + "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") @@ -265,14 +272,24 @@ def declaration_problems(block: Mapping[str, Any], spec: Any) -> list[str]: ) continue found.extend(_operation_problems(operation, effect)) - for key in ("params", "describe"): + for key in ("params", "describe", "only"): 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)) + found.extend( + _fix_problems(spec, operation, fixes, block.get("only", {}).get(name)) + ) + for name, exposed in block.get("only", {}).items(): + operation = operations.get(name) + if operation is not None and name in selected: + found.extend( + _only_problems( + spec, operation, exposed, block.get("params", {}).get(name, {}) + ) + ) return found @@ -333,9 +350,61 @@ def _operation_problems(operation: Operation, effect: str) -> list[str]: 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] = [] +@dataclass(frozen=True) +class _BodyLeaf: + """Where a dotted name landed in the body: a schema when the walk could + enumerate its way down, or the problem that stopped it. Neither means the + walk left enumerable ground, so the miss is not certain.""" + + schema: Mapping[str, Any] | None = None + problem: str | None = None + # Whether every segment was found in enumerated properties. A path into + # a freeform object is possible, never certain. + certain: bool = True + + +def _walk_body(spec: Any, operation: Operation, segments: tuple[str, ...]) -> _BodyLeaf: + """Follow dotted segments through the body's properties, resolving and + folding each level the way the top level was.""" + if operation.body is None: + return _BodyLeaf( + problem=f"reaches into the JSON body, and {operation.id} declares none" + ) + node: Mapping[str, Any] | None = operation.body + trail: list[str] = [] + for segment in segments: + if node is None: + return _BodyLeaf(certain=False) + declared = node.get("type") + if declared is not None and declared != "object": + return _BodyLeaf( + problem=f"{'.'.join(trail)!r} is a {declared}, which holds " + "no properties" + ) + properties = node.get("properties") + if not isinstance(properties, Mapping): + return _BodyLeaf(certain=False) + if segment not in properties: + where = ".".join(trail) or "the body" + known = ", ".join(sorted(map(str, properties))) + return _BodyLeaf( + problem=f"{segment!r} is not a property of {where} " + f"(properties: {known})" + ) + child = _resolve(spec, properties[segment]) + if isinstance(child, Mapping): + child = _fold_allof(spec, child) + node = child if isinstance(child, Mapping) else None + trail.append(segment) + return _BodyLeaf(schema=node) + + +def _argument_maps( + operation: Operation, +) -> tuple[dict[str, Any], set[str], dict[str, Any], bool]: + """The names a declaration can speak about, by where they live: request + parameters, header names, top-level body properties, and whether the body + could be enumerated at all.""" parameters: dict[str, Any] = {} header_names: set[str] = set() for parameter in operation.parameters: @@ -351,6 +420,19 @@ def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: else {} ) enumerable = operation.body is None or isinstance(properties, Mapping) + return parameters, header_names, body_properties, enumerable + + +def _fix_problems( + spec: Any, + operation: Operation, + fixes: Mapping[str, Any], + only: Iterable[str] | None = None, +) -> list[str]: + """What keeps the declared parameter fixes from being applied.""" + found: list[str] = [] + parameters, header_names, body_properties, enumerable = _argument_maps(operation) + tree = allow_tree(only) if only is not None else None for name, fix in fixes.items(): if name in header_names: found.append( @@ -367,7 +449,25 @@ def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: "cannot choose between them" ) continue + if ( + (name in parameters or name in body_properties) + and "." in name + and _certainly_in_body(spec, operation, name) + ): + # A dot may sit in a parameter's literal name or mark a path into + # the body; when both readings hold, neither was declared. + found.append( + f"params.{operation.id}: {name!r} names both a parameter and " + "a path into the body; the fix cannot choose between them" + ) + continue if name not in parameters and name not in body_properties: + if "." in name: + found.extend( + f"params.{operation.id}: {name!r} {message}" + for message in _dotted_problems(spec, operation, name, fix, tree) + ) + continue # A body whose properties cannot be enumerated may still hold # the name; only a miss that is certain is reported. if enumerable: @@ -378,6 +478,12 @@ def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: ) continue if "prefix" in fix or "suffix" in fix: + if not exposes(tree, (name,)): + found.append( + f"params.{operation.id}.{name}: prefix and suffix wrap " + f"what the model writes, and only does not expose {name!r}" + ) + continue 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": @@ -386,3 +492,146 @@ def _fix_problems(operation: Operation, fixes: Mapping[str, Any]) -> list[str]: f"string parameter, and {name!r} is {declared}" ) return found + + +def _only_problems( + spec: Any, + operation: Operation, + exposed: Iterable[str], + fixes: Mapping[str, Any], +) -> list[str]: + """What keeps the declared exposure list from carving the operation.""" + found: list[str] = [] + parameters, header_names, body_properties, enumerable = _argument_maps(operation) + exposed = tuple(exposed) + # A dotted entry still sends its top-level argument, just narrowed. + covered = {entry.split(".")[0] for entry in exposed} + fixed = {name for name, fix in fixes.items() if "value" in fix} + for name in _required_names(operation, parameters): + if name not in covered and name not in fixed: + found.append( + f"only.{operation.id}: {name!r} is required, and it is " + "neither listed nor fixed; every request needs it" + ) + for entry in exposed: + if "value" in fixes.get(entry, {}): + found.append( + f"only.{operation.id}: {entry!r} is fixed by params; a fixed " + "parameter is not the model's to write" + ) + continue + if entry in header_names: + found.append( + f"only.{operation.id}: {entry!r} is a header parameter, " + "which a declaration does not send" + ) + continue + literal = entry in parameters or entry in body_properties + if literal and "." in entry and _certainly_in_body(spec, operation, entry): + found.append( + f"only.{operation.id}: {entry!r} names both a parameter and " + "a path into the body; the entry cannot choose between them" + ) + continue + if literal: + continue + if "." in entry: + leaf = _walk_body(spec, operation, tuple(entry.split("."))) + if leaf.problem is not None: + found.append(f"only.{operation.id}: {entry!r} {leaf.problem}") + continue + if enumerable: + known = sorted({**body_properties, **parameters}) + found.append( + f"only.{operation.id}: {entry!r} names no parameter of " + f"{operation.id} (parameters: {', '.join(known)})" + ) + return found + + +def allow_tree(entries: Iterable[str]) -> dict[str, Any]: + """An ``only`` list as a tree of what the model may write. + + A node of None means the whole subtree is the model's; a mapping narrows + it to the named children. A bare name is broader than any dotted entry + under it, so it wins. + """ + tree: dict[str, Any] = {} + for entry in entries: + segments = str(entry).split(".") + node = tree + for segment in segments[:-1]: + if segment in node and node[segment] is None: + break + node = node.setdefault(segment, {}) + else: + node[segments[-1]] = None + return tree + + +def exposes(tree: Mapping[str, Any] | None, path: Iterable[str]) -> bool: + """Whether the model may write the value at path under this tree. + + No tree at all means everything is the model's; landing on a mapping + means the object itself is written, if only its named children. + """ + if tree is None: + return True + node: Any = tree + for segment in path: + if node is None: + return True + if not isinstance(node, Mapping) or segment not in node: + return False + node = node[segment] + return True + + +def _required_names(operation: Operation, parameters: Mapping[str, Any]) -> list[str]: + """Names a request cannot go without: required query and path parameters, + then the body's own required properties.""" + names = [ + str(parameter.get("name")) + for parameter in operation.parameters + if parameter.get("required") and str(parameter.get("name")) in parameters + ] + required = (operation.body or {}).get("required") + if isinstance(required, list): + names.extend(str(name) for name in required if name not in names) + return names + + +def _certainly_in_body(spec: Any, operation: Operation, name: str) -> bool: + leaf = _walk_body(spec, operation, tuple(name.split("."))) + return leaf.problem is None and leaf.certain + + +def _dotted_problems( + spec: Any, + operation: Operation, + name: str, + fix: Mapping[str, Any], + tree: Mapping[str, Any] | None, +) -> list[str]: + """What keeps one dotted fix from reaching its place in the body.""" + segments = tuple(name.split(".")) + leaf = _walk_body(spec, operation, segments) + if leaf.problem is not None: + return [leaf.problem] + if "value" in fix and not exposes(tree, segments[:-1]): + # The fix rides on its parent object; a parent never sent would + # leave the constraint silently unapplied. + return ["is pinned inside a parent that only does not send"] + if "prefix" in fix or "suffix" in fix: + if not exposes(tree, segments): + return [ + "takes a prefix or suffix, which wrap what the model " + "writes, and only does not expose it" + ] + declared = leaf.schema.get("type") if leaf.schema is not None else None + if declared is not None and declared != "string": + return [ + "takes a prefix or suffix, which need a string property, " + f"and it is {declared}" + ] + return [] diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index 586f94a..1279a8a 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -30,7 +30,13 @@ 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 +from gete.openapi import ( + Operation, + allow_tree, + declaration_problems, + load_spec, + read_operations, +) @dataclass(frozen=True) @@ -49,6 +55,73 @@ class Argument: value: Any = None prefix: str = "" suffix: str = "" + # For a body argument only partly the model's: the tree of what may pass. + # None means everything; anything outside the tree is dropped unsent. + allowed: Mapping[str, Any] | None = None + + +@dataclass(frozen=True) +class NestedFix: + """One constraint inside the JSON body, addressed by a dotted path. + + A fixed value is written wherever its parent object is being sent - + overwriting anything found there, never conjuring the parent up. prefix + and suffix wrap the value the model wrote at the path, when there is one. + """ + + path: tuple[str, ...] + fixed: bool = False + value: Any = None + prefix: str = "" + suffix: str = "" + + +def _filtered(value: Any, tree: Mapping[str, Any] | None) -> Any: + """What of the model's value may pass: the tree's subtrees of it. + + A value where the tree expects an object cannot be carved, so nothing of + it passes - the declaration wins over what the model wrote, as it does + for a smuggled fixed parameter. + """ + if tree is None: + return value + if not isinstance(value, Mapping): + return None + kept: dict[str, Any] = {} + for key, item in value.items(): + if key in tree: + child = _filtered(item, tree[key]) + if child is not None: + kept[key] = child + return kept + + +def _apply_nested(body: dict[str, Any], fix: NestedFix, tool: str) -> None: + node: Any = body + walked: list[str] = [] + for segment in fix.path[:-1]: + if not isinstance(node, dict): + # The declaration says an object sits here; sending the request + # around the fix would drop the constraint on the floor. + raise GeteError( + f"{tool}: {'.'.join(walked)} is not an object, and " + f"{'.'.join(fix.path)} is declared inside it" + ) + if segment not in node: + # The parent is not being sent, so neither is the field it holds. + return + node = node[segment] + walked.append(segment) + if not isinstance(node, dict): + raise GeteError( + f"{tool}: {'.'.join(walked)} is not an object, and " + f"{'.'.join(fix.path)} is declared inside it" + ) + leaf = fix.path[-1] + if fix.fixed: + node[leaf] = fix.value + elif leaf in node and node[leaf] is not None: + node[leaf] = f"{fix.prefix}{node[leaf]}{fix.suffix}" class OpenApiTool(BaseTool): @@ -63,6 +136,7 @@ def __init__( path: str, connection: Connection, arguments: Iterable[Argument], + nested_fixes: Iterable[NestedFix] = (), declaration: Any, require_confirmation: bool, ) -> None: @@ -71,6 +145,7 @@ def __init__( self._path = path self._connection = connection self._arguments = tuple(arguments) + self._nested_fixes = tuple(nested_fixes) self._declaration = declaration self._require_confirmation = require_confirmation @@ -98,6 +173,10 @@ async def run_async(self, *, args: dict[str, Any], tool_context: Any) -> Any: value = argument.value if argument.fixed else args.get(argument.py_name) if value is None: continue + if argument.allowed is not None: + value = _filtered(value, argument.allowed) + if value is None: + continue if argument.prefix or argument.suffix: value = f"{argument.prefix}{value}{argument.suffix}" if argument.location == "path": @@ -111,6 +190,8 @@ async def run_async(self, *, args: dict[str, Any], tool_context: Any) -> Any: query[argument.name] = value else: body[argument.name] = value + for fix in self._nested_fixes: + _apply_nested(body, fix, self.name) if "{" in path: raise GeteError(f"{self.name}: a path parameter was not given") url = root.rstrip("/") + path @@ -215,6 +296,7 @@ def openapi_toolset( parsed, name=str(name), fixes=spec.get("params", {}).get(name, {}), + only=spec.get("only", {}).get(name), description=describe.get(name), does_not=does_not, connection=connection, @@ -266,6 +348,7 @@ def _tool( *, name: str, fixes: Mapping[str, Mapping[str, Any]], + only: Iterable[str] | None = None, description: str | None, does_not: str | None, connection: Connection, @@ -273,9 +356,11 @@ def _tool( ) -> OpenApiTool: """One tool: the declaration without the fixed parameters, and the arguments that rebuild the request from what the model writes.""" + tree = allow_tree(only) if only is not None else None arguments: list[Argument] = [] visible: list[Any] = [] applied: set[str] = set() + nested = _nested_fixes(parsed, fixes, applied) for parameter in parsed.parameters: if parameter.param_location in ("header", "cookie"): # Never model-driven; validate refused the required ones. @@ -294,6 +379,14 @@ def _tool( ) ) continue + if tree is not None and parameter.original_name not in tree: + # Not the model's to write: neither shown nor ever sent. + continue + allowed = tree.get(parameter.original_name) if tree is not None else None + if allowed is not None and parameter.param_location == "body": + _narrow_declared(parameter.param_schema, allowed) + else: + allowed = None arguments.append( Argument( name=parameter.original_name, @@ -301,6 +394,7 @@ def _tool( location=parameter.param_location, prefix=str(fix.get("prefix", "")), suffix=str(fix.get("suffix", "")), + allowed=allowed, ) ) visible.append(parameter) @@ -331,6 +425,90 @@ def _tool( path=str(parsed.endpoint.path), connection=connection, arguments=arguments, + nested_fixes=nested, declaration=declaration, require_confirmation=require_confirmation, ) + + +def _nested_fixes( + parsed: Any, fixes: Mapping[str, Mapping[str, Any]], applied: set[str] +) -> list[NestedFix]: + """The fixes that address a path into the body rather than a parameter. + + A name that matches a parameter literally is that parameter's, exactly as + validate read it; only what matches nothing literally is read as a path. + A fixed value's leaf is taken out of the declared schema - its value is + declared, so there is nothing for the model to say there. + """ + literal = { + parameter.original_name + for parameter in parsed.parameters + if parameter.param_location not in ("header", "cookie") + } + body_parameters = { + parameter.original_name: parameter + for parameter in parsed.parameters + if parameter.param_location == "body" + } + found: list[NestedFix] = [] + for name, fix in fixes.items(): + if name in literal or "." not in name: + continue + segments = tuple(name.split(".")) + parameter = body_parameters.get(segments[0]) + if parameter is None: + continue # reported with the other unapplied fixes + applied.add(name) + if "value" in fix: + _prune_declared(parameter.param_schema, segments[1:]) + found.append(NestedFix(path=segments, fixed=True, value=fix["value"])) + else: + found.append( + NestedFix( + path=segments, + prefix=str(fix.get("prefix", "")), + suffix=str(fix.get("suffix", "")), + ) + ) + return found + + +def _narrow_declared(schema: Any, tree: Mapping[str, Any]) -> None: + """Show the model just the subtree an only entry names. + + Best effort like _prune_declared: what the schema cannot show, the + request-time filter still keeps from being sent. + """ + properties = getattr(schema, "properties", None) + if not isinstance(properties, dict): + return + for key in list(properties): + if key not in tree: + del properties[key] + elif isinstance(tree[key], Mapping): + _narrow_declared(properties[key], tree[key]) + required = getattr(schema, "required", None) + if isinstance(required, list): + schema.required = [name for name in required if name in tree] or None + + +def _prune_declared(schema: Any, segments: tuple[str, ...]) -> None: + """Take one nested property out of the parsed schema. + + Best effort by design: a schema that cannot be walked cannot show the + property to the model either, and the request-time overwrite holds + regardless of what the model was shown. + """ + for segment in segments[:-1]: + properties = getattr(schema, "properties", None) + if not isinstance(properties, Mapping) or segment not in properties: + return + schema = properties[segment] + properties = getattr(schema, "properties", None) + leaf = segments[-1] + if isinstance(properties, dict): + properties.pop(leaf, None) + required = getattr(schema, "required", None) + if isinstance(required, list) and leaf in required: + schema.required = [name for name in required if name != leaf] or None diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index 36b987f..8c682a4 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -289,7 +289,7 @@ "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.", + "description": "Constraints on the operations' parameters, by operationId, then by parameter or body property name. A dotted name reaches into the JSON body when no parameter carries the name literally. What was pinned in code must not be lost by moving to a declaration.", "type": "object", "additionalProperties": { "type": "object", @@ -299,6 +299,19 @@ } } }, + "only": { + "description": "The arguments the model may write, by operationId. Everything unlisted is taken out of the declaration and never sent; a params value still rides. A dotted name exposes just that path into the JSON body. Naming what goes out fails safe when the description grows: a new field stays unexposed until someone declares it.", + "type": "object", + "additionalProperties": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + } + }, "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", diff --git a/tests/test_openapi.py b/tests/test_openapi.py index 701b868..c9db73a 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -400,6 +400,214 @@ def test_a_body_declared_only_as_alternatives_is_refused() -> None: assert "properties" in found[0] +def with_comment_body(**extra: Any) -> dict[str, Any]: + """UpdateTicket's body nested two levels deep: ticket.comment.public + decides whether the requester is mailed, and a declaration must be able + to pin it.""" + return with_update_body( + { + "type": "object", + "properties": { + "ticket": { + "type": "object", + "properties": { + "status": {"type": "string"}, + "comment": { + "type": "object", + "properties": { + "body": {"type": "string"}, + "public": {"type": "boolean"}, + }, + }, + }, + } + }, + }, + **extra, + ) + + +def test_a_dotted_fix_reaches_into_the_body() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + assert declaration_problems(sound, with_comment_body()) == [] + + +def test_a_dotted_fix_whose_leaf_is_missing_names_the_level_that_missed() -> None: + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.pubilc": {"value": False}}}, + ), + with_comment_body(), + ) + assert len(found) == 1 + assert "'pubilc' is not a property of ticket.comment" in found[0] + # The properties that do exist are named, so the fix can be corrected. + assert "body" in found[0] and "public" in found[0] + + +def test_a_dotted_fix_resolves_through_a_reference() -> None: + """Nested properties are commonly written as $ref; the walk resolves them + the way the top level was resolved.""" + spec = with_update_body( + { + "type": "object", + "properties": {"ticket": {"$ref": "#/components/schemas/Ticket"}}, + }, + Ticket={ + "type": "object", + "properties": {"comment": {"$ref": "#/components/schemas/Comment"}}, + }, + Comment={"type": "object", "properties": {"public": {"type": "boolean"}}}, + ) + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + assert declaration_problems(sound, spec) == [] + + +def test_a_dotted_fix_resolves_through_a_nested_allof() -> None: + spec = with_update_body( + { + "type": "object", + "properties": { + "ticket": { + "allOf": [ + {"$ref": "#/components/schemas/TicketCore"}, + {"properties": {"tags": {"type": "array"}}}, + ] + } + }, + }, + TicketCore={ + "type": "object", + "properties": {"comment": {"type": "object"}}, + }, + ) + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment": {"value": {"public": False}}}}, + ) + assert declaration_problems(sound, spec) == [] + + +def test_a_dotted_fix_on_an_operation_without_a_body_is_reported() -> None: + found = declaration_problems( + block( + params={"ShowTicket": {"ticket.comment": {"value": 1}}}, + operations=["ShowTicket"], + ), + SPEC, + ) + assert len(found) == 1 + assert "declares none" in found[0] + + +def test_a_parameter_whose_name_holds_a_dot_is_matched_before_any_path() -> None: + """Query parameters with dots in their names exist in the wild; a literal + match must keep meaning what it always meant.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + {"name": "page.size", "in": "query", "schema": {"type": "integer"}} + ) + sound = block(params={"ListSearchResults": {"page.size": {"value": 25}}}) + assert declaration_problems(sound, spec) == [] + + +def test_a_dotted_prefix_needs_a_string_property() -> None: + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"prefix": "x"}}}, + ), + with_comment_body(), + ) + assert len(found) == 1 + assert "string" in found[0] and "boolean" in found[0] + + +def test_a_dotted_prefix_on_a_string_property_passes() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.body": {"prefix": "[agent] "}}}, + ) + assert declaration_problems(sound, with_comment_body()) == [] + + +def test_a_dotted_fix_into_unenumerable_ground_is_not_reported() -> None: + """A freeform object may still hold the path; only a certain miss is + reported, as for flat names.""" + spec = with_update_body( + {"type": "object", "properties": {"meta": {"type": "object"}}} + ) + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"meta.note.kind": {"value": "internal"}}}, + ) + assert declaration_problems(sound, spec) == [] + + +def test_a_dotted_fix_into_a_scalar_is_reported() -> None: + """A string holds no properties, so the miss is certain.""" + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.body.tone": {"value": "x"}}}, + ), + with_comment_body(), + ) + assert len(found) == 1 + assert "holds no properties" in found[0] + + +def test_a_name_that_is_both_a_parameter_and_a_body_path_is_refused() -> None: + """Like the flat ambiguity: the fix cannot choose between the query + parameter 'ticket.comment' and the body's ticket.comment.""" + spec = with_comment_body() + spec["paths"]["/tickets/{ticket_id}"]["put"]["parameters"] = [ + {"name": "ticket.comment", "in": "query", "schema": {"type": "string"}} + ] + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment": {"value": "x"}}}, + ), + spec, + ) + assert len(found) == 1 + assert "cannot choose" in found[0] + + +def test_a_literal_match_wins_over_a_merely_possible_body_path() -> None: + """A freeform body may hold anything; a possibility must not take a + literally matching parameter away.""" + spec = with_update_body( + {"type": "object", "properties": {"meta": {"type": "object"}}} + ) + spec["paths"]["/tickets/{ticket_id}"]["put"]["parameters"] = [ + {"name": "meta.note", "in": "query", "schema": {"type": "string"}} + ] + sound = block( + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"meta.note": {"value": "x"}}}, + ) + assert declaration_problems(sound, spec) == [] + + 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)) @@ -418,6 +626,155 @@ def test_a_fix_matching_a_parameter_and_a_body_property_is_refused() -> None: assert "both" in found[0] +def test_only_must_name_a_selected_operation() -> None: + found = declaration_problems(block(only={"ShowTicket": ["ticket_id"]}), SPEC) + assert found == ["only: 'ShowTicket' is not one of this block's operations"] + + +def test_an_only_entry_that_names_no_parameter_is_reported() -> None: + found = declaration_problems( + block(only={"ListSearchResults": ["query", "sort"]}), SPEC + ) + assert len(found) == 1 + assert "sort" in found[0] + assert "query" in found[0] and "per_page" in found[0] + + +def test_an_only_entry_may_reach_into_the_body() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + ) + assert declaration_problems(sound, with_comment_body()) == [] + + +def test_only_on_one_operation_leaves_the_others_whole() -> None: + sound = block( + operations=["ListSearchResults", "ShowTicket"], + only={"ListSearchResults": ["query"]}, + ) + assert declaration_problems(sound, SPEC) == [] + + +def test_an_only_entry_naming_a_header_is_reported() -> None: + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + {"name": "X-Trace", "in": "header", "schema": {"type": "string"}} + ) + found = declaration_problems( + block(only={"ListSearchResults": ["query", "X-Trace"]}), spec + ) + assert len(found) == 1 + assert "header" in found[0] + + +def test_a_required_parameter_left_out_of_only_is_reported() -> None: + """Every request needs it; leaving it out is not narrowing but breaking.""" + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket.comment.body"]}, + ), + with_comment_body(), + ) + assert len(found) == 1 + assert "ticket_id" in found[0] + assert "required" in found[0] + + +def test_a_required_parameter_left_out_of_only_but_fixed_passes() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket.comment.body"]}, + params={"UpdateTicket": {"ticket_id": {"value": 7}}}, + ) + assert declaration_problems(sound, with_comment_body()) == [] + + +def test_a_required_body_property_left_out_of_only_is_reported() -> None: + spec = with_update_body( + { + "type": "object", + "required": ["ticket"], + "properties": { + "ticket": {"type": "object"}, + "audit": {"type": "object"}, + }, + } + ) + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "audit"]}, + ), + spec, + ) + assert len(found) == 1 + assert "'ticket'" in found[0] and "required" in found[0] + + +def test_a_name_listed_in_only_and_fixed_by_value_is_refused() -> None: + """Fixed means the declaration's, listed means the model's; it cannot be + both.""" + found = declaration_problems( + block( + only={"ListSearchResults": ["query", "per_page"]}, + params={"ListSearchResults": {"per_page": {"value": 25}}}, + ), + SPEC, + ) + assert len(found) == 1 + assert "per_page" in found[0] + + +def test_a_prefix_on_a_parameter_only_does_not_expose_is_refused() -> None: + """A prefix wraps what the model writes, and the model writes nothing + there.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + {"name": "sort", "in": "query", "schema": {"type": "string"}} + ) + found = declaration_problems( + block( + only={"ListSearchResults": ["query"]}, + params={"ListSearchResults": {"sort": {"prefix": "-"}}}, + ), + spec, + ) + assert len(found) == 1 + assert "'sort'" in found[0] and "only" in found[0] + + +def test_a_nested_fix_under_a_parent_only_does_not_send_is_refused() -> None: + """The fix rides on its parent object; a parent that is never sent would + leave the constraint silently unapplied.""" + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id"]}, + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ), + with_comment_body(), + ) + assert len(found) == 1 + assert "ticket.comment.public" in found[0] and "only" in found[0] + + +def test_a_nested_fix_under_an_exposed_parent_passes_with_only() -> None: + sound = block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + assert declaration_problems(sound, with_comment_body()) == [] + + def test_a_get_with_a_request_body_is_refused() -> None: """The client sends no body on a GET; the operation would lose arguments.""" spec = json.loads(json.dumps(SPEC)) diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index d0d12cf..3ca910e 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -10,7 +10,7 @@ from gete.connection import Registry from gete.declaration import RESOLVED_FILE, Agent, load_project, resolve -from gete.errors import DeclarationError +from gete.errors import DeclarationError, GeteError from gete.request_context import clear_tool_call from gete.runtime import build from gete.runtime.openapi import OpenApiToolset, openapi_toolset @@ -122,6 +122,7 @@ def toolset( operations: list[str] | None = None, effect: str = "read", params: dict[str, Any] | None = None, + only: dict[str, list[str]] | None = None, describe: dict[str, str] | None = None, does_not: str | None = None, confirm: bool = False, @@ -136,6 +137,8 @@ def toolset( } if params: spec["params"] = params + if only: + spec["only"] = only if describe: spec["describe"] = describe if does_not: @@ -218,6 +221,49 @@ async def test_confirmation_can_name_a_single_tool(tmp_path: Path) -> None: assert await show.check_require_confirmation({}, context()) is True +def nested_comment_spec() -> dict[str, Any]: + """UpdateTicket's body nested two levels deep: ticket.comment.public + decides whether the requester is mailed.""" + return update_body( + { + "type": "object", + "properties": { + "ticket": { + "type": "object", + "properties": { + "status": {"type": "string"}, + "comment": { + "type": "object", + "properties": { + "body": {"type": "string"}, + "public": {"type": "boolean"}, + }, + }, + }, + } + }, + } + ) + + +async def test_a_nested_fixed_value_is_not_in_the_declaration( + tmp_path: Path, +) -> None: + """Its value is declared, so there is nothing for the model to say, + however deep it sits.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + update = (await built.get_tools(context()))[0] + declared = update._get_declaration().model_dump_json(exclude_none=True) + assert "comment" in declared and "body" in declared + assert "public" not in declared + + def test_build_refuses_a_declaration_the_description_cannot_carry( tmp_path: Path, ) -> None: @@ -400,6 +446,182 @@ async def test_an_allof_body_sends_its_properties_at_the_top_level( assert client.calls[0]["body"] == {"status": "open", "comment": "hi"} +async def test_a_nested_fixed_value_rides_wherever_its_parent_is_sent( + tmp_path: Path, client: RecordingClient +) -> None: + """Even a value smuggled into the nested object is overridden by the + declared one.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + await run( + built, + "UpdateTicket", + { + "ticket_id": "7", + "ticket": {"comment": {"body": "done", "public": True}}, + }, + ) + assert client.calls[0]["body"] == { + "ticket": {"comment": {"body": "done", "public": False}} + } + + +async def test_a_nested_fixed_value_never_conjures_its_parent_up( + tmp_path: Path, client: RecordingClient +) -> None: + """No comment written means no comment sent; an empty comment carrying + only the fix would be a write the model never made.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + await run(built, "UpdateTicket", {"ticket_id": "7", "ticket": {"status": "solved"}}) + assert client.calls[0]["body"] == {"ticket": {"status": "solved"}} + + +async def test_a_nested_prefix_wraps_what_the_model_wrote( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.body": {"prefix": "[agent] "}}}, + ) + await run( + built, + "UpdateTicket", + {"ticket_id": "7", "ticket": {"comment": {"body": "done"}}}, + ) + assert client.calls[0]["body"] == {"ticket": {"comment": {"body": "[agent] done"}}} + + +async def test_a_nested_fix_on_a_parent_that_is_no_object_refuses_the_request( + tmp_path: Path, client: RecordingClient +) -> None: + """The declaration says an object sits there; sending the request around + the fix would drop the constraint on the floor.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + with pytest.raises(GeteError, match="ticket.comment"): + await run( + built, "UpdateTicket", {"ticket_id": "7", "ticket": {"comment": "done"}} + ) + assert client.calls == [] + + +async def test_only_takes_the_unlisted_out_of_declaration_and_request( + tmp_path: Path, client: RecordingClient +) -> None: + """What only leaves out is not narrowed but gone: not shown, and not + sent even when smuggled into the arguments.""" + built = toolset( + tmp_path, + only={"ListSearchResults": ["query"]}, + ) + search = next( + t for t in await built.get_tools(context()) if t.name == "ListSearchResults" + ) + declared = search._get_declaration().model_dump_json(exclude_none=True) + assert "query" in declared + assert "per_page" not in declared + await run(built, "ListSearchResults", {"query": "x", "per_page": 100}) + assert client.calls[0]["params"] == {"query": "x"} + + +async def test_a_dotted_only_entry_narrows_the_declared_body( + tmp_path: Path, client: RecordingClient +) -> None: + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + ) + update = (await built.get_tools(context()))[0] + declared = update._get_declaration().model_dump_json(exclude_none=True) + assert "comment" in declared and "body" in declared + assert "status" not in declared and "public" not in declared + + +async def test_a_dotted_only_entry_filters_what_the_model_smuggles( + tmp_path: Path, client: RecordingClient +) -> None: + """The declaration wins over what the model wrote, as it does for a + smuggled fixed parameter.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + ) + await run( + built, + "UpdateTicket", + { + "ticket_id": "7", + "ticket": { + "status": "closed", + "comment": {"body": "done", "public": True}, + }, + }, + ) + assert client.calls[0]["body"] == {"ticket": {"comment": {"body": "done"}}} + + +async def test_a_fixed_value_still_rides_when_only_leaves_it_out( + tmp_path: Path, client: RecordingClient +) -> None: + """only says what the model may write; what the declaration fixed is the + declaration's, and rides regardless.""" + built = toolset( + tmp_path, + only={"ListSearchResults": ["query"]}, + params={"ListSearchResults": {"per_page": {"value": 25}}}, + ) + await run(built, "ListSearchResults", {"query": "x"}) + assert client.calls[0]["params"] == {"query": "x", "per_page": 25} + + +async def test_only_and_a_nested_fix_compose( + tmp_path: Path, client: RecordingClient +) -> None: + """The helpdesk case in full: the model writes nothing but the comment + text, and the declaration keeps the comment internal.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + params={"UpdateTicket": {"ticket.comment.public": {"value": False}}}, + ) + await run( + built, + "UpdateTicket", + {"ticket_id": "7", "ticket": {"comment": {"body": "done", "public": True}}}, + ) + assert client.calls[0]["body"] == { + "ticket": {"comment": {"body": "done", "public": False}} + } + + async def test_a_delete_operation_uses_the_delete_verb( tmp_path: Path, client: RecordingClient ) -> None: diff --git a/tests/test_schema.py b/tests/test_schema.py index 412577e..30d453f 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -286,6 +286,10 @@ def test_an_optional_scope_needs_a_non_empty_explanation() -> None: } }, {"openapi": {**OPENAPI_TOOL["openapi"], "params": {"ListThings": {"q": {}}}}}, + # An empty only would expose nothing at all; leaving only out is how + # everything is exposed. + {"openapi": {**OPENAPI_TOOL["openapi"], "only": {"ListThings": []}}}, + {"openapi": {**OPENAPI_TOOL["openapi"], "only": {"ListThings": "query"}}}, { "openapi": { **OPENAPI_TOOL["openapi"], @@ -335,6 +339,13 @@ def test_tool_must_be_exactly_one_supported_kind(tool: dict[str, Any]) -> None: "describe": {"ListThings": "List the things."}, } }, + { + "openapi": { + **OPENAPI_TOOL["openapi"], + # A dotted name reaches into the JSON body. + "only": {"ListThings": ["query", "thing.note.kind"]}, + } + }, ], ) def test_supported_tool_shapes_pass(tool: dict[str, Any]) -> None: From 6a0ac99864432844a97a683618b71912b2501cff Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:46:35 +0900 Subject: [PATCH 2/3] Read an only entry the way params reads a name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dot in an only entry was always read as a path into the body, while params gives a literal match the first claim. The two readings drifted apart: a parameter whose name carries a dot validated cleanly when listed, and the runtime then silently took it out of the declaration - and a dotted entry keyed exposure by its first segment, so a request parameter sharing that name rode along whole, never listed, against only's promise that everything unlisted stays unsent. Now a name some parameter carries literally stays whole on both sides, and the readings that cannot be told apart are refused like the existing two-places rule: an entry naming both a request parameter and a body property, and a body path whose first segment a request parameter also claims. 🤖 Generated with Claude Code --- README.md | 6 ++-- src/gete/openapi.py | 41 ++++++++++++++++++---- src/gete/runtime/openapi.py | 19 +++++----- src/gete/schema/agent.json | 2 +- tests/test_openapi.py | 66 +++++++++++++++++++++++++++++++++++ tests/test_runtime_openapi.py | 22 ++++++++++++ 6 files changed, 138 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 88c8143..295b60f 100644 --- a/README.md +++ b/README.md @@ -199,8 +199,10 @@ tools: accept the whole record — status, assignee, tags — when an agent is only meant to add a comment. Everything `only` leaves unlisted is taken out of the declaration and never sent, even smuggled into the arguments; a - `params` value still rides. Counting up what goes out fails safe as the - description grows: a new field stays unexposed until someone declares it. + `params` value still rides. A name a parameter carries literally stays + 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. - **`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 5b68d20..4933d9a 100644 --- a/src/gete/openapi.py +++ b/src/gete/openapi.py @@ -432,7 +432,11 @@ def _fix_problems( """What keeps the declared parameter fixes from being applied.""" found: list[str] = [] parameters, header_names, body_properties, enumerable = _argument_maps(operation) - tree = allow_tree(only) if only is not None else None + tree = ( + allow_tree(only, literal=[*parameters, *body_properties]) + if only is not None + else None + ) for name, fix in fixes.items(): if name in header_names: found.append( @@ -504,8 +508,9 @@ def _only_problems( found: list[str] = [] parameters, header_names, body_properties, enumerable = _argument_maps(operation) exposed = tuple(exposed) - # A dotted entry still sends its top-level argument, just narrowed. - covered = {entry.split(".")[0] for entry in exposed} + # A dotted entry still sends its top-level argument, just narrowed. The + # tree's own keys are the names the runtime will hold parameters against. + covered = set(allow_tree(exposed, literal=[*parameters, *body_properties])) fixed = {name for name, fix in fixes.items() if "value" in fix} for name in _required_names(operation, parameters): if name not in covered and name not in fixed: @@ -526,6 +531,15 @@ def _only_problems( "which a declaration does not send" ) continue + if entry in parameters and entry in body_properties: + # The runtime exposes by name; one name in two places would + # expose both, and the declaration said neither. + found.append( + f"only.{operation.id}: {entry!r} names both a request " + f"parameter and a body property of {operation.id}; the " + "entry cannot choose between them" + ) + continue literal = entry in parameters or entry in body_properties if literal and "." in entry and _certainly_in_body(spec, operation, entry): found.append( @@ -536,9 +550,18 @@ def _only_problems( if literal: continue if "." in entry: - leaf = _walk_body(spec, operation, tuple(entry.split("."))) + segments = tuple(entry.split(".")) + leaf = _walk_body(spec, operation, segments) if leaf.problem is not None: found.append(f"only.{operation.id}: {entry!r} {leaf.problem}") + elif segments[0] in parameters: + # The runtime keys exposure by the first segment; a request + # parameter with that name would ride along whole. + found.append( + f"only.{operation.id}: {entry!r} reaches into the body " + f"through {segments[0]!r}, which also names a request " + "parameter; the entry cannot choose between them" + ) continue if enumerable: known = sorted({**body_properties, **parameters}) @@ -549,16 +572,20 @@ def _only_problems( return found -def allow_tree(entries: Iterable[str]) -> dict[str, Any]: +def allow_tree(entries: Iterable[str], literal: Iterable[str] = ()) -> dict[str, Any]: """An ``only`` list as a tree of what the model may write. A node of None means the whole subtree is the model's; a mapping narrows it to the named children. A bare name is broader than any dotted entry - under it, so it wins. + under it, so it wins. A name in ``literal`` is one some parameter + carries, dots and all, and stays whole - a dot marks a path only when + nothing claims the name literally, exactly as ``params`` reads it. """ + names = set(map(str, literal)) tree: dict[str, Any] = {} for entry in entries: - segments = str(entry).split(".") + text = str(entry) + segments = [text] if text in names else text.split(".") node = tree for segment in segments[:-1]: if segment in node and node[segment] is None: diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index 1279a8a..f50c5bf 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -356,11 +356,16 @@ def _tool( ) -> OpenApiTool: """One tool: the declaration without the fixed parameters, and the arguments that rebuild the request from what the model writes.""" - tree = allow_tree(only) if only is not None else None + literal = { + parameter.original_name + for parameter in parsed.parameters + if parameter.param_location not in ("header", "cookie") + } + tree = allow_tree(only, literal=literal) if only is not None else None arguments: list[Argument] = [] visible: list[Any] = [] applied: set[str] = set() - nested = _nested_fixes(parsed, fixes, applied) + nested = _nested_fixes(parsed, fixes, applied, literal) for parameter in parsed.parameters: if parameter.param_location in ("header", "cookie"): # Never model-driven; validate refused the required ones. @@ -432,7 +437,10 @@ def _tool( def _nested_fixes( - parsed: Any, fixes: Mapping[str, Mapping[str, Any]], applied: set[str] + parsed: Any, + fixes: Mapping[str, Mapping[str, Any]], + applied: set[str], + literal: set[str], ) -> list[NestedFix]: """The fixes that address a path into the body rather than a parameter. @@ -441,11 +449,6 @@ def _nested_fixes( A fixed value's leaf is taken out of the declared schema - its value is declared, so there is nothing for the model to say there. """ - literal = { - parameter.original_name - for parameter in parsed.parameters - if parameter.param_location not in ("header", "cookie") - } body_parameters = { parameter.original_name: parameter for parameter in parsed.parameters diff --git a/src/gete/schema/agent.json b/src/gete/schema/agent.json index 8c682a4..5b900a7 100644 --- a/src/gete/schema/agent.json +++ b/src/gete/schema/agent.json @@ -300,7 +300,7 @@ } }, "only": { - "description": "The arguments the model may write, by operationId. Everything unlisted is taken out of the declaration and never sent; a params value still rides. A dotted name exposes just that path into the JSON body. Naming what goes out fails safe when the description grows: a new field stays unexposed until someone declares it.", + "description": "The arguments the model may write, by operationId. Everything unlisted is taken out of the declaration and never sent; a params value still rides. A dotted name exposes just that path into the JSON body when no parameter carries the name literally. Naming what goes out fails safe when the description grows: a new field stays unexposed until someone declares it.", "type": "object", "additionalProperties": { "type": "array", diff --git a/tests/test_openapi.py b/tests/test_openapi.py index c9db73a..17ce9d0 100644 --- a/tests/test_openapi.py +++ b/tests/test_openapi.py @@ -775,6 +775,72 @@ def test_a_nested_fix_under_an_exposed_parent_passes_with_only() -> None: assert declaration_problems(sound, with_comment_body()) == [] +def test_an_only_entry_naming_a_dotted_parameter_is_matched_literally() -> None: + """A dot in a listed name is only a path when nothing carries the name + literally, exactly as params reads it - required or not.""" + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + { + "name": "page.size", + "in": "query", + "required": True, + "schema": {"type": "integer"}, + } + ) + sound = block(only={"ListSearchResults": ["query", "page.size"]}) + assert declaration_problems(sound, spec) == [] + + +def test_a_prefix_on_a_dotted_parameter_listed_in_only_passes() -> None: + spec = json.loads(json.dumps(SPEC)) + spec["paths"]["/search"]["get"]["parameters"].append( + {"name": "page.size", "in": "query", "schema": {"type": "string"}} + ) + sound = block( + only={"ListSearchResults": ["query", "page.size"]}, + params={"ListSearchResults": {"page.size": {"prefix": "p"}}}, + ) + assert declaration_problems(sound, spec) == [] + + +def test_an_only_entry_matching_a_parameter_and_a_body_property_is_refused() -> None: + """The runtime exposes by name; one name in two places would expose + both, and the declaration said neither.""" + 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", + only={"UpdateTicket": ["ticket_id", "ticket"]}, + ), + spec, + ) + assert len(found) == 1 + assert "both" in found[0] + + +def test_an_only_path_through_a_name_a_parameter_carries_is_refused() -> None: + """The runtime keys exposure by the path's first segment; a query + parameter with that name would ride along whole, never listed.""" + spec = with_comment_body() + spec["paths"]["/tickets/{ticket_id}"]["put"]["parameters"] = [ + {"name": "ticket", "in": "query", "schema": {"type": "string"}} + ] + found = declaration_problems( + block( + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + ), + spec, + ) + assert len(found) == 1 + assert "'ticket'" in found[0] and "cannot choose" in found[0] + + def test_a_get_with_a_request_body_is_refused() -> None: """The client sends no body on a GET; the operation would lose arguments.""" spec = json.loads(json.dumps(SPEC)) diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index 3ca910e..de823a4 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -622,6 +622,28 @@ async def test_only_and_a_nested_fix_compose( } +async def test_a_dotted_parameter_listed_in_only_stays_offered( + tmp_path: Path, client: RecordingClient +) -> None: + """A dot in a listed name is only a path when nothing carries the name + literally, exactly as params reads it.""" + document = copy.deepcopy(SPEC) + document["paths"]["/search"]["get"]["parameters"].append( + {"name": "page.size", "in": "query", "schema": {"type": "integer"}} + ) + built = toolset( + tmp_path, + document=document, + operations=["ListSearchResults"], + only={"ListSearchResults": ["query", "page.size"]}, + ) + search = (await built.get_tools(context()))[0] + offered = {a.name: a.py_name for a in search._arguments if not a.fixed} + assert "page.size" in offered + await run(built, "ListSearchResults", {"query": "x", offered["page.size"]: 5}) + assert client.calls[0]["params"] == {"query": "x", "page.size": 5} + + async def test_a_delete_operation_uses_the_delete_verb( tmp_path: Path, client: RecordingClient ) -> None: From 5a55fd9bf5810a87b7e87827fe38489c36f5cc4e Mon Sep 17 00:00:00 2001 From: haruotsu <65439874+haruotsu@users.noreply.github.com> Date: Thu, 27 Aug 2026 23:47:09 +0900 Subject: [PATCH 3/3] Drop a parent the only filter empties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When everything the model wrote inside an object was unlisted, the filter kept the emptied shell and the request carried it - a write the model never made, and one the nested-fix path already refuses to conjure. Nothing surviving the filter now means nothing is sent, the same reading "no comment written means no comment sent" gives the fixes. 🤖 Generated with Claude Code --- src/gete/runtime/openapi.py | 6 ++++-- tests/test_runtime_openapi.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/gete/runtime/openapi.py b/src/gete/runtime/openapi.py index f50c5bf..7fa326f 100644 --- a/src/gete/runtime/openapi.py +++ b/src/gete/runtime/openapi.py @@ -81,7 +81,9 @@ def _filtered(value: Any, tree: Mapping[str, Any] | None) -> Any: A value where the tree expects an object cannot be carved, so nothing of it passes - the declaration wins over what the model wrote, as it does - for a smuggled fixed parameter. + for a smuggled fixed parameter. An object the filter empties is dropped + whole: an emptied parent riding along would be a write the model never + made. """ if tree is None: return value @@ -93,7 +95,7 @@ def _filtered(value: Any, tree: Mapping[str, Any] | None) -> Any: child = _filtered(item, tree[key]) if child is not None: kept[key] = child - return kept + return kept or None def _apply_nested(body: dict[str, Any], fix: NestedFix, tool: str) -> None: diff --git a/tests/test_runtime_openapi.py b/tests/test_runtime_openapi.py index de823a4..921a55f 100644 --- a/tests/test_runtime_openapi.py +++ b/tests/test_runtime_openapi.py @@ -644,6 +644,22 @@ async def test_a_dotted_parameter_listed_in_only_stays_offered( assert client.calls[0]["params"] == {"query": "x", "page.size": 5} +async def test_a_parent_the_filter_empties_is_not_sent( + tmp_path: Path, client: RecordingClient +) -> None: + """Nothing of the write survived the filter; an emptied ticket riding + along would be a write the model never made.""" + built = toolset( + tmp_path, + document=nested_comment_spec(), + operations=["UpdateTicket"], + effect="write", + only={"UpdateTicket": ["ticket_id", "ticket.comment.body"]}, + ) + await run(built, "UpdateTicket", {"ticket_id": "7", "ticket": {"status": "x"}}) + assert client.calls[0]["body"] is None + + async def test_a_delete_operation_uses_the_delete_verb( tmp_path: Path, client: RecordingClient ) -> None: