Merge branch 'main' into codex/repro-adk-run-cancel-cleanup

Author: rohityan

src/google/adk/artifacts/artifact_util.py

@@ -134,3 +134,32 @@ def validate_artifact_reference_scope(
         "Session-scoped artifact references must stay within the same"
         " session scope."
     )
+
+
+def validate_path_segment(value: str, field_name: str) -> None:
+  """Rejects values that could alter the constructed path.
+
+  Args:
+    value: The caller-supplied identifier (e.g. user_id or session_id).
+    field_name: Human-readable name used in the error message.
+
+  Raises:
+    InputValidationError: If the value contains path separators, traversal
+      segments, or null bytes.
+  """
+  if not value:
+    raise input_validation_error.InputValidationError(
+        f"{field_name} must not be empty."
+    )
+  if "\x00" in value:
+    raise input_validation_error.InputValidationError(
+        f"{field_name} must not contain null bytes."
+    )
+  if "/" in value or "\\" in value:
+    raise input_validation_error.InputValidationError(
+        f"{field_name} {value!r} must not contain path separators."
+    )
+  if value in (".", "..") or ".." in value.split("/"):
+    raise input_validation_error.InputValidationError(
+        f"{field_name} {value!r} must not contain traversal segments."
+    )

src/google/adk/artifacts/file_artifact_service.py

@@ -34,6 +34,7 @@
 from pydantic import ValidationError
 from typing_extensions import override
 
+from . import artifact_util
 from ..errors.input_validation_error import InputValidationError
 from .base_artifact_service import ArtifactVersion
 from .base_artifact_service import BaseArtifactService
@@ -142,39 +143,14 @@ def _is_user_scoped(session_id: Optional[str], filename: str) -> bool:
   return session_id is None or _file_has_user_namespace(filename)
 
 
-def _validate_path_segment(value: str, field_name: str) -> None:
-  """Rejects values that could alter the constructed filesystem path.
-
-  Args:
-    value: The caller-supplied identifier (e.g. user_id or session_id).
-    field_name: Human-readable name used in the error message.
-
-  Raises:
-    InputValidationError: If the value contains path separators, traversal
-      segments, or null bytes.
-  """
-  if not value:
-    raise InputValidationError(f"{field_name} must not be empty.")
-  if "\x00" in value:
-    raise InputValidationError(f"{field_name} must not contain null bytes.")
-  if "/" in value or "\\" in value:
-    raise InputValidationError(
-        f"{field_name} {value!r} must not contain path separators."
-    )
-  if value in (".", "..") or ".." in value.split("/"):
-    raise InputValidationError(
-        f"{field_name} {value!r} must not contain traversal segments."
-    )
-
-
 def _user_artifacts_dir(base_root: Path) -> Path:
   """Returns the path that stores user-scoped artifacts."""
   return base_root / "artifacts"
 
 
 def _session_artifacts_dir(base_root: Path, session_id: str) -> Path:
   """Returns the path that stores session-scoped artifacts."""
-  _validate_path_segment(session_id, "session_id")
+  artifact_util.validate_path_segment(session_id, "session_id")
   return base_root / "sessions" / session_id / "artifacts"
 
 
@@ -256,7 +232,7 @@ def __init__(self, root_dir: Path | str):
 
   def _base_root(self, user_id: str, /) -> Path:
     """Returns the artifacts root directory for a user."""
