fix(openapi): let operation parameters override path-level ones

src/google/adk/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.py

@@ -20,6 +20,7 @@
 from typing import List
 from typing import Optional
 from typing import Set
+from typing import Tuple
 
 from fastapi.openapi.models import Operation
 from pydantic import BaseModel
@@ -171,6 +172,47 @@ def sanitize_recursive(obj: Any, *, in_schema: bool) -> Any:
 
     return sanitize_recursive(openapi_spec, in_schema=False)
 
+  def _merge_parameters(
+      self,
+      operation_parameters: List[Dict[str, Any]],
+      path_parameters: List[Dict[str, Any]],
+  ) -> List[Dict[str, Any]]:
+    """Merges an operation's parameters over the path-level ones.
+
+    Per the OpenAPI 3 Path Item Object, an operation-level parameter
+    overrides a path-level parameter that has the same `name` and `in`.
+    Declaring a shared parameter once at the path level and refining it on
+    an operation (a more specific description, pattern or enum) is common,
+    so both must not be collected: the duplicate would otherwise be renamed
+    to `<name>_0` and asked of the model as a second required argument.
+
+    Args:
+      operation_parameters: The parameters declared on the operation.
+      path_parameters: The parameters declared on the path item.
+
+    Returns:
+      The operation's parameters followed by the path-level parameters that
+      the operation does not override.
+    """
+    merged: List[Dict[str, Any]] = list(operation_parameters)
+
+    overridden: Set[Tuple[Any, Any]] = set()
+    for parameter in merged:
+      key = (parameter.get("name"), parameter.get("in"))
+      # Skip malformed parameters so they are never deduplicated away.
+      if key[0] is not None and key[1] is not None:
+        overridden.add(key)
+
+    for parameter in path_parameters:
+      key = (parameter.get("name"), parameter.get("in"))
+      if key in overridden:
+        continue
+      if key[0] is not None and key[1] is not None:
+        overridden.add(key)
+      merged.append(parameter)
+
+    return merged
+
   def _collect_operations(
       self, openapi_spec: Dict[str, Any]
   ) -> List[ParsedOperation]:
@@ -205,10 +247,12 @@ def _collect_operations(
         if operation_dict is None:
           continue
 
-        # Append path-level parameters
-        operation_dict["parameters"] = operation_dict.get(
-            "parameters", []
-        ) + path_item.get("parameters", [])
+        # Merge path-level parameters in, letting the operation's own
+        # parameters override the ones it re-declares.
+        operation_dict["parameters"] = self._merge_parameters(
+            operation_dict.get("parameters", []),
+            path_item.get("parameters", []),
+        )
 
         # If operation ID is missing, assign an operation id based on path
         # and method

tests/unittests/tools/openapi_tool/openapi_spec_parser/test_openapi_spec_parser.py

@@ -866,3 +866,91 @@ def test_sanitize_schema_types_removes_all_invalid_list(openapi_spec_generator):
 
   # Type field should be removed entirely
   assert "type" not in sanitized["schema"]
+
+
+def create_spec_with_path_level_parameters() -> Dict[str, Any]:
+  """Creates a spec whose path item and operation both declare accountId."""
+  return {
+      "openapi": "3.0.0",
+      "info": {"title": "CRM API", "version": "1.0.0"},
+      "servers": [{"url": "https://crm.example.com"}],
+      "paths": {
+          "/accounts/{accountId}": {
+              "parameters": [
+                  {
+                      "name": "accountId",
+                      "in": "path",
+                      "required": True,
+                      "description": "Account id (path level).",
+                      "schema": {"type": "string"},
+                  },
+                  {
+                      "name": "trace",
+                      "in": "query",
+                      "description": "Trace flag (path level).",
+                      "schema": {"type": "boolean"},
+                  },
+              ],
+              "get": {
+                  "operationId": "getAccount",
+                  "parameters": [
+                      {
+                          "name": "accountId",
+                          "in": "path",
+                          "required": True,
+                          "description": "Account id (operation level).",
+                          "schema": {"type": "string", "pattern": "^ACC-"},
+                      },
+                  ],
+                  "responses": {"200": {"description": "Successful response"}},
+              },
+          }
+      },
+  }
+
+
+def test_operation_parameter_overrides_path_level_parameter(
+    openapi_spec_generator,
+):
+  """Test that an operation parameter overrides the path-level one."""
+  spec = create_spec_with_path_level_parameters()
+
+  op = openapi_spec_generator.parse(spec)[0]
+
+  # The path-level declaration must not survive as a second argument.
+  assert [param.py_name for param in op.parameters] == ["account_id", "trace"]
+  account_id = op.parameters[0]
+  assert account_id.original_name == "accountId"
+  assert account_id.description == "Account id (operation level)."
+
+
+def test_path_level_parameters_are_still_collected(openapi_spec_generator):
+  """Test that path-level parameters the operation does not redeclare remain."""
+  spec = create_spec_with_path_level_parameters()
+
+  op = openapi_spec_generator.parse(spec)[0]
+
+  trace = op.parameters[1]
+  assert trace.original_name == "trace"
+  assert trace.param_location == "query"
+  assert trace.description == "Trace flag (path level)."
+
+
+def test_parameters_with_same_name_different_location_are_both_kept(
+    openapi_spec_generator,
+):
+  """Test that `in` participates in the override key."""
+  spec = create_spec_with_path_level_parameters()
+  spec["paths"]["/accounts/{accountId}"]["get"]["parameters"].append({
+      "name": "accountId",
+      "in": "query",
+      "description": "Account id as a query parameter.",
+      "schema": {"type": "string"},
+  })
+
+  op = openapi_spec_generator.parse(spec)[0]
+
+  # The path-level `accountId` is overridden, the query one is not.
+  names = [param.py_name for param in op.parameters]
+  assert names == ["account_id", "account_id_0", "trace"]
+  assert op.parameters[1].param_location == "query"