-    _validate_path_segment(user_id, "user_id")
+    artifact_util.validate_path_segment(user_id, "user_id")
     return self.root_dir / "users" / user_id
 
   def _scope_root(
@@ -269,7 +245,7 @@ def _scope_root(
     base = self._base_root(user_id)
     if _is_user_scoped(session_id, filename):
       return _user_artifacts_dir(base)
-    if not session_id:
+    if session_id is None:
       raise InputValidationError(
           "Session ID must be provided for session-scoped artifacts."
       )
@@ -543,7 +519,7 @@ def _list_artifact_keys_sync(
 
     base_root = self._base_root(user_id)
 
-    if session_id:
+    if session_id is not None:
       session_root = _session_artifacts_dir(base_root, session_id)
       for artifact_dir in _iter_artifact_dirs(session_root):
         metadata = self._latest_metadata(artifact_dir)

src/google/adk/artifacts/gcs_artifact_service.py

@@ -167,13 +167,16 @@ def _get_blob_prefix(
       session_id: Optional[str] = None,
   ) -> str:
     """Constructs the blob name prefix in GCS for a given artifact."""
+    artifact_util.validate_path_segment(app_name, "app_name")
+    artifact_util.validate_path_segment(user_id, "user_id")
     if self._file_has_user_namespace(filename):
       return f"{app_name}/{user_id}/user/{filename}"
 
     if session_id is None:
       raise InputValidationError(
           "Session ID must be provided for session-scoped artifacts."
       )
+    artifact_util.validate_path_segment(session_id, "session_id")
     return f"{app_name}/{user_id}/{session_id}/{filename}"
 
   def _get_blob_name(
@@ -368,6 +371,10 @@ def _load_artifact(
   def _list_artifact_keys(
       self, app_name: str, user_id: str, session_id: Optional[str]
   ) -> list[str]:
+    artifact_util.validate_path_segment(app_name, "app_name")
+    artifact_util.validate_path_segment(user_id, "user_id")
+    if session_id is not None:
+      artifact_util.validate_path_segment(session_id, "session_id")
     filenames = set()
 
     if session_id:

src/google/adk/artifacts/in_memory_artifact_service.py

@@ -85,13 +85,16 @@ def _artifact_path(
     Returns:
         The constructed artifact path.
     """
+    artifact_util.validate_path_segment(app_name, "app_name")
+    artifact_util.validate_path_segment(user_id, "user_id")
     if self._file_has_user_namespace(filename):
       return f"{app_name}/{user_id}/user/{filename}"
 
     if session_id is None:
       raise InputValidationError(
           "Session ID must be provided for session-scoped artifacts."
       )
+    artifact_util.validate_path_segment(session_id, "session_id")
     return f"{app_name}/{user_id}/{session_id}/{filename}"
 
   @override
@@ -215,6 +218,10 @@ async def load_artifact(
   async def list_artifact_keys(
       self, *, app_name: str, user_id: str, session_id: Optional[str] = None
   ) -> list[str]:
+    artifact_util.validate_path_segment(app_name, "app_name")
+    artifact_util.validate_path_segment(user_id, "user_id")
+    if session_id is not None:
+      artifact_util.validate_path_segment(session_id, "session_id")
     usernamespace_prefix = f"{app_name}/{user_id}/user/"
     session_prefix = (
         f"{app_name}/{user_id}/{session_id}/" if session_id else None

src/google/adk/plugins/bigquery_agent_analytics_plugin.py

@@ -306,6 +306,84 @@ def _get_tool_origin(
   return "UNKNOWN"
 
 
+def _extract_tool_declarations(
+    tools_dict: dict[str, "BaseTool"],
+) -> list[dict[str, Any]]:
+  """Extracts structured tool metadata for the ``LLM_REQUEST`` event.
+
+  Earlier versions logged only the tool names (``list(tools_dict.keys())``).
+  Downstream consumers such as online evaluation need the tool *description* and
+  *parameter schema* to judge whether the model selected and invoked the right
+  tool, so this returns one structured entry per tool instead of a bare name.
+
+  Each entry always carries ``name`` and, when available, ``description`` and
+  ``parameters`` (the OpenAPI parameter schema from the tool's
+  ``FunctionDeclaration``). Extraction is best-effort and per-tool: a tool whose
+  declaration cannot be resolved still contributes its name and description, so
+  one misbehaving tool never drops the whole ``tools`` attribute.
+
+  Args:
+      tools_dict: Mapping of tool name to ``BaseTool`` from ``LlmRequest``.
+
+  Returns:
+      A list of ``{"name", "description"?, "parameters"?}`` dicts.
+  """
+  tools: list[dict[str, Any]] = []
+  for name, tool in tools_dict.items():
+    # Fall back to the dict key when the tool has no (or a falsy) name.
+    entry: dict[str, Any] = {"name": getattr(tool, "name", None) or name}
+    description = getattr(tool, "description", None)
+    if description:
+      entry["description"] = description
+
+    # The parameter schema lives on the tool's FunctionDeclaration, which some
+    # tools (e.g. built-in tools) do not provide. Resolve defensively so a
+    # single failing tool does not discard the whole tools list.
+    #
+    # Note: FunctionTool._get_declaration() rebuilds the declaration from the
+    # function signature on each call (no caching), so this repeats work the
+    # framework already did when assembling the request. Acceptable for typical
+    # toolsets; revisit with a cache if it shows up on the hot path.
+    declaration = None
+    try:
+      get_declaration = getattr(tool, "_get_declaration", None)
+      if callable(get_declaration):
+        declaration = get_declaration()
+    except Exception:  # pylint: disable=broad-except
+      logger.debug("Failed to get declaration for tool %s", name, exc_info=True)
+
+    if declaration is not None:
+      if "description" not in entry:
+        decl_description = getattr(declaration, "description", None)
+        if decl_description:
+          entry["description"] = decl_description
+      # A declaration carries its parameter schema in one of two shapes: the
+      # structured `parameters` Schema, or a raw JSON-schema dict in
+      # `parameters_json_schema`. Several tools (MCP, OpenAPI, skill, node, and
+      # environment tools) populate only the latter, and model adapters prefer
+      # it, so prefer it here too and fall back to `parameters` otherwise.
+      json_schema = getattr(declaration, "parameters_json_schema", None)
+      if json_schema is not None:
+        entry["parameters"] = json_schema
+      else:
+        parameters = getattr(declaration, "parameters", None)
+        if parameters is not None:
+          try:
+            entry["parameters"] = parameters.model_dump(
+                exclude_none=True, mode="json"
+            )
+          except Exception:  # pylint: disable=broad-except
+            # Leave parameters off if the schema is not JSON-serializable.
+            logger.debug(
+                "Failed to serialize parameters for tool %s",
+                name,
+                exc_info=True,
+            )
+
+    tools.append(entry)
+  return tools
+
+
 _SENSITIVE_KEYS = frozenset({
     "client_secret",
     "access_token",
@@ -4028,7 +4106,8 @@ async def before_model_callback(
     """
 
     # 5. Attributes (Config & Tools)
-    attributes = {}
+    attributes: dict[str, Any] = {}
+    tools_truncated = False
     if llm_request.config:
       config_dict = {}
       for field_name in [
@@ -4057,13 +4136,21 @@ async def before_model_callback(
         attributes["labels"] = labels
 
     if hasattr(llm_request, "tools_dict") and llm_request.tools_dict:
-      attributes["tools"] = list(llm_request.tools_dict.keys())
+      # Route tool declarations through the shared safety pipeline so unbounded
+      # descriptions / parameter schemas are size-capped and sensitive keys are
+      # redacted, consistent with every other captured attribute.
+      tools, tools_truncated = _recursive_smart_truncate(
+          _extract_tool_declarations(llm_request.tools_dict),
+          self.config.max_content_length,
+      )
+      attributes["tools"] = tools
 
     TraceManager.push_span(callback_context, "llm_request")
     await self._log_event(
         "LLM_REQUEST",
         callback_context,
         raw_content=llm_request,
+        is_truncated=tools_truncated,
         event_data=EventData(
             model=llm_request.model,
             extra_attributes=attributes,

src/google/adk/telemetry/_metrics.py

@@ -99,6 +99,7 @@
     unit="1",
     description="Number of inference (model) calls per agent invocation.",
     explicit_bucket_boundaries_advisory=[
+        0,
         1,
         2,
         3,
@@ -118,6 +119,7 @@
     unit="1",
     description="Number of tool calls per agent invocation.",
     explicit_bucket_boundaries_advisory=[
+        0,
         1,
         2,
         3